feat: Improve errorlint with better variable naming and error handling

- Add function for meaningful error variable names
- Preserve original "ok" variable names in type assertions
- Use errors.As() with generated variable names
- Add tests for custom and wrapped errors

Signed-off-by: Kemal Akkoyun <[email protected]>

Author: kakkoyun

errorlint/lint.go

@@ -489,7 +489,9 @@ func LintErrorTypeAssertions(fset *token.FileSet, info *TypesInfoExt) []analysis
 		// For assignment statements like: targetErr, ok := err.(*SomeError)
 		if assign, ok := parent.(*ast.AssignStmt); ok && len(assign.Lhs) == 2 {
 			if id, ok := assign.Lhs[0].(*ast.Ident); ok {
-				varName := id.Name
+				// Generate a suitable variable name, handling underscore case
+				// Example: _, ok := err.(*MyError) -> myError := &MyError{}; ok := errors.As(err, &myError)
+				varName := generateErrorVarName(id.Name, baseType)
 
 				// If this is part of an if statement initialization
 				ifParent, isIfInit := info.NodeParent[assign].(*ast.IfStmt)
@@ -532,9 +534,20 @@ func LintErrorTypeAssertions(fset *token.FileSet, info *TypesInfoExt) []analysis
 				} else {
 					varDecl = fmt.Sprintf("var %s %s", varName, baseType)
 				}
+
+				// Preserve the original name of the "ok" variable
+				// Example: myErr, wasFound := err.(*MyError)
+				// Should use "wasFound" in the transformed code, not just "ok"
+				okName := "ok" // Default
+				if len(assign.Lhs) > 1 {
+					if okIdent, okOk := assign.Lhs[1].(*ast.Ident); okOk && okIdent.Name != "_" {
+						okName = okIdent.Name
+					}
+				}
+
 				// Align with golden file format
-				replacement := fmt.Sprintf("%s\nok := errors.As(%s, &%s)",
-					varDecl, errExpr, varName)
+				replacement := fmt.Sprintf("%s\n%s := errors.As(%s, &%s)",
+					varDecl, okName, errExpr, varName)
 
 				diagnostic.SuggestedFixes = []analysis.SuggestedFix{{
 					Message: "Use errors.As() for type assertions on errors",
@@ -551,15 +564,15 @@ func LintErrorTypeAssertions(fset *token.FileSet, info *TypesInfoExt) []analysis
 
 		if _, ok := parent.(*ast.IfStmt); ok {
 			// For if statements without initialization but with direct type assertion in condition
-			// This case is less common but could happen
+			varName := generateErrorVarName("target", baseType)
 			var varDecl string
 			if isPointerType {
-				varDecl = fmt.Sprintf("target := &%s{}", baseType)
+				varDecl = fmt.Sprintf("%s := &%s{}", varName, baseType)
 			} else {
-				varDecl = fmt.Sprintf("var target %s", baseType)
+				varDecl = fmt.Sprintf("var %s %s", varName, baseType)
 			}
-			replacement := fmt.Sprintf("%s\nif errors.As(%s, &target)",
-				varDecl, errExpr)
+			replacement := fmt.Sprintf("%s\nif errors.As(%s, &%s)",
+				varDecl, errExpr, varName)
 
 			diagnostic.SuggestedFixes = []analysis.SuggestedFix{{
 				Message: "Use errors.As() for type assertions on errors",
@@ -573,17 +586,19 @@ func LintErrorTypeAssertions(fset *token.FileSet, info *TypesInfoExt) []analysis
 			continue
 		}
 
-		// For standalone type assertions: err.(*SomeError) or err.(SomeError).
-		// Create an anonymous function that handles the errors.As call.
+		// Handle standalone type assertions without assignment
+		// Example: _ = err.(*MyError)
+		// Transforms to: _ = func() *MyError { var target *MyError; _ = errors.As(err, &target); return target }()
+		varName := generateErrorVarName("target", baseType)
 		var targetDecl string
 		if isPointerType {
-			targetDecl = fmt.Sprintf("target := &%s{}", baseType)
+			targetDecl = fmt.Sprintf("%s := &%s{}", varName, baseType)
 		} else {
-			targetDecl = fmt.Sprintf("var target %s", baseType)
+			targetDecl = fmt.Sprintf("var %s %s", varName, baseType)
 		}
 
-		replacement := fmt.Sprintf("func() %s {\n\t%s\n\t_ = errors.As(%s, &target)\n\treturn target\n}()",
-			targetType, targetDecl, errExpr)
+		replacement := fmt.Sprintf("func() %s {\n\t%s\n\t_ = errors.As(%s, &%s)\n\treturn %s\n}()",
+			targetType, targetDecl, errExpr, varName, varName)
 
 		diagnostic.SuggestedFixes = []analysis.SuggestedFix{{
 			Message: "Use errors.As() for type assertions on errors",
@@ -830,3 +845,40 @@ func implementsError(t types.Type) bool {
 
 	return false
 }
+
+// generateErrorVarName creates an appropriate variable name for error type assertions.
+// If originalName is "_" or a generic placeholder, it generates a more meaningful name
+// based on the error type, following Go naming conventions:
+//
+// Examples:
+//   - originalName="_", typeName="MyError" → "myError" (camelCase conversion)
+//   - originalName="_", typeName="pkg.CustomError" → "customError" (package prefix removed)
+//   - originalName="existingName" → "existingName" (original name preserved)
+//   - originalName="_", typeName="" → "myErr" (fallback for unknown types)
+//
+// This helps ensure code readability when converting type assertions to errors.As calls,
+// particularly when dealing with underscore identifiers that can't be referenced.
+func generateErrorVarName(originalName, typeName string) string {
+	// If the original name is not an underscore, use it
+	if originalName != "_" {
+		return originalName
+	}
+
+	// Handle underscore case by generating a name based on the type
+	// Strip any package prefix like "pkg."
+	if lastDot := strings.LastIndex(typeName, "."); lastDot >= 0 {
+		typeName = typeName[lastDot+1:]
+	}
+
+	// Convert first letter to lowercase for camelCase
+	if len(typeName) > 0 {
+		firstChar := strings.ToLower(typeName[:1])
+		if len(typeName) > 1 {
+			return firstChar + typeName[1:]
+		}
+		return firstChar
+	}
+
+	// If we couldn't determine a good name, use default.
+	return "anErr"
+}

errorlint/testdata/src/errorassert/errorassert.go

@@ -24,6 +24,23 @@ func (ValueError) Error() string {
 	return "value error"
 }
 
+// CustomUnwrapError demonstrates custom unwrap implementation
+type CustomUnwrapError struct {
+	inner error
+}
+
+func (e *CustomUnwrapError) Error() string { return "custom: " + e.inner.Error() }
+func (e *CustomUnwrapError) Unwrap() error { return e.inner }
+
+// CustomIsError demonstrates custom Is implementation
+type CustomIsError struct{}
+
+func (*CustomIsError) Error() string { return "custom is error" }
+func (*CustomIsError) Is(target error) bool {
+	_, ok := target.(*MyError)
+	return ok
+}
+
 func doSomething() error {
 	return &MyError{}
 }
@@ -40,6 +57,18 @@ func doSomethingValueWrapped() error {
 	return fmt.Errorf("wrapped value: %w", ValueError{})
 }
 
+func getCustomUnwrapError() error {
+	return &CustomUnwrapError{inner: &MyError{}}
+}
+
+func getCustomIsError() error {
+	return &CustomIsError{}
+}
+
+func deepWrappedError() error {
+	return fmt.Errorf("level1: %w", fmt.Errorf("level2: %w", &MyError{}))
+}
+
 // This should be flagged - direct type assertion
 func TypeAssertionDirect() {
 	err := doSomething()
@@ -149,3 +178,56 @@ func NonErrorTypeAssertion() {
 		fmt.Println(s)
 	}
 }
+
+// This should be flagged - type assertion on an error with custom unwrap
+func TypeAssertCustomUnwrap() {
+	err := getCustomUnwrapError()
+	me, ok := err.(*MyError) // want "type assertion on error will fail on wrapped errors. Use errors.As to check for specific errors"
+	if ok {
+		fmt.Println("got my error:", me)
+	}
+}
+
+// This should be flagged - type assertion on deeply wrapped error
+func TypeAssertDeepWrapped() {
+	err := deepWrappedError()
+	me, ok := err.(*MyError) // want "type assertion on error will fail on wrapped errors. Use errors.As to check for specific errors"
+	if ok {
+		fmt.Println("got my error:", me)
+	}
+}
+
+// This should be flagged - type assertion on error with custom Is method
+func TypeAssertCustomIs() {
+	err := getCustomIsError()
+	me, ok := err.(*MyError) // want "type assertion on error will fail on wrapped errors. Use errors.As to check for specific errors"
+	if ok {
+		fmt.Println("got my error:", me)
+	}
+}
+
+// This tests the error conversion case
+func ErrorConversion() {
+	var err error = &MyError{}
+	var iface interface{} = err
+
+	// This should NOT be flagged - not asserting on an error type
+	_, ok1 := iface.(*MyError)
+
+	// This should be flagged - asserting on an error type
+	_, ok2 := err.(*MyError) // want "type assertion on error will fail on wrapped errors. Use errors.As to check for specific errors"
+
+	fmt.Println(ok1, ok2)
+}
+
+// This should NOT be flagged - using errors.As with a pointer to a pointer
+func UsingErrorsAsWithPointerToPointer() {
+	var err error = &MyError{}
+
+	me := new(*MyError)
+	if errors.As(err, me) {
+		fmt.Println(me)
+	} else {
+		fmt.Println("-")
+	}
+}

errorlint/testdata/src/errorassert/errorassert.go.golden

@@ -24,6 +24,23 @@ func (ValueError) Error() string {
 	return "value error"
 }
 
+// CustomUnwrapError demonstrates custom unwrap implementation
+type CustomUnwrapError struct {
+	inner error
+}
+
+func (e *CustomUnwrapError) Error() string { return "custom: " + e.inner.Error() }
+func (e *CustomUnwrapError) Unwrap() error { return e.inner }
+
+// CustomIsError demonstrates custom Is implementation
+type CustomIsError struct{}
+
+func (*CustomIsError) Error() string { return "custom is error" }
+func (*CustomIsError) Is(target error) bool {
+	_, ok := target.(*MyError)
+	return ok
+}
+
 func doSomething() error {
 	return &MyError{}
 }
@@ -40,6 +57,18 @@ func doSomethingValueWrapped() error {
 	return fmt.Errorf("wrapped value: %w", ValueError{})
 }
 
+func getCustomUnwrapError() error {
+	return &CustomUnwrapError{inner: &MyError{}}
+}
+
+func getCustomIsError() error {
+	return &CustomIsError{}
+}
+
+func deepWrappedError() error {
+	return fmt.Errorf("level1: %w", fmt.Errorf("level2: %w", &MyError{}))
+}
+
 // This should be flagged - direct type assertion
 func TypeAssertionDirect() {
 	err := doSomething()
@@ -169,4 +198,61 @@ func NonErrorTypeAssertion() {
 	if s, ok := i.(string); ok {
 		fmt.Println(s)
 	}
-}
+}
+
+// This should be flagged - type assertion on an error with custom unwrap
+func TypeAssertCustomUnwrap() {
+	err := getCustomUnwrapError()
+	me := &MyError{}
+	ok := errors.As(err, &me) // want "type assertion on error will fail on wrapped errors. Use errors.As to check for specific errors"
+	if ok {
+		fmt.Println("got my error:", me)
+	}
+}
+
+// This should be flagged - type assertion on deeply wrapped error
+func TypeAssertDeepWrapped() {
+	err := deepWrappedError()
+	me := &MyError{}
+	ok := errors.As(err, &me) // want "type assertion on error will fail on wrapped errors. Use errors.As to check for specific errors"
+	if ok {
+		fmt.Println("got my error:", me)
+	}
+}
+
+// This should be flagged - type assertion on error with custom Is method
+func TypeAssertCustomIs() {
+	err := getCustomIsError()
+	me := &MyError{}
+	ok := errors.As(err, &me) // want "type assertion on error will fail on wrapped errors. Use errors.As to check for specific errors"
+	if ok {
+		fmt.Println("got my error:", me)
+	}
+}
+
+// This tests the error conversion case
+func ErrorConversion() {
+	var err error = &MyError{}
+	var iface interface{} = err
+
+	// This should NOT be flagged - not asserting on an error type
+	_, ok1 := iface.(*MyError)
+
+	// This should be flagged - asserting on an error type
+	myError := &MyError{}
+	ok2 := errors.As(err, &myError) // want "type assertion on error will fail on wrapped errors. Use errors.As to check for specific errors"
+
+	fmt.Println(ok1, ok2)
+}
+
+// This should NOT be flagged - using errors.As with a pointer to a pointer
+func UsingErrorsAsWithPointerToPointer() {
+	var err error = &MyError{}
+
+	me := new(*MyError)
+	if errors.As(err, me) {
+		fmt.Println(me)
+	} else {
+		fmt.Println("-")
+	}
+}

errorlint/testdata/src/errorcompare/errorcompare.go

@@ -20,6 +20,33 @@ func doAnotherThing() error {
 	return io.EOF
 }
 
+type MyError struct{}
+
+func (*MyError) Error() string {
+	return "my custom error"
+}
+
+// CustomUnwrapError demonstrates custom unwrap implementation
+type CustomUnwrapError struct {
+	inner error
+}
+
+func (e *CustomUnwrapError) Error() string { return "custom: " + e.inner.Error() }
+func (e *CustomUnwrapError) Unwrap() error { return e.inner }
+
+// CustomIsError demonstrates custom Is implementation
+type CustomIsError struct{}
+
+func (*CustomIsError) Error() string { return "custom is error" }
+func (*CustomIsError) Is(target error) bool {
+	_, ok := target.(*MyError)
+	return ok
+}
+
+func getCustomIsError() error {
+	return &CustomIsError{}
+}
+
 // This should be flagged - direct comparison with ==
 func CompareWithEquals() {
 	err := doSomething()
@@ -131,3 +158,11 @@ func SwitchOnNonErrorValue() {
 		fmt.Println("Unknown status")
 	}
 }
+
+// This should NOT be flagged - using errors.Is with custom Is method
+func UsingErrorsIsWithCustomIs() {
+	err := getCustomIsError()
+	if errors.Is(err, &MyError{}) {
+		fmt.Println("is my error")
+	}
+}