Use golangci-lint

Signed-off-by: Mark S. Lewis <Mark.S.Lewis@outlook.com>

.github/workflows/build.yml

@@ -25,24 +25,19 @@ jobs:
     - uses: actions/checkout@v4
     - name: install Tools
       run: |
-        go install honnef.co/go/tools/cmd/staticcheck@latest
-        go install github.com/securego/gosec/v2/cmd/gosec@latest
-        go install github.com/cucumber/godog/cmd/godog@latest
-        go install golang.org/x/tools/cmd/goimports@latest
         npm install -g license-check-and-add@4.0.5
 
-    - name: Vet and lint
-      run: ci/scripts/lint.sh
+    - name: Lint
+      run: make lint
 
     - name: Check Licenses
       run: license-check-and-add check -f ci/license-config.json
 
     - name: Run tests (excluding fv)
-      run: go test -race $(go list ./... | grep -v functionaltests)
+      run: make unit-test
 
     - name: Run functional tests
-      working-directory: internal/functionaltests
-      run: godog run features/*
+      run: make functional-test
 
     - name: Check tutorial contents
       run: ci/scripts/tutorial-checks.sh

.github/workflows/vulnerability-scan.yml

@@ -13,9 +13,7 @@ jobs:
       - name: Set up Go
         uses: actions/setup-go@v5
         with:
-          go-version: "1.22"
+          go-version: stable
           check-latest: true
-      - name: Install govulncheck
-        run: go install golang.org/x/vuln/cmd/govulncheck@latest
       - name: Scan
-        run: govulncheck ./...
+        run: make scan

.golangci.yml

@@ -0,0 +1,23 @@
+# See https://golangci-lint.run/usage/configuration/
+
+run:
+  timeout: 5m
+
+linters:
+  disable-all: true
+  enable:
+    - errcheck
+    - gocyclo
+    - gofmt
+    - goimports
+    - gosec
+    - gosimple
+    - govet
+    - ineffassign
+    - misspell
+    - typecheck
+    - unused
+
+linters-settings:
+  gocyclo:
+    min-complexity: 18

Makefile

@@ -0,0 +1,39 @@
+# Copyright the Hyperledger Fabric contributors. All rights reserved.
+# SPDX-License-Identifier: Apache-2.0
+
+base_dir := $(patsubst %/,%,$(dir $(realpath $(lastword $(MAKEFILE_LIST)))))
+functional_dir := $(base_dir)/internal/functionaltests
+go_bin_dir := $(shell go env GOPATH)/bin
+
+.PHONY: lint
+lint: staticcheck golangci-lint
+
+.PHONY: staticcheck
+staticcheck:
+	go install honnef.co/go/tools/cmd/staticcheck@latest
+	staticcheck -f stylish './...'
+
+.PHONY: install-golangci-lint
+install-golangci-lint:
+	curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b '$(go_bin_dir)'
+
+$(go_bin_dir)/golangci-lint:
+	$(MAKE) install-golangci-lint
+
+.PHONY: golangci-lint
+golangci-lint: $(go_bin_dir)/golangci-lint
+	golangci-lint run
+
+.PHONY: unit-test
+unit-test:
+	go test -race $$(go list '$(base_dir)/...' | grep -v functionaltests)
+
+.PHONY: functional-test
+functional-test:
+	go install github.com/cucumber/godog/cmd/godog@v0.12
+	cd '$(functional_dir)' && godog run features/*
+
+.PHONY: scan
+scan:
+	go install golang.org/x/vuln/cmd/govulncheck@latest
+	govulncheck '$(base_dir)/...'

contractapi/contract_chaincode.go

@@ -6,7 +6,6 @@ package contractapi
 import (
 	"encoding/json"
 	"fmt"
-	"io/ioutil"
 	"log"
 	"os"
 	"reflect"
@@ -87,11 +86,11 @@ func NewChaincode(contracts ...ContractInterface) (*ContractChaincode, error) {
 	sysC := new(SystemContract)
 	sysC.Name = SystemContractName
 
-	cc.addContract(sysC, ciMethods) // should never error as system contract is good
-
-	err := cc.augmentMetadata()
+	if err := cc.addContract(sysC, ciMethods); err != nil {
+		return nil, err
+	}
 
-	if err != nil {
+	if err := cc.augmentMetadata(); err != nil {
 		return nil, err
 	}
 
@@ -219,9 +218,9 @@ func (cc *ContractChaincode) Invoke(stub shim.ChaincodeStubInterface) peer.Respo
 	} else {
 		var transactionSchema *metadata.TransactionMetadata
 
-		for _, v := range cc.metadata.Contracts[ns].Transactions {
+		for i, v := range cc.metadata.Contracts[ns].Transactions {
 			if v.Name == fn {
-				transactionSchema = &v
+				transactionSchema = &cc.metadata.Contracts[ns].Transactions[i]
 				break
 			}
 		}
@@ -254,7 +253,7 @@ func (cc *ContractChaincode) addContract(contract ContractInterface, excludeFunc
 	}
 
 	if _, ok := cc.contracts[ns]; ok {
-		return fmt.Errorf("Multiple contracts being merged into chaincode with name %s", ns)
+		return fmt.Errorf("multiple contracts being merged into chaincode with name %s", ns)
 	}
 
 	ccn := contractChaincodeContract{}
@@ -335,7 +334,7 @@ func (cc *ContractChaincode) addContract(contract ContractInterface, excludeFunc
 	}
 
 	if len(ccn.functions) == 0 {
-		return fmt.Errorf("Contracts are required to have at least 1 (non-ignored) public method. Contract %s has none. Method names that have been ignored: %s", ns, utils.SliceAsCommaSentence(excludeFuncs))
+		return fmt.Errorf("contracts are required to have at least 1 (non-ignored) public method. Contract %s has none. Method names that have been ignored: %s", ns, utils.SliceAsCommaSentence(excludeFuncs))
 	}
 
 	cc.contracts[ns] = ccn
@@ -390,27 +389,24 @@ func (cc *ContractChaincode) reflectMetadata() metadata.ContractChaincodeMetadat
 func (cc *ContractChaincode) augmentMetadata() error {
 	fileMetadata, err := metadata.ReadMetadataFile()
 
-	if err != nil && !strings.Contains(err.Error(), "Failed to read metadata from file") {
+	if err != nil && !strings.Contains(err.Error(), "failed to read metadata from file") {
 		return err
 	}
 
 	reflectedMetadata := cc.reflectMetadata()
 
 	fileMetadata.Append(reflectedMetadata)
 	err = fileMetadata.CompileSchemas()
-
 	if err != nil {
 		return err
 	}
 
 	err = metadata.ValidateAgainstSchema(fileMetadata)
-
 	if err != nil {
 		return err
 	}
 
 	cc.metadata = fileMetadata
-
 	return nil
 }
 
@@ -441,7 +437,7 @@ func loadChaincodeServerConfig() (*shim.ChaincodeServer, error) {
 
 	tlsProps, err := loadTLSProperties()
 	if err != nil {
-		log.Panicf("Error creating getting TLS properties: %v", err)
+		log.Panicf("error creating getting TLS properties: %v", err)
 	}
 
 	server := &shim.ChaincodeServer{
@@ -466,18 +462,18 @@ func loadTLSProperties() (*shim.TLSProperties, error) {
 	var keyBytes, certBytes, rootBytes []byte
 	var err error
 
-	keyBytes, err = ioutil.ReadFile(key)
+	keyBytes, err = os.ReadFile(key)
 	if err != nil {
 		return nil, fmt.Errorf("error while reading the crypto file: %s", err)
 	}
 
-	certBytes, err = ioutil.ReadFile(cert)
+	certBytes, err = os.ReadFile(cert)
 	if err != nil {
 		return nil, fmt.Errorf("error while reading the crypto file: %s", err)
 	}
 
 	if root != "" {
-		rootBytes, err = ioutil.ReadFile(root)
+		rootBytes, err = os.ReadFile(root)
 		if err != nil {
 			return nil, fmt.Errorf("error while reading the crypto file: %s", err)
 		}

contractapi/contract_chaincode_test.go

@@ -12,6 +12,7 @@ import (
 	"testing"
 
 	"github.com/hyperledger/fabric-chaincode-go/shim"
+	//lint:ignore SA1019 TODO: needs to be removed
 	"github.com/hyperledger/fabric-chaincode-go/shimtest"
 	"github.com/hyperledger/fabric-contract-api-go/internal"
 	"github.com/hyperledger/fabric-contract-api-go/internal/utils"
@@ -32,6 +33,7 @@ const standardTxID = "1234567890"
 
 type simpleStruct struct {
 	Prop1 string `json:"prop1"`
+	//lint:ignore U1000 unused
 	prop2 string
 }
 
@@ -51,6 +53,7 @@ type privateContract struct {
 	Contract
 }
 
+//lint:ignore U1000 unused
 func (pc *privateContract) privateMethod() int64 {
 	return 1
 }
@@ -165,7 +168,7 @@ func testContractChaincodeContractMatchesContract(t *testing.T, actual contractC
 		assert.Equal(t, expected.afterTransaction.ReflectMetadata("", nil), actual.afterTransaction.ReflectMetadata("", nil), "should have matching before transactions")
 	}
 
-	assert.Equal(t, expected.transactionContextHandler, actual.transactionContextHandler, "should have matching transation contexts")
+	assert.Equal(t, expected.transactionContextHandler, actual.transactionContextHandler, "should have matching transaction contexts")
 
 	for idx, cf := range actual.functions {
 		assert.Equal(t, cf.ReflectMetadata("", nil), expected.functions[idx].ReflectMetadata("", nil), "should have matching functions")
@@ -440,7 +443,8 @@ func TestAugmentMetadata(t *testing.T) {
 		Info: info,
 	}
 
-	cc.augmentMetadata()
+	err := cc.augmentMetadata()
+	assert.NoError(t, err)
 
 	assert.Equal(t, cc.reflectMetadata(), cc.metadata, "should return reflected metadata when none supplied as file")
 }
@@ -472,20 +476,20 @@ func TestAddContract(t *testing.T) {
 	mc = new(myContract)
 	mc.Name = "customname"
 	err = cc.addContract(mc, []string{})
-	assert.EqualError(t, err, "Multiple contracts being merged into chaincode with name customname", "should error when contract already exists with name")
+	assert.EqualError(t, err, "multiple contracts being merged into chaincode with name customname", "should error when contract already exists with name")
 
 	// should error when no public functions
 	cc = new(ContractChaincode)
 	cc.contracts = make(map[string]contractChaincodeContract)
 	ic := new(emptyContract)
 	err = cc.addContract(ic, defaultExcludes)
-	assert.EqualError(t, err, fmt.Sprintf("Contracts are required to have at least 1 (non-ignored) public method. Contract emptyContract has none. Method names that have been ignored: %s", utils.SliceAsCommaSentence(defaultExcludes)), "should error when contract has no public functions")
+	assert.EqualError(t, err, fmt.Sprintf("contracts are required to have at least 1 (non-ignored) public method. Contract emptyContract has none. Method names that have been ignored: %s", utils.SliceAsCommaSentence(defaultExcludes)), "should error when contract has no public functions")
 
 	cc = new(ContractChaincode)
 	cc.contracts = make(map[string]contractChaincodeContract)
 	pc := new(privateContract)
 	err = cc.addContract(pc, defaultExcludes)
-	assert.EqualError(t, err, fmt.Sprintf("Contracts are required to have at least 1 (non-ignored) public method. Contract privateContract has none. Method names that have been ignored: %s", utils.SliceAsCommaSentence(defaultExcludes)), "should error when contract has no public functions but private ones")
+	assert.EqualError(t, err, fmt.Sprintf("contracts are required to have at least 1 (non-ignored) public method. Contract privateContract has none. Method names that have been ignored: %s", utils.SliceAsCommaSentence(defaultExcludes)), "should error when contract has no public functions but private ones")
 
 	// should add by default name
 	existingCCC := contractChaincodeContract{

contractapi/shared_test.go

@@ -5,6 +5,7 @@ package contractapi
 
 type myContract struct {
 	Contract
+	//lint:ignore U1000 unused
 	called []string
 }
 

contractapi/transaction_context_test.go

@@ -7,6 +7,7 @@ import (
 	"crypto/x509"
 	"testing"
 
+	//lint:ignore SA1019 TODO: needs to be removed
 	"github.com/hyperledger/fabric-chaincode-go/shimtest"
 	"github.com/stretchr/testify/assert"
 )

internal/contract_function.go

@@ -113,7 +113,7 @@ func (cf *ContractFunction) formatArgs(ctx reflect.Value, supplementaryMetadata
 
 	if supplementaryMetadata != nil {
 		if len(supplementaryMetadata) != numParams {
-			return nil, fmt.Errorf("Incorrect number of params in supplementary metadata. Expected %d, received %d", numParams, len(supplementaryMetadata))
+			return nil, fmt.Errorf("incorrect number of params in supplementary metadata. Expected %d, received %d", numParams, len(supplementaryMetadata))
 		}
 	}
 
@@ -124,7 +124,7 @@ func (cf *ContractFunction) formatArgs(ctx reflect.Value, supplementaryMetadata
 	}
 
 	if len(params) < numParams {
-		return nil, fmt.Errorf("Incorrect number of params. Expected %d, received %d", numParams, len(params))
+		return nil, fmt.Errorf("incorrect number of params. Expected %d, received %d", numParams, len(params))
 	}
 
 	channels := []chan formatArgResult{}
@@ -148,7 +148,7 @@ func (cf *ContractFunction) formatArgs(ctx reflect.Value, supplementaryMetadata
 		for res := range channel {
 
 			if res.err != nil {
-				return nil, fmt.Errorf("Error managing parameter%s. %s", res.paramName, res.err.Error())
+				return nil, fmt.Errorf("error managing parameter%s. %s", res.paramName, res.err.Error())
 			}
 
 			values = append(values, res.converted)
@@ -212,7 +212,7 @@ func (cf *ContractFunction) handleResponse(response []reflect.Value, returnsMeta
 				successString, err = serializer.ToString(successResponse, cf.returns.success, returnsMetadata, components)
 
 				if err != nil {
-					return "", nil, fmt.Errorf("Error handling success response. %s", err.Error())
+					return "", nil, fmt.Errorf("error handling success response. %s", err.Error())
 				}
 			}
 
@@ -245,7 +245,7 @@ func NewContractFunctionFromFunc(fn interface{}, callType CallType, contextHandl
 	fnValue := reflect.ValueOf(fn)
 
 	if fnType.Kind() != reflect.Func {
-		return nil, fmt.Errorf("Cannot create new contract function from %s. Can only use func", fnType.Kind())
+		return nil, fmt.Errorf("cannot create new contract function from %s. Can only use func", fnType.Kind())
 	}
 
 	myMethod := reflect.Method{}
@@ -327,7 +327,7 @@ func methodToContractFunctionParams(typeMethod reflect.Method, contextHandlerTyp
 		if typeError != nil && !isCtx {
 			return contractFunctionParams{}, fmt.Errorf("%s contains invalid parameter type. %s", methodName, typeError.Error())
 		} else if i != startIndex && isCtx {
-			return contractFunctionParams{}, fmt.Errorf("Functions requiring the TransactionContext must require it as the first parameter. %s takes it in as parameter %d", methodName, i-startIndex)
+			return contractFunctionParams{}, fmt.Errorf("functions requiring the TransactionContext must require it as the first parameter. %s takes it in as parameter %d", methodName, i-startIndex)
 		} else if isCtx {
 			usesCtx = contextHandlerType
 		} else {
@@ -349,7 +349,7 @@ func methodToContractFunctionReturns(typeMethod reflect.Method) (contractFunctio
 	}
 
 	if numOut > 2 {
-		return contractFunctionReturns{}, fmt.Errorf("Functions may only return a maximum of two values. %s returns %d", methodName, numOut)
+		return contractFunctionReturns{}, fmt.Errorf("functions may only return a maximum of two values. %s returns %d", methodName, numOut)
 	} else if numOut == 1 {
 		outType := typeMethod.Type.Out(0)