|
| 1 | +package analyzers |
| 2 | + |
| 3 | +import ( |
| 4 | + "bytes" |
| 5 | + "go/ast" |
| 6 | + "go/format" |
| 7 | + "go/token" |
| 8 | + "go/types" |
| 9 | + |
| 10 | + "golang.org/x/tools/go/analysis" |
| 11 | +) |
| 12 | + |
| 13 | +var SetupLogErrorCheck = &analysis.Analyzer{ |
| 14 | + Name: "setuplogerrorcheck", |
| 15 | + Doc: "Detects improper usage of logger.Error() calls, ensuring the first argument is a non-nil error.", |
| 16 | + Run: runSetupLogErrorCheck, |
| 17 | +} |
| 18 | + |
| 19 | +func runSetupLogErrorCheck(pass *analysis.Pass) (interface{}, error) { |
| 20 | + for _, f := range pass.Files { |
| 21 | + ast.Inspect(f, func(n ast.Node) bool { |
| 22 | + callExpr, ok := n.(*ast.CallExpr) |
| 23 | + if !ok { |
| 24 | + return true |
| 25 | + } |
| 26 | + |
| 27 | + // Ensure function being called is logger.Error |
| 28 | + selectorExpr, ok := callExpr.Fun.(*ast.SelectorExpr) |
| 29 | + if !ok || selectorExpr.Sel.Name != "Error" { |
| 30 | + return true |
| 31 | + } |
| 32 | + |
| 33 | + // Ensure receiver (logger) is identified |
| 34 | + ident, ok := selectorExpr.X.(*ast.Ident) |
| 35 | + if !ok { |
| 36 | + return true |
| 37 | + } |
| 38 | + |
| 39 | + // Verify if the receiver is logr.Logger |
| 40 | + obj := pass.TypesInfo.ObjectOf(ident) |
| 41 | + if obj == nil { |
| 42 | + return true |
| 43 | + } |
| 44 | + |
| 45 | + named, ok := obj.Type().(*types.Named) |
| 46 | + if !ok || named.Obj().Pkg() == nil || named.Obj().Pkg().Path() != "github.com/go-logr/logr" || named.Obj().Name() != "Logger" { |
| 47 | + return true |
| 48 | + } |
| 49 | + |
| 50 | + if len(callExpr.Args) == 0 { |
| 51 | + return true |
| 52 | + } |
| 53 | + |
| 54 | + // Get the actual source code line where the issue occurs |
| 55 | + var srcBuffer bytes.Buffer |
| 56 | + if err := format.Node(&srcBuffer, pass.Fset, callExpr); err != nil { |
| 57 | + return true |
| 58 | + } |
| 59 | + sourceLine := srcBuffer.String() |
| 60 | + |
| 61 | + // Check if the first argument of the error log is nil |
| 62 | + firstArg, ok := callExpr.Args[0].(*ast.Ident) |
| 63 | + if ok && firstArg.Name == "nil" { |
| 64 | + suggestedError := "errors.New(\"kind error (i.e. configuration error)\")" |
| 65 | + suggestedMessage := "\"error message describing the failed operation\"" |
| 66 | + |
| 67 | + if len(callExpr.Args) > 1 { |
| 68 | + if msgArg, ok := callExpr.Args[1].(*ast.BasicLit); ok && msgArg.Kind == token.STRING { |
| 69 | + suggestedMessage = msgArg.Value |
| 70 | + } |
| 71 | + } |
| 72 | + |
| 73 | + pass.Reportf(callExpr.Pos(), |
| 74 | + "Incorrect usage of 'logger.Error(nil, ...)'. The first argument must be a non-nil 'error'. "+ |
| 75 | + "Passing 'nil' results in silent failures, making debugging harder.\n\n"+ |
| 76 | + "\U0001F41B **What is wrong?**\n %s\n\n"+ |
| 77 | + "\U0001F4A1 **How to solve? Return the error, i.e.:**\n logger.Error(%s, %s, \"key\", value)\n\n", |
| 78 | + sourceLine, suggestedError, suggestedMessage) |
| 79 | + return true |
| 80 | + } |
| 81 | + |
| 82 | + // Ensure at least two arguments exist (error + message) |
| 83 | + if len(callExpr.Args) < 2 { |
| 84 | + pass.Reportf(callExpr.Pos(), |
| 85 | + "Incorrect usage of 'logger.Error(error, ...)'. Expected at least an error and a message string.\n\n"+ |
| 86 | + "\U0001F41B **What is wrong?**\n %s\n\n"+ |
| 87 | + "\U0001F4A1 **How to solve?**\n Provide a message, e.g. logger.Error(err, \"descriptive message\")\n\n", |
| 88 | + sourceLine) |
| 89 | + return true |
| 90 | + } |
| 91 | + |
| 92 | + // Ensure key-value pairs (if any) are valid |
| 93 | + if (len(callExpr.Args)-2)%2 != 0 { |
| 94 | + pass.Reportf(callExpr.Pos(), |
| 95 | + "Incorrect usage of 'logger.Error(error, \"msg\", ...)'. Key-value pairs must be provided after the message, but an odd number of arguments was found.\n\n"+ |
| 96 | + "\U0001F41B **What is wrong?**\n %s\n\n"+ |
| 97 | + "\U0001F4A1 **How to solve?**\n Ensure all key-value pairs are complete, e.g. logger.Error(err, \"msg\", \"key\", value, \"key2\", value2)\n\n", |
| 98 | + sourceLine) |
| 99 | + return true |
| 100 | + } |
| 101 | + |
| 102 | + for i := 2; i < len(callExpr.Args); i += 2 { |
| 103 | + keyArg := callExpr.Args[i] |
| 104 | + keyType := pass.TypesInfo.TypeOf(keyArg) |
| 105 | + if keyType == nil || keyType.String() != "string" { |
| 106 | + pass.Reportf(callExpr.Pos(), |
| 107 | + "Incorrect usage of 'logger.Error(error, \"msg\", key, value)'. Keys in key-value pairs must be strings, but got: %s.\n\n"+ |
| 108 | + "\U0001F41B **What is wrong?**\n %s\n\n"+ |
| 109 | + "\U0001F4A1 **How to solve?**\n Ensure keys are strings, e.g. logger.Error(err, \"msg\", \"key\", value)\n\n", |
| 110 | + keyType, sourceLine) |
| 111 | + return true |
| 112 | + } |
| 113 | + } |
| 114 | + |
| 115 | + return true |
| 116 | + }) |
| 117 | + } |
| 118 | + return nil, nil |
| 119 | +} |
0 commit comments