introduce stricter linter configuration #1169 (#1170)

Signed-off-by: Angelo De Caro <adc@zurich.ibm.com>

Author: adecaro

.github/workflows/golangci-lint.yml

@@ -25,7 +25,7 @@ jobs:
           go-version-file: "go.mod"
           cache: false
       - name: golangci-lint
-        uses: golangci/golangci-lint-action@v7
+        uses: golangci/golangci-lint-action@v8
         with:
-          version: v2.0
+          version: v2.1
           args: --timeout=30m

.golangci.yml

@@ -0,0 +1,231 @@
+version: "2"
+linters:
+  enable:
+#    - containedctx
+#    - contextcheck
+#    - depguard
+#    - dupl
+#    - errname
+    - errorlint
+#    - fatcontext
+#    - forcetypeassert
+    - ginkgolinter
+#    - gocognit
+#    - goconst
+#    - gocritic
+#    - godot
+#    - gosec
+#    - intrange
+#    - ireturn
+#    - lll
+#    - maintidx
+    - mirror
+    - misspell
+    - nilerr
+    - nilnesserr
+    - nolintlint
+#    - paralleltest
+#    - prealloc
+#    - promlinter
+#    - revive
+#    - rowserrcheck
+#    - sqlclosecheck
+#    - testifylint
+    - thelper
+    - unconvert
+#    - unparam
+    - usetesting
+    - wastedassign
+    - whitespace
+    - govet
+#    - staticcheck
+    - canonicalheader
+#    - copyloopvar
+    - dupword
+#    - err113
+    - exptostd
+    - goheader
+    - importas
+#    - intrange
+    - nakedret
+    - sloglint
+    - usestdlibvars
+    # wrapcheck is not working as expected. It is categorizing internal packages as external.
+    #- wrapcheck
+  settings:
+    depguard:
+      rules:
+        main:
+          deny:
+            - pkg: github.com/pkg/errors
+              desc: github.com/pkg/errors is no longer maintained
+    errcheck:
+      # Report about not checking of errors in type assertions: `a := b.(MyStruct)`.
+      # Such cases aren't reported by default.
+#      Default: false
+      check-type-assertions: false
+    errorlint:
+      # Check whether fmt.Errorf uses the %w verb for formatting errors.
+      # See the https://github.com/polyfloyd/go-errorlint for caveats.
+      # Default: true
+      errorf: true
+      # Check for plain type assertions and type switches.
+      # Default: true
+      asserts: false
+      # Check for plain error comparisons.
+      # Default: true
+      comparison: true
+    fatcontext:
+      check-struct-pointers: true
+    gocognit:
+      min-complexity: 15
+    gosec:
+      excludes:
+        - G204
+        - G404
+        - G306
+    govet:
+      disable:
+        - fieldalignment
+      # Enable all analyzers.
+      # Default: false
+      enable-all: false
+    iface:
+      enable:
+        - identical # Identifies interfaces in the same package that have identical method sets.
+        - unused # Identifies interfaces that are not used anywhere in the same package where the interface is defined.
+        - opaque # Identifies functions that return interfaces, but the actual returned value is always a single concrete implementation.
+    ireturn:
+      # By default, it allows using errors, empty interfaces, anonymous interfaces,
+      # and interfaces provided by the standard library.
+      allow:
+        - error
+        - empty
+        - anon
+        - stdlib
+        - generic
+        - (or|er)$
+        - T
+        - github.com/prometheus/client_golang/prometheus.Counter
+        - github.com/prometheus/client_golang/prometheus.Gauge
+        - github.com/prometheus/client_golang/prometheus.Histogram
+    lll:
+      # Max line length, lines longer will be reported.
+      # '\t' is counted as 1 character by default, and can be changed with the tab-width option.
+      # Default: 120.
+      line-length: 240
+    maintidx:
+      under: 20
+    nolintlint:
+      require-specific: true
+    revive:
+      enable-all-rules: true
+      rules:
+        - name: argument-limit
+          arguments:
+            - 5
+        - name: line-length-limit
+          arguments:
+            - 240
+        - name: file-header
+          disabled: true
+        - name: package-comments
+          disabled: true
+        - name: max-public-structs
+          disabled: true
+        - name: banned-characters
+          disabled: true
+        - name: cognitive-complexity
+          disabled: true
+        - name: cyclomatic
+          disabled: true
+        - name: function-length
+          disabled: true
+        - name: function-result-limit
+          arguments:
+            - 3
+        - name: add-constant
+          disabled: true
+        - name: unhandled-error
+          arguments:
+            - fmt.Printf
+            - fmt.Println
+        - name: confusing-naming
+          disabled: true
+        - name: comment-spacings
+          arguments:
+            - 'nolint:'
+    rowserrcheck:
+      packages:
+        - github.com/jackc/pgx/v5
+        - github.com/jackc/pgx/v4
+    wrapcheck:
+      # An array of strings specifying additional substrings of signatures to ignore.
+      # Unlike 'ignoreSigs', this option extends the default set (or the set specified in 'ignoreSigs') without replacing it entirely.
+      # This allows you to add specific signatures to the ignore list
+      # while retaining the defaults or any items in 'ignoreSigs'.
+      # Default: []
+      extra-ignore-sigs:
+        - .CustomError(
+        - .SpecificWrap(
+
+      # An array of strings that specify substrings of signatures to ignore.
+      # If this set, it will override the default set of ignored signatures.
+      # See https://github.com/tomarrell/wrapcheck#configuration for more information.
+      # Default: [".Errorf(", "errors.New(", "errors.Unwrap(", "errors.Join(", ".Wrap(", ".Wrapf(", ".WithMessage(", ".WithMessagef(", ".WithStack("]
+      ignore-sigs:
+        - errors.New(
+        - errors.Newf(
+        - errors.Unwrap(
+        - errors.Join(
+        - .Wrap(
+        - .Wrapf(
+        - status.Error(
+        - .Wait() # The error reported by errorgroup.Wait() (external) actually
+                  # originates from the internal code.
+      # An array of strings that specify regular expressions of signatures to ignore.
+      # Default: []
+      ignore-sig-regexps:
+        - \.New.*Error\(
+      # An array of strings that specify globs of packages to ignore.
+      # Default: []
+      ignore-package-globs:
+        - github.ibm.com/decentralized-trust-research/scalable-committer/utils/grpcerror
+      # An array of strings that specify regular expressions of interfaces to ignore.
+      # Default: []
+      # ignoreInterfaceRegexps:
+      #   - ^(?i)c(?-i)ach(ing|e)
+  exclusions:
+      # Mode of the generated files analysis.
+      #
+      # - `strict`: sources are excluded by strictly following the Go generated file convention.
+      #    Source files that have lines matching only the following regular expression will be excluded: `^// Code generated .* DO NOT EDIT\.$`
+      #    This line must appear before the first non-comment, non-blank text in the file.
+      #    https://go.dev/s/generatedcode
+      # - `lax`: sources are excluded if they contain lines like `autogenerated file`, `code generated`, `do not edit`, etc.
+      # - `disable`: disable the generated files exclusion.
+      #
+      # Default: lax
+      generated: lax
+      # Log a warning if an exclusion rule is unused.
+      # Default: false
+      warn-unused: true
+      # Predefined exclusion rules.
+      # Default: []
+      presets:
+        - common-false-positives
+#formatters:
+#  enable:
+#    - gofumpt
+#    - goimports
+#  settings:
+#    gofumpt:
+#      extra-rules: true
+#    goimports:
+#      # A comma-separated list of prefixes, which, if set, checks import paths
+#      # with the given prefixes are grouped after 3rd-party packages.
+#      # Default: ""
+#      local-prefixes:
+#        - github.ibm.com/decentralized-trust-research/idap
+#  exclusions:
+#    generated: lax

Makefile

@@ -145,6 +145,11 @@ lint:
 	@echo "Running Go Linters..."
 	golangci-lint run --color=always --timeout=4m
 
+.PHONY: lint-auto-fix
+lint-auto-fix:
+	@echo "Running Go Linters with auto-fix..."
+	golangci-lint run --color=always --timeout=4m --fix
+
 .PHONY: install-linter-tool
 install-linter-tool:
 	@echo "Installing golangci Linter"

checks.mk

@@ -41,7 +41,7 @@ govet:
 misspell:
 	@echo Running misspell
 	@{ \
-	OUTPUT="$$(misspell . || true)"; \
+	OUTPUT="$$(find . -type f | grep -v '.golangci.yml' | xargs misspell || true)"; \
 	if [ -n "$$OUTPUT" ]; then \
 		echo "The following files are have spelling errors:"; \
 		echo "$$OUTPUT"; \

cmd/tokengen/main_test.go

@@ -37,8 +37,7 @@ func TestGenFullSuccess(t *testing.T) {
 	gt.Expect(err).NotTo(HaveOccurred())
 	defer gexec.CleanupBuildArtifacts()
 
-	tempOutput, err := os.MkdirTemp("", "tokengen-test")
-	gt.Expect(err).NotTo(HaveOccurred())
+	tempOutput := t.TempDir()
 	defer utils.IgnoreErrorWithOneArg(os.RemoveAll, tempOutput)
 
 	testGenRun(
@@ -73,8 +72,7 @@ func TestFullUpdate(t *testing.T) {
 	gt.Expect(err).NotTo(HaveOccurred())
 	defer gexec.CleanupBuildArtifacts()
 
-	tempOutput, err := os.MkdirTemp("", "tokengen-update-test")
-	gt.Expect(err).NotTo(HaveOccurred())
+	tempOutput := t.TempDir()
 	defer utils.IgnoreErrorWithOneArg(os.RemoveAll, tempOutput)
 
 	// Switching the auditor and issuer certs to test the update function
@@ -110,8 +108,7 @@ func TestPartialUpdate(t *testing.T) {
 	gt.Expect(err).NotTo(HaveOccurred())
 	defer gexec.CleanupBuildArtifacts()
 
-	tempOutput, err := os.MkdirTemp("", "tokengen-update-test")
-	gt.Expect(err).NotTo(HaveOccurred())
+	tempOutput := t.TempDir()
 	defer utils.IgnoreErrorWithOneArg(os.RemoveAll, tempOutput)
 
 	// Only changing the issuer cert to also use the auditor cert.

integration/nwo/token/common/ca.go

@@ -215,7 +215,7 @@ func (i *IdemixCASupport) Gen(owner string) (res token.IdentityConfiguration, er
 	cmd := common.NewCommand(fabricCAClientExePath, registerCommand)
 	sess, err := i.StartSession(cmd, registerCommand.SessionName())
 	if err != nil {
-		return
+		return res, err
 	}
 	gomega.Eventually(sess, i.EventuallyTimeout).Should(gexec.Exit(0))
 
@@ -230,13 +230,13 @@ func (i *IdemixCASupport) Gen(owner string) (res token.IdentityConfiguration, er
 	cmd = common.NewCommand(fabricCAClientExePath, enrollCommand)
 	sess, err = i.StartSession(cmd, enrollCommand.SessionName())
 	if err != nil {
-		return
+		return res, err
 	}
 	gomega.Eventually(sess, i.EventuallyTimeout).Should(gexec.Exit(0))
 
 	res.ID = owner
 	res.URL = userOutput
-	return
+	return res, err
 }
 
 func (i *IdemixCASupport) GenerateConfiguration() error {

integration/nwo/token/generators/fabtoken/fabtoken.go

@@ -231,7 +231,6 @@ func (d *CryptoMaterialGenerator) Generate(tms *topology.TMS, n *node.Node, wall
 		identities = append(identities, id)
 	}
 	return identities
-
 }
 
 func (d *CryptoMaterialGenerator) GenerateCryptoConfig(output string, layout *Layout) {

integration/nwo/txgen/model/distribution.go

@@ -25,7 +25,7 @@ func (d *Distribution) GetAmounts(total api.Amount) ([]api.Amount, api.Error) {
 	}
 
 	msg := fmt.Sprintf("Unknown type in distribution: %s", *d)
-	//logging.Logger.Errorf(msg)
+	// logging.Logger.Errorf(msg)
 
 	return nil, api.NewBadRequestError(nil, msg)
 }
@@ -92,7 +92,7 @@ func (d *Distribution) convertToIntegers(input []string) ([]api.Amount, api.Erro
 		num, err := strconv.Atoi(v)
 		if err != nil {
 			msg := fmt.Sprintf("Can't convert %s to intereger", v)
-			//logging.Logger.Errorf(msg)
+			// logging.Logger.Errorf(msg)
 			return nil, api.NewBadRequestError(nil, msg)
 		}
 		ints[i] = api.Amount(num)

integration/nwo/txgen/service/config/reader.go

@@ -50,7 +50,7 @@ func Load() (*model.Configuration, api.Error) {
 	err := viper.Unmarshal(&config)
 	if err != nil {
 		appErr := api.NewInternalServerError(
-			fmt.Errorf("cannot unmarshal the configuration file: %s", err),
+			fmt.Errorf("cannot unmarshal the configuration file: %w", err),
 			"Internal Server Error",
 		)
 		return nil, appErr
@@ -69,7 +69,7 @@ func loadConfig(configFile string) api.Error {
 	err := viper.MergeInConfig()
 	if err != nil {
 		return api.NewInternalServerError(
-			fmt.Errorf("couldn't read the config file '%s': %s", configFile, err),
+			fmt.Errorf("couldn't read the config file '%s': %w", configFile, err),
 			"Internal Server Error",
 		)
 	}
@@ -82,7 +82,7 @@ func loadDefaultConfig() api.Error {
 	configuration, err := embeddedFiles.ReadFile("config.yaml")
 	if err != nil {
 		return api.NewInternalServerError(
-			fmt.Errorf("couldn't find the default config file 'config.yaml': %s", err),
+			fmt.Errorf("couldn't find the default config file 'config.yaml': %w", err),
 			"Internal Server Error",
 		)
 	}
@@ -92,7 +92,7 @@ func loadDefaultConfig() api.Error {
 	err = viper.ReadConfig(bytes.NewReader(configuration))
 	if err != nil {
 		return api.NewInternalServerError(
-			fmt.Errorf("couldn't read the default config file 'config.yaml': %s", err),
+			fmt.Errorf("couldn't read the default config file 'config.yaml': %w", err),
 			"Internal Server Error",
 		)
 	}

integration/nwo/txgen/service/metrics/provider.go

@@ -57,7 +57,6 @@ func (h *histogram) Observe(v float64) {
 		h.sum += v
 		h.n++
 	})
-
 }
 func (h *histogram) Min() float64 { return h.min }
 func (h *histogram) Max() float64 { return h.max }