[SourceKit] Don’t provide incorrect semantic highlighting of nil in case statements as enum decl element
#60328
Conversation
ahoppen
force-pushed
the
pr/nil-highlighting-in-case
branch
from
2b75a15 to
51bb420
Compare
| if (D == D->getASTContext().getOptionalNoneDecl() && Length == 3) { | ||
| // If a 'nil' literal occurs in a swift-case statement, it gets replaced | ||
| // by a reference to 'Optional.none' in the AST. We want to continue | ||
| // highlighting 'nil' as a keyword and not as an enum element. | ||
| // Detect this case if 'none' was spelled with 3 characters, which | ||
| // indicates that it was rewritten from 'nil'. |
There was a problem hiding this comment.
Length == 3 might be enough in real world, but SM.extractText(Range, BufferID) == "nil" is more explicit, safe, and not so heavy. If we don't have strong reason not to, could you check the actual spelling?
There was a problem hiding this comment.
Also, other "deciding annotate or not" checks are in visitDeclReference() instead of annotate(). Could you move this logic to it?
There was a problem hiding this comment.
Good suggestions, thanks. I though I would have needed to go to the lexer to get the original source text but of course I don’t.
ahoppen
force-pushed
the
pr/nil-highlighting-in-case
branch
from
51bb420 to
872f01d
Compare
|
@swift-ci Please smoke test |
| // Detect this case if 'none' was spelled with 3 characters, which | ||
| // indicates that it was rewritten from 'nil'. |
There was a problem hiding this comment.
The last sentence is not correct anymore :)
There was a problem hiding this comment.
Thanks for catching it. I removed the last sentence.
| @@ -963,6 +963,17 @@ class SemanticAnnotator : public SourceEntityWalker { | |||
| if (AvailableAttr::isUnavailable(D)) | |||
| return true; | |||
|
|
|||
| auto &SM = D->getASTContext().SourceMgr; | |||
| if (D == D->getASTContext().getOptionalNoneDecl() && | |||
| SM.extractText(Range) == "nil") { | |||
There was a problem hiding this comment.
| SM.extractText(Range) == "nil") { | |
| SM.extractText(Range, BufferID) == "nil") { |
So it skips findBufferContainingLoc() operation
… case statements as enum decl element If a 'nil' literal occurs in a swift-case statment, it gets replaced by a reference to 'Optional.none' in the AST. We want to continue highlighting 'nil' as a keyword and not as an enum element. Resolves swiftlang/sourcekit-lsp#599 rdar://97961865
ahoppen
force-pushed
the
pr/nil-highlighting-in-case
branch
from
872f01d to
1e9eac6
Compare
|
@swift-ci Please smoke test |
If a 'nil' literal occurs in a swift-case statement, it gets replaced by a reference to 'Optional.none' in the AST. We want to continue highlighting 'nil' as a keyword and not as an enum element. Detect this case if 'none' was spelled with 3 characters, which indicates that it was rewritten from 'nil'.
Resolves swiftlang/sourcekit-lsp#599
rdar://97961865