D551: import-checker: allow direct symbol import if the symbol is a module

[quark (Jun Wu)]

quark created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  This would make the checker more friendly for 3rd-party code. For example,
  
  In remotefilelog/x.py, it may have:
  
    from . import shallowutils
  
  This triggers "direct symbol import shallowutils from remotefilelog" today.
  Since "shallowutils" itself is a module, the import should be allowed. This
  patch makes it so.
  
  It seems the warning is mainly to avoid the situation where other code could
  wrap some symbols (typically by `extensions.wrapfunction`) after import, and
  the existing code would still be using old symbols. But it's rare to have
  an entire module replaced. So I think this change is reasonable.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D551

AFFECTED FILES
  contrib/import-checker.py
  tests/test-imports-checker.t

CHANGE DETAILS

diff --git a/tests/test-imports-checker.t b/tests/test-imports-checker.t
--- a/tests/test-imports-checker.t
+++ b/tests/test-imports-checker.t
@@ -129,8 +129,6 @@
   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
-  testpackage/importfromrelative.py:2: direct symbol import foo from testpackage.unsorted
-  testpackage/importsymbolfromsub.py:2: direct symbol import nonmodule from testpackage.subpackage
   testpackage/latesymbolimport.py:3: symbol import follows non-symbol import: mercurial.node
   testpackage/multiple.py:2: multiple imported names: os, sys
   testpackage/multiplegroups.py:3: multiple "from . import" statements
@@ -141,7 +139,6 @@
   testpackage/subpackage/levelpriority.py:3: higher-level import should come first: testpackage
   testpackage/subpackage/localimport.py:7: multiple "from .. import" statements
   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]
diff --git a/contrib/import-checker.py b/contrib/import-checker.py
--- a/contrib/import-checker.py
+++ b/contrib/import-checker.py
@@ -499,7 +499,8 @@
                     seenlocal = fullname
 
             # Direct symbol import is only allowed from certain modules and
-            # must occur before non-symbol imports.
+            # must occur before non-symbol imports, or the symbol itself is a
+            # local module.
             found = fromlocal(node.module, node.level)
             if found and found[2]:  # node.module is a package
                 prefix = found[0] + '.'
@@ -509,7 +510,9 @@
                 symbols = (n.name for n in node.names)
             symbols = [sym for sym in symbols if sym not in directsymbols]
             if node.module and node.col_offset == root_col_offset:
-                if symbols and fullname not in allowsymbolimports:
+                if (symbols and fullname not in allowsymbolimports
+                    and fullname not in localmods
+                    and fullname + '.__init__' not in localmods):
                     yield msg('direct symbol import %s from %s',
                               ', '.join(symbols), fullname)
 



To: quark, #hg-reviewers
Cc: mercurial-devel