Skip to content

uncheckedtypeassertion: recognize safe comma-ok form in var init and parenthesized assertions #39774

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 6 commits into from
Jun 17, 2026
+68 −6
Merged

uncheckedtypeassertion: recognize safe comma-ok form in var init and parenthesized assertions #39774

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d307820
Initial plan
Copilot Jun 17, 2026
08be1f2
Plan uncheckedtypeassertion safe-form fix
Copilot Jun 17, 2026
fd0d486
Fix uncheckedtypeassertion safe-form detection
Copilot Jun 17, 2026
8148795
Drop unrelated workflow lockfile changes from PR
Copilot Jun 17, 2026
d46167e
Revert unintended workflow lockfile edits
Copilot Jun 17, 2026
5709918
Add missing uncheckedtypeassertion paren fixture coverage
Copilot Jun 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions ...ters/uncheckedtypeassertion/testdata/src/uncheckedtypeassertion/uncheckedtypeassertion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,54 @@ func GoodTwoValueBlankOk(v interface{}) string {
return s
}

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

@github-actions github-actions Bot Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
	}
}


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

@github-actions github-actions Bot Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/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 github-actions Bot Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
	}
}

if ok {
fmt.Println(s)
}
}

@github-actions github-actions Bot Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/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.


// 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)
}
}

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

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

// Bad: single-value var declaration may panic.
func BadSingleValueVarDecl(v interface{}) {
var s = v.(string) // want `type assertion x\.\(string\) is unchecked and may panic`
fmt.Println(s)
}

func SuppressedPreviousLine(v interface{}) string {
//nolint:uncheckedtypeassertion
return v.(string)
Expand Down
26 changes: 20 additions & 6 deletions pkg/linters/uncheckedtypeassertion/uncheckedtypeassertion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,8 @@ func inspectTypeAssertExpr(pass *analysis.Pass, noLintLinesByFile map[string]map

// 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
}
if isSafeTwoValueAssertion(typeAssert, parents) {
return
}
}

Expand All @@ -96,8 +94,24 @@ func inspectTypeAssertExpr(pass *analysis.Pass, noLintLinesByFile map[string]map
)
}

func isSafeTwoValueAssertion(assign *ast.AssignStmt) bool {
return len(assign.Lhs) == 2 && len(assign.Rhs) == 1
func isSafeTwoValueAssertion(typeAssert *ast.TypeAssertExpr, parents map[ast.Node]ast.Node) bool {
parent := parents[typeAssert]
for parent != nil {
paren, ok := parent.(*ast.ParenExpr)
if !ok {
break
}
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
default:
return false
}
}

// buildParentMap constructs a map from each AST node to its direct parent node.
Expand Down
Loading