From 722446bab92ad13489d73119d534c9a7cf8f8b2c Mon Sep 17 00:00:00 2001 From: Laura M?dioni Date: Mon, 9 Nov 2015 18:01:06 +0100 Subject: Check imports are ordered (standard, 3rd party, local) and grouped by package related to issue #692 --- CONTRIBUTORS.txt | 2 +- pylint/checkers/imports.py | 84 +++++++++++++++++++++- pylint/test/functional/class_members_py27.py | 2 +- pylint/test/functional/deprecated_module_py2.py | 2 +- .../functional/logging_format_interpolation.py | 2 +- pylint/test/functional/no_name_in_module.py | 2 +- pylint/test/functional/ungrouped_imports.py | 20 ++++++ pylint/test/functional/ungrouped_imports.txt | 5 ++ pylint/test/functional/unpacking_non_sequence.py | 2 +- pylint/test/functional/unsubscriptable_value.py | 2 +- pylint/test/functional/wrong_import_order.py | 11 +++ pylint/test/functional/wrong_import_order.txt | 4 ++ pylint/test/input/func_w0404.py | 2 +- pylint/test/input/func_w0405.py | 2 +- 14 files changed, 132 insertions(+), 10 deletions(-) create mode 100644 pylint/test/functional/ungrouped_imports.py create mode 100644 pylint/test/functional/ungrouped_imports.txt create mode 100644 pylint/test/functional/wrong_import_order.py create mode 100644 pylint/test/functional/wrong_import_order.txt diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index f05e053..276a63d 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -75,4 +75,4 @@ Order doesn't matter (not that much, at least ;) * Laura Medioni (Logilab, on behalf of the CNES): misplaced-comparison-constant, no-classmethod-decorator, no-staticmethod-decorator, too-many-nested-blocks, - too-many-boolean-expressions, unneeded-not \ No newline at end of file + too-many-boolean-expressions, unneeded-not, wrong-import-order, ungrouped-imports \ No newline at end of file diff --git a/pylint/checkers/imports.py b/pylint/checkers/imports.py index 59c6ed0..fc854ab 100644 --- a/pylint/checkers/imports.py +++ b/pylint/checkers/imports.py @@ -15,6 +15,7 @@ # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. """imports checkers for Python code""" +import os import sys from collections import defaultdict, Counter @@ -22,7 +23,8 @@ import six import astroid from astroid import are_exclusive -from astroid.modutils import get_module_part, is_standard_module +from astroid.modutils import (EXT_LIB_DIR, get_module_part, is_standard_module, + file_from_modpath) from pylint.interfaces import IAstroidChecker from pylint.utils import EmptyReport, get_global_option @@ -178,6 +180,13 @@ MSGS = { 'multiple-imports', 'Used when import statement importing multiple modules is ' 'detected.'), + 'C0411': ('%s comes before %s', + 'wrong-import-order', + 'Used when PEP8 import order is not observed (standard imports ' + 'first, then third-party libraries, then local imports)'), + 'C0412': ('Imports from package %s are not grouped', + 'ungrouped-imports', + 'Used when imports are not grouped by packages'), } class ImportsChecker(BaseChecker): @@ -227,11 +236,13 @@ given file (report RP0402 must not be disabled)'} given file (report RP0402 must not be disabled)'} ), ) + ext_lib_dir = os.path.normcase(os.path.abspath(EXT_LIB_DIR)) def __init__(self, linter=None): BaseChecker.__init__(self, linter) self.stats = None self.import_graph = None + self._imports_stack = [] self.__int_dep_info = self.__ext_dep_info = None self.reports = (('RP0401', 'External dependencies', self.report_external_dependencies), @@ -265,6 +276,11 @@ given file (report RP0402 must not be disabled)'} for name in names: self._check_deprecated_module(node, name) importedmodnode = self.get_imported_module(node, name) + if isinstance(node.scope(), astroid.Module): + importedname = importedmodnode.name if importedmodnode else None + if not importedname: + importedname = node.names[0][0].split('.')[0] + self._imports_stack.append((node, importedname)) if importedmodnode is None: continue self._check_relative_import(modnode, node, importedmodnode, name) @@ -292,6 +308,11 @@ given file (report RP0402 must not be disabled)'} self.add_message('wildcard-import', args=basename, node=node) modnode = node.root() importedmodnode = self.get_imported_module(node, basename) + if isinstance(node.scope(), astroid.Module): + importedname = importedmodnode.name if importedmodnode else None + if not importedname: + importedname = node.names[0][0].split('.')[0] + self._imports_stack.append((node, importedname)) if importedmodnode is None: return self._check_relative_import(modnode, node, importedmodnode, basename) @@ -309,6 +330,67 @@ given file (report RP0402 must not be disabled)'} self.add_message('reimported', node=node, args=(name, node.fromlineno)) + @check_messages('wrong-import-order', 'ungrouped-imports') + def leave_module(self, node): + # check imports are grouped by category (standard, 3rd party, local) + std_imports, ext_imports, loc_imports = self._check_imports_order(node) + # check imports are grouped by package within a given category + for imports in (std_imports, ext_imports, loc_imports): + packages = [] + for imp in imports: + if packages and packages[-1] == imp[1]: + continue + # check if an import from the same package has already been made + for package in packages: + if imp[1] == package: + self.add_message('ungrouped-imports', node=imp[0], + args=package) + break + packages.append(imp[1]) + self._imports_stack = [] + + def _check_imports_order(self, node): + """Checks imports of module `node` are grouped by category + + Imports must follow this order: standard, 3rd party, local + """ + extern_imports = [] + local_imports = [] + std_imports = [] + for node, modname in self._imports_stack: + if not modname: + local_imports.append((node, modname)) + continue + package = modname.split('.')[0] + if is_standard_module(modname): + std_imports.append((node, package)) + wrong_import = extern_imports or local_imports + if not wrong_import: + continue + self.add_message('wrong-import-order', node=node, + args=('standard import "%s"' % node.as_string(), + '"%s"' % wrong_import[0][0].as_string())) + else: + try: + filename = file_from_modpath([package]) + except ImportError: + local_imports.append((node, package)) + continue + if not filename: + local_imports.append((node, package)) + continue + filename = os.path.normcase(os.path.abspath(filename)) + if not filename.startswith(self.ext_lib_dir): + local_imports.append((node, package)) + continue + extern_imports.append((node, package)) + if not local_imports: + continue + self.add_message('wrong-import-order', node=node, + args=('external import "%s"' % node.as_string(), + '"%s"' % local_imports[0][0].as_string())) + return std_imports, extern_imports, local_imports + def get_imported_module(self, importnode, modname): try: return importnode.do_import_module(modname) diff --git a/pylint/test/functional/class_members_py27.py b/pylint/test/functional/class_members_py27.py index 415a890..97f2962 100644 --- a/pylint/test/functional/class_members_py27.py +++ b/pylint/test/functional/class_members_py27.py @@ -1,5 +1,5 @@ """ Various tests for class members access. """ -# pylint: disable=R0903,print-statement,no-absolute-import, metaclass-assignment,import-error,no-init,missing-docstring +# pylint: disable=R0903,print-statement,no-absolute-import, metaclass-assignment,import-error,no-init,missing-docstring, wrong-import-order from missing import Missing class MyClass(object): """class docstring""" diff --git a/pylint/test/functional/deprecated_module_py2.py b/pylint/test/functional/deprecated_module_py2.py index 399ed87..b0d1865 100644 --- a/pylint/test/functional/deprecated_module_py2.py +++ b/pylint/test/functional/deprecated_module_py2.py @@ -1,5 +1,5 @@ """Test deprecated modules.""" -# pylint: disable=unused-import,no-name-in-module,import-error +# pylint: disable=unused-import,no-name-in-module,import-error,ungrouped-imports import Bastion # [deprecated-module] import rexec # [deprecated-module] diff --git a/pylint/test/functional/logging_format_interpolation.py b/pylint/test/functional/logging_format_interpolation.py index 85117bf..5432d33 100644 --- a/pylint/test/functional/logging_format_interpolation.py +++ b/pylint/test/functional/logging_format_interpolation.py @@ -1,4 +1,4 @@ -# pylint: disable=E1101, no-absolute-import, import-error,line-too-long, missing-docstring +# pylint: disable=E1101, no-absolute-import, import-error,line-too-long, missing-docstring,wrong-import-order try: import __builtin__ as builtins diff --git a/pylint/test/functional/no_name_in_module.py b/pylint/test/functional/no_name_in_module.py index ba2ee9c..712611f 100644 --- a/pylint/test/functional/no_name_in_module.py +++ b/pylint/test/functional/no_name_in_module.py @@ -1,4 +1,4 @@ -#pylint: disable=W0401,W0611,no-absolute-import,invalid-name,import-error,bare-except,broad-except +#pylint: disable=W0401,W0611,no-absolute-import,invalid-name,import-error,bare-except,broad-except,wrong-import-order,ungrouped-imports """check unexistant names imported are reported""" from __future__ import print_function diff --git a/pylint/test/functional/ungrouped_imports.py b/pylint/test/functional/ungrouped_imports.py new file mode 100644 index 0000000..9626062 --- /dev/null +++ b/pylint/test/functional/ungrouped_imports.py @@ -0,0 +1,20 @@ +"""Checks import order rule""" +# pylint: disable=unused-import,relative-import,wrong-import-order,using-constant-test + +import six +import logging.config +import os.path +from astroid import are_exclusive +import logging # [ungrouped-imports] +import unused_import +try: + import os # [ungrouped-imports] +except ImportError: + pass +from os import pardir +import scipy +from os import sep +import astroid # [ungrouped-imports] +if True: + import logging.handlers # [ungrouped-imports] +from os.path import join # [ungrouped-imports] diff --git a/pylint/test/functional/ungrouped_imports.txt b/pylint/test/functional/ungrouped_imports.txt new file mode 100644 index 0000000..c29bb18 --- /dev/null +++ b/pylint/test/functional/ungrouped_imports.txt @@ -0,0 +1,5 @@ +ungrouped-imports:8::Imports from package logging are not grouped +ungrouped-imports:11::Imports from package os are not grouped +ungrouped-imports:17::Imports from package astroid are not grouped +ungrouped-imports:19::Imports from package logging are not grouped +ungrouped-imports:20::Imports from package os are not grouped diff --git a/pylint/test/functional/unpacking_non_sequence.py b/pylint/test/functional/unpacking_non_sequence.py index 1e5de23..c03b63d 100644 --- a/pylint/test/functional/unpacking_non_sequence.py +++ b/pylint/test/functional/unpacking_non_sequence.py @@ -3,8 +3,8 @@ # pylint: disable=too-few-public-methods, invalid-name, attribute-defined-outside-init, unused-variable, no-absolute-import # pylint: disable=using-constant-test, no-init from os import rename as nonseq_func -from functional.unpacking import nonseq from six import with_metaclass +from functional.unpacking import nonseq __revision__ = 0 diff --git a/pylint/test/functional/unsubscriptable_value.py b/pylint/test/functional/unsubscriptable_value.py index 221bd17..64cafaf 100644 --- a/pylint/test/functional/unsubscriptable_value.py +++ b/pylint/test/functional/unsubscriptable_value.py @@ -3,7 +3,7 @@ Checks that value used in a subscript supports subscription (i.e. defines __getitem__ method). """ # pylint: disable=missing-docstring,pointless-statement,expression-not-assigned -# pylint: disable=too-few-public-methods,import-error,invalid-name +# pylint: disable=too-few-public-methods,import-error,invalid-name,wrong-import-order import six # primitives diff --git a/pylint/test/functional/wrong_import_order.py b/pylint/test/functional/wrong_import_order.py new file mode 100644 index 0000000..9fc3996 --- /dev/null +++ b/pylint/test/functional/wrong_import_order.py @@ -0,0 +1,11 @@ +"""Checks import order rule""" +# pylint: disable=unused-import,relative-import,ungrouped-imports + +import six +import os.path # [wrong-import-order] +from astroid import are_exclusive +import sys # [wrong-import-order] +import datetime # [wrong-import-order] +import unused_import +import scipy # [wrong-import-order] +import astroid diff --git a/pylint/test/functional/wrong_import_order.txt b/pylint/test/functional/wrong_import_order.txt new file mode 100644 index 0000000..537355c --- /dev/null +++ b/pylint/test/functional/wrong_import_order.txt @@ -0,0 +1,4 @@ +wrong-import-order:5::standard import "import os.path" comes before "import six" +wrong-import-order:7::standard import "import sys" comes before "import six" +wrong-import-order:8::standard import "import datetime" comes before "import six" +wrong-import-order:10::external import "import scipy" comes before "from astroid import are_exclusive" diff --git a/pylint/test/input/func_w0404.py b/pylint/test/input/func_w0404.py index d65da51..b9169d7 100644 --- a/pylint/test/input/func_w0404.py +++ b/pylint/test/input/func_w0404.py @@ -10,7 +10,7 @@ from xml.etree import ElementTree from email import encoders import email.encoders -import sys +import sys #pylint: disable=ungrouped-imports def no_reimport(): """docstring""" diff --git a/pylint/test/input/func_w0405.py b/pylint/test/input/func_w0405.py index f1c1677..3f6efc7 100644 --- a/pylint/test/input/func_w0405.py +++ b/pylint/test/input/func_w0405.py @@ -2,7 +2,7 @@ """ from __future__ import absolute_import, print_function __revision__ = 0 -# pylint: disable=using-constant-test +# pylint: disable=using-constant-test,ungrouped-imports import os from os.path import join, exists -- cgit v1.2.1