Merge pull request #1627 from dgageot/linters

Enable more linters and fix existing issues

Author: dgageot

.golangci.yml

@@ -3,30 +3,62 @@ run:
   tests: true
 linters:
   enable:
+    - asasalint
+    - asciicheck
+    - bidichk
     - containedctx
     - copyloopvar
+    - decorder
     - depguard
+    - dogsled
+    - durationcheck
     - errcheck
+    - errname
+    - errorlint
+    - exptostd
+    - fatcontext
+    - forbidigo
     - ginkgolinter
+    - gocheckcompilerdirectives
+    - gochecknoinits
+    - gochecksumtype
     - gocritic
+    - gomoddirectives
+    - gomodguard
+    - goprintffuncname
     - govet
+    - grouper
+    - iface
     - importas # Enforces consistent import aliases.
+    - inamedparam
     - ineffassign
     - intrange
+    - iotamixing
+    - loggercheck
     - misspell
+    - mirror
     - nakedret
     - nolintlint
+    - nosprintfhostport
+    - nilnesserr
+    - predeclared
+    - reassign
+    - recvcheck
     - revive
+    - rowserrcheck
+    - sloglint
     - staticcheck
+    - testableexamples
     - testifylint
     - thelper
     - unconvert
     - unparam
     - unused
     - usestdlibvars
-    - forbidigo
-    - iotamixing
-    - gochecknoinits
+    - unqueryvet
+    - usetesting
+    - whitespace
+    - wastedassign
   settings:
     forbidigo:
       forbid:
@@ -63,6 +95,9 @@ linters:
           deny:
             - pkg: github.com/stretchr/testify
               desc: don't use testify in production code
+    gomoddirectives:
+      replace-allow-list:
+        - github.com/charmbracelet/ultraviolet
     gocritic:
       disabled-checks:
         - dupImport

cmd/root/run.go

@@ -2,6 +2,7 @@ package root
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"log/slog"
@@ -422,7 +423,8 @@ func (f *runExecFlags) handleExecMode(ctx context.Context, out *cli.Printer, rt
 		OutputJSON:     f.outputJSON,
 		AutoApprove:    f.autoApprove,
 	}, rt, sess, execArgs)
-	if cliErr, ok := err.(cli.RuntimeError); ok {
+	var cliErr cli.RuntimeError
+	if errors.As(err, &cliErr) {
 		return RuntimeError{Err: cliErr.Err}
 	}
 	return err

pkg/app/transcript/transcript.go

@@ -87,7 +87,7 @@ func toJSONString(builder *strings.Builder, in string) {
 	if err := json.Unmarshal([]byte(in), &content); err == nil {
 		if formatted, err := json.MarshalIndent(content, "", "  "); err == nil {
 			builder.WriteString("```json\n")
-			builder.WriteString(string(formatted))
+			builder.Write(formatted)
 			builder.WriteString("\n```\n")
 		} else {
 			builder.WriteString(in)

pkg/config/auto.go

@@ -32,11 +32,11 @@ var cloudProviders = []providerConfig{
 	}, "AWS_ACCESS_KEY_ID (or AWS_PROFILE, AWS_ROLE_ARN, AWS_BEARER_TOKEN_BEDROCK)"},
 }
 
-// ErrAutoModelFallback is returned when auto model selection fails because
+// AutoModelFallbackError is returned when auto model selection fails because
 // no providers are available (no API keys configured and DMR not installed).
-type ErrAutoModelFallback struct{}
+type AutoModelFallbackError struct{}
 
-func (e *ErrAutoModelFallback) Error() string {
+func (e *AutoModelFallbackError) Error() string {
 	var hints []string
 	for _, p := range cloudProviders {
 		hints = append(hints, fmt.Sprintf("    - %s: %s", p.name, p.hint))

pkg/config/config_test.go

@@ -271,7 +271,9 @@ func TestCheckRequiredEnvVars(t *testing.T) {
 				require.NoError(t, err)
 			} else {
 				require.Error(t, err)
-				assert.Equal(t, test.expectedMissing, err.(*environment.RequiredEnvError).Missing)
+				var reqErr *environment.RequiredEnvError
+				require.ErrorAs(t, err, &reqErr)
+				assert.Equal(t, test.expectedMissing, reqErr.Missing)
 			}
 		})
 	}

pkg/config/latest/types.go

@@ -484,7 +484,7 @@ type SandboxConfig struct {
 // DeferConfig represents the deferred loading configuration for a toolset.
 // It can be either a boolean (true to defer all tools) or a slice of strings
 // (list of tool names to defer).
-type DeferConfig struct {
+type DeferConfig struct { //nolint:recvcheck // MarshalYAML must use value receiver for YAML slice encoding, UnmarshalYAML must use pointer
 	// DeferAll is true when all tools should be deferred
 	DeferAll bool `json:"-"`
 	// Tools is the list of specific tool names to defer (empty if DeferAll is true)
@@ -635,7 +635,7 @@ func (c *RAGConfig) GetRespectVCS() bool {
 
 // RAGStrategyConfig represents a single retrieval strategy configuration
 // Strategy-specific fields are stored in Params (validated by strategy implementation)
-type RAGStrategyConfig struct {
+type RAGStrategyConfig struct { //nolint:recvcheck // Marshal methods must use value receiver for YAML/JSON slice encoding, Unmarshal must use pointer
 	Type     string            `json:"type"`               // Strategy type: "chunked-embeddings", "bm25", etc.
 	Docs     []string          `json:"docs,omitempty"`     // Strategy-specific documents (augments shared docs)
 	Database RAGDatabaseConfig `json:"database,omitempty"` // Database configuration

pkg/config/types/commands.go

@@ -112,7 +112,7 @@ func (c *Commands) UnmarshalYAML(unmarshal func(any) error) error {
 
 // parseCommandValue parses a command value which can be either:
 // - a simple string (becomes the instruction)
-// - a map with description/instruction fields
+// - a map with description/instruction fields.
 func parseCommandValue(v any) (Command, error) {
 	switch val := v.(type) {
 	case string:

pkg/config/v0/types.go

@@ -8,7 +8,7 @@ import (
 
 const Version = "0"
 
-// Toolset represents a tool configuration
+// Toolset represents a tool configuration.
 type Toolset struct {
 	Type     string             `json:"type,omitempty" yaml:"type,omitempty"`
 	Command  string             `json:"command,omitempty" yaml:"command,omitempty"`
@@ -25,7 +25,7 @@ type Remote struct {
 	Headers       map[string]string `json:"headers,omitempty" yaml:"headers,omitempty"`
 }
 
-// Ensure that either Command or Remote is set, but not both empty
+// Ensure that either Command or Remote is set, but not both empty.
 func (t *Toolset) validate() error {
 	if t.Type != "mcp" {
 		return nil

pkg/config/v2/types.go

@@ -251,7 +251,7 @@ type RAGConfig struct {
 
 // RAGStrategyConfig represents a single retrieval strategy configuration
 // Strategy-specific fields are stored in Params (validated by strategy implementation)
-type RAGStrategyConfig struct {
+type RAGStrategyConfig struct { //nolint:recvcheck // Marshal methods must use value receiver for YAML/JSON slice encoding, Unmarshal must use pointer
 	Type     string            `json:"type"`               // Strategy type: "chunked-embeddings", "bm25", etc.
 	Docs     []string          `json:"docs,omitempty"`     // Strategy-specific documents (augments shared docs)
 	Database RAGDatabaseConfig `json:"database,omitempty"` // Database configuration

pkg/config/v3/types.go

@@ -230,7 +230,7 @@ type SandboxConfig struct {
 // DeferConfig represents the deferred loading configuration for a toolset.
 // It can be either a boolean (true to defer all tools) or a slice of strings
 // (list of tool names to defer).
-type DeferConfig struct {
+type DeferConfig struct { //nolint:recvcheck // MarshalYAML must use value receiver for YAML slice encoding, UnmarshalYAML must use pointer
 	// DeferAll is true when all tools should be deferred
 	DeferAll bool `json:"-"`
 	// Tools is the list of specific tool names to defer (empty if DeferAll is true)
@@ -381,7 +381,7 @@ func (c *RAGConfig) GetRespectVCS() bool {
 
 // RAGStrategyConfig represents a single retrieval strategy configuration
 // Strategy-specific fields are stored in Params (validated by strategy implementation)
-type RAGStrategyConfig struct {
+type RAGStrategyConfig struct { //nolint:recvcheck // Marshal methods must use value receiver for YAML/JSON slice encoding, Unmarshal must use pointer
 	Type     string            `json:"type"`               // Strategy type: "chunked-embeddings", "bm25", etc.
 	Docs     []string          `json:"docs,omitempty"`     // Strategy-specific documents (augments shared docs)
 	Database RAGDatabaseConfig `json:"database,omitempty"` // Database configuration