diff --git a/compiler/noirc_frontend/src/ast/statement.rs b/compiler/noirc_frontend/src/ast/statement.rs index 48e34ad7fc9..3e6a140ff93 100644 --- a/compiler/noirc_frontend/src/ast/statement.rs +++ b/compiler/noirc_frontend/src/ast/statement.rs @@ -10,6 +10,7 @@ use super::{ BlockExpression, Expression, ExpressionKind, IndexExpression, MemberAccessExpression, MethodCallExpression, UnresolvedType, }; +use crate::hir::resolution::resolver::SELF_TYPE_NAME; use crate::lexer::token::SpannedToken; use crate::macros_api::SecondaryAttribute; use crate::parser::{ParserError, ParserErrorReason}; @@ -165,6 +166,12 @@ impl StatementKind { #[derive(Eq, Debug, Clone)] pub struct Ident(pub Spanned); +impl Ident { + pub fn is_self_type_name(&self) -> bool { + self.0.contents == SELF_TYPE_NAME + } +} + impl PartialEq for Ident { fn eq(&self, other: &Ident) -> bool { self.0.contents == other.0.contents diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index e013cf7b4f1..5c3866816a6 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -403,6 +403,7 @@ impl<'context> Elaborator<'context> { constructor: ConstructorExpression, ) -> (HirExpression, Type) { let span = constructor.type_name.span(); + let is_self_type = constructor.type_name.last_segment().is_self_type_name(); let (r#type, struct_generics) = if let Some(struct_id) = constructor.struct_type { let typ = self.interner.get_struct(struct_id); @@ -433,7 +434,7 @@ impl<'context> Elaborator<'context> { }); let referenced = ReferenceId::Struct(struct_type.borrow().id); - let reference = ReferenceId::Variable(Location::new(span, self.file)); + let reference = ReferenceId::Variable(Location::new(span, self.file), is_self_type); self.interner.add_reference(referenced, reference); (expr, Type::Struct(struct_type, generics)) diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 2065657c864..7c829b5df47 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -1449,11 +1449,13 @@ impl<'context> Elaborator<'context> { self.generics.clear(); if let Some(trait_id) = trait_id { + let trait_name = trait_impl.trait_path.last_segment(); + let referenced = ReferenceId::Trait(trait_id); - let reference = ReferenceId::Variable(Location::new( - trait_impl.trait_path.last_segment().span(), - trait_impl.file_id, - )); + let reference = ReferenceId::Variable( + Location::new(trait_name.span(), trait_impl.file_id), + trait_name.is_self_type_name(), + ); self.interner.add_reference(referenced, reference); } } diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index e3c854d615d..81fffb522bf 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -159,6 +159,7 @@ impl<'context> Elaborator<'context> { new_definitions: &mut Vec, ) -> HirPattern { let name_span = name.last_segment().span(); + let is_self_type = name.last_segment().is_self_type_name(); let error_identifier = |this: &mut Self| { // Must create a name here to return a HirPattern::Identifier. Allowing @@ -200,7 +201,7 @@ impl<'context> Elaborator<'context> { ); let referenced = ReferenceId::Struct(struct_type.borrow().id); - let reference = ReferenceId::Variable(Location::new(name_span, self.file)); + let reference = ReferenceId::Variable(Location::new(name_span, self.file), is_self_type); self.interner.add_reference(referenced, reference); HirPattern::Struct(expected_type, fields, location) @@ -437,6 +438,7 @@ impl<'context> Elaborator<'context> { // Otherwise, then it is referring to an Identifier // This lookup allows support of such statements: let x = foo::bar::SOME_GLOBAL + 10; // If the expression is a singular indent, we search the resolver's current scope as normal. + let is_self_type_name = path.last_segment().is_self_type_name(); let (hir_ident, var_scope_index) = self.get_ident_from_path(path); if hir_ident.id != DefinitionId::dummy_id() { @@ -446,7 +448,7 @@ impl<'context> Elaborator<'context> { self.interner.add_function_dependency(current_item, func_id); } - let variable = ReferenceId::Variable(hir_ident.location); + let variable = ReferenceId::Variable(hir_ident.location, is_self_type_name); let function = ReferenceId::Function(func_id); self.interner.add_reference(function, variable); } @@ -458,7 +460,7 @@ impl<'context> Elaborator<'context> { self.interner.add_global_dependency(current_item, global_id); } - let variable = ReferenceId::Variable(hir_ident.location); + let variable = ReferenceId::Variable(hir_ident.location, is_self_type_name); let global = ReferenceId::Global(global_id); self.interner.add_reference(global, variable); } diff --git a/compiler/noirc_frontend/src/elaborator/scope.rs b/compiler/noirc_frontend/src/elaborator/scope.rs index b253e272982..6c556e000d5 100644 --- a/compiler/noirc_frontend/src/elaborator/scope.rs +++ b/compiler/noirc_frontend/src/elaborator/scope.rs @@ -53,7 +53,10 @@ impl<'context> Elaborator<'context> { resolver.resolve(self.def_maps, path.clone(), &mut Some(&mut references))?; for (referenced, ident) in references.iter().zip(path.segments) { - let reference = ReferenceId::Variable(Location::new(ident.span(), self.file)); + let reference = ReferenceId::Variable( + Location::new(ident.span(), self.file), + ident.is_self_type_name(), + ); self.interner.add_reference(*referenced, reference); } } else { diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index b6212abb4a2..f0625308ec6 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -58,6 +58,12 @@ impl<'context> Elaborator<'context> { use crate::ast::UnresolvedTypeData::*; let span = typ.span; + let (is_self_type_name, is_synthetic) = if let Named(ref named_path, _, synthetic) = typ.typ + { + (named_path.last_segment().is_self_type_name(), synthetic) + } else { + (false, false) + }; let resolved_type = match typ.typ { FieldElement => Type::FieldElement, @@ -155,9 +161,14 @@ impl<'context> Elaborator<'context> { Location::new(unresolved_span, self.file), ); - let referenced = ReferenceId::Struct(struct_type.borrow().id); - let reference = ReferenceId::Variable(Location::new(unresolved_span, self.file)); - self.interner.add_reference(referenced, reference); + if !is_synthetic { + let referenced = ReferenceId::Struct(struct_type.borrow().id); + let reference = ReferenceId::Variable( + Location::new(unresolved_span, self.file), + is_self_type_name, + ); + 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 cbe9f3eb7fe..5b553b55257 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -343,7 +343,8 @@ impl DefCollector { let file_id = current_def_map.file_id(module_id); for (referenced, ident) in references.iter().zip(&collected_import.path.segments) { - let reference = ReferenceId::Variable(Location::new(ident.span(), file_id)); + let reference = + ReferenceId::Variable(Location::new(ident.span(), file_id), false); context.def_interner.add_reference(*referenced, reference); } @@ -512,15 +513,15 @@ fn add_import_reference( match def_id { crate::macros_api::ModuleDefId::FunctionId(func_id) => { - let variable = ReferenceId::Variable(Location::new(name.span(), file_id)); + let variable = ReferenceId::Variable(Location::new(name.span(), file_id), false); interner.add_reference(ReferenceId::Function(func_id), variable); } crate::macros_api::ModuleDefId::TypeId(struct_id) => { - let variable = ReferenceId::Variable(Location::new(name.span(), file_id)); + let variable = ReferenceId::Variable(Location::new(name.span(), file_id), false); interner.add_reference(ReferenceId::Struct(struct_id), variable); } crate::macros_api::ModuleDefId::TraitId(trait_id) => { - let variable = ReferenceId::Variable(Location::new(name.span(), file_id)); + let variable = ReferenceId::Variable(Location::new(name.span(), file_id), false); interner.add_reference(ReferenceId::Trait(trait_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 138a37f4174..b94c9dca90d 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -652,7 +652,7 @@ impl<'a> ModCollector<'a> { Ok(child_mod_id) => { // Track that the "foo" in `mod foo;` points to the module "foo" let referenced = ReferenceId::Module(child_mod_id); - let reference = ReferenceId::Variable(location); + let reference = ReferenceId::Variable(location, false); context.def_interner.add_reference(referenced, reference); errors.extend(collect_defs( diff --git a/compiler/noirc_frontend/src/locations.rs b/compiler/noirc_frontend/src/locations.rs index c142d10319b..9b03cb932b2 100644 --- a/compiler/noirc_frontend/src/locations.rs +++ b/compiler/noirc_frontend/src/locations.rs @@ -37,7 +37,7 @@ impl NodeInterner { ReferenceId::Trait(id) => self.trait_location(&id), ReferenceId::Global(id) => self.get_global(id).location, ReferenceId::Alias(id) => self.get_type_alias(id).borrow().location, - ReferenceId::Variable(location) => location, + ReferenceId::Variable(location, _) => location, } } @@ -83,47 +83,62 @@ impl NodeInterner { .map(|node_index| self.reference_location(self.reference_graph[node_index])) } - // Is the given location known to this interner? - pub fn is_location_known(&self, location: Location) -> bool { - self.location_indices.get_node_from_location(location).is_some() + // Returns the `ReferenceId` that exists at a given location, if any. + pub fn reference_at_location(&self, location: Location) -> Option { + self.location_indices.get_node_from_location(location)?; + + let node_index = self.location_indices.get_node_from_location(location)?; + Some(self.reference_graph[node_index]) } // Starting at the given location, find the node referenced by it. Then, gather // all locations that reference that node, and return all of them - // (the references and optionally the reference node if `include_reference` is true). + // (the references and optionally the referenced node if `include_referencedd` is true). + // If `include_self_type_name` is true, references where "Self" is written are returned, + // otherwise they are not. // Returns `None` if the location is not known to this interner. pub fn find_all_references( &self, location: Location, - include_reference: bool, + include_referenced: bool, + include_self_type_name: bool, ) -> Option> { let node_index = self.location_indices.get_node_from_location(location)?; 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_reference) - } - - ReferenceId::Variable(_) => { + ReferenceId::Function(_) | ReferenceId::Struct(_) | ReferenceId::Trait(_) => 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(referenced_node_index, include_reference) + self.find_all_references_for_index( + referenced_node_index, + include_referenced, + include_self_type_name, + ) } }; Some(found_locations) } // Given a referenced node index, find all references to it and return their locations, optionally together - // with the reference node's location if `include_reference` is true. + // with the reference node's location if `include_referenced` is true. + // If `include_self_type_name` is true, references where "Self" is written are returned, + // otherwise they are not. fn find_all_references_for_index( &self, referenced_node_index: PetGraphIndex, - include_reference: bool, + include_referenced: bool, + include_self_type_name: bool, ) -> Vec { let id = self.reference_graph[referenced_node_index]; let mut edit_locations = Vec::new(); - if include_reference { + if include_referenced && (include_self_type_name || !id.is_self_type_name()) { edit_locations.push(self.reference_location(id)); } @@ -131,7 +146,9 @@ impl NodeInterner { .neighbors_directed(referenced_node_index, petgraph::Direction::Incoming) .for_each(|reference_node_index| { let id = self.reference_graph[reference_node_index]; - edit_locations.push(self.reference_location(id)); + if include_self_type_name || !id.is_self_type_name() { + edit_locations.push(self.reference_location(id)); + } }); edit_locations } diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 76a67e3977c..fd63ef66795 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -251,7 +251,13 @@ pub enum ReferenceId { Global(GlobalId), Function(FuncId), Alias(TypeAliasId), - Variable(Location), + Variable(Location, bool /* is Self */), +} + +impl ReferenceId { + pub fn is_self_type_name(&self) -> bool { + matches!(self, Self::Variable(_, true)) + } } /// A trait implementation is either a normal implementation that is present in the source diff --git a/tooling/lsp/src/requests/references.rs b/tooling/lsp/src/requests/references.rs index d35ec4a86d8..f8c23632936 100644 --- a/tooling/lsp/src/requests/references.rs +++ b/tooling/lsp/src/requests/references.rs @@ -13,7 +13,7 @@ pub(crate) fn on_references_request( ) -> impl Future>, ResponseError>> { let result = process_request(state, params.text_document_position, |location, interner, files| { - interner.find_all_references(location, params.context.include_declaration).map( + interner.find_all_references(location, params.context.include_declaration, true).map( |locations| { locations .iter() diff --git a/tooling/lsp/src/requests/rename.rs b/tooling/lsp/src/requests/rename.rs index 67853c12b81..0664c5f0667 100644 --- a/tooling/lsp/src/requests/rename.rs +++ b/tooling/lsp/src/requests/rename.rs @@ -7,6 +7,7 @@ use async_lsp::ResponseError; use lsp_types::{ PrepareRenameResponse, RenameParams, TextDocumentPositionParams, TextEdit, Url, WorkspaceEdit, }; +use noirc_frontend::node_interner::ReferenceId; use crate::LspState; @@ -17,7 +18,13 @@ pub(crate) fn on_prepare_rename_request( params: TextDocumentPositionParams, ) -> impl Future, ResponseError>> { let result = process_request(state, params, |location, interner, _| { - let rename_possible = interner.is_location_known(location); + let reference_id = interner.reference_at_location(location); + let rename_possible = match reference_id { + // Rename shouldn't be possible when triggered on top of "Self" + Some(ReferenceId::Variable(_, true /* is self type name */)) => false, + Some(_) => true, + None => false, + }; Some(PrepareRenameResponse::DefaultBehavior { default_behavior: rename_possible }) }); future::ready(result) @@ -29,29 +36,30 @@ pub(crate) fn on_rename_request( ) -> impl Future, ResponseError>> { let result = process_request(state, params.text_document_position, |location, interner, files| { - let rename_changes = interner.find_all_references(location, true).map(|locations| { - let rs = locations.iter().fold( - HashMap::new(), - |mut acc: HashMap>, location| { - let file_id = location.file; - let span = location.span; - - let Some(lsp_location) = to_lsp_location(files, file_id, span) else { - return acc; - }; - - let edit = TextEdit { - range: lsp_location.range, - new_text: params.new_name.clone(), - }; - - acc.entry(lsp_location.uri).or_default().push(edit); - - acc - }, - ); - rs - }); + let rename_changes = + interner.find_all_references(location, true, false).map(|locations| { + let rs = locations.iter().fold( + HashMap::new(), + |mut acc: HashMap>, location| { + let file_id = location.file; + let span = location.span; + + let Some(lsp_location) = to_lsp_location(files, file_id, span) else { + return acc; + }; + + let edit = TextEdit { + range: lsp_location.range, + new_text: params.new_name.clone(), + }; + + acc.entry(lsp_location.uri).or_default().push(edit); + + acc + }, + ); + rs + }); let response = WorkspaceEdit { changes: rename_changes, @@ -103,12 +111,19 @@ mod rename_tests { let mut changes: Vec = changes.values().flatten().map(|edit| edit.range).collect(); changes.sort_by_key(|range| (range.start.line, range.start.character)); + if changes != ranges { + let extra_in_changes: Vec<_> = + changes.iter().filter(|range| !ranges.contains(range)).collect(); + let extra_in_ranges: Vec<_> = + ranges.iter().filter(|range| !changes.contains(range)).collect(); + panic!("Rename locations did not match.\nThese renames were not found: {:?}\nThese renames should not have been found: {:?}", extra_in_ranges, extra_in_changes); + } assert_eq!(changes, ranges); } } #[test] - async fn test_on_prepare_rename_request_cannot_be_applied() { + async fn test_on_prepare_rename_request_cannot_be_applied_if_there_are_no_matches() { let (mut state, noir_text_document) = test_utils::init_lsp_server("rename_function").await; let params = TextDocumentPositionParams { @@ -126,6 +141,25 @@ mod rename_tests { ); } + #[test] + async fn test_on_prepare_rename_request_cannot_be_applied_on_self_type_name() { + let (mut state, noir_text_document) = test_utils::init_lsp_server("rename_struct").await; + + let params = TextDocumentPositionParams { + text_document: lsp_types::TextDocumentIdentifier { uri: noir_text_document }, + position: lsp_types::Position { line: 11, character: 24 }, // At "Self" + }; + + let response = on_prepare_rename_request(&mut state, params) + .await + .expect("Could not execute on_prepare_rename_request"); + + assert_eq!( + response, + Some(PrepareRenameResponse::DefaultBehavior { default_behavior: false }) + ); + } + #[test] async fn test_rename_function() { check_rename_succeeds("rename_function", "another_function").await; diff --git a/tooling/lsp/test_programs/rename_struct/src/main.nr b/tooling/lsp/test_programs/rename_struct/src/main.nr index 93a0779cf3b..d9da3e75763 100644 --- a/tooling/lsp/test_programs/rename_struct/src/main.nr +++ b/tooling/lsp/test_programs/rename_struct/src/main.nr @@ -6,6 +6,12 @@ mod foo { impl Foo { fn foo() {} + + fn bar(self) {} + + fn baz() -> Self { + Self { field: 1 } + } } } }