-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Show class that originally defined attribute in output #6806
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
9f12405
46cf534
4cb85e8
2969caa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -929,8 +929,8 @@ def check_func_def(self, defn: FuncItem, typ: CallableType, name: Optional[str]) | |
| msg += "tuple argument {}".format(name[12:]) | ||
| else: | ||
| msg += 'argument "{}"'.format(name) | ||
| self.check_simple_assignment(arg.variable.type, arg.initializer, | ||
| context=arg, msg=msg, lvalue_name='argument', rvalue_name='default') | ||
| self.check_simple_assignment(arg.variable.type, arg.initializer, context=arg, | ||
| msg=msg, lvalue_name='argument has type', rvalue_name='default has type') | ||
|
|
||
| # Type check body in a new scope. | ||
| with self.binder.top_frame_context(): | ||
|
|
@@ -1765,8 +1765,8 @@ def check_import(self, node: ImportBase) -> None: | |
| message = '{} "{}"'.format(message_registry.INCOMPATIBLE_IMPORT_OF, | ||
| cast(NameExpr, assign.rvalue).name) | ||
| self.check_simple_assignment(lvalue_type, assign.rvalue, node, | ||
| msg=message, lvalue_name='local name', | ||
| rvalue_name='imported name') | ||
| msg=message, lvalue_name='local name has type', | ||
| rvalue_name='imported name has type') | ||
|
|
||
| # | ||
| # Statements | ||
|
|
@@ -2025,7 +2025,7 @@ def check_compatibility_super(self, lvalue: RefExpr, lvalue_type: Optional[Type] | |
| return self.check_subtype(compare_type, base_type, lvalue, | ||
| message_registry.INCOMPATIBLE_TYPES_IN_ASSIGNMENT, | ||
| 'expression has type', | ||
| 'base class "%s" defined the type as' % base.name()) | ||
| message_registry.PARENT_CLASS_MISMATCH.format(base.name())) | ||
| return True | ||
|
|
||
| def lvalue_type_from_base(self, expr_node: Var, | ||
|
|
@@ -2579,8 +2579,8 @@ def set_inference_error_fallback_type(self, var: Var, lvalue: Lvalue, type: Type | |
| def check_simple_assignment(self, lvalue_type: Optional[Type], rvalue: Expression, | ||
| context: Context, | ||
| msg: str = message_registry.INCOMPATIBLE_TYPES_IN_ASSIGNMENT, | ||
| lvalue_name: str = 'variable', | ||
| rvalue_name: str = 'expression') -> Type: | ||
| lvalue_name: str = 'variable has type', | ||
| rvalue_name: str = 'expression has type') -> Type: | ||
| if self.is_stub and isinstance(rvalue, EllipsisExpr): | ||
| # '...' is always a valid initializer in a stub. | ||
| return AnyType(TypeOfAny.special_form) | ||
|
|
@@ -2593,13 +2593,13 @@ def check_simple_assignment(self, lvalue_type: Optional[Type], rvalue: Expressio | |
| if isinstance(lvalue_type, DeletedType): | ||
| self.msg.deleted_as_lvalue(lvalue_type, context) | ||
| elif lvalue_type: | ||
| self.check_subtype(rvalue_type, lvalue_type, context, msg, | ||
| '{} has type'.format(rvalue_name), | ||
| '{} has type'.format(lvalue_name)) | ||
| self.check_subtype(rvalue_type, lvalue_type, context, msg, rvalue_name, | ||
| lvalue_name) | ||
| return rvalue_type | ||
|
|
||
| def check_member_assignment(self, instance_type: Type, attribute_type: Type, | ||
| rvalue: Expression, context: Context) -> Tuple[Type, Type, bool]: | ||
| rvalue: Expression, lvalue: MemberExpr | ||
| ) -> Tuple[Type, Type, bool]: | ||
| """Type member assignment. | ||
|
|
||
| This defers to check_simple_assignment, unless the member expression | ||
|
|
@@ -2612,31 +2612,39 @@ def check_member_assignment(self, instance_type: Type, attribute_type: Type, | |
| care about interaction between binder and __set__(). | ||
| """ | ||
| # Descriptors don't participate in class-attribute access | ||
| if ((isinstance(instance_type, FunctionLike) and instance_type.is_type_obj()) or | ||
| isinstance(instance_type, TypeType)): | ||
| rvalue_type = self.check_simple_assignment(attribute_type, rvalue, context) | ||
| if (isinstance(instance_type, CallableType) and instance_type.is_type_obj()): | ||
| rvalue_type = self.check_parent_member_assignment(instance_type.ret_type, | ||
| attribute_type, lvalue, | ||
| attribute_type, rvalue) | ||
| return rvalue_type, attribute_type, True | ||
|
|
||
| elif isinstance(instance_type, TypeType): | ||
| rvalue_type = self.check_parent_member_assignment(instance_type.item, | ||
| attribute_type, lvalue, | ||
| attribute_type, rvalue) | ||
| return rvalue_type, attribute_type, True | ||
|
|
||
| if not isinstance(attribute_type, Instance): | ||
| # TODO: support __set__() for union types. | ||
| rvalue_type = self.check_simple_assignment(attribute_type, rvalue, context) | ||
| rvalue_type = self.check_simple_assignment(attribute_type, rvalue, lvalue) | ||
| return rvalue_type, attribute_type, True | ||
|
|
||
| get_type = analyze_descriptor_access( | ||
| instance_type, attribute_type, self.named_type, | ||
| self.msg, context, chk=self) | ||
| self.msg, lvalue, chk=self) | ||
| if not attribute_type.type.has_readable_member('__set__'): | ||
| # If there is no __set__, we type-check that the assigned value matches | ||
| # the return type of __get__. This doesn't match the python semantics, | ||
| # (which allow you to override the descriptor with any value), but preserves | ||
| # the type of accessing the attribute (even after the override). | ||
| rvalue_type = self.check_simple_assignment(get_type, rvalue, context) | ||
| rvalue_type = self.check_parent_member_assignment(instance_type, get_type, | ||
| lvalue, attribute_type, rvalue) | ||
| return rvalue_type, get_type, True | ||
|
|
||
| dunder_set = attribute_type.type.get_method('__set__') | ||
| if dunder_set is None: | ||
| self.msg.fail(message_registry.DESCRIPTOR_SET_NOT_CALLABLE.format(attribute_type), | ||
| context) | ||
| lvalue) | ||
| return AnyType(TypeOfAny.from_error), get_type, False | ||
|
|
||
| function = function_type(dunder_set, self.named_type('builtins.function')) | ||
|
|
@@ -2645,18 +2653,18 @@ def check_member_assignment(self, instance_type: Type, attribute_type: Type, | |
| dunder_set_type = expand_type_by_instance(bound_method, typ) | ||
|
|
||
| # Here we just infer the type, the result should be type-checked like a normal assignment. | ||
| # For this we use the rvalue as type context. | ||
| # For this we use the rvalue as type lvalue. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The mass replace context -> lvalue went wrong. |
||
| self.msg.disable_errors() | ||
| _, inferred_dunder_set_type = self.expr_checker.check_call( | ||
| dunder_set_type, [TempNode(instance_type), rvalue], | ||
| [nodes.ARG_POS, nodes.ARG_POS], context) | ||
| [nodes.ARG_POS, nodes.ARG_POS], lvalue) | ||
| self.msg.enable_errors() | ||
|
|
||
| # And now we type check the call second time, to show errors related | ||
| # to wrong arguments count, etc. | ||
| self.expr_checker.check_call( | ||
| dunder_set_type, [TempNode(instance_type), TempNode(AnyType(TypeOfAny.special_form))], | ||
| [nodes.ARG_POS, nodes.ARG_POS], context) | ||
| [nodes.ARG_POS, nodes.ARG_POS], lvalue) | ||
|
|
||
| # should be handled by get_method above | ||
| assert isinstance(inferred_dunder_set_type, CallableType) | ||
|
|
@@ -2670,10 +2678,53 @@ def check_member_assignment(self, instance_type: Type, attribute_type: Type, | |
| # and '__get__' type is narrower than '__set__', then we invoke the binder to narrow type | ||
| # by this assignment. Technically, this is not safe, but in practice this is | ||
| # what a user expects. | ||
| rvalue_type = self.check_simple_assignment(set_type, rvalue, context) | ||
| rvalue_type = self.check_simple_assignment(set_type, rvalue, lvalue) | ||
| infer = is_subtype(rvalue_type, get_type) and is_subtype(get_type, set_type) | ||
| return rvalue_type if infer else set_type, get_type, infer | ||
|
|
||
| def check_parent_member_assignment(self, lvalue_base_type: Type, | ||
| lvalue_type: Type, lvalue: MemberExpr, | ||
| rvalue_type: Type, rvalue: Expression) -> Type: | ||
| """Type inherited member assignment. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't parse this as English sentence, could you please reformulate? |
||
|
|
||
| This defers to check_simple_assignment if the attribute is local to the instance_type, | ||
| otherwise it will inform the user where the attribute type was originally defined. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would say explicitly that it does exactly the same as |
||
|
|
||
| Return the inferred rvalue_type. | ||
| """ | ||
| if (isinstance(lvalue_base_type, Instance) and | ||
| lvalue.name not in lvalue_base_type.type.names): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you invert this check and put the last return here? This will save one indent level for the whole function body. |
||
| lvalue_warn = message_registry.PARENT_CLASS_MISMATCH | ||
| metaclass = lvalue_base_type.type.metaclass_type | ||
| if metaclass and metaclass.has_readable_member(lvalue.name): | ||
| if lvalue.name in metaclass.type.names: | ||
| meta_attribute = metaclass.type.names[lvalue.name].node | ||
| if not (isinstance(meta_attribute, Var) and meta_attribute.info.is_generic()): | ||
| return self.check_simple_assignment( | ||
| lvalue_type, rvalue, lvalue, | ||
| msg=message_registry.INCOMPATIBLE_TYPES_IN_ASSIGNMENT, | ||
| lvalue_name=message_registry.METACLASS_MISMATCH.format( | ||
| metaclass.type.defn.name)) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This whole part is duplicated below. Can you try factoring it out? |
||
| else: | ||
| # attribute is defined further up in the metaclass MRO | ||
| lvalue_warn = 'base class "{{}}" of {}'.format( | ||
| message_registry.METACLASS_MISMATCH.format( | ||
| metaclass.type.defn.name)) | ||
| lvalue_base_type = metaclass | ||
| if lvalue_base_type.has_readable_member(lvalue.name): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this just a performance optimization? If yes, I would remove it. It will not give much speed-up and the logic in this method is already hard to follow. |
||
| for base in lvalue_base_type.type.bases: | ||
| if lvalue.name in base.type.names: | ||
| parent_context = base.type.names[lvalue.name].node | ||
| if isinstance(parent_context, Var) and parent_context.info.is_generic(): | ||
| # Attribute may have been defined in this class but its type is | ||
| # probably defined elsewhere, possibly in the instance itself. | ||
| continue | ||
| return self.check_simple_assignment( | ||
| lvalue_type, rvalue, lvalue, | ||
| msg=message_registry.INCOMPATIBLE_TYPES_IN_ASSIGNMENT, | ||
| lvalue_name=lvalue_warn.format(base.type.defn.name)) | ||
| return self.check_simple_assignment(lvalue_type, rvalue, lvalue) | ||
|
|
||
| def check_indexed_assignment(self, lvalue: IndexExpr, | ||
| rvalue: Expression, context: Context) -> None: | ||
| """Type check indexed assignment base[index] = rvalue. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2542,3 +2542,108 @@ def f() -> int: ... | |
| [file p/d.py] | ||
| import p | ||
| def f() -> int: ... | ||
|
|
||
| [case testFactory] | ||
| from typing import Type | ||
| class A: | ||
| x = "foo" | ||
| def f() -> Type[A]: | ||
| return A | ||
| def g() -> A: | ||
| return A() | ||
|
|
||
| f().x = "bar" | ||
| f().x = 0 # E: Incompatible types in assignment (expression has type "int", variable has type "str") | ||
| f()().x = "baz" | ||
| f()().x = 1 # E: Incompatible types in assignment (expression has type "int", variable has type "str") | ||
| g().x = "" | ||
| g().x = 2 # E: Incompatible types in assignment (expression has type "int", variable has type "str") | ||
|
|
||
| [case testFactoryWithInheritance] | ||
| from typing import Type | ||
| class A: | ||
| x = "foo" | ||
| class B(A): | ||
| pass | ||
| def f() -> Type[B]: | ||
| return B | ||
| def g() -> B: | ||
| return B() | ||
|
|
||
| f().x = "bar" | ||
| f().x = 0 # E: Incompatible types in assignment (expression has type "int", base class "A" defined the type as "str") | ||
| f()().x = "baz" | ||
| f()().x = 1 # E: Incompatible types in assignment (expression has type "int", base class "A" defined the type as "str") | ||
| g().x = "" | ||
| g().x = 2 # E: Incompatible types in assignment (expression has type "int", base class "A" defined the type as "str") | ||
|
|
||
| [case testRecursiveCallable] | ||
| from typing import Optional | ||
| class A: | ||
| x = 0 # Type: int | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| def __init__(self, b: bool = True) -> None: | ||
| self.b = b | ||
| def __call__(self, b: Optional[bool] = None) -> A: | ||
| if b is not None: | ||
| self.b = b | ||
| return self | ||
|
|
||
| A.x = 1 | ||
| A().x = 2 | ||
| A(True).x = 3 | ||
| A()().x = 4 | ||
| A(True)().x = 5 | ||
| A()()().x = 6 | ||
| A(True)()().x = 7 | ||
| a = A() | ||
| a().x = 8 | ||
| a = A(True) | ||
| a().x = 9 | ||
|
|
||
| A.x = "" # E: Incompatible types in assignment (expression has type "str", variable has type "int") | ||
| A().x = "" # E: Incompatible types in assignment (expression has type "str", variable has type "int") | ||
| A(False).x = "" # E: Incompatible types in assignment (expression has type "str", variable has type "int") | ||
| A()().x = "" # E: Incompatible types in assignment (expression has type "str", variable has type "int") | ||
| A(False)().x = "" # E: Incompatible types in assignment (expression has type "str", variable has type "int") | ||
| A()()().x = "" # E: Incompatible types in assignment (expression has type "str", variable has type "int") | ||
| A(False)()().x = "" # E: Incompatible types in assignment (expression has type "str", variable has type "int") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you need al these test cases? What do they test? |
||
| a = A() | ||
| a().x = "" # E: Incompatible types in assignment (expression has type "str", variable has type "int") | ||
| a = A() | ||
| a(False).x = "" # E: Incompatible types in assignment (expression has type "str", variable has type "int") | ||
|
|
||
| A.b = True | ||
| A(True) | ||
| A(True).b = False | ||
| A(True)(False) | ||
| A(True)(False)(True).b = False | ||
| A(True)(None) | ||
| A(True)().b = False | ||
| A(True)(None).b = True | ||
| A(True)(None).b = 1 # E: Incompatible types in assignment (expression has type "int", variable has type "bool") | ||
| A(True)()(False).b = "" # E: Incompatible types in assignment (expression has type "str", variable has type "bool") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same question here. |
||
|
|
||
| [builtins fixtures/bool.pyi] | ||
|
|
||
| [case testMultiLayerCallable] | ||
| class A: | ||
| y = 0 # Type: int | ||
| class B(A): | ||
| def __init__(self, z: int) -> None: | ||
| self.z = z | ||
| def foo(x: int) -> B: | ||
| return B(x) | ||
| def bar(x: int) -> B: | ||
| return foo(x) | ||
|
|
||
| a = bar(1) | ||
| a.z = 2 | ||
| a.z = "" # E: Incompatible types in assignment (expression has type "str", variable has type "int") | ||
| a.y = 2 | ||
| a.y = "" # E: Incompatible types in assignment (expression has type "str", base class "A" defined the type as "int") | ||
|
|
||
| b = foo(3) | ||
| a.z = 4 | ||
| a.z = "" # E: Incompatible types in assignment (expression has type "str", variable has type "int") | ||
| a.y = 4 | ||
| a.y = "" # E: Incompatible types in assignment (expression has type "str", base class "A" defined the type as "int") | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have a test case for a situation where
__get__is involved?