[tools] go/analysis/passes/modernize: errorsastype: handle another pattern
3 views
Skip to first unread message
Madeline Kalil (Gerrit)
Jun 3, 2026, 4:19:02 PMJun 3
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Madeline Kalil has uploaded the change for review![]( "Open in Gerrit")
Commit message
go/analysis/passes/modernize: errorsastype: handle another pattern
Handle the case where the call to errors.As is a condition
in an if statement, and the init block of the if statement
creates the typed error variable.
if myerr := new(os.PathError); !errors.As(err, &myerr) { ... }
=>
if errors.AsType[&os.PathError](err) { ... }
For golang/go#75692
Change-Id: Iea8e46691e3f580bc483575f1771b539a9bc38b3
Change diff
diff --git a/go/analysis/passes/modernize/errorsastype.go b/go/analysis/passes/modernize/errorsastype.go
index f52f202..5c92fb4 100644
--- a/go/analysis/passes/modernize/errorsastype.go
+++ b/go/analysis/passes/modernize/errorsastype.go
@@ -33,7 +33,7 @@
}
// errorsastype offers a fix to replace error.As with the newer
-// errors.AsType[T] following this pattern:
+// errors.AsType[T] following these patterns:
//
// var myerr *MyErr
// if errors.As(err, &myerr) { ... }
@@ -42,6 +42,14 @@
//
// if myerr, ok := errors.AsType[*MyErr](err); ok { ... }
//
+// and:
+//
+// if myerr := new(MyErr); errors.As(err, &myerr) { ... }
+//
+// =>
+//
+// if myerr, ok := errors.AsType[*MyErr](err); ok { ... }
+//
// (In principle several of these can then be chained using if/else,
// but we don't attempt that.)
//
@@ -63,7 +71,6 @@
// stylistic.
//
// TODO(adonovan): support more cases:
-// - if myerr := new(E); errors.As(err, myerr); { ... }
// - if errors.As(err, myerr) && othercond { ... }
func errorsastype(pass *analysis.Pass) (any, error) {
var (
@@ -122,9 +129,24 @@
// the argument in errors.As must lie inside the if statement.
usesV := moreiters.Len(index.Uses(v)) > 1
+ var deleteErrDecl []analysis.TextEdit
+ ifStmt := curIfStmt.Node().(*ast.IfStmt)
+ if ifStmt.Init == curDeclStmt.Node() {
+ // if myerr := new(MyErr); errors.As(err, &myerr) { ... }
+ // ---------------------
+ deleteErrDecl = []analysis.TextEdit{
+ {
+ Pos: ifStmt.Init.Pos(),
+ End: ifStmt.Cond.Pos(),
+ },
+ }
+ } else {
+ // Delete "var myerr *MyErr"
+ deleteErrDecl = refactor.DeleteStmt(pass.Fset.File(call.Fun.Pos()), curDeclStmt)
+ }
+
edits := append(
- // delete "var myerr *MyErr"
- refactor.DeleteStmt(pass.Fset.File(call.Fun.Pos()), curDeclStmt),
+ deleteErrDecl,
// if errors.As (err, &myerr) { ... }
// ------------- -------------- -------- ----
// if myerr, ok := errors.AsType[*MyErr](err ); ok { ... }
@@ -175,15 +197,18 @@
return nil, nil
}
-// canUseErrorsAsType reports whether curCall is a call to errors.As beneath an
-// if statement, preceded by a declaration of the typed error var. The var must
-// not be used outside the if statement.
+// canUseErrorsAsType reports whether curCall is one of the following:
+// 1. a call to errors.As beneath an if statement, preceded by a declaration of
+// the typed error var. The var must not be used outside the if statement.
+// 2. a call to errors.As in the condition block of an if statement, where the
+// init block creates the typed error var.
// If the conditions are met, it returns the error var, the cursor for its
// DeclStmt, and the cursor for the IfStmt that contains the call to errors.As.
// Otherwise it returns a nil error var.
func canUseErrorsAsType(info *types.Info, index *typeindex.Index, curCall inspector.Cursor) (_ *types.Var, curDeclStmt, curIfStmt inspector.Cursor) {
curCond := curCall
- if curCond.ParentEdgeKind() == edge.UnaryExpr_X { // if !errors.As(err, &v)
+ negated := curCond.ParentEdgeKind() == edge.UnaryExpr_X
+ if negated { // if !errors.As(err, &v)
curCond = curCond.Parent()
}
if curCond.ParentEdgeKind() != edge.IfStmt_Cond {
@@ -191,9 +216,6 @@
}
curIfStmt = curCond.Parent()
ifStmt := curIfStmt.Node().(*ast.IfStmt)
- if ifStmt.Init != nil {
- return // if statement already has an init part
- }
unary, ok := curCall.Node().(*ast.CallExpr).Args[1].(*ast.UnaryExpr)
if !ok || unary.Op != token.AND {
return // 2nd arg is not &var
@@ -203,6 +225,9 @@
return // not a simple ident (local var)
}
v := info.Uses[id].(*types.Var)
+ if v.Pkg() != nil && v.Parent() == v.Pkg().Scope() {
+ return // reject package-level variables
+ }
curDef, ok := index.Def(v)
if !ok {
return // var is not local (e.g. dot-imported)
@@ -216,23 +241,56 @@
return // v used before/after if statement
}
}
- if curDef.ParentEdgeKind() != edge.ValueSpec_Names {
- return // v not declared by "var v T"
- }
- var (
- curSpec = curDef.Parent() // ValueSpec
- curDecl = curSpec.Parent() // GenDecl
- spec = curSpec.Node().(*ast.ValueSpec)
- )
- if len(spec.Names) != 1 || len(spec.Values) != 0 ||
- len(curDecl.Node().(*ast.GenDecl).Specs) != 1 {
- return // not a simple "var v T" decl
- }
- // Have:
- // var v *MyErr
- // ...
- // if errors.As(err, &v) { ... }
- // with no uses of v outside the IfStmt.
- return v, curDecl.Parent(), curIfStmt // curDecl.Parent() is a DeclStmt
+ switch curDef.ParentEdgeKind() {
+ case edge.AssignStmt_Lhs:
+ // Want:
+ // if myerr := new(MyErr); errors.As(err, &myerr) { ... }
+ assign := curDef.Parent().Node().(*ast.AssignStmt)
+ if assign.Tok != token.DEFINE || len(assign.Lhs) != 1 || ifStmt.Init != assign {
+ return
+ }
+ // To avoid semantic changes, reject when the condition is negated or when
+ // there is an else case: when the errors.As check fails, "myerr" is the
+ // zero value of the error type in the original block, and nil in the
+ // transformed block.
+ if negated || ifStmt.Else != nil {
+ return
+ }
+ if !isCallToNew(info, assign.Rhs[0]) {
+ return
+ }
+ return v, curDef.Parent(), curIfStmt
+ case edge.ValueSpec_Names:
+ // Want:
+ // var v *MyErr
+ // ...
+ // if errors.As(err, &v) { ... }
+ // with no uses of v outside the IfStmt.
+ if ifStmt.Init != nil {
+ return // has unrelated init statement
+ }
+ var (
+ curSpec = curDef.Parent() // ValueSpec
+ curDecl = curSpec.Parent() // GenDecl
+ spec = curSpec.Node().(*ast.ValueSpec)
+ )
+ if len(spec.Names) != 1 || len(spec.Values) != 0 ||
+ len(curDecl.Node().(*ast.GenDecl).Specs) != 1 {
+ return // not a simple "var v T" decl
+ }
+ return v, curDecl.Parent(), curIfStmt // curDecl.Parent() is a DeclStmt
+ default:
+ return
+ }
+}
+
+// isCallToNew reports whether expr is a call to the builtin function new(T).
+func isCallToNew(info *types.Info, expr ast.Expr) bool {
+ call, ok := ast.Unparen(expr).(*ast.CallExpr)
+ if !ok || len(call.Args) != 1 {
+ return false
+ }
+ id, ok := call.Fun.(*ast.Ident)
+ return ok && info.ObjectOf(id) == builtinNew
}
diff --git a/go/analysis/passes/modernize/testdata/src/errorsastype/errorsastype.go b/go/analysis/passes/modernize/testdata/src/errorsastype/errorsastype.go
index ae8e9d4..759cdd4 100644
--- a/go/analysis/passes/modernize/testdata/src/errorsastype/errorsastype.go
+++ b/go/analysis/passes/modernize/testdata/src/errorsastype/errorsastype.go
@@ -5,6 +5,8 @@
"os"
)
+var pkgErr *os.PathError
+
func _(err error) {
{
var patherr *os.PathError
@@ -40,6 +42,11 @@
}
print(patherr)
}
+ {
+ if errors.As(err, &pkgErr) { // nope: package-level variable
+ print(pkgErr)
+ }
+ }
// Test of 'ok' var shadowing/freshness.
const ok = 1
@@ -85,4 +92,33 @@
print(patherr)
}
}
+ // Error in init of if block case.
+ {
+ if myerr := new(os.PathError); errors.As(err, &myerr) { // want `errors.As can be simplified using AsType\[\*os.PathError\]`
+ print(myerr)
+ }
+ }
+ {
+ if myerr := new(os.PathError); !errors.As(err, &myerr) { // nope: negated condition with init statement
+ print(myerr)
+ }
+ }
+ {
+ if myerr := new(os.PathError); errors.As(err, &myerr) { // nope: has else branch
+ print(myerr)
+ } else {
+ print("not myerr")
+ }
+ }
+ {
+ getErr := func() *os.PathError { return nil }
+ if myerr := getErr(); errors.As(err, &myerr) { // nope: RHS has potential side effects
+ print(myerr)
+ }
+ }
+ {
+ if myerr := (&os.PathError{Path: "foo"}); errors.As(err, &myerr) { // nope: not a call to new
+ print(myerr)
+ }
+ }
}
diff --git a/go/analysis/passes/modernize/testdata/src/errorsastype/errorsastype.go.golden b/go/analysis/passes/modernize/testdata/src/errorsastype/errorsastype.go.golden
index 0e1ca1c..a29a5cf 100644
--- a/go/analysis/passes/modernize/testdata/src/errorsastype/errorsastype.go.golden
+++ b/go/analysis/passes/modernize/testdata/src/errorsastype/errorsastype.go.golden
@@ -5,6 +5,8 @@
"os"
)
+var pkgErr *os.PathError
+
func _(err error) {
{
if patherr, ok := errors.AsType[*os.PathError](err); ok { // want `errors.As can be simplified using AsType\[\*os.PathError\]`
@@ -37,6 +39,11 @@
}
print(patherr)
}
+ {
+ if errors.As(err, &pkgErr) { // nope: package-level variable
+ print(pkgErr)
+ }
+ }
// Test of 'ok' var shadowing/freshness.
const ok = 1
@@ -76,4 +83,33 @@
print(patherr)
}
}
+ // Error in init of if block case.
+ {
+ if myerr, ok := errors.AsType[*os.PathError](err); ok { // want `errors.As can be simplified using AsType\[\*os.PathError\]`
+ print(myerr)
+ }
+ }
+ {
+ if myerr := new(os.PathError); !errors.As(err, &myerr) { // nope: negated condition with init statement
+ print(myerr)
+ }
+ }
+ {
+ if myerr := new(os.PathError); errors.As(err, &myerr) { // nope: has else branch
+ print(myerr)
+ } else {
+ print("not myerr")
+ }
+ }
+ {
+ getErr := func() *os.PathError { return nil }
+ if myerr := getErr(); errors.As(err, &myerr) { // nope: RHS has potential side effects
+ print(myerr)
+ }
+ }
+ {
+ if myerr := (&os.PathError{Path: "foo"}); errors.As(err, &myerr) { // nope: not a call to new
+ print(myerr)
+ }
+ }
}
Change information
Files:
- M go/analysis/passes/modernize/errorsastype.go
- M go/analysis/passes/modernize/testdata/src/errorsastype/errorsastype.go
- M go/analysis/passes/modernize/testdata/src/errorsastype/errorsastype.go.golden
Change size: M
Delta: 3 files changed, 159 insertions(+), 29 deletions(-)
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Madeline Kalil (Gerrit)
Jun 3, 2026, 4:21:44 PMJun 3
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Madeline Kalil uploaded new patchset![]( "Open in Gerrit")
Madeline Kalil uploaded patch set #2 to this change.
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Madeline Kalil (Gerrit)
Jun 3, 2026, 4:24:10 PMJun 3
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Madeline Kalil uploaded new patchset![]( "Open in Gerrit")
Madeline Kalil uploaded patch set #3 to this change.
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Madeline Kalil (Gerrit)
Jun 3, 2026, 4:25:28 PMJun 3
to goph...@pubsubhelper.golang.org, Alan Donovan, golang-co...@googlegroups.com
Attention needed from Alan Donovan
Madeline Kalil voted Commit-Queue+1![]( "Open in Gerrit")
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mateusz Poliwczak (Gerrit)
Jun 4, 2026, 7:46:59 AMJun 4
to Madeline Kalil, goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, Alan Donovan, golang-co...@googlegroups.com
Attention needed from Alan Donovan and Madeline Kalil
Mateusz Poliwczak added 1 comment![]( "Open in Gerrit")
Commit Message
Line 17, Patchset 3 (Latest):
if errors.AsType[&os.PathError](err) { ... }Mateusz Poliwczak . unresolved
```suggestion
if errors.AsType[*os.PathError](err) { ... }
```
Related details
Attention is currently required from:
- Alan Donovan
- Madeline Kalil
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Madeline Kalil (Gerrit)
Jun 4, 2026, 9:51:25 AMJun 4
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Alan Donovan and Madeline Kalil
Madeline Kalil uploaded new patchset![]( "Open in Gerrit")
Madeline Kalil uploaded patch set #4 to this change.
Related details
Attention is currently required from:
- Alan Donovan
- Madeline Kalil
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Madeline Kalil (Gerrit)
Jun 4, 2026, 9:52:48 AMJun 4
to goph...@pubsubhelper.golang.org, Mateusz Poliwczak, golang...@luci-project-accounts.iam.gserviceaccount.com, Alan Donovan, golang-co...@googlegroups.com
Attention needed from Alan Donovan and Mateusz Poliwczak
Madeline Kalil added 1 comment![]( "Open in Gerrit")
Commit Message
```suggestion
if errors.AsType[*os.PathError](err) { ... }
```
Attention is currently required from:
- Alan Donovan
- Mateusz Poliwczak
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Putman (Gerrit)
2:57 PM (8 hours ago) 2:57 PM
to Madeline Kalil, goph...@pubsubhelper.golang.org, Mateusz Poliwczak, golang...@luci-project-accounts.iam.gserviceaccount.com, Alan Donovan, golang-co...@googlegroups.com
Attention needed from Alan Donovan, Madeline Kalil and Mateusz Poliwczak
Alex Putman added 2 comments![]( "Open in Gerrit")
File go/analysis/passes/modernize/errorsastype.go
Line 208, Patchset 4 (Latest):
func canUseErrorsAsType(info *types.Info, index *typeindex.Index, curCall inspector.Cursor) (_ *types.Var, curDeclStmt, curIfStmt inspector.Cursor) {Alex Putman . unresolved
Ok this might be a bit confusing, but right now, you have two locations that conditionally handle the two cases. Here and where I commented "here".
I think that can all be merged into this one function and would easily allow for the third case that hasn't been implemented yet (still in the TODO). What this function would need to return is 3 things:
1. The loc range to add the `myerr := new(MyErr);`
* for the case (1) where the declaration precedes the if statement, this would be an empty range at the start of the if statement.
* for the case (2) that the if statement has the declaration, this will be the range of that entire declaration.
2. The loc range to add the conditional (the `ok`) in the if statement.
* for all three cases (two for now) this would be the range that `errors.As(err, &myerr)` currently exists in. This would mean that `negated` wouldn't need to be known about necessarily in the above method, as it just replaces that part of the condition after the `!`.
3. A list of additional deletes required.
* this would only be used for case 1 for now, deleting the preceding declaration.
4. And whatever ast node info the method above would still need.
I could be wrong, but I think this would more easily allow for less individual `TextEdit`s above.
Then for condition three, this function would just need to be able to find subconditions that match `errors.As(err, &myerr)` and none of the code above would need to change.
Related details
Attention is currently required from:
- Alan Donovan
- Madeline Kalil
- Mateusz Poliwczak
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages