diff --git a/crates/ty_ide/src/semantic_tokens.rs b/crates/ty_ide/src/semantic_tokens.rs index aa74cc29cceb1a..8b4f406ebfb710 100644 --- a/crates/ty_ide/src/semantic_tokens.rs +++ b/crates/ty_ide/src/semantic_tokens.rs @@ -46,8 +46,10 @@ use std::ops::Deref; use ty_python_semantic::semantic_index::definition::Definition; use ty_python_semantic::types::TypeVarKind; use ty_python_semantic::{ - HasType, SemanticModel, semantic_index::definition::DefinitionKind, types::Type, - types::ide_support::definition_for_name, + HasType, SemanticModel, + semantic_index::definition::DefinitionKind, + types::Type, + types::ide_support::{definition_for_name, static_member_type_for_attribute}, }; /// Semantic token types supported by the language server. @@ -467,6 +469,7 @@ impl<'db> SemanticTokenVisitor<'db> { } } + let db = self.model.db(); let attr_name_str = attr_name.id.as_str(); let mut modifiers = SemanticTokenModifier::empty(); @@ -475,12 +478,13 @@ impl<'db> SemanticTokenVisitor<'db> { } let elements = if let Some(union) = ty.as_union() { - union.elements(self.model.db()) + union.elements(db) } else { std::slice::from_ref(&ty) }; let mut token_type = UnifiedTokenType::None; + let mut all_properties_are_readonly = true; for element in elements { // Classify based on the inferred type of the attribute @@ -500,8 +504,9 @@ impl<'db> SemanticTokenVisitor<'db> { // Module accessed as an attribute (e.g., from os import path) token_type.add(SemanticTokenType::Namespace); } - ty if ty.is_property_instance() => { + Type::PropertyInstance(property) => { token_type.add(SemanticTokenType::Property); + all_properties_are_readonly &= property.setter(db).is_none(); } _ => { token_type = UnifiedTokenType::Fallback; @@ -510,6 +515,9 @@ impl<'db> SemanticTokenVisitor<'db> { } if let Some(uniform) = token_type.into_semantic_token_type() { + if uniform == SemanticTokenType::Property && all_properties_are_readonly { + modifiers |= SemanticTokenModifier::READONLY; + } return (uniform, modifiers); } @@ -895,7 +903,8 @@ impl SourceOrderVisitor<'_> for SemanticTokenVisitor<'_> { self.visit_expr(&attr.value); // Then add token for the attribute name (e.g., 'path' in 'os.path') - let ty = expr.inferred_type(self.model).unwrap_or(Type::unknown()); + let ty = static_member_type_for_attribute(self.model, attr) + .unwrap_or_else(|| expr.inferred_type(self.model).unwrap_or(Type::unknown())); let (token_type, modifiers) = self.classify_from_type_for_attribute(ty, &attr.attr); self.add_token(&attr.attr, token_type, modifiers); } @@ -1787,7 +1796,7 @@ b: list["int | str"] | None c: "list[int | str] | None" d: "list[int | str]" | "None" e: 'list["int | str"] | "None"' -f: """'list["int | str"]' | 'None'""" +f: """'list["int | str"]' | 'None'""" "#, ); @@ -1899,7 +1908,7 @@ t = MyClass.prop # prop should be property on the class itself "CONSTANT" @ 413..421: Variable [readonly] "w" @ 483..484: Variable [definition] "obj" @ 487..490: Variable - "prop" @ 491..495: Variable + "prop" @ 491..495: Property [readonly] "v" @ 534..535: Variable [definition] "MyClass" @ 538..545: Class "method" @ 546..552: Method @@ -1908,7 +1917,204 @@ t = MyClass.prop # prop should be property on the class itself "__name__" @ 605..613: Variable "t" @ 651..652: Variable [definition] "MyClass" @ 655..662: Class - "prop" @ 663..667: Property + "prop" @ 663..667: Property [readonly] + "#); + } + + #[test] + fn property_with_return_annotation() { + let test = SemanticTokenTest::new( + " +class Foo: + @property + def prop(self) -> int: + return 4 + +foo = Foo() +w = foo.prop +x = Foo.prop +", + ); + + let tokens = test.highlight_file(); + + assert_snapshot!(test.to_snapshot(&tokens), @r#" + "Foo" @ 7..10: Class [definition] + "property" @ 17..25: Decorator + "prop" @ 34..38: Method [definition] + "self" @ 39..43: SelfParameter [definition] + "int" @ 48..51: Class + "4" @ 68..69: Number + "foo" @ 71..74: Variable [definition] + "Foo" @ 77..80: Class + "w" @ 83..84: Variable [definition] + "foo" @ 87..90: Variable + "prop" @ 91..95: Property [readonly] + "x" @ 96..97: Variable [definition] + "Foo" @ 100..103: Class + "prop" @ 104..108: Property [readonly] + "#); + } + + #[test] + fn property_readonly_modifier() { + // Verify that the readonly modifier is set for getter-only properties + // and NOT set for properties that also have a setter. + let test = SemanticTokenTest::new( + " +class Config: + @property + def read_only(self) -> str: + return 'value' + + @property + def read_write(self) -> int: + return self._x + + @read_write.setter + def read_write(self, value: int) -> None: + self._x = value + +cfg = Config() +a = cfg.read_only +b = cfg.read_write +", + ); + + let tokens = test.highlight_file(); + + assert_snapshot!(test.to_snapshot(&tokens), @r#" + "Config" @ 7..13: Class [definition] + "property" @ 20..28: Decorator + "read_only" @ 37..46: Method [definition] + "self" @ 47..51: SelfParameter [definition] + "str" @ 56..59: Class + "'value'" @ 76..83: String + "property" @ 90..98: Decorator + "read_write" @ 107..117: Method [definition] + "self" @ 118..122: SelfParameter [definition] + "int" @ 127..130: Class + "self" @ 147..151: SelfParameter + "_x" @ 152..154: Variable + "read_write" @ 161..171: Method + "setter" @ 172..178: Method + "read_write" @ 187..197: Method [definition] + "self" @ 198..202: SelfParameter [definition] + "value" @ 204..209: Parameter [definition] + "int" @ 211..214: Class + "None" @ 219..223: BuiltinConstant + "self" @ 233..237: SelfParameter + "_x" @ 238..240: Variable + "value" @ 243..248: Parameter + "cfg" @ 250..253: Variable [definition] + "Config" @ 256..262: Class + "a" @ 265..266: Variable [definition] + "cfg" @ 269..272: Variable + "read_only" @ 273..282: Property [readonly] + "b" @ 283..284: Variable [definition] + "cfg" @ 287..290: Variable + "read_write" @ 291..301: Property + "#); + } + + #[test] + fn property_union_with_non_property_falls_back() { + let test = SemanticTokenTest::new( + " +class WithProperty: + @property + def value(self) -> int: + return 1 + +class WithAttribute: + value = 2 + +def f(obj: WithProperty | WithAttribute): + return obj.value +", + ); + + let tokens = test.highlight_file(); + + assert_snapshot!(test.to_snapshot(&tokens), @r#" + "WithProperty" @ 7..19: Class [definition] + "property" @ 26..34: Decorator + "value" @ 43..48: Method [definition] + "self" @ 49..53: SelfParameter [definition] + "int" @ 58..61: Class + "1" @ 78..79: Number + "WithAttribute" @ 87..100: Class [definition] + "value" @ 106..111: Variable [definition] + "2" @ 114..115: Number + "f" @ 121..122: Function [definition] + "obj" @ 123..126: Parameter [definition] + "WithProperty" @ 128..140: Class + "WithAttribute" @ 143..156: Class + "obj" @ 170..173: Parameter + "value" @ 174..179: Variable + "#); + } + + #[test] + fn property_union_readonly_only_if_all_variants_are_readonly() { + let test = SemanticTokenTest::new( + " +from random import random + +class ReadOnly: + @property + def value(self) -> int: + return 1 + +class ReadWrite: + @property + def value(self) -> int: + return self._value + + @value.setter + def value(self, new_value: int) -> None: + self._value = new_value + +obj = ReadOnly() if random() else ReadWrite() +x = obj.value +", + ); + + let tokens = test.highlight_file(); + + assert_snapshot!(test.to_snapshot(&tokens), @r#" + "random" @ 6..12: Namespace + "random" @ 20..26: Method + "ReadOnly" @ 34..42: Class [definition] + "property" @ 49..57: Decorator + "value" @ 66..71: Method [definition] + "self" @ 72..76: SelfParameter [definition] + "int" @ 81..84: Class + "1" @ 101..102: Number + "ReadWrite" @ 110..119: Class [definition] + "property" @ 126..134: Decorator + "value" @ 143..148: Method [definition] + "self" @ 149..153: SelfParameter [definition] + "int" @ 158..161: Class + "self" @ 178..182: SelfParameter + "_value" @ 183..189: Variable + "value" @ 196..201: Method + "setter" @ 202..208: Method + "value" @ 217..222: Method [definition] + "self" @ 223..227: SelfParameter [definition] + "new_value" @ 229..238: Parameter [definition] + "int" @ 240..243: Class + "None" @ 248..252: BuiltinConstant + "self" @ 262..266: SelfParameter + "_value" @ 267..273: Variable + "new_value" @ 276..285: Parameter + "obj" @ 287..290: Variable [definition] + "ReadOnly" @ 293..301: Class + "random" @ 307..313: Variable + "ReadWrite" @ 321..330: Class + "x" @ 333..334: Variable [definition] + "obj" @ 337..340: Variable + "value" @ 341..346: Property "#); } @@ -2024,7 +2230,7 @@ x = foobar_cls.prop # prop should be property "CONSTANT" @ 470..478: Variable [readonly] "w" @ 561..562: Variable [definition] "foobar" @ 565..571: Variable - "prop" @ 572..576: Variable + "prop" @ 572..576: Property [readonly] "foobar_cls" @ 636..646: Variable [definition] "Foo" @ 649..652: Class "random" @ 656..662: Variable @@ -2034,7 +2240,7 @@ x = foobar_cls.prop # prop should be property "method" @ 689..695: Method "x" @ 760..761: Variable [definition] "foobar_cls" @ 764..774: Variable - "prop" @ 775..779: Property + "prop" @ 775..779: Property [readonly] "#); } @@ -2112,10 +2318,10 @@ q = Baz.prop # prop should be property on the class as well "CONSTANT" @ 502..510: Variable [readonly] "r" @ 558..559: Variable [definition] "baz" @ 562..565: Variable - "prop" @ 566..570: Variable + "prop" @ 566..570: Property [readonly] "q" @ 604..605: Variable [definition] "Baz" @ 608..611: Class - "prop" @ 612..616: Property + "prop" @ 612..616: Property [readonly] "#); } @@ -2148,7 +2354,7 @@ class Baz: prop: str = \"hello\" baz = Baz() -s = baz.method +s = baz.method t = baz.CONSTANT r = baz.prop q = Baz.prop @@ -2189,15 +2395,15 @@ q = Baz.prop "s" @ 392..393: Variable [definition] "baz" @ 396..399: Variable "method" @ 400..406: Variable - "t" @ 408..409: Variable [definition] - "baz" @ 412..415: Variable - "CONSTANT" @ 416..424: Variable [readonly] - "r" @ 425..426: Variable [definition] - "baz" @ 429..432: Variable - "prop" @ 433..437: Variable - "q" @ 438..439: Variable [definition] - "Baz" @ 442..445: Class - "prop" @ 446..450: Variable + "t" @ 407..408: Variable [definition] + "baz" @ 411..414: Variable + "CONSTANT" @ 415..423: Variable [readonly] + "r" @ 424..425: Variable [definition] + "baz" @ 428..431: Variable + "prop" @ 432..436: Variable + "q" @ 437..438: Variable [definition] + "Baz" @ 441..444: Class + "prop" @ 445..449: Variable "#); } @@ -2383,7 +2589,7 @@ class MyClass: def __init__(self): pass """unrelated string""" - + x: str = "hello" "#, ); @@ -2414,7 +2620,7 @@ What a good module wooo def my_func(): pass """unrelated string""" - + x: str = "hello" "#, ); @@ -3050,10 +3256,10 @@ class BoundedContainer[T: int, U = str]: "wrapper" @ 339..346: Function [definition] "args" @ 348..352: Parameter [definition] "P" @ 354..355: Variable - "args" @ 356..360: Variable + "args" @ 356..360: Property [readonly] "kwargs" @ 364..370: Parameter [definition] "P" @ 372..373: Variable - "kwargs" @ 374..380: Variable + "kwargs" @ 374..380: Property [readonly] "str" @ 385..388: Class "str" @ 405..408: Class "func" @ 409..413: Parameter diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index 91bfabf8fc5ad7..903981b1e56cb2 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -455,8 +455,8 @@ pub(crate) use todo_type; /// Represents an instance of `builtins.property`. #[salsa::interned(debug, heap_size=ruff_memory_usage::heap_size)] pub struct PropertyInstanceType<'db> { - getter: Option>, - setter: Option>, + pub getter: Option>, + pub setter: Option>, } fn walk_property_instance_type<'db, V: visitor::TypeVisitor<'db> + ?Sized>( @@ -1170,6 +1170,13 @@ impl<'db> Type<'db> { } } + pub const fn as_property_instance(self) -> Option> { + match self { + Type::PropertyInstance(property) => Some(property), + _ => None, + } + } + pub const fn as_class_literal(self) -> Option> { match self { Type::ClassLiteral(class_type) => Some(class_type), diff --git a/crates/ty_python_semantic/src/types/ide_support.rs b/crates/ty_python_semantic/src/types/ide_support.rs index 5eb5e6249789fb..df8dd699804bcc 100644 --- a/crates/ty_python_semantic/src/types/ide_support.rs +++ b/crates/ty_python_semantic/src/types/ide_support.rs @@ -327,6 +327,18 @@ pub fn definitions_for_attribute<'db>( resolved } +/// Returns the descriptor object type for an attribute expression `x.y`, without invoking the +/// descriptor protocol. This corresponds to `inspect.getattr_static(x, "y")` at the type level. +pub fn static_member_type_for_attribute<'db>( + model: &SemanticModel<'db>, + attribute: &ast::ExprAttribute, +) -> Option> { + let lhs_ty = attribute.value.inferred_type(model)?; + lhs_ty + .static_member(model.db(), attribute.attr.as_str()) + .ignore_possibly_undefined() +} + fn definitions_for_attribute_in_class_hierarchy<'db>( class_literal: &ClassLiteral<'db>, model: &SemanticModel<'db>,