[ty] Omit semantic tokens for unresolved symbols#24718
[ty] Omit semantic tokens for unresolved symbols#24718charliermarsh merged 4 commits intoastral-sh:mainfrom
Conversation
denyszhak
requested review from
AlexWaygood,
BurntSushi,
Gankra,
MichaReiser,
carljm,
dcreager and
sharkdp
as code owners
| tokens: Vec<SemanticToken>, | ||
| } | ||
|
|
||
| enum UnifiedTokenType { |
There was a problem hiding this comment.
I think this can be moved back into the local function.
charliermarsh
changed the title
| z = obj.CONSTANT # CONSTANT should be variable with readonly modifier | ||
| w = obj.prop # prop should be property | ||
| v = MyClass.method # method should be method (function) | ||
| u = List.__name__ # __name__ should be variable |
There was a problem hiding this comment.
The test expectation here is now outdated. Can we say why we no longer expect a variable here (should probably be a property)
There was a problem hiding this comment.
List.__name__ currently goes into the unresolved-attribute path but there is no reason not to keep it as resolvable class attr instead (like MyClass__name__) and you are right this should be a property now, not a variable.
| "app" @ 26..29: Variable | ||
| "route" @ 30..35: Variable |
There was a problem hiding this comment.
I'm not sure if we should update this test. We're now clearly testing less than before.
There was a problem hiding this comment.
app.route is unresolved now so it's consistent with what the change does. A better way would be to make the decorator resolvable, though. I can send a small follow-up PR and reference the same issue for context.
3 participants
Closes astral-sh/ty#3090
Summary
Stop emitting fallback semantic tokens for unknown symbols. Unresolved names, imports, and attributes now omit semantic tokens instead of being classified as Variable, so editors fall back to grammar highlighting.
Test Plan
Added tests and updated snapshots for older ones.