You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Adds a new custom go/analysis linter, walkfuncerrshadow, that flags a specific filepath.Walk/filepath.WalkDir anti-pattern found repeatedly in this codebase:
Here the callback's err parameter shadows the outer err variable that holds the Walk/WalkDir call's own return value. This is confusing to read (which err is being checked/returned at any point?) and is an easy trap when refactoring: code added after the closure that references err may silently use the wrong variable if the outer declaration is later changed from := to =, or vice versa.
Evidence
Scanning pkg/ and cmd/ (excluding _test.go files) found this exact pattern in at least 9 places, including:
pkg/cli/logs_download.go:192, :505
pkg/cli/compile_watch.go:82
pkg/cli/trial_support.go:58
pkg/cli/remove_command.go:222, :342, :370
pkg/cli/dependency_graph.go:66
pkg/cli/logs_metrics.go:187, :680
Running the new linter against the full ./pkg/... and ./cmd/... tree confirms it flags exactly these call sites and no false positives, while correctly ignoring filepath.Walk calls where the callback uses a distinct parameter name (e.g. walkErr) or where the outer variable is not named err.
Implementation
New package pkg/linters/walkfuncerrshadow/ following the largefunc reference layout:
walkfuncerrshadow.go — the analyzer, using the shared nolint, filecheck, and astutil helper packages
walkfuncerrshadow_test.go — analysistest-based test
Registered in pkg/linters/registry.go (All()), and documented in pkg/linters/doc.go and pkg/linters/README.md to keep the doc-sync tests passing.
Testing
go test ./pkg/linters/... — all pass, including the new package and the doc/spec consistency tests
go build ./cmd/linters and ran the resulting binary against ./pkg/..../cmd/... — confirms the 9 real occurrences above and no unexpected findings
make fmt
Warning
Protected Files — Push Permission Denied
This was originally intended as a pull request, but the patch modifies protected files. A human must create the pull request manually.
Protected files
README.md
The push was rejected because GitHub Actions does not have workflows permission to push these changes, and is never allowed to make such changes, or other authorization being used does not have this permission.
Create the pull request manually
# Download the patch from the workflow run
gh run download 30710875181 -n agent -D /tmp/agent-30710875181
# Create a new branch
git checkout -b linter-miner/walkfuncerrshadow-a4b0c9f03f201baf main
# Apply the patch (--3way handles cross-repo patches)
git am --3way /tmp/agent-30710875181/aw-linter-miner-walkfuncerrshadow.patch
# Push the branch and create the pull request
git push origin linter-miner/walkfuncerrshadow-a4b0c9f03f201baf
gh pr create --title '[linter-miner] Add walkfuncerrshadow linter: detect err-shadowing in filepath.Walk callbacks' --base main --head linter-miner/walkfuncerrshadow-a4b0c9f03f201baf --repo github/gh-aw
Generated by Linter Miner · auto · 120.8 AIC · ⌖ 10.4 AIC · ⊞ 7.7K · ◷
Summary
Adds a new custom
go/analysislinter,walkfuncerrshadow, that flags a specificfilepath.Walk/filepath.WalkDiranti-pattern found repeatedly in this codebase:Here the callback's
errparameter shadows the outererrvariable that holds theWalk/WalkDircall's own return value. This is confusing to read (whicherris being checked/returned at any point?) and is an easy trap when refactoring: code added after the closure that referenceserrmay silently use the wrong variable if the outer declaration is later changed from:=to=, or vice versa.Evidence
Scanning
pkg/andcmd/(excluding_test.gofiles) found this exact pattern in at least 9 places, including:pkg/cli/logs_download.go:192,:505pkg/cli/compile_watch.go:82pkg/cli/trial_support.go:58pkg/cli/remove_command.go:222,:342,:370pkg/cli/dependency_graph.go:66pkg/cli/logs_metrics.go:187,:680Running the new linter against the full
./pkg/...and./cmd/...tree confirms it flags exactly these call sites and no false positives, while correctly ignoringfilepath.Walkcalls where the callback uses a distinct parameter name (e.g.walkErr) or where the outer variable is not namederr.Implementation
pkg/linters/walkfuncerrshadow/following thelargefuncreference layout:walkfuncerrshadow.go— the analyzer, using the sharednolint,filecheck, andastutilhelper packageswalkfuncerrshadow_test.go—analysistest-based testtestdata/src/walkfuncerrshadow/walkfuncerrshadow.go— fixture covering flagged and non-flagged cases (Walk,WalkDir, distinct param name, distinct outer variable name)pkg/linters/registry.go(All()), and documented inpkg/linters/doc.goandpkg/linters/README.mdto keep the doc-sync tests passing.Testing
go test ./pkg/linters/...— all pass, including the new package and the doc/spec consistency testsgo build ./cmd/lintersand ran the resulting binary against./pkg/..../cmd/...— confirms the 9 real occurrences above and no unexpected findingsmake fmtWarning
Protected Files — Push Permission Denied
This was originally intended as a pull request, but the patch modifies protected files. A human must create the pull request manually.
Protected files
README.mdThe push was rejected because GitHub Actions does not have
workflowspermission to push these changes, and is never allowed to make such changes, or other authorization being used does not have this permission.Create the pull request manually