Skip to content

[Sema/Index] Resolve #keyPath components so they get handled by indexing, semantic highlighting, etc #33320

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 2 commits into from
Aug 6, 2020

Conversation

nathawes
Copy link
Contributor

@nathawes nathawes commented Aug 5, 2020

This is a re-do of #33245 to account for source breakage.

Unlike \keypath expressions, the components of #keyPath() expressions weren't being resolved, so the index wouldn't pick up references for their qualifying types/properties. Semantic highlighting, cursor info, etc wouldn't work on them either. My fix for this part is largely based on @rockbruno's closed PR here: #20020 (thanks @rockbruno!)

The change to resolve ObjC #keyPath expression components caused some source breakage as they are now being checked for availability issues. To avoid that this change also updates availability checking to demote error diagnostics to warnings within #keyPath expressions. There were cases in the source compat suite where unavailable properties were used in #keyPath expressions, but worked correctly at runtime because the properties' ObjC runtime name was still correct (in this case the same as its renamed-to property in Swift).

Also fixes a code completion bug where it was reporting members from the Swift rather than ObjC side of bridged types (#keyPath(MyClass.someString.count) is invalid, but keyPath(MyClass.someString.length) is valid).

Resolves rdar://problem/61573935

Unlike \keypath expressions, only the property components of #keypath
expressions were being resolved, so index wouldn't pick up references for their
qualifying types.

Also fixes a code completion bug where it was reporting members from the Swift
rather than ObjC side of bridged types.

Resolves rdar://problem/61573935
@nathawes nathawes requested review from xedin and rintaro August 5, 2020 22:37
@nathawes
Copy link
Contributor Author

nathawes commented Aug 5, 2020

@swift-ci please test

@nathawes
Copy link
Contributor Author

nathawes commented Aug 5, 2020

@swift-ci please test source compatibility

Copy link
Contributor

@xedin xedin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me.

@@ -4657,13 +4657,24 @@ ERROR(availability_decl_unavailable, none,
"%select{ in %3|}2%select{|: %4}4",
(unsigned, DeclName, bool, StringRef, StringRef))

WARNING(availability_decl_unavailable_warn, none,
"%select{getter for |setter for |}0%1 is unavailable"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add both of these new diagnostics to localization/diagnostics/en.yaml

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh neat! Will do.

@xedin xedin requested a review from CodaFi August 5, 2020 22:44
@swift-ci
Copy link
Contributor

swift-ci commented Aug 5, 2020

Build failed
Swift Test Linux Platform
Git Sha - 94e02b16801e33ac36c48b2cb2c8b6813f21b16d

@@ -2508,7 +2523,8 @@ class AvailabilityWalker : public ASTWalker {
auto *decl = component.getDeclRef().getDecl();
auto loc = component.getLoc();
SourceRange range(loc, loc);
diagAvailability(decl, range, nullptr);
diagAvailability(decl, range, nullptr,
DeclAvailabilityFlag::ForObjCKeyPath);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, this needs to check it's actually an ObjC rather than Swift keypath first. Fixing...

@swift-ci
Copy link
Contributor

swift-ci commented Aug 5, 2020

Build failed
Swift Test OS X Platform
Git Sha - 94e02b16801e33ac36c48b2cb2c8b6813f21b16d

… keypaths to prevent source breakage.

The change to resolve ObjC #keyPath expression components caused some source
breakage as they are now being checked for availability issues. This change
updates availability checking to demote error diagnostics to warnings
within #keyPath expressions. There were cases in the source compat suite where
unavailble properites were used in #keyPath expressions, but they caused no
issues at runtime because the properties' ObjC runtime name was still correct
(e.g. the same as its renamed-to property in Swift).
@nathawes
Copy link
Contributor Author

nathawes commented Aug 6, 2020

@swift-ci please test

@nathawes
Copy link
Contributor Author

nathawes commented Aug 6, 2020

@swift-ci please test source compatibility

@swift-ci
Copy link
Contributor

swift-ci commented Aug 6, 2020

Build failed
Swift Test Linux Platform
Git Sha - 94e02b16801e33ac36c48b2cb2c8b6813f21b16d

@swift-ci
Copy link
Contributor

swift-ci commented Aug 6, 2020

Build failed
Swift Test OS X Platform
Git Sha - 94e02b16801e33ac36c48b2cb2c8b6813f21b16d

@rintaro
Copy link
Member

rintaro commented Aug 6, 2020

Windows: Build Status

@nathawes nathawes merged commit 69abfc1 into swiftlang:master Aug 6, 2020
@nathawes nathawes deleted the index-key-paths branch August 6, 2020 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants