[linter-miner] Add type-assertion-ok-discarded linter

docs/adr/61388-add-type-assertion-ok-discarded-linter.md

@@ -0,0 +1,46 @@
+# ADR-61388: Add type-assertion-ok-discarded linter
+
+**Date**: 2026-09-16
+**Status**: Draft
+**Deciders**: gh-aw maintainers
+
+---
+
+### Context
+
+This pull request adds a new Go analyzer under `pkg/linters/typeassertionokdiscarded/` and registers it in the shared linter registry. The implementation targets a specific pattern: two-value type assertions where the second `ok` result is explicitly discarded with `_`, as shown in the new analyzer test fixtures and described in the PR body. The repository already contains linter infrastructure, generated-file skipping, nolint support, and a complementary `uncheckedtypeassertion` analyzer, so the architectural question is whether this codebase should treat discarded-`ok` type assertions as a first-class lint violation. The non-negotiable constraint visible in the PR is to implement the check as a standard AST-based analyzer that integrates with the existing linter suite and test harness.
+
+### Decision
+
+We will add a dedicated `typeassertionokdiscarded` analyzer to the gh-aw linter suite to report two-value type assertions whose `ok` result is discarded with the blank identifier. The analyzer will inspect `ast.TypeAssertExpr` nodes, determine whether they participate in a two-value assignment or declaration with `_` in the second position, and emit a diagnostic directing authors to either use the single-value assertion form or actually check the `ok` result. We chose this because the PR evidence shows the repository wants explicit enforcement of this type-assertion anti-pattern through the same reusable analyzer framework used by other custom Go linters.
+
+### Alternatives Considered
+
+#### Alternative 1: Rely on the existing uncheckedtypeassertion linter only
+
+The repository already has an `uncheckedtypeassertion` analyzer, so one alternative was to keep enforcing only single-value assertion misuse and leave discarded-`ok` cases unaddressed. This was considered because it avoids another custom linter and keeps type-assertion guidance consolidated in one existing rule. It was not chosen because the PR body and new fixtures make clear that discarded-`ok` assertions are treated as a distinct anti-pattern with different remediation: either intentionally use the single-value form or check `ok` instead of discarding it.
+
+#### Alternative 2: Depend on code review or a generic external linter rule
+
+Another option was to document this as a style expectation and catch it during review, or to wait for a generic upstream lint rule to cover it. This was considered because it would avoid maintaining a repository-specific analyzer with AST parent tracking and tests. It was not chosen because the diff explicitly invests in an in-repo analyzer integrated with the current registry, generated-file handling, nolint directives, and analysistest fixtures, indicating the decision is to automate enforcement rather than rely on manual review or unavailable generic tooling.
+
+### Consequences
+
+#### Positive
+- The repository gains automated enforcement for a specific misleading type-assertion pattern that was previously easy to miss in review.
+- The new rule integrates with the existing custom linter registry, test harness, generated-file filtering, and `nolint` support.
+- The test fixtures document accepted and rejected type-assertion forms, making the intended coding standard more explicit.
+
+#### Negative
+- The project must maintain another custom analyzer, including AST parent mapping logic and ongoing compatibility with analyzer infrastructure.
+- Some contributors may need to rewrite existing code patterns or add justified suppressions when this rule is enabled against broader code.
+- The linter suite becomes slightly more complex because type-assertion guidance is now split across complementary analyzers.
+
+#### Neutral
+- The change affects static analysis behavior and tests, but does not alter runtime behavior of production code directly.
+- The analyzer distinguishes only assignments and declarations with a blank second result, so other type-assertion patterns remain governed by existing rules.
+- Registration in `pkg/linters/registry.go` makes the new analyzer part of the standard linter bundle used by the repository.
+
+---
+
+*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*

pkg/linters/README.md

@@ -72,6 +72,7 @@ This package currently provides custom Go analyzers in the following subpackages
 - `timenowsub` — reports `time.Now().Sub(t)` calls that should be simplified to `time.Since(t)`.
 - `tolowerequalfold` — reports case-insensitive string comparisons using `strings.ToLower`/`ToUpper` that should use `strings.EqualFold`.
 - `trimleftright` — reports `strings.TrimLeft`/`TrimRight` calls with a multi-character literal cutset where `TrimPrefix`/`TrimSuffix` was likely intended.
