Skip to content

Commit

Permalink
feat(analyzer): recognize custom error values in return (#102)
Browse files Browse the repository at this point in the history
---------

Co-authored-by: xobotyi <[email protected]>
  • Loading branch information
ruslanSorokin and xobotyi authored May 23, 2024
1 parent 7ae7de4 commit 5f38877
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 25 deletions.
37 changes: 37 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,3 +99,40 @@ var b Shape = a.Shape{
Length: 5,
}
```

### Errors handling

In order to avoid unnecessary noise, when dealing with non-pointer types returned along with errors - `exhaustruct` will
ignore non-error types, checking only structures satisfying `error` interface.

```go
package main

import "errors"

type Shape struct {
Length int
Width int
}

func NewShape() (Shape, error) {
return Shape{}, errors.New("error") // will not raise an error
}

type MyError struct {
Err error
}

func (e MyError) Error() string {
return e.Err.Error()
}

func NewSquare() (Shape, error) {
return Shape{}, MyError{Err: errors.New("error")} // will not raise an error
}

func NewCircle() (Shape, error) {
return Shape{}, MyError{} // will raise "main.MyError is missing field Err"
}

```
45 changes: 36 additions & 9 deletions analyzer/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,9 @@ func (a *analyzer) newVisitor(pass *analysis.Pass) func(n ast.Node, push bool, s

if len(lit.Elts) == 0 {
if ret, ok := stackParentIsReturn(stack); ok {
if returnContainsNonNilError(pass, ret) {
if returnContainsNonNilError(pass, ret, n) {
// it is okay to return uninitialized structure in case struct's direct parent is
// a return statement containing non-nil error
//
// we're unable to check if returned error is custom, but at least we're able to
// cover str [error] type.
return true
}
}
Expand Down Expand Up @@ -184,17 +181,47 @@ func getStructType(pass *analysis.Pass, lit *ast.CompositeLit) (*types.Struct, *

func stackParentIsReturn(stack []ast.Node) (*ast.ReturnStmt, bool) {
// it is safe to skip boundary check, since stack always has at least one element
// - whole file.
ret, ok := stack[len(stack)-2].(*ast.ReturnStmt)
// we also have no reason to check the first element, since it is always a file
for i := len(stack) - 2; i > 0; i-- {

Check failure on line 185 in analyzer/analyzer.go

View workflow job for this annotation

GitHub Actions / Lint (1.21, ubuntu-latest)

Magic number: 2, in <operation> detected (gomnd)
switch st := stack[i].(type) {
case *ast.ReturnStmt:
return st, true

return ret, ok
case *ast.UnaryExpr:
// in case we're dealing with pointers - it is still viable to check pointer's
// parent for return statement
continue

default:
return nil, false
}
}

return nil, false
}

func returnContainsNonNilError(pass *analysis.Pass, ret *ast.ReturnStmt) bool {
// errorIface is a type that represents [error] interface and all types will be
// compared against.
var errorIface = types.Universe.Lookup("error").Type().Underlying().(*types.Interface)

Check failure on line 205 in analyzer/analyzer.go

View workflow job for this annotation

GitHub Actions / Lint (1.21, ubuntu-latest)

type assertion must be checked (forcetypeassert)

func returnContainsNonNilError(pass *analysis.Pass, ret *ast.ReturnStmt, except ast.Node) bool {
// errors are mostly located at the end of return statement, so we're starting
// from the end.
for i := len(ret.Results) - 1; i >= 0; i-- {
if pass.TypesInfo.TypeOf(ret.Results[i]).String() == "error" {
ri := ret.Results[i]

// skip current node
if ri == except {
continue
}

if un, ok := ri.(*ast.UnaryExpr); ok {
if un.X == except {
continue
}
}

if types.Implements(pass.TypesInfo.TypeOf(ri), errorIface) {
return true
}
}
Expand Down
4 changes: 2 additions & 2 deletions analyzer/analyzer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ func TestAnalyzer(t *testing.T) {
assert.Error(t, err)

a, err = analyzer.NewAnalyzer(
[]string{`.*[Tt]est.*`, `.*External`, `.*Embedded`, `.*\.<anonymous>`},
[]string{`.*[Tt]est.*`, `.*External`, `.*Embedded`, `.*\.<anonymous>`, `j\..*Error`},
[]string{`.*Excluded$`, `e\.<anonymous>`},
)
require.NoError(t, err)

analysistest.Run(t, testdataPath, a, "i", "e")
analysistest.Run(t, testdataPath, a, "i", "e", "j")
}
14 changes: 0 additions & 14 deletions analyzer/testdata/src/i/i.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
package i

import (
"errors"

"e"
)

Expand Down Expand Up @@ -64,18 +62,6 @@ func shouldFailRequiredOmitted() {
}
}

func shouldPassEmptyStructWithNonNilErr() (Test, error) {
return Test{}, errors.New("some error")
}

func shouldFailEmptyStructWithNilErr() (Test, error) {
return Test{}, nil // want "i.Test is missing fields A, B, C, D"
}

func shouldFailEmptyNestedStructWithNonNilErr() ([]Test, error) {
return []Test{{}}, nil // want "i.Test is missing fields A, B, C, D"
}

func shouldPassUnnamed() {
_ = []Test{{"", 0, 0.0, false, ""}}
}
Expand Down
52 changes: 52 additions & 0 deletions analyzer/testdata/src/j/j.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package j

import (
"fmt"
"os"
)

type Test struct {
A string
}

type AError struct{}

func (AError) Error() string { return "error message" }

type BError struct{ msg string }

func (e BError) Error() string { return e.msg }

func shouldPassEmptyStructWithConcreteAError() (Test, *AError) {
return Test{}, &AError{}
}

func shouldFailEmptyStructWithEmptyBError() (Test, error) {
return Test{}, &BError{} // want "j.BError is missing field msg"
}

func shouldFailEmptyStructWithNilConcreteError() (Test, *BError) {
return Test{}, nil // want "j.Test is missing field A"
}

func shouldPassEmptyStructWithFmtError() (Test, error) {
return Test{}, fmt.Errorf("error message")
}

func shouldPassStaticError() (Test, error) {
return Test{}, os.ErrNotExist
}

func shouldPassAnonymousFunctionReturningError() (Test, error) {
return Test{}, func() error { return nil }()
}

func shouldFailAnonymousFunctionReturningEmptyError() (Test, error) {
fn := func() error { return &BError{} } // want "j.BError is missing field msg"

return Test{}, fn()
}

func shouldFailEmptyNestedStructWithNonNilErr() ([]Test, error) {
return []Test{{}}, os.ErrNotExist // want "j.Test is missing field A"
}

0 comments on commit 5f38877

Please sign in to comment.