From aa4a585c9e6ff98568259f0869c4027765dbfcb6 Mon Sep 17 00:00:00 2001 From: Michael Lee Date: Tue, 19 Jul 2016 13:55:37 -0700 Subject: [PATCH 1/6] Modify tests to check for correct behavior --- test-data/unit/check-ignore.test | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/test-data/unit/check-ignore.test b/test-data/unit/check-ignore.test index 4f5e04de3ab6..15e8a9483f64 100644 --- a/test-data/unit/check-ignore.test +++ b/test-data/unit/check-ignore.test @@ -36,7 +36,22 @@ x # E: Name 'x' is not defined import m # type: ignore from m import a # type: ignore [file m.py] -+ # A parse error, but we shouldn't even parse m.py ++ +[out] +main:1: note: In module imported here: +tmp/m.py:1: error: Parse error before end of line + +[case testIgnoreAppliesOnlyToMissing] +import a # type: ignore +import b # type: ignore +reveal_type(a.foo) # E: Revealed type is 'Any' +reveal_type(b.foo) # E: Revealed type is 'builtins.int' +a.bar() +b.bar() # E: Name 'bar' is not defined + +[file b.py] +foo = 3 + [out] [case testIgnoreImportStarFromBadModule] From 4e59477796a456362638bb41c6454b4f0dc63e0b Mon Sep 17 00:00:00 2001 From: Michael Lee Date: Tue, 19 Jul 2016 13:55:58 -0700 Subject: [PATCH 2/6] Prevent ignored modules from being entirely unchecked --- mypy/build.py | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/mypy/build.py b/mypy/build.py index 7dc88a2db939..806e5f282deb 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -416,23 +416,12 @@ def parse_file(self, id: str, path: str, source: str) -> MypyFile: tree = parse(source, path, self.errors, options=self.options) tree._fullname = id - # We don't want to warn about 'type: ignore' comments on - # imports, but we're about to modify tree.imports, so grab - # these first. - import_lines = set(node.line for node in tree.imports) - - # Skip imports that have been ignored (so that we can ignore a C extension module without - # stub, for example), except for 'from x import *', because we wouldn't be able to - # determine which names should be defined unless we process the module. We can still - # ignore errors such as redefinitions when using the latter form. - imports = [node for node in tree.imports - if node.line not in tree.ignored_lines or isinstance(node, ImportAll)] - tree.imports = imports - if self.errors.num_messages() != num_errs: self.log("Bailing due to parse errors") self.errors.raise_error() + # We don't want to warn about 'type: ignore' comments on imports + import_lines = set(node.line for node in tree.imports) self.errors.set_file_ignored_lines(path, tree.ignored_lines) self.errors.mark_file_ignored_lines_used(path, import_lines) return tree From c3ddcc71c3bf3f0b606e8b0641d7251bc320e7ba Mon Sep 17 00:00:00 2001 From: Michael Lee Date: Tue, 19 Jul 2016 13:56:25 -0700 Subject: [PATCH 3/6] Add clarifying note to docs about ignore comments on imports --- docs/source/common_issues.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/source/common_issues.rst b/docs/source/common_issues.rst index 956bf38a4943..4cdf61324609 100644 --- a/docs/source/common_issues.rst +++ b/docs/source/common_issues.rst @@ -36,6 +36,13 @@ error: The second line is now fine, since the ignore comment causes the name ``frobnicate`` to get an implicit ``Any`` type. +.. note:: + + The ``# type: ignore`` comment will only assign the implicit ``Any`` + type if mypy cannot find information about that particular module. So, + if we did have a stub available for ``frobnicate`` then mypy will + ignore the ``# type: ignore`` comment and typecheck the stub as usual. + Types of empty collections -------------------------- From fcdc3245060a7f31859229c6721225766cc78dc7 Mon Sep 17 00:00:00 2001 From: Michael Lee Date: Tue, 19 Jul 2016 14:04:39 -0700 Subject: [PATCH 4/6] Port over unit tests from pull request #1906 --- test-data/unit/check-incremental.test | 62 +++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/test-data/unit/check-incremental.test b/test-data/unit/check-incremental.test index 43592d6db2c7..ec1c55510c99 100644 --- a/test-data/unit/check-incremental.test +++ b/test-data/unit/check-incremental.test @@ -185,3 +185,65 @@ from parent import b [stale parent.a] [out] + +[case testIncrementalWithTypeIgnoreOnDirectImport] +import a, b + +[file a.py] +import b # type: ignore + +[file b.py] +import c + +[file c.py] + +[stale] +[out] + +[case testIncrementalWithTypeIgnoreOnImportFrom] +import a, b + +[file a.py] +from b import something # type: ignore + +[file b.py] +import c +something = 3 + +[file c.py] + +[stale] +[out] + +[case testIncrementalWithPartialTypeIgnore] +import a # type: ignore +import a.b + +[file a/__init__.py] + +[file a/b.py] + +[stale] +[out] + +[case testIncrementalAnyIsDifferentFromIgnore] +import b + +[file b.py] +from typing import Any +import a.b + +[file b.py.next] +from typing import Any + +a = 3 # type: Any +import a.b + +[file a/__init__.py] + +[file a/b.py] + +[stale b] +[out] +main:1: note: In module imported here: +tmp/b.py:4: error: Name 'a' already defined From 2f0f116e074a8ebad249b8e28c82c33e17c2a75e Mon Sep 17 00:00:00 2001 From: Michael Lee Date: Tue, 19 Jul 2016 14:52:10 -0700 Subject: [PATCH 5/6] Fix typos and clarify comments --- docs/source/common_issues.rst | 2 +- mypy/build.py | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/source/common_issues.rst b/docs/source/common_issues.rst index 4cdf61324609..0b72ee5fc7a6 100644 --- a/docs/source/common_issues.rst +++ b/docs/source/common_issues.rst @@ -40,7 +40,7 @@ The second line is now fine, since the ignore comment causes the name The ``# type: ignore`` comment will only assign the implicit ``Any`` type if mypy cannot find information about that particular module. So, - if we did have a stub available for ``frobnicate`` then mypy will + if we did have a stub available for ``frobnicate`` then mypy would ignore the ``# type: ignore`` comment and typecheck the stub as usual. Types of empty collections diff --git a/mypy/build.py b/mypy/build.py index 806e5f282deb..9e7af6f80931 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -420,7 +420,12 @@ def parse_file(self, id: str, path: str, source: str) -> MypyFile: self.log("Bailing due to parse errors") self.errors.raise_error() - # We don't want to warn about 'type: ignore' comments on imports + # We don't want to warn about unused 'type: ignore' comments on imports. + # That way, if we want to conditionally import a module that is valid + # only on one particular platform, we can silence the import so we don't + # get spurious error messages when mypy tries checking it. For example, + # we might want to conditionally import a Python 2 only module while + # checking the file as Python 3. import_lines = set(node.line for node in tree.imports) self.errors.set_file_ignored_lines(path, tree.ignored_lines) self.errors.mark_file_ignored_lines_used(path, import_lines) From cbd97d93ca29e3063fa0d1de99e0f428d6e0cb03 Mon Sep 17 00:00:00 2001 From: Michael Lee Date: Tue, 19 Jul 2016 14:53:23 -0700 Subject: [PATCH 6/6] Fix bug in tests --- test-data/unit/check-ignore.test | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test-data/unit/check-ignore.test b/test-data/unit/check-ignore.test index 15e8a9483f64..98c6e05bd363 100644 --- a/test-data/unit/check-ignore.test +++ b/test-data/unit/check-ignore.test @@ -47,11 +47,12 @@ import b # type: ignore reveal_type(a.foo) # E: Revealed type is 'Any' reveal_type(b.foo) # E: Revealed type is 'builtins.int' a.bar() -b.bar() # E: Name 'bar' is not defined +b.bar() # E: "module" has no attribute "bar" [file b.py] foo = 3 +[builtins fixtures/module_all.py] [out] [case testIgnoreImportStarFromBadModule]