Skip to content
This repository was archived by the owner on Nov 10, 2025. It is now read-only.

Commit 2b5235a

Browse files
ccoVeillepolyfloyd
authored andcommitted
fix: type assertion can reuse existing error variable
This is an edge case that can lead to invalid code. This commit is basic, it prevents the linter to provide a fix that cannot work.
1 parent 9b25878 commit 2b5235a

File tree

3 files changed

+51
-15
lines changed

3 files changed

+51
-15
lines changed

errorlint/lint.go

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -648,15 +648,28 @@ func LintErrorTypeAssertions(fset *token.FileSet, info *TypesInfoExt) []analysis
648648
// Get the error variable being type-switched on
649649
errExpr := typeAssert.X
650650

651+
// a flag to know if we can fix the issue with a [analysis.SuggestedFix]
652+
canFix := true
653+
651654
// Determine if this is a type switch with assignment (switch e := err.(type))
652655
var assignIdent *ast.Ident
653656
var useShadowVar bool
654657
if assignStmt, ok := typeSwitch.Assign.(*ast.AssignStmt); ok {
655658
// This is a type switch with assignment like: switch e := err.(type)
656659
if len(assignStmt.Lhs) == 1 {
657660
if id, ok := assignStmt.Lhs[0].(*ast.Ident); ok {
658-
assignIdent = id
659-
useShadowVar = true
661+
if exprToString(errExpr) == id.Name {
662+
// the switch with assignment is like switch err := err.(type)
663+
// we cannot reuse err, otherwise it would lead to errors(err, &err)
664+
canFix = false
665+
666+
// TODO - suggest a fix with a new variable name instead?
667+
// the issue is with the fact each branch should have a new variable name
668+
} else {
669+
// the variable names are different, we can reuse the assigned variable
670+
assignIdent = id
671+
useShadowVar = true
672+
}
660673
}
661674
}
662675
}
@@ -814,20 +827,21 @@ func LintErrorTypeAssertions(fset *token.FileSet, info *TypesInfoExt) []analysis
814827
newSwitchStmt.Body.List[i] = newCaseClause
815828
}
816829

817-
// Print the resulting block to get the fix text.
818-
var buf bytes.Buffer
819-
printer.Fprint(&buf, token.NewFileSet(), blockStmt)
820-
fixText := buf.String()
821-
822-
diagnostic.SuggestedFixes = []analysis.SuggestedFix{{
823-
Message: "Convert type switch to use errors.As",
824-
TextEdits: []analysis.TextEdit{{
825-
Pos: typeSwitch.Pos(),
826-
End: typeSwitch.End(),
827-
NewText: []byte(fixText),
828-
}},
829-
}}
830+
if canFix {
831+
// Print the resulting block to get the fix text.
832+
var buf bytes.Buffer
833+
printer.Fprint(&buf, token.NewFileSet(), blockStmt)
834+
fixText := buf.String()
830835

836+
diagnostic.SuggestedFixes = []analysis.SuggestedFix{{
837+
Message: "Convert type switch to use errors.As",
838+
TextEdits: []analysis.TextEdit{{
839+
Pos: typeSwitch.Pos(),
840+
End: typeSwitch.End(),
841+
NewText: []byte(fixText),
842+
}},
843+
}}
844+
}
831845
lints = append(lints, diagnostic)
832846
}
833847

errorlint/testdata/src/errorassert/errorassert.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,17 @@ func TypeSwitchWithAssignment() {
155155
}
156156
}
157157

158+
// This should be flagged - type switch with assignment reuse
159+
func TypeSwitchWithAssignmentReuse() {
160+
err := doSomething()
161+
switch err := err.(type) { // want "type switch on error will fail on wrapped errors. Use errors.As to check for specific errors"
162+
case *MyError:
163+
fmt.Println("my error:", err)
164+
default:
165+
fmt.Println("unknown error:", err)
166+
}
167+
}
168+
158169
// This should NOT be flagged - using errors.As
159170
func UsingErrorsAs() {
160171
err := doSomethingWrapped()

errorlint/testdata/src/errorassert/errorassert.go.golden

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,17 @@ func TypeSwitchWithAssignment() {
176176
}
177177
}
178178

179+
// This should be flagged - type switch with assignment reuse
180+
func TypeSwitchWithAssignmentReuse() {
181+
err := doSomething()
182+
switch err := err.(type) { // want "type switch on error will fail on wrapped errors. Use errors.As to check for specific errors"
183+
case *MyError:
184+
fmt.Println("my error:", err)
185+
default:
186+
fmt.Println("unknown error:", err)
187+
}
188+
}
189+
179190
// This should NOT be flagged - using errors.As
180191
func UsingErrorsAs() {
181192
err := doSomethingWrapped()

0 commit comments

Comments
 (0)