Skip to content

fix: Renaming both struct and function with the same name #19306

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

fix: Renaming both struct and function with the same name #19306

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2eb6e74
fix: Renaming both struct and function with the same name
asuto15 Mar 6, 2025
fe8645b
refactor: Add size argument for test consistency
asuto15 Mar 6, 2025
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
146 changes: 126 additions & 20 deletions crates/ide/src/rename.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use ide_db::{
FileId, FileRange, RootDatabase,
};
use itertools::Itertools;
use stdx::{always, never};
use stdx::never;
use syntax::{ast, AstNode, SyntaxKind, SyntaxNode, TextRange, TextSize};

use ide_db::text_edit::TextEdit;
Expand All @@ -33,19 +33,22 @@ pub(crate) fn prepare_rename(
let source_file = sema.parse_guess_edition(position.file_id);
let syntax = source_file.syntax();

let res = find_definitions(&sema, syntax, position)?
.map(|(frange, kind, def)| {
// ensure all ranges are valid
let primary_defs: Vec<_> = find_definitions(&sema, syntax, position)?
.filter(|(frange, _, _)| {
frange.range.contains_inclusive(position.offset) && frange.file_id == position.file_id
})
.collect();

if primary_defs.is_empty() {
bail!("No references found at position");
}

let merged_range_result = primary_defs
.into_iter()
.map(|(frange, kind, def)| {
if def.range_for_rename(&sema).is_none() {
bail!("No references found at position")
}

always!(
frange.range.contains_inclusive(position.offset)
&& frange.file_id == position.file_id
);

Ok(match kind {
SyntaxKind::LIFETIME => {
TextRange::new(frange.range.start() + TextSize::from(1), frange.range.end())
Expand All @@ -54,17 +57,15 @@ pub(crate) fn prepare_rename(
})
})
.reduce(|acc, cur| match (acc, cur) {
// ensure all ranges are the same
(Ok(acc_inner), Ok(cur_inner)) if acc_inner == cur_inner => Ok(acc_inner),
(e @ Err(_), _) | (_, e @ Err(_)) => e,
_ => bail!("inconsistent text range"),
});

match res {
// ensure at least one definition was found
Some(res) => res.map(|range| RangeInfo::new(range, ())),
None => bail!("No references found at position"),
}
let merged_range =
merged_range_result.ok_or_else(|| format_err!("No references found at position"))??;

Ok(RangeInfo::new(merged_range, ()))
}

// Feature: Rename
Expand Down Expand Up @@ -294,12 +295,74 @@ fn find_definitions(
// FIXME: some semantic duplication between "empty vec" and "Err()"
Err(format_err!("No references found at position"))
} else {
// remove duplicates, comparing `Definition`s
Ok(v.into_iter()
let mut defs = v
.into_iter()
.unique_by(|&(.., def)| def)
.map(|(a, b, c)| (a.into(), b, c))
.collect::<Vec<_>>()
.into_iter())
.collect::<Vec<_>>();

// Determine whether related definitions with the same name
// (e.g., a function and a struct) should also be included.
let mut additional_defs = Vec::new();

for (_, _, def) in &defs {
if let Some(name) = def.name(sema.db) {
let name_str = name.as_str().to_owned();

// For structs, look for functions with the same name
if let Definition::Adt(adt) = def {
let module = adt.module(sema.db);
for item in module.declarations(sema.db) {
if let hir::ModuleDef::Function(func) = item {
let func_name = func.name(sema.db);
if func_name.as_str() == name_str {
let func_def = Definition::Function(func);
if !defs.iter().any(|(.., d)| d == &func_def) {
if let Some(range) = func_def.range_for_rename(sema) {
if range.file_id == file_id {
additional_defs.push((
FileRange { file_id, range: range.range },
SyntaxKind::FN_KW,
func_def,
));
}
}
}
}
}
}
}

// For functions, look for structs with the same name
if let Definition::Function(func) = def {
let module = func.module(sema.db);
for item in module.declarations(sema.db) {
if let hir::ModuleDef::Adt(adt) = item {
let adt_name = adt.name(sema.db);
if adt_name.as_str() == name_str {
let adt_def = Definition::Adt(adt);
if !defs.iter().any(|(.., d)| d == &adt_def) {
if let Some(range) = adt_def.range_for_rename(sema) {
if range.file_id == file_id {
additional_defs.push((
FileRange { file_id, range: range.range },
SyntaxKind::STRUCT_KW,
adt_def,
));
}
}
}
}
}
}
}
}
}

// Add the additional definitions to our list
defs.extend(additional_defs);

Ok(defs.into_iter())
}
}
Err(e) => Err(e),
Expand Down Expand Up @@ -3197,6 +3260,49 @@ trait Trait<T> {
trait Trait<U> {
fn foo() -> impl use<U> Trait {}
}
"#,
);
}

#[test]
fn rename_both_struct_and_fn() {
check(
"baz",
r#"
pub use foo::bar;

mod foo {
pub struct bar$0 {
size: usize,
}

pub fn bar(size: usize) -> bar {
bar { size: 0 }
}
}

#[test]
fn test_bar() {
bar(5);
}
"#,
r#"
pub use foo::baz;

mod foo {
pub struct baz {
size: usize,
}

pub fn baz(size: usize) -> baz {
baz { size: 0 }
}
}

#[test]
fn test_bar() {
baz(5);
}
"#,
);
}
Expand Down
Loading