diff --git a/errorlint/lint.go b/errorlint/lint.go index 140182c..b7d3cbd 100644 --- a/errorlint/lint.go +++ b/errorlint/lint.go @@ -648,6 +648,9 @@ func LintErrorTypeAssertions(fset *token.FileSet, info *TypesInfoExt) []analysis // Get the error variable being type-switched on errExpr := typeAssert.X + // a flag to know if we can fix the issue with a [analysis.SuggestedFix] + canFix := true + // Determine if this is a type switch with assignment (switch e := err.(type)) var assignIdent *ast.Ident var useShadowVar bool @@ -655,8 +658,18 @@ func LintErrorTypeAssertions(fset *token.FileSet, info *TypesInfoExt) []analysis // This is a type switch with assignment like: switch e := err.(type) if len(assignStmt.Lhs) == 1 { if id, ok := assignStmt.Lhs[0].(*ast.Ident); ok { - assignIdent = id - useShadowVar = true + if exprToString(errExpr) == id.Name { + // the switch with assignment is like switch err := err.(type) + // we cannot reuse err, otherwise it would lead to errors(err, &err) + canFix = false + + // TODO - suggest a fix with a new variable name instead? + // the issue is with the fact each branch should have a new variable name + } else { + // the variable names are different, we can reuse the assigned variable + assignIdent = id + useShadowVar = true + } } } } @@ -814,20 +827,21 @@ func LintErrorTypeAssertions(fset *token.FileSet, info *TypesInfoExt) []analysis newSwitchStmt.Body.List[i] = newCaseClause } - // Print the resulting block to get the fix text. - var buf bytes.Buffer - printer.Fprint(&buf, token.NewFileSet(), blockStmt) - fixText := buf.String() - - diagnostic.SuggestedFixes = []analysis.SuggestedFix{{ - Message: "Convert type switch to use errors.As", - TextEdits: []analysis.TextEdit{{ - Pos: typeSwitch.Pos(), - End: typeSwitch.End(), - NewText: []byte(fixText), - }}, - }} + if canFix { + // Print the resulting block to get the fix text. + var buf bytes.Buffer + printer.Fprint(&buf, token.NewFileSet(), blockStmt) + fixText := buf.String() + diagnostic.SuggestedFixes = []analysis.SuggestedFix{{ + Message: "Convert type switch to use errors.As", + TextEdits: []analysis.TextEdit{{ + Pos: typeSwitch.Pos(), + End: typeSwitch.End(), + NewText: []byte(fixText), + }}, + }} + } lints = append(lints, diagnostic) } diff --git a/errorlint/testdata/src/errorassert/errorassert.go b/errorlint/testdata/src/errorassert/errorassert.go index 3faf2ed..4881ca8 100644 --- a/errorlint/testdata/src/errorassert/errorassert.go +++ b/errorlint/testdata/src/errorassert/errorassert.go @@ -155,6 +155,17 @@ func TypeSwitchWithAssignment() { } } +// This should be flagged - type switch with assignment reuse +func TypeSwitchWithAssignmentReuse() { + err := doSomething() + switch err := err.(type) { // want "type switch on error will fail on wrapped errors. Use errors.As to check for specific errors" + case *MyError: + fmt.Println("my error:", err) + default: + fmt.Println("unknown error:", err) + } +} + // This should NOT be flagged - using errors.As func UsingErrorsAs() { err := doSomethingWrapped() diff --git a/errorlint/testdata/src/errorassert/errorassert.go.golden b/errorlint/testdata/src/errorassert/errorassert.go.golden index bb0bea6..53e06cc 100644 --- a/errorlint/testdata/src/errorassert/errorassert.go.golden +++ b/errorlint/testdata/src/errorassert/errorassert.go.golden @@ -176,6 +176,17 @@ func TypeSwitchWithAssignment() { } } +// This should be flagged - type switch with assignment reuse +func TypeSwitchWithAssignmentReuse() { + err := doSomething() + switch err := err.(type) { // want "type switch on error will fail on wrapped errors. Use errors.As to check for specific errors" + case *MyError: + fmt.Println("my error:", err) + default: + fmt.Println("unknown error:", err) + } +} + // This should NOT be flagged - using errors.As func UsingErrorsAs() { err := doSomethingWrapped()