Skip to content

Fix: Handle Self replacement contextually in inline assists #20049

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
184 changes: 175 additions & 9 deletions crates/ide-assists/src/handlers/inline_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use ide_db::{
};
use itertools::{Itertools, izip};
use syntax::{
AstNode, NodeOrToken, SyntaxKind,
AstNode, NodeOrToken, SyntaxKind, SyntaxToken,
ast::{
self, HasArgList, HasGenericArgs, Pat, PathExpr, edit::IndentLevel, edit_in_place::Indent,
},
Expand Down Expand Up @@ -311,6 +311,80 @@ fn get_fn_params(
Some(params)
}

fn is_self_in_expression_context(self_token: &SyntaxToken) -> bool {
let mut current = self_token.parent();
while let Some(node) = current {
// Check for function call expressions: Self::method() or Self(...)
if let Some(call_expr) = ast::CallExpr::cast(node.clone()) {
if let Some(expr) = call_expr.expr() {
if expr.syntax().text_range().contains_range(self_token.text_range()) {
return true;
}
}
}

if let Some(method_call) = ast::MethodCallExpr::cast(node.clone()) {
if let Some(receiver) = method_call.receiver() {
if receiver.syntax().text_range().contains_range(self_token.text_range()) {
return true;
}
}
}

if let Some(_path_expr) = ast::PathExpr::cast(node.clone()) {
return true;
}

// Check for record expressions (struct construction)
if let Some(record_expr) = ast::RecordExpr::cast(node.clone()) {
if let Some(path) = record_expr.path() {
if path.syntax().text_range().contains_range(self_token.text_range()) {
return true;
}
}
}

// Stop at certain boundaries (type/pattern contexts)
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 get_qualified_type_for_turbofish(ty: &ast::Type) -> String {
match ty {
ast::Type::PathType(path_type) => {
if let Some(path) = path_type.path() {
// For turbofish, we need the full path but potentially without generic args
// depending on context. For now, use the bare name.
if let Some(segment) = path.segments().last() {
if let Some(name) = segment.name_ref() {
return name.text().to_string();
}
}
}
}
_ => {}
}
ty.syntax().text().to_string()
}

fn have_same_self_type(source_impl: &ast::Impl, target_impl: &ast::Impl) -> bool {
match (source_impl.self_ty(), target_impl.self_ty()) {
(Some(source_ty), Some(target_ty)) => {
// Compare the textual representation of the types
source_ty.syntax().text() == target_ty.syntax().text()
}
_ => false,
}
}

fn inline(
sema: &Semantics<'_, RootDatabase>,
function_def_file_id: EditionedFileId,
Expand Down Expand Up @@ -391,22 +465,46 @@ fn inline(
// We should place the following code after last usage of `usages_for_locals`
// because `ted::replace` will change the offset in syntax tree, which makes
// `FileReference` incorrect
if let Some(imp) =
if let Some(source_impl) =
sema.ancestors_with_macros(fn_body.syntax().clone()).find_map(ast::Impl::cast)
{
if !node.syntax().ancestors().any(|anc| &anc == imp.syntax()) {
if let Some(t) = imp.self_ty() {
while let Some(self_tok) = body
// Check if the target (call site) is also in an impl block
let target_impl = node.syntax().ancestors().find_map(ast::Impl::cast);

let should_replace_self = match target_impl {
Some(target_impl) => {
// Both source and target are in impl blocks
// Only replace Self if they have different Self types
!have_same_self_type(&source_impl, &target_impl)
}
None => {
// Target is not in an impl block, so we must replace Self
true
}
};

if should_replace_self {
if let Some(self_ty) = source_impl.self_ty() {
let self_tokens: Vec<_> = body
.syntax()
.descendants_with_tokens()
.filter_map(NodeOrToken::into_token)
.find(|tok| tok.kind() == SyntaxKind::SELF_TYPE_KW)
{
let replace_with = t.clone_subtree().syntax().clone_for_update();
ted::replace(self_tok, replace_with);
.filter(|tok| tok.kind() == SyntaxKind::SELF_TYPE_KW)
.collect();

// Replace each Self token based on its context
for self_tok in self_tokens {
let replacement = if is_self_in_expression_context(&self_tok) {
let qualified_name = get_qualified_type_for_turbofish(&self_ty);
make::name_ref(&qualified_name).syntax().clone_for_update()
} else {
self_ty.clone_subtree().syntax().clone_for_update()
};
ted::replace(self_tok, replacement);
}
}
}
// If same Self type context, leave Self as-is (it remains valid)
}

let mut func_let_vars: BTreeSet<String> = BTreeSet::new();
Expand Down Expand Up @@ -1832,4 +1930,72 @@ fn f() {
"#,
);
}

#[test]
fn inline_call_generic_self_constructor() {
check_assist(
inline_call,
r#"
struct Generic<T>(T);

impl<T> Generic<T> {
fn new(value: T) -> Self {
Self(value)
}
}

fn main() {
let x = Generic::<i32>::new$0(42);
}
"#,
r#"
struct Generic<T>(T);

impl<T> Generic<T> {
fn new(value: T) -> Self {
Self(value)
}
}

fn main() {
let x = Generic(42);
}
"#,
)
}

#[test]
fn inline_call_generic_self_type_position() {
check_assist(
inline_call,
r#"
struct Generic<T>(T);

impl<T> Generic<T> {
fn identity(self) -> Self {
self
}
}

fn main() {
let x = Generic(42);
let y = x.identity$0();
}
"#,
r#"
struct Generic<T>(T);

impl<T> Generic<T> {
fn identity(self) -> Self {
self
}
}

fn main() {
let x = Generic(42);
let y = x;
}
"#,
)
}
}