diff --git a/ChangeLog b/ChangeLog index 0d4d4dc9b9..8c923262d4 100644 --- a/ChangeLog +++ b/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 diff --git a/doc/whatsnew/2.4.rst b/doc/whatsnew/2.4.rst index e5e5d86afe..af39a70c04 100644 --- a/doc/whatsnew/2.4.rst +++ b/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 diff --git a/pylint/checkers/imports.py b/pylint/checkers/imports.py index fc90344d76..49973194a5 100644 --- a/pylint/checkers/imports.py +++ b/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": "", + "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 """ diff --git a/pylint/graph.py b/pylint/graph.py index 3eb4802275..0dc7a1460b 100644 --- a/pylint/graph.py +++ b/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' diff --git a/pylint/reporters/text.py b/pylint/reporters/text.py index 34c584419f..ce741740b6 100644 --- a/pylint/reporters/text.py +++ b/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) diff --git a/tests/functional/g/globals.py b/tests/functional/g/globals.py index 1a24ff7363..e7f3cc85bd 100644 --- a/tests/functional/g/globals.py +++ b/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 diff --git a/tests/functional/i/invalid_exceptions_raised.py b/tests/functional/i/invalid_exceptions_raised.py index 78e6b65a09..34f740de4c 100644 --- a/tests/functional/i/invalid_exceptions_raised.py +++ b/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 diff --git a/tests/functional/i/invalid_name.py b/tests/functional/i/invalid_name.py index 1a949904ed..83dd22360e 100644 --- a/tests/functional/i/invalid_name.py +++ b/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: diff --git a/tests/functional/import_outside_toplevel.py b/tests/functional/import_outside_toplevel.py new file mode 100644 index 0000000000..0575e59b29 --- /dev/null +++ b/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] diff --git a/tests/functional/import_outside_toplevel.txt b/tests/functional/import_outside_toplevel.txt new file mode 100644 index 0000000000..53f9595d2c --- /dev/null +++ b/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) diff --git a/tests/functional/m/member_checks.py b/tests/functional/m/member_checks.py index 8d1d636dcc..ad7793fd51 100644 --- a/tests/functional/m/member_checks.py +++ b/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): diff --git a/tests/functional/r/raising_format_tuple.py b/tests/functional/r/raising_format_tuple.py index cfdda10232..0f3a3349a0 100644 --- a/tests/functional/r/raising_format_tuple.py +++ b/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] diff --git a/tests/functional/u/undefined_variable.py b/tests/functional/u/undefined_variable.py index 6197945731..0967f7a304 100644 --- a/tests/functional/u/undefined_variable.py +++ b/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 diff --git a/tests/functional/u/unused_variable.py b/tests/functional/u/unused_variable.py index e475a90389..cb5215d99e 100644 --- a/tests/functional/u/unused_variable.py +++ b/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] diff --git a/tests/functional/w/wrong_import_position4.py b/tests/functional/w/wrong_import_position4.py index 3f1174f9dc..c728241b73 100644 --- a/tests/functional/w/wrong_import_position4.py +++ b/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 diff --git a/tests/input/func_w0404.py b/tests/input/func_w0404.py index 2ce7d651c4..1a3db7f66c 100644 --- a/tests/input/func_w0404.py +++ b/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 diff --git a/tests/input/func_w0405.py b/tests/input/func_w0405.py index 50a069d7a9..95c4d134f6 100644 --- a/tests/input/func_w0405.py +++ b/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 diff --git a/tests/input/func_w0612.py b/tests/input/func_w0612.py index 3d873bbebd..2c6fee8b8d 100644 --- a/tests/input/func_w0612.py +++ b/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 diff --git a/tests/unittest_checker_imports.py b/tests/unittest_checker_imports.py index 898be4ef59..2f27e4c526 100644 --- a/tests/unittest_checker_imports.py +++ b/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") )