[5.1][IDE][Index] Property wrapper rename support #25871
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
added 6 commits
June 28, 2019 14:57
…m attributes This fixes custom attribute syntax highlighting on parameters and functions (where function builders can be applied). They weren't being walked in the function position previously and were walked out of source order in the parameter position. It also fixes rename of the property wrapper and function builder type names that can appear in custom attributes, as well as rename of property wrapper constructors, that can appear after the type names, e.g. `@Wrapper(initialValue: 10)`. The index now also records these constructor occurrences, along with implicit occurrences whenever a constructor is called via default value assignment, e.g. `@Wrapper var foo = 10`, so that finding calls/references to the constructor includes these locations. Resolves rdar://problem/49036613 Resolves rdar://problem/50073641
…esized $-prefixed backing property This patch achieves this by updating indexing to reporting the position of `foo` in occurrences of `$foo` as an occurrence of the `foo` symbol, so that renames initiated on occurrences of the `foo` symbol will also result in occurrences of the `$foo` symbol being updated correctly. This also means find-references on foo will show places where $foo is used. Making rename work in the other direction (invoking rename on $foo upating foo occurrences too) is still todo.
…er backing properties as the underlying property This is the final piece to get rename and findRelatedIdents to include all foo and $foo occurrences, for example, in the set of occurrences to be renamed or shown, regardless of whether they were initiated on a foo or $foo. Resolves rdar://problem/51695783.
…Value rather than value Plus other small cleanups to comments and variable names.
…pers Make sure they handle the case when a property wrapper type's constructor is called with the first argument coming from the var initializer, and the rest from the custom attribute's argument.
…rty wrapper changes The backing property for 'foo' is now '_foo', and the projected value '$foo'. This updates Indexing to report occurrences of foo within both $foo and _foo occurrences (rather than just $foo - the old _foo). FindRelatedIdents was similarlar updated, so it reports 'foo' ranges in both _foo and $foo. CursorInfo now reports the USR, documentation, and location of foo when invoked occurrences of $foo or _foo, but now leaves the name, type, and annotated declaration of _foo/$foo as is. Having the same USR ensures rename invoked on any of them will still rename via foo. Reporting foo's documentation comment instead is just to present something more useful to the user.
|
@swift-ci please test |
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.
Cherry-pick of #25758 for swift-5.1-branch
This PR fixes up rename support for property wrappers. There were three main issues this addresses:
@propertyWrapperstruct was failing due to rename'sNameMatcherusing the range of aDeclitself, not including its attributes, to decide if it contained a location that needed to be resolved.@propertyWrapperstruct would not pick up occurrences of that constructor's argument labels used in custom attributes, e.g. renamingMyWrapper'sinit(initialValue:minValue:maxValue)constructor would miss the argument labels in custom attributes like@MyWrapper(initialValue: 50, minValue: 0, maxValue: 100). This is because the indexer didn't report an occurrence of the constructor call in custom attributes. Rename'sNameMatcheralso needed to be updated to handle resolving these newly-added occurrences, since the call expression in the AST doesn't have an explicit base (MyWrapperis not part of the call expression, but is where the index reports the call taking place).foo) wasn't updating the corresponding synthesized$-prefixed backing property ($foo). The synthesized backing property declarations were implicit, so the index wasn't recording them, and even if it did, it's a different property so wouldn't be picked up automatically.To resolve this, the index now records an occurrence of the wrapped property within the synthesized backing property occurrences (i.e. on the
foowithin each$foo) so that rename on the wrapped property updates the synthesized backing property occurrences too. To support rename being invoked via an occurrence of a synthesized property, theCusorInforequest that determines which symbol is renamed for a given location now treats occurrences of synthesized properties as if they were the corresponding wrapped properties. TheFindRelatedIdentsrequest was also similarly updated so that rename-like features that are based on it (like Xcode's edit-all-in-scope) can get both the underlying and synthesized property ranges, regardless of which is was invoked on.This PR also includes some small and/or incidental IDE issue fixes around custom attributes:
Resolves rdar://problem/49036613
Resolves rdar://problem/50073641
Resolves rdar://problem/51695783
Resolves rdar://problem/52053678