New check: import-outside-toplevel (close #3067) (#3079)

ChangeLog

@@ -7,6 +7,11 @@ What's New in Pylint 2.4.0?
 
 Release date: TBA
 
+* New check: ``import-outside-toplevel``
+
+  This check warns when modules are imported from places other than a
+  module toplevel, e.g. inside a function or a class.
+
 * Support forward references for ``function-redefined`` check.
 
   Close #2540

doc/whatsnew/2.4.rst

@@ -13,6 +13,11 @@ Summary -- Release highlights
 New checkers
 ============
 
+* ``import-outside-toplevel``
+
+  This check warns when modules are imported from places other than a
+  module toplevel, e.g. inside a function or a class.
+
 * Added a new check, ``consider-using-sys-exit``
 
   This check is emitted when we detect that a quit() or exit() is invoked

pylint/checkers/imports.py

@@ -278,6 +278,12 @@ def _make_graph(filename, dep_info, sect, gtype):
         "Used when an import alias is same as original package."
         "e.g using import numpy as numpy instead of import numpy as np",
     ),
+    "C0415": (
+        "Import outside toplevel (%s)",
+        "import-outside-toplevel",
+        "Used when an import statement is used anywhere other than the module "
+        "toplevel. Move this import to the top of the file.",
+    ),
 }
 
 
@@ -374,6 +380,16 @@ class ImportsChecker(BaseChecker):
                 "a third party library.",
             },
         ),
