[SourceKit] Update related identifiers request to serve local rename #70287
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
ahoppen
requested review from
bnbarham,
rintaro,
hamishknight,
zoecarver and
CodaFi
as code owners
|
@swift-ci Please smoke test |
ahoppen
changed the title
This will allow us to get syntactic rename ranges from the locations returned by related identifiers.
ahoppen
force-pushed
the
ahoppen/related-identifiers-for-rename
branch
from
0f45f21 to
9a8e9b7
Compare
bnbarham
approved these changes
Dec 7, 2023
| RenameLocs Locs = localRenameLocs(SrcFile, Info->VD); | ||
|
|
||
| std::string OldName = Locs.getLocations().front().OldName.str(); | ||
| for (auto loc : Locs.getLocations()) { |
There was a problem hiding this comment.
Probably optimized out, but I'd surround in #ifndef NDEBUG if you want this. This would be the main case for a separate request I suppose, since ideally we wouldn't need OldName on every single returned location.
ahoppen
force-pushed
the
ahoppen/related-identifiers-for-rename
branch
from
9a8e9b7 to
bdd51c3
Compare
|
@swift-ci Please smoke test |
bnbarham
approved these changes
Dec 7, 2023
Comment on lines
2573
to
2575
| for (size_t index = 0; index < ResolvedLocs.size(); ++index) { | ||
| auto RenameLoc = Locs.getLocations()[index]; | ||
| auto ResolvedLoc = ResolvedLocs[index]; |
There was a problem hiding this comment.
LLVM has a zip in STLExtras.h. Could use llvm::zip_equal which assumes they're the same size.
…d by the related identifiers request While we don't want to return non-editable base names for Edit-All-In-Scope, we do want to return them if the related identifiers response is used as the input for find-syntactic-rename-ranges.
This allows us to at least rename the base of operators. The labels don’t matter too much since they only occur on the function definition.
ahoppen
force-pushed
the
ahoppen/related-identifiers-for-rename
branch
from
bdd51c3 to
516836c
Compare
|
@swift-ci Please smoke test |
hamishknight
approved these changes
Dec 8, 2023
Comment on lines
+2574
to
+2575
| for (auto [RenameLoc, ResolvedLoc] : | ||
| llvm::zip_equal(Locs.getLocations(), ResolvedLocs)) { |
There was a problem hiding this comment.
TIL about structured bindings, neat!
There was a problem hiding this comment.
I only found them by searching for zip_equal in the LLVM repo as well
|
@swift-ci Please smoke test macOS |
drodriguez
added a commit
to drodriguez/swift
that referenced
this pull request
Dec 15, 2023
Some of the tests now depend on features provided by Swift Syntax, and will fail to pass if building without Swift Syntax support. This commits marks all the tests I know fail to pass when Swift Syntax is disabled. - swiftlang#70281 modified `ASTGen/verify-parse.swift` - swiftlang#70287 created `refactoring/SyntacticRename/operator.swift` and `SourceKit/RelatedIdents/operator.swift` - swiftlang#70389 modified `SourceKit/Refactoring/basic.swift` and indirectly modified all `refactoring/ExtractFunction/*` tests.
drodriguez
added a commit
to drodriguez/swift
that referenced
this pull request
Dec 15, 2023
Some of the tests now depend on features provided by Swift Syntax, and will fail to pass if building without Swift Syntax support. This commits marks all the tests I know fail to pass when Swift Syntax is disabled. - swiftlang#70281 modified `ASTGen/verify-parse.swift` - swiftlang#70287 created `refactoring/SyntacticRename/operator.swift` and `SourceKit/RelatedIdents/operator.swift` - swiftlang#70389 modified `SourceKit/Refactoring/basic.swift` and indirectly modified all `refactoring/ExtractFunction/*` tests.
drodriguez
added a commit
that referenced
this pull request
Dec 15, 2023
Some of the tests now depend on features provided by Swift Syntax, and will fail to pass if building without Swift Syntax support. This commits marks all the tests I know fail to pass when Swift Syntax is disabled. - #70281 modified `ASTGen/verify-parse.swift` - #70287 created `refactoring/SyntacticRename/operator.swift` and `SourceKit/RelatedIdents/operator.swift` - #70389 modified `SourceKit/Refactoring/basic.swift` and indirectly modified all `refactoring/ExtractFunction/*` tests.
meg-gupta
pushed a commit
to meg-gupta/swift
that referenced
this pull request
Dec 21, 2023
Some of the tests now depend on features provided by Swift Syntax, and will fail to pass if building without Swift Syntax support. This commits marks all the tests I know fail to pass when Swift Syntax is disabled. - swiftlang#70281 modified `ASTGen/verify-parse.swift` - swiftlang#70287 created `refactoring/SyntacticRename/operator.swift` and `SourceKit/RelatedIdents/operator.swift` - swiftlang#70389 modified `SourceKit/Refactoring/basic.swift` and indirectly modified all `refactoring/ExtractFunction/*` tests.
Catfish-Man
pushed a commit
to Catfish-Man/swift
that referenced
this pull request
Jan 19, 2024
Some of the tests now depend on features provided by Swift Syntax, and will fail to pass if building without Swift Syntax support. This commits marks all the tests I know fail to pass when Swift Syntax is disabled. - swiftlang#70281 modified `ASTGen/verify-parse.swift` - swiftlang#70287 created `refactoring/SyntacticRename/operator.swift` and `SourceKit/RelatedIdents/operator.swift` - swiftlang#70389 modified `SourceKit/Refactoring/basic.swift` and indirectly modified all `refactoring/ExtractFunction/*` tests.
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.
This augments the related identifiers request to return more information that’s needed for local rename.
In particular this is:
foo(a:)). This is needed so we can pass it tofind-syntactic-rename-ranges, which will verify that the name at any given location matches this old nameinit. While we don't want to return non-editable base names for Edit-All-In-Scope, we do want to return them if the related identifiers response is used as the input forfind-syntactic-rename-rangesto rename initializer argument labels.rdar://118994821