Skip to content

Commit

Permalink
feat: lsp rename/find-all-references for type aliases (#5414)
Browse files Browse the repository at this point in the history
# 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.
  • Loading branch information
asterite authored Jul 8, 2024
1 parent b58a034 commit 24c621f
Show file tree
Hide file tree
Showing 9 changed files with 82 additions and 50 deletions.
30 changes: 18 additions & 12 deletions compiler/noirc_frontend/src/elaborator/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
_ => (),
}
}

Expand Down
4 changes: 4 additions & 0 deletions compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
_ => (),
}
}
Expand Down
6 changes: 2 additions & 4 deletions compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -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(
Expand Down Expand Up @@ -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);
Expand Down
34 changes: 24 additions & 10 deletions compiler/noirc_frontend/src/locations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
Expand Down Expand Up @@ -107,13 +118,16 @@ impl NodeInterner {

let reference_node = self.reference_graph[node_index];
let found_locations: Vec<Location> = 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(
Expand Down
24 changes: 0 additions & 24 deletions compiler/noirc_frontend/src/node_interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,6 @@ pub struct NodeInterner {
// The location of each module
module_locations: HashMap<ModuleId, Location>,

// The location of each struct name
struct_name_locations: HashMap<StructId, Location>,

// The location of each trait name
trait_name_locations: HashMap<TraitId, Location>,

/// 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<DependencyId, ()>,
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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);
}
Expand Down
5 changes: 5 additions & 0 deletions tooling/lsp/src/requests/rename.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
6 changes: 6 additions & 0 deletions tooling/lsp/test_programs/rename_function/src/main.nr
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,9 @@ mod foo {
crate::another_function()
}
}

use foo::some_other_function as bar;

fn x() {
bar();
}
6 changes: 6 additions & 0 deletions tooling/lsp/test_programs/rename_type_alias/Nargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "rename_type_alias"
type = "bin"
authors = [""]

[dependencies]
17 changes: 17 additions & 0 deletions tooling/lsp/test_programs/rename_type_alias/src/main.nr
Original file line number Diff line number Diff line change
@@ -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
}

0 comments on commit 24c621f

Please sign in to comment.