From 24c621fa96783373ab81da66cb6076e130c4a3a5 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 8 Jul 2024 14:39:04 -0300 Subject: [PATCH] feat: lsp rename/find-all-references for type aliases (#5414) # Description ## Problem Resolves #5358 ## Summary Based on top of #5411 Implements the usual rename/find-all-references but this time for type aliases. I noticed some things related to name locations could be simplified (not stored in a duplicate way) so there's also a small simplification here. https://github.com/noir-lang/noir/assets/209371/77508029-7002-4708-9cc1-24fc879630db ## Additional Context None. ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- .../noirc_frontend/src/elaborator/types.rs | 30 +++++++++------- .../src/hir/def_collector/dc_crate.rs | 4 +++ .../src/hir/def_collector/dc_mod.rs | 6 ++-- compiler/noirc_frontend/src/locations.rs | 34 +++++++++++++------ compiler/noirc_frontend/src/node_interner.rs | 24 ------------- tooling/lsp/src/requests/rename.rs | 5 +++ .../test_programs/rename_function/src/main.nr | 6 ++++ .../rename_type_alias/Nargo.toml | 6 ++++ .../rename_type_alias/src/main.nr | 17 ++++++++++ 9 files changed, 82 insertions(+), 50 deletions(-) create mode 100644 tooling/lsp/test_programs/rename_type_alias/Nargo.toml create mode 100644 tooling/lsp/test_programs/rename_type_alias/src/main.nr diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index f0625308ec6..1aebe6a0ee2 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -153,22 +153,28 @@ impl<'context> Elaborator<'context> { Resolved(id) => self.interner.get_quoted_type(id).clone(), }; - if let Type::Struct(ref struct_type, _) = resolved_type { - if let Some(unresolved_span) = typ.span { - // Record the location of the type reference - self.interner.push_type_ref_location( - resolved_type.clone(), - Location::new(unresolved_span, self.file), - ); - - if !is_synthetic { - let referenced = ReferenceId::Struct(struct_type.borrow().id); - let reference = ReferenceId::Variable( + if let Some(unresolved_span) = typ.span { + let reference = + ReferenceId::Variable(Location::new(unresolved_span, self.file), is_self_type_name); + + match resolved_type { + Type::Struct(ref struct_type, _) => { + // Record the location of the type reference + self.interner.push_type_ref_location( + resolved_type.clone(), Location::new(unresolved_span, self.file), - is_self_type_name, ); + + if !is_synthetic { + let referenced = ReferenceId::Struct(struct_type.borrow().id); + self.interner.add_reference(referenced, reference); + } + } + Type::Alias(ref alias_type, _) => { + let referenced = ReferenceId::Alias(alias_type.borrow().id); self.interner.add_reference(referenced, reference); } + _ => (), } } diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 5b553b55257..ee9981863cb 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -524,6 +524,10 @@ fn add_import_reference( let variable = ReferenceId::Variable(Location::new(name.span(), file_id), false); interner.add_reference(ReferenceId::Trait(trait_id), variable); } + crate::macros_api::ModuleDefId::TypeAliasId(type_alias_id) => { + let variable = ReferenceId::Variable(Location::new(name.span(), file_id), false); + interner.add_reference(ReferenceId::Alias(type_alias_id), variable); + } _ => (), } } diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index b94c9dca90d..483b061998e 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -268,7 +268,6 @@ impl<'a> ModCollector<'a> { let mut definition_errors = vec![]; for struct_definition in types { let name = struct_definition.name.clone(); - let name_location = Location::new(name.span(), self.file_id); let unresolved = UnresolvedStruct { file_id: self.file_id, @@ -319,7 +318,6 @@ impl<'a> ModCollector<'a> { // And store the TypeId -> StructType mapping somewhere it is reachable self.def_collector.items.types.insert(id, unresolved); - context.def_interner.add_struct_location(id, name_location); context.def_interner.add_definition_location(ReferenceId::Struct(id)); } definition_errors @@ -366,6 +364,8 @@ impl<'a> ModCollector<'a> { } self.def_collector.items.type_aliases.insert(type_alias_id, unresolved); + + context.def_interner.add_definition_location(ReferenceId::Alias(type_alias_id)); } errors } @@ -381,7 +381,6 @@ impl<'a> ModCollector<'a> { let mut errors: Vec<(CompilationError, FileId)> = vec![]; for trait_definition in traits { let name = trait_definition.name.clone(); - let name_location = Location::new(name.span(), self.file_id); // Create the corresponding module for the trait namespace let trait_id = match self.push_child_module( @@ -533,7 +532,6 @@ impl<'a> ModCollector<'a> { }; context.def_interner.push_empty_trait(trait_id, &unresolved, resolved_generics); - context.def_interner.add_trait_location(trait_id, name_location); context.def_interner.add_definition_location(ReferenceId::Trait(trait_id)); self.def_collector.items.traits.insert(trait_id, unresolved); diff --git a/compiler/noirc_frontend/src/locations.rs b/compiler/noirc_frontend/src/locations.rs index 9b03cb932b2..30095fbb8c7 100644 --- a/compiler/noirc_frontend/src/locations.rs +++ b/compiler/noirc_frontend/src/locations.rs @@ -33,10 +33,21 @@ impl NodeInterner { match reference { ReferenceId::Module(id) => self.module_location(&id), ReferenceId::Function(id) => self.function_modifiers(&id).name_location, - ReferenceId::Struct(id) => self.struct_location(&id), - ReferenceId::Trait(id) => self.trait_location(&id), + ReferenceId::Struct(id) => { + let struct_type = self.get_struct(id); + let struct_type = struct_type.borrow(); + Location::new(struct_type.name.span(), struct_type.location.file) + } + ReferenceId::Trait(id) => { + let trait_type = self.get_trait(id); + Location::new(trait_type.name.span(), trait_type.location.file) + } ReferenceId::Global(id) => self.get_global(id).location, - ReferenceId::Alias(id) => self.get_type_alias(id).borrow().location, + ReferenceId::Alias(id) => { + let alias_type = self.get_type_alias(id); + let alias_type = alias_type.borrow(); + Location::new(alias_type.name.span(), alias_type.location.file) + } ReferenceId::Variable(location, _) => location, } } @@ -107,13 +118,16 @@ impl NodeInterner { let reference_node = self.reference_graph[node_index]; let found_locations: Vec = match reference_node { - ReferenceId::Alias(_) | ReferenceId::Global(_) | ReferenceId::Module(_) => todo!(), - ReferenceId::Function(_) | ReferenceId::Struct(_) | ReferenceId::Trait(_) => self - .find_all_references_for_index( - node_index, - include_referenced, - include_self_type_name, - ), + ReferenceId::Global(_) | ReferenceId::Module(_) => todo!(), + ReferenceId::Function(_) + | ReferenceId::Struct(_) + | ReferenceId::Trait(_) + | ReferenceId::Alias(_) => self.find_all_references_for_index( + node_index, + include_referenced, + include_self_type_name, + ), + ReferenceId::Variable(_, _) => { let referenced_node_index = self.referenced_index(node_index)?; self.find_all_references_for_index( diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index fd63ef66795..1618058a9f0 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -68,12 +68,6 @@ pub struct NodeInterner { // The location of each module module_locations: HashMap, - // The location of each struct name - struct_name_locations: HashMap, - - // The location of each trait name - trait_name_locations: HashMap, - /// This graph tracks dependencies between different global definitions. /// This is used to ensure the absence of dependency cycles for globals and types. dependency_graph: DiGraph, @@ -553,8 +547,6 @@ impl Default for NodeInterner { function_modifiers: HashMap::new(), function_modules: HashMap::new(), module_locations: HashMap::new(), - struct_name_locations: HashMap::new(), - trait_name_locations: HashMap::new(), func_id_to_trait: HashMap::new(), dependency_graph: petgraph::graph::DiGraph::new(), dependency_graph_indices: HashMap::new(), @@ -986,22 +978,6 @@ impl NodeInterner { &self.struct_attributes[struct_id] } - pub fn add_struct_location(&mut self, struct_id: StructId, location: Location) { - self.struct_name_locations.insert(struct_id, location); - } - - pub fn struct_location(&self, struct_id: &StructId) -> Location { - self.struct_name_locations[struct_id] - } - - pub fn add_trait_location(&mut self, trait_id: TraitId, location: Location) { - self.trait_name_locations.insert(trait_id, location); - } - - pub fn trait_location(&self, trait_id: &TraitId) -> Location { - self.trait_name_locations[trait_id] - } - pub fn add_module_location(&mut self, module_id: ModuleId, location: Location) { self.module_locations.insert(module_id, location); } diff --git a/tooling/lsp/src/requests/rename.rs b/tooling/lsp/src/requests/rename.rs index 0664c5f0667..54c3297afb9 100644 --- a/tooling/lsp/src/requests/rename.rs +++ b/tooling/lsp/src/requests/rename.rs @@ -184,4 +184,9 @@ mod rename_tests { async fn test_rename_trait() { check_rename_succeeds("rename_trait", "Foo").await; } + + #[test] + async fn test_rename_type_alias() { + check_rename_succeeds("rename_type_alias", "Bar").await; + } } diff --git a/tooling/lsp/test_programs/rename_function/src/main.nr b/tooling/lsp/test_programs/rename_function/src/main.nr index ad526f10966..7a70084276e 100644 --- a/tooling/lsp/test_programs/rename_function/src/main.nr +++ b/tooling/lsp/test_programs/rename_function/src/main.nr @@ -19,3 +19,9 @@ mod foo { crate::another_function() } } + +use foo::some_other_function as bar; + +fn x() { + bar(); +} diff --git a/tooling/lsp/test_programs/rename_type_alias/Nargo.toml b/tooling/lsp/test_programs/rename_type_alias/Nargo.toml new file mode 100644 index 00000000000..1b95727ed8d --- /dev/null +++ b/tooling/lsp/test_programs/rename_type_alias/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "rename_type_alias" +type = "bin" +authors = [""] + +[dependencies] diff --git a/tooling/lsp/test_programs/rename_type_alias/src/main.nr b/tooling/lsp/test_programs/rename_type_alias/src/main.nr new file mode 100644 index 00000000000..4a0c497a21f --- /dev/null +++ b/tooling/lsp/test_programs/rename_type_alias/src/main.nr @@ -0,0 +1,17 @@ +mod foo { + struct Foo { + } + + type Bar = Foo; +} + +use foo::Foo; +use foo::Bar; + +fn main() { + let x: Bar = Foo {}; +} + +fn x(b: Bar) -> Bar { + b +}