spack style: import-check -> import, fix bugs, exclude spack.pkg (#47690)

13 files changed, 50 insertions, 13 deletions

diff --git a/lib/spack/spack/cmd/modules/lmod.py b/lib/spack/spack/cmd/modules/lmod.py
index 4fd6992a47..3f2f7d2ec0 100644
--- a/lib/spack/spack/cmd/modules/lmod.py
+++ b/lib/spack/spack/cmd/modules/lmod.py
@@ -8,6 +8,7 @@ import functools
 import spack.cmd.common.arguments
 import spack.cmd.modules
 import spack.config
+import spack.modules
 import spack.modules.lmod
 
 
diff --git a/lib/spack/spack/cmd/modules/tcl.py b/lib/spack/spack/cmd/modules/tcl.py
index 4ca8ece704..e31d5bcc46 100644
--- a/lib/spack/spack/cmd/modules/tcl.py
+++ b/lib/spack/spack/cmd/modules/tcl.py
@@ -7,6 +7,7 @@ import functools
 import spack.cmd.common.arguments
 import spack.cmd.modules
 import spack.config
+import spack.modules
 import spack.modules.tcl
 
 
diff --git a/lib/spack/spack/cmd/style.py b/lib/spack/spack/cmd/style.py
index 5925d1120e..9d164875ae 100644
--- a/lib/spack/spack/cmd/style.py
+++ b/lib/spack/spack/cmd/style.py
@@ -15,6 +15,7 @@ import llnl.util.tty.color as color
 from llnl.util.filesystem import working_dir
 
 import spack.paths
+import spack.repo
 import spack.util.git
 from spack.util.executable import Executable, which
 
@@ -38,7 +39,7 @@ exclude_directories = [os.path.relpath(spack.paths.external_path, spack.paths.pr
 #: double-check the results of other tools (if, e.g., --fix was provided)
 #: The list maps an executable name to a method to ensure the tool is
 #: bootstrapped or present in the environment.
-tool_names = ["import-check", "isort", "black", "flake8", "mypy"]
+tool_names = ["import", "isort", "black", "flake8", "mypy"]
 
 #: warnings to ignore in mypy
 mypy_ignores = [
@@ -370,10 +371,19 @@ def run_black(black_cmd, file_list, args):
 
 def _module_part(root: str, expr: str):
     parts = expr.split(".")
+    # spack.pkg is for repositories, don't try to resolve it here.
+    if ".".join(parts[:2]) == spack.repo.ROOT_PYTHON_NAMESPACE:
+        return None
     while parts:
         f1 = os.path.join(root, "lib", "spack", *parts) + ".py"
         f2 = os.path.join(root, "lib", "spack", *parts, "__init__.py")
-        if os.path.exists(f1) or os.path.exists(f2):
+
+        if (
+            os.path.exists(f1)
+            # ensure case sensitive match
+            and f"{parts[-1]}.py" in os.listdir(os.path.dirname(f1))
+            or os.path.exists(f2)
+        ):
             return ".".join(parts)
         parts.pop()
     return None
@@ -389,7 +399,7 @@ def _run_import_check(
     out=sys.stdout,
 ):
     if sys.version_info < (3, 9):
-        print("import-check requires Python 3.9 or later")
+        print("import check requires Python 3.9 or later")
         return 0
 
     is_use = re.compile(r"(?<!from )(?<!import )(?:llnl|spack)\.[a-zA-Z0-9_\.]+")
@@ -431,10 +441,11 @@ def _run_import_check(
             module = _module_part(root, m.group(0))
             if not module or module in to_add:
                 continue
-            if f"import {module}" not in filtered_contents:
-                to_add.add(module)
-                exit_code = 1
-                print(f"{pretty_path}: missing import: {module}", file=out)
+            if re.search(rf"import {re.escape(module)}\b(?!\.)", contents):
+                continue
+            to_add.add(module)
+            exit_code = 1
+            print(f"{pretty_path}: missing import: {module} ({m.group(0)})", file=out)
 
         if not fix or not to_add and not to_remove:
             continue
@@ -465,7 +476,7 @@ def _run_import_check(
     return exit_code
 
 
-@tool("import-check", external=False)
+@tool("import", external=False)
 def run_import_check(import_check_cmd, file_list, args):
     exit_code = _run_import_check(
         file_list,
@@ -474,7 +485,7 @@ def run_import_check(import_check_cmd, file_list, args):
         root=args.root,
         working_dir=args.initial_working_dir,
     )
-    print_tool_result("import-check", exit_code)
+    print_tool_result("import", exit_code)
     return exit_code
 
 
diff --git a/lib/spack/spack/directives.py b/lib/spack/spack/directives.py
index e8a89dc5a5..44d95f3837 100644
--- a/lib/spack/spack/directives.py
+++ b/lib/spack/spack/directives.py
@@ -36,7 +36,6 @@ import os.path
 import re
 from typing import Any, Callable, List, Optional, Tuple, Union
 
-import llnl.util.lang
 import llnl.util.tty.color
 
 import spack.deptypes as dt
diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index 4b35a093b8..a57e1183b4 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -27,6 +27,7 @@ from llnl.util.lang import elide_list
 
 import spack
 import spack.binary_distribution
+import spack.compiler
 import spack.compilers
 import spack.concretize
 import spack.config
diff --git a/lib/spack/spack/test/cmd/ci.py b/lib/spack/spack/test/cmd/ci.py
index 36aa992c63..bc50ca683f 100644
--- a/lib/spack/spack/test/cmd/ci.py
+++ b/lib/spack/spack/test/cmd/ci.py
@@ -17,6 +17,7 @@ from llnl.util.filesystem import mkdirp, working_dir
 import spack
 import spack.binary_distribution
 import spack.ci as ci
+import spack.cmd
 import spack.cmd.ci
 import spack.environment as ev
 import spack.hash_types as ht
diff --git a/lib/spack/spack/test/cmd/pkg.py b/lib/spack/spack/test/cmd/pkg.py
index 8dfdf4773d..8fdc89b1fd 100644
--- a/lib/spack/spack/test/cmd/pkg.py
+++ b/lib/spack/spack/test/cmd/pkg.py
@@ -10,6 +10,7 @@ import pytest
 
 from llnl.util.filesystem import mkdirp, working_dir
 
+import spack.cmd
 import spack.cmd.pkg
 import spack.main
 import spack.paths
diff --git a/lib/spack/spack/test/cmd/style.py b/lib/spack/spack/test/cmd/style.py
index eab4de18dd..ee33de9b4b 100644
--- a/lib/spack/spack/test/cmd/style.py
+++ b/lib/spack/spack/test/cmd/style.py
@@ -295,7 +295,7 @@ def test_style_with_black(flake8_package_with_errors):
 
 
 def test_skip_tools():
-    output = style("--skip", "import-check,isort,mypy,black,flake8")
+    output = style("--skip", "import,isort,mypy,black,flake8")
     assert "Nothing to run" in output
 
 
@@ -314,6 +314,7 @@ class Example(spack.build_systems.autotools.AutotoolsPackage):
 def foo(config: "spack.error.SpackError"):
     # the type hint is quoted, so it should not be removed
     spack.util.executable.Executable("example")
+    print(spack.__version__)
 '''
     file.write_text(contents)
     root = str(tmp_path)
@@ -330,6 +331,7 @@ def foo(config: "spack.error.SpackError"):
 
     assert "issues.py: redundant import: spack.cmd" in output
     assert "issues.py: redundant import: spack.config" not in output  # comment prevents removal
+    assert "issues.py: missing import: spack" in output  # used by spack.__version__
     assert "issues.py: missing import: spack.build_systems.autotools" in output
     assert "issues.py: missing import: spack.util.executable" in output
     assert "issues.py: missing import: spack.error" not in output  # not directly used
@@ -349,6 +351,7 @@ def foo(config: "spack.error.SpackError"):
     output = output_buf.getvalue()
     assert exit_code == 1
     assert "issues.py: redundant import: spack.cmd" in output
+    assert "issues.py: missing import: spack" in output
     assert "issues.py: missing import: spack.build_systems.autotools" in output
     assert "issues.py: missing import: spack.util.executable" in output
 
@@ -369,8 +372,9 @@ def foo(config: "spack.error.SpackError"):
     # check that the file was fixed
     new_contents = file.read_text()
     assert "import spack.cmd" not in new_contents
-    assert "import spack.build_systems.autotools" in new_contents
-    assert "import spack.util.executable" in new_contents
+    assert "import spack\n" in new_contents
+    assert "import spack.build_systems.autotools\n" in new_contents
+    assert "import spack.util.executable\n" in new_contents
 
 
 @pytest.mark.skipif(sys.version_info < (3, 9), reason="requires Python 3.9+")
@@ -389,3 +393,16 @@ def test_run_import_check_syntax_error_and_missing(tmp_path: pathlib.Path):
     assert "syntax-error.py: could not parse" in output
     assert "missing.py: could not parse" in output
     assert exit_code == 1
+
+
+def test_case_sensitive_imports(tmp_path: pathlib.Path):
+    # example.Example is a name, while example.example is a module.
+    (tmp_path / "lib" / "spack" / "example").mkdir(parents=True)
+    (tmp_path / "lib" / "spack" / "example" / "__init__.py").write_text("class Example:\n    pass")
+    (tmp_path / "lib" / "spack" / "example" / "example.py").write_text("foo = 1")
+    assert spack.cmd.style._module_part(str(tmp_path), "example.Example") == "example"
+
+
+def test_pkg_imports():
+    assert spack.cmd.style._module_part(spack.paths.prefix, "spack.pkg.builtin.boost") is None
+    assert spack.cmd.style._module_part(spack.paths.prefix, "spack.pkg") is None
diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py
index a5a4fc9787..52e1cc9e04 100644
--- a/lib/spack/spack/test/config.py
+++ b/lib/spack/spack/test/config.py
@@ -15,6 +15,7 @@ import pytest
 import llnl.util.tty as tty
 from llnl.util.filesystem import join_path, touch, touchp
 
+import spack
 import spack.config
 import spack.directory_layout
 import spack.environment as ev
diff --git a/lib/spack/spack/test/modules/common.py b/lib/spack/spack/test/modules/common.py
index 7190689f82..c7f63f64b6 100644
--- a/lib/spack/spack/test/modules/common.py
+++ b/lib/spack/spack/test/modules/common.py
@@ -12,6 +12,7 @@ from llnl.util.symlink import readlink
 import spack.cmd.modules
 import spack.config
 import spack.error
+import spack.modules
 import spack.modules.common
 import spack.modules.tcl
 import spack.package_base
diff --git a/lib/spack/spack/test/package_class.py b/lib/spack/spack/test/package_class.py
index a8c541930b..72eaa1f739 100644
--- a/lib/spack/spack/test/package_class.py
+++ b/lib/spack/spack/test/package_class.py
@@ -21,6 +21,7 @@ import spack.compilers
 import spack.deptypes as dt
 import spack.error
 import spack.install_test
+import spack.package
 import spack.package_base
 import spack.repo
 import spack.spec
diff --git a/lib/spack/spack/util/path.py b/lib/spack/spack/util/path.py
index 5e7fa3a797..5967c595b4 100644
--- a/lib/spack/spack/util/path.py
+++ b/lib/spack/spack/util/path.py
@@ -55,6 +55,7 @@ NOMATCH = object()
 # Substitutions to perform
 def replacements():
     # break circular imports
+    import spack
     import spack.environment as ev
     import spack.paths
 
diff --git a/lib/spack/spack/util/web.py b/lib/spack/spack/util/web.py
index 892b64d333..7175629254 100644
--- a/lib/spack/spack/util/web.py
+++ b/lib/spack/spack/util/web.py
@@ -26,6 +26,7 @@ import llnl.url
 from llnl.util import lang, tty
 from llnl.util.filesystem import mkdirp, rename, working_dir
 
+import spack
 import spack.config
 import spack.error
 import spack.util.executable