3542: Renames work on struct field shorthands r=matklad a=m-n

When renaming either a local or a struct field, struct field shorthands are now renamed correctly.

Happy to refactor this if it doesn't fit the design of the code. Thanks for adding the suggestion of where to start on the issue.

I wasn't sure if I should also look at the behavior of renaming when placing the cursor at the field shorthand; the following describes the behavior with this patch:

```rust
#[test]
fn test_rename_field_shorthand_for_unspecified() {
    // when renaming a shorthand, should we have a way to specify
    // between renaming the field and the local?
    //
    // If not is this the correct default?
    test_rename(
        r#"
struct Foo {
    i: i32,
}
 impl Foo {
    fn new(i: i32) -> Self {
        Self { i<|> }
    }
}
"#,
        "j",
        r#"
struct Foo {
    i: i32,
}
 impl Foo {
    fn new(j: i32) -> Self {
        Self { i: j }
    }
}
"#,
    );
}
```
Resolves #3431

Co-authored-by: Matt Niemeir <matt.niemeir@gmail.com>
This commit is contained in:
bors[bot] 2020-03-11 09:22:09 +00:00 committed by GitHub
commit bf77850c81
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 210 additions and 26 deletions

View file

@ -9,7 +9,8 @@ use ra_syntax::{
use ra_text_edit::TextEdit; use ra_text_edit::TextEdit;
use crate::{ use crate::{
FileId, FilePosition, FileSystemEdit, RangeInfo, SourceChange, SourceFileEdit, TextRange, FilePosition, FileSystemEdit, RangeInfo, Reference, ReferenceKind, SourceChange,
SourceFileEdit, TextRange,
}; };
use super::find_all_refs; use super::find_all_refs;
@ -46,12 +47,29 @@ fn find_name_and_module_at_offset(
Some((ast_name, ast_module)) Some((ast_name, ast_module))
} }
fn source_edit_from_file_id_range( fn source_edit_from_reference(reference: Reference, new_name: &str) -> SourceFileEdit {
file_id: FileId, let mut replacement_text = String::new();
range: TextRange, let file_id = reference.file_range.file_id;
new_name: &str, let range = match reference.kind {
) -> SourceFileEdit { ReferenceKind::StructFieldShorthandForField => {
SourceFileEdit { file_id, edit: TextEdit::replace(range, new_name.into()) } replacement_text.push_str(new_name);
replacement_text.push_str(": ");
TextRange::from_to(
reference.file_range.range.start(),
reference.file_range.range.start(),
)
}
ReferenceKind::StructFieldShorthandForLocal => {
replacement_text.push_str(": ");
replacement_text.push_str(new_name);
TextRange::from_to(reference.file_range.range.end(), reference.file_range.range.end())
}
_ => {
replacement_text.push_str(new_name);
reference.file_range.range
}
};
SourceFileEdit { file_id, edit: TextEdit::replace(range, replacement_text) }
} }
fn rename_mod( fn rename_mod(
@ -99,13 +117,10 @@ fn rename_mod(
source_file_edits.push(edit); source_file_edits.push(edit);
if let Some(RangeInfo { range: _, info: refs }) = find_all_refs(sema.db, position, None) { if let Some(RangeInfo { range: _, info: refs }) = find_all_refs(sema.db, position, None) {
let ref_edits = refs.references.into_iter().map(|reference| { let ref_edits = refs
source_edit_from_file_id_range( .references
reference.file_range.file_id, .into_iter()
reference.file_range.range, .map(|reference| source_edit_from_reference(reference, new_name));
new_name,
)
});
source_file_edits.extend(ref_edits); source_file_edits.extend(ref_edits);
} }
@ -121,13 +136,7 @@ fn rename_reference(
let edit = refs let edit = refs
.into_iter() .into_iter()
.map(|reference| { .map(|reference| source_edit_from_reference(reference, new_name))
source_edit_from_file_id_range(
reference.file_range.file_id,
reference.file_range.range,
new_name,
)
})
.collect::<Vec<_>>(); .collect::<Vec<_>>();
if edit.is_empty() { if edit.is_empty() {
@ -285,6 +294,163 @@ mod tests {
); );
} }
#[test]
fn test_rename_struct_field() {
test_rename(
r#"
struct Foo {
i<|>: i32,
}
impl Foo {
fn new(i: i32) -> Self {
Self { i: i }
}
}
"#,
"j",
r#"
struct Foo {
j: i32,
}
impl Foo {
fn new(i: i32) -> Self {
Self { j: i }
}
}
"#,
);
}
#[test]
fn test_rename_struct_field_for_shorthand() {
test_rename(
r#"
struct Foo {
i<|>: i32,
}
impl Foo {
fn new(i: i32) -> Self {
Self { i }
}
}
"#,
"j",
r#"
struct Foo {
j: i32,
}
impl Foo {
fn new(i: i32) -> Self {
Self { j: i }
}
}
"#,
);
}
#[test]
fn test_rename_local_for_field_shorthand() {
test_rename(
r#"
struct Foo {
i: i32,
}
impl Foo {
fn new(i<|>: i32) -> Self {
Self { i }
}
}
"#,
"j",
r#"
struct Foo {
i: i32,
}
impl Foo {
fn new(j: i32) -> Self {
Self { i: j }
}
}
"#,
);
}
#[test]
fn test_field_shorthand_correct_struct() {
test_rename(
r#"
struct Foo {
i<|>: i32,
}
struct Bar {
i: i32,
}
impl Bar {
fn new(i: i32) -> Self {
Self { i }
}
}
"#,
"j",
r#"
struct Foo {
j: i32,
}
struct Bar {
i: i32,
}
impl Bar {
fn new(i: i32) -> Self {
Self { i }
}
}
"#,
);
}
#[test]
fn test_shadow_local_for_struct_shorthand() {
test_rename(
r#"
struct Foo {
i: i32,
}
fn baz(i<|>: i32) -> Self {
let x = Foo { i };
{
let i = 0;
Foo { i }
}
}
"#,
"j",
r#"
struct Foo {
i: i32,
}
fn baz(j: i32) -> Self {
let x = Foo { i: j };
{
let i = 0;
Foo { i }
}
}
"#,
);
}
#[test] #[test]
fn test_rename_mod() { fn test_rename_mod() {
let (analysis, position) = analysis_and_position( let (analysis, position) = analysis_and_position(

View file

@ -17,7 +17,7 @@ use rustc_hash::FxHashMap;
use test_utils::tested_by; use test_utils::tested_by;
use crate::{ use crate::{
defs::{classify_name_ref, Definition}, defs::{classify_name_ref, Definition, NameRefClass},
RootDatabase, RootDatabase,
}; };
@ -30,6 +30,8 @@ pub struct Reference {
#[derive(Debug, Clone, PartialEq)] #[derive(Debug, Clone, PartialEq)]
pub enum ReferenceKind { pub enum ReferenceKind {
StructFieldShorthandForField,
StructFieldShorthandForLocal,
StructLiteral, StructLiteral,
Other, Other,
} }
@ -237,9 +239,8 @@ impl Definition {
// FIXME: reuse sb // FIXME: reuse sb
// See https://github.com/rust-lang/rust/pull/68198#issuecomment-574269098 // See https://github.com/rust-lang/rust/pull/68198#issuecomment-574269098
if let Some(d) = classify_name_ref(&sema, &name_ref) { match classify_name_ref(&sema, &name_ref) {
let d = d.definition(); Some(NameRefClass::Definition(def)) if &def == self => {
if &d == self {
let kind = if is_record_lit_name_ref(&name_ref) let kind = if is_record_lit_name_ref(&name_ref)
|| is_call_expr_name_ref(&name_ref) || is_call_expr_name_ref(&name_ref)
{ {
@ -252,9 +253,26 @@ impl Definition {
refs.push(Reference { refs.push(Reference {
file_range, file_range,
kind, kind,
access: reference_access(&d, &name_ref), access: reference_access(&def, &name_ref),
}); });
} }
Some(NameRefClass::FieldShorthand { local, field }) => {
match self {
Definition::StructField(_) if &field == self => refs.push(Reference {
file_range: sema.original_range(name_ref.syntax()),
kind: ReferenceKind::StructFieldShorthandForField,
access: reference_access(&field, &name_ref),
}),
Definition::Local(l) if &local == l => refs.push(Reference {
file_range: sema.original_range(name_ref.syntax()),
kind: ReferenceKind::StructFieldShorthandForLocal,
access: reference_access(&Definition::Local(local), &name_ref),
}),
_ => {} // not a usage
};
}
_ => {} // not a usage
} }
} }
} }