From d35e2118bfa4bf88994446c68754ee8945746c4d Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Mon, 3 Sep 2018 15:56:05 +0100 Subject: [PATCH 1/3] Original node wins in case of a redefinition --- mypy/semanal.py | 8 ++ test-data/unit/check-incremental.test | 107 ++++++++++++++++++++++++++ 2 files changed, 115 insertions(+) diff --git a/mypy/semanal.py b/mypy/semanal.py index 3120a7b6a595..a92780d48302 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -3284,8 +3284,14 @@ def add_symbol(self, name: str, node: SymbolTableNode, if not (node.kind == MODULE_REF and self.locals[-1][name].node == node.node): self.name_already_defined(name, context, self.locals[-1][name]) + return self.locals[-1][name] = node elif self.type: + existing = self.type.names.get(name) + if existing and isinstance(existing.node, TypeInfo) and existing.node != node.node: + # Classes can never be redefined + self.name_already_defined(name, context, existing) + return self.type.names[name] = node else: existing = self.globals.get(name) @@ -3301,6 +3307,7 @@ def add_symbol(self, name: str, node: SymbolTableNode, ok = True if not ok: self.name_already_defined(name, context, existing) + return self.globals[name] = node def add_local(self, node: Union[Var, FuncDef, OverloadedFuncDef], ctx: Context) -> None: @@ -3308,6 +3315,7 @@ def add_local(self, node: Union[Var, FuncDef, OverloadedFuncDef], ctx: Context) name = node.name() if name in self.locals[-1]: self.name_already_defined(name, ctx, self.locals[-1][name]) + return node._fullname = name self.locals[-1][name] = SymbolTableNode(LDEF, node) diff --git a/test-data/unit/check-incremental.test b/test-data/unit/check-incremental.test index 7be110c72cae..5fd707e4c34d 100644 --- a/test-data/unit/check-incremental.test +++ b/test-data/unit/check-incremental.test @@ -4884,3 +4884,110 @@ def f(x: str) -> None: pass [out] [out2] main:2: error: Argument 1 to "f" has incompatible type "int"; expected "str" + +-- These tests should just not crash +[case testOverrideByBadVar] +import a +[file a.py] +import lib +x = 1 +[file a.py.2] +import lib +x = 2 +[file lib.py] +class Slow: + pass + +s: Slow +from cext import Slow # type: ignore +[out] +[out2] + +[case testOverrideByBadVarAlias] +import a +[file a.py] +import lib +x = 1 +[file a.py.2] +import lib +x = 2 +[file lib.py] +class Slow: + pass + +A = Slow +from cext import Slow # type: ignore +[out] +[out2] + +[case testOverrideByBadVarClass] +import a +[file a.py] +import lib +x = 1 +[file a.py.2] +import lib +x = 2 +[file lib.py] +class C: + class Slow: + pass + s: Slow + from cext import Slow # type: ignore +[out] +[out2] + +[case testOverrideByBadVarClassAlias] +import a +[file a.py] +import lib +x = 1 +[file a.py.2] +import lib +x = 2 +[file lib.py] +class C: + class Slow: + pass + A = Slow + from cext import Slow # type: ignore +[out] +[out2] + +[case testOverrideByBadVarExisting] +import a +[file a.py] +import lib +x = 1 +[file a.py.2] +import lib +x = 2 +[file lib.py] +class Slow: + pass + +s: Slow +from cext import Slow # type: ignore +[file cext.py] +Slow = 1 +[out] +[out2] + +[case testOverrideByBadVarAliasExisting] +import a +[file a.py] +import lib +x = 1 +[file a.py.2] +import lib +x = 2 +[file lib.py] +class Slow: + pass + +A = Slow +from cext import Slow # type: ignore +[file cext.py] +Slow = 1 +[out] +[out2] From a698c55c7af3e880e556727052e1b15f0a19c072 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Tue, 4 Sep 2018 14:21:06 +0100 Subject: [PATCH 2/3] Remove exception for classes and sync first pass --- mypy/semanal.py | 3 +-- mypy/semanal_pass1.py | 2 ++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/mypy/semanal.py b/mypy/semanal.py index a92780d48302..6fe47d2957b2 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -3288,8 +3288,7 @@ def add_symbol(self, name: str, node: SymbolTableNode, self.locals[-1][name] = node elif self.type: existing = self.type.names.get(name) - if existing and isinstance(existing.node, TypeInfo) and existing.node != node.node: - # Classes can never be redefined + if existing and existing.node != node.node: self.name_already_defined(name, context, existing) return self.type.names[name] = node diff --git a/mypy/semanal_pass1.py b/mypy/semanal_pass1.py index 87e728f69e5e..59e9dfa714af 100644 --- a/mypy/semanal_pass1.py +++ b/mypy/semanal_pass1.py @@ -348,6 +348,7 @@ def add_symbol(self, name: str, node: SymbolTableNode, if not (node.kind == MODULE_REF and self.sem.locals[-1][name].node == node.node): self.sem.name_already_defined(name, context, self.sem.locals[-1][name]) + return self.sem.locals[-1][name] = node else: assert self.sem.type is None # Pass 1 doesn't look inside classes @@ -364,5 +365,6 @@ def add_symbol(self, name: str, node: SymbolTableNode, ok = True if not ok: self.sem.name_already_defined(name, context, existing) + return elif not existing: self.sem.globals[name] = node From 898bcc519eecff74ab32a57cf38bc71f155bb979 Mon Sep 17 00:00:00 2001 From: Ivan Levkivskyi Date: Tue, 4 Sep 2018 15:46:34 +0100 Subject: [PATCH 3/3] Take care of redefinition by a method; tests; comments --- mypy/semanal.py | 7 +++++- test-data/unit/check-incremental.test | 36 +++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/mypy/semanal.py b/mypy/semanal.py index 6fe47d2957b2..3b9b96ca0ba1 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -387,10 +387,12 @@ def _visit_func_def(self, defn: FuncDef) -> None: # a common stack of namespaces. As the 3 kinds of namespaces have # different semantics, this wouldn't always work, but it might still # be a win. + # Also we can re-use some logic in self.add_symbol(). if self.is_class_scope(): # Method definition assert self.type is not None, "Type not set at class scope" defn.info = self.type + add_symbol = True if not defn.is_decorated and not defn.is_overload: if (defn.name() in self.type.names and self.type.names[defn.name()].node != defn): @@ -399,7 +401,9 @@ def _visit_func_def(self, defn: FuncDef) -> None: if not self.set_original_def(n, defn): self.name_already_defined(defn.name(), defn, self.type.names[defn.name()]) - self.type.names[defn.name()] = SymbolTableNode(MDEF, defn) + add_symbol = False + if add_symbol: + self.type.names[defn.name()] = SymbolTableNode(MDEF, defn) self.prepare_method_signature(defn, self.type) elif self.is_func_scope(): # Nested function @@ -3277,6 +3281,7 @@ def add_symbol(self, name: str, node: SymbolTableNode, context: Context) -> None: # NOTE: This logic mostly parallels SemanticAnalyzerPass1.add_symbol. If you change # this, you may have to change the other method as well. + # TODO: Combine these methods in the first and second pass into a single one. if self.is_func_scope(): assert self.locals[-1] is not None if name in self.locals[-1]: diff --git a/test-data/unit/check-incremental.test b/test-data/unit/check-incremental.test index 63a631124510..ccd55b8666c5 100644 --- a/test-data/unit/check-incremental.test +++ b/test-data/unit/check-incremental.test @@ -4992,6 +4992,42 @@ Slow = 1 [out] [out2] +[case testOverrideByBadFunction] +import a +[file a.py] +import lib +x = 1 +[file a.py.2] +import lib +x = 2 +[file lib.py] +class C: + class Slow: + pass + + s: Slow + def Slow() -> None: ... # type: ignore +[out] +[out2] + +[case testOverrideByBadVarLocal] +import a +[file a.py] +import lib +x = 1 +[file a.py.2] +import lib +x = 2 +[file lib.py] +def outer() -> None: + class Slow: + pass + + s: Slow + from cext import Slow # type: ignore +[out] +[out2] + [case testFollowImportSkipNotInvalidatedOnPresent] # flags: --follow-imports=skip # cmd: mypy -m main