Add revive linter

.golangci.yaml

@@ -10,6 +10,7 @@ linters:
     - intrange
     - mirror
     - perfsprint
+    - revive
     - staticcheck
     - testifylint
     - unconvert
@@ -27,6 +28,24 @@ linters:
         - (*github.com/spf13/cobra.Command).MarkFlagRequired
         - (*github.com/spf13/pflag.FlagSet).MarkDeprecated
         - (*github.com/spf13/pflag.FlagSet).MarkHidden
+    revive:
+      rules:
+        - name: blank-imports
+        - name: context-as-argument
+          exclude:
+            - TEST
+            - "**/integration/**"
+            - "**/internal/testcli/**"
+            - "**/internal/testutil/**"
+            - "**/libs/testdiff/**"
+            - "**/libs/python/pythontest/**"
+        - name: context-keys-type
+        - name: error-return
+        - name: errorf
+        - name: range-val-in-closure
+        - name: waitgroup-by-value
+        - name: unconditional-recursion
+        - name: use-waitgroup-go
     gocritic:
       disable-all: true
       enabled-checks:

bundle/internal/validation/enum.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
+	"go/format"
 	"os"
 	"path/filepath"
 	"reflect"
@@ -236,9 +237,16 @@ func generateEnumFields(outPath string) error {
 		return fmt.Errorf("failed to execute template: %w", err)
 	}
 
+	// Format the generated source with gofmt-equivalent rules so minor template
+	// whitespace changes don't create noisy diffs in the checked-in artifacts.
+	formatted, err := format.Source(generatedCode.Bytes())
+	if err != nil {
+		return fmt.Errorf("failed to format generated code: %w", err)
+	}
+
 	// Write generated code to file
 	filePath := filepath.Join(outPath, "enum_fields.go")
-	if err := os.WriteFile(filePath, generatedCode.Bytes(), 0o644); err != nil {
+	if err := os.WriteFile(filePath, formatted, 0o644); err != nil {
 		return fmt.Errorf("failed to write generated code: %w", err)
 	}
 
@@ -251,10 +259,6 @@ const enumValidationTemplate = `package generated
 // THIS FILE IS AUTOGENERATED.
 // DO NOT EDIT THIS FILE DIRECTLY.
 
-import (
-	_ "github.com/databricks/cli/libs/dyn"
-)
-
 // EnumFields maps [dyn.Pattern] to valid enum values they should have.
 var EnumFields = map[string][]string{
 {{- range . }}

bundle/internal/validation/required.go

@@ -3,6 +3,7 @@ package main
 import (
 	"bytes"
 	"fmt"
+	"go/format"
 	"os"
 	"path/filepath"
 	"reflect"
@@ -185,9 +186,16 @@ func generateRequiredFields(outPath string) error {
 		return fmt.Errorf("failed to execute template: %w", err)
 	}
 
+	// Format the generated source with gofmt-equivalent rules so minor template
+	// whitespace changes don't create noisy diffs in the checked-in artifacts.
+	formatted, err := format.Source(generatedCode.Bytes())
+	if err != nil {
+		return fmt.Errorf("failed to format generated code: %w", err)
+	}
+
 	// Write generated code to file
 	filePath := filepath.Join(outPath, "required_fields.go")
-	if err := os.WriteFile(filePath, generatedCode.Bytes(), 0o644); err != nil {
+	if err := os.WriteFile(filePath, formatted, 0o644); err != nil {
 		return fmt.Errorf("failed to write generated code: %w", err)
 	}
 
@@ -200,10 +208,6 @@ const validationTemplate = `package generated
 // THIS FILE IS AUTOGENERATED.
 // DO NOT EDIT THIS FILE DIRECTLY.
 
-import (
-	_ "github.com/databricks/cli/libs/dyn"
-)
-
 // RequiredFields maps [dyn.Pattern] to required fields they should have.
 var RequiredFields = map[string][]string{
 {{- range . }}

bundle/mutator_read_only.go

@@ -38,15 +38,13 @@ func ApplyParallel(ctx context.Context, b *Bundle, mutators ...ReadOnlyMutator)
 	}
 
 	for ind, m := range mutators {
-		wg.Add(1)
-		go func() {
-			defer wg.Done()
+		wg.Go(func() {
 			// We're not using bundle.ApplyContext here because we don't do copy between typed and dynamic values
 			diags := m.Apply(contexts[ind], b)
 			for _, d := range diags {
 				logdiag.LogDiag(ctx, d)
 			}
-		}()
+		})
 	}
 
 	wg.Wait()

cmd/auth/profiles.go

@@ -115,14 +115,12 @@ func newProfilesCommand() *cobra.Command {
 			if profile.IsEmpty() {
 				continue
 			}
-			wg.Add(1)
-			go func() {
+			wg.Go(func() {
 				ctx := cmd.Context()
 				t := time.Now()
 				profile.Load(ctx, iniFile.Path(), skipValidate)
 				log.Debugf(ctx, "Profile %q took %s to load", profile.Name, time.Since(t))
-				wg.Done()
-			}()
+			})
 			profiles = append(profiles, profile)
 		}
 		wg.Wait()

cmd/auth/token.go

@@ -129,8 +129,8 @@ func loadToken(ctx context.Context, args loadTokenArgs) (*oauth2.Token, error) {
 			// This is captured in an acceptance test under "cmd/auth/token".
 			err = errors.New("cache: databricks OAuth is not configured for this host")
 		}
-		if err, ok := auth.RewriteAuthError(ctx, args.authArguments.Host, args.authArguments.AccountID, args.profileName, err); ok {
-			return nil, err
+		if rewritten, rewrittenErr := auth.RewriteAuthError(ctx, args.authArguments.Host, args.authArguments.AccountID, args.profileName, err); rewritten {
+			return nil, rewrittenErr
 		}
 		helpMsg := helpfulError(ctx, args.profileName, oauthArgument)
 		return nil, fmt.Errorf("%w. %s", err, helpMsg)

cmd/root/auth.go

@@ -314,6 +314,8 @@ func emptyHttpRequest(ctx context.Context) *http.Request {
 }
 
 func renderError(ctx context.Context, cfg *config.Config, err error) error {
-	err, _ = auth.RewriteAuthError(ctx, cfg.Host, cfg.AccountID, cfg.Profile, err)
+	if rewritten, newErr := auth.RewriteAuthError(ctx, cfg.Host, cfg.AccountID, cfg.Profile, err); rewritten {
+		return newErr
+	}
 	return err
 }

experimental/ssh/internal/proxy/client_server_test.go

@@ -165,9 +165,7 @@ func TestHandover(t *testing.T) {
 	}
 
 	wg := sync.WaitGroup{}
-	wg.Add(1)
-	go func() {
-		defer wg.Done()
+	wg.Go(func() {
 		handoverCount := 0
 		for {
 			select {
@@ -182,7 +180,7 @@ func TestHandover(t *testing.T) {
 				time.Sleep(time.Millisecond)
 			}
 		}
-	}()
+	})
 
 	wg.Wait()
 

experimental/ssh/internal/proxy/connections_test.go

@@ -91,13 +91,11 @@ func TestConnectionsManager_ThreadSafety(t *testing.T) {
 	var wg sync.WaitGroup
 
 	for i := range numGoroutines {
-		wg.Add(1)
-		go func() {
-			defer wg.Done()
+		wg.Go(func() {
 			for j := range numOperationsPerGoroutine {
 				cm.TryAdd(fmt.Sprintf("conn-%d-%d", i, j), &proxyConnection{})
 			}
-		}()
+		})
 	}
 
 	wg.Wait()
@@ -108,33 +106,27 @@ func TestConnectionsManager_ThreadSafety(t *testing.T) {
 
 	// Now do concurrent gets, removes, and counts
 	for i := range numGoroutines {
-		wg.Add(1)
-		go func() {
-			defer wg.Done()
+		wg.Go(func() {
 			for j := range numOperationsPerGoroutine {
 				cm.Get(fmt.Sprintf("conn-%d-%d", i, j))
 			}
-		}()
+		})
 	}
 
 	for i := range numGoroutines {
-		wg.Add(1)
-		go func() {
-			defer wg.Done()
+		wg.Go(func() {
 			for j := range numOperationsPerGoroutine {
 				cm.Remove(fmt.Sprintf("conn-%d-%d", i, j))
 			}
-		}()
+		})
 	}
 
 	for range numGoroutines {
-		wg.Add(1)
-		go func() {
-			defer wg.Done()
+		wg.Go(func() {
 			for range numOperationsPerGoroutine {
 				cm.Count()
 			}
-		}()
+		})
 	}
 
 	wg.Wait()

integration/libs/locker/locker_test.go

@@ -39,12 +39,10 @@ func TestLock(t *testing.T) {
 
 	var wg sync.WaitGroup
 	for currentIndex := range numConcurrentLocks {
-		wg.Add(1)
-		go func() {
-			defer wg.Done()
+		wg.Go(func() {
 			time.Sleep(time.Duration(rand.Intn(100)) * time.Millisecond)
 			lockerErrs[currentIndex] = lockers[currentIndex].Lock(ctx, false)
-		}()
+		})
 	}
 	wg.Wait()
 

internal/testcli/runner.go

@@ -45,10 +45,8 @@ type Runner struct {
 
 func consumeLines(ctx context.Context, wg *sync.WaitGroup, r io.Reader) <-chan string {
 	ch := make(chan string, 30000)
-	wg.Add(1)
-	go func() {
+	wg.Go(func() {
 		defer close(ch)
-		defer wg.Done()
 		scanner := bufio.NewScanner(r)
 		for scanner.Scan() {
 			// We expect to be able to always send these lines into the channel.
@@ -63,7 +61,7 @@ func consumeLines(ctx context.Context, wg *sync.WaitGroup, r io.Reader) <-chan s
 				panic("line buffer is full")
 			}
 		}
-	}()
+	})
 	return ch
 }
 
@@ -250,11 +248,9 @@ func (r *Runner) Eventually(condition func() bool, waitFor, tick time.Duration,
 	defer wg.Wait()
 
 	// Kick off condition check immediately.
-	wg.Add(1)
-	go func() {
-		defer wg.Done()
+	wg.Go(func() {
 		ch <- condition()
-	}()
+	})
 
 	for tick := ticker.C; ; {
 		select {
@@ -266,11 +262,9 @@ func (r *Runner) Eventually(condition func() bool, waitFor, tick time.Duration,
 			return
 		case <-tick:
 			tick = nil
-			wg.Add(1)
-			go func() {
-				defer wg.Done()
+			wg.Go(func() {
 				ch <- condition()
-			}()
+			})
 		case v := <-ch:
 			if v {
 				return

libs/auth/error.go

@@ -9,19 +9,19 @@ import (
 )
 
 // RewriteAuthError rewrites the error message for invalid refresh token error.
-// It returns the rewritten error and a boolean indicating whether the error was rewritten.
-func RewriteAuthError(ctx context.Context, host, accountId, profile string, err error) (error, bool) {
+// It returns whether the error was rewritten and the rewritten error.
+func RewriteAuthError(ctx context.Context, host, accountId, profile string, err error) (bool, error) {
 	target := &u2m.InvalidRefreshTokenError{}
 	if errors.As(err, &target) {
 		oauthArgument, err := AuthArguments{host, accountId}.ToOAuthArgument()
 		if err != nil {
-			return err, false
+			return false, err
 		}
 		msg := `A new access token could not be retrieved because the refresh token is invalid. To reauthenticate, run the following command:
   $ ` + BuildLoginCommand(ctx, profile, oauthArgument)
-		return errors.New(msg), true
+		return true, errors.New(msg)
 	}
-	return err, false
+	return false, err
 }
 
 // BuildLoginCommand builds the login command for the given OAuth argument or profile.

libs/cmdio/render.go

@@ -227,7 +227,7 @@ func newBufferedFlusher(w io.Writer) writeFlusher {
 	}
 }
 
-func renderWithTemplate(r any, ctx context.Context, outputFormat flags.Output, w io.Writer, headerTemplate, template string) error {
+func renderWithTemplate(ctx context.Context, r any, outputFormat flags.Output, w io.Writer, headerTemplate, template string) error {
 	// TODO: add terminal width & white/dark theme detection
 	switch outputFormat {
 	case flags.OutputJSON:
@@ -260,30 +260,30 @@ func Render(ctx context.Context, v any) error {
 	if _, ok := v.(listingInterface); ok {
 		panic("use RenderIterator instead")
 	}
-	return renderWithTemplate(newRenderer(v), ctx, c.outputFormat, c.out, c.headerTemplate, c.template)
+	return renderWithTemplate(ctx, newRenderer(v), c.outputFormat, c.out, c.headerTemplate, c.template)
 }
 
 func RenderIterator[T any](ctx context.Context, i listing.Iterator[T]) error {
 	c := fromContext(ctx)
-	return renderWithTemplate(newIteratorRenderer(i), ctx, c.outputFormat, c.out, c.headerTemplate, c.template)
+	return renderWithTemplate(ctx, newIteratorRenderer(i), c.outputFormat, c.out, c.headerTemplate, c.template)
 }
 
 func RenderWithTemplate(ctx context.Context, v any, headerTemplate, template string) error {
 	c := fromContext(ctx)
 	if _, ok := v.(listingInterface); ok {
 		panic("use RenderIteratorWithTemplate instead")
 	}
-	return renderWithTemplate(newRenderer(v), ctx, c.outputFormat, c.out, headerTemplate, template)
+	return renderWithTemplate(ctx, newRenderer(v), c.outputFormat, c.out, headerTemplate, template)
 }
 
 func RenderIteratorWithTemplate[T any](ctx context.Context, i listing.Iterator[T], headerTemplate, template string) error {
 	c := fromContext(ctx)
-	return renderWithTemplate(newIteratorRenderer(i), ctx, c.outputFormat, c.out, headerTemplate, template)
+	return renderWithTemplate(ctx, newIteratorRenderer(i), c.outputFormat, c.out, headerTemplate, template)
 }
 
 func RenderIteratorJson[T any](ctx context.Context, i listing.Iterator[T]) error {
 	c := fromContext(ctx)
-	return renderWithTemplate(newIteratorRenderer(i), ctx, c.outputFormat, c.out, c.headerTemplate, c.template)
+	return renderWithTemplate(ctx, newIteratorRenderer(i), c.outputFormat, c.out, c.headerTemplate, c.template)
 }
 
 var renderFuncMap = template.FuncMap{