From fc2f5773a1656533619694e6d9e7b322d97a807b Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Wed, 21 Sep 2016 15:32:42 -0700 Subject: [PATCH 1/7] Add a new, lower priority for imports inside "if MYPY" --- mypy/build.py | 27 +++++++++++++++++++------ mypy/nodes.py | 7 +++++-- mypy/semanal.py | 53 ++++++++++++++++++++++++++++++++++++++----------- 3 files changed, 67 insertions(+), 20 deletions(-) diff --git a/mypy/build.py b/mypy/build.py index 289a22b499bb..ca758f20a6de 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -24,7 +24,8 @@ from typing import (AbstractSet, Dict, Iterable, Iterator, List, NamedTuple, Optional, Set, Tuple, Union) -from mypy.nodes import (MypyFile, Import, ImportFrom, ImportAll) +from mypy.types import Type +from mypy.nodes import (MypyFile, Node, ImportBase, Import, ImportFrom, ImportAll) from mypy.semanal import FirstPass, SemanticAnalyzer, ThirdPass from mypy.checker import TypeChecker from mypy.indirection import TypeIndirectionVisitor @@ -307,9 +308,22 @@ def default_lib_path(data_dir: str, pyversion: Tuple[int, int]) -> List[str]: PRI_MED = 10 # top-level "import X" PRI_LOW = 20 # either form inside a function PRI_INDIRECT = 30 # an indirect dependency +PRI_MYPY = 40 # inside "if MYPY" or "if typing.TYPE_CHECKING" PRI_ALL = 99 # include all priorities +def import_priority(imp: ImportBase, toplevel_priority: int) -> int: + """Compute import priority from an import node.""" + if imp.is_mypy_only: + # Inside "if MYPY" or "if typing.TYPE_CHECKING" + return PRI_MYPY + if not imp.is_top_level: + # Inside a function + return PRI_LOW + # A regular import; priority determined by argument. + return toplevel_priority + + # TODO: Get rid of all_types. It's not used except for one log message. # Maybe we could instead publish a map from module ID to its type_map. class BuildManager: @@ -395,20 +409,21 @@ def correct_rel_imp(imp: Union[ImportFrom, ImportAll]) -> str: for imp in file.imports: if not imp.is_unreachable: if isinstance(imp, Import): - pri = PRI_MED if imp.is_top_level else PRI_LOW + pri = import_priority(imp, PRI_MED) + ancestor_pri = import_priority(imp, PRI_LOW) for id, _ in imp.ids: ancestor_parts = id.split(".")[:-1] ancestors = [] for part in ancestor_parts: ancestors.append(part) - res.append((PRI_LOW, ".".join(ancestors), imp.line)) + res.append((ancestor_pri, ".".join(ancestors), imp.line)) res.append((pri, id, imp.line)) elif isinstance(imp, ImportFrom): cur_id = correct_rel_imp(imp) pos = len(res) all_are_submodules = True # Also add any imported names that are submodules. - pri = PRI_MED if imp.is_top_level else PRI_LOW + pri = import_priority(imp, PRI_MED) for name, __ in imp.names: sub_id = cur_id + '.' + name if self.is_module(sub_id): @@ -421,10 +436,10 @@ def correct_rel_imp(imp: Union[ImportFrom, ImportAll]) -> str: # cur_id is also a dependency, and we should # insert it *before* any submodules. if not all_are_submodules: - pri = PRI_HIGH if imp.is_top_level else PRI_LOW + pri = import_priority(imp, PRI_HIGH) res.insert(pos, ((pri, cur_id, imp.line))) elif isinstance(imp, ImportAll): - pri = PRI_HIGH if imp.is_top_level else PRI_LOW + pri = import_priority(imp, PRI_HIGH) res.append((pri, correct_rel_imp(imp), imp.line)) return res diff --git a/mypy/nodes.py b/mypy/nodes.py index 36cb0a2a2210..a680b80d800e 100644 --- a/mypy/nodes.py +++ b/mypy/nodes.py @@ -285,8 +285,11 @@ def deserialize(cls, data: JsonDict) -> 'MypyFile': class ImportBase(Statement): """Base class for all import statements.""" - is_unreachable = False - is_top_level = False # Set by semanal.FirstPass + + is_unreachable = False # Set by semanal.FirstPass if inside `if False` etc. + is_top_level = False # Ditto if outside any class or def + is_mypy_only = False # Ditto if inside `if TYPE_CHECKING` or `if MYPY` + # If an import replaces existing definitions, we construct dummy assignment # statements that assign the imported names to the names in the current scope, # for type checking purposes. Example: diff --git a/mypy/semanal.py b/mypy/semanal.py index 652f68683eeb..72624cb39780 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -84,10 +84,20 @@ T = TypeVar('T') -# Inferred value of an expression. -ALWAYS_TRUE = 0 -ALWAYS_FALSE = 1 -TRUTH_VALUE_UNKNOWN = 2 +# Inferred truth value of an expression. +ALWAYS_TRUE = 1 +MYPY_TRUE = 2 # True in mypy, False at runtime +ALWAYS_FALSE = 3 +MYPY_FALSE = 4 # False in mypy, True at runtime +TRUTH_VALUE_UNKNOWN = 5 + +inverted_truth_mapping = { + ALWAYS_TRUE: ALWAYS_FALSE, + ALWAYS_FALSE: ALWAYS_TRUE, + TRUTH_VALUE_UNKNOWN: TRUTH_VALUE_UNKNOWN, + MYPY_TRUE: MYPY_FALSE, + MYPY_FALSE: MYPY_TRUE, +} # Map from obsolete name to the current spelling. obsolete_name_mapping = { @@ -3082,12 +3092,16 @@ def infer_reachability_of_if_statement(s: IfStmt, platform: str) -> None: for i in range(len(s.expr)): result = infer_if_condition_value(s.expr[i], pyversion, platform) - if result == ALWAYS_FALSE: + if result in (ALWAYS_FALSE, MYPY_FALSE): # The condition is always false, so we skip the if/elif body. mark_block_unreachable(s.body[i]) - elif result == ALWAYS_TRUE: + elif result in (ALWAYS_TRUE, MYPY_TRUE): # This condition is always true, so all of the remaining # elif/else bodies will never be executed. + if result == MYPY_TRUE: + # This condition is false at runtime; this will affect + # import priorities. + mark_block_mypy_only(s.body[i]) for body in s.body[i + 1:]: mark_block_unreachable(body) if s.else_body: @@ -3099,7 +3113,8 @@ def infer_if_condition_value(expr: Expression, pyversion: Tuple[int, int], platf """Infer whether if condition is always true/false. Return ALWAYS_TRUE if always true, ALWAYS_FALSE if always false, - and TRUTH_VALUE_UNKNOWN otherwise. + MYPY_TRUE if true under mypy and false at runtime, MYPY_FALSE if + false under mypy and true at runtime, else TRUTH_VALUE_UNKNOWN. """ name = '' negated = False @@ -3123,12 +3138,9 @@ def infer_if_condition_value(expr: Expression, pyversion: Tuple[int, int], platf elif name == 'PY3': result = ALWAYS_TRUE if pyversion[0] == 3 else ALWAYS_FALSE elif name == 'MYPY' or name == 'TYPE_CHECKING': - result = ALWAYS_TRUE + result = MYPY_TRUE if negated: - if result == ALWAYS_TRUE: - result = ALWAYS_FALSE - elif result == ALWAYS_FALSE: - result = ALWAYS_TRUE + result = inverted_truth_mapping[result] return result @@ -3303,6 +3315,23 @@ def visit_import_all(self, node: ImportAll) -> None: node.is_unreachable = True +def mark_block_mypy_only(block: Block) -> None: + block.accept(MarkImportsMypyOnlyVisitor()) + + +class MarkImportsMypyOnlyVisitor(TraverserVisitor): + """Visitor that sets is_mypy_only (which affects priority).""" + + def visit_import(self, node: Import) -> None: + node.is_mypy_only = True + + def visit_import_from(self, node: ImportFrom) -> None: + node.is_mypy_only = True + + def visit_import_all(self, node: ImportAll) -> None: + node.is_mypy_only = True + + def is_identity_signature(sig: Type) -> bool: """Is type a callable of form T -> T (where T is a type variable)?""" if isinstance(sig, CallableType) and sig.arg_kinds == [ARG_POS]: From 8bab47b9a5e4f8a3908f2065e4763744c3726e06 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Fri, 23 Sep 2016 12:07:49 -0700 Subject: [PATCH 2/7] Update docstrings/comments in response to review. --- mypy/build.py | 4 ++-- mypy/semanal.py | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/mypy/build.py b/mypy/build.py index ca758f20a6de..3b5cf4510fee 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -1718,8 +1718,8 @@ def order_ascc(graph: Graph, ascc: AbstractSet[str], pri_max: int = PRI_ALL) -> each SCC thus found. The recursion is bounded because at each recursion the spread in priorities is (at least) one less. - In practice there are only a few priority levels (currently - N=3) and in the worst case we just carry out the same algorithm + In practice there are only a few priority levels (less than a + dozen) and in the worst case we just carry out the same algorithm for finding SCCs N times. Thus the complexity is no worse than the complexity of the original SCC-finding algorithm -- see strongly_connected_components() below for a reference. diff --git a/mypy/semanal.py b/mypy/semanal.py index 72624cb39780..1e80a2c0ab8f 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -3093,11 +3093,11 @@ def infer_reachability_of_if_statement(s: IfStmt, for i in range(len(s.expr)): result = infer_if_condition_value(s.expr[i], pyversion, platform) if result in (ALWAYS_FALSE, MYPY_FALSE): - # The condition is always false, so we skip the if/elif body. + # The condition is considered always false, so we skip the if/elif body. mark_block_unreachable(s.body[i]) elif result in (ALWAYS_TRUE, MYPY_TRUE): - # This condition is always true, so all of the remaining - # elif/else bodies will never be executed. + # This condition is considered always true, so all of the remaining + # elif/else bodies should not be checked. if result == MYPY_TRUE: # This condition is false at runtime; this will affect # import priorities. From a078583e1839cd36e9b2bd8a069ac8d939170c96 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Sun, 25 Sep 2016 14:19:34 -0700 Subject: [PATCH 3/7] Add unittest by @elazarg --- test-data/unit/check-modules.test | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test-data/unit/check-modules.test b/test-data/unit/check-modules.test index d488a39d5a4d..8aac9840097e 100644 --- a/test-data/unit/check-modules.test +++ b/test-data/unit/check-modules.test @@ -1313,3 +1313,27 @@ pass [file b] pass [out] + +[case testTypeCheckPrio] +# cmd: mypy -m part1 part2 part3 part4 + +[file part1.py] +from part3 import Thing +class FirstThing: pass + +[file part2.py] +from part4 import part4_thing as Thing + +[file part3.py] +from part2 import Thing +reveal_type(Thing) + +[file part4.py] +from typing import TYPE_CHECKING +if TYPE_CHECKING: + from part1 import FirstThing +def part4_thing(a: int) -> str: pass + +[builtins fixtures/bool.pyi] +[out] +tmp/part3.py:2: error: Revealed type is 'def (a: builtins.int) -> builtins.str' From 9a3308dc7b2bbcc0631e2484667c669019385bf5 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Thu, 29 Sep 2016 13:15:59 -0700 Subject: [PATCH 4/7] Tweak import_priority; gives slightly fewer errors --- mypy/build.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/mypy/build.py b/mypy/build.py index 3b5cf4510fee..01e6ad62f2cb 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -308,18 +308,17 @@ def default_lib_path(data_dir: str, pyversion: Tuple[int, int]) -> List[str]: PRI_MED = 10 # top-level "import X" PRI_LOW = 20 # either form inside a function PRI_INDIRECT = 30 # an indirect dependency -PRI_MYPY = 40 # inside "if MYPY" or "if typing.TYPE_CHECKING" PRI_ALL = 99 # include all priorities def import_priority(imp: ImportBase, toplevel_priority: int) -> int: """Compute import priority from an import node.""" - if imp.is_mypy_only: - # Inside "if MYPY" or "if typing.TYPE_CHECKING" - return PRI_MYPY if not imp.is_top_level: # Inside a function return PRI_LOW + if imp.is_mypy_only: + # Inside "if MYPY" or "if typing.TYPE_CHECKING" + return max(PRI_MED, toplevel_priority) # A regular import; priority determined by argument. return toplevel_priority From bb263917b9a2d69ae7e6397be85f5e7357b28bd7 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Mon, 24 Oct 2016 10:19:50 -0700 Subject: [PATCH 5/7] Silence lint --- mypy/build.py | 1 - 1 file changed, 1 deletion(-) diff --git a/mypy/build.py b/mypy/build.py index 01e6ad62f2cb..cd6680e46165 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -24,7 +24,6 @@ from typing import (AbstractSet, Dict, Iterable, Iterator, List, NamedTuple, Optional, Set, Tuple, Union) -from mypy.types import Type from mypy.nodes import (MypyFile, Node, ImportBase, Import, ImportFrom, ImportAll) from mypy.semanal import FirstPass, SemanticAnalyzer, ThirdPass from mypy.checker import TypeChecker From dc1f8c2c0fdceac33f6990a2c6769cf5b45588c0 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Wed, 26 Oct 2016 09:09:05 -0700 Subject: [PATCH 6/7] Add separate PRI_MYPY priority, below PRI_LOW --- mypy/build.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mypy/build.py b/mypy/build.py index cd6680e46165..3cc24218706c 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -306,6 +306,7 @@ def default_lib_path(data_dir: str, pyversion: Tuple[int, int]) -> List[str]: PRI_HIGH = 5 # top-level "from X import blah" PRI_MED = 10 # top-level "import X" PRI_LOW = 20 # either form inside a function +PRI_MYPY = 25 # inside "if MYPY" or "if TYPE_CHECKING" PRI_INDIRECT = 30 # an indirect dependency PRI_ALL = 99 # include all priorities @@ -317,7 +318,7 @@ def import_priority(imp: ImportBase, toplevel_priority: int) -> int: return PRI_LOW if imp.is_mypy_only: # Inside "if MYPY" or "if typing.TYPE_CHECKING" - return max(PRI_MED, toplevel_priority) + return max(PRI_MYPY, toplevel_priority) # A regular import; priority determined by argument. return toplevel_priority From fa707f68063dca21cd017d3488278ae5d95615b6 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Wed, 26 Oct 2016 09:50:35 -0700 Subject: [PATCH 7/7] Silence lint --- mypy/build.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/build.py b/mypy/build.py index 3cc24218706c..58e25da34cac 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -306,7 +306,7 @@ def default_lib_path(data_dir: str, pyversion: Tuple[int, int]) -> List[str]: PRI_HIGH = 5 # top-level "from X import blah" PRI_MED = 10 # top-level "import X" PRI_LOW = 20 # either form inside a function -PRI_MYPY = 25 # inside "if MYPY" or "if TYPE_CHECKING" +PRI_MYPY = 25 # inside "if MYPY" or "if TYPE_CHECKING" PRI_INDIRECT = 30 # an indirect dependency PRI_ALL = 99 # include all priorities