Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add --almost-silent, which modifies --silent-imports to warn about silenced imports #1371

Merged
merged 6 commits into from
Apr 13, 2016

Conversation

gvanrossum
Copy link
Member

Also deprecated --silent (at least in --help output).

Fixes #1367.

…lenced imports.

Also deprecated --silent (at least in --help output).
@gvanrossum
Copy link
Member Author

I should add some discussion. The existing --silent-imports flag does two different things:

  • It prevents following imports of .py files that aren't mentioned on the command line; it treats these as if they are missing. This is the block with the comment "# In silent mode, don't import .py files".
  • It suppresses error messages about missing modules. This is the block with the comment "# Could not find a module [...]".

The error messages about missing modules (when not suppressed) take on different forms depending on whether the missing module is recognized as a stdlib module or a well-known third-party module, or neither. The decision with form to use is in module_not_found().

The new --almost-silent flag implies --silent-imports but modifies it in two ways:

  • When an import of an existing module is prevented (first bullet above), it adds a note indicating that this happens. There are two forms, depending on whether this is an ancestor or not (see below).
  • It prevents the suppression of error messages about missing modules (second bullet above).

The note about a suppressed module takes a different form depending on whether the import being suppressed is an implied import of a module's ancestor (parent) package, or a regular import. This difference is important, because these are really two quite different things being suppressed.

  • The non-ancestor case is the simplest and the most common: we have a module given on the command line, e.g. foo.py, containing the line "import bar", where bar.py is not given on the command line. Without --silent-imports or --almost-silent, mypy would follows import, so it would parse and analyze bar.py (once it has seen the "import bar" in foo.py). With --silent-imports, it still locates bar.py, but then realizes bar.py is not given on the command line (and is not a stub) so it prevents the import, instead giving main.bar the type Any. With --almost-silent, it does the same thing, but it also prints a note about this incident. This note appears on the line in foo.py that says "import bar", and the text of the note is "Import of 'bar' silently ignored". (The first time such a note is presented it adds some clarifying notes as well.) Of course there are variants, e.g. bar could be a submodule of a package, or the import could be something like "from bar import baz", but the gist of the error is the same.
  • The ancestor case is more subtle. Extending the previous example, assume foo.py contains "import bar.baz", and "bar/baz.py" is also present on the command line. Then the import is allowed. However, an import of bar/baz.py implies an import of the parent package, and in particular, both CPython and mypy without --silent-imports will load bar/init.py, and take this to define the contents of the package bar, which is the ancestor (parent) of module bar.baz. (In sys.modules, a package is simply a module with children.) Now (for good reasons that would take too long to explain here) --silent-imports also prevents loading of bar/init.py in this case, unless that file (or the package directory, bar) is also given on the command line. So --almost-silent also print a note about this incident (and a further clarification the first time). This note must look different, because the reasons for this suppression are different (and also because such ancestors are treated differently by the dependency management code -- see the two different calls to State(...) in the while-loop in load_graph()).

PS: A thought that just popped into my head: maybe the note about a suppressed ancestor dependency would look better if it was associated with the "import bar.baz" line in foo.py, rather than with the first line of bar/baz.py.

@gvanrossum
Copy link
Member Author

Well, the idea from that PS is no good, because guess what -- the only ancestors that are ever being suppressed are the ancestors of files passed on the command line. For example, suppose we have a.py:

from b.c import d

and b/c.py:

d = 42

Now if we run:

mypy -s a.py b/c.py

then mypy will suppress the ancestor package b when it's processing b/c.py, not when it's processing a.py, so there is no import context for b. (And in fact this makes sense -- it would come to the same conclusion even if a.py was not given.)

…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.
@@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops! Should be '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):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No type annotation

@JukkaL
Copy link
Collaborator

JukkaL commented Apr 13, 2016

LGTM. Have you thought about giving a special message if a base class is Any due to silent imports?

@gvanrossum gvanrossum merged commit 388be20 into master Apr 13, 2016
@gvanrossum
Copy link
Member Author

I filed #1374 to track the idea of warning about a base class being Any.

@gvanrossum gvanrossum deleted the almost-silent branch April 13, 2016 17:50
manager.errors.set_import_context([])
manager.errors.set_file(ancestor_for.xpath)
manager.errors.report(-1, "Ancestor package '%s' silently ignored" % (id,),
severity='note', only_once=True)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you intend to apply this only_once here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did -- if a package has many children it would be annoying to hear about this each time. And there's no possibility that some of the children have their ancestor ignored while others don't -- it's one ancestor and it's either ignored or not.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants