Patchwork [2,of,2] import-checker: tell which symbol causes "direct symbol import"

login
register
mail settings
Submitter Yuya Nishihara
Date Dec. 7, 2015, 3 p.m.
Message ID <15e6bb4db3cc1e500f2a.1449500409@mimosa>
Download mbox | patch
Permalink /patch/11903/
State Accepted
Headers show

Comments

Yuya Nishihara - Dec. 7, 2015, 3 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1449379715 -32400
#      Sun Dec 06 14:28:35 2015 +0900
# Node ID 15e6bb4db3cc1e500f2a0adb8876e4c96d2e36fc
# Parent  451851fe49b160239248661e0ef9dc907529b51e
import-checker: tell which symbol causes "direct symbol import"

This would be sometimes useful to understand why import-checker.py complains
about it.
Augie Fackler - Dec. 7, 2015, 10:40 p.m.
On Tue, Dec 08, 2015 at 12:00:09AM +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1449379715 -32400
> #      Sun Dec 06 14:28:35 2015 +0900
> # Node ID 15e6bb4db3cc1e500f2a0adb8876e4c96d2e36fc
> # Parent  451851fe49b160239248661e0ef9dc907529b51e
> import-checker: tell which symbol causes "direct symbol import"

Queued these, thanks!

>
> This would be sometimes useful to understand why import-checker.py complains
> about it.
>
> diff --git a/contrib/import-checker.py b/contrib/import-checker.py
> --- a/contrib/import-checker.py
> +++ b/contrib/import-checker.py
> @@ -453,7 +453,8 @@ def verify_modern_convention(module, roo
>                      symbols = [n.name for n in node.names]
>
>                  if symbols and fullname not in allowsymbolimports:
> -                    yield msg('direct symbol import from %s', fullname)
> +                    yield msg('direct symbol import %s from %s',
> +                              ', '.join(symbols), fullname)
>
>                  if symbols and seennonsymbolrelative:
>                      yield msg('symbol import follows non-symbol import: %s',
> 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
> @@ -117,8 +117,8 @@ Run additional tests for the import chec
>    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 from testpackage.unsorted
> -  testpackage/importsymbolfromsub.py:2: direct symbol import from testpackage.subpackage
> +  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
> @@ -129,7 +129,7 @@ Run additional tests for the import chec
>    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 from testpackage.unsorted
> +  testpackage/symbolimport.py:2: direct symbol import foo from testpackage.unsorted
>    testpackage/unsorted.py:3: imports not lexically sorted: os < sys
>    [1]
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/contrib/import-checker.py b/contrib/import-checker.py
--- a/contrib/import-checker.py
+++ b/contrib/import-checker.py
@@ -453,7 +453,8 @@  def verify_modern_convention(module, roo
                     symbols = [n.name for n in node.names]
 
                 if symbols and fullname not in allowsymbolimports:
-                    yield msg('direct symbol import from %s', fullname)
+                    yield msg('direct symbol import %s from %s',
+                              ', '.join(symbols), fullname)
 
                 if symbols and seennonsymbolrelative:
                     yield msg('symbol import follows non-symbol import: %s',
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
@@ -117,8 +117,8 @@  Run additional tests for the import chec
   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 from testpackage.unsorted
-  testpackage/importsymbolfromsub.py:2: direct symbol import from testpackage.subpackage
+  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
@@ -129,7 +129,7 @@  Run additional tests for the import chec
   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 from testpackage.unsorted
+  testpackage/symbolimport.py:2: direct symbol import foo from testpackage.unsorted
   testpackage/unsorted.py:3: imports not lexically sorted: os < sys
   [1]