+- `typeassertionokdiscarded` — reports two-value type assertions whose `ok` result is discarded.
 - `uncheckedtypeassertion` — reports single-value type assertions where unchecked panics are possible.
 - `uncheckedflushreturn` — reports `Flush()` method calls where the error return is discarded, which silently drops buffered data on failure.
 - `wgdonenotdeferred` — reports non-deferred `sync.WaitGroup.Done()` calls that can deadlock on panics or early returns.
@@ -171,6 +172,7 @@ environment variable and gates findings on the recorded execution hit count for
 | `timenowsub` | Custom `go/analysis` analyzer that flags `time.Now().Sub(t)` calls that should use `time.Since(t)` |
 | `tolowerequalfold` | Custom `go/analysis` analyzer that flags case-insensitive comparisons via `strings.ToLower`/`ToUpper` that should use `strings.EqualFold` |
 | `trimleftright` | Custom `go/analysis` analyzer that flags `strings.TrimLeft`/`TrimRight` calls with a multi-character literal cutset where `TrimPrefix`/`TrimSuffix` was likely intended |
+| `typeassertionokdiscarded` | Custom `go/analysis` analyzer that flags two-value type assertions whose `ok` result is discarded |
 | `uncheckedtypeassertion` | Custom `go/analysis` analyzer that flags unchecked single-value type assertions |
 | `uncheckedflushreturn` | Custom `go/analysis` analyzer that flags `Flush()` method calls where the error return is discarded |
 | `walkfuncerrshadow` | Custom `go/analysis` analyzer that flags `filepath.Walk`/`filepath.WalkDir` callbacks whose `err` parameter shadows an outer `err` variable assigned from the walk call |
@@ -308,6 +310,7 @@ _ = trimleftright.Analyzer
 - `github.com/github/gh-aw/pkg/linters/timesleepnocontext` — time-sleep-no-context analyzer subpackage
 - `github.com/github/gh-aw/pkg/linters/tolowerequalfold` — to-lower-equal-fold analyzer subpackage
 - `github.com/github/gh-aw/pkg/linters/trimleftright` — trim-left-right analyzer subpackage
+- `github.com/github/gh-aw/pkg/linters/typeassertionokdiscarded` — type-assertion-ok-discarded analyzer subpackage
 - `github.com/github/gh-aw/pkg/linters/uncheckedtypeassertion` — unchecked-type-assertion analyzer subpackage
 - `github.com/github/gh-aw/pkg/linters/uncheckedflushreturn` — unchecked-flush-return analyzer subpackage
 - `github.com/github/gh-aw/pkg/linters/walkfuncerrshadow` — walk-func-err-shadow analyzer subpackage

pkg/linters/doc.go

@@ -1,6 +1,6 @@
 // Package linters is a namespace for gh-aw's custom Go analysis linters.
 //
-// All 71 active analyzers:
+// All 72 active analyzers:
 //
 //   - appendbytestring — flags append(b, []byte(s)...) calls where s is a string that can be simplified to append(b, s...)
 //   - appendoneelement — flags append(s, []T{x}...) calls where a single-element slice literal is spread and can be simplified to append(s, x)
@@ -68,6 +68,7 @@
 //   - timenowsub — reports time.Now().Sub(t) calls that should be simplified to time.Since(t)
 //   - tolowerequalfold — flags case-insensitive comparisons via ToLower/ToUpper that should use EqualFold
 //   - trimleftright — flags strings.TrimLeft/TrimRight calls with a multi-character literal cutset where TrimPrefix/TrimSuffix was likely intended
+//   - typeassertionokdiscarded — flags two-value type assertions whose ok result is discarded
 //   - uncheckedtypeassertion — flags unchecked single-value type assertions
 //   - uncheckedflushreturn — flags Flush() method calls where the error return is discarded
 //   - walkfuncerrshadow — flags filepath.Walk/WalkDir callbacks whose err parameter shadows an outer err variable assigned from the walk call

pkg/linters/doc_sync_test.go

