Skip to content

Commit 5062d0d

Browse files
[ty] Omit semantic tokens for unresolved symbols (astral-sh#24718)
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. <p> <img src="https://github.com/user-attachments/assets/fb570a55-504f-4745-b1c0-53e6a56b8798" width="49%" /> <img src="https://github.com/user-attachments/assets/a1b0dd28-aaa8-417d-ba9a-916de6635495" width="49%" /> </p> ## Test Plan Added tests and updated snapshots for older ones. --------- Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
1 parent cd8e76f commit 5062d0d

1 file changed

Lines changed: 88 additions & 50 deletions

File tree

crates/ty_ide/src/semantic_tokens.rs

Lines changed: 88 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,10 @@ impl<'db> SemanticTokenVisitor<'db> {
270270
&& name.len() > 1
271271
}
272272

273-
fn classify_name(&self, name: &ast::ExprName) -> (SemanticTokenType, SemanticTokenModifier) {
273+
fn classify_name(
274+
&self,
275+
name: &ast::ExprName,
276+
) -> Option<(SemanticTokenType, SemanticTokenModifier)> {
274277
// First try to classify the token based on its definition kind.
275278
let definition = definition_for_name(
276279
self.model,
@@ -281,7 +284,7 @@ impl<'db> SemanticTokenVisitor<'db> {
281284
if let Some(definition) = definition {
282285
let name_str = name.id.as_str();
283286
if let Some(classification) = self.classify_from_definition(definition, name_str) {
284-
return classification;
287+
return Some(classification);
285288
}
286289
}
287290

@@ -387,14 +390,18 @@ impl<'db> SemanticTokenVisitor<'db> {
387390
&self,
388391
ty: Type,
389392
name_str: &str,
390-
) -> (SemanticTokenType, SemanticTokenModifier) {
393+
) -> Option<(SemanticTokenType, SemanticTokenModifier)> {
394+
if ty.is_unknown() {
395+
return None;
396+
}
397+
391398
let mut modifiers = SemanticTokenModifier::empty();
392399

393400
if let Some(classification) = self.classify_type_form_expr(ty) {
394-
return classification;
401+
return Some(classification);
395402
}
396403

397-
match ty {
404+
Some(match ty {
398405
Type::ClassLiteral(_) => (SemanticTokenType::Class, modifiers),
399406
Type::TypeVar(_) => (SemanticTokenType::TypeParameter, modifiers),
400407
Type::FunctionLiteral(_) => {
@@ -415,7 +422,7 @@ impl<'db> SemanticTokenVisitor<'db> {
415422
// For other types (variables, modules, etc.), assume variable
416423
(SemanticTokenType::Variable, modifiers)
417424
}
418-
}
425+
})
419426
}
420427

421428
fn classify_type_form_expr(
@@ -444,7 +451,7 @@ impl<'db> SemanticTokenVisitor<'db> {
444451
&self,
445452
ty: Type,
446453
attr_name: &ast::Identifier,
447-
) -> (SemanticTokenType, SemanticTokenModifier) {
454+
) -> Option<(SemanticTokenType, SemanticTokenModifier)> {
448455
enum UnifiedTokenType {
449456
None,
450457
/// All types have the same semantic token type
@@ -470,12 +477,16 @@ impl<'db> SemanticTokenVisitor<'db> {
470477
}
471478
}
472479

480+
if ty.is_unknown() {
481+
return None;
482+
}
483+
473484
let db = self.model.db();
474485
let attr_name_str = attr_name.id.as_str();
475486
let mut modifiers = SemanticTokenModifier::empty();
476487

477488
if let Some(classification) = self.classify_type_form_expr(ty) {
478-
return classification;
489+
return Some(classification);
479490
}
480491

481492
let elements = if let Some(union) = ty.as_union() {
@@ -519,7 +530,7 @@ impl<'db> SemanticTokenVisitor<'db> {
519530
if uniform == SemanticTokenType::Property && all_properties_are_readonly {
520531
modifiers |= SemanticTokenModifier::READONLY;
521532
}
522-
return (uniform, modifiers);
533+
return Some((uniform, modifiers));
523534
}
524535

525536
// Check for constant naming convention
@@ -529,7 +540,7 @@ impl<'db> SemanticTokenVisitor<'db> {
529540

530541
// For other types (variables, constants, etc.), classify as variable
531542
// Should this always be property?
532-
(SemanticTokenType::Variable, modifiers)
543+
Some((SemanticTokenType::Variable, modifiers))
533544
}
534545

535546
fn classify_parameter(
@@ -599,7 +610,7 @@ impl<'db> SemanticTokenVisitor<'db> {
599610
&self,
600611
ty: Type,
601612
local_name: &ast::Identifier,
602-
) -> (SemanticTokenType, SemanticTokenModifier) {
613+
) -> Option<(SemanticTokenType, SemanticTokenModifier)> {
603614
self.classify_from_type_and_name_str(ty, local_name.id.as_str())
604615
}
605616

@@ -772,14 +783,16 @@ impl SourceOrderVisitor<'_> for SemanticTokenVisitor<'_> {
772783
for alias in &import.names {
773784
// Get the type of the imported name
774785
let ty = alias.inferred_type(self.model).unwrap_or(Type::unknown());
775-
let (token_type, modifiers) = self.classify_from_alias_type(ty, &alias.name);
776-
777-
// Add token for the imported name (Y in "from X import Y" or "from X import Y as Z")
778-
self.add_token(&alias.name, token_type, modifiers);
786+
if let Some((token_type, modifiers)) =
787+
self.classify_from_alias_type(ty, &alias.name)
788+
{
789+
// Add token for the imported name (Y in "from X import Y" or "from X import Y as Z")
790+
self.add_token(&alias.name, token_type, modifiers);
779791

780-
// For aliased imports (from X import Y as Z), also add a token for the alias Z
781-
if let Some(asname) = &alias.asname {
782-
self.add_token(asname, token_type, modifiers);
792+
// For aliased imports (from X import Y as Z), also add a token for the alias Z
793+
if let Some(asname) = &alias.asname {
794+
self.add_token(asname, token_type, modifiers);
795+
}
783796
}
784797
}
785798
}
@@ -902,11 +915,12 @@ impl SourceOrderVisitor<'_> for SemanticTokenVisitor<'_> {
902915
fn visit_expr(&mut self, expr: &Expr) {
903916
match expr {
904917
ast::Expr::Name(name) => {
905-
let (token_type, mut modifiers) = self.classify_name(name);
906-
if self.in_target_creating_definition && name.ctx.is_store() {
907-
modifiers |= SemanticTokenModifier::DEFINITION;
918+
if let Some((token_type, mut modifiers)) = self.classify_name(name) {
919+
if self.in_target_creating_definition && name.ctx.is_store() {
920+
modifiers |= SemanticTokenModifier::DEFINITION;
921+
}
922+
self.add_token(name, token_type, modifiers);
908923
}
909-
self.add_token(name, token_type, modifiers);
910924
walk_expr(self, expr);
911925
}
912926
ast::Expr::Attribute(attr) => {
@@ -916,8 +930,11 @@ impl SourceOrderVisitor<'_> for SemanticTokenVisitor<'_> {
916930
// Then add token for the attribute name (e.g., 'path' in 'os.path')
917931
let ty = static_member_type_for_attribute(self.model, attr)
918932
.unwrap_or_else(|| expr.inferred_type(self.model).unwrap_or(Type::unknown()));
919-
let (token_type, modifiers) = self.classify_from_type_for_attribute(ty, &attr.attr);
920-
self.add_token(&attr.attr, token_type, modifiers);
933+
if let Some((token_type, modifiers)) =
934+
self.classify_from_type_for_attribute(ty, &attr.attr)
935+
{
936+
self.add_token(&attr.attr, token_type, modifiers);
937+
}
921938
}
922939
ast::Expr::NumberLiteral(_) => {
923940
self.add_token(
@@ -1230,11 +1247,7 @@ mod tests {
12301247

12311248
let tokens = test.highlight_file();
12321249

1233-
assert_snapshot!(test.to_snapshot(&tokens), @r#"
1234-
"Foo" @ 6..9: Class [definition]
1235-
"x" @ 12..13: Variable
1236-
"m" @ 15..16: Variable
1237-
"#);
1250+
assert_snapshot!(test.to_snapshot(&tokens), @r#""Foo" @ 6..9: Class [definition]"#);
12381251
}
12391252

12401253
#[test]
@@ -1759,9 +1772,6 @@ from mymodule import CONSTANT, my_function, MyClass
17591772
"Dict" @ 104..108: Variable
17601773
"Optional" @ 110..118: Variable
17611774
"mymodule" @ 124..132: Namespace
1762-
"CONSTANT" @ 140..148: Variable [readonly]
1763-
"my_function" @ 150..161: Variable
1764-
"MyClass" @ 163..170: Variable
17651775
"#);
17661776
}
17671777

@@ -1797,7 +1807,6 @@ w5: "float
17971807
"\"hello\"" @ 53..60: String
17981808
"w2" @ 61..63: Variable [definition]
17991809
"int" @ 66..69: Class
1800-
"sr" @ 72..74: Variable
18011810
"\"hello\"" @ 78..85: String
18021811
"w3" @ 86..88: Variable [definition]
18031812
"\"int | \"" @ 90..98: String
@@ -1941,7 +1950,6 @@ t = MyClass.prop # prop should be property on the class itself
19411950
"method" @ 546..552: Method
19421951
"u" @ 596..597: Variable [definition]
19431952
"List" @ 600..604: Variable
1944-
"__name__" @ 605..613: Variable
19451953
"t" @ 651..652: Variable [definition]
19461954
"MyClass" @ 655..662: Class
19471955
"prop" @ 663..667: Property [readonly]
@@ -2172,7 +2180,6 @@ y = obj.unknown_attr # Should fall back to variable
21722180
"some_attr" @ 125..134: Variable
21732181
"y" @ 187..188: Variable [definition]
21742182
"obj" @ 191..194: Variable
2175-
"unknown_attr" @ 195..207: Variable
21762183
"#);
21772184
}
21782185

@@ -3499,10 +3506,8 @@ class BoundedContainer[T: int, U = str]:
34993506
"func_paramspec" @ 266..280: Function [definition]
35003507
"P" @ 283..284: TypeParameter [definition]
35013508
"func" @ 286..290: Parameter [definition]
3502-
"Callable" @ 292..300: Variable
35033509
"P" @ 301..302: Variable
35043510
"int" @ 304..307: Class
3505-
"Callable" @ 313..321: Variable
35063511
"P" @ 322..323: Variable
35073512
"str" @ 325..328: Class
35083513
"wrapper" @ 339..346: Function [definition]
@@ -3614,8 +3619,6 @@ class MyClass:
36143619
assert_snapshot!(test.to_snapshot(&tokens), @r#"
36153620
"staticmethod" @ 2..14: Decorator
36163621
"property" @ 16..24: Decorator
3617-
"app" @ 26..29: Variable
3618-
"route" @ 30..35: Variable
36193622
"\"/path\"" @ 36..43: String
36203623
"my_function" @ 49..60: Function [definition]
36213624
"dataclass" @ 75..84: Decorator
@@ -3937,14 +3940,6 @@ def test():
39373940
"d" @ 145..146: Variable
39383941
"e" @ 148..149: Variable
39393942
"f" @ 151..152: Variable
3940-
"x" @ 165..166: Variable
3941-
"y" @ 169..170: Variable
3942-
"a" @ 173..174: Variable
3943-
"b" @ 177..178: Variable
3944-
"c" @ 181..182: Variable
3945-
"d" @ 185..186: Variable
3946-
"e" @ 189..190: Variable
3947-
"f" @ 193..194: Variable
39483943
"#);
39493944
}
39503945

@@ -4076,10 +4071,7 @@ class C:
40764071
"non_annotated" @ 111..124: Variable
40774072
"1" @ 127..128: Number
40784073
"self" @ 137..141: SelfParameter
4079-
"x" @ 142..143: Variable
4080-
"test" @ 144..148: Variable
40814074
"self" @ 159..163: SelfParameter
4082-
"x" @ 164..165: Variable
40834075
"#);
40844076
}
40854077

@@ -4265,6 +4257,52 @@ from collections.abc import Set as AbstractSet
42654257
"#);
42664258
}
42674259

4260+
#[test]
4261+
fn unresolved_names_do_not_receive_semantic_tokens() {
4262+
let test = SemanticTokenTest::new(
4263+
r#"
4264+
def f():
4265+
missing()
4266+
"#,
4267+
);
4268+
4269+
let tokens = test.highlight_file();
4270+
assert_snapshot!(test.to_snapshot(&tokens), @r#""f" @ 5..6: Function [definition]"#);
4271+
}
4272+
4273+
#[test]
4274+
fn unresolved_attributes_do_not_receive_semantic_tokens() {
4275+
let test = SemanticTokenTest::new(
4276+
r#"
4277+
class C: ...
4278+
4279+
def f(c: C):
4280+
c.missing()
4281+
"#,
4282+
);
4283+
4284+
let tokens = test.highlight_file();
4285+
assert_snapshot!(test.to_snapshot(&tokens), @r#"
4286+
"C" @ 7..8: Class [definition]
4287+
"f" @ 19..20: Function [definition]
4288+
"c" @ 21..22: Parameter [definition]
4289+
"C" @ 24..25: Class
4290+
"c" @ 32..33: Parameter
4291+
"#);
4292+
}
4293+
4294+
#[test]
4295+
fn unresolved_imported_names_do_not_receive_semantic_tokens() {
4296+
let test = SemanticTokenTest::new(
4297+
r#"
4298+
from pathlib import Missing as Alias
4299+
"#,
4300+
);
4301+
4302+
let tokens = test.highlight_file();
4303+
assert_snapshot!(test.to_snapshot(&tokens), @r#""pathlib" @ 6..13: Namespace"#);
4304+
}
4305+
42684306
pub(super) struct SemanticTokenTest {
42694307
pub(super) db: ty_project::TestDb,
42704308
file: File,

0 commit comments

Comments
 (0)