+        (
+            "import-outside-toplevel",
+            {
+                "default": (),
+                "type": "csv",
+                "metavar": "<modules>",
+                "help": "Modules that can be imported outside the toplevel "
+                "of other modules.",
+            },
+        ),
         (
             "analyse-fallback-blocks",
             {
@@ -472,6 +488,7 @@ def visit_import(self, node):
         """triggered when an import statement is seen"""
         self._check_reimport(node)
         self._check_import_as_rename(node)
+        self._check_toplevel(node)
 
         modnode = node.root()
         names = [name for name, _ in node.names]
@@ -965,6 +982,27 @@ def _wildcard_import_is_allowed(self, imported_module):
             and "__all__" in imported_module.locals
         )
 
+    def _check_toplevel(self, node):
+        """Check whether the import is made outside the module toplevel.
+        """
+        # If the scope of the import is a module, then obviously it is
+        # not outside the module toplevel.
+        if isinstance(node.scope(), astroid.Module):
+            return
+
+        # Get the full names of all the imports that are not
+        # whitelisted.
+        not_whitelisted = [
+            name[0]
+            for name in node.names
+            if name[0] not in self.config.import_outside_toplevel
+        ]
+
+        if not_whitelisted:
+            self.add_message(
+                "import-outside-toplevel", args=", ".join(not_whitelisted), node=node
+            )
+
 
 def register(linter):
     """required method to auto register this checker """

pylint/graph.py

@@ -14,6 +14,7 @@
 import codecs
 import os
 import os.path as osp
+import subprocess
 import sys
 import tempfile
 
@@ -80,8 +81,6 @@ def generate(self, outputfile=None, dotfile=None, mapfile=None):
         :rtype: str
         :return: a path to the generated file
         """
-        import subprocess  # introduced in py 2.4
-
         name = self.graphname
         if not dotfile:
             # if 'outputfile' is a dot file use it as 'dotfile'

pylint/reporters/text.py

@@ -200,7 +200,7 @@ def __init__(self, output=None, color_mapping=None):
         ansi_terms = ["xterm-16color", "xterm-256color"]
         if os.environ.get("TERM") not in ansi_terms:
             if sys.platform == "win32":
-                # pylint: disable=import-error
+                # pylint: disable=import-error,import-outside-toplevel
                 import colorama
 
                 self.out = colorama.AnsiToWin32(self.out)

tests/functional/g/globals.py

@@ -29,4 +29,4 @@ def define_constant():
 def global_with_import():
     """should only warn for global-statement"""
     global sys  # [global-statement]
-    import sys
+    import sys  # pylint: disable=import-outside-toplevel

tests/functional/i/invalid_exceptions_raised.py

@@ -1,4 +1,4 @@
-# pylint:disable=too-few-public-methods,no-init,import-error,missing-docstring, not-callable, useless-object-inheritance
+# pylint:disable=too-few-public-methods,no-init,import-error,missing-docstring, not-callable, useless-object-inheritance,import-outside-toplevel
 """test pb with exceptions and old/new style classes"""
 
 
@@ -77,7 +77,7 @@ def bad_case9():
 
 def unknown_bases():
     """Don't emit when we don't know the bases."""
-    from lala import bala
+    from lala import bala  # pylint: disable=import-outside-toplevel
     class MyException(bala):
         pass
     raise MyException

tests/functional/i/invalid_name.py

@@ -1,5 +1,5 @@
 """ Tests for invalid-name checker. """
-# pylint: disable=unused-import, no-absolute-import, wrong-import-position
+# pylint: disable=unused-import, no-absolute-import, wrong-import-position,import-outside-toplevel
 
 AAA = 24
 try:

tests/functional/import_outside_toplevel.py

@@ -0,0 +1,34 @@
+# pylint: disable=unused-import,multiple-imports,no-self-use,missing-docstring,invalid-name,too-few-public-methods
+
+import abc
+
+if 4 == 5:
+    import ast
+
+
+def f():
+    import symtable  # [import-outside-toplevel]
+
+
+def g():
+    import os, sys  # [import-outside-toplevel]
+
+
+def h():
+    import time as thyme  # [import-outside-toplevel]
+
+
+def i():
+    import random as rand, socket as sock  # [import-outside-toplevel]
+
+
+class C:
+    import tokenize  # [import-outside-toplevel]
+
+    def j(self):
+        import turtle  # [import-outside-toplevel]
+
+
+def k(flag):
+    if flag:
+        import tabnanny  # [import-outside-toplevel]

tests/functional/import_outside_toplevel.txt

@@ -0,0 +1,7 @@
+import-outside-toplevel:10:f:Import outside toplevel (symtable)
+import-outside-toplevel:14:g:Import outside toplevel (os, sys)
+import-outside-toplevel:18:h:Import outside toplevel (time)
+import-outside-toplevel:22:i:Import outside toplevel (random, socket)
+import-outside-toplevel:26:C:Import outside toplevel (tokenize)
+import-outside-toplevel:29:C.j:Import outside toplevel (turtle)
+import-outside-toplevel:34:k:Import outside toplevel (tabnanny)

tests/functional/m/member_checks.py

@@ -1,5 +1,5 @@
 # pylint: disable=print-statement,missing-docstring,no-self-use,too-few-public-methods,bare-except,broad-except, useless-object-inheritance
-# pylint: disable=using-constant-test,expression-not-assigned, assigning-non-slot, unused-variable,pointless-statement, wrong-import-order, wrong-import-position
+# pylint: disable=using-constant-test,expression-not-assigned, assigning-non-slot, unused-variable,pointless-statement, wrong-import-order, wrong-import-position,import-outside-toplevel
 from __future__ import print_function
 import six
 class Provider(object):

tests/functional/r/raising_format_tuple.py

@@ -47,5 +47,5 @@ def bad_unicode(arg):
 
 def raise_something_without_name(arg):
     '''Regression test for nodes without .node attribute'''
-    import standard_exceptions  # pylint: disable=import-error
+    import standard_exceptions  # pylint: disable=import-error,import-outside-toplevel
     raise standard_exceptions.MyException(u'An %s', arg)  # [raising-format-tuple]

tests/functional/u/undefined_variable.py

@@ -1,4 +1,4 @@
-# pylint: disable=missing-docstring, multiple-statements, useless-object-inheritance
+# pylint: disable=missing-docstring, multiple-statements, useless-object-inheritance,import-outside-toplevel
 # pylint: disable=too-few-public-methods, no-init, no-self-use,bare-except,broad-except, import-error
 from __future__ import print_function
 DEFINED = 1

tests/functional/u/unused_variable.py

@@ -1,4 +1,4 @@
-# pylint: disable=missing-docstring, invalid-name, too-few-public-methods, no-self-use, useless-object-inheritance
+# pylint: disable=missing-docstring, invalid-name, too-few-public-methods, no-self-use, useless-object-inheritance,import-outside-toplevel
 
 def test_regression_737():
     import xml # [unused-import]

tests/functional/w/wrong_import_position4.py

@@ -1,5 +1,5 @@
 """Checks import position rule"""
-# pylint: disable=unused-import,relative-import,ungrouped-imports,import-error,no-name-in-module,relative-beyond-top-level,unused-variable
+# pylint: disable=unused-import,relative-import,ungrouped-imports,import-error,no-name-in-module,relative-beyond-top-level,unused-variable,import-outside-toplevel
 def method1():
     """Method 1"""
     import x

tests/input/func_w0404.py

@@ -14,13 +14,13 @@
 
 def no_reimport():
     """docstring"""
-    import os
+    import os  #pylint: disable=import-outside-toplevel
     print(os)
 
 
 def reimport():
     """This function contains a reimport."""
-    import sys
+    import sys  #pylint: disable=import-outside-toplevel
     del sys
 
 

tests/input/func_w0405.py

@@ -2,7 +2,7 @@
 """
 from __future__ import absolute_import, print_function
 
-# pylint: disable=using-constant-test,ungrouped-imports,wrong-import-position
+# pylint: disable=using-constant-test,ungrouped-imports,wrong-import-position,import-outside-toplevel
 import os
 from os.path import join, exists
 import os

tests/input/func_w0612.py

@@ -1,6 +1,6 @@
 """test unused variable
 """
-# pylint: disable=invalid-name, redefined-outer-name, no-absolute-import
+# pylint: disable=invalid-name, redefined-outer-name, no-absolute-import,import-outside-toplevel
 from __future__ import print_function
 PATH = OS = collections = deque = None
 

tests/unittest_checker_imports.py

@@ -26,6 +26,30 @@ class TestImportsChecker(CheckerTestCase):
 
     CHECKER_CLASS = imports.ImportsChecker
 
+    @set_config(import_outside_toplevel=("astroid",))
+    def test_import_outside_toplevel(self):
+        node = astroid.extract_node(
+            """
+        def f():
+            import astroid
+        """
+        ).body[0]
+
+        with self.assertNoMessages():
+            self.checker.visit_import(node)
+
+        node = astroid.extract_node(
+            """
+        def g():
+            import pylint
+        """
+        ).body[0]
+
+        with self.assertAddsMessages(
+            Message("import-outside-toplevel", node=node, args="pylint")
+        ):
+            self.checker.visit_import(node)
+
     @set_config(
         ignored_modules=("external_module", "fake_module.submodule", "foo", "bar")
     )