fix: use *bool pointers for config boolean fields (closes #134)

This fixes the boolean merging logic issue where boolean fields could
not be explicitly set to false via environment variables to override
config file values.

Changes:
- Change all boolean config fields from `bool` to `*bool` pointers:
  - Format.UppercaseKeywords, Format.Compact
  - Validation.StrictMode, Validation.Recursive
  - Output.Verbose
  - Analyze.Security, Analyze.Performance, Analyze.Complexity, Analyze.All
  - Server.MetricsEnabled

- Add helper functions in config.go:
  - Bool(v bool) *bool - creates a pointer to a bool
  - BoolValue(p *bool) bool - returns false if nil
  - BoolValueOr(p *bool, defaultVal bool) bool - returns default if nil

- Update mergeInto() to use != nil checks instead of truthy checks,
  allowing explicit false values to override defaults

- Update LoadFromEnvironment() to use Bool() helper
- Update ToLSPSettings/FromLSPSettings to use BoolValue()/Bool()
- Update all tests to use BoolValue() for assertions

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

Author: 2 people

pkg/config/config.go

@@ -5,6 +5,28 @@ import (
 	"time"
 )
 
+// Bool returns a pointer to the given bool value.
+// Use this helper when setting boolean config values.
+func Bool(v bool) *bool {
+	return &v
+}
+
+// BoolValue returns the value of a bool pointer, or false if nil.
+func BoolValue(p *bool) bool {
+	if p == nil {
+		return false
+	}
+	return *p
+}
+
+// BoolValueOr returns the value of a bool pointer, or the default if nil.
+func BoolValueOr(p *bool, defaultVal bool) bool {
+	if p == nil {
+		return defaultVal
+	}
+	return *p
+}
+
 // Config represents unified GoSQLX configuration that can be shared across
 // CLI, LSP server, and VSCode extension. It supports loading from files,
 // environment variables, and LSP initialization options.
@@ -20,17 +42,17 @@ type Config struct {
 
 // FormatConfig holds SQL formatting options
 type FormatConfig struct {
-	Indent            int  `yaml:"indent" json:"indent"`                        // Number of spaces for indentation (default: 2)
-	UppercaseKeywords bool `yaml:"uppercase_keywords" json:"uppercaseKeywords"` // Convert SQL keywords to uppercase (default: true)
-	MaxLineLength     int  `yaml:"max_line_length" json:"maxLineLength"`        // Maximum line length before wrapping (default: 120)
-	Compact           bool `yaml:"compact" json:"compact"`                      // Use compact formatting (default: false)
+	Indent            int   `yaml:"indent" json:"indent"`                        // Number of spaces for indentation (default: 2)
+	UppercaseKeywords *bool `yaml:"uppercase_keywords" json:"uppercaseKeywords"` // Convert SQL keywords to uppercase (default: true)
+	MaxLineLength     int   `yaml:"max_line_length" json:"maxLineLength"`        // Maximum line length before wrapping (default: 120)
+	Compact           *bool `yaml:"compact" json:"compact"`                      // Use compact formatting (default: false)
 }
 
 // ValidationConfig holds SQL validation options
 type ValidationConfig struct {
 	Dialect    string         `yaml:"dialect" json:"dialect"`        // SQL dialect: postgresql, mysql, sqlserver, oracle, sqlite (default: "postgresql")
-	StrictMode bool           `yaml:"strict_mode" json:"strictMode"` // Enable strict validation mode (default: false)
-	Recursive  bool           `yaml:"recursive" json:"recursive"`    // Recursively validate files in directories (default: false)
+	StrictMode *bool          `yaml:"strict_mode" json:"strictMode"` // Enable strict validation mode (default: false)
+	Recursive  *bool          `yaml:"recursive" json:"recursive"`    // Recursively validate files in directories (default: false)
 	Pattern    string         `yaml:"pattern" json:"pattern"`        // File pattern for recursive validation (default: "*.sql")
 	Security   SecurityConfig `yaml:"security" json:"security"`      // Security validation settings
 }
@@ -43,15 +65,15 @@ type SecurityConfig struct {
 // OutputConfig holds output formatting options
 type OutputConfig struct {
 	Format  string `yaml:"format" json:"format"`   // Output format: text, json, yaml (default: "text")
-	Verbose bool   `yaml:"verbose" json:"verbose"` // Enable verbose output (default: false)
+	Verbose *bool  `yaml:"verbose" json:"verbose"` // Enable verbose output (default: false)
 }
 
 // AnalyzeConfig holds analysis options
 type AnalyzeConfig struct {
-	Security    bool `yaml:"security" json:"security"`       // Enable security analysis (default: false)
-	Performance bool `yaml:"performance" json:"performance"` // Enable performance analysis (default: false)
-	Complexity  bool `yaml:"complexity" json:"complexity"`   // Enable complexity analysis (default: false)
-	All         bool `yaml:"all" json:"all"`                 // Enable all analysis types (default: false)
+	Security    *bool `yaml:"security" json:"security"`       // Enable security analysis (default: false)
+	Performance *bool `yaml:"performance" json:"performance"` // Enable performance analysis (default: false)
+	Complexity  *bool `yaml:"complexity" json:"complexity"`   // Enable complexity analysis (default: false)
+	All         *bool `yaml:"all" json:"all"`                 // Enable all analysis types (default: false)
 }
 
 // LSPConfig holds LSP server-specific settings
@@ -68,7 +90,7 @@ type LSPConfig struct {
 type ServerConfig struct {
 	LogLevel        string        `yaml:"log_level" json:"logLevel"`               // Log level: debug, info, warn, error (default: "info")
 	LogFile         string        `yaml:"log_file" json:"logFile"`                 // Log file path (default: "" for stderr)
-	MetricsEnabled  bool          `yaml:"metrics_enabled" json:"metricsEnabled"`   // Enable metrics collection (default: true)
+	MetricsEnabled  *bool         `yaml:"metrics_enabled" json:"metricsEnabled"`   // Enable metrics collection (default: true)
 	ShutdownTimeout time.Duration `yaml:"shutdown_timeout" json:"shutdownTimeout"` // Graceful shutdown timeout (default: 5s)
 }
 
@@ -77,28 +99,28 @@ func DefaultConfig() *Config {
 	return &Config{
 		Format: FormatConfig{
 			Indent:            2,
-			UppercaseKeywords: true,
+			UppercaseKeywords: Bool(true),
 			MaxLineLength:     120,
-			Compact:           false,
+			Compact:           Bool(false),
 		},
 		Validation: ValidationConfig{
 			Dialect:    "postgresql",
-			StrictMode: false,
-			Recursive:  false,
+			StrictMode: Bool(false),
+			Recursive:  Bool(false),
 			Pattern:    "*.sql",
 			Security: SecurityConfig{
 				MaxFileSize: 10 * 1024 * 1024, // 10MB
 			},
 		},
 		Output: OutputConfig{
 			Format:  "text",
-			Verbose: false,
+			Verbose: Bool(false),
 		},
 		Analyze: AnalyzeConfig{
-			Security:    false,
-			Performance: false,
-			Complexity:  false,
-			All:         false,
+			Security:    Bool(false),
+			Performance: Bool(false),
+			Complexity:  Bool(false),
+			All:         Bool(false),
 		},
 		LSP: LSPConfig{
 			RateLimitRequests: 100,
@@ -111,7 +133,7 @@ func DefaultConfig() *Config {
 		Server: ServerConfig{
 			LogLevel:        "info",
 			LogFile:         "",
-			MetricsEnabled:  true,
+			MetricsEnabled:  Bool(true),
 			ShutdownTimeout: 5 * time.Second,
 		},
 		Source: "default",

pkg/config/config_test.go

@@ -12,7 +12,7 @@ func TestDefaultConfig(t *testing.T) {
 	if cfg.Format.Indent != 2 {
 		t.Errorf("expected indent=2, got %d", cfg.Format.Indent)
 	}
-	if !cfg.Format.UppercaseKeywords {
+	if !BoolValue(cfg.Format.UppercaseKeywords) {
 		t.Error("expected uppercase_keywords=true")
 	}
 	if cfg.Format.MaxLineLength != 120 {
@@ -50,7 +50,7 @@ func TestDefaultConfig(t *testing.T) {
 	if cfg.Server.LogLevel != "info" {
 		t.Errorf("expected log_level=info, got %s", cfg.Server.LogLevel)
 	}
-	if !cfg.Server.MetricsEnabled {
+	if !BoolValue(cfg.Server.MetricsEnabled) {
 		t.Error("expected metrics_enabled=true")
 	}
 

pkg/config/loader.go

@@ -109,7 +109,7 @@ func LoadFromEnvironment(prefix string) (*Config, error) {
 	}
 	if v := os.Getenv(prefix + "FORMAT_UPPERCASE_KEYWORDS"); v != "" {
 		if b, err := strconv.ParseBool(v); err == nil {
-			config.Format.UppercaseKeywords = b
+			config.Format.UppercaseKeywords = Bool(b)
 		}
 	}
 	if v := os.Getenv(prefix + "FORMAT_MAX_LINE_LENGTH"); v != "" {
@@ -119,7 +119,7 @@ func LoadFromEnvironment(prefix string) (*Config, error) {
 	}
 	if v := os.Getenv(prefix + "FORMAT_COMPACT"); v != "" {
 		if b, err := strconv.ParseBool(v); err == nil {
-			config.Format.Compact = b
+			config.Format.Compact = Bool(b)
 		}
 	}
 
@@ -129,12 +129,12 @@ func LoadFromEnvironment(prefix string) (*Config, error) {
 	}
 	if v := os.Getenv(prefix + "VALIDATION_STRICT_MODE"); v != "" {
 		if b, err := strconv.ParseBool(v); err == nil {
-			config.Validation.StrictMode = b
+			config.Validation.StrictMode = Bool(b)
 		}
 	}
 	if v := os.Getenv(prefix + "VALIDATION_RECURSIVE"); v != "" {
 		if b, err := strconv.ParseBool(v); err == nil {
-			config.Validation.Recursive = b
+			config.Validation.Recursive = Bool(b)
 		}
 	}
 	if v := os.Getenv(prefix + "VALIDATION_PATTERN"); v != "" {
@@ -152,29 +152,29 @@ func LoadFromEnvironment(prefix string) (*Config, error) {
 	}
 	if v := os.Getenv(prefix + "OUTPUT_VERBOSE"); v != "" {
 		if b, err := strconv.ParseBool(v); err == nil {
-			config.Output.Verbose = b
+			config.Output.Verbose = Bool(b)
 		}
 	}
 
 	// Analyze settings
 	if v := os.Getenv(prefix + "ANALYZE_SECURITY"); v != "" {
 		if b, err := strconv.ParseBool(v); err == nil {
-			config.Analyze.Security = b
+			config.Analyze.Security = Bool(b)
 		}
 	}
 	if v := os.Getenv(prefix + "ANALYZE_PERFORMANCE"); v != "" {
 		if b, err := strconv.ParseBool(v); err == nil {
-			config.Analyze.Performance = b
+			config.Analyze.Performance = Bool(b)
 		}
 	}
 	if v := os.Getenv(prefix + "ANALYZE_COMPLEXITY"); v != "" {
 		if b, err := strconv.ParseBool(v); err == nil {
-			config.Analyze.Complexity = b
+			config.Analyze.Complexity = Bool(b)
 		}
 	}
 	if v := os.Getenv(prefix + "ANALYZE_ALL"); v != "" {
 		if b, err := strconv.ParseBool(v); err == nil {
-			config.Analyze.All = b
+			config.Analyze.All = Bool(b)
 		}
 	}
 
@@ -217,7 +217,7 @@ func LoadFromEnvironment(prefix string) (*Config, error) {
 	}
 	if v := os.Getenv(prefix + "SERVER_METRICS_ENABLED"); v != "" {
 		if b, err := strconv.ParseBool(v); err == nil {
-			config.Server.MetricsEnabled = b
+			config.Server.MetricsEnabled = Bool(b)
 		}
 	}
 	if v := os.Getenv(prefix + "SERVER_SHUTDOWN_TIMEOUT"); v != "" {
@@ -257,36 +257,32 @@ func Merge(configs ...*Config) *Config {
 	return result
 }
 
-// mergeInto merges src into dst, with non-zero values from src taking precedence.
-//
-// NOTE: Boolean fields are only merged when true. This is a known limitation that
-// prevents setting booleans to false via environment variables to override a config
-// file that has them set to true. A proper fix would require using *bool pointers
-// for all boolean fields to distinguish between "not set" and "explicitly set to false".
-// See: https://github.com/ajitpratap0/GoSQLX/issues/134 for tracking issue.
+// mergeInto merges src into dst, with non-nil/non-zero values from src taking precedence.
+// Boolean fields use *bool pointers to distinguish between "not set" (nil) and
+// "explicitly set to false" (*false), allowing proper override behavior.
 func mergeInto(dst, src *Config) {
 	// Merge Format
 	if src.Format.Indent != 0 {
 		dst.Format.Indent = src.Format.Indent
 	}
-	if src.Format.UppercaseKeywords {
+	if src.Format.UppercaseKeywords != nil {
 		dst.Format.UppercaseKeywords = src.Format.UppercaseKeywords
 	}
 	if src.Format.MaxLineLength != 0 {
 		dst.Format.MaxLineLength = src.Format.MaxLineLength
 	}
-	if src.Format.Compact {
+	if src.Format.Compact != nil {
 		dst.Format.Compact = src.Format.Compact
 	}
 
 	// Merge Validation
 	if src.Validation.Dialect != "" {
 		dst.Validation.Dialect = src.Validation.Dialect
 	}
-	if src.Validation.StrictMode {
+	if src.Validation.StrictMode != nil {
 		dst.Validation.StrictMode = src.Validation.StrictMode
 	}
-	if src.Validation.Recursive {
+	if src.Validation.Recursive != nil {
 		dst.Validation.Recursive = src.Validation.Recursive
 	}
 	if src.Validation.Pattern != "" {
@@ -300,21 +296,21 @@ func mergeInto(dst, src *Config) {
 	if src.Output.Format != "" {
 		dst.Output.Format = src.Output.Format
 	}
-	if src.Output.Verbose {
+	if src.Output.Verbose != nil {
 		dst.Output.Verbose = src.Output.Verbose
 	}
 
 	// Merge Analyze
-	if src.Analyze.Security {
+	if src.Analyze.Security != nil {
 		dst.Analyze.Security = src.Analyze.Security
 	}
-	if src.Analyze.Performance {
+	if src.Analyze.Performance != nil {
 		dst.Analyze.Performance = src.Analyze.Performance
 	}
-	if src.Analyze.Complexity {
+	if src.Analyze.Complexity != nil {
 		dst.Analyze.Complexity = src.Analyze.Complexity
 	}
-	if src.Analyze.All {
+	if src.Analyze.All != nil {
 		dst.Analyze.All = src.Analyze.All
 	}
 
@@ -345,7 +341,7 @@ func mergeInto(dst, src *Config) {
 	if src.Server.LogFile != "" {
 		dst.Server.LogFile = src.Server.LogFile
 	}
-	if src.Server.MetricsEnabled {
+	if src.Server.MetricsEnabled != nil {
 		dst.Server.MetricsEnabled = src.Server.MetricsEnabled
 	}
 	if src.Server.ShutdownTimeout != 0 {

pkg/config/loader_test.go

@@ -210,7 +210,7 @@ func TestLoadFromEnvironment(t *testing.T) {
 	if cfg.Validation.Dialect != "mysql" {
 		t.Errorf("expected dialect=mysql, got %s", cfg.Validation.Dialect)
 	}
-	if !cfg.Validation.StrictMode {
+	if !BoolValue(cfg.Validation.StrictMode) {
 		t.Error("expected strict_mode=true")
 	}
 	if cfg.LSP.RateLimitRequests != 200 {