fix: address architect review — port rules fix, add tests, fix bugs

- Port lintFile rules-filtering logic from PR #312 (allRules + selection)
- Move package-level mutable vars into actionFlags struct
- Replace *string pointers in optimizecmd with cmdutil.RootFlags
- Fix IsValidSQLFileExtension('') returning true
- Add tests for cmdutil package (IsValidSQLFileExtension, ExpandDirectory, LooksLikeSQL)

Author: Ajit Pratap Singh

cmd/gosqlx/cmd/root.go

@@ -4,6 +4,7 @@ import (
 	"github.com/spf13/cobra"
 
 	"github.com/ajitpratap0/GoSQLX/cmd/gosqlx/internal/actioncmd"
+	"github.com/ajitpratap0/GoSQLX/cmd/gosqlx/internal/cmdutil"
 	"github.com/ajitpratap0/GoSQLX/cmd/gosqlx/internal/lspcmd"
 	"github.com/ajitpratap0/GoSQLX/cmd/gosqlx/internal/optimizecmd"
 )
@@ -167,5 +168,9 @@ func init() {
 	// Register subcommands from internal packages
 	rootCmd.AddCommand(lspcmd.NewCmd())
 	rootCmd.AddCommand(actioncmd.NewCmd())
-	rootCmd.AddCommand(optimizecmd.NewCmd(&outputFile, &format))
+	rootCmd.AddCommand(optimizecmd.NewCmd(&cmdutil.RootFlags{
+		Verbose:    &verbose,
+		OutputFile: &outputFile,
+		Format:     &format,
+	}))
 }

cmd/gosqlx/internal/actioncmd/action.go

@@ -36,16 +36,20 @@ func builtinRules() []linter.Rule {
 	}
 }
 
-var (
-	actionFiles    string
-	actionRules    string
-	actionSeverity string
-	actionConfig   string
-	actionTimeout  int
-)
+// actionFlags holds the flag values for the action subcommand,
+// avoiding package-level mutable state.
+type actionFlags struct {
+	files    string
+	rules    string
+	severity string
+	config   string
+	timeout  int
+}
 
 // NewCmd returns the action cobra.Command.
 func NewCmd() *cobra.Command {
+	f := &actionFlags{}
+
 	cmd := &cobra.Command{
 		Use:   "action",
 		Short: "Run GoSQLX checks for GitHub Actions CI",
@@ -61,14 +65,16 @@ Environment variables (also settable via flags):
   SEVERITY   - threshold: error, warning, info (default: warning)
   CONFIG     - path to .gosqlx.yml config file
   TIMEOUT    - per-file timeout in seconds (default: 600)`,
-		RunE: runAction,
+		RunE: func(cmd *cobra.Command, args []string) error {
+			return runAction(cmd, args, f)
+		},
 	}
 
-	cmd.Flags().StringVar(&actionFiles, "files", "", "glob pattern for SQL files (env: SQL_FILES)")
-	cmd.Flags().StringVar(&actionRules, "rules", "", "comma-separated lint rules (env: RULES)")
-	cmd.Flags().StringVar(&actionSeverity, "severity", "", "severity threshold: error, warning, info (env: SEVERITY)")
-	cmd.Flags().StringVar(&actionConfig, "config", "", "path to config file (env: CONFIG)")
-	cmd.Flags().IntVar(&actionTimeout, "timeout", 0, "per-file timeout in seconds (env: TIMEOUT)")
+	cmd.Flags().StringVar(&f.files, "files", "", "glob pattern for SQL files (env: SQL_FILES)")
+	cmd.Flags().StringVar(&f.rules, "rules", "", "comma-separated lint rules (env: RULES)")
+	cmd.Flags().StringVar(&f.severity, "severity", "", "severity threshold: error, warning, info (env: SEVERITY)")
+	cmd.Flags().StringVar(&f.config, "config", "", "path to config file (env: CONFIG)")
+	cmd.Flags().IntVar(&f.timeout, "timeout", 0, "per-file timeout in seconds (env: TIMEOUT)")
 
 	return cmd
 }
@@ -83,13 +89,13 @@ func envDefault(flagVal, envKey, fallback string) string {
 	return fallback
 }
 
-func runAction(_ *cobra.Command, _ []string) error {
-	pattern := envDefault(actionFiles, "SQL_FILES", "**/*.sql")
-	rules := envDefault(actionRules, "RULES", "")
-	severity := envDefault(actionSeverity, "SEVERITY", "warning")
-	cfgPath := envDefault(actionConfig, "CONFIG", "")
+func runAction(_ *cobra.Command, _ []string, f *actionFlags) error {
+	pattern := envDefault(f.files, "SQL_FILES", "**/*.sql")
+	rules := envDefault(f.rules, "RULES", "")
+	severity := envDefault(f.severity, "SEVERITY", "warning")
+	cfgPath := envDefault(f.config, "CONFIG", "")
 
-	timeoutSec := actionTimeout
+	timeoutSec := f.timeout
 	if timeoutSec == 0 {
 		if v := os.Getenv("TIMEOUT"); v != "" {
 			if parsed, err := strconv.Atoi(v); err == nil {

cmd/gosqlx/internal/cmdutil/input.go

@@ -91,7 +91,7 @@ func DetectAndReadInput(input string) (*InputResult, error) {
 // IsValidSQLFileExtension checks if the file extension is acceptable for SQL.
 func IsValidSQLFileExtension(ext string) bool {
 	switch ext {
-	case ".sql", ".txt", "":
+	case ".sql", ".txt":
 		return true
 	default:
 		return false

cmd/gosqlx/internal/cmdutil/input_test.go

@@ -0,0 +1,71 @@
+package cmdutil
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func TestIsValidSQLFileExtension(t *testing.T) {
+	tests := []struct {
+		ext  string
+		want bool
+	}{
+		{".sql", true},
+		{".txt", true},
+		{"", false},
+		{".go", false},
+		{".csv", false},
+		{".SQL", false}, // case-sensitive
+	}
+	for _, tt := range tests {
+		if got := IsValidSQLFileExtension(tt.ext); got != tt.want {
+			t.Errorf("IsValidSQLFileExtension(%q) = %v, want %v", tt.ext, got, tt.want)
+		}
+	}
+}
+
+func TestExpandDirectory(t *testing.T) {
+	dir := t.TempDir()
+
+	// Create test files
+	os.WriteFile(filepath.Join(dir, "a.sql"), []byte("SELECT 1"), 0644)
+	os.WriteFile(filepath.Join(dir, "b.txt"), []byte("SELECT 2"), 0644)
+	os.WriteFile(filepath.Join(dir, "c.go"), []byte("package x"), 0644)
+	os.Mkdir(filepath.Join(dir, "subdir"), 0755)
+
+	files, err := ExpandDirectory(dir)
+	if err != nil {
+		t.Fatalf("ExpandDirectory() error: %v", err)
+	}
+
+	// Should find .sql and .txt but not .go or subdir
+	if len(files) != 2 {
+		t.Errorf("ExpandDirectory() returned %d files, want 2: %v", len(files), files)
+	}
+}
+
+func TestExpandDirectory_NotExist(t *testing.T) {
+	_, err := ExpandDirectory("/nonexistent/path")
+	if err == nil {
+		t.Error("ExpandDirectory() expected error for nonexistent path")
+	}
+}
+
+func TestLooksLikeSQL(t *testing.T) {
+	tests := []struct {
+		input string
+		want  bool
+	}{
+		{"SELECT * FROM t", true},
+		{"INSERT INTO t VALUES (1)", true},
+		{"hello world", false},
+		{"", false},
+		{"CREATE TABLE t (id INT)", true},
+	}
+	for _, tt := range tests {
+		if got := LooksLikeSQL(tt.input); got != tt.want {
+			t.Errorf("LooksLikeSQL(%q) = %v, want %v", tt.input, got, tt.want)
+		}
+	}
+}

cmd/gosqlx/internal/optimizecmd/optimize.go

@@ -16,9 +16,8 @@ import (
 
 // NewCmd returns the optimize cobra.Command.
 //
-// The outputFile and format parameters are pointers to the root command's
-// persistent flag values, allowing this subcommand to access global flags.
-func NewCmd(outputFile *string, format *string) *cobra.Command {
+// rf provides access to the root command's persistent flag values.
+func NewCmd(rf *cmdutil.RootFlags) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "optimize [file|sql]",
 		Short: "Analyze SQL for optimization opportunities",
@@ -48,7 +47,7 @@ Output includes an optimization score (0-100), complexity classification,
 and detailed suggestions.`,
 		Args: cobra.MaximumNArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return runOptimize(cmd, args, outputFile, format)
+			return runOptimize(cmd, args, rf.OutputFile, rf.Format)
 		},
 	}