@@ -38,6 +38,7 @@ var notYetEnforced = map[string]string{
 	"sprintferrdot":               "has not yet completed an enforcement-readiness audit",
 	"ssljson":                     "has not yet completed an enforcement-readiness audit",
 	"stringsconcatloop":           "has not yet completed an enforcement-readiness audit",
+	"typeassertionokdiscarded":    "existing production violations need remediation before enforcement; nolint suppression already works",
 }
 
 // TestDocGo_CountMatchesBullets validates that the "All N active analyzers:"

pkg/linters/internal/astutil/astutil.go

@@ -463,6 +463,54 @@ func FileForPos(files []*ast.File, pos token.Pos) *ast.File {
 	return nil
 }
 
+// BuildParentMap constructs a map from each AST node to its direct parent node.
+func BuildParentMap(root ast.Node) map[ast.Node]ast.Node {
+	parents := make(map[ast.Node]ast.Node)
+	var stack []ast.Node
+
+	ast.Inspect(root, func(n ast.Node) bool {
+		if n == nil {
+			if len(stack) > 0 {
+				stack = stack[:len(stack)-1]
+			}
+			return false
+		}
+		if len(stack) > 0 {
+			parents[n] = stack[len(stack)-1]
+		}
+		stack = append(stack, n)
+		return true
+	})
+
+	return parents
+}
+
+// TwoValueTypeAssertionOKIdent returns the ok identifier for a two-value type
+// assertion assignment or variable declaration.
+func TwoValueTypeAssertionOKIdent(typeAssert *ast.TypeAssertExpr, parents map[ast.Node]ast.Node) (*ast.Ident, bool) {
+	parent := parents[typeAssert]
+	for {
+		paren, ok := parent.(*ast.ParenExpr)
+		if !ok {
+			break
+		}
+		parent = parents[paren]
+	}
+
+	switch p := parent.(type) {
+	case *ast.AssignStmt:
+		if len(p.Lhs) == 2 && len(p.Rhs) == 1 {
+			okIdent, ok := p.Lhs[1].(*ast.Ident)
+			return okIdent, ok
+		}
+	case *ast.ValueSpec:
+		if len(p.Names) == 2 && len(p.Values) == 1 {
+			return p.Names[1], true
+		}
+	}
+	return nil, false
+}
+
 // CountPkgUsesInFile returns the number of times the package at pkgPath is
 // referenced as a selector base within file (e.g. each "fmt.X" call counts
 // as one use of the "fmt" package).

pkg/linters/registry.go

