Use resolved paths in SSR rules #5518
Merged
Conversation
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
Previously we had: - Multiple rules - Each rule had its pattern parsed as an expression, path etc This meant that there were two levels at which there could be multiple rules. Now we just have multiple rules. If a pattern can parse as more than one kind of thing, then they get stored as multiple separate rules. We also now don't have separate fields for the different kinds of things that a pattern can parse as. This makes adding new kinds of things simpler. Previously, add_search_pattern would construct a rule with a dummy replacement. Now the replacement is an Option. This is slightly cleaner and also opens the way for parsing the replacement template as the same kind of thing as the search pattern.
This is in preparation for a subsequent commit where we add special handling for paths in the template, allowing them to be qualified differently in different contexts.
Also renamed find_matches to slow_scan_node to reflect that it's a slow way to do things. Actually the name came from a later commit and probably makes more sense once there's an alternative.
The methods `edits_for_file` and `find_matches_in_file` are replaced with just `edits` and `matches`. This simplifies the API a bit, but more importantly it makes it possible in a subsequent commit for SSR to decide to not search all files.
In a later commit, paths in templates will be resolved. This allows us to render the path with appropriate qualifiers for its context. Here we prepare for that change by updating existing tests where I'd previously not bothered to define the items that the template referred to.
These tests already pass, however once we switch to non-recursive search, it'd be easy for these tests to not pass.
Previously, submatches were handled simply by searching in placeholders for more matches. That only works if we search all nodes in the tree recursively. In a subsequent commit, I intend to make search not always be recursive recursive. This commit prepares for that by finding all matches, even if they overlap, then nesting them and removing overlapping matches.
In a subsequent commit, it will be used for resolving paths.
Also render template paths appropriately for their context.
When the search pattern contains a path, this substantially speeds up finding matches, especially if the path references a private item.
It currently does the wrong thing when the use declaration contains braces.
This differs from how this used to work before I removed it in that: a) It's only one direction. Function calls in the pattern can match method calls in the code, but not the other way around. b) We now check that the function call in the pattern resolves to the same function as the method call in the code. The lack of (b) was the reason I felt the need to remove the feature before.
matklad
reviewed
Jul 24, 2020
| Some(NameRefClass::Definition(resolved.into())) | ||
| } | ||
|
|
||
| impl From<PathResolution> for Definition { |
| if (!client) return; | ||
| if (!editor || !client) return; | ||
|
|
||
| const position = editor.selection.active; |
There was a problem hiding this comment.
I think this is OK as first solution, but we probably should move to that "ssr via assit" idea some time in the future. Currently, the association between position and action is pretty weak.
It might also be a good idea to think about how this should work in the CLI setting.
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The main user-visible changes are:
foo()won't matchfoo()if it's a different functionfoo(), but will matchbar::foo()if it's the samefoo.foo::Barwill render as justBarinside the modulefoo, but might render asbaz::foo::Barfrom elsewhere.