Wire jsonmarshalignoredeerror into filecheck and skip generated/test files

[Copilot]

jsonmarshalignoredeerror was the only linter in the registry not using internal/filecheck, so it reported discarded json.Marshal/json.Unmarshal errors in generated and _test.go files unlike its peers. This aligns it with the shared skip contract used by the rest of the analyzer suite.

• Analyzer dependency parity

  • Added filecheck.Analyzer to jsonmarshalignoredeerror.Analyzer.Requires.

• Filename skip guard in both report paths

  • In run(), added generatedFiles := filecheck.Index(pass).
  • Guarded both AssignStmt and ExprStmt branches with filecheck.ShouldSkipFilename(...) before calling check/report logic.

• Regression fixture for generated-file behavior

  • Added testdata/src/jsonmarshalignoredeerror/generated.go with discarded json.Marshal error patterns to ensure generated files remain excluded.

position := pass.Fset.PositionFor(stmt.Pos(), false)
if filecheck.ShouldSkipFilename(position.Filename, generatedFiles) {
	return
}

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

[github-actions]

✅ Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #48776 does not have the 'implementation' label and has only 25 new lines of code in business logic directories (threshold: 100).

[github-actions]

✅ Test Quality Sentinel completed test quality analysis.

No test files were added or modified in this PR. Test Quality Sentinel skipped.

[github-actions]

✅ PR Code Quality Reviewer completed the code quality review.

[Copilot]

Pull request overview

Aligns jsonmarshalignoredeerror with the shared generated/test-file exclusion behavior.

Changes:

• Adds the filecheck analyzer dependency.
• Skips assignments and expressions in excluded files.
• Adds a generated-file regression fixture.

Show a summary per file

File	Description
pkg/linters/jsonmarshalignoredeerror/jsonmarshalignoredeerror.go	Integrates filename exclusion checks.
pkg/linters/jsonmarshalignoredeerror/testdata/src/jsonmarshalignoredeerror/generated.go	Tests generated assignment exclusion; bare expressions remain uncovered.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Files not reviewed (1)

• pkg/linters/jsonmarshalignoredeerror/testdata/src/jsonmarshalignoredeerror/generated.go: Generated file

• Files reviewed: 1/2 changed files
• Comments generated: 1
• Review effort level: Medium

[github-actions]

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

[github-actions]

The implementation is correct and consistent with how peer linters use filecheck. The filecheck.Analyzer dependency is properly added to Requires, filecheck.Index is called once before the node traversal, and both AssignStmt and ExprStmt branches are guarded with ShouldSkipFilename. The test fixture correctly has no want directives on the generated-file violations.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 14 AIC · ⌖ 5.18 AIC · ⊞ 5K

[github-actions]

Skills-Based Review 🧠

Applied /tdd — two test-coverage gaps found; no blocking correctness issues.

📋 Key Themes & Highlights

Key Themes

• Incomplete fixture coverage: generated.go only exercises the AssignStmt skip path; the ExprStmt path is unverified.
• Missing _test.go fixture: Peer linters include a test-file fixture to confirm _test.go files are silently skipped; this PR does not.

Positive Highlights

• ✅ Implementation correctly follows the exact pattern used by all peer linters — filecheck.Index + ShouldSkipFilename guarding both case branches.
• ✅ filecheck.Analyzer properly added to Requires, ensuring the index is available before run() executes.
• ✅ Error from filecheck.Index is propagated correctly.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 27.9 AIC · ⌖ 4.87 AIC · ⊞ 6.7K
_{Comment /matt to run again}

[github-actions]

[/tdd] The fixture only exercises the AssignStmt branch (json.Marshal with discard); the ExprStmt branch (json.Unmarshal bare call, no assignment) is unverified.

💡 Add the missing variant

// Code generated by tests. DO NOT EDIT.

package jsonmarshalignoredeerror

import "encoding/json"

func generatedBad() {
	f := Foo{X: 1}
	val, _ := json.Marshal(f)   // AssignStmt path – already covered
	_ = val
	json.Unmarshal([]byte(`{}`), &f) // ExprStmt path – should also be skipped silently
}

Without a // want directive on the Unmarshal call the test will fail if the ExprStmt guard is accidentally removed, giving a regression signal.

@copilot please address this.

[github-actions]

[/tdd] No _test.go fixture is included, so the _test.go skip path goes unverified. Peer linters (wgdonenotdeferred) include a testdata/src/.../wgdonenotdeferred_test.go fixture to confirm the guard works.

💡 Add a test-file fixture

Create testdata/src/jsonmarshalignoredeerror/jsonmarshalignoredeerror_test.go:

package jsonmarshalignoredeerror

import "encoding/json"

func TestSkippedTestFile(f Foo) {
	v, _ := json.Marshal(f)              // AssignStmt in _test.go — should be skipped
	_ = v
	json.Unmarshal([]byte(`{}`), &f)     // ExprStmt in _test.go — should be skipped
}

No // want directives means the test harness will fail if either call produces a diagnostic.

@copilot please address this.

[github-actions]

Verdict: Approve — no blocking issues

This change wires jsonmarshalignoredeerror into the shared filecheck skip contract, exactly mirroring the pattern used by every other analyzer in the suite (bytescomparestring, mapclearloop, wgdonenotdeferred, etc.). The implementation is correct: it adds filecheck.Analyzer as a dependency, indexes generated files once per pass, and checks ShouldSkipFilename at both call sites (AssignStmt and ExprStmt) before running the discard checks. The added generated.go testdata fixture correctly has no want annotations, so analysistest.Run will fail if the skip logic regresses.

Notes

Ran go test ./pkg/linters/jsonmarshalignoredeerror/... — passes.

No correctness, concurrency, security, or maintainability issues found in the diff. The duplicated position := pass.Fset.PositionFor(...) in the two switch branches is a minor stylistic duplication but is consistent with how other linters in this repo structure the same check, so not worth flagging as a blocker.

🔎 Code quality review by PR Code Quality Reviewer · sonnet50 · 37.8 AIC · ⌖ 4.3 AIC · ⊞ 7.4K
_{Comment /review to run again}

[pelikhan]

@copilot run pr-finisher skill

[Copilot]

@copilot run pr-finisher skill

Added json.Marshal(f) (bare ExprStmt call) to generated.go in commit that addresses this. The fixture now independently covers both the AssignStmt and ExprStmt skip guard paths.

[github-actions]

🎉 This pull request is included in a new release.

Release: v0.83.5