diff --git a/Cargo.lock b/Cargo.lock index 37376ad7c80..0233e1c6f2d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2919,6 +2919,7 @@ dependencies = [ "num-bigint", "num-traits", "petgraph", + "rangemap", "regex", "rustc-hash", "serde", @@ -3566,6 +3567,12 @@ dependencies = [ "rand_core 0.6.4", ] +[[package]] +name = "rangemap" +version = "1.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "977b1e897f9d764566891689e642653e5ed90c6895106acd005eb4c1d0203991" + [[package]] name = "rayon" version = "1.8.0" diff --git a/compiler/noirc_frontend/Cargo.toml b/compiler/noirc_frontend/Cargo.toml index cc5a9b1e652..052d2c5f484 100644 --- a/compiler/noirc_frontend/Cargo.toml +++ b/compiler/noirc_frontend/Cargo.toml @@ -29,6 +29,7 @@ regex = "1.9.1" cfg-if = "1.0.0" tracing.workspace = true petgraph = "0.6" +rangemap = "1.4.0" lalrpop-util = { version = "0.20.2", features = ["lexer"] } diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 4f04f5c523c..58a7a57bb3f 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -14,7 +14,7 @@ use crate::{ stmt::HirPattern, }, macros_api::{HirExpression, Ident, Path, Pattern}, - node_interner::{DefinitionId, DefinitionKind, ExprId, GlobalId, TraitImplKind}, + node_interner::{DefinitionId, DefinitionKind, DependencyId, ExprId, GlobalId, TraitImplKind}, Shared, StructType, Type, TypeBindings, }; @@ -418,10 +418,14 @@ impl<'context> Elaborator<'context> { if hir_ident.id != DefinitionId::dummy_id() { match self.interner.definition(hir_ident.id).kind { - DefinitionKind::Function(id) => { + DefinitionKind::Function(func_id) => { if let Some(current_item) = self.current_item { - self.interner.add_function_dependency(current_item, id); + self.interner.add_function_dependency(current_item, func_id); } + + let variable = DependencyId::Variable(hir_ident.location); + let function = DependencyId::Function(func_id); + self.interner.add_reference(function, variable); } DefinitionKind::Global(global_id) => { if let Some(global) = self.unresolved_globals.remove(&global_id) { @@ -430,6 +434,10 @@ impl<'context> Elaborator<'context> { if let Some(current_item) = self.current_item { self.interner.add_global_dependency(current_item, global_id); } + + let variable = DependencyId::Variable(hir_ident.location); + let global = DependencyId::Global(global_id); + self.interner.add_reference(global, variable); } DefinitionKind::GenericType(_) => { // Initialize numeric generics to a polymorphic integer type in case @@ -575,7 +583,10 @@ impl<'context> Elaborator<'context> { } pub fn get_ident_from_path(&mut self, path: Path) -> (HirIdent, usize) { - let location = Location::new(path.span(), self.file); + let location = Location::new( + path.segments.last().expect("ice: path without segments").span(), + self.file, + ); let error = match path.as_ident().map(|ident| self.use_variable(ident)) { Some(Ok(found)) => return found, 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 e6a98d9264d..6858e10a175 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -20,17 +20,18 @@ use crate::hir::Context; use crate::macros_api::{MacroError, MacroProcessor}; use crate::node_interner::{ - FuncId, GlobalId, NodeInterner, StructId, TraitId, TraitImplId, TypeAliasId, + DependencyId, FuncId, GlobalId, NodeInterner, StructId, TraitId, TraitImplId, TypeAliasId, }; use crate::ast::{ ExpressionKind, Ident, LetStatement, Literal, NoirFunction, NoirStruct, NoirTrait, NoirTypeAlias, Path, PathKind, UnresolvedGenerics, UnresolvedTraitConstraint, UnresolvedType, }; + use crate::parser::{ParserError, SortedModule}; use fm::FileId; use iter_extended::vecmap; -use noirc_errors::{CustomDiagnostic, Span}; +use noirc_errors::{CustomDiagnostic, Location, Span}; use rustc_hash::FxHashMap as HashMap; use std::collections::BTreeMap; @@ -328,6 +329,7 @@ impl DefCollector { // Resolve unresolved imports collected from the crate, one by one. for collected_import in std::mem::take(&mut def_collector.imports) { + let module_id = collected_import.module_id; match resolve_import(crate_id, &collected_import, &context.def_maps) { Ok(resolved_import) => { if let Some(error) = resolved_import.error { @@ -345,6 +347,9 @@ impl DefCollector { let result = current_def_map.modules[resolved_import.module_scope.0] .import(name.clone(), ns, resolved_import.is_prelude); + let file_id = current_def_map.file_id(module_id); + add_import_reference(ns, &name, &mut context.def_interner, file_id); + if let Err((first_def, second_def)) = result { let err = DefCollectorErrorKind::Duplicate { typ: DuplicateType::Import, @@ -468,6 +473,22 @@ impl DefCollector { } } +fn add_import_reference( + def_id: crate::macros_api::ModuleDefId, + name: &Ident, + interner: &mut NodeInterner, + file_id: FileId, +) { + if name.span() == Span::empty(0) { + // We ignore empty spans at 0 location, this must be Stdlib + return; + } + if let crate::macros_api::ModuleDefId::FunctionId(func_id) = def_id { + let variable = DependencyId::Variable(Location::new(name.span(), file_id)); + interner.add_reference_for(DependencyId::Function(func_id), variable); + } +} + fn inject_prelude( crate_id: CrateId, context: &Context, 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 3a54771dee8..aebc649b7b2 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -415,6 +415,7 @@ impl<'a> ModCollector<'a> { let func_id = context.def_interner.push_empty_fn(); method_ids.insert(name.to_string(), func_id); + let location = Location::new(name.span(), self.file_id); let modifiers = FunctionModifiers { name: name.to_string(), visibility: ItemVisibility::Public, @@ -423,9 +424,9 @@ impl<'a> ModCollector<'a> { is_unconstrained: false, generic_count: generics.len(), is_comptime: false, + name_location: location, }; - let location = Location::new(name.span(), self.file_id); context .def_interner .push_function_definition(func_id, modifiers, trait_id.0, location); diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 6d547aaf0b7..2eb33f7603f 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -1512,7 +1512,7 @@ impl<'a> Resolver<'a> { // 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 (hir_ident, var_scope_index) = self.get_ident_from_path(path); + let (hir_ident, var_scope_index) = self.get_ident_from_path(path.clone()); if hir_ident.id != DefinitionId::dummy_id() { match self.interner.definition(hir_ident.id).kind { diff --git a/compiler/noirc_frontend/src/lib.rs b/compiler/noirc_frontend/src/lib.rs index b05c635f436..b14f65a3e35 100644 --- a/compiler/noirc_frontend/src/lib.rs +++ b/compiler/noirc_frontend/src/lib.rs @@ -15,6 +15,7 @@ pub mod debug; pub mod elaborator; pub mod graph; pub mod lexer; +pub mod locations; pub mod monomorphization; pub mod node_interner; pub mod parser; diff --git a/compiler/noirc_frontend/src/locations.rs b/compiler/noirc_frontend/src/locations.rs new file mode 100644 index 00000000000..dd6a3412a40 --- /dev/null +++ b/compiler/noirc_frontend/src/locations.rs @@ -0,0 +1,122 @@ +use fm::FileId; +use noirc_errors::Location; +use rangemap::RangeMap; +use rustc_hash::FxHashMap; + +use crate::{macros_api::NodeInterner, node_interner::DependencyId}; +use petgraph::prelude::NodeIndex as PetGraphIndex; + +#[derive(Debug, Default)] +pub(crate) struct LocationIndices { + map_file_to_range: FxHashMap>, +} + +impl LocationIndices { + pub(crate) fn add_location(&mut self, location: Location, node_index: PetGraphIndex) { + // Some location spans are empty: maybe they are from ficticious nodes? + if location.span.start() == location.span.end() { + return; + } + + let range_map = self.map_file_to_range.entry(location.file).or_default(); + range_map.insert(location.span.start()..location.span.end(), node_index); + } + + pub(crate) fn get_node_from_location(&self, location: Location) -> Option { + let range_map = self.map_file_to_range.get(&location.file)?; + Some(*range_map.get(&location.span.start())?) + } +} + +impl NodeInterner { + pub fn dependency_location(&self, dependency: DependencyId) -> Location { + match dependency { + DependencyId::Function(id) => self.function_modifiers(&id).name_location, + DependencyId::Struct(id) => self.get_struct(id).borrow().location, + DependencyId::Global(id) => self.get_global(id).location, + DependencyId::Alias(id) => self.get_type_alias(id).borrow().location, + DependencyId::Variable(location) => location, + } + } + + pub(crate) fn add_reference(&mut self, referenced: DependencyId, reference: DependencyId) { + let referenced_index = self.get_or_insert_reference(referenced); + let reference_index = self.reference_graph.add_node(reference); + + let referenced_location = self.dependency_location(referenced); + let reference_location = self.dependency_location(reference); + + self.reference_graph.add_edge(referenced_index, reference_index, ()); + self.location_indices.add_location(referenced_location, referenced_index); + self.location_indices.add_location(reference_location, reference_index); + } + + pub(crate) fn add_reference_for( + &mut self, + referenced_id: DependencyId, + reference: DependencyId, + ) { + let Some(referenced_index) = self.reference_graph_indices.get(&referenced_id) else { + panic!("Compiler Error: Referenced index not found") + }; + + let reference_location = self.dependency_location(reference); + let reference_index = self.reference_graph.add_node(reference); + self.reference_graph.add_edge(*referenced_index, reference_index, ()); + self.location_indices.add_location(reference_location, reference_index); + } + + pub(crate) fn add_definition_location(&mut self, referenced: DependencyId) { + let referenced_index = self.get_or_insert_reference(referenced); + let referenced_location = self.dependency_location(referenced); + self.location_indices.add_location(referenced_location, referenced_index); + } + + #[tracing::instrument(skip(self), ret)] + pub(crate) fn get_or_insert_reference(&mut self, id: DependencyId) -> PetGraphIndex { + if let Some(index) = self.reference_graph_indices.get(&id) { + return *index; + } + + let index = self.reference_graph.add_node(id); + self.reference_graph_indices.insert(id, index); + index + } + + pub fn check_rename_possible(&self, location: Location) -> bool { + self.location_indices.get_node_from_location(location).is_some() + } + + pub fn find_rename_symbols_at(&self, location: Location) -> 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 { + DependencyId::Alias(_) | DependencyId::Struct(_) | DependencyId::Global(_) => todo!(), + DependencyId::Function(_) => self.get_edit_locations(node_index), + + DependencyId::Variable(_) => { + let referenced_node_index = self + .reference_graph + .neighbors_directed(node_index, petgraph::Direction::Incoming) + .next()?; + + self.get_edit_locations(referenced_node_index) + } + }; + Some(found_locations) + } + + fn get_edit_locations(&self, referenced_node_index: PetGraphIndex) -> Vec { + let id = self.reference_graph[referenced_node_index]; + let mut edit_locations = vec![self.dependency_location(id)]; + + self.reference_graph + .neighbors_directed(referenced_node_index, petgraph::Direction::Outgoing) + .for_each(|reference_node_index| { + let id = self.reference_graph[reference_node_index]; + edit_locations.push(self.dependency_location(id)); + }); + edit_locations + } +} diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 17531d09eac..2a4b4962e74 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -31,6 +31,7 @@ use crate::hir_def::{ function::{FuncMeta, HirFunction}, stmt::HirStatement, }; +use crate::locations::LocationIndices; use crate::token::{Attributes, SecondaryAttribute}; use crate::GenericTypeVars; use crate::Generics; @@ -64,7 +65,7 @@ pub struct NodeInterner { function_modules: HashMap, /// This graph tracks dependencies between different global definitions. - /// This is used to ensure the absense of dependency cycles for globals and types. + /// This is used to ensure the absence of dependency cycles for globals and types. dependency_graph: DiGraph, /// To keep track of where each DependencyId is in `dependency_graph`, we need @@ -182,6 +183,15 @@ pub struct NodeInterner { /// and creating a `Token::QuotedType(id)` from this id. We cannot create a token holding /// the actual type since types do not implement Send or Sync. quoted_types: noirc_arena::Arena, + + /// Store the location of the references in the graph + pub(crate) reference_graph: DiGraph, + + /// Tracks the index of the references in the graph + pub(crate) reference_graph_indices: HashMap, + + /// Store the location of the references in the graph + pub(crate) location_indices: LocationIndices, } /// A dependency in the dependency graph may be a type or a definition. @@ -200,6 +210,7 @@ pub enum DependencyId { Global(GlobalId), Function(FuncId), Alias(TypeAliasId), + Variable(Location), } /// A trait implementation is either a normal implementation that is present in the source @@ -258,6 +269,9 @@ pub struct FunctionModifiers { pub generic_count: usize, pub is_comptime: bool, + + /// The location of the function's name rather than the entire function + pub name_location: Location, } impl FunctionModifiers { @@ -272,6 +286,7 @@ impl FunctionModifiers { is_unconstrained: false, generic_count: 0, is_comptime: false, + name_location: Location::dummy(), } } } @@ -516,6 +531,9 @@ impl Default for NodeInterner { type_alias_ref: Vec::new(), type_ref_locations: Vec::new(), quoted_types: Default::default(), + location_indices: LocationIndices::default(), + reference_graph: petgraph::graph::DiGraph::new(), + reference_graph_indices: HashMap::new(), }; // An empty block expression is used often, we add this into the `node` on startup @@ -804,8 +822,14 @@ impl NodeInterner { is_unconstrained: function.is_unconstrained, generic_count: function.generics.len(), is_comptime: function.is_comptime, + name_location: Location::new(function.name.span(), location.file), }; - self.push_function_definition(id, modifiers, module, location) + let definition_id = self.push_function_definition(id, modifiers, module, location); + + // This needs to be done after pushing the definition since it will reference the + // location that was stored + self.add_definition_location(DependencyId::Function(id)); + definition_id } pub fn push_function_definition( @@ -1664,13 +1688,13 @@ impl NodeInterner { self.add_dependency(dependent, DependencyId::Alias(dependency)); } - fn add_dependency(&mut self, dependent: DependencyId, dependency: DependencyId) { + pub fn add_dependency(&mut self, dependent: DependencyId, dependency: DependencyId) { let dependent_index = self.get_or_insert_dependency(dependent); let dependency_index = self.get_or_insert_dependency(dependency); self.dependency_graph.update_edge(dependent_index, dependency_index, ()); } - fn get_or_insert_dependency(&mut self, id: DependencyId) -> PetGraphIndex { + pub fn get_or_insert_dependency(&mut self, id: DependencyId) -> PetGraphIndex { if let Some(index) = self.dependency_graph_indices.get(&id) { return *index; } @@ -1721,6 +1745,11 @@ impl NodeInterner { } // Mutually recursive functions are allowed DependencyId::Function(_) => (), + // Local variables should never be in a dependency cycle, scoping rules + // prevents referring to them before they're defined + DependencyId::Variable(loc) => unreachable!( + "Variable used at location {loc:?} caught in a dependency cycle" + ), } } } @@ -1742,6 +1771,9 @@ impl NodeInterner { DependencyId::Global(id) => { Cow::Borrowed(self.get_global(id).ident.0.contents.as_ref()) } + DependencyId::Variable(loc) => { + unreachable!("Variable used at location {loc:?} caught in a dependency cycle") + } }; let mut cycle = index_to_string(scc[start_index]).to_string(); diff --git a/cspell.json b/cspell.json index 2fb20ae2ba4..b3a39108f24 100644 --- a/cspell.json +++ b/cspell.json @@ -150,6 +150,7 @@ "nouner", "pedersen", "peekable", + "petgraph", "plonkc", "PLONKish", "pprof", @@ -161,6 +162,7 @@ "pseudocode", "pubkey", "quantile", + "rangemap", "repr", "reqwest", "rfind", @@ -185,6 +187,7 @@ "subtyping", "swcurve", "Taiko", + "tarjan", "tecurve", "tempdir", "tempfile", @@ -203,6 +206,7 @@ "urem", "USERPROFILE", "vecmap", + "vitkov", "wasi", "wasmer", "Weierstraß", diff --git a/tooling/lsp/src/lib.rs b/tooling/lsp/src/lib.rs index 304a2d34e47..92924e701a6 100644 --- a/tooling/lsp/src/lib.rs +++ b/tooling/lsp/src/lib.rs @@ -20,7 +20,10 @@ use async_lsp::{ }; use fm::{codespan_files as files, FileManager}; use fxhash::FxHashSet; -use lsp_types::CodeLens; +use lsp_types::{ + request::{PrepareRenameRequest, Rename}, + CodeLens, +}; use nargo::{ package::{Package, PackageType}, parse_all, @@ -43,8 +46,8 @@ use notifications::{ }; use requests::{ on_code_lens_request, on_formatting, on_goto_declaration_request, on_goto_definition_request, - on_goto_type_definition_request, on_initialize, on_profile_run_request, on_shutdown, - on_test_run_request, on_tests_request, + on_goto_type_definition_request, on_initialize, on_prepare_rename_request, + on_profile_run_request, on_rename_request, on_shutdown, on_test_run_request, on_tests_request, }; use serde_json::Value as JsonValue; use thiserror::Error; @@ -55,6 +58,9 @@ mod requests; mod solver; mod types; +#[cfg(test)] +mod test_utils; + use solver::WrapperSolver; use types::{notification, request, NargoTest, NargoTestId, Position, Range, Url}; @@ -119,6 +125,8 @@ impl NargoLspService { .request::(on_goto_definition_request) .request::(on_goto_declaration_request) .request::(on_goto_type_definition_request) + .request::(on_prepare_rename_request) + .request::(on_rename_request) .notification::(on_initialized) .notification::(on_did_change_configuration) .notification::(on_did_open_text_document) diff --git a/tooling/lsp/src/requests/goto_definition.rs b/tooling/lsp/src/requests/goto_definition.rs index 3a92e28cc11..4985c565e06 100644 --- a/tooling/lsp/src/requests/goto_definition.rs +++ b/tooling/lsp/src/requests/goto_definition.rs @@ -76,59 +76,42 @@ fn on_goto_definition_inner( #[cfg(test)] mod goto_definition_tests { + use std::panic; - use acvm::blackbox_solver::StubbedBlackBoxSolver; - use async_lsp::ClientSocket; - use lsp_types::{Position, Url}; + use crate::test_utils; + use lsp_types::{Position, Range}; use tokio::test; use super::*; #[test] async fn test_on_goto_definition() { - let client = ClientSocket::new_closed(); - let mut state = LspState::new(&client, StubbedBlackBoxSolver); - - let root_path = std::env::current_dir() - .unwrap() - .join("../../test_programs/execution_success/7_function") - .canonicalize() - .expect("Could not resolve root path"); - let noir_text_document = Url::from_file_path(root_path.join("src/main.nr").as_path()) - .expect("Could not convert text document path to URI"); - let root_uri = Some( - Url::from_file_path(root_path.as_path()).expect("Could not convert root path to URI"), - ); - - #[allow(deprecated)] - let initialize_params = lsp_types::InitializeParams { - process_id: Default::default(), - root_path: None, - root_uri, - initialization_options: None, - capabilities: Default::default(), - trace: Some(lsp_types::TraceValue::Verbose), - workspace_folders: None, - client_info: None, - locale: None, - }; - let _initialize_response = crate::requests::on_initialize(&mut state, initialize_params) - .await - .expect("Could not initialize LSP server"); + let (mut state, noir_text_document) = test_utils::init_lsp_server("go_to_definition").await; let params = GotoDefinitionParams { text_document_position_params: lsp_types::TextDocumentPositionParams { text_document: lsp_types::TextDocumentIdentifier { uri: noir_text_document }, - position: Position { line: 95, character: 5 }, + position: Position { line: 9, character: 12 }, // Right at the beginning of "another_function" }, work_done_progress_params: Default::default(), partial_result_params: Default::default(), }; - let response = on_goto_definition_request(&mut state, params) + let response: GotoDefinitionResponse = on_goto_definition_request(&mut state, params) .await - .expect("Could execute on_goto_definition_request"); - - assert!(&response.is_some()); + .expect("Could execute on_goto_definition_request") + .expect("Didn't get a goto definition response"); + + if let GotoDefinitionResponse::Scalar(location) = response { + assert_eq!( + location.range, + Range { + start: Position { line: 4, character: 3 }, + end: Position { line: 4, character: 19 }, + } + ); + } else { + panic!("Expected a scalar response"); + }; } } diff --git a/tooling/lsp/src/requests/mod.rs b/tooling/lsp/src/requests/mod.rs index 769e9ba17ce..545b5fef3d2 100644 --- a/tooling/lsp/src/requests/mod.rs +++ b/tooling/lsp/src/requests/mod.rs @@ -5,7 +5,7 @@ use async_lsp::{ErrorCode, ResponseError}; use fm::{codespan_files::Error, FileMap, PathString}; use lsp_types::{ DeclarationCapability, Location, Position, TextDocumentSyncCapability, TextDocumentSyncKind, - TypeDefinitionProviderCapability, Url, + TypeDefinitionProviderCapability, Url, WorkDoneProgressOptions, }; use nargo_fmt::Config; use serde::{Deserialize, Serialize}; @@ -29,6 +29,7 @@ mod code_lens_request; mod goto_declaration; mod goto_definition; mod profile_run; +mod rename; mod test_run; mod tests; @@ -36,7 +37,8 @@ pub(crate) use { code_lens_request::collect_lenses_for_package, code_lens_request::on_code_lens_request, goto_declaration::on_goto_declaration_request, goto_definition::on_goto_definition_request, goto_definition::on_goto_type_definition_request, profile_run::on_profile_run_request, - test_run::on_test_run_request, tests::on_tests_request, + rename::on_prepare_rename_request, rename::on_rename_request, test_run::on_test_run_request, + tests::on_tests_request, }; /// LSP client will send initialization request after the server has started. @@ -106,6 +108,12 @@ pub(crate) fn on_initialize( definition_provider: Some(lsp_types::OneOf::Left(true)), declaration_provider: Some(DeclarationCapability::Simple(true)), type_definition_provider: Some(TypeDefinitionProviderCapability::Simple(true)), + rename_provider: Some(lsp_types::OneOf::Right(lsp_types::RenameOptions { + prepare_provider: Some(true), + work_done_progress_options: WorkDoneProgressOptions { + work_done_progress: None, + }, + })), }, server_info: None, }) diff --git a/tooling/lsp/src/requests/rename.rs b/tooling/lsp/src/requests/rename.rs new file mode 100644 index 00000000000..9f6416a2c63 --- /dev/null +++ b/tooling/lsp/src/requests/rename.rs @@ -0,0 +1,249 @@ +use std::{ + collections::HashMap, + future::{self, Future}, +}; + +use async_lsp::{ErrorCode, ResponseError}; +use fm::FileMap; +use lsp_types::{ + PrepareRenameResponse, RenameParams, TextDocumentPositionParams, TextEdit, Url, WorkspaceEdit, +}; +use nargo::insert_all_files_for_workspace_into_file_manager; +use noirc_driver::file_manager_with_stdlib; +use noirc_errors::Location; +use noirc_frontend::macros_api::NodeInterner; + +use crate::{parse_diff, resolve_workspace_for_source_path, LspState}; + +use super::{position_to_byte_index, to_lsp_location}; + +pub(crate) fn on_prepare_rename_request( + state: &mut LspState, + params: TextDocumentPositionParams, +) -> impl Future, ResponseError>> { + let result = process_rename_request(state, params, |search_for_location, interner, _| { + let rename_possible = interner.check_rename_possible(search_for_location); + Some(PrepareRenameResponse::DefaultBehavior { default_behavior: rename_possible }) + }); + future::ready(result) +} + +pub(crate) fn on_rename_request( + state: &mut LspState, + params: RenameParams, +) -> impl Future, ResponseError>> { + let result = process_rename_request( + state, + params.text_document_position, + |search_for_location, interner, files| { + let rename_changes = + interner.find_rename_symbols_at(search_for_location).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, + document_changes: None, + change_annotations: None, + }; + + Some(response) + }, + ); + future::ready(result) +} + +fn process_rename_request( + state: &mut LspState, + text_document_position_params: TextDocumentPositionParams, + callback: F, +) -> Result +where + F: FnOnce(Location, &NodeInterner, &FileMap) -> T, +{ + let file_path = + text_document_position_params.text_document.uri.to_file_path().map_err(|_| { + ResponseError::new(ErrorCode::REQUEST_FAILED, "URI is not a valid file path") + })?; + + let workspace = resolve_workspace_for_source_path(file_path.as_path()).unwrap(); + let package = workspace.members.first().unwrap(); + + let package_root_path: String = package.root_dir.as_os_str().to_string_lossy().into(); + + let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); + insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); + let parsed_files = parse_diff(&workspace_file_manager, state); + + let (mut context, crate_id) = + nargo::prepare_package(&workspace_file_manager, &parsed_files, package); + + let interner; + if let Some(def_interner) = state.cached_definitions.get(&package_root_path) { + interner = def_interner; + } else { + // We ignore the warnings and errors produced by compilation while resolving the definition + let _ = noirc_driver::check_crate(&mut context, crate_id, false, false, false); + interner = &context.def_interner; + } + + let files = context.file_manager.as_file_map(); + let file_id = context.file_manager.name_to_id(file_path.clone()).ok_or(ResponseError::new( + ErrorCode::REQUEST_FAILED, + format!("Could not find file in file manager. File path: {:?}", file_path), + ))?; + let byte_index = + position_to_byte_index(files, file_id, &text_document_position_params.position).map_err( + |err| { + ResponseError::new( + ErrorCode::REQUEST_FAILED, + format!("Could not convert position to byte index. Error: {:?}", err), + ) + }, + )?; + + let search_for_location = noirc_errors::Location { + file: file_id, + span: noirc_errors::Span::single_char(byte_index as u32), + }; + + Ok(callback(search_for_location, interner, files)) +} + +#[cfg(test)] +mod rename_tests { + use super::*; + use crate::test_utils; + use lsp_types::{Position, Range, WorkDoneProgressParams}; + use tokio::test; + + async fn check_rename_succeeds(directory: &str, name: &str, ranges: &[Range]) { + let (mut state, noir_text_document) = test_utils::init_lsp_server(directory).await; + + let main_path = noir_text_document.path(); + + // As we process the rename requests we'll check that the request position actually + // includes the target name. + let file_contents = std::fs::read_to_string(main_path) + .unwrap_or_else(|_| panic!("Couldn't read file {}", main_path)); + + let file_lines: Vec<&str> = file_contents.lines().collect(); + + // Test renaming works on any instance of the symbol. + for target_range in ranges { + assert_eq!(target_range.start.line, target_range.end.line); + + // Check that the range includes the target name + let line = file_lines[target_range.start.line as usize]; + let chunk = + &line[target_range.start.character as usize..target_range.end.character as usize]; + assert_eq!(chunk, name); + + let target_position = target_range.start; + + let params = RenameParams { + text_document_position: TextDocumentPositionParams { + text_document: lsp_types::TextDocumentIdentifier { + uri: noir_text_document.clone(), + }, + position: target_position, + }, + new_name: "renamed_function".to_string(), + work_done_progress_params: WorkDoneProgressParams { work_done_token: None }, + }; + + let response = on_rename_request(&mut state, params) + .await + .expect("Could not execute on_prepare_rename_request") + .unwrap(); + + let changes = response.changes.expect("Expected to find rename changes"); + let mut changes: Vec = + changes.values().flatten().map(|edit| edit.range).collect(); + changes.sort_by_key(|range| range.start.line); + assert_eq!(changes, ranges); + } + } + + #[test] + async fn test_on_prepare_rename_request_cannot_be_applied() { + let (mut state, noir_text_document) = test_utils::init_lsp_server("rename").await; + + let params = TextDocumentPositionParams { + text_document: lsp_types::TextDocumentIdentifier { uri: noir_text_document }, + position: lsp_types::Position { line: 0, character: 0 }, // This is at the "f" of an "fn" keyword + }; + + 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_on_rename_request() { + const ANOTHER_FUNCTION_REFERENCE: Range = Range { + start: Position { line: 9, character: 12 }, + end: Position { line: 9, character: 28 }, + }; + const ANOTHER_FUNCTION_DECLARATION: Range = Range { + start: Position { line: 4, character: 3 }, + end: Position { line: 4, character: 19 }, + }; + // The ranges of positions which represent the usage of the `another_function` symbol. + const ANOTHER_FUNCTION_RANGES: &[Range] = &[ + ANOTHER_FUNCTION_DECLARATION, + ANOTHER_FUNCTION_REFERENCE, + Range { + start: Position { line: 13, character: 12 }, + end: Position { line: 13, character: 28 }, + }, + Range { + start: Position { line: 19, character: 15 }, + end: Position { line: 19, character: 31 }, + }, + ]; + + check_rename_succeeds("rename", "another_function", ANOTHER_FUNCTION_RANGES).await; + } + + #[test] + async fn test_on_rename_request_works_with_qualified_path() { + const BAR_FUNCTION_REFERENCE: Range = Range { + start: Position { line: 1, character: 9 }, + end: Position { line: 1, character: 12 }, + }; + const BAR_FUNCTION_DECLARATION: Range = Range { + start: Position { line: 5, character: 11 }, + end: Position { line: 5, character: 14 }, + }; + // The ranges of positions which represent the usage of the `bar` symbol. + const BAR_FUNCTION_RANGES: &[Range] = &[BAR_FUNCTION_REFERENCE, BAR_FUNCTION_DECLARATION]; + + check_rename_succeeds("rename_qualified", "bar", BAR_FUNCTION_RANGES).await; + } +} diff --git a/tooling/lsp/src/test_utils.rs b/tooling/lsp/src/test_utils.rs new file mode 100644 index 00000000000..dcaec2fd615 --- /dev/null +++ b/tooling/lsp/src/test_utils.rs @@ -0,0 +1,39 @@ +use crate::LspState; +use acvm::blackbox_solver::StubbedBlackBoxSolver; +use async_lsp::ClientSocket; +use lsp_types::Url; + +pub(crate) async fn init_lsp_server(directory: &str) -> (LspState, Url) { + let client = ClientSocket::new_closed(); + let mut state = LspState::new(&client, StubbedBlackBoxSolver); + + let root_path = std::env::current_dir() + .unwrap() + .join("test_programs") + .join(directory) + .canonicalize() + .expect("Could not resolve root path"); + let noir_text_document = Url::from_file_path(root_path.join("src/main.nr").as_path()) + .expect("Could not convert text document path to URI"); + let root_uri = + Some(Url::from_file_path(root_path.as_path()).expect("Could not convert root path to URI")); + + #[allow(deprecated)] + let initialize_params = lsp_types::InitializeParams { + process_id: Default::default(), + root_path: None, + root_uri, + initialization_options: None, + capabilities: Default::default(), + trace: Some(lsp_types::TraceValue::Verbose), + workspace_folders: None, + client_info: None, + locale: None, + }; + + let _initialize_response = crate::requests::on_initialize(&mut state, initialize_params) + .await + .expect("Could not initialize LSP server"); + + (state, noir_text_document) +} diff --git a/tooling/lsp/src/types.rs b/tooling/lsp/src/types.rs index e3492f21346..7239b1db685 100644 --- a/tooling/lsp/src/types.rs +++ b/tooling/lsp/src/types.rs @@ -1,6 +1,7 @@ use fm::FileId; use lsp_types::{ - DeclarationCapability, DefinitionOptions, OneOf, TypeDefinitionProviderCapability, + DeclarationCapability, DefinitionOptions, OneOf, RenameOptions, + TypeDefinitionProviderCapability, }; use noirc_driver::DebugFile; use noirc_errors::{debug_info::OpCodesCount, Location}; @@ -135,6 +136,10 @@ pub(crate) struct ServerCapabilities { /// The server handles and provides custom nargo messages. #[serde(skip_serializing_if = "Option::is_none")] pub(crate) nargo: Option, + + /// The server provides rename support. + #[serde(skip_serializing_if = "Option::is_none")] + pub(crate) rename_provider: Option>, } #[derive(Debug, PartialEq, Clone, Default, Deserialize, Serialize)] diff --git a/tooling/lsp/test_programs/go_to_definition/Nargo.toml b/tooling/lsp/test_programs/go_to_definition/Nargo.toml new file mode 100644 index 00000000000..c894a050c40 --- /dev/null +++ b/tooling/lsp/test_programs/go_to_definition/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "go_to_definition" +type = "bin" +authors = [""] + +[dependencies] diff --git a/tooling/lsp/test_programs/go_to_definition/src/main.nr b/tooling/lsp/test_programs/go_to_definition/src/main.nr new file mode 100644 index 00000000000..c27f8fed868 --- /dev/null +++ b/tooling/lsp/test_programs/go_to_definition/src/main.nr @@ -0,0 +1,11 @@ +fn some_function() -> Field { + 1 + 2 +} + +fn another_function() -> Field { + 3 + 4 +} + +fn main() { + let _ = another_function(); +} diff --git a/tooling/lsp/test_programs/rename/Nargo.toml b/tooling/lsp/test_programs/rename/Nargo.toml new file mode 100644 index 00000000000..2d5b6415dc9 --- /dev/null +++ b/tooling/lsp/test_programs/rename/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "rename" +type = "bin" +authors = [""] + +[dependencies] diff --git a/tooling/lsp/test_programs/rename/src/main.nr b/tooling/lsp/test_programs/rename/src/main.nr new file mode 100644 index 00000000000..4c28249582e --- /dev/null +++ b/tooling/lsp/test_programs/rename/src/main.nr @@ -0,0 +1,22 @@ +fn some_function() -> Field { + 1 + 2 +} + +fn another_function() -> Field { + 3 + 4 +} + +fn main() { + let _ = another_function(); + + let _ = 1; + + let _ = another_function(); +} + + +mod foo { + fn yet_another_function() -> Field { + crate::another_function() + } +} \ No newline at end of file diff --git a/tooling/lsp/test_programs/rename_qualified/Nargo.toml b/tooling/lsp/test_programs/rename_qualified/Nargo.toml new file mode 100644 index 00000000000..7de13ef6b34 --- /dev/null +++ b/tooling/lsp/test_programs/rename_qualified/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "rename_qualified" +type = "bin" +authors = [""] + +[dependencies] diff --git a/tooling/lsp/test_programs/rename_qualified/src/main.nr b/tooling/lsp/test_programs/rename_qualified/src/main.nr new file mode 100644 index 00000000000..f1b77796210 --- /dev/null +++ b/tooling/lsp/test_programs/rename_qualified/src/main.nr @@ -0,0 +1,9 @@ +fn main() -> pub Field { + foo::bar() +} + +mod foo { + pub fn bar() -> Field { + 1 + } +}