From e7396424cab065a8df5bf94b8d35d56d56ee4f96 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Tue, 12 Apr 2016 16:39:52 -0700 Subject: [PATCH 1/5] Add --almost-silent, which modifies --silent-imports to warn about silenced imports. Also deprecated --silent (at least in --help output). --- mypy/build.py | 52 ++++++++++++++++++++++++++++++++++++++++++++------- mypy/main.py | 11 +++++++++-- 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/mypy/build.py b/mypy/build.py index cf12f63bba4b..aee8cbf762ca 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -57,6 +57,7 @@ DUMP_TYPE_STATS = 'dump-type-stats' DUMP_INFER_STATS = 'dump-infer-stats' SILENT_IMPORTS = 'silent-imports' # Silence imports of .py files +ALMOST_SILENT = 'silent-imports' # If SILENT_IMPORTS: report silenced imports as errors INCREMENTAL = 'incremental' # Incremental mode: use the cache FAST_PARSER = 'fast-parser' # Use experimental fast parser # Disallow calling untyped functions from typed ones @@ -941,7 +942,7 @@ def __init__(self, manager: BuildManager, caller_state: 'State' = None, caller_line: int = 0, - is_ancestor: bool = False, + ancestor_for: 'State' = None, ) -> None: assert id or path or source is not None, "Neither id, path nor source given" self.manager = manager @@ -971,7 +972,12 @@ def __init__(self, if path: # In silent mode, don't import .py files. if (SILENT_IMPORTS in manager.flags and - path.endswith('.py') and (caller_state or is_ancestor)): + path.endswith('.py') and (caller_state or ancestor_for)): + if ALMOST_SILENT in manager.flags: + if ancestor_for: + self.skipping_ancestor(id, path, ancestor_for) + else: + self.skipping_module(id, path) path = None manager.missing_modules.add(id) raise ModuleNotFound @@ -980,10 +986,12 @@ def __init__(self, # misspelled module name, missing stub, module not in # search path or the module has not been installed. if caller_state: - if not (SILENT_IMPORTS in manager.flags or - (caller_state.tree is not None and - (caller_line in caller_state.tree.ignored_lines or - 'import' in caller_state.tree.weak_opts))): + suppress_message = ((SILENT_IMPORTS in manager.flags and + ALMOST_SILENT not in manager.flags) or + (caller_state.tree is not None and + (caller_line in caller_state.tree.ignored_lines or + 'import' in caller_state.tree.weak_opts))) + if not suppress_message: save_import_context = manager.errors.import_context() manager.errors.set_import_context(caller_state.import_context) manager.module_not_found(caller_state.xpath, caller_line, id) @@ -1009,6 +1017,36 @@ def __init__(self, # Parse the file (and then some) to get the dependencies. self.parse_file() + def skipping_ancestor(self, id, path, ancestor_for): + # TODO: Read the path (the __init__.py file) and return + # immediately if it's empty or only contains comments. + # But beware, some package may be the ancestor of many modules, + # so we'd need to cache the decision. + manager = self.manager + manager.errors.set_file(ancestor_for.xpath) + manager.errors.report(1, "Ancestor package '%s' silently ignored" % (id,), + severity='note', only_once=True) + manager.errors.report(1, "(Using --silent-imports, submodule passed on command line)", + severity='note', only_once=True) + manager.errors.report(1, "(This note brought to you by --almost-silent)", + severity='note', only_once=True) + + def skipping_module(self, id, path): + assert self.caller_state, (id, path) + manager = self.manager + save_import_context = manager.errors.import_context() + if self.caller_state: + manager.errors.set_import_context(self.caller_state.import_context) + manager.errors.set_file(self.caller_state.xpath) + line = self.caller_line + manager.errors.report(line, "Import of '%s' silently ignored" % (id,), + severity='note') + manager.errors.report(line, "(Using --silent-imports, module not passed on command line)", + severity='note', only_once=True) + manager.errors.report(line, "(This note courtesy of --almost-silent)", + severity='note', only_once=True) + manager.errors.set_import_context(save_import_context) + def add_ancestors(self) -> None: # All parent packages are new ancestors. ancestors = [] @@ -1209,7 +1247,7 @@ def load_graph(sources: List[BuildSource], manager: BuildManager) -> Graph: # TODO: Why not 'if dep not in st.dependencies' ? # Ancestors don't have import context. newst = State(id=dep, path=None, source=None, manager=manager, - is_ancestor=True) + ancestor_for=st) else: newst = State(id=dep, path=None, source=None, manager=manager, caller_state=st, caller_line=st.dep_line_map.get(dep, 1)) diff --git a/mypy/main.py b/mypy/main.py index d8b18b1b617b..1fb54612b1c1 100644 --- a/mypy/main.py +++ b/mypy/main.py @@ -130,8 +130,12 @@ def parse_version(v): help='use Python x.y') parser.add_argument('--py2', dest='python_version', action='store_const', const=defaults.PYTHON2_VERSION, help="use Python 2 mode") - parser.add_argument('-s', '--silent-imports', '--silent', action='store_true', + parser.add_argument('-s', '--silent-imports', action='store_true', help="don't follow imports to .py files") + parser.add_argument('--silent', action='store_true', + help="deprecated name for --silent-imports") + parser.add_argument('--almost-silent', action='store_true', + help="like --silent-imports but reports the imports as errors") parser.add_argument('--disallow-untyped-calls', action='store_true', help="disallow calling functions without type annotations" " from functions with type annotations") @@ -208,8 +212,11 @@ def parse_version(v): if args.inferstats: options.build_flags.append(build.DUMP_INFER_STATS) - if args.silent_imports: + if args.silent_imports or args.silent: options.build_flags.append(build.SILENT_IMPORTS) + if args.almost_silent: + options.build_flags.append(build.SILENT_IMPORTS) + options.build_flags.append(build.ALMOST_SILENT) if args.disallow_untyped_calls: options.build_flags.append(build.DISALLOW_UNTYPED_CALLS) From e0c1d3ce0a73614f82534fe65ee6c3579dde26aa Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Tue, 12 Apr 2016 18:52:13 -0700 Subject: [PATCH 2/5] Suppress line number for "ancestor" messages. Also drop a redundant `if`. Suppressing the line number was David's idea. It's easy. But it has one downside: in Emacs' grep/compile mode, you can't click on the line to go to the file. Maybe that's okay, I have to think about it. See the conversation in the PR for why my other idea (using the import line as the context) won't work. --- mypy/build.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/mypy/build.py b/mypy/build.py index aee8cbf762ca..7e98620a2b6c 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -1023,22 +1023,22 @@ def skipping_ancestor(self, id, path, ancestor_for): # But beware, some package may be the ancestor of many modules, # so we'd need to cache the decision. manager = self.manager + manager.errors.set_import_context([]) manager.errors.set_file(ancestor_for.xpath) - manager.errors.report(1, "Ancestor package '%s' silently ignored" % (id,), + manager.errors.report(-1, "Ancestor package '%s' silently ignored" % (id,), severity='note', only_once=True) - manager.errors.report(1, "(Using --silent-imports, submodule passed on command line)", + manager.errors.report(-1, "(Using --silent-imports, submodule passed on command line)", severity='note', only_once=True) - manager.errors.report(1, "(This note brought to you by --almost-silent)", + manager.errors.report(-1, "(This note brought to you by --almost-silent)", severity='note', only_once=True) def skipping_module(self, id, path): assert self.caller_state, (id, path) manager = self.manager save_import_context = manager.errors.import_context() - if self.caller_state: - manager.errors.set_import_context(self.caller_state.import_context) - manager.errors.set_file(self.caller_state.xpath) - line = self.caller_line + manager.errors.set_import_context(self.caller_state.import_context) + manager.errors.set_file(self.caller_state.xpath) + line = self.caller_line manager.errors.report(line, "Import of '%s' silently ignored" % (id,), severity='note') manager.errors.report(line, "(Using --silent-imports, module not passed on command line)", From 5d08cf80448cfc62db9d7a08f649f1494e0c0c1b Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Tue, 12 Apr 2016 20:27:21 -0700 Subject: [PATCH 3/5] Oops. Make ALMOST_SILENT distinguishable from SILENT_IMPORTS. --- mypy/build.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/build.py b/mypy/build.py index 7e98620a2b6c..c3a2fac48f41 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -57,7 +57,7 @@ DUMP_TYPE_STATS = 'dump-type-stats' DUMP_INFER_STATS = 'dump-infer-stats' SILENT_IMPORTS = 'silent-imports' # Silence imports of .py files -ALMOST_SILENT = 'silent-imports' # If SILENT_IMPORTS: report silenced imports as errors +ALMOST_SILENT = 'almost-silent' # If SILENT_IMPORTS: report silenced imports as errors INCREMENTAL = 'incremental' # Incremental mode: use the cache FAST_PARSER = 'fast-parser' # Use experimental fast parser # Disallow calling untyped functions from typed ones From 8c5b7b9ea94ddd8551d92985096dcc6290f230bc Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Tue, 12 Apr 2016 21:03:59 -0700 Subject: [PATCH 4/5] Add unittest for almost-silent. --- mypy/build.py | 17 +++++++++-------- mypy/test/data/check-modules.test | 23 +++++++++++++++++++++++ 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/mypy/build.py b/mypy/build.py index c3a2fac48f41..f15c58e7834d 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -973,14 +973,15 @@ def __init__(self, # In silent mode, don't import .py files. if (SILENT_IMPORTS in manager.flags and path.endswith('.py') and (caller_state or ancestor_for)): - if ALMOST_SILENT in manager.flags: - if ancestor_for: - self.skipping_ancestor(id, path, ancestor_for) - else: - self.skipping_module(id, path) - path = None - manager.missing_modules.add(id) - raise ModuleNotFound + if id != 'builtins': + if ALMOST_SILENT in manager.flags: + if ancestor_for: + self.skipping_ancestor(id, path, ancestor_for) + else: + self.skipping_module(id, path) + path = None + manager.missing_modules.add(id) + raise ModuleNotFound else: # Could not find a module. Typically the reason is a # misspelled module name, missing stub, module not in diff --git a/mypy/test/data/check-modules.test b/mypy/test/data/check-modules.test index 9963adf9d232..ddd3291178eb 100644 --- a/mypy/test/data/check-modules.test +++ b/mypy/test/data/check-modules.test @@ -698,3 +698,26 @@ class C(B): pass [out] tmp/bar.py:1: error: Module has no attribute 'B' + +[case testImportSuppressedWhileAlmostSilent] +# cmd: mypy -m main +[file main.py] +# flags: silent-imports almost-silent +import mod +[file mod.py] +[builtins fixtures/module.py] +[out] +tmp/main.py:2: note: Import of 'mod' silently ignored +tmp/main.py:2: note: (Using --silent-imports, module not passed on command line) +tmp/main.py:2: note: (This note courtesy of --almost-silent) + +[case testAncestorSuppressedWhileAlmostSilent] +# cmd: mypy -m foo.bar +[file foo/bar.py] +# flags: silent-imports almost-silent +[file foo/__init__.py] +[builtins fixtures/module.py] +[out] +tmp/foo/bar.py: note: Ancestor package 'foo' silently ignored +tmp/foo/bar.py: note: (Using --silent-imports, submodule passed on command line) +tmp/foo/bar.py: note: (This note brought to you by --almost-silent) From 12a31ceacdfa3acba0da49a6d59be1bacd0a5fef Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Wed, 13 Apr 2016 09:10:27 -0700 Subject: [PATCH 5/5] Add annotations (review feedback). --- mypy/build.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mypy/build.py b/mypy/build.py index f15c58e7834d..c0aa48bc326a 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -1018,7 +1018,7 @@ def __init__(self, # Parse the file (and then some) to get the dependencies. self.parse_file() - def skipping_ancestor(self, id, path, ancestor_for): + def skipping_ancestor(self, id: str, path: str, ancestor_for: 'State') -> None: # TODO: Read the path (the __init__.py file) and return # immediately if it's empty or only contains comments. # But beware, some package may be the ancestor of many modules, @@ -1033,7 +1033,7 @@ def skipping_ancestor(self, id, path, ancestor_for): manager.errors.report(-1, "(This note brought to you by --almost-silent)", severity='note', only_once=True) - def skipping_module(self, id, path): + def skipping_module(self, id: str, path: str) -> None: assert self.caller_state, (id, path) manager = self.manager save_import_context = manager.errors.import_context()