diff --git a/CHANGELOG.md b/CHANGELOG.md index 53cb74a..ea5be6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ ## Unreleased +- Parse `.gitignore`, `.ignore`, and `.git/info/exclude` during working-tree scans. +- Add `--exclude-tests` to skip well-known test-case paths from aggregate scan analysis. +- Replace the linear score deduction with a bounded issue-density curve so noisy large repositories do not collapse to zero. +- Add aggregate code/blank/comment-only/inline-comment line statistics to scan and diff reports, and use code lines for issue-density scoring when available. +- Add explicit scan and diff status, regression, threshold, and incomplete-report metadata. +- Count files skipped due to size or per-file analysis budget, and mark affected reports incomplete. +- Stream blob analysis so scans retain aggregate facts instead of repository-wide raw blob contents. + ## 0.0.1 - 2026-06-30 - Initial public code-signal scanner. diff --git a/README.md b/README.md index c39e8f5..5876553 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,9 @@ scanner scan . # Emit aggregate JSON scanner scan . --json +# Exclude well-known test files and test directories +scanner scan . --exclude-tests + # Detect languages and module roots scanner detect . @@ -92,6 +95,22 @@ code-signal detects and analyzes these languages with built-in lexical/parser ch Detection uses extensions, shebangs, and common manifests such as `go.mod`, `package.json`, `pyproject.toml`, `Cargo.toml`, `pom.xml`, Gradle files, `.csproj`, `.sln`, `composer.json`, `Gemfile`, `Dockerfile`, `Containerfile`, and `*.dockerfile`. +## Aggregate line statistics + +Scan JSON includes `lines_scanned`, `blank_lines`, `code_lines`, `comment_lines`, `comment_only_lines`, `inline_comment_lines`, and `comment_density_percent`. The same aggregate fields are reported for `totals`, `by_language`, and `by_module`; diff JSON includes before/after/delta values under `totals_delta`. + +`lines_scanned` is physical scanned lines. `code_lines` excludes blank lines and comment-only lines, but keeps lines that contain code plus an inline comment. The score model uses `code_lines` when available and falls back to physical lines for older reports. + +Comment detection is lexical and language-aware for supported languages: it recognizes `//`, `#`, `/* ... */`, Rust nested block comments, Ruby `=begin`/`=end`, and standalone Python triple-quoted doc/comment blocks where those forms are valid. It ignores common markers inside quoted strings/raw strings/template literals and JavaScript/TypeScript regex literals, and never emits per-file locations or source snippets. + +## Ignore files, test exclusion, and skip policy + +Working-tree scans parse `.gitignore`, `.ignore`, and `.git/info/exclude` using gitignore-style rules before reading candidate files. + +Pass `--exclude-tests` to `scan` or `diff` to skip well-known test-case paths from aggregate analysis. The built-in defaults cover common Go (`*_test.go`, `testdata`), Java (`src/test/**`, `*Test.java`, `*Tests.java`, `*IT.java`), Python (`tests/**`, `test_*.py`, `*_test.py`, `conftest.py`), TypeScript/JavaScript (`__tests__/**`, `*.test.*`, `*.spec.*`, `cypress/**`, `playwright/**`, `e2e/**`), and Rust (`tests/**`, `benches/**`, `*_test.rs`) conventions. + +Built-in skips still apply for common dependency, generated, build, coverage, and minified paths such as `node_modules`, `vendor`, `target`, `dist`, `build`, `.next`, `coverage`, `third_party`, `generated`, `gen`, `*.min.js`, `*.min.css`, `*.pb.go`, and `*_generated.go`. + ## Configuration Configuration is optional. If present, `scanner.json` is loaded from the scanned repository root or from `--config`. @@ -99,10 +118,12 @@ Configuration is optional. If present, `scanner.json` is loaded from the scanned ```json { "scan": { - "default_timeout_seconds": 30, + "default_timeout_seconds": 0, + "max_file_analysis_ms": 2000, "max_file_bytes": 1048576, "follow_symlinks": false, - "workers": 0 + "workers": 0, + "exclude_tests": false }, "score": { "fail_under": 75, @@ -120,6 +141,26 @@ Configuration is optional. If present, `scanner.json` is loaded from the scanned Unknown config fields and trailing JSON values are rejected. +`scan.default_timeout_seconds` is disabled by default; use `--timeout` or set a +positive value when a whole-command emergency brake is needed. Files larger than +`scan.max_file_bytes` are skipped before content is read. Files whose built-in +analysis exceeds `scan.max_file_analysis_ms` are skipped after the bounded +attempt. These are included in `totals.files_skipped_due_to_size` or +`totals.files_skipped_due_to_timeout`. When this happens, scan and diff reports +set `incomplete: true`, so the score is clearly a quick optimistic signal over +the scanned subset. + +## Score model + +The score is a bounded weighted issue-density signal, not a SonarQube debt rating: + +```text +weighted_density_per_kloc = round((errors*10 + warnings*4 + info*1) * 1000 / max(code_lines, 1000)) +score = round(100 * 100 / (100 + weighted_density_per_kloc)) +``` + +Error caps still apply after density scoring: any error caps the score at `89`, and five or more errors cap it at `69`. Bands are `excellent` for `90+`, `good` for `75-89`, `needs work` for `50-74`, and `poor` below `50`. + ## Privacy and output contract Reports are aggregate-only. Facts and report JSON do not include file paths, line/column coordinates, snippets, clone groups, per-file findings, or source text. The scanner reads local files and local git objects only; it performs no telemetry, update checks, remote rule downloads, package installs, or network calls. diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 933acbf..b6012fa 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -23,6 +23,7 @@ const ( flagBase = "--base" flagConfig = "--config" + flagExcludeTests = "--exclude-tests" flagFailOnRegression = "--fail-on-regression" flagFailUnder = "--fail-under" flagHead = "--head" @@ -89,17 +90,19 @@ type diffArgs struct { configPath string json bool failOnRegression bool + excludeTests bool timeout time.Duration timeoutSet bool } type scanArgs struct { - path string - configPath string - json bool - failUnder *int - timeout time.Duration - timeoutSet bool + path string + configPath string + json bool + failUnder *int + excludeTests bool + timeout time.Duration + timeoutSet bool } type detectArgs struct { @@ -119,7 +122,7 @@ func runDiff(ctx context.Context, args []string, stdout io.Writer) (int, error) } if parsed.timeoutSet { var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(ctx, parsed.timeout) + ctx, cancel = commandContext(ctx, config.Config{}, parsed.timeout, true) defer cancel() } if err := resolveDiffRepoAndScope(ctx, &parsed); err != nil { @@ -129,6 +132,17 @@ func runDiff(ctx context.Context, args []string, stdout io.Writer) (int, error) if err != nil { return 2, err } + if parsed.excludeTests { + cfg.Scan.ExcludeTests = true + } + if parsed.failOnRegression { + cfg.Diff.FailOnScoreDrop = true + } + if !parsed.timeoutSet { + var cancel context.CancelFunc + ctx, cancel = commandContext(ctx, cfg, 0, false) + defer cancel() + } diff, err := scan.Diff(ctx, parsed.repo, parsed.base, parsed.head, parsed.scopePath, scan.DiffOptions{Config: cfg}) if err != nil { return 1, err @@ -151,15 +165,19 @@ func runScan(ctx context.Context, args []string, stdout io.Writer) (int, error) if err != nil { return 2, err } - if parsed.timeoutSet { - var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(ctx, parsed.timeout) - defer cancel() - } cfg, err := loadConfig(parsed.configPath, parsed.path) if err != nil { return 2, err } + if parsed.excludeTests { + cfg.Scan.ExcludeTests = true + } + if parsed.failUnder != nil { + cfg.Score.FailUnder = *parsed.failUnder + } + var cancel context.CancelFunc + ctx, cancel = commandContext(ctx, cfg, parsed.timeout, parsed.timeoutSet) + defer cancel() scanReport, err := scan.ScanPath(ctx, parsed.path, scan.ScanOptions{Config: cfg}) if err != nil { return 1, err @@ -186,6 +204,8 @@ func runDetect(ctx context.Context, args []string, stdout io.Writer) (int, error if err != nil { return 2, err } + ctx, cancel := commandContext(ctx, cfg, 0, false) + defer cancel() detected, err := scan.DetectPath(ctx, parsed.path, scan.DetectOptions{Config: cfg}) if err != nil { return 1, err @@ -260,6 +280,9 @@ func parseDiffFlag(args []string, index int, parsed *diffArgs) (int, bool, error case flagConfig: next, err := assignFlagValue(args, index, flagConfig, &parsed.configPath) return next, true, err + case flagExcludeTests: + parsed.excludeTests = true + return index, true, nil case flagTimeout: next, err := assignTimeoutValue(args, index, &parsed.timeout, &parsed.timeoutSet) return next, true, err @@ -303,6 +326,9 @@ func parseScanFlag(args []string, index int, parsed *scanArgs) (int, bool, error case flagConfig: next, err := assignFlagValue(args, index, flagConfig, &parsed.configPath) return next, true, err + case flagExcludeTests: + parsed.excludeTests = true + return index, true, nil case flagTimeout: next, err := assignTimeoutValue(args, index, &parsed.timeout, &parsed.timeoutSet) return next, true, err @@ -417,6 +443,16 @@ func parseTimeout(value string) (time.Duration, error) { return time.Duration(seconds) * time.Second, nil } +func commandContext(ctx context.Context, cfg config.Config, timeout time.Duration, timeoutSet bool) (context.Context, context.CancelFunc) { + if timeoutSet { + return context.WithTimeout(ctx, timeout) + } + if cfg.Scan.DefaultTimeoutSeconds <= 0 { + return context.WithCancel(ctx) + } + return context.WithTimeout(ctx, time.Duration(cfg.Scan.DefaultTimeoutSeconds)*time.Second) +} + func appendPositional(command string, positionals []string, arg string) ([]string, error) { if strings.HasPrefix(arg, "-") { return positionals, fmt.Errorf("unknown %s flag %q", command, arg) @@ -539,10 +575,17 @@ func writeString(stdout io.Writer, value string) error { func printScan(stdout io.Writer, scanReport model.ScanReport) error { var builder strings.Builder fmt.Fprintf(&builder, "Score: %d (%s)\n", scanReport.Score, scanReport.Band) + fmt.Fprintf(&builder, "Status: %s (score %d, fail-under %d)\n", scanReport.Status, scanReport.Score, scanReport.Thresholds.FailUnder) + if scanReport.Incomplete { + fmt.Fprintf(&builder, "Incomplete: true (%s)\n", strings.Join(scanReport.IncompleteReasons, ", ")) + } fmt.Fprintf(&builder, "Issues: %d (errors: %d, warnings: %d, info: %d)\n", scanReport.Totals.Issues, scanReport.Totals.Errors, scanReport.Totals.Warnings, scanReport.Totals.Info) - fmt.Fprintf(&builder, "Files scanned: %d | Lines scanned: %d | Files skipped: %d\n", - scanReport.Totals.FilesScanned, scanReport.Totals.LinesScanned, scanReport.Totals.FilesSkipped) + fmt.Fprintf(&builder, "Files scanned: %d | Lines scanned: %d | Code lines: %d | Files skipped: %d\n", + scanReport.Totals.FilesScanned, scanReport.Totals.LinesScanned, scanReport.Totals.CodeLines, scanReport.Totals.FilesSkipped) + fmt.Fprintf(&builder, "Comments: %d lines (%.1f%%) | Comment-only: %d | Inline: %d | Blank: %d\n", + scanReport.Totals.CommentLines, scanReport.Totals.CommentDensityPercent, + scanReport.Totals.CommentOnlyLines, scanReport.Totals.InlineCommentLines, scanReport.Totals.BlankLines) fmt.Fprintf(&builder, "Languages: %s | Modules: %d (%s)\n", joinOrNone(scanLanguages(scanReport)), scanModuleCount(scanReport), moduleProfileText(scanReport)) printDuplicationSummary(&builder, scanReport.Duplication) diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index 45801d0..8edd32a 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -3,11 +3,15 @@ package cli import ( "bytes" "context" + "encoding/json" "errors" "os" "path/filepath" "strings" "testing" + "time" + + "github.com/randomcodespace/code-signal/internal/config" ) func TestUnknownCommandExitsTwo(t *testing.T) { @@ -71,6 +75,93 @@ func TestScanCommandKeepsZeroExitForLowScore(t *testing.T) { if !strings.Contains(stdout.String(), "Score:") { t.Fatalf("stdout = %q, want score output", stdout.String()) } + if !strings.Contains(stdout.String(), "Status: failed") { + t.Fatalf("stdout = %q, want failed status output", stdout.String()) + } +} + +func TestScanCommandPrintsIncompleteWhenFileSkippedDueToSize(t *testing.T) { + root := t.TempDir() + writeCLITestFile(t, root, "scanner.json", `{"scan":{"max_file_bytes":32}}`) + writeCLITestFile(t, root, "small.go", "package main\n") + writeCLITestFile(t, root, "large.go", "package main\n"+strings.Repeat("x", 128)+"\n") + + var stdout, stderr bytes.Buffer + code := Run(context.Background(), []string{"scan", root}, &stdout, &stderr) + if code != 0 { + t.Fatalf("code = %d stdout = %q stderr = %q, want 0", code, stdout.String(), stderr.String()) + } + if !strings.Contains(stdout.String(), "Incomplete: true") { + t.Fatalf("stdout = %q, want incomplete marker", stdout.String()) + } + if !strings.Contains(stdout.String(), "files_skipped_due_to_size") { + t.Fatalf("stdout = %q, want size skip reason", stdout.String()) + } +} + +func TestScanCommandJSONAppliesFailUnderToStatus(t *testing.T) { + root := t.TempDir() + writeCLITestFile(t, root, "main.go", "package main\n// "+strings.Repeat("x", 200)+"\n") + + var stdout, stderr bytes.Buffer + code := Run(context.Background(), []string{"scan", "--json", "--fail-under", "100", root}, &stdout, &stderr) + if code != 0 { + t.Fatalf("code = %d stdout = %q stderr = %q, want 0 for completed scan regardless of score", code, stdout.String(), stderr.String()) + } + var decoded struct { + Status string `json:"status"` + Passed bool `json:"passed"` + Thresholds struct { + FailUnder int `json:"fail_under"` + } `json:"thresholds"` + } + if err := json.Unmarshal(stdout.Bytes(), &decoded); err != nil { + t.Fatalf("unmarshal scan JSON: %v\n%s", err, stdout.String()) + } + if decoded.Status != "failed" || decoded.Passed { + t.Fatalf("state = status %q passed %t, want failed", decoded.Status, decoded.Passed) + } + if decoded.Thresholds.FailUnder != 100 { + t.Fatalf("fail_under = %d, want CLI override", decoded.Thresholds.FailUnder) + } +} +func TestScanCommandExcludeTestsUsesDefaultPatterns(t *testing.T) { + root := t.TempDir() + writeCLITestFile(t, root, "cmd/main.go", "package main\n") + longLine := "// " + strings.Repeat("x", 140) + "\n" + for _, name := range []string{ + "cmd/main_test.go", + "testdata/golden.go", + "src/test/java/AppTest.java", + "tests/test_app.py", + "web/src/app.spec.ts", + "web/src/__tests__/app.test.js", + "crates/core/tests/integration.rs", + } { + writeCLITestFile(t, root, name, longLine) + } + + var stdout, stderr bytes.Buffer + code := Run(context.Background(), []string{"scan", "--exclude-tests", "--json", root}, &stdout, &stderr) + if code != 0 { + t.Fatalf("code = %d stdout = %q stderr = %q, want 0", code, stdout.String(), stderr.String()) + } + var decoded struct { + Totals struct { + FilesScanned int `json:"files_scanned"` + FilesSkipped int `json:"files_skipped"` + Warnings int `json:"warnings"` + } `json:"totals"` + } + if err := json.Unmarshal(stdout.Bytes(), &decoded); err != nil { + t.Fatalf("unmarshal scan JSON: %v\n%s", err, stdout.String()) + } + if decoded.Totals.FilesScanned != 1 || decoded.Totals.FilesSkipped != 7 { + t.Fatalf("totals = %#v, want one source file scanned and seven test files skipped", decoded.Totals) + } + if decoded.Totals.Warnings != 0 { + t.Fatalf("warnings = %d, want ignored test long lines excluded", decoded.Totals.Warnings) + } } func TestUnknownCommandReturnsCLIError(t *testing.T) { @@ -134,6 +225,49 @@ func TestRunReturnsOneForRuntimeInputErrors(t *testing.T) { } } +func TestCommandContextUsesConfigDefaultTimeoutAndFlagOverride(t *testing.T) { + cfg := config.Default() + + ctx, cancel := commandContext(context.Background(), cfg, 0, false) + defer cancel() + if _, ok := ctx.Deadline(); ok { + t.Fatal("commandContext() deadline present, want no scanner-imposed command timeout by default") + } + + cfg.Scan.DefaultTimeoutSeconds = 7 + + ctx, cancel = commandContext(context.Background(), cfg, 0, false) + defer cancel() + deadline, ok := ctx.Deadline() + if !ok { + t.Fatal("commandContext() deadline missing, want config default timeout") + } + if remaining := time.Until(deadline); remaining <= 6*time.Second || remaining > 7*time.Second { + t.Fatalf("default timeout remaining = %v, want about 7s", remaining) + } + + ctx, cancel = commandContext(context.Background(), cfg, 2*time.Second, true) + defer cancel() + deadline, ok = ctx.Deadline() + if !ok { + t.Fatal("commandContext() deadline missing, want CLI timeout") + } + if remaining := time.Until(deadline); remaining <= time.Second || remaining > 2*time.Second { + t.Fatalf("flag timeout remaining = %v, want about 2s", remaining) + } +} + +func writeCLITestFile(t *testing.T, root, name, content string) { + t.Helper() + path := filepath.Join(root, filepath.FromSlash(name)) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatalf("mkdir %s: %v", filepath.Dir(path), err) + } + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatalf("write %s: %v", name, err) + } +} + var errFailingWrite = errors.New("write failed") type failingWriter struct{} diff --git a/internal/cli/task7_cli_test.go b/internal/cli/task7_cli_test.go index 4eabfb8..f804c49 100644 --- a/internal/cli/task7_cli_test.go +++ b/internal/cli/task7_cli_test.go @@ -60,11 +60,14 @@ func TestDiffCommandJSONOutputIsAggregateOnly(t *testing.T) { if err := json.Unmarshal(stdout.Bytes(), &decoded); err != nil { t.Fatalf("unmarshal diff JSON: %v\n%s", err, stdout.String()) } - for _, key := range []string{"score_before", "score_after", "score_delta", "totals_delta", "severity_delta", "category_delta", "language_delta", "module_delta"} { + for _, key := range []string{"status", "passed", "regression", "thresholds", "score_before", "score_after", "score_delta", "totals_delta", "severity_delta", "category_delta", "language_delta", "module_delta"} { if _, ok := decoded[key]; !ok { t.Fatalf("diff JSON missing %q: %s", key, stdout.String()) } } + if decoded["status"] != "failed" || decoded["passed"] != false || decoded["regression"] != true { + t.Fatalf("diff state = status %#v passed %#v regression %#v, want failed regression", decoded["status"], decoded["passed"], decoded["regression"]) + } forbidden := map[string]bool{"findings": true, "files": true, "path": true, "line": true, "column": true, "snippet": true, "new": true, "resolved": true} if key, ok := containsForbiddenJSONKey(decoded, forbidden); ok { t.Fatalf("diff JSON contains forbidden file-level key %q: %s", key, stdout.String()) diff --git a/internal/config/config.go b/internal/config/config.go index 48cb297..2dc91ef 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -18,10 +18,12 @@ type Config struct { // ScanConfig controls bounded local scan behavior. type ScanConfig struct { - DefaultTimeoutSeconds int `json:"default_timeout_seconds"` - MaxFileBytes int64 `json:"max_file_bytes"` - FollowSymlinks bool `json:"follow_symlinks"` - Workers int `json:"workers"` + DefaultTimeoutSeconds int `json:"default_timeout_seconds"` + MaxFileAnalysisMilliseconds int `json:"max_file_analysis_ms"` + MaxFileBytes int64 `json:"max_file_bytes"` + FollowSymlinks bool `json:"follow_symlinks"` + Workers int `json:"workers"` + ExcludeTests bool `json:"exclude_tests"` } // ScoreConfig controls aggregate score deductions. @@ -46,10 +48,12 @@ type configFile struct { } type scanFile struct { - DefaultTimeoutSeconds *int `json:"default_timeout_seconds"` - MaxFileBytes *int64 `json:"max_file_bytes"` - FollowSymlinks *bool `json:"follow_symlinks"` - Workers *int `json:"workers"` + DefaultTimeoutSeconds *int `json:"default_timeout_seconds"` + MaxFileAnalysisMilliseconds *int `json:"max_file_analysis_ms"` + MaxFileBytes *int64 `json:"max_file_bytes"` + FollowSymlinks *bool `json:"follow_symlinks"` + Workers *int `json:"workers"` + ExcludeTests *bool `json:"exclude_tests"` } type scoreFile struct { @@ -69,6 +73,9 @@ type diffFile struct { func Load(path string) (Config, error) { cfg := Default() if path == "" { + if err := validateConfig(cfg); err != nil { + return Config{}, err + } return cfg, nil } @@ -92,6 +99,9 @@ func Load(path string) (Config, error) { return Config{}, err } mergeConfig(&cfg, file) + if err := validateConfig(cfg); err != nil { + return Config{}, fmt.Errorf("validate %s: %w", configPath, err) + } return cfg, nil } @@ -110,10 +120,11 @@ func rejectTrailingJSON(decoder *json.Decoder, configPath string) error { func Default() Config { return Config{ Scan: ScanConfig{ - DefaultTimeoutSeconds: 30, - MaxFileBytes: 1_048_576, - FollowSymlinks: false, - Workers: 0, + DefaultTimeoutSeconds: 0, + MaxFileAnalysisMilliseconds: 2_000, + MaxFileBytes: 1_048_576, + FollowSymlinks: false, + Workers: 0, }, Score: ScoreConfig{ FailUnder: 75, @@ -142,6 +153,9 @@ func mergeScanConfig(cfg *ScanConfig, file *scanFile) { if file.DefaultTimeoutSeconds != nil { cfg.DefaultTimeoutSeconds = *file.DefaultTimeoutSeconds } + if file.MaxFileAnalysisMilliseconds != nil { + cfg.MaxFileAnalysisMilliseconds = *file.MaxFileAnalysisMilliseconds + } if file.MaxFileBytes != nil { cfg.MaxFileBytes = *file.MaxFileBytes } @@ -151,6 +165,9 @@ func mergeScanConfig(cfg *ScanConfig, file *scanFile) { if file.Workers != nil { cfg.Workers = *file.Workers } + if file.ExcludeTests != nil { + cfg.ExcludeTests = *file.ExcludeTests + } } func mergeScoreConfig(cfg *ScoreConfig, file *scoreFile) { @@ -185,3 +202,30 @@ func mergeDiffConfig(cfg *DiffConfig, file *diffFile) { cfg.CategoryRegressionThreshold = *file.CategoryRegressionThreshold } } + +func validateConfig(cfg Config) error { + switch { + case cfg.Scan.DefaultTimeoutSeconds < 0: + return fmt.Errorf("scan.default_timeout_seconds must be non-negative") + case cfg.Scan.MaxFileAnalysisMilliseconds < 0: + return fmt.Errorf("scan.max_file_analysis_ms must be non-negative") + case cfg.Scan.MaxFileBytes <= 0: + return fmt.Errorf("scan.max_file_bytes must be positive") + case cfg.Scan.Workers < 0: + return fmt.Errorf("scan.workers must be non-negative") + case cfg.Score.FailUnder < 0 || cfg.Score.FailUnder > 100: + return fmt.Errorf("score.fail_under must be between 0 and 100") + case cfg.Score.Error <= 0: + return fmt.Errorf("score.error must be positive") + case cfg.Score.Warning <= 0: + return fmt.Errorf("score.warning must be positive") + case cfg.Score.Info <= 0: + return fmt.Errorf("score.info must be positive") + case cfg.Diff.MaxAllowedNewErrors < 0: + return fmt.Errorf("diff.max_allowed_new_errors must be non-negative") + case cfg.Diff.CategoryRegressionThreshold <= 0: + return fmt.Errorf("diff.category_regression_threshold must be positive") + default: + return nil + } +} diff --git a/internal/config/config_test.go b/internal/config/config_test.go index f3b6adc..08f2794 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -31,6 +31,12 @@ func TestDefaultConfigContainsOnlyBuiltInSections(t *testing.T) { if len(decoded) != 0 { t.Fatalf("default config JSON = %s, want only built-in scan/score/diff sections", encoded) } + if cfg.Scan.DefaultTimeoutSeconds != 0 { + t.Fatalf("DefaultTimeoutSeconds = %d, want 0 disabled by default", cfg.Scan.DefaultTimeoutSeconds) + } + if cfg.Scan.MaxFileAnalysisMilliseconds <= 0 { + t.Fatalf("MaxFileAnalysisMilliseconds = %d, want positive per-file budget", cfg.Scan.MaxFileAnalysisMilliseconds) + } } func TestLoadMergesScannerJSONScanAndScoreOverrides(t *testing.T) { @@ -39,9 +45,11 @@ func TestLoadMergesScannerJSONScanAndScoreOverrides(t *testing.T) { json := `{ "scan": { "default_timeout_seconds": 7, + "max_file_analysis_ms": 17, "max_file_bytes": 2048, "follow_symlinks": true, - "workers": 3 + "workers": 3, + "exclude_tests": true }, "score": { "fail_under": 91, @@ -61,6 +69,9 @@ func TestLoadMergesScannerJSONScanAndScoreOverrides(t *testing.T) { if cfg.Scan.DefaultTimeoutSeconds != 7 { t.Fatalf("DefaultTimeoutSeconds = %d, want 7", cfg.Scan.DefaultTimeoutSeconds) } + if cfg.Scan.MaxFileAnalysisMilliseconds != 17 { + t.Fatalf("MaxFileAnalysisMilliseconds = %d, want 17", cfg.Scan.MaxFileAnalysisMilliseconds) + } if cfg.Scan.MaxFileBytes != 2048 { t.Fatalf("MaxFileBytes = %d, want 2048", cfg.Scan.MaxFileBytes) } @@ -70,6 +81,9 @@ func TestLoadMergesScannerJSONScanAndScoreOverrides(t *testing.T) { if cfg.Scan.Workers != 3 { t.Fatalf("Workers = %d, want 3", cfg.Scan.Workers) } + if !cfg.Scan.ExcludeTests { + t.Fatalf("ExcludeTests = false, want true") + } if cfg.Score.FailUnder != 91 || cfg.Score.Error != 15 || cfg.Score.Warning != 5 || cfg.Score.Info != 2 { t.Fatalf("Score = %#v, want overridden values", cfg.Score) } @@ -118,3 +132,36 @@ func TestLoadRejectsTrailingJSONValues(t *testing.T) { t.Fatalf("Load(scanner.json) error = %v, want trailing JSON error", err) } } + +func TestLoadRejectsInvalidNumericValues(t *testing.T) { + tests := map[string]struct { + raw string + wantField string + }{ + "timeout_negative": {raw: `{"scan":{"default_timeout_seconds":-1}}`, wantField: "scan.default_timeout_seconds"}, + "file_timeout_negative": {raw: `{"scan":{"max_file_analysis_ms":-1}}`, wantField: "scan.max_file_analysis_ms"}, + "max_bytes_zero": {raw: `{"scan":{"max_file_bytes":0}}`, wantField: "scan.max_file_bytes"}, + "negative_workers": {raw: `{"scan":{"workers":-1}}`, wantField: "scan.workers"}, + "fail_under_high": {raw: `{"score":{"fail_under":101}}`, wantField: "score.fail_under"}, + "score_weight_zero": {raw: `{"score":{"error":0}}`, wantField: "score.error"}, + "new_errors_low": {raw: `{"diff":{"max_allowed_new_errors":-1}}`, wantField: "diff.max_allowed_new_errors"}, + "category_zero": {raw: `{"diff":{"category_regression_threshold":0}}`, wantField: "diff.category_regression_threshold"}, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "scanner.json") + if err := os.WriteFile(path, []byte(tt.raw), 0o600); err != nil { + t.Fatalf("write scanner.json: %v", err) + } + + _, err := Load(path) + if err == nil { + t.Fatal("Load(scanner.json) error = nil, want validation error") + } + if !strings.Contains(err.Error(), tt.wantField) { + t.Fatalf("Load(scanner.json) error = %v, want field %q", err, tt.wantField) + } + }) + } +} diff --git a/internal/delta/delta.go b/internal/delta/delta.go index 4b5d34c..241eeeb 100644 --- a/internal/delta/delta.go +++ b/internal/delta/delta.go @@ -10,31 +10,64 @@ import ( // Compare returns aggregate before/after deltas between two complete scan reports. func Compare(base, head model.ScanReport) model.DiffReport { return model.DiffReport{ - ScoreBefore: base.Score, - ScoreAfter: head.Score, - ScoreDelta: head.Score - base.Score, - TotalsDelta: compareTotals(base.Totals, head.Totals), - SeverityDelta: compareSeverity(base.Totals, head.Totals), - CategoryDelta: compareStringLikeCounts(categoryCounts(base.ByCategory), categoryCounts(head.ByCategory)), - LanguageDelta: compareStringLikeCounts(groupCounts(base.ByLanguage), groupCounts(head.ByLanguage)), - ModuleDelta: compareStringLikeCounts(groupCounts(base.ByModule), groupCounts(head.ByModule)), - RuleDelta: compareStringLikeCounts(ruleCounts(base.ByRule), ruleCounts(head.ByRule)), - DuplicationDelta: compareDuplication(base.Duplication, head.Duplication), + Incomplete: base.Incomplete || head.Incomplete, + IncompleteReasons: mergeIncompleteReasons(base.IncompleteReasons, head.IncompleteReasons), + ScoreBefore: base.Score, + ScoreAfter: head.Score, + ScoreDelta: head.Score - base.Score, + TotalsDelta: compareTotals(base.Totals, head.Totals), + SeverityDelta: compareSeverity(base.Totals, head.Totals), + CategoryDelta: compareStringLikeCounts(categoryCounts(base.ByCategory), categoryCounts(head.ByCategory)), + LanguageDelta: compareStringLikeCounts(groupCounts(base.ByLanguage), groupCounts(head.ByLanguage)), + ModuleDelta: compareStringLikeCounts(groupCounts(base.ByModule), groupCounts(head.ByModule)), + RuleDelta: compareStringLikeCounts(ruleCounts(base.ByRule), ruleCounts(head.ByRule)), + DuplicationDelta: compareDuplication(base.Duplication, head.Duplication), } } func compareTotals(base, head model.ScanTotals) model.TotalsDelta { return model.TotalsDelta{ - FilesScanned: countDelta(base.FilesScanned, head.FilesScanned), - FilesSkipped: countDelta(base.FilesSkipped, head.FilesSkipped), - LinesScanned: countDelta(base.LinesScanned, head.LinesScanned), - Issues: countDelta(base.Issues, head.Issues), - Errors: countDelta(base.Errors, head.Errors), - Warnings: countDelta(base.Warnings, head.Warnings), - Info: countDelta(base.Info, head.Info), + FilesScanned: countDelta(base.FilesScanned, head.FilesScanned), + FilesSkipped: countDelta(base.FilesSkipped, head.FilesSkipped), + FilesSkippedDueToSize: countDelta(base.FilesSkippedDueToSize, head.FilesSkippedDueToSize), + FilesSkippedDueToTimeout: countDelta(base.FilesSkippedDueToTimeout, head.FilesSkippedDueToTimeout), + LinesScanned: countDelta(base.LinesScanned, head.LinesScanned), + BlankLines: countDelta(base.BlankLines, head.BlankLines), + CodeLines: countDelta(base.CodeLines, head.CodeLines), + CommentLines: countDelta(base.CommentLines, head.CommentLines), + CommentOnlyLines: countDelta(base.CommentOnlyLines, head.CommentOnlyLines), + InlineCommentLines: countDelta(base.InlineCommentLines, head.InlineCommentLines), + CommentDensityPercent: percentDelta(base.CommentDensityPercent, head.CommentDensityPercent), + Issues: countDelta(base.Issues, head.Issues), + Errors: countDelta(base.Errors, head.Errors), + Warnings: countDelta(base.Warnings, head.Warnings), + Info: countDelta(base.Info, head.Info), } } +func mergeIncompleteReasons(left, right []string) []string { + if len(left) == 0 && len(right) == 0 { + return nil + } + seen := make(map[string]bool, len(left)+len(right)) + for _, reason := range left { + if reason != "" { + seen[reason] = true + } + } + for _, reason := range right { + if reason != "" { + seen[reason] = true + } + } + reasons := make([]string, 0, len(seen)) + for reason := range seen { + reasons = append(reasons, reason) + } + sort.Strings(reasons) + return reasons +} + func compareSeverity(base, head model.ScanTotals) map[model.Severity]model.CountDelta { return map[model.Severity]model.CountDelta{ model.SeverityError: countDelta(base.Errors, head.Errors), @@ -128,3 +161,9 @@ func countDelta(before, after int) model.CountDelta { Delta: after - before, } } + +func percentDelta(before, after float64) model.PercentDelta { + before = roundTenth(before) + after = roundTenth(after) + return model.PercentDelta{Before: before, After: after, Delta: roundTenth(after - before)} +} diff --git a/internal/duplicate/duplicate.go b/internal/duplicate/duplicate.go index 4698b8b..05e8686 100644 --- a/internal/duplicate/duplicate.go +++ b/internal/duplicate/duplicate.go @@ -32,6 +32,13 @@ type windowOwner struct { module string } +type factKey struct { + blobSHA string + language string +} + +type factIndex map[factKey]BlobFacts + func DefaultConfig() Config { return Config{WindowLines: defaultWindowLines} } @@ -45,11 +52,12 @@ func AnalyzeBlob(language string, data []byte, cfg Config) BlobFacts { func AggregateSnapshot(entries []model.SnapshotEntry, facts map[model.BlobKey]BlobFacts, cfg Config) Summary { cfg = cfg.withDefaults() summary := Summary{ByLanguage: make(map[string]int), ByModule: make(map[string]int)} + index := indexFacts(facts) counts := make(map[uint64]int) owners := make(map[uint64]windowOwner) for _, entry := range entries { - fact, ok := lookupFacts(entry, facts) + fact, ok := lookupFacts(entry, index) if !ok { continue } @@ -75,12 +83,12 @@ func AggregateSnapshot(entries []model.SnapshotEntry, facts map[model.BlobKey]Bl summary.ByModule[owner.module]++ } - summary.Stats.DuplicatedLogicalLines = duplicatedLogicalLineCoverage(entries, facts, duplicateHashes, cfg.WindowLines) + summary.Stats.DuplicatedLogicalLines = duplicatedLogicalLineCoverage(entries, index, duplicateHashes, cfg.WindowLines) summary.Stats.DensityPercent = densityPercent(summary.Stats.DuplicatedLogicalLines, summary.TotalLogicalLines) return summary } -func duplicatedLogicalLineCoverage(entries []model.SnapshotEntry, facts map[model.BlobKey]BlobFacts, duplicateHashes map[uint64]struct{}, windowSize int) int { +func duplicatedLogicalLineCoverage(entries []model.SnapshotEntry, facts factIndex, duplicateHashes map[uint64]struct{}, windowSize int) int { if len(duplicateHashes) == 0 || windowSize <= 0 { return 0 } @@ -225,16 +233,26 @@ func uniqueWindows(windows []uint64) []uint64 { return out } -func lookupFacts(entry model.SnapshotEntry, facts map[model.BlobKey]BlobFacts) (BlobFacts, bool) { - if len(facts) == 0 || entry.BlobSHA == "" { - return BlobFacts{}, false +func indexFacts(facts map[model.BlobKey]BlobFacts) factIndex { + if len(facts) == 0 { + return nil } - for candidate, fact := range facts { - if candidate.BlobSHA == entry.BlobSHA && candidate.Language == entry.Language { - return fact, true + index := make(factIndex, len(facts)) + for key, fact := range facts { + if key.BlobSHA == "" { + continue } + index[factKey{blobSHA: key.BlobSHA, language: key.Language}] = fact + } + return index +} + +func lookupFacts(entry model.SnapshotEntry, facts factIndex) (BlobFacts, bool) { + if len(facts) == 0 || entry.BlobSHA == "" { + return BlobFacts{}, false } - return BlobFacts{}, false + fact, ok := facts[factKey{blobSHA: entry.BlobSHA, language: entry.Language}] + return fact, ok } func groupLanguage(language string) string { diff --git a/internal/duplicate/duplicate_test.go b/internal/duplicate/duplicate_test.go index 4348f84..5e6b701 100644 --- a/internal/duplicate/duplicate_test.go +++ b/internal/duplicate/duplicate_test.go @@ -89,6 +89,27 @@ func TestAggregateSnapshotCoalescesAdjacentDuplicatedWindows(t *testing.T) { } } +func TestFactIndexMatchesByBlobAndLanguage(t *testing.T) { + goFact := BlobFacts{LogicalLines: 3} + jsFact := BlobFacts{LogicalLines: 5} + index := indexFacts(map[model.BlobKey]BlobFacts{ + {BlobSHA: "same", Language: "go", ConfigHash: "a", RulesetVersion: "r"}: goFact, + {BlobSHA: "same", Language: "javascript", ConfigHash: "b", RulesetVersion: "r"}: jsFact, + }) + + got, ok := lookupFacts(model.SnapshotEntry{BlobSHA: "same", Language: "go"}, index) + if !ok || got.LogicalLines != goFact.LogicalLines { + t.Fatalf("lookup go fact = %#v, %t; want %#v, true", got, ok, goFact) + } + got, ok = lookupFacts(model.SnapshotEntry{BlobSHA: "same", Language: "javascript"}, index) + if !ok || got.LogicalLines != jsFact.LogicalLines { + t.Fatalf("lookup javascript fact = %#v, %t; want %#v, true", got, ok, jsFact) + } + if _, ok := lookupFacts(model.SnapshotEntry{BlobSHA: "same", Language: "python"}, index); ok { + t.Fatal("lookup python fact = true, want false for unmatched language") + } +} + func testKey(sha string) model.BlobKey { return model.BlobKey{BlobSHA: sha, Language: "go", ConfigHash: "test", RulesetVersion: rules.RulesetVersion()} } diff --git a/internal/generic/generic.go b/internal/generic/generic.go index f1159c1..85a4808 100644 --- a/internal/generic/generic.go +++ b/internal/generic/generic.go @@ -87,11 +87,17 @@ func AggregateSnapshot(entries []model.SnapshotEntry, facts map[model.BlobKey]mo totals.FilesScanned++ totals.LinesScanned += blobFacts.Lines + totals.BlankLines += blobFacts.BlankLines + totals.CodeLines += blobFacts.CodeLines + totals.CommentLines += blobFacts.CommentLines + totals.CommentOnlyLines += blobFacts.CommentOnlyLines + totals.InlineCommentLines += blobFacts.InlineCommentLines totals.Errors += blobFacts.Issues.Errors totals.Warnings += blobFacts.Issues.Warnings totals.Info += blobFacts.Issues.Info totals.Issues += blobFacts.Issues.Errors + blobFacts.Issues.Warnings + blobFacts.Issues.Info } + totals.CommentDensityPercent = model.LineDensityPercent(totals.CommentLines, totals.LinesScanned) return totals } @@ -141,14 +147,19 @@ type nestingTracker struct { } type languageRuleLane struct { - style bool - todo bool - deepNest bool - bracedNest bool - lineCommentMarkers []string - blockComments bool - rawBackticks bool - riskyShell bool + style bool + todo bool + deepNest bool + bracedNest bool + lineCommentMarkers []string + hashRequiresBoundary bool + blockComments bool + nestedBlockComments bool + rawBackticks bool + regexLiterals bool + pythonDocStrings bool + rubyBlockComments bool + riskyShell bool } func blobSkipReason(data []byte, cfg Config) string { @@ -167,40 +178,84 @@ func blobSkipReason(data []byte, cfg Config) string { func analyzeBlobLines(language string, data []byte, cfg Config, catalog rules.Catalog) model.BlobFacts { facts := model.BlobFacts{} scanner := newBlobLineScanner(data) - nesting := newNestingTracker(cfg) lane := ruleLaneForLanguage(language) - comments := newCommentScanner(lane) - riskyShell := newRiskyShellScanner(language) - bracedNesting := newBracedNestingTracker(language, cfg, lane) - mergeConflict := mergeConflictScanner{} - mergeConflicts := 0 + state := newBlobLineAnalysis(language, cfg, catalog, lane, &facts) for scanner.Scan() { - line := strings.TrimSuffix(scanner.Text(), "\r") - facts.Lines++ - counts := countLineIssues(line, cfg, nesting, lane) - counts.mergeConflicts = mergeConflict.count(line) - mergeConflicts += counts.mergeConflicts - if lane.todo { - counts.todos = comments.countTodoMarkers(line) - } - if lane.riskyShell { - counts.riskyShell = riskyShell.count(line) - } - if lane.bracedNest { - counts.deepNesting += bracedNesting.count(line) - } - addLineIssues(&facts, catalog, counts) + state.analyzeLine(strings.TrimSuffix(scanner.Text(), "\r")) } if lane.riskyShell { - addLineIssues(&facts, catalog, lineIssueCounts{riskyShell: riskyShell.flush()}) + addLineIssues(&facts, catalog, lineIssueCounts{riskyShell: state.riskyShell.flush()}) } addScannerIssue(&facts, catalog, scanner.Err(), lane) - addLanguageIssues(&facts, catalog, language, data, cfg, mergeConflicts > 0) + addLanguageIssues(&facts, catalog, language, data, cfg, state.mergeConflicts > 0) return facts } +type blobLineAnalysis struct { + facts *model.BlobFacts + catalog rules.Catalog + cfg Config + nesting nestingTracker + lane languageRuleLane + comments commentScanner + riskyShell riskyShellScanner + bracedNesting bracedNestingTracker + mergeConflict mergeConflictScanner + mergeConflicts int +} + +func newBlobLineAnalysis(language string, cfg Config, catalog rules.Catalog, lane languageRuleLane, facts *model.BlobFacts) blobLineAnalysis { + return blobLineAnalysis{ + facts: facts, + catalog: catalog, + cfg: cfg, + nesting: newNestingTracker(cfg), + lane: lane, + comments: newCommentScanner(lane), + riskyShell: newRiskyShellScanner(language), + bracedNesting: newBracedNestingTracker(language, cfg, lane), + } +} + +func (analysis *blobLineAnalysis) analyzeLine(line string) { + analysis.facts.Lines++ + if strings.TrimSpace(line) == "" { + analysis.facts.BlankLines++ + } + counts := countLineIssues(line, analysis.cfg, analysis.nesting, analysis.lane) + counts.mergeConflicts = analysis.mergeConflict.count(line) + analysis.mergeConflicts += counts.mergeConflicts + comment := analysis.comments.scan(line) + addCommentLineFacts(analysis.facts, comment) + if analysis.lane.todo { + counts.todos = countTodoMarkers(comment.text) + } + if analysis.lane.riskyShell { + counts.riskyShell = analysis.riskyShell.count(line) + } + if analysis.lane.bracedNest { + counts.deepNesting += analysis.bracedNesting.count(line) + } + addLineIssues(analysis.facts, analysis.catalog, counts) +} + +func addCommentLineFacts(facts *model.BlobFacts, comment commentScanResult) { + if comment.hasCode { + facts.CodeLines++ + } + if !comment.hasComment { + return + } + facts.CommentLines++ + if comment.hasCode { + facts.InlineCommentLines++ + return + } + facts.CommentOnlyLines++ +} + func newBlobLineScanner(data []byte) *bufio.Scanner { scanner := bufio.NewScanner(bytes.NewReader(data)) scanner.Buffer(make([]byte, 0, 64*1024), maxScannerTokenBytes) @@ -216,15 +271,25 @@ func newNestingTracker(cfg Config) nestingTracker { func ruleLaneForLanguage(language string) languageRuleLane { switch language { - case "json", "toml", "yaml": + case "json": return languageRuleLane{} + case "toml": + return languageRuleLane{lineCommentMarkers: []string{"#"}} + case "yaml": + return languageRuleLane{lineCommentMarkers: []string{"#"}, hashRequiresBoundary: true} case "go": return languageRuleLane{style: true, todo: true, lineCommentMarkers: []string{"//"}, blockComments: true, rawBackticks: true} - case "python", "ruby": - return languageRuleLane{style: true, todo: true, deepNest: language == "python", lineCommentMarkers: []string{"#"}} + case "python": + return languageRuleLane{style: true, todo: true, deepNest: true, lineCommentMarkers: []string{"#"}, pythonDocStrings: true} + case "ruby": + return languageRuleLane{style: true, todo: true, lineCommentMarkers: []string{"#"}, rubyBlockComments: true} case "dockerfile", "shell": - return languageRuleLane{style: true, todo: true, riskyShell: true, lineCommentMarkers: []string{"#"}} - case "java", "csharp", "rust", "javascript", "typescript": + return languageRuleLane{style: true, todo: true, riskyShell: true, lineCommentMarkers: []string{"#"}, hashRequiresBoundary: true} + case "rust": + return languageRuleLane{style: true, todo: true, bracedNest: true, lineCommentMarkers: []string{"//"}, blockComments: true, nestedBlockComments: true} + case "javascript", "typescript": + return languageRuleLane{style: true, todo: true, bracedNest: true, lineCommentMarkers: []string{"//"}, blockComments: true, regexLiterals: true} + case "java", "csharp": return languageRuleLane{style: true, todo: true, bracedNest: true, lineCommentMarkers: []string{"//"}, blockComments: true} case "php": return languageRuleLane{style: true, todo: true, bracedNest: true, lineCommentMarkers: []string{"//", "#"}, blockComments: true} @@ -995,24 +1060,38 @@ func addSeverity(counts *model.IssueCounts, severity model.Severity, count int) } type commentScanner struct { - lineMarkers []string - blockComments bool - inBlock bool - quoteState commentQuoteState + lineMarkers []string + hashRequiresBoundary bool + blockComments bool + nestedBlockComments bool + blockDepth int + quoteState commentQuoteState + regexLiterals bool + pythonDocStrings bool + pythonDocQuote string + rubyBlockComments bool + inRubyBlock bool +} + +type commentScanResult struct { + text string + hasComment bool + hasCode bool } func newCommentScanner(lane languageRuleLane) commentScanner { return commentScanner{ - lineMarkers: lane.lineCommentMarkers, - blockComments: lane.blockComments, - quoteState: commentQuoteState{escapeBacktick: !lane.rawBackticks}, + lineMarkers: lane.lineCommentMarkers, + hashRequiresBoundary: lane.hashRequiresBoundary, + blockComments: lane.blockComments, + nestedBlockComments: lane.nestedBlockComments, + quoteState: commentQuoteState{escapeBacktick: !lane.rawBackticks}, + regexLiterals: lane.regexLiterals, + pythonDocStrings: lane.pythonDocStrings, + rubyBlockComments: lane.rubyBlockComments, } } -func (scanner *commentScanner) countTodoMarkers(line string) int { - return countTodoMarkers(scanner.text(line)) -} - func countTodoMarkers(line string) int { count := 0 for _, marker := range []string{"TODO", "FIXME", "HACK"} { @@ -1029,37 +1108,144 @@ type commentQuoteState struct { escaped bool } -func (scanner *commentScanner) text(line string) string { +func (scanner *commentScanner) scan(line string) commentScanResult { + result := commentScanResult{} var comments []string for i := 0; i < len(line); { - if scanner.inBlock { - text, next, closed := blockCommentSegment(line, i) - comments = append(comments, text) - scanner.inBlock = !closed + if next, ok := scanner.consumeActiveComment(line, i, &result, &comments); ok { i = next continue } - if scanner.quoteState.consume(line[i]) || scanner.quoteState.quoted() { - i++ + if next, ok := scanner.consumeStandaloneCommentStart(line, i, &result, &comments); ok { + i = next continue } - if marker, ok := matchingCommentMarker(line[i:], scanner.lineMarkers); ok { - comments = append(comments, line[i+len(marker):]) + if next, ok := scanner.consumeQuotedCodeToken(line, i, &result); ok { + i = next + continue + } + next, stop, ok := scanner.consumeCommentStart(line, i, &result, &comments) + if stop { break } - if scanner.blockComments && strings.HasPrefix(line[i:], "/*") { - text, next, closed := blockCommentSegment(line, i+len("/*")) - comments = append(comments, text) - scanner.inBlock = !closed + if ok { i = next continue } - i++ + i = consumePlainCodeToken(line, i, &result) } scanner.quoteState.finishLine() - return strings.Join(comments, "\n") + result.text = strings.Join(comments, "\n") + return result } +func (scanner *commentScanner) consumeActiveComment(line string, index int, result *commentScanResult, comments *[]string) (int, bool) { + switch { + case scanner.inRubyBlock: + text, next, closed := rubyBlockCommentSegment(line, index) + appendComment(comments, result, text) + scanner.inRubyBlock = !closed + return next, true + case scanner.pythonDocQuote != "": + text, next, closed := quotedBlockSegment(line, index, scanner.pythonDocQuote) + appendComment(comments, result, text) + if closed { + scanner.pythonDocQuote = "" + } + return next, true + case scanner.blockDepth > 0: + text, next := scanner.blockCommentSegment(line, index) + appendComment(comments, result, text) + return next, true + default: + return index, false + } +} + +func (scanner *commentScanner) consumeStandaloneCommentStart(line string, index int, result *commentScanResult, comments *[]string) (int, bool) { + if scanner.rubyBlockComments && isRubyBlockCommentStart(line, index, result.hasCode) { + text, next, closed := rubyBlockCommentSegment(line, index) + appendComment(comments, result, text) + scanner.inRubyBlock = !closed + return next, true + } + if scanner.pythonDocStrings { + if quote, found := pythonDocStringStart(line, index, result.hasCode); found { + scanner.pythonDocQuote = quote + text, next, closed := quotedBlockSegment(line, index+len(quote), quote) + appendComment(comments, result, text) + if closed { + scanner.pythonDocQuote = "" + } + return next, true + } + } + return index, false +} + +func (scanner *commentScanner) consumeCommentStart(line string, index int, result *commentScanResult, comments *[]string) (next int, stop bool, ok bool) { + if marker, found := scanner.matchingCommentMarker(line, index); found { + appendComment(comments, result, line[index+len(marker):]) + return len(line), true, true + } + if scanner.blockComments && strings.HasPrefix(line[index:], "/*") { + scanner.blockDepth = 1 + text, next := scanner.blockCommentSegment(line, index+len("/*")) + appendComment(comments, result, text) + return next, false, true + } + return index, false, false +} + +func (scanner *commentScanner) consumeQuotedCodeToken(line string, index int, result *commentScanResult) (int, bool) { + if scanner.quoteState.consume(line[index]) || scanner.quoteState.quoted() { + result.hasCode = true + return index + 1, true + } + if scanner.regexLiterals && line[index] == '/' && isLikelyRegexLiteralStart(line, index) { + if next, ok := skipRegexLiteral(line, index); ok { + result.hasCode = true + return next, true + } + } + return index, false +} + +func consumePlainCodeToken(line string, index int, result *commentScanResult) int { + if !isHorizontalSpace(line[index]) { + result.hasCode = true + } + return index + 1 +} + +func appendComment(comments *[]string, result *commentScanResult, text string) { + *comments = append(*comments, text) + result.hasComment = true +} + +func (scanner *commentScanner) blockCommentSegment(line string, start int) (text string, next int) { + var segment strings.Builder + for i := start; i < len(line); { + if scanner.nestedBlockComments && strings.HasPrefix(line[i:], "/*") { + scanner.blockDepth++ + segment.WriteString("/*") + i += len("/*") + continue + } + if strings.HasPrefix(line[i:], "*/") { + scanner.blockDepth-- + i += len("*/") + if scanner.blockDepth == 0 { + return segment.String(), i + } + segment.WriteString("*/") + continue + } + segment.WriteByte(line[i]) + i++ + } + return segment.String(), len(line) +} func blockCommentSegment(line string, start int) (text string, next int, closed bool) { end := strings.Index(line[start:], "*/") if end < 0 { @@ -1128,6 +1314,48 @@ func (state *commentQuoteState) toggleBacktick() bool { return true } +func rubyBlockCommentSegment(line string, start int) (text string, next int, closed bool) { + trimmed := strings.TrimSpace(line[start:]) + return line[start:], len(line), strings.HasPrefix(trimmed, "=end") +} + +func isRubyBlockCommentStart(line string, index int, hasCode bool) bool { + return !hasCode && onlyHorizontalSpace(line[:index]) && strings.HasPrefix(line[index:], "=begin") +} + +func pythonDocStringStart(line string, index int, hasCode bool) (string, bool) { + if hasCode || !onlyHorizontalSpace(line[:index]) { + return "", false + } + for _, quote := range []string{`"""`, `'''`} { + if strings.HasPrefix(line[index:], quote) { + return quote, true + } + } + return "", false +} + +func quotedBlockSegment(line string, start int, quote string) (text string, next int, closed bool) { + end := strings.Index(line[start:], quote) + if end < 0 { + return line[start:], len(line), false + } + end += start + return line[start:end], end + len(quote), true +} + +func (scanner commentScanner) matchingCommentMarker(line string, index int) (string, bool) { + for _, marker := range scanner.lineMarkers { + if !strings.HasPrefix(line[index:], marker) { + continue + } + if marker == "#" && scanner.hashRequiresBoundary && !startsShellComment(line, index) { + continue + } + return marker, true + } + return "", false +} func matchingCommentMarker(text string, commentMarkers []string) (string, bool) { for _, marker := range commentMarkers { if strings.HasPrefix(text, marker) { @@ -1137,6 +1365,88 @@ func matchingCommentMarker(text string, commentMarkers []string) (string, bool) return "", false } +func isLikelyRegexLiteralStart(line string, index int) bool { + if strings.HasPrefix(line[index:], "//") || strings.HasPrefix(line[index:], "/*") { + return false + } + prev := previousSignificantByte(line, index) + if prev == 0 || strings.ContainsRune("=({[,!:?;&|+-*~^<>", rune(prev)) { + return true + } + switch previousIdentifier(line, index) { + case "return", "case", "throw", "yield", "await", "typeof", "delete", "void", "in", "of": + return true + default: + return false + } +} + +func skipRegexLiteral(line string, start int) (next int, ok bool) { + escaped := false + inClass := false + for i := start + 1; i < len(line); i++ { + switch char := line[i]; { + case escaped: + escaped = false + case char == '\\': + escaped = true + case char == '[': + inClass = true + case char == ']': + inClass = false + case char == '/' && !inClass: + i++ + for i < len(line) && isRegexFlag(line[i]) { + i++ + } + return i, true + } + } + return start, false +} + +func previousSignificantByte(line string, index int) byte { + for i := index - 1; i >= 0; i-- { + if !isHorizontalSpace(line[i]) { + return line[i] + } + } + return 0 +} + +func previousIdentifier(line string, index int) string { + i := index - 1 + for i >= 0 && isHorizontalSpace(line[i]) { + i-- + } + end := i + 1 + for i >= 0 && isIdentifierByte(line[i]) { + i-- + } + return line[i+1 : end] +} + +func isRegexFlag(char byte) bool { + return char >= 'a' && char <= 'z' || char >= 'A' && char <= 'Z' +} + +func onlyHorizontalSpace(text string) bool { + for i := 0; i < len(text); i++ { + if !isHorizontalSpace(text[i]) { + return false + } + } + return true +} + +func isHorizontalSpace(char byte) bool { + return char == ' ' || char == '\t' +} + +func isIdentifierByte(char byte) bool { + return char >= 'a' && char <= 'z' || char >= 'A' && char <= 'Z' || char >= '0' && char <= '9' || char == '_' || char == '$' +} + func hasTrailingWhitespace(line string) bool { return strings.HasSuffix(line, " ") || strings.HasSuffix(line, "\t") } diff --git a/internal/generic/generic_test.go b/internal/generic/generic_test.go index 33725ea..76a73f2 100644 --- a/internal/generic/generic_test.go +++ b/internal/generic/generic_test.go @@ -108,6 +108,121 @@ func TestAnalyzeBlobCountsGenericRulesAggregately(t *testing.T) { } } +func TestAnalyzeBlobClassifiesCommentAndCodeLinesAggregately(t *testing.T) { + data := strings.Join([]string{ + "package main", + "", + "const url = \"https://example.invalid/not-comment\"", + "// package comment", + "func main() { println(url) } // inline comment", + "/* block start", + "block middle", + "*/", + "const raw = `// not a comment`", + }, "\n") + "\n" + + facts := AnalyzeBlob(testKeyForLanguage("comments", "go"), []byte(data), Config{}, rules.DefaultCatalog()) + payload, err := json.Marshal(facts) + if err != nil { + t.Fatal(err) + } + var decoded map[string]any + if err := json.Unmarshal(payload, &decoded); err != nil { + t.Fatalf("unmarshal facts JSON: %v", err) + } + assertFactJSONNumber(t, decoded, "lines", 9) + assertFactJSONNumber(t, decoded, "blank_lines", 1) + assertFactJSONNumber(t, decoded, "code_lines", 4) + assertFactJSONNumber(t, decoded, "comment_lines", 5) + assertFactJSONNumber(t, decoded, "comment_only_lines", 4) + assertFactJSONNumber(t, decoded, "inline_comment_lines", 1) +} + +func TestAnalyzeBlobCountsSupportedLanguageCommentForms(t *testing.T) { + tests := []struct { + name string + language string + data string + commentLines float64 + commentOnlyLines float64 + inlineCommentLines float64 + codeLines float64 + }{ + { + name: "yaml hash comments require a boundary", + language: "yaml", + data: "# header\nname: value # inline\nurl: https://example.invalid/#fragment\n", + commentLines: 2, + commentOnlyLines: 1, + inlineCommentLines: 1, + codeLines: 2, + }, + { + name: "toml hash comments ignore quoted hashes", + language: "toml", + data: "# header\nname = \"value\" # inline\nurl = \"https://example.invalid/#fragment\"\n", + commentLines: 2, + commentOnlyLines: 1, + inlineCommentLines: 1, + codeLines: 2, + }, + { + name: "python docstrings count as comment-only lines", + language: "python", + data: "\"\"\"module TODO\"\"\"\ndef f():\n \"\"\"function doc\n still doc\n \"\"\"\n return \"# not comment\" # inline\n", + commentLines: 5, + commentOnlyLines: 4, + inlineCommentLines: 1, + codeLines: 2, + }, + { + name: "javascript regex literals do not create comments", + language: "javascript", + data: "const re = /[//]/;\nconst escaped = /\\/\\//; // inline\n", + commentLines: 1, + commentOnlyLines: 0, + inlineCommentLines: 1, + codeLines: 2, + }, + { + name: "rust nested block comments stay comment-only", + language: "rust", + data: "/* outer\n/* inner */\nstill outer */\nfn main() {}\n", + commentLines: 3, + commentOnlyLines: 3, + inlineCommentLines: 0, + codeLines: 1, + }, + { + name: "ruby block comments stay comment-only", + language: "ruby", + data: "=begin\nblock\n=end\nputs \"# not comment\" # inline\n", + commentLines: 4, + commentOnlyLines: 3, + inlineCommentLines: 1, + codeLines: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + facts := AnalyzeBlob(testKeyForLanguage("comments-"+tt.language, tt.language), []byte(tt.data), Config{}, rules.DefaultCatalog()) + payload, err := json.Marshal(facts) + if err != nil { + t.Fatal(err) + } + var decoded map[string]any + if err := json.Unmarshal(payload, &decoded); err != nil { + t.Fatalf("unmarshal facts JSON: %v", err) + } + assertFactJSONNumber(t, decoded, "comment_lines", tt.commentLines) + assertFactJSONNumber(t, decoded, "comment_only_lines", tt.commentOnlyLines) + assertFactJSONNumber(t, decoded, "inline_comment_lines", tt.inlineCommentLines) + assertFactJSONNumber(t, decoded, "code_lines", tt.codeLines) + }) + } +} + func TestAnalyzeBlobSkipsGeneratedAndMinifiedContent(t *testing.T) { catalog := rules.DefaultCatalog() tests := []struct { @@ -680,6 +795,17 @@ func highComplexityGoClosureSource() string { "\t\treturn x\n\t}\n}\n" } +func assertFactJSONNumber(t *testing.T, values map[string]any, key string, want float64) { + t.Helper() + got, ok := values[key] + if !ok && want == 0 { + return + } + if got != want { + t.Fatalf("%s = %#v, want %.1f in %#v", key, got, want, values) + } +} + func testKey(sha string) model.BlobKey { return testKeyForLanguage(sha, "go") } diff --git a/internal/model/model.go b/internal/model/model.go index e9b5be8..2fbf368 100644 --- a/internal/model/model.go +++ b/internal/model/model.go @@ -9,6 +9,19 @@ const ( SeverityInfo Severity = "info" ) +// ReportStatus is the stable aggregate pass/fail state for scan and diff reports. +type ReportStatus string + +const ( + ReportStatusPassed ReportStatus = "passed" + ReportStatusFailed ReportStatus = "failed" +) + +const ( + IncompleteReasonFilesSkippedDueToSize = "files_skipped_due_to_size" + IncompleteReasonFilesSkippedDueToTimeout = "files_skipped_due_to_timeout" +) + // Category is the stable issue category used in aggregate reports. type Category string @@ -85,47 +98,83 @@ type BlobKey struct { // BlobFacts stores aggregate facts for a single analyzed blob. It intentionally has // no file findings, snippets, or line/column locations. type BlobFacts struct { - Lines int `json:"lines"` - Issues IssueCounts `json:"issues"` - ByCategory map[Category]IssueCounts `json:"by_category,omitempty"` - ByRule map[string]RuleCount `json:"by_rule,omitempty"` - Skipped bool `json:"skipped,omitempty"` - SkipReason string `json:"skip_reason,omitempty"` + Lines int `json:"lines"` + BlankLines int `json:"blank_lines,omitempty"` + CodeLines int `json:"code_lines,omitempty"` + CommentLines int `json:"comment_lines,omitempty"` + CommentOnlyLines int `json:"comment_only_lines,omitempty"` + InlineCommentLines int `json:"inline_comment_lines,omitempty"` + Issues IssueCounts `json:"issues"` + ByCategory map[Category]IssueCounts `json:"by_category,omitempty"` + ByRule map[string]RuleCount `json:"by_rule,omitempty"` + Skipped bool `json:"skipped,omitempty"` + SkipReason string `json:"skip_reason,omitempty"` } // ScanTotals stores top-level aggregate counters for one complete snapshot scan. type ScanTotals struct { - FilesScanned int `json:"files_scanned"` - FilesSkipped int `json:"files_skipped"` - LinesScanned int `json:"lines_scanned"` - Issues int `json:"issues"` - Errors int `json:"errors"` - Warnings int `json:"warnings"` - Info int `json:"info"` + FilesScanned int `json:"files_scanned"` + FilesSkipped int `json:"files_skipped"` + FilesSkippedDueToSize int `json:"files_skipped_due_to_size"` + FilesSkippedDueToTimeout int `json:"files_skipped_due_to_timeout"` + LinesScanned int `json:"lines_scanned"` + BlankLines int `json:"blank_lines"` + CodeLines int `json:"code_lines"` + CommentLines int `json:"comment_lines"` + CommentOnlyLines int `json:"comment_only_lines"` + InlineCommentLines int `json:"inline_comment_lines"` + CommentDensityPercent float64 `json:"comment_density_percent"` + Issues int `json:"issues"` + Errors int `json:"errors"` + Warnings int `json:"warnings"` + Info int `json:"info"` +} + +// ScanThresholds stores the aggregate quality gate applied to one scan. +type ScanThresholds struct { + FailUnder int `json:"fail_under"` +} + +// DiffThresholds stores the aggregate regression gates applied to one diff. +type DiffThresholds struct { + FailOnScoreDrop bool `json:"fail_on_score_drop"` + MaxAllowedNewErrors int `json:"max_allowed_new_errors"` + CategoryRegressionThreshold int `json:"category_regression_threshold"` } // GroupCounts stores aggregate counters for a language or module grouping. type GroupCounts struct { - Files int `json:"files"` - Lines int `json:"lines"` - Issues int `json:"issues"` - Errors int `json:"errors"` - Warnings int `json:"warnings"` - Info int `json:"info"` + Files int `json:"files"` + Lines int `json:"lines"` + BlankLines int `json:"blank_lines"` + CodeLines int `json:"code_lines"` + CommentLines int `json:"comment_lines"` + CommentOnlyLines int `json:"comment_only_lines"` + InlineCommentLines int `json:"inline_comment_lines"` + CommentDensityPercent float64 `json:"comment_density_percent"` + Issues int `json:"issues"` + Errors int `json:"errors"` + Warnings int `json:"warnings"` + Info int `json:"info"` } // ScanReport is the aggregate report for one complete snapshot scan. type ScanReport struct { - Score int `json:"score"` - Band string `json:"band"` - RulesetVersion string `json:"ruleset_version,omitempty"` - Totals ScanTotals `json:"totals"` - ByCategory map[Category]IssueCounts `json:"by_category,omitempty"` - ByLanguage map[string]GroupCounts `json:"by_language,omitempty"` - ByModule map[string]GroupCounts `json:"by_module,omitempty"` - ByRule map[string]RuleCount `json:"by_rule,omitempty"` - Duplication DuplicationStats `json:"duplication,omitempty"` - Detection DetectedProject `json:"detection,omitempty"` + Score int `json:"score"` + Band string `json:"band"` + Status ReportStatus `json:"status"` + Passed bool `json:"passed"` + Thresholds ScanThresholds `json:"thresholds"` + Incomplete bool `json:"incomplete"` + IncompleteReasons []string `json:"incomplete_reasons,omitempty"` + RulesetVersion string `json:"ruleset_version,omitempty"` + Totals ScanTotals `json:"totals"` + ByCategory map[Category]IssueCounts `json:"by_category,omitempty"` + ByLanguage map[string]GroupCounts `json:"by_language,omitempty"` + ByModule map[string]GroupCounts `json:"by_module,omitempty"` + ByRule map[string]RuleCount `json:"by_rule,omitempty"` + Duplication DuplicationStats `json:"duplication,omitempty"` + Detection DetectedProject `json:"detection,omitempty"` } // CountDelta stores an aggregate before/after/delta value. @@ -135,32 +184,61 @@ type CountDelta struct { Delta int `json:"delta"` } +// PercentDelta stores an aggregate before/after/delta percentage value. +type PercentDelta struct { + Before float64 `json:"before"` + After float64 `json:"after"` + Delta float64 `json:"delta"` +} + // TotalsDelta stores top-level aggregate deltas between complete scans. type TotalsDelta struct { - FilesScanned CountDelta `json:"files_scanned"` - FilesSkipped CountDelta `json:"files_skipped"` - LinesScanned CountDelta `json:"lines_scanned"` - Issues CountDelta `json:"issues"` - Errors CountDelta `json:"errors"` - Warnings CountDelta `json:"warnings"` - Info CountDelta `json:"info"` + FilesScanned CountDelta `json:"files_scanned"` + FilesSkipped CountDelta `json:"files_skipped"` + FilesSkippedDueToSize CountDelta `json:"files_skipped_due_to_size"` + FilesSkippedDueToTimeout CountDelta `json:"files_skipped_due_to_timeout"` + LinesScanned CountDelta `json:"lines_scanned"` + BlankLines CountDelta `json:"blank_lines"` + CodeLines CountDelta `json:"code_lines"` + CommentLines CountDelta `json:"comment_lines"` + CommentOnlyLines CountDelta `json:"comment_only_lines"` + InlineCommentLines CountDelta `json:"inline_comment_lines"` + CommentDensityPercent PercentDelta `json:"comment_density_percent"` + Issues CountDelta `json:"issues"` + Errors CountDelta `json:"errors"` + Warnings CountDelta `json:"warnings"` + Info CountDelta `json:"info"` } // DiffReport is the aggregate comparison between two complete snapshot scans. type DiffReport struct { - Base string `json:"base"` - Head string `json:"head"` - Scope string `json:"scope"` - ScoreBefore int `json:"score_before"` - ScoreAfter int `json:"score_after"` - ScoreDelta int `json:"score_delta"` - TotalsDelta TotalsDelta `json:"totals_delta"` - SeverityDelta map[Severity]CountDelta `json:"severity_delta,omitempty"` - CategoryDelta map[Category]CountDelta `json:"category_delta,omitempty"` - LanguageDelta map[string]CountDelta `json:"language_delta,omitempty"` - ModuleDelta map[string]CountDelta `json:"module_delta,omitempty"` - RuleDelta map[string]CountDelta `json:"rule_delta,omitempty"` - DuplicationDelta DuplicationDelta `json:"duplication_delta,omitempty"` + Base string `json:"base"` + Head string `json:"head"` + Scope string `json:"scope"` + Status ReportStatus `json:"status"` + Passed bool `json:"passed"` + Regression bool `json:"regression"` + Thresholds DiffThresholds `json:"thresholds"` + Incomplete bool `json:"incomplete"` + IncompleteReasons []string `json:"incomplete_reasons,omitempty"` + ScoreBefore int `json:"score_before"` + ScoreAfter int `json:"score_after"` + ScoreDelta int `json:"score_delta"` + TotalsDelta TotalsDelta `json:"totals_delta"` + SeverityDelta map[Severity]CountDelta `json:"severity_delta,omitempty"` + CategoryDelta map[Category]CountDelta `json:"category_delta,omitempty"` + LanguageDelta map[string]CountDelta `json:"language_delta,omitempty"` + ModuleDelta map[string]CountDelta `json:"module_delta,omitempty"` + RuleDelta map[string]CountDelta `json:"rule_delta,omitempty"` + DuplicationDelta DuplicationDelta `json:"duplication_delta,omitempty"` +} + +// LineDensityPercent returns part/total as a percentage rounded to one decimal. +func LineDensityPercent(part, total int) float64 { + if part <= 0 || total <= 0 { + return 0 + } + return float64((int64(part)*1000+int64(total)/2)/int64(total)) / 10 } // DetectedProject describes aggregate project detection results. diff --git a/internal/report/report.go b/internal/report/report.go index 4871c3f..c77b67a 100644 --- a/internal/report/report.go +++ b/internal/report/report.go @@ -14,10 +14,22 @@ import ( func HumanDiff(diff model.DiffReport) string { var b strings.Builder fmt.Fprintf(&b, "Quality change: %+d points (%d -> %d, %s)\n", diff.ScoreDelta, diff.ScoreBefore, diff.ScoreAfter, band(diff.ScoreAfter)) + if diff.Status != "" { + fmt.Fprintf(&b, "Status: %s (regression: %t)\n", diff.Status, diff.Regression) + } + if diff.Incomplete { + fmt.Fprintf(&b, "Incomplete: true (%s)\n", strings.Join(diff.IncompleteReasons, ", ")) + } fmt.Fprintf(&b, "Scan scope: %s snapshots\n", diff.Scope) - fmt.Fprintf(&b, "Files scanned: %d -> %d (%+d) | Lines scanned: %d -> %d (%+d)\n", + fmt.Fprintf(&b, "Files scanned: %d -> %d (%+d) | Lines scanned: %d -> %d (%+d) | Code lines: %d -> %d (%+d)\n", diff.TotalsDelta.FilesScanned.Before, diff.TotalsDelta.FilesScanned.After, diff.TotalsDelta.FilesScanned.Delta, - diff.TotalsDelta.LinesScanned.Before, diff.TotalsDelta.LinesScanned.After, diff.TotalsDelta.LinesScanned.Delta) + diff.TotalsDelta.LinesScanned.Before, diff.TotalsDelta.LinesScanned.After, diff.TotalsDelta.LinesScanned.Delta, + diff.TotalsDelta.CodeLines.Before, diff.TotalsDelta.CodeLines.After, diff.TotalsDelta.CodeLines.Delta) + fmt.Fprintf(&b, "Comments: %d -> %d (%+d) | comment-only %d -> %d (%+d) | inline %d -> %d (%+d) | density %.1f%% -> %.1f%% (%+.1f%%)\n", + diff.TotalsDelta.CommentLines.Before, diff.TotalsDelta.CommentLines.After, diff.TotalsDelta.CommentLines.Delta, + diff.TotalsDelta.CommentOnlyLines.Before, diff.TotalsDelta.CommentOnlyLines.After, diff.TotalsDelta.CommentOnlyLines.Delta, + diff.TotalsDelta.InlineCommentLines.Before, diff.TotalsDelta.InlineCommentLines.After, diff.TotalsDelta.InlineCommentLines.Delta, + diff.TotalsDelta.CommentDensityPercent.Before, diff.TotalsDelta.CommentDensityPercent.After, diff.TotalsDelta.CommentDensityPercent.Delta) fmt.Fprintf(&b, "Languages: %s | Modules: %s\n", displayList(languageKeys(diff.LanguageDelta), displayLanguage), displayList(stringKeys(diff.ModuleDelta), identity)) fmt.Fprintf(&b, "Issues: %d -> %d (%+d) | Errors: %d -> %d (%+d) | Warnings: %d -> %d (%+d) | Info: %d -> %d (%+d)\n", diff.TotalsDelta.Issues.Before, diff.TotalsDelta.Issues.After, diff.TotalsDelta.Issues.Delta, diff --git a/internal/report/report_test.go b/internal/report/report_test.go index 0da2bb1..d3cbda3 100644 --- a/internal/report/report_test.go +++ b/internal/report/report_test.go @@ -10,9 +10,16 @@ import ( func TestHumanDiffRendersAggregateSections(t *testing.T) { diff := model.DiffReport{ - Base: "base", - Head: "head", - Scope: "complete", + Base: "base", + Head: "head", + Scope: "complete", + Status: "failed", + Passed: false, + Regression: true, + Incomplete: true, + IncompleteReasons: []string{ + "files_skipped_due_to_size", + }, ScoreBefore: 99, ScoreAfter: 85, ScoreDelta: -14, @@ -47,7 +54,7 @@ func TestHumanDiffRendersAggregateSections(t *testing.T) { } text := HumanDiff(diff) - for _, want := range []string{"Quality change:", "Scan scope: complete snapshots", "Duplication:", "Category delta:", "Language delta:", "Module delta:", "Security", "TypeScript"} { + for _, want := range []string{"Quality change:", "Status: failed", "Incomplete: true", "Scan scope: complete snapshots", "Duplication:", "Category delta:", "Language delta:", "Module delta:", "Security", "TypeScript"} { if !strings.Contains(text, want) { t.Fatalf("HumanDiff() = %q, want %q", text, want) } @@ -61,9 +68,16 @@ func TestHumanDiffRendersAggregateSections(t *testing.T) { func TestJSONRendersAggregateSchemaOnly(t *testing.T) { diff := model.DiffReport{ - Base: "base", - Head: "head", - Scope: "complete", + Base: "base", + Head: "head", + Scope: "complete", + Status: "failed", + Passed: false, + Regression: true, + Incomplete: true, + IncompleteReasons: []string{ + "files_skipped_due_to_size", + }, ScoreBefore: 99, ScoreAfter: 85, ScoreDelta: -14, @@ -93,7 +107,7 @@ func TestJSONRendersAggregateSchemaOnly(t *testing.T) { if err := json.Unmarshal(encoded, &decoded); err != nil { t.Fatalf("unmarshal JSON output: %v\n%s", err, encoded) } - for _, key := range []string{"base", "head", "scope", "score_before", "score_after", "score_delta", "totals_delta", "severity_delta", "category_delta", "language_delta", "module_delta", "duplication_delta"} { + for _, key := range []string{"base", "head", "scope", "status", "passed", "regression", "incomplete", "incomplete_reasons", "score_before", "score_after", "score_delta", "totals_delta", "severity_delta", "category_delta", "language_delta", "module_delta", "duplication_delta"} { if _, ok := decoded[key]; !ok { t.Fatalf("JSON output missing %q: %s", key, encoded) } diff --git a/internal/scan/ignore.go b/internal/scan/ignore.go new file mode 100644 index 0000000..17ad226 --- /dev/null +++ b/internal/scan/ignore.go @@ -0,0 +1,204 @@ +package scan + +import ( + "fmt" + "os" + "path" + "path/filepath" + "strings" +) + +var ignoreFileNames = [...]string{".gitignore", ".ignore"} + +type ignoreMatcher struct { + rules []ignoreRule +} + +type ignoreRule struct { + base string + pattern string + negated bool + dirOnly bool + anchored bool + hasSlash bool +} + +func newIgnoreMatcher(root string) (*ignoreMatcher, error) { + matcher := &ignoreMatcher{} + if err := matcher.loadFile(filepath.Join(root, ".git", "info", "exclude"), ""); err != nil { + return nil, err + } + if err := matcher.loadDir(root, ""); err != nil { + return nil, err + } + return matcher, nil +} + +func (matcher *ignoreMatcher) loadDir(absDir, relDir string) error { + relDir = cleanIgnorePath(relDir) + for _, name := range ignoreFileNames { + if err := matcher.loadFile(filepath.Join(absDir, name), relDir); err != nil { + return err + } + } + return nil +} + +func (matcher *ignoreMatcher) loadFile(name, relDir string) error { + data, err := os.ReadFile(name) + if os.IsNotExist(err) { + return nil + } + if err != nil { + return fmt.Errorf("read ignore file %s: %w", name, err) + } + for _, raw := range strings.Split(string(data), "\n") { + rule, ok := parseIgnoreRule(raw, relDir) + if ok { + matcher.rules = append(matcher.rules, rule) + } + } + return nil +} + +func parseIgnoreRule(raw, base string) (ignoreRule, bool) { + line := trimIgnoreLine(raw) + if line == "" || strings.HasPrefix(line, "#") { + return ignoreRule{}, false + } + + negated := false + if strings.HasPrefix(line, "!") { + negated = true + line = line[1:] + } else if strings.HasPrefix(line, `\!`) || strings.HasPrefix(line, `\#`) { + line = line[1:] + } + + line = strings.ReplaceAll(line, `\ `, " ") + line = filepath.ToSlash(line) + anchored := strings.HasPrefix(line, "/") + for strings.HasPrefix(line, "/") { + line = line[1:] + } + dirOnly := strings.HasSuffix(line, "/") + for strings.HasSuffix(line, "/") { + line = strings.TrimSuffix(line, "/") + } + line = cleanIgnorePath(line) + if line == "" || line == "." { + return ignoreRule{}, false + } + + return ignoreRule{ + base: cleanIgnorePath(base), + pattern: line, + negated: negated, + dirOnly: dirOnly, + anchored: anchored, + hasSlash: strings.Contains(line, "/"), + }, true +} + +func trimIgnoreLine(line string) string { + line = strings.TrimSuffix(line, "\r") + for len(line) > 0 { + last := line[len(line)-1] + if last != ' ' && last != '\t' { + return line + } + backslashes := 0 + for i := len(line) - 2; i >= 0 && line[i] == '\\'; i-- { + backslashes++ + } + if backslashes%2 == 1 { + return line + } + line = line[:len(line)-1] + } + return line +} + +func (matcher *ignoreMatcher) ignored(rel string, isDir bool) bool { + rel = cleanIgnorePath(rel) + if rel == "" || rel == "." { + return false + } + ignored := false + for _, rule := range matcher.rules { + if rule.matches(rel, isDir) { + ignored = !rule.negated + } + } + return ignored +} + +func (rule ignoreRule) matches(rel string, isDir bool) bool { + sub, ok := ignoreSubpath(rel, rule.base) + if !ok || sub == "" || sub == "." { + return false + } + if rule.dirOnly && !isDir { + return false + } + if rule.anchored || rule.hasSlash { + return matchIgnorePath(rule.pattern, sub) + } + for _, part := range strings.Split(sub, "/") { + if matchIgnoreSegment(rule.pattern, part) { + return true + } + } + return false +} + +func ignoreSubpath(rel, base string) (string, bool) { + if base == "" || base == "." { + return rel, true + } + if rel == base { + return "", true + } + prefix := base + "/" + if strings.HasPrefix(rel, prefix) { + return strings.TrimPrefix(rel, prefix), true + } + return "", false +} + +func matchIgnorePath(pattern, target string) bool { + return matchIgnoreSegments(strings.Split(pattern, "/"), strings.Split(target, "/")) +} + +func matchIgnoreSegments(pattern, target []string) bool { + if len(pattern) == 0 { + return len(target) == 0 + } + if pattern[0] == "**" { + if matchIgnoreSegments(pattern[1:], target) { + return true + } + return len(target) > 0 && matchIgnoreSegments(pattern, target[1:]) + } + if len(target) == 0 || !matchIgnoreSegment(pattern[0], target[0]) { + return false + } + return matchIgnoreSegments(pattern[1:], target[1:]) +} + +func matchIgnoreSegment(pattern, name string) bool { + matched, err := path.Match(pattern, name) + if err != nil { + return pattern == name + } + return matched +} + +func cleanIgnorePath(name string) string { + name = filepath.ToSlash(name) + name = path.Clean(name) + if name == "." { + return "" + } + return name +} diff --git a/internal/scan/scan.go b/internal/scan/scan.go index 33076cb..b74c383 100644 --- a/internal/scan/scan.go +++ b/internal/scan/scan.go @@ -8,10 +8,9 @@ import ( "io/fs" "os" "path/filepath" - "runtime" "sort" "strings" - "sync" + "time" "github.com/randomcodespace/code-signal/internal/config" "github.com/randomcodespace/code-signal/internal/delta" @@ -66,15 +65,15 @@ func ScanPath(ctx context.Context, root string, opts ScanOptions) (model.ScanRep root = "." } cfg := effectiveConfig(opts.Config) - entries, dataByBlob, previews, err := walkWorkingTree(ctx, root, cfg) + working, err := walkWorkingTree(ctx, root, cfg) if err != nil { return model.ScanReport{}, err } if opts.Stats != nil { - opts.Stats.HeadEntries = len(entries) + opts.Stats.HeadEntries = len(working.entries) } - snapshot := buildDetectedSnapshot(entries, previews) - facts, err := analyzeSnapshotUnion(ctx, [][]model.SnapshotEntry{snapshot.entries}, dataByBlob, cfg, opts.Stats) + snapshot := buildDetectedSnapshot(working.entries, working.previews) + facts, err := analyzeWorkingTreeSnapshot(ctx, root, snapshot.entries, cfg, opts.Stats) if err != nil { return model.ScanReport{}, err } @@ -116,20 +115,22 @@ func Diff(ctx context.Context, repo, base, head, scopePath string, opts DiffOpti opts.Stats.HeadEntries = len(headEntries) } - baseData, basePreviews, err := readGitSnapshot(ctx, repo, baseRef, baseEntries, cfg, opts.Stats) + basePreviews, err := readGitPreviews(ctx, repo, baseRef, baseEntries, cfg) if err != nil { return model.DiffReport{}, err } - headData, headPreviews, err := readGitSnapshot(ctx, repo, headRef, headEntries, cfg, opts.Stats) + headPreviews, err := readGitPreviews(ctx, repo, headRef, headEntries, cfg) if err != nil { return model.DiffReport{}, err } - dataByBlob := mergeBlobData(baseData, headData) baseSnapshot := buildDetectedSnapshot(baseEntries, basePreviews) headSnapshot := buildDetectedSnapshot(headEntries, headPreviews) - facts, err := analyzeSnapshotUnion(ctx, [][]model.SnapshotEntry{baseSnapshot.entries, headSnapshot.entries}, dataByBlob, cfg, opts.Stats) + facts, err := analyzeGitSnapshots(ctx, repo, []gitSnapshot{ + {ref: baseRef, entries: baseSnapshot.entries}, + {ref: headRef, entries: headSnapshot.entries}, + }, cfg, opts.Stats) if err != nil { return model.DiffReport{}, err } @@ -140,6 +141,7 @@ func Diff(ctx context.Context, repo, base, head, scopePath string, opts DiffOpti diff.Base = baseRef diff.Head = headRef diff.Scope = completeScope + annotateDiffState(&diff, cfg) return diff, nil } @@ -153,6 +155,13 @@ type blobAnalysis struct { duplicate map[model.BlobKey]duplicate.BlobFacts } +func newBlobAnalysis(size int) blobAnalysis { + return blobAnalysis{ + generic: make(map[model.BlobKey]model.BlobFacts, size), + duplicate: make(map[model.BlobKey]duplicate.BlobFacts, size), + } +} + func buildDetectedSnapshot(entries []model.TreeEntry, previews map[string][]byte) detectedSnapshot { return detectedSnapshot{ entries: detect.Entries(".", entries, previews), @@ -166,11 +175,11 @@ func DetectPath(ctx context.Context, root string, opts DetectOptions) (model.Det ctx = context.Background() } cfg := effectiveConfig(opts.Config) - entries, _, previews, err := walkWorkingTree(ctx, root, cfg) + snapshot, err := walkWorkingTree(ctx, root, cfg) if err != nil { return model.DetectedProject{}, err } - return detect.DetectProject(".", fileViews(entries, previews)), nil + return detect.DetectProject(".", fileViews(snapshot.entries, snapshot.previews)), nil } // HasRegression applies the configured aggregate diff thresholds. @@ -195,38 +204,40 @@ func HasRegression(diff model.DiffReport, cfg config.Config) bool { } type workingTreeSnapshot struct { - entries []model.TreeEntry - dataByBlob map[string][]byte - previews map[string][]byte + entries []model.TreeEntry + previews map[string][]byte } func newWorkingTreeSnapshot() workingTreeSnapshot { return workingTreeSnapshot{ - entries: make([]model.TreeEntry, 0), - dataByBlob: make(map[string][]byte), - previews: make(map[string][]byte), + entries: make([]model.TreeEntry, 0), + previews: make(map[string][]byte), } } -func walkWorkingTree(ctx context.Context, root string, cfg config.Config) ([]model.TreeEntry, map[string][]byte, map[string][]byte, error) { +func walkWorkingTree(ctx context.Context, root string, cfg config.Config) (workingTreeSnapshot, error) { if ctx == nil { ctx = context.Background() } if err := ctx.Err(); err != nil { - return nil, nil, nil, err + return workingTreeSnapshot{}, err } root = cleanScanRoot(root) + matcher, err := newIgnoreMatcher(root) + if err != nil { + return workingTreeSnapshot{}, err + } snapshot := newWorkingTreeSnapshot() - err := filepath.WalkDir(root, func(name string, entry fs.DirEntry, err error) error { - return snapshot.addWalkEntry(ctx, root, name, entry, err, cfg) + err = filepath.WalkDir(root, func(name string, entry fs.DirEntry, err error) error { + return snapshot.addWalkEntry(ctx, root, name, entry, err, cfg, matcher) }) if err != nil { - return nil, nil, nil, err + return workingTreeSnapshot{}, err } snapshot.sortEntries() - return snapshot.entries, snapshot.dataByBlob, snapshot.previews, nil + return snapshot, nil } func cleanScanRoot(root string) string { @@ -236,7 +247,7 @@ func cleanScanRoot(root string) string { return filepath.Clean(root) } -func (snapshot *workingTreeSnapshot) addWalkEntry(ctx context.Context, root, name string, entry fs.DirEntry, walkErr error, cfg config.Config) error { +func (snapshot *workingTreeSnapshot) addWalkEntry(ctx context.Context, root, name string, entry fs.DirEntry, walkErr error, cfg config.Config, matcher *ignoreMatcher) error { if err := ctx.Err(); err != nil { return err } @@ -248,7 +259,16 @@ func (snapshot *workingTreeSnapshot) addWalkEntry(ctx context.Context, root, nam return nil } if entry.IsDir() { - return skipIgnoredDir(entry) + if err := skipIgnoredDir(entry); err != nil { + return err + } + if matcher.ignored(rel, true) { + return filepath.SkipDir + } + return matcher.loadDir(name, rel) + } + if matcher.ignored(rel, false) { + return nil } info, skip, err := workingTreeFileInfo(name, entry, cfg) @@ -319,9 +339,6 @@ func (snapshot *workingTreeSnapshot) addReadableEntry(ctx context.Context, name, } sha := gitBlobSHA(data) - if _, ok := snapshot.dataByBlob[sha]; !ok { - snapshot.dataByBlob[sha] = data - } snapshot.entries = append(snapshot.entries, model.TreeEntry{Path: rel, BlobSHA: sha, Size: int64(len(data))}) snapshot.previews[rel] = preview(data) return nil @@ -333,160 +350,185 @@ func (snapshot *workingTreeSnapshot) sortEntries() { }) } -func readGitSnapshot(ctx context.Context, repo, ref string, entries []model.TreeEntry, cfg config.Config, stats *Stats) (map[string][]byte, map[string][]byte, error) { - dataByBlob := make(map[string][]byte) +func readGitPreviews(ctx context.Context, repo, ref string, entries []model.TreeEntry, cfg config.Config) (map[string][]byte, error) { previews := make(map[string][]byte, len(entries)) readable := entriesWithinMaxFileBytes(entries, cfg) if len(readable) == 0 { - return dataByBlob, previews, nil - } - if stats != nil { - stats.BatchedBlobReads++ + return previews, nil } err := gitlocal.ReadBlobs(ctx, repo, ref, readable, func(entry model.TreeEntry, data []byte) error { - if _, ok := dataByBlob[entry.BlobSHA]; !ok { - dataByBlob[entry.BlobSHA] = append([]byte(nil), data...) - } previews[entry.Path] = preview(data) return nil }) if err != nil { - return nil, nil, err + return nil, err } - return dataByBlob, previews, nil + return previews, nil } -func entriesWithinMaxFileBytes(entries []model.TreeEntry, cfg config.Config) []model.TreeEntry { - readable := make([]model.TreeEntry, 0, len(entries)) +func analyzeWorkingTreeSnapshot(ctx context.Context, root string, entries []model.SnapshotEntry, cfg config.Config, stats *Stats) (blobAnalysis, error) { + representatives := representativePaths(entries, cfg) + facts := newBlobAnalysis(len(representatives)) + keys := sortedBlobKeys(representatives) + for _, key := range keys { + if err := ctx.Err(); err != nil { + return blobAnalysis{}, err + } + data, err := os.ReadFile(filepath.Join(root, filepath.FromSlash(representatives[key]))) + if err != nil { + return blobAnalysis{}, err + } + addBlobFacts(&facts, key, data, cfg) + } + recordAnalysisStats(stats, len(keys), len(facts.generic), workersForSequential(len(keys))) + return facts, nil +} + +func representativePaths(entries []model.SnapshotEntry, cfg config.Config) map[model.BlobKey]string { + representatives := make(map[model.BlobKey]string) for _, entry := range entries { - if exceedsMaxFileBytes(entry.Size, cfg) { + if !builtInCandidateEntry(entry, cfg) { continue } - readable = append(readable, entry) + key := blobKey(entry, cfg) + if _, ok := representatives[key]; !ok { + representatives[key] = entry.Path + } } - return readable + return representatives } -func exceedsMaxFileBytes(size int64, cfg config.Config) bool { - return cfg.Scan.MaxFileBytes > 0 && size > cfg.Scan.MaxFileBytes +type gitSnapshot struct { + ref string + entries []model.SnapshotEntry } -func analyzeSnapshotUnion(ctx context.Context, groups [][]model.SnapshotEntry, dataByBlob map[string][]byte, cfg config.Config, stats *Stats) (blobAnalysis, error) { - keys := uniqueBlobKeys(groups, cfg) - if stats != nil { - stats.BuiltInBlobKeys = len(keys) - } - facts, workers, err := analyzeBlobKeys(ctx, keys, dataByBlob, cfg) - if stats != nil { - stats.WorkersUsed = workers - stats.BuiltInAnalyses = len(facts.generic) - } - return facts, err +type gitBlobRepresentative struct { + key model.BlobKey + entry model.TreeEntry } -func analyzeBlobKeys(ctx context.Context, keys []model.BlobKey, dataByBlob map[string][]byte, cfg config.Config) (blobAnalysis, int, error) { - if len(keys) == 0 { - return blobAnalysis{ - generic: map[model.BlobKey]model.BlobFacts{}, - duplicate: map[model.BlobKey]duplicate.BlobFacts{}, - }, 0, nil +func analyzeGitSnapshots(ctx context.Context, repo string, snapshots []gitSnapshot, cfg config.Config, stats *Stats) (blobAnalysis, error) { + byRef := make(map[string][]gitBlobRepresentative) + seen := make(map[model.BlobKey]struct{}) + for _, snapshot := range snapshots { + for _, entry := range snapshot.entries { + if !builtInCandidateEntry(entry, cfg) { + continue + } + key := blobKey(entry, cfg) + if _, ok := seen[key]; ok { + continue + } + seen[key] = struct{}{} + byRef[snapshot.ref] = append(byRef[snapshot.ref], gitBlobRepresentative{ + key: key, + entry: model.TreeEntry{Path: entry.Path, BlobSHA: entry.BlobSHA, Size: entry.Size}, + }) + } } - workers := boundedWorkers(cfg.Scan.Workers, len(keys)) - analyzer := newBlobAnalyzer(ctx, dataByBlob, cfg, len(keys)) - analyzer.run(keys, workers) - return blobAnalysis{generic: analyzer.genericFacts, duplicate: analyzer.duplicateFacts}, workers, analyzer.err() -} - -type blobAnalyzer struct { - ctx context.Context - dataByBlob map[string][]byte - genericCfg generic.Config - duplicateCfg duplicate.Config - catalog rules.Catalog - jobs chan model.BlobKey - genericFacts map[model.BlobKey]model.BlobFacts - duplicateFacts map[model.BlobKey]duplicate.BlobFacts - mu sync.Mutex - wg sync.WaitGroup - firstErr error + facts := newBlobAnalysis(len(seen)) + processedRefs := make(map[string]struct{}, len(byRef)) + for _, snapshot := range snapshots { + if _, ok := processedRefs[snapshot.ref]; ok { + continue + } + processedRefs[snapshot.ref] = struct{}{} + reps := byRef[snapshot.ref] + if len(reps) == 0 { + continue + } + if stats != nil { + stats.BatchedBlobReads++ + } + keyByEntry := make(map[model.TreeEntry]model.BlobKey, len(reps)) + entries := make([]model.TreeEntry, 0, len(reps)) + for _, rep := range reps { + keyByEntry[rep.entry] = rep.key + entries = append(entries, rep.entry) + } + if err := gitlocal.ReadBlobs(ctx, repo, snapshot.ref, entries, func(entry model.TreeEntry, data []byte) error { + addBlobFacts(&facts, keyByEntry[entry], data, cfg) + return nil + }); err != nil { + return blobAnalysis{}, err + } + } + recordAnalysisStats(stats, len(seen), len(facts.generic), workersForSequential(len(seen))) + return facts, nil } -func newBlobAnalyzer(ctx context.Context, dataByBlob map[string][]byte, cfg config.Config, keyCount int) *blobAnalyzer { - return &blobAnalyzer{ - ctx: ctx, - dataByBlob: dataByBlob, - genericCfg: genericConfig(cfg), - duplicateCfg: duplicateConfig(cfg), - catalog: rules.DefaultCatalog(), - jobs: make(chan model.BlobKey), - genericFacts: make(map[model.BlobKey]model.BlobFacts, keyCount), - duplicateFacts: make(map[model.BlobKey]duplicate.BlobFacts, keyCount), +func sortedBlobKeys(values map[model.BlobKey]string) []model.BlobKey { + keys := make([]model.BlobKey, 0, len(values)) + for key := range values { + keys = append(keys, key) } + sortBlobKeys(keys) + return keys } -func (analyzer *blobAnalyzer) run(keys []model.BlobKey, workers int) { - analyzer.startWorkers(workers) - analyzer.enqueue(keys) - analyzer.wg.Wait() +func sortBlobKeys(keys []model.BlobKey) { + sort.Slice(keys, func(i, j int) bool { + if keys[i].BlobSHA != keys[j].BlobSHA { + return keys[i].BlobSHA < keys[j].BlobSHA + } + if keys[i].Language != keys[j].Language { + return keys[i].Language < keys[j].Language + } + if keys[i].ConfigHash != keys[j].ConfigHash { + return keys[i].ConfigHash < keys[j].ConfigHash + } + return keys[i].RulesetVersion < keys[j].RulesetVersion + }) } -func (analyzer *blobAnalyzer) startWorkers(workers int) { - for i := 0; i < workers; i++ { - analyzer.wg.Add(1) - go analyzer.runWorker() +func workersForSequential(keys int) int { + if keys == 0 { + return 0 } + return 1 } -func (analyzer *blobAnalyzer) runWorker() { - defer analyzer.wg.Done() - for key := range analyzer.jobs { - analyzer.analyzeKey(key) +func recordAnalysisStats(stats *Stats, keys, analyses, workers int) { + if stats == nil { + return } + stats.BuiltInBlobKeys = keys + stats.BuiltInAnalyses = analyses + stats.WorkersUsed = workers } -func (analyzer *blobAnalyzer) enqueue(keys []model.BlobKey) { - for _, key := range keys { - if analyzer.err() != nil { - break +func entriesWithinMaxFileBytes(entries []model.TreeEntry, cfg config.Config) []model.TreeEntry { + readable := make([]model.TreeEntry, 0, len(entries)) + for _, entry := range entries { + if exceedsMaxFileBytes(entry.Size, cfg) { + continue } - analyzer.jobs <- key + readable = append(readable, entry) } - close(analyzer.jobs) + return readable } -func (analyzer *blobAnalyzer) analyzeKey(key model.BlobKey) { - if err := analyzer.ctx.Err(); err != nil { - analyzer.setErr(err) - return - } - - data, ok := analyzer.dataByBlob[key.BlobSHA] - if !ok { - analyzer.setErr(fmt.Errorf("blob %s content missing", key.BlobSHA)) - return - } - - genericFact := generic.AnalyzeBlob(key, data, analyzer.genericCfg, analyzer.catalog) - duplicateFact := duplicate.AnalyzeBlob(key.Language, data, analyzer.duplicateCfg) - analyzer.mu.Lock() - analyzer.genericFacts[key] = genericFact - analyzer.duplicateFacts[key] = duplicateFact - analyzer.mu.Unlock() +func exceedsMaxFileBytes(size int64, cfg config.Config) bool { + return cfg.Scan.MaxFileBytes > 0 && size > cfg.Scan.MaxFileBytes } -func (analyzer *blobAnalyzer) setErr(err error) { - analyzer.mu.Lock() - defer analyzer.mu.Unlock() - if analyzer.firstErr == nil { - analyzer.firstErr = err +func analyzeBlobData(key model.BlobKey, data []byte, cfg config.Config) (model.BlobFacts, duplicate.BlobFacts) { + start := time.Now() + genericFact := generic.AnalyzeBlob(key, data, genericConfig(cfg), rules.DefaultCatalog()) + duplicateFact := duplicate.AnalyzeBlob(key.Language, data, duplicateConfig(cfg)) + if analysisExceededBudget(start, fileAnalysisTimeout(cfg)) { + genericFact = model.BlobFacts{Skipped: true, SkipReason: model.IncompleteReasonFilesSkippedDueToTimeout} + duplicateFact = duplicate.BlobFacts{} } + return genericFact, duplicateFact } -func (analyzer *blobAnalyzer) err() error { - analyzer.mu.Lock() - defer analyzer.mu.Unlock() - return analyzer.firstErr +func addBlobFacts(facts *blobAnalysis, key model.BlobKey, data []byte, cfg config.Config) { + genericFact, duplicateFact := analyzeBlobData(key, data, cfg) + facts.generic[key] = genericFact + facts.duplicate[key] = duplicateFact } func buildReport(entries []model.SnapshotEntry, detection model.DetectedProject, facts blobAnalysis, cfg config.Config) model.ScanReport { @@ -499,6 +541,8 @@ func buildReport(entries []model.SnapshotEntry, detection model.DetectedProject, duplicateConfig(cfg), ) addDuplicateSummary(&report, duplicateSummary) + finalizeCommentStats(&report) + annotateScanCompleteness(&report) scoreReport(&report, cfg) return report } @@ -525,9 +569,15 @@ func addDetectedGroups(report *model.ScanReport, detection model.DetectedProject func addSnapshotFacts(report *model.ScanReport, entries []model.SnapshotEntry, facts map[model.BlobKey]model.BlobFacts, cfg config.Config) { for _, entry := range entries { - fact, ok := factForEntry(entry, facts, cfg) + fact, ok, skippedDueToSize, skippedDueToTimeout := factForEntry(entry, facts, cfg) if !ok { report.Totals.FilesSkipped++ + if skippedDueToSize { + report.Totals.FilesSkippedDueToSize++ + } + if skippedDueToTimeout { + report.Totals.FilesSkippedDueToTimeout++ + } continue } addFactToReport(report, entry, fact) @@ -632,27 +682,84 @@ func allocateIssueCounts(raw map[string]int, total int) map[string]int { return out } -func factForEntry(entry model.SnapshotEntry, facts map[model.BlobKey]model.BlobFacts, cfg config.Config) (model.BlobFacts, bool) { - if !builtInCandidateEntry(entry, cfg) { - return model.BlobFacts{}, false +func factForEntry(entry model.SnapshotEntry, facts map[model.BlobKey]model.BlobFacts, cfg config.Config) (model.BlobFacts, bool, bool, bool) { + if shouldSkipPath(entry.Path) || shouldExcludeTestPath(entry.Path, cfg) { + return model.BlobFacts{}, false, false, false + } + if exceedsMaxFileBytes(entry.Size, cfg) { + return model.BlobFacts{}, false, true, false + } + if entry.BlobSHA == "" { + return model.BlobFacts{}, false, false, false } fact, ok := facts[blobKey(entry, cfg)] - if !ok || fact.Skipped { - return model.BlobFacts{}, false + if !ok { + return model.BlobFacts{}, false, false, false } - return fact, true + if fact.Skipped { + return model.BlobFacts{}, false, false, fact.SkipReason == model.IncompleteReasonFilesSkippedDueToTimeout + } + return fact, true, false, false } func scoreReport(report *model.ScanReport, cfg config.Config) { scored := score.Calculate(report.Totals, scoreConfig(cfg)) report.Score = scored.Score report.Band = scored.Band + report.Thresholds = model.ScanThresholds{FailUnder: cfg.Score.FailUnder} + report.Passed = report.Score >= cfg.Score.FailUnder + report.Status = reportStatus(report.Passed) +} + +func annotateScanCompleteness(report *model.ScanReport) { + if report.Totals.FilesSkippedDueToSize == 0 && report.Totals.FilesSkippedDueToTimeout == 0 { + return + } + report.Incomplete = true + if report.Totals.FilesSkippedDueToSize > 0 { + report.IncompleteReasons = appendIncompleteReason(report.IncompleteReasons, model.IncompleteReasonFilesSkippedDueToSize) + } + if report.Totals.FilesSkippedDueToTimeout > 0 { + report.IncompleteReasons = appendIncompleteReason(report.IncompleteReasons, model.IncompleteReasonFilesSkippedDueToTimeout) + } +} + +func appendIncompleteReason(reasons []string, reason string) []string { + for _, existing := range reasons { + if existing == reason { + return reasons + } + } + return append(reasons, reason) +} + +func annotateDiffState(diff *model.DiffReport, cfg config.Config) { + diff.Thresholds = model.DiffThresholds{ + FailOnScoreDrop: cfg.Diff.FailOnScoreDrop, + MaxAllowedNewErrors: cfg.Diff.MaxAllowedNewErrors, + CategoryRegressionThreshold: cfg.Diff.CategoryRegressionThreshold, + } + diff.Regression = HasRegression(*diff, cfg) + diff.Passed = !diff.Regression + diff.Status = reportStatus(diff.Passed) +} + +func reportStatus(passed bool) model.ReportStatus { + if passed { + return model.ReportStatusPassed + } + return model.ReportStatusFailed } func addFactToReport(report *model.ScanReport, entry model.SnapshotEntry, fact model.BlobFacts) { issues := issueTotal(fact.Issues) report.Totals.FilesScanned++ report.Totals.LinesScanned += fact.Lines + report.Totals.BlankLines += fact.BlankLines + report.Totals.CodeLines += fact.CodeLines + report.Totals.CommentLines += fact.CommentLines + report.Totals.CommentOnlyLines += fact.CommentOnlyLines + report.Totals.InlineCommentLines += fact.InlineCommentLines report.Totals.Errors += fact.Issues.Errors report.Totals.Warnings += fact.Issues.Warnings report.Totals.Info += fact.Issues.Info @@ -694,43 +801,31 @@ func addFactToReport(report *model.ScanReport, entry model.SnapshotEntry, fact m func addGroupFact(group *model.GroupCounts, fact model.BlobFacts, issues int) { group.Files++ group.Lines += fact.Lines + group.BlankLines += fact.BlankLines + group.CodeLines += fact.CodeLines + group.CommentLines += fact.CommentLines + group.CommentOnlyLines += fact.CommentOnlyLines + group.InlineCommentLines += fact.InlineCommentLines group.Issues += issues group.Errors += fact.Issues.Errors group.Warnings += fact.Issues.Warnings group.Info += fact.Issues.Info } -func uniqueBlobKeys(groups [][]model.SnapshotEntry, cfg config.Config) []model.BlobKey { - seen := make(map[model.BlobKey]bool) - for _, entries := range groups { - for _, entry := range entries { - if !builtInCandidateEntry(entry, cfg) { - continue - } - seen[blobKey(entry, cfg)] = true - } +func finalizeCommentStats(report *model.ScanReport) { + report.Totals.CommentDensityPercent = model.LineDensityPercent(report.Totals.CommentLines, report.Totals.LinesScanned) + for language, group := range report.ByLanguage { + group.CommentDensityPercent = model.LineDensityPercent(group.CommentLines, group.Lines) + report.ByLanguage[language] = group } - keys := make([]model.BlobKey, 0, len(seen)) - for key := range seen { - keys = append(keys, key) + for module, group := range report.ByModule { + group.CommentDensityPercent = model.LineDensityPercent(group.CommentLines, group.Lines) + report.ByModule[module] = group } - sort.Slice(keys, func(i, j int) bool { - if keys[i].BlobSHA != keys[j].BlobSHA { - return keys[i].BlobSHA < keys[j].BlobSHA - } - if keys[i].Language != keys[j].Language { - return keys[i].Language < keys[j].Language - } - if keys[i].ConfigHash != keys[j].ConfigHash { - return keys[i].ConfigHash < keys[j].ConfigHash - } - return keys[i].RulesetVersion < keys[j].RulesetVersion - }) - return keys } func builtInCandidateEntry(entry model.SnapshotEntry, cfg config.Config) bool { - return entry.BlobSHA != "" && !shouldSkipPath(entry.Path) && !exceedsMaxFileBytes(entry.Size, cfg) + return entry.BlobSHA != "" && !shouldSkipPath(entry.Path) && !shouldExcludeTestPath(entry.Path, cfg) && !exceedsMaxFileBytes(entry.Size, cfg) } func blobKey(entry model.SnapshotEntry, cfg config.Config) model.BlobKey { @@ -745,19 +840,6 @@ func fileViews(entries []model.TreeEntry, previews map[string][]byte) []detect.F return views } -func mergeBlobData(left, right map[string][]byte) map[string][]byte { - merged := make(map[string][]byte, len(left)+len(right)) - for key, value := range left { - merged[key] = value - } - for key, value := range right { - if _, ok := merged[key]; !ok { - merged[key] = value - } - } - return merged -} - func preview(data []byte) []byte { if len(data) > previewByteSize { data = data[:previewByteSize] @@ -767,7 +849,7 @@ func preview(data []byte) []byte { func configHash(cfg config.Config) string { h := sha1.New() - fmt.Fprintf(h, "scan:%d:%t:%d|score:%d:%d:%d:%d", cfg.Scan.MaxFileBytes, cfg.Scan.FollowSymlinks, cfg.Scan.Workers, cfg.Score.FailUnder, cfg.Score.Error, cfg.Score.Warning, cfg.Score.Info) + fmt.Fprintf(h, "scan:%d:%d:%t:%d|score:%d:%d:%d:%d", cfg.Scan.MaxFileAnalysisMilliseconds, cfg.Scan.MaxFileBytes, cfg.Scan.FollowSymlinks, cfg.Scan.Workers, cfg.Score.FailUnder, cfg.Score.Error, cfg.Score.Warning, cfg.Score.Info) return hex.EncodeToString(h.Sum(nil))[:12] } @@ -783,28 +865,35 @@ func effectiveConfig(cfg config.Config) config.Config { if cfg.Scan.DefaultTimeoutSeconds == 0 { cfg.Scan.DefaultTimeoutSeconds = defaults.Scan.DefaultTimeoutSeconds } + if cfg.Scan.MaxFileAnalysisMilliseconds == 0 { + cfg.Scan.MaxFileAnalysisMilliseconds = defaults.Scan.MaxFileAnalysisMilliseconds + } if cfg.Scan.MaxFileBytes == 0 { cfg.Scan.MaxFileBytes = defaults.Scan.MaxFileBytes } - if cfg.Score.FailUnder == 0 { - cfg.Score.FailUnder = defaults.Score.FailUnder - } - if cfg.Score.Error == 0 { - cfg.Score.Error = defaults.Score.Error - } - if cfg.Score.Warning == 0 { - cfg.Score.Warning = defaults.Score.Warning - } - if cfg.Score.Info == 0 { - cfg.Score.Info = defaults.Score.Info - } - if cfg.Diff.MaxAllowedNewErrors == 0 { - cfg.Diff.MaxAllowedNewErrors = defaults.Diff.MaxAllowedNewErrors + if cfg.Score == (config.ScoreConfig{}) { + cfg.Score = defaults.Score + } else { + if cfg.Score.Error == 0 { + cfg.Score.Error = defaults.Score.Error + } + if cfg.Score.Warning == 0 { + cfg.Score.Warning = defaults.Score.Warning + } + if cfg.Score.Info == 0 { + cfg.Score.Info = defaults.Score.Info + } } - if cfg.Diff.CategoryRegressionThreshold == 0 { - cfg.Diff.CategoryRegressionThreshold = defaults.Diff.CategoryRegressionThreshold + if cfg.Diff == (config.DiffConfig{}) { + cfg.Diff = defaults.Diff + } else { + if cfg.Diff.MaxAllowedNewErrors == 0 { + cfg.Diff.MaxAllowedNewErrors = defaults.Diff.MaxAllowedNewErrors + } + if cfg.Diff.CategoryRegressionThreshold == 0 { + cfg.Diff.CategoryRegressionThreshold = defaults.Diff.CategoryRegressionThreshold + } } - cfg.Diff.FailOnScoreDrop = cfg.Diff.FailOnScoreDrop || defaults.Diff.FailOnScoreDrop return cfg } @@ -816,6 +905,17 @@ func genericConfig(cfg config.Config) generic.Config { return genericCfg } +func fileAnalysisTimeout(cfg config.Config) time.Duration { + if cfg.Scan.MaxFileAnalysisMilliseconds <= 0 { + return 0 + } + return time.Duration(cfg.Scan.MaxFileAnalysisMilliseconds) * time.Millisecond +} + +func analysisExceededBudget(start time.Time, max time.Duration) bool { + return max > 0 && time.Since(start) > max +} + func duplicateConfig(cfg config.Config) duplicate.Config { return duplicate.DefaultConfig() } @@ -838,23 +938,6 @@ func gitScopePath(scopePath string) string { return scopePath } -func boundedWorkers(configured, workItems int) int { - if workItems <= 0 { - return 0 - } - workers := configured - if workers <= 0 { - workers = runtime.NumCPU() - } - if workers < 1 { - workers = 1 - } - if workers > workItems { - workers = workItems - } - return workers -} - func issueTotal(counts model.IssueCounts) int { return counts.Errors + counts.Warnings + counts.Info } diff --git a/internal/scan/scan_test.go b/internal/scan/scan_test.go index f3260e8..bd1ece0 100644 --- a/internal/scan/scan_test.go +++ b/internal/scan/scan_test.go @@ -33,6 +33,12 @@ func TestDiffScansCompleteSnapshotsAndAggregatesDeltas(t *testing.T) { if diff.ScoreDelta >= 0 || diff.ScoreAfter >= diff.ScoreBefore { t.Fatalf("score delta = before %d after %d delta %d, want regression", diff.ScoreBefore, diff.ScoreAfter, diff.ScoreDelta) } + if !diff.Regression || diff.Passed || diff.Status != "failed" { + t.Fatalf("state = status %q passed %t regression %t, want failed regression", diff.Status, diff.Passed, diff.Regression) + } + if !diff.Thresholds.FailOnScoreDrop || diff.Thresholds.MaxAllowedNewErrors != 0 || diff.Thresholds.CategoryRegressionThreshold != 1 { + t.Fatalf("thresholds = %#v, want default diff regression thresholds", diff.Thresholds) + } if got := diff.SeverityDelta[model.SeverityError]; got.Delta != 1 || got.Before != 0 || got.After != 1 { t.Fatalf("error severity delta = %#v, want 0 -> 1", got) } @@ -71,6 +77,46 @@ func TestDiffScansCompleteSnapshotsAndAggregatesDeltas(t *testing.T) { } } +func TestScanPathReportsQualityGateState(t *testing.T) { + root := t.TempDir() + writeRepoFile(t, root, "main.go", "package main\n// "+strings.Repeat("x", 200)+"\n") + + cfg := config.Default() + cfg.Score.FailUnder = 100 + report, err := ScanPath(context.Background(), root, ScanOptions{Config: cfg}) + if err != nil { + t.Fatalf("ScanPath() error = %v", err) + } + if report.Score >= 100 { + t.Fatalf("score = %d, want fixture below perfect score", report.Score) + } + if report.Passed || report.Status != "failed" { + t.Fatalf("state = status %q passed %t, want failed", report.Status, report.Passed) + } + if report.Thresholds.FailUnder != 100 { + t.Fatalf("fail_under = %d, want configured threshold", report.Thresholds.FailUnder) + } +} + +func TestScanPathAnalyzesDuplicateWorkingTreeBlobsOnce(t *testing.T) { + root := t.TempDir() + content := "package main\nfunc main() {}\n" + writeRepoFile(t, root, "a.go", content) + writeRepoFile(t, root, "b.go", content) + + var stats Stats + report, err := ScanPath(context.Background(), root, ScanOptions{Stats: &stats}) + if err != nil { + t.Fatalf("ScanPath() error = %v", err) + } + if report.Totals.FilesScanned != 2 { + t.Fatalf("FilesScanned = %d, want two file occurrences", report.Totals.FilesScanned) + } + if stats.BuiltInBlobKeys != 1 || stats.BuiltInAnalyses != 1 { + t.Fatalf("analysis stats = %#v, want one unique blob analyzed once", stats) + } +} + func TestScanPathReportsAggregateJSONParseErrors(t *testing.T) { root := t.TempDir() writeRepoFile(t, root, "app.json", "{\"name\":") @@ -96,6 +142,45 @@ func TestScanPathReportsAggregateJSONParseErrors(t *testing.T) { } } +func TestScanPathReportsCommentLineStats(t *testing.T) { + root := t.TempDir() + writeRepoFile(t, root, "main.go", strings.Join([]string{ + "package main", + "// file comment", + "func main() {", + "println(\"ok\") // inline comment", + "/* block comment */", + "}", + }, "\n")+"\n") + + report, err := ScanPath(context.Background(), root, ScanOptions{}) + if err != nil { + t.Fatalf("ScanPath() error = %v", err) + } + payload, err := json.Marshal(report) + if err != nil { + t.Fatal(err) + } + var decoded map[string]any + if err := json.Unmarshal(payload, &decoded); err != nil { + t.Fatalf("unmarshal report JSON: %v", err) + } + assertJSONNumber(t, decoded, "totals.blank_lines", 0) + assertJSONNumber(t, decoded, "totals.code_lines", 4) + assertJSONNumber(t, decoded, "totals.comment_lines", 3) + assertJSONNumber(t, decoded, "totals.comment_only_lines", 2) + assertJSONNumber(t, decoded, "totals.inline_comment_lines", 1) + assertJSONNumber(t, decoded, "totals.comment_density_percent", 50) + assertJSONNumber(t, decoded, "by_language.go.comment_lines", 3) + assertJSONNumber(t, decoded, "by_language.go.comment_only_lines", 2) + assertJSONNumber(t, decoded, "by_language.go.inline_comment_lines", 1) + assertJSONNumber(t, decoded, "by_language.go.comment_density_percent", 50) + assertJSONNumberPath(t, decoded, []string{"by_module", ".", "comment_lines"}, 3) + assertJSONNumberPath(t, decoded, []string{"by_module", ".", "comment_only_lines"}, 2) + assertJSONNumberPath(t, decoded, []string{"by_module", ".", "inline_comment_lines"}, 1) + assertJSONNumberPath(t, decoded, []string{"by_module", ".", "comment_density_percent"}, 50) +} + func TestScanPathAggregatesGoParseAndComplexityAggregateOnly(t *testing.T) { root := t.TempDir() writeRepoFile(t, root, "services/api/go.mod", "module example.test/api\n\ngo 1.26\n") @@ -255,6 +340,41 @@ func TestDiffReportsDuplicateDeltaFromCompleteSnapshots(t *testing.T) { } } +func TestDiffStatusHonorsDisabledScoreDropGate(t *testing.T) { + repo := t.TempDir() + runGit(t, repo, "init") + runGit(t, repo, "config", "user.email", "scanner@example.test") + runGit(t, repo, "config", "user.name", "Scanner Test") + + writeRepoFile(t, repo, "main.go", "package main\n") + runGit(t, repo, "add", ".") + runGit(t, repo, "commit", "-m", "base") + base := runGitOutput(t, repo, "rev-parse", "HEAD") + + writeRepoFile(t, repo, "main.go", "package main\n// "+strings.Repeat("x", 200)+"\n") + runGit(t, repo, "add", ".") + runGit(t, repo, "commit", "-m", "head") + head := runGitOutput(t, repo, "rev-parse", "HEAD") + + cfg := config.Default() + cfg.Diff.FailOnScoreDrop = false + cfg.Diff.MaxAllowedNewErrors = 99 + cfg.Diff.CategoryRegressionThreshold = 99 + diff, err := Diff(context.Background(), repo, base, head, ".", DiffOptions{Config: cfg}) + if err != nil { + t.Fatalf("Diff() error = %v", err) + } + if diff.ScoreDelta >= 0 { + t.Fatalf("score delta = %d, want score drop fixture", diff.ScoreDelta) + } + if diff.Regression || !diff.Passed || diff.Status != "passed" { + t.Fatalf("state = status %q passed %t regression %t, want passed when score-drop gate is disabled", diff.Status, diff.Passed, diff.Regression) + } + if diff.Thresholds.FailOnScoreDrop { + t.Fatalf("thresholds = %#v, want score-drop gate disabled", diff.Thresholds) + } +} + func TestScanPathIgnoresSkippedEntriesDuringDuplicateAggregation(t *testing.T) { root := t.TempDir() content := strings.Join([]string{ @@ -291,6 +411,75 @@ func TestScanPathIgnoresSkippedEntriesDuringDuplicateAggregation(t *testing.T) { t.Fatalf("issue totals = %#v maintainability = %#v, want no issues from skipped duplicate occurrence", report.Totals, report.ByCategory[model.CategoryMaintainability]) } } +func TestScanPathHonorsGitignorePatterns(t *testing.T) { + root := t.TempDir() + writeRepoFile(t, root, ".gitignore", strings.Join([]string{ + "ignored/", + "*.skip.go", + "!important.skip.go", + "/root-only.go", + "", + }, "\n")) + writeRepoFile(t, root, "main.go", "package main\n") + writeRepoFile(t, root, "ignored/broken.go", "package p\nfunc broken( {\n") + writeRepoFile(t, root, "broken.skip.go", "package p\nfunc broken( {\n") + writeRepoFile(t, root, "important.skip.go", "package p\nfunc broken( {\n") + writeRepoFile(t, root, "root-only.go", "package p\nfunc broken( {\n") + writeRepoFile(t, root, "nested/root-only.go", "package p\nfunc broken( {\n") + + report, err := ScanPath(context.Background(), root, ScanOptions{}) + if err != nil { + t.Fatalf("ScanPath() error = %v", err) + } + if report.Totals.FilesScanned != 3 { + t.Fatalf("files scanned = %d, want main.go, important.skip.go, and nested/root-only.go", report.Totals.FilesScanned) + } + if report.Totals.Errors != 2 { + t.Fatalf("errors = %d, want only the unignored parse errors", report.Totals.Errors) + } +} + +func TestScanPathHonorsNestedIgnoreFiles(t *testing.T) { + root := t.TempDir() + writeRepoFile(t, root, ".ignore", "*.generated.go\n") + writeRepoFile(t, root, "pkg/.gitignore", "*.tmp.go\n!keep.tmp.go\n") + writeRepoFile(t, root, "pkg/main.go", "package pkg\n") + writeRepoFile(t, root, "pkg/drop.tmp.go", "package p\nfunc broken( {\n") + writeRepoFile(t, root, "pkg/keep.tmp.go", "package p\nfunc broken( {\n") + writeRepoFile(t, root, "root.generated.go", "package p\nfunc broken( {\n") + + report, err := ScanPath(context.Background(), root, ScanOptions{}) + if err != nil { + t.Fatalf("ScanPath() error = %v", err) + } + if report.Totals.FilesScanned != 2 { + t.Fatalf("files scanned = %d, want pkg/main.go and pkg/keep.tmp.go", report.Totals.FilesScanned) + } + if report.Totals.Errors != 1 { + t.Fatalf("errors = %d, want only the unignored nested parse error", report.Totals.Errors) + } +} + +func TestScanPathLetsGitignoreOverrideGitInfoExclude(t *testing.T) { + root := t.TempDir() + writeRepoFile(t, root, ".git/info/exclude", "*.local.go\n") + writeRepoFile(t, root, ".gitignore", "!keep.local.go\n") + writeRepoFile(t, root, "main.go", "package main\n") + writeRepoFile(t, root, "drop.local.go", "package p\nfunc broken( {\n") + writeRepoFile(t, root, "keep.local.go", "package p\nfunc broken( {\n") + + report, err := ScanPath(context.Background(), root, ScanOptions{}) + if err != nil { + t.Fatalf("ScanPath() error = %v", err) + } + if report.Totals.FilesScanned != 2 { + t.Fatalf("files scanned = %d, want main.go and keep.local.go", report.Totals.FilesScanned) + } + if report.Totals.Errors != 1 { + t.Fatalf("errors = %d, want only the unignored local parse error", report.Totals.Errors) + } +} + func TestScanPathIgnoresClaudeWorktrees(t *testing.T) { root := t.TempDir() writeRepoFile(t, root, ".claude/worktrees/branch/go.mod", "module example.com/worktree\n\ngo 1.22\n") @@ -335,6 +524,52 @@ func TestScanPathSkipsOversizedWorkingTreeFileBeforeRead(t *testing.T) { if report.Totals.FilesScanned != 1 || report.Totals.FilesSkipped != 1 || report.Totals.LinesScanned != 1 { t.Fatalf("totals = %#v, want oversized file skipped by metadata before read", report.Totals) } + if !report.Incomplete { + t.Fatal("Incomplete = false, want true when a file is skipped due to size") + } + if report.Totals.FilesSkippedDueToSize != 1 { + t.Fatalf("FilesSkippedDueToSize = %d, want 1", report.Totals.FilesSkippedDueToSize) + } + if !containsString(report.IncompleteReasons, "files_skipped_due_to_size") { + t.Fatalf("IncompleteReasons = %#v, want files_skipped_due_to_size", report.IncompleteReasons) + } +} + +func TestFactForEntryClassifiesAnalysisTimeoutSkip(t *testing.T) { + cfg := effectiveConfig(config.Default()) + entry := model.SnapshotEntry{Path: "main.go", BlobSHA: "sha", Size: 12, Language: "go", Module: "."} + facts := map[model.BlobKey]model.BlobFacts{ + blobKey(entry, cfg): { + Skipped: true, + SkipReason: model.IncompleteReasonFilesSkippedDueToTimeout, + }, + } + + _, ok, skippedDueToSize, skippedDueToTimeout := factForEntry(entry, facts, cfg) + if ok { + t.Fatal("factForEntry() ok = true, want timeout-skipped blob to be skipped") + } + if skippedDueToSize { + t.Fatal("skippedDueToSize = true, want false") + } + if !skippedDueToTimeout { + t.Fatal("skippedDueToTimeout = false, want true") + } +} + +func TestAnnotateScanCompletenessIncludesAnalysisTimeout(t *testing.T) { + report := model.ScanReport{ + Totals: model.ScanTotals{FilesSkippedDueToTimeout: 1}, + } + + annotateScanCompleteness(&report) + + if !report.Incomplete { + t.Fatal("Incomplete = false, want true") + } + if !containsString(report.IncompleteReasons, model.IncompleteReasonFilesSkippedDueToTimeout) { + t.Fatalf("IncompleteReasons = %#v, want %q", report.IncompleteReasons, model.IncompleteReasonFilesSkippedDueToTimeout) + } } func TestScanPathHonorsCanceledContextBeforeWorkingTreeReads(t *testing.T) { @@ -406,6 +641,15 @@ func TestDiffSkipsOversizedGitBlobBeforeReadBlobs(t *testing.T) { if got := diff.TotalsDelta.FilesSkipped; got.Before != 0 || got.After != 1 || got.Delta != 1 { t.Fatalf("files skipped delta = %#v, want oversized head blob counted skipped", got) } + if got := diff.TotalsDelta.FilesSkippedDueToSize; got.Before != 0 || got.After != 1 || got.Delta != 1 { + t.Fatalf("files skipped due to size delta = %#v, want oversized head blob counted by size reason", got) + } + if !diff.Incomplete { + t.Fatal("Incomplete = false, want true when either snapshot skips files due to size") + } + if !containsString(diff.IncompleteReasons, "files_skipped_due_to_size") { + t.Fatalf("IncompleteReasons = %#v, want files_skipped_due_to_size", diff.IncompleteReasons) + } if got := diff.TotalsDelta.LinesScanned; got.Before != 1 || got.After != 0 || got.Delta != -1 { t.Fatalf("lines scanned delta = %#v, want oversized head blob not read for lines", got) } @@ -475,6 +719,35 @@ func writeRepoFile(t *testing.T, repo, name, content string) { } } +func assertJSONNumber(t *testing.T, root map[string]any, dottedPath string, want float64) { + t.Helper() + assertJSONNumberPath(t, root, strings.Split(dottedPath, "."), want) +} + +func assertJSONNumberPath(t *testing.T, root map[string]any, path []string, want float64) { + t.Helper() + var current any = root + for _, key := range path { + object, ok := current.(map[string]any) + if !ok { + t.Fatalf("%s parent = %#v, want object", strings.Join(path, "."), current) + } + current = object[key] + } + if current != want { + t.Fatalf("%s = %#v, want %.1f", strings.Join(path, "."), current, want) + } +} + +func containsString(values []string, want string) bool { + for _, value := range values { + if value == want { + return true + } + } + return false +} + func listenUnixSocket(t *testing.T, path string) net.Listener { t.Helper() listener, err := net.Listen("unix", path) diff --git a/internal/scan/testpath.go b/internal/scan/testpath.go new file mode 100644 index 0000000..bf1ee6d --- /dev/null +++ b/internal/scan/testpath.go @@ -0,0 +1,74 @@ +package scan + +import ( + "path/filepath" + "strings" + + "github.com/randomcodespace/code-signal/internal/config" +) + +func shouldExcludeTestPath(name string, cfg config.Config) bool { + if !cfg.Scan.ExcludeTests { + return false + } + return isWellKnownTestPath(name) +} + +func isWellKnownTestPath(name string) bool { + slashPath := filepath.ToSlash(name) + clean := strings.ToLower(slashPath) + parts := strings.Split(clean, "/") + base := parts[len(parts)-1] + originalBase := slashPath + if index := strings.LastIndexByte(slashPath, '/'); index >= 0 { + originalBase = slashPath[index+1:] + } + + if hasTestDirectory(parts) { + return true + } + return isGoTestFile(base) || isJavaTestFile(originalBase) || isPythonTestFile(base) || isJavaScriptTestFile(base) || isRustTestFile(base) +} + +func hasTestDirectory(parts []string) bool { + for i, part := range parts[:max(0, len(parts)-1)] { + switch part { + case "test", "tests", "testdata", "__tests__", "e2e", "cypress", "playwright", "benches": + return true + case "src": + if i+1 < len(parts)-1 && (parts[i+1] == "test" || parts[i+1] == "integrationtest") { + return true + } + } + } + return false +} + +func isGoTestFile(base string) bool { + return strings.HasSuffix(base, "_test.go") +} + +func isJavaTestFile(base string) bool { + if !strings.HasSuffix(base, ".java") { + return false + } + className := strings.TrimSuffix(base, ".java") + return strings.HasSuffix(className, "Test") || strings.HasSuffix(className, "Tests") || strings.HasSuffix(className, "TestCase") || strings.HasSuffix(className, "IT") || strings.HasSuffix(className, "ITCase") +} + +func isPythonTestFile(base string) bool { + return base == "conftest.py" || strings.HasPrefix(base, "test_") && strings.HasSuffix(base, ".py") || strings.HasSuffix(base, "_test.py") +} + +func isJavaScriptTestFile(base string) bool { + switch filepath.Ext(base) { + case ".js", ".jsx", ".mjs", ".cjs", ".ts", ".tsx", ".mts", ".cts": + return strings.Contains(base, ".test.") || strings.Contains(base, ".spec.") + default: + return false + } +} + +func isRustTestFile(base string) bool { + return strings.HasSuffix(base, "_test.rs") || strings.HasSuffix(base, "_tests.rs") +} diff --git a/internal/scan/testpath_test.go b/internal/scan/testpath_test.go new file mode 100644 index 0000000..61291c9 --- /dev/null +++ b/internal/scan/testpath_test.go @@ -0,0 +1,41 @@ +package scan + +import "testing" + +func TestIsWellKnownTestPathMatchesLanguageDefaults(t *testing.T) { + for _, name := range []string{ + "cmd/main_test.go", + "testdata/golden.go", + "src/test/java/AppTest.java", + "src/integrationTest/java/AppIT.java", + "tests/test_app.py", + "pkg/test_module.py", + "web/src/__tests__/app.js", + "web/src/app.spec.ts", + "web/src/app.test.tsx", + "web/e2e/login.ts", + "crates/core/tests/integration.rs", + "crates/core/benches/throughput.rs", + "crates/core/src/parser_test.rs", + } { + if !isWellKnownTestPath(name) { + t.Fatalf("isWellKnownTestPath(%q) = false, want true", name) + } + } +} + +func TestIsWellKnownTestPathAvoidsSourceFalsePositives(t *testing.T) { + for _, name := range []string{ + "src/main.go", + "src/main/java/Credit.java", + "src/main/java/Contest.java", + "src/main/python/contest.py", + "web/src/spectator.ts", + "web/src/latest.ts", + "crates/core/src/lib.rs", + } { + if isWellKnownTestPath(name) { + t.Fatalf("isWellKnownTestPath(%q) = true, want false", name) + } + } +} diff --git a/internal/score/score.go b/internal/score/score.go index 3ecf4bd..63486af 100644 --- a/internal/score/score.go +++ b/internal/score/score.go @@ -3,13 +3,14 @@ package score import "github.com/randomcodespace/code-signal/internal/model" const ( - defaultErrorDeduction = 10 - defaultWarningDeduction = 4 - defaultInfoDeduction = 1 - defaultAnyErrorCap = 89 - defaultManyErrorsThreshold = 5 - defaultManyErrorsCap = 69 - minimumScoredLines = 1000 + defaultErrorDeduction = 10 + defaultWarningDeduction = 4 + defaultInfoDeduction = 1 + defaultAnyErrorCap = 89 + defaultManyErrorsThreshold = 5 + defaultManyErrorsCap = 69 + defaultTargetDensityPerKLOC = 100 + minimumScoredLines = 1000 ) // Config contains package-local scoring options. It intentionally does not @@ -36,13 +37,13 @@ func DefaultConfig() Config { } // Calculate returns the aggregate quality score for one complete scan. Issue -// deductions are normalized by scanned lines so large repositories are scored by -// issue density, not absolute size. +// weights are normalized by scanned lines, then converted through a bounded +// density curve so noisy repositories remain comparable instead of collapsing +// to zero once they cross a fixed linear threshold. func Calculate(totals model.ScanTotals, cfg Config) model.Score { cfg = cfg.withDefaults() - value := 100 - weightedDeductionPerKLOC(totals, cfg) - + value := boundedDensityScore(weightedDensityPerKLOC(totals, cfg), defaultTargetDensityPerKLOC) if totals.Errors >= cfg.ManyErrorsThreshold { value = capScore(value, cfg.ManyErrorsCap) } else if totals.Errors > 0 { @@ -91,20 +92,37 @@ func (cfg Config) withDefaults() Config { return cfg } -func weightedDeductionPerKLOC(totals model.ScanTotals, cfg Config) int { +func weightedDensityPerKLOC(totals model.ScanTotals, cfg Config) int { weighted := int64(totals.Errors)*int64(cfg.ErrorDeduction) + int64(totals.Warnings)*int64(cfg.WarningDeduction) + int64(totals.Info)*int64(cfg.InfoDeduction) if weighted <= 0 { return 0 } - lines := int64(totals.LinesScanned) + lines := int64(scoringLines(totals)) if lines < minimumScoredLines { lines = minimumScoredLines } return int((weighted*1000 + lines/2) / lines) } +func scoringLines(totals model.ScanTotals) int { + if totals.CodeLines > 0 { + return totals.CodeLines + } + return totals.LinesScanned +} + +func boundedDensityScore(density int, target int) int { + if density <= 0 { + return 100 + } + if target <= 0 { + target = defaultTargetDensityPerKLOC + } + return int((int64(100*target) + int64(target+density)/2) / int64(target+density)) +} + func capScore(score, cap int) int { if score > cap { return cap diff --git a/internal/score/score_test.go b/internal/score/score_test.go index e02ffc6..6b37b23 100644 --- a/internal/score/score_test.go +++ b/internal/score/score_test.go @@ -8,11 +8,11 @@ import ( func TestCalculateAppliesDefaultDeductions(t *testing.T) { got := Calculate(model.ScanTotals{Warnings: 2, Info: 3}, Config{}) - if got.Score != 89 { - t.Fatalf("Score = %d, want 89", got.Score) + if got.Score != 90 { + t.Fatalf("Score = %d, want 90", got.Score) } - if got.Band != "good" { - t.Fatalf("Band = %q, want good", got.Band) + if got.Band != "excellent" { + t.Fatalf("Band = %q, want excellent", got.Band) } } @@ -26,6 +26,36 @@ func TestCalculateNormalizesDeductionsByScannedLines(t *testing.T) { } } +func TestCalculateNormalizesDeductionsByCodeLinesWhenAvailable(t *testing.T) { + got := Calculate(model.ScanTotals{LinesScanned: 10_000, CodeLines: 5_000, Warnings: 125}, Config{}) + if got.Score != 50 { + t.Fatalf("Score = %d, want 50", got.Score) + } + if got.Band != "needs work" { + t.Fatalf("Band = %q, want needs work", got.Band) + } +} + +func TestCalculateKeepsHighDensityScoresUseful(t *testing.T) { + got := Calculate(model.ScanTotals{LinesScanned: 10_000, Warnings: 250}, Config{}) + if got.Score != 50 { + t.Fatalf("Score = %d, want 50", got.Score) + } + if got.Band != "needs work" { + t.Fatalf("Band = %q, want needs work", got.Band) + } +} + +func TestCalculateKeepsDensityMonotonic(t *testing.T) { + low := Calculate(model.ScanTotals{LinesScanned: 10_000, Warnings: 125}, Config{}) + medium := Calculate(model.ScanTotals{LinesScanned: 10_000, Warnings: 250}, Config{}) + high := Calculate(model.ScanTotals{LinesScanned: 10_000, Warnings: 500}, Config{}) + + if !(low.Score > medium.Score && medium.Score > high.Score) { + t.Fatalf("scores = low %d medium %d high %d, want strictly decreasing", low.Score, medium.Score, high.Score) + } +} + func TestCalculateCapsScoresForErrors(t *testing.T) { tests := []struct { name string @@ -48,7 +78,7 @@ func TestCalculateCapsScoresForErrors(t *testing.T) { { name: "caps do not raise lower scores", totals: model.ScanTotals{Errors: 10, Warnings: 10, Info: 10}, - want: 0, + want: 40, }, } @@ -62,10 +92,10 @@ func TestCalculateCapsScoresForErrors(t *testing.T) { } } -func TestCalculateClampsScore(t *testing.T) { +func TestCalculateDoesNotCollapseHighInfoDensityToZero(t *testing.T) { got := Calculate(model.ScanTotals{Info: 200}, Config{}) - if got.Score != 0 { - t.Fatalf("Score = %d, want 0", got.Score) + if got.Score != 33 { + t.Fatalf("Score = %d, want 33", got.Score) } if got.Band != "poor" { t.Fatalf("Band = %q, want poor", got.Band)