@@ -69,6 +69,7 @@ import (
 	"github.com/github/gh-aw/pkg/linters/timesleepnocontext"
 	"github.com/github/gh-aw/pkg/linters/tolowerequalfold"
 	"github.com/github/gh-aw/pkg/linters/trimleftright"
+	"github.com/github/gh-aw/pkg/linters/typeassertionokdiscarded"
 	"github.com/github/gh-aw/pkg/linters/uncheckedflushreturn"
 	"github.com/github/gh-aw/pkg/linters/uncheckedtypeassertion"
 	"github.com/github/gh-aw/pkg/linters/walkfuncerrshadow"
@@ -152,6 +153,7 @@ var allAnalyzers = []*analysis.Analyzer{
 	timenowsub.Analyzer,
 	tolowerequalfold.Analyzer,
 	trimleftright.Analyzer,
+	typeassertionokdiscarded.Analyzer,
 	uncheckedtypeassertion.Analyzer,
 	uncheckedflushreturn.Analyzer,
 	walkfuncerrshadow.Analyzer,

pkg/linters/spec_test.go

@@ -77,6 +77,7 @@ import (
 	"github.com/github/gh-aw/pkg/linters/timesleepnocontext"
 	"github.com/github/gh-aw/pkg/linters/tolowerequalfold"
 	"github.com/github/gh-aw/pkg/linters/trimleftright"
+	"github.com/github/gh-aw/pkg/linters/typeassertionokdiscarded"
 	"github.com/github/gh-aw/pkg/linters/uncheckedflushreturn"
 	"github.com/github/gh-aw/pkg/linters/uncheckedtypeassertion"
 	"github.com/github/gh-aw/pkg/linters/walkfuncerrshadow"
@@ -109,7 +110,7 @@ type docAnalyzer struct {
 //	logfatallibrary, manualmutexunlock, manualpathconcat, mapclearloop, mapdeletecheck, nilctxpassed, osexitinlibrary, osgetenvlibrary, ossetenvlibrary, packagelevelmutableslicemap, panic-in-library-code, rawloginlib,
 //	regexpcompileinfunction, regexpdynamicpattern, seenmapbool, slicemakezerolength, sortslice, sprintferrdot, sprintferrorsnew, sprintfbool, sprintfint, ssljson,
 //	strconvparseignorederror, stringbytesroundtrip, stringreplaceminusone, stringsconcatloop, stringscountcontains, stringsindexcontains, stringsindexhasprefix, stringsjoinone, timeafterleak, timesleepnocontext, timenowsub,
-//	tolowerequalfold, trimleftright, uncheckedflushreturn, uncheckedtypeassertion, walkfuncerrshadow, wgdonenotdeferred, writebytestring
+//	tolowerequalfold, trimleftright, typeassertionokdiscarded, uncheckedflushreturn, uncheckedtypeassertion, walkfuncerrshadow, wgdonenotdeferred, writebytestring
 func documentedAnalyzers() []docAnalyzer {
 	return []docAnalyzer{
 		{"appendbytestring", appendbytestring.Analyzer},
@@ -178,6 +179,7 @@ func documentedAnalyzers() []docAnalyzer {
 		{"timenowsub", timenowsub.Analyzer},
 		{"tolowerequalfold", tolowerequalfold.Analyzer},
 		{"trimleftright", trimleftright.Analyzer},
+		{"typeassertionokdiscarded", typeassertionokdiscarded.Analyzer},
 		{"uncheckedtypeassertion", uncheckedtypeassertion.Analyzer},
 		{"uncheckedflushreturn", uncheckedflushreturn.Analyzer},
 		{"walkfuncerrshadow", walkfuncerrshadow.Analyzer},

pkg/linters/typeassertionokdiscarded/testdata/src/typeassertionokdiscarded/typeassertionokdiscarded.go

@@ -0,0 +1,74 @@
+package typeassertionokdiscarded
+
+import "fmt"
+
+// Bad: two-value type assertion with blank ok identifier.
+func BadBlankOkAssign(v interface{}) {
+	s, _ := v.(string) // want `type assertion ok value is explicitly discarded with blank identifier`
+	fmt.Println(s)
+}
+
+// Bad: two-value var declaration with blank ok identifier.
+func BadBlankOkVarDecl(v interface{}) {
+	var s, _ = v.(string) // want `type assertion ok value is explicitly discarded with blank identifier`
+	fmt.Println(s)
+}
+
+// Bad: two-value reassignment with blank ok identifier.
+func BadBlankOkReassign(v interface{}) string {
+	var s string
+	s, _ = v.(string) // want `type assertion ok value is explicitly discarded with blank identifier`
+	return s
+}
+
+// Good: single-value type assertion that may panic.
+func GoodSingleValue(v interface{}) string {
+	return v.(string)
+}
+
+// Good: single-value assignment.
+func GoodSingleValueAssign(v interface{}) {
+	s := v.(string)
+	fmt.Println(s)
+}
+
+// Good: two-value assertion with checked ok.
+func GoodTwoValueChecked(v interface{}) {
+	s, ok := v.(string)
+	if ok {
+		fmt.Println(s)
+	}
+}
+
+// Good: two-value assertion with named ok variable.
+func GoodTwoValueNamed(v interface{}) {
+	s, err := v.(string)
+	fmt.Println(s, err)
+}
+
+// Good: type switch is safe.
+func GoodTypeSwitch(v interface{}) {
+	switch t := v.(type) {
+	case string:
+		fmt.Println(t)
+	}
+}
+
+// Good: parenthesized two-value assignment with checked ok.
+func GoodParenTwoValueChecked(v interface{}) {
+	s, ok := (v.(string))
+	if ok {
+		fmt.Println(s)
+	}
+}
+
+// Good: parenthesized single-value assertion.
+func GoodParenSingleValue(v interface{}) string {
+	return (v.(string))
+}
+
+func suppressed(v interface{}) {
+	//nolint:typeassertionokdiscarded
+	s, _ := v.(string)
+	fmt.Println(s)
+}

pkg/linters/typeassertionokdiscarded/typeassertionokdiscarded.go

@@ -0,0 +1,88 @@
+// Package typeassertionokdiscarded implements a Go analysis linter that flags
+// type assertions in the two-value form where the ok return is explicitly
+// discarded via blank identifier, which can silently accept a zero value.
+package typeassertionokdiscarded
+
+import (
+	"go/ast"
+
+	"golang.org/x/tools/go/analysis"
+
+	"github.com/github/gh-aw/pkg/linters/internal/analyzerutil"
+	"github.com/github/gh-aw/pkg/linters/internal/astutil"
+	"github.com/github/gh-aw/pkg/linters/internal/filecheck"
+	"github.com/github/gh-aw/pkg/linters/internal/nolint"
+)
+
+// Analyzer is the type-assertion-ok-discarded analysis pass.
+var Analyzer = analyzerutil.New("typeassertionokdiscarded", "reports type assertions using the two-value form where the ok return is explicitly discarded via blank identifier, which can silently accept a zero value", run)
+
+func run(pass *analysis.Pass) (any, error) {
+	noLintIndex, generatedFiles, err := analyzerutil.Indexes(pass)
+	if err != nil {
+		return nil, err
+	}
+
+	// Build a parent map for each file so we can detect two-value assignments.
+	fileParents := make(map[*ast.File]map[ast.Node]ast.Node)
+	for _, f := range pass.Files {
+		fileParents[f] = astutil.BuildParentMap(f)
+	}
+
+	nodeFilter := []ast.Node{
+		(*ast.TypeAssertExpr)(nil),
+	}
+
+	return analyzerutil.Preorder(pass, nodeFilter, func(n ast.Node) {
+		inspectTypeAssertExpr(pass, noLintIndex, generatedFiles, fileParents, n)
+	})
+}
+
+func inspectTypeAssertExpr(pass *analysis.Pass, noLintIndex nolint.DirectiveIndex, generatedFiles filecheck.GeneratedIndex, fileParents map[*ast.File]map[ast.Node]ast.Node, n ast.Node) {
+	typeAssert, ok := n.(*ast.TypeAssertExpr)
+	if !ok {
+		return
+	}
+
+	// Type-switch guards have nil Type; skip them.
+	if typeAssert.Type == nil {
+		return
+	}
+
+	pos := pass.Fset.PositionFor(typeAssert.Pos(), false)
+	if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) {
+		return
+	}
+
+	// Find the parent map for the file containing this node.
+	f := astutil.FileForPos(pass.Files, typeAssert.Pos())
+	var parents map[ast.Node]ast.Node
+	if f != nil {
+		parents = fileParents[f]
+	}
+
+	// Check if this is a two-value assignment where the ok is blank.
+	if parents != nil {
+		if isTwoValueBlankOkAssertion(typeAssert, parents) {
+			if nolint.HasDirectiveForLinter(pos, noLintIndex, "typeassertionokdiscarded") {
+				return
+			}
+
+			t := pass.TypesInfo.TypeOf(typeAssert.Type)
+			if t == nil {
+				return
+			}
+
+			pass.ReportRangef(
+				typeAssert,
+				"type assertion ok value is explicitly discarded with blank identifier; use single-value form x.(%s) or check the ok value instead",
+				t,
+			)
+		}
+	}
+}
+
+func isTwoValueBlankOkAssertion(typeAssert *ast.TypeAssertExpr, parents map[ast.Node]ast.Node) bool {
+	okIdent, isTwoValue := astutil.TwoValueTypeAssertionOKIdent(typeAssert, parents)
+	return isTwoValue && okIdent.Name == "_"
+}

pkg/linters/typeassertionokdiscarded/typeassertionokdiscarded_test.go

@@ -0,0 +1,17 @@
+//go:build !integration
+
+package typeassertionokdiscarded_test
+
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+
+	"github.com/github/gh-aw/pkg/linters/typeassertionokdiscarded"
+)
+
+func TestAnalyzer(t *testing.T) {
+	t.Parallel()
+	testdata := analysistest.TestData()
+	analysistest.Run(t, testdata, typeassertionokdiscarded.Analyzer, "typeassertionokdiscarded")
+}