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

.golangci.yaml

@@ -67,6 +67,12 @@ linters-settings:
     - pkg: github.com/blang/semver/v4
       alias: bsemver
 
+custom:
+  setuplognilerrorcheck:
+    path: ./bin/custom-linter
+    description: Runs the custom linter that checks for 'setupLog.Error(nil, ...)'
+    required: true
+
 output:
   formats:
     - format: tab

Makefile

@@ -95,7 +95,7 @@ help-extended: #HELP Display extended help.
 #SECTION Development
 
 .PHONY: lint
-lint: $(GOLANGCI_LINT) #HELP Run golangci linter.
+lint: custom-linter $(GOLANGCI_LINT) #HELP Run golangci linter.
 	$(GOLANGCI_LINT) run --build-tags $(GO_BUILD_TAGS) $(GOLANGCI_LINT_ARGS)
 
 .PHONY: tidy
@@ -285,6 +285,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
+custom-linter:
+	cd ./hack/ci/custom-linters/cmd/ && go build -o ../../../../bin/custom-linter
+
+
 .PHONY: build-deps
 build-deps: manifests generate fmt vet
 

go.mod

@@ -24,6 +24,7 @@ require (
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.10.0
 	golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56
+	golang.org/x/tools v0.28.0
 	gopkg.in/yaml.v2 v2.4.0
 	helm.sh/helm/v3 v3.16.4
 	k8s.io/api v0.31.4
@@ -230,7 +231,6 @@ require (
 	golang.org/x/term v0.28.0 // indirect
 	golang.org/x/text v0.21.0 // indirect
 	golang.org/x/time v0.5.0 // indirect
-	golang.org/x/tools v0.28.0 // indirect
 	gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect
 	google.golang.org/genproto v0.0.0-20240311173647-c811ad7063a7 // indirect
 	google.golang.org/genproto/googleapis/api v0.0.0-20240814211410-ddb44dafa142 // indirect

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,5 @@
+module github.com/github.com/operator-framework/operator-controller/custom-linters
+
+go 1.23.4
+
+require golang.org/x/tools v0.29.0

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,43 @@
+package setuplognilerrorcheck
+
+import (
+	"go/ast"
+	"golang.org/x/tools/go/analysis"
+)
+
+var SetupLogNilErrorCheck = &analysis.Analyzer{
+	Name: "setuplognilerrorcheck",
+	Doc:  "check for setupLog.Error(nil, ...) calls with nil as the first argument",
+	Run:  runSetupLogNilErrorCheck,
+}
+
+func runSetupLogNilErrorCheck(pass *analysis.Pass) (interface{}, error) {
+	for _, f := range pass.Files {
+		ast.Inspect(f, func(n ast.Node) bool {
+			expr, ok := n.(*ast.CallExpr)
+			if !ok {
+				return true
+			}
+
+			selectorExpr, ok := expr.Fun.(*ast.SelectorExpr)
+			if !ok || selectorExpr.Sel.Name != "Error" {
+				return true
+			}
+
+			i, ok := selectorExpr.X.(*ast.Ident)
+			if !ok || i.Name != "setupLog" {
+				return true
+			}
+
+			if len(expr.Args) > 0 {
+				if arg, ok := expr.Args[0].(*ast.Ident); ok && arg.Name == "nil" {
+					pass.Reportf(expr.Pos(), "Avoid using 'setupLog.Error(nil, ...)'. Instead, use 'errors.New()' "+
+						"or 'fmt.Errorf()' to ensure logs are created. Using 'nil' for errors can result in silent "+
+						"failures, making bugs harder to detect.")
+				}
+			}
+			return true
+		})
+	}
+	return nil, nil
+}