Skip to content

jsonmarshalignoredeerror: only linter (1/59) missing filecheck wiring — fires on _test.go and generated files unlike every sibli [Content truncated due to length] #48767

Description

@github-actions

Summary

jsonmarshalignoredeerror (pkg/linters/jsonmarshalignoredeerror/jsonmarshalignoredeerror.go) is the only analyzer in the 59-linter registry (pkg/linters/registry.go) that does not wire internal/filecheck. Every other linter both Requires: filecheck.Analyzer and calls filecheck.ShouldSkipFilename(...) before reporting, which since the last audit cycle centrally skips both generated files and _test.go files (see pkg/linters/internal/filecheck/filecheck.go:76-78, ShouldSkipFilename = IsTestFile || IsGeneratedFile). jsonmarshalignoredeerror has neither the Requires entry nor a ShouldSkipFilename call anywhere in its run.

Evidence

  1. Registry-wide check of the Requires list for all 59 analyzers: jsonmarshalignoredeerror is the sole one missing filecheck.Analyzer:
    pkg/linters/jsonmarshalignoredeerror/jsonmarshalignoredeerror.go:21:
    Requires: []*analysis.Analyzer{inspect.Analyzer, nolint.Analyzer},
    (58/59 siblings list inspect.Analyzer, nolint.Analyzer, filecheck.Analyzer.)
  2. run() in that file never imports internal/filecheck and never calls ShouldSkipFilename — every ast.AssignStmt/ast.ExprStmt in the package is inspected unconditionally.
  3. Its direct sibling in the same "discarded error" family, strconvparseignorederror, wires filecheck.Analyzer and ships a testdata/src/strconvparseignorederror/generated.go fixture proving the generated-file skip. jsonmarshalignoredeerror's testdata/ has no equivalent file (only jsonmarshalignoredeerror.go), and its _test.go has zero references to filecheck/IsTestFile/ShouldSkipFilename.
  4. Historically this class of gap (missing internal/filecheck skip logic) was the trigger for the IsTestFile centralization: as of this run no linter calls filecheck.IsTestFile directly anymore — they all get test-file skip for free via ShouldSkipFilename, except this one, which gets neither test-file nor generated-file protection.

Impact

  • Generated-file false positives: any vendored/generated Go file (protobuf, mockgen, codegen) that does _, _ = json.Marshal(x) or _ = json.Unmarshal(data, &v) would be flagged, even though every other analyzer in the suite intentionally exempts generated code.
  • Test-file false positives: intentionally-discarded errors in test fixtures/doubles (_ = json.Unmarshal(badJSON, &v) to set up a failure scenario) would also be flagged, unlike all 58 siblings which skip _test.go.
  • No current prod violation was found in pkg/ today, so this is latent rather than CI-breaking, but it is a straightforward parity gap versus the rest of the registry and will surface as soon as a generated or test file contains the pattern.

Recommendation

  • Add filecheck.Analyzer to jsonmarshalignoredeerror's Requires.
  • Call filecheck.Index(pass) in run() and guard both checkDiscardedJSONAssign/checkDiscardedJSONExpr call sites with filecheck.ShouldSkipFilename(pos.Filename, generatedFiles), mirroring strconvparseignorederror.go.
  • Add a testdata/src/jsonmarshalignoredeerror/generated.go fixture (copy the pattern from strconvparseignorederror's) to lock in the skip behavior and prevent regression.

Validation checklist

  • filecheck.Analyzer added to Requires
  • ShouldSkipFilename guards both assign and bare-expr code paths
  • New generated.go testdata fixture added and asserted clean
  • Existing _test.go cases still pass unchanged

Effort

Small — mirrors an existing, already-correct sibling implementation (strconvparseignorederror) almost line for line.

Generated by 🤖 Sergo - Serena Go Expert · age00 · 173.6 AIC · ⌖ 5.9 AIC · ⊞ 5.8K ·

  • expires on Aug 4, 2026, 8:58 PM UTC-08:00

Metadata

Metadata

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