diff --git a/jscan/CHANGELOG.md b/jscan/CHANGELOG.md index fa3e181..9efeebb 100644 --- a/jscan/CHANGELOG.md +++ b/jscan/CHANGELOG.md @@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Report a project scale line (Micro, Small, Medium, Large, Enterprise, classified by analyzed file count) directly below the health score in the terminal summary, the text output, and the HTML report header. The `summary` object of the JSON and YAML output gains `project_scale` and `total_loc`. The scale is contextual only and does not affect the health score - Apply `analysis.include_patterns`, `analysis.recursive`, `complexity.report_unchanged`, `dead_code.min_severity`, `dead_code.sort_by`, and `output.sort_by`, which until now were validated and then ignored. Include patterns select from the file types jscan can parse, using the same matching rules as `exclude_patterns`; a file named directly on the command line is analyzed whether or not it matches - Warn on stderr, naming each key, when a configuration file sets keys that no command reads. Misspelled keys are reported the same way +- Report directory complexity rollups. The complexity section of the JSON, YAML, and CSV output gains a `by_directory` array with the function count, average and maximum complexity, high-risk count, and average and maximum nesting depth of each directory, and the text output and the HTML complexity tab gain a matching table. Directory paths are relative to the deepest directory that contains every analyzed file +- Report per-module quality hotspots. The analyze JSON and YAML output gains a `module_quality` array joining, per file, its line count, complexity rollups, and dead-code rollups, with the module name from dependency analysis; the text output, the CSV output, and a new Modules tab in the HTML report show the same table. The rollups are taken before `min_complexity` and `min_severity` are applied, so a filtered report still reports what each module carries ### Changed @@ -22,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Measure the `nesting_depth` metric, which was reported as 0 for every function because the calculation was never called and, when called, counted a function's control structures instead of measuring how deeply they nest. An `else if` continues the chain its `if` opened, a `catch` clause stays at the level of its `try`, and a nested function is measured on its own - Count one decision point per `case` label so a `switch` scores like the equivalent `if` chain instead of adding 1 no matter how many cases it has, and report the case count in the previously always-zero `switch_cases` metric. A `default` clause adds nothing, matching `else`. Switch-heavy code (reducers, dispatchers, state machines) now scores higher, so some functions cross the medium/high risk thresholds and complexity scores drop; the code did not get worse, it was under-measured. The same fix corrects a decision point that was lost when a branch sat inside a `try` block, so an `if` inside `try` now adds 1 as it does anywhere else - Keep the braces of a `switch` body out of its parsed case list, which also stops them from appearing as empty case branches in the control flow graph - Match `analysis.exclude_patterns` against whole path segments instead of any substring of a file's path, so the default entries `out` and `dist` no longer drop `src/routes/`, `src/layout/`, `src/checkout/`, or `src/utils/distance.ts`. Patterns containing a slash are matched against the path, with `**` spanning any number of directories, and patterns are now evaluated relative to the analyzed path so a parent directory named `build` no longer excludes an entire project. Affected projects will see more files analyzed and therefore different scores diff --git a/jscan/SYNC.md b/jscan/SYNC.md index 1266011..f5c0dcd 100644 --- a/jscan/SYNC.md +++ b/jscan/SYNC.md @@ -69,7 +69,7 @@ No row currently uses **sync**: every area that was language-independent enough Out of sync scope, but reference points when considering porting features to jscan: -- Cognitive complexity: `cognitive_complexity.go`, `nesting_depth.go`, `raw_metrics.go` +- Cognitive complexity: `cognitive_complexity.go`, `raw_metrics.go`. (`nesting_depth.go` no longer belongs here: jscan measures nesting depth its own way in `internal/analyzer/complexity.go`, see "Ported with divergences") - LCOM4: `lcom.go`, `domain/lcom.go` — the union-find (shared-variable + call-edge unions) and risk assessment now live in `core/lcom.ComputeLCOM4` / `AssessRisk` (pyscn Phase 3, PR #673); adopting this in jscan means pulling from `core/lcom` directly, not porting pyscn's original implementation. Python method/attribute extraction (decorator exclusion, `@property` reclassification, ctypes `_fields_`) stays pyscn-local - DFA (unused-variable detection): `dfa.go`, `dfa_builder.go` — def-use chain construction and reaching-def linking now live in `core/dfa` (pyscn Phase 3, PR #673) via the `RefExtractor`/`ParamExtractor` extension points; Python def/use extraction stays pyscn-local - DI anti-pattern detection: `di_antipattern_detector.go` plus `di_*.go`, `*_detector.go`, `framework_patterns.go` @@ -77,7 +77,18 @@ Out of sync scope, but reference points when considering porting features to jsc - Re-export resolution: `reexport_resolver.go` - Improvement suggestions: `domain/suggestion.go` - MCP server: `mcp/`, `cmd/pyscn-mcp/` -- Module / directory quality rollups: `domain/module_quality.go`, `app/module_quality.go`, `app/directory_complexity.go`, `service/directory_complexity_formatter.go` plus `AnalyzeResponse.ModuleQuality`, `ComplexityResponse.ByDirectory` / `ModuleRollups`, `DeadCodeResponse.ModuleRollups` — new pyscn feature (2026-07-26–27, #686 per-module hotspots + #533/#687 directory rollups) joining per-file complexity, dead-code and module metadata into a `module_quality` array, and aggregating reported complexity per project-root-relative directory into `by_directory`, with text/CSV/HTML tables for both. ~20 files across domain/app/service; rollups are deliberately computed *before* presentation filters (`min_complexity`, dead-code severity), which is why both response types carry a `json:"-"` `ModuleRollups` map. Two of the six module-complexity fields (`AverageCognitiveComplexity`) and both directory nesting fields have no jscan backing metric — jscan computes `NestingDepth` per function but not cognitive complexity. Tracked for jscan as [polyscan#31](https://github.com/ludo-technologies/polyscan/issues/31) +*(Module / directory quality rollups were on this list until [polyscan#31](https://github.com/ludo-technologies/polyscan/issues/31); see "Ported with divergences" below.)* + +### Ported with divergences + +- **Module / directory quality rollups** (pyscn #686 + #533/#687, ported for jscan in polyscan#31) — jscan has `domain/module_quality.go`, `domain/directory_complexity.go`, `service/module_quality.go`, `service/directory_complexity_formatter.go`, plus `ComplexityResponse.ByDirectory` / `ModuleRollups`, `DeadCodeResponse.ModuleRollups`, and a `module_quality` array on the analyze JSON/YAML output. JSON field names match pyscn. The divergences, all deliberate: + - **No `average_cognitive_complexity`** — cognitive complexity is still unported (see the list above). The field is absent rather than reported as zero + - **No `function_count`** — in pyscn it is the module metadata's function count next to the complexity records' `analyzed_function_count`. jscan's complexity analysis records every function it parses, so the two counts are the same number and only one is reported + - **`lines_of_code` comes from the complexity service**, not from module metadata: it is the only per-file reader that already holds the file content. jscan's dependency analysis leaves `ModuleDependencyMetrics.LinesOfCode` / `FunctionCount` unpopulated, so `module_quality` reads only `ModuleName` from it. Line counting matches pyscn's `countSourceLines` + - **The aggregation lives in the service layer, not the use case** — jscan's `analyze` command calls the services directly, so the use-case seam pyscn joins at does not sit on jscan's main path. `BuildModuleQuality` is a `service` function next to `BuildAnalyzeSummary`, which the analyze command already shares + - **The directory root is the analyzed files' common ancestor**, not the caller-facing path set: the services receive collected file lists, not the caller's paths. Reported paths therefore strip the shared prefix of the selection, and a single-directory run reports one `.` row + - **HTML tables are not sortable** — jscan's report has no table sorting anywhere, and adding it to only these two tables would be inconsistent + - **`nesting_depth` was implemented as part of the port.** `CalculateNestingDepth` existed but was never called and counted control structures instead of measuring depth, so the published per-function `nesting_depth` was always 0. It now measures real depth (else-if chains stay flat, a catch clause stays at its try's level, nested functions are measured separately) and the directory rollups aggregate it - Module community detection: `internal/analyzer/community_metrics.go`, `domain/community.go`, `app/community_usecase.go`, `service/community_analysis_service.go`, `service/community_formatter.go`, `service/community_config_loader.go` — new pyscn feature (2026-06-22–30, #582/#583/#602) clustering modules into communities (modularity, cross-community edges, bridge modules, package/layer alignment) with an opt-in Health Score penalty (`MaxCommunityPenalty`, max 10). Entirely new subsystem with CLI/config/MCP wiring across ~15 files; not present in jscan at all. The Health Score integration in `domain/analyze.go` only activates when `CommunitiesEnabled` and `CommunityCount >= 2`, so it has no effect on jscan's (unmodified) grade computation as long as the feature stays unported ## Pending changes @@ -131,8 +142,8 @@ Skipped during the 2026-07-25 sync (`249b121`..`6af3ee7`): Skipped during the 2026-07-28 sync (`6af3ee7`..`81b7ec0`): - **Architecture-suggestion refinements** (`3e7d83c`/#647 dedup + `file_path`/`start_line`, `a146222` preserve distinct violations, `ab8f944` prioritize critical suggestions, `0edce40` identity-dimension tests, `5554baf` per-problem violation merge + shared category cap) — all in `domain/suggestion.go`, which is on the unported-features list ("Improvement suggestions"). jscan has no suggestion subsystem to apply these to; port them together with `domain/suggestion.go` if it is ever adopted -- **Module quality rollups** (`91075ec`, `270eb98`, `ca0c48b`, `ed9602c`, `499a05f`, `5e193e9`, `8c32d21`, `db2de9b`, `6a51bc8`, `0043feb`, plus `fe247fa`/#686 and the unmapped app/service/report commits `3093066`, `77af996`, `52ddb7f`, `9f60bd7`, `9394e7c`, `72cde27`, `df7803f`, `f2b7a3d`, `9487428`, `4c26413`) — new pyscn subsystem; see "pyscn-specific, unported features" and [polyscan#31](https://github.com/ludo-technologies/polyscan/issues/31) -- **Directory quality rollups** (`d985d0f`, `21c8636`, `f0713b7`, `3308c94`, `b6e5e3c`, `be35e03`, `6209e60`, plus the unmapped `39ca5ec`, `5b7226f`, `535c8e6`, `72337d2`, `af33f81`, `d9d8c64`, `ce8d982`, `d39ddef`, `86e5875`, `86d3ed2`, `3dd97bf`, `40dd367` — #533/#687) — second half of the same feature; same tracking issue. Note `d9d8c64` (buffer the analyze text report and propagate a single write error) is a genuinely portable formatter fix, but it exists only to wrap the new directory/module sections — jscan's `AnalyzeFormatter` writes with `fmt.Fprint` throughout and has no equivalent error-propagation seam to fix in isolation +- **Module quality rollups** (`91075ec`, `270eb98`, `ca0c48b`, `ed9602c`, `499a05f`, `5e193e9`, `8c32d21`, `db2de9b`, `6a51bc8`, `0043feb`, plus `fe247fa`/#686 and the unmapped app/service/report commits `3093066`, `77af996`, `52ddb7f`, `9f60bd7`, `9394e7c`, `72cde27`, `df7803f`, `f2b7a3d`, `9487428`, `4c26413`) — new pyscn subsystem; skipped by the sync run and **ported afterwards in [polyscan#31](https://github.com/ludo-technologies/polyscan/issues/31)**, see "Ported with divergences" +- **Directory quality rollups** (`d985d0f`, `21c8636`, `f0713b7`, `3308c94`, `b6e5e3c`, `be35e03`, `6209e60`, plus the unmapped `39ca5ec`, `5b7226f`, `535c8e6`, `72337d2`, `af33f81`, `d9d8c64`, `ce8d982`, `d39ddef`, `86e5875`, `86d3ed2`, `3dd97bf`, `40dd367` — #533/#687) — second half of the same feature; same tracking issue. Note `d9d8c64` (buffer the analyze text report and propagate a single write error) is a genuinely portable formatter fix, but it exists only to wrap the new directory/module sections — jscan's `AnalyzeFormatter` writes with `fmt.Fprint` throughout and has no equivalent error-propagation seam to fix in isolation. The rollups themselves were **ported afterwards in [polyscan#31](https://github.com/ludo-technologies/polyscan/issues/31)** - **MCP registry listing** (`300e1c9`, `9c3ab20`) — jscan has no MCP server ## Sync history diff --git a/jscan/domain/complexity.go b/jscan/domain/complexity.go index 1c2c1ca..ef98c23 100644 --- a/jscan/domain/complexity.go +++ b/jscan/domain/complexity.go @@ -2,6 +2,7 @@ package domain import ( "context" + "encoding/json" "io" ) @@ -132,11 +133,52 @@ type ComplexitySummary struct { ComplexityDistribution map[string]int `json:"complexity_distribution,omitempty" yaml:"complexity_distribution,omitempty"` } +// DirectoryComplexityMetrics aggregates reported ComplexityResponse.Functions +// entries for one project-root-relative directory. +type DirectoryComplexityMetrics struct { + DirectoryPath string `json:"directory_path" yaml:"directory_path"` + FunctionCount int `json:"function_count" yaml:"function_count"` + AverageComplexity float64 `json:"average_complexity" yaml:"average_complexity"` + MaxComplexity int `json:"max_complexity" yaml:"max_complexity"` + HighRiskFunctionCount int `json:"high_risk_function_count" yaml:"high_risk_function_count"` + AverageNestingDepth float64 `json:"average_nesting_depth" yaml:"average_nesting_depth"` + MaxNestingDepth int `json:"max_nesting_depth" yaml:"max_nesting_depth"` +} + +// DirectoryComplexityMetricsList is the stable serialized collection contract. +// A zero value is encoded as an empty array so callers never need to distinguish +// an uninitialized collection from a completed analysis with no reported rows. +type DirectoryComplexityMetricsList []DirectoryComplexityMetrics + +// MarshalJSON encodes an uninitialized collection as an empty JSON array. +func (metrics DirectoryComplexityMetricsList) MarshalJSON() ([]byte, error) { + if metrics == nil { + return []byte("[]"), nil + } + type plainDirectoryComplexityMetricsList DirectoryComplexityMetricsList + return json.Marshal(plainDirectoryComplexityMetricsList(metrics)) +} + +// MarshalYAML encodes an uninitialized collection as an empty YAML array. +func (metrics DirectoryComplexityMetricsList) MarshalYAML() (interface{}, error) { + if metrics == nil { + return []DirectoryComplexityMetrics{}, nil + } + return []DirectoryComplexityMetrics(metrics), nil +} + // ComplexityResponse represents the complete analysis result type ComplexityResponse struct { // Analysis results - Functions []FunctionComplexity `json:"functions" yaml:"functions"` - Summary ComplexitySummary `json:"summary" yaml:"summary"` + Functions []FunctionComplexity `json:"functions" yaml:"functions"` + ByDirectory DirectoryComplexityMetricsList `json:"by_directory" yaml:"by_directory"` + Summary ComplexitySummary `json:"summary" yaml:"summary"` + + // ModuleRollups are derived before the report filters are applied, so they + // describe the whole analyzed population rather than what min/max complexity + // left visible. They are consumed by the unified analyze command and are not + // part of standalone complexity output. + ModuleRollups map[string]ModuleComplexityMetrics `json:"-" yaml:"-"` // Warnings and issues Warnings []string `json:"warnings,omitempty" yaml:"warnings,omitempty"` diff --git a/jscan/domain/dead_code.go b/jscan/domain/dead_code.go index ee0d0a8..9f9502e 100644 --- a/jscan/domain/dead_code.go +++ b/jscan/domain/dead_code.go @@ -155,6 +155,12 @@ type DeadCodeResponse struct { Files []FileDeadCode `json:"files"` Summary DeadCodeSummary `json:"summary"` + // ModuleRollups are derived before the severity filter is applied, so they + // describe every finding the detectors produced rather than the subset the + // report shows. They are consumed by the unified analyze command and are not + // part of standalone dead code output. + ModuleRollups map[string]ModuleDeadCodeMetrics `json:"-" yaml:"-"` + // Warnings and issues Warnings []string `json:"warnings"` Errors []string `json:"errors"` diff --git a/jscan/domain/directory_complexity.go b/jscan/domain/directory_complexity.go new file mode 100644 index 0000000..2aa4d4c --- /dev/null +++ b/jscan/domain/directory_complexity.go @@ -0,0 +1,168 @@ +package domain + +import ( + "fmt" + "path/filepath" + "sort" + "strings" +) + +// directoryComplexityAccumulator carries the running sums a directory average +// needs; only the finished metrics are handed back to callers. +type directoryComplexityAccumulator struct { + metrics DirectoryComplexityMetrics + totalComplexity int + totalNestingDepth int +} + +// ComplexityDirectoryRoot returns the deepest directory that contains every +// analyzed file, which is the root the reported directory paths are relative +// to. Files participate through their parent directory; the scope is never +// widened past what the caller actually asked to analyze. +func ComplexityDirectoryRoot(files []string) (string, error) { + if len(files) == 0 { + return "", fmt.Errorf("at least one analyzed file is required") + } + + var root string + for _, file := range files { + identity, err := absoluteDirectory(file) + if err != nil { + return "", err + } + + if root == "" { + root = identity + continue + } + if filepath.VolumeName(root) != filepath.VolumeName(identity) { + return "", fmt.Errorf("analyzed files do not share a filesystem volume") + } + root = commonDirectory(root, identity) + } + return root, nil +} + +// AggregateComplexityByDirectory groups the reported function population by its +// direct project-root-relative directory. This is the only owner of directory +// grouping and directory-level complexity arithmetic. +func AggregateComplexityByDirectory(functions []FunctionComplexity, projectRoot string) (DirectoryComplexityMetricsList, error) { + if len(functions) == 0 { + return DirectoryComplexityMetricsList{}, nil + } + + rootIdentity, err := absolutePath(projectRoot) + if err != nil { + return nil, err + } + + directories := make(map[string]*directoryComplexityAccumulator) + for _, function := range functions { + fileIdentity, err := absolutePath(function.FilePath) + if err != nil { + return nil, err + } + relativePath, err := filepath.Rel(rootIdentity, fileIdentity) + if err != nil { + return nil, fmt.Errorf("make function file path %q relative to root %q: %w", function.FilePath, projectRoot, err) + } + if pathEscapesRoot(relativePath) { + return nil, fmt.Errorf("function file path %q is outside complexity directory root %q", function.FilePath, projectRoot) + } + + directoryPath := filepath.Dir(relativePath) + accumulator := directories[directoryPath] + if accumulator == nil { + accumulator = &directoryComplexityAccumulator{ + metrics: DirectoryComplexityMetrics{DirectoryPath: directoryPath}, + } + directories[directoryPath] = accumulator + } + accumulator.addFunction(function) + } + + result := make(DirectoryComplexityMetricsList, 0, len(directories)) + for _, directory := range directories { + directory.finishAverages() + result = append(result, directory.metrics) + } + sortDirectoryComplexity(result) + return result, nil +} + +func (a *directoryComplexityAccumulator) addFunction(function FunctionComplexity) { + a.metrics.FunctionCount++ + a.totalComplexity += function.Metrics.Complexity + a.totalNestingDepth += function.Metrics.NestingDepth + if function.Metrics.Complexity > a.metrics.MaxComplexity { + a.metrics.MaxComplexity = function.Metrics.Complexity + } + if function.Metrics.NestingDepth > a.metrics.MaxNestingDepth { + a.metrics.MaxNestingDepth = function.Metrics.NestingDepth + } + if function.RiskLevel == RiskLevelHigh { + a.metrics.HighRiskFunctionCount++ + } +} + +func (a *directoryComplexityAccumulator) finishAverages() { + count := float64(a.metrics.FunctionCount) + a.metrics.AverageComplexity = float64(a.totalComplexity) / count + a.metrics.AverageNestingDepth = float64(a.totalNestingDepth) / count +} + +// sortDirectoryComplexity ranks the worst directories first so that a truncated +// report still shows the ones worth acting on. +func sortDirectoryComplexity(directories DirectoryComplexityMetricsList) { + sort.Slice(directories, func(i, j int) bool { + left, right := directories[i], directories[j] + if left.HighRiskFunctionCount != right.HighRiskFunctionCount { + return left.HighRiskFunctionCount > right.HighRiskFunctionCount + } + if left.MaxComplexity != right.MaxComplexity { + return left.MaxComplexity > right.MaxComplexity + } + if left.AverageComplexity != right.AverageComplexity { + return left.AverageComplexity > right.AverageComplexity + } + return left.DirectoryPath < right.DirectoryPath + }) +} + +// commonDirectory returns the deepest directory containing both arguments. +func commonDirectory(left, right string) string { + for { + relative, err := filepath.Rel(left, right) + if err == nil && !pathEscapesRoot(relative) { + return left + } + parent := filepath.Dir(left) + if parent == left { + return left + } + left = parent + } +} + +func pathEscapesRoot(relativePath string) bool { + return relativePath == ".." || strings.HasPrefix(relativePath, ".."+string(filepath.Separator)) +} + +// absolutePath resolves a caller-facing path to the identity used for grouping. +// Paths are compared as identities only; the caller's own spelling is what gets +// reported back. +func absolutePath(path string) (string, error) { + absolute, err := filepath.Abs(path) + if err != nil { + return "", fmt.Errorf("resolve path %q: %w", path, err) + } + return absolute, nil +} + +func absoluteDirectory(file string) (string, error) { + absolute, err := absolutePath(file) + if err != nil { + return "", err + } + return filepath.Dir(absolute), nil +} diff --git a/jscan/domain/directory_complexity_test.go b/jscan/domain/directory_complexity_test.go new file mode 100644 index 0000000..5dce600 --- /dev/null +++ b/jscan/domain/directory_complexity_test.go @@ -0,0 +1,139 @@ +package domain + +import ( + "encoding/json" + "path/filepath" + "testing" +) + +func directoryFunction(filePath string, complexity, nesting int, risk RiskLevel) FunctionComplexity { + return FunctionComplexity{ + Name: "fn", + FilePath: filePath, + Metrics: ComplexityMetrics{ + Complexity: complexity, + NestingDepth: nesting, + }, + RiskLevel: risk, + } +} + +func TestComplexityDirectoryRootIsTheCommonAncestorOfTheAnalyzedFiles(t *testing.T) { + root, err := ComplexityDirectoryRoot([]string{"src/a/one.ts", "src/b/two.ts", "src/index.ts"}) + if err != nil { + t.Fatalf("ComplexityDirectoryRoot failed: %v", err) + } + + expected, err := filepath.Abs("src") + if err != nil { + t.Fatalf("failed to resolve expected root: %v", err) + } + if root != expected { + t.Errorf("expected root %q, got %q", expected, root) + } +} + +func TestComplexityDirectoryRootOfOneFileIsItsDirectory(t *testing.T) { + root, err := ComplexityDirectoryRoot([]string{"src/a/one.ts"}) + if err != nil { + t.Fatalf("ComplexityDirectoryRoot failed: %v", err) + } + + expected, err := filepath.Abs(filepath.Join("src", "a")) + if err != nil { + t.Fatalf("failed to resolve expected root: %v", err) + } + if root != expected { + t.Errorf("expected root %q, got %q", expected, root) + } +} + +func TestComplexityDirectoryRootRequiresAtLeastOneFile(t *testing.T) { + if _, err := ComplexityDirectoryRoot(nil); err == nil { + t.Error("expected an error when no files were analyzed") + } +} + +func TestAggregateComplexityByDirectoryGroupsByDirectAncestor(t *testing.T) { + root, err := filepath.Abs("src") + if err != nil { + t.Fatalf("failed to resolve root: %v", err) + } + + directories, err := AggregateComplexityByDirectory([]FunctionComplexity{ + directoryFunction("src/a/one.ts", 4, 2, RiskLevelLow), + directoryFunction("src/a/two.ts", 10, 4, RiskLevelHigh), + directoryFunction("src/index.ts", 2, 1, RiskLevelLow), + }, root) + if err != nil { + t.Fatalf("AggregateComplexityByDirectory failed: %v", err) + } + + if len(directories) != 2 { + t.Fatalf("expected 2 directories, got %d", len(directories)) + } + + // The high-risk directory ranks first. + worst := directories[0] + if worst.DirectoryPath != "a" { + t.Errorf("expected directory %q first, got %q", "a", worst.DirectoryPath) + } + if worst.FunctionCount != 2 { + t.Errorf("expected 2 functions, got %d", worst.FunctionCount) + } + if worst.AverageComplexity != 7 { + t.Errorf("expected average complexity 7, got %.2f", worst.AverageComplexity) + } + if worst.MaxComplexity != 10 { + t.Errorf("expected max complexity 10, got %d", worst.MaxComplexity) + } + if worst.HighRiskFunctionCount != 1 { + t.Errorf("expected 1 high-risk function, got %d", worst.HighRiskFunctionCount) + } + if worst.AverageNestingDepth != 3 { + t.Errorf("expected average nesting depth 3, got %.2f", worst.AverageNestingDepth) + } + if worst.MaxNestingDepth != 4 { + t.Errorf("expected max nesting depth 4, got %d", worst.MaxNestingDepth) + } + + if directories[1].DirectoryPath != "." { + t.Errorf("expected the root directory to be reported as %q, got %q", ".", directories[1].DirectoryPath) + } +} + +func TestAggregateComplexityByDirectoryRejectsFilesOutsideTheRoot(t *testing.T) { + root, err := filepath.Abs("src") + if err != nil { + t.Fatalf("failed to resolve root: %v", err) + } + + if _, err := AggregateComplexityByDirectory([]FunctionComplexity{ + directoryFunction("vendor/other.ts", 1, 0, RiskLevelLow), + }, root); err == nil { + t.Error("expected an error for a function outside the directory root") + } +} + +func TestAggregateComplexityByDirectoryWithoutFunctionsIsEmpty(t *testing.T) { + directories, err := AggregateComplexityByDirectory(nil, "src") + if err != nil { + t.Fatalf("AggregateComplexityByDirectory failed: %v", err) + } + if len(directories) != 0 { + t.Errorf("expected no directories, got %d", len(directories)) + } +} + +func TestDirectoryComplexityMetricsListEncodesNilAsEmptyArray(t *testing.T) { + encoded, err := json.Marshal(struct { + ByDirectory DirectoryComplexityMetricsList `json:"by_directory"` + }{}) + if err != nil { + t.Fatalf("failed to encode: %v", err) + } + + if string(encoded) != `{"by_directory":[]}` { + t.Errorf("expected an empty array, got %s", encoded) + } +} diff --git a/jscan/domain/module_quality.go b/jscan/domain/module_quality.go new file mode 100644 index 0000000..cc94c9e --- /dev/null +++ b/jscan/domain/module_quality.go @@ -0,0 +1,101 @@ +package domain + +import ( + "path/filepath" + "sort" +) + +// ModuleComplexityMetrics is the canonical module-level complexity contract. +// AnalyzedFunctionCount includes every function-level complexity record before +// presentation filters. +// +// LinesOfCode belongs to the same rollup because the complexity service is the +// only per-file reader that already holds the file content: counting lines +// there costs nothing, while a separate pass would re-read the project. +// +// pyscn also reports AverageCognitiveComplexity here. jscan has no cognitive +// complexity metric, so the field is absent rather than reported as zero. +type ModuleComplexityMetrics struct { + LinesOfCode int `json:"lines_of_code" yaml:"lines_of_code"` + AnalyzedFunctionCount int `json:"analyzed_function_count" yaml:"analyzed_function_count"` + AverageComplexity float64 `json:"average_complexity" yaml:"average_complexity"` + MaxComplexity int `json:"max_complexity" yaml:"max_complexity"` + HighRiskFunctionCount int `json:"high_risk_function_count" yaml:"high_risk_function_count"` + ExceptionHandlerCount int `json:"exception_handler_count" yaml:"exception_handler_count"` +} + +// ModuleDeadCodeMetrics is the canonical module-level dead-code contract. +// Both counts describe findings the detectors produced before severity +// filtering, so they do not shrink when a report raises min_severity. +type ModuleDeadCodeMetrics struct { + DeadCodeFindingCount int `json:"dead_code_finding_count" yaml:"dead_code_finding_count"` + DeadCodeBlockCount int `json:"dead_code_block_count" yaml:"dead_code_block_count"` +} + +// ModuleQualityMetrics is the public per-file view assembled by unified +// analysis. ModuleName is only known when dependency analysis ran. +type ModuleQualityMetrics struct { + ModuleName string `json:"module_name,omitempty" yaml:"module_name,omitempty"` + FilePath string `json:"file_path" yaml:"file_path"` + ModuleComplexityMetrics `yaml:",inline"` + ModuleDeadCodeMetrics `yaml:",inline"` +} + +type moduleComplexityAccumulator struct { + metrics ModuleComplexityMetrics + totalComplexity int +} + +// AggregateComplexityByModule derives module metrics from the complete, +// pre-filter complexity population owned by the complexity service. +func AggregateComplexityByModule(functions []FunctionComplexity) map[string]ModuleComplexityMetrics { + modules := make(map[string]*moduleComplexityAccumulator) + for _, function := range functions { + key := filepath.Clean(function.FilePath) + module := modules[key] + if module == nil { + module = &moduleComplexityAccumulator{} + modules[key] = module + } + + module.metrics.AnalyzedFunctionCount++ + module.totalComplexity += function.Metrics.Complexity + module.metrics.ExceptionHandlerCount += function.Metrics.ExceptionHandlers + if function.Metrics.Complexity > module.metrics.MaxComplexity { + module.metrics.MaxComplexity = function.Metrics.Complexity + } + if function.RiskLevel == RiskLevelHigh { + module.metrics.HighRiskFunctionCount++ + } + } + + result := make(map[string]ModuleComplexityMetrics, len(modules)) + for path, module := range modules { + if module.metrics.AnalyzedFunctionCount > 0 { + module.metrics.AverageComplexity = float64(module.totalComplexity) / float64(module.metrics.AnalyzedFunctionCount) + } + result[path] = module.metrics + } + return result +} + +// SortModuleQuality ranks the worst modules first so that a truncated report +// still shows the ones worth acting on. +func SortModuleQuality(modules []ModuleQualityMetrics) { + sort.Slice(modules, func(i, j int) bool { + left, right := modules[i], modules[j] + if left.HighRiskFunctionCount != right.HighRiskFunctionCount { + return left.HighRiskFunctionCount > right.HighRiskFunctionCount + } + if left.MaxComplexity != right.MaxComplexity { + return left.MaxComplexity > right.MaxComplexity + } + if left.AverageComplexity != right.AverageComplexity { + return left.AverageComplexity > right.AverageComplexity + } + if left.DeadCodeFindingCount != right.DeadCodeFindingCount { + return left.DeadCodeFindingCount > right.DeadCodeFindingCount + } + return left.FilePath < right.FilePath + }) +} diff --git a/jscan/domain/module_quality_test.go b/jscan/domain/module_quality_test.go new file mode 100644 index 0000000..6853942 --- /dev/null +++ b/jscan/domain/module_quality_test.go @@ -0,0 +1,69 @@ +package domain + +import "testing" + +func TestAggregateComplexityByModuleSummarizesEachFile(t *testing.T) { + rollups := AggregateComplexityByModule([]FunctionComplexity{ + { + FilePath: "./src/a.ts", + Metrics: ComplexityMetrics{Complexity: 4, ExceptionHandlers: 1}, + RiskLevel: RiskLevelLow, + }, + { + FilePath: "src/a.ts", + Metrics: ComplexityMetrics{Complexity: 12, ExceptionHandlers: 2}, + RiskLevel: RiskLevelHigh, + }, + { + FilePath: "src/b.ts", + Metrics: ComplexityMetrics{Complexity: 2}, + RiskLevel: RiskLevelLow, + }, + }) + + if len(rollups) != 2 { + t.Fatalf("expected 2 modules, got %d", len(rollups)) + } + + // Both spellings of the same file are one module. + module := rollups["src/a.ts"] + if module.AnalyzedFunctionCount != 2 { + t.Errorf("expected 2 analyzed functions, got %d", module.AnalyzedFunctionCount) + } + if module.AverageComplexity != 8 { + t.Errorf("expected average complexity 8, got %.2f", module.AverageComplexity) + } + if module.MaxComplexity != 12 { + t.Errorf("expected max complexity 12, got %d", module.MaxComplexity) + } + if module.HighRiskFunctionCount != 1 { + t.Errorf("expected 1 high-risk function, got %d", module.HighRiskFunctionCount) + } + if module.ExceptionHandlerCount != 3 { + t.Errorf("expected 3 exception handlers, got %d", module.ExceptionHandlerCount) + } +} + +func TestAggregateComplexityByModuleWithoutFunctionsIsEmpty(t *testing.T) { + if rollups := AggregateComplexityByModule(nil); len(rollups) != 0 { + t.Errorf("expected no modules, got %d", len(rollups)) + } +} + +func TestSortModuleQualityRanksTheWorstModulesFirst(t *testing.T) { + modules := []ModuleQualityMetrics{ + {FilePath: "quiet.ts"}, + {FilePath: "dead.ts", ModuleDeadCodeMetrics: ModuleDeadCodeMetrics{DeadCodeFindingCount: 5}}, + {FilePath: "complex.ts", ModuleComplexityMetrics: ModuleComplexityMetrics{MaxComplexity: 30}}, + {FilePath: "risky.ts", ModuleComplexityMetrics: ModuleComplexityMetrics{HighRiskFunctionCount: 1, MaxComplexity: 21}}, + } + + SortModuleQuality(modules) + + expected := []string{"risky.ts", "complex.ts", "dead.ts", "quiet.ts"} + for index, filePath := range expected { + if modules[index].FilePath != filePath { + t.Errorf("expected %q at position %d, got %q", filePath, index, modules[index].FilePath) + } + } +} diff --git a/jscan/internal/analyzer/complexity.go b/jscan/internal/analyzer/complexity.go index 1001d17..544b31a 100644 --- a/jscan/internal/analyzer/complexity.go +++ b/jscan/internal/analyzer/complexity.go @@ -146,6 +146,7 @@ func CalculateComplexityWithConfig(cfg *CFG, complexityConfig *config.Complexity result.StartCol = functionNode.Location.StartCol result.EndLine = functionNode.Location.EndLine result.SwitchCases = countSwitchCases(functionNode) + result.NestingDepth = CalculateNestingDepth(functionNode) } return result @@ -168,37 +169,83 @@ func countSwitchCases(functionNode *parser.Node) int { return switchCases } -// CalculateNestingDepth calculates the maximum nesting depth of a function +// CalculateNestingDepth returns the deepest chain of nested control structures +// inside a function. The function body itself is depth 0, so a single loop or +// branch is depth 1. +// +// Nested functions are skipped because they get their own CFG and result, the +// same boundary countSwitchCases uses. func CalculateNestingDepth(node *parser.Node) int { if node == nil { return 0 } - maxDepth := 0 - currentDepth := 0 + deepest := 0 + for _, child := range parser.OrderedChildren(node) { + if depth := nestingDepthOf(child, 0); depth > deepest { + deepest = depth + } + } + return deepest +} + +// nestingDepthOf returns the deepest level reached inside node, which sits at +// the given level before its own contribution is counted. +func nestingDepthOf(node *parser.Node, level int) int { + if node == nil || isFunctionNode(node) { + return level + } - node.Walk(func(n *parser.Node) bool { - // Increment depth for control structures - if isControlStructure(n) { - currentDepth++ - if currentDepth > maxDepth { - maxDepth = currentDepth - } + if isControlStructure(node) { + level++ + } + + deepest := level + for _, child := range parser.OrderedChildren(node) { + childLevel := level + // `else if` continues the chain its outer if opened rather than + // starting a deeper one, so the whole chain reads as one level. A + // plain `else { ... }` block is left alone: code nested in it really + // is one level deeper. + if node.Type == parser.NodeIfStatement && child == node.Alternate && isElseIf(child) { + childLevel = level - 1 } + if depth := nestingDepthOf(child, childLevel); depth > deepest { + deepest = depth + } + } + return deepest +} +// isElseIf reports whether an if statement's alternate continues an else-if +// chain. The parser keeps the else clause as a wrapper node, so the chain is +// recognized by the clause holding an if statement instead of a block. +func isElseIf(alternate *parser.Node) bool { + if alternate == nil { + return false + } + if alternate.Type == parser.NodeIfStatement { return true - }) - - return maxDepth + } + for _, child := range parser.OrderedChildren(alternate) { + switch child.Type { + case parser.NodeIfStatement: + return true + case parser.NodeBlockStatement: + return false + } + } + return false } -// isControlStructure checks if a node is a control structure +// isControlStructure checks if a node opens a nesting level. A catch clause +// does not: it is part of the try statement that already opened one. func isControlStructure(node *parser.Node) bool { switch node.Type { case parser.NodeIfStatement, parser.NodeSwitchStatement, parser.NodeForStatement, parser.NodeForInStatement, parser.NodeForOfStatement, parser.NodeWhileStatement, parser.NodeDoWhileStatement, - parser.NodeTryStatement, parser.NodeCatchClause: + parser.NodeTryStatement: return true } return false diff --git a/jscan/internal/analyzer/complexity_test.go b/jscan/internal/analyzer/complexity_test.go index 093a175..79ef321 100644 --- a/jscan/internal/analyzer/complexity_test.go +++ b/jscan/internal/analyzer/complexity_test.go @@ -413,9 +413,95 @@ func TestCalculateNestingDepth_MixedControlStructures(t *testing.T) { funcNode := findFunction(ast, "test") depth := CalculateNestingDepth(funcNode) - // for -> if -> try -> catch = at least 3-4 levels - if depth < 3 { - t.Errorf("Mixed control structures should have depth >= 3, got %d", depth) + // for -> if -> try; the catch clause belongs to the try it is part of. + if depth != 3 { + t.Errorf("Mixed control structures should have depth 3, got %d", depth) + } +} + +func TestCalculateNestingDepth_SiblingsAreNotCumulative(t *testing.T) { + code := ` + function test(x) { + if (x) { a(); } + if (x) { b(); } + for (const item of x) { c(item); } + } + ` + ast := parseJS(t, code) + funcNode := findFunction(ast, "test") + + depth := CalculateNestingDepth(funcNode) + if depth != 1 { + t.Errorf("Sibling control structures should have depth 1, got %d", depth) + } +} + +func TestCalculateNestingDepth_ElseIfChainStaysFlat(t *testing.T) { + code := ` + function test(x) { + if (x === 1) { a(); } + else if (x === 2) { b(); } + else if (x === 3) { c(); } + else { d(); } + } + ` + ast := parseJS(t, code) + funcNode := findFunction(ast, "test") + + depth := CalculateNestingDepth(funcNode) + if depth != 1 { + t.Errorf("else-if chain should have depth 1, got %d", depth) + } +} + +func TestCalculateNestingDepth_ElseBlockNests(t *testing.T) { + code := ` + function test(x) { + if (x) { a(); } + else { if (x > 1) { b(); } } + } + ` + ast := parseJS(t, code) + funcNode := findFunction(ast, "test") + + depth := CalculateNestingDepth(funcNode) + if depth != 2 { + t.Errorf("if nested in an else block should have depth 2, got %d", depth) + } +} + +func TestCalculateNestingDepth_CatchBodyNests(t *testing.T) { + code := ` + function test(x) { + try { a(); } catch (e) { if (x) { b(); } } + } + ` + ast := parseJS(t, code) + funcNode := findFunction(ast, "test") + + depth := CalculateNestingDepth(funcNode) + if depth != 2 { + t.Errorf("if inside a catch clause should have depth 2, got %d", depth) + } +} + +func TestCalculateNestingDepth_NestedFunctionsExcluded(t *testing.T) { + code := ` + function test(x) { + if (x) { + const inner = function () { + for (const item of x) { if (item) { a(item); } } + }; + inner(); + } + } + ` + ast := parseJS(t, code) + funcNode := findFunction(ast, "test") + + depth := CalculateNestingDepth(funcNode) + if depth != 1 { + t.Errorf("Nested function bodies should not count, got %d", depth) } } @@ -429,7 +515,6 @@ func TestIsControlStructure(t *testing.T) { parser.NodeWhileStatement, parser.NodeDoWhileStatement, parser.NodeTryStatement, - parser.NodeCatchClause, } for _, nodeType := range controlStructures { @@ -445,6 +530,10 @@ func TestIsControlStructure(t *testing.T) { parser.NodeReturnStatement, parser.NodeFunction, parser.NodeArrowFunction, + // A catch clause belongs to the try statement that already opened a + // nesting level; counting it again would double the depth of every + // try/catch. + parser.NodeCatchClause, } for _, nodeType := range nonControlStructures { diff --git a/jscan/service/complexity_service.go b/jscan/service/complexity_service.go index a21be19..10ef710 100644 --- a/jscan/service/complexity_service.go +++ b/jscan/service/complexity_service.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "os" + "path/filepath" "sort" "time" @@ -40,7 +41,8 @@ func (s *ComplexityServiceImpl) Analyze(ctx context.Context, req domain.Complexi var allFunctions []domain.FunctionComplexity var warnings []string var errors []string - filesProcessed := 0 + var analyzedPaths []string + linesOfCode := make(map[string]int, len(req.Paths)) // Set up progress tracking (use no-op if progress manager not set) var task domain.TaskProgress = &NoOpTaskProgress{} @@ -50,47 +52,84 @@ func (s *ComplexityServiceImpl) Analyze(ctx context.Context, req domain.Complexi defer task.Complete() results := analyzeFilesConcurrently(ctx, req.Paths, task, - func(ctx context.Context, filePath string) fileAnalysis[[]domain.FunctionComplexity] { - functions, fileWarnings, fileErrors := s.analyzeFile(ctx, filePath, req) - return fileAnalysis[[]domain.FunctionComplexity]{value: functions, warnings: fileWarnings, errors: fileErrors} + func(ctx context.Context, filePath string) fileAnalysis[fileComplexity] { + file, fileWarnings, fileErrors := s.analyzeFile(ctx, filePath, req) + return fileAnalysis[fileComplexity]{value: file, warnings: fileWarnings, errors: fileErrors} }) if ctx.Err() != nil { return nil, fmt.Errorf("complexity analysis cancelled: %w", ctx.Err()) } - for _, result := range results { + for index, result := range results { if len(result.errors) > 0 { errors = append(errors, result.errors...) continue // Skip this file but continue with others } - allFunctions = append(allFunctions, result.value...) + allFunctions = append(allFunctions, result.value.functions...) warnings = append(warnings, result.warnings...) - filesProcessed++ + filePath := req.Paths[index] + analyzedPaths = append(analyzedPaths, filePath) + linesOfCode[filepath.Clean(filePath)] = result.value.linesOfCode } if len(allFunctions) == 0 { return nil, domain.NewAnalysisError("no functions found to analyze", nil) } + // Roll up per module before filtering: the rollups describe the analyzed + // population, not the subset min/max complexity leaves visible. + moduleRollups := moduleComplexityRollups(allFunctions, linesOfCode) + // Filter and sort results filteredFunctions, functionsParsed := s.filterFunctions(allFunctions, req) sortedFunctions := s.sortFunctions(filteredFunctions, req.SortBy) + byDirectory, err := aggregateDirectoryComplexity(sortedFunctions, analyzedPaths) + if err != nil { + return nil, domain.NewAnalysisError("failed to aggregate directory complexity", err) + } + // Generate summary - summary := s.generateSummary(sortedFunctions, filesProcessed, req, functionsParsed) + summary := s.generateSummary(sortedFunctions, len(analyzedPaths), req, functionsParsed) return &domain.ComplexityResponse{ - Functions: sortedFunctions, - Summary: summary, - Warnings: warnings, - Errors: errors, - GeneratedAt: time.Now().Format(time.RFC3339), - Version: version.Version, - Config: s.buildConfigForResponse(req), + Functions: sortedFunctions, + ByDirectory: byDirectory, + Summary: summary, + ModuleRollups: moduleRollups, + Warnings: warnings, + Errors: errors, + GeneratedAt: time.Now().Format(time.RFC3339), + Version: version.Version, + Config: s.buildConfigForResponse(req), }, nil } +// moduleComplexityRollups joins the per-module complexity aggregation with the +// line counts only this service holds. Files that parsed without yielding a +// single function still get an entry, so the module report can show a large +// file that has no functions to blame. +func moduleComplexityRollups(functions []domain.FunctionComplexity, linesOfCode map[string]int) map[string]domain.ModuleComplexityMetrics { + rollups := domain.AggregateComplexityByModule(functions) + for path, lines := range linesOfCode { + metrics := rollups[path] + metrics.LinesOfCode = lines + rollups[path] = metrics + } + return rollups +} + +// aggregateDirectoryComplexity reports the directory rollups relative to the +// deepest directory that contains every analyzed file. +func aggregateDirectoryComplexity(functions []domain.FunctionComplexity, analyzedPaths []string) (domain.DirectoryComplexityMetricsList, error) { + projectRoot, err := domain.ComplexityDirectoryRoot(analyzedPaths) + if err != nil { + return nil, err + } + return domain.AggregateComplexityByDirectory(functions, projectRoot) +} + // AnalyzeFile analyzes a single JavaScript/TypeScript file func (s *ComplexityServiceImpl) AnalyzeFile(ctx context.Context, filePath string, req domain.ComplexityRequest) (*domain.ComplexityResponse, error) { // Update the request to analyze only this file @@ -100,9 +139,29 @@ func (s *ComplexityServiceImpl) AnalyzeFile(ctx context.Context, filePath string return s.Analyze(ctx, singleFileReq) } +// fileComplexity is everything complexity analysis derives from one file: the +// per-function results plus the file-level size the module rollups report. +type fileComplexity struct { + functions []domain.FunctionComplexity + linesOfCode int +} + +// countSourceLines counts the physical lines of a source file. The last line +// counts whether or not it ends in a newline, matching pyscn so module line +// counts mean the same thing in both tools' reports. +func countSourceLines(content []byte) int { + lines := 1 + for _, b := range content { + if b == '\n' { + lines++ + } + } + return lines +} + // analyzeFile performs complexity analysis on a single file -func (s *ComplexityServiceImpl) analyzeFile(ctx context.Context, filePath string, req domain.ComplexityRequest) ([]domain.FunctionComplexity, []string, []string) { - var functions []domain.FunctionComplexity +func (s *ComplexityServiceImpl) analyzeFile(ctx context.Context, filePath string, req domain.ComplexityRequest) (fileComplexity, []string, []string) { + var file fileComplexity var warnings []string var errors []string @@ -110,14 +169,15 @@ func (s *ComplexityServiceImpl) analyzeFile(ctx context.Context, filePath string content, err := s.readFile(filePath) if err != nil { errors = append(errors, fmt.Sprintf("[%s] Failed to read file: %v", filePath, err)) - return functions, warnings, errors + return file, warnings, errors } + file.linesOfCode = countSourceLines(content) // Parse JavaScript/TypeScript ast, err := parser.ParseForLanguage(filePath, content) if err != nil { errors = append(errors, fmt.Sprintf("[%s] Failed to parse: %v", filePath, err)) - return functions, warnings, errors + return file, warnings, errors } // Build CFGs for all functions @@ -125,7 +185,7 @@ func (s *ComplexityServiceImpl) analyzeFile(ctx context.Context, filePath string cfgs, err := builder.BuildAll(ast) if err != nil { errors = append(errors, fmt.Sprintf("[%s] Failed to build CFG: %v", filePath, err)) - return functions, warnings, errors + return file, warnings, errors } // Analyze complexity for each function @@ -156,10 +216,10 @@ func (s *ComplexityServiceImpl) analyzeFile(ctx context.Context, filePath string RiskLevel: domain.RiskLevel(result.RiskLevel), } - functions = append(functions, funcComplexity) + file.functions = append(file.functions, funcComplexity) } - return functions, warnings, errors + return file, warnings, errors } // filterFunctions returns the visible functions plus the count of functions that diff --git a/jscan/service/complexity_service_test.go b/jscan/service/complexity_service_test.go index d50ab56..2d1302d 100644 --- a/jscan/service/complexity_service_test.go +++ b/jscan/service/complexity_service_test.go @@ -307,6 +307,110 @@ function branchy(x) { } } +func TestComplexityService_Analyze_ModuleRollupsPrecedeReportFilters(t *testing.T) { + tempDir := t.TempDir() + jsFile := filepath.Join(tempDir, "mixed.js") + content := ` +function trivialA() { return 1; } +function trivialB() { return 2; } +function branchy(x) { + if (x > 0) { return 1; } + if (x < 0) { return -1; } + return 0; +} +` + if err := os.WriteFile(jsFile, []byte(content), 0644); err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + + service := NewComplexityService(&config.ComplexityConfig{ + LowThreshold: 5, + MediumThreshold: 10, + Enabled: true, + ReportUnchanged: true, + }) + + resp, err := service.Analyze(context.Background(), domain.ComplexityRequest{ + Paths: []string{jsFile}, + MinComplexity: 2, + }) + if err != nil { + t.Fatalf("Analyze should not return error: %v", err) + } + + rollup, ok := resp.ModuleRollups[jsFile] + if !ok { + t.Fatalf("expected a rollup for %s, got %v", jsFile, resp.ModuleRollups) + } + if rollup.AnalyzedFunctionCount != 3 { + t.Errorf("rollups should count the functions min_complexity dropped, got %d", rollup.AnalyzedFunctionCount) + } + if rollup.MaxComplexity != 3 { + t.Errorf("MaxComplexity should be 3, got %d", rollup.MaxComplexity) + } + if rollup.LinesOfCode != 9 { + t.Errorf("LinesOfCode should be 9, got %d", rollup.LinesOfCode) + } +} + +func TestComplexityService_Analyze_ByDirectoryReportsRootRelativePaths(t *testing.T) { + tempDir := t.TempDir() + nested := filepath.Join(tempDir, "nested") + if err := os.MkdirAll(nested, 0755); err != nil { + t.Fatalf("Failed to create directory: %v", err) + } + + rootFile := filepath.Join(tempDir, "root.js") + nestedFile := filepath.Join(nested, "deep.js") + if err := os.WriteFile(rootFile, []byte("function plain() { return 1; }\n"), 0644); err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + nestedContent := ` +function branchy(x) { + for (const item of x) { + if (item) { return item; } + } + return null; +} +` + if err := os.WriteFile(nestedFile, []byte(nestedContent), 0644); err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + + service := NewComplexityService(&config.ComplexityConfig{ + LowThreshold: 5, + MediumThreshold: 10, + Enabled: true, + ReportUnchanged: true, + }) + + resp, err := service.Analyze(context.Background(), domain.ComplexityRequest{ + Paths: []string{rootFile, nestedFile}, + }) + if err != nil { + t.Fatalf("Analyze should not return error: %v", err) + } + + byPath := make(map[string]domain.DirectoryComplexityMetrics, len(resp.ByDirectory)) + for _, directory := range resp.ByDirectory { + byPath[directory.DirectoryPath] = directory + } + + if len(byPath) != 2 { + t.Fatalf("expected 2 directories, got %v", resp.ByDirectory) + } + if root, ok := byPath["."]; !ok || root.FunctionCount != 1 { + t.Errorf("expected one function directly in the root, got %+v", byPath) + } + deep, ok := byPath["nested"] + if !ok { + t.Fatalf("expected a rollup for the nested directory, got %v", resp.ByDirectory) + } + if deep.MaxNestingDepth != 2 { + t.Errorf("expected a max nesting depth of 2, got %d", deep.MaxNestingDepth) + } +} + func TestComplexityService_filterFunctions(t *testing.T) { cfg := &config.ComplexityConfig{ LowThreshold: 5, diff --git a/jscan/service/dead_code_aggregate.go b/jscan/service/dead_code_aggregate.go index 8776d5c..f9b1098 100644 --- a/jscan/service/dead_code_aggregate.go +++ b/jscan/service/dead_code_aggregate.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "os" + "path/filepath" "sort" "time" @@ -66,6 +67,24 @@ func scanFileForDeadCode(moduleAnalyzer *analyzer.ModuleAnalyzer, filePath strin return scan } +// moduleDeadCodeRollup accumulates the per-module dead-code counts the unified +// analyze report joins with complexity. It is fed from the detector output +// rather than from the response, so raising min_severity hides findings from +// the report without shrinking a module's measured dead-code weight. +type moduleDeadCodeRollup map[string]domain.ModuleDeadCodeMetrics + +func newModuleDeadCodeRollup() moduleDeadCodeRollup { + return make(moduleDeadCodeRollup) +} + +func (r moduleDeadCodeRollup) add(filePath string, findings, blocks int) { + key := filepath.Clean(filePath) + metrics := r[key] + metrics.DeadCodeFindingCount += findings + metrics.DeadCodeBlockCount += blocks + r[key] = metrics +} + // AnalyzeDeadCode runs dead code analysis using the shared aggregation path. func AnalyzeDeadCode(ctx context.Context, req domain.DeadCodeRequest) (*domain.DeadCodeResponse, error) { return AnalyzeDeadCodeWithTask(ctx, req, nil) @@ -105,6 +124,13 @@ func AnalyzeDeadCodeWithTask(ctx context.Context, req domain.DeadCodeRequest, ta allModuleInfos := make(map[string]*domain.ModuleInfo) analyzedFiles := make(map[string]bool) unusedFuncDedup := make(map[string]map[int]bool) // filePath -> startLine -> true + // The module rollups count what the detectors produced, so they need their + // own dedup of the same locations: unusedFuncDedup only remembers findings + // that passed the severity filter, which is the right rule for the report + // (a hidden finding must not suppress the one remaining report of that + // location) but would let a dropped finding be counted twice here. + rollup := newModuleDeadCodeRollup() + rollupFuncDedup := make(map[string]map[int]bool) addFileLevelFinding := func(f domain.DeadCodeFinding) { if !f.Severity.IsAtLeast(minSeverity) { @@ -170,6 +196,8 @@ func AnalyzeDeadCodeWithTask(ctx context.Context, req domain.DeadCodeRequest, ta fileDeadBlocks := 0 fileTotalBlocks := 0 + rollup.add(filePath, len(scan.value.unusedImports), 0) + for _, finding := range scan.value.unusedImports { f := domain.DeadCodeFinding{ Location: domain.DeadCodeLocation{ @@ -215,6 +243,7 @@ func AnalyzeDeadCodeWithTask(ctx context.Context, req domain.DeadCodeRequest, ta totalFunctions++ fileTotalBlocks += result.TotalBlocks fileDeadBlocks += result.DeadBlocks + rollup.add(filePath, len(result.Findings), result.DeadBlocks) var findings []domain.DeadCodeFinding for _, finding := range result.Findings { @@ -315,6 +344,12 @@ func AnalyzeDeadCodeWithTask(ctx context.Context, req domain.DeadCodeRequest, ta Description: finding.Description, } + rollup.add(finding.FilePath, 1, 0) + if rollupFuncDedup[finding.FilePath] == nil { + rollupFuncDedup[finding.FilePath] = make(map[int]bool) + } + rollupFuncDedup[finding.FilePath][finding.StartLine] = true + addFileLevelFinding(f) if f.Severity.IsAtLeast(minSeverity) { if unusedFuncDedup[finding.FilePath] == nil { @@ -332,6 +367,10 @@ func AnalyzeDeadCodeWithTask(ctx context.Context, req domain.DeadCodeRequest, ta default: } + if lines, ok := rollupFuncDedup[finding.FilePath]; !ok || !lines[finding.StartLine] { + rollup.add(finding.FilePath, 1, 0) + } + if lines, ok := unusedFuncDedup[finding.FilePath]; ok && lines[finding.StartLine] { continue } @@ -365,6 +404,7 @@ func AnalyzeDeadCodeWithTask(ctx context.Context, req domain.DeadCodeRequest, ta Severity: domain.DeadCodeSeverity(finding.Severity), Description: finding.Description, } + rollup.add(finding.FilePath, 1, 0) addFileLevelFinding(f) } @@ -423,12 +463,13 @@ func AnalyzeDeadCodeWithTask(ctx context.Context, req domain.DeadCodeRequest, ta } return &domain.DeadCodeResponse{ - Files: files, - Summary: summary, - Warnings: warnings, - Errors: errors, - GeneratedAt: time.Now().Format(time.RFC3339), - Version: version.Version, + Files: files, + Summary: summary, + ModuleRollups: rollup, + Warnings: warnings, + Errors: errors, + GeneratedAt: time.Now().Format(time.RFC3339), + Version: version.Version, Config: map[string]interface{}{ "min_severity": minSeverity, "sort_by": sortBy, diff --git a/jscan/service/dead_code_service_test.go b/jscan/service/dead_code_service_test.go index 35d96d0..854f099 100644 --- a/jscan/service/dead_code_service_test.go +++ b/jscan/service/dead_code_service_test.go @@ -56,6 +56,63 @@ function noDeadCode() { } } +func TestDeadCodeServiceAnalyze_ModuleRollupsPrecedeTheSeverityFilter(t *testing.T) { + tempDir := t.TempDir() + helperFile := filepath.Join(tempDir, "helper.js") + if err := os.WriteFile(helperFile, []byte("export function helper() { return 1; }\n"), 0644); err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + + testFile := filepath.Join(tempDir, "test.js") + content := ` +import { helper } from "./helper"; + +export function hasDeadCode() { + return 42; + console.log("never executed"); +} +` + if err := os.WriteFile(testFile, []byte(content), 0644); err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + + svc := NewDeadCodeService() + request := domain.DeadCodeRequest{ + Paths: []string{testFile, helperFile}, + SortBy: domain.DeadCodeSortBySeverity, + } + + request.MinSeverity = domain.DeadCodeSeverityInfo + reported, err := svc.Analyze(context.Background(), request) + if err != nil { + t.Fatalf("Analyze failed: %v", err) + } + + // The same project, reported at the strictest severity: the report shrinks, + // the rollups must not. + request.MinSeverity = domain.DeadCodeSeverityCritical + filtered, err := svc.Analyze(context.Background(), request) + if err != nil { + t.Fatalf("Analyze failed: %v", err) + } + + if filtered.Summary.TotalFindings >= reported.Summary.TotalFindings { + t.Fatalf("expected the critical-only report to drop findings, got %d of %d", + filtered.Summary.TotalFindings, reported.Summary.TotalFindings) + } + + rollup, ok := filtered.ModuleRollups[testFile] + if !ok { + t.Fatalf("expected a rollup for %s, got %v", testFile, filtered.ModuleRollups) + } + if rollup != reported.ModuleRollups[testFile] { + t.Errorf("rollups should not depend on min_severity: %+v vs %+v", rollup, reported.ModuleRollups[testFile]) + } + if rollup.DeadCodeFindingCount == 0 || rollup.DeadCodeBlockCount == 0 { + t.Errorf("expected the rollup to count the detected dead code, got %+v", rollup) + } +} + func TestDeadCodeServiceAnalyzeFile(t *testing.T) { // Create a temp file with dead code tempDir := t.TempDir() diff --git a/jscan/service/directory_complexity_formatter.go b/jscan/service/directory_complexity_formatter.go new file mode 100644 index 0000000..20f8c86 --- /dev/null +++ b/jscan/service/directory_complexity_formatter.go @@ -0,0 +1,28 @@ +package service + +import ( + "fmt" + "io" + + "github.com/ludo-technologies/polyscan/jscan/domain" +) + +// writeDirectoryComplexityText renders the directory rollups as plain text. +// Directories arrive ranked worst-first, so a reader who stops early has still +// seen the ones worth acting on. +func writeDirectoryComplexityText(writer io.Writer, directories domain.DirectoryComplexityMetricsList) { + if len(directories) == 0 { + return + } + + fmt.Fprintf(writer, "Directory Complexity:\n") + for _, directory := range directories { + fmt.Fprintf(writer, " %s\n", directory.DirectoryPath) + fmt.Fprintf(writer, " Functions: %d\n", directory.FunctionCount) + fmt.Fprintf(writer, " Complexity: avg %.2f, max %d, high-risk %d\n", + directory.AverageComplexity, directory.MaxComplexity, directory.HighRiskFunctionCount) + fmt.Fprintf(writer, " Nesting: avg %.2f, max %d\n", + directory.AverageNestingDepth, directory.MaxNestingDepth) + } + fmt.Fprintf(writer, "\n") +} diff --git a/jscan/service/html_formatter.go b/jscan/service/html_formatter.go index 9dd5062..7f6d4cd 100644 --- a/jscan/service/html_formatter.go +++ b/jscan/service/html_formatter.go @@ -13,20 +13,22 @@ import ( // HTMLData represents the data for HTML template type HTMLData struct { - GeneratedAt string - Duration int64 - Version string - Complexity *domain.ComplexityResponse - DeadCode *domain.DeadCodeResponse - Clone *domain.CloneResponse - CBO *domain.CBOResponse - Deps *domain.DependencyGraphResponse - Summary *domain.AnalyzeSummary - HasComplexity bool - HasDeadCode bool - HasClone bool - HasCBO bool - HasDeps bool + GeneratedAt string + Duration int64 + Version string + Complexity *domain.ComplexityResponse + DeadCode *domain.DeadCodeResponse + Clone *domain.CloneResponse + CBO *domain.CBOResponse + Deps *domain.DependencyGraphResponse + ModuleQuality []domain.ModuleQualityMetrics + Summary *domain.AnalyzeSummary + HasComplexity bool + HasDeadCode bool + HasClone bool + HasCBO bool + HasDeps bool + HasModuleQuality bool } // WriteHTML writes the analysis result as HTML @@ -56,22 +58,25 @@ func (f *OutputFormatterImpl) WriteHTML( // Build summary (reuse shared logic to avoid score divergence across output formats) summary := BuildAnalyzeSummary(complexityResponse, deadCodeResponse, cloneResponse, cboResponse, depsResponse) + moduleQuality := BuildModuleQuality(complexityResponse, deadCodeResponse, depsResponse) data := HTMLData{ - GeneratedAt: now.Format("2006-01-02 15:04:05"), - Duration: duration.Milliseconds(), - Version: version.Version, - Complexity: complexityResponse, - DeadCode: deadCodeResponse, - Clone: cloneResponse, - CBO: cboResponse, - Deps: depsResponse, - Summary: summary, - HasComplexity: complexityResponse != nil, - HasDeadCode: deadCodeResponse != nil, - HasClone: cloneResponse != nil, - HasCBO: cboResponse != nil, - HasDeps: depsResponse != nil, + GeneratedAt: now.Format("2006-01-02 15:04:05"), + Duration: duration.Milliseconds(), + Version: version.Version, + Complexity: complexityResponse, + DeadCode: deadCodeResponse, + Clone: cloneResponse, + CBO: cboResponse, + Deps: depsResponse, + ModuleQuality: moduleQuality, + Summary: summary, + HasComplexity: complexityResponse != nil, + HasDeadCode: deadCodeResponse != nil, + HasClone: cloneResponse != nil, + HasCBO: cboResponse != nil, + HasDeps: depsResponse != nil, + HasModuleQuality: len(moduleQuality) > 0, } funcMap := template.FuncMap{ @@ -347,6 +352,9 @@ const htmlTemplate = ` {{if .HasDeps}} {{end}} + {{if .HasModuleQuality}} + + {{end}}
| Directory | +Functions | +Avg CC | +Max CC | +High Risk | +Avg Nesting | +Max Nesting | +
|---|---|---|---|---|---|---|
| {{.DirectoryPath}} | +{{.FunctionCount}} | +{{printf "%.2f" .AverageComplexity}} | +{{.MaxComplexity}} | +{{.HighRiskFunctionCount}} | +{{printf "%.2f" .AverageNestingDepth}} | +{{.MaxNestingDepth}} | +
| Module | +File | +LOC | +Analyzed | +Avg CC | +Max CC | +High Risk | +Handlers | +Dead Findings | +Dead Blocks | +
|---|---|---|---|---|---|---|---|---|---|
| {{if $module.ModuleName}}{{$module.ModuleName}}{{else}}—{{end}} | +{{$module.FilePath}} | +{{$module.LinesOfCode}} | +{{$module.AnalyzedFunctionCount}} | +{{printf "%.2f" $module.AverageComplexity}} | +{{$module.MaxComplexity}} | +{{$module.HighRiskFunctionCount}} | +{{$module.ExceptionHandlerCount}} | +{{$module.DeadCodeFindingCount}} | +{{$module.DeadCodeBlockCount}} | +
Showing top 20 of {{len .ModuleQuality}} modules
+ {{end}} +