fix: derive Go import identifiers from the package, not the path segment - #703
Open
dudell-bud wants to merge 1 commit into
Open
fix: derive Go import identifiers from the package, not the path segment#703dudell-bud wants to merge 1 commit into
dudell-bud wants to merge 1 commit into
Conversation
The unused-import detector took the last path segment as the in-code identifier. In Go that is frequently wrong, so every affected import was reported as unused. Scanning a real Go repo produced 20 false positives and no true positives: _ "github.com/joho/godotenv/autoload" -> "unused import: autoload" "gopkg.in/yaml.v3" -> "unused import: v3" "math/rand/v2" -> "unused import: v2" "github.com/go-playground/validator/v10"-> "unused import: v10" "github.com/anthropics/anthropic-sdk-go"-> "unused import: anthropic-sdk-go" These are used as yaml., rand., validator. and anthropic. respectively; the first is a blank import that exists only for its init() side effect. Note that a hyphenated segment can never be a Go identifier at all. There is also a general argument: go build fails on a genuinely unused import, so a Go module that compiles cannot have one, and any finding of this class on a compiling module is a false positive. Go now gets its own resolver that skips blank and dot imports, strips major-version segments (/v2, .v3), and returns None when the identifier cannot be determined from the path alone — callers treat None as "do not report", because a false negative is far cheaper than a false positive here. Verified: full suite passes (5825 passed, 3 skipped); scanning the repo that surfaced this drops its 17 unused-import findings to 0 with no new findings.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The unused-import detector uses the last path segment of an import path as the in-code identifier. In Go that is frequently wrong, so every affected import gets reported as unused.
Scanning a real Go repo (
desloppify --lang go scan) produced 20 false positives and no true positives:_ "github.com/joho/godotenv/autoload"unused import: autoloadinit()side effects"gopkg.in/yaml.v3"unused import: v3yaml.Unmarshal(...)"math/rand/v2"unused import: v2rand.N(...)"github.com/go-playground/validator/v10"unused import: v10validator.New()"github.com/anthropics/anthropic-sdk-go"unused import: anthropic-sdk-goanthropic.MessageParamNote the last row can never be right:
-is not legal in a Go identifier.There is also a general argument for treating this class conservatively:
go buildfails on a genuinely unused import, so a Go module that compiles cannot have one. Any finding of this class on a compiling module is therefore a false positive.Fix
Go now gets its own identifier resolver instead of the generic path-segment heuristic:
_) and dot (.) imports are skipped. A blank import exists purely for itsinit()side effect and is never "used" by design; a dot import injects every exported name with no qualifier to search for./v2,/v10, andgopkg.in-styleyaml.v3.None, and callers treatNoneas "do not report". Where the package clause cannot be determined from the path alone (e.g. a hyphenated repo name), staying silent is the right call: a false negative costs far less than a false positive.Verification
python -m pytest desloppify/tests/ -q→ 5825 passed, 3 skippeddesloppify/tests/lang/common/test_go_unused_imports.pycovering each case in the table above.🤖 Generated with Claude Code