WIP: enable linting and bingo management of the lint tool

.bingo/Variables.mk

@@ -29,6 +29,12 @@ $(GINKGO): $(BINGO_DIR)/ginkgo.mod
 	@echo "(re)installing $(GOBIN)/ginkgo-v2.20.2"
 	@cd $(BINGO_DIR) && GOWORK=off $(GO) build -mod=mod -modfile=ginkgo.mod -o=$(GOBIN)/ginkgo-v2.20.2 "github.com/onsi/ginkgo/v2/ginkgo"
 
+GOLANGCI_LINT := $(GOBIN)/golangci-lint-v1.61.0
+$(GOLANGCI_LINT): $(BINGO_DIR)/golangci-lint.mod
+	@# Install binary/ries using Go 1.14+ build command. This is using bwplotka/bingo-controlled, separate go module with pinned dependencies.
+	@echo "(re)installing $(GOBIN)/golangci-lint-v1.61.0"
+	@cd $(BINGO_DIR) && GOWORK=off $(GO) build -mod=mod -modfile=golangci-lint.mod -o=$(GOBIN)/golangci-lint-v1.61.0 "github.com/golangci/golangci-lint/cmd/golangci-lint"
+
 GORELEASER := $(GOBIN)/goreleaser-v1.26.2
 $(GORELEASER): $(BINGO_DIR)/goreleaser.mod
 	@# Install binary/ries using Go 1.14+ build command. This is using bwplotka/bingo-controlled, separate go module with pinned dependencies.

.bingo/golangci-lint.mod

@@ -0,0 +1,5 @@
+module _ // Auto generated by https://github.com/bwplotka/bingo. DO NOT EDIT
+
+go 1.22.5
+
+require github.com/golangci/golangci-lint v1.61.0 // cmd/golangci-lint

.bingo/variables.env

@@ -12,6 +12,8 @@ BINGO="${GOBIN}/bingo-v0.9.0"
 
 GINKGO="${GOBIN}/ginkgo-v2.20.2"
 
+GOLANGCI_LINT="${GOBIN}/golangci-lint-v1.61.0"
+
 GORELEASER="${GOBIN}/goreleaser-v1.26.2"
 
 PROTOC_GEN_GO_GRPC="${GOBIN}/protoc-gen-go-grpc-v1.3.0"

.golangci.yaml

@@ -0,0 +1,73 @@
+########
+# NOTE
+#
+#  This file is duplicated in the following repos:
+#    - operator-framework/kubectl-operator
+#    - operator-framework/catalogd
+#    - operator-framework/operator-controller
+#    - operator-framework/operator-registry
+#
+#  If you are making a change, please make it in ALL
+#  of the above repositories!
+#
+#  TODO: Find a way to have a shared golangci config.
+########
+
+run:
+  # Default timeout is 1m, up to give more room
+  timeout: 4m
+
+linters:
+  enable:
+  - asciicheck
+  - bodyclose
+  - errorlint
+  - gci
+  - gofmt
+  - gosec
+  - importas
+  - misspell
+  - nestif
+  - nonamedreturns
+  - prealloc
+  - stylecheck
+  - testifylint
+  - tparallel
+  - unconvert
+  - unparam
+  - unused
+  - whitespace
+
+linters-settings:
+  gci:
+    sections:
+      - standard
+      - dot
+      - default
+      - prefix(github.com/operator-framework)
+      - localmodule
+    custom-order: true
+
+  errorlint:
+    errorf: false
+
+  importas:
+    alias:
+    - pkg: k8s.io/apimachinery/pkg/apis/meta/v1
+      alias: metav1
+    - pkg: k8s.io/apimachinery/pkg/api/errors
+      alias: apierrors
+    - pkg: k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1
+      alias: apiextensionsv1
+    - pkg: k8s.io/apimachinery/pkg/util/runtime
+      alias: utilruntime
+    - pkg: "^k8s\\.io/api/([^/]+)/(v[^/]+)$"
+      alias: $1$2
+    - pkg: sigs.k8s.io/controller-runtime
+      alias: ctrl
+    - pkg: github.com/blang/semver/v4
+      alias: bsemver
+
+output:
+  formats:
+    - format: tab

Makefile

@@ -102,8 +102,14 @@ image-upstream:
 	docker build -f upstream-example.Dockerfile .
 
 .PHONY: lint
-lint:
-	find . -type f -name '*.go' ! -name '*.pb.go' -print0 | xargs -0 goimports -w
+#lint:
+#	find . -type f -name '*.go' ! -name '*.pb.go' -print0 | xargs -0 goimports -w
+lint: $(GOLANGCI_LINT)
+	$(GOLANGCI_LINT) run $(GOLANGCI_LINT_ARGS)
+
+.PHONY: fix-lint
+fix-lint: $(GOLANGCI_LINT)
+	$(GOLANGCI_LINT) run --fix $(GOLANGCI_LINT_ARGS)
 
 .PHONY: bingo-upgrade
 bingo-upgrade: $(BINGO) #EXHELP Upgrade tools

alpha/action/migrate_test.go

@@ -125,7 +125,7 @@ func TestMigrate(t *testing.T) {
 				return
 			}
 			actualFS := os.DirFS(s.migrate.OutputDir)
-			fs.WalkDir(actualFS, ".", func(path string, d fs.DirEntry, err error) error {
+			_ = fs.WalkDir(actualFS, ".", func(path string, d fs.DirEntry, err error) error {
 				require.NoError(t, err)
 				if d.IsDir() {
 					return nil
@@ -291,7 +291,6 @@ relatedImages:
   name: operator
 schema: olm.bundle
 `
-
 }
 
 func migrateBarCatalogSqlite() string {

alpha/action/migrations/migrations_test.go

@@ -8,10 +8,12 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/stretchr/testify/require"
+
 	"github.com/operator-framework/api/pkg/operators/v1alpha1"
+
 	"github.com/operator-framework/operator-registry/alpha/declcfg"
 	"github.com/operator-framework/operator-registry/alpha/property"
-	"github.com/stretchr/testify/require"
 )
 
 func TestMigrations(t *testing.T) {
@@ -57,7 +59,7 @@ func TestMigrations(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			var config declcfg.DeclarativeConfig = unmigratedCatalogFBC()
+			var config = unmigratedCatalogFBC()
 
 			for _, m := range test.migrators.Migrations {
 				err := m.Migrate(&config)

alpha/action/render_test.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"embed"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"io/fs"
 	"os"
@@ -13,7 +12,6 @@ import (
 	"testing/fstest"
 	"text/template"
 
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"k8s.io/apimachinery/pkg/util/yaml"
 
@@ -86,11 +84,11 @@ func TestRender(t *testing.T) {
 		image.SimpleReference("test.registry/foo-operator/foo-bundle:v0.1.0"): "testdata/foo-bundle-v0.1.0",
 		image.SimpleReference("test.registry/foo-operator/foo-bundle:v0.2.0"): "testdata/foo-bundle-v0.2.0",
 	}
-	assert.NoError(t, generateSqliteFile(dbFile, imageMap))
+	require.NoError(t, generateSqliteFile(dbFile, imageMap))
 	testMigrations := migrations.Migrations{
 		Migrations: []migrations.Migration{
 			fauxMigration{"faux-migration", "my help text", func(d *declcfg.DeclarativeConfig) error {
-				for i, _ := range d.Bundles {
+				for i := range d.Bundles {
 					d.Bundles[i].Name = fmt.Sprintf("%s-MIGRATED", d.Bundles[i].Name)
 				}
 				return nil
@@ -1235,7 +1233,7 @@ func TestAllowRefMask(t *testing.T) {
 		image.SimpleReference("test.registry/foo-operator/foo-bundle:v0.1.0"): "testdata/foo-bundle-v0.1.0",
 		image.SimpleReference("test.registry/foo-operator/foo-bundle:v0.2.0"): "testdata/foo-bundle-v0.2.0",
 	}
-	assert.NoError(t, generateSqliteFile(dbFile, imageMap))
+	require.NoError(t, generateSqliteFile(dbFile, imageMap))
 
 	specs := []spec{
 		{
@@ -1365,18 +1363,17 @@ func TestAllowRefMask(t *testing.T) {
 	for _, s := range specs {
 		t.Run(s.name, func(t *testing.T) {
 			_, err := s.render.Run(context.Background())
-			require.True(t, errors.Is(err, s.expectErr), "expected error %#v to be %#v", err, s.expectErr)
+			require.ErrorIs(t, err, s.expectErr, "expected error %#v to be %#v", err, s.expectErr)
 		})
 	}
 }
 
 func TestAllowRefMaskAllowed(t *testing.T) {
 	type spec struct {
-		name   string
-		mask   action.RefType
-		pass   []action.RefType
-		fail   []action.RefType
-		expect bool
+		name string
+		mask action.RefType
+		pass []action.RefType
+		fail []action.RefType
 	}
 
 	specs := []spec{

alpha/declcfg/declcfg.go

@@ -6,13 +6,12 @@ import (
 	"errors"
 	"fmt"
 
-	prettyunmarshaler "github.com/operator-framework/operator-registry/pkg/prettyunmarshaler"
-
 	"golang.org/x/text/cases"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/sets"
 
 	"github.com/operator-framework/operator-registry/alpha/property"
+	prettyunmarshaler "github.com/operator-framework/operator-registry/pkg/prettyunmarshaler"
 )
 
 const (

alpha/declcfg/declcfg_to_model.go

@@ -178,7 +178,6 @@ func ConvertToModel(cfg DeclarativeConfig) (model.Model, error) {
 	deprecationsByPackage := sets.New[string]()
 
 	for i, deprecation := range cfg.Deprecations {
-
 		// no need to validate schema, since it could not be unmarshaled if missing/invalid
 
 		if deprecation.Package == "" {

alpha/declcfg/declcfg_to_model_test.go

@@ -484,7 +484,6 @@ func TestConvertToModelBundle(t *testing.T) {
 	assert.Len(t, b.PropertiesP.BundleObjects, 2)
 	assert.Len(t, b.PropertiesP.Packages, 1)
 	assert.Equal(t, semver.MustParse("0.1.0"), b.Version)
-
 }
 
 func TestConvertToModelRoundtrip(t *testing.T) {
@@ -499,7 +498,7 @@ func TestConvertToModelRoundtrip(t *testing.T) {
 
 	assert.Equal(t, expected.Packages, actual.Packages)
 	assert.Equal(t, expected.Bundles, actual.Bundles)
-	assert.Len(t, actual.Others, 0, "expected unrecognized schemas not to make the roundtrip")
+	assert.Empty(t, actual.Others, "expected unrecognized schemas not to make the roundtrip")
 }
 
 func hasError(expectedError string) require.ErrorAssertionFunc {

alpha/declcfg/helpers_test.go

@@ -146,15 +146,15 @@ func withNoBundleData() func(*Bundle) {
 }
 
 func newTestBundle(packageName, version string, opts ...bundleOpt) Bundle {
-	csvJson := fmt.Sprintf(`{"kind": "ClusterServiceVersion", "apiVersion": "operators.coreos.com/v1alpha1", "metadata":{"name":%q}}`, testBundleName(packageName, version))
+	csvJSON := fmt.Sprintf(`{"kind": "ClusterServiceVersion", "apiVersion": "operators.coreos.com/v1alpha1", "metadata":{"name":%q}}`, testBundleName(packageName, version))
 	b := Bundle{
 		Schema:  SchemaBundle,
 		Name:    testBundleName(packageName, version),
 		Package: packageName,
 		Image:   testBundleImage(packageName, version),
 		Properties: []property.Property{
 			property.MustBuildPackage(packageName, version),
-			property.MustBuildBundleObject([]byte(csvJson)),
+			property.MustBuildBundleObject([]byte(csvJSON)),
 			property.MustBuildBundleObject([]byte(`{"kind": "CustomResourceDefinition", "apiVersion": "apiextensions.k8s.io/v1"}`)),
 		},
 		RelatedImages: []RelatedImage{
@@ -163,9 +163,9 @@ func newTestBundle(packageName, version string, opts ...bundleOpt) Bundle {
 				Image: testBundleImage(packageName, version),
 			},
 		},
-		CsvJSON: csvJson,
+		CsvJSON: csvJSON,
 		Objects: []string{
-			csvJson,
+			csvJSON,
 			`{"kind": "CustomResourceDefinition", "apiVersion": "apiextensions.k8s.io/v1"}`,
 		},
 	}

alpha/declcfg/load_benchmark_test.go

@@ -2,17 +2,19 @@ package declcfg_test
 
 import (
 	"context"
+	"crypto/rand"
 	"encoding/base64"
 	"fmt"
-	"math/rand"
+	"math/big"
 	"os"
 	"runtime"
 	"testing"
 
 	"github.com/blang/semver/v4"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
 	"github.com/operator-framework/api/pkg/lib/version"
 	"github.com/operator-framework/api/pkg/operators/v1alpha1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"github.com/operator-framework/operator-registry/alpha/declcfg"
 	"github.com/operator-framework/operator-registry/alpha/property"
@@ -64,7 +66,11 @@ func generateFBC(b *testing.B, numPackages, numChannels, numBundles int) *declcf
 		})
 	}
 	for i := 0; i < numChannels; i++ {
-		pkgName := fbc.Packages[rand.Intn(numPackages)].Name
+		r, err := rand.Int(rand.Reader, big.NewInt(int64(numPackages)))
+		if err != nil {
+			b.Error(err)
+		}
+		pkgName := fbc.Packages[r.Int64()].Name
 		channelName := fmt.Sprintf("channel-%d", i)
 		fbc.Channels = append(fbc.Channels, declcfg.Channel{
 			Schema:  declcfg.SchemaChannel,
@@ -73,7 +79,11 @@ func generateFBC(b *testing.B, numPackages, numChannels, numBundles int) *declcf
 		})
 	}
 	for i := 0; i < numBundles; i++ {
-		pkgName := fbc.Packages[rand.Intn(numPackages)].Name
+		r, err := rand.Int(rand.Reader, big.NewInt(int64(numPackages)))
+		if err != nil {
+			b.Error(err)
+		}
+		pkgName := fbc.Packages[r.Int64()].Name
 		bundleName := fmt.Sprintf("bundle-%d", i)
 		version := fmt.Sprintf("0.%d.0", i)
 		bundle := declcfg.Bundle{
@@ -93,7 +103,11 @@ func generateFBC(b *testing.B, numPackages, numChannels, numBundles int) *declcf
 		bundle.Properties = append(bundle.Properties, property.MustBuildCSVMetadata(csv))
 		fbc.Bundles = append(fbc.Bundles, bundle)
 
-		chIdx := rand.Intn(numChannels)
+		p, err := rand.Int(rand.Reader, big.NewInt(int64(numChannels)))
+		if err != nil {
+			b.Error(err)
+		}
+		chIdx := p.Int64()
 		ch := fbc.Channels[chIdx]
 		replaces := ""
 		if len(ch.Entries) > 0 {

alpha/declcfg/load_test.go

@@ -989,9 +989,9 @@ func TestLoadFile(t *testing.T) {
 			path:      "unrecognized-schema.json",
 			assertion: require.NoError,
 			expect: func(t *testing.T, d *DeclarativeConfig) {
-				require.Equal(t, 1, len(d.Packages))
-				require.Equal(t, 1, len(d.Bundles))
-				require.Equal(t, 1, len(d.Others))
+				require.Len(t, d.Packages, 1)
+				require.Len(t, d.Bundles, 1)
+				require.Len(t, d.Others, 1)
 			},
 		},
 		{
@@ -1000,9 +1000,9 @@ func TestLoadFile(t *testing.T) {
 			path:      "etcd.yaml",
 			assertion: require.NoError,
 			expect: func(t *testing.T, d *DeclarativeConfig) {
-				require.Equal(t, 1, len(d.Packages))
-				require.Equal(t, 6, len(d.Bundles))
-				require.Equal(t, 0, len(d.Others))
+				require.Len(t, d.Packages, 1)
+				require.Len(t, d.Bundles, 6)
+				require.Empty(t, d.Others)
 			},
 		},
 		{
@@ -1011,10 +1011,10 @@ func TestLoadFile(t *testing.T) {
 			path:      "deprecations.yaml",
 			assertion: require.NoError,
 			expect: func(t *testing.T, d *DeclarativeConfig) {
-				require.Equal(t, 0, len(d.Packages))
-				require.Equal(t, 0, len(d.Bundles))
-				require.Equal(t, 0, len(d.Others))
-				require.Equal(t, 1, len(d.Deprecations))
+				require.Empty(t, d.Packages)
+				require.Empty(t, d.Bundles)
+				require.Empty(t, d.Others)
+				require.Len(t, d.Deprecations, 1)
 			},
 		},
 	}