From a7cd7e2f996d891228b6aeb72dba2958e1ef2523 Mon Sep 17 00:00:00 2001 From: David Peter Date: Mon, 13 Jan 2025 15:39:20 +0100 Subject: [PATCH 01/18] [red-knot] Initial tests for instance attributes --- .../unsupported_type_qualifiers.md | 5 +- .../resources/mdtest/attributes.md | 232 +++++++++++++++++- .../src/types/infer.rs | 4 +- 3 files changed, 228 insertions(+), 13 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/annotations/unsupported_type_qualifiers.md b/crates/red_knot_python_semantic/resources/mdtest/annotations/unsupported_type_qualifiers.md index 0a61157d62d9d7..7065406d60a102 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/annotations/unsupported_type_qualifiers.md +++ b/crates/red_knot_python_semantic/resources/mdtest/annotations/unsupported_type_qualifiers.md @@ -6,14 +6,11 @@ Several type qualifiers are unsupported by red-knot currently. However, we also false-positive errors if you use one in an annotation: ```py -from typing_extensions import Final, ClassVar, Required, NotRequired, ReadOnly, TypedDict +from typing_extensions import Final, Required, NotRequired, ReadOnly, TypedDict X: Final = 42 Y: Final[int] = 42 -class Foo: - A: ClassVar[int] = 42 - # TODO: `TypedDict` is actually valid as a base # error: [invalid-base] class Bar(TypedDict): diff --git a/crates/red_knot_python_semantic/resources/mdtest/attributes.md b/crates/red_knot_python_semantic/resources/mdtest/attributes.md index c79cff95095223..b2a2a5d34ab4f1 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/attributes.md +++ b/crates/red_knot_python_semantic/resources/mdtest/attributes.md @@ -2,6 +2,212 @@ Tests for attribute access on various kinds of types. +## Class and instance variables + +### Pure instance variables + +#### Variable only declared/defined in `__init__` + +Variables only defined in `__init__` are pure instance variables. They can not be accessed on the +class itself. + +```py +class C: + def __init__(self, value2: int) -> None: + self.pure_instance_variable1 = "value set in __init__" + self.pure_instance_variable2 = value2 + self.pure_instance_variable3: bytes + +c_instance = C(1) + +# TODO: should be `Literal["value set in __init__"]` (or `str` which would probably be more generally useful) +reveal_type(c_instance.pure_instance_variable1) # revealed: @Todo(instance attributes) + +# TODO: should be `int` +reveal_type(c_instance.pure_instance_variable2) # revealed: @Todo(instance attributes) + +# TODO: should be `bytes` +reveal_type(c_instance.pure_instance_variable3) # revealed: @Todo(instance attributes) + +c_instance.pure_instance_variable1 = "value set on instance" + +# TODO: this should be an error (incompatible types in assignment) +c_instance.pure_instance_variable2 = "incompatible" + +# TODO: we already show an error here but the message might be improved? +# mypy shows no error here, but pyright raises "reportAttributeAccessIssue" +# error: [unresolved-attribute] "Type `Literal[C]` has no attribute `pure_instance_variable1`" +reveal_type(C.pure_instance_variable1) # revealed: Unknown + +# TODO: this should be an error (pure instance variables can not be accessed on the class) +# mypy shows no error here, but pyright raises "reportAttributeAccessIssue" +C.pure_instance_variable1 = "overwritten on class" + +# TODO: should ideally be `Literal["value set on instance"]` +reveal_type(c_instance.pure_instance_variable1) # revealed: @Todo(instance attributes) +``` + +#### Variable declared in class body and defined in `__init__` + +The same rule applies even if the variable is *declared* (not defined!) in the class body: it is +still a pure instance variable. + +```py +class C: + pure_instance_variable: str + + def __init__(self) -> None: + self.pure_instance_variable = "value set in __init__" + +c_instance = C() + +# TODO: should be `Literal["value set in __init__"]` (or `str` which would probably be more generally useful) +reveal_type(c_instance.pure_instance_variable) # revealed: @Todo(instance attributes) + +# TODO: we currently plan to emit a diagnostic here. Note that both mypy +# and pyright show no error in this case! So we may reconsider this in +# the future, if it turns out to produce too many false positives. +reveal_type(C.pure_instance_variable) # revealed: str + +# TODO: same as above. We plan to emit a diagnostic here, even if both mypy +# and pyright allow this. +C.pure_instance_variable = "overwritten on class" + +# TODO: this should be an error (incompatible types in assignment) +c_instance.pure_instance_variable = 1 +``` + +#### Variable declared in class body and defined in unrelated method + +We also recognize pure instance variables if they are defined in a method that is not `__init__`. + +```py +class C: + pure_instance_variable: str + + def set_instance_variable(self) -> None: + self.pure_instance_variable = "value set in method" + +c_instance = C() + +# for a more realistic example, let's actually call the method +c_instance.set_instance_variable() + +# TODO: should be `str` +reveal_type(c_instance.pure_instance_variable) # revealed: @Todo(instance attributes) + +# TODO: See above, we currently plan to emit diagnostics for both of these lines, +# even if mypy/pyright do not. +reveal_type(C.pure_instance_variable) # revealed: str +C.pure_instance_variable = "overwritten on class" +``` + +### Pure class variables (`ClassVar`) + +#### Annotated with `ClassVar` type qualifier + +Class variables annotated with the [`typing.ClassVar`] type qualifier are pure class variables. They +can not be accessed on instances. + +```py +from typing import ClassVar + +class C: + pure_class_variable: ClassVar[str] = "value in class body" + +reveal_type(C.pure_class_variable) # revealed: str + +c_instance = C() + +# TODO: This should be `str`. It is okay to access a pure class variable on an instance. +reveal_type(c_instance.pure_class_variable) # revealed: @Todo(instance attributes) + +# TODO: should raise an error. It is not allowed to reassign a pure class variable on an instance. +c_instance.pure_class_variable = "value set on instance" + +C.pure_class_variable = "overwritten on class" + +# TODO: should ideally be `Literal["overwritten on class"]`, but not a priority +reveal_type(C.pure_class_variable) # revealed: str + +# TODO: should raise an error (incompatible types in assignment) +C.pure_class_variable = 1 +``` + +#### Variable only mentioned in a class method + +We also consider a class variable to be a pure class variable if it is only mentioned in a class +method. + +```py +class C: + @classmethod + def class_method(cls): + cls.pure_class_variable = "value set in class method" + +# for a more realistic example, let's actually call the method +C.class_method() + +# TODO: mypy shows an error here, pyright does not. What should we do? +# error: [unresolved-attribute] +reveal_type(C.pure_class_variable) # revealed: Unknown + +C.pure_class_variable = "overwritten on class" + +# TODO: should be `Literal["overwritten on class"]` +# error: [unresolved-attribute] +reveal_type(C.pure_class_variable) # revealed: Unknown + +c_instance = C() +# TODO: should be `Literal["overwritten on class"]` or `str` +reveal_type(c_instance.pure_class_variable) # revealed: @Todo(instance attributes) + +# TODO: should raise an error. +c_instance.pure_class_variable = "value set on instance" +``` + +### "Regular" class variables + +These are instance attributes, but the fact that we can see that they have a definition (not just a +declaration) in the class body means that reading the value from the class body is also permitted. +This is the only difference for these attributes as opposed to "pure" instance attributes. + +#### Basic + +```py +class C: + regular_class_variable: str = "value in class body" + + def instance_method(self): + self.regular_class_variable = "value set in instance method" + + @classmethod + def class_method(cls): + cls.regular_class_variable = "value set in class method" + +reveal_type(C.regular_class_variable) # revealed: str + +c_instance = C() + +# TODO: should be `str` +reveal_type(c_instance.regular_class_variable) # revealed: @Todo(instance attributes) + +c_instance.regular_class_variable = "value set on instance" + +reveal_type(C.regular_class_variable) # revealed: str + +# TODO: should ideally be Literal["value set on instance"], or still `str` +reveal_type(c_instance.regular_class_variable) # revealed: @Todo(instance attributes) + +C.regular_class_variable = "overwritten on class" + +# TODO: should ideally be `Literal["overwritten on class"]` +reveal_type(C.regular_class_variable) # revealed: str + +# TODO: should still be `Literal["value set on instance"]` +reveal_type(c_instance.regular_class_variable) # revealed: @Todo(instance attributes) +``` + ## Union of attributes ```py @@ -24,7 +230,9 @@ def _(flag: bool): reveal_type(C2.x) # revealed: Literal[3, 4] ``` -## Inherited attributes +## Inherited class attributes + +### Basic ```py class A: @@ -36,7 +244,7 @@ class C(B): ... reveal_type(C.X) # revealed: Literal["foo"] ``` -## Inherited attributes (multiple inheritance) +### Multiple inheritance ```py class O: ... @@ -104,7 +312,7 @@ def _(flag: bool, flag1: bool, flag2: bool): reveal_type(C.x) # revealed: Literal[1, 2, 3] ``` -## Unions with all paths unbound +### Unions with all paths unbound If the symbol is unbound in all elements of the union, we detect that: @@ -158,7 +366,9 @@ class Foo: ... reveal_type(Foo.__class__) # revealed: Literal[type] ``` -## Function-literal attributes +## Literal types + +### Function-literal attributes Most attribute accesses on function-literal types are delegated to `types.FunctionType`, since all functions are instances of that class: @@ -179,7 +389,7 @@ reveal_type(f.__get__) # revealed: @Todo(`__get__` method on functions) reveal_type(f.__call__) # revealed: @Todo(`__call__` method on functions) ``` -## Int-literal attributes +### Int-literal attributes Most attribute accesses on int-literal types are delegated to `builtins.int`, since all literal integers are instances of that class: @@ -196,7 +406,7 @@ reveal_type((2).numerator) # revealed: Literal[2] reveal_type((2).real) # revealed: Literal[2] ``` -## Literal `bool` attributes +### Bool-literal attributes Most attribute accesses on bool-literal types are delegated to `builtins.bool`, since all literal bols are instances of that class: @@ -213,7 +423,7 @@ reveal_type(True.numerator) # revealed: Literal[1] reveal_type(False.real) # revealed: Literal[0] ``` -## Bytes-literal attributes +### Bytes-literal attributes All attribute access on literal `bytes` types is currently delegated to `buitins.bytes`: @@ -221,3 +431,11 @@ All attribute access on literal `bytes` types is currently delegated to `buitins reveal_type(b"foo".join) # revealed: @Todo(instance attributes) reveal_type(b"foo".endswith) # revealed: @Todo(instance attributes) ``` + +## References + +Some of the tests in the *Class and instance variables* section draw inspiration from +[pyright's documentation] on this topic. + +[pyright's documentation]: https://microsoft.github.io/pyright/#/type-concepts-advanced?id=class-and-instance-variables +[`typing.classvar`]: https://docs.python.org/3/library/typing.html#typing.ClassVar diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 194ce6c6347489..17bf3caabb0b54 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -5346,8 +5346,8 @@ impl<'db> TypeInferenceBuilder<'db> { todo_type!("`NotRequired[]` type qualifier") } KnownInstanceType::ClassVar => { - self.infer_type_expression(arguments_slice); - todo_type!("`ClassVar[]` type qualifier") + let ty = self.infer_type_expression(arguments_slice); + ty } KnownInstanceType::Final => { self.infer_type_expression(arguments_slice); From 72bc9dd6e0d341f4aae9da1cdc3a48dc5e15d3f2 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 15 Jan 2025 14:40:24 +0100 Subject: [PATCH 02/18] Definition => binding --- crates/red_knot_python_semantic/resources/mdtest/attributes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/attributes.md b/crates/red_knot_python_semantic/resources/mdtest/attributes.md index b2a2a5d34ab4f1..e19e1b1df605f3 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/attributes.md +++ b/crates/red_knot_python_semantic/resources/mdtest/attributes.md @@ -168,7 +168,7 @@ c_instance.pure_class_variable = "value set on instance" ### "Regular" class variables -These are instance attributes, but the fact that we can see that they have a definition (not just a +These are instance attributes, but the fact that we can see that they have a binding (not a declaration) in the class body means that reading the value from the class body is also permitted. This is the only difference for these attributes as opposed to "pure" instance attributes. From 32ada584abaa3fe5e519dec4d6fd4a547bd3fcf0 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 15 Jan 2025 14:40:36 +0100 Subject: [PATCH 03/18] Remove edge case for now --- .../red_knot_python_semantic/resources/mdtest/attributes.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/attributes.md b/crates/red_knot_python_semantic/resources/mdtest/attributes.md index e19e1b1df605f3..e9b05f26ef6395 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/attributes.md +++ b/crates/red_knot_python_semantic/resources/mdtest/attributes.md @@ -181,10 +181,6 @@ class C: def instance_method(self): self.regular_class_variable = "value set in instance method" - @classmethod - def class_method(cls): - cls.regular_class_variable = "value set in class method" - reveal_type(C.regular_class_variable) # revealed: str c_instance = C() From d7845e25a20c0efd5262cdd35a730e5e1909dc62 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 15 Jan 2025 14:43:58 +0100 Subject: [PATCH 04/18] Add declared-and-bound example --- .../resources/mdtest/attributes.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/crates/red_knot_python_semantic/resources/mdtest/attributes.md b/crates/red_knot_python_semantic/resources/mdtest/attributes.md index e9b05f26ef6395..3302548948b919 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/attributes.md +++ b/crates/red_knot_python_semantic/resources/mdtest/attributes.md @@ -14,10 +14,18 @@ class itself. ```py class C: def __init__(self, value2: int) -> None: + # bound but not declared self.pure_instance_variable1 = "value set in __init__" + + # bound but not declared - with type inferred from parameter self.pure_instance_variable2 = value2 + + # declared but not bound self.pure_instance_variable3: bytes + # declared and bound + self.pure_instance_variable4: bool = True + c_instance = C(1) # TODO: should be `Literal["value set in __init__"]` (or `str` which would probably be more generally useful) @@ -29,6 +37,9 @@ reveal_type(c_instance.pure_instance_variable2) # revealed: @Todo(instance attr # TODO: should be `bytes` reveal_type(c_instance.pure_instance_variable3) # revealed: @Todo(instance attributes) +# TODO: should be `Literal[True]` (or `bool`) +reveal_type(c_instance.pure_instance_variable4) # revealed: @Todo(instance attributes) + c_instance.pure_instance_variable1 = "value set on instance" # TODO: this should be an error (incompatible types in assignment) From 3331d3036b39e37d291fb8ec11645e03aa7c4916 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 15 Jan 2025 14:46:32 +0100 Subject: [PATCH 05/18] Add possibly undeclared/unbound --- .../resources/mdtest/attributes.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/attributes.md b/crates/red_knot_python_semantic/resources/mdtest/attributes.md index 3302548948b919..1bf741f0617da2 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/attributes.md +++ b/crates/red_knot_python_semantic/resources/mdtest/attributes.md @@ -13,7 +13,7 @@ class itself. ```py class C: - def __init__(self, value2: int) -> None: + def __init__(self, value2: int, flag: bool = False) -> None: # bound but not declared self.pure_instance_variable1 = "value set in __init__" @@ -26,6 +26,10 @@ class C: # declared and bound self.pure_instance_variable4: bool = True + # possibly unbdeclared/unbound + if flag: + self.pure_instance_variable5: str = "possibly set in __init__" + c_instance = C(1) # TODO: should be `Literal["value set in __init__"]` (or `str` which would probably be more generally useful) @@ -40,6 +44,11 @@ reveal_type(c_instance.pure_instance_variable3) # revealed: @Todo(instance attr # TODO: should be `Literal[True]` (or `bool`) reveal_type(c_instance.pure_instance_variable4) # revealed: @Todo(instance attributes) +# TODO: should be `Literal["possibly set in __init__"]` (or `str`) +# We probably don't want to emit a diagnostic for this being possibly undeclared/unbound. +# mypy and pyright do not show an error here. +reveal_type(c_instance.pure_instance_variable5) # revealed: @Todo(instance attributes) + c_instance.pure_instance_variable1 = "value set on instance" # TODO: this should be an error (incompatible types in assignment) From cc56a965c022b5777f04c143f13a36656eb840d6 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 15 Jan 2025 14:50:45 +0100 Subject: [PATCH 06/18] accessed => overwritten --- .../red_knot_python_semantic/resources/mdtest/attributes.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/attributes.md b/crates/red_knot_python_semantic/resources/mdtest/attributes.md index 1bf741f0617da2..e53de3348c8bde 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/attributes.md +++ b/crates/red_knot_python_semantic/resources/mdtest/attributes.md @@ -8,7 +8,7 @@ Tests for attribute access on various kinds of types. #### Variable only declared/defined in `__init__` -Variables only defined in `__init__` are pure instance variables. They can not be accessed on the +Variables only defined in `__init__` are pure instance variables. They cannot be accessed on the class itself. ```py @@ -59,7 +59,7 @@ c_instance.pure_instance_variable2 = "incompatible" # error: [unresolved-attribute] "Type `Literal[C]` has no attribute `pure_instance_variable1`" reveal_type(C.pure_instance_variable1) # revealed: Unknown -# TODO: this should be an error (pure instance variables can not be accessed on the class) +# TODO: this should be an error (pure instance variables cannot be accessed on the class) # mypy shows no error here, but pyright raises "reportAttributeAccessIssue" C.pure_instance_variable1 = "overwritten on class" @@ -127,7 +127,7 @@ C.pure_instance_variable = "overwritten on class" #### Annotated with `ClassVar` type qualifier Class variables annotated with the [`typing.ClassVar`] type qualifier are pure class variables. They -can not be accessed on instances. +cannot be overwritten on instances. ```py from typing import ClassVar From 106d6132cf16b29bd5c8c78129e3b4089eac6fdb Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 15 Jan 2025 14:51:51 +0100 Subject: [PATCH 07/18] Expected type is str --- crates/red_knot_python_semantic/resources/mdtest/attributes.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/attributes.md b/crates/red_knot_python_semantic/resources/mdtest/attributes.md index e53de3348c8bde..d102e7449e5f7e 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/attributes.md +++ b/crates/red_knot_python_semantic/resources/mdtest/attributes.md @@ -64,6 +64,7 @@ reveal_type(C.pure_instance_variable1) # revealed: Unknown C.pure_instance_variable1 = "overwritten on class" # TODO: should ideally be `Literal["value set on instance"]` +# (due to earlier assignment of the attribute from the global scope) reveal_type(c_instance.pure_instance_variable1) # revealed: @Todo(instance attributes) ``` @@ -81,7 +82,7 @@ class C: c_instance = C() -# TODO: should be `Literal["value set in __init__"]` (or `str` which would probably be more generally useful) +# TODO: should be `str` reveal_type(c_instance.pure_instance_variable) # revealed: @Todo(instance attributes) # TODO: we currently plan to emit a diagnostic here. Note that both mypy From fbf4a1bc57405c2602737f247dfcc66661e77ef8 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 15 Jan 2025 15:00:14 +0100 Subject: [PATCH 08/18] Add Subclass test --- .../red_knot_python_semantic/resources/mdtest/attributes.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/red_knot_python_semantic/resources/mdtest/attributes.md b/crates/red_knot_python_semantic/resources/mdtest/attributes.md index d102e7449e5f7e..21e705e7e2ff54 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/attributes.md +++ b/crates/red_knot_python_semantic/resources/mdtest/attributes.md @@ -153,6 +153,11 @@ reveal_type(C.pure_class_variable) # revealed: str # TODO: should raise an error (incompatible types in assignment) C.pure_class_variable = 1 + +class Subclass(C): + pure_class_variable: ClassVar[str] = "overwritten on subclass" + +reveal_type(Subclass.pure_class_variable) # revealed: str ``` #### Variable only mentioned in a class method From a7d211ebe4d001f1d13e2fbd4082ba16e0a1dbb6 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 15 Jan 2025 15:00:31 +0100 Subject: [PATCH 09/18] Describe intended behavior --- crates/red_knot_python_semantic/resources/mdtest/attributes.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/attributes.md b/crates/red_knot_python_semantic/resources/mdtest/attributes.md index 21e705e7e2ff54..80f766c8dde7a5 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/attributes.md +++ b/crates/red_knot_python_semantic/resources/mdtest/attributes.md @@ -174,7 +174,8 @@ class C: # for a more realistic example, let's actually call the method C.class_method() -# TODO: mypy shows an error here, pyright does not. What should we do? +# TODO: We currently plan to support this and show no error here. +# mypy shows an error here, pyright does not. # error: [unresolved-attribute] reveal_type(C.pure_class_variable) # revealed: Unknown From a3692f72202b0795f9dbe172df13400fc824c935 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 15 Jan 2025 15:01:38 +0100 Subject: [PATCH 10/18] Do not use 'Regular class variable' terminology --- .../resources/mdtest/attributes.md | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/attributes.md b/crates/red_knot_python_semantic/resources/mdtest/attributes.md index 80f766c8dde7a5..ca042ce8694b90 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/attributes.md +++ b/crates/red_knot_python_semantic/resources/mdtest/attributes.md @@ -193,42 +193,43 @@ reveal_type(c_instance.pure_class_variable) # revealed: @Todo(instance attribut c_instance.pure_class_variable = "value set on instance" ``` -### "Regular" class variables +### Instance variables with class-level default values These are instance attributes, but the fact that we can see that they have a binding (not a -declaration) in the class body means that reading the value from the class body is also permitted. -This is the only difference for these attributes as opposed to "pure" instance attributes. +declaration) in the class body means that reading the value from the class directly is also +permitted. This is the only difference for these attributes as opposed to "pure" instance +attributes. #### Basic ```py class C: - regular_class_variable: str = "value in class body" + variable_with_class_default: str = "value in class body" def instance_method(self): - self.regular_class_variable = "value set in instance method" + self.variable_with_class_default = "value set in instance method" -reveal_type(C.regular_class_variable) # revealed: str +reveal_type(C.variable_with_class_default) # revealed: str c_instance = C() # TODO: should be `str` -reveal_type(c_instance.regular_class_variable) # revealed: @Todo(instance attributes) +reveal_type(c_instance.variable_with_class_default) # revealed: @Todo(instance attributes) -c_instance.regular_class_variable = "value set on instance" +c_instance.variable_with_class_default = "value set on instance" -reveal_type(C.regular_class_variable) # revealed: str +reveal_type(C.variable_with_class_default) # revealed: str # TODO: should ideally be Literal["value set on instance"], or still `str` -reveal_type(c_instance.regular_class_variable) # revealed: @Todo(instance attributes) +reveal_type(c_instance.variable_with_class_default) # revealed: @Todo(instance attributes) -C.regular_class_variable = "overwritten on class" +C.variable_with_class_default = "overwritten on class" # TODO: should ideally be `Literal["overwritten on class"]` -reveal_type(C.regular_class_variable) # revealed: str +reveal_type(C.variable_with_class_default) # revealed: str # TODO: should still be `Literal["value set on instance"]` -reveal_type(c_instance.regular_class_variable) # revealed: @Todo(instance attributes) +reveal_type(c_instance.variable_with_class_default) # revealed: @Todo(instance attributes) ``` ## Union of attributes From 0ba2ed0f8ea9345936554728191a9842b5b7dbd4 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 15 Jan 2025 15:16:13 +0100 Subject: [PATCH 11/18] Add pure-instance-variable case with no binding --- .../resources/mdtest/attributes.md | 46 ++++++++++++++----- 1 file changed, 34 insertions(+), 12 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/attributes.md b/crates/red_knot_python_semantic/resources/mdtest/attributes.md index ca042ce8694b90..356c6657a1b134 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/attributes.md +++ b/crates/red_knot_python_semantic/resources/mdtest/attributes.md @@ -6,10 +6,10 @@ Tests for attribute access on various kinds of types. ### Pure instance variables -#### Variable only declared/defined in `__init__` +#### Variable only declared/bound in `__init__` -Variables only defined in `__init__` are pure instance variables. They cannot be accessed on the -class itself. +Variables only declared and/or bound in `__init__` are pure instance variables. They cannot be +accessed on the class itself. ```py class C: @@ -68,10 +68,10 @@ C.pure_instance_variable1 = "overwritten on class" reveal_type(c_instance.pure_instance_variable1) # revealed: @Todo(instance attributes) ``` -#### Variable declared in class body and defined in `__init__` +#### Variable declared in class body and declared/bound in `__init__` -The same rule applies even if the variable is *declared* (not defined!) in the class body: it is -still a pure instance variable. +The same rule applies even if the variable is *declared* (not bound!) in the class body: it is still +a pure instance variable. ```py class C: @@ -98,16 +98,14 @@ C.pure_instance_variable = "overwritten on class" c_instance.pure_instance_variable = 1 ``` -#### Variable declared in class body and defined in unrelated method +#### Variable only defined in unrelated method We also recognize pure instance variables if they are defined in a method that is not `__init__`. ```py class C: - pure_instance_variable: str - def set_instance_variable(self) -> None: - self.pure_instance_variable = "value set in method" + self.pure_instance_variable: str = "value set in method" c_instance = C() @@ -117,9 +115,33 @@ c_instance.set_instance_variable() # TODO: should be `str` reveal_type(c_instance.pure_instance_variable) # revealed: @Todo(instance attributes) -# TODO: See above, we currently plan to emit diagnostics for both of these lines, -# even if mypy/pyright do not. +# TODO: We already show an error here, but the message might be improved? +# error: [unresolved-attribute] +reveal_type(C.pure_instance_variable) # revealed: Unknown + +# TODO: this should be an error +C.pure_instance_variable = "overwritten on class" +``` + +#### Variable declared in class body and not bound anywhere + +If a variable is declared in the class body but not bound anywhere, we still consider it a pure +instance variable and allow access to it via instances. + +```py +class C: + pure_instance_variable: str + +c_instance = C() + +# TODO: should be 'str' +reveal_type(c_instance.pure_instance_variable) # revealed: @Todo(instance attributes) + +# TODO: mypy and pyright do not show an error here, but we plan to emit a diagnostic. +# The type could be changed to 'Unknown' if we decide to emit an error? reveal_type(C.pure_instance_variable) # revealed: str + +# TODO: mypy and pyright do not show an error here, but we plan to emit one. C.pure_instance_variable = "overwritten on class" ``` From 5a1bcb7f41058f09cb90f93b5581af21627d4e4d Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 15 Jan 2025 15:17:37 +0100 Subject: [PATCH 12/18] Fix typo --- crates/red_knot_python_semantic/resources/mdtest/attributes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/attributes.md b/crates/red_knot_python_semantic/resources/mdtest/attributes.md index 356c6657a1b134..a8b3932e123515 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/attributes.md +++ b/crates/red_knot_python_semantic/resources/mdtest/attributes.md @@ -26,7 +26,7 @@ class C: # declared and bound self.pure_instance_variable4: bool = True - # possibly unbdeclared/unbound + # possibly undeclared/unbound if flag: self.pure_instance_variable5: str = "possibly set in __init__" From 4c82e04711828c53c0f9f7dbaf4e1fe7823c5594 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 15 Jan 2025 15:19:15 +0100 Subject: [PATCH 13/18] Add pure 'ClassVar' annotation example --- .../resources/mdtest/attributes.md | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/attributes.md b/crates/red_knot_python_semantic/resources/mdtest/attributes.md index a8b3932e123515..7e26a6e6211a3d 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/attributes.md +++ b/crates/red_knot_python_semantic/resources/mdtest/attributes.md @@ -156,30 +156,34 @@ cannot be overwritten on instances. from typing import ClassVar class C: - pure_class_variable: ClassVar[str] = "value in class body" + pure_class_variable1: ClassVar[str] = "value in class body" + pure_class_variable2: ClassVar = 1 -reveal_type(C.pure_class_variable) # revealed: str +reveal_type(C.pure_class_variable1) # revealed: str + +# TODO: this should be `Literal[1]` +reveal_type(C.pure_class_variable2) # revealed: @Todo(Unsupported or invalid type in a type expression) c_instance = C() # TODO: This should be `str`. It is okay to access a pure class variable on an instance. -reveal_type(c_instance.pure_class_variable) # revealed: @Todo(instance attributes) +reveal_type(c_instance.pure_class_variable1) # revealed: @Todo(instance attributes) # TODO: should raise an error. It is not allowed to reassign a pure class variable on an instance. -c_instance.pure_class_variable = "value set on instance" +c_instance.pure_class_variable1 = "value set on instance" -C.pure_class_variable = "overwritten on class" +C.pure_class_variable1 = "overwritten on class" # TODO: should ideally be `Literal["overwritten on class"]`, but not a priority -reveal_type(C.pure_class_variable) # revealed: str +reveal_type(C.pure_class_variable1) # revealed: str # TODO: should raise an error (incompatible types in assignment) -C.pure_class_variable = 1 +C.pure_class_variable1 = 1 class Subclass(C): - pure_class_variable: ClassVar[str] = "overwritten on subclass" + pure_class_variable1: ClassVar[str] = "overwritten on subclass" -reveal_type(Subclass.pure_class_variable) # revealed: str +reveal_type(Subclass.pure_class_variable1) # revealed: str ``` #### Variable only mentioned in a class method From 1547914dd5ef7bdcc57d1cb0df4c1a1c91794208 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 15 Jan 2025 15:21:39 +0100 Subject: [PATCH 14/18] Minor clarification --- crates/red_knot_python_semantic/resources/mdtest/attributes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/attributes.md b/crates/red_knot_python_semantic/resources/mdtest/attributes.md index 7e26a6e6211a3d..b5e9ea65f4a44f 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/attributes.md +++ b/crates/red_knot_python_semantic/resources/mdtest/attributes.md @@ -150,7 +150,7 @@ C.pure_instance_variable = "overwritten on class" #### Annotated with `ClassVar` type qualifier Class variables annotated with the [`typing.ClassVar`] type qualifier are pure class variables. They -cannot be overwritten on instances. +cannot be overwritten on instances, but they can be accessed on instances. ```py from typing import ClassVar From 2395aa92e5beab0f850111059d5da55a4b290ad6 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 15 Jan 2025 15:22:54 +0100 Subject: [PATCH 15/18] Add typing spec reference --- crates/red_knot_python_semantic/resources/mdtest/attributes.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/red_knot_python_semantic/resources/mdtest/attributes.md b/crates/red_knot_python_semantic/resources/mdtest/attributes.md index b5e9ea65f4a44f..249a7a7d76345a 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/attributes.md +++ b/crates/red_knot_python_semantic/resources/mdtest/attributes.md @@ -152,6 +152,8 @@ C.pure_instance_variable = "overwritten on class" Class variables annotated with the [`typing.ClassVar`] type qualifier are pure class variables. They cannot be overwritten on instances, but they can be accessed on instances. +For more details, see the [typing documentation on `ClassVar`]. + ```py from typing import ClassVar @@ -488,4 +490,5 @@ Some of the tests in the *Class and instance variables* section draw inspiration [pyright's documentation] on this topic. [pyright's documentation]: https://microsoft.github.io/pyright/#/type-concepts-advanced?id=class-and-instance-variables +[typing documentation on `classvar`]: https://typing.readthedocs.io/en/latest/spec/class-compat.html#classvar [`typing.classvar`]: https://docs.python.org/3/library/typing.html#typing.ClassVar From 733c1a7a390a26bf483065fe527387b32c442b1f Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 15 Jan 2025 15:35:53 +0100 Subject: [PATCH 16/18] Typing spec on 'ClassVar' --- .../red_knot_python_semantic/resources/mdtest/attributes.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/attributes.md b/crates/red_knot_python_semantic/resources/mdtest/attributes.md index 249a7a7d76345a..330406e14eeb4e 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/attributes.md +++ b/crates/red_knot_python_semantic/resources/mdtest/attributes.md @@ -152,7 +152,7 @@ C.pure_instance_variable = "overwritten on class" Class variables annotated with the [`typing.ClassVar`] type qualifier are pure class variables. They cannot be overwritten on instances, but they can be accessed on instances. -For more details, see the [typing documentation on `ClassVar`]. +For more details, see the [typing spec on `ClassVar`]. ```py from typing import ClassVar @@ -490,5 +490,5 @@ Some of the tests in the *Class and instance variables* section draw inspiration [pyright's documentation] on this topic. [pyright's documentation]: https://microsoft.github.io/pyright/#/type-concepts-advanced?id=class-and-instance-variables -[typing documentation on `classvar`]: https://typing.readthedocs.io/en/latest/spec/class-compat.html#classvar +[typing spec on `classvar`]: https://typing.readthedocs.io/en/latest/spec/class-compat.html#classvar [`typing.classvar`]: https://docs.python.org/3/library/typing.html#typing.ClassVar From 87305b12fc7a58e5e580e11bdb55cd0f357848be Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 15 Jan 2025 15:37:49 +0100 Subject: [PATCH 17/18] Other valid answers --- crates/red_knot_python_semantic/resources/mdtest/attributes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/attributes.md b/crates/red_knot_python_semantic/resources/mdtest/attributes.md index 330406e14eeb4e..3f5c83de6e2e0f 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/attributes.md +++ b/crates/red_knot_python_semantic/resources/mdtest/attributes.md @@ -163,7 +163,7 @@ class C: reveal_type(C.pure_class_variable1) # revealed: str -# TODO: this should be `Literal[1]` +# TODO: this should be `Literal[1]`, `int`, or maybe `Unknown | Literal[1]` / `Unknown | int` reveal_type(C.pure_class_variable2) # revealed: @Todo(Unsupported or invalid type in a type expression) c_instance = C() From b2c1dc908b5fa4f8109f129f6f5fe271254cea61 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 15 Jan 2025 15:38:55 +0100 Subject: [PATCH 18/18] Remove annotation --- .../red_knot_python_semantic/resources/mdtest/attributes.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/attributes.md b/crates/red_knot_python_semantic/resources/mdtest/attributes.md index 3f5c83de6e2e0f..b1db9e28d317e7 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/attributes.md +++ b/crates/red_knot_python_semantic/resources/mdtest/attributes.md @@ -105,14 +105,14 @@ We also recognize pure instance variables if they are defined in a method that i ```py class C: def set_instance_variable(self) -> None: - self.pure_instance_variable: str = "value set in method" + self.pure_instance_variable = "value set in method" c_instance = C() # for a more realistic example, let's actually call the method c_instance.set_instance_variable() -# TODO: should be `str` +# TODO: should be `Literal["value set in method"]` or `str` reveal_type(c_instance.pure_instance_variable) # revealed: @Todo(instance attributes) # TODO: We already show an error here, but the message might be improved?