Merge pull request #47 from StyraInc/golangci-lint

Golangci lint

.github/workflows/build.yaml

@@ -15,6 +15,7 @@ jobs:
       - uses: open-policy-agent/setup-opa@v2
         with:
           version: latest
+      - uses: golangci/golangci-lint-action@v3
       - run: opa check --strict bundle
       - run: opa test --bundle bundle
       - run: go test ./...

.golangci.yaml

@@ -0,0 +1,30 @@
+linters:
+  enable-all: true
+  disable:
+    # deprecated
+    - deadcode
+    - exhaustivestruct
+    - golint
+    - ifshort
+    - interfacer
+    - maligned
+    - nosnakecase
+    - scopelint
+    - structcheck
+    - varcheck
+    # does not work with generics (yet)
+    - rowserrcheck
+    - wastedassign
+    # annoying
+    - varnamelen
+    - nonamedreturns
+    - testpackage
+    - gomnd
+    - godox
+    - exhaustruct
+    - cyclop
+linters-settings:
+  tagliatelle:
+    case:
+      rules:
+        json: snake

internal/io/io.go

@@ -2,44 +2,48 @@ package io
 
 import (
 	"encoding/json"
-	"gopkg.in/yaml.v3"
+	"fmt"
 	"io"
 	files "io/fs"
 	"log"
 	"os"
 	"strings"
 
 	"github.com/open-policy-agent/opa/bundle"
+	"gopkg.in/yaml.v3"
 )
 
+//nolint:gochecknoglobals
 var excludeTestsFilter = func(abspath string, info files.FileInfo, depth int) bool {
 	return strings.HasSuffix(info.Name(), "_test.rego")
 }
 
-// LoadRegalBundleFS loads bundle embedded from policy and data directory
+// LoadRegalBundleFS loads bundle embedded from policy and data directory.
 func LoadRegalBundleFS(fs files.FS) (bundle.Bundle, error) {
 	embedLoader, err := bundle.NewFSLoader(fs)
 	if err != nil {
-		return bundle.Bundle{}, err
+		return bundle.Bundle{}, fmt.Errorf("failed to load bundle from filesystem: %w", err)
 	}
 
+	//nolint:wrapcheck
 	return bundle.NewCustomReader(embedLoader.WithFilter(excludeTestsFilter)).
 		WithSkipBundleVerification(true).
 		WithProcessAnnotations(true).
 		WithBundleName("regal").
 		Read()
 }
 
-// LoadRegalBundlePath loads bundle from path
+// LoadRegalBundlePath loads bundle from path.
 func LoadRegalBundlePath(path string) (bundle.Bundle, error) {
+	//nolint:wrapcheck
 	return bundle.NewCustomReader(bundle.NewDirectoryLoader(path).WithFilter(excludeTestsFilter)).
 		WithSkipBundleVerification(true).
 		WithProcessAnnotations(true).
 		WithBundleName("regal").
 		Read()
 }
 
-// MustLoadRegalBundleFS loads bundle embedded from policy and data directory, exit on failure
+// MustLoadRegalBundleFS loads bundle embedded from policy and data directory, exit on failure.
 func MustLoadRegalBundleFS(fs files.FS) bundle.Bundle {
 	regalBundle, err := LoadRegalBundleFS(fs)
 	if err != nil {
@@ -49,7 +53,7 @@ func MustLoadRegalBundleFS(fs files.FS) bundle.Bundle {
 	return regalBundle
 }
 
-// MustLoadRegalBundlePath loads bundle from path, exit on failure
+// MustLoadRegalBundlePath loads bundle from path, exit on failure.
 func MustLoadRegalBundlePath(path string) bundle.Bundle {
 	regalBundle, err := LoadRegalBundlePath(path)
 	if err != nil {
@@ -59,33 +63,37 @@ func MustLoadRegalBundlePath(path string) bundle.Bundle {
 	return regalBundle
 }
 
-// MustJSON marshal to JSON or exit
+// MustJSON marshal to JSON or exit.
 func MustJSON(x any) []byte {
 	bytes, err := json.Marshal(x)
 	if err != nil {
 		log.Fatal(err)
 	}
+
 	return bytes
 }
 
-// JSONRoundTrip convert any value to JSON and back again
+// JSONRoundTrip convert any value to JSON and back again.
 func JSONRoundTrip(from any, to any) error {
 	bs, err := json.Marshal(from)
 	if err != nil {
-		return err
+		return fmt.Errorf("failed JSON marshalling %w", err)
 	}
+
+	//nolint:wrapcheck
 	return json.Unmarshal(bs, to)
 }
 
-// MustYAMLToMap creates a map from reader, expecting YAML content, or fail
+// MustYAMLToMap creates a map from reader, expecting YAML content, or fail.
 func MustYAMLToMap(from io.Reader) (m map[string]any) {
 	if err := yaml.NewDecoder(from).Decode(&m); err != nil {
 		log.Fatal(err)
 	}
+
 	return m
 }
 
-// CloseFileIgnore closes file ignoring errors, mainly for deferred cleanup
+// CloseFileIgnore closes file ignoring errors, mainly for deferred cleanup.
 func CloseFileIgnore(file *os.File) {
 	_ = file.Close()
 }

internal/io/io_test.go

@@ -10,6 +10,7 @@ func TestJSONRoundTrip(t *testing.T) {
 	type foo struct {
 		Bar string `json:"bar"`
 	}
+
 	m := map[string]any{"bar": "foo"}
 	f := foo{}
 

internal/test/test.go

@@ -1,11 +1,13 @@
+//nolint:gochecknoglobals
 package test
 
 import (
-	"github.com/open-policy-agent/opa/bundle"
-	rio "github.com/styrainc/regal/internal/io"
 	"path/filepath"
 	"sync"
 	"testing"
+
+	"github.com/open-policy-agent/opa/bundle"
+	rio "github.com/styrainc/regal/internal/io"
 )
 
 const regalBundleDir = "../../bundle"
@@ -17,7 +19,7 @@ var regalBundle *bundle.Bundle
 // GetRegalBundle allows reusing the same Regal rule bundle in tests
 // without having to reload it from disk each time (i.e. a singleton)
 // Note that tests making use of this must *not* make any modifications
-// to the contents of the bundle
+// to the contents of the bundle.
 func GetRegalBundle(t *testing.T) bundle.Bundle {
 	t.Helper()
 

internal/util/util.go

@@ -1,12 +1,14 @@
 package util
 
-// Keys returns slice of keys from map
+// Keys returns slice of keys from map.
 func Keys[K comparable, V any](m map[K]V) []K {
 	ks := make([]K, len(m))
 	i := 0
+
 	for k := range m {
 		ks[i] = k
 		i++
 	}
+
 	return ks
 }

main.go

@@ -3,15 +3,14 @@ package main
 import (
 	"context"
 	"embed"
-	"fmt"
-	"github.com/styrainc/regal/pkg/config"
 	"io/fs"
 	"log"
 	"os"
 	"time"
 
 	"github.com/open-policy-agent/opa/loader"
 	rio "github.com/styrainc/regal/internal/io"
+	"github.com/styrainc/regal/pkg/config"
 	"github.com/styrainc/regal/pkg/linter"
 )
 
@@ -37,6 +36,7 @@ func main() {
 	}
 
 	regalRules := rio.MustLoadRegalBundleFS(bfs)
+
 	policies, err := loader.AllRegos(os.Args[1:])
 	if err != nil {
 		log.Fatal(err)
@@ -63,9 +63,11 @@ func main() {
 
 	rep, err := regal.Lint(ctx, policies)
 	if err != nil {
-		log.Fatal(err)
+		defer func() {
+			log.Fatal(err)
+		}()
+	} else {
+		// TODO: Create reporter interface and implementations
+		log.Println(rep)
 	}
-
-	// TODO: Create reporter interface and implementations
-	fmt.Println(rep)
 }

pkg/config/config.go

@@ -2,6 +2,7 @@ package config
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -19,31 +20,37 @@ type Rule struct {
 	Extra   ExtraAttributes
 }
 
-const configFileRelLocation = ".regal/config.yaml"
-const pathSeparator = string(os.PathSeparator)
+const (
+	configFileRelLocation = ".regal/config.yaml"
+	pathSeparator         = string(os.PathSeparator)
+)
 
 // FindConfig searches for .regal/config.yaml first in the directory of path,
 // and if not found, in the parent directory, and if not found, in the parent's
 // parent directory, and so on.
 func FindConfig(path string) (*os.File, error) {
 	finfo, err := os.Stat(path)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to stat path %v: %w", path, err)
 	}
+
 	dir := path
+
 	if !finfo.IsDir() {
 		dir = filepath.Dir(path)
 	}
 
 	for {
 		searchPath := filepath.Join(pathSeparator, dir, configFileRelLocation)
 		config, err := os.Open(searchPath)
+
 		if err == nil {
 			return config, nil
 		}
+
 		if searchPath == pathSeparator+configFileRelLocation {
 			// Stop traversing at the root path
-			return nil, err
+			return nil, fmt.Errorf("can't traverse past root directory %w", err)
 		}
 
 		// Move up one level in the directory tree
@@ -54,25 +61,30 @@ func FindConfig(path string) (*os.File, error) {
 }
 
 func (rule Rule) MarshalJSON() ([]byte, error) {
-	var result = make(map[string]any)
+	result := make(map[string]any)
 	result["enabled"] = rule.Enabled
 
 	for key, val := range rule.Extra {
 		result[key] = val
 	}
 
+	//nolint:wrapcheck
 	return json.Marshal(&result)
 }
 
+var errEnabledMustBeBoolean = errors.New("value of 'enabled' must be boolean")
+
 func (rule *Rule) UnmarshalJSON(data []byte) error {
 	var result map[string]any
 	if err := json.Unmarshal(data, &result); err != nil {
-		return err
+		return fmt.Errorf("unmarshalling rule failed %w", err)
 	}
+
 	enabled, ok := result["enabled"].(bool)
 	if !ok {
-		return fmt.Errorf("value of 'enabled' must be boolean")
+		return errEnabledMustBeBoolean
 	}
+
 	delete(result, "enabled")
 
 	rule.Enabled = enabled