Patchwork [4,of,8] import-checker: extend check of symbol-import order to all local modules

login
register
mail settings
Submitter Yuya Nishihara
Date May 14, 2016, 7:33 a.m.
Message ID <1aac72b5342d50b9a07e.1463211229@mimosa>
Download mbox | patch
Permalink /patch/15111/
State Accepted
Headers show

Comments

Yuya Nishihara - May 14, 2016, 7:33 a.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1463200773 -32400
#      Sat May 14 13:39:33 2016 +0900
# Node ID 1aac72b5342d50b9a07eb7747165d7cb8321fdaf
# Parent  c227e9d4e8c137fd1dd67ebd6b21aae8dfab68d1
import-checker: extend check of symbol-import order to all local modules

It doesn't make sense that (a) is allowed whereas (b) is disallowed.

 a) from mercurial import hg
    from mercurial.i18n import _

 b) from . import hg
    from .i18n import _

Patch

diff --git a/contrib/import-checker.py b/contrib/import-checker.py
--- a/contrib/import-checker.py
+++ b/contrib/import-checker.py
@@ -370,7 +370,7 @@  def verify_modern_convention(module, roo
     * Symbols can only be imported from specific modules (see
       `allowsymbolimports`). For other modules, first import the module then
       assign the symbol to a module-level variable. In addition, these imports
-      must be performed before other relative imports. This rule only
+      must be performed before other local imports. This rule only
       applies to import statements outside of any blocks.
     * Relative imports from the standard library are not allowed.
     * Certain modules must be aliased to alternate names to avoid aliasing
@@ -381,8 +381,8 @@  def verify_modern_convention(module, roo
 
     # Whether a local/non-stdlib import has been performed.
     seenlocal = None
-    # Whether a relative, non-symbol import has been seen.
-    seennonsymbolrelative = False
+    # Whether a local/non-stdlib, non-symbol import has been seen.
+    seennonsymbollocal = False
     # The last name to be imported (for sorting).
     lastname = None
     # Relative import levels encountered so far.
@@ -468,13 +468,14 @@  def verify_modern_convention(module, roo
                     yield msg('direct symbol import %s from %s',
                               ', '.join(symbols), fullname)
 
-                if symbols and seennonsymbolrelative:
+                if symbols and seennonsymbollocal:
                     yield msg('symbol import follows non-symbol import: %s',
                               fullname)
+            if not symbols and fullname not in stdlib_modules:
+                seennonsymbollocal = True
 
             if not node.module:
                 assert node.level
-                seennonsymbolrelative = True
 
                 # Only allow 1 group per level.
                 if (node.level in seenlevels
diff --git a/tests/test-check-module-imports.t b/tests/test-check-module-imports.t
--- a/tests/test-check-module-imports.t
+++ b/tests/test-check-module-imports.t
@@ -114,7 +114,16 @@  Run additional tests for the import chec
   > from testpackage.unsorted import foo
   > EOF
 
-  $ python "$import_checker" testpackage/*.py testpackage/subpackage/*.py
+  $ mkdir testpackage2
+  $ touch testpackage2/__init__.py
+
+  $ cat > testpackage2/latesymbolimport.py << EOF
+  > from __future__ import absolute_import
+  > from testpackage import unsorted
+  > from mercurial.node import hex
+  > EOF
+
+  $ python "$import_checker" testpackage*/*.py testpackage/subpackage/*.py
   testpackage/importalias.py:2: ui module must be "as" aliased to uimod
   testpackage/importfromalias.py:2: ui from testpackage must be "as" aliased to uimod
   testpackage/importfromrelative.py:2: import should be relative: testpackage.unsorted
@@ -132,6 +141,7 @@  Run additional tests for the import chec
   testpackage/subpackage/localimport.py:8: import should be relative: testpackage.subpackage.levelpriority
   testpackage/symbolimport.py:2: direct symbol import foo from testpackage.unsorted
   testpackage/unsorted.py:3: imports not lexically sorted: os < sys
+  testpackage2/latesymbolimport.py:3: symbol import follows non-symbol import: mercurial.node
   [1]
 
   $ cd "$TESTDIR"/..