diff --git a/desloppify/languages/_framework/treesitter/analysis/unused_imports.py b/desloppify/languages/_framework/treesitter/analysis/unused_imports.py index 666f694d1..bc8cc23a2 100644 --- a/desloppify/languages/_framework/treesitter/analysis/unused_imports.py +++ b/desloppify/languages/_framework/treesitter/analysis/unused_imports.py @@ -94,6 +94,13 @@ def detect_unused_imports( if not raw_path: continue + # Go blank (`_ "pkg"`) and dot (`. "pkg"`) imports are never + # "unused" by design: blank imports exist purely for their + # init() side effects, and dot imports inject every exported + # name into scope with no single identifier to search for. + if spec.grammar == "go" and _is_go_blank_or_dot_import(import_node): + continue + # Get the import statement's line range so we can exclude it # from the search. import_start = import_node.start_byte @@ -121,8 +128,15 @@ def detect_unused_imports( # When an alias is present, search for the alias name instead. alias_name = _extract_alias(import_node) - # Extract the imported name from the path. - name = alias_name or _extract_import_name(raw_path) + # Extract the imported name from the path. Go's in-code package + # identifier comes from the package clause, not necessarily the + # last path segment (versioned modules, gopkg.in-style versioning, + # hyphenated repo names), so it gets its own conservative resolver + # instead of the generic path-segment heuristic used elsewhere. + if alias_name is None and spec.grammar == "go": + name = _extract_go_package_name(raw_path) + else: + name = alias_name or _extract_import_name(raw_path) if not name: continue @@ -477,6 +491,68 @@ def _extract_grouped_import_names(import_path: str) -> list[str] | None: return names or None +_GO_VALID_IDENTIFIER_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$") +_GO_MAJOR_VERSION_SEGMENT_RE = re.compile(r"^v[0-9]+$") +_GO_VERSIONED_SEGMENT_RE = re.compile(r"^([A-Za-z_][A-Za-z0-9_]*)\.v[0-9]+$") + + +def _is_go_blank_or_dot_import(import_node) -> bool: + """Return True for Go blank (``_ "pkg"``) or dot (``. "pkg"``) imports. + + Blank imports exist purely for their ``init()`` side effects and are + never "used" by design. Dot imports inject every exported name from the + package directly into the file's scope with no qualifier, so there is + no single identifier that could be searched for. Both must be excluded + from unused-import reporting rather than guessed at. + """ + for i in range(import_node.child_count): + if import_node.children[i].type in ("blank_identifier", "dot"): + return True + return False + + +def _extract_go_package_name(import_path: str) -> str | None: + """Best-effort derivation of a Go package's in-code identifier. + + Go's identifier is whatever the dependency declares in its ``package`` + clause, which frequently differs from the last import-path segment: + + "gopkg.in/yaml.v3" -> "yaml" + "math/rand/v2" -> "rand" + "github.com/go-playground/validator/v10" -> "validator" + "github.com/anthropics/anthropic-sdk-go" -> None (unresolvable) + + Returns None when the identifier can't be confidently determined from + the path alone. Callers must treat None as "do not report a finding" -- + a false negative here is far cheaper than a false positive. + """ + segments = [s for s in import_path.split("/") if s] + if not segments: + return None + + candidate = segments[-1] + + # Go modules major-version suffix (e.g. ".../v2", ".../v10"): the version + # is its own path segment and is never part of the package identifier. + if len(segments) > 1 and _GO_MAJOR_VERSION_SEGMENT_RE.match(candidate): + candidate = segments[-2] + + # gopkg.in-style versioning embeds the version in the segment itself + # (e.g. "yaml.v3", "mgo.v2") instead of as a separate path segment. + versioned = _GO_VERSIONED_SEGMENT_RE.match(candidate) + if versioned: + candidate = versioned.group(1) + + if not _GO_VALID_IDENTIFIER_RE.match(candidate): + # e.g. "anthropic-sdk-go": hyphens can't appear in a Go identifier, + # so the path base can't be the identifier either. The real package + # name lives in the dependency's package clause, which isn't + # available here -- don't guess. + return None + + return candidate + + def _extract_import_name(import_path: str) -> str: """Extract the usable name from an import path. diff --git a/desloppify/tests/lang/common/test_go_unused_imports.py b/desloppify/tests/lang/common/test_go_unused_imports.py new file mode 100644 index 000000000..55d2fd262 --- /dev/null +++ b/desloppify/tests/lang/common/test_go_unused_imports.py @@ -0,0 +1,235 @@ +"""Regression tests for Go source unused-import detection. + +Go's in-code package identifier is defined by the dependency's ``package`` +clause, not the last import-path segment. These tests cover the false +positives that heuristic mismatch previously produced -- blank imports, dot +imports, versioned module paths, and hyphenated/otherwise-invalid path +segments -- plus confirmation that genuinely unused and aliased imports are +still detected correctly. +""" + +from __future__ import annotations + +import textwrap + + +def _detect(tmp_path, contents: str): + from desloppify.languages._framework.treesitter.analysis.unused_imports import ( + detect_unused_imports, + ) + from desloppify.languages._framework.treesitter.specs.compiled import GO_SPEC + + script = tmp_path / "main.go" + script.write_text(textwrap.dedent(contents).lstrip()) + return detect_unused_imports([str(script)], GO_SPEC) + + +def test_go_blank_import_is_never_flagged(tmp_path): + findings = _detect( + tmp_path, + """ + package main + + import ( + _ "github.com/joho/godotenv/autoload" + ) + + func main() {} + """, + ) + + assert findings == [] + + +def test_go_dot_import_is_never_flagged(tmp_path): + findings = _detect( + tmp_path, + """ + package main + + import ( + . "fmt" + ) + + func main() { + Println("hi") + } + """, + ) + + assert findings == [] + + +def test_go_gopkg_in_versioned_path_uses_package_name(tmp_path): + findings = _detect( + tmp_path, + """ + package main + + import ( + "gopkg.in/yaml.v3" + ) + + func main() { + yaml.Unmarshal(nil, nil) + } + """, + ) + + assert findings == [] + + +def test_go_modules_major_version_suffix_uses_package_name(tmp_path): + findings = _detect( + tmp_path, + """ + package main + + import ( + "math/rand/v2" + ) + + func main() { + rand.N(10) + } + """, + ) + + assert findings == [] + + +def test_go_modules_double_digit_major_version_suffix_uses_package_name(tmp_path): + findings = _detect( + tmp_path, + """ + package main + + import ( + "github.com/go-playground/validator/v10" + ) + + func main() { + validator.New() + } + """, + ) + + assert findings == [] + + +def test_go_hyphenated_repo_name_is_not_reported(tmp_path): + """Package name ("anthropic") differs from the repo/dir name and the + path base contains a hyphen, which can never be a valid Go identifier. + Since the real name can't be resolved from the path alone, this must + not be reported -- even though it's actually used below. + """ + findings = _detect( + tmp_path, + """ + package main + + import ( + "github.com/anthropics/anthropic-sdk-go" + ) + + func main() { + var _ anthropic.MessageParam + } + """, + ) + + assert findings == [] + + +def test_go_hyphenated_repo_name_still_not_reported_when_actually_unused(tmp_path): + """Same as above, but the import is genuinely unused. Because the + identifier can't be confidently derived from the path, we must stay + silent rather than guess -- a false negative here is far cheaper than + a false positive. + """ + findings = _detect( + tmp_path, + """ + package main + + import ( + "github.com/anthropics/anthropic-sdk-go" + ) + + func main() {} + """, + ) + + assert findings == [] + + +def test_go_aliased_import_uses_alias_name(tmp_path): + findings = _detect( + tmp_path, + """ + package main + + import ( + myyaml "gopkg.in/yaml.v2" + ) + + func main() { + myyaml.Unmarshal(nil, nil) + } + """, + ) + + assert findings == [] + + +def test_go_unused_aliased_import_is_still_flagged(tmp_path): + findings = _detect( + tmp_path, + """ + package main + + import ( + unusedpkg "fmt" + ) + + func main() {} + """, + ) + + assert [entry["name"] for entry in findings] == ["unusedpkg"] + + +def test_go_genuinely_unused_plain_import_is_still_flagged(tmp_path): + findings = _detect( + tmp_path, + """ + package main + + import ( + "os" + ) + + func main() {} + """, + ) + + assert [entry["name"] for entry in findings] == ["os"] + + +def test_go_used_plain_import_is_not_flagged(tmp_path): + findings = _detect( + tmp_path, + """ + package main + + import ( + "fmt" + ) + + func main() { + fmt.Println("hi") + } + """, + ) + + assert findings == [] diff --git a/desloppify/tests/lang/common/test_treesitter_analysis_direct.py b/desloppify/tests/lang/common/test_treesitter_analysis_direct.py index c06e2e2a2..d88f3239c 100644 --- a/desloppify/tests/lang/common/test_treesitter_analysis_direct.py +++ b/desloppify/tests/lang/common/test_treesitter_analysis_direct.py @@ -219,3 +219,46 @@ def test_unused_import_helpers_and_detection(monkeypatch) -> None: spec = SimpleNamespace(grammar="py", import_query="query") entries = unused_imports_mod.detect_unused_imports(["src/app.py"], spec) assert entries == [{"file": "src/app.py", "line": 1, "name": "module"}] + + +def test_go_blank_and_dot_import_detection_helper() -> None: + blank_import = FakeNode( + "import_spec", children=[FakeNode("blank_identifier", text="_")] + ) + dot_import = FakeNode("import_spec", children=[FakeNode("dot", text=".")]) + plain_import = FakeNode( + "import_spec", children=[FakeNode("interpreted_string_literal", text='"fmt"')] + ) + aliased_import = FakeNode( + "import_spec", children=[FakeNode("package_identifier", text="myyaml")] + ) + + assert unused_imports_mod._is_go_blank_or_dot_import(blank_import) is True + assert unused_imports_mod._is_go_blank_or_dot_import(dot_import) is True + assert unused_imports_mod._is_go_blank_or_dot_import(plain_import) is False + assert unused_imports_mod._is_go_blank_or_dot_import(aliased_import) is False + + +def test_go_package_name_resolution_helper() -> None: + resolve = unused_imports_mod._extract_go_package_name + + # Plain path: package name is the last segment. + assert resolve("fmt") == "fmt" + assert resolve("os") == "os" + + # gopkg.in-style versioning embeds the version in the last segment. + assert resolve("gopkg.in/yaml.v3") == "yaml" + assert resolve("gopkg.in/mgo.v2") == "mgo" + + # Go modules major-version suffix is its own path segment. + assert resolve("math/rand/v2") == "rand" + assert resolve("github.com/go-playground/validator/v10") == "validator" + + # Hyphenated path segments can never be valid Go identifiers, and the + # real package name isn't derivable from the path alone -- don't guess. + assert resolve("github.com/anthropics/anthropic-sdk-go") is None + + # A bare major-version-looking segment with no previous path segment to + # fall back to is left as-is; "v2" alone is a syntactically valid Go + # identifier, so it is returned rather than stripped to nothing. + assert resolve("v2") == "v2"