From 18cd922aabb4a3c318d3d57749b940a13ae4a50c Mon Sep 17 00:00:00 2001 From: Harmen Stoppels Date: Tue, 11 Feb 2025 19:30:50 +0100 Subject: [PATCH] style.py: fix false negative in redundant import statements (#48980) --- lib/spack/spack/cmd/style.py | 7 ++++--- lib/spack/spack/test/cmd/style.py | 4 ++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/spack/spack/cmd/style.py b/lib/spack/spack/cmd/style.py index 82df6ad0f80..887698ddd75 100644 --- a/lib/spack/spack/cmd/style.py +++ b/lib/spack/spack/cmd/style.py @@ -6,7 +6,7 @@ import os import re import sys -from itertools import zip_longest +from itertools import islice, zip_longest from typing import Dict, List, Optional import llnl.util.tty as tty @@ -423,7 +423,8 @@ def _run_import_check( continue for m in is_abs_import.finditer(contents): - if contents.count(m.group(1)) == 1: + # Find at most two occurences: the first is the import itself, the second is its usage. + if len(list(islice(re.finditer(rf"{re.escape(m.group(1))}(?!\w)", contents), 2))) == 1: to_remove.append(m.group(0)) exit_code = 1 print(f"{pretty_path}: redundant import: {m.group(1)}", file=out) @@ -438,7 +439,7 @@ def _run_import_check( module = _module_part(root, m.group(0)) if not module or module in to_add: continue - if re.search(rf"import {re.escape(module)}\b(?!\.)", contents): + if re.search(rf"import {re.escape(module)}(?!\w|\.)", contents): continue to_add.add(module) exit_code = 1 diff --git a/lib/spack/spack/test/cmd/style.py b/lib/spack/spack/test/cmd/style.py index 6cbd97456d8..9a21deff678 100644 --- a/lib/spack/spack/test/cmd/style.py +++ b/lib/spack/spack/test/cmd/style.py @@ -304,6 +304,8 @@ def test_run_import_check(tmp_path: pathlib.Path): contents = ''' import spack.cmd import spack.config # do not drop this import because of this comment +import spack.repo +import spack.repo_utils # this comment about spack.error should not be removed class Example(spack.build_systems.autotools.AutotoolsPackage): @@ -314,6 +316,7 @@ 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__) + print(spack.repo_utils.__file__) ''' file.write_text(contents) root = str(tmp_path) @@ -329,6 +332,7 @@ def foo(config: "spack.error.SpackError"): output = output_buf.getvalue() assert "issues.py: redundant import: spack.cmd" in output + assert "issues.py: redundant import: spack.repo" 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