-
Notifications
You must be signed in to change notification settings - Fork 374
feat(lsp): allow function rename #4294
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
52 commits
Select commit
Hold shift + click to select a range
f941c04
feat(lsp): allow function rename
kobyhallx 8456a58
Merge remote-tracking branch 'origin/master' into kh-rename-functions
kobyhallx daf83ed
Merge branch 'master' into kh-rename-functions
kobyhallx e685934
chore: move collection point
kobyhallx c9ba252
feat: add func rename with imports
kobyhallx df38fb6
chore: cleanup
kobyhallx d73f177
Merge remote-tracking branch 'origin/master' into kh-rename-functions
kobyhallx 3cef0a4
Merge branch 'master' into kh-rename-functions
kobyhallx 39d7d56
chore: remove globals for now
kobyhallx d77236c
chore: clippy
kobyhallx 2d124bd
chore: clippy - unnecesary ret
kobyhallx 09e898f
chore: clippy - map instead and_then
kobyhallx 78913ac
Merge branch 'master' into kh-rename-functions
kobyhallx 9718234
Merge branch 'master' into kh-rename-functions
kobyhallx ab455b1
chore: review nits
kobyhallx d7cd372
chore: spells
kobyhallx 4421a2d
chore: name change
kobyhallx d564ac8
chore: name change
kobyhallx f0b81a2
Fix bug with renaming imported functions from submodules
jfecher 0b0eca7
Add unreachable cases
jfecher 7cb7bc7
Merge branch 'master' into kh-rename-functions
jfecher 33b50dc
Fix merge
jfecher 7280c9d
Merge branch 'master' into kh-rename-functions
0802086
clippy
8122dc8
Merge branch 'kh-rename-functions' of https://github.com/noir-lang/no…
a9b785c
Fmt and clippy
62c5cad
Merge branch 'master' into kh-rename-functions
TomAFrench a34e4db
Merge branch 'master' into kh-rename-functions
TomAFrench d598adf
chore: fix merge
TomAFrench 9d5577c
Merge branch 'master' into kh-rename-functions
asterite 7724b01
Add tests for lsp rename, and use the legacy resolver to make them work
asterite d18e0a9
Implement lsp rename using elaborator
asterite 52ce934
Check that matches actually match the name being renamed
asterite 4db9e6c
Use separate programs for lsp tests
asterite aee04c7
clippy
asterite 58bc455
Merge branch 'master' into kh-rename-functions
asterite 2d89ab4
Merge branch 'master' into kh-rename-functions
asterite 2d92617
Undo changes to resolver
asterite a2fc465
Don't use legacy resolver in another place either
asterite 8b5aaae
Skip empty location spans (otherwise they produce a panic)
asterite 33a695c
Merge branch 'master' into kh-rename-functions
asterite 4da3335
Remove some code duplication in lsp rename tests
asterite b9f3d76
Extract test_utils::init_lsp_server
asterite bf87231
process_rename_request only needs TextDocumentPositionParams
asterite 07b2ccb
chore: refactor tests to make it easier to check all cases
TomAFrench b8dbd04
chore: fix type
TomAFrench 684389a
chore: fmt
TomAFrench 7a2f0f7
chore: add instance of qualified path to test program
TomAFrench 7532717
Let lsp rename function work well with qualified paths
asterite 303dc9b
Remove extra tests, and move consts inside functions
asterite 641c74f
Merge branch 'master' into kh-rename-functions
asterite 777bbbf
Merge branch 'master' into kh-rename-functions
TomAFrench File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<FileId, RangeMap<u32, PetGraphIndex>>, | ||
| } | ||
|
|
||
| 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<PetGraphIndex> { | ||
| 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<Vec<Location>> { | ||
| let node_index = self.location_indices.get_node_from_location(location)?; | ||
|
|
||
| let reference_node = self.reference_graph[node_index]; | ||
| let found_locations: Vec<Location> = match reference_node { | ||
| DependencyId::Alias(_) | DependencyId::Struct(_) | DependencyId::Global(_) => todo!(), | ||
TomAFrench marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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<Location> { | ||
| 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 | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.