Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(analyzer): recognize custom error values in return #102

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 @@

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 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"
}
Loading