Skip to content

[IDE][Parse] Fix syntax coloring ranges for type attributes #26418

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 3 commits into from
Aug 9, 2019

Conversation

nathawes
Copy link
Contributor

@nathawes nathawes commented Jul 30, 2019

When looking for the SyntaxNode corresponding to a type attribute (like @escaping), ModelASTWalker would look for one whose range started at the type attribute's source location. It never found one, though, because the SyntaxNode's range included the @, while the type attribute's source location pointed to the name after the @. The result was that type attribute tokens were highlighted as unknown, rather than built-in attributes. This PR changes type attribute source locations to point to the @ rather than the name (which matches the start location of DeclAttributes too). This PR also fixes a bug I noticed where the fixit for removing @escaping on non-function types would compute the incorrect range for the attribute.

TypeAttributes would ideally keep SourceLocs for both the @ and the attribute name since things like @ /*this is fine...*/ escaping are valid. I didn't do that since it represents which attributes are on a type via a fixed-length array of SourceLocs, with one entry for every possible type attribute kind, where each SourceLoc's validity encodes whether the attribute is present or not. Extending this to a list of SourceRanges would work, but seemed like a waste of memory. Given that the @ is usually next to the attribute name, it's needed purely for syntax highlighting, and we'll hopefully be using the libSyntax tree to track source locations in future, I let that case be for now.

@nathawes nathawes requested a review from rintaro July 30, 2019 21:49
@nathawes
Copy link
Contributor Author

@swift-ci please test

@nathawes nathawes force-pushed the type-attr-coloring branch from 8823131 to ad8e699 Compare August 5, 2019 18:11
Nathan Hawes added 3 commits August 5, 2019 11:36
When looking for the SyntaxNode corresponding to a type attribute (like
@escaping), ModelASTWalker would look for one whose range *started* at the type
attribute's source location. It never found one, though, because the
SyntaxNode's range included the @, while the type attribute's source location
pointed to the name *after* the @.
@nathawes nathawes force-pushed the type-attr-coloring branch from ad8e699 to 4d2aca1 Compare August 5, 2019 18:36
@nathawes
Copy link
Contributor Author

nathawes commented Aug 5, 2019

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented Aug 5, 2019

Build failed
Swift Test OS X Platform
Git Sha - 8823131f389aadf2235be3e36f1b04edec0d5cba

@swift-ci
Copy link
Contributor

swift-ci commented Aug 5, 2019

Build failed
Swift Test Linux Platform
Git Sha - 8823131f389aadf2235be3e36f1b04edec0d5cba

@nathawes
Copy link
Contributor Author

nathawes commented Aug 5, 2019

@swift-ci please test Linux

Copy link
Member

@rintaro rintaro left a comment

Choose a reason for hiding this comment

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

Looks good!

@nathawes
Copy link
Contributor Author

nathawes commented Aug 9, 2019

@swift-ci test and merge

@swift-ci swift-ci merged commit c43eb9b into swiftlang:master Aug 9, 2019
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.

3 participants