Skip to content

globwalkignorederror + strconvparseignorederror: shared matcher still misses bare ExprStmt discards (3rd occurrence, sibling fix #66009

Description

@github-actions

Problem

pkg/linters/internal/astutil.MatchDiscardedErrorCall (astutil.go:50-79) is the single shared helper now used by jsonmarshalignoredeerror, globwalkignorederror, and strconvparseignorederror after the recent duplicate-code consolidation refactor (#65436 / #65567). It only matches an *ast.AssignStmt of the exact two-value form value, _ := pkg.Func(...) (astutil.go:51: len(assign.Lhs) != 2 || len(assign.Rhs) != 1). It has no *ast.ExprStmt branch.

globwalkignorederror.go:34 and strconvparseignorederror.go:35 each register nodeFilter := []ast.Node{(*ast.AssignStmt)(nil)} only, so a bare statement call such as os.ReadDir(dir) or strconv.Atoi(s) (legal Go, silently discards both return values with no assignment at all) is completely invisible to both linters.

This is a reverse-phantom

The identical gap was filed as #61265 (2026-09-16) and refiled as #63094 (2026-09-22) after #61265 auto-expired not_planned without a fix. #63094 itself auto-expired not_planned and was never refiled in the runs since, because the consolidation refactor touched this exact code path and the topic fell outside the bounded 3-run strategy-history window. Re-reading the current code confirms the bug is still fully present in both linters today, the 3rd occurrence of this lineage.

The fix template already exists next to the bug

jsonmarshalignoredeerror.go, the third caller of the very same shared helper in the very same refactor, already does this correctly: it registers a dual nodeFilter := []ast.Node{(*ast.AssignStmt)(nil), (*ast.ExprStmt)(nil)} (line 28), calls MatchDiscardedErrorCall for the AssignStmt case exactly like its siblings, and additionally dispatches *ast.ExprStmt nodes to its own checkDiscardedJSONExpr (lines 37, 76-88) which type-asserts stmt.X.(*ast.CallExpr) directly. The consolidation refactor copied the AssignStmt-matching logic into a shared helper but did not carry over this companion ExprStmt handling to globwalkignorederror or strconvparseignorederror, even though the reference implementation sits in the same package tree.

Impact

Both linters are CI-enforced (native and wasm, cgo.yml LINTER_FLAGS). No live production false negative was found via grep for bare strconv.Atoi/filepath.Glob/os.ReadDir statement calls in pkg/ today, so this is latent rather than actively masking a bug, but it is a zero-test-coverage CI-enforcement hole for one of the most common real-world ways to accidentally drop these errors: a bare statement call with no blank assignment at all.

Recommendation

Add an *ast.ExprStmt branch to both globwalkignorederror.run and strconvparseignorederror.run, following the exact jsonmarshalignoredeerror pattern: extend each nodeFilter to include (*ast.ExprStmt)(nil), then for *ast.ExprStmt nodes type-assert stmt.X.(*ast.CallExpr) and reuse each linters existing pkgPath/funcName matching logic (astutil.PackageCall is already available and used elsewhere for this).

Validation checklist

  • globwalkignorederror gains an ExprStmt branch mirroring jsonmarshalignoredeerror checkDiscardedJSONExpr
  • strconvparseignorederror gains the same
  • New testdata cases cover bare os.ReadDir(dir) / filepath.Glob(pattern) / strconv.Atoi(s) statement calls with no assignment
  • Consider whether MatchDiscardedErrorCall itself should grow ExprStmt support so future callers get it for free, instead of each caller re-implementing the companion branch

Effort

Small, two linters, each needing a few lines mirroring an already-shipped reference implementation in the same codebase.

Generated by 🤖 Sergo - Serena Go Expert · claude · agent · 219.5 AIC · ⌖ 6.36 AIC · ⊞ 5.2K · ◷

  • expires on Oct 12, 2026, 8:02 PM UTC-08:00

Activity

  1. pelikhan commented on Oct 11, 2026

    @pelikhan
    Collaborator

    Reviewed against completed essentials issue #67451 (#67451) and merged PR #67544 (#67544).

    The shared matcher and affected analyzers now handle bare expression statements that discard error returns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    cookieIssue Monster Loves Cookies!sergo

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions