Fix: Handle Self replacement contextually in inline assists

[phyBrackets]

Only replace Self when inlining across different impl contexts, and use
bare type names for expression contexts to avoid turbofish issues.

Fixes #19827

[Veykril]

So, we should make PathTransform do this transformation (it might already have parts of it implemented), and then use that in the other places in this assist (so far its only used for generic argument rewriting). That way we don't special case things to this assist when its actually a more general applicable transformation

[phyBrackets]

Thanks @Veykril for the feedback, I think it does make sense that we should leverage and extend the existing PathTransform mechanism that's already used for generic argument rewriting. I have updated the patch based on your review.

[Veykril]

This can be simplified (and also made more robust by that)

Suggested change

	fn is_path_in_expression_context(path: &ast::Path) -> bool {
	let mut current = path.syntax().parent();
	while let Some(node) = current {
	// Expression contexts where we need bare type names
	if let Some(call_expr) = ast::CallExpr::cast(node.clone()) {
	if let Some(expr) = call_expr.expr() {
	if expr.syntax().text_range().contains_range(path.syntax().text_range()) {
	return true;
	}
	}
	}
	if ast::PathExpr::cast(node.clone()).is_some() {
	return true;
	}
	if let Some(record_expr) = ast::RecordExpr::cast(node.clone()) {
	if let Some(record_path) = record_expr.path() {
	if record_path.syntax().text_range().contains_range(path.syntax().text_range()) {
	return true;
	}
	}
	}
	// Stop at type/pattern boundaries
	if ast::Type::cast(node.clone()).is_some()
	|| ast::Pat::cast(node.clone()).is_some()
	|| ast::RetType::cast(node.clone()).is_some()
	{
	return false;
	}
	current = node.parent();
	}
	false
	}
	fn is_path_in_expression_context(path: &ast::Path) -> bool {
	let path = path.top_path();
	let Some(parent) = path.syntax().parent() else { return false; };
	parent.kind() == SyntaxKind::PATH_EXPR
	}

I don't think we need to consider record expression constructors, as their path is actually in type namespace if I am not mistaken.

[phyBrackets]

Okay, my original concern was about struct constructor calls like Foo { field: value } where we might want Foo instead of Self::Foo, but thinking about it more, record expressions are indeed type namespace references and even if we wanted bare names there, PathTransform should handle the namespace correctly, so this complexity isn't justified indeed, thanks.

[Veykril]

We can always add back heuristics later that can scan for cases where we can simplify the path, but for now that's not really necessary I think

[Veykril]

I think the more correct approach would be to replace the Self with the fully qualified path in turbofished form instead of dropping the generics entirely.

[rustbot]

☔ The latest upstream changes (possibly #21804) made this pull request unmergeable. Please resolve the merge conflicts.

[ChayimFriedman2]

OP didn't respond, closing. Feel free to reopen/open a new PR if you want to resume work on this.