diff --git a/errorlint/analysis.go b/errorlint/analysis.go index a26067b..c0760fb 100644 --- a/errorlint/analysis.go +++ b/errorlint/analysis.go @@ -40,7 +40,7 @@ func run(pass *analysis.Pass) (interface{}, error) { var lints []analysis.Diagnostic extInfo := newTypesInfoExt(pass) if checkComparison { - l := LintErrorComparisons(extInfo) + l := LintErrorComparisons(pass.Fset, extInfo) lints = append(lints, l...) } if checkAsserts { @@ -51,6 +51,10 @@ func run(pass *analysis.Pass) (interface{}, error) { l := LintFmtErrorfCalls(pass.Fset, *pass.TypesInfo, checkErrorfMulti) lints = append(lints, l...) } + + // Resolve conflicts between type assertion and error comparison diagnostics + lints = resolveConflicts(lints, extInfo) + sort.Sort(ByPosition(lints)) for _, l := range lints { @@ -78,11 +82,15 @@ func newTypesInfoExt(pass *analysis.Pass) *TypesInfoExt { } stack := []ast.Node{file} ast.Inspect(file, func(n ast.Node) bool { - nodeParent[n] = stack[len(stack)-1] - if n == nil { - stack = stack[:len(stack)-1] - } else { + if n != nil && len(stack) > 0 { + // Only set parent if the node is not the same as the current stack top + // This prevents self-references that cause infinite loops + if n != stack[len(stack)-1] { + nodeParent[n] = stack[len(stack)-1] + } stack = append(stack, n) + } else if n == nil { + stack = stack[:len(stack)-1] } return true }) diff --git a/errorlint/analysis_test.go b/errorlint/analysis_test.go index a7243cd..56ac809 100644 --- a/errorlint/analysis_test.go +++ b/errorlint/analysis_test.go @@ -41,7 +41,12 @@ func TestAllowedComparisons(t *testing.T) { func TestIssueRegressions(t *testing.T) { analyzer := NewAnalyzer() - analysistest.Run(t, analysistest.TestData(), analyzer, "issues") + analysistest.Run(t, analysistest.TestData(), analyzer, "issues/lint") +} + +func TestIssueFixRegressions(t *testing.T) { + analyzer := NewAnalyzer() + analysistest.RunWithSuggestedFixes(t, analysistest.TestData(), analyzer, "issues/fix") } func TestErrorComparisonFixes(t *testing.T) { @@ -53,3 +58,8 @@ func TestErrorTypeAssertionFixes(t *testing.T) { analyzer := NewAnalyzer() analysistest.RunWithSuggestedFixes(t, analysistest.TestData(), analyzer, "errorassert") } + +func TestASTManipulationFixes(t *testing.T) { + analyzer := NewAnalyzer() + analysistest.RunWithSuggestedFixes(t, analysistest.TestData(), analyzer, "astmanipulation") +} diff --git a/errorlint/lint.go b/errorlint/lint.go index 9ef6d58..c1e50c8 100644 --- a/errorlint/lint.go +++ b/errorlint/lint.go @@ -13,6 +13,130 @@ import ( "golang.org/x/tools/go/analysis" ) +type diagnosticCategory string + +const ( + categoryTypeAssertion diagnosticCategory = "type-assertion" + categoryErrorComparison diagnosticCategory = "error-comparison" + categoryCombinedError diagnosticCategory = "combined-error" + categoryFormatVerb diagnosticCategory = "format-verb" + unknownDiagnostic diagnosticCategory = "" +) + +const ( + msgTypeAssertion = "type assertion on error" + msgErrorComparison = "comparing with" +) + +func hasConflictingDiagnostics(lints []analysis.Diagnostic) bool { + var hasTypeAssertion, hasErrorComparison bool + + for _, lint := range lints { + switch diagnosticCategory(lint.Category) { + case categoryTypeAssertion: + hasTypeAssertion = true + case categoryErrorComparison: + hasErrorComparison = true + case categoryCombinedError: + return true + } + + if hasTypeAssertion && hasErrorComparison { + return true + } + } + + return false +} + +func extractTypeAssignment(init ast.Stmt) (*ast.AssignStmt, *ast.TypeAssertExpr) { + assignStmt, ok := init.(*ast.AssignStmt) + if !ok || len(assignStmt.Lhs) != 2 || len(assignStmt.Rhs) != 1 { + return nil, nil + } + + typeAssert, ok := assignStmt.Rhs[0].(*ast.TypeAssertExpr) + if !ok { + return nil, nil + } + + return assignStmt, typeAssert +} + +func extractComparison(cond ast.Expr) *ast.BinaryExpr { + binExpr, ok := cond.(*ast.BinaryExpr) + if !ok || binExpr.Op != token.LAND { + return nil + } + + if _, ok := binExpr.X.(*ast.Ident); !ok { + return nil + } + rightBinExpr, ok := binExpr.Y.(*ast.BinaryExpr) + if !ok || (rightBinExpr.Op != token.EQL && rightBinExpr.Op != token.NEQ) { + return nil + } + + return rightBinExpr +} + +func buildVarDeclaration(assertion typeAssertion) string { + targetTypeStr := exprToString(assertion.targetType) + if baseType, found := strings.CutPrefix(targetTypeStr, "*"); found { + return fmt.Sprintf("%s := &%s{}", assertion.varName, baseType) + } + return fmt.Sprintf("var %s %s", assertion.varName, targetTypeStr) +} + +func buildErrorsIsCall(comp comparison) string { + comparisonTarget := exprToString(comp.target) + comparisonExpr := exprToString(comp.expr) + + if comp.negated { + return fmt.Sprintf("!errors.Is(%s, %s)", comparisonExpr, comparisonTarget) + } + return fmt.Sprintf("errors.Is(%s, %s)", comparisonExpr, comparisonTarget) +} + +func formatBodyStmts(bodyStmts []ast.Stmt) string { + if len(bodyStmts) == 0 { + return "" + } + + var bodyBuf bytes.Buffer + for _, stmt := range bodyStmts { + if err := printer.Fprint(&bodyBuf, token.NewFileSet(), stmt); err != nil { + // TODO: How to handle this? Panic? + continue + } + bodyBuf.WriteString("\n\t\t") + } + return strings.TrimSpace(bodyBuf.String()) +} + +func groupDiagnosticsByIfStmt(lints []analysis.Diagnostic, extInfo *TypesInfoExt) (map[*ast.IfStmt][]analysis.Diagnostic, []analysis.Diagnostic) { + ifGroups := make(map[*ast.IfStmt][]analysis.Diagnostic) + var otherLints []analysis.Diagnostic + + for _, lint := range lints { + node := findNodeAtPosition(extInfo, lint.Pos) + if node == nil { + otherLints = append(otherLints, lint) + continue + } + + ifStmt := statementContainsIf(extInfo, node) + if ifStmt == nil { + otherLints = append(otherLints, lint) + continue + } + + ifGroups[ifStmt] = append(ifGroups[ifStmt], lint) + } + + return ifGroups, otherLints +} + type ByPosition []analysis.Diagnostic func (l ByPosition) Len() int { return len(l) } @@ -58,8 +182,9 @@ func LintFmtErrorfCalls(fset *token.FileSet, info types.Info, multipleWraps bool wrapCount++ if wrapCount > 1 { lints = append(lints, analysis.Diagnostic{ - Message: "only one %w verb is permitted per format string", - Pos: arg.Pos(), + Category: string(categoryFormatVerb), + Message: "only one %w verb is permitted per format string", + Pos: arg.Pos(), }) break } @@ -67,8 +192,9 @@ func LintFmtErrorfCalls(fset *token.FileSet, info types.Info, multipleWraps bool if wrapCount == 0 { lints = append(lints, analysis.Diagnostic{ - Message: "non-wrapping format verb for fmt.Errorf. Use `%w` to format errors", - Pos: args[i].Pos(), + Category: string(categoryFormatVerb), + Message: "non-wrapping format verb for fmt.Errorf. Use `%w` to format errors", + Pos: args[i].Pos(), }) break } @@ -98,8 +224,9 @@ func LintFmtErrorfCalls(fset *token.FileSet, info types.Info, multipleWraps bool strStart := call.Args[0].Pos() if lint == nil { lint = &analysis.Diagnostic{ - Message: "non-wrapping format verb for fmt.Errorf. Use `%w` to format errors", - Pos: arg.Pos(), + Category: string(categoryFormatVerb), + Message: "non-wrapping format verb for fmt.Errorf. Use `%w` to format errors", + Pos: arg.Pos(), } } fixMessage := "Use `%w` to format errors" @@ -165,7 +292,7 @@ func isFmtErrorfCallExpr(info types.Info, expr ast.Expr) (*ast.CallExpr, bool) { return nil, false } -func LintErrorComparisons(info *TypesInfoExt) []analysis.Diagnostic { +func LintErrorComparisons(fset *token.FileSet, info *TypesInfoExt) []analysis.Diagnostic { var lints []analysis.Diagnostic // Check for error comparisons. @@ -196,8 +323,9 @@ func LintErrorComparisons(info *TypesInfoExt) []analysis.Diagnostic { } diagnostic := analysis.Diagnostic{ - Message: fmt.Sprintf("comparing with %s will fail on wrapped errors. Use errors.Is to check for a specific error", binExpr.Op), - Pos: binExpr.Pos(), + Category: string(categoryErrorComparison), + Message: fmt.Sprintf("comparing with %s will fail on wrapped errors. Use errors.Is to check for a specific error", binExpr.Op), + Pos: binExpr.Pos(), } // Add suggested fix. @@ -221,13 +349,20 @@ func LintErrorComparisons(info *TypesInfoExt) []analysis.Diagnostic { replacement = "!" + replacement } + textEdits := []analysis.TextEdit{{ + Pos: binExpr.Pos(), + End: binExpr.End(), + NewText: []byte(replacement), + }} + + if file := findContainingFile(info, binExpr); file != nil && needsErrorsImport(file) { + importFix := createImportFix(file) + textEdits = append(textEdits, importFix.TextEdits...) + } + diagnostic.SuggestedFixes = []analysis.SuggestedFix{{ - Message: "Use errors.Is() to compare errors", - TextEdits: []analysis.TextEdit{{ - Pos: binExpr.Pos(), - End: binExpr.End(), - NewText: []byte(replacement), - }}, + Message: "Use errors.Is() to compare errors", + TextEdits: textEdits, }} lints = append(lints, diagnostic) @@ -271,8 +406,9 @@ func LintErrorComparisons(info *TypesInfoExt) []analysis.Diagnostic { if switchComparesNonNil(switchStmt) { diagnostic := analysis.Diagnostic{ - Message: "switch on an error will fail on wrapped errors. Use errors.Is to check for specific errors", - Pos: problematicCaseClause.Pos(), + Category: string(categoryErrorComparison), + Message: "switch on an error will fail on wrapped errors. Use errors.Is to check for specific errors", + Pos: problematicCaseClause.Pos(), } // Create a simpler version of the fix for switch statements @@ -335,7 +471,10 @@ func LintErrorComparisons(info *TypesInfoExt) []analysis.Diagnostic { // Print the modified AST to get the fix text. var buf bytes.Buffer - printer.Fprint(&buf, token.NewFileSet(), newSwitchStmt) + if err := printer.Fprint(&buf, token.NewFileSet(), newSwitchStmt); err != nil { + // TODO: How to handle this? Panic? + continue + } fixText := buf.String() diagnostic.SuggestedFixes = []analysis.SuggestedFix{{ @@ -354,7 +493,49 @@ func LintErrorComparisons(info *TypesInfoExt) []analysis.Diagnostic { return lints } -// exprToString converts an expression to its string representation. +func findContainingFile(info *TypesInfoExt, node ast.Node) *ast.File { + current := node + for current != nil { + if file, ok := current.(*ast.File); ok { + return file + } + current = info.NodeParent[current] + } + return nil +} + +func needsErrorsImport(file *ast.File) bool { + for _, imp := range file.Imports { + if imp.Path.Value == `"errors"` { + return false + } + } + return true +} + +func createImportFix(file *ast.File) analysis.SuggestedFix { + var importPos token.Pos + var newText string + + if len(file.Imports) > 0 { + lastImport := file.Imports[len(file.Imports)-1] + importPos = lastImport.End() + newText = "\n\t\"errors\"" + } else { + importPos = file.Name.End() + newText = "\n\nimport \"errors\"" + } + + return analysis.SuggestedFix{ + Message: "Add missing errors import", + TextEdits: []analysis.TextEdit{{ + Pos: importPos, + End: importPos, + NewText: []byte(newText), + }}, + } +} + func exprToString(expr ast.Expr) string { switch e := expr.(type) { case *ast.Ident: @@ -471,8 +652,9 @@ func LintErrorTypeAssertions(fset *token.FileSet, info *TypesInfoExt) []analysis } diagnostic := analysis.Diagnostic{ - Message: "type assertion on error will fail on wrapped errors. Use errors.As to check for specific errors", - Pos: typeAssert.Pos(), + Category: string(categoryTypeAssertion), + Message: "type assertion on error will fail on wrapped errors. Use errors.As to check for specific errors", + Pos: typeAssert.Pos(), } // Create suggested fix for type assertion @@ -635,8 +817,9 @@ func LintErrorTypeAssertions(fset *token.FileSet, info *TypesInfoExt) []analysis } diagnostic := analysis.Diagnostic{ - Message: "type switch on error will fail on wrapped errors. Use errors.As to check for specific errors", - Pos: typeAssert.Pos(), + Category: string(categoryTypeAssertion), + Message: "type switch on error will fail on wrapped errors. Use errors.As to check for specific errors", + Pos: typeAssert.Pos(), } // Transform type switch into a switch statement with errors.As in each case @@ -804,7 +987,9 @@ func LintErrorTypeAssertions(fset *token.FileSet, info *TypesInfoExt) []analysis // Print the resulting block to get the fix text. var buf bytes.Buffer - printer.Fprint(&buf, token.NewFileSet(), blockStmt) + if err := printer.Fprint(&buf, token.NewFileSet(), blockStmt); err != nil { + continue + } fixText := buf.String() diagnostic.SuggestedFixes = []analysis.SuggestedFix{{ @@ -880,3 +1065,247 @@ func generateErrorVarName(originalName, typeName string) string { // If we couldn't determine a good name, use default. return "anErr" } + +func resolveConflicts(lints []analysis.Diagnostic, extInfo *TypesInfoExt) []analysis.Diagnostic { + ifGroups, otherLints := groupDiagnosticsByIfStmt(lints, extInfo) + + var result []analysis.Diagnostic + + for ifStmt, groupLints := range ifGroups { + if len(groupLints) <= 1 { + result = append(result, groupLints...) + continue + } + + if hasConflictingDiagnostics(groupLints) { + if combined := createCombinedDiagnostic(ifStmt, groupLints, extInfo); combined != nil { + result = append(result, *combined) + continue + } + } + + result = append(result, groupLints...) + } + + return append(result, otherLints...) +} + +func findNodeAtPosition(extInfo *TypesInfoExt, pos token.Pos) ast.Node { + // First check type-checked expressions (most common case) + for node := range extInfo.TypesInfo.Types { + if nodeContainsPos(node, pos) { + return node + } + } + + // Fallback: check scopes + for scope := range extInfo.TypesInfo.Scopes { + if nodeContainsPos(scope, pos) { + return scope + } + } + + return nil +} + +// nodeContainsPos checks if a node contains the given position +func nodeContainsPos(node ast.Node, pos token.Pos) bool { + return node.Pos() <= pos && pos < node.End() +} + +// statementContainsIf finds the if statement that contains the given node +// by walking up the AST parent chain. +func statementContainsIf(extInfo *TypesInfoExt, node ast.Node) *ast.IfStmt { + current := node + for current != nil { + if ifStmt, ok := current.(*ast.IfStmt); ok { + return ifStmt + } + parent := extInfo.NodeParent[current] + if parent == nil { + break + } + current = parent + } + return nil +} + +// createCombinedDiagnostic creates a single diagnostic that handles both +// type assertion and error comparison issues in the same if statement. +// It generates a combined suggested fix that uses both errors.As and errors.Is. +func createCombinedDiagnostic(ifStmt *ast.IfStmt, lints []analysis.Diagnostic, extInfo *TypesInfoExt) *analysis.Diagnostic { + // Find the earliest position for the combined diagnostic + earliestPos := token.NoPos + for _, lint := range lints { + if earliestPos == token.NoPos || lint.Pos < earliestPos { + earliestPos = lint.Pos + } + } + + // Create the combined diagnostic + combined := &analysis.Diagnostic{ + Category: string(categoryCombinedError), + Pos: earliestPos, + Message: "type assertion and error comparison will fail on wrapped errors. Use errors.As and errors.Is to check for specific errors", + } + + // Try to create a combined fix for the if statement + suggestedFix := combinedFix(ifStmt, extInfo) + if suggestedFix != nil { + combined.SuggestedFixes = []analysis.SuggestedFix{*suggestedFix} + } + + return combined +} + +// combinedFix creates a suggested fix that handles both type assertion +// and error comparison in the same if statement. +// Transforms: if e, ok := err.(*Type); ok && e.Field == value { +// Into: e := &Type{}; if errors.As(err, &e) && errors.Is(e.Field, value) { +func combinedFix(ifStmt *ast.IfStmt, extInfo *TypesInfoExt) *analysis.SuggestedFix { + // Parse the if statement structure to extract components + components := parseIfComponents(ifStmt) + if components == nil { + return nil + } + + // Check if this is an else-if statement + components.context.isElseIf = isElseIfStatement(ifStmt, extInfo) + + // Handle else-if transformations carefully + // Note: else-if transformations change structure but preserve functionality + + // Build the replacement text using the extracted components + replacement := buildReplacement(components) + if replacement == "" { + return nil + } + + // Determine the replacement range based on whether it's an else-if + endPos := ifStmt.Body.Pos() + if components.context.isElseIf { + // For else-if cases, we need to replace from the "if" to the end of the block + // to properly handle the transformation + endPos = ifStmt.Body.End() + } + + textEdits := []analysis.TextEdit{{ + Pos: ifStmt.Pos(), + End: endPos, + NewText: []byte(replacement), + }} + + // Add errors import if needed + if file := findContainingFile(extInfo, ifStmt); file != nil && needsErrorsImport(file) { + importFix := createImportFix(file) + textEdits = append(textEdits, importFix.TextEdits...) + } + + return &analysis.SuggestedFix{ + Message: "Use errors.As and errors.Is for error handling", + TextEdits: textEdits, + } +} + +// isElseIfStatement checks if the given if statement is part of an else-if construct +// by checking if it's in the Else field of a parent if statement. +func isElseIfStatement(ifStmt *ast.IfStmt, extInfo *TypesInfoExt) bool { + parent := extInfo.NodeParent[ifStmt] + if parent == nil { + return false + } + + // Check if the parent is an if statement's Else field + if parentIf, ok := parent.(*ast.IfStmt); ok { + return parentIf.Else == ifStmt + } + + return false +} + +// typeAssertion holds type assertion specific data +type typeAssertion struct { + varName string + errorExpr ast.Expr + targetType ast.Expr +} + +// comparison holds error comparison specific data +type comparison struct { + expr ast.Expr + target ast.Expr + negated bool +} + +// controlFlowContext holds if statement controlFlowContext information +type controlFlowContext struct { + isElseIf bool + bodyStmts []ast.Stmt +} + +// ifComponents holds the parsed components of an if statement +// that can be converted to use errors.As and errors.Is. +type ifComponents struct { + assertion typeAssertion + comparison comparison + context controlFlowContext +} + +// parseIfComponents extracts the components of the if statement pattern +// we want to fix: if e, ok := err.(*Type); ok && e.Field == value { +func parseIfComponents(ifStmt *ast.IfStmt) *ifComponents { + if ifStmt.Init == nil || ifStmt.Cond == nil { + return nil + } + + assignStmt, typeAssert := extractTypeAssignment(ifStmt.Init) + if assignStmt == nil || typeAssert == nil { + return nil + } + + rightBinExpr := extractComparison(ifStmt.Cond) + if rightBinExpr == nil { + return nil + } + + varIdent, ok := assignStmt.Lhs[0].(*ast.Ident) + if !ok { + return nil + } + + return &ifComponents{ + assertion: typeAssertion{ + varName: varIdent.Name, + errorExpr: typeAssert.X, + targetType: typeAssert.Type, + }, + comparison: comparison{ + expr: rightBinExpr.X, + target: rightBinExpr.Y, + negated: rightBinExpr.Op == token.NEQ, + }, + context: controlFlowContext{ + isElseIf: false, // Will be set by the calling function if needed + bodyStmts: ifStmt.Body.List, // Capture body statements + }, + } +} + +// buildReplacement creates the replacement text using proper formatting. +// It generates code like: e := &Type{}; if errors.As(err, &e) && errors.Is(e.Field, value) { +func buildReplacement(components *ifComponents) string { + var ( + errExpr = exprToString(components.assertion.errorExpr) + varDecl = buildVarDeclaration(components.assertion) + errorsIsCall = buildErrorsIsCall(components.comparison) + ) + + if components.context.isElseIf { + bodyText := formatBodyStmts(components.context.bodyStmts) + return fmt.Sprintf("{\n\t\t%s\n\t\tif errors.As(%s, &%s) && %s {\n\t\t\t%s\n\t\t}\n\t}", + varDecl, errExpr, components.assertion.varName, errorsIsCall, bodyText) + } + + return fmt.Sprintf("%s\n\tif errors.As(%s, &%s) && %s ", + varDecl, errExpr, components.assertion.varName, errorsIsCall) +} diff --git a/errorlint/testdata/src/astmanipulation/comments.go b/errorlint/testdata/src/astmanipulation/comments.go new file mode 100644 index 0000000..3ff358c --- /dev/null +++ b/errorlint/testdata/src/astmanipulation/comments.go @@ -0,0 +1,39 @@ +package astmanipulation + +import ( + "os" + "syscall" +) + +// Test inline comment preservation +func InlineComments() { + var err error + + if err == syscall.ENOENT { // want "comparing with == will fail on wrapped errors. Use errors.Is to check for a specific error" + // This block comment should be preserved + println("File not found") + } +} + +// Test else-if comment preservation (main PR issue) +func ElseIfComments() { + var err error + + if err != nil { + println("error occurred") + } else if e, ok := err.(*os.PathError); ok && e.Err == syscall.ESRCH { // want "type assertion and error comparison will fail on wrapped errors. Use errors.As and errors.Is to check for specific errors" + // If the process exits while reading its /proc/$PID/maps, the kernel will + // return ESRCH. Handle it as if the process did not exist. + println("Process not found") + } +} + +// Test block comments +func BlockComments() { + var err error + + /* Pre-condition block comment */ + if e, ok := err.(*os.PathError); ok { // want "type assertion on error will fail on wrapped errors. Use errors.As to check for specific errors" + /* Inline block comment */ println("Path error:", e.Path) + } +} diff --git a/errorlint/testdata/src/astmanipulation/comments.go.golden b/errorlint/testdata/src/astmanipulation/comments.go.golden new file mode 100644 index 0000000..449bb57 --- /dev/null +++ b/errorlint/testdata/src/astmanipulation/comments.go.golden @@ -0,0 +1,42 @@ +package astmanipulation + +import ( + "errors" + "os" + "syscall" +) + +// Test inline comment preservation +func InlineComments() { + var err error + + if errors.Is(err, syscall.ENOENT) { // want "comparing with == will fail on wrapped errors. Use errors.Is to check for a specific error" + // This block comment should be preserved + println("File not found") + } +} + +// Test else-if comment preservation (main PR issue) +func ElseIfComments() { + var err error + + if err != nil { + println("error occurred") + } else { + e := &os.PathError{} + if errors.As(err, &e) && errors.Is(e.Err, syscall.ESRCH) { + println("Process not found") + } + } +} + +// Test block comments +func BlockComments() { + var err error + + /* Pre-condition block comment */ + e := &os.PathError{} + if errors.As(err, &e) { // want "type assertion on error will fail on wrapped errors. Use errors.As to check for specific errors" + /* Inline block comment */ println("Path error:", e.Path) + } +} \ No newline at end of file diff --git a/errorlint/testdata/src/astmanipulation/imports.go b/errorlint/testdata/src/astmanipulation/imports.go new file mode 100644 index 0000000..b32e82e --- /dev/null +++ b/errorlint/testdata/src/astmanipulation/imports.go @@ -0,0 +1,25 @@ +package astmanipulation + +import ( + "io" + "os" +) + +// Test missing errors import with simple comparison +func MissingImportSimple() { + var err error + var n int + + if n == 0 || (err != nil && err != io.EOF) { // want "comparing with != will fail on wrapped errors. Use errors.Is to check for a specific error" + return + } +} + +// Test missing errors import with type assertion +func MissingImportTypeAssertion() { + var err error + + if e, ok := err.(*os.PathError); ok { // want "type assertion on error will fail on wrapped errors. Use errors.As to check for specific errors" + println("Path error:", e.Path) + } +} \ No newline at end of file diff --git a/errorlint/testdata/src/astmanipulation/imports.go.golden b/errorlint/testdata/src/astmanipulation/imports.go.golden new file mode 100644 index 0000000..74c4d54 --- /dev/null +++ b/errorlint/testdata/src/astmanipulation/imports.go.golden @@ -0,0 +1,27 @@ +package astmanipulation + +import ( + "errors" + "io" + "os" +) + +// Test missing errors import with simple comparison +func MissingImportSimple() { + var err error + var n int + + if n == 0 || (err != nil && !errors.Is(err, io.EOF)) { // want "comparing with != will fail on wrapped errors. Use errors.Is to check for a specific error" + return + } +} + +// Test missing errors import with type assertion +func MissingImportTypeAssertion() { + var err error + + e := &os.PathError{} + if errors.As(err, &e) { // want "type assertion on error will fail on wrapped errors. Use errors.As to check for specific errors" + println("Path error:", e.Path) + } +} \ No newline at end of file diff --git a/errorlint/testdata/src/issues/fix/github-100.go b/errorlint/testdata/src/issues/fix/github-100.go new file mode 100644 index 0000000..c9e1294 --- /dev/null +++ b/errorlint/testdata/src/issues/fix/github-100.go @@ -0,0 +1,36 @@ +package issues + +import ( + "os" + "syscall" +) + +// Test case for https://github.com/polyfloyd/go-errorlint/issues/100 +// This reproduces the conflicting edits issue where both type assertion +// and error comparison rules apply to the same line of code. +func ConflictingEdits() { + var err error = &os.PathError{Err: syscall.ESRCH} + + // This line should trigger both: + // 1. Type assertion linting (err.(*os.PathError)) + // 2. Error comparison linting (e.Err == syscall.ESRCH) + // Leading to a combined fix instead of conflicting edits + if e, ok := err.(*os.PathError); ok && e.Err == syscall.ESRCH { // want "type assertion and error comparison will fail on wrapped errors. Use errors.As and errors.Is to check for specific errors" + println("Found PathError with ESRCH") + } + + // Additional test cases with similar patterns + if pathErr, ok := err.(*os.PathError); ok && pathErr.Err == syscall.ENOENT { // want "type assertion and error comparison will fail on wrapped errors. Use errors.As and errors.Is to check for specific errors" + println("Found PathError with ENOENT") + } + + // Test the exact pattern from the original issue #100 + // This is the actual problematic line that caused conflicting edits + if err != nil { + println("error occurred") + } else if e, ok := err.(*os.PathError); ok && e.Err == syscall.ESRCH { // want "type assertion and error comparison will fail on wrapped errors. Use errors.As and errors.Is to check for specific errors" + // If the process exits while reading its /proc/$PID/maps, the kernel will + // return ESRCH. Handle it as if the process did not exist. + println("Found PathError with ESRCH in else if - from original issue") + } +} \ No newline at end of file diff --git a/errorlint/testdata/src/issues/fix/github-100.go.golden b/errorlint/testdata/src/issues/fix/github-100.go.golden new file mode 100644 index 0000000..d4d23f9 --- /dev/null +++ b/errorlint/testdata/src/issues/fix/github-100.go.golden @@ -0,0 +1,40 @@ +package issues + +import ( + "errors" + "os" + "syscall" +) + +// Test case for https://github.com/polyfloyd/go-errorlint/issues/100 +// This reproduces the conflicting edits issue where both type assertion +// and error comparison rules apply to the same line of code. +func ConflictingEdits() { + var err error = &os.PathError{Err: syscall.ESRCH} + + // This line should trigger both: + // 1. Type assertion linting (err.(*os.PathError)) + // 2. Error comparison linting (e.Err == syscall.ESRCH) + // Leading to a combined fix instead of conflicting edits + e := &os.PathError{} + if errors.As(err, &e) && errors.Is(e.Err, syscall.ESRCH) { // want "type assertion and error comparison will fail on wrapped errors. Use errors.As and errors.Is to check for specific errors" + println("Found PathError with ESRCH") + } + + // Additional test cases with similar patterns + pathErr := &os.PathError{} + if errors.As(err, &pathErr) && errors.Is(pathErr.Err, syscall.ENOENT) { // want "type assertion and error comparison will fail on wrapped errors. Use errors.As and errors.Is to check for specific errors" + println("Found PathError with ENOENT") + } + + // Test the exact pattern from the original issue #100 + // This is the actual problematic line that caused conflicting edits + if err != nil { + println("error occurred") + } else { + e := &os.PathError{} + if errors.As(err, &e) && errors.Is(e.Err, syscall.ESRCH) { + println("Found PathError with ESRCH in else if - from original issue") + } + } +} \ No newline at end of file diff --git a/errorlint/testdata/src/issues/github-11.go b/errorlint/testdata/src/issues/lint/github-11.go similarity index 100% rename from errorlint/testdata/src/issues/github-11.go rename to errorlint/testdata/src/issues/lint/github-11.go diff --git a/errorlint/testdata/src/issues/github-19.go b/errorlint/testdata/src/issues/lint/github-19.go similarity index 100% rename from errorlint/testdata/src/issues/github-19.go rename to errorlint/testdata/src/issues/lint/github-19.go diff --git a/errorlint/testdata/src/issues/github-21.go b/errorlint/testdata/src/issues/lint/github-21.go similarity index 100% rename from errorlint/testdata/src/issues/github-21.go rename to errorlint/testdata/src/issues/lint/github-21.go diff --git a/errorlint/testdata/src/issues/github-26.go b/errorlint/testdata/src/issues/lint/github-26.go similarity index 100% rename from errorlint/testdata/src/issues/github-26.go rename to errorlint/testdata/src/issues/lint/github-26.go diff --git a/errorlint/testdata/src/issues/github-29.go b/errorlint/testdata/src/issues/lint/github-29.go similarity index 100% rename from errorlint/testdata/src/issues/github-29.go rename to errorlint/testdata/src/issues/lint/github-29.go diff --git a/errorlint/testdata/src/issues/github-42.go b/errorlint/testdata/src/issues/lint/github-42.go similarity index 100% rename from errorlint/testdata/src/issues/github-42.go rename to errorlint/testdata/src/issues/lint/github-42.go diff --git a/errorlint/testdata/src/issues/github-50.go b/errorlint/testdata/src/issues/lint/github-50.go similarity index 100% rename from errorlint/testdata/src/issues/github-50.go rename to errorlint/testdata/src/issues/lint/github-50.go diff --git a/errorlint/testdata/src/issues/github-54.go b/errorlint/testdata/src/issues/lint/github-54.go similarity index 100% rename from errorlint/testdata/src/issues/github-54.go rename to errorlint/testdata/src/issues/lint/github-54.go diff --git a/errorlint/testdata/src/issues/github-57.go b/errorlint/testdata/src/issues/lint/github-57.go similarity index 100% rename from errorlint/testdata/src/issues/github-57.go rename to errorlint/testdata/src/issues/lint/github-57.go diff --git a/errorlint/testdata/src/issues/github-61.go b/errorlint/testdata/src/issues/lint/github-61.go similarity index 100% rename from errorlint/testdata/src/issues/github-61.go rename to errorlint/testdata/src/issues/lint/github-61.go