Skip to content

Commit

Permalink
feat: detect and fix err.String() calls
Browse files Browse the repository at this point in the history
```diff
-fmt.Errorf("failed for %s with error: %s", "foo", err.Error())
+fmt.Errorf("failed for %s with error: %w", "foo", err)
```
  • Loading branch information
dnwe committed Nov 5, 2020
1 parent 2ac108a commit d4e9251
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 1 deletion.
30 changes: 29 additions & 1 deletion errwrap/errwrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,21 @@ func run(pass *analysis.Pass) (interface{}, error) {
var errIndex int
for i, arg := range call.Args {
if t := pass.TypesInfo.TypeOf(arg); t != nil {
if t.String() == "error" {
switch t.String() {
case "error":
hasError = true
errIndex = i
case "string":
if expr, ok := arg.(*ast.CallExpr); ok {
if sel, ok := expr.Fun.(*ast.SelectorExpr); ok {
if id, ok := sel.X.(*ast.Ident); ok {
if pass.TypesInfo.TypeOf(id).String() == "error" {
hasError = true
errIndex = i
}
}
}
}
}
}
}
Expand Down Expand Up @@ -137,6 +149,22 @@ func run(pass *analysis.Pass) (interface{}, error) {
}
}

if pass.TypesInfo.TypeOf(call.Args[errIndex]).String() == "string" {
arg := call.Args[errIndex]
if expr, ok := arg.(*ast.CallExpr); ok {
if sel, ok := expr.Fun.(*ast.SelectorExpr); ok {
if id, ok := sel.X.(*ast.Ident); ok {
// remove the .String call from the error
call.Args[errIndex] = &ast.BasicLit{
Value: id.String(),
ValuePos: sel.X.Pos(),
Kind: token.STRING,
}
}
}
}
}

newExpr := render(pass.Fset, call)

pass.Report(analysis.Diagnostic{
Expand Down
4 changes: 4 additions & 0 deletions errwrap/errwrap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ func Test(t *testing.T) {
name: "too many formatting directives",
dir: "d",
},
{
name: "wrap the error.String() with error-wrapping directive",
dir: "e",
},
} {
t.Run(tcase.name, func(t *testing.T) {
analysistest.Run(t, testdata, errwrap.Analyzer, tcase.dir)
Expand Down
11 changes: 11 additions & 0 deletions errwrap/testdata/src/e/e.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package e

import (
"errors"
"fmt"
)

func foo() error {
err := errors.New("bar!")
return fmt.Errorf("failed for %s with error: %s", "foo", err.Error()) // want `call could wrap the error with error-wrapping directive %w`
}

0 comments on commit d4e9251

Please sign in to comment.