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 _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel