fix: address architect review — add tests, unify rule registry

Author: Ajit Pratap Singh

cmd/gosqlx/cmd/action.go

@@ -18,6 +18,23 @@ import (
 	"github.com/ajitpratap0/GoSQLX/pkg/sql/parser"
 )
 
+// builtinRules returns all built-in lint rules. This is the single source of
+// truth — both defaultLinter() and allRules() derive from it.
+func builtinRules() []linter.Rule {
+	return []linter.Rule{
+		whitespace.NewTrailingWhitespaceRule(),       // L001
+		whitespace.NewMixedIndentationRule(),         // L002
+		whitespace.NewConsecutiveBlankLinesRule(2),    // L003
+		whitespace.NewIndentationDepthRule(10, 4),    // L004
+		whitespace.NewLongLinesRule(120),             // L005
+		style.NewColumnAlignmentRule(),               // L006
+		keywords.NewKeywordCaseRule(keywords.CaseUpper), // L007
+		style.NewCommaPlacementRule(style.CommaTrailing), // L008
+		style.NewAliasingConsistencyRule(true),       // L009
+		whitespace.NewRedundantWhitespaceRule(),      // L010
+	}
+}
+
 var (
 	actionFiles    string
 	actionRules    string
@@ -293,24 +310,14 @@ type lintViolation struct {
 	Message  string
 }
 
-// defaultLinter creates a linter with standard rules.
+// defaultLinter creates a linter with all built-in rules.
 func defaultLinter() *linter.Linter {
-	return linter.New(
-		whitespace.NewTrailingWhitespaceRule(),
-		whitespace.NewMixedIndentationRule(),
-		keywords.NewKeywordCaseRule(keywords.CaseUpper),
-		style.NewColumnAlignmentRule(),
-	)
+	return linter.New(builtinRules()...)
 }
 
 // allRules returns every built-in rule keyed by its ID.
 func allRules() map[string]linter.Rule {
-	all := []linter.Rule{
-		whitespace.NewTrailingWhitespaceRule(),
-		whitespace.NewMixedIndentationRule(),
-		keywords.NewKeywordCaseRule(keywords.CaseUpper),
-		style.NewColumnAlignmentRule(),
-	}
+	all := builtinRules()
 	m := make(map[string]linter.Rule, len(all))
 	for _, r := range all {
 		m[r.ID()] = r

cmd/gosqlx/cmd/action_test.go

@@ -0,0 +1,59 @@
+package cmd
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+func writeTempSQL(t *testing.T, content string) string {
+	t.Helper()
+	p := filepath.Join(t.TempDir(), "test.sql")
+	if err := os.WriteFile(p, []byte(content), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	return p
+}
+
+func TestLintFile_FilterSpecificRule(t *testing.T) {
+	// Trailing whitespace triggers L001; lowercase keyword triggers L007.
+	path := writeTempSQL(t, "select 1; \n")
+
+	violations := lintFile(path, []string{"L001"})
+	if len(violations) == 0 {
+		t.Fatal("expected at least one L001 violation for trailing whitespace")
+	}
+	for _, v := range violations {
+		if !strings.Contains(v.Message, "L001") {
+			t.Errorf("expected only L001 violations, got: %s", v.Message)
+		}
+	}
+}
+
+func TestLintFile_EmptyRulesRunsAll(t *testing.T) {
+	// Trailing whitespace (L001) + lowercase keyword (L007).
+	path := writeTempSQL(t, "select 1; \n")
+
+	violations := lintFile(path, nil)
+	ruleIDs := make(map[string]bool)
+	for _, v := range violations {
+		if i := strings.Index(v.Message, "["); i >= 0 {
+			if j := strings.Index(v.Message[i:], "]"); j > 0 {
+				ruleIDs[v.Message[i+1:i+j]] = true
+			}
+		}
+	}
+	if len(ruleIDs) < 2 {
+		t.Errorf("expected violations from multiple rules, got: %v", ruleIDs)
+	}
+}
+
+func TestLintFile_InvalidRuleIgnored(t *testing.T) {
+	path := writeTempSQL(t, "SELECT 1;\n")
+
+	violations := lintFile(path, []string{"INVALID_RULE"})
+	if len(violations) != 0 {
+		t.Errorf("expected no violations for invalid rule, got %d", len(violations))
+	}
+}

pkg/advisor/rules.go

@@ -171,7 +171,8 @@ func hasJoinCondition(expr ast.Expression, tableNames []string) bool {
 		return false
 	}
 
-	if e, ok := expr.(*ast.BinaryExpression); ok {
+	switch e := expr.(type) { //nolint:gocritic // singleCaseSwitch: intentional — may grow to handle more expression types
+	case *ast.BinaryExpression:
 		if e.Operator == "AND" || e.Operator == "OR" {
 			return hasJoinCondition(e.Left, tableNames) || hasJoinCondition(e.Right, tableNames)
 		}
@@ -375,7 +376,8 @@ func containsOrCondition(expr ast.Expression) bool {
 		return false
 	}
 
-	if e, ok := expr.(*ast.BinaryExpression); ok {
+	switch e := expr.(type) { //nolint:gocritic // singleCaseSwitch: intentional — may grow to handle more expression types
+	case *ast.BinaryExpression:
 		if strings.EqualFold(e.Operator, "OR") {
 			// Check if the OR operates on different columns
 			leftCols := collectColumnNames(e.Left)

pkg/sql/ast/value.go

@@ -333,8 +333,9 @@ func (d DateTimeField) String() string {
 		return "MICROSECOND"
 	case Microseconds:
 		return "MICROSECONDS"
-	case Millenium: //nolint:misspell // intentional: SQL accepts both spellings
-		return "MILLENIUM" //nolint:misspell // intentional: matches SQL keyword
+	//nolint:misspell // intentional: SQL accepts both spellings (Millenium / MILLENIUM)
+	case Millenium:
+		return "MILLENIUM"
 	case Millennium:
 		return "MILLENNIUM"
 	case Millisecond: