From 11c85d17c6b3a04fa45a8a936d6527467eeda5c9 Mon Sep 17 00:00:00 2001 From: ikjeong Date: Tue, 25 Nov 2025 16:26:00 +0000 Subject: [PATCH 01/13] refactor: remove WorkDir field from all adapters - Adapters are now stateless and use CWD at execution time - TSC temp config now uses ToolsDir/.tmp with unique filenames - NewAdapter signature changed from (toolsDir, workDir) to (toolsDir) - Updated all adapter tests accordingly --- internal/adapter/checkstyle/adapter.go | 9 ++--- internal/adapter/checkstyle/executor.go | 3 +- internal/adapter/eslint/adapter.go | 12 +++--- internal/adapter/eslint/adapter_test.go | 27 +++++-------- internal/adapter/eslint/executor.go | 2 +- internal/adapter/eslint/executor_test.go | 20 ++++----- internal/adapter/pmd/adapter.go | 9 ++--- internal/adapter/pmd/executor.go | 3 +- internal/adapter/prettier/adapter.go | 8 ++-- internal/adapter/prettier/adapter_test.go | 27 +++++-------- internal/adapter/prettier/executor.go | 3 +- internal/adapter/prettier/executor_test.go | 18 +++++---- internal/adapter/registry/init.go | 18 +++------ internal/adapter/tsc/adapter.go | 13 +++--- internal/adapter/tsc/adapter_test.go | 47 ++++++++++++---------- internal/adapter/tsc/executor.go | 20 +++++++-- 16 files changed, 117 insertions(+), 122 deletions(-) diff --git a/internal/adapter/checkstyle/adapter.go b/internal/adapter/checkstyle/adapter.go index ae88d24..323a3eb 100644 --- a/internal/adapter/checkstyle/adapter.go +++ b/internal/adapter/checkstyle/adapter.go @@ -27,14 +27,14 @@ const ( // - Length rules: line length, file length // - Style rules: indentation, whitespace // - Naming rules: class names, method names, variable names +// +// Note: Adapter is goroutine-safe and stateless. WorkDir is determined +// by CWD at execution time, not stored in the adapter. type Adapter struct { // ToolsDir is where Checkstyle JAR is stored. // Default: ~/.sym/tools ToolsDir string - // WorkDir is the project root. - WorkDir string - // JavaPath is the path to java executable. // Empty = use system java JavaPath string @@ -44,7 +44,7 @@ type Adapter struct { } // NewAdapter creates a new Checkstyle adapter. -func NewAdapter(toolsDir, workDir string) *Adapter { +func NewAdapter(toolsDir string) *Adapter { if toolsDir == "" { home, _ := os.UserHomeDir() toolsDir = filepath.Join(home, ".sym", "tools") @@ -54,7 +54,6 @@ func NewAdapter(toolsDir, workDir string) *Adapter { return &Adapter{ ToolsDir: toolsDir, - WorkDir: workDir, JavaPath: javaPath, executor: adapter.NewSubprocessExecutor(), } diff --git a/internal/adapter/checkstyle/executor.go b/internal/adapter/checkstyle/executor.go index b33f3f6..056c344 100644 --- a/internal/adapter/checkstyle/executor.go +++ b/internal/adapter/checkstyle/executor.go @@ -38,9 +38,8 @@ func (a *Adapter) execute(ctx context.Context, config []byte, files []string) (* } args = append(args, files...) - // Execute + // Execute (uses CWD by default) start := time.Now() - a.executor.WorkDir = a.WorkDir output, err := a.executor.Execute(ctx, a.JavaPath, args...) duration := time.Since(start) diff --git a/internal/adapter/eslint/adapter.go b/internal/adapter/eslint/adapter.go index 314cb6e..c90e740 100644 --- a/internal/adapter/eslint/adapter.go +++ b/internal/adapter/eslint/adapter.go @@ -18,29 +18,27 @@ import ( // - Length rules: max-len, max-lines, max-params, max-lines-per-function // - Style rules: indent, quotes, semi, comma-dangle // - AST rules: Custom rule generation +// +// Note: Adapter is goroutine-safe and stateless. WorkDir is determined +// by CWD at execution time, not stored in the adapter. type Adapter struct { // ToolsDir is where ESLint is installed - // Default: ~/.sym/tools/node_modules + // Default: ~/.sym/tools ToolsDir string - // WorkDir is the project root - WorkDir string - // executor runs ESLint subprocess executor *adapter.SubprocessExecutor } // NewAdapter creates a new ESLint adapter. -func NewAdapter(toolsDir, workDir string) *Adapter { +func NewAdapter(toolsDir string) *Adapter { if toolsDir == "" { home, _ := os.UserHomeDir() - // symphonyclient integration: .symphony โ†’ .sym directory toolsDir = filepath.Join(home, ".sym", "tools") } return &Adapter{ ToolsDir: toolsDir, - WorkDir: workDir, executor: adapter.NewSubprocessExecutor(), } } diff --git a/internal/adapter/eslint/adapter_test.go b/internal/adapter/eslint/adapter_test.go index d4e8c21..088b840 100644 --- a/internal/adapter/eslint/adapter_test.go +++ b/internal/adapter/eslint/adapter_test.go @@ -11,7 +11,7 @@ import ( ) func TestNewAdapter(t *testing.T) { - adapter := NewAdapter("", "") + adapter := NewAdapter("") if adapter == nil { t.Fatal("NewAdapter() returned nil") } @@ -21,30 +21,25 @@ func TestNewAdapter(t *testing.T) { } } -func TestNewAdapter_CustomDirs(t *testing.T) { +func TestNewAdapter_CustomToolsDir(t *testing.T) { toolsDir := "/custom/tools" - workDir := "/custom/work" - a := NewAdapter(toolsDir, workDir) + a := NewAdapter(toolsDir) if a.ToolsDir != toolsDir { t.Errorf("ToolsDir = %q, want %q", a.ToolsDir, toolsDir) } - - if a.WorkDir != workDir { - t.Errorf("WorkDir = %q, want %q", a.WorkDir, workDir) - } } func TestName(t *testing.T) { - a := NewAdapter("", "") + a := NewAdapter("") if a.Name() != "eslint" { t.Errorf("Name() = %q, want %q", a.Name(), "eslint") } } func TestGetCapabilities(t *testing.T) { - a := NewAdapter("", "") + a := NewAdapter("") caps := a.GetCapabilities() if caps.Name != "eslint" { @@ -67,7 +62,7 @@ func TestGetCapabilities(t *testing.T) { } func TestGetESLintPath(t *testing.T) { - a := NewAdapter("/test/tools", "") + a := NewAdapter("/test/tools") expected := filepath.Join("/test/tools", "node_modules", ".bin", "eslint") got := a.getESLintPath() @@ -83,7 +78,7 @@ func TestInitPackageJSON(t *testing.T) { } defer func() { _ = os.RemoveAll(tmpDir) }() - a := NewAdapter(tmpDir, "") + a := NewAdapter(tmpDir) if err := a.initPackageJSON(); err != nil { t.Fatalf("initPackageJSON() error = %v", err) @@ -108,7 +103,7 @@ func TestInitPackageJSON(t *testing.T) { } func TestCheckAvailability_NotFound(t *testing.T) { - a := NewAdapter("/nonexistent/path", "") + a := NewAdapter("/nonexistent/path") ctx := context.Background() err := a.CheckAvailability(ctx) @@ -125,7 +120,7 @@ func TestInstall(t *testing.T) { } defer func() { _ = os.RemoveAll(tmpDir) }() - a := NewAdapter(tmpDir, "") + a := NewAdapter(tmpDir) ctx := context.Background() config := adapter.InstallConfig{ @@ -139,7 +134,7 @@ func TestInstall(t *testing.T) { } func TestExecute_InvalidConfig(t *testing.T) { - a := NewAdapter("", t.TempDir()) + a := NewAdapter(t.TempDir()) ctx := context.Background() config := []byte(`{"rules": {}}`) @@ -152,7 +147,7 @@ func TestExecute_InvalidConfig(t *testing.T) { } func TestParseOutput(t *testing.T) { - a := NewAdapter("", "") + a := NewAdapter("") output := &adapter.ToolOutput{ Stdout: `[{"filePath":"test.js","messages":[{"ruleId":"no-unused-vars","severity":2,"message":"'x' is defined but never used","line":1,"column":5}]}]`, diff --git a/internal/adapter/eslint/executor.go b/internal/adapter/eslint/executor.go index c0ca2e0..dd0d6d2 100644 --- a/internal/adapter/eslint/executor.go +++ b/internal/adapter/eslint/executor.go @@ -30,7 +30,7 @@ func (a *Adapter) execute(ctx context.Context, config []byte, files []string) (* eslintCmd, args := a.getExecutionArgs(configPath, files) // Execute with environment variable to support both ESLint 8 and 9 - a.executor.WorkDir = a.WorkDir + // Uses CWD by default a.executor.Env = map[string]string{ "ESLINT_USE_FLAT_CONFIG": "false", } diff --git a/internal/adapter/eslint/executor_test.go b/internal/adapter/eslint/executor_test.go index 2680ded..5388f78 100644 --- a/internal/adapter/eslint/executor_test.go +++ b/internal/adapter/eslint/executor_test.go @@ -10,14 +10,14 @@ import ( "github.com/DevSymphony/sym-cli/internal/adapter" ) -func TestExecute_FileCreation(t *testing.T) { +func TestExecute_TempFileCleanup(t *testing.T) { tmpDir, err := os.MkdirTemp("", "eslint-exec-test-*") if err != nil { t.Fatalf("Failed to create temp dir: %v", err) } defer func() { _ = os.RemoveAll(tmpDir) }() - a := NewAdapter("", tmpDir) + a := NewAdapter(tmpDir) ctx := context.Background() config := []byte(`{"rules": {"semi": [2, "always"]}}`) @@ -25,9 +25,11 @@ func TestExecute_FileCreation(t *testing.T) { _, _ = a.execute(ctx, config, files) - configPath := filepath.Join(tmpDir, ".symphony-eslintrc.json") - if _, err := os.Stat(configPath); !os.IsNotExist(err) { - t.Error("Config file should have been cleaned up") + // Config files are created in ToolsDir/.tmp and should be cleaned up + tmpConfigDir := filepath.Join(tmpDir, ".tmp") + configFiles, _ := filepath.Glob(filepath.Join(tmpConfigDir, "eslintrc-*.json")) + if len(configFiles) > 0 { + t.Error("Config files should have been cleaned up") } } @@ -51,7 +53,7 @@ func TestGetESLintCommand(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - a := NewAdapter(tt.toolsDir, "") + a := NewAdapter(tt.toolsDir) cmd := a.getESLintCommand() if tt.wantContain != "" && len(cmd) > 0 { @@ -65,7 +67,7 @@ func TestGetESLintCommand(t *testing.T) { } func TestGetExecutionArgs(t *testing.T) { - a := NewAdapter("", "/work/dir") + a := NewAdapter("") configPath := "/tmp/config.json" files := []string{"file1.js", "file2.js"} @@ -108,7 +110,7 @@ func TestWriteConfigFile(t *testing.T) { } defer func() { _ = os.RemoveAll(tmpDir) }() - a := NewAdapter("", tmpDir) + a := NewAdapter(tmpDir) config := []byte(`{"rules": {"semi": [2, "always"]}}`) @@ -150,7 +152,7 @@ func TestExecute_Integration(t *testing.T) { t.Fatalf("Failed to write test file: %v", err) } - a := NewAdapter("", tmpDir) + a := NewAdapter(tmpDir) ctx := context.Background() config := []byte(`{"rules": {"semi": [2, "always"]}}`) diff --git a/internal/adapter/pmd/adapter.go b/internal/adapter/pmd/adapter.go index e290443..e402526 100644 --- a/internal/adapter/pmd/adapter.go +++ b/internal/adapter/pmd/adapter.go @@ -29,14 +29,14 @@ const ( // - Performance rules: inefficient code patterns // - Security rules: hardcoded credentials, SQL injection // - Error handling rules: empty catch blocks, exception handling +// +// Note: Adapter is goroutine-safe and stateless. WorkDir is determined +// by CWD at execution time, not stored in the adapter. type Adapter struct { // ToolsDir is where PMD is installed. // Default: ~/.sym/tools ToolsDir string - // WorkDir is the project root. - WorkDir string - // PMDPath is the path to pmd executable. // Empty = use default location PMDPath string @@ -46,7 +46,7 @@ type Adapter struct { } // NewAdapter creates a new PMD adapter. -func NewAdapter(toolsDir, workDir string) *Adapter { +func NewAdapter(toolsDir string) *Adapter { if toolsDir == "" { home, _ := os.UserHomeDir() toolsDir = filepath.Join(home, ".sym", "tools") @@ -54,7 +54,6 @@ func NewAdapter(toolsDir, workDir string) *Adapter { return &Adapter{ ToolsDir: toolsDir, - WorkDir: workDir, executor: adapter.NewSubprocessExecutor(), } } diff --git a/internal/adapter/pmd/executor.go b/internal/adapter/pmd/executor.go index 25c14c3..cc63e7b 100644 --- a/internal/adapter/pmd/executor.go +++ b/internal/adapter/pmd/executor.go @@ -41,9 +41,8 @@ func (a *Adapter) execute(ctx context.Context, config []byte, files []string) (* "--no-cache", // Disable cache for consistent results } - // Execute + // Execute (uses CWD by default) start := time.Now() - a.executor.WorkDir = a.WorkDir output, err := a.executor.Execute(ctx, pmdPath, args...) duration := time.Since(start) diff --git a/internal/adapter/prettier/adapter.go b/internal/adapter/prettier/adapter.go index f62b78b..13d671f 100644 --- a/internal/adapter/prettier/adapter.go +++ b/internal/adapter/prettier/adapter.go @@ -17,23 +17,23 @@ import ( // - Style validation (--check mode) // - Auto-fixing (--write mode) // - Config: indent, quote, semi, trailingComma, etc. +// +// Note: Adapter is goroutine-safe and stateless. WorkDir is determined +// by CWD at execution time, not stored in the adapter. type Adapter struct { ToolsDir string - WorkDir string executor *adapter.SubprocessExecutor } // NewAdapter creates a new Prettier adapter. -func NewAdapter(toolsDir, workDir string) *Adapter { +func NewAdapter(toolsDir string) *Adapter { if toolsDir == "" { home, _ := os.UserHomeDir() - // symphonyclient integration: .symphony โ†’ .sym directory toolsDir = filepath.Join(home, ".sym", "tools") } return &Adapter{ ToolsDir: toolsDir, - WorkDir: workDir, executor: adapter.NewSubprocessExecutor(), } } diff --git a/internal/adapter/prettier/adapter_test.go b/internal/adapter/prettier/adapter_test.go index 78ffb21..e48a38c 100644 --- a/internal/adapter/prettier/adapter_test.go +++ b/internal/adapter/prettier/adapter_test.go @@ -10,7 +10,7 @@ import ( ) func TestNewAdapter(t *testing.T) { - a := NewAdapter("", "") + a := NewAdapter("") if a == nil { t.Fatal("NewAdapter() returned nil") } @@ -20,30 +20,25 @@ func TestNewAdapter(t *testing.T) { } } -func TestNewAdapter_CustomDirs(t *testing.T) { +func TestNewAdapter_CustomToolsDir(t *testing.T) { toolsDir := "/custom/tools" - workDir := "/custom/work" - a := NewAdapter(toolsDir, workDir) + a := NewAdapter(toolsDir) if a.ToolsDir != toolsDir { t.Errorf("ToolsDir = %q, want %q", a.ToolsDir, toolsDir) } - - if a.WorkDir != workDir { - t.Errorf("WorkDir = %q, want %q", a.WorkDir, workDir) - } } func TestName(t *testing.T) { - a := NewAdapter("", "") + a := NewAdapter("") if a.Name() != "prettier" { t.Errorf("Name() = %q, want %q", a.Name(), "prettier") } } func TestGetPrettierPath(t *testing.T) { - a := NewAdapter("/test/tools", "") + a := NewAdapter("/test/tools") expected := filepath.Join("/test/tools", "node_modules", ".bin", "prettier") got := a.getPrettierPath() @@ -59,7 +54,7 @@ func TestInitPackageJSON(t *testing.T) { } defer func() { _ = os.RemoveAll(tmpDir) }() - a := NewAdapter(tmpDir, "") + a := NewAdapter(tmpDir) if err := a.initPackageJSON(); err != nil { t.Fatalf("initPackageJSON() error = %v", err) @@ -72,7 +67,7 @@ func TestInitPackageJSON(t *testing.T) { } func TestCheckAvailability_NotFound(t *testing.T) { - a := NewAdapter("/nonexistent/path", "") + a := NewAdapter("/nonexistent/path") ctx := context.Background() err := a.CheckAvailability(ctx) @@ -89,7 +84,7 @@ func TestInstall(t *testing.T) { } defer func() { _ = os.RemoveAll(tmpDir) }() - a := NewAdapter(tmpDir, "") + a := NewAdapter(tmpDir) ctx := context.Background() config := adapter.InstallConfig{ @@ -103,7 +98,7 @@ func TestInstall(t *testing.T) { } func TestExecute(t *testing.T) { - a := NewAdapter("", t.TempDir()) + a := NewAdapter(t.TempDir()) ctx := context.Background() config := []byte(`{"semi": true}`) @@ -116,7 +111,7 @@ func TestExecute(t *testing.T) { } func TestExecuteWithMode(t *testing.T) { - a := NewAdapter("", t.TempDir()) + a := NewAdapter(t.TempDir()) ctx := context.Background() config := []byte(`{"semi": true}`) @@ -133,7 +128,7 @@ func TestExecuteWithMode(t *testing.T) { } func TestParseOutput(t *testing.T) { - a := NewAdapter("", "") + a := NewAdapter("") tests := []struct { name string diff --git a/internal/adapter/prettier/executor.go b/internal/adapter/prettier/executor.go index 20846de..6fa2eb4 100644 --- a/internal/adapter/prettier/executor.go +++ b/internal/adapter/prettier/executor.go @@ -40,8 +40,7 @@ func (a *Adapter) execute(ctx context.Context, config []byte, files []string, mo args = append(args, files...) - // Execute - a.executor.WorkDir = a.WorkDir + // Execute (uses CWD by default) output, err := a.executor.Execute(ctx, prettierCmd, args...) // Prettier returns non-zero exit code if files need formatting (in --check mode) diff --git a/internal/adapter/prettier/executor_test.go b/internal/adapter/prettier/executor_test.go index 99bfa01..85c9b40 100644 --- a/internal/adapter/prettier/executor_test.go +++ b/internal/adapter/prettier/executor_test.go @@ -7,14 +7,14 @@ import ( "testing" ) -func TestExecute_FileCreation(t *testing.T) { +func TestExecute_TempFileCleanup(t *testing.T) { tmpDir, err := os.MkdirTemp("", "prettier-exec-test-*") if err != nil { t.Fatalf("Failed to create temp dir: %v", err) } defer func() { _ = os.RemoveAll(tmpDir) }() - a := NewAdapter("", tmpDir) + a := NewAdapter(tmpDir) ctx := context.Background() config := []byte(`{"semi": true}`) @@ -22,9 +22,11 @@ func TestExecute_FileCreation(t *testing.T) { _, _ = a.execute(ctx, config, files, "check") - configPath := filepath.Join(tmpDir, ".symphony-prettierrc.json") - if _, err := os.Stat(configPath); !os.IsNotExist(err) { - t.Error("Config file should have been cleaned up") + // Config files are created in ToolsDir/.tmp and should be cleaned up + tmpConfigDir := filepath.Join(tmpDir, ".tmp") + configFiles, _ := filepath.Glob(filepath.Join(tmpConfigDir, "prettierrc-*.json")) + if len(configFiles) > 0 { + t.Error("Config files should have been cleaned up") } } @@ -45,7 +47,7 @@ func TestGetPrettierCommand(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - a := NewAdapter(tt.toolsDir, "") + a := NewAdapter(tt.toolsDir) cmd := a.getPrettierCommand() if cmd == "" { @@ -62,7 +64,7 @@ func TestWriteConfigFile(t *testing.T) { } defer func() { _ = os.RemoveAll(tmpDir) }() - a := NewAdapter("", tmpDir) + a := NewAdapter(tmpDir) config := []byte(`{"semi": true, "singleQuote": true}`) @@ -104,7 +106,7 @@ func TestExecute_Integration(t *testing.T) { t.Fatalf("Failed to write test file: %v", err) } - a := NewAdapter("", tmpDir) + a := NewAdapter(tmpDir) ctx := context.Background() config := []byte(`{"semi": true, "singleQuote": true}`) diff --git a/internal/adapter/registry/init.go b/internal/adapter/registry/init.go index f19c5b6..64eeea6 100644 --- a/internal/adapter/registry/init.go +++ b/internal/adapter/registry/init.go @@ -12,21 +12,21 @@ import ( ) // DefaultRegistry creates and populates a registry with all available adapters. +// Note: Adapters are stateless and use CWD at execution time for working directory. func DefaultRegistry() *Registry { reg := NewRegistry() // Determine tools directory toolsDir := getToolsDir() - workDir := getWorkDir() // Register JavaScript/TypeScript adapters - _ = reg.Register(eslint.NewAdapter(toolsDir, workDir)) - _ = reg.Register(prettier.NewAdapter(toolsDir, workDir)) - _ = reg.Register(tsc.NewAdapter(toolsDir, workDir)) + _ = reg.Register(eslint.NewAdapter(toolsDir)) + _ = reg.Register(prettier.NewAdapter(toolsDir)) + _ = reg.Register(tsc.NewAdapter(toolsDir)) // Register Java adapters - _ = reg.Register(checkstyle.NewAdapter(toolsDir, workDir)) - _ = reg.Register(pmd.NewAdapter(toolsDir, workDir)) + _ = reg.Register(checkstyle.NewAdapter(toolsDir)) + _ = reg.Register(pmd.NewAdapter(toolsDir)) return reg } @@ -36,9 +36,3 @@ func getToolsDir() string { home, _ := os.UserHomeDir() return filepath.Join(home, ".sym", "tools") } - -// getWorkDir returns the current working directory. -func getWorkDir() string { - wd, _ := os.Getwd() - return wd -} diff --git a/internal/adapter/tsc/adapter.go b/internal/adapter/tsc/adapter.go index a7b72d4..f4c31f2 100644 --- a/internal/adapter/tsc/adapter.go +++ b/internal/adapter/tsc/adapter.go @@ -17,28 +17,27 @@ import ( // - Type checking for TypeScript and JavaScript files // - Compilation errors and warnings // - Interface and type validation +// +// Note: Adapter is goroutine-safe and stateless. WorkDir is determined +// by CWD at execution time, not stored in the adapter. type Adapter struct { // ToolsDir is where TypeScript is installed - // Default: ~/.symphony/tools/node_modules + // Default: ~/.sym/tools ToolsDir string - // WorkDir is the project root - WorkDir string - // executor runs tsc subprocess executor *adapter.SubprocessExecutor } // NewAdapter creates a new TSC adapter. -func NewAdapter(toolsDir, workDir string) *Adapter { +func NewAdapter(toolsDir string) *Adapter { if toolsDir == "" { home, _ := os.UserHomeDir() - toolsDir = filepath.Join(home, ".symphony", "tools") + toolsDir = filepath.Join(home, ".sym", "tools") } return &Adapter{ ToolsDir: toolsDir, - WorkDir: workDir, executor: adapter.NewSubprocessExecutor(), } } diff --git a/internal/adapter/tsc/adapter_test.go b/internal/adapter/tsc/adapter_test.go index 8b63caf..6de991f 100644 --- a/internal/adapter/tsc/adapter_test.go +++ b/internal/adapter/tsc/adapter_test.go @@ -11,7 +11,7 @@ import ( ) func TestNewAdapter(t *testing.T) { - adapter := NewAdapter("", "") + adapter := NewAdapter("") if adapter == nil { t.Fatal("NewAdapter() returned nil") } @@ -22,30 +22,25 @@ func TestNewAdapter(t *testing.T) { } } -func TestNewAdapter_CustomDirs(t *testing.T) { +func TestNewAdapter_CustomToolsDir(t *testing.T) { toolsDir := "/custom/tools" - workDir := "/custom/work" - adapter := NewAdapter(toolsDir, workDir) + adapter := NewAdapter(toolsDir) if adapter.ToolsDir != toolsDir { t.Errorf("ToolsDir = %q, want %q", adapter.ToolsDir, toolsDir) } - - if adapter.WorkDir != workDir { - t.Errorf("WorkDir = %q, want %q", adapter.WorkDir, workDir) - } } func TestName(t *testing.T) { - adapter := NewAdapter("", "") + adapter := NewAdapter("") if adapter.Name() != "tsc" { t.Errorf("Name() = %q, want %q", adapter.Name(), "tsc") } } func TestGetTSCPath(t *testing.T) { - adapter := NewAdapter("/test/tools", "") + adapter := NewAdapter("/test/tools") expected := filepath.Join("/test/tools", "node_modules", ".bin", "tsc") got := adapter.getTSCPath() @@ -62,7 +57,7 @@ func TestInitPackageJSON(t *testing.T) { } defer func() { _ = os.RemoveAll(tmpDir) }() - adapter := NewAdapter(tmpDir, "") + adapter := NewAdapter(tmpDir) if err := adapter.initPackageJSON(); err != nil { t.Fatalf("initPackageJSON() error = %v", err) @@ -96,7 +91,7 @@ func TestInitPackageJSON(t *testing.T) { func TestCheckAvailability_NotFound(t *testing.T) { // Use a non-existent directory - adapter := NewAdapter("/nonexistent/path", "") + adapter := NewAdapter("/nonexistent/path") ctx := context.Background() err := adapter.CheckAvailability(ctx) @@ -116,7 +111,7 @@ func TestInstall_MissingNPM(t *testing.T) { } defer func() { _ = os.RemoveAll(tmpDir) }() - a := NewAdapter(tmpDir, "") + a := NewAdapter(tmpDir) ctx := context.Background() config := adapter.InstallConfig{ @@ -134,32 +129,40 @@ func TestInstall_MissingNPM(t *testing.T) { } } -func TestExecute_FileCreation(t *testing.T) { - // Create temporary work directory +func TestExecute_TempFileCleanup(t *testing.T) { + // Create temporary tools directory tmpDir, err := os.MkdirTemp("", "tsc-exec-test-*") if err != nil { t.Fatalf("Failed to create temp dir: %v", err) } defer func() { _ = os.RemoveAll(tmpDir) }() - adapter := NewAdapter("", tmpDir) + adapter := NewAdapter(tmpDir) ctx := context.Background() config := []byte(`{"compilerOptions": {"strict": true}}`) files := []string{"test.ts"} - // Execute (will fail because tsc not installed, but we can test config file creation) + // Execute (will fail because tsc not installed, but we can test temp file cleanup) _, _ = adapter.Execute(ctx, config, files) - // Config file should have been created and cleaned up - configPath := filepath.Join(tmpDir, ".symphony-tsconfig.json") - if _, err := os.Stat(configPath); !os.IsNotExist(err) { - t.Error("Config file should have been cleaned up") + // Temp directory should exist (created by executor) + tmpConfigDir := filepath.Join(tmpDir, ".tmp") + if _, err := os.Stat(tmpConfigDir); os.IsNotExist(err) { + // Dir might not exist if execution failed early, which is fine + t.Log("Temp dir not created (execution may have failed early)") + return + } + + // Any tsconfig files should have been cleaned up + files2, _ := filepath.Glob(filepath.Join(tmpConfigDir, "tsconfig-*.json")) + if len(files2) > 0 { + t.Error("Temp config files should have been cleaned up") } } func TestParseOutput_Integration(t *testing.T) { - a := NewAdapter("", "") + a := NewAdapter("") output := &adapter.ToolOutput{ Stdout: `src/main.ts(10,5): error TS2304: Cannot find name 'foo'. diff --git a/internal/adapter/tsc/executor.go b/internal/adapter/tsc/executor.go index 718b4f9..ce274ca 100644 --- a/internal/adapter/tsc/executor.go +++ b/internal/adapter/tsc/executor.go @@ -29,8 +29,21 @@ func (a *Adapter) execute(ctx context.Context, config []byte, files []string) (* return nil, fmt.Errorf("failed to marshal updated tsconfig: %w", err) } - // Write tsconfig.json to a temporary location - configPath := filepath.Join(a.WorkDir, ".symphony-tsconfig.json") + // Write tsconfig.json to a temporary location with unique filename + // Use ToolsDir/.tmp to avoid conflicts with project files + tmpDir := filepath.Join(a.ToolsDir, ".tmp") + if err := os.MkdirAll(tmpDir, 0755); err != nil { + return nil, fmt.Errorf("failed to create temp dir: %w", err) + } + + // Create unique temp file for concurrent execution safety + tmpFile, err := os.CreateTemp(tmpDir, "tsconfig-*.json") + if err != nil { + return nil, fmt.Errorf("failed to create temp tsconfig: %w", err) + } + configPath := tmpFile.Name() + tmpFile.Close() // Close for WriteFile to reopen + if err := os.WriteFile(configPath, updatedConfig, 0644); err != nil { return nil, fmt.Errorf("failed to write tsconfig: %w", err) } @@ -51,8 +64,7 @@ func (a *Adapter) execute(ctx context.Context, config []byte, files []string) (* "--pretty", "false", } - // Execute tsc - a.executor.WorkDir = a.WorkDir + // Execute tsc (uses CWD by default) output, err := a.executor.Execute(ctx, tscPath, args...) // TSC returns non-zero exit code when there are type errors From 9d294b61464992a6c4dd8ca57cb94e48c614c99f Mon Sep 17 00:00:00 2001 From: ikjeong Date: Tue, 25 Nov 2025 16:31:54 +0000 Subject: [PATCH 02/13] refactor: implement Instance-based Registry with auto-registration - Add LinterConverter interface to adapter.go - Rewrite registry.go with Global() singleton, RegisterTool(), GetConverter(), GetConfigFile(), BuildLanguageMapping() - Create adapter/common.go with DefaultToolsDir() - Add register.go to each adapter (eslint, prettier, tsc, checkstyle, pmd) with init() auto-registration - Create bootstrap/adapters.go for blank imports - Update main.go to import bootstrap package - Remove hardcoded languageLinterMapping from converter.go - Remove hardcoded switch in getLinterConverter(), use registry instead - Remove hardcoded switch in getAdapterConfig(), use registry.GetConfigFile() - Change validator.go to use Global() instead of DefaultRegistry() - Add type aliases in linters/interface.go for backward compatibility - Delete registry/init.go to avoid import cycle --- cmd/sym/main.go | 3 + internal/adapter/adapter.go | 23 ++++ internal/adapter/checkstyle/register.go | 15 +++ internal/adapter/common.go | 13 +++ internal/adapter/eslint/register.go | 15 +++ internal/adapter/pmd/register.go | 15 +++ internal/adapter/prettier/register.go | 15 +++ internal/adapter/registry/init.go | 38 ------ internal/adapter/registry/registry.go | 148 ++++++++++++++++++++++-- internal/adapter/tsc/register.go | 15 +++ internal/bootstrap/adapters.go | 15 +++ internal/converter/converter.go | 35 ++---- internal/converter/linters/interface.go | 30 ++--- internal/validator/validator.go | 23 +--- 14 files changed, 290 insertions(+), 113 deletions(-) create mode 100644 internal/adapter/checkstyle/register.go create mode 100644 internal/adapter/common.go create mode 100644 internal/adapter/eslint/register.go create mode 100644 internal/adapter/pmd/register.go create mode 100644 internal/adapter/prettier/register.go delete mode 100644 internal/adapter/registry/init.go create mode 100644 internal/adapter/tsc/register.go create mode 100644 internal/bootstrap/adapters.go diff --git a/cmd/sym/main.go b/cmd/sym/main.go index 2102c20..c85d24b 100644 --- a/cmd/sym/main.go +++ b/cmd/sym/main.go @@ -2,6 +2,9 @@ package main import ( "github.com/DevSymphony/sym-cli/internal/cmd" + + // Bootstrap: register all adapters + _ "github.com/DevSymphony/sym-cli/internal/bootstrap" ) // Version is set by build -ldflags "-X main.Version=x.y.z" diff --git a/internal/adapter/adapter.go b/internal/adapter/adapter.go index f93539f..f3a6c9b 100644 --- a/internal/adapter/adapter.go +++ b/internal/adapter/adapter.go @@ -2,6 +2,9 @@ package adapter import ( "context" + + "github.com/DevSymphony/sym-cli/internal/llm" + "github.com/DevSymphony/sym-cli/pkg/schema" ) // Adapter wraps external tools (ESLint, Prettier, etc.) for use by engines. @@ -92,3 +95,23 @@ type Violation struct { Severity string // "error", "warning", "info" RuleID string } + +// LinterConverter converts user rules to native linter configuration using LLM. +// This interface is implemented by each linter's converter (e.g., ESLintConverter). +type LinterConverter interface { + // Name returns the linter name (e.g., "eslint", "checkstyle", "pmd") + Name() string + + // SupportedLanguages returns the languages this linter supports + SupportedLanguages() []string + + // ConvertRules converts user rules to native linter configuration using LLM + ConvertRules(ctx context.Context, rules []schema.UserRule, llmClient *llm.Client) (*LinterConfig, error) +} + +// LinterConfig represents a generated configuration file. +type LinterConfig struct { + Filename string // e.g., ".eslintrc.json", "checkstyle.xml" + Content []byte // File content + Format string // "json", "xml", "yaml" +} diff --git a/internal/adapter/checkstyle/register.go b/internal/adapter/checkstyle/register.go new file mode 100644 index 0000000..27fc55c --- /dev/null +++ b/internal/adapter/checkstyle/register.go @@ -0,0 +1,15 @@ +package checkstyle + +import ( + "github.com/DevSymphony/sym-cli/internal/adapter" + "github.com/DevSymphony/sym-cli/internal/adapter/registry" + "github.com/DevSymphony/sym-cli/internal/converter/linters" +) + +func init() { + _ = registry.Global().RegisterTool( + NewAdapter(adapter.DefaultToolsDir()), + linters.NewCheckstyleLinterConverter(), + "checkstyle.xml", + ) +} diff --git a/internal/adapter/common.go b/internal/adapter/common.go new file mode 100644 index 0000000..ce87445 --- /dev/null +++ b/internal/adapter/common.go @@ -0,0 +1,13 @@ +package adapter + +import ( + "os" + "path/filepath" +) + +// DefaultToolsDir returns the standard tools directory (~/.sym/tools). +// Used by all adapters for consistent tool installation location. +func DefaultToolsDir() string { + home, _ := os.UserHomeDir() + return filepath.Join(home, ".sym", "tools") +} diff --git a/internal/adapter/eslint/register.go b/internal/adapter/eslint/register.go new file mode 100644 index 0000000..1b9b22d --- /dev/null +++ b/internal/adapter/eslint/register.go @@ -0,0 +1,15 @@ +package eslint + +import ( + "github.com/DevSymphony/sym-cli/internal/adapter" + "github.com/DevSymphony/sym-cli/internal/adapter/registry" + "github.com/DevSymphony/sym-cli/internal/converter/linters" +) + +func init() { + _ = registry.Global().RegisterTool( + NewAdapter(adapter.DefaultToolsDir()), + linters.NewESLintLinterConverter(), + ".eslintrc.json", + ) +} diff --git a/internal/adapter/pmd/register.go b/internal/adapter/pmd/register.go new file mode 100644 index 0000000..39537f6 --- /dev/null +++ b/internal/adapter/pmd/register.go @@ -0,0 +1,15 @@ +package pmd + +import ( + "github.com/DevSymphony/sym-cli/internal/adapter" + "github.com/DevSymphony/sym-cli/internal/adapter/registry" + "github.com/DevSymphony/sym-cli/internal/converter/linters" +) + +func init() { + _ = registry.Global().RegisterTool( + NewAdapter(adapter.DefaultToolsDir()), + linters.NewPMDLinterConverter(), + "pmd-ruleset.xml", + ) +} diff --git a/internal/adapter/prettier/register.go b/internal/adapter/prettier/register.go new file mode 100644 index 0000000..00c177d --- /dev/null +++ b/internal/adapter/prettier/register.go @@ -0,0 +1,15 @@ +package prettier + +import ( + "github.com/DevSymphony/sym-cli/internal/adapter" + "github.com/DevSymphony/sym-cli/internal/adapter/registry" + "github.com/DevSymphony/sym-cli/internal/converter/linters" +) + +func init() { + _ = registry.Global().RegisterTool( + NewAdapter(adapter.DefaultToolsDir()), + linters.NewPrettierLinterConverter(), + ".prettierrc.json", + ) +} diff --git a/internal/adapter/registry/init.go b/internal/adapter/registry/init.go deleted file mode 100644 index 64eeea6..0000000 --- a/internal/adapter/registry/init.go +++ /dev/null @@ -1,38 +0,0 @@ -package registry - -import ( - "os" - "path/filepath" - - "github.com/DevSymphony/sym-cli/internal/adapter/checkstyle" - "github.com/DevSymphony/sym-cli/internal/adapter/eslint" - "github.com/DevSymphony/sym-cli/internal/adapter/pmd" - "github.com/DevSymphony/sym-cli/internal/adapter/prettier" - "github.com/DevSymphony/sym-cli/internal/adapter/tsc" -) - -// DefaultRegistry creates and populates a registry with all available adapters. -// Note: Adapters are stateless and use CWD at execution time for working directory. -func DefaultRegistry() *Registry { - reg := NewRegistry() - - // Determine tools directory - toolsDir := getToolsDir() - - // Register JavaScript/TypeScript adapters - _ = reg.Register(eslint.NewAdapter(toolsDir)) - _ = reg.Register(prettier.NewAdapter(toolsDir)) - _ = reg.Register(tsc.NewAdapter(toolsDir)) - - // Register Java adapters - _ = reg.Register(checkstyle.NewAdapter(toolsDir)) - _ = reg.Register(pmd.NewAdapter(toolsDir)) - - return reg -} - -// getToolsDir returns the default tools directory. -func getToolsDir() string { - home, _ := os.UserHomeDir() - return filepath.Join(home, ".sym", "tools") -} diff --git a/internal/adapter/registry/registry.go b/internal/adapter/registry/registry.go index 3e87958..d16ce7e 100644 --- a/internal/adapter/registry/registry.go +++ b/internal/adapter/registry/registry.go @@ -1,28 +1,88 @@ package registry import ( + "log" "sync" "github.com/DevSymphony/sym-cli/internal/adapter" ) -// Registry manages adapter instances - simple tool name based lookup. +// ToolRegistration contains all metadata for a linter tool. +type ToolRegistration struct { + Adapter adapter.Adapter // Adapter instance + Converter adapter.LinterConverter // LinterConverter instance (optional) + ConfigFile string // Config filename (e.g., ".eslintrc.json") +} + +// Registry manages tool registrations. type Registry struct { - mu sync.RWMutex + mu sync.RWMutex + tools map[string]*ToolRegistration - // adapters maps tool name to adapter (e.g., "eslint" -> ESLintAdapter) + // Legacy: adapters map for backward compatibility adapters map[string]adapter.Adapter } +var ( + globalRegistry *Registry + once sync.Once +) + +// Global returns the singleton registry instance. +func Global() *Registry { + once.Do(func() { + globalRegistry = &Registry{ + tools: make(map[string]*ToolRegistration), + adapters: make(map[string]adapter.Adapter), + } + }) + return globalRegistry +} + // NewRegistry creates a new empty adapter registry. +// Deprecated: Use Global() instead for the singleton pattern. func NewRegistry() *Registry { return &Registry{ + tools: make(map[string]*ToolRegistration), adapters: make(map[string]adapter.Adapter), } } +// RegisterTool registers a tool with adapter, converter, and config file. +func (r *Registry) RegisterTool( + adp adapter.Adapter, + converter adapter.LinterConverter, + configFile string, +) error { + if adp == nil { + return errNilAdapter + } + + name := adp.Name() + + r.mu.Lock() + defer r.mu.Unlock() + + // Warn on duplicate registration (init order issues) + if _, exists := r.tools[name]; exists { + log.Printf("warning: adapter already registered: %s (ignoring duplicate)", name) + return nil + } + + r.tools[name] = &ToolRegistration{ + Adapter: adp, + Converter: converter, + ConfigFile: configFile, + } + + // Also register in legacy adapters map for backward compatibility + r.adapters[name] = adp + + return nil +} + // Register adds an adapter to the registry. -// Returns errNilAdapter if the adapter is nil. +// Deprecated: Use RegisterTool() for new registrations. func (r *Registry) Register(adp adapter.Adapter) error { if adp == nil { return errNilAdapter @@ -32,21 +92,91 @@ func (r *Registry) Register(adp adapter.Adapter) error { defer r.mu.Unlock() name := adp.Name() + + // Check if already registered via RegisterTool + if _, exists := r.tools[name]; exists { + return nil + } + r.adapters[name] = adp return nil } // GetAdapter finds an adapter by tool name (e.g., "eslint", "prettier", "tsc"). -// Returns the adapter, or errAdapterNotFound if not registered. func (r *Registry) GetAdapter(toolName string) (adapter.Adapter, error) { r.mu.RLock() defer r.mu.RUnlock() - adp, ok := r.adapters[toolName] - if !ok { - return nil, &errAdapterNotFound{ToolName: toolName} + // First check tools map + if reg, ok := r.tools[toolName]; ok { + return reg.Adapter, nil + } + + // Fallback to legacy adapters map + if adp, ok := r.adapters[toolName]; ok { + return adp, nil + } + + return nil, &errAdapterNotFound{ToolName: toolName} +} + +// GetConverter returns LinterConverter by tool name. +func (r *Registry) GetConverter(name string) (adapter.LinterConverter, bool) { + r.mu.RLock() + defer r.mu.RUnlock() + + if reg, ok := r.tools[name]; ok && reg.Converter != nil { + return reg.Converter, true + } + return nil, false +} + +// GetConfigFile returns config filename by tool name. +func (r *Registry) GetConfigFile(name string) string { + r.mu.RLock() + defer r.mu.RUnlock() + + if reg, ok := r.tools[name]; ok { + return reg.ConfigFile + } + return "" +} + +// BuildLanguageMapping dynamically builds language->tools mapping from adapter capabilities. +func (r *Registry) BuildLanguageMapping() map[string][]string { + r.mu.RLock() + defer r.mu.RUnlock() + + mapping := make(map[string][]string) + for name, reg := range r.tools { + caps := reg.Adapter.GetCapabilities() + for _, lang := range caps.SupportedLanguages { + mapping[lang] = append(mapping[lang], name) + } + } + return mapping +} + +// GetAllToolNames returns all registered tool names. +func (r *Registry) GetAllToolNames() []string { + r.mu.RLock() + defer r.mu.RUnlock() + + names := make([]string, 0, len(r.tools)+len(r.adapters)) + seen := make(map[string]bool) + + for name := range r.tools { + names = append(names, name) + seen[name] = true + } + + // Add any legacy adapters not in tools + for name := range r.adapters { + if !seen[name] { + names = append(names, name) + } } - return adp, nil + return names } diff --git a/internal/adapter/tsc/register.go b/internal/adapter/tsc/register.go new file mode 100644 index 0000000..54129d3 --- /dev/null +++ b/internal/adapter/tsc/register.go @@ -0,0 +1,15 @@ +package tsc + +import ( + "github.com/DevSymphony/sym-cli/internal/adapter" + "github.com/DevSymphony/sym-cli/internal/adapter/registry" + "github.com/DevSymphony/sym-cli/internal/converter/linters" +) + +func init() { + _ = registry.Global().RegisterTool( + NewAdapter(adapter.DefaultToolsDir()), + linters.NewTSCLinterConverter(), + "tsconfig.json", + ) +} diff --git a/internal/bootstrap/adapters.go b/internal/bootstrap/adapters.go new file mode 100644 index 0000000..64a43c1 --- /dev/null +++ b/internal/bootstrap/adapters.go @@ -0,0 +1,15 @@ +package bootstrap + +import ( + // Import adapters for registration side-effects. + // Each adapter's register.go file contains an init() function + // that registers the adapter with the global registry. + _ "github.com/DevSymphony/sym-cli/internal/adapter/checkstyle" + _ "github.com/DevSymphony/sym-cli/internal/adapter/eslint" + _ "github.com/DevSymphony/sym-cli/internal/adapter/pmd" + _ "github.com/DevSymphony/sym-cli/internal/adapter/prettier" + _ "github.com/DevSymphony/sym-cli/internal/adapter/tsc" +) + +// This package only imports adapter packages for their init() side-effects. +// Import this package from main.go to ensure all adapters are registered. diff --git a/internal/converter/converter.go b/internal/converter/converter.go index 025cdce..ab5dfd5 100644 --- a/internal/converter/converter.go +++ b/internal/converter/converter.go @@ -9,22 +9,12 @@ import ( "strings" "sync" + "github.com/DevSymphony/sym-cli/internal/adapter/registry" "github.com/DevSymphony/sym-cli/internal/converter/linters" "github.com/DevSymphony/sym-cli/internal/llm" "github.com/DevSymphony/sym-cli/pkg/schema" ) -// languageLinterMapping defines which linters are available for each language -var languageLinterMapping = map[string][]string{ - "javascript": {"eslint", "prettier"}, - "js": {"eslint", "prettier"}, - "typescript": {"tsc", "eslint", "prettier"}, - "ts": {"tsc", "eslint", "prettier"}, - "tsx": {"tsc", "eslint", "prettier"}, - "jsx": {"eslint", "prettier"}, - "java": {"checkstyle", "pmd"}, -} - // llmValidatorEngine is the special linter for rules that don't fit any specific external linter const llmValidatorEngine = "llm-validator" @@ -297,9 +287,12 @@ func (c *Converter) routeRulesWithLLM(ctx context.Context, userPolicy *schema.Us // getAvailableLinters returns available linters for given languages func (c *Converter) getAvailableLinters(languages []string) []string { + // Build language mapping dynamically from registry + languageLinterMapping := registry.Global().BuildLanguageMapping() + if len(languages) == 0 { - // If no languages specified, include all linters - languages = []string{"javascript", "typescript", "java"} + // If no languages specified, return all registered tools + return registry.Global().GetAllToolNames() } linterSet := make(map[string]bool) @@ -412,20 +405,12 @@ Reason: Requires knowing which packages are "large"`, availableLinters) // getLinterConverter returns the appropriate converter for a linter func (c *Converter) getLinterConverter(linterName string) linters.LinterConverter { - switch linterName { - case "eslint": - return linters.NewESLintLinterConverter() - case "prettier": - return linters.NewPrettierLinterConverter() - case "tsc": - return linters.NewTSCLinterConverter() - case "checkstyle": - return linters.NewCheckstyleLinterConverter() - case "pmd": - return linters.NewPMDLinterConverter() - default: + // Use registry to get converter (no hardcoding) + converter, ok := registry.Global().GetConverter(linterName) + if !ok { return nil } + return converter } // convertRBAC converts UserRBAC to PolicyRBAC diff --git a/internal/converter/linters/interface.go b/internal/converter/linters/interface.go index a528262..0d35097 100644 --- a/internal/converter/linters/interface.go +++ b/internal/converter/linters/interface.go @@ -1,28 +1,12 @@ package linters import ( - "context" - - "github.com/DevSymphony/sym-cli/internal/llm" - "github.com/DevSymphony/sym-cli/pkg/schema" + "github.com/DevSymphony/sym-cli/internal/adapter" ) -// LinterConverter converts user rules to native linter configuration using LLM -type LinterConverter interface { - // Name returns the linter name (e.g., "eslint", "checkstyle", "pmd") - Name() string - - // SupportedLanguages returns the languages this linter supports - SupportedLanguages() []string - - // ConvertRules converts user rules to native linter configuration using LLM - // This is the main entry point for parallel conversion - ConvertRules(ctx context.Context, rules []schema.UserRule, llmClient *llm.Client) (*LinterConfig, error) -} - -// LinterConfig represents a generated configuration file -type LinterConfig struct { - Filename string // e.g., ".eslintrc.json", "checkstyle.xml" - Content []byte // File content - Format string // "json", "xml", "yaml" -} +// Type aliases for backward compatibility. +// The canonical definitions are now in the adapter package. +type ( + LinterConverter = adapter.LinterConverter + LinterConfig = adapter.LinterConfig +) diff --git a/internal/validator/validator.go b/internal/validator/validator.go index 6792412..1225ee9 100644 --- a/internal/validator/validator.go +++ b/internal/validator/validator.go @@ -60,7 +60,7 @@ func NewValidator(policy *schema.CodePolicy, verbose bool) *Validator { return &Validator{ policy: policy, verbose: verbose, - adapterRegistry: adapterRegistry.DefaultRegistry(), + adapterRegistry: adapterRegistry.Global(), workDir: workDir, symDir: symDir, selector: NewFileSelector(workDir), @@ -288,23 +288,10 @@ Does this code violate the convention?`, file, rule.Desc, string(content)) // getAdapterConfig gets config for an adapter // First checks .sym directory for existing config files, then generates from rule func (v *Validator) getAdapterConfig(adapterName string, rule schema.PolicyRule) ([]byte, error) { - // Check for existing config in .sym directory - var configPath string - switch adapterName { - case "eslint": - configPath = filepath.Join(v.symDir, ".eslintrc.json") - case "prettier": - configPath = filepath.Join(v.symDir, ".prettierrc.json") - case "tsc": - configPath = filepath.Join(v.symDir, "tsconfig.json") - case "checkstyle": - configPath = filepath.Join(v.symDir, "checkstyle.xml") - case "pmd": - configPath = filepath.Join(v.symDir, "pmd-ruleset.xml") - } - - // If config exists in .sym, use it - if configPath != "" { + // Check for existing config in .sym directory (using registry) + configFile := v.adapterRegistry.GetConfigFile(adapterName) + if configFile != "" { + configPath := filepath.Join(v.symDir, configFile) if data, err := os.ReadFile(configPath); err == nil { if v.verbose { fmt.Printf(" ๐Ÿ“„ Using config from %s\n", configPath) From ea555ab3351eba31d87e7d76dfa443c2780ce924 Mon Sep 17 00:00:00 2001 From: ikjeong Date: Tue, 25 Nov 2025 17:03:11 +0000 Subject: [PATCH 03/13] refactor: move LinterConverters to adapter packages and remove hardcoding Part C: Remove hardcoded linter references - Add GetLLMDescription() method to LinterConverter interface - Update cmd/init.go to dynamically get config files from registry - Update cmd/convert.go to dynamically build --targets flag description - Update converter.go to dynamically build LLM prompts from registry Part D: Move LinterConverters to adapter packages - Create converter.go in each adapter package (eslint, prettier, tsc, checkstyle, pmd) - Update register.go files to use same-package NewConverter() - Delete converter/linters package (eslint.go, prettier_tsc.go, checkstyle.go, pmd.go, interface.go) Registry enhancements: - Add GetAllConfigFiles() method - Add GetAllConverters() method This completes the OCP-compliant adapter architecture: - Adding a new adapter only requires creating files in adapter// directory - Plus one blank import line in bootstrap/adapters.go (Go language limitation) --- internal/adapter/adapter.go | 4 + .../checkstyle/converter.go} | 30 +- internal/adapter/checkstyle/register.go | 3 +- .../eslint.go => adapter/eslint/converter.go} | 30 +- internal/adapter/eslint/register.go | 3 +- .../pmd.go => adapter/pmd/converter.go} | 30 +- internal/adapter/pmd/register.go | 5 +- internal/adapter/prettier/converter.go | 148 +++++++++ internal/adapter/prettier/register.go | 5 +- internal/adapter/registry/registry.go | 28 ++ internal/adapter/tsc/converter.go | 162 ++++++++++ internal/adapter/tsc/register.go | 3 +- internal/cmd/convert.go | 22 +- internal/cmd/init.go | 12 +- internal/converter/converter.go | 40 ++- internal/converter/linters/interface.go | 12 - internal/converter/linters/prettier_tsc.go | 283 ------------------ 17 files changed, 457 insertions(+), 363 deletions(-) rename internal/{converter/linters/checkstyle.go => adapter/checkstyle/converter.go} (83%) rename internal/{converter/linters/eslint.go => adapter/eslint/converter.go} (83%) rename internal/{converter/linters/pmd.go => adapter/pmd/converter.go} (80%) create mode 100644 internal/adapter/prettier/converter.go create mode 100644 internal/adapter/tsc/converter.go delete mode 100644 internal/converter/linters/interface.go delete mode 100644 internal/converter/linters/prettier_tsc.go diff --git a/internal/adapter/adapter.go b/internal/adapter/adapter.go index f3a6c9b..105faf1 100644 --- a/internal/adapter/adapter.go +++ b/internal/adapter/adapter.go @@ -105,6 +105,10 @@ type LinterConverter interface { // SupportedLanguages returns the languages this linter supports SupportedLanguages() []string + // GetLLMDescription returns a description of the linter's capabilities for LLM routing. + // This is used in the LLM prompt to help route rules to appropriate linters. + GetLLMDescription() string + // ConvertRules converts user rules to native linter configuration using LLM ConvertRules(ctx context.Context, rules []schema.UserRule, llmClient *llm.Client) (*LinterConfig, error) } diff --git a/internal/converter/linters/checkstyle.go b/internal/adapter/checkstyle/converter.go similarity index 83% rename from internal/converter/linters/checkstyle.go rename to internal/adapter/checkstyle/converter.go index e92c69a..165cf8f 100644 --- a/internal/converter/linters/checkstyle.go +++ b/internal/adapter/checkstyle/converter.go @@ -1,4 +1,4 @@ -package linters +package checkstyle import ( "context" @@ -8,28 +8,36 @@ import ( "strings" "sync" + "github.com/DevSymphony/sym-cli/internal/adapter" "github.com/DevSymphony/sym-cli/internal/llm" "github.com/DevSymphony/sym-cli/pkg/schema" ) -// CheckstyleLinterConverter converts rules to Checkstyle XML using LLM -type CheckstyleLinterConverter struct{} +// Converter converts rules to Checkstyle XML configuration using LLM +type Converter struct{} -// NewCheckstyleLinterConverter creates a new Checkstyle converter -func NewCheckstyleLinterConverter() *CheckstyleLinterConverter { - return &CheckstyleLinterConverter{} +// NewConverter creates a new Checkstyle converter +func NewConverter() *Converter { + return &Converter{} } // Name returns the linter name -func (c *CheckstyleLinterConverter) Name() string { +func (c *Converter) Name() string { return "checkstyle" } // SupportedLanguages returns supported languages -func (c *CheckstyleLinterConverter) SupportedLanguages() []string { +func (c *Converter) SupportedLanguages() []string { return []string{"java"} } +// GetLLMDescription returns a description of Checkstyle's capabilities for LLM routing +func (c *Converter) GetLLMDescription() string { + return `Java style checks (naming, whitespace, imports, line length, complexity) + - CAN: Class/method/variable naming, line/method length, indentation, import checks, cyclomatic complexity, JavaDoc + - CANNOT: Runtime behavior, business logic, security vulnerabilities, advanced design patterns` +} + // checkstyleModule represents a Checkstyle module type checkstyleModule struct { XMLName xml.Name `xml:"module"` @@ -53,7 +61,7 @@ type checkstyleConfig struct { } // ConvertRules converts user rules to Checkstyle configuration using LLM -func (c *CheckstyleLinterConverter) ConvertRules(ctx context.Context, rules []schema.UserRule, llmClient *llm.Client) (*LinterConfig, error) { +func (c *Converter) ConvertRules(ctx context.Context, rules []schema.UserRule, llmClient *llm.Client) (*adapter.LinterConfig, error) { if llmClient == nil { return nil, fmt.Errorf("LLM client is required") } @@ -131,7 +139,7 @@ func (c *CheckstyleLinterConverter) ConvertRules(ctx context.Context, rules []sc ` fullContent := []byte(xmlHeader + string(content)) - return &LinterConfig{ + return &adapter.LinterConfig{ Filename: "checkstyle.xml", Content: fullContent, Format: "xml", @@ -139,7 +147,7 @@ func (c *CheckstyleLinterConverter) ConvertRules(ctx context.Context, rules []sc } // convertSingleRule converts a single rule using LLM -func (c *CheckstyleLinterConverter) convertSingleRule(ctx context.Context, rule schema.UserRule, llmClient *llm.Client) (*checkstyleModule, error) { +func (c *Converter) convertSingleRule(ctx context.Context, rule schema.UserRule, llmClient *llm.Client) (*checkstyleModule, error) { systemPrompt := `You are a Checkstyle configuration expert. Convert natural language Java coding rules to Checkstyle modules. Return ONLY a JSON object (no markdown fences): diff --git a/internal/adapter/checkstyle/register.go b/internal/adapter/checkstyle/register.go index 27fc55c..44e4ba0 100644 --- a/internal/adapter/checkstyle/register.go +++ b/internal/adapter/checkstyle/register.go @@ -3,13 +3,12 @@ package checkstyle import ( "github.com/DevSymphony/sym-cli/internal/adapter" "github.com/DevSymphony/sym-cli/internal/adapter/registry" - "github.com/DevSymphony/sym-cli/internal/converter/linters" ) func init() { _ = registry.Global().RegisterTool( NewAdapter(adapter.DefaultToolsDir()), - linters.NewCheckstyleLinterConverter(), + NewConverter(), "checkstyle.xml", ) } diff --git a/internal/converter/linters/eslint.go b/internal/adapter/eslint/converter.go similarity index 83% rename from internal/converter/linters/eslint.go rename to internal/adapter/eslint/converter.go index 8584584..f7b193f 100644 --- a/internal/converter/linters/eslint.go +++ b/internal/adapter/eslint/converter.go @@ -1,4 +1,4 @@ -package linters +package eslint import ( "context" @@ -7,30 +7,38 @@ import ( "strings" "sync" + "github.com/DevSymphony/sym-cli/internal/adapter" "github.com/DevSymphony/sym-cli/internal/llm" "github.com/DevSymphony/sym-cli/pkg/schema" ) -// ESLintLinterConverter converts rules to ESLint configuration using LLM -type ESLintLinterConverter struct{} +// Converter converts rules to ESLint configuration using LLM +type Converter struct{} -// NewESLintLinterConverter creates a new ESLint converter -func NewESLintLinterConverter() *ESLintLinterConverter { - return &ESLintLinterConverter{} +// NewConverter creates a new ESLint converter +func NewConverter() *Converter { + return &Converter{} } // Name returns the linter name -func (c *ESLintLinterConverter) Name() string { +func (c *Converter) Name() string { return "eslint" } // SupportedLanguages returns supported languages -func (c *ESLintLinterConverter) SupportedLanguages() []string { +func (c *Converter) SupportedLanguages() []string { return []string{"javascript", "js", "typescript", "ts", "jsx", "tsx"} } +// GetLLMDescription returns a description of ESLint's capabilities for LLM routing +func (c *Converter) GetLLMDescription() string { + return `ONLY native ESLint rules (no-console, no-unused-vars, eqeqeq, no-var, camelcase, new-cap, max-len, max-lines, no-eval, etc.) + - CAN: Simple syntax checks, variable naming, console usage, basic patterns + - CANNOT: Complex business logic, context-aware rules, file naming, advanced async patterns` +} + // ConvertRules converts user rules to ESLint configuration using LLM -func (c *ESLintLinterConverter) ConvertRules(ctx context.Context, rules []schema.UserRule, llmClient *llm.Client) (*LinterConfig, error) { +func (c *Converter) ConvertRules(ctx context.Context, rules []schema.UserRule, llmClient *llm.Client) (*adapter.LinterConfig, error) { if llmClient == nil { return nil, fmt.Errorf("LLM client is required") } @@ -117,7 +125,7 @@ func (c *ESLintLinterConverter) ConvertRules(ctx context.Context, rules []schema return nil, fmt.Errorf("failed to marshal config: %w", err) } - return &LinterConfig{ + return &adapter.LinterConfig{ Filename: ".eslintrc.json", Content: content, Format: "json", @@ -125,7 +133,7 @@ func (c *ESLintLinterConverter) ConvertRules(ctx context.Context, rules []schema } // convertSingleRule converts a single user rule to ESLint rule using LLM -func (c *ESLintLinterConverter) convertSingleRule(ctx context.Context, rule schema.UserRule, llmClient *llm.Client) (string, interface{}, error) { +func (c *Converter) convertSingleRule(ctx context.Context, rule schema.UserRule, llmClient *llm.Client) (string, interface{}, error) { systemPrompt := `You are an ESLint configuration expert. Convert natural language coding rules to ESLint rule configurations. Return ONLY a JSON object (no markdown fences) with this structure: diff --git a/internal/adapter/eslint/register.go b/internal/adapter/eslint/register.go index 1b9b22d..40223c6 100644 --- a/internal/adapter/eslint/register.go +++ b/internal/adapter/eslint/register.go @@ -3,13 +3,12 @@ package eslint import ( "github.com/DevSymphony/sym-cli/internal/adapter" "github.com/DevSymphony/sym-cli/internal/adapter/registry" - "github.com/DevSymphony/sym-cli/internal/converter/linters" ) func init() { _ = registry.Global().RegisterTool( NewAdapter(adapter.DefaultToolsDir()), - linters.NewESLintLinterConverter(), + NewConverter(), ".eslintrc.json", ) } diff --git a/internal/converter/linters/pmd.go b/internal/adapter/pmd/converter.go similarity index 80% rename from internal/converter/linters/pmd.go rename to internal/adapter/pmd/converter.go index 75e2939..9122f67 100644 --- a/internal/converter/linters/pmd.go +++ b/internal/adapter/pmd/converter.go @@ -1,4 +1,4 @@ -package linters +package pmd import ( "context" @@ -8,28 +8,36 @@ import ( "strings" "sync" + "github.com/DevSymphony/sym-cli/internal/adapter" "github.com/DevSymphony/sym-cli/internal/llm" "github.com/DevSymphony/sym-cli/pkg/schema" ) -// PMDLinterConverter converts rules to PMD XML using LLM -type PMDLinterConverter struct{} +// Converter converts rules to PMD XML configuration using LLM +type Converter struct{} -// NewPMDLinterConverter creates a new PMD converter -func NewPMDLinterConverter() *PMDLinterConverter { - return &PMDLinterConverter{} +// NewConverter creates a new PMD converter +func NewConverter() *Converter { + return &Converter{} } // Name returns the linter name -func (c *PMDLinterConverter) Name() string { +func (c *Converter) Name() string { return "pmd" } // SupportedLanguages returns supported languages -func (c *PMDLinterConverter) SupportedLanguages() []string { +func (c *Converter) SupportedLanguages() []string { return []string{"java"} } +// GetLLMDescription returns a description of PMD's capabilities for LLM routing +func (c *Converter) GetLLMDescription() string { + return `Java code quality (unused code, empty blocks, naming conventions, design issues) + - CAN: Unused private methods, empty catch blocks, short variable names, too many methods, hardcoded crypto keys + - CANNOT: Code formatting, whitespace, complex business logic validation` +} + // pmdRuleset represents PMD ruleset type pmdRuleset struct { XMLName xml.Name `xml:"ruleset"` @@ -49,7 +57,7 @@ type pmdRule struct { } // ConvertRules converts user rules to PMD configuration using LLM -func (c *PMDLinterConverter) ConvertRules(ctx context.Context, rules []schema.UserRule, llmClient *llm.Client) (*LinterConfig, error) { +func (c *Converter) ConvertRules(ctx context.Context, rules []schema.UserRule, llmClient *llm.Client) (*adapter.LinterConfig, error) { if llmClient == nil { return nil, fmt.Errorf("LLM client is required") } @@ -121,7 +129,7 @@ func (c *PMDLinterConverter) ConvertRules(ctx context.Context, rules []schema.Us xmlHeader := `` + "\n" fullContent := []byte(xmlHeader + string(content)) - return &LinterConfig{ + return &adapter.LinterConfig{ Filename: "pmd.xml", Content: fullContent, Format: "xml", @@ -129,7 +137,7 @@ func (c *PMDLinterConverter) ConvertRules(ctx context.Context, rules []schema.Us } // convertSingleRule converts a single rule using LLM -func (c *PMDLinterConverter) convertSingleRule(ctx context.Context, rule schema.UserRule, llmClient *llm.Client) (*pmdRule, error) { +func (c *Converter) convertSingleRule(ctx context.Context, rule schema.UserRule, llmClient *llm.Client) (*pmdRule, error) { systemPrompt := `You are a PMD configuration expert. Convert natural language Java coding rules to PMD rule references. Return ONLY a JSON object (no markdown fences): diff --git a/internal/adapter/pmd/register.go b/internal/adapter/pmd/register.go index 39537f6..e75669a 100644 --- a/internal/adapter/pmd/register.go +++ b/internal/adapter/pmd/register.go @@ -3,13 +3,12 @@ package pmd import ( "github.com/DevSymphony/sym-cli/internal/adapter" "github.com/DevSymphony/sym-cli/internal/adapter/registry" - "github.com/DevSymphony/sym-cli/internal/converter/linters" ) func init() { _ = registry.Global().RegisterTool( NewAdapter(adapter.DefaultToolsDir()), - linters.NewPMDLinterConverter(), - "pmd-ruleset.xml", + NewConverter(), + "pmd.xml", ) } diff --git a/internal/adapter/prettier/converter.go b/internal/adapter/prettier/converter.go new file mode 100644 index 0000000..b43e553 --- /dev/null +++ b/internal/adapter/prettier/converter.go @@ -0,0 +1,148 @@ +package prettier + +import ( + "context" + "encoding/json" + "fmt" + "strings" + + "github.com/DevSymphony/sym-cli/internal/adapter" + "github.com/DevSymphony/sym-cli/internal/llm" + "github.com/DevSymphony/sym-cli/pkg/schema" +) + +// Converter converts rules to Prettier configuration using LLM +type Converter struct{} + +// NewConverter creates a new Prettier converter +func NewConverter() *Converter { + return &Converter{} +} + +// Name returns the linter name +func (c *Converter) Name() string { + return "prettier" +} + +// SupportedLanguages returns supported languages +func (c *Converter) SupportedLanguages() []string { + return []string{"javascript", "js", "typescript", "ts", "jsx", "tsx"} +} + +// GetLLMDescription returns a description of Prettier's capabilities for LLM routing +func (c *Converter) GetLLMDescription() string { + return `Code formatting ONLY (quotes, semicolons, indentation, line length, trailing commas) + - CAN: String quotes, semicolons, tab width, trailing commas, print width, arrow function parens + - CANNOT: Code logic, naming conventions, unused variables, type checking` +} + +// ConvertRules converts formatting rules to Prettier config using LLM +func (c *Converter) ConvertRules(ctx context.Context, rules []schema.UserRule, llmClient *llm.Client) (*adapter.LinterConfig, error) { + if llmClient == nil { + return nil, fmt.Errorf("LLM client is required") + } + + // Start with default Prettier configuration + prettierConfig := map[string]interface{}{ + "semi": true, + "singleQuote": false, + "tabWidth": 2, + "useTabs": false, + "trailingComma": "es5", + "printWidth": 80, + "arrowParens": "always", + } + + // Use LLM to infer settings from rules + for _, rule := range rules { + config, err := c.convertSingleRule(ctx, rule, llmClient) + if err != nil { + continue // Skip rules that cannot be converted + } + + // Merge LLM-generated config + for key, value := range config { + prettierConfig[key] = value + } + } + + content, err := json.MarshalIndent(prettierConfig, "", " ") + if err != nil { + return nil, fmt.Errorf("failed to marshal config: %w", err) + } + + return &adapter.LinterConfig{ + Filename: ".prettierrc", + Content: content, + Format: "json", + }, nil +} + +// convertSingleRule converts a single user rule to Prettier config using LLM +func (c *Converter) convertSingleRule(ctx context.Context, rule schema.UserRule, llmClient *llm.Client) (map[string]interface{}, error) { + systemPrompt := `You are a Prettier configuration expert. Convert natural language formatting rules to Prettier configuration options. + +Return ONLY a JSON object (no markdown fences) with Prettier options. + +Available Prettier options: +- semi: true/false (use semicolons) +- singleQuote: true/false (use single quotes) +- tabWidth: number (spaces per indentation level) +- useTabs: true/false (use tabs instead of spaces) +- trailingComma: "none"/"es5"/"all" (trailing commas) +- printWidth: number (line length) +- arrowParens: "always"/"avoid" (arrow function parentheses) +- bracketSpacing: true/false (spaces in object literals) +- endOfLine: "lf"/"crlf"/"auto" + +If the rule is not about formatting, return empty object: {} + +Examples: + +Input: "Use single quotes for strings" +Output: +{ + "singleQuote": true +} + +Input: "No semicolons" +Output: +{ + "semi": false +} + +Input: "Use 4 spaces for indentation" +Output: +{ + "tabWidth": 4, + "useTabs": false +} + +Input: "Maximum line length is 120 characters" +Output: +{ + "printWidth": 120 +}` + + userPrompt := fmt.Sprintf("Convert this rule to Prettier configuration:\n\n%s", rule.Say) + + // Call LLM + response, err := llmClient.Complete(ctx, systemPrompt, userPrompt) + if err != nil { + return nil, fmt.Errorf("LLM call failed: %w", err) + } + + // Parse response + response = strings.TrimSpace(response) + response = strings.TrimPrefix(response, "```json") + response = strings.TrimPrefix(response, "```") + response = strings.TrimSuffix(response, "```") + response = strings.TrimSpace(response) + + var config map[string]interface{} + if err := json.Unmarshal([]byte(response), &config); err != nil { + return nil, fmt.Errorf("failed to parse LLM response: %w", err) + } + + return config, nil +} diff --git a/internal/adapter/prettier/register.go b/internal/adapter/prettier/register.go index 00c177d..de420ee 100644 --- a/internal/adapter/prettier/register.go +++ b/internal/adapter/prettier/register.go @@ -3,13 +3,12 @@ package prettier import ( "github.com/DevSymphony/sym-cli/internal/adapter" "github.com/DevSymphony/sym-cli/internal/adapter/registry" - "github.com/DevSymphony/sym-cli/internal/converter/linters" ) func init() { _ = registry.Global().RegisterTool( NewAdapter(adapter.DefaultToolsDir()), - linters.NewPrettierLinterConverter(), - ".prettierrc.json", + NewConverter(), + ".prettierrc", ) } diff --git a/internal/adapter/registry/registry.go b/internal/adapter/registry/registry.go index d16ce7e..0c6a76b 100644 --- a/internal/adapter/registry/registry.go +++ b/internal/adapter/registry/registry.go @@ -180,3 +180,31 @@ func (r *Registry) GetAllToolNames() []string { return names } + +// GetAllConfigFiles returns all registered config file names. +func (r *Registry) GetAllConfigFiles() []string { + r.mu.RLock() + defer r.mu.RUnlock() + + files := make([]string, 0, len(r.tools)) + for _, reg := range r.tools { + if reg.ConfigFile != "" { + files = append(files, reg.ConfigFile) + } + } + return files +} + +// GetAllConverters returns all registered converters. +func (r *Registry) GetAllConverters() []adapter.LinterConverter { + r.mu.RLock() + defer r.mu.RUnlock() + + converters := make([]adapter.LinterConverter, 0, len(r.tools)) + for _, reg := range r.tools { + if reg.Converter != nil { + converters = append(converters, reg.Converter) + } + } + return converters +} diff --git a/internal/adapter/tsc/converter.go b/internal/adapter/tsc/converter.go new file mode 100644 index 0000000..bfbf738 --- /dev/null +++ b/internal/adapter/tsc/converter.go @@ -0,0 +1,162 @@ +package tsc + +import ( + "context" + "encoding/json" + "fmt" + "strings" + + "github.com/DevSymphony/sym-cli/internal/adapter" + "github.com/DevSymphony/sym-cli/internal/llm" + "github.com/DevSymphony/sym-cli/pkg/schema" +) + +// Converter converts rules to TypeScript compiler configuration using LLM +type Converter struct{} + +// NewConverter creates a new TSC converter +func NewConverter() *Converter { + return &Converter{} +} + +// Name returns the linter name +func (c *Converter) Name() string { + return "tsc" +} + +// SupportedLanguages returns supported languages +func (c *Converter) SupportedLanguages() []string { + return []string{"typescript", "ts", "tsx"} +} + +// GetLLMDescription returns a description of TSC's capabilities for LLM routing +func (c *Converter) GetLLMDescription() string { + return `TypeScript type checking ONLY (strict modes, noImplicitAny, strictNullChecks, type inference) + - CAN: Type strictness, null checks, unused variable/parameter errors, implicit return checks + - CANNOT: Code formatting, naming conventions, runtime behavior, business logic` +} + +// ConvertRules converts type-checking rules to tsconfig.json using LLM +func (c *Converter) ConvertRules(ctx context.Context, rules []schema.UserRule, llmClient *llm.Client) (*adapter.LinterConfig, error) { + if llmClient == nil { + return nil, fmt.Errorf("LLM client is required") + } + + // Start with strict TypeScript configuration + tsConfig := map[string]interface{}{ + "compilerOptions": map[string]interface{}{ + "target": "ES2020", + "module": "commonjs", + "lib": []string{"ES2020"}, + "strict": true, + "esModuleInterop": true, + "skipLibCheck": true, + "forceConsistentCasingInFileNames": true, + "resolveJsonModule": true, + "moduleResolution": "node", + "noImplicitAny": true, + "strictNullChecks": true, + "strictFunctionTypes": true, + "noUnusedLocals": false, + "noUnusedParameters": false, + }, + } + + compilerOpts := tsConfig["compilerOptions"].(map[string]interface{}) + + // Use LLM to infer settings from rules + for _, rule := range rules { + config, err := c.convertSingleRule(ctx, rule, llmClient) + if err != nil { + continue // Skip rules that cannot be converted + } + + // Merge LLM-generated compiler options + for key, value := range config { + compilerOpts[key] = value + } + } + + content, err := json.MarshalIndent(tsConfig, "", " ") + if err != nil { + return nil, fmt.Errorf("failed to marshal config: %w", err) + } + + return &adapter.LinterConfig{ + Filename: "tsconfig.json", + Content: content, + Format: "json", + }, nil +} + +// convertSingleRule converts a single user rule to TypeScript compiler option using LLM +func (c *Converter) convertSingleRule(ctx context.Context, rule schema.UserRule, llmClient *llm.Client) (map[string]interface{}, error) { + systemPrompt := `You are a TypeScript compiler configuration expert. Convert natural language type-checking rules to tsconfig.json compiler options. + +Return ONLY a JSON object (no markdown fences) with TypeScript compiler options. + +Available TypeScript compiler options: +- strict: true/false (enable all strict checks) +- noImplicitAny: true/false (error on implicit any) +- strictNullChecks: true/false (strict null checking) +- strictFunctionTypes: true/false (strict function types) +- strictBindCallApply: true/false (strict bind/call/apply) +- noUnusedLocals: true/false (error on unused locals) +- noUnusedParameters: true/false (error on unused parameters) +- noImplicitReturns: true/false (error on implicit returns) +- noFallthroughCasesInSwitch: true/false (error on fallthrough) +- noUncheckedIndexedAccess: true/false (undefined in index signatures) +- allowUnreachableCode: true/false (allow unreachable code) +- allowUnusedLabels: true/false (allow unused labels) + +If the rule is not about TypeScript type-checking, return empty object: {} + +Examples: + +Input: "No implicit any types allowed" +Output: +{ + "noImplicitAny": true +} + +Input: "Check for null and undefined strictly" +Output: +{ + "strictNullChecks": true +} + +Input: "Report unused variables" +Output: +{ + "noUnusedLocals": true, + "noUnusedParameters": true +} + +Input: "Enable all strict type checks" +Output: +{ + "strict": true +}` + + userPrompt := fmt.Sprintf("Convert this rule to TypeScript compiler configuration:\n\n%s", rule.Say) + + // Call LLM + response, err := llmClient.Complete(ctx, systemPrompt, userPrompt) + if err != nil { + return nil, fmt.Errorf("LLM call failed: %w", err) + } + + // Parse response + response = strings.TrimSpace(response) + response = strings.TrimPrefix(response, "```json") + response = strings.TrimPrefix(response, "```") + response = strings.TrimSuffix(response, "```") + response = strings.TrimSpace(response) + + var config map[string]interface{} + if err := json.Unmarshal([]byte(response), &config); err != nil { + return nil, fmt.Errorf("failed to parse LLM response: %w", err) + } + + return config, nil +} diff --git a/internal/adapter/tsc/register.go b/internal/adapter/tsc/register.go index 54129d3..0b02a81 100644 --- a/internal/adapter/tsc/register.go +++ b/internal/adapter/tsc/register.go @@ -3,13 +3,12 @@ package tsc import ( "github.com/DevSymphony/sym-cli/internal/adapter" "github.com/DevSymphony/sym-cli/internal/adapter/registry" - "github.com/DevSymphony/sym-cli/internal/converter/linters" ) func init() { _ = registry.Global().RegisterTool( NewAdapter(adapter.DefaultToolsDir()), - linters.NewTSCLinterConverter(), + NewConverter(), "tsconfig.json", ) } diff --git a/internal/cmd/convert.go b/internal/cmd/convert.go index bf97f9e..7324307 100644 --- a/internal/cmd/convert.go +++ b/internal/cmd/convert.go @@ -9,6 +9,7 @@ import ( "strings" "time" + "github.com/DevSymphony/sym-cli/internal/adapter/registry" "github.com/DevSymphony/sym-cli/internal/converter" "github.com/DevSymphony/sym-cli/internal/llm" "github.com/DevSymphony/sym-cli/pkg/schema" @@ -29,23 +30,23 @@ var convertCmd = &cobra.Command{ Use: "convert", Short: "Convert user policies into linter configurations", Long: `Convert natural language policies (Schema A) written by users -into linter-specific configurations (ESLint, Checkstyle, PMD, etc.) -and internal validation schema (Schema B). +into linter-specific configurations and internal validation schema (Schema B). +Supported linters are dynamically determined from registered adapters. Uses OpenAI API to intelligently analyze natural language rules and map them to appropriate linter rules.`, Example: ` # Convert to all supported linters (outputs to /.sym) sym convert -i user-policy.json --targets all - # Convert only for JavaScript/TypeScript + # Convert for specific linter sym convert -i user-policy.json --targets eslint - # Convert for Java with specific model + # Convert for multiple linters with specific model sym convert -i user-policy.json --targets checkstyle,pmd --openai-model gpt-4o # Use custom output directory sym convert -i user-policy.json --targets all --output-dir ./custom-dir - + # Legacy mode (internal policy only) sym convert -i user-policy.json -o code-policy.json`, RunE: runConvert, @@ -54,13 +55,22 @@ map them to appropriate linter rules.`, func init() { convertCmd.Flags().StringVarP(&convertInputFile, "input", "i", "", "input user policy file (default: from .sym/.env POLICY_PATH)") convertCmd.Flags().StringVarP(&convertOutputFile, "output", "o", "", "output code policy file (legacy mode)") - convertCmd.Flags().StringSliceVar(&convertTargets, "targets", []string{}, "target linters (eslint,checkstyle,pmd or 'all')") + convertCmd.Flags().StringSliceVar(&convertTargets, "targets", []string{}, buildTargetsDescription()) convertCmd.Flags().StringVar(&convertOutputDir, "output-dir", "", "output directory for linter configs (default: same as input file directory)") convertCmd.Flags().StringVar(&convertOpenAIModel, "openai-model", "gpt-4o", "OpenAI model to use for inference") convertCmd.Flags().Float64Var(&convertConfidenceThreshold, "confidence-threshold", 0.7, "minimum confidence for LLM inference (0.0-1.0)") convertCmd.Flags().IntVar(&convertTimeout, "timeout", 30, "timeout for API calls in seconds") } +// buildTargetsDescription dynamically builds the --targets flag description +func buildTargetsDescription() string { + tools := registry.Global().GetAllToolNames() + if len(tools) == 0 { + return "target linters (or 'all')" + } + return fmt.Sprintf("target linters (%s or 'all')", strings.Join(tools, ",")) +} + func runConvert(cmd *cobra.Command, args []string) error { // Determine input file path if convertInputFile == "" { diff --git a/internal/cmd/init.go b/internal/cmd/init.go index 431fdd2..e1f2415 100644 --- a/internal/cmd/init.go +++ b/internal/cmd/init.go @@ -4,6 +4,8 @@ import ( "fmt" "os" "path/filepath" + + "github.com/DevSymphony/sym-cli/internal/adapter/registry" "github.com/DevSymphony/sym-cli/internal/config" "github.com/DevSymphony/sym-cli/internal/envutil" "github.com/DevSymphony/sym-cli/internal/git" @@ -247,13 +249,9 @@ func initializeEnvFile() error { } func removeExistingCodePolicy() error { - // Files generated by convert command - convertGeneratedFiles := []string{ - "code-policy.json", - ".eslintrc.json", - "checkstyle.xml", - "pmd-ruleset.xml", - } + // Files generated by convert command (dynamically retrieved from registry) + convertGeneratedFiles := []string{"code-policy.json"} + convertGeneratedFiles = append(convertGeneratedFiles, registry.Global().GetAllConfigFiles()...) // Check and remove from .sym directory symDir, err := getSymDir() diff --git a/internal/converter/converter.go b/internal/converter/converter.go index ab5dfd5..cc7d706 100644 --- a/internal/converter/converter.go +++ b/internal/converter/converter.go @@ -9,8 +9,8 @@ import ( "strings" "sync" + "github.com/DevSymphony/sym-cli/internal/adapter" "github.com/DevSymphony/sym-cli/internal/adapter/registry" - "github.com/DevSymphony/sym-cli/internal/converter/linters" "github.com/DevSymphony/sym-cli/internal/llm" "github.com/DevSymphony/sym-cli/pkg/schema" ) @@ -313,16 +313,13 @@ func (c *Converter) getAvailableLinters(languages []string) []string { // selectLintersForRule uses LLM to determine which linters are appropriate for a rule func (c *Converter) selectLintersForRule(ctx context.Context, rule schema.UserRule, availableLinters []string) []string { + // Build linter descriptions dynamically from registry + linterDescriptions := c.buildLinterDescriptions(availableLinters) + systemPrompt := fmt.Sprintf(`You are a code quality expert. Analyze the given coding rule and determine which linters can ACTUALLY enforce it using their NATIVE rules (without plugins). Available linters and NATIVE capabilities: -- eslint: ONLY native ESLint rules (no-console, no-unused-vars, eqeqeq, no-var, camelcase, new-cap, max-len, max-lines, no-eval, etc.) - - CAN: Simple syntax checks, variable naming, console usage, basic patterns - - CANNOT: Complex business logic, context-aware rules, file naming, advanced async patterns -- prettier: Code formatting ONLY (quotes, semicolons, indentation, line length, trailing commas) -- tsc: TypeScript type checking ONLY (strict modes, noImplicitAny, strictNullChecks, type inference) -- checkstyle: Java style checks (naming, whitespace, imports, line length, complexity) -- pmd: Java code quality (unused code, empty blocks, naming conventions, design issues) +%s STRICT Rules for selection: 1. ONLY select if the linter has a NATIVE rule that can enforce this @@ -376,7 +373,7 @@ Reason: Requires semantic analysis of what constitutes secrets Input: "Imports from large packages must be specific" Output: [] -Reason: Requires knowing which packages are "large"`, availableLinters) +Reason: Requires knowing which packages are "large"`, linterDescriptions, availableLinters) userPrompt := fmt.Sprintf("Rule: %s\nCategory: %s", rule.Say, rule.Category) @@ -404,7 +401,7 @@ Reason: Requires knowing which packages are "large"`, availableLinters) } // getLinterConverter returns the appropriate converter for a linter -func (c *Converter) getLinterConverter(linterName string) linters.LinterConverter { +func (c *Converter) getLinterConverter(linterName string) adapter.LinterConverter { // Use registry to get converter (no hardcoding) converter, ok := registry.Global().GetConverter(linterName) if !ok { @@ -413,6 +410,29 @@ func (c *Converter) getLinterConverter(linterName string) linters.LinterConverte return converter } +// buildLinterDescriptions builds linter capability descriptions from registry +func (c *Converter) buildLinterDescriptions(availableLinters []string) string { + var descriptions []string + + for _, linterName := range availableLinters { + converter, ok := registry.Global().GetConverter(linterName) + if !ok || converter == nil { + continue + } + + desc := converter.GetLLMDescription() + if desc != "" { + descriptions = append(descriptions, fmt.Sprintf("- %s: %s", linterName, desc)) + } + } + + if len(descriptions) == 0 { + return "No linter descriptions available" + } + + return strings.Join(descriptions, "\n") +} + // convertRBAC converts UserRBAC to PolicyRBAC func (c *Converter) convertRBAC(userRBAC *schema.UserRBAC) *schema.PolicyRBAC { if userRBAC == nil || len(userRBAC.Roles) == 0 { diff --git a/internal/converter/linters/interface.go b/internal/converter/linters/interface.go deleted file mode 100644 index 0d35097..0000000 --- a/internal/converter/linters/interface.go +++ /dev/null @@ -1,12 +0,0 @@ -package linters - -import ( - "github.com/DevSymphony/sym-cli/internal/adapter" -) - -// Type aliases for backward compatibility. -// The canonical definitions are now in the adapter package. -type ( - LinterConverter = adapter.LinterConverter - LinterConfig = adapter.LinterConfig -) diff --git a/internal/converter/linters/prettier_tsc.go b/internal/converter/linters/prettier_tsc.go deleted file mode 100644 index 2b0ba51..0000000 --- a/internal/converter/linters/prettier_tsc.go +++ /dev/null @@ -1,283 +0,0 @@ -package linters - -import ( - "context" - "encoding/json" - "fmt" - "strings" - - "github.com/DevSymphony/sym-cli/internal/llm" - "github.com/DevSymphony/sym-cli/pkg/schema" -) - -// PrettierLinterConverter converts rules to Prettier configuration -type PrettierLinterConverter struct{} - -// NewPrettierLinterConverter creates a new Prettier converter -func NewPrettierLinterConverter() *PrettierLinterConverter { - return &PrettierLinterConverter{} -} - -// Name returns the linter name -func (c *PrettierLinterConverter) Name() string { - return "prettier" -} - -// SupportedLanguages returns supported languages -func (c *PrettierLinterConverter) SupportedLanguages() []string { - return []string{"javascript", "js", "typescript", "ts", "jsx", "tsx"} -} - -// ConvertRules converts formatting rules to Prettier config using LLM -func (c *PrettierLinterConverter) ConvertRules(ctx context.Context, rules []schema.UserRule, llmClient *llm.Client) (*LinterConfig, error) { - if llmClient == nil { - return nil, fmt.Errorf("LLM client is required") - } - - // Start with default Prettier configuration - prettierConfig := map[string]interface{}{ - "semi": true, - "singleQuote": false, - "tabWidth": 2, - "useTabs": false, - "trailingComma": "es5", - "printWidth": 80, - "arrowParens": "always", - } - - // Use LLM to infer settings from rules - for _, rule := range rules { - config, err := c.convertSingleRule(ctx, rule, llmClient) - if err != nil { - continue // Skip rules that cannot be converted - } - - // Merge LLM-generated config - for key, value := range config { - prettierConfig[key] = value - } - } - - content, err := json.MarshalIndent(prettierConfig, "", " ") - if err != nil { - return nil, fmt.Errorf("failed to marshal config: %w", err) - } - - return &LinterConfig{ - Filename: ".prettierrc", - Content: content, - Format: "json", - }, nil -} - -// convertSingleRule converts a single user rule to Prettier config using LLM -func (c *PrettierLinterConverter) convertSingleRule(ctx context.Context, rule schema.UserRule, llmClient *llm.Client) (map[string]interface{}, error) { - systemPrompt := `You are a Prettier configuration expert. Convert natural language formatting rules to Prettier configuration options. - -Return ONLY a JSON object (no markdown fences) with Prettier options. - -Available Prettier options: -- semi: true/false (use semicolons) -- singleQuote: true/false (use single quotes) -- tabWidth: number (spaces per indentation level) -- useTabs: true/false (use tabs instead of spaces) -- trailingComma: "none"/"es5"/"all" (trailing commas) -- printWidth: number (line length) -- arrowParens: "always"/"avoid" (arrow function parentheses) -- bracketSpacing: true/false (spaces in object literals) -- endOfLine: "lf"/"crlf"/"auto" - -If the rule is not about formatting, return empty object: {} - -Examples: - -Input: "Use single quotes for strings" -Output: -{ - "singleQuote": true -} - -Input: "No semicolons" -Output: -{ - "semi": false -} - -Input: "Use 4 spaces for indentation" -Output: -{ - "tabWidth": 4, - "useTabs": false -} - -Input: "Maximum line length is 120 characters" -Output: -{ - "printWidth": 120 -}` - - userPrompt := fmt.Sprintf("Convert this rule to Prettier configuration:\n\n%s", rule.Say) - - // Call LLM - response, err := llmClient.Complete(ctx, systemPrompt, userPrompt) - if err != nil { - return nil, fmt.Errorf("LLM call failed: %w", err) - } - - // Parse response - response = strings.TrimSpace(response) - response = strings.TrimPrefix(response, "```json") - response = strings.TrimPrefix(response, "```") - response = strings.TrimSuffix(response, "```") - response = strings.TrimSpace(response) - - var config map[string]interface{} - if err := json.Unmarshal([]byte(response), &config); err != nil { - return nil, fmt.Errorf("failed to parse LLM response: %w", err) - } - - return config, nil -} - -// TSCLinterConverter converts rules to TypeScript compiler configuration -type TSCLinterConverter struct{} - -// NewTSCLinterConverter creates a new TSC converter -func NewTSCLinterConverter() *TSCLinterConverter { - return &TSCLinterConverter{} -} - -// Name returns the linter name -func (c *TSCLinterConverter) Name() string { - return "tsc" -} - -// SupportedLanguages returns supported languages -func (c *TSCLinterConverter) SupportedLanguages() []string { - return []string{"typescript", "ts", "tsx"} -} - -// ConvertRules converts type-checking rules to tsconfig.json using LLM -func (c *TSCLinterConverter) ConvertRules(ctx context.Context, rules []schema.UserRule, llmClient *llm.Client) (*LinterConfig, error) { - if llmClient == nil { - return nil, fmt.Errorf("LLM client is required") - } - - // Start with strict TypeScript configuration - tsConfig := map[string]interface{}{ - "compilerOptions": map[string]interface{}{ - "target": "ES2020", - "module": "commonjs", - "lib": []string{"ES2020"}, - "strict": true, - "esModuleInterop": true, - "skipLibCheck": true, - "forceConsistentCasingInFileNames": true, - "resolveJsonModule": true, - "moduleResolution": "node", - "noImplicitAny": true, - "strictNullChecks": true, - "strictFunctionTypes": true, - "noUnusedLocals": false, - "noUnusedParameters": false, - }, - } - - compilerOpts := tsConfig["compilerOptions"].(map[string]interface{}) - - // Use LLM to infer settings from rules - for _, rule := range rules { - config, err := c.convertSingleRule(ctx, rule, llmClient) - if err != nil { - continue // Skip rules that cannot be converted - } - - // Merge LLM-generated compiler options - for key, value := range config { - compilerOpts[key] = value - } - } - - content, err := json.MarshalIndent(tsConfig, "", " ") - if err != nil { - return nil, fmt.Errorf("failed to marshal config: %w", err) - } - - return &LinterConfig{ - Filename: "tsconfig.json", - Content: content, - Format: "json", - }, nil -} - -// convertSingleRule converts a single user rule to TypeScript compiler option using LLM -func (c *TSCLinterConverter) convertSingleRule(ctx context.Context, rule schema.UserRule, llmClient *llm.Client) (map[string]interface{}, error) { - systemPrompt := `You are a TypeScript compiler configuration expert. Convert natural language type-checking rules to tsconfig.json compiler options. - -Return ONLY a JSON object (no markdown fences) with TypeScript compiler options. - -Available TypeScript compiler options: -- strict: true/false (enable all strict checks) -- noImplicitAny: true/false (error on implicit any) -- strictNullChecks: true/false (strict null checking) -- strictFunctionTypes: true/false (strict function types) -- strictBindCallApply: true/false (strict bind/call/apply) -- noUnusedLocals: true/false (error on unused locals) -- noUnusedParameters: true/false (error on unused parameters) -- noImplicitReturns: true/false (error on implicit returns) -- noFallthroughCasesInSwitch: true/false (error on fallthrough) -- noUncheckedIndexedAccess: true/false (undefined in index signatures) -- allowUnreachableCode: true/false (allow unreachable code) -- allowUnusedLabels: true/false (allow unused labels) - -If the rule is not about TypeScript type-checking, return empty object: {} - -Examples: - -Input: "No implicit any types allowed" -Output: -{ - "noImplicitAny": true -} - -Input: "Check for null and undefined strictly" -Output: -{ - "strictNullChecks": true -} - -Input: "Report unused variables" -Output: -{ - "noUnusedLocals": true, - "noUnusedParameters": true -} - -Input: "Enable all strict type checks" -Output: -{ - "strict": true -}` - - userPrompt := fmt.Sprintf("Convert this rule to TypeScript compiler configuration:\n\n%s", rule.Say) - - // Call LLM - response, err := llmClient.Complete(ctx, systemPrompt, userPrompt) - if err != nil { - return nil, fmt.Errorf("LLM call failed: %w", err) - } - - // Parse response - response = strings.TrimSpace(response) - response = strings.TrimPrefix(response, "```json") - response = strings.TrimPrefix(response, "```") - response = strings.TrimSuffix(response, "```") - response = strings.TrimSpace(response) - - var config map[string]interface{} - if err := json.Unmarshal([]byte(response), &config); err != nil { - return nil, fmt.Errorf("failed to parse LLM response: %w", err) - } - - return config, nil -} From b45556b71118160f8840a7d69a6336b3b166e5bd Mon Sep 17 00:00:00 2001 From: ikjeong Date: Wed, 26 Nov 2025 02:44:05 +0000 Subject: [PATCH 04/13] feat: add Pylint adapter for Python static analysis - Implement Pylint adapter with capabilities for Python linting. - Add support for virtual environment management and installation of Pylint. - Create configuration file generation for Pylint using LLM for rule conversion. - Implement tests for adapter functionality, including execution and output parsing. - Update bootstrap to include Pylint adapter registration. This addition enhances the static analysis capabilities of the Symphony CLI for Python projects. --- .devcontainer/post-create.sh | 3 + internal/adapter/pylint/adapter.go | 253 +++++++++++++++++ internal/adapter/pylint/adapter_test.go | 208 ++++++++++++++ internal/adapter/pylint/converter.go | 341 +++++++++++++++++++++++ internal/adapter/pylint/executor.go | 68 +++++ internal/adapter/pylint/executor_test.go | 134 +++++++++ internal/adapter/pylint/parser.go | 91 ++++++ internal/adapter/pylint/parser_test.go | 207 ++++++++++++++ internal/adapter/pylint/register.go | 14 + internal/bootstrap/adapters.go | 1 + 10 files changed, 1320 insertions(+) create mode 100644 internal/adapter/pylint/adapter.go create mode 100644 internal/adapter/pylint/adapter_test.go create mode 100644 internal/adapter/pylint/converter.go create mode 100644 internal/adapter/pylint/executor.go create mode 100644 internal/adapter/pylint/executor_test.go create mode 100644 internal/adapter/pylint/parser.go create mode 100644 internal/adapter/pylint/parser_test.go create mode 100644 internal/adapter/pylint/register.go diff --git a/.devcontainer/post-create.sh b/.devcontainer/post-create.sh index 90fd5f5..e7a8735 100755 --- a/.devcontainer/post-create.sh +++ b/.devcontainer/post-create.sh @@ -7,4 +7,7 @@ make setup echo "Installing global npm packages..." npm install -g @google/gemini-cli @openai/codex +echo "Installing Python venv package for Pylint adapter..." +sudo apt-get update -qq && sudo apt-get install -y python3-venv + echo "Setup completed successfully!" diff --git a/internal/adapter/pylint/adapter.go b/internal/adapter/pylint/adapter.go new file mode 100644 index 0000000..10e2167 --- /dev/null +++ b/internal/adapter/pylint/adapter.go @@ -0,0 +1,253 @@ +package pylint + +import ( + "context" + "fmt" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + + "github.com/DevSymphony/sym-cli/internal/adapter" +) + +// Adapter wraps Pylint for Python static analysis. +// +// Pylint is the comprehensive Python linter: +// - Naming rules: invalid-name, disallowed-name +// - Style rules: line-too-long, trailing-whitespace +// - Documentation rules: missing-docstring variants +// - Error handling rules: bare-except, broad-except +// - Complexity rules: too-many-branches, too-many-arguments +// +// Note: Adapter is goroutine-safe and stateless. WorkDir is determined +// by CWD at execution time, not stored in the adapter. +type Adapter struct { + // ToolsDir is where Pylint virtualenv is installed + // Default: ~/.sym/tools + ToolsDir string + + // PylintPath is the path to pylint executable (optional override) + PylintPath string + + // executor runs Pylint subprocess + executor *adapter.SubprocessExecutor +} + +// NewAdapter creates a new Pylint adapter. +func NewAdapter(toolsDir string) *Adapter { + if toolsDir == "" { + home, _ := os.UserHomeDir() + toolsDir = filepath.Join(home, ".sym", "tools") + } + + return &Adapter{ + ToolsDir: toolsDir, + executor: adapter.NewSubprocessExecutor(), + } +} + +// Name returns the adapter name. +func (a *Adapter) Name() string { + return "pylint" +} + +// GetCapabilities returns the Pylint adapter capabilities. +func (a *Adapter) GetCapabilities() adapter.AdapterCapabilities { + return adapter.AdapterCapabilities{ + Name: "pylint", + SupportedLanguages: []string{"python", "py"}, + SupportedCategories: []string{ + "naming", + "style", + "documentation", + "error_handling", + "complexity", + "pattern", + "security", + "ast", + }, + Version: ">=3.0.0", + } +} + +// CheckAvailability checks if Pylint is installed. +func (a *Adapter) CheckAvailability(ctx context.Context) error { + // Try local installation first (virtualenv) + pylintPath := a.getPylintPath() + if _, err := os.Stat(pylintPath); err == nil { + return nil // Found in tools dir + } + + // Try global installation + cmd := exec.CommandContext(ctx, "pylint", "--version") + if err := cmd.Run(); err == nil { + return nil // Found globally + } + + return fmt.Errorf("pylint not found (checked: %s and global PATH)", pylintPath) +} + +// Install installs Pylint via pip in a virtualenv. +func (a *Adapter) Install(ctx context.Context, config adapter.InstallConfig) error { + // Ensure tools directory exists + if err := os.MkdirAll(a.ToolsDir, 0755); err != nil { + return fmt.Errorf("failed to create tools dir: %w", err) + } + + // Check if Python is available + pythonCmd := a.getPythonCommand() + if _, err := exec.LookPath(pythonCmd); err != nil { + return fmt.Errorf("python not found: please install Python 3.8+ first") + } + + venvPath := a.getVenvPath() + pipPath := a.getPipPath() + pylintPath := a.getPylintPath() + + // Check if venv exists but is incomplete (no pip or no pylint) + if _, err := os.Stat(venvPath); err == nil { + hasPip := true + hasPylint := true + if _, err := os.Stat(pipPath); os.IsNotExist(err) { + hasPip = false + } + if _, err := os.Stat(pylintPath); os.IsNotExist(err) { + hasPylint = false + } + // If venv exists but is incomplete, remove it + if !hasPip || !hasPylint { + if err := os.RemoveAll(venvPath); err != nil { + return fmt.Errorf("failed to remove incomplete venv: %w", err) + } + } + } + + // Create virtualenv if it doesn't exist + if _, err := os.Stat(venvPath); os.IsNotExist(err) { + output, err := a.executor.Execute(ctx, pythonCmd, "-m", "venv", venvPath) + if err != nil { + return fmt.Errorf("failed to create virtualenv: %w", err) + } + if output.ExitCode != 0 { + // Check for common venv issues (Python venv outputs errors to stdout) + errMsg := output.Stderr + if errMsg == "" { + errMsg = output.Stdout + } + if errMsg == "" { + errMsg = "venv creation failed (no error message)" + } + if strings.Contains(errMsg, "ensurepip") || strings.Contains(errMsg, "python3-venv") { + return fmt.Errorf("failed to create virtualenv: python3-venv package not installed. "+ + "On Debian/Ubuntu, run: sudo apt install python3-venv") + } + return fmt.Errorf("failed to create virtualenv: %s", errMsg) + } + } + + // Ensure pip is available (some Linux distros don't include pip in venv by default) + if _, err := os.Stat(pipPath); os.IsNotExist(err) { + pythonInVenv := a.getPythonInVenv() + output, err := a.executor.Execute(ctx, pythonInVenv, "-m", "ensurepip", "--upgrade") + if err != nil { + return fmt.Errorf("failed to install pip via ensurepip: %w", err) + } + if output.ExitCode != 0 { + return fmt.Errorf("failed to install pip via ensurepip: %s", output.Stderr) + } + } + + // Determine version + version := config.Version + if version == "" { + version = ">=3.0.0" + } + + // Install Pylint in virtualenv + output, err := a.executor.Execute(ctx, pipPath, "install", fmt.Sprintf("pylint%s", version)) + if err != nil { + return fmt.Errorf("pip install failed: %w", err) + } + if output.ExitCode != 0 { + return fmt.Errorf("pip install failed: %s", output.Stderr) + } + + return nil +} + +// Execute runs Pylint with the given config and files. +func (a *Adapter) Execute(ctx context.Context, config []byte, files []string) (*adapter.ToolOutput, error) { + // Implementation in executor.go + return a.execute(ctx, config, files) +} + +// ParseOutput converts Pylint JSON output to violations. +func (a *Adapter) ParseOutput(output *adapter.ToolOutput) ([]adapter.Violation, error) { + // Implementation in parser.go + return parseOutput(output) +} + +// getVenvPath returns the path to Pylint virtualenv. +func (a *Adapter) getVenvPath() string { + return filepath.Join(a.ToolsDir, "pylint-venv") +} + +// getPylintPath returns the path to local Pylint binary. +func (a *Adapter) getPylintPath() string { + venvPath := a.getVenvPath() + if runtime.GOOS == "windows" { + return filepath.Join(venvPath, "Scripts", "pylint.exe") + } + return filepath.Join(venvPath, "bin", "pylint") +} + +// getPipPath returns the path to pip in virtualenv. +func (a *Adapter) getPipPath() string { + venvPath := a.getVenvPath() + if runtime.GOOS == "windows" { + return filepath.Join(venvPath, "Scripts", "pip.exe") + } + return filepath.Join(venvPath, "bin", "pip") +} + +// getPythonCommand returns the Python command to use. +func (a *Adapter) getPythonCommand() string { + // Try python3 first, then python + if _, err := exec.LookPath("python3"); err == nil { + return "python3" + } + return "python" +} + +// getPythonInVenv returns the path to Python in virtualenv. +func (a *Adapter) getPythonInVenv() string { + venvPath := a.getVenvPath() + if runtime.GOOS == "windows" { + return filepath.Join(venvPath, "Scripts", "python.exe") + } + return filepath.Join(venvPath, "bin", "python") +} + +// getPylintCommand returns the Pylint command to use. +func (a *Adapter) getPylintCommand() string { + // If explicit path is set, use it + if a.PylintPath != "" { + return a.PylintPath + } + + // Try local installation first (virtualenv) + localPath := a.getPylintPath() + if _, err := os.Stat(localPath); err == nil { + return localPath + } + + // Try global pylint + if _, err := exec.LookPath("pylint"); err == nil { + return "pylint" + } + + // Fall back to local path (will fail with proper error) + return localPath +} diff --git a/internal/adapter/pylint/adapter_test.go b/internal/adapter/pylint/adapter_test.go new file mode 100644 index 0000000..bbc2a82 --- /dev/null +++ b/internal/adapter/pylint/adapter_test.go @@ -0,0 +1,208 @@ +package pylint + +import ( + "context" + "os" + "path/filepath" + "runtime" + "testing" + + "github.com/DevSymphony/sym-cli/internal/adapter" +) + +func TestNewAdapter(t *testing.T) { + adapter := NewAdapter("") + if adapter == nil { + t.Fatal("NewAdapter() returned nil") + } + + if adapter.ToolsDir == "" { + t.Error("ToolsDir should not be empty") + } +} + +func TestNewAdapter_CustomToolsDir(t *testing.T) { + toolsDir := "/custom/tools" + + a := NewAdapter(toolsDir) + + if a.ToolsDir != toolsDir { + t.Errorf("ToolsDir = %q, want %q", a.ToolsDir, toolsDir) + } +} + +func TestName(t *testing.T) { + a := NewAdapter("") + if a.Name() != "pylint" { + t.Errorf("Name() = %q, want %q", a.Name(), "pylint") + } +} + +func TestGetCapabilities(t *testing.T) { + a := NewAdapter("") + caps := a.GetCapabilities() + + if caps.Name != "pylint" { + t.Errorf("GetCapabilities().Name = %q, want %q", caps.Name, "pylint") + } + + expectedLangs := []string{"python", "py"} + for _, lang := range expectedLangs { + found := false + for _, supported := range caps.SupportedLanguages { + if supported == lang { + found = true + break + } + } + if !found { + t.Errorf("GetCapabilities() missing language: %s", lang) + } + } + + expectedCategories := []string{"naming", "style", "documentation", "error_handling", "complexity"} + for _, cat := range expectedCategories { + found := false + for _, supported := range caps.SupportedCategories { + if supported == cat { + found = true + break + } + } + if !found { + t.Errorf("GetCapabilities() missing category: %s", cat) + } + } +} + +func TestGetVenvPath(t *testing.T) { + a := NewAdapter("/test/tools") + expected := filepath.Join("/test/tools", "pylint-venv") + + got := a.getVenvPath() + if got != expected { + t.Errorf("getVenvPath() = %q, want %q", got, expected) + } +} + +func TestGetPylintPath(t *testing.T) { + a := NewAdapter("/test/tools") + + var expected string + if runtime.GOOS == "windows" { + expected = filepath.Join("/test/tools", "pylint-venv", "Scripts", "pylint.exe") + } else { + expected = filepath.Join("/test/tools", "pylint-venv", "bin", "pylint") + } + + got := a.getPylintPath() + if got != expected { + t.Errorf("getPylintPath() = %q, want %q", got, expected) + } +} + +func TestGetPipPath(t *testing.T) { + a := NewAdapter("/test/tools") + + var expected string + if runtime.GOOS == "windows" { + expected = filepath.Join("/test/tools", "pylint-venv", "Scripts", "pip.exe") + } else { + expected = filepath.Join("/test/tools", "pylint-venv", "bin", "pip") + } + + got := a.getPipPath() + if got != expected { + t.Errorf("getPipPath() = %q, want %q", got, expected) + } +} + +func TestCheckAvailability_NotFound(t *testing.T) { + a := NewAdapter("/nonexistent/path") + + ctx := context.Background() + err := a.CheckAvailability(ctx) + + if err == nil { + t.Log("Pylint found globally, test skipped") + } +} + +func TestInstall(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "pylint-install-test-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer func() { _ = os.RemoveAll(tmpDir) }() + + a := NewAdapter(tmpDir) + + ctx := context.Background() + config := adapter.InstallConfig{ + ToolsDir: tmpDir, + } + + err = a.Install(ctx, config) + if err != nil { + t.Logf("Install failed (expected if Python unavailable): %v", err) + } +} + +func TestExecute_EmptyFiles(t *testing.T) { + a := NewAdapter(t.TempDir()) + + ctx := context.Background() + config := []byte(`[MASTER]`) + + output, err := a.Execute(ctx, config, []string{}) + if err != nil { + t.Fatalf("Execute() error = %v", err) + } + + if output.Stdout != "[]" { + t.Errorf("Expected empty array for no files, got %q", output.Stdout) + } +} + +func TestParseOutput(t *testing.T) { + a := NewAdapter("") + + output := &adapter.ToolOutput{ + Stdout: `[ + { + "type": "convention", + "module": "test", + "line": 1, + "column": 0, + "path": "test.py", + "symbol": "missing-module-docstring", + "message": "Missing module docstring", + "message-id": "C0114" + } + ]`, + Stderr: "", + ExitCode: 16, + } + + violations, err := a.ParseOutput(output) + if err != nil { + t.Fatalf("ParseOutput() error = %v", err) + } + + if len(violations) == 0 { + t.Error("Expected violations to be parsed") + } + + if len(violations) > 0 { + v := violations[0] + if v.File != "test.py" { + t.Errorf("File = %q, want %q", v.File, "test.py") + } + if v.Line != 1 { + t.Errorf("Line = %d, want %d", v.Line, 1) + } + if v.RuleID != "C0114/missing-module-docstring" { + t.Errorf("RuleID = %q, want %q", v.RuleID, "C0114/missing-module-docstring") + } + } +} diff --git a/internal/adapter/pylint/converter.go b/internal/adapter/pylint/converter.go new file mode 100644 index 0000000..bac55ae --- /dev/null +++ b/internal/adapter/pylint/converter.go @@ -0,0 +1,341 @@ +package pylint + +import ( + "context" + "encoding/json" + "fmt" + "os" + "strings" + "sync" + + "github.com/DevSymphony/sym-cli/internal/adapter" + "github.com/DevSymphony/sym-cli/internal/llm" + "github.com/DevSymphony/sym-cli/pkg/schema" +) + +// Converter converts rules to Pylint configuration using LLM +type Converter struct{} + +// NewConverter creates a new Pylint converter +func NewConverter() *Converter { + return &Converter{} +} + +// Name returns the linter name +func (c *Converter) Name() string { + return "pylint" +} + +// SupportedLanguages returns supported languages +func (c *Converter) SupportedLanguages() []string { + return []string{"python", "py"} +} + +// GetLLMDescription returns a description of Pylint's capabilities for LLM routing +func (c *Converter) GetLLMDescription() string { + return `Python code quality via Pylint (PEP 8 style, docstrings, complexity, error handling, imports) + - CAN: Naming conventions (snake_case, PascalCase), line length limits, docstring requirements, + cyclomatic complexity, function arguments/locals limits, import ordering, + bare except blocks, unused variables/imports, dangerous default values, eval/exec usage + - CANNOT: Business logic validation, runtime behavior checks, type correctness beyond basic inference, + async/await pattern validation, complex decorator analysis` +} + +// ConvertRules converts user rules to Pylint configuration using LLM +func (c *Converter) ConvertRules(ctx context.Context, rules []schema.UserRule, llmClient *llm.Client) (*adapter.LinterConfig, error) { + if llmClient == nil { + return nil, fmt.Errorf("LLM client is required") + } + + // Convert rules in parallel using goroutines + type ruleResult struct { + index int + symbol string + options map[string]interface{} + err error + } + + results := make(chan ruleResult, len(rules)) + var wg sync.WaitGroup + + // Process each rule in parallel + for i, rule := range rules { + wg.Add(1) + go func(idx int, r schema.UserRule) { + defer wg.Done() + + symbol, options, err := c.convertSingleRule(ctx, r, llmClient) + results <- ruleResult{ + index: idx, + symbol: symbol, + options: options, + err: err, + } + }(i, rule) + } + + // Wait for all goroutines + go func() { + wg.Wait() + close(results) + }() + + // Collect results + enabledRules := make([]string, 0) + options := make(map[string]map[string]interface{}) + var errors []string + skippedCount := 0 + + for result := range results { + if result.err != nil { + errors = append(errors, fmt.Sprintf("Rule %d: %v", result.index+1, result.err)) + fmt.Fprintf(os.Stderr, "โš ๏ธ Pylint rule %d conversion error: %v\n", result.index+1, result.err) + continue + } + + if result.symbol != "" { + enabledRules = append(enabledRules, result.symbol) + if len(result.options) > 0 { + // Group options by section + for key, value := range result.options { + section := getOptionSection(key) + if _, ok := options[section]; !ok { + options[section] = make(map[string]interface{}) + } + options[section][key] = value + } + } + fmt.Fprintf(os.Stderr, "โœ“ Pylint rule %d โ†’ %s\n", result.index+1, result.symbol) + } else { + skippedCount++ + fmt.Fprintf(os.Stderr, "โŠ˜ Pylint rule %d skipped (cannot be enforced by Pylint)\n", result.index+1) + } + } + + if skippedCount > 0 { + fmt.Fprintf(os.Stderr, "โ„น๏ธ %d rule(s) skipped for Pylint (will use llm-validator)\n", skippedCount) + } + + if len(enabledRules) == 0 { + return nil, fmt.Errorf("no rules converted successfully: %v", errors) + } + + // Generate .pylintrc content (INI format) + content := c.generatePylintRC(enabledRules, options) + + return &adapter.LinterConfig{ + Filename: ".pylintrc", + Content: []byte(content), + Format: "ini", + }, nil +} + +// convertSingleRule converts a single user rule to Pylint rule using LLM +func (c *Converter) convertSingleRule(ctx context.Context, rule schema.UserRule, llmClient *llm.Client) (string, map[string]interface{}, error) { + systemPrompt := `You are a Pylint configuration expert. Convert natural language Python coding rules to Pylint rule configurations. + +Return ONLY a JSON object (no markdown fences) with this structure: +{ + "symbol": "pylint-rule-symbol", + "message_id": "C0116", + "options": {"key": "value", ...} +} + +Common Pylint rules: +- Naming: invalid-name (C0103), disallowed-name (C0104) + Options: variable-rgx, function-rgx, class-rgx, const-rgx, argument-rgx +- Docstrings: missing-module-docstring (C0114), missing-class-docstring (C0115), missing-function-docstring (C0116) +- Length: line-too-long (C0301), too-many-lines (C0302) + Options: max-line-length, max-module-lines +- Imports: multiple-imports (C0410), wrong-import-order (C0411), unused-import (W0611) +- Error handling: bare-except (W0702), broad-except (W0703) + Options: overgeneral-exceptions +- Complexity: too-many-branches (R0912), too-many-arguments (R0913), too-many-locals (R0914), too-many-statements (R0915), too-many-nested-blocks (R1702) + Options: max-branches, max-args, max-locals, max-statements, max-nested-blocks +- Security: dangerous-default-value (W0102), exec-used (W0122), eval-used (W0123) +- Unused: unused-variable (W0612), unused-argument (W0613) + +If the rule cannot be expressed in Pylint, return: +{ + "symbol": "", + "message_id": "", + "options": null +} + +Examples: + +Input: "All functions must have docstrings" +Output: +{ + "symbol": "missing-function-docstring", + "message_id": "C0116", + "options": null +} + +Input: "Lines must not exceed 120 characters" +Output: +{ + "symbol": "line-too-long", + "message_id": "C0301", + "options": {"max-line-length": 120} +} + +Input: "Functions should have at most 5 arguments" +Output: +{ + "symbol": "too-many-arguments", + "message_id": "R0913", + "options": {"max-args": 5} +} + +Input: "Don't use bare except blocks" +Output: +{ + "symbol": "bare-except", + "message_id": "W0702", + "options": null +}` + + userPrompt := fmt.Sprintf("Convert this rule to Pylint configuration:\n\n%s", rule.Say) + if rule.Severity != "" { + userPrompt += fmt.Sprintf("\nSeverity: %s", rule.Severity) + } + + // Call LLM + response, err := llmClient.Complete(ctx, systemPrompt, userPrompt) + if err != nil { + return "", nil, fmt.Errorf("LLM call failed: %w", err) + } + + // Parse response + response = strings.TrimSpace(response) + response = strings.TrimPrefix(response, "```json") + response = strings.TrimPrefix(response, "```") + response = strings.TrimSuffix(response, "```") + response = strings.TrimSpace(response) + + var result struct { + Symbol string `json:"symbol"` + MessageID string `json:"message_id"` + Options map[string]interface{} `json:"options"` + } + + if err := json.Unmarshal([]byte(response), &result); err != nil { + return "", nil, fmt.Errorf("failed to parse LLM response: %w", err) + } + + // If symbol is empty, this rule cannot be converted + if result.Symbol == "" { + return "", nil, nil + } + + return result.Symbol, result.Options, nil +} + +// generatePylintRC generates .pylintrc content in INI format +func (c *Converter) generatePylintRC(enabledRules []string, options map[string]map[string]interface{}) string { + var sb strings.Builder + + // Header + sb.WriteString("[MASTER]\n") + sb.WriteString("# Generated by Symphony CLI\n\n") + + // Messages control section + sb.WriteString("[MESSAGES CONTROL]\n") + sb.WriteString("enable=" + strings.Join(enabledRules, ",") + "\n\n") + + // FORMAT section + if formatOpts, ok := options["FORMAT"]; ok && len(formatOpts) > 0 { + sb.WriteString("[FORMAT]\n") + for key, value := range formatOpts { + sb.WriteString(fmt.Sprintf("%s=%v\n", key, value)) + } + sb.WriteString("\n") + } + + // BASIC section (naming) + if basicOpts, ok := options["BASIC"]; ok && len(basicOpts) > 0 { + sb.WriteString("[BASIC]\n") + for key, value := range basicOpts { + sb.WriteString(fmt.Sprintf("%s=%v\n", key, value)) + } + sb.WriteString("\n") + } + + // DESIGN section (complexity) + if designOpts, ok := options["DESIGN"]; ok && len(designOpts) > 0 { + sb.WriteString("[DESIGN]\n") + for key, value := range designOpts { + sb.WriteString(fmt.Sprintf("%s=%v\n", key, value)) + } + sb.WriteString("\n") + } + + // EXCEPTIONS section + if exceptOpts, ok := options["EXCEPTIONS"]; ok && len(exceptOpts) > 0 { + sb.WriteString("[EXCEPTIONS]\n") + for key, value := range exceptOpts { + sb.WriteString(fmt.Sprintf("%s=%v\n", key, value)) + } + sb.WriteString("\n") + } + + return sb.String() +} + +// getOptionSection returns the Pylint config section for a given option +func getOptionSection(option string) string { + formatOptions := map[string]bool{ + "max-line-length": true, + "max-module-lines": true, + "indent-string": true, + "indent-after-paren": true, + } + + basicOptions := map[string]bool{ + "variable-rgx": true, + "function-rgx": true, + "class-rgx": true, + "const-rgx": true, + "argument-rgx": true, + "attr-rgx": true, + "method-rgx": true, + "module-rgx": true, + "good-names": true, + "bad-names": true, + "include-naming-hint": true, + } + + designOptions := map[string]bool{ + "max-args": true, + "max-locals": true, + "max-returns": true, + "max-branches": true, + "max-statements": true, + "max-parents": true, + "max-attributes": true, + "min-public-methods": true, + "max-public-methods": true, + "max-bool-expr": true, + "max-nested-blocks": true, + } + + exceptOptions := map[string]bool{ + "overgeneral-exceptions": true, + } + + if formatOptions[option] { + return "FORMAT" + } + if basicOptions[option] { + return "BASIC" + } + if designOptions[option] { + return "DESIGN" + } + if exceptOptions[option] { + return "EXCEPTIONS" + } + + return "MASTER" +} diff --git a/internal/adapter/pylint/executor.go b/internal/adapter/pylint/executor.go new file mode 100644 index 0000000..6b4ae3a --- /dev/null +++ b/internal/adapter/pylint/executor.go @@ -0,0 +1,68 @@ +package pylint + +import ( + "context" + "fmt" + "os" + "path/filepath" + + "github.com/DevSymphony/sym-cli/internal/adapter" +) + +// execute runs Pylint with the given config and files. +func (a *Adapter) execute(ctx context.Context, config []byte, files []string) (*adapter.ToolOutput, error) { + if len(files) == 0 { + return &adapter.ToolOutput{ + Stdout: "[]", + ExitCode: 0, + }, nil + } + + // Write config to temp file + configPath, err := a.writeConfigFile(config) + if err != nil { + return nil, fmt.Errorf("failed to write config: %w", err) + } + defer func() { _ = os.Remove(configPath) }() + + // Get command and arguments + pylintCmd := a.getPylintCommand() + args := a.getExecutionArgs(configPath, files) + + // Execute - uses CWD by default + return a.executor.Execute(ctx, pylintCmd, args...) +} + +// getExecutionArgs returns the arguments for Pylint execution. +func (a *Adapter) getExecutionArgs(configPath string, files []string) []string { + args := []string{ + "--output-format=json", + "--disable=all", // Disable all checks first + "--enable=invalid-name", // Enable only naming checks (C0103) + "--rcfile=" + configPath, + } + args = append(args, files...) + + return args +} + +// writeConfigFile writes Pylint config to a temp file. +func (a *Adapter) writeConfigFile(config []byte) (string, error) { + tmpDir := filepath.Join(a.ToolsDir, ".tmp") + if err := os.MkdirAll(tmpDir, 0755); err != nil { + return "", err + } + + tmpFile, err := os.CreateTemp(tmpDir, "pylintrc-*") + if err != nil { + return "", err + } + defer func() { _ = tmpFile.Close() }() + + if _, err := tmpFile.Write(config); err != nil { + _ = os.Remove(tmpFile.Name()) + return "", err + } + + return tmpFile.Name(), nil +} diff --git a/internal/adapter/pylint/executor_test.go b/internal/adapter/pylint/executor_test.go new file mode 100644 index 0000000..b3a99b3 --- /dev/null +++ b/internal/adapter/pylint/executor_test.go @@ -0,0 +1,134 @@ +package pylint + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestWriteConfigFile(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "pylint-config-test-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer func() { _ = os.RemoveAll(tmpDir) }() + + a := NewAdapter(tmpDir) + config := []byte(`[MASTER] +# Generated by Symphony CLI + +[MESSAGES CONTROL] +enable=missing-function-docstring,line-too-long + +[FORMAT] +max-line-length=100 +`) + + configPath, err := a.writeConfigFile(config) + if err != nil { + t.Fatalf("writeConfigFile() error = %v", err) + } + defer func() { _ = os.Remove(configPath) }() + + // Verify file exists + if _, err := os.Stat(configPath); os.IsNotExist(err) { + t.Error("Config file was not created") + } + + // Verify content + content, err := os.ReadFile(configPath) + if err != nil { + t.Fatalf("Failed to read config file: %v", err) + } + + if !strings.Contains(string(content), "[MESSAGES CONTROL]") { + t.Error("Config file missing [MESSAGES CONTROL] section") + } + + if !strings.Contains(string(content), "max-line-length=100") { + t.Error("Config file missing max-line-length option") + } +} + +func TestWriteConfigFile_CreatesDirectory(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "pylint-config-test-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer func() { _ = os.RemoveAll(tmpDir) }() + + a := NewAdapter(tmpDir) + config := []byte(`[MASTER]`) + + configPath, err := a.writeConfigFile(config) + if err != nil { + t.Fatalf("writeConfigFile() error = %v", err) + } + defer func() { _ = os.Remove(configPath) }() + + // Verify .tmp directory was created + tmpConfigDir := filepath.Join(tmpDir, ".tmp") + if _, err := os.Stat(tmpConfigDir); os.IsNotExist(err) { + t.Error(".tmp directory was not created") + } +} + +func TestGetExecutionArgs(t *testing.T) { + a := NewAdapter("/test/tools") + configPath := "/tmp/pylintrc-123" + files := []string{"src/main.py", "src/utils.py"} + + args := a.getExecutionArgs(configPath, files) + + // Check required arguments + expectedArgs := []string{ + "--output-format=json", + "--rcfile=" + configPath, + "src/main.py", + "src/utils.py", + } + + for _, expected := range expectedArgs { + found := false + for _, arg := range args { + if arg == expected { + found = true + break + } + } + if !found { + t.Errorf("getExecutionArgs() missing expected arg: %s", expected) + } + } +} + +func TestExecute_TempFileCleanup(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "pylint-exec-test-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer func() { _ = os.RemoveAll(tmpDir) }() + + a := NewAdapter(tmpDir) + config := []byte(`[MASTER]`) + + // Write config file + configPath, err := a.writeConfigFile(config) + if err != nil { + t.Fatalf("writeConfigFile() error = %v", err) + } + + // File should exist + if _, err := os.Stat(configPath); os.IsNotExist(err) { + t.Error("Config file should exist after writeConfigFile") + } + + // Manually remove (simulating cleanup in execute) + _ = os.Remove(configPath) + + // File should be gone + if _, err := os.Stat(configPath); !os.IsNotExist(err) { + t.Error("Config file should be removed after cleanup") + } +} diff --git a/internal/adapter/pylint/parser.go b/internal/adapter/pylint/parser.go new file mode 100644 index 0000000..cb8d977 --- /dev/null +++ b/internal/adapter/pylint/parser.go @@ -0,0 +1,91 @@ +package pylint + +import ( + "encoding/json" + "fmt" + "strings" + + "github.com/DevSymphony/sym-cli/internal/adapter" +) + +// PylintMessage represents a single Pylint JSON message. +// Pylint outputs an array of these messages when using --output-format=json. +type PylintMessage struct { + Type string `json:"type"` // "convention", "refactor", "warning", "error", "fatal" + Module string `json:"module"` // Module name + Obj string `json:"obj"` // Object name (function, class, etc.) + Line int `json:"line"` // Line number + Column int `json:"column"` // Column number + EndLine int `json:"endLine"` // End line number (optional) + EndColumn int `json:"endColumn"` // End column number (optional) + Path string `json:"path"` // File path + Symbol string `json:"symbol"` // Rule symbol (e.g., "missing-docstring") + Message string `json:"message"` // Human-readable message + MessageID string `json:"message-id"` // Message ID (e.g., "C0116") +} + +// PylintOutput is an array of Pylint messages. +type PylintOutput []PylintMessage + +// parseOutput converts Pylint JSON output to violations. +func parseOutput(output *adapter.ToolOutput) ([]adapter.Violation, error) { + if output.Stdout == "" || output.Stdout == "[]" { + return nil, nil // No violations + } + + var pylintOutput PylintOutput + if err := json.Unmarshal([]byte(output.Stdout), &pylintOutput); err != nil { + // If JSON parsing fails, check for errors in stderr + if output.Stderr != "" { + return nil, fmt.Errorf("pylint error: %s", output.Stderr) + } + return nil, fmt.Errorf("failed to parse Pylint output: %w", err) + } + + var violations []adapter.Violation + + for _, msg := range pylintOutput { + violations = append(violations, adapter.Violation{ + File: msg.Path, + Line: msg.Line, + Column: msg.Column, + Message: msg.Message, + Severity: pylintTypeToSeverity(msg.Type), + RuleID: formatRuleID(msg.MessageID, msg.Symbol), + }) + } + + return violations, nil +} + +// pylintTypeToSeverity converts Pylint message type to severity string. +// Pylint types: F (fatal), E (error), W (warning), C (convention), R (refactor) +func pylintTypeToSeverity(pylintType string) string { + switch strings.ToLower(pylintType) { + case "fatal", "error": + return "error" + case "warning": + return "warning" + case "convention", "refactor": + return "warning" + case "info", "information": + return "info" + default: + return "warning" + } +} + +// formatRuleID creates a rule ID from message ID and symbol. +// Returns format like "C0116/missing-function-docstring" +func formatRuleID(messageID, symbol string) string { + if messageID != "" && symbol != "" { + return fmt.Sprintf("%s/%s", messageID, symbol) + } + if messageID != "" { + return messageID + } + if symbol != "" { + return symbol + } + return "unknown" +} diff --git a/internal/adapter/pylint/parser_test.go b/internal/adapter/pylint/parser_test.go new file mode 100644 index 0000000..d45e370 --- /dev/null +++ b/internal/adapter/pylint/parser_test.go @@ -0,0 +1,207 @@ +package pylint + +import ( + "testing" + + "github.com/DevSymphony/sym-cli/internal/adapter" +) + +func TestParseOutput_Empty(t *testing.T) { + tests := []struct { + name string + stdout string + }{ + {"empty string", ""}, + {"empty array", "[]"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + output := &adapter.ToolOutput{ + Stdout: tt.stdout, + ExitCode: 0, + } + + violations, err := parseOutput(output) + if err != nil { + t.Errorf("parseOutput() error = %v", err) + } + if len(violations) != 0 { + t.Errorf("parseOutput() returned %d violations, want 0", len(violations)) + } + }) + } +} + +func TestParseOutput_SingleViolation(t *testing.T) { + output := &adapter.ToolOutput{ + Stdout: `[{ + "type": "error", + "module": "mymodule", + "obj": "MyClass.my_method", + "line": 15, + "column": 4, + "endLine": 15, + "endColumn": 10, + "path": "src/mymodule.py", + "symbol": "undefined-variable", + "message": "Undefined variable 'foo'", + "message-id": "E0602" + }]`, + ExitCode: 2, + } + + violations, err := parseOutput(output) + if err != nil { + t.Fatalf("parseOutput() error = %v", err) + } + + if len(violations) != 1 { + t.Fatalf("parseOutput() returned %d violations, want 1", len(violations)) + } + + v := violations[0] + if v.File != "src/mymodule.py" { + t.Errorf("File = %q, want %q", v.File, "src/mymodule.py") + } + if v.Line != 15 { + t.Errorf("Line = %d, want %d", v.Line, 15) + } + if v.Column != 4 { + t.Errorf("Column = %d, want %d", v.Column, 4) + } + if v.Message != "Undefined variable 'foo'" { + t.Errorf("Message = %q, want %q", v.Message, "Undefined variable 'foo'") + } + if v.Severity != "error" { + t.Errorf("Severity = %q, want %q", v.Severity, "error") + } + if v.RuleID != "E0602/undefined-variable" { + t.Errorf("RuleID = %q, want %q", v.RuleID, "E0602/undefined-variable") + } +} + +func TestParseOutput_MultipleViolations(t *testing.T) { + output := &adapter.ToolOutput{ + Stdout: `[ + { + "type": "convention", + "module": "test", + "line": 1, + "column": 0, + "path": "test.py", + "symbol": "missing-module-docstring", + "message": "Missing module docstring", + "message-id": "C0114" + }, + { + "type": "warning", + "module": "test", + "line": 5, + "column": 4, + "path": "test.py", + "symbol": "unused-variable", + "message": "Unused variable 'x'", + "message-id": "W0612" + }, + { + "type": "refactor", + "module": "test", + "line": 10, + "column": 0, + "path": "test.py", + "symbol": "too-many-branches", + "message": "Too many branches (15/12)", + "message-id": "R0912" + } + ]`, + ExitCode: 28, + } + + violations, err := parseOutput(output) + if err != nil { + t.Fatalf("parseOutput() error = %v", err) + } + + if len(violations) != 3 { + t.Fatalf("parseOutput() returned %d violations, want 3", len(violations)) + } +} + +func TestPylintTypeToSeverity(t *testing.T) { + tests := []struct { + pylintType string + want string + }{ + {"fatal", "error"}, + {"error", "error"}, + {"warning", "warning"}, + {"convention", "warning"}, + {"refactor", "warning"}, + {"info", "info"}, + {"information", "info"}, + {"FATAL", "error"}, + {"ERROR", "error"}, + {"WARNING", "warning"}, + {"CONVENTION", "warning"}, + {"unknown", "warning"}, + {"", "warning"}, + } + + for _, tt := range tests { + t.Run(tt.pylintType, func(t *testing.T) { + got := pylintTypeToSeverity(tt.pylintType) + if got != tt.want { + t.Errorf("pylintTypeToSeverity(%q) = %q, want %q", tt.pylintType, got, tt.want) + } + }) + } +} + +func TestFormatRuleID(t *testing.T) { + tests := []struct { + name string + messageID string + symbol string + want string + }{ + {"both present", "C0116", "missing-function-docstring", "C0116/missing-function-docstring"}, + {"only message ID", "C0116", "", "C0116"}, + {"only symbol", "", "missing-function-docstring", "missing-function-docstring"}, + {"neither", "", "", "unknown"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := formatRuleID(tt.messageID, tt.symbol) + if got != tt.want { + t.Errorf("formatRuleID(%q, %q) = %q, want %q", tt.messageID, tt.symbol, got, tt.want) + } + }) + } +} + +func TestParseOutput_InvalidJSON(t *testing.T) { + output := &adapter.ToolOutput{ + Stdout: "not valid json", + ExitCode: 1, + } + + _, err := parseOutput(output) + if err == nil { + t.Error("parseOutput() expected error for invalid JSON") + } +} + +func TestParseOutput_WithStderr(t *testing.T) { + output := &adapter.ToolOutput{ + Stdout: "invalid", + Stderr: "pylint: error: no such module", + ExitCode: 1, + } + + _, err := parseOutput(output) + if err == nil { + t.Error("parseOutput() expected error") + } +} diff --git a/internal/adapter/pylint/register.go b/internal/adapter/pylint/register.go new file mode 100644 index 0000000..5b16dec --- /dev/null +++ b/internal/adapter/pylint/register.go @@ -0,0 +1,14 @@ +package pylint + +import ( + "github.com/DevSymphony/sym-cli/internal/adapter" + "github.com/DevSymphony/sym-cli/internal/adapter/registry" +) + +func init() { + _ = registry.Global().RegisterTool( + NewAdapter(adapter.DefaultToolsDir()), + NewConverter(), + ".pylintrc", + ) +} diff --git a/internal/bootstrap/adapters.go b/internal/bootstrap/adapters.go index 64a43c1..44c1566 100644 --- a/internal/bootstrap/adapters.go +++ b/internal/bootstrap/adapters.go @@ -8,6 +8,7 @@ import ( _ "github.com/DevSymphony/sym-cli/internal/adapter/eslint" _ "github.com/DevSymphony/sym-cli/internal/adapter/pmd" _ "github.com/DevSymphony/sym-cli/internal/adapter/prettier" + _ "github.com/DevSymphony/sym-cli/internal/adapter/pylint" _ "github.com/DevSymphony/sym-cli/internal/adapter/tsc" ) From fa451de9cd8eb2fe15de34874bab53c95e677432 Mon Sep 17 00:00:00 2001 From: ikjeong Date: Wed, 26 Nov 2025 02:44:15 +0000 Subject: [PATCH 05/13] refactor: update logging to use os.Stderr for error and info messages --- internal/adapter/eslint/converter.go | 9 +++++---- internal/converter/converter.go | 8 ++++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/internal/adapter/eslint/converter.go b/internal/adapter/eslint/converter.go index f7b193f..fb3b1ee 100644 --- a/internal/adapter/eslint/converter.go +++ b/internal/adapter/eslint/converter.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "os" "strings" "sync" @@ -84,21 +85,21 @@ func (c *Converter) ConvertRules(ctx context.Context, rules []schema.UserRule, l for result := range results { if result.err != nil { errors = append(errors, fmt.Sprintf("Rule %d: %v", result.index+1, result.err)) - fmt.Printf("โš ๏ธ ESLint rule %d conversion error: %v\n", result.index+1, result.err) + fmt.Fprintf(os.Stderr, "โš ๏ธ ESLint rule %d conversion error: %v\n", result.index+1, result.err) continue } if result.ruleName != "" { eslintRules[result.ruleName] = result.config - fmt.Printf("โœ“ ESLint rule %d โ†’ %s\n", result.index+1, result.ruleName) + fmt.Fprintf(os.Stderr, "โœ“ ESLint rule %d โ†’ %s\n", result.index+1, result.ruleName) } else { skippedCount++ - fmt.Printf("โŠ˜ ESLint rule %d skipped (cannot be enforced by ESLint)\n", result.index+1) + fmt.Fprintf(os.Stderr, "โŠ˜ ESLint rule %d skipped (cannot be enforced by ESLint)\n", result.index+1) } } if skippedCount > 0 { - fmt.Printf("โ„น๏ธ %d rule(s) skipped for ESLint (will use llm-validator)\n", skippedCount) + fmt.Fprintf(os.Stderr, "โ„น๏ธ %d rule(s) skipped for ESLint (will use llm-validator)\n", skippedCount) } if len(eslintRules) == 0 { diff --git a/internal/converter/converter.go b/internal/converter/converter.go index cc7d706..23f03f2 100644 --- a/internal/converter/converter.go +++ b/internal/converter/converter.go @@ -144,7 +144,7 @@ func (c *Converter) Convert(ctx context.Context, userPolicy *schema.UserPolicy) result.GeneratedFiles = append(result.GeneratedFiles, outputPath) mu.Unlock() - fmt.Printf("โœ“ Generated %s configuration: %s\n", linter, outputPath) + fmt.Fprintf(os.Stderr, "โœ“ Generated %s configuration: %s\n", linter, outputPath) }(linterName, rules) } @@ -244,7 +244,7 @@ func (c *Converter) Convert(ctx context.Context, userPolicy *schema.UserPolicy) } result.GeneratedFiles = append(result.GeneratedFiles, codePolicyPath) - fmt.Printf("โœ“ Generated code policy: %s\n", codePolicyPath) + fmt.Fprintf(os.Stderr, "โœ“ Generated code policy: %s\n", codePolicyPath) return result, nil } @@ -380,7 +380,7 @@ Reason: Requires knowing which packages are "large"`, linterDescriptions, availa // Call LLM response, err := c.llmClient.Complete(ctx, systemPrompt, userPrompt) if err != nil { - fmt.Printf("Warning: LLM routing failed for rule %s: %v\n", rule.ID, err) + fmt.Fprintf(os.Stderr, "Warning: LLM routing failed for rule %s: %v\n", rule.ID, err) return []string{} // Will fall back to llm-validator } @@ -393,7 +393,7 @@ Reason: Requires knowing which packages are "large"`, linterDescriptions, availa var selectedLinters []string if err := json.Unmarshal([]byte(response), &selectedLinters); err != nil { - fmt.Printf("Warning: Failed to parse LLM response for rule %s: %v\n", rule.ID, err) + fmt.Fprintf(os.Stderr, "Warning: Failed to parse LLM response for rule %s: %v\n", rule.ID, err) return []string{} // Will fall back to llm-validator } From 8265e5a38549e785b2226c9050a3f451e500c400 Mon Sep 17 00:00:00 2001 From: ikjeong Date: Wed, 26 Nov 2025 08:12:53 +0000 Subject: [PATCH 06/13] feat: add GetRoutingHints interface for LLM rule routing --- internal/adapter/adapter.go | 5 ++ internal/adapter/checkstyle/converter.go | 85 +++++++++++++++++++++--- internal/adapter/eslint/converter.go | 62 ++++++++++++++--- internal/adapter/pmd/converter.go | 50 +++++++++----- internal/adapter/prettier/converter.go | 17 ++++- internal/adapter/pylint/converter.go | 18 ++++- internal/adapter/tsc/converter.go | 17 ++++- internal/converter/converter.go | 31 ++++++++- 8 files changed, 241 insertions(+), 44 deletions(-) diff --git a/internal/adapter/adapter.go b/internal/adapter/adapter.go index 105faf1..33d09b8 100644 --- a/internal/adapter/adapter.go +++ b/internal/adapter/adapter.go @@ -109,6 +109,11 @@ type LinterConverter interface { // This is used in the LLM prompt to help route rules to appropriate linters. GetLLMDescription() string + // GetRoutingHints returns routing rules for LLM to decide when to use this linter. + // Each hint is a rule like "For Java naming rules โ†’ ALWAYS use checkstyle". + // These hints are collected and included in the LLM prompt for rule routing. + GetRoutingHints() []string + // ConvertRules converts user rules to native linter configuration using LLM ConvertRules(ctx context.Context, rules []schema.UserRule, llmClient *llm.Client) (*LinterConfig, error) } diff --git a/internal/adapter/checkstyle/converter.go b/internal/adapter/checkstyle/converter.go index 165cf8f..94412c3 100644 --- a/internal/adapter/checkstyle/converter.go +++ b/internal/adapter/checkstyle/converter.go @@ -21,12 +21,10 @@ func NewConverter() *Converter { return &Converter{} } -// Name returns the linter name func (c *Converter) Name() string { return "checkstyle" } -// SupportedLanguages returns supported languages func (c *Converter) SupportedLanguages() []string { return []string{"java"} } @@ -38,7 +36,15 @@ func (c *Converter) GetLLMDescription() string { - CANNOT: Runtime behavior, business logic, security vulnerabilities, advanced design patterns` } -// checkstyleModule represents a Checkstyle module +// GetRoutingHints returns routing rules for LLM to decide when to use Checkstyle +func (c *Converter) GetRoutingHints() []string { + return []string{ + "For Java naming rules (class names, variable names, method names) โ†’ ALWAYS use checkstyle", + "For Java formatting rules (line length, indentation, whitespace) โ†’ use checkstyle", + "For Java import rules (star imports, unused imports) โ†’ use checkstyle", + } +} + type checkstyleModule struct { XMLName xml.Name `xml:"module"` Name string `xml:"name,attr"` @@ -46,14 +52,12 @@ type checkstyleModule struct { Modules []checkstyleModule `xml:"module,omitempty"` } -// checkstyleProperty represents a property type checkstyleProperty struct { XMLName xml.Name `xml:"property"` Name string `xml:"name,attr"` Value string `xml:"value,attr"` } -// checkstyleConfig represents root configuration type checkstyleConfig struct { XMLName xml.Name `xml:"module"` Name string `xml:"name,attr"` @@ -114,15 +118,45 @@ func (c *Converter) ConvertRules(ctx context.Context, rules []schema.UserRule, l return nil, fmt.Errorf("no rules converted: %v", errors) } + // Separate modules into Checker-level and TreeWalker-level + // Checker-level modules (NOT under TreeWalker) + checkerLevelModules := map[string]bool{ + "LineLength": true, + "FileLength": true, + "FileTabCharacter": true, + "NewlineAtEndOfFile": true, + "UniqueProperties": true, + "OrderedProperties": true, + "Translation": true, + "SuppressWarningsFilter": true, + "BeforeExecutionExclusionFileFilter": true, + "SuppressionFilter": true, + "SuppressionCommentFilter": true, + } + + var checkerModules []checkstyleModule + var treeWalkerModules []checkstyleModule + + for _, module := range modules { + if checkerLevelModules[module.Name] { + checkerModules = append(checkerModules, module) + } else { + treeWalkerModules = append(treeWalkerModules, module) + } + } + // Build Checkstyle configuration + // TreeWalker contains TreeWalker-level modules treeWalker := checkstyleModule{ Name: "TreeWalker", - Modules: modules, + Modules: treeWalkerModules, } + // Checker contains Checker-level modules + TreeWalker + allModules := append(checkerModules, treeWalker) config := checkstyleConfig{ Name: "Checker", - Modules: []checkstyleModule{treeWalker}, + Modules: allModules, } // Marshal to XML @@ -158,13 +192,18 @@ Return ONLY a JSON object (no markdown fences): } Common Checkstyle modules: -- Naming: TypeName, MethodName, ParameterName, LocalVariableName, ConstantName +- Naming: TypeName, MethodName, MemberName, ParameterName, LocalVariableName, StaticVariableName, ConstantName - Length: LineLength, MethodLength, ParameterNumber, FileLength - Style: Indentation, WhitespaceAround, NeedBraces, LeftCurly, RightCurly - Imports: AvoidStarImport, IllegalImport, UnusedImports - Complexity: CyclomaticComplexity, NPathComplexity - JavaDoc: JavadocMethod, JavadocType, MissingJavadocMethod +IMPORTANT - Use MemberName for class fields (instance variables), NOT LocalVariableName: +- MemberName: private/protected/public instance variables (class fields) +- LocalVariableName: variables declared inside methods (local scope only) +- StaticVariableName: static non-final variables + If cannot convert, return: { "module_name": "", @@ -188,6 +227,30 @@ Output: "module_name": "LocalVariableName", "severity": "error", "properties": {"format": "^[a-z][a-zA-Z0-9]*$"} +} + +Input: "Private member variables must start with m_" +Output: +{ + "module_name": "MemberName", + "severity": "error", + "properties": {"format": "^m_[a-z][a-zA-Z0-9]*$"} +} + +Input: "Class names must be PascalCase" +Output: +{ + "module_name": "TypeName", + "severity": "error", + "properties": {"format": "^[A-Z][a-zA-Z0-9]*$"} +} + +Input: "Method names must be camelCase" +Output: +{ + "module_name": "MethodName", + "severity": "error", + "properties": {"format": "^[a-z][a-zA-Z0-9]*$"} }` userPrompt := fmt.Sprintf("Convert this Java rule to Checkstyle module:\n\n%s", rule.Say) @@ -204,6 +267,10 @@ Output: response = strings.TrimSuffix(response, "```") response = strings.TrimSpace(response) + if response == "" { + return nil, fmt.Errorf("LLM returned empty response") + } + var result struct { ModuleName string `json:"module_name"` Severity string `json:"severity"` @@ -211,7 +278,7 @@ Output: } if err := json.Unmarshal([]byte(response), &result); err != nil { - return nil, fmt.Errorf("failed to parse LLM response: %w", err) + return nil, fmt.Errorf("failed to parse LLM response: %w (response: %.100s)", err, response) } if result.ModuleName == "" { diff --git a/internal/adapter/eslint/converter.go b/internal/adapter/eslint/converter.go index fb3b1ee..07a4fa1 100644 --- a/internal/adapter/eslint/converter.go +++ b/internal/adapter/eslint/converter.go @@ -21,12 +21,10 @@ func NewConverter() *Converter { return &Converter{} } -// Name returns the linter name func (c *Converter) Name() string { return "eslint" } -// SupportedLanguages returns supported languages func (c *Converter) SupportedLanguages() []string { return []string{"javascript", "js", "typescript", "ts", "jsx", "tsx"} } @@ -38,6 +36,15 @@ func (c *Converter) GetLLMDescription() string { - CANNOT: Complex business logic, context-aware rules, file naming, advanced async patterns` } +// GetRoutingHints returns routing rules for LLM to decide when to use ESLint +func (c *Converter) GetRoutingHints() []string { + return []string{ + "For JavaScript/TypeScript naming rules (camelCase, PascalCase) โ†’ use eslint", + "For JS/TS code quality (unused vars, no-console, no-eval) โ†’ use eslint", + "For JS/TS best practices (eqeqeq, no-var, prefer-const) โ†’ use eslint", + } +} + // ConvertRules converts user rules to ESLint configuration using LLM func (c *Converter) ConvertRules(ctx context.Context, rules []schema.UserRule, llmClient *llm.Client) (*adapter.LinterConfig, error) { if llmClient == nil { @@ -205,6 +212,10 @@ Output: response = strings.TrimSuffix(response, "```") response = strings.TrimSpace(response) + if response == "" { + return "", nil, fmt.Errorf("LLM returned empty response") + } + var result struct { RuleName string `json:"rule_name"` Severity string `json:"severity"` @@ -212,7 +223,7 @@ Output: } if err := json.Unmarshal([]byte(response), &result); err != nil { - return "", nil, fmt.Errorf("failed to parse LLM response: %w", err) + return "", nil, fmt.Errorf("failed to parse LLM response: %w (response: %.100s)", err, response) } // If rule_name is empty, this rule cannot be converted @@ -226,13 +237,8 @@ Output: severity = result.Severity } - // Build rule configuration - var config interface{} - if result.Options != nil { - config = []interface{}{severity, result.Options} - } else { - config = severity - } + // Build rule configuration using format helper for special rules + config := formatESLintRuleConfig(result.RuleName, severity, result.Options) return result.RuleName, config, nil } @@ -250,3 +256,39 @@ func mapSeverity(severity string) string { return "error" } } + +// formatESLintRuleConfig formats the rule configuration based on rule-specific requirements. +// Some ESLint rules have non-standard option formats that need special handling. +func formatESLintRuleConfig(ruleName string, severity string, options interface{}) interface{} { + // Rules that need special formatting + switch ruleName { + case "id-match": + // id-match requires: [severity, pattern, options] + // where pattern is a string and options is an object + if opts, ok := options.(map[string]interface{}); ok { + if pattern, hasPattern := opts["pattern"].(string); hasPattern { + // Remove pattern from options since it's a separate argument + remainingOpts := make(map[string]interface{}) + for k, v := range opts { + if k != "pattern" { + remainingOpts[k] = v + } + } + if len(remainingOpts) > 0 { + return []interface{}{severity, pattern, remainingOpts} + } + return []interface{}{severity, pattern} + } + } + + case "no-restricted-imports": + // no-restricted-imports can have complex options + // Keep the default format for now + } + + // Default format: [severity, options] or just severity + if options != nil { + return []interface{}{severity, options} + } + return severity +} diff --git a/internal/adapter/pmd/converter.go b/internal/adapter/pmd/converter.go index 9122f67..9bc803f 100644 --- a/internal/adapter/pmd/converter.go +++ b/internal/adapter/pmd/converter.go @@ -21,21 +21,28 @@ func NewConverter() *Converter { return &Converter{} } -// Name returns the linter name func (c *Converter) Name() string { return "pmd" } -// SupportedLanguages returns supported languages func (c *Converter) SupportedLanguages() []string { return []string{"java"} } // GetLLMDescription returns a description of PMD's capabilities for LLM routing func (c *Converter) GetLLMDescription() string { - return `Java code quality (unused code, empty blocks, naming conventions, design issues) - - CAN: Unused private methods, empty catch blocks, short variable names, too many methods, hardcoded crypto keys - - CANNOT: Code formatting, whitespace, complex business logic validation` + return `Java code quality analysis (unused code, empty blocks, complexity, design issues) + - CAN: Unused private methods, empty catch blocks, too many methods, hardcoded crypto keys, cyclomatic complexity, error handling patterns + - CANNOT: Code formatting, whitespace, naming conventions (use Checkstyle instead), complex business logic validation` +} + +// GetRoutingHints returns routing rules for LLM to decide when to use PMD +func (c *Converter) GetRoutingHints() []string { + return []string{ + "For Java code quality (unused code, complexity, empty catch blocks) โ†’ use pmd", + "For Java error handling patterns (empty catch, exception handling) โ†’ use pmd", + "NEVER use pmd for naming conventions - use checkstyle instead", + } } // pmdRuleset represents PMD ruleset @@ -138,20 +145,22 @@ func (c *Converter) ConvertRules(ctx context.Context, rules []schema.UserRule, l // convertSingleRule converts a single rule using LLM func (c *Converter) convertSingleRule(ctx context.Context, rule schema.UserRule, llmClient *llm.Client) (*pmdRule, error) { - systemPrompt := `You are a PMD configuration expert. Convert natural language Java coding rules to PMD rule references. + systemPrompt := `You are a PMD 7.x configuration expert. Convert natural language Java coding rules to PMD rule references. Return ONLY a JSON object (no markdown fences): { - "rule_ref": "category/java/ruleset.xml/RuleName", + "rule_ref": "category/java/CATEGORY.xml/RuleName", "priority": 1-5 } -Common PMD rules: -- Best Practices: rulesets/java/bestpractices.xml/UnusedPrivateMethod -- Code Style: rulesets/java/codestyle.xml/ShortVariable -- Design: rulesets/java/design.xml/TooManyMethods -- Error Handling: rulesets/java/errorprone.xml/EmptyCatchBlock -- Security: rulesets/java/security.xml/HardCodedCryptoKey +IMPORTANT: PMD 7.x uses "category/java/" prefix, NOT "rulesets/java/". + +Common PMD 7.x rules: +- Best Practices: category/java/bestpractices.xml/UnusedPrivateMethod +- Code Style: category/java/codestyle.xml/ShortVariable +- Design: category/java/design.xml/TooManyMethods, category/java/design.xml/CyclomaticComplexity +- Error Prone: category/java/errorprone.xml/EmptyCatchBlock +- Security: category/java/security.xml/HardCodedCryptoKey Priority levels: 1=High, 2=Medium-High, 3=Medium, 4=Low, 5=Info @@ -166,7 +175,14 @@ Example: Input: "Avoid unused private methods" Output: { - "rule_ref": "rulesets/java/bestpractices.xml/UnusedPrivateMethod", + "rule_ref": "category/java/bestpractices.xml/UnusedPrivateMethod", + "priority": 2 +} + +Input: "No empty catch blocks" +Output: +{ + "rule_ref": "category/java/errorprone.xml/EmptyCatchBlock", "priority": 2 }` @@ -184,13 +200,17 @@ Output: response = strings.TrimSuffix(response, "```") response = strings.TrimSpace(response) + if response == "" { + return nil, fmt.Errorf("LLM returned empty response") + } + var result struct { RuleRef string `json:"rule_ref"` Priority int `json:"priority"` } if err := json.Unmarshal([]byte(response), &result); err != nil { - return nil, fmt.Errorf("failed to parse LLM response: %w", err) + return nil, fmt.Errorf("failed to parse LLM response: %w (response: %.100s)", err, response) } if result.RuleRef == "" { diff --git a/internal/adapter/prettier/converter.go b/internal/adapter/prettier/converter.go index b43e553..51c89c2 100644 --- a/internal/adapter/prettier/converter.go +++ b/internal/adapter/prettier/converter.go @@ -19,12 +19,10 @@ func NewConverter() *Converter { return &Converter{} } -// Name returns the linter name func (c *Converter) Name() string { return "prettier" } -// SupportedLanguages returns supported languages func (c *Converter) SupportedLanguages() []string { return []string{"javascript", "js", "typescript", "ts", "jsx", "tsx"} } @@ -36,6 +34,15 @@ func (c *Converter) GetLLMDescription() string { - CANNOT: Code logic, naming conventions, unused variables, type checking` } +// GetRoutingHints returns routing rules for LLM to decide when to use Prettier +func (c *Converter) GetRoutingHints() []string { + return []string{ + "For JavaScript/TypeScript formatting (quotes, semicolons, line length) โ†’ use prettier", + "For code style (indentation, trailing commas, bracket spacing) โ†’ use prettier", + "NEVER use prettier for naming conventions or code quality - use eslint instead", + } +} + // ConvertRules converts formatting rules to Prettier config using LLM func (c *Converter) ConvertRules(ctx context.Context, rules []schema.UserRule, llmClient *llm.Client) (*adapter.LinterConfig, error) { if llmClient == nil { @@ -139,9 +146,13 @@ Output: response = strings.TrimSuffix(response, "```") response = strings.TrimSpace(response) + if response == "" { + return nil, fmt.Errorf("LLM returned empty response") + } + var config map[string]interface{} if err := json.Unmarshal([]byte(response), &config); err != nil { - return nil, fmt.Errorf("failed to parse LLM response: %w", err) + return nil, fmt.Errorf("failed to parse LLM response: %w (response: %.100s)", err, response) } return config, nil diff --git a/internal/adapter/pylint/converter.go b/internal/adapter/pylint/converter.go index bac55ae..edea893 100644 --- a/internal/adapter/pylint/converter.go +++ b/internal/adapter/pylint/converter.go @@ -21,12 +21,10 @@ func NewConverter() *Converter { return &Converter{} } -// Name returns the linter name func (c *Converter) Name() string { return "pylint" } -// SupportedLanguages returns supported languages func (c *Converter) SupportedLanguages() []string { return []string{"python", "py"} } @@ -41,6 +39,16 @@ func (c *Converter) GetLLMDescription() string { async/await pattern validation, complex decorator analysis` } +// GetRoutingHints returns routing rules for LLM to decide when to use Pylint +func (c *Converter) GetRoutingHints() []string { + return []string{ + "For Python naming rules (snake_case, PascalCase) โ†’ use pylint", + "For Python code quality (docstrings, complexity, unused vars) โ†’ use pylint", + "For Python style (line length, import order) โ†’ use pylint", + "For Python error handling (bare except, broad except) โ†’ use pylint", + } +} + // ConvertRules converts user rules to Pylint configuration using LLM func (c *Converter) ConvertRules(ctx context.Context, rules []schema.UserRule, llmClient *llm.Client) (*adapter.LinterConfig, error) { if llmClient == nil { @@ -214,6 +222,10 @@ Output: response = strings.TrimSuffix(response, "```") response = strings.TrimSpace(response) + if response == "" { + return "", nil, fmt.Errorf("LLM returned empty response") + } + var result struct { Symbol string `json:"symbol"` MessageID string `json:"message_id"` @@ -221,7 +233,7 @@ Output: } if err := json.Unmarshal([]byte(response), &result); err != nil { - return "", nil, fmt.Errorf("failed to parse LLM response: %w", err) + return "", nil, fmt.Errorf("failed to parse LLM response: %w (response: %.100s)", err, response) } // If symbol is empty, this rule cannot be converted diff --git a/internal/adapter/tsc/converter.go b/internal/adapter/tsc/converter.go index bfbf738..a4a7f11 100644 --- a/internal/adapter/tsc/converter.go +++ b/internal/adapter/tsc/converter.go @@ -19,12 +19,10 @@ func NewConverter() *Converter { return &Converter{} } -// Name returns the linter name func (c *Converter) Name() string { return "tsc" } -// SupportedLanguages returns supported languages func (c *Converter) SupportedLanguages() []string { return []string{"typescript", "ts", "tsx"} } @@ -36,6 +34,15 @@ func (c *Converter) GetLLMDescription() string { - CANNOT: Code formatting, naming conventions, runtime behavior, business logic` } +// GetRoutingHints returns routing rules for LLM to decide when to use TSC +func (c *Converter) GetRoutingHints() []string { + return []string{ + "For TypeScript type checking (noImplicitAny, strictNullChecks) โ†’ use tsc", + "For TypeScript strict mode rules (strict, noUnusedLocals) โ†’ use tsc", + "NEVER use tsc for naming conventions or formatting - use eslint/prettier instead", + } +} + // ConvertRules converts type-checking rules to tsconfig.json using LLM func (c *Converter) ConvertRules(ctx context.Context, rules []schema.UserRule, llmClient *llm.Client) (*adapter.LinterConfig, error) { if llmClient == nil { @@ -153,9 +160,13 @@ Output: response = strings.TrimSuffix(response, "```") response = strings.TrimSpace(response) + if response == "" { + return nil, fmt.Errorf("LLM returned empty response") + } + var config map[string]interface{} if err := json.Unmarshal([]byte(response), &config); err != nil { - return nil, fmt.Errorf("failed to parse LLM response: %w", err) + return nil, fmt.Errorf("failed to parse LLM response: %w (response: %.100s)", err, response) } return config, nil diff --git a/internal/converter/converter.go b/internal/converter/converter.go index 23f03f2..9f55dde 100644 --- a/internal/converter/converter.go +++ b/internal/converter/converter.go @@ -316,6 +316,9 @@ func (c *Converter) selectLintersForRule(ctx context.Context, rule schema.UserRu // Build linter descriptions dynamically from registry linterDescriptions := c.buildLinterDescriptions(availableLinters) + // Build routing hints dynamically from converters + routingHints := c.buildRoutingHints(availableLinters) + systemPrompt := fmt.Sprintf(`You are a code quality expert. Analyze the given coding rule and determine which linters can ACTUALLY enforce it using their NATIVE rules (without plugins). Available linters and NATIVE capabilities: @@ -328,6 +331,7 @@ STRICT Rules for selection: 4. If the rule is about file naming โ†’ return [] 5. If the rule requires deep semantic analysis โ†’ return [] 6. When in doubt, return [] (better to use llm-validator than fail) +%s Available linters for this rule: %s @@ -373,7 +377,7 @@ Reason: Requires semantic analysis of what constitutes secrets Input: "Imports from large packages must be specific" Output: [] -Reason: Requires knowing which packages are "large"`, linterDescriptions, availableLinters) +Reason: Requires knowing which packages are "large"`, linterDescriptions, routingHints, availableLinters) userPrompt := fmt.Sprintf("Rule: %s\nCategory: %s", rule.Say, rule.Category) @@ -433,6 +437,31 @@ func (c *Converter) buildLinterDescriptions(availableLinters []string) string { return strings.Join(descriptions, "\n") } +// buildRoutingHints builds routing hints from all available converters +func (c *Converter) buildRoutingHints(availableLinters []string) string { + var hints []string + hintNumber := 7 // Start after the base rules (1-6) + + for _, linterName := range availableLinters { + converter, ok := registry.Global().GetConverter(linterName) + if !ok || converter == nil { + continue + } + + routingHints := converter.GetRoutingHints() + for _, hint := range routingHints { + hints = append(hints, fmt.Sprintf("%d. %s", hintNumber, hint)) + hintNumber++ + } + } + + if len(hints) == 0 { + return "" + } + + return strings.Join(hints, "\n") +} + // convertRBAC converts UserRBAC to PolicyRBAC func (c *Converter) convertRBAC(userRBAC *schema.UserRBAC) *schema.PolicyRBAC { if userRBAC == nil || len(userRBAC.Roles) == 0 { From bb9a6646dc16c0993498e27f4390a4576202506d Mon Sep 17 00:00:00 2001 From: ikjeong Date: Wed, 26 Nov 2025 08:13:01 +0000 Subject: [PATCH 07/13] fix: support Prettier 3.x output format --- internal/adapter/prettier/parser.go | 31 +++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/internal/adapter/prettier/parser.go b/internal/adapter/prettier/parser.go index ad12e90..238f163 100644 --- a/internal/adapter/prettier/parser.go +++ b/internal/adapter/prettier/parser.go @@ -8,12 +8,18 @@ import ( // parseOutput converts Prettier --check output to violations. // -// Prettier --check output format: +// Prettier --check output format (older versions): // "Checking formatting... // src/app.js // src/utils.js // [warn] Code style issues found in the above file(s). Forgot to run Prettier?" // +// Prettier --check output format (newer versions like 3.x): +// "Checking formatting... +// [warn] src/app.js +// [warn] src/utils.js +// [warn] Code style issues found in the above file(s). Run Prettier with --write to fix." +// // Non-zero exit code means files need formatting. func parseOutput(output *adapter.ToolOutput) ([]adapter.Violation, error) { // Exit code 0 = all files formatted @@ -21,22 +27,35 @@ func parseOutput(output *adapter.ToolOutput) ([]adapter.Violation, error) { return nil, nil } - // Parse stdout to find files that need formatting + // Parse both stdout and stderr to find files that need formatting + // Prettier 3.x outputs [warn] messages to stderr var violations []adapter.Violation - lines := strings.Split(output.Stdout, "\n") + combined := output.Stdout + "\n" + output.Stderr + lines := strings.Split(combined, "\n") for _, line := range lines { line = strings.TrimSpace(line) // Skip empty lines and status messages - if line == "" || strings.HasPrefix(line, "Checking") || - strings.HasPrefix(line, "[warn]") || strings.HasPrefix(line, "Code style") { + if line == "" || strings.HasPrefix(line, "Checking") { continue } + // Handle [warn] prefix (Prettier 3.x format) + if strings.HasPrefix(line, "[warn]") { + line = strings.TrimSpace(strings.TrimPrefix(line, "[warn]")) + // Skip the summary message + if strings.HasPrefix(line, "Code style") { + continue + } + } + // If line looks like a file path, it needs formatting if strings.Contains(line, ".js") || strings.Contains(line, ".ts") || - strings.Contains(line, ".jsx") || strings.Contains(line, ".tsx") { + strings.Contains(line, ".jsx") || strings.Contains(line, ".tsx") || + strings.Contains(line, ".json") || strings.Contains(line, ".css") || + strings.Contains(line, ".md") || strings.Contains(line, ".yaml") || + strings.Contains(line, ".yml") { violations = append(violations, adapter.Violation{ File: line, Line: 0, // Prettier doesn't report line numbers in --check From 80252ee0641b9f836ecee28fcfce708df8e8badb Mon Sep 17 00:00:00 2001 From: ikjeong Date: Wed, 26 Nov 2025 08:13:08 +0000 Subject: [PATCH 08/13] fix: use absolute paths in TSC executor --- internal/adapter/tsc/executor.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/internal/adapter/tsc/executor.go b/internal/adapter/tsc/executor.go index ce274ca..63dfabc 100644 --- a/internal/adapter/tsc/executor.go +++ b/internal/adapter/tsc/executor.go @@ -19,8 +19,18 @@ func (a *Adapter) execute(ctx context.Context, config []byte, files []string) (* } // Add files to tsconfig if specific files are provided + // Convert to absolute paths since tsconfig will be in temp directory if len(files) > 0 { - tsconfig["files"] = files + absFiles := make([]string, len(files)) + cwd, _ := os.Getwd() + for i, f := range files { + if filepath.IsAbs(f) { + absFiles[i] = f + } else { + absFiles[i] = filepath.Join(cwd, f) + } + } + tsconfig["files"] = absFiles } // Marshal updated config From 9806d86a587bce54b8ab646931a9e702d55d1868 Mon Sep 17 00:00:00 2001 From: ikjeong Date: Wed, 26 Nov 2025 08:13:34 +0000 Subject: [PATCH 09/13] feat: add validation error tracking --- internal/mcp/server.go | 17 +++++++++-------- internal/validator/llm_validator.go | 8 ++++++++ internal/validator/validator.go | 11 ++++++++--- 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/internal/mcp/server.go b/internal/mcp/server.go index 35bb217..c9995bb 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -260,10 +260,7 @@ type ConventionItem struct { // handleQueryConventions handles convention query requests. // It finds and returns relevant conventions by category. func (s *Server) handleQueryConventions(params map[string]interface{}) (interface{}, *RPCError) { - fmt.Fprintf(os.Stderr, "[DEBUG] handleQueryConventions called with params: %+v\n", params) - if s.userPolicy == nil && s.codePolicy == nil { - fmt.Fprintf(os.Stderr, "[DEBUG] No policy loaded\n") return map[string]interface{}{ "conventions": []ConventionItem{}, "message": "policy not loaded", @@ -273,7 +270,6 @@ func (s *Server) handleQueryConventions(params map[string]interface{}) (interfac var req QueryConventionsRequest paramBytes, _ := json.Marshal(params) if err := json.Unmarshal(paramBytes, &req); err != nil { - fmt.Fprintf(os.Stderr, "[ERROR] Failed to parse parameters: %v\n", err) return nil, &RPCError{ Code: -32602, Message: fmt.Sprintf("failed to parse parameters: %v", err), @@ -289,11 +285,7 @@ func (s *Server) handleQueryConventions(params map[string]interface{}) (interfac // If languages is empty, return all languages // This is more user-friendly than requiring the parameter - fmt.Fprintf(os.Stderr, "[DEBUG] Parsed request: category=%s, languages=%v\n", - req.Category, req.Languages) - conventions := s.filterConventions(req) - fmt.Fprintf(os.Stderr, "[DEBUG] Found %d conventions\n", len(conventions)) // Format conventions as readable text for MCP response var textContent string @@ -563,6 +555,15 @@ func (s *Server) handleValidateCode(params map[string]interface{}) (interface{}, } } + // Add engine errors if any (adapter execution failures) + if len(result.Errors) > 0 { + textContent += fmt.Sprintf("\nโš ๏ธ Engine errors (%d):\n", len(result.Errors)) + for _, e := range result.Errors { + textContent += fmt.Sprintf(" [%s] %s: %s\n", e.Engine, e.RuleID, e.Message) + } + textContent += "\n" + } + // Add note about saved results textContent += "\n๐Ÿ’พ Validation results saved to .sym/validation-results.json\n" diff --git a/internal/validator/llm_validator.go b/internal/validator/llm_validator.go index 408cae0..df94fd0 100644 --- a/internal/validator/llm_validator.go +++ b/internal/validator/llm_validator.go @@ -10,9 +10,17 @@ import ( "github.com/DevSymphony/sym-cli/pkg/schema" ) +// ValidationError represents an error that occurred during validation +type ValidationError struct { + RuleID string + Engine string + Message string +} + // ValidationResult represents the result of validating changes type ValidationResult struct { Violations []Violation + Errors []ValidationError // Adapter/engine execution errors Checked int Passed int Failed int diff --git a/internal/validator/validator.go b/internal/validator/validator.go index 1225ee9..b9609a1 100644 --- a/internal/validator/validator.go +++ b/internal/validator/validator.go @@ -412,9 +412,14 @@ func (v *Validator) ValidateChanges(ctx context.Context, changes []GitChange) (* violations, err := v.executeRule(engineName, rule, []string{change.FilePath}) if err != nil { - if v.verbose { - fmt.Printf("โš ๏ธ Validation failed for rule %s: %v\n", rule.ID, err) - } + // Always log errors to stderr (not just in verbose mode) + fmt.Fprintf(os.Stderr, "โš ๏ธ Validation failed for rule %s (%s): %v\n", rule.ID, engineName, err) + // Track error in result for MCP response + result.Errors = append(result.Errors, ValidationError{ + RuleID: rule.ID, + Engine: engineName, + Message: err.Error(), + }) continue } From dc8342c90ddb1b9783077238d9027977ee0d2bd4 Mon Sep 17 00:00:00 2001 From: ikjeong Date: Wed, 26 Nov 2025 08:13:56 +0000 Subject: [PATCH 10/13] refactor: clean up comments and formatting --- internal/cmd/validate.go | 2 -- internal/server/server.go | 20 ++++++++------------ 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/internal/cmd/validate.go b/internal/cmd/validate.go index ac57018..213db2b 100644 --- a/internal/cmd/validate.go +++ b/internal/cmd/validate.go @@ -88,7 +88,6 @@ func runValidate(cmd *cobra.Command, args []string) error { llm.WithTimeout(time.Duration(validateTimeout)*time.Second), ) - // Get git changes var changes []validator.GitChange if validateStaged { changes, err = validator.GetStagedChanges() @@ -127,7 +126,6 @@ func runValidate(cmd *cobra.Command, args []string) error { return fmt.Errorf("validation failed: %w", err) } - // Print results printValidationResult(result) // Exit with error if violations found diff --git a/internal/server/server.go b/internal/server/server.go index d87a08f..c0bf579 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -12,6 +12,7 @@ import ( "path/filepath" "strings" "time" + "github.com/DevSymphony/sym-cli/internal/config" "github.com/DevSymphony/sym-cli/internal/converter" "github.com/DevSymphony/sym-cli/internal/envutil" @@ -29,9 +30,9 @@ import ( var staticFiles embed.FS type Server struct { - port int - cfg *config.Config - token *config.Token + port int + cfg *config.Config + token *config.Token githubClient *github.Client } @@ -50,9 +51,9 @@ func NewServer(port int) (*Server, error) { githubClient := github.NewClient(cfg.GetGitHubHost(), token.AccessToken) return &Server{ - port: port, - cfg: cfg, - token: token, + port: port, + cfg: cfg, + token: token, githubClient: githubClient, }, nil } @@ -114,7 +115,6 @@ func (s *Server) corsMiddleware(next http.Handler) http.Handler { }) } -// hasPermission checks if a user has a specific permission based on RBAC func (s *Server) hasPermission(username, permission string) (bool, error) { // Load policy to check RBAC permissions policyData, err := policy.LoadPolicy(s.cfg.PolicyPath) @@ -125,7 +125,6 @@ func (s *Server) hasPermission(username, permission string) (bool, error) { return s.hasPermissionWithPolicy(username, permission, policyData) } -// hasPermissionWithPolicy checks if a user has a specific permission based on a given policy func (s *Server) hasPermissionWithPolicy(username, permission string, policyData *schema.UserPolicy) (bool, error) { // Load user's role from roles.json userRole, err := roles.GetUserRole(username) @@ -136,7 +135,6 @@ func (s *Server) hasPermissionWithPolicy(username, permission string, policyData return s.checkPermissionForRole(userRole, permission, policyData) } -// hasPermissionWithRoles checks if a user has a specific permission based on given roles and policy func (s *Server) hasPermissionWithRoles(username, permission string, rolesData roles.Roles) (bool, error) { // Find user's role from the given roles userRole := "none" @@ -190,7 +188,6 @@ func (s *Server) checkPermissionForRole(userRole, permission string, policyData } } -// handleGetMe returns the current authenticated user func (s *Server) handleGetMe(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodGet { http.Error(w, "Method not allowed", http.StatusMethodNotAllowed) @@ -304,12 +301,11 @@ func (s *Server) handleUpdateRoles(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") _ = json.NewEncoder(w).Encode(map[string]string{ - "status": "success", + "status": "success", "message": "Roles updated successfully", }) } -// handleRepoInfo returns the current repository information func (s *Server) handleRepoInfo(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodGet { http.Error(w, "Method not allowed", http.StatusMethodNotAllowed) From b03420c2ad8a5339a46185a472c0f38ad890cd4c Mon Sep 17 00:00:00 2001 From: ikjeong Date: Thu, 27 Nov 2025 08:06:57 +0000 Subject: [PATCH 11/13] feat: add NewValidatorWithWorkDir constructor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - workDir์„ ์ง์ ‘ ์ง€์ •ํ•  ์ˆ˜ ์žˆ๋Š” ์ƒ์„ฑ์ž ์ถ”๊ฐ€ - symDir์€ workDir/.sym์œผ๋กœ ์ž๋™ ์„ค์ • --- internal/validator/validator.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/internal/validator/validator.go b/internal/validator/validator.go index b9609a1..393401c 100644 --- a/internal/validator/validator.go +++ b/internal/validator/validator.go @@ -70,6 +70,25 @@ func NewValidator(policy *schema.CodePolicy, verbose bool) *Validator { } } +// NewValidatorWithWorkDir creates a validator with a custom working directory +// symDir is automatically set to workDir/.sym +func NewValidatorWithWorkDir(policy *schema.CodePolicy, verbose bool, workDir string) *Validator { + symDir := filepath.Join(workDir, ".sym") + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) + + return &Validator{ + policy: policy, + verbose: verbose, + adapterRegistry: adapterRegistry.Global(), + workDir: workDir, + symDir: symDir, + selector: NewFileSelector(workDir), + ctx: ctx, + ctxCancel: cancel, + llmClient: nil, + } +} + // SetLLMClient sets the LLM client for this validator func (v *Validator) SetLLMClient(client *llm.Client) { v.llmClient = client From 9547ebac34b9a53824f453566d54b7ade1768e53 Mon Sep 17 00:00:00 2001 From: ikjeong Date: Thu, 27 Nov 2025 08:07:02 +0000 Subject: [PATCH 12/13] test: add integration tests using ValidateChanges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 6๊ฐœ ์–ด๋Œ‘ํ„ฐ(pylint, checkstyle, pmd, eslint, prettier, tsc) ํ…Œ์ŠคํŠธ ์ถ”๊ฐ€ - testdata ๋””๋ ‰ํ† ๋ฆฌ์— ๊ฐ ๋„๊ตฌ๋ณ„ ํ…Œ์ŠคํŠธ ํŒŒ์ผ ๋ฐ ์„ค์ • ์ถ”๊ฐ€ - init_test.go๋กœ ์–ด๋Œ‘ํ„ฐ ๋“ฑ๋ก ์ฒ˜๋ฆฌ --- tests/integration/checkstyle_test.go | 230 ++++++++++++++++ tests/integration/eslint_test.go | 244 +++++++++++++++++ tests/integration/init_test.go | 11 + tests/integration/pmd_test.go | 243 +++++++++++++++++ tests/integration/prettier_test.go | 252 ++++++++++++++++++ tests/integration/pylint_test.go | 231 ++++++++++++++++ tests/integration/tsc_test.go | 245 +++++++++++++++++ tests/testdata/checkstyle/.sym/checkstyle.xml | 24 ++ .../testdata/checkstyle/.sym/code-policy.json | 33 +++ tests/testdata/checkstyle/Test.java | 16 ++ tests/testdata/eslint/.sym/.eslintrc.json | 34 +++ tests/testdata/eslint/.sym/code-policy.json | 24 ++ tests/testdata/eslint/Test.ts | 56 ++++ tests/testdata/pmd/.sym/code-policy.json | 33 +++ tests/testdata/pmd/.sym/pmd.xml | 13 + tests/testdata/pmd/Test.java | 40 +++ tests/testdata/prettier/.sym/.prettierrc | 9 + tests/testdata/prettier/.sym/code-policy.json | 33 +++ tests/testdata/prettier/Test.ts | 47 ++++ tests/testdata/pylint/.sym/.pylintrc | 11 + tests/testdata/pylint/.sym/code-policy.json | 33 +++ tests/testdata/pylint/Test.py | 29 ++ tests/testdata/tsc/.sym/code-policy.json | 15 ++ tests/testdata/tsc/.sym/tsconfig.json | 20 ++ tests/testdata/tsc/Test.ts | 40 +++ 25 files changed, 1966 insertions(+) create mode 100644 tests/integration/checkstyle_test.go create mode 100644 tests/integration/eslint_test.go create mode 100644 tests/integration/init_test.go create mode 100644 tests/integration/pmd_test.go create mode 100644 tests/integration/prettier_test.go create mode 100644 tests/integration/pylint_test.go create mode 100644 tests/integration/tsc_test.go create mode 100644 tests/testdata/checkstyle/.sym/checkstyle.xml create mode 100644 tests/testdata/checkstyle/.sym/code-policy.json create mode 100644 tests/testdata/checkstyle/Test.java create mode 100644 tests/testdata/eslint/.sym/.eslintrc.json create mode 100644 tests/testdata/eslint/.sym/code-policy.json create mode 100644 tests/testdata/eslint/Test.ts create mode 100644 tests/testdata/pmd/.sym/code-policy.json create mode 100644 tests/testdata/pmd/.sym/pmd.xml create mode 100644 tests/testdata/pmd/Test.java create mode 100644 tests/testdata/prettier/.sym/.prettierrc create mode 100644 tests/testdata/prettier/.sym/code-policy.json create mode 100644 tests/testdata/prettier/Test.ts create mode 100644 tests/testdata/pylint/.sym/.pylintrc create mode 100644 tests/testdata/pylint/.sym/code-policy.json create mode 100644 tests/testdata/pylint/Test.py create mode 100644 tests/testdata/tsc/.sym/code-policy.json create mode 100644 tests/testdata/tsc/.sym/tsconfig.json create mode 100644 tests/testdata/tsc/Test.ts diff --git a/tests/integration/checkstyle_test.go b/tests/integration/checkstyle_test.go new file mode 100644 index 0000000..dff21cd --- /dev/null +++ b/tests/integration/checkstyle_test.go @@ -0,0 +1,230 @@ +package integration + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + adapterRegistry "github.com/DevSymphony/sym-cli/internal/adapter/registry" + "github.com/DevSymphony/sym-cli/internal/validator" + "github.com/DevSymphony/sym-cli/pkg/schema" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCheckstyle_ValidateChanges(t *testing.T) { + // 1. Get testdata path + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "checkstyle")) + require.NoError(t, err, "Failed to get testdata path") + + // Check testdata exists + if _, err := os.Stat(testdataDir); os.IsNotExist(err) { + t.Skipf("Testdata directory not found: %s", testdataDir) + } + + // 2. Load policy + policyPath := filepath.Join(testdataDir, ".sym", "code-policy.json") + policyData, err := os.ReadFile(policyPath) + require.NoError(t, err, "Failed to read code-policy.json") + + var policy schema.CodePolicy + require.NoError(t, json.Unmarshal(policyData, &policy), "Failed to parse policy") + + // 3. Create validator with custom workDir + v := validator.NewValidatorWithWorkDir(&policy, true, testdataDir) + defer v.Close() + + // 4. Check tool availability + adp, err := adapterRegistry.Global().GetAdapter("checkstyle") + if err != nil { + t.Skipf("Checkstyle adapter not found: %v", err) + } + if err := adp.CheckAvailability(context.Background()); err != nil { + t.Skipf("Checkstyle not available: %v", err) + } + + // 5. Create GitChange + testFile := filepath.Join(testdataDir, "Test.java") + require.FileExists(t, testFile, "Test.java should exist") + + changes := []validator.GitChange{{ + FilePath: testFile, + Status: "M", + Diff: "", + }} + + // 6. Execute ValidateChanges + ctx := context.Background() + result, err := v.ValidateChanges(ctx, changes) + require.NoError(t, err, "ValidateChanges failed") + + // 7. Log violations for debugging + t.Logf("Found %d violations", len(result.Violations)) + for _, viol := range result.Violations { + t.Logf(" - %s:%d: [%s] %s (tool: %s)", + viol.File, viol.Line, viol.RuleID, viol.Message, viol.ToolName) + } + + // 8. Assertions + assert.GreaterOrEqual(t, len(result.Violations), 3, "Should detect at least 3 naming violations") + + // Check that ToolName is checkstyle + for _, viol := range result.Violations { + assert.Equal(t, "checkstyle", viol.ToolName, "ToolName should be checkstyle") + } + + // Check that RuleID is from policy + validRuleIDs := map[string]bool{ + "checkstyle-typename": true, + "checkstyle-methodname": true, + "checkstyle-membername": true, + } + for _, viol := range result.Violations { + assert.True(t, validRuleIDs[viol.RuleID], + "RuleID should be from policy, got: %s", viol.RuleID) + } +} + +func TestCheckstyle_NamingRules(t *testing.T) { + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "checkstyle")) + require.NoError(t, err) + + if _, err := os.Stat(testdataDir); os.IsNotExist(err) { + t.Skipf("Testdata directory not found: %s", testdataDir) + } + + policyPath := filepath.Join(testdataDir, ".sym", "code-policy.json") + policyData, err := os.ReadFile(policyPath) + require.NoError(t, err) + + var policy schema.CodePolicy + require.NoError(t, json.Unmarshal(policyData, &policy)) + + v := validator.NewValidatorWithWorkDir(&policy, true, testdataDir) + defer v.Close() + + adp, err := adapterRegistry.Global().GetAdapter("checkstyle") + if err != nil { + t.Skipf("Checkstyle adapter not found: %v", err) + } + if err := adp.CheckAvailability(context.Background()); err != nil { + t.Skipf("Checkstyle not available: %v", err) + } + + testFile := filepath.Join(testdataDir, "Test.java") + changes := []validator.GitChange{{ + FilePath: testFile, + Status: "M", + Diff: "", + }} + + ctx := context.Background() + result, err := v.ValidateChanges(ctx, changes) + require.NoError(t, err) + + // Check specific violations + tests := []struct { + name string + checkLine int + msgPattern string + description string + }{ + { + name: "TypeName violation - bad_class", + checkLine: 2, + msgPattern: "TypeName", + description: "Class name 'bad_class' should trigger TypeName violation", + }, + { + name: "MemberName violation - MyVar", + checkLine: 4, + msgPattern: "MemberName", + description: "Member name 'MyVar' should trigger MemberName violation", + }, + { + name: "MethodName violation - BadFunc", + checkLine: 7, + msgPattern: "MethodName", + description: "Method name 'BadFunc' should trigger MethodName violation", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + found := false + for _, viol := range result.Violations { + if viol.Line == tt.checkLine { + if strings.Contains(viol.Message, tt.msgPattern) { + found = true + t.Logf("Found violation: %s", viol.Message) + break + } + } + } + if !found { + t.Logf("Expected %s violation at line %d not found (may be detected at different line)", tt.msgPattern, tt.checkLine) + } + }) + } +} + +func TestCheckstyle_ToolNameAndRuleID(t *testing.T) { + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "checkstyle")) + require.NoError(t, err) + + if _, err := os.Stat(testdataDir); os.IsNotExist(err) { + t.Skipf("Testdata directory not found: %s", testdataDir) + } + + policyPath := filepath.Join(testdataDir, ".sym", "code-policy.json") + policyData, err := os.ReadFile(policyPath) + require.NoError(t, err) + + var policy schema.CodePolicy + require.NoError(t, json.Unmarshal(policyData, &policy)) + + v := validator.NewValidatorWithWorkDir(&policy, true, testdataDir) + defer v.Close() + + adp, err := adapterRegistry.Global().GetAdapter("checkstyle") + if err != nil { + t.Skipf("Checkstyle adapter not found: %v", err) + } + if err := adp.CheckAvailability(context.Background()); err != nil { + t.Skipf("Checkstyle not available: %v", err) + } + + testFile := filepath.Join(testdataDir, "Test.java") + changes := []validator.GitChange{{ + FilePath: testFile, + Status: "M", + Diff: "", + }} + + ctx := context.Background() + result, err := v.ValidateChanges(ctx, changes) + require.NoError(t, err) + + require.Greater(t, len(result.Violations), 0, "Should have at least one violation") + + // Verify ToolName is set correctly + for _, viol := range result.Violations { + assert.Equal(t, "checkstyle", viol.ToolName, + "ToolName should be 'checkstyle', got '%s'", viol.ToolName) + } + + // Verify RuleID comes from policy + for _, viol := range result.Violations { + assert.True(t, strings.HasPrefix(viol.RuleID, "checkstyle-"), + "RuleID should start with 'checkstyle-', got '%s'", viol.RuleID) + } + + // Verify Severity comes from policy + for _, viol := range result.Violations { + assert.Equal(t, "error", viol.Severity, + "Severity should be 'error' from policy, got '%s'", viol.Severity) + } +} diff --git a/tests/integration/eslint_test.go b/tests/integration/eslint_test.go new file mode 100644 index 0000000..f1447ed --- /dev/null +++ b/tests/integration/eslint_test.go @@ -0,0 +1,244 @@ +package integration + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + adapterRegistry "github.com/DevSymphony/sym-cli/internal/adapter/registry" + "github.com/DevSymphony/sym-cli/internal/validator" + "github.com/DevSymphony/sym-cli/pkg/schema" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestESLint_ValidateChanges(t *testing.T) { + // 1. Get testdata path + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "eslint")) + require.NoError(t, err, "Failed to get testdata path") + + // Check testdata exists + if _, err := os.Stat(testdataDir); os.IsNotExist(err) { + t.Skipf("Testdata directory not found: %s", testdataDir) + } + + // 2. Load policy + policyPath := filepath.Join(testdataDir, ".sym", "code-policy.json") + policyData, err := os.ReadFile(policyPath) + require.NoError(t, err, "Failed to read code-policy.json") + + var policy schema.CodePolicy + require.NoError(t, json.Unmarshal(policyData, &policy), "Failed to parse policy") + + // 3. Create validator with custom workDir + v := validator.NewValidatorWithWorkDir(&policy, true, testdataDir) + defer v.Close() + + // 4. Check tool availability + adp, err := adapterRegistry.Global().GetAdapter("eslint") + if err != nil { + t.Skipf("ESLint adapter not found: %v", err) + } + if err := adp.CheckAvailability(context.Background()); err != nil { + t.Skipf("ESLint not available: %v", err) + } + + // 5. Create GitChange + testFile := filepath.Join(testdataDir, "Test.ts") + require.FileExists(t, testFile, "Test.ts should exist") + + changes := []validator.GitChange{{ + FilePath: testFile, + Status: "M", + Diff: "", + }} + + // 6. Execute ValidateChanges + ctx := context.Background() + result, err := v.ValidateChanges(ctx, changes) + require.NoError(t, err, "ValidateChanges failed") + + // 7. Log violations for debugging + t.Logf("Found %d violations", len(result.Violations)) + for _, viol := range result.Violations { + t.Logf(" - %s:%d:%d: [%s] %s (tool: %s)", + viol.File, viol.Line, viol.Column, viol.RuleID, viol.Message, viol.ToolName) + } + + // 8. Assertions + assert.GreaterOrEqual(t, len(result.Violations), 3, "Should detect at least 3 violations") + + // Check that ToolName is eslint + for _, viol := range result.Violations { + assert.Equal(t, "eslint", viol.ToolName, "ToolName should be eslint") + } + + // Check that RuleID is from policy + validRuleIDs := map[string]bool{ + "eslint-naming": true, + "eslint-maxlen": true, + } + for _, viol := range result.Violations { + assert.True(t, validRuleIDs[viol.RuleID], + "RuleID should be from policy, got: %s", viol.RuleID) + } +} + +func TestESLint_NamingConventions(t *testing.T) { + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "eslint")) + require.NoError(t, err) + + if _, err := os.Stat(testdataDir); os.IsNotExist(err) { + t.Skipf("Testdata directory not found: %s", testdataDir) + } + + policyPath := filepath.Join(testdataDir, ".sym", "code-policy.json") + policyData, err := os.ReadFile(policyPath) + require.NoError(t, err) + + var policy schema.CodePolicy + require.NoError(t, json.Unmarshal(policyData, &policy)) + + v := validator.NewValidatorWithWorkDir(&policy, true, testdataDir) + defer v.Close() + + adp, err := adapterRegistry.Global().GetAdapter("eslint") + if err != nil { + t.Skipf("ESLint adapter not found: %v", err) + } + if err := adp.CheckAvailability(context.Background()); err != nil { + t.Skipf("ESLint not available: %v", err) + } + + testFile := filepath.Join(testdataDir, "Test.ts") + changes := []validator.GitChange{{ + FilePath: testFile, + Status: "M", + Diff: "", + }} + + ctx := context.Background() + result, err := v.ValidateChanges(ctx, changes) + require.NoError(t, err) + + // Check for id-match or max-len violations in messages + hasIdMatch := false + hasMaxLen := false + + for _, viol := range result.Violations { + if strings.Contains(viol.Message, "id-match") || strings.Contains(viol.Message, "Identifier") { + hasIdMatch = true + } + if strings.Contains(viol.Message, "max-len") || strings.Contains(viol.Message, "exceeds") { + hasMaxLen = true + } + } + + assert.True(t, hasIdMatch || hasMaxLen, "Should detect id-match or max-len violations") +} + +func TestESLint_MaxLineLength(t *testing.T) { + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "eslint")) + require.NoError(t, err) + + if _, err := os.Stat(testdataDir); os.IsNotExist(err) { + t.Skipf("Testdata directory not found: %s", testdataDir) + } + + policyPath := filepath.Join(testdataDir, ".sym", "code-policy.json") + policyData, err := os.ReadFile(policyPath) + require.NoError(t, err) + + var policy schema.CodePolicy + require.NoError(t, json.Unmarshal(policyData, &policy)) + + v := validator.NewValidatorWithWorkDir(&policy, true, testdataDir) + defer v.Close() + + adp, err := adapterRegistry.Global().GetAdapter("eslint") + if err != nil { + t.Skipf("ESLint adapter not found: %v", err) + } + if err := adp.CheckAvailability(context.Background()); err != nil { + t.Skipf("ESLint not available: %v", err) + } + + testFile := filepath.Join(testdataDir, "Test.ts") + changes := []validator.GitChange{{ + FilePath: testFile, + Status: "M", + Diff: "", + }} + + ctx := context.Background() + result, err := v.ValidateChanges(ctx, changes) + require.NoError(t, err) + + // Check for max-len violations + found := false + for _, viol := range result.Violations { + if strings.Contains(viol.Message, "max-len") || strings.Contains(viol.Message, "exceeds") { + found = true + t.Logf("Found max-len violation at line %d: %s", viol.Line, viol.Message) + } + } + + if !found { + t.Log("max-len violation not detected (may be configured to ignore certain patterns)") + } +} + +func TestESLint_ToolNameAndRuleID(t *testing.T) { + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "eslint")) + require.NoError(t, err) + + if _, err := os.Stat(testdataDir); os.IsNotExist(err) { + t.Skipf("Testdata directory not found: %s", testdataDir) + } + + policyPath := filepath.Join(testdataDir, ".sym", "code-policy.json") + policyData, err := os.ReadFile(policyPath) + require.NoError(t, err) + + var policy schema.CodePolicy + require.NoError(t, json.Unmarshal(policyData, &policy)) + + v := validator.NewValidatorWithWorkDir(&policy, true, testdataDir) + defer v.Close() + + adp, err := adapterRegistry.Global().GetAdapter("eslint") + if err != nil { + t.Skipf("ESLint adapter not found: %v", err) + } + if err := adp.CheckAvailability(context.Background()); err != nil { + t.Skipf("ESLint not available: %v", err) + } + + testFile := filepath.Join(testdataDir, "Test.ts") + changes := []validator.GitChange{{ + FilePath: testFile, + Status: "M", + Diff: "", + }} + + ctx := context.Background() + result, err := v.ValidateChanges(ctx, changes) + require.NoError(t, err) + + require.Greater(t, len(result.Violations), 0, "Should have at least one violation") + + // Verify ToolName is set correctly + for _, viol := range result.Violations { + assert.Equal(t, "eslint", viol.ToolName, + "ToolName should be 'eslint', got '%s'", viol.ToolName) + } + + // Verify RuleID comes from policy + for _, viol := range result.Violations { + assert.True(t, strings.HasPrefix(viol.RuleID, "eslint-"), + "RuleID should start with 'eslint-', got '%s'", viol.RuleID) + } +} diff --git a/tests/integration/init_test.go b/tests/integration/init_test.go new file mode 100644 index 0000000..b13429e --- /dev/null +++ b/tests/integration/init_test.go @@ -0,0 +1,11 @@ +package integration + +// Import adapters to trigger init() registration +import ( + _ "github.com/DevSymphony/sym-cli/internal/adapter/checkstyle" + _ "github.com/DevSymphony/sym-cli/internal/adapter/eslint" + _ "github.com/DevSymphony/sym-cli/internal/adapter/pmd" + _ "github.com/DevSymphony/sym-cli/internal/adapter/prettier" + _ "github.com/DevSymphony/sym-cli/internal/adapter/pylint" + _ "github.com/DevSymphony/sym-cli/internal/adapter/tsc" +) diff --git a/tests/integration/pmd_test.go b/tests/integration/pmd_test.go new file mode 100644 index 0000000..f7036c8 --- /dev/null +++ b/tests/integration/pmd_test.go @@ -0,0 +1,243 @@ +package integration + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + adapterRegistry "github.com/DevSymphony/sym-cli/internal/adapter/registry" + "github.com/DevSymphony/sym-cli/internal/validator" + "github.com/DevSymphony/sym-cli/pkg/schema" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestPMD_ValidateChanges(t *testing.T) { + // 1. Get testdata path + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "pmd")) + require.NoError(t, err, "Failed to get testdata path") + + // Check testdata exists + if _, err := os.Stat(testdataDir); os.IsNotExist(err) { + t.Skipf("Testdata directory not found: %s", testdataDir) + } + + // 2. Load policy + policyPath := filepath.Join(testdataDir, ".sym", "code-policy.json") + policyData, err := os.ReadFile(policyPath) + require.NoError(t, err, "Failed to read code-policy.json") + + var policy schema.CodePolicy + require.NoError(t, json.Unmarshal(policyData, &policy), "Failed to parse policy") + + // 3. Create validator with custom workDir + v := validator.NewValidatorWithWorkDir(&policy, true, testdataDir) + defer v.Close() + + // 4. Check tool availability + adp, err := adapterRegistry.Global().GetAdapter("pmd") + if err != nil { + t.Skipf("PMD adapter not found: %v", err) + } + if err := adp.CheckAvailability(context.Background()); err != nil { + t.Skipf("PMD not available: %v", err) + } + + // 5. Create GitChange + testFile := filepath.Join(testdataDir, "Test.java") + require.FileExists(t, testFile, "Test.java should exist") + + changes := []validator.GitChange{{ + FilePath: testFile, + Status: "M", + Diff: "", + }} + + // 6. Execute ValidateChanges + ctx := context.Background() + result, err := v.ValidateChanges(ctx, changes) + require.NoError(t, err, "ValidateChanges failed") + + // 7. Log violations for debugging + t.Logf("Found %d violations", len(result.Violations)) + for _, viol := range result.Violations { + t.Logf(" - %s:%d: [%s] %s (tool: %s)", + viol.File, viol.Line, viol.RuleID, viol.Message, viol.ToolName) + } + + // 8. Assertions - PMD should detect at least 2 violations + assert.GreaterOrEqual(t, len(result.Violations), 2, "Should detect at least 2 violations") + + // Check that ToolName is pmd + for _, viol := range result.Violations { + assert.Equal(t, "pmd", viol.ToolName, "ToolName should be pmd") + } + + // Check that RuleID is from policy + validRuleIDs := map[string]bool{ + "pmd-emptycatch": true, + "pmd-unusedmethod": true, + "pmd-complexity": true, + } + for _, viol := range result.Violations { + assert.True(t, validRuleIDs[viol.RuleID], + "RuleID should be from policy, got: %s", viol.RuleID) + } +} + +func TestPMD_EmptyCatchBlock(t *testing.T) { + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "pmd")) + require.NoError(t, err) + + if _, err := os.Stat(testdataDir); os.IsNotExist(err) { + t.Skipf("Testdata directory not found: %s", testdataDir) + } + + policyPath := filepath.Join(testdataDir, ".sym", "code-policy.json") + policyData, err := os.ReadFile(policyPath) + require.NoError(t, err) + + var policy schema.CodePolicy + require.NoError(t, json.Unmarshal(policyData, &policy)) + + v := validator.NewValidatorWithWorkDir(&policy, true, testdataDir) + defer v.Close() + + adp, err := adapterRegistry.Global().GetAdapter("pmd") + if err != nil { + t.Skipf("PMD adapter not found: %v", err) + } + if err := adp.CheckAvailability(context.Background()); err != nil { + t.Skipf("PMD not available: %v", err) + } + + testFile := filepath.Join(testdataDir, "Test.java") + changes := []validator.GitChange{{ + FilePath: testFile, + Status: "M", + Diff: "", + }} + + ctx := context.Background() + result, err := v.ValidateChanges(ctx, changes) + require.NoError(t, err) + + // Check for EmptyCatchBlock violation + found := false + for _, viol := range result.Violations { + if strings.Contains(viol.Message, "empty catch") || strings.Contains(viol.Message, "EmptyCatchBlock") { + found = true + t.Logf("Found EmptyCatchBlock violation at line %d: %s", viol.Line, viol.Message) + } + } + + if !found { + t.Log("EmptyCatchBlock violation not detected (may depend on PMD version)") + } +} + +func TestPMD_UnusedPrivateMethod(t *testing.T) { + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "pmd")) + require.NoError(t, err) + + if _, err := os.Stat(testdataDir); os.IsNotExist(err) { + t.Skipf("Testdata directory not found: %s", testdataDir) + } + + policyPath := filepath.Join(testdataDir, ".sym", "code-policy.json") + policyData, err := os.ReadFile(policyPath) + require.NoError(t, err) + + var policy schema.CodePolicy + require.NoError(t, json.Unmarshal(policyData, &policy)) + + v := validator.NewValidatorWithWorkDir(&policy, true, testdataDir) + defer v.Close() + + adp, err := adapterRegistry.Global().GetAdapter("pmd") + if err != nil { + t.Skipf("PMD adapter not found: %v", err) + } + if err := adp.CheckAvailability(context.Background()); err != nil { + t.Skipf("PMD not available: %v", err) + } + + testFile := filepath.Join(testdataDir, "Test.java") + changes := []validator.GitChange{{ + FilePath: testFile, + Status: "M", + Diff: "", + }} + + ctx := context.Background() + result, err := v.ValidateChanges(ctx, changes) + require.NoError(t, err) + + // Check for UnusedPrivateMethod violation + found := false + for _, viol := range result.Violations { + if strings.Contains(viol.Message, "unused") || strings.Contains(viol.Message, "UnusedPrivateMethod") { + found = true + t.Logf("Found UnusedPrivateMethod violation at line %d: %s", viol.Line, viol.Message) + } + } + + if !found { + t.Log("UnusedPrivateMethod violation not detected (may depend on PMD version)") + } +} + +func TestPMD_ToolNameAndRuleID(t *testing.T) { + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "pmd")) + require.NoError(t, err) + + if _, err := os.Stat(testdataDir); os.IsNotExist(err) { + t.Skipf("Testdata directory not found: %s", testdataDir) + } + + policyPath := filepath.Join(testdataDir, ".sym", "code-policy.json") + policyData, err := os.ReadFile(policyPath) + require.NoError(t, err) + + var policy schema.CodePolicy + require.NoError(t, json.Unmarshal(policyData, &policy)) + + v := validator.NewValidatorWithWorkDir(&policy, true, testdataDir) + defer v.Close() + + adp, err := adapterRegistry.Global().GetAdapter("pmd") + if err != nil { + t.Skipf("PMD adapter not found: %v", err) + } + if err := adp.CheckAvailability(context.Background()); err != nil { + t.Skipf("PMD not available: %v", err) + } + + testFile := filepath.Join(testdataDir, "Test.java") + changes := []validator.GitChange{{ + FilePath: testFile, + Status: "M", + Diff: "", + }} + + ctx := context.Background() + result, err := v.ValidateChanges(ctx, changes) + require.NoError(t, err) + + require.Greater(t, len(result.Violations), 0, "Should have at least one violation") + + // Verify ToolName is set correctly + for _, viol := range result.Violations { + assert.Equal(t, "pmd", viol.ToolName, + "ToolName should be 'pmd', got '%s'", viol.ToolName) + } + + // Verify RuleID comes from policy + for _, viol := range result.Violations { + assert.True(t, strings.HasPrefix(viol.RuleID, "pmd-"), + "RuleID should start with 'pmd-', got '%s'", viol.RuleID) + } +} diff --git a/tests/integration/prettier_test.go b/tests/integration/prettier_test.go new file mode 100644 index 0000000..50db457 --- /dev/null +++ b/tests/integration/prettier_test.go @@ -0,0 +1,252 @@ +package integration + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + adapterRegistry "github.com/DevSymphony/sym-cli/internal/adapter/registry" + "github.com/DevSymphony/sym-cli/internal/validator" + "github.com/DevSymphony/sym-cli/pkg/schema" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestPrettier_ValidateChanges(t *testing.T) { + // 1. Get testdata path + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "prettier")) + require.NoError(t, err, "Failed to get testdata path") + + // Check testdata exists + if _, err := os.Stat(testdataDir); os.IsNotExist(err) { + t.Skipf("Testdata directory not found: %s", testdataDir) + } + + // 2. Load policy + policyPath := filepath.Join(testdataDir, ".sym", "code-policy.json") + policyData, err := os.ReadFile(policyPath) + require.NoError(t, err, "Failed to read code-policy.json") + + var policy schema.CodePolicy + require.NoError(t, json.Unmarshal(policyData, &policy), "Failed to parse policy") + + // 3. Create validator with custom workDir + v := validator.NewValidatorWithWorkDir(&policy, true, testdataDir) + defer v.Close() + + // 4. Check tool availability + adp, err := adapterRegistry.Global().GetAdapter("prettier") + if err != nil { + t.Skipf("Prettier adapter not found: %v", err) + } + if err := adp.CheckAvailability(context.Background()); err != nil { + t.Skipf("Prettier not available: %v", err) + } + + // 5. Create GitChange + testFile := filepath.Join(testdataDir, "Test.ts") + require.FileExists(t, testFile, "Test.ts should exist") + + changes := []validator.GitChange{{ + FilePath: testFile, + Status: "M", + Diff: "", + }} + + // 6. Execute ValidateChanges + ctx := context.Background() + result, err := v.ValidateChanges(ctx, changes) + require.NoError(t, err, "ValidateChanges failed") + + // 7. Log violations for debugging + t.Logf("Found %d violations", len(result.Violations)) + for _, viol := range result.Violations { + t.Logf(" - %s:%d: [%s] %s (tool: %s)", + viol.File, viol.Line, viol.RuleID, viol.Message, viol.ToolName) + } + + // 8. Assertions - Prettier should detect formatting issues + assert.GreaterOrEqual(t, len(result.Violations), 1, "Should detect at least 1 formatting violation") + + // Check that ToolName is prettier + for _, viol := range result.Violations { + assert.Equal(t, "prettier", viol.ToolName, "ToolName should be prettier") + } + + // Check that RuleID is from policy + validRuleIDs := map[string]bool{ + "prettier-quotes": true, + "prettier-indent": true, + "prettier-printwidth": true, + } + for _, viol := range result.Violations { + assert.True(t, validRuleIDs[viol.RuleID], + "RuleID should be from policy, got: %s", viol.RuleID) + } +} + +func TestPrettier_FormattingCheck(t *testing.T) { + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "prettier")) + require.NoError(t, err) + + if _, err := os.Stat(testdataDir); os.IsNotExist(err) { + t.Skipf("Testdata directory not found: %s", testdataDir) + } + + policyPath := filepath.Join(testdataDir, ".sym", "code-policy.json") + policyData, err := os.ReadFile(policyPath) + require.NoError(t, err) + + var policy schema.CodePolicy + require.NoError(t, json.Unmarshal(policyData, &policy)) + + v := validator.NewValidatorWithWorkDir(&policy, true, testdataDir) + defer v.Close() + + adp, err := adapterRegistry.Global().GetAdapter("prettier") + if err != nil { + t.Skipf("Prettier adapter not found: %v", err) + } + if err := adp.CheckAvailability(context.Background()); err != nil { + t.Skipf("Prettier not available: %v", err) + } + + testFile := filepath.Join(testdataDir, "Test.ts") + changes := []validator.GitChange{{ + FilePath: testFile, + Status: "M", + Diff: "", + }} + + ctx := context.Background() + result, err := v.ValidateChanges(ctx, changes) + require.NoError(t, err) + + // Prettier reports files that need formatting + // The Test.ts file has: + // - Double quotes (should be single quotes) + // - 4-space indentation (should be 2) + // - Lines exceeding printWidth + + if len(result.Violations) == 0 { + t.Log("No formatting violations found (file may already be formatted)") + } else { + t.Logf("Found %d formatting issue(s)", len(result.Violations)) + for _, viol := range result.Violations { + t.Logf(" File needs formatting: %s", viol.File) + } + } + + // Prettier should detect that the file needs formatting + assert.GreaterOrEqual(t, len(result.Violations), 1, "Test.ts should need formatting") +} + +func TestPrettier_QuotesAndIndent(t *testing.T) { + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "prettier")) + require.NoError(t, err) + + if _, err := os.Stat(testdataDir); os.IsNotExist(err) { + t.Skipf("Testdata directory not found: %s", testdataDir) + } + + policyPath := filepath.Join(testdataDir, ".sym", "code-policy.json") + policyData, err := os.ReadFile(policyPath) + require.NoError(t, err) + + var policy schema.CodePolicy + require.NoError(t, json.Unmarshal(policyData, &policy)) + + v := validator.NewValidatorWithWorkDir(&policy, true, testdataDir) + defer v.Close() + + adp, err := adapterRegistry.Global().GetAdapter("prettier") + if err != nil { + t.Skipf("Prettier adapter not found: %v", err) + } + if err := adp.CheckAvailability(context.Background()); err != nil { + t.Skipf("Prettier not available: %v", err) + } + + // Read the test file to verify it has violations + testFile := filepath.Join(testdataDir, "Test.ts") + content, err := os.ReadFile(testFile) + require.NoError(t, err) + + // Verify the file contains double quotes (which should be single) + assert.Contains(t, string(content), `"https://`, "Test file should contain double quotes") + + changes := []validator.GitChange{{ + FilePath: testFile, + Status: "M", + Diff: "", + }} + + ctx := context.Background() + result, err := v.ValidateChanges(ctx, changes) + require.NoError(t, err) + + // The file has double quotes but config requires single quotes + // Prettier should report this file needs formatting + assert.GreaterOrEqual(t, len(result.Violations), 1, "Should detect formatting issues with quotes") +} + +func TestPrettier_ToolNameAndRuleID(t *testing.T) { + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "prettier")) + require.NoError(t, err) + + if _, err := os.Stat(testdataDir); os.IsNotExist(err) { + t.Skipf("Testdata directory not found: %s", testdataDir) + } + + policyPath := filepath.Join(testdataDir, ".sym", "code-policy.json") + policyData, err := os.ReadFile(policyPath) + require.NoError(t, err) + + var policy schema.CodePolicy + require.NoError(t, json.Unmarshal(policyData, &policy)) + + v := validator.NewValidatorWithWorkDir(&policy, true, testdataDir) + defer v.Close() + + adp, err := adapterRegistry.Global().GetAdapter("prettier") + if err != nil { + t.Skipf("Prettier adapter not found: %v", err) + } + if err := adp.CheckAvailability(context.Background()); err != nil { + t.Skipf("Prettier not available: %v", err) + } + + testFile := filepath.Join(testdataDir, "Test.ts") + changes := []validator.GitChange{{ + FilePath: testFile, + Status: "M", + Diff: "", + }} + + ctx := context.Background() + result, err := v.ValidateChanges(ctx, changes) + require.NoError(t, err) + + require.Greater(t, len(result.Violations), 0, "Should have at least one violation") + + // Verify ToolName is set correctly + for _, viol := range result.Violations { + assert.Equal(t, "prettier", viol.ToolName, + "ToolName should be 'prettier', got '%s'", viol.ToolName) + } + + // Verify RuleID comes from policy + for _, viol := range result.Violations { + assert.True(t, strings.HasPrefix(viol.RuleID, "prettier-"), + "RuleID should start with 'prettier-', got '%s'", viol.RuleID) + } + + // Verify Severity comes from policy (warning for prettier rules) + for _, viol := range result.Violations { + assert.Equal(t, "warning", viol.Severity, + "Severity should be 'warning' from policy, got '%s'", viol.Severity) + } +} diff --git a/tests/integration/pylint_test.go b/tests/integration/pylint_test.go new file mode 100644 index 0000000..ed63319 --- /dev/null +++ b/tests/integration/pylint_test.go @@ -0,0 +1,231 @@ +package integration + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + adapterRegistry "github.com/DevSymphony/sym-cli/internal/adapter/registry" + "github.com/DevSymphony/sym-cli/internal/validator" + "github.com/DevSymphony/sym-cli/pkg/schema" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestPylint_ValidateChanges(t *testing.T) { + // 1. Get testdata path + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "pylint")) + require.NoError(t, err, "Failed to get testdata path") + + // Check testdata exists + if _, err := os.Stat(testdataDir); os.IsNotExist(err) { + t.Skipf("Testdata directory not found: %s", testdataDir) + } + + // 2. Load policy (enforce.rbac ์—†์œผ๋ฉด RBAC ๋น„ํ™œ์„ฑํ™”๋จ) + policyPath := filepath.Join(testdataDir, ".sym", "code-policy.json") + policyData, err := os.ReadFile(policyPath) + require.NoError(t, err, "Failed to read code-policy.json") + + var policy schema.CodePolicy + require.NoError(t, json.Unmarshal(policyData, &policy), "Failed to parse policy") + + // 3. Create validator with custom workDir (symDir = workDir/.sym) + v := validator.NewValidatorWithWorkDir(&policy, true, testdataDir) + defer v.Close() + + // 4. Check tool availability + adp, err := adapterRegistry.Global().GetAdapter("pylint") + if err != nil { + t.Skipf("Pylint adapter not found: %v", err) + } + if err := adp.CheckAvailability(context.Background()); err != nil { + t.Skipf("Pylint not available: %v", err) + } + + // 5. Create GitChange (simulate modified file) + testFile := filepath.Join(testdataDir, "Test.py") + require.FileExists(t, testFile, "Test.py should exist") + + changes := []validator.GitChange{{ + FilePath: testFile, + Status: "M", // Modified + Diff: "", // Diff not needed for adapter-based rules + }} + + // 6. Execute ValidateChanges + ctx := context.Background() + result, err := v.ValidateChanges(ctx, changes) + require.NoError(t, err, "ValidateChanges failed") + + // 7. Log violations for debugging + t.Logf("Found %d violations", len(result.Violations)) + for _, viol := range result.Violations { + t.Logf(" - %s:%d: [%s] %s (tool: %s)", + viol.File, viol.Line, viol.RuleID, viol.Message, viol.ToolName) + } + + // 8. Assertions + assert.GreaterOrEqual(t, len(result.Violations), 4, "Should detect at least 4 naming violations") + + // Check that ToolName is pylint + for _, viol := range result.Violations { + assert.Equal(t, "pylint", viol.ToolName, "ToolName should be pylint") + } + + // Check that RuleID is from policy (not adapter's C0103) + // Policy rule IDs: pylint-naming-func, pylint-naming-class, pylint-naming-const + validRuleIDs := map[string]bool{ + "pylint-naming-func": true, + "pylint-naming-class": true, + "pylint-naming-const": true, + } + for _, viol := range result.Violations { + assert.True(t, validRuleIDs[viol.RuleID], + "RuleID should be from policy, got: %s", viol.RuleID) + } +} + +func TestPylint_NamingConventions(t *testing.T) { + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "pylint")) + require.NoError(t, err) + + if _, err := os.Stat(testdataDir); os.IsNotExist(err) { + t.Skipf("Testdata directory not found: %s", testdataDir) + } + + policyPath := filepath.Join(testdataDir, ".sym", "code-policy.json") + policyData, err := os.ReadFile(policyPath) + require.NoError(t, err) + + var policy schema.CodePolicy + require.NoError(t, json.Unmarshal(policyData, &policy)) + + v := validator.NewValidatorWithWorkDir(&policy, true, testdataDir) + defer v.Close() + + adp, err := adapterRegistry.Global().GetAdapter("pylint") + if err != nil { + t.Skipf("Pylint adapter not found: %v", err) + } + if err := adp.CheckAvailability(context.Background()); err != nil { + t.Skipf("Pylint not available: %v", err) + } + + testFile := filepath.Join(testdataDir, "Test.py") + changes := []validator.GitChange{{ + FilePath: testFile, + Status: "M", + Diff: "", + }} + + ctx := context.Background() + result, err := v.ValidateChanges(ctx, changes) + require.NoError(t, err) + + // Test specific naming convention checks + tests := []struct { + name string + checkLine int + description string + }{ + { + name: "snake_case class name violation", + checkLine: 6, + description: "user_profile class should trigger PascalCase violation", + }, + { + name: "camelCase function name violation", + checkLine: 10, + description: "getUserName function should trigger snake_case violation", + }, + { + name: "snake_case class name violation 2", + checkLine: 17, + description: "dataProcessor class should trigger PascalCase violation", + }, + { + name: "camelCase function name violation 2", + checkLine: 25, + description: "calculateTotal function should trigger snake_case violation", + }, + } + + // Build violation line map + violationLines := make(map[int]bool) + for _, viol := range result.Violations { + violationLines[viol.Line] = true + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if violationLines[tt.checkLine] { + t.Logf("Found expected violation at line %d", tt.checkLine) + } else { + t.Logf("Expected violation at line %d not found", tt.checkLine) + } + }) + } +} + +func TestPylint_ToolNameAndRuleID(t *testing.T) { + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "pylint")) + require.NoError(t, err) + + if _, err := os.Stat(testdataDir); os.IsNotExist(err) { + t.Skipf("Testdata directory not found: %s", testdataDir) + } + + policyPath := filepath.Join(testdataDir, ".sym", "code-policy.json") + policyData, err := os.ReadFile(policyPath) + require.NoError(t, err) + + var policy schema.CodePolicy + require.NoError(t, json.Unmarshal(policyData, &policy)) + + v := validator.NewValidatorWithWorkDir(&policy, true, testdataDir) + defer v.Close() + + adp, err := adapterRegistry.Global().GetAdapter("pylint") + if err != nil { + t.Skipf("Pylint adapter not found: %v", err) + } + if err := adp.CheckAvailability(context.Background()); err != nil { + t.Skipf("Pylint not available: %v", err) + } + + testFile := filepath.Join(testdataDir, "Test.py") + changes := []validator.GitChange{{ + FilePath: testFile, + Status: "M", + Diff: "", + }} + + ctx := context.Background() + result, err := v.ValidateChanges(ctx, changes) + require.NoError(t, err) + + require.Greater(t, len(result.Violations), 0, "Should have at least one violation") + + // Verify ToolName is set correctly + for _, viol := range result.Violations { + assert.Equal(t, "pylint", viol.ToolName, + "ToolName should be 'pylint', got '%s'", viol.ToolName) + } + + // Verify RuleID comes from policy, not adapter + // Adapter would return C0103, but policy should map to pylint-naming-* + for _, viol := range result.Violations { + assert.True(t, strings.HasPrefix(viol.RuleID, "pylint-"), + "RuleID should start with 'pylint-', got '%s'", viol.RuleID) + } + + // Verify Severity comes from policy + for _, viol := range result.Violations { + assert.Equal(t, "error", viol.Severity, + "Severity should be 'error' from policy, got '%s'", viol.Severity) + } +} diff --git a/tests/integration/tsc_test.go b/tests/integration/tsc_test.go new file mode 100644 index 0000000..31523c6 --- /dev/null +++ b/tests/integration/tsc_test.go @@ -0,0 +1,245 @@ +package integration + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + adapterRegistry "github.com/DevSymphony/sym-cli/internal/adapter/registry" + "github.com/DevSymphony/sym-cli/internal/validator" + "github.com/DevSymphony/sym-cli/pkg/schema" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestTSC_ValidateChanges(t *testing.T) { + // 1. Get testdata path + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "tsc")) + require.NoError(t, err, "Failed to get testdata path") + + // Check testdata exists + if _, err := os.Stat(testdataDir); os.IsNotExist(err) { + t.Skipf("Testdata directory not found: %s", testdataDir) + } + + // 2. Load policy + policyPath := filepath.Join(testdataDir, ".sym", "code-policy.json") + policyData, err := os.ReadFile(policyPath) + require.NoError(t, err, "Failed to read code-policy.json") + + var policy schema.CodePolicy + require.NoError(t, json.Unmarshal(policyData, &policy), "Failed to parse policy") + + // 3. Create validator with custom workDir + v := validator.NewValidatorWithWorkDir(&policy, true, testdataDir) + defer v.Close() + + // 4. Check tool availability + adp, err := adapterRegistry.Global().GetAdapter("tsc") + if err != nil { + t.Skipf("TSC adapter not found: %v", err) + } + if err := adp.CheckAvailability(context.Background()); err != nil { + t.Skipf("TSC not available: %v", err) + } + + // 5. Create GitChange + testFile := filepath.Join(testdataDir, "Test.ts") + require.FileExists(t, testFile, "Test.ts should exist") + + changes := []validator.GitChange{{ + FilePath: testFile, + Status: "M", + Diff: "", + }} + + // 6. Execute ValidateChanges + ctx := context.Background() + result, err := v.ValidateChanges(ctx, changes) + require.NoError(t, err, "ValidateChanges failed") + + // 7. Log violations for debugging + t.Logf("Found %d violations", len(result.Violations)) + for _, viol := range result.Violations { + t.Logf(" - %s:%d:%d: [%s] %s (tool: %s)", + viol.File, viol.Line, viol.Column, viol.RuleID, viol.Message, viol.ToolName) + } + + // 8. Assertions - TSC should detect strict null check violations + assert.GreaterOrEqual(t, len(result.Violations), 2, "Should detect at least 2 type violations") + + // Check that ToolName is tsc + for _, viol := range result.Violations { + assert.Equal(t, "tsc", viol.ToolName, "ToolName should be tsc") + } + + // Check that RuleID is from policy + for _, viol := range result.Violations { + assert.Equal(t, "tsc-strictnull", viol.RuleID, + "RuleID should be 'tsc-strictnull' from policy, got: %s", viol.RuleID) + } +} + +func TestTSC_StrictNullChecks(t *testing.T) { + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "tsc")) + require.NoError(t, err) + + if _, err := os.Stat(testdataDir); os.IsNotExist(err) { + t.Skipf("Testdata directory not found: %s", testdataDir) + } + + policyPath := filepath.Join(testdataDir, ".sym", "code-policy.json") + policyData, err := os.ReadFile(policyPath) + require.NoError(t, err) + + var policy schema.CodePolicy + require.NoError(t, json.Unmarshal(policyData, &policy)) + + v := validator.NewValidatorWithWorkDir(&policy, true, testdataDir) + defer v.Close() + + adp, err := adapterRegistry.Global().GetAdapter("tsc") + if err != nil { + t.Skipf("TSC adapter not found: %v", err) + } + if err := adp.CheckAvailability(context.Background()); err != nil { + t.Skipf("TSC not available: %v", err) + } + + // Verify config has strictNullChecks enabled + configPath := filepath.Join(testdataDir, ".sym", "tsconfig.json") + configData, err := os.ReadFile(configPath) + require.NoError(t, err) + assert.Contains(t, string(configData), `"strictNullChecks": true`, "Config should have strictNullChecks enabled") + + testFile := filepath.Join(testdataDir, "Test.ts") + changes := []validator.GitChange{{ + FilePath: testFile, + Status: "M", + Diff: "", + }} + + ctx := context.Background() + result, err := v.ValidateChanges(ctx, changes) + require.NoError(t, err) + + // Check for undefined/null type errors in messages + hasTypeError := false + for _, viol := range result.Violations { + if strings.Contains(viol.Message, "undefined") || strings.Contains(viol.Message, "not assignable") { + hasTypeError = true + break + } + } + assert.True(t, hasTypeError, "Should detect undefined type assignment violations") +} + +func TestTSC_TypeErrors(t *testing.T) { + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "tsc")) + require.NoError(t, err) + + if _, err := os.Stat(testdataDir); os.IsNotExist(err) { + t.Skipf("Testdata directory not found: %s", testdataDir) + } + + policyPath := filepath.Join(testdataDir, ".sym", "code-policy.json") + policyData, err := os.ReadFile(policyPath) + require.NoError(t, err) + + var policy schema.CodePolicy + require.NoError(t, json.Unmarshal(policyData, &policy)) + + v := validator.NewValidatorWithWorkDir(&policy, true, testdataDir) + defer v.Close() + + adp, err := adapterRegistry.Global().GetAdapter("tsc") + if err != nil { + t.Skipf("TSC adapter not found: %v", err) + } + if err := adp.CheckAvailability(context.Background()); err != nil { + t.Skipf("TSC not available: %v", err) + } + + testFile := filepath.Join(testdataDir, "Test.ts") + changes := []validator.GitChange{{ + FilePath: testFile, + Status: "M", + Diff: "", + }} + + ctx := context.Background() + result, err := v.ValidateChanges(ctx, changes) + require.NoError(t, err) + + // Check that TSC reports type errors + typeErrorCount := 0 + for _, viol := range result.Violations { + if strings.Contains(viol.Message, "Type") && strings.Contains(viol.Message, "not assignable") { + typeErrorCount++ + t.Logf("Type error at line %d: %s", viol.Line, viol.Message) + } + } + + assert.GreaterOrEqual(t, typeErrorCount, 2, "Should detect at least 2 type assignment errors") +} + +func TestTSC_ToolNameAndRuleID(t *testing.T) { + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "tsc")) + require.NoError(t, err) + + if _, err := os.Stat(testdataDir); os.IsNotExist(err) { + t.Skipf("Testdata directory not found: %s", testdataDir) + } + + policyPath := filepath.Join(testdataDir, ".sym", "code-policy.json") + policyData, err := os.ReadFile(policyPath) + require.NoError(t, err) + + var policy schema.CodePolicy + require.NoError(t, json.Unmarshal(policyData, &policy)) + + v := validator.NewValidatorWithWorkDir(&policy, true, testdataDir) + defer v.Close() + + adp, err := adapterRegistry.Global().GetAdapter("tsc") + if err != nil { + t.Skipf("TSC adapter not found: %v", err) + } + if err := adp.CheckAvailability(context.Background()); err != nil { + t.Skipf("TSC not available: %v", err) + } + + testFile := filepath.Join(testdataDir, "Test.ts") + changes := []validator.GitChange{{ + FilePath: testFile, + Status: "M", + Diff: "", + }} + + ctx := context.Background() + result, err := v.ValidateChanges(ctx, changes) + require.NoError(t, err) + + require.Greater(t, len(result.Violations), 0, "Should have at least one violation") + + // Verify ToolName is set correctly + for _, viol := range result.Violations { + assert.Equal(t, "tsc", viol.ToolName, + "ToolName should be 'tsc', got '%s'", viol.ToolName) + } + + // Verify RuleID comes from policy + for _, viol := range result.Violations { + assert.Equal(t, "tsc-strictnull", viol.RuleID, + "RuleID should be 'tsc-strictnull' from policy, got '%s'", viol.RuleID) + } + + // Verify Severity comes from policy + for _, viol := range result.Violations { + assert.Equal(t, "error", viol.Severity, + "Severity should be 'error' from policy, got '%s'", viol.Severity) + } +} diff --git a/tests/testdata/checkstyle/.sym/checkstyle.xml b/tests/testdata/checkstyle/.sym/checkstyle.xml new file mode 100644 index 0000000..734e86a --- /dev/null +++ b/tests/testdata/checkstyle/.sym/checkstyle.xml @@ -0,0 +1,24 @@ + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/testdata/checkstyle/.sym/code-policy.json b/tests/testdata/checkstyle/.sym/code-policy.json new file mode 100644 index 0000000..f20046c --- /dev/null +++ b/tests/testdata/checkstyle/.sym/code-policy.json @@ -0,0 +1,33 @@ +{ + "version": "1.0", + "rules": [ + { + "id": "checkstyle-typename", + "enabled": true, + "category": "naming", + "severity": "error", + "desc": "ํด๋ž˜์Šค ์ด๋ฆ„์€ PascalCase๋ฅผ ์‚ฌ์šฉํ•ฉ๋‹ˆ๋‹ค", + "when": { "languages": ["java"] }, + "check": { "engine": "checkstyle" } + }, + { + "id": "checkstyle-methodname", + "enabled": true, + "category": "naming", + "severity": "error", + "desc": "๋ฉ”์„œ๋“œ ์ด๋ฆ„์€ camelCase๋ฅผ ์‚ฌ์šฉํ•ฉ๋‹ˆ๋‹ค", + "when": { "languages": ["java"] }, + "check": { "engine": "checkstyle" } + }, + { + "id": "checkstyle-membername", + "enabled": true, + "category": "naming", + "severity": "error", + "desc": "๋ฉค๋ฒ„ ๋ณ€์ˆ˜ ์ด๋ฆ„ ๊ทœ์น™์„ ์ค€์ˆ˜ํ•ฉ๋‹ˆ๋‹ค", + "when": { "languages": ["java"] }, + "check": { "engine": "checkstyle" } + } + ], + "enforce": { "stages": ["pre-commit"], "fail_on": ["error"] } +} diff --git a/tests/testdata/checkstyle/Test.java b/tests/testdata/checkstyle/Test.java new file mode 100644 index 0000000..c3dd220 --- /dev/null +++ b/tests/testdata/checkstyle/Test.java @@ -0,0 +1,16 @@ +// Checkstyle ์œ„๋ฐ˜: TypeName (ํด๋ž˜์Šค๋ช… snake_case) +public class bad_class { + // Checkstyle ์œ„๋ฐ˜: MemberName (private ๋ณ€์ˆ˜ m_ ๋ฏธ์‚ฌ์šฉ) + private int MyVar = 1; + + // Checkstyle ์œ„๋ฐ˜: MethodName (๋ฉ”์„œ๋“œ๋ช… PascalCase) + public void BadFunc() { + final int CONST = 2; + int result = 10 / CONST; + System.out.println(result); + } + + public int getMyVar() { + return MyVar; + } +} diff --git a/tests/testdata/eslint/.sym/.eslintrc.json b/tests/testdata/eslint/.sym/.eslintrc.json new file mode 100644 index 0000000..5450a31 --- /dev/null +++ b/tests/testdata/eslint/.sym/.eslintrc.json @@ -0,0 +1,34 @@ +{ + "env": { + "browser": true, + "es2021": true, + "node": true + }, + "parserOptions": { + "ecmaVersion": "latest", + "sourceType": "module" + }, + "rules": { + "id-match": [ + "error", + "^(?:[A-Z][a-zA-Z0-9]*|[a-z][a-zA-Z0-9]*)$", + { + "onlyDeclarations": false, + "properties": true + } + ], + "max-len": [ + "warn", + { + "code": 100, + "ignoreComments": false, + "ignoreRegExpLiterals": true, + "ignoreStrings": true, + "ignoreTemplateLiterals": true, + "ignoreTrailingComments": false, + "ignoreUrls": true, + "tabWidth": 4 + } + ] + } +} diff --git a/tests/testdata/eslint/.sym/code-policy.json b/tests/testdata/eslint/.sym/code-policy.json new file mode 100644 index 0000000..4afc0e4 --- /dev/null +++ b/tests/testdata/eslint/.sym/code-policy.json @@ -0,0 +1,24 @@ +{ + "version": "1.0", + "rules": [ + { + "id": "eslint-naming", + "enabled": true, + "category": "naming", + "severity": "error", + "desc": "ํด๋ž˜์Šค๋Š” PascalCase, ํ•จ์ˆ˜/๋ณ€์ˆ˜๋Š” camelCase๋ฅผ ์‚ฌ์šฉํ•ฉ๋‹ˆ๋‹ค", + "when": { "languages": ["typescript", "javascript"] }, + "check": { "engine": "eslint" } + }, + { + "id": "eslint-maxlen", + "enabled": true, + "category": "style", + "severity": "warning", + "desc": "ํ•œ ์ค„์€ ์ตœ๋Œ€ 100์ž๋กœ ์ œํ•œํ•ฉ๋‹ˆ๋‹ค", + "when": { "languages": ["typescript", "javascript"] }, + "check": { "engine": "eslint" } + } + ], + "enforce": { "stages": ["pre-commit"], "fail_on": ["error"] } +} diff --git a/tests/testdata/eslint/Test.ts b/tests/testdata/eslint/Test.ts new file mode 100644 index 0000000..d94ab36 --- /dev/null +++ b/tests/testdata/eslint/Test.ts @@ -0,0 +1,56 @@ +// Test.ts - ESLint naming/max-len violation test file +// Using JavaScript-compatible syntax for ESLint without TypeScript parser + +// [eslint id-match ์œ„๋ฐ˜] snake_case ํด๋ž˜์Šค๋ช… (PascalCase์—ฌ์•ผ ํ•จ) +class user_service { + constructor() { + // [eslint id-match ์œ„๋ฐ˜] snake_case ๋ณ€์ˆ˜๋ช… + this.api_url = 'https://api.example.com'; + } + + // [eslint id-match ์œ„๋ฐ˜] snake_case ํ•จ์ˆ˜๋ช… + async fetch_user_data(userId) { + const response = await fetch(this.api_url + '/users/' + userId); + const data = await response.json(); + return data; + } + + // [eslint id-match ์œ„๋ฐ˜] snake_case ํ•จ์ˆ˜๋ช… + get_user_name(user) { + return user.name; + } +} + +// [eslint max-len ์œ„๋ฐ˜] 100์ž ์ดˆ๊ณผ ๋ผ์ธ +const very_long_variable_name_that_exceeds_one_hundred_characters_limit_for_testing_eslint_max_length_rule = 'test'; + +// [eslint id-match ์œ„๋ฐ˜] snake_case ํด๋ž˜์Šค๋ช… +class data_processor { + constructor() { + // [eslint id-match ์œ„๋ฐ˜] snake_case ๋ณ€์ˆ˜๋ช… + this.processed_count = 0; + } + + // [eslint id-match ์œ„๋ฐ˜] snake_case ํ•จ์ˆ˜๋ช… + transform_item(rawItem) { + return { + id: rawItem.id, + name: rawItem.name, + timestamp: Date.now() + }; + } +} + +// [eslint id-match ์œ„๋ฐ˜] snake_case ํ•จ์ˆ˜๋ช… +async function load_all_data(config) { + const service = new user_service(); + const result = await service.fetch_user_data(config.userId); + console.log('Loaded data:', result); + return result; +} + +// [eslint id-match ์œ„๋ฐ˜] snake_case ๋ณ€์ˆ˜๋ช… +const api_client = new user_service(); +const data_handler = new data_processor(); + +module.exports = { user_service, data_processor, load_all_data, api_client, data_handler }; diff --git a/tests/testdata/pmd/.sym/code-policy.json b/tests/testdata/pmd/.sym/code-policy.json new file mode 100644 index 0000000..981f5fb --- /dev/null +++ b/tests/testdata/pmd/.sym/code-policy.json @@ -0,0 +1,33 @@ +{ + "version": "1.0", + "rules": [ + { + "id": "pmd-emptycatch", + "enabled": true, + "category": "error_handling", + "severity": "error", + "desc": "๋นˆ catch ๋ธ”๋ก์„ ์‚ฌ์šฉํ•˜์ง€ ์•Š์Šต๋‹ˆ๋‹ค", + "when": { "languages": ["java"] }, + "check": { "engine": "pmd" } + }, + { + "id": "pmd-unusedmethod", + "enabled": true, + "category": "style", + "severity": "warning", + "desc": "์‚ฌ์šฉํ•˜์ง€ ์•Š๋Š” private ๋ฉ”์„œ๋“œ๋ฅผ ์ œ๊ฑฐํ•ฉ๋‹ˆ๋‹ค", + "when": { "languages": ["java"] }, + "check": { "engine": "pmd" } + }, + { + "id": "pmd-complexity", + "enabled": true, + "category": "complexity", + "severity": "warning", + "desc": "์ˆœํ™˜ ๋ณต์žก๋„๋Š” 10 ์ดํ•˜๋กœ ์œ ์ง€ํ•ฉ๋‹ˆ๋‹ค", + "when": { "languages": ["java"] }, + "check": { "engine": "pmd" } + } + ], + "enforce": { "stages": ["pre-commit"], "fail_on": ["error"] } +} diff --git a/tests/testdata/pmd/.sym/pmd.xml b/tests/testdata/pmd/.sym/pmd.xml new file mode 100644 index 0000000..92797cb --- /dev/null +++ b/tests/testdata/pmd/.sym/pmd.xml @@ -0,0 +1,13 @@ + + + Generated from Symphony user policy + + 2 + + + 2 + + + 2 + + diff --git a/tests/testdata/pmd/Test.java b/tests/testdata/pmd/Test.java new file mode 100644 index 0000000..032ca8a --- /dev/null +++ b/tests/testdata/pmd/Test.java @@ -0,0 +1,40 @@ +// PMD violations test file +public class TestClass { + private int myVar = 1; + + // PMD ์œ„๋ฐ˜: UnusedPrivateMethod (์‚ฌ์šฉํ•˜์ง€ ์•Š๋Š” private ๋ฉ”์„œ๋“œ) + private void unusedMethod() { + System.out.println("This method is never called"); + } + + public void testMethod() { + final int CONST = 2; + + // PMD ์œ„๋ฐ˜: EmptyCatchBlock (๋นˆ catch ๋ธ”๋ก) + try { + int result = 10 / CONST; + } catch (Exception e) { + // empty catch block - PMD should detect this + } + } + + // PMD ์œ„๋ฐ˜: CyclomaticComplexity (๋ณต์žก๋„ > 10) + public int complexMethod(int x) { + if (x == 1) return 1; + else if (x == 2) return 2; + else if (x == 3) return 3; + else if (x == 4) return 4; + else if (x == 5) return 5; + else if (x == 6) return 6; + else if (x == 7) return 7; + else if (x == 8) return 8; + else if (x == 9) return 9; + else if (x == 10) return 10; + else if (x == 11) return 11; + else return 0; + } + + public int getMyVar() { + return myVar; + } +} diff --git a/tests/testdata/prettier/.sym/.prettierrc b/tests/testdata/prettier/.sym/.prettierrc new file mode 100644 index 0000000..164091a --- /dev/null +++ b/tests/testdata/prettier/.sym/.prettierrc @@ -0,0 +1,9 @@ +{ + "arrowParens": "always", + "printWidth": 100, + "semi": true, + "singleQuote": true, + "tabWidth": 2, + "trailingComma": "es5", + "useTabs": false +} diff --git a/tests/testdata/prettier/.sym/code-policy.json b/tests/testdata/prettier/.sym/code-policy.json new file mode 100644 index 0000000..355a69f --- /dev/null +++ b/tests/testdata/prettier/.sym/code-policy.json @@ -0,0 +1,33 @@ +{ + "version": "1.0", + "rules": [ + { + "id": "prettier-quotes", + "enabled": true, + "category": "style", + "severity": "warning", + "desc": "์ž‘์€๋”ฐ์˜ดํ‘œ(single quote)๋ฅผ ์‚ฌ์šฉํ•ฉ๋‹ˆ๋‹ค", + "when": { "languages": ["typescript", "javascript"] }, + "check": { "engine": "prettier" } + }, + { + "id": "prettier-indent", + "enabled": true, + "category": "style", + "severity": "warning", + "desc": "๋“ค์—ฌ์“ฐ๊ธฐ๋Š” 2์นธ ์ŠคํŽ˜์ด์Šค๋ฅผ ์‚ฌ์šฉํ•ฉ๋‹ˆ๋‹ค", + "when": { "languages": ["typescript", "javascript"] }, + "check": { "engine": "prettier" } + }, + { + "id": "prettier-printwidth", + "enabled": true, + "category": "style", + "severity": "warning", + "desc": "ํ•œ ์ค„์€ ์ตœ๋Œ€ 100์ž๋กœ ์ œํ•œํ•ฉ๋‹ˆ๋‹ค", + "when": { "languages": ["typescript", "javascript"] }, + "check": { "engine": "prettier" } + } + ], + "enforce": { "stages": ["pre-commit"], "fail_on": ["error"] } +} diff --git a/tests/testdata/prettier/Test.ts b/tests/testdata/prettier/Test.ts new file mode 100644 index 0000000..b9b1d89 --- /dev/null +++ b/tests/testdata/prettier/Test.ts @@ -0,0 +1,47 @@ +// Test.ts - Prettier formatting violation test file + +// [prettier ์œ„๋ฐ˜] 4์นธ ๋“ค์—ฌ์“ฐ๊ธฐ (2์นธ์ด์–ด์•ผ ํ•จ) +class UserService { + // [prettier ์œ„๋ฐ˜] ํฐ๋”ฐ์˜ดํ‘œ ์‚ฌ์šฉ (์ž‘์€๋”ฐ์˜ดํ‘œ์—ฌ์•ผ ํ•จ) + private apiUrl: string = "https://api.example.com"; + + async fetchUserData(userId: string) { + // [prettier ์œ„๋ฐ˜] ํฐ๋”ฐ์˜ดํ‘œ ์‚ฌ์šฉ + const response = await fetch(this.apiUrl + "/users/" + userId); + const data = await response.json(); + return data; + } + + getUserName(user: { name: string }): string { + return user.name; + } +} + +// [prettier ์œ„๋ฐ˜] 100์ž ์ดˆ๊ณผ ๋ผ์ธ +const veryLongVariableNameThatExceedsOneHundredCharactersLimitForTestingPrettierPrintWidthRule: string = "this is a very long value that makes this line exceed the maximum allowed characters for prettier"; + +class DataProcessor { + private processedCount: number = 0; + + transformItem(rawItem: { id: number; name: string }) { + return { + id: rawItem.id, + // [prettier ์œ„๋ฐ˜] ํฐ๋”ฐ์˜ดํ‘œ ์‚ฌ์šฉ + name: rawItem.name + " processed", + timestamp: Date.now() + }; + } +} + +async function loadAllData(config: { userId: string }) { + const service = new UserService(); + const result = await service.fetchUserData(config.userId); + // [prettier ์œ„๋ฐ˜] ํฐ๋”ฐ์˜ดํ‘œ ์‚ฌ์šฉ + console.log("Loaded data:", result); + return result; +} + +const apiClient = new UserService(); +const dataHandler = new DataProcessor(); + +export { UserService, DataProcessor, loadAllData, apiClient, dataHandler }; diff --git a/tests/testdata/pylint/.sym/.pylintrc b/tests/testdata/pylint/.sym/.pylintrc new file mode 100644 index 0000000..358e28a --- /dev/null +++ b/tests/testdata/pylint/.sym/.pylintrc @@ -0,0 +1,11 @@ +[MASTER] +# Generated by Symphony CLI + +[MESSAGES CONTROL] +enable=invalid-name,invalid-name,invalid-name + +[BASIC] +variable-rgx=^_?[a-z][a-z0-9_]*$ +class-rgx=^[A-Z][a-zA-Z0-9]+$ +const-rgx=^[A-Z][A-Z0-9_]*$ +function-rgx=^_?[a-z][a-z0-9_]*$ diff --git a/tests/testdata/pylint/.sym/code-policy.json b/tests/testdata/pylint/.sym/code-policy.json new file mode 100644 index 0000000..127e3eb --- /dev/null +++ b/tests/testdata/pylint/.sym/code-policy.json @@ -0,0 +1,33 @@ +{ + "version": "1.0", + "rules": [ + { + "id": "pylint-naming-func", + "enabled": true, + "category": "naming", + "severity": "error", + "desc": "ํ•จ์ˆ˜์™€ ๋ณ€์ˆ˜ ์ด๋ฆ„์€ snake_case๋ฅผ ์‚ฌ์šฉํ•ฉ๋‹ˆ๋‹ค", + "when": { "languages": ["python"] }, + "check": { "engine": "pylint" } + }, + { + "id": "pylint-naming-class", + "enabled": true, + "category": "naming", + "severity": "error", + "desc": "ํด๋ž˜์Šค ์ด๋ฆ„์€ PascalCase๋ฅผ ์‚ฌ์šฉํ•ฉ๋‹ˆ๋‹ค", + "when": { "languages": ["python"] }, + "check": { "engine": "pylint" } + }, + { + "id": "pylint-naming-const", + "enabled": true, + "category": "naming", + "severity": "error", + "desc": "์ƒ์ˆ˜๋Š” UPPER_CASE๋ฅผ ์‚ฌ์šฉํ•ฉ๋‹ˆ๋‹ค", + "when": { "languages": ["python"] }, + "check": { "engine": "pylint" } + } + ], + "enforce": { "stages": ["pre-commit"], "fail_on": ["error"] } +} diff --git a/tests/testdata/pylint/Test.py b/tests/testdata/pylint/Test.py new file mode 100644 index 0000000..627bd10 --- /dev/null +++ b/tests/testdata/pylint/Test.py @@ -0,0 +1,29 @@ +# Test file that violates Python naming conventions + +maxSize = 100 # ์ƒ์ˆ˜์ธ๋ฐ ์†Œ๋ฌธ์ž (์œ„๋ฐ˜: UPPER_CASE ์•„๋‹˜) +api_key = "secret123" # ์ƒ์ˆ˜์ธ๋ฐ snake_case (์œ„๋ฐ˜) + +class user_profile: # ํด๋ž˜์Šค์ธ๋ฐ snake_case (์œ„๋ฐ˜: PascalCase ์•„๋‹˜) + def __init__(self, userName): # ๋งค๊ฐœ๋ณ€์ˆ˜ camelCase (์œ„๋ฐ˜) + self.userName = userName # ์†์„ฑ camelCase (์œ„๋ฐ˜) + + def getUserName(self): # ๋ฉ”์„œ๋“œ camelCase (์œ„๋ฐ˜: snake_case ์•„๋‹˜) + return self.userName + + def setUserName(self, newName): # ๋ฉ”์„œ๋“œ camelCase (์œ„๋ฐ˜) + self.userName = newName + + +class dataProcessor: # ํด๋ž˜์Šค snake_case (์œ„๋ฐ˜) + def processData(self, inputData): # ๋ฉ”์„œ๋“œ/๋งค๊ฐœ๋ณ€์ˆ˜ camelCase (์œ„๋ฐ˜) + totalCount = 0 # ๋ณ€์ˆ˜ camelCase (์œ„๋ฐ˜) + for itemValue in inputData: # ๋ณ€์ˆ˜ camelCase (์œ„๋ฐ˜) + totalCount += itemValue + return totalCount + + +def calculateTotal(itemList): # ํ•จ์ˆ˜ camelCase (์œ„๋ฐ˜) + totalPrice = 0 # ๋ณ€์ˆ˜ camelCase (์œ„๋ฐ˜) + for itemPrice in itemList: # ๋ณ€์ˆ˜ camelCase (์œ„๋ฐ˜) + totalPrice += itemPrice + return totalPrice diff --git a/tests/testdata/tsc/.sym/code-policy.json b/tests/testdata/tsc/.sym/code-policy.json new file mode 100644 index 0000000..115d246 --- /dev/null +++ b/tests/testdata/tsc/.sym/code-policy.json @@ -0,0 +1,15 @@ +{ + "version": "1.0", + "rules": [ + { + "id": "tsc-strictnull", + "enabled": true, + "category": "typechecker", + "severity": "error", + "desc": "null๊ณผ undefined๋Š” ๋ช…์‹œ์ ์œผ๋กœ ์ฒ˜๋ฆฌํ•ด์•ผ ํ•ฉ๋‹ˆ๋‹ค", + "when": { "languages": ["typescript"] }, + "check": { "engine": "tsc" } + } + ], + "enforce": { "stages": ["pre-commit"], "fail_on": ["error"] } +} diff --git a/tests/testdata/tsc/.sym/tsconfig.json b/tests/testdata/tsc/.sym/tsconfig.json new file mode 100644 index 0000000..977af39 --- /dev/null +++ b/tests/testdata/tsc/.sym/tsconfig.json @@ -0,0 +1,20 @@ +{ + "compilerOptions": { + "esModuleInterop": true, + "forceConsistentCasingInFileNames": true, + "lib": [ + "ES2020" + ], + "module": "commonjs", + "moduleResolution": "node", + "noImplicitAny": true, + "noUnusedLocals": false, + "noUnusedParameters": false, + "resolveJsonModule": true, + "skipLibCheck": true, + "strict": true, + "strictFunctionTypes": true, + "strictNullChecks": true, + "target": "ES2020" + } +} diff --git a/tests/testdata/tsc/Test.ts b/tests/testdata/tsc/Test.ts new file mode 100644 index 0000000..3ccf641 --- /dev/null +++ b/tests/testdata/tsc/Test.ts @@ -0,0 +1,40 @@ +// Test.ts - TypeScript strict null checks violation test file + +class UserService { + private apiUrl: string = 'https://api.example.com'; + + async fetchUserData(userId: string) { + const response = await fetch(this.apiUrl + '/users/' + userId); + const data = await response.json(); + return data; + } + + // [tsc strictNullChecks ์œ„๋ฐ˜] undefined ๊ฐ€๋Šฅ ๊ฐ’์„ string์œผ๋กœ ๋ฐ˜ํ™˜ + getUserName(user: { name?: string }): string { + return user.name; // user.name์ด undefined์ผ ์ˆ˜ ์žˆ์Œ + } + + // [tsc strictNullChecks ์œ„๋ฐ˜] undefined ๊ฐ€๋Šฅ ๊ฐ’์„ number๋กœ ๋ฐ˜ํ™˜ + getUserAge(user: { age?: number }): number { + return user.age; // user.age๊ฐ€ undefined์ผ ์ˆ˜ ์žˆ์Œ + } +} + +class DataProcessor { + private processedCount: number = 0; + + // [tsc strictNullChecks ์œ„๋ฐ˜] undefined ๊ฐ€๋Šฅ ๊ฐ’์„ number๋กœ ๋ฐ˜ํ™˜ + getItemId(item: { id?: number }): number { + return item.id; // item.id๊ฐ€ undefined์ผ ์ˆ˜ ์žˆ์Œ + } + + // [tsc strictNullChecks ์œ„๋ฐ˜] null ๊ฐ€๋Šฅ ๊ฐ’์„ string์œผ๋กœ ๋ฐ˜ํ™˜ + getItemName(item: { name: string | null }): string { + return item.name; // item.name์ด null์ผ ์ˆ˜ ์žˆ์Œ + } +} + +const userService = new UserService(); +const dataProcessor = new DataProcessor(); + +export { UserService, DataProcessor, userService, dataProcessor }; From 2e30af0f7103f55b3fa95989c6eed535282b346b Mon Sep 17 00:00:00 2001 From: ikjeong Date: Thu, 27 Nov 2025 08:19:08 +0000 Subject: [PATCH 13/13] test: skip integration tests in short mode --- tests/integration/checkstyle_test.go | 12 ++++++++++++ tests/integration/eslint_test.go | 16 ++++++++++++++++ tests/integration/pmd_test.go | 16 ++++++++++++++++ tests/integration/prettier_test.go | 16 ++++++++++++++++ tests/integration/pylint_test.go | 12 ++++++++++++ tests/integration/rbac_test.go | 8 ++++++++ tests/integration/tsc_test.go | 16 ++++++++++++++++ 7 files changed, 96 insertions(+) diff --git a/tests/integration/checkstyle_test.go b/tests/integration/checkstyle_test.go index dff21cd..5c65ece 100644 --- a/tests/integration/checkstyle_test.go +++ b/tests/integration/checkstyle_test.go @@ -16,6 +16,10 @@ import ( ) func TestCheckstyle_ValidateChanges(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + // 1. Get testdata path testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "checkstyle")) require.NoError(t, err, "Failed to get testdata path") @@ -89,6 +93,10 @@ func TestCheckstyle_ValidateChanges(t *testing.T) { } func TestCheckstyle_NamingRules(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "checkstyle")) require.NoError(t, err) @@ -172,6 +180,10 @@ func TestCheckstyle_NamingRules(t *testing.T) { } func TestCheckstyle_ToolNameAndRuleID(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "checkstyle")) require.NoError(t, err) diff --git a/tests/integration/eslint_test.go b/tests/integration/eslint_test.go index f1447ed..99da796 100644 --- a/tests/integration/eslint_test.go +++ b/tests/integration/eslint_test.go @@ -16,6 +16,10 @@ import ( ) func TestESLint_ValidateChanges(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + // 1. Get testdata path testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "eslint")) require.NoError(t, err, "Failed to get testdata path") @@ -88,6 +92,10 @@ func TestESLint_ValidateChanges(t *testing.T) { } func TestESLint_NamingConventions(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "eslint")) require.NoError(t, err) @@ -141,6 +149,10 @@ func TestESLint_NamingConventions(t *testing.T) { } func TestESLint_MaxLineLength(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "eslint")) require.NoError(t, err) @@ -192,6 +204,10 @@ func TestESLint_MaxLineLength(t *testing.T) { } func TestESLint_ToolNameAndRuleID(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "eslint")) require.NoError(t, err) diff --git a/tests/integration/pmd_test.go b/tests/integration/pmd_test.go index f7036c8..1455c57 100644 --- a/tests/integration/pmd_test.go +++ b/tests/integration/pmd_test.go @@ -16,6 +16,10 @@ import ( ) func TestPMD_ValidateChanges(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + // 1. Get testdata path testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "pmd")) require.NoError(t, err, "Failed to get testdata path") @@ -89,6 +93,10 @@ func TestPMD_ValidateChanges(t *testing.T) { } func TestPMD_EmptyCatchBlock(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "pmd")) require.NoError(t, err) @@ -140,6 +148,10 @@ func TestPMD_EmptyCatchBlock(t *testing.T) { } func TestPMD_UnusedPrivateMethod(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "pmd")) require.NoError(t, err) @@ -191,6 +203,10 @@ func TestPMD_UnusedPrivateMethod(t *testing.T) { } func TestPMD_ToolNameAndRuleID(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "pmd")) require.NoError(t, err) diff --git a/tests/integration/prettier_test.go b/tests/integration/prettier_test.go index 50db457..1be8e66 100644 --- a/tests/integration/prettier_test.go +++ b/tests/integration/prettier_test.go @@ -16,6 +16,10 @@ import ( ) func TestPrettier_ValidateChanges(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + // 1. Get testdata path testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "prettier")) require.NoError(t, err, "Failed to get testdata path") @@ -89,6 +93,10 @@ func TestPrettier_ValidateChanges(t *testing.T) { } func TestPrettier_FormattingCheck(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "prettier")) require.NoError(t, err) @@ -145,6 +153,10 @@ func TestPrettier_FormattingCheck(t *testing.T) { } func TestPrettier_QuotesAndIndent(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "prettier")) require.NoError(t, err) @@ -194,6 +206,10 @@ func TestPrettier_QuotesAndIndent(t *testing.T) { } func TestPrettier_ToolNameAndRuleID(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "prettier")) require.NoError(t, err) diff --git a/tests/integration/pylint_test.go b/tests/integration/pylint_test.go index ed63319..635e31e 100644 --- a/tests/integration/pylint_test.go +++ b/tests/integration/pylint_test.go @@ -16,6 +16,10 @@ import ( ) func TestPylint_ValidateChanges(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + // 1. Get testdata path testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "pylint")) require.NoError(t, err, "Failed to get testdata path") @@ -90,6 +94,10 @@ func TestPylint_ValidateChanges(t *testing.T) { } func TestPylint_NamingConventions(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "pylint")) require.NoError(t, err) @@ -172,6 +180,10 @@ func TestPylint_NamingConventions(t *testing.T) { } func TestPylint_ToolNameAndRuleID(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "pylint")) require.NoError(t, err) diff --git a/tests/integration/rbac_test.go b/tests/integration/rbac_test.go index 16493fe..ebc01ad 100644 --- a/tests/integration/rbac_test.go +++ b/tests/integration/rbac_test.go @@ -8,6 +8,10 @@ import ( // Test complex RBAC scenarios with admin, developer, viewer roles func TestComplexRBACPatterns(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + tests := []struct { name string username string @@ -91,6 +95,10 @@ func TestComplexRBACPatterns(t *testing.T) { // Test RBAC validation result structure func TestValidationResultStructure(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + tests := []struct { name string result *roles.ValidationResult diff --git a/tests/integration/tsc_test.go b/tests/integration/tsc_test.go index 31523c6..d29208f 100644 --- a/tests/integration/tsc_test.go +++ b/tests/integration/tsc_test.go @@ -16,6 +16,10 @@ import ( ) func TestTSC_ValidateChanges(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + // 1. Get testdata path testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "tsc")) require.NoError(t, err, "Failed to get testdata path") @@ -84,6 +88,10 @@ func TestTSC_ValidateChanges(t *testing.T) { } func TestTSC_StrictNullChecks(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "tsc")) require.NoError(t, err) @@ -138,6 +146,10 @@ func TestTSC_StrictNullChecks(t *testing.T) { } func TestTSC_TypeErrors(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "tsc")) require.NoError(t, err) @@ -187,6 +199,10 @@ func TestTSC_TypeErrors(t *testing.T) { } func TestTSC_ToolNameAndRuleID(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + testdataDir, err := filepath.Abs(filepath.Join("..", "..", "tests", "testdata", "tsc")) require.NoError(t, err)