golangci: bump version, addess all new findings

The previous version has been failing without any good reason for me, so let's try this. About the version pick: It's not the latest version (v1.62.0 at the moment), because that would introduce a new revive rule, redeclares-builtin-id, and that flags every variable called `min` or `max` in the code base. I had started addressing these, but they were just too many. The new issues related to this version are mostly that it complains whenever it finds a non-static string that makes its way into a printf- like function. However, that's a common pattern in some place here, so I've sprinkled some nolint:govet on it. Signed-off-by: Stephan Renatus <[email protected]>
open-policy-agent · Nov 14, 2024 · 2a57f71 · 2a57f71
1 parent cca332f
commit 2a57f71
Show file tree

Hide file tree

Showing 35 changed files with 95 additions and 99 deletions.
diff --git a/.golangci.yaml b/.golangci.yaml
@@ -1,6 +1,9 @@
 run:
   timeout: 5m
 
+issues:
+  max-same-issues: 0 # don't hide issues in CI runs because they are the same type
+
 linter-settings:
   lll:
     line-length: 200

diff --git a/Makefile b/Makefile
@@ -28,7 +28,7 @@ ifeq ($(WASM_ENABLED),1)
 GO_TAGS = -tags=opa_wasm
 endif
 
-GOLANGCI_LINT_VERSION := v1.59.1
+GOLANGCI_LINT_VERSION := v1.60.1
 YAML_LINT_VERSION := 0.29.0
 YAML_LINT_FORMAT ?= auto
 

diff --git a/ast/check.go b/ast/check.go
@@ -293,7 +293,7 @@ func (tc *typeChecker) checkRule(env *TypeEnv, as *AnnotationSet, rule *Rule) {
 				var err error
 				tpe, err = nestedObject(cpy, objPath, typeV)
 				if err != nil {
-					tc.err([]*Error{NewError(TypeErr, rule.Head.Location, err.Error())})
+					tc.err([]*Error{NewError(TypeErr, rule.Head.Location, err.Error())}) //nolint:govet
 					tpe = nil
 				}
 			} else {
@@ -1310,7 +1310,7 @@ func processAnnotation(ss *SchemaSet, annot *SchemaAnnotation, rule *Rule, allow
 
 	tpe, err := loadSchema(schema, allowNet)
 	if err != nil {
-		return nil, NewError(TypeErr, rule.Location, err.Error())
+		return nil, NewError(TypeErr, rule.Location, err.Error()) //nolint:govet
 	}
 
 	return tpe, nil

diff --git a/ast/compile.go b/ast/compile.go
@@ -840,7 +840,7 @@ func (c *Compiler) PassesTypeCheckRules(rules []*Rule) Errors {
 
 			tpe, err := loadSchema(schema, allowNet)
 			if err != nil {
-				return Errors{NewError(TypeErr, nil, err.Error())}
+				return Errors{NewError(TypeErr, nil, err.Error())} //nolint:govet
 			}
 			c.inputType = tpe
 		}
@@ -1171,7 +1171,7 @@ func (c *Compiler) checkRuleConflicts() {
 							continue // don't self-conflict
 						}
 						msg := fmt.Sprintf("%v conflicts with rule %v defined at %v", childMod.Package, rule.Head.Ref(), rule.Loc())
-						c.err(NewError(TypeErr, mod.Package.Loc(), msg))
+						c.err(NewError(TypeErr, mod.Package.Loc(), msg)) //nolint:govet
 					}
 				}
 			}
@@ -1652,7 +1652,7 @@ func (c *Compiler) init() {
 		if schema := c.schemaSet.Get(SchemaRootRef); schema != nil {
 			tpe, err := loadSchema(schema, c.capabilities.AllowNet)
 			if err != nil {
-				c.err(NewError(TypeErr, nil, err.Error()))
+				c.err(NewError(TypeErr, nil, err.Error())) //nolint:govet
 			} else {
 				c.inputType = tpe
 			}
@@ -1774,7 +1774,7 @@ func (c *Compiler) resolveAllRefs() {
 		WalkRules(mod, func(rule *Rule) bool {
 			err := resolveRefsInRule(globals, rule)
 			if err != nil {
-				c.err(NewError(CompileErr, rule.Location, err.Error()))
+				c.err(NewError(CompileErr, rule.Location, err.Error())) //nolint:govet
 			}
 			return false
 		})
@@ -1799,7 +1799,7 @@ func (c *Compiler) resolveAllRefs() {
 
 		parsed, err := c.moduleLoader(c.Modules)
 		if err != nil {
-			c.err(NewError(CompileErr, nil, err.Error()))
+			c.err(NewError(CompileErr, nil, err.Error())) //nolint:govet
 			return
 		}
 
@@ -2690,7 +2690,7 @@ func (vis *ruleArgLocalRewriter) Visit(x interface{}) Visitor {
 			Walk(vis, vcpy)
 			return k, vcpy, nil
 		}); err != nil {
-			vis.errs = append(vis.errs, NewError(CompileErr, t.Location, err.Error()))
+			vis.errs = append(vis.errs, NewError(CompileErr, t.Location, err.Error())) //nolint:govet
 		} else {
 			t.Value = cpy
 		}
@@ -5271,7 +5271,7 @@ func rewriteEveryStatement(g *localVarGenerator, stack *localDeclaredVars, expr
 		if v := every.Key.Value.(Var); !v.IsWildcard() {
 			gv, err := rewriteDeclaredVar(g, stack, v, declaredVar)
 			if err != nil {
-				return nil, append(errs, NewError(CompileErr, every.Loc(), err.Error()))
+				return nil, append(errs, NewError(CompileErr, every.Loc(), err.Error())) //nolint:govet
 			}
 			every.Key.Value = gv
 		}
@@ -5283,7 +5283,7 @@ func rewriteEveryStatement(g *localVarGenerator, stack *localDeclaredVars, expr
 	if v := every.Value.Value.(Var); !v.IsWildcard() {
 		gv, err := rewriteDeclaredVar(g, stack, v, declaredVar)
 		if err != nil {
-			return nil, append(errs, NewError(CompileErr, every.Loc(), err.Error()))
+			return nil, append(errs, NewError(CompileErr, every.Loc(), err.Error())) //nolint:govet
 		}
 		every.Value.Value = gv
 	}
@@ -5301,7 +5301,7 @@ func rewriteSomeDeclStatement(g *localVarGenerator, stack *localDeclaredVars, ex
 		switch v := decl.Symbols[i].Value.(type) {
 		case Var:
 			if _, err := rewriteDeclaredVar(g, stack, v, declaredVar); err != nil {
-				return nil, append(errs, NewError(CompileErr, decl.Loc(), err.Error()))
+				return nil, append(errs, NewError(CompileErr, decl.Loc(), err.Error())) //nolint:govet
 			}
 		case Call:
 			var key, val, container *Term
@@ -5329,7 +5329,7 @@ func rewriteSomeDeclStatement(g *localVarGenerator, stack *localDeclaredVars, ex
 
 			for _, v0 := range outputVarsForExprEq(e, container.Vars()).Sorted() {
 				if _, err := rewriteDeclaredVar(g, stack, v0, declaredVar); err != nil {
-					return nil, append(errs, NewError(CompileErr, decl.Loc(), err.Error()))
+					return nil, append(errs, NewError(CompileErr, decl.Loc(), err.Error())) //nolint:govet
 				}
 			}
 			return rewriteDeclaredVarsInExpr(g, stack, e, errs, strict)
@@ -5383,7 +5383,7 @@ func rewriteDeclaredAssignment(g *localVarGenerator, stack *localDeclaredVars, e
 		switch v := t.Value.(type) {
 		case Var:
 			if gv, err := rewriteDeclaredVar(g, stack, v, assignedVar); err != nil {
-				errs = append(errs, NewError(CompileErr, t.Location, err.Error()))
+				errs = append(errs, NewError(CompileErr, t.Location, err.Error())) //nolint:govet
 			} else {
 				t.Value = gv
 			}
@@ -5398,7 +5398,7 @@ func rewriteDeclaredAssignment(g *localVarGenerator, stack *localDeclaredVars, e
 		case Ref:
 			if RootDocumentRefs.Contains(t) {
 				if gv, err := rewriteDeclaredVar(g, stack, v[0].Value.(Var), assignedVar); err != nil {
-					errs = append(errs, NewError(CompileErr, t.Location, err.Error()))
+					errs = append(errs, NewError(CompileErr, t.Location, err.Error())) //nolint:govet
 				} else {
 					t.Value = gv
 				}

diff --git a/ast/parser.go b/ast/parser.go
@@ -2094,7 +2094,7 @@ func (p *Parser) genwildcard() string {
 }
 
 func (p *Parser) error(loc *location.Location, reason string) {
-	p.errorf(loc, reason)
+	p.errorf(loc, reason) //nolint:govet
 }
 
 func (p *Parser) errorf(loc *location.Location, f string, a ...interface{}) {

diff --git a/ast/parser_ext.go b/ast/parser_ext.go
@@ -704,7 +704,7 @@ func parseModule(filename string, stmts []Statement, comments []*Comment, regoCo
 		case Body:
 			rule, err := ParseRuleFromBody(mod, stmt)
 			if err != nil {
-				errs = append(errs, NewError(ParseErr, stmt[0].Location, err.Error()))
+				errs = append(errs, NewError(ParseErr, stmt[0].Location, err.Error())) //nolint:govet
 				continue
 			}
 			rule.generatedBody = true

diff --git a/ast/rego_v1.go b/ast/rego_v1.go
@@ -192,7 +192,7 @@ func checkRegoV1Rule(rule *Rule, opts RegoCheckOptions) Errors {
 	var errs Errors
 
 	if opts.NoKeywordsAsRuleNames && IsKeywordInRegoVersion(rule.Head.Name.String(), RegoV1) {
-		errs = append(errs, NewError(ParseErr, rule.Location, fmt.Sprintf("%s keyword cannot be used for rule name", rule.Head.Name.String())))
+		errs = append(errs, NewError(ParseErr, rule.Location, "%s keyword cannot be used for rule name", rule.Head.Name.String()))
 	}
 	if opts.RequireRuleBodyOrValue && rule.generatedBody && rule.Head.generatedValue {
 		errs = append(errs, NewError(ParseErr, rule.Location, "%s must have value assignment and/or body declaration", t))

diff --git a/ast/term_test.go b/ast/term_test.go
@@ -723,13 +723,11 @@ func TestSetEqual(t *testing.T) {
 		a := MustParseTerm(tc.a)
 		b := MustParseTerm(tc.b)
 		if a.Equal(b) != tc.expected {
-			var msg string
 			if tc.expected {
-				msg = fmt.Sprintf("Expected %v to equal %v", a, b)
+				t.Errorf("Expected %v to equal %v", a, b)
 			} else {
-				msg = fmt.Sprintf("Expected %v to NOT equal %v", a, b)
+				t.Errorf("Expected %v to NOT equal %v", a, b)
 			}
-			t.Errorf(msg)
 		}
 	}
 }

diff --git a/cmd/eval.go b/cmd/eval.go
@@ -392,7 +392,7 @@ func eval(args []string, params evalCommandParams, w io.Writer) (bool, error) {
 			for _, t := range timers {
 				val, ok := t[name].(int64)
 				if !ok {
-					return false, fmt.Errorf("missing timer for %s" + name)
+					return false, fmt.Errorf("missing timer for %s", name)
 				}
 				vals = append(vals, val)
 			}

diff --git a/cmd/eval_test.go b/cmd/eval_test.go
@@ -9,6 +9,7 @@ import (
 	"bufio"
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -506,7 +507,7 @@ func testEvalWithSchemasAnnotationButNoSchemaFlag(policy string) error {
 		var defined bool
 		defined, err = eval([]string{query}, params, &buf)
 		if !defined || err != nil {
-			err = fmt.Errorf(buf.String())
+			err = errors.New(buf.String())
 		}
 	})
 

diff --git a/cmd/fmt.go b/cmd/fmt.go
@@ -223,9 +223,9 @@ func formatStdin(params *fmtCommandParams, r io.Reader, w io.Writer) error {
 	return err
 }
 
-func doDiff(old, new []byte) (diffString string) {
+func doDiff(a, b []byte) (diffString string) { // "a" is old, "b" is new
 	dmp := diffmatchpatch.New()
-	diffs := dmp.DiffMain(string(old), string(new), false)
+	diffs := dmp.DiffMain(string(a), string(b), false)
 	return dmp.DiffPrettyText(diffs)
 }
 

diff --git a/cmd/fmt_test.go b/cmd/fmt_test.go
@@ -2,6 +2,7 @@ package cmd
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -98,8 +99,8 @@ type errorWriter struct {
 	ErrMsg string
 }
 
-func (ew errorWriter) Write(_ []byte) (n int, err error) {
-	return 0, fmt.Errorf(ew.ErrMsg)
+func (ew errorWriter) Write([]byte) (int, error) {
+	return 0, errors.New(ew.ErrMsg)
 }
 
 func TestFmtFormatFile(t *testing.T) {

diff --git a/cmd/test.go b/cmd/test.go
@@ -6,6 +6,7 @@ package cmd
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -373,7 +374,7 @@ func compileAndSetupTests(ctx context.Context, testParams testCommandParams, sto
 		if testParams.benchmark {
 			errMsg := "coverage reporting is not supported when benchmarking tests"
 			fmt.Fprintln(testParams.errOutput, errMsg)
-			return nil, nil, fmt.Errorf(errMsg)
+			return nil, nil, errors.New(errMsg)
 		}
 		cov = cover.New()
 		coverTracer = cov

diff --git a/dependencies/deps_test.go b/dependencies/deps_test.go
@@ -358,7 +358,7 @@ func TestDependencies(t *testing.T) {
 			})
 
 			mod := compiler.Modules["test"]
-			min, full := runDeps(t, mod)
+			minOfAll, full := runDeps(t, mod)
 
 			// Test that we get the same result by analyzing all the
 			// rules separately.
@@ -369,7 +369,7 @@ func TestDependencies(t *testing.T) {
 				fullRules = append(fullRules, f...)
 			}
 
-			assertRefSliceEq(t, exp, min)
+			assertRefSliceEq(t, exp, minOfAll)
 			assertRefSliceEq(t, exp, minRules)
 
 			for _, full := range test.full {

diff --git a/format/format.go b/format/format.go
@@ -1036,7 +1036,7 @@ func (w *writer) writeObjectComprehension(object *ast.ObjectComprehension, loc *
 	return w.writeComprehension('{', '}', object.Value, object.Body, loc, comments)
 }
 
-func (w *writer) writeComprehension(open, close byte, term *ast.Term, body ast.Body, loc *ast.Location, comments []*ast.Comment) []*ast.Comment {
+func (w *writer) writeComprehension(openChar, closeChar byte, term *ast.Term, body ast.Body, loc *ast.Location, comments []*ast.Comment) []*ast.Comment {
 	if term.Location.Row-loc.Row >= 1 {
 		w.endLine()
 		w.startLine()
@@ -1050,10 +1050,10 @@ func (w *writer) writeComprehension(open, close byte, term *ast.Term, body ast.B
 	comments = w.writeTermParens(parens, term, comments)
 	w.write(" |")
 
-	return w.writeComprehensionBody(open, close, body, term.Location, loc, comments)
+	return w.writeComprehensionBody(openChar, closeChar, body, term.Location, loc, comments)
 }
 
-func (w *writer) writeComprehensionBody(open, close byte, body ast.Body, term, compr *ast.Location, comments []*ast.Comment) []*ast.Comment {
+func (w *writer) writeComprehensionBody(openChar, closeChar byte, body ast.Body, term, compr *ast.Location, comments []*ast.Comment) []*ast.Comment {
 	exprs := make([]interface{}, 0, len(body))
 	for _, expr := range body {
 		exprs = append(exprs, expr)
@@ -1077,7 +1077,7 @@ func (w *writer) writeComprehensionBody(open, close byte, body ast.Body, term, c
 		comments = w.writeExpr(body[i], comments)
 	}
 
-	return w.insertComments(comments, closingLoc(0, 0, open, close, compr))
+	return w.insertComments(comments, closingLoc(0, 0, openChar, closeChar, compr))
 }
 
 func (w *writer) writeImports(imports []*ast.Import, comments []*ast.Comment) []*ast.Comment {
@@ -1403,7 +1403,7 @@ func getLoc(x interface{}) *ast.Location {
 	}
 }
 
-func closingLoc(skipOpen, skipClose, open, close byte, loc *ast.Location) *ast.Location {
+func closingLoc(skipOpen, skipClose, openChar, closeChar byte, loc *ast.Location) *ast.Location {
 	i, offset := 0, 0
 
 	// Skip past parens/brackets/braces in rule heads.
@@ -1412,7 +1412,7 @@ func closingLoc(skipOpen, skipClose, open, close byte, loc *ast.Location) *ast.L
 	}
 
 	for ; i < len(loc.Text); i++ {
-		if loc.Text[i] == open {
+		if loc.Text[i] == openChar {
 			break
 		}
 	}
@@ -1429,9 +1429,9 @@ func closingLoc(skipOpen, skipClose, open, close byte, loc *ast.Location) *ast.L
 		}
 
 		switch loc.Text[i] {
-		case open:
+		case openChar:
 			state++
-		case close:
+		case closeChar:
 			state--
 		case '\n':
 			offset++
@@ -1441,10 +1441,10 @@ func closingLoc(skipOpen, skipClose, open, close byte, loc *ast.Location) *ast.L
 	return &ast.Location{Row: loc.Row + offset}
 }
 
-func skipPast(open, close byte, loc *ast.Location) (int, int) {
+func skipPast(openChar, closeChar byte, loc *ast.Location) (int, int) {
 	i := 0
 	for ; i < len(loc.Text); i++ {
-		if loc.Text[i] == open {
+		if loc.Text[i] == openChar {
 			break
 		}
 	}
@@ -1458,9 +1458,9 @@ func skipPast(open, close byte, loc *ast.Location) (int, int) {
 		}
 
 		switch loc.Text[i] {
-		case open:
+		case openChar:
 			state++
-		case close:
+		case closeChar:
 			state--
 		case '\n':
 			offset++