uncheckedtypeassertion: recognize safe comma-ok form in var init and parenthesized assertions by Copilot · Pull Request #39774 · github/gh-aw

Copilot · 2026-06-17T12:19:07Z

uncheckedtypeassertion only treated x.(T) as safe when its direct parent was an AssignStmt, which produced false positives for valid comma-ok initializations and parenthesized assertions. This PR broadens safe-form detection to match Go’s assignment/initialization semantics without weakening existing unsafe checks.

Safe-form parent resolution
- Unwraps *ast.ParenExpr ancestors before evaluating assertion context.
- Detects comma-ok safety across both assignment and initialization nodes.
Expanded safe contexts
- Keeps existing safe handling for:
  - v, ok := x.(T)
  - v, ok = x.(T)
- Adds safe handling for:
  - var v, ok = x.(T)
  - v, ok := (x.(T))
Preserved unsafe reporting
- Single-value assertions in initialization/expression contexts remain reportable (e.g., var v = x.(T), _ = x.(T), f(x.(T))).
Analyzer fixture updates
- Adds negative fixtures for var v, ok = ... and parenthesized comma-ok assignment.
- Adds/keeps positive fixture coverage for single-value var initialization.

parent := parents[typeAssert]
for paren, ok := parent.(*ast.ParenExpr); ok; paren, ok = parent.(*ast.ParenExpr) {
	parent = parents[paren]
}

switch p := parent.(type) {
case *ast.AssignStmt:
	return len(p.Lhs) == 2 && len(p.Rhs) == 1
case *ast.ValueSpec:
	return len(p.Names) == 2 && len(p.Values) == 1
}

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

Copilot

Pull request overview

This PR updates the uncheckedtypeassertion Go analyzer to recognize additional safe comma-ok type assertion contexts by unwrapping parenthesized assertions and treating var v, ok = x.(T) as safe, reducing false positives without relaxing unsafe single-value assertion reporting.

Changes:

Expand “safe two-value assertion” detection to unwrap *ast.ParenExpr parents and accept *ast.ValueSpec (var v, ok = ...) in addition to *ast.AssignStmt.
Update analyzer fixtures to cover safe var v, ok = ... and parenthesized comma-ok assignment, and keep coverage for unsafe single-value var v = ....
Includes workflow .lock.yml schema changes for pull_request_number validation across multiple generated workflows.

Show a summary per file

File	Description
pkg/linters/uncheckedtypeassertion/uncheckedtypeassertion.go	Broadens safe-context detection for comma-ok assertions (paren unwrapping + `ValueSpec`).
pkg/linters/uncheckedtypeassertion/testdata/src/uncheckedtypeassertion/uncheckedtypeassertion.go	Adds/adjusts fixtures for new safe and still-unsafe assertion forms.
.github/workflows/test-quality-sentinel.lock.yml	Generated lockfile update affecting `pull_request_number` validation.
.github/workflows/smoke-copilot.lock.yml	Generated lockfile update affecting `pull_request_number` validation.
.github/workflows/smoke-copilot-arm.lock.yml	Generated lockfile update affecting `pull_request_number` validation.
.github/workflows/smoke-copilot-aoai-entra.lock.yml	Generated lockfile update affecting `pull_request_number` validation.
.github/workflows/smoke-copilot-aoai-apikey.lock.yml	Generated lockfile update affecting `pull_request_number` validation.
.github/workflows/smoke-claude.lock.yml	Generated lockfile update affecting `pull_request_number` validation.
.github/workflows/security-review.lock.yml	Generated lockfile update affecting `pull_request_number` validation.
.github/workflows/refiner.lock.yml	Generated lockfile update affecting `pull_request_number` validation.
.github/workflows/pr-triage-agent.lock.yml	Generated lockfile update affecting `pull_request_number` validation.
.github/workflows/pr-nitpick-reviewer.lock.yml	Generated lockfile update affecting `pull_request_number` validation.
.github/workflows/pr-code-quality-reviewer.lock.yml	Generated lockfile update affecting `pull_request_number` validation.
.github/workflows/mattpocock-skills-reviewer.lock.yml	Generated lockfile update affecting `pull_request_number` validation.
.github/workflows/grumpy-reviewer.lock.yml	Generated lockfile update affecting `pull_request_number` validation.

Copilot's findings

Tip

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

Files reviewed: 15/15 changed files
Comments generated: 13

                  },
                  "pull_request_number": {
-                    "optionalPositiveInteger": true
+                    "issueOrPRNumber": true


                  },
                  "pull_request_number": {
-                    "optionalPositiveInteger": true
+                    "issueOrPRNumber": true


                  },
                  "pull_request_number": {
-                    "optionalPositiveInteger": true
+                    "issueOrPRNumber": true


                  },
                  "pull_request_number": {
-                    "optionalPositiveInteger": true
+                    "issueOrPRNumber": true


                  },
                  "pull_request_number": {
-                    "optionalPositiveInteger": true
+                    "issueOrPRNumber": true


                  },
                  "pull_request_number": {
-                    "optionalPositiveInteger": true
+                    "issueOrPRNumber": true


                  },
                  "pull_request_number": {
-                    "optionalPositiveInteger": true
+                    "issueOrPRNumber": true


                  },
                  "pull_request_number": {
-                    "optionalPositiveInteger": true
+                    "issueOrPRNumber": true


                  },
                  "pull_request_number": {
-                    "optionalPositiveInteger": true
+                    "issueOrPRNumber": true


                  },
                  "pull_request_number": {
-                    "optionalPositiveInteger": true
+                    "issueOrPRNumber": true


pelikhan · 2026-06-17T13:37:31Z

@copilot run pr-finisher skill

github-actions · 2026-06-17T13:37:35Z

✅ PR Code Quality Reviewer completed the code quality review.

github-actions · 2026-06-17T13:37:42Z

✅ Test Quality Sentinel completed test quality analysis.

No _test.go files were added or modified in this PR. Test Quality Sentinel skipped. Note: the PR does add three new annotated scenarios to the analysistest testdata file (GoodTwoValueVarDecl, GoodTwoValueParen, BadSingleValueVarDecl), which extend the behavioral coverage of the existing TestAnalyzer test — but testdata files are not scored by this sentinel.

github-actions · 2026-06-17T13:37:49Z

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

github-actions · 2026-06-17T13:38:02Z

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

No ADR enforcement needed for PR #39774: it does not have the 'implementation' label (has_implementation_label=false) and has only 42 new lines of code in business logic directories (≤100 threshold, no custom .design-gate.yml config).

github-actions

Skills-Based Review 🧠

Applied /tdd and /diagnose — approving with two minor test-coverage suggestions.

📋 Key Themes & Highlights

Positive Highlights

✅ Root cause correctly identified: the old parents[typeAssert].(*ast.AssignStmt) lookup missed both *ast.ParenExpr wrappers and *ast.ValueSpec parents.
✅ ParenExpr unwrap loop is clean: iterates until a non-paren ancestor is found; handles the nil-parent edge case by falling through to default: false — correct and safe.
✅ ValueSpec check mirrors the AssignStmt check: len(p.Names) == 2 && len(p.Values) == 1 precisely matches the comma-ok grammar for var declarations.
✅ No weakening of unsafe reporting: single-value var declarations (var s = x.(T)) correctly remain flagged, confirmed by BadSingleValueVarDecl.
✅ Lock-file changes are auto-generated (schema rename optionalPositiveInteger → issueOrPRNumber): no manual review needed.

Minor Test Gaps (non-blocking)

Two combinations of the new code paths aren't exercised by any fixture:

Parenthesized var-declaration: var s, ok = (v.(string)) — exercises the loop and the ValueSpec branch together (see inline comment at line 52).
Double-parens: s, ok := ((v.(string))) — confirms the loop iterates more than once (see inline comment at line 48).

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

github-actions · 2026-06-17T13:48:00Z

+	if ok {
+		fmt.Println(s)
+	}
+}


[/tdd] Missing combined test: var s, ok = (v.(string)) — parenthesized var-declaration is not exercised, even though it threads through both new code paths (ParenExpr unwrapping and the ValueSpec case). If a future refactor accidentally broke the interaction, no test would catch it.

💡 Suggested fixture

// Good: parenthesized two-value var declaration is safe. func GoodTwoValueParenVarDecl(v interface{}) { var s, ok = (v.(string)) if ok { fmt.Println(s) } }

This is the intersection of GoodTwoValueVarDecl and GoodTwoValueParen and is the only path that exercises the ParenExpr-unwrap loop together with the *ast.ValueSpec branch.

github-actions · 2026-06-17T13:48:00Z

+
+// Good: parenthesized two-value assignment is safe.
+func GoodTwoValueParen(v interface{}) {
+	s, ok := (v.(string))


[/tdd] Consider adding a multi-level parens test: s, ok := ((v.(string))). The loop at isSafeTwoValueAssertion is designed to handle any depth of nesting, but no fixture confirms this works for more than one level.

💡 Suggested fixture

// Good: double-parenthesized two-value assignment is safe. func GoodTwoValueDoubleParen(v interface{}) { s, ok := ((v.(string))) if ok { fmt.Println(s) } }

Without this, if the loop were accidentally changed to only strip a single ParenExpr, double-paren cases would silently become false positives again.

github-actions

Non-blocking: logic is correct, two test coverage gaps

The core change is sound: paren-unwrapping and ValueSpec detection are both correctly implemented and preserve all pre-existing unsafe-detection semantics.

Gaps found (non-blocking)

Medium — paren + ValueSpec combo untested (see inline comment at GoodTwoValueVarDecl)

The paren-unwrapping loop is exercised for AssignStmt (via GoodTwoValueParen), but not for ValueSpec. The pattern var s, ok = (v.(string)) should be added as a fixture.

Low — regular re-assignment with parens untested (see inline comment at GoodTwoValueParen)

v, ok = (x.(T)) (using = instead of :=) is structurally identical in the AST but is not exercised by any fixture.

Note: existing comments on the 13 .lock.yml changes already flag those as unrelated; no additional comment added here.

🔎 Code quality review by PR Code Quality Reviewer

github-actions · 2026-06-17T13:51:16Z

+	if ok {
+		fmt.Println(s)
+	}
+}


Paren + ValueSpec combination is not tested, so the paren-unwrapping path for the new ValueSpec branch has zero coverage.

💡 Suggested addition

GoodTwoValueVarDecl tests var s, ok = v.(string) (ValueSpec branch) and GoodTwoValueParen tests s, ok := (v.(string)) (paren→AssignStmt). Neither tests the cross-product: var s, ok = (v.(string)), which goes through the paren-unwrapping loop and then lands on ValueSpec.

If the loop were ever changed to stop early (e.g. breaking on nil before reaching the real parent), this combination would silently produce a false positive and no existing test would catch it.

Add:

// Good: parenthesized two-value var declaration is safe. func GoodTwoValueVarDeclParen(v interface{}) { var s, ok = (v.(string)) if ok { fmt.Println(s) } }

github-actions · 2026-06-17T13:51:17Z

+
+// Good: parenthesized two-value assignment is safe.
+func GoodTwoValueParen(v interface{}) {
+	s, ok := (v.(string))


v, ok = (x.(T)) (paren + regular re-assignment) is not covered. The only paren fixture uses := (short var decl), leaving the equivalent re-assignment path untested.

💡 Suggested addition

Both := and = produce an AssignStmt, so both are handled by the same case *ast.AssignStmt branch. But a regression that accidentally checked assign.Tok == token.DEFINE would only be caught if the = form is also exercised.

Add:

// Good: parenthesized two-value re-assignment is safe. func GoodTwoValueParenAssign(v interface{}) { var s string var ok bool s, ok = (v.(string)) if ok { fmt.Println(s) } }

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

Copilot · 2026-06-17T14:09:00Z

@copilot run pr-finisher skill

Completed a pr-finisher pass.

Addressed the scope issue by removing unrelated workflow lockfile churn and added the missing uncheckedtypeassertion fixture coverage requested by bot review. Latest commit: 5709918b30.

Initial plan

d307820

Copilot AI assigned Copilot and pelikhan Jun 17, 2026

Copilot started work on behalf of pelikhan June 17, 2026 12:27 View session

Copilot AI and others added 2 commits June 17, 2026 12:31

Plan uncheckedtypeassertion safe-form fix

08be1f2

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

Fix uncheckedtypeassertion safe-form detection

fd0d486

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

Copilot AI changed the title ~~[WIP] Fix unchecked type assertion detection for initialization~~ uncheckedtypeassertion: recognize safe comma-ok form in var init and parenthesized assertions Jun 17, 2026

Copilot finished work on behalf of pelikhan June 17, 2026 12:41

Copilot AI requested a review from pelikhan June 17, 2026 12:41

github-actions Bot mentioned this pull request Jun 17, 2026

[team-status] Daily Status — June 17, 2026 #39786

Closed

pelikhan marked this pull request as ready for review June 17, 2026 13:26

Copilot AI review requested due to automatic review settings June 17, 2026 13:26

Copilot started reviewing on behalf of pelikhan June 17, 2026 13:27 View session

Copilot AI reviewed Jun 17, 2026

View reviewed changes

github-actions Bot mentioned this pull request Jun 17, 2026

[aw] No-Op Runs #38951

Closed

Copilot started work on behalf of pelikhan June 17, 2026 13:38 View session

github-actions Bot approved these changes Jun 17, 2026

View reviewed changes

github-actions Bot reviewed Jun 17, 2026

View reviewed changes

Copilot AI and others added 3 commits June 17, 2026 13:53

Drop unrelated workflow lockfile changes from PR

8148795

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

Revert unintended workflow lockfile edits

d46167e

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

Add missing uncheckedtypeassertion paren fixture coverage

5709918

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

Copilot finished work on behalf of pelikhan June 17, 2026 14:09

github-actions Bot mentioned this pull request Jun 17, 2026

[Contribution Check Report] Contribution Check — 2026-06-17 #39716

Closed

pelikhan merged commit 503db73 into main Jun 17, 2026
29 checks passed

pelikhan deleted the copilot/fix-unsafe-type-assertion branch June 17, 2026 14:25

Conversation

Copilot AI commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

pelikhan commented Jun 17, 2026

Uh oh!

github-actions Bot commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Skills-Based Review 🧠

Positive Highlights

Minor Test Gaps (non-blocking)

Uh oh!

github-actions Bot Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Non-blocking: logic is correct, two test coverage gaps

Uh oh!

github-actions Bot Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Jun 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Jun 17, 2026 •

edited

Loading

github-actions Bot commented Jun 17, 2026 •

edited

Loading

github-actions Bot commented Jun 17, 2026 •

edited

Loading

github-actions Bot commented Jun 17, 2026 •

edited

Loading

github-actions Bot commented Jun 17, 2026 •

edited

Loading