Patchwork [1,of,2] import-checker: allow import of child modules from package root

login
register
mail settings
Submitter Yuya Nishihara
Date Nov. 15, 2015, 1:31 p.m.
Message ID <24d83ae70e628ed137f5.1447594287@mimosa>
Download mbox | patch
Permalink /patch/11403/
State Accepted
Delegated to: Pierre-Yves David
Headers show

Comments

Yuya Nishihara - Nov. 15, 2015, 1:31 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1446305842 -32400
#      Sun Nov 01 00:37:22 2015 +0900
# Node ID 24d83ae70e628ed137f5af4e253ab4204597a247
# Parent  32bd195ab78752a7324a2f40a93f8e6026d3f9bc
import-checker: allow import of child modules from package root

I got the following error by rewriting hgweb/__init__.py to use
absolute_import, which is obviously wrong:

  Import cycle: mercurial.hgweb.__init__ -> mercurial.hgweb.__init__

"from foo import bar" should not make a cycle if "foo" is a package and
if "bar" is a module or a package. On the other hand, it should be detected
as a cycle if "bar" is a non-module name. Both cases are doc-tested already,
so this patch does not add new doctest.

Patch

diff --git a/contrib/import-checker.py b/contrib/import-checker.py
--- a/contrib/import-checker.py
+++ b/contrib/import-checker.py
@@ -239,7 +239,7 @@  def imported_modules(source, modulename,
     >>> sorted(imported_modules(
     ...        'import foo1; from bar import bar1',
     ...        modulename, localmods))
-    ['foo.bar.__init__', 'foo.bar.bar1', 'foo.foo1']
+    ['foo.bar.bar1', 'foo.foo1']
     >>> sorted(imported_modules(
     ...        'from bar.bar1 import name1, name2, name3',
     ...        modulename, localmods))
@@ -286,19 +286,26 @@  def imported_modules(source, modulename,
                 continue
 
             absname, dottedpath, hassubmod = found
-            yield dottedpath
             if not hassubmod:
+                # "dottedpath" is not a package; must be imported
+                yield dottedpath
                 # examination of "node.names" should be redundant
                 # e.g.: from mercurial.node import nullid, nullrev
                 continue
 
+            modnotfound = False
             prefix = absname + '.'
             for n in node.names:
                 found = fromlocal(prefix + n.name)
                 if not found:
                     # this should be a function or a property of "node.module"
+                    modnotfound = True
                     continue
                 yield found[1]
+            if modnotfound:
+                # "dottedpath" is a package, but imported because of non-module
+                # lookup
+                yield dottedpath
 
 def verify_import_convention(module, source):
     """Verify imports match our established coding convention.
diff --git a/tests/test-module-imports.t b/tests/test-module-imports.t
--- a/tests/test-module-imports.t
+++ b/tests/test-module-imports.t
@@ -68,6 +68,12 @@  Run additional tests for the import chec
   > from .. import parent
   > EOF
 
+  $ touch testpackage/subpackage/foo.py
+  $ cat > testpackage/subpackage/__init__.py << EOF
+  > from __future__ import absolute_import
+  > from . import levelpriority  # should not cause cycle
+  > EOF
+
   $ cat > testpackage/sortedentries.py << EOF
   > from __future__ import absolute_import
   > from . import (