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

---------

Co-authored-by: xobotyi <[email protected]>

Author: ruslanSorokin

README.md

@@ -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"
+}
+
+```

analyzer/analyzer.go

@@ -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
 				}
 			}
@@ -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-- {
+		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)
+
+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
 		}
 	}

analyzer/analyzer_test.go

@@ -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")
 }

analyzer/testdata/src/i/i.go

@@ -2,8 +2,6 @@
 package i
 
 import (
-	"errors"
-
 	"e"
 )
 
@@ -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, ""}}
 }

analyzer/testdata/src/j/j.go

@@ -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"
+}