[linter-miner] feat(linters): add uncheckedtypeassertion linter (run #18) (#34738)

* feat(linters): add uncheckedtypeassertion linter

Reports single-value type assertions x.(T) that may panic at runtime
when the dynamic type does not match. Flags cases where the safe
two-value form v, ok := x.(T) should be used instead.

Evidence: issue #34580 — unchecked type assertion in GraphQL response
parsing caused a panic on unexpected nil/wrong-type values, while
sibling fields used the safe ok idiom.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* docs(adr): add draft ADR-34738 for uncheckedtypeassertion linter

Generated by the Design Decision Gate. Documents the decision to add a
new in-house static analyzer that flags single-value type assertions
x.(T) which may panic at runtime, recommending the safe two-value
form v, ok := x.(T) instead.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(linters): satisfy Go lint in uncheckedtypeassertion analyzer

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

* fix(linters): satisfy Go lint in uncheckedtypeassertion analyzer

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Peli de Halleux <pelikhan@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Author: 5 people

cmd/linters/main.go

@@ -29,6 +29,7 @@ import (
 	"github.com/github/gh-aw/pkg/linters/rawloginlib"
 	"github.com/github/gh-aw/pkg/linters/regexpcompileinfunction"
 	"github.com/github/gh-aw/pkg/linters/ssljson"
+	"github.com/github/gh-aw/pkg/linters/uncheckedtypeassertion"
 )
 
 func main() {
@@ -46,5 +47,6 @@ func main() {
 		rawloginlib.Analyzer,
 		regexpcompileinfunction.Analyzer,
 		ssljson.Analyzer,
+		uncheckedtypeassertion.Analyzer,
 	)
 }

docs/adr/34738-add-uncheckedtypeassertion-linter.md

@@ -0,0 +1,92 @@
+# ADR-34738: Add uncheckedtypeassertion Linter
+
+**Date**: 2026-05-25
+**Status**: Draft
+**Deciders**: Unknown
+
+---
+
+## Part 1 — Narrative (Human-Friendly)
+
+### Context
+
+Issue #34580 reported a runtime panic in GraphQL response parsing caused by an unchecked single-value type assertion `project["id"].(string)` when the API returned a `nil` or unexpected-type value, while sibling fields in the same block used the safe two-value `s, ok := v.(string)` idiom. A pattern scan turned up additional single-value assertions across `pkg/` that follow the same risky shape. Go's single-value type assertion `x.(T)` is a latent panic whenever the asserted dynamic type does not hold, whereas the two-value form `v, ok := x.(T)` returns the zero value and `false` and is safe to recover from. The repository already houses a family of small, focused, in-house analyzers under `pkg/linters/` (e.g., `fileclosenotdeferred`, `manualmutexunlock`, `fprintlnsprintf`, `regexpcompileinfunction`) registered through `cmd/linters/main.go`, so the established convention is to add another analyzer rather than rely on review discipline or an external tool.
+
+### Decision
+
+We will add a new static-analysis linter, `uncheckedtypeassertion`, that flags every `*ast.TypeAssertExpr` whose result is consumed in a single-value position and recommends rewriting the call site as the safe two-value form `v, ok := x.(T)`. The linter lives under `pkg/linters/uncheckedtypeassertion/`, is registered in `cmd/linters/main.go` alongside the existing analyzers, walks each `*ast.TypeAssertExpr` via the shared `inspect.Analyzer`, builds a per-file parent map to decide whether the assertion sits on the RHS of an assignment with two LHS targets, and skips type-switch guards (`v.(type)`, identified by `TypeAssertExpr.Type == nil`) and test files (via `pkg/linters/internal/filecheck.IsTestFile`). The implementation mirrors the structure of ADR-34498 (`fprintlnsprintf`), ADR-34091 (`manualmutexunlock`), and ADR-33834 (`fileclosenotdeferred`) for consistency.
+
+### Alternatives Considered
+
+#### Alternative 1: Fix the known instances and rely on review
+
+Patch the unchecked `project["id"].(string)` call site flagged in issue #34580 and the additional sites surfaced by the pattern scan, then trust reviewers to catch new instances. Rejected because the same shape appeared in multiple independent sites across `pkg/`, indicating that human review has not been sufficient to catch it. A mechanical check on every PR is cheaper than reviewer attention and cannot regress as new code lands.
+
+#### Alternative 2: Use a third-party linter (e.g., `gocritic`, `forcetypeassert`)
+
+`forcetypeassert` (and overlapping checks in `gocritic`, `staticcheck`) detect single-value type assertions. Rejected to stay consistent with the project's convention of small, focused, in-house analyzers under `pkg/linters/`, each as its own package with custom logic. Pulling in an external linter for a single rule introduces a new dependency surface, inconsistent rule configuration, and noise from rules the project has not opted into.
+
+#### Alternative 3: Type-based filtering via `go/types`
+
+Use `pass.TypesInfo` to suppress the diagnostic when the asserted type is itself an `interface{...}` that the operand already satisfies, or when the operand is statically known to be of the asserted type. Rejected because the analyzer is intentionally syntactic and structural: any single-value type assertion is a panic risk in production code regardless of static narrowing, and a uniform rule is easier to enforce than a context-sensitive one. False positives can be silenced at the call site by switching to the two-value form, which is always a safe rewrite.
+
+### Consequences
+
+#### Positive
+- New `x.(T)` single-value type assertions introduced after merge are caught by `make golint-custom` and fail in CI rather than landing on `main`, preventing future recurrences of the issue #34580 panic class.
+- The linter follows the same `pkg/linters/<name>/` layout, `Analyzer` shape, and `testdata` convention as sibling analyzers, so contributors can extend it without learning a new pattern.
+- Creates incentive to clean up the pre-existing single-value assertion sites surfaced by the pattern scan to maintain a clean linter signal.
+
+#### Negative
+- Detection is structural: it flags every single-value type assertion outside test files, including cases where the author has consciously decided that a panic is acceptable (e.g., assertions on values the caller has already type-checked through an unrelated path). False positives must be silenced by rewriting to the two-value form `v, _ := x.(T)`, even when the discard `_` is the only practical handling.
+- The pre-existing single-value assertion sites are not fixed in this PR, so the linter cannot be made a blocking CI gate without follow-up work or suppression.
+- Adds one more analyzer to the registry, marginally increasing `cmd/linters/main.go` compile and run time, plus a per-file parent-map construction pass which is `O(n)` in AST node count per file.
+
+#### Neutral
+- Test files are deliberately excluded via `filecheck.IsTestFile`, matching the convention used by sibling linters. Tests may legitimately use single-value assertions on fixture data where a panic is the desired failure mode.
+- Type-switch guards (`switch v.(type)`) are not flagged because the analyzer skips `TypeAssertExpr` nodes with `Type == nil`. This matches Go's own definition: a type-switch guard is not a runtime-panicking assertion.
+- The diagnostic is positional only — it does not emit a suggested-fix code action. Authors must rewrite the call manually.
+- The parent map is constructed per `*ast.File` rather than reused across analyzers, because the existing `inspect.Analyzer` does not expose parent links. The duplication is local to this analyzer and acceptable for the analyzer's size.
+
+---
+
+## Part 2 — Normative Specification (RFC 2119)
+
+> The key words **MUST**, **MUST NOT**, **REQUIRED**, **SHALL**, **SHALL NOT**, **SHOULD**, **SHOULD NOT**, **RECOMMENDED**, **MAY**, and **OPTIONAL** in this section are to be interpreted as described in [RFC 2119](https://www.rfc-editor.org/rfc/rfc2119).
+
+### Linter Behaviour
+
+1. The analyzer **MUST** be exported as `uncheckedtypeassertion.Analyzer` with `Name` equal to `"uncheckedtypeassertion"`.
+2. The analyzer **MUST** report a diagnostic for every `*ast.TypeAssertExpr` whose `Type` field is non-nil and whose direct parent in the containing file's AST is **not** an `*ast.AssignStmt` with exactly two LHS expressions and exactly one RHS expression.
+3. The analyzer **MUST NOT** report a diagnostic for any `*ast.TypeAssertExpr` whose `Type` field is `nil` (i.e., a type-switch guard `v.(type)`).
+4. The analyzer **MUST NOT** report a diagnostic when the parent `*ast.AssignStmt` has `len(Lhs) == 2 && len(Rhs) == 1`, regardless of whether the second LHS is the blank identifier `_` or a named `ok` variable.
+5. The analyzer **MUST NOT** report a diagnostic when the containing file is a Go test file as determined by `pkg/linters/internal/filecheck.IsTestFile`.
+6. The diagnostic `Pos`/`End` range **MUST** cover the entire `*ast.TypeAssertExpr` node (i.e., `pass.ReportRangef(typeAssert, ...)`).
+7. The diagnostic `Message` **SHOULD** include both the rendered asserted type and a recommendation to use the two-value form, of the shape `"type assertion x.(T) is unchecked and may panic; use the two-value form v, ok := x.(T) instead"`, so downstream tooling can match on a stable substring.
+8. The analyzer **MUST** declare `inspect.Analyzer` in its `Requires` list.
+9. The analyzer **MAY** rely on `pass.TypesInfo.TypeOf(typeAssert.Type)` to render the asserted type in the diagnostic; if the resolved type is `nil` the analyzer **MUST** silently skip emitting the diagnostic for that node rather than panicking or emitting a malformed message.
+
+### Parent Resolution
+
+1. The analyzer **MUST** construct a per-file map from each AST node to its direct parent node before the inspect pass evaluates type-assertion nodes, so that the two-value-form detection in requirement 4 is decidable in `O(1)` per node.
+2. The parent map **MUST** be scoped to the `*ast.File` that contains the inspected node; nodes from a different file in the same package **MUST NOT** be present in the same parent map.
+
+### Registration
+
+1. The analyzer **MUST** be registered in `cmd/linters/main.go` via the `multichecker.Main` argument list alongside the existing custom analyzers.
+2. The package import in `cmd/linters/main.go` **MUST** use the path `github.com/github/gh-aw/pkg/linters/uncheckedtypeassertion`.
+
+### Package Layout
+
+1. The linter source **MUST** live under `pkg/linters/uncheckedtypeassertion/`.
+2. Test fixtures **MUST** live under `pkg/linters/uncheckedtypeassertion/testdata/src/uncheckedtypeassertion/` and **MUST** use `// want` comments compatible with `golang.org/x/tools/go/analysis/analysistest`.
+3. The test fixtures **MUST** include at least one positive case for each of: a single-value assertion used as a `return` expression, and a single-value assertion bound via `:=` to a single LHS.
+4. The test fixtures **MUST** include at least one negative case for each of: the two-value `v, ok :=` form, the two-value `v, _ :=` form with blank `ok`, and a type-switch guard `v.(type)`.
+
+### Conformance
+
+An implementation is considered conformant with this ADR if it satisfies all **MUST** and **MUST NOT** requirements above. Failure to meet any **MUST** or **MUST NOT** requirement constitutes non-conformance.
+
+---
+
+*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/26413798298) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*

pkg/linters/uncheckedtypeassertion/testdata/src/uncheckedtypeassertion/uncheckedtypeassertion.go

@@ -0,0 +1,36 @@
+package uncheckedtypeassertion
+
+import "fmt"
+
+// Good: two-value type assertion is safe.
+func GoodTwoValue(v interface{}) {
+	s, ok := v.(string)
+	if ok {
+		fmt.Println(s)
+	}
+}
+
+// Good: type switch is safe — not flagged.
+func GoodTypeSwitch(v interface{}) {
+	switch t := v.(type) {
+	case string:
+		fmt.Println(t)
+	}
+}
+
+// Bad: single-value assertion may panic.
+func BadSingleValue(v interface{}) string {
+	return v.(string) // want `type assertion x\.\(string\) is unchecked and may panic`
+}
+
+// Bad: single-value assertion stored in variable.
+func BadSingleValueAssign(v interface{}) {
+	s := v.(string) // want `type assertion x\.\(string\) is unchecked and may panic`
+	fmt.Println(s)
+}
+
+// Good: two-value form with blank ok is still two-value.
+func GoodTwoValueBlankOk(v interface{}) string {
+	s, _ := v.(string)
+	return s
+}

pkg/linters/uncheckedtypeassertion/uncheckedtypeassertion.go

@@ -0,0 +1,115 @@
+// Package uncheckedtypeassertion implements a Go analysis linter that flags
+// single-value type assertions x.(T) that may panic at runtime if the dynamic
+// type does not match, and where the two-value safe form x.(T) is not used.
+package uncheckedtypeassertion
+
+import (
+	"fmt"
+	"go/ast"
+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/inspect"
+	"golang.org/x/tools/go/ast/inspector"
+
+	"github.com/github/gh-aw/pkg/linters/internal/filecheck"
+)
+
+// Analyzer is the unchecked-type-assertion analysis pass.
+var Analyzer = &analysis.Analyzer{
+	Name:     "uncheckedtypeassertion",
+	Doc:      "reports single-value type assertions that may panic if the dynamic type does not match",
+	URL:      "https://github.com/github/gh-aw/tree/main/pkg/linters/uncheckedtypeassertion",
+	Requires: []*analysis.Analyzer{inspect.Analyzer},
+	Run:      run,
+}
+
+func run(pass *analysis.Pass) (any, error) {
+	insp, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+	if !ok {
+		return nil, fmt.Errorf("inspect result has unexpected type %T", pass.ResultOf[inspect.Analyzer])
+	}
+
+	// Build a parent map for each file so we can detect the two-value form.
+	fileParents := make(map[*ast.File]map[ast.Node]ast.Node)
+	for _, f := range pass.Files {
+		fileParents[f] = buildParentMap(f)
+	}
+
+	nodeFilter := []ast.Node{
+		(*ast.TypeAssertExpr)(nil),
+	}
+
+	insp.Preorder(nodeFilter, func(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.IsTestFile(pos.Filename) {
+			return
+		}
+
+		// Find the parent map for the file containing this node.
+		var parents map[ast.Node]ast.Node
+		for _, f := range pass.Files {
+			if f.Pos() <= typeAssert.Pos() && typeAssert.Pos() <= f.End() {
+				parents = fileParents[f]
+				break
+			}
+		}
+
+		// Skip the safe two-value form:  v, ok := x.(T)  or  v, ok = x.(T)
+		if parents != nil {
+			if assign, ok := parents[typeAssert].(*ast.AssignStmt); ok {
+				if isSafeTwoValueAssertion(assign) {
+					return
+				}
+			}
+		}
+
+		t := pass.TypesInfo.TypeOf(typeAssert.Type)
+		if t == nil {
+			return
+		}
+
+		pass.ReportRangef(
+			typeAssert,
+			"type assertion x.(%s) is unchecked and may panic; use the two-value form v, ok := x.(%s) instead",
+			t, t,
+		)
+	})
+
+	return nil, nil
+}
+
+func isSafeTwoValueAssertion(assign *ast.AssignStmt) bool {
+	return len(assign.Lhs) == 2 && len(assign.Rhs) == 1
+}
+
+// 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
+}

pkg/linters/uncheckedtypeassertion/uncheckedtypeassertion_test.go

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