Change check-lint to use golangci-lint (#3465)

golint is deprecated. The author of the code no longer supports the
codebase. golangci-lint is faster than golint, and is in use by other
opa repositories (e.g. Gatekeeper).

This commit changes tools.go to reference golangci (so it ends up in
vendor) and modifies check-lint to use golangci instead.

Breaking API Changes:

- plugins/rest/rest.go: Fix typo "AllowInsureTLS" -> "AllowInsecureTLS"
- storage/errors.go: Removed unused IndexingNotSupportedErr

Signed-off-by: Will Beason <willbeason@google.com>

.golangci.yaml

@@ -0,0 +1,27 @@
+linter-settings:
+  lll:
+    line-length: 200
+
+  misspell:
+    locale: US
+
+linters:
+  disable-all: true
+  enable:
+    - errcheck
+    - govet
+    - ineffassign
+#    - golint # deprecated and no longer supported
+    - revive # replacement for golint
+    - goconst
+    - gofmt
+    - goimports
+    - unused
+    - varcheck
+    - deadcode
+    - misspell
+    - typecheck
+    - structcheck
+    - staticcheck
+    - gosimple
+    # - gosec # too many false positives

ast/capabilities.go

@@ -34,10 +34,7 @@ func CapabilitiesForThisVersion() *Capabilities {
 		f.WasmABIVersions = append(f.WasmABIVersions, WasmABIVersion{Version: vers[0], Minor: vers[1]})
 	}
 
-	for _, bi := range Builtins {
-		f.Builtins = append(f.Builtins, bi)
-	}
-
+	f.Builtins = append(f.Builtins, Builtins...)
 	sort.Slice(f.Builtins, func(i, j int) bool {
 		return f.Builtins[i].Name < f.Builtins[j].Name
 	})

ast/check.go

@@ -180,7 +180,7 @@ func (tc *typeChecker) checkRule(env *TypeEnv, as *annotationSet, rule *Rule) {
 
 	if schemaAnnots := getRuleAnnotation(as, rule); schemaAnnots != nil {
 		for _, schemaAnnot := range schemaAnnots {
-			ref, refType, err := processAnnotation(tc.ss, schemaAnnot, env, rule)
+			ref, refType, err := processAnnotation(tc.ss, schemaAnnot, rule)
 			if err != nil {
 				tc.err([]*Error{err})
 				continue
@@ -379,24 +379,24 @@ func unify2(env *TypeEnv, a *Term, typeA types.Type, b *Term, typeB types.Type)
 
 	switch a.Value.(type) {
 	case *Array:
-		return unify2Array(env, a, typeA, b, typeB)
+		return unify2Array(env, a, b)
 	case *object:
-		return unify2Object(env, a, typeA, b, typeB)
+		return unify2Object(env, a, b)
 	case Var:
 		switch b.Value.(type) {
 		case Var:
 			return unify1(env, a, types.A, false) && unify1(env, b, env.Get(a), false)
 		case *Array:
-			return unify2Array(env, b, typeB, a, typeA)
+			return unify2Array(env, b, a)
 		case *object:
-			return unify2Object(env, b, typeB, a, typeA)
+			return unify2Object(env, b, a)
 		}
 	}
 
 	return false
 }
 
-func unify2Array(env *TypeEnv, a *Term, typeA types.Type, b *Term, typeB types.Type) bool {
+func unify2Array(env *TypeEnv, a *Term, b *Term) bool {
 	arr := a.Value.(*Array)
 	switch bv := b.Value.(type) {
 	case *Array:
@@ -414,7 +414,7 @@ func unify2Array(env *TypeEnv, a *Term, typeA types.Type, b *Term, typeB types.T
 	return false
 }
 
-func unify2Object(env *TypeEnv, a *Term, typeA types.Type, b *Term, typeB types.Type) bool {
+func unify2Object(env *TypeEnv, a *Term, b *Term) bool {
 	obj := a.Value.(Object)
 	switch bv := b.Value.(type) {
 	case *object:
@@ -682,7 +682,7 @@ func (rc *refChecker) checkRef(curr *TypeEnv, node *typeTreeNode, ref Ref, idx i
 	// potentially refers to data for which no type information exists,
 	// checking should never fail.
 	node.Children().Iter(func(_, child util.T) bool {
-		rc.checkRef(curr, child.(*typeTreeNode), ref, idx+1)
+		_ = rc.checkRef(curr, child.(*typeTreeNode), ref, idx+1) // ignore error
 		return false
 	})
 
@@ -1062,7 +1062,7 @@ func getPrefix(env *TypeEnv, ref Ref) (Ref, types.Type) {
 
 // override takes a type t and returns a type obtained from t where the path represented by ref within it has type o (overriding the original type of that path)
 func override(ref Ref, t types.Type, o types.Type, rule *Rule) (types.Type, *Error) {
-	newStaticProps := []*types.StaticProperty{}
+	var newStaticProps []*types.StaticProperty
 	obj, ok := t.(*types.Object)
 	if !ok {
 		newType, err := getObjectType(ref, o, rule, types.NewDynamicProperty(types.A, types.A))
@@ -1158,7 +1158,7 @@ func getRuleAnnotation(as *annotationSet, rule *Rule) (result []*SchemaAnnotatio
 	return result
 }
 
-func processAnnotation(ss *SchemaSet, annot *SchemaAnnotation, env *TypeEnv, rule *Rule) (Ref, types.Type, *Error) {
+func processAnnotation(ss *SchemaSet, annot *SchemaAnnotation, rule *Rule) (Ref, types.Type, *Error) {
 
 	var schema interface{}
 

ast/compile.go

@@ -214,8 +214,6 @@ type QueryCompilerStageDefinition struct {
 	Stage      QueryCompilerStage
 }
 
-const compileStageMetricPrefex = "ast_compile_stage_"
-
 // NewCompiler returns a new empty compiler.
 func NewCompiler() *Compiler {
 
@@ -821,13 +819,13 @@ func (c *Compiler) checkSafetyRuleBodies() {
 		WalkRules(m, func(r *Rule) bool {
 			safe := ReservedVars.Copy()
 			safe.Update(r.Head.Args.Vars())
-			r.Body = c.checkBodySafety(safe, m, r.Body)
+			r.Body = c.checkBodySafety(safe, r.Body)
 			return false
 		})
 	}
 }
 
-func (c *Compiler) checkBodySafety(safe VarSet, m *Module, b Body) Body {
+func (c *Compiler) checkBodySafety(safe VarSet, b Body) Body {
 	reordered, unsafe := reorderBodyForSafety(c.builtins, c.GetArity, safe, b)
 	if errs := safetyErrorSlice(unsafe); len(errs) > 0 {
 		for _, err := range errs {
@@ -1139,7 +1137,7 @@ func (c *Compiler) rewriteComprehensionTerms() {
 	f := newEqualityFactory(c.localvargen)
 	for _, name := range c.sorted {
 		mod := c.Modules[name]
-		rewriteComprehensionTerms(f, mod)
+		_, _ = rewriteComprehensionTerms(f, mod) // ignore error
 	}
 }
 
@@ -1434,7 +1432,7 @@ func (c *Compiler) rewriteWithModifiers() {
 
 			return body, nil
 		})
-		Transform(t, mod)
+		_, _ = Transform(t, mod) // ignore error
 	}
 }
 
@@ -1850,7 +1848,7 @@ type comprehensionIndexRegressionCheckVisitor struct {
 	worse      bool
 }
 
-// TOOD(tsandall): Improve this so that users can either supply this list explicitly
+// TODO(tsandall): Improve this so that users can either supply this list explicitly
 // or the information is maintained on the built-in function declaration. What we really
 // need to know is whether the built-in function allows callers to push down output
 // values or not. It's unlikely that anything outside of OPA does this today so this
@@ -1901,7 +1899,6 @@ func (vis *comprehensionIndexRegressionCheckVisitor) assertEmptyIntersection(vs
 
 type comprehensionIndexNestedCandidateVisitor struct {
 	candidates VarSet
-	nested     bool
 	found      bool
 }
 
@@ -2144,20 +2141,20 @@ func (g *Graph) Sort() (sorted []util.T, ok bool) {
 		return g.sorted, true
 	}
 
-	sort := &graphSort{
+	sorter := &graphSort{
 		sorted: make([]util.T, 0, len(g.nodes)),
 		deps:   g.Dependencies,
 		marked: map[util.T]struct{}{},
 		temp:   map[util.T]struct{}{},
 	}
 
 	for node := range g.nodes {
-		if !sort.Visit(node) {
+		if !sorter.Visit(node) {
 			return nil, false
 		}
 	}
 
-	g.sorted = sort.sorted
+	g.sorted = sorter.sorted
 	return g.sorted, true
 }
 
@@ -3074,7 +3071,7 @@ func rewriteEquals(x interface{}) {
 		}
 		return x, nil
 	})
-	Transform(t, x)
+	_, _ = Transform(t, x) // ignore error
 }
 
 // rewriteDynamics will rewrite the body so that dynamic terms (i.e., refs and

ast/compile_test.go

@@ -790,10 +790,10 @@ func TestCompilerCheckSafetyBodyErrors(t *testing.T) {
 			// Build slice of expected error messages.
 			expected := []string{}
 
-			MustParseTerm(tc.expected).Value.(Set).Iter(func(x *Term) error {
+			_ = MustParseTerm(tc.expected).Value.(Set).Iter(func(x *Term) error {
 				expected = append(expected, makeErrMsg(string(x.Value.(Var))))
 				return nil
-			})
+			}) // cannot return error
 
 			sort.Strings(expected)
 
@@ -2709,11 +2709,11 @@ func TestCompilerSetGraph(t *testing.T) {
 		},
 		{
 			x:    q,
-			want: map[util.T]struct{}{p: struct{}{}, mod5.Rules[1]: struct{}{}, mod5.Rules[3]: struct{}{}, mod5.Rules[5]: struct{}{}},
+			want: map[util.T]struct{}{p: {}, mod5.Rules[1]: {}, mod5.Rules[3]: {}, mod5.Rules[5]: {}},
 		},
 		{
 			x:    r,
-			want: map[util.T]struct{}{p: struct{}{}},
+			want: map[util.T]struct{}{p: {}},
 		},
 	}
 
@@ -3302,11 +3302,11 @@ r3 = 3`,
 func TestCompileCustomBuiltins(t *testing.T) {
 
 	compiler := NewCompiler().WithBuiltins(map[string]*Builtin{
-		"baz": &Builtin{
+		"baz": {
 			Name: "baz",
 			Decl: types.NewFunction([]types.Type{types.S}, types.A),
 		},
-		"foo.bar": &Builtin{
+		"foo.bar": {
 			Name: "foo.bar",
 			Decl: types.NewFunction([]types.Type{types.S}, types.A),
 		},
@@ -3999,7 +3999,7 @@ func TestQueryCompilerWithStageAfterWithMetrics(t *testing.T) {
 
 func TestQueryCompilerWithUnsafeBuiltins(t *testing.T) {
 	c := NewCompiler().WithUnsafeBuiltins(map[string]struct{}{
-		"count": struct{}{},
+		"count": {},
 	})
 
 	_, err := c.QueryCompiler().WithUnsafeBuiltins(map[string]struct{}{}).Compile(MustParseBody("count([])"))
@@ -4258,7 +4258,7 @@ func TestCompilerWithUnsafeBuiltins(t *testing.T) {
 	// Rego includes a number of built-in functions. In some cases, you may not
 	// want all builtins to be available to a program. This test shows how to
 	// mark a built-in as unsafe.
-	compiler := NewCompiler().WithUnsafeBuiltins(map[string]struct{}{"re_match": struct{}{}})
+	compiler := NewCompiler().WithUnsafeBuiltins(map[string]struct{}{"re_match": {}})
 
 	// This query should not compile because the `re_match` built-in is no
 	// longer available.

ast/errors.go

@@ -24,9 +24,9 @@ func (e Errors) Error() string {
 		return fmt.Sprintf("1 error occurred: %v", e[0].Error())
 	}
 
-	s := []string{}
-	for _, err := range e {
-		s = append(s, err.Error())
+	s := make([]string, len(e))
+	for i, err := range e {
+		s[i] = err.Error()
 	}
 
 	return fmt.Sprintf("%d errors occurred:\n%s", len(e), strings.Join(s, "\n"))
@@ -124,7 +124,6 @@ func NewError(code string, loc *Location, f string, a ...interface{}) *Error {
 
 var (
 	errPartialRuleAssignOperator = fmt.Errorf("partial rules must use = operator (not := operator)")
-	errElseAssignOperator        = fmt.Errorf("else keyword cannot be used on rule declared with := operator")
 	errFunctionAssignOperator    = fmt.Errorf("functions must use = operator (not := operator)")
 )
 

ast/index.go

@@ -6,7 +6,6 @@ package ast
 
 import (
 	"fmt"
-	"io"
 	"sort"
 	"strings"
 
@@ -421,7 +420,7 @@ func (node *trieNode) String() string {
 		flags = append(flags, fmt.Sprintf("array:%p", node.array))
 	}
 	if len(node.scalars) > 0 {
-		buf := []string{}
+		buf := make([]string, 0, len(node.scalars))
 		for k, v := range node.scalars {
 			buf = append(buf, fmt.Sprintf("scalar(%v):%p", k, v))
 		}
@@ -575,15 +574,21 @@ func (node *trieNode) traverse(resolver ValueResolver, tr *trieTraversalResult)
 	}
 
 	if node.undefined != nil {
-		node.undefined.Traverse(resolver, tr)
+		err = node.undefined.Traverse(resolver, tr)
+		if err != nil {
+			return err
+		}
 	}
 
 	if v == nil {
 		return nil
 	}
 
 	if node.any != nil {
-		node.any.Traverse(resolver, tr)
+		err = node.any.Traverse(resolver, tr)
+		if err != nil {
+			return err
+		}
 	}
 
 	if err := node.traverseValue(resolver, tr, v); err != nil {
@@ -632,7 +637,10 @@ func (node *trieNode) traverseArray(resolver ValueResolver, tr *trieTraversalRes
 	}
 
 	if node.any != nil {
-		node.any.traverseArray(resolver, tr, arr.Slice(1, -1))
+		err := node.any.traverseArray(resolver, tr, arr.Slice(1, -1))
+		if err != nil {
+			return err
+		}
 	}
 
 	child, ok := node.scalars[head]
@@ -674,18 +682,6 @@ func (node *trieNode) traverseUnknown(resolver ValueResolver, tr *trieTraversalR
 	return nil
 }
 
-type triePrinter struct {
-	depth int
-	w     io.Writer
-}
-
-func (p triePrinter) Do(x interface{}) trieWalker {
-	padding := strings.Repeat(" ", p.depth)
-	fmt.Fprintf(p.w, "%v%v\n", padding, x)
-	p.depth++
-	return p
-}
-
 func eqOperandsToRefAndValue(isVirtual func(Ref) bool, a, b *Term) (Ref, Value, bool) {
 
 	ref, ok := a.Value.(Ref)

ast/internal/scanner/scanner.go

@@ -19,14 +19,13 @@ const bom = 0xFEFF
 // Scanner is used to tokenize an input stream of
 // Rego source code.
 type Scanner struct {
-	offset   int
-	row      int
-	col      int
-	bs       []byte
-	curr     rune
-	width    int
-	errors   []Error
-	filename string
+	offset int
+	row    int
+	col    int
+	bs     []byte
+	curr   rune
+	width  int
+	errors []Error
 }
 
 // Error represents a scanner error.
@@ -347,13 +346,6 @@ func (s *Scanner) next() {
 	}
 }
 
-func (s *Scanner) peek(i int) rune {
-	if s.offset+i < len(s.bs) {
-		return rune(s.bs[s.offset+i])
-	}
-	return 0
-}
-
 func (s *Scanner) literalStart() int {
 	// The current offset is at the first character past the literal delimiter (#, ", `, etc.)
 	// Need to subtract width of first character (plus one for the delimiter).