Skip to content

Commit

Permalink
Add custom Linter to check 'setupLog.Error(nil, ...)'
Browse files Browse the repository at this point in the history
New custom linter under hack/ci project was created to allow
us to define custom linters for the project
  • Loading branch information
camilamacedo86 committed Jan 12, 2025
1 parent 75bb73e commit 1dd674a
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 2 deletions.
11 changes: 10 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -285,6 +289,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

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
16 changes: 16 additions & 0 deletions hack/ci/custom-linters/cmd/main.go
Original file line number Diff line number Diff line change
@@ -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...)
}
5 changes: 5 additions & 0 deletions hack/ci/custom-linters/go.mod
Original file line number Diff line number Diff line change
@@ -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
8 changes: 8 additions & 0 deletions hack/ci/custom-linters/go.sum
Original file line number Diff line number Diff line change
@@ -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=
Original file line number Diff line number Diff line change
@@ -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
}

0 comments on commit 1dd674a

Please sign in to comment.