diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b2afde..757a286 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,17 @@ ## [Unreleased] ### Added +- Go language support with 9 slop detection patterns: + - `placeholder_panic_go` - panic("TODO: ...") placeholder + - `go_fmt_debugging` - fmt.Print/Println/Printf debug statements + - `go_log_debugging` - log.Print/Println/Printf debug logging + - `go_spew_debugging` - spew.Dump/Sdump debug output + - `go_empty_error_check` - empty if err != nil {} blocks + - `go_discarded_error` - _ = someFunc() discarding errors + - `go_bare_os_exit` - os.Exit without defer cleanup + - `go_empty_interface_param` - interface{} parameters + - `go_todo_empty_func` - empty function bodies with TODO comments +- golangci-lint integration in Phase 2 pipeline - Java language support with 10 slop detection patterns: - `placeholder_unsupported_java` - throw new UnsupportedOperationException() - `java_sysout_debugging` - System.out/err.println() debug output diff --git a/__tests__/slop-patterns-go.test.js b/__tests__/slop-patterns-go.test.js new file mode 100644 index 0000000..9993a85 --- /dev/null +++ b/__tests__/slop-patterns-go.test.js @@ -0,0 +1,443 @@ +/** + * Tests for Go slop detection patterns + * Covers all 9 Go-specific patterns (1 existing + 8 new) + */ + +const { + slopPatterns, + getPatternsForLanguage, + getPatternsForLanguageOnly, + isFileExcluded, + hasLanguage +} = require('../lib/patterns/slop-patterns'); + +// ============================================================================ +// Integration tests +// ============================================================================ + +describe('Go language integration', () => { + test('hasLanguage("go") returns true', () => { + expect(hasLanguage('go')).toBe(true); + }); + + test('getPatternsForLanguageOnly("go") returns exactly 9 patterns', () => { + const goOnly = getPatternsForLanguageOnly('go'); + expect(Object.keys(goOnly)).toHaveLength(9); + }); + + test('all 9 Go pattern names are present', () => { + const names = Object.keys(getPatternsForLanguageOnly('go')); + expect(names).toContain('placeholder_panic_go'); + expect(names).toContain('go_fmt_debugging'); + expect(names).toContain('go_log_debugging'); + expect(names).toContain('go_spew_debugging'); + expect(names).toContain('go_empty_error_check'); + expect(names).toContain('go_discarded_error'); + expect(names).toContain('go_bare_os_exit'); + expect(names).toContain('go_empty_interface_param'); + expect(names).toContain('go_todo_empty_func'); + }); + + test('getPatternsForLanguage("go") includes universal patterns', () => { + const goAll = getPatternsForLanguage('go'); + const goOnly = getPatternsForLanguageOnly('go'); + expect(Object.keys(goAll).length).toBeGreaterThan(Object.keys(goOnly).length); + }); + + test('all Go patterns have required fields', () => { + for (const [, p] of Object.entries(getPatternsForLanguageOnly('go'))) { + expect(p).toHaveProperty('pattern'); + expect(p).toHaveProperty('exclude'); + expect(p).toHaveProperty('severity'); + expect(p).toHaveProperty('autoFix'); + expect(p).toHaveProperty('language', 'go'); + expect(p).toHaveProperty('description'); + expect(typeof p.description).toBe('string'); + expect(Array.isArray(p.exclude)).toBe(true); + } + }); +}); + +// ============================================================================ +// placeholder_panic_go (existing pattern) +// ============================================================================ + +describe('placeholder_panic_go', () => { + const { pattern, exclude } = slopPatterns.placeholder_panic_go; + + test('matches panic("TODO: implement")', () => { + expect(pattern.test('panic("TODO: implement this")')).toBe(true); + }); + + test('matches panic with "not implemented"', () => { + expect(pattern.test('panic("not implemented yet")')).toBe(true); + }); + + test('does not match panic with normal message', () => { + expect(pattern.test('panic("unexpected state: invalid")')).toBe(false); + }); + + test('excludes test files', () => { + expect(isFileExcluded('handler_test.go', exclude)).toBe(true); + }); + + test('excludes testdata directory', () => { + expect(isFileExcluded('pkg/testdata/fixture.go', exclude)).toBe(true); + }); + + test('does not exclude regular source files', () => { + expect(isFileExcluded('pkg/handler.go', exclude)).toBe(false); + }); +}); + +// ============================================================================ +// go_fmt_debugging +// ============================================================================ + +describe('go_fmt_debugging', () => { + const { pattern, exclude } = slopPatterns.go_fmt_debugging; + + test('matches fmt.Println()', () => { + expect(pattern.test('fmt.Println("debug value:", x)')).toBe(true); + }); + + test('matches fmt.Printf()', () => { + expect(pattern.test('fmt.Printf("value: %v\\n", x)')).toBe(true); + }); + + test('matches fmt.Print()', () => { + expect(pattern.test('fmt.Print("hello")')).toBe(true); + }); + + test('does not match fmt.Sprintf()', () => { + expect(pattern.test('s := fmt.Sprintf("value: %d", x)')).toBe(false); + }); + + test('does not match fmt.Errorf()', () => { + expect(pattern.test('return fmt.Errorf("failed: %w", err)')).toBe(false); + }); + + test('does not match fmt.Fprintf()', () => { + expect(pattern.test('fmt.Fprintf(w, "response")')).toBe(false); + }); + + test('excludes test files', () => { + expect(isFileExcluded('handler_test.go', exclude)).toBe(true); + }); + + test('excludes testdata directory', () => { + expect(isFileExcluded('pkg/testdata/fixture.go', exclude)).toBe(true); + }); + + test('excludes cmd directory', () => { + expect(isFileExcluded('cmd/server/main.go', exclude)).toBe(true); + }); + + test('excludes main.go', () => { + expect(isFileExcluded('main.go', exclude)).toBe(true); + }); + + test('does not exclude regular source files', () => { + expect(isFileExcluded('pkg/handler.go', exclude)).toBe(false); + expect(isFileExcluded('internal/service/worker.go', exclude)).toBe(false); + }); +}); + +// ============================================================================ +// go_log_debugging +// ============================================================================ + +describe('go_log_debugging', () => { + const { pattern, exclude } = slopPatterns.go_log_debugging; + + test('matches log.Println()', () => { + expect(pattern.test('log.Println("starting server")')).toBe(true); + }); + + test('matches log.Printf()', () => { + expect(pattern.test('log.Printf("port: %d", port)')).toBe(true); + }); + + test('matches log.Fatal()', () => { + expect(pattern.test('log.Fatal(err)')).toBe(true); + }); + + test('matches log.Fatalf()', () => { + expect(pattern.test('log.Fatalf("failed: %v", err)')).toBe(true); + }); + + test('matches log.Panicf()', () => { + expect(pattern.test('log.Panicf("critical: %v", err)')).toBe(true); + }); + + test('does not match slog.Info()', () => { + expect(pattern.test('slog.Info("starting server")')).toBe(false); + }); + + test('does not match zap.Logger', () => { + expect(pattern.test('logger.Info("started")')).toBe(false); + }); + + test('severity is low', () => { + expect(slopPatterns.go_log_debugging.severity).toBe('low'); + }); + + test('excludes test files', () => { + expect(isFileExcluded('handler_test.go', exclude)).toBe(true); + }); + + test('excludes cmd directory and main.go', () => { + expect(isFileExcluded('cmd/cli/root.go', exclude)).toBe(true); + expect(isFileExcluded('main.go', exclude)).toBe(true); + }); + + test('does not exclude regular source files', () => { + expect(isFileExcluded('internal/worker.go', exclude)).toBe(false); + }); +}); + +// ============================================================================ +// go_spew_debugging +// ============================================================================ + +describe('go_spew_debugging', () => { + const { pattern, exclude } = slopPatterns.go_spew_debugging; + + test('matches spew.Dump()', () => { + expect(pattern.test('spew.Dump(config)')).toBe(true); + }); + + test('matches spew.Printf()', () => { + expect(pattern.test('spew.Printf("config: %v", c)')).toBe(true); + }); + + test('matches spew.Sdump()', () => { + expect(pattern.test('s := spew.Sdump(obj)')).toBe(true); + }); + + test('matches pp.Println()', () => { + expect(pattern.test('pp.Println(result)')).toBe(true); + }); + + test('matches pp.Print()', () => { + expect(pattern.test('pp.Print(data)')).toBe(true); + }); + + test('does not match spew in comments', () => { + expect(pattern.test('// use spew for debugging')).toBe(false); + }); + + test('excludes test files', () => { + expect(isFileExcluded('handler_test.go', exclude)).toBe(true); + }); + + test('excludes testdata directory', () => { + expect(isFileExcluded('pkg/testdata/debug.go', exclude)).toBe(true); + }); + + test('does not exclude regular source files', () => { + expect(isFileExcluded('internal/service.go', exclude)).toBe(false); + }); +}); + +// ============================================================================ +// go_empty_error_check +// ============================================================================ + +describe('go_empty_error_check', () => { + const { pattern, exclude } = slopPatterns.go_empty_error_check; + + test('matches if err != nil {}', () => { + expect(pattern.test('if err != nil {}')).toBe(true); + }); + + test('matches with extra spacing', () => { + expect(pattern.test('if err != nil { }')).toBe(true); + }); + + test('does not match if err != nil with handler', () => { + expect(pattern.test('if err != nil { return err }')).toBe(false); + }); + + test('does not match if err != nil with log', () => { + expect(pattern.test('if err != nil { log.Fatal(err) }')).toBe(false); + }); + + test('severity is high', () => { + expect(slopPatterns.go_empty_error_check.severity).toBe('high'); + }); + + test('excludes test files', () => { + expect(isFileExcluded('service_test.go', exclude)).toBe(true); + }); + + test('does not exclude regular source files', () => { + expect(isFileExcluded('service.go', exclude)).toBe(false); + }); +}); + +// ============================================================================ +// go_discarded_error +// ============================================================================ + +describe('go_discarded_error', () => { + const { pattern, exclude } = slopPatterns.go_discarded_error; + + test('matches _ = doSomething()', () => { + expect(pattern.test('_ = doSomething()')).toBe(true); + }); + + test('matches _ = os.Remove(path)', () => { + expect(pattern.test('_ = os.Remove(path)')).toBe(true); + }); + + test('matches _ = file.Close()', () => { + expect(pattern.test('_ = file.Close()')).toBe(true); + }); + + test('does not match for _, v := range', () => { + // This is a critical false positive guard - range iteration is not error discarding + expect(pattern.test('for _, v := range items {')).toBe(false); + }); + + test('does not match _, ok := m[key]', () => { + // Map existence check - the _ discards value, not an error + expect(pattern.test('_, ok := m[key]')).toBe(false); + }); + + test('excludes test files', () => { + expect(isFileExcluded('handler_test.go', exclude)).toBe(true); + }); + + test('does not exclude regular source files', () => { + expect(isFileExcluded('handler.go', exclude)).toBe(false); + }); +}); + +// ============================================================================ +// go_bare_os_exit +// ============================================================================ + +describe('go_bare_os_exit', () => { + const { pattern, exclude } = slopPatterns.go_bare_os_exit; + + test('matches os.Exit(1)', () => { + expect(pattern.test('os.Exit(1)')).toBe(true); + }); + + test('matches os.Exit(0)', () => { + expect(pattern.test('os.Exit(0)')).toBe(true); + }); + + test('does not match os.Exitcode', () => { + expect(pattern.test('code := os.Exitcode')).toBe(false); + }); + + test('excludes test files', () => { + expect(isFileExcluded('main_test.go', exclude)).toBe(true); + }); + + test('excludes cmd directory', () => { + expect(isFileExcluded('cmd/server/main.go', exclude)).toBe(true); + }); + + test('excludes main.go', () => { + expect(isFileExcluded('main.go', exclude)).toBe(true); + }); + + test('does not exclude regular source files', () => { + expect(isFileExcluded('internal/shutdown.go', exclude)).toBe(false); + }); +}); + +// ============================================================================ +// go_empty_interface_param +// ============================================================================ + +describe('go_empty_interface_param', () => { + const { pattern, exclude } = slopPatterns.go_empty_interface_param; + + test('matches func with interface{} param', () => { + expect(pattern.test('func Process(data interface{}')).toBe(true); + }); + + test('matches func with mixed params including interface{}', () => { + expect(pattern.test('func Handle(ctx context.Context, data interface{}')).toBe(true); + }); + + test('matches func with variadic interface{} param', () => { + expect(pattern.test('func Log(args ...interface{}')).toBe(true); + }); + + test('does not match func without interface{}', () => { + expect(pattern.test('func Process(data string)')).toBe(false); + }); + + test('does not match interface definition', () => { + expect(pattern.test('type Handler interface {')).toBe(false); + }); + + test('severity is low', () => { + expect(slopPatterns.go_empty_interface_param.severity).toBe('low'); + }); + + test('excludes test files', () => { + expect(isFileExcluded('handler_test.go', exclude)).toBe(true); + }); + + test('does not exclude regular source files', () => { + expect(isFileExcluded('handler.go', exclude)).toBe(false); + }); +}); + +// ============================================================================ +// go_todo_empty_func +// ============================================================================ + +describe('go_todo_empty_func', () => { + const { pattern, exclude } = slopPatterns.go_todo_empty_func; + + test('matches func with TODO comment', () => { + expect(pattern.test('func Process(x int) error { // TODO implement')).toBe(true); + }); + + test('matches func with FIXME comment', () => { + expect(pattern.test('func Handle(r *Request) { // FIXME')).toBe(true); + }); + + test('matches func with HACK comment', () => { + expect(pattern.test('func Validate(s string) bool { // HACK')).toBe(true); + }); + + test('matches func with named return values', () => { + expect(pattern.test('func Process() (result int, err error) { // TODO implement')).toBe(true); + }); + + test('matches func with multiple unnamed returns', () => { + expect(pattern.test('func Get() (int, error) { // TODO')).toBe(true); + }); + + test('does not match func with real body', () => { + expect(pattern.test('func Process(x int) error { return nil }')).toBe(false); + }); + + test('does not match func with TODO in body after code', () => { + expect(pattern.test('func Process(x int) error { x++; // TODO add more')).toBe(false); + }); + + test('severity is high', () => { + expect(slopPatterns.go_todo_empty_func.severity).toBe('high'); + }); + + test('excludes test files', () => { + expect(isFileExcluded('handler_test.go', exclude)).toBe(true); + }); + + test('excludes testdata directory', () => { + expect(isFileExcluded('pkg/testdata/stub.go', exclude)).toBe(true); + }); + + test('does not exclude regular source files', () => { + expect(isFileExcluded('internal/handler.go', exclude)).toBe(false); + }); +}); diff --git a/lib/patterns/cli-enhancers.js b/lib/patterns/cli-enhancers.js index ecf2599..83e09de 100644 --- a/lib/patterns/cli-enhancers.js +++ b/lib/patterns/cli-enhancers.js @@ -571,6 +571,82 @@ function runComplexityAnalysis(repoPath, targetFiles, options = {}) { return results.length > 0 ? results : null; } +/** + * Run Go lint analysis using golangci-lint + * + * @param {string} repoPath - Repository root path + * @param {string[]} targetFiles - Files to analyze (used for filtering results) + * @param {Object} options - Options + * @returns {Array|null} Lint findings, or null if tool not available + */ +function runGolangciLint(repoPath, targetFiles, options = {}) { + if (!isToolAvailable(CLI_TOOLS.golangci_lint.checkCommand)) { + return null; + } + + try { + // Run golangci-lint with JSON output, zero exit code on issues + const args = [ + 'run', + '--out-format=json', + '--issues-exit-code=0', + './...' + ]; + + const result = execFileSync('golangci-lint', args, { + stdio: ['pipe', 'pipe', 'pipe'], + timeout: 120000, + windowsHide: true, + cwd: repoPath, + encoding: 'utf8' + }); + + // Parse JSON output + try { + const report = JSON.parse(result); + const findings = []; + + if (report.Issues) { + for (const issue of report.Issues) { + // Map golangci-lint severity to deslop severity + let severity = 'medium'; + if (issue.Severity === 'error') { + severity = 'high'; + } else if (issue.Severity === 'warning') { + severity = 'medium'; + } else if (issue.Severity === 'info') { + severity = 'low'; + } + + findings.push({ + file: issue.Pos?.Filename || 'unknown', + line: issue.Pos?.Line || 0, + column: issue.Pos?.Column || 0, + message: issue.Text || '', + linter: issue.FromLinter || 'unknown', + severity + }); + } + } + + // Filter to target files when specified + if (targetFiles && targetFiles.length > 0) { + return findings.filter(f => + targetFiles.some(tf => tf.endsWith(f.file) || f.file.endsWith(tf)) + ); + } + + return findings; + } catch { + // JSON parsing failed - invalid output from tool + return []; + } + } catch { + // Tool execution failed or not available + return null; + } +} + /** * Get user-friendly message about missing tools (language-aware) * @@ -636,6 +712,7 @@ module.exports = { runDuplicateDetection, runDependencyAnalysis, runComplexityAnalysis, + runGolangciLint, getMissingToolsMessage, getToolDefinitions, getSupportedLanguages, diff --git a/lib/patterns/pipeline.js b/lib/patterns/pipeline.js index 70cb079..5eb87da 100644 --- a/lib/patterns/pipeline.js +++ b/lib/patterns/pipeline.js @@ -752,6 +752,27 @@ function runPhase2(repoPath, cliTools, targetFiles) { } } + // Go lint analysis with golangci-lint + if (cliTools.golangci_lint) { + const golangciResults = cliEnhancers.runGolangciLint(repoPath, targetFiles); + if (golangciResults) { + for (const result of golangciResults) { + findings.push({ + file: result.file, + line: result.line || 0, + patternName: 'golangci_lint_issue', + severity: result.severity || 'medium', + certainty: CERTAINTY.LOW, + description: `golangci-lint (${result.linter}): ${result.message}`, + autoFix: 'flag', + content: result.message.substring(0, 100), + phase: 2, + details: result + }); + } + } + } + return findings; } diff --git a/lib/patterns/slop-patterns.js b/lib/patterns/slop-patterns.js index fe120fa..385583a 100644 --- a/lib/patterns/slop-patterns.js +++ b/lib/patterns/slop-patterns.js @@ -574,6 +574,115 @@ const slopPatterns = { description: 'Go panic("TODO: ...") placeholder' }, + // ============================================================================ + // Go patterns - debugging, error handling, and code quality + // Discovered via false negative hunting against real-world Go codebases + // ============================================================================ + + /** + * Go: fmt.Print/Println/Printf debugging + * Debug print statements left in production code - use a logging package instead + */ + go_fmt_debugging: { + pattern: /\bfmt\.Print(?:ln|f)?\s*\(/, + exclude: ['*_test.go', '**/testdata/**', 'cmd/**', '**/cmd/**', 'main.go'], + severity: 'medium', + autoFix: 'remove', + language: 'go', + description: 'Go fmt.Print() debug output in production code - use a logging package' + }, + + /** + * Go: log.Print/Fatal/Panic debugging + * Standard library log calls often indicate leftover debugging + */ + go_log_debugging: { + pattern: /\blog\.(?:Print|Fatal|Panic)(?:ln|f)?\s*\(/, + exclude: ['*_test.go', '**/testdata/**', 'cmd/**', '**/cmd/**', 'main.go'], + severity: 'low', + autoFix: 'flag', + language: 'go', + description: 'Go log.Print/Fatal/Panic() - consider structured logging' + }, + + /** + * Go: spew/pp debug dumping + * Third-party debug dump libraries left in production code + */ + go_spew_debugging: { + pattern: /\b(?:spew\.(?:Dump|Printf|Sdump)|pp\.Print(?:ln)?)\s*\(/, + exclude: ['*_test.go', '**/testdata/**'], + severity: 'medium', + autoFix: 'remove', + language: 'go', + description: 'Go spew/pp debug dumping left in production code' + }, + + /** + * Go: empty error check (if err != nil {}) + * Silently swallowing errors is a common AI slop pattern + */ + go_empty_error_check: { + pattern: /if\s+err\s*!=\s*nil\s*\{\s*\}/, + exclude: ['*_test.go', '**/testdata/**'], + severity: 'high', + autoFix: 'flag', + language: 'go', + description: 'Go empty error check - silently swallowing errors' + }, + + /** + * Go: discarded error via blank identifier + * Assigning function return to _ discards potential errors + */ + go_discarded_error: { + pattern: /\b_\s*=\s*\w+(?:\.\w+){0,10}\s*\(/, + exclude: ['*_test.go', '**/testdata/**'], + severity: 'medium', + autoFix: 'flag', + language: 'go', + description: 'Go discarded error via blank identifier' + }, + + /** + * Go: bare os.Exit() in non-main code + * os.Exit() skips deferred functions and should only be in main() + */ + go_bare_os_exit: { + pattern: /\bos\.Exit\s*\(/, + exclude: ['*_test.go', '**/testdata/**', 'cmd/**', '**/cmd/**', 'main.go'], + severity: 'medium', + autoFix: 'flag', + language: 'go', + description: 'Go os.Exit() in non-main code - skips deferred functions' + }, + + /** + * Go: empty interface{} parameter + * Prefer specific types or any (Go 1.18+) over interface{} + */ + go_empty_interface_param: { + pattern: /func\s+\w+\s*\([^)]{0,200}\binterface\s*\{\s*\}/, + exclude: ['*_test.go', '**/testdata/**'], + severity: 'low', + autoFix: 'flag', + language: 'go', + description: 'Go empty interface{} parameter - prefer specific types or any' + }, + + /** + * Go: TODO/FIXME/HACK comment inside empty function body + * Function with only a TODO comment indicates unfinished implementation + */ + go_todo_empty_func: { + pattern: /func\s+\w+\s*\([^)]{0,200}\)\s*(?:\([^)]{0,100}\)|\S+)?\s*\{\s*\/\/\s*(?:TODO|FIXME|HACK)/, + exclude: ['*_test.go', '**/testdata/**'], + severity: 'high', + autoFix: 'flag', + language: 'go', + description: 'Go function with only TODO/FIXME/HACK comment - unfinished implementation' + }, + /** * Java: throw new UnsupportedOperationException() */ diff --git a/references/slop-categories.md b/references/slop-categories.md index 796dd28..3f1623c 100644 --- a/references/slop-categories.md +++ b/references/slop-categories.md @@ -163,6 +163,20 @@ C patterns (prefix `c_`) apply to both C and C++ files. C++ patterns (prefix `cp | kotlin_swallowed_error | runCatching{}.getOrNull() silently swallows errors | medium | | kotlin_suppress_annotation | @Suppress("UNCHECKED_CAST") | low | +### Go + +| Pattern | Description | Severity | +|---------|-------------|----------| +| placeholder_panic_go | panic("TODO: ...") placeholder | high | +| go_fmt_debugging | fmt.Print/Println/Printf debug statements | medium | +| go_log_debugging | log.Print/Println/Printf debug logging | low | +| go_spew_debugging | spew.Dump/Sdump debug output | medium | +| go_empty_error_check | Empty if err != nil {} blocks | high | +| go_discarded_error | _ = someFunc() discarding errors | medium | +| go_bare_os_exit | os.Exit without defer cleanup | medium | +| go_empty_interface_param | interface{} parameters - prefer specific types | low | +| go_todo_empty_func | Empty function bodies with TODO comments | high | + ### Shell/Bash | Pattern | Description | Severity |