globwalkignorederror + strconvparseignorederror: AssignStmt-only filter misses bare ExprStmt discards
- Dominant language
- Go
- Stars
- 5.1k
- Forks
- 541
- Avg merge
- 5h 48m
- Merged PRs (30d)
- 773
Description
**Linters affected:** `globwalkignorederror`, `strconvparseignorederror` (both CI-enforced, native + wasm, `-test=false`)
**Bug class:** `node_filter_too_narrow` — same class already fixed in the sibling `jsonmarshalignoredeerror` linter (issue #39982), but never applied here.
**Problem**
Both linters register only `*ast.AssignStmt` in their `nodeFilter` and require an exact 2-lhs/1-rhs shape with `Lhs[1]` being the blank identifier:
- `pkg/linters/globwalkignorederror/globwalkignorederror.go:33,46` — `nodeFilter := []ast.Node{(*ast.AssignStmt)(nil)}` then `if len(assign.Lhs) != 2 || len(assign.Rhs) != 1 { return }`
- `pkg/linters/strconvparseignorederror/strconvparseignorederror.go:35,48` — identical shape
This only catches the `v, _ := f()` spelling. It completely misses a bare call statement that discards both return values with no assignment at all, e.g. `os.ReadDir(dir)` or `strconv.Atoi(s)` used as a standalone `*ast.ExprStmt`. That form is legal Go (any call may be used as a statement, silently dropping every return value) and is exactly as silent a failure mode as the `_, _ :=` case both linters already flag.
**Reference fix already in the codebase**
`pkg/linters/jsonmarshalignoredeerror/jsonmarshalignoredeerror.go` hit this identical gap (issue #39982) and was fixed by adding `*ast.ExprStmt` to the node filter alongside `*ast.AssignStmt`:
```go
nodeFilter := []ast.Node{(*ast.AssignStmt)(nil), (*ast.ExprStmt)(nil)}
...
case *ast.ExprStmt:
checkDiscardedJSONExpr(pass, stmt, noLintIndex)
```
with `checkDiscardedJSONExpr` unwrapping `stmt.X.(*ast.CallExpr)` and running the same package/function match used for the assignment case. Neither `globwalkignorederror` nor `strconvparseignorederror` has an equivalent branch.
**Verified gap, not just theory**
- `pkg/linters/globwalkignorederror/testdata/src/globwalkignorederror/globwalkignorederror.go` and `pkg/linters/strconvparseignorederror/testdata/src/strconvparseignorederror/strconvparseignorederror.go` contain zero `ExprStmt` test cases — every `bad()`/`good()`/`suppressed()` fixture uses the `x, _ := f()` assignment form only.
- Both linters are wired into `.github/workflows/cgo.yml` `LINTER_FLAGS` for both the native and wasm CI jobs with `-test=false`, so they are meant to be a complete production gate, not a partial one.
- No live production site currently uses the bare-statement form (checked via grep across `pkg/`), so this is a latent enforcement gap rather than an active false-negative today — but it is a silent hole in a linter that is already CI-enforced, so any future PR that writes `os.ReadDir(dir)` or `strconv.Atoi(s)` as a bare statement passes the gate unnoticed.
**Recommendation**
Add an `*ast.ExprStmt` branch to both linters, mirroring the `checkDiscardedJSONExpr` helper in `jsonmarshalignoredeerror`: unwrap `stmt.X` as `*ast.CallExpr`, run the existing selector/package-match logic (`checkedFuncs` for globwalkignorederror, `strconvParseFuncs` for strconvparseignorederror), and report the same message used for the assignment case. Add one `ExprStmt`-shaped case to each testdata fixture (`want` comment included) to lock in coverage.
**Validation checklist**
- [ ] `os.ReadDir(dir)` / `filepath.Glob(pattern)` as a bare statement is flagged by `globwalkignorederror`
- [ ] `strconv.Atoi(s)` (and siblings) as a bare statement is flagged by `strconvparseignorederror`
- [ ] Existing assignment-form test cases still pass unchanged
- [ ] `nolint:globwalkignorederror` / `nolint:strconvparseignorederror` on the bare-statement form still suppresses correctly
**Effort:** small — one ExprStmt case per linter, following the exact pattern already proven in jsonmarshalignoredeerror.
> [!WARNING]
>
> Firewall blocked 1 domain
>
> The following domain was blocked by the firewall during workflow execution:
>
> - `api.anthropic.com`
>
> To allow these domains, add them to the `network.allowed` list in your workflow frontmatter:
>
> ```yaml
> network:
> allowed:
> - defaults
> - "api.anthropic.com"
> ```
>
> See [Network Configuration](https://github.github.com/gh-aw/reference/network/) for more information.
>
>
> Generated by [🤖 Sergo - Serena Go Expert](https://github.com/github/gh-aw/actions/runs/35053130946) · claude · agent · 320.4 AIC · ⌖ 7.91 AIC · ⊞ 6.8K · [◷](https://github.com/search?q=repo%3Agithub%2Fgh-aw+is%3Aissue+%22gh-aw-workflow-call-id%3A+github%2Fgh-aw%2Fsergo%22&type=issues)
> - [x] expires on Sep 22, 2026, 8:00 PM UTC-08:00
Contributor guide
Research direction
Start in pkg/linters/globwalkignorederror/globwalkignorederror.go and pkg/linters/strconvparseignorederror/strconvparseignorederror.go, then compare the ExprStmt handling in pkg/linters/jsonmarshalignoredeerror/jsonmarshalignoredeerror.go. Add matching bare-call coverage to each linter's testdata fixture and run the linter tests. Done means bare discarded calls are reported, existing assignments still pass, and nolint suppression works.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 88/100