Skip to content

Commit 1ae108d

Browse files
ajitpratap0Ajit Pratap Singh
andauthored
refactor: Split cmd/ god package into sub-packages (phase 1) (#314)
* refactor: split cmd/gosqlx/cmd/ god package — extract lsp, action, optimize into sub-packages Extract the most independent subcommands (lsp, action, optimize) from the 13.7K LOC cmd/gosqlx/cmd/ god package into dedicated sub-packages under cmd/gosqlx/internal/: - cmd/gosqlx/internal/lspcmd/ — LSP server command (zero deps on cmd pkg) - cmd/gosqlx/internal/actioncmd/ — GitHub Actions CI command (self-contained) - cmd/gosqlx/internal/optimizecmd/ — SQL optimization command - cmd/gosqlx/internal/cmdutil/ — shared utilities (stdin, input detection, flags) Each sub-package exports a NewCmd() constructor returning *cobra.Command. Root command registers them via rootCmd.AddCommand(). Remaining commands (validate, format, analyze, parse, lint, config, watch) stay in cmd/gosqlx/cmd/ for now due to heavy cross-dependencies between their types (ValidatorOptions, CLIFormatterOptions, etc.). These can be split in follow-up PRs once shared types are extracted to cmdutil. No behavior changes — pure structural refactor. * 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) * fix: move action_test.go to actioncmd package after refactor --------- Co-authored-by: Ajit Pratap Singh <ajitpratapsingh@Ajits-Mac-mini-2655.local>
1 parent f93b3f3 commit 1ae108d

File tree

8 files changed

+525
-97
lines changed

8 files changed

+525
-97
lines changed

cmd/gosqlx/cmd/root.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@ package cmd
22

33
import (
44
"github.com/spf13/cobra"
5+
6+
"github.com/ajitpratap0/GoSQLX/cmd/gosqlx/internal/actioncmd"
7+
"github.com/ajitpratap0/GoSQLX/cmd/gosqlx/internal/cmdutil"
8+
"github.com/ajitpratap0/GoSQLX/cmd/gosqlx/internal/lspcmd"
9+
"github.com/ajitpratap0/GoSQLX/cmd/gosqlx/internal/optimizecmd"
510
)
611

712
// Version is the current version of gosqlx CLI.
@@ -159,4 +164,13 @@ func init() {
159164
rootCmd.PersistentFlags().BoolVarP(&verbose, "verbose", "v", false, "enable verbose output")
160165
rootCmd.PersistentFlags().StringVarP(&outputFile, "output", "o", "", "output file (default: stdout)")
161166
rootCmd.PersistentFlags().StringVarP(&format, "format", "f", "auto", "output format: json, yaml, table, tree, auto")
167+
168+
// Register subcommands from internal packages
169+
rootCmd.AddCommand(lspcmd.NewCmd())
170+
rootCmd.AddCommand(actioncmd.NewCmd())
171+
rootCmd.AddCommand(optimizecmd.NewCmd(&cmdutil.RootFlags{
172+
Verbose: &verbose,
173+
OutputFile: &outputFile,
174+
Format: &format,
175+
}))
162176
}
Lines changed: 36 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
package cmd
1+
// Package actioncmd implements the gosqlx action subcommand for GitHub Actions CI.
2+
package actioncmd
23

34
import (
45
"fmt"
@@ -35,20 +36,24 @@ func builtinRules() []linter.Rule {
3536
}
3637
}
3738

38-
var (
39-
actionFiles string
40-
actionRules string
41-
actionSeverity string
42-
actionConfig string
43-
actionTimeout int
44-
)
39+
// actionFlags holds the flag values for the action subcommand,
40+
// avoiding package-level mutable state.
41+
type actionFlags struct {
42+
files string
43+
rules string
44+
severity string
45+
config string
46+
timeout int
47+
}
48+
49+
// NewCmd returns the action cobra.Command.
50+
func NewCmd() *cobra.Command {
51+
f := &actionFlags{}
4552

46-
// actionCmd implements the GitHub Actions entrypoint as a Go subcommand.
47-
// It finds SQL files, runs lint + validate, and outputs GitHub Actions annotations.
48-
var actionCmd = &cobra.Command{
49-
Use: "action",
50-
Short: "Run GoSQLX checks for GitHub Actions CI",
51-
Long: `Run SQL validation and linting with GitHub Actions annotation output.
53+
cmd := &cobra.Command{
54+
Use: "action",
55+
Short: "Run GoSQLX checks for GitHub Actions CI",
56+
Long: `Run SQL validation and linting with GitHub Actions annotation output.
5257
5358
This command replaces the shell-based entrypoint for the GoSQLX GitHub Action.
5459
It finds SQL files matching a glob pattern, runs validation and linting on each,
@@ -60,20 +65,20 @@ Environment variables (also settable via flags):
6065
SEVERITY - threshold: error, warning, info (default: warning)
6166
CONFIG - path to .gosqlx.yml config file
6267
TIMEOUT - per-file timeout in seconds (default: 600)`,
63-
RunE: runAction,
64-
}
68+
RunE: func(cmd *cobra.Command, args []string) error {
69+
return runAction(cmd, args, f)
70+
},
71+
}
6572

66-
func init() {
67-
actionCmd.Flags().StringVar(&actionFiles, "files", "", "glob pattern for SQL files (env: SQL_FILES)")
68-
actionCmd.Flags().StringVar(&actionRules, "rules", "", "comma-separated lint rules (env: RULES)")
69-
actionCmd.Flags().StringVar(&actionSeverity, "severity", "", "severity threshold: error, warning, info (env: SEVERITY)")
70-
actionCmd.Flags().StringVar(&actionConfig, "config", "", "path to config file (env: CONFIG)")
71-
actionCmd.Flags().IntVar(&actionTimeout, "timeout", 0, "per-file timeout in seconds (env: TIMEOUT)")
73+
cmd.Flags().StringVar(&f.files, "files", "", "glob pattern for SQL files (env: SQL_FILES)")
74+
cmd.Flags().StringVar(&f.rules, "rules", "", "comma-separated lint rules (env: RULES)")
75+
cmd.Flags().StringVar(&f.severity, "severity", "", "severity threshold: error, warning, info (env: SEVERITY)")
76+
cmd.Flags().StringVar(&f.config, "config", "", "path to config file (env: CONFIG)")
77+
cmd.Flags().IntVar(&f.timeout, "timeout", 0, "per-file timeout in seconds (env: TIMEOUT)")
7278

73-
rootCmd.AddCommand(actionCmd)
79+
return cmd
7480
}
7581

76-
// envDefault returns the flag value if non-empty, otherwise the env var, otherwise the fallback.
7782
func envDefault(flagVal, envKey, fallback string) string {
7883
if flagVal != "" {
7984
return flagVal
@@ -84,13 +89,13 @@ func envDefault(flagVal, envKey, fallback string) string {
8489
return fallback
8590
}
8691

87-
func runAction(_ *cobra.Command, _ []string) error {
88-
pattern := envDefault(actionFiles, "SQL_FILES", "**/*.sql")
89-
rules := envDefault(actionRules, "RULES", "")
90-
severity := envDefault(actionSeverity, "SEVERITY", "warning")
91-
cfgPath := envDefault(actionConfig, "CONFIG", "")
92+
func runAction(_ *cobra.Command, _ []string, f *actionFlags) error {
93+
pattern := envDefault(f.files, "SQL_FILES", "**/*.sql")
94+
rules := envDefault(f.rules, "RULES", "")
95+
severity := envDefault(f.severity, "SEVERITY", "warning")
96+
cfgPath := envDefault(f.config, "CONFIG", "")
9297

93-
timeoutSec := actionTimeout
98+
timeoutSec := f.timeout
9499
if timeoutSec == 0 {
95100
if v := os.Getenv("TIMEOUT"); v != "" {
96101
if parsed, err := strconv.Atoi(v); err == nil {
@@ -116,7 +121,6 @@ func runAction(_ *cobra.Command, _ []string) error {
116121
}
117122
}
118123

119-
// Find SQL files
120124
files, err := findSQLFiles(pattern)
121125
if err != nil {
122126
return fmt.Errorf("finding SQL files: %w", err)
@@ -127,7 +131,6 @@ func runAction(_ *cobra.Command, _ []string) error {
127131
}
128132
fmt.Printf("Found %d SQL file(s)\n", len(files))
129133

130-
// Parse rules
131134
var ruleList []string
132135
if rules != "" {
133136
for _, r := range strings.Split(rules, ",") {
@@ -151,7 +154,6 @@ func runAction(_ *cobra.Command, _ []string) error {
151154
for _, file := range files {
152155
displayFile := strings.TrimPrefix(file, "./")
153156

154-
// Validate
155157
vErr := validateFileWithTimeout(file, timeout)
156158
if vErr != nil {
157159
validateErrors++
@@ -171,7 +173,6 @@ func runAction(_ *cobra.Command, _ []string) error {
171173
totalValid++
172174
}
173175

174-
// Lint
175176
violations := lintFile(file, ruleList)
176177
for _, v := range violations {
177178
level := "notice"
@@ -186,7 +187,6 @@ func runAction(_ *cobra.Command, _ []string) error {
186187
}
187188
}
188189

189-
// Summary
190190
fmt.Println()
191191
fmt.Println("==============================")
192192
fmt.Println(" GoSQLX Results Summary")
@@ -198,12 +198,10 @@ func runAction(_ *cobra.Command, _ []string) error {
198198
fmt.Printf(" Lint warnings: %d\n", lintWarnings)
199199
fmt.Println("==============================")
200200

201-
// GitHub step summary
202201
if summaryPath := os.Getenv("GITHUB_STEP_SUMMARY"); summaryPath != "" {
203202
writeStepSummary(summaryPath, len(files), totalValid, validateErrors, lintErrors, lintWarnings)
204203
}
205204

206-
// Exit code based on severity
207205
fail := false
208206
switch severity {
209207
case "error":
@@ -218,7 +216,6 @@ func runAction(_ *cobra.Command, _ []string) error {
218216
return nil
219217
}
220218

221-
// ghAnnotation prints a GitHub Actions annotation.
222219
func ghAnnotation(level, file string, line int, msg string) {
223220
params := ""
224221
if file != "" {
@@ -234,7 +231,6 @@ func ghAnnotation(level, file string, line int, msg string) {
234231
}
235232
}
236233

237-
// extractLineNumber extracts a line number from text matching "line N".
238234
func extractLineNumber(re *regexp.Regexp, text string) int {
239235
m := re.FindStringSubmatch(text)
240236
if len(m) >= 2 {
@@ -244,15 +240,14 @@ func extractLineNumber(re *regexp.Regexp, text string) int {
244240
return 0
245241
}
246242

247-
// findSQLFiles locates SQL files matching the given glob pattern.
248243
func findSQLFiles(pattern string) ([]string, error) {
249244
var files []string
250245

251246
switch {
252247
case pattern == "**/*.sql":
253248
err := filepath.Walk(".", func(path string, info os.FileInfo, err error) error {
254249
if err != nil {
255-
return nil // skip errors
250+
return nil
256251
}
257252
if !info.IsDir() && strings.HasSuffix(strings.ToLower(path), ".sql") {
258253
files = append(files, path)
@@ -273,7 +268,6 @@ func findSQLFiles(pattern string) ([]string, error) {
273268
}
274269
}
275270
default:
276-
// Try filepath.Glob first, fall back to Walk with path matching
277271
matched, err := filepath.Glob(pattern)
278272
if err == nil && len(matched) > 0 {
279273
for _, m := range matched {
@@ -283,7 +277,6 @@ func findSQLFiles(pattern string) ([]string, error) {
283277
}
284278
}
285279
} else {
286-
// Walk and match with filepath.Match
287280
err = filepath.Walk(".", func(path string, info os.FileInfo, walkErr error) error {
288281
if walkErr != nil {
289282
return nil
@@ -303,7 +296,6 @@ func findSQLFiles(pattern string) ([]string, error) {
303296
return files, nil
304297
}
305298

306-
// lintViolation represents a single lint finding.
307299
type lintViolation struct {
308300
Line int
309301
Severity string
@@ -365,7 +357,6 @@ func lintFile(filePath string, ruleList []string) []lintViolation {
365357
return violations
366358
}
367359

368-
// validateFileWithTimeout validates a SQL file with a timeout.
369360
func validateFileWithTimeout(filePath string, timeout time.Duration) error {
370361
type result struct {
371362
err error
@@ -389,7 +380,6 @@ func validateFileWithTimeout(filePath string, timeout time.Duration) error {
389380
}
390381
}
391382

392-
// writeStepSummary appends a markdown summary to the GitHub step summary file.
393383
func writeStepSummary(path string, total, valid, valErrors, lintErrors, lintWarnings int) {
394384
f, err := os.OpenFile(filepath.Clean(path), os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o600)
395385
if err != nil {

cmd/gosqlx/cmd/action_test.go renamed to cmd/gosqlx/internal/actioncmd/action_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package cmd
1+
package actioncmd
22

33
import (
44
"os"
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
package cmdutil
2+
3+
import (
4+
"github.com/spf13/cobra"
5+
"github.com/spf13/pflag"
6+
)
7+
8+
// RootFlags holds pointers to the root command's persistent flag values.
9+
// Sub-packages receive this to access global flags without circular imports.
10+
type RootFlags struct {
11+
Verbose *bool
12+
OutputFile *string
13+
Format *string
14+
}
15+
16+
// Valid output format constants
17+
const (
18+
OutputFormatText = "text"
19+
OutputFormatJSON = "json"
20+
OutputFormatSARIF = "sarif"
21+
)
22+
23+
// ValidOutputFormats lists all supported output formats for validation
24+
var ValidOutputFormats = []string{OutputFormatText, OutputFormatJSON, OutputFormatSARIF}
25+
26+
// TrackChangedFlags returns a map of flag names that were explicitly set on the command line.
27+
// This includes both local flags and parent persistent flags.
28+
func TrackChangedFlags(cmd *cobra.Command) map[string]bool {
29+
flagsChanged := make(map[string]bool)
30+
cmd.Flags().Visit(func(f *pflag.Flag) {
31+
flagsChanged[f.Name] = true
32+
})
33+
if cmd.Parent() != nil && cmd.Parent().PersistentFlags() != nil {
34+
cmd.Parent().PersistentFlags().Visit(func(f *pflag.Flag) {
35+
flagsChanged[f.Name] = true
36+
})
37+
}
38+
return flagsChanged
39+
}

0 commit comments

Comments
 (0)