From e55a44a831477e2fc8e11340c3d91db883b97c8e Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sat, 14 Nov 2020 17:49:36 +0100 Subject: [PATCH 1/4] Use shorthand record syntax when renaming struct initializer field --- crates/ide/src/references/rename.rs | 49 ++++++++++++++++++++++++++--- crates/ide_db/src/search.rs | 16 ++++++---- crates/syntax/src/ast/expr_ext.rs | 12 +++++++ crates/syntax/src/ast/node_ext.rs | 10 +----- 4 files changed, 68 insertions(+), 19 deletions(-) diff --git a/crates/ide/src/references/rename.rs b/crates/ide/src/references/rename.rs index 26ac2371a527..b3ade20ef0a2 100644 --- a/crates/ide/src/references/rename.rs +++ b/crates/ide/src/references/rename.rs @@ -106,7 +106,11 @@ fn find_module_at_offset( Some(module) } -fn source_edit_from_reference(reference: Reference, new_name: &str) -> SourceFileEdit { +fn source_edit_from_reference( + sema: &Semantics, + reference: Reference, + new_name: &str, +) -> SourceFileEdit { let mut replacement_text = String::new(); let file_id = reference.file_range.file_id; let range = match reference.kind { @@ -122,6 +126,22 @@ fn source_edit_from_reference(reference: Reference, new_name: &str) -> SourceFil replacement_text.push_str(new_name); TextRange::new(reference.file_range.range.end(), reference.file_range.range.end()) } + ReferenceKind::RecordExprField => { + replacement_text.push_str(new_name); + let mut range = reference.file_range.range; + if let Some(field_expr) = syntax::algo::find_node_at_range::( + sema.parse(file_id).syntax(), + reference.file_range.range, + ) { + // use shorthand initializer if we were to write foo: foo + if let Some(name) = field_expr.expr().and_then(|e| e.name_ref()) { + if &name.to_string() == new_name { + range = field_expr.syntax().text_range(); + } + } + } + range + } _ => { replacement_text.push_str(new_name); reference.file_range.range @@ -170,7 +190,7 @@ fn rename_mod( let ref_edits = refs .references .into_iter() - .map(|reference| source_edit_from_reference(reference, new_name)); + .map(|reference| source_edit_from_reference(sema, reference, new_name)); source_file_edits.extend(ref_edits); Ok(RangeInfo::new(range, SourceChange::from_edits(source_file_edits, file_system_edits))) @@ -211,7 +231,7 @@ fn rename_to_self( let mut edits = usages .into_iter() - .map(|reference| source_edit_from_reference(reference, "self")) + .map(|reference| source_edit_from_reference(sema, reference, "self")) .collect::>(); edits.push(SourceFileEdit { @@ -300,7 +320,7 @@ fn rename_reference( let edit = refs .into_iter() - .map(|reference| source_edit_from_reference(reference, new_name)) + .map(|reference| source_edit_from_reference(sema, reference, new_name)) .collect::>(); if edit.is_empty() { @@ -1094,6 +1114,27 @@ impl Foo { foo.i } } +"#, + ); + } + + #[test] + fn test_initializer_use_field_init_shorthand() { + check( + "bar", + r#" +struct Foo { i<|>: i32 } + +fn foo(bar: i32) -> Foo { + Foo { i: bar } +} +"#, + r#" +struct Foo { bar: i32 } + +fn foo(bar: i32) -> Foo { + Foo { bar } +} "#, ); } diff --git a/crates/ide_db/src/search.rs b/crates/ide_db/src/search.rs index a243352406ee..4248606c8a35 100644 --- a/crates/ide_db/src/search.rs +++ b/crates/ide_db/src/search.rs @@ -30,6 +30,7 @@ pub enum ReferenceKind { FieldShorthandForField, FieldShorthandForLocal, StructLiteral, + RecordExprField, Other, } @@ -278,12 +279,15 @@ impl<'a> FindUsages<'a> { ) -> bool { match NameRefClass::classify(self.sema, &name_ref) { Some(NameRefClass::Definition(def)) if &def == self.def => { - let kind = if is_record_lit_name_ref(&name_ref) || is_call_expr_name_ref(&name_ref) - { - ReferenceKind::StructLiteral - } else { - ReferenceKind::Other - }; + let kind = + if name_ref.syntax().parent().and_then(ast::RecordExprField::cast).is_some() { + ReferenceKind::RecordExprField + } else if is_record_lit_name_ref(&name_ref) || is_call_expr_name_ref(&name_ref) + { + ReferenceKind::StructLiteral + } else { + ReferenceKind::Other + }; let reference = Reference { file_range: self.sema.original_range(name_ref.syntax()), diff --git a/crates/syntax/src/ast/expr_ext.rs b/crates/syntax/src/ast/expr_ext.rs index 9253c97d084f..e4a9b945c98f 100644 --- a/crates/syntax/src/ast/expr_ext.rs +++ b/crates/syntax/src/ast/expr_ext.rs @@ -22,6 +22,18 @@ impl ast::Expr { _ => false, } } + + pub fn name_ref(&self) -> Option { + if let ast::Expr::PathExpr(expr) = self { + let path = expr.path()?; + let segment = path.segment()?; + let name_ref = segment.name_ref()?; + if path.qualifier().is_none() { + return Some(name_ref); + } + } + None + } } #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/crates/syntax/src/ast/node_ext.rs b/crates/syntax/src/ast/node_ext.rs index ce35ac01afd3..b70b840b81ab 100644 --- a/crates/syntax/src/ast/node_ext.rs +++ b/crates/syntax/src/ast/node_ext.rs @@ -203,15 +203,7 @@ impl ast::RecordExprField { if let Some(name_ref) = self.name_ref() { return Some(name_ref); } - if let Some(ast::Expr::PathExpr(expr)) = self.expr() { - let path = expr.path()?; - let segment = path.segment()?; - let name_ref = segment.name_ref()?; - if path.qualifier().is_none() { - return Some(name_ref); - } - } - None + self.expr()?.name_ref() } } From 924eecf4af4d57c597c2e77c5e58c22b2a37bdb6 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sat, 14 Nov 2020 19:11:09 +0100 Subject: [PATCH 2/4] Properly handle shorthands in destructure patterns when renaming --- crates/ide/src/references.rs | 21 +++++++---- crates/ide/src/references/rename.rs | 54 +++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 6 deletions(-) diff --git a/crates/ide/src/references.rs b/crates/ide/src/references.rs index e05465b32e9d..10cd42032a32 100644 --- a/crates/ide/src/references.rs +++ b/crates/ide/src/references.rs @@ -110,14 +110,23 @@ pub(crate) fn find_all_refs( .filter(|r| search_kind == ReferenceKind::Other || search_kind == r.kind) .collect(); - let decl_range = def.try_to_nav(sema.db)?.focus_or_full_range(); - - let declaration = Declaration { - nav: def.try_to_nav(sema.db)?, - kind: ReferenceKind::Other, - access: decl_access(&def, &syntax, decl_range), + let nav = def.try_to_nav(sema.db)?; + let decl_range = nav.focus_or_full_range(); + + let mut kind = ReferenceKind::Other; + if let Definition::Local(local) = def { + if let either::Either::Left(pat) = local.source(sema.db).value { + if matches!( + pat.syntax().parent().and_then(ast::RecordPatField::cast), + Some(pat_field) if pat_field.name_ref().is_none() + ) { + kind = ReferenceKind::FieldShorthandForLocal; + } + } }; + let declaration = Declaration { nav, kind, access: decl_access(&def, &syntax, decl_range) }; + Some(RangeInfo::new(range, ReferenceSearchResult { declaration, references })) } diff --git a/crates/ide/src/references/rename.rs b/crates/ide/src/references/rename.rs index b3ade20ef0a2..bc4aa25bf8ff 100644 --- a/crates/ide/src/references/rename.rs +++ b/crates/ide/src/references/rename.rs @@ -1135,6 +1135,60 @@ struct Foo { bar: i32 } fn foo(bar: i32) -> Foo { Foo { bar } } +"#, + ); + } + + #[test] + fn test_rename_binding_in_destructure_pat_shorthand() { + check( + "bar", + r#" +struct Foo { + i: i32, +} + +fn foo(foo: Foo) { + let Foo { i } = foo; + let _ = i<|>; +} +"#, + r#" +struct Foo { + i: i32, +} + +fn foo(foo: Foo) { + let Foo { i: bar } = foo; + let _ = bar; +} +"#, + ); + } + + #[test] + fn test_rename_binding_in_destructure_pat() { + check( + "bar", + r#" +struct Foo { + i: i32, +} + +fn foo(foo: Foo) { + let Foo { i: b } = foo; + let _ = b<|>; +} +"#, + r#" +struct Foo { + i: i32, +} + +fn foo(foo: Foo) { + let Foo { i: bar } = foo; + let _ = bar; +} "#, ); } From cb60708274dc7c8cff281364507d23047cd482cd Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sat, 14 Nov 2020 19:57:47 +0100 Subject: [PATCH 3/4] Use shorthand field syntax in destructures --- crates/ide/src/references.rs | 4 +- crates/ide/src/references/rename.rs | 77 ++++++++++++++++++++++------- crates/ide_db/src/search.rs | 32 ++++++++---- 3 files changed, 84 insertions(+), 29 deletions(-) diff --git a/crates/ide/src/references.rs b/crates/ide/src/references.rs index 10cd42032a32..5693dd400e27 100644 --- a/crates/ide/src/references.rs +++ b/crates/ide/src/references.rs @@ -622,7 +622,7 @@ fn foo() { expect![[r#" f RECORD_FIELD FileId(0) 15..21 15..16 Other - FileId(0) 55..56 Other Read + FileId(0) 55..56 RecordFieldExprOrPat Read FileId(0) 68..69 Other Write "#]], ); @@ -757,7 +757,7 @@ fn f() -> m::En { expect![[r#" field RECORD_FIELD FileId(0) 56..65 56..61 Other - FileId(0) 125..130 Other Read + FileId(0) 125..130 RecordFieldExprOrPat Read "#]], ); } diff --git a/crates/ide/src/references/rename.rs b/crates/ide/src/references/rename.rs index bc4aa25bf8ff..449cfa4aedb1 100644 --- a/crates/ide/src/references/rename.rs +++ b/crates/ide/src/references/rename.rs @@ -1,7 +1,7 @@ //! FIXME: write short doc here use hir::{Module, ModuleDef, ModuleSource, Semantics}; -use ide_db::base_db::SourceDatabaseExt; +use ide_db::base_db::{FileRange, SourceDatabaseExt}; use ide_db::{ defs::{Definition, NameClass, NameRefClass}, RootDatabase, @@ -112,7 +112,6 @@ fn source_edit_from_reference( new_name: &str, ) -> SourceFileEdit { let mut replacement_text = String::new(); - let file_id = reference.file_range.file_id; let range = match reference.kind { ReferenceKind::FieldShorthandForField => { mark::hit!(test_rename_struct_field_for_shorthand); @@ -126,28 +125,49 @@ fn source_edit_from_reference( replacement_text.push_str(new_name); TextRange::new(reference.file_range.range.end(), reference.file_range.range.end()) } - ReferenceKind::RecordExprField => { + ReferenceKind::RecordFieldExprOrPat => { replacement_text.push_str(new_name); - let mut range = reference.file_range.range; - if let Some(field_expr) = syntax::algo::find_node_at_range::( - sema.parse(file_id).syntax(), - reference.file_range.range, - ) { - // use shorthand initializer if we were to write foo: foo - if let Some(name) = field_expr.expr().and_then(|e| e.name_ref()) { - if &name.to_string() == new_name { - range = field_expr.syntax().text_range(); - } - } - } - range + edit_text_range_for_record_field_expr_or_pat(sema, reference.file_range, new_name) } _ => { replacement_text.push_str(new_name); reference.file_range.range } }; - SourceFileEdit { file_id, edit: TextEdit::replace(range, replacement_text) } + SourceFileEdit { + file_id: reference.file_range.file_id, + edit: TextEdit::replace(range, replacement_text), + } +} + +fn edit_text_range_for_record_field_expr_or_pat( + sema: &Semantics, + file_range: FileRange, + new_name: &str, +) -> TextRange { + let mut range = file_range.range; + let source_file = sema.parse(file_range.file_id); + let file_syntax = source_file.syntax(); + if let Some(field_expr) = + syntax::algo::find_node_at_range::(file_syntax, range) + { + match field_expr.expr().and_then(|e| e.name_ref()) { + Some(name) if &name.to_string() == new_name => range = field_expr.syntax().text_range(), + _ => (), + } + } else if let Some(field_pat) = + syntax::algo::find_node_at_range::(file_syntax, range) + { + match field_pat.pat() { + Some(ast::Pat::IdentPat(pat)) + if pat.name().map(|n| n.to_string()).as_deref() == Some(new_name) => + { + range = field_pat.syntax().text_range() + } + _ => (), + } + } + range } fn rename_mod( @@ -1189,6 +1209,29 @@ fn foo(foo: Foo) { let Foo { i: bar } = foo; let _ = bar; } +"#, + ); + } + + #[test] + fn test_struct_field_destructure_into_shorthand() { + check( + "baz", + r#" +struct Foo { i<|>: i32 } + +fn foo(foo: Foo) { + let Foo { i: baz } = foo; + let _ = baz; +} +"#, + r#" +struct Foo { baz: i32 } + +fn foo(foo: Foo) { + let Foo { baz } = foo; + let _ = baz; +} "#, ); } diff --git a/crates/ide_db/src/search.rs b/crates/ide_db/src/search.rs index 4248606c8a35..a3e765d0598d 100644 --- a/crates/ide_db/src/search.rs +++ b/crates/ide_db/src/search.rs @@ -30,7 +30,7 @@ pub enum ReferenceKind { FieldShorthandForField, FieldShorthandForLocal, StructLiteral, - RecordExprField, + RecordFieldExprOrPat, Other, } @@ -279,15 +279,13 @@ impl<'a> FindUsages<'a> { ) -> bool { match NameRefClass::classify(self.sema, &name_ref) { Some(NameRefClass::Definition(def)) if &def == self.def => { - let kind = - if name_ref.syntax().parent().and_then(ast::RecordExprField::cast).is_some() { - ReferenceKind::RecordExprField - } else if is_record_lit_name_ref(&name_ref) || is_call_expr_name_ref(&name_ref) - { - ReferenceKind::StructLiteral - } else { - ReferenceKind::Other - }; + let kind = if is_record_field_expr_or_pat(&name_ref) { + ReferenceKind::RecordFieldExprOrPat + } else if is_record_lit_name_ref(&name_ref) || is_call_expr_name_ref(&name_ref) { + ReferenceKind::StructLiteral + } else { + ReferenceKind::Other + }; let reference = Reference { file_range: self.sema.original_range(name_ref.syntax()), @@ -389,3 +387,17 @@ fn is_record_lit_name_ref(name_ref: &ast::NameRef) -> bool { .map(|p| p.name_ref().as_ref() == Some(name_ref)) .unwrap_or(false) } + +fn is_record_field_expr_or_pat(name_ref: &ast::NameRef) -> bool { + if let Some(parent) = name_ref.syntax().parent() { + match_ast! { + match parent { + ast::RecordExprField(it) => true, + ast::RecordPatField(_it) => true, + _ => false, + } + } + } else { + false + } +} From f3e297331c5c4b8c03810acc745ae5b4a9c4f71f Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 15 Nov 2020 16:11:06 +0100 Subject: [PATCH 4/4] Cleanup edit_text_range_for_record_field_expr_or_pat --- crates/ide/src/references/rename.rs | 105 ++++++++++++++++------------ 1 file changed, 59 insertions(+), 46 deletions(-) diff --git a/crates/ide/src/references/rename.rs b/crates/ide/src/references/rename.rs index 449cfa4aedb1..b8725693aa67 100644 --- a/crates/ide/src/references/rename.rs +++ b/crates/ide/src/references/rename.rs @@ -126,6 +126,7 @@ fn source_edit_from_reference( TextRange::new(reference.file_range.range.end(), reference.file_range.range.end()) } ReferenceKind::RecordFieldExprOrPat => { + mark::hit!(test_rename_field_expr_pat); replacement_text.push_str(new_name); edit_text_range_for_record_field_expr_or_pat(sema, reference.file_range, new_name) } @@ -145,29 +146,27 @@ fn edit_text_range_for_record_field_expr_or_pat( file_range: FileRange, new_name: &str, ) -> TextRange { - let mut range = file_range.range; let source_file = sema.parse(file_range.file_id); let file_syntax = source_file.syntax(); - if let Some(field_expr) = - syntax::algo::find_node_at_range::(file_syntax, range) - { - match field_expr.expr().and_then(|e| e.name_ref()) { - Some(name) if &name.to_string() == new_name => range = field_expr.syntax().text_range(), - _ => (), - } - } else if let Some(field_pat) = - syntax::algo::find_node_at_range::(file_syntax, range) - { - match field_pat.pat() { - Some(ast::Pat::IdentPat(pat)) - if pat.name().map(|n| n.to_string()).as_deref() == Some(new_name) => - { - range = field_pat.syntax().text_range() - } - _ => (), - } - } - range + let original_range = file_range.range; + + syntax::algo::find_node_at_range::(file_syntax, original_range) + .and_then(|field_expr| match field_expr.expr().and_then(|e| e.name_ref()) { + Some(name) if &name.to_string() == new_name => Some(field_expr.syntax().text_range()), + _ => None, + }) + .or_else(|| { + syntax::algo::find_node_at_range::(file_syntax, original_range) + .and_then(|field_pat| match field_pat.pat() { + Some(ast::Pat::IdentPat(pat)) + if pat.name().map(|n| n.to_string()).as_deref() == Some(new_name) => + { + Some(field_pat.syntax().text_range()) + } + _ => None, + }) + }) + .unwrap_or(original_range) } fn rename_mod( @@ -1140,6 +1139,7 @@ impl Foo { #[test] fn test_initializer_use_field_init_shorthand() { + mark::check!(test_rename_field_expr_pat); check( "bar", r#" @@ -1160,27 +1160,23 @@ fn foo(bar: i32) -> Foo { } #[test] - fn test_rename_binding_in_destructure_pat_shorthand() { + fn test_struct_field_destructure_into_shorthand() { check( - "bar", + "baz", r#" -struct Foo { - i: i32, -} +struct Foo { i<|>: i32 } fn foo(foo: Foo) { - let Foo { i } = foo; - let _ = i<|>; + let Foo { i: baz } = foo; + let _ = baz; } "#, r#" -struct Foo { - i: i32, -} +struct Foo { baz: i32 } fn foo(foo: Foo) { - let Foo { i: bar } = foo; - let _ = bar; + let Foo { baz } = foo; + let _ = baz; } "#, ); @@ -1188,6 +1184,16 @@ fn foo(foo: Foo) { #[test] fn test_rename_binding_in_destructure_pat() { + let expected_fixture = r#" +struct Foo { + i: i32, +} + +fn foo(foo: Foo) { + let Foo { i: bar } = foo; + let _ = bar; +} +"#; check( "bar", r#" @@ -1200,39 +1206,46 @@ fn foo(foo: Foo) { let _ = b<|>; } "#, + expected_fixture, + ); + check( + "bar", r#" struct Foo { i: i32, } fn foo(foo: Foo) { - let Foo { i: bar } = foo; - let _ = bar; + let Foo { i } = foo; + let _ = i<|>; } "#, + expected_fixture, ); } #[test] - fn test_struct_field_destructure_into_shorthand() { + fn test_rename_binding_in_destructure_param_pat() { check( - "baz", + "bar", r#" -struct Foo { i<|>: i32 } +struct Foo { + i: i32 +} -fn foo(foo: Foo) { - let Foo { i: baz } = foo; - let _ = baz; +fn foo(Foo { i }: foo) -> i32 { + i<|> } "#, r#" -struct Foo { baz: i32 } +struct Foo { + i: i32 +} -fn foo(foo: Foo) { - let Foo { baz } = foo; - let _ = baz; +fn foo(Foo { i: bar }: foo) -> i32 { + bar } "#, - ); + ) } }