Integrate golangci-lint into CI/CD pipeline (#1346)

Co-authored-by: ccoVeille <3875889+ccoVeille@users.noreply.github.com>
Co-authored-by: Oleksandr Redko <oleksandr.red+github@gmail.com>

.github/workflows/lint.yaml

@@ -50,3 +50,8 @@ jobs:
 
     - name: Run revive
       run: revive --config revive.toml ./...
+
+    - name: Run golangci-lint
+      uses: golangci/golangci-lint-action@v8
+      with:
+        version: v2.1.6

.golangci.yml

@@ -0,0 +1,75 @@
+# This configuration for golangci-lint.
+# See https://golangci-lint.run/usage/configuration/ for details.
+
+version: "2"
+
+linters:
+  default: none
+  enable:
+    - govet
+    - ineffassign
+    - misspell
+    - nolintlint
+    - revive
+    - staticcheck
+    - unused
+
+  settings:
+    govet:
+      enable-all: true
+      disable:
+        - fieldalignment
+        - shadow
+    nolintlint:
+      allow-unused: true
+      require-explanation: true
+      require-specific: true
+    # The following settings should match revive.toml
+    revive:
+      severity: warning
+      confidence: 0.8
+      rules:
+        - name: bare-return
+        - name: blank-imports
+        - name: comment-spacings
+        - name: constant-logical-expr
+        - name: context-as-argument
+        - name: context-keys-type
+        - name: deep-exit
+        - name: dot-imports
+        - name: empty-block
+        - name: empty-lines
+        - name: enforce-map-style
+          arguments: ["literal"]
+        - name: enforce-slice-style
+          arguments: ["literal"]
+        - name: error-naming
+        - name: error-return
+        - name: error-strings
+        - name: errorf
+        - name: exported
+        - name: filename-format
+          # Override the default pattern to forbid .go files with uppercase letters and dashes.
+          arguments: ["^[_a-z][_a-z0-9]*\\.go$"]
+        - name: identical-branches
+        - name: increment-decrement
+        - name: indent-error-flow
+        - name: line-length-limit
+          arguments: [200]
+        - name: package-comments
+        - name: range
+        - name: receiver-naming
+        - name: redefines-builtin-id
+        - name: redundant-build-tag
+        - name: superfluous-else
+        - name: time-naming
+        - name: unexported-naming
+        - name: unexported-return
+        - name: unnecessary-stmt
+        - name: unreachable-code
+        - name: unused-parameter
+        - name: unused-receiver
+        - name: useless-break
+        - name: use-any
+        - name: var-declaration
+        - name: var-naming

formatter/formatter_test.go

@@ -61,11 +61,11 @@ Warnings:
 		},
 		{
 			formatter: &formatter.JSON{},
-			want:      `[{"Severity":"warning","Failure":"test failure","RuleName":"rule","Category":"cat","Position":{"Start":{"Filename":"test.go","Offset":0,"Line":2,"Column":5},"End":{"Filename":"test.go","Offset":0,"Line":2,"Column":10}},"Confidence":0,"ReplacementLine":""}]`,
+			want:      `[{"Severity":"warning","Failure":"test failure","RuleName":"rule","Category":"cat","Position":{"Start":{"Filename":"test.go","Offset":0,"Line":2,"Column":5},"End":{"Filename":"test.go","Offset":0,"Line":2,"Column":10}},"Confidence":0,"ReplacementLine":""}]`, //nolint:revive // line-length-limit
 		},
 		{
 			formatter: &formatter.NDJSON{},
-			want:      `{"Severity":"warning","Failure":"test failure","RuleName":"rule","Category":"cat","Position":{"Start":{"Filename":"test.go","Offset":0,"Line":2,"Column":5},"End":{"Filename":"test.go","Offset":0,"Line":2,"Column":10}},"Confidence":0,"ReplacementLine":""}`,
+			want:      `{"Severity":"warning","Failure":"test failure","RuleName":"rule","Category":"cat","Position":{"Start":{"Filename":"test.go","Offset":0,"Line":2,"Column":5},"End":{"Filename":"test.go","Offset":0,"Line":2,"Column":10}},"Confidence":0,"ReplacementLine":""}`, //nolint:revive // line-length-limit
 		},
 		{
 			formatter: &formatter.Plain{},

formatter/friendly.go

@@ -73,7 +73,7 @@ func (f *Friendly) printHeaderRow(sb *strings.Builder, failure lint.Failure, sev
 }
 
 func (*Friendly) printFilePosition(sb *strings.Builder, failure lint.Failure) {
-	sb.WriteString(fmt.Sprintf("  %s:%d:%d", failure.GetFilename(), failure.Position.Start.Line, failure.Position.Start.Column))
+	fmt.Fprintf(sb, "  %s:%d:%d", failure.GetFilename(), failure.Position.Start.Line, failure.Position.Start.Column)
 }
 
 type statEntry struct {

lint/package.go

@@ -58,9 +58,10 @@ func (p *Package) IsMain() bool {
 	p.Lock()
 	defer p.Unlock()
 
-	if p.main == trueValue {
+	switch p.main {
+	case trueValue:
 		return true
-	} else if p.main == falseValue {
+	case falseValue:
 		return false
 	}
 	for _, f := range p.files {

rule/datarace.go

@@ -22,7 +22,7 @@ func (r *DataRaceRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
 
 		funcResults := funcDecl.Type.Results
 
-		// TODO: ast.Object is deprecated
+		//nolint:staticcheck // TODO: ast.Object is deprecated
 		returnIDs := map[*ast.Object]struct{}{}
 		if funcResults != nil {
 			returnIDs = r.extractReturnIDs(funcResults.List)
@@ -35,7 +35,7 @@ func (r *DataRaceRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
 		fl := &lintFunctionForDataRaces{
 			onFailure: onFailure,
 			returnIDs: returnIDs,
-			rangeIDs:  map[*ast.Object]struct{}{}, // TODO: ast.Object is deprecated
+			rangeIDs:  map[*ast.Object]struct{}{}, //nolint:staticcheck // TODO: ast.Object is deprecated
 			go122for:  isGo122,
 		}
 
@@ -50,7 +50,7 @@ func (*DataRaceRule) Name() string {
 	return "datarace"
 }
 
-// TODO: ast.Object is deprecated
+//nolint:staticcheck // TODO: ast.Object is deprecated
 func (*DataRaceRule) extractReturnIDs(fields []*ast.Field) map[*ast.Object]struct{} {
 	r := map[*ast.Object]struct{}{}
 	for _, f := range fields {
@@ -65,8 +65,8 @@ func (*DataRaceRule) extractReturnIDs(fields []*ast.Field) map[*ast.Object]struc
 type lintFunctionForDataRaces struct {
 	_         struct{}
 	onFailure func(failure lint.Failure)
-	returnIDs map[*ast.Object]struct{} // TODO: ast.Object is deprecated
-	rangeIDs  map[*ast.Object]struct{} // TODO: ast.Object is deprecated
+	returnIDs map[*ast.Object]struct{} //nolint:staticcheck // TODO: ast.Object is deprecated
+	rangeIDs  map[*ast.Object]struct{} //nolint:staticcheck // TODO: ast.Object is deprecated
 
 	go122for bool
 }

rule/early_return.go

@@ -52,7 +52,7 @@ func (*EarlyReturnRule) Name() string {
 
 func (e *EarlyReturnRule) checkIfElse(chain ifelse.Chain) (string, bool) {
 	if chain.HasElse {
-		if !chain.Else.BranchKind.Deviates() {
+		if !chain.Else.Deviates() {
 			// this rule only applies if the else-block deviates control flow
 			return "", false
 		}

rule/error_strings_test.go

@@ -34,7 +34,7 @@ func TestErrorStringsRule_Configure(t *testing.T) {
 		{
 			name:      "Invalid function",
 			arguments: lint.Arguments{"errors."},
-			wantErr:   errors.New("found invalid custom function: errors."),
+			wantErr:   errors.New("found invalid custom function: errors."), //nolint:revive // error-strings: it's ok for tests
 		},
 		{
 			name:      "Invalid custom function",

rule/if_return.go

@@ -44,6 +44,7 @@ func (w *lintElseError) Visit(node ast.Node) ast.Visitor {
 				continue
 			}
 			assign, ok := s.Init.(*ast.AssignStmt)
+			//nolint:staticcheck // QF1001: it's readable enough
 			if !ok || len(assign.Lhs) != 1 || !(assign.Tok == token.DEFINE || assign.Tok == token.ASSIGN) {
 				continue
 			}

rule/import_alias_naming_test.go

@@ -39,7 +39,7 @@ func TestImportAliasNamingRule_Configure(t *testing.T) {
 			}},
 			wantErr:        nil,
 			wantAllowRegex: regexp.MustCompile("^[a-z][a-z0-9]*$"),
-			wantDenyRegex:  regexp.MustCompile("^v\\d+$"),
+			wantDenyRegex:  regexp.MustCompile(`^v\d+$`),
 		},
 		{
 			name: "valid map lowercased arguments",
@@ -49,7 +49,7 @@ func TestImportAliasNamingRule_Configure(t *testing.T) {
 			}},
 			wantErr:        nil,
 			wantAllowRegex: regexp.MustCompile("^[a-z][a-z0-9]*$"),
-			wantDenyRegex:  regexp.MustCompile("^v\\d+$"),
+			wantDenyRegex:  regexp.MustCompile(`^v\d+$`),
 		},
 		{
 			name: "valid map kebab-cased arguments",
@@ -59,7 +59,7 @@ func TestImportAliasNamingRule_Configure(t *testing.T) {
 			}},
 			wantErr:        nil,
 			wantAllowRegex: regexp.MustCompile("^[a-z][a-z0-9]*$"),
-			wantDenyRegex:  regexp.MustCompile("^v\\d+$"),
+			wantDenyRegex:  regexp.MustCompile(`^v\d+$`),
 		},
 		{
 			name:      "invalid argument type",

rule/import_shadowing.go

@@ -28,7 +28,7 @@ func (*ImportShadowingRule) Apply(file *lint.File, _ lint.Arguments) []lint.Fail
 		onFailure: func(failure lint.Failure) {
 			failures = append(failures, failure)
 		},
-		alreadySeen: map[*ast.Object]struct{}{}, // TODO: ast.Object is deprecated
+		alreadySeen: map[*ast.Object]struct{}{}, //nolint:staticcheck // TODO: ast.Object is deprecated
 		skipIdents:  map[*ast.Ident]struct{}{},
 	}
 
@@ -62,7 +62,7 @@ type importShadowing struct {
 	packageNameIdent *ast.Ident
 	importNames      map[string]struct{}
 	onFailure        func(lint.Failure)
-	alreadySeen      map[*ast.Object]struct{} // TODO: ast.Object is deprecated
+	alreadySeen      map[*ast.Object]struct{} //nolint:staticcheck // TODO: ast.Object is deprecated
 	skipIdents       map[*ast.Ident]struct{}
 }
 

rule/range_val_address.go

@@ -70,7 +70,7 @@ func (w rangeValAddress) Visit(node ast.Node) ast.Visitor {
 
 type rangeBodyVisitor struct {
 	valueIsStarExpr bool
-	valueID         *ast.Object // TODO: ast.Object is deprecated
+	valueID         *ast.Object //nolint:staticcheck // TODO: ast.Object is deprecated
 	onFailure       func(lint.Failure)
 }
 

rule/unused_param.go

@@ -143,7 +143,7 @@ func (w lintUnusedParamRule) Visit(node ast.Node) ast.Visitor {
 	return w // full method body was inspected
 }
 
-// TODO: ast.Object is deprecated
+//nolint:staticcheck // TODO: ast.Object is deprecated
 func retrieveNamedParams(params *ast.FieldList) map[*ast.Object]bool {
 	result := map[*ast.Object]bool{}
 	if params.List == nil {