chore: pull in lint rules from spicedb

.golangci.yaml

@@ -4,26 +4,33 @@ linters:
   enable:
     - "bidichk"
     - "bodyclose"
+    - "depguard"
     - "errcheck"
     - "errname"
     - "errorlint"
+    - "gocritic"
     - "goprintffuncname"
     - "gosec"
     - "govet"
     - "importas"
     - "ineffassign"
     - "makezero"
+    - "perfsprint"
     - "prealloc"
     - "predeclared"
     - "promlinter"
     - "revive"
     - "rowserrcheck"
+    - "spancheck"
     - "staticcheck"
+    - "tagalign"
+    - "testifylint"
+    - "tparallel"
     - "unconvert"
-    - "unused"
     - "usetesting"
     - "wastedassign"
     - "whitespace"
+    - "unused"
   exclusions:
     generated: "lax"
     presets:
@@ -35,12 +42,29 @@ linters:
       - "third_party$"
       - "builtin$"
       - "examples$"
+  settings:
+    depguard:
+      rules:
+        main:
+          deny:
+            - pkg: "k8s.io/utils/strings/slices$"
+              desc: "use github.com/samber/lo"
+    staticcheck:
+      checks:
+        - "all"
 formatters:
   enable:
     - "gci"
+    - "gofmt"
     - "gofumpt"
     - "goimports"
   settings:
+    gofmt:
+      rewrite-rules:
+        - pattern: "interface{}"
+          replacement: "any"
+        - pattern: "a[b:len(a)]"
+          replacement: "a[b:]"
     gci:
       sections:
         - "standard"

internal/cmd/backup.go

@@ -113,9 +113,7 @@ func registerBackupCmd(rootCmd *cobra.Command) {
 		Use:   "redact <filename>",
 		Short: "Redact a backup file to remove sensitive information",
 		Args:  commands.ValidationWrapper(cobra.ExactArgs(1)),
-		RunE: func(cmd *cobra.Command, args []string) error {
-			return backupRedactCmdFunc(cmd, args)
-		},
+		RunE:  backupRedactCmdFunc,
 	}
 
 	rootCmd.AddCommand(backupCmd)
@@ -364,11 +362,12 @@ func backupCreateCmdFunc(cmd *cobra.Command, args []string) (err error) {
 			Uint64("processed", relsProcessed).
 			Uint64("throughput", perSec(relsProcessed, time.Since(relationshipReadStart))).
 			Stringer("elapsed", time.Since(relationshipReadStart).Round(time.Second))
-		if isCanceled(err) {
+		switch {
+		case isCanceled(err):
 			evt.Msg("backup canceled - resume by restarting the backup command")
-		} else if err != nil {
+		case err != nil:
 			evt.Msg("backup failed")
-		} else {
+		default:
 			evt.Msg("finished backup")
 		}
 	}()
@@ -490,7 +489,7 @@ func encoderForNewBackup(cmd *cobra.Command, c client.Client, backupFile *os.Fil
 		return nil, nil, fmt.Errorf("error reading schema: %w", err)
 	}
 	if schemaResp.ReadAt == nil {
-		return nil, nil, fmt.Errorf("`backup` is not supported on this version of SpiceDB")
+		return nil, nil, errors.New("`backup` is not supported on this version of SpiceDB")
 	}
 	schema := schemaResp.SchemaText
 
@@ -550,17 +549,20 @@ func openProgressFile(backupFileName string, backupAlreadyExisted bool) (*os.Fil
 	// if a backup existed
 	var fileMode int
 	readCursor, err := os.ReadFile(progressFileName)
-	if backupAlreadyExisted && (os.IsNotExist(err) || len(readCursor) == 0) {
-		return nil, nil, fmt.Errorf("backup file %s already exists", backupFileName)
-	} else if backupAlreadyExisted && err == nil {
-		cursor = &v1.Cursor{
-			Token: string(readCursor),
+	if backupAlreadyExisted {
+		if os.IsNotExist(err) || len(readCursor) == 0 {
+			return nil, nil, fmt.Errorf("backup file %s already exists", backupFileName)
 		}
+		if err == nil {
+			cursor = &v1.Cursor{
+				Token: string(readCursor),
+			}
 
-		// if backup existed and there is a progress marker, the latter should not be truncated to make sure the
-		// cursor stays around in case of a failure before we even start ingesting from bulk export
-		fileMode = os.O_WRONLY | os.O_CREATE
-		log.Info().Str("filename", backupFileName).Msg("backup file already exists, will resume")
+			// if backup existed and there is a progress marker, the latter should not be truncated to make sure the
+			// cursor stays around in case of a failure before we even start ingesting from bulk export
+			fileMode = os.O_WRONLY | os.O_CREATE
+			log.Info().Str("filename", backupFileName).Msg("backup file already exists, will resume")
+		}
 	} else {
 		// if a backup did not exist, make sure to truncate the progress file
 		fileMode = os.O_WRONLY | os.O_CREATE | os.O_TRUNC
@@ -733,7 +735,7 @@ func backupParseRevisionCmdFunc(_ *cobra.Command, out io.Writer, args []string)
 
 	loadedToken := decoder.ZedToken()
 	if loadedToken == nil {
-		return fmt.Errorf("failed to parse decoded revision")
+		return errors.New("failed to parse decoded revision")
 	}
 
 	_, err = fmt.Fprintln(out, loadedToken.Token)

internal/cmd/backup_test.go

@@ -45,6 +45,7 @@ var testRelationships = []string{
 }
 
 func TestFilterSchemaDefs(t *testing.T) {
+	t.Parallel()
 	for _, tt := range []struct {
 		name         string
 		inputSchema  string
@@ -139,6 +140,7 @@ func TestFilterSchemaDefs(t *testing.T) {
 }
 
 func TestBackupParseRelsCmdFunc(t *testing.T) {
+	t.Parallel()
 	for _, tt := range []struct {
 		name          string
 		filter        string
@@ -192,6 +194,7 @@ func TestBackupParseRelsCmdFunc(t *testing.T) {
 }
 
 func TestBackupParseRevisionCmdFunc(t *testing.T) {
+	t.Parallel()
 	cmd := zedtesting.CreateTestCobraCommandWithFlagValue(t, zedtesting.StringFlag{FlagName: "prefix-filter", FlagValue: "test"})
 	backupName := createTestBackup(t, testSchema, testRelationships)
 	f, err := os.CreateTemp(t.TempDir(), "parse-output")
@@ -209,6 +212,7 @@ func TestBackupParseRevisionCmdFunc(t *testing.T) {
 }
 
 func TestBackupParseSchemaCmdFunc(t *testing.T) {
+	t.Parallel()
 	for _, tt := range []struct {
 		name          string
 		filter        string
@@ -727,8 +731,8 @@ func TestTakeBackupRecoversFromRetryableErrors(t *testing.T) {
 	require.NoError(t, err)
 
 	require.Len(t, actualRels, 2, "expecting two rels in the realized list")
-	require.Equal(t, actualRels[0].Resource.ObjectId, "foo")
-	require.Equal(t, actualRels[1].Resource.ObjectId, "bar")
+	require.Equal(t, "foo", actualRels[0].Resource.ObjectId)
+	require.Equal(t, "bar", actualRels[1].Resource.ObjectId)
 
 	client.assertAllRecvCalls()
 }

internal/cmd/cmd.go

@@ -183,11 +183,12 @@ func handleError(command *cobra.Command, err error) error {
 		cmdToExecute = command
 	}
 
-	if errors.Is(err, commands.ValidationError{}) {
+	switch {
+	case errors.Is(err, commands.ValidationError{}):
 		_ = flagError(cmdToExecute, err)
-	} else if err != nil && strings.Contains(err.Error(), "unknown command") {
+	case err != nil && strings.Contains(err.Error(), "unknown command"):
 		_ = flagError(cmdToExecute, err)
-	} else if !errors.Is(err, errParsing) {
+	case !errors.Is(err, errParsing):
 		log.Err(err).Msg("terminated with errors")
 	}
 

internal/cmd/cmd_test.go

@@ -79,7 +79,7 @@ func TestCommandOutput(t *testing.T) {
 			if tt.expectStdErrorMsg != "" {
 				require.Contains(t, string(stdErrBytes), tt.expectStdErrorMsg)
 			} else {
-				require.Len(t, stdErrBytes, 0)
+				require.Empty(t, stdErrBytes)
 			}
 			require.Equal(t, tt.expectFlagErrorCalled, flagErrorCalled)
 		})

internal/cmd/import_test.go

@@ -4,6 +4,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	v1 "github.com/authzed/authzed-go/proto/authzed/api/v1"
@@ -29,7 +30,7 @@ func TestImportCmdHappyPath(t *testing.T) {
 	ctx := t.Context()
 	srv := zedtesting.NewTestServer(ctx, t)
 	go func() {
-		require.NoError(srv.Run(ctx))
+		assert.NoError(t, srv.Run(ctx))
 	}()
 	conn, err := srv.GRPCDialContext(ctx)
 	require.NoError(err)
@@ -57,5 +58,5 @@ func TestImportCmdHappyPath(t *testing.T) {
 		Resource:    &v1.ObjectReference{ObjectType: "resource", ObjectId: "1"},
 	})
 	require.NoError(err)
-	require.Equal(resp.Permissionship, v1.CheckPermissionResponse_PERMISSIONSHIP_HAS_PERMISSION)
+	require.Equal(v1.CheckPermissionResponse_PERMISSIONSHIP_HAS_PERMISSION, resp.Permissionship)
 }

internal/cmd/restorer.go

@@ -260,7 +260,7 @@ func (r *restorer) commitStream(ctx context.Context, bulkImportClient v1.Permiss
 		r.writtenRels += numLoaded
 	case conflict && r.conflictStrategy == Fail:
 		r.bar.Describe("conflict detected, aborting restore")
-		return fmt.Errorf("duplicate relationships found")
+		return errors.New("duplicate relationships found")
 	case retryable:
 		r.bar.Describe("retrying after error")
 		r.totalRetries++
@@ -291,7 +291,7 @@ func (r *restorer) commitStream(ctx context.Context, bulkImportClient v1.Permiss
 
 	writtenAndSkipped, err := safecast.Convert[int64](r.writtenRels + r.skippedRels)
 	if err != nil {
-		return fmt.Errorf("too many written and skipped rels for an int64")
+		return errors.New("too many written and skipped rels for an int64")
 	}
 
 	if err := r.bar.Set64(writtenAndSkipped); err != nil {

internal/cmd/restorer_test.go

@@ -104,14 +104,17 @@ func TestRestorer(t *testing.T) {
 			expectedRetries := uint(0)
 			var expectsError error
 			for _, err := range tt.commitErrors {
-				if isRetryableError(err) {
-					expectedRetries++
-					if tt.disableRetryErrors {
-						expectsError = err
+				switch {
+				case isRetryableError(err):
+					{
+						expectedRetries++
+						if tt.disableRetryErrors {
+							expectsError = err
+						}
 					}
-				} else if isAlreadyExistsError(err) {
+				case isAlreadyExistsError(err):
 					expectedConflicts++
-				} else {
+				default:
 					expectsError = err
 				}
 			}

internal/cmd/schema_test.go

@@ -128,65 +128,69 @@ caveat test/some_caveat(someCondition int) {
 
 func TestSchemaCompile(t *testing.T) {
 	t.Parallel()
+	require := require.New(t)
 
-	testCases := map[string]struct {
-		files     []string
-		out       string
-		expectErr error
-		expectStr string
-	}{
-		`file_not_found`: {
-			files: []string{
-				filepath.Join("preview-test", "nonexistent.zed"),
-			},
-			expectErr: fs.ErrNotExist,
-		},
-		`happy_path`: {
-			files: []string{
-				filepath.Join("preview-test", "composable-schema-root.zed"),
-			},
-			expectStr: `definition user {}
+	files := []string{filepath.Join("preview-test", "composable-schema-root.zed")}
+	expected := `definition user {}
 
 definition resource {
 	relation user: user
 	permission view = user
 }
-`,
-		},
-		`cannot_be_compiled_because_using_reserved_keyword`: {
-			files: []string{
-				filepath.Join("preview-test", "composable-schema-invalid-root.zed"),
-			},
-			expectErr: compiler.BaseCompilerError{},
-		},
-	}
+`
 
-	for name, tc := range testCases {
-		t.Run(name, func(t *testing.T) {
-			t.Parallel()
-			require := require.New(t)
+	tempOutFile := filepath.Join(t.TempDir(), "out.zed")
+	cmd := zedtesting.CreateTestCobraCommandWithFlagValue(t,
+		zedtesting.StringFlag{FlagName: "out", FlagValue: tempOutFile})
 
-			tempOutFile := filepath.Join(t.TempDir(), "out.zed")
-			cmd := zedtesting.CreateTestCobraCommandWithFlagValue(t,
-				zedtesting.StringFlag{FlagName: "out", FlagValue: tempOutFile})
-
-			mockTermCheckerr := &mockTermChecker{returnVal: false}
-			err := schemaCompileCmdFunc(cmd, tc.files, mockTermCheckerr)
-			if tc.expectErr == nil {
-				require.NoError(err)
-				tempOutString, err := os.ReadFile(tempOutFile)
-				require.NoError(err)
-				require.Equal(tc.expectStr, string(tempOutString))
-				// TODO re-enable after adding a test that uses stdout
-				// require.Equal(int(os.Stdout.Fd()), mockTermCheckerr.capturedFd, "expected stdout to be checked for terminal")
-			} else {
-				require.Error(err)
-				require.ErrorAs(err, &tc.expectErr)
-			}
-		})
-	}
+	mockTermCheckerr := &mockTermChecker{returnVal: false}
+	err := schemaCompileCmdFunc(cmd, files, mockTermCheckerr)
+
+	require.NoError(err)
+	tempOutString, err := os.ReadFile(tempOutFile)
+	require.NoError(err)
+	require.Equal(expected, string(tempOutString))
+	// TODO re-enable after adding a test that uses stdout
+	// require.Equal(int(os.Stdout.Fd()), mockTermCheckerr.capturedFd, "expected stdout to be checked for terminal")
+}
+
+func TestSchemaCompileFileNotFound(t *testing.T) {
+	t.Parallel()
+	require := require.New(t)
+
+	files := []string{filepath.Join("preview-test", "nonexistent.zed")}
+
+	tempOutFile := filepath.Join(t.TempDir(), "out.zed")
+	cmd := zedtesting.CreateTestCobraCommandWithFlagValue(t,
+		zedtesting.StringFlag{FlagName: "out", FlagValue: tempOutFile})
+
+	mockTermCheckerr := &mockTermChecker{returnVal: false}
+	err := schemaCompileCmdFunc(cmd, files, mockTermCheckerr)
+	require.Error(err)
+	require.ErrorIs(err, fs.ErrNotExist)
+}
+
+func TestSchemaCompileFailureFromReservedKeyword(t *testing.T) {
+	t.Parallel()
+	require := require.New(t)
+
+	files := []string{filepath.Join("preview-test", "composable-schema-invalid-root.zed")}
+	var expectedErr compiler.BaseCompilerError
+
+	tempOutFile := filepath.Join(t.TempDir(), "out.zed")
+	cmd := zedtesting.CreateTestCobraCommandWithFlagValue(t,
+		zedtesting.StringFlag{FlagName: "out", FlagValue: tempOutFile})
+
+	mockTermCheckerr := &mockTermChecker{returnVal: false}
+	err := schemaCompileCmdFunc(cmd, files, mockTermCheckerr)
+	require.Error(err)
+	require.ErrorAs(err, &expectedErr)
 }
 
+// TODO: refactor the impl function to provide a pipe or buffer directly and delegate the input selection to
+// another function
+//
+//nolint:tparallel // these tests can't be parallel because they muck around with the definition of os.Stdin.
 func TestSchemaWrite(t *testing.T) {
 	t.Parallel()