From 246f393f14ac7d5c22c19b08f763ab4aa80dd02c Mon Sep 17 00:00:00 2001 From: Ekin Dursun Date: Wed, 17 Apr 2019 12:04:54 +0300 Subject: [PATCH 1/7] Fix --- mypy/newsemanal/semanal.py | 2 ++ mypy/semanal.py | 2 ++ test-data/unit/check-classes.test | 6 ++++++ test-data/unit/check-newsemanal.test | 6 ++++++ 4 files changed, 16 insertions(+) diff --git a/mypy/newsemanal/semanal.py b/mypy/newsemanal/semanal.py index 6037fc52bd2c..51346011ecd8 100644 --- a/mypy/newsemanal/semanal.py +++ b/mypy/newsemanal/semanal.py @@ -2555,6 +2555,8 @@ def analyze_member_lvalue(self, lval: MemberExpr, explicit_type: bool, is_final: if cur_node and is_final: # Overrides will be checked in type checker. self.fail("Cannot redefine an existing name as final", lval) + if cur_node and explicit_type: + self.name_already_defined('self.' + lval.name, lval, cur_node) # If the attribute of self is not defined in superclasses, create a new Var, ... if ((node is None or isinstance(node.node, Var) and node.node.is_abstract_var) or # ... also an explicit declaration on self also creates a new Var. diff --git a/mypy/semanal.py b/mypy/semanal.py index 5826ad0a3f45..c6baacb410a2 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -2288,6 +2288,8 @@ def analyze_member_lvalue(self, lval: MemberExpr, explicit_type: bool, is_final: if cur_node and is_final: # Overrides will be checked in type checker. self.fail("Cannot redefine an existing name as final", lval) + if cur_node and explicit_type: + self.name_already_defined('self.' + lval.name, lval, cur_node) # If the attribute of self is not defined in superclasses, create a new Var, ... if ((node is None or isinstance(node.node, Var) and node.node.is_abstract_var) or # ... also an explicit declaration on self also creates a new Var. diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test index e1e9e0d27517..49c61e38c031 100644 --- a/test-data/unit/check-classes.test +++ b/test-data/unit/check-classes.test @@ -5837,3 +5837,9 @@ def test() -> None: return reveal_type(x) # E: Revealed type is 'Union[Type[__main__.One], Type[__main__.Other]]' [builtins fixtures/isinstancelist.pyi] + +[case testMemberRedefinition] +class C: + def __init__(self) -> None: + self.foo = 12 + self.foo: int = 12 # E: Name 'self.foo' already defined on line 3 diff --git a/test-data/unit/check-newsemanal.test b/test-data/unit/check-newsemanal.test index 859841abc3a7..334afe36c7f6 100644 --- a/test-data/unit/check-newsemanal.test +++ b/test-data/unit/check-newsemanal.test @@ -2179,3 +2179,9 @@ else: def f(x: str) -> None: ... def f(x) -> None: y() # E: "str" not callable + +[case testNewAnalyzerClassMemberRedefinition] +class C: + def __init__(self) -> None: + self.foo = 12 + self.foo: int = 12 # E: Name 'self.foo' already defined on line 3 From 76f9c776e95452b77ab5fbedd1d13aa5ed0d0d8e Mon Sep 17 00:00:00 2001 From: Ekin Dursun Date: Wed, 17 Apr 2019 16:09:58 +0300 Subject: [PATCH 2/7] Fix and make tests pass --- mypy/newsemanal/semanal.py | 3 ++- mypy/semanal.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/mypy/newsemanal/semanal.py b/mypy/newsemanal/semanal.py index 51346011ecd8..4a611f73fb41 100644 --- a/mypy/newsemanal/semanal.py +++ b/mypy/newsemanal/semanal.py @@ -2555,7 +2555,8 @@ def analyze_member_lvalue(self, lval: MemberExpr, explicit_type: bool, is_final: if cur_node and is_final: # Overrides will be checked in type checker. self.fail("Cannot redefine an existing name as final", lval) - if cur_node and explicit_type: + if (not lval.node and cur_node and isinstance(cur_node.node, Var) and + cur_node.node.is_inferred and explicit_type): self.name_already_defined('self.' + lval.name, lval, cur_node) # If the attribute of self is not defined in superclasses, create a new Var, ... if ((node is None or isinstance(node.node, Var) and node.node.is_abstract_var) or diff --git a/mypy/semanal.py b/mypy/semanal.py index c6baacb410a2..402dc9c2aaad 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -2288,7 +2288,8 @@ def analyze_member_lvalue(self, lval: MemberExpr, explicit_type: bool, is_final: if cur_node and is_final: # Overrides will be checked in type checker. self.fail("Cannot redefine an existing name as final", lval) - if cur_node and explicit_type: + if (cur_node and isinstance(cur_node.node, Var) and cur_node.node.is_inferred and + explicit_type): self.name_already_defined('self.' + lval.name, lval, cur_node) # If the attribute of self is not defined in superclasses, create a new Var, ... if ((node is None or isinstance(node.node, Var) and node.node.is_abstract_var) or From 1f3de97903c2b2a6ac5a15445f846580585cdbd0 Mon Sep 17 00:00:00 2001 From: Ekin Dursun Date: Fri, 26 Apr 2019 12:33:50 +0300 Subject: [PATCH 3/7] Change 'Name' to 'Attribute' --- mypy/newsemanal/semanal.py | 20 +++++++++++++++----- mypy/semanal.py | 19 +++++++++++++++---- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/mypy/newsemanal/semanal.py b/mypy/newsemanal/semanal.py index 02ed47156de4..5bffef162a79 100644 --- a/mypy/newsemanal/semanal.py +++ b/mypy/newsemanal/semanal.py @@ -2565,7 +2565,7 @@ def analyze_member_lvalue(self, lval: MemberExpr, explicit_type: bool, is_final: self.fail("Cannot redefine an existing name as final", lval) if (not lval.node and cur_node and isinstance(cur_node.node, Var) and cur_node.node.is_inferred and explicit_type): - self.name_already_defined('self.' + lval.name, lval, cur_node) + self.attribute_already_defined(lval.name, lval, cur_node) # If the attribute of self is not defined in superclasses, create a new Var, ... if ((node is None or isinstance(node.node, Var) and node.node.is_abstract_var) or # ... also an explicit declaration on self also creates a new Var. @@ -4301,9 +4301,9 @@ def name_not_defined(self, name: str, ctx: Context, namespace: Optional[str] = N # Yes. Generate a helpful note. self.add_fixture_note(fullname, ctx) - def name_already_defined(self, name: str, ctx: Context, - original_ctx: Optional[Union[SymbolTableNode, SymbolNode]] = None - ) -> None: + def already_defined(self, name: str, ctx: Context, + original_ctx: Optional[Union[SymbolTableNode, SymbolNode]] = None, *, + noun: str) -> None: if isinstance(original_ctx, SymbolTableNode): node = original_ctx.node # type: Optional[SymbolNode] elif isinstance(original_ctx, SymbolNode): @@ -4322,7 +4322,17 @@ def name_already_defined(self, name: str, ctx: Context, extra_msg = ' on line {}'.format(node.line) else: extra_msg = ' (possibly by an import)' - self.fail("Name '{}' already defined{}".format(unmangle(name), extra_msg), ctx) + self.fail("{} '{}' already defined{}".format(noun, unmangle(name), extra_msg), ctx) + + def name_already_defined(self, name: str, ctx: Context, + original_ctx: Optional[Union[SymbolTableNode, SymbolNode]] = None + ) -> None: + self.already_defined(name, ctx, original_ctx, noun='Name') + + def attribute_already_defined(self, name: str, ctx: Context, + original_ctx: Optional[Union[SymbolTableNode, SymbolNode]] = None + ) -> None: + self.already_defined(name, ctx, original_ctx, noun='Attribute') def is_local_name(self, name: str) -> bool: """Does name look like reference to a definition in the current module?""" diff --git a/mypy/semanal.py b/mypy/semanal.py index 402dc9c2aaad..10fa112eab61 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -2290,7 +2290,7 @@ def analyze_member_lvalue(self, lval: MemberExpr, explicit_type: bool, is_final: self.fail("Cannot redefine an existing name as final", lval) if (cur_node and isinstance(cur_node.node, Var) and cur_node.node.is_inferred and explicit_type): - self.name_already_defined('self.' + lval.name, lval, cur_node) + self.attribute_already_defined(lval.name, lval, cur_node) # If the attribute of self is not defined in superclasses, create a new Var, ... if ((node is None or isinstance(node.node, Var) and node.node.is_abstract_var) or # ... also an explicit declaration on self also creates a new Var. @@ -3739,8 +3739,9 @@ def name_not_defined(self, name: str, ctx: Context) -> None: # Yes. Generate a helpful note. self.add_fixture_note(fullname, ctx) - def name_already_defined(self, name: str, ctx: Context, - original_ctx: Optional[Union[SymbolTableNode, SymbolNode]] = None) -> None: + def already_defined(self, name: str, ctx: Context, + original_ctx: Optional[Union[SymbolTableNode, SymbolNode]] = None, *, + noun: str) -> None: if isinstance(original_ctx, SymbolTableNode): node = original_ctx.node elif isinstance(original_ctx, SymbolNode): @@ -3755,7 +3756,17 @@ def name_already_defined(self, name: str, ctx: Context, extra_msg = ' on line {}'.format(node.line) else: extra_msg = ' (possibly by an import)' - self.fail("Name '{}' already defined{}".format(unmangle(name), extra_msg), ctx) + self.fail("{} '{}' already defined{}".format(noun, unmangle(name), extra_msg), ctx) + + def name_already_defined(self, name: str, ctx: Context, + original_ctx: Optional[Union[SymbolTableNode, SymbolNode]] = None + ) -> None: + self.already_defined(name, ctx, original_ctx, noun='Name') + + def attribute_already_defined(self, name: str, ctx: Context, + original_ctx: Optional[Union[SymbolTableNode, SymbolNode]] = None + ) -> None: + self.already_defined(name, ctx, original_ctx, noun='Attribute') def fail(self, msg: str, ctx: Context, serious: bool = False, *, blocker: bool = False) -> None: From bf042b0c5ec362fd9f74a5a19d697cc0e639ae73 Mon Sep 17 00:00:00 2001 From: Ekin Dursun Date: Fri, 26 Apr 2019 16:10:07 +0300 Subject: [PATCH 4/7] Change tests --- test-data/unit/check-classes.test | 2 +- test-data/unit/check-newsemanal.test | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test index 49c61e38c031..5f44551ea0de 100644 --- a/test-data/unit/check-classes.test +++ b/test-data/unit/check-classes.test @@ -5842,4 +5842,4 @@ def test() -> None: class C: def __init__(self) -> None: self.foo = 12 - self.foo: int = 12 # E: Name 'self.foo' already defined on line 3 + self.foo: int = 12 # E: Attribute 'foo' already defined on line 3 diff --git a/test-data/unit/check-newsemanal.test b/test-data/unit/check-newsemanal.test index 4ad794b4a61f..3b8b3db832f5 100644 --- a/test-data/unit/check-newsemanal.test +++ b/test-data/unit/check-newsemanal.test @@ -2183,8 +2183,8 @@ else: class C: def __init__(self) -> None: self.foo = 12 - self.foo: int = 12 # E: Name 'self.foo' already defined on line 3 - + self.foo: int = 12 # E: Attribute 'foo' already defined on line 3 + [case testNewAnalyzerFirstAliasTargetWins] if int(): Alias = DesiredTarget From 79e6374a46e76cb8d4848de769a1dd2159aab59f Mon Sep 17 00:00:00 2001 From: Ekin Dursun Date: Mon, 29 Apr 2019 14:08:47 +0300 Subject: [PATCH 5/7] Remove redundant test --- test-data/unit/check-newsemanal.test | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test-data/unit/check-newsemanal.test b/test-data/unit/check-newsemanal.test index 3b8b3db832f5..a192f9485eea 100644 --- a/test-data/unit/check-newsemanal.test +++ b/test-data/unit/check-newsemanal.test @@ -2179,12 +2179,6 @@ else: def f(x) -> None: y() # E: "str" not callable -[case testNewAnalyzerClassMemberRedefinition] -class C: - def __init__(self) -> None: - self.foo = 12 - self.foo: int = 12 # E: Attribute 'foo' already defined on line 3 - [case testNewAnalyzerFirstAliasTargetWins] if int(): Alias = DesiredTarget From 4b7ab06026ec2a8fc1ab481db8a5fef2bcb38ba5 Mon Sep 17 00:00:00 2001 From: Ekin Dursun Date: Mon, 29 Apr 2019 14:12:48 +0300 Subject: [PATCH 6/7] Add test --- test-data/unit/check-classes.test | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test index 22d6972a7345..f40373c30211 100644 --- a/test-data/unit/check-classes.test +++ b/test-data/unit/check-classes.test @@ -5844,6 +5844,12 @@ class C: self.foo = 12 self.foo: int = 12 # E: Attribute 'foo' already defined on line 3 +[case testMemberRedefinitionDefinedInClass] +class C: + foo = 12 + def __init__(self) -> None: + self.foo: int = 12 # E: Attribute 'foo' already defined on line 2 + [case testAbstractInit] from abc import abstractmethod, ABCMeta class A(metaclass=ABCMeta): From f8fe2178f39c08665b6de13b83fad7dbcd7cec9c Mon Sep 17 00:00:00 2001 From: Ekin Dursun Date: Tue, 30 Apr 2019 10:16:28 +0300 Subject: [PATCH 7/7] Add comments --- mypy/newsemanal/semanal.py | 2 ++ mypy/semanal.py | 2 ++ 2 files changed, 4 insertions(+) diff --git a/mypy/newsemanal/semanal.py b/mypy/newsemanal/semanal.py index eb00011ca8f9..f557e604173f 100644 --- a/mypy/newsemanal/semanal.py +++ b/mypy/newsemanal/semanal.py @@ -2563,6 +2563,8 @@ def analyze_member_lvalue(self, lval: MemberExpr, explicit_type: bool, is_final: if cur_node and is_final: # Overrides will be checked in type checker. self.fail("Cannot redefine an existing name as final", lval) + # On first encounter with this definition, if this attribute was defined before + # with an inferred type and it's marked with an explicit type now, give an error. if (not lval.node and cur_node and isinstance(cur_node.node, Var) and cur_node.node.is_inferred and explicit_type): self.attribute_already_defined(lval.name, lval, cur_node) diff --git a/mypy/semanal.py b/mypy/semanal.py index 4ea2b34f9e03..72a7c9320972 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -2288,6 +2288,8 @@ def analyze_member_lvalue(self, lval: MemberExpr, explicit_type: bool, is_final: if cur_node and is_final: # Overrides will be checked in type checker. self.fail("Cannot redefine an existing name as final", lval) + # If this attribute was defined before with an inferred type and it's marked + # with an explicit type now, give an error. if (cur_node and isinstance(cur_node.node, Var) and cur_node.node.is_inferred and explicit_type): self.attribute_already_defined(lval.name, lval, cur_node)