Add custom Linter to check 'setupLog.Error(nil, ...)'

New custom linter under hack/ci project was created to allow
us to define custom linters for the project

Author: camilamacedo86

Makefile

@@ -95,9 +95,13 @@ help-extended: #HELP Display extended help.
 #SECTION Development
 
 .PHONY: lint
-lint: $(GOLANGCI_LINT) #HELP Run golangci linter.
+lint: lint-custom $(GOLANGCI_LINT) #HELP Run golangci linter.
 	$(GOLANGCI_LINT) run --build-tags $(GO_BUILD_TAGS) $(GOLANGCI_LINT_ARGS)
 
+.PHONY: lint-custom
+lint-custom: custom-linter-build
+	go vet -vettool=./bin/custom-linter ./...
+
 .PHONY: tidy
 tidy: #HELP Update dependencies.
 	# Force tidy to use the version already in go.mod
@@ -290,6 +294,11 @@ BINARIES=operator-controller
 $(BINARIES):
 	go build $(GO_BUILD_FLAGS) -tags '$(GO_BUILD_TAGS)' -ldflags '$(GO_BUILD_LDFLAGS)' -gcflags '$(GO_BUILD_GCFLAGS)' -asmflags '$(GO_BUILD_ASMFLAGS)' -o $(BUILDBIN)/$@ ./cmd/$@
 
+.PHONY: custom-linter-build
+custom-linter-build:
+	cd ./hack/ci/custom-linters/cmd/ && go build -o ../../../../bin/custom-linter
+
+
 .PHONY: build-deps
 build-deps: manifests generate fmt vet
 

hack/ci/custom-linters/cmd/main.go

@@ -0,0 +1,16 @@
+package main
+
+import (
+	"github.com/github.com/operator-framework/operator-controller/custom-linters/setuplognilerrorcheck"
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/unitchecker"
+)
+
+// Define the custom Linters implemented in the project
+var customLinters = []*analysis.Analyzer{
+	setuplognilerrorcheck.SetupLogNilErrorCheck,
+}
+
+func main() {
+	unitchecker.Main(customLinters...)
+}

hack/ci/custom-linters/go.mod

@@ -0,0 +1,10 @@
+module github.com/github.com/operator-framework/operator-controller/custom-linters
+
+go 1.23.4
+
+require golang.org/x/tools v0.29.0
+
+require (
+	golang.org/x/mod v0.22.0 // indirect
+	golang.org/x/sync v0.10.0 // indirect
+)

hack/ci/custom-linters/go.sum

@@ -0,0 +1,8 @@
+github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
+github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
+golang.org/x/mod v0.22.0 h1:D4nJWe9zXqHOmWqj4VMOJhvzj7bEZg4wEYa759z1pH4=
+golang.org/x/mod v0.22.0/go.mod h1:6SkKJ3Xj0I0BrPOZoBy3bdMptDDU9oJrpohJ3eWZ1fY=
+golang.org/x/sync v0.10.0 h1:3NQrjDixjgGwUOCaF8w2+VYHv0Ve/vGYSbdkTa98gmQ=
+golang.org/x/sync v0.10.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
+golang.org/x/tools v0.29.0 h1:Xx0h3TtM9rzQpQuR4dKLrdglAmCEN5Oi+P74JdhdzXE=
+golang.org/x/tools v0.29.0/go.mod h1:KMQVMRsVxU6nHCFXrBPhDB8XncLNLM0lIy/F14RP588=

hack/ci/custom-linters/setuplognilerrorcheck/setuplognilerrorcheck.go

@@ -0,0 +1,86 @@
+package setuplognilerrorcheck
+
+import (
+	"bytes"
+	"fmt"
+	"go/ast"
+	"go/format"
+	"go/token"
+	"go/types"
+
+	"golang.org/x/tools/go/analysis"
+)
+
+var SetupLogNilErrorCheck = &analysis.Analyzer{
+	Name: "setuplognilerrorcheck",
+	Doc:  "Detects improper usage of logger.Error() calls, ensuring the first argument is a non-nil error.",
+	Run:  runSetupLogNilErrorCheck,
+}
+
+func runSetupLogNilErrorCheck(pass *analysis.Pass) (interface{}, error) {
+	for _, f := range pass.Files {
+		ast.Inspect(f, func(n ast.Node) bool {
+			callExpr, ok := n.(*ast.CallExpr)
+			if !ok {
+				return true
+			}
+
+			// Ensure function being called is logger.Error
+			selectorExpr, ok := callExpr.Fun.(*ast.SelectorExpr)
+			if !ok || selectorExpr.Sel.Name != "Error" {
+				return true
+			}
+
+			// Ensure receiver (logger) is identified
+			ident, ok := selectorExpr.X.(*ast.Ident)
+			if !ok {
+				return true
+			}
+
+			// Verify if the receiver is logr.Logger
+			obj := pass.TypesInfo.ObjectOf(ident)
+			if obj == nil {
+				return true
+			}
+
+			if named, ok := obj.Type().(*types.Named); ok {
+				if named.Obj().Pkg() != nil && named.Obj().Pkg().Path() == "github.com/go-logr/logr" && named.Obj().Name() == "Logger" {
+					if len(callExpr.Args) > 0 {
+						firstArg := callExpr.Args[0]
+
+						// Default suggested fix (kind error instead of using log message as error)
+						suggestedError := "errors.New(\"kind error (i.e. configuration error)\")"
+						suggestedMessage := "\"error message describing the failed operation\""
+
+						// Extract log message (second argument) for proper logging
+						if len(callExpr.Args) > 1 {
+							if msgArg, ok := callExpr.Args[1].(*ast.BasicLit); ok && msgArg.Kind == token.STRING {
+								suggestedMessage = msgArg.Value // Use the actual log message correctly
+							}
+						}
+
+						// Get the actual source code line where the issue occurs
+						var srcBuffer bytes.Buffer
+						if err := format.Node(&srcBuffer, pass.Fset, callExpr); err == nil {
+							sourceLine := srcBuffer.String()
+
+							// Case 1: First argument is nil
+							if arg, ok := firstArg.(*ast.Ident); ok && arg.Name == "nil" {
+								pass.Reportf(callExpr.Pos(),
+									"Incorrect usage of 'logger.Error(nil, ...)'. The first argument must be a non-nil 'error'. "+
+										"Passing 'nil' results in silent failures, making debugging harder.\n\n"+
+										"\U0001F41B **What is wrong?**\n"+
+										fmt.Sprintf("   %s\n\n", sourceLine)+
+										"\U0001F4A1 **How to solve? Return the error, i.e.:**\n"+
+										fmt.Sprintf("   logger.Error(%s, %s, \"key\", value)\n\n", suggestedError, suggestedMessage))
+							}
+						}
+					}
+				}
+			}
+
+			return true
+		})
+	}
+	return nil, nil
+}