diff --git a/.github/dependabot.yml b/.github/dependabot.yml index e2abb7d..8855f89 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -5,6 +5,10 @@ updates: schedule: interval: 'weekly' open-pull-requests-limit: 10 + ignore: + - dependency-name: '@types/node' + update-types: + - 'version-update:semver-major' - package-ecosystem: 'github-actions' directory: '/' diff --git a/.oxlintrc.json b/.oxlintrc.json index 34f050b..59e709a 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -17,8 +17,14 @@ }, "rules": { - "complexity/max-cyclomatic": ["warn", { "max": 10 }], - "complexity/max-cognitive": ["warn", { "max": 15, "enableExtraction": true }], + "complexity/complexity": [ + "warn", + { + "cyclomatic": 10, + "cognitive": 15, + "enableExtraction": true + } + ], "no-console": "warn", "no-debugger": "error", diff --git a/CHANGELOG.md b/CHANGELOG.md index 7866bcf..70061d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Performance optimization: `minLines` option to skip complexity analysis for small functions. Default: 10 lines. + +## [1.0.0-rc.1] - 2026-02-08 + +### Added + +- **New `complexity/complexity` rule** - Optimized rule that checks both cyclomatic and cognitive complexity in a single AST walk (17% faster than separate rules) +- Export extraction analysis types and functions from public API - Test fixtures for Svelte (`.svelte`) and Astro (`.astro`) files - Documented framework support: React, Vue, Angular, Svelte, Astro, Solid, Qwik @@ -16,11 +24,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Clean up unused parameters left over from v0.3.2 refactoring across internal APIs +### Deprecated + +- `complexity/max-cyclomatic` - Use `complexity/complexity` instead +- `complexity/max-cognitive` - Use `complexity/complexity` instead + ### Fixed +- Detect `this` references in extraction candidates and flag as medium-confidence issue. +- Detect mutating method calls (`push`, `sort`, `set`, `delete`, etc.) as variable mutations in extraction analysis. - Strengthen extraction tests: replace weak/guarded assertions with exact values and rewrite inline fixtures that produced zero candidates. - Fix `hasEarlyReturn` to use AST-based detection. - Fix `suggestFunctionName` producing incorrect names; replaced with `"extracted"` placeholder. +- Fix exported `MaxCognitiveOptions` type missing extraction and tip-threshold options added in v0.3.0. ## [0.3.2] - 2026-02-01 @@ -91,7 +107,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - GitHub Actions CI pipeline - Pre-commit hooks with Husky -[Unreleased]: https://github.com/itaymendel/oxlint-plugin-complexity/compare/v0.3.2...HEAD +[Unreleased]: https://github.com/itaymendel/oxlint-plugin-complexity/compare/v1.0.0-rc.1...HEAD +[1.0.0-rc.1]: https://github.com/itaymendel/oxlint-plugin-complexity/compare/v0.3.2...v1.0.0-rc.1 [0.3.2]: https://github.com/itaymendel/oxlint-plugin-complexity/compare/v0.3.1...v0.3.2 [0.3.1]: https://github.com/itaymendel/oxlint-plugin-complexity/compare/v0.3.0...v0.3.1 [0.3.0]: https://github.com/itaymendel/oxlint-plugin-complexity/compare/v0.2.0...v0.3.0 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f1cf278..b0db749 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -3,8 +3,8 @@ ## Development Setup ```bash -npm install -npm run build +pnpm install +pnpm run build ``` ## Testing @@ -12,9 +12,9 @@ npm run build Run the test suite: ```bash -npm test # Watch mode -npm run test:run # Single run -npm run lint # Dogfood: lint this plugin with itself +pnpm test # Watch mode +pnpm run test:run # Single run +pnpm run lint # Dogfood: lint this plugin with itself ``` ### Fixture-Based Testing diff --git a/README.md b/README.md index ea16ba9..ea6acdb 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ Cyclomatic and cognitive complexity rules for [oxlint](https://oxc.rs/docs/guide - **Framework support:** React, Vue, Angular, Svelte, Astro, Solid, Qwik - **File types:** `.js` `.mjs` `.cjs` `.ts` `.tsx` `.jsx` `.vue` `.svelte` `.astro` -> **Note:** Only cognitive complexity tracks nesting depth, which enables more actionable suggestions, so refactoring tips available only there. +> **Note:** Refactoring tips require cognitive complexity (only it tracks nesting depth). ## Quick Start @@ -23,18 +23,23 @@ npm install oxlint-plugin-complexity --save-dev { "jsPlugins": ["oxlint-plugin-complexity"], "rules": { - "complexity/max-cyclomatic": ["error", { "max": 20 }], - "complexity/max-cognitive": ["error", { "max": 15 }] + "complexity/complexity": [ + "error", + { + "cyclomatic": 20, + "cognitive": 15 + } + ] } } ``` ## Actionable Error Messages -Error messages include a summary and detailed line-by-line breakdown with the top offender highlighted. When deep nesting is detected, a refactoring tip is shown: +Error messages show a summary, line-by-line breakdown, and refactoring tips for deep nesting: ``` -complexity(max-cognitive): Function 'processData' has Cognitive Complexity of 15. +complexity(complexity): Function 'processData' has Cognitive Complexity of 15. Maximum allowed is 10. [if: +14, for: +1] Breakdown: @@ -66,58 +71,58 @@ function processData(items, mode, config) { } ``` -## Rules +## Rule Configuration -### `complexity/max-cyclomatic` +```jsonc +{ + "complexity/complexity": [ + "error", + { + // Complexity thresholds + "cyclomatic": 20, // Default: 20 + "cognitive": 15, // Default: 15 + + // Performance optimization (optional) + "minLines": 10, // Default: 10 (skip functions <10 lines like getters; 0 = analyze all; counts comments/blanks) + + // Extraction suggestions (optional) + "enableExtraction": true, // Default: false + "extractionMultiplier": 1.5, // Default: 1.5 (triggers at 1.5× cognitive threshold) + "minExtractionPercentage": 30, // Default: 30 (min % of total complexity to suggest) + + // Refactoring tip thresholds (optional, set to 0 to disable) + "nestingTipThreshold": 3, // Default: 3 + "elseIfChainThreshold": 4, // Default: 4 + "logicalOperatorThreshold": 3, // Default: 3 + }, + ], +} +``` -Enforces maximum [cyclomatic complexity](https://en.wikipedia.org/wiki/Cyclomatic_complexity) (default: 20). +### Cyclomatic Complexity + +Counts decision points in code. [Learn more](https://en.wikipedia.org/wiki/Cyclomatic_complexity) **+1 for:** `if`, `for`, `for...in`, `for...of`, `while`, `do...while`, `case`, `catch`, `? :`, `&&`, `||`, `??` -### `complexity/max-cognitive` +### Cognitive Complexity -Enforces maximum [cognitive complexity](https://www.sonarsource.com/resources/cognitive-complexity/) (default: 15). +Measures how difficult code is to understand by penalizing nesting. [Learn more](https://www.sonarsource.com/resources/cognitive-complexity/) - **+1 for:** `if`/`for`/`while`/`switch`/`catch`/`? :` (+nesting), `else`, logical sequence changes, nested functions, recursion - **Excluded:** React components (PascalCase + returns JSX), default value patterns (`a || []`) -#### Refactoring Tips +### Refactoring Tips -The plugin detects common complexity patterns and provides actionable tips. All thresholds are configurable (set to `0` to disable): +Detects common complexity patterns and provides actionable tips: -```jsonc -{ - "complexity/max-cognitive": [ - "error", - { - "max": 15, - "nestingTipThreshold": 3, - "elseIfChainThreshold": 4, - "logicalOperatorThreshold": 3, - }, - ], -} -``` +- **Deep nesting** (`nestingTipThreshold`): Suggests extracting inner loops/conditions +- **Long else-if chains** (`elseIfChainThreshold`): Recommends lookup tables or strategy pattern +- **Logical operator sequences** (`logicalOperatorThreshold`): Suggests extracting boolean expressions -#### Extraction Suggestions (Experimental) +### Extraction Suggestions -Enable `enableExtraction` to get refactoring suggestions for complex functions. Analyzes variable flow to identify extractable code blocks and potential issues. - -```jsonc -{ - "complexity/max-cognitive": [ - "error", - { - "max": 15, - "enableExtraction": true, - // Only suggest extractions when complexity exceeds 1.5× of max-cognitive threshold - "extractionMultiplier": 1.5, - // Only suggest blocks containing at least this % of total complexity - "minExtractionPercentage": 30, - }, - ], -} -``` +When `enableExtraction: true`, analyzes variable flow to identify extractable code blocks: **Example output:** @@ -143,6 +148,36 @@ Inputs: config: Config, results: number[] Suggested: processBlock(config: Config, results: number[]): void ``` +#### Known Limitations + +Extraction suggestions use static analysis heuristics and may miss: + +- **Globals/module variables** (not tracked by variable flow analysis) +- **Complex flows** (closures, dynamic properties, indirect mutations) + +Always review suggestions before applying, even when marked "high confidence". + +--- + +## Migration from v0.x to v1.0 + +**v1.0:** Combined rule for better performance. Separate rules deprecated: + +```diff +// .oxlintrc.json +{ + "jsPlugins": ["oxlint-plugin-complexity"], + "rules": { +- "complexity/max-cyclomatic": ["error", { "max": 20 }], +- "complexity/max-cognitive": ["error", { "max": 15 }] ++ "complexity/complexity": ["error", { ++ "cyclomatic": 20, ++ "cognitive": 15 ++ }] + } +} +``` + --- ## Attribution diff --git a/package.json b/package.json index 9b45fcf..15a175e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "oxlint-plugin-complexity", - "version": "0.3.2", + "version": "1.0.0-rc.2", "description": "Cyclomatic and cognitive complexity rules for oxlint", "keywords": [ "oxlint", @@ -55,6 +55,7 @@ "oxc-parser": "^0.112.0", "oxlint": "^1.39.0", "prettier": "3.8.1", + "tsx": "^4.21.0", "typescript": "^5.9.3", "vitest": "^4.0.17" }, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 63e4993..6e13dc7 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -26,12 +26,15 @@ importers: prettier: specifier: 3.8.1 version: 3.8.1 + tsx: + specifier: ^4.21.0 + version: 4.21.0 typescript: specifier: ^5.9.3 version: 5.9.3 vitest: specifier: ^4.0.17 - version: 4.0.18(@types/node@22.19.5) + version: 4.0.18(@types/node@22.19.5)(tsx@4.21.0) packages: @@ -577,6 +580,9 @@ packages: engines: {node: ^8.16.0 || ^10.6.0 || >=11.0.0} os: [darwin] + get-tsconfig@4.13.6: + resolution: {integrity: sha512-shZT/QMiSHc/YBLxxOkMtgSid5HFoauqCE3/exfsEcwg1WkeqjG+V40yBbBrsD+jW2HDXcs28xOfcbm2jI8Ddw==} + husky@9.1.7: resolution: {integrity: sha512-5gs5ytaNjBrh5Ow3zrvdUUY+0VxIuWVL4i9irt6friV+BqdCfmV11CQTWMiBYWHbXhco+J1kHfTOUkePhCDvMA==} engines: {node: '>=18'} @@ -626,6 +632,9 @@ packages: engines: {node: '>=14'} hasBin: true + resolve-pkg-maps@1.0.0: + resolution: {integrity: sha512-seS2Tj26TBVOC2NIc2rOe2y2ZO7efxITtLZcGSOnHHNOQ7CkiUBfw0Iw2ck6xkIhPwLhKNLS8BO+hEpngQlqzw==} + rollup@4.57.0: resolution: {integrity: sha512-e5lPJi/aui4TO1LpAXIRLySmwXSE8k3b9zoGfd42p67wzxog4WHjiZF3M2uheQih4DGyc25QEV4yRBbpueNiUA==} engines: {node: '>=18.0.0', npm: '>=8.0.0'} @@ -662,6 +671,11 @@ packages: tslib@2.8.1: resolution: {integrity: sha512-oJFu94HQb+KVduSUQL7wnpmqnfmLsOA/nAh6b6EH0wCEoK0/mPeXU6c3wKDV83MkOuHPRHtSXKKU99IBazS/2w==} + tsx@4.21.0: + resolution: {integrity: sha512-5C1sg4USs1lfG0GFb2RLXsdpXqBSEhAaA/0kPL01wxzpMqLILNxIxIOKiILz+cdg/pLnOUxFYOR5yhHU666wbw==} + engines: {node: '>=18.0.0'} + hasBin: true + typescript@5.9.3: resolution: {integrity: sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==} engines: {node: '>=14.17'} @@ -1046,13 +1060,13 @@ snapshots: chai: 6.2.2 tinyrainbow: 3.0.3 - '@vitest/mocker@4.0.18(vite@7.3.1(@types/node@22.19.5))': + '@vitest/mocker@4.0.18(vite@7.3.1(@types/node@22.19.5)(tsx@4.21.0))': dependencies: '@vitest/spy': 4.0.18 estree-walker: 3.0.3 magic-string: 0.30.21 optionalDependencies: - vite: 7.3.1(@types/node@22.19.5) + vite: 7.3.1(@types/node@22.19.5)(tsx@4.21.0) '@vitest/pretty-format@4.0.18': dependencies: @@ -1124,6 +1138,10 @@ snapshots: fsevents@2.3.3: optional: true + get-tsconfig@4.13.6: + dependencies: + resolve-pkg-maps: 1.0.0 + husky@9.1.7: {} magic-string@0.30.21: @@ -1184,6 +1202,8 @@ snapshots: prettier@3.8.1: {} + resolve-pkg-maps@1.0.0: {} + rollup@4.57.0: dependencies: '@types/estree': 1.0.8 @@ -1237,11 +1257,18 @@ snapshots: tslib@2.8.1: optional: true + tsx@4.21.0: + dependencies: + esbuild: 0.27.2 + get-tsconfig: 4.13.6 + optionalDependencies: + fsevents: 2.3.3 + typescript@5.9.3: {} undici-types@6.21.0: {} - vite@7.3.1(@types/node@22.19.5): + vite@7.3.1(@types/node@22.19.5)(tsx@4.21.0): dependencies: esbuild: 0.27.2 fdir: 6.5.0(picomatch@4.0.3) @@ -1252,11 +1279,12 @@ snapshots: optionalDependencies: '@types/node': 22.19.5 fsevents: 2.3.3 + tsx: 4.21.0 - vitest@4.0.18(@types/node@22.19.5): + vitest@4.0.18(@types/node@22.19.5)(tsx@4.21.0): dependencies: '@vitest/expect': 4.0.18 - '@vitest/mocker': 4.0.18(vite@7.3.1(@types/node@22.19.5)) + '@vitest/mocker': 4.0.18(vite@7.3.1(@types/node@22.19.5)(tsx@4.21.0)) '@vitest/pretty-format': 4.0.18 '@vitest/runner': 4.0.18 '@vitest/snapshot': 4.0.18 @@ -1273,7 +1301,7 @@ snapshots: tinyexec: 1.0.2 tinyglobby: 0.2.15 tinyrainbow: 3.0.3 - vite: 7.3.1(@types/node@22.19.5) + vite: 7.3.1(@types/node@22.19.5)(tsx@4.21.0) why-is-node-running: 2.3.0 optionalDependencies: '@types/node': 22.19.5 diff --git a/src/cognitive/patterns.ts b/src/cognitive/patterns.ts index e93136a..b009155 100644 --- a/src/cognitive/patterns.ts +++ b/src/cognitive/patterns.ts @@ -1,6 +1,8 @@ import type { Context, ESTreeNode, LogicalExpressionNode, IfStatementNode } from '../types.js'; +import { includes } from '../utils.js'; const DEFAULT_VALUE_OPERATORS = ['||', '??'] as const; +const LITERAL_TYPES = new Set(['Literal', 'ArrayExpression', 'ObjectExpression']); export function isElseIf(node: IfStatementNode): boolean { const parent = node.parent as IfStatementNode | undefined; @@ -46,13 +48,12 @@ function getChainRoot( export function isDefaultValuePattern(node: LogicalExpressionNode, context: Context): boolean { const { operator } = node; - if (!DEFAULT_VALUE_OPERATORS.includes(operator as (typeof DEFAULT_VALUE_OPERATORS)[number])) { + if (!includes(DEFAULT_VALUE_OPERATORS, operator)) { return false; } const rightmost = getRightmostInChain(node, DEFAULT_VALUE_OPERATORS); - const literalTypes = ['Literal', 'ArrayExpression', 'ObjectExpression']; - if (!literalTypes.includes(rightmost.type)) { + if (!LITERAL_TYPES.has(rightmost.type)) { return false; } diff --git a/src/cognitive/react.ts b/src/cognitive/react.ts deleted file mode 100644 index 10d64bb..0000000 --- a/src/cognitive/react.ts +++ /dev/null @@ -1,94 +0,0 @@ -import type { - ESTreeNode, - FunctionNode, - LogicalExpressionNode, - ConditionalExpressionNode, - BlockStatementNode, -} from '../types.js'; - -function containsJsx(node: ESTreeNode | null | undefined): boolean { - if (!node) return false; - - if (node.type === 'JSXElement' || node.type === 'JSXFragment') { - return true; - } - - // Handle conditional: condition ? : - if (node.type === 'ConditionalExpression') { - const condNode = node as ConditionalExpressionNode; - return ( - containsJsx(condNode.consequent as ESTreeNode) || - containsJsx(condNode.alternate as ESTreeNode) - ); - } - - // Handle logical expressions: show && or show || - if (node.type === 'LogicalExpression') { - const logicNode = node as LogicalExpressionNode; - return containsJsx(logicNode.left as ESTreeNode) || containsJsx(logicNode.right as ESTreeNode); - } - - return false; -} - -/** Check if a return statement contains JSX */ -function checkReturnStatement(statement: ESTreeNode): boolean { - if (statement.type !== 'ReturnStatement') return false; - const returnStmt = statement as { argument?: ESTreeNode | null }; - return containsJsx(returnStmt.argument); -} - -/** Check if a statement has a body block containing JSX returns */ -function checkBodyBlock(statement: ESTreeNode): boolean { - if (!('body' in statement) || !statement.body) return false; - const body = statement.body; - if (Array.isArray(body) || body.type !== 'BlockStatement') return false; - return hasJsxReturn(body as BlockStatementNode); -} - -/** Check if a statement has a consequent block containing JSX returns */ -function checkConsequentBlock(statement: ESTreeNode): boolean { - if (!('consequent' in statement) || !statement.consequent) return false; - const consequent = statement.consequent as ESTreeNode; - if (consequent.type !== 'BlockStatement') return false; - return hasJsxReturn(consequent as BlockStatementNode); -} - -function hasJsxReturn(node: BlockStatementNode): boolean { - if (!node?.body) return false; - - return node.body.some( - (statement: ESTreeNode) => - checkReturnStatement(statement) || - checkBodyBlock(statement) || - checkConsequentBlock(statement) - ); -} - -/** - * Check if a function looks like a React functional component - * - Name starts with uppercase letter - * - Returns JSX - */ -export function isReactComponent(node: FunctionNode, name: string | null): boolean { - if (!name || !/^[A-Z]/.test(name)) { - return false; - } - - // Check if function body returns JSX - // MethodDefinition has value.body, Function/Arrow have body directly - const body = 'value' in node ? node.value.body : 'body' in node ? node.body : null; - if (!body) return false; - - // Arrow function with direct JSX return: () =>
- if (body.type === 'JSXElement' || body.type === 'JSXFragment') { - return true; - } - - // Function with block body - check for JSX returns - if (body.type === 'BlockStatement') { - return hasJsxReturn(body as BlockStatementNode); - } - - return false; -} diff --git a/src/cognitive/visitor.ts b/src/cognitive/visitor.ts index 5585cad..2b48709 100644 --- a/src/cognitive/visitor.ts +++ b/src/cognitive/visitor.ts @@ -14,7 +14,6 @@ import type { FunctionScope, } from '../types.js'; import { isElseIf, isDefaultValuePattern, isJsxShortCircuit } from './patterns.js'; -import { isReactComponent } from './react.js'; import { isRecursiveCall } from './recursion.js'; import { createComplexityPoint, DEFAULT_COMPLEXITY_INCREMENT, getFunctionName } from '../utils.js'; import { createComplexityVisitor } from '../visitor.js'; @@ -24,7 +23,6 @@ import type { VariableInfo } from '../extraction/types.js'; interface CognitiveFunctionScope extends FunctionScope { nestingLevel: number; nestingNodes: Set; - isReactComponent: boolean; hasRecursiveCall: boolean; } @@ -36,20 +34,23 @@ interface VisitorContext { ruleContext: Context; } -function handleNestingChange( +function handleNestingEnter( node: ESTreeNode, - getCurrentScope: () => CognitiveFunctionScope | undefined, - direction: 'enter' | 'exit' + getCurrentScope: () => CognitiveFunctionScope | undefined ): void { const scope = getCurrentScope(); if (!scope?.nestingNodes.has(node)) return; + scope.nestingLevel++; +} - if (direction === 'enter') { - scope.nestingLevel++; - } else { - scope.nestingLevel--; - scope.nestingNodes.delete(node); - } +function handleNestingExit( + node: ESTreeNode, + getCurrentScope: () => CognitiveFunctionScope | undefined +): void { + const scope = getCurrentScope(); + if (!scope?.nestingNodes.has(node)) return; + scope.nestingLevel--; + scope.nestingNodes.delete(node); } function handleIfStatement(node: IfStatementNode, ctx: VisitorContext): void { @@ -99,8 +100,8 @@ function buildVisitorHandlers(baseVisitor: Partial, ctx: VisitorContext return { ...baseVisitor, - '*': (node: ESTreeNode) => handleNestingChange(node, ctx.getCurrentScope, 'enter'), - '*:exit': (node: ESTreeNode) => handleNestingChange(node, ctx.getCurrentScope, 'exit'), + '*': (node: ESTreeNode) => handleNestingEnter(node, ctx.getCurrentScope), + '*:exit': (node: ESTreeNode) => handleNestingExit(node, ctx.getCurrentScope), CallExpression(node: CallExpressionNode): void { const scope = ctx.getCurrentScope(); @@ -161,7 +162,6 @@ function createCognitiveVisitorCore( points: [], nestingLevel: 0, nestingNodes: new Set(), - isReactComponent: isReactComponent(node as FunctionNode, name), hasRecursiveCall: false, }), diff --git a/src/combined-visitor.ts b/src/combined-visitor.ts new file mode 100644 index 0000000..3a97d63 --- /dev/null +++ b/src/combined-visitor.ts @@ -0,0 +1,314 @@ +import type { + Visitor, + ESTreeNode, + FunctionScope, + ComplexityPoint, + LogicalExpressionNode, + SwitchCaseNode, + IfStatementNode, + CatchClauseNode, + AssignmentExpressionNode, + LabeledJumpStatementNode, + CallExpressionNode, + ConditionalExpressionNode, + Context, +} from './types.js'; +import { createComplexityVisitor } from './visitor.js'; +import { + BASE_FUNCTION_COMPLEXITY, + LOGICAL_OPERATORS, + LOGICAL_ASSIGNMENT_OPERATORS, + createComplexityPoint, + DEFAULT_COMPLEXITY_INCREMENT, + includes, +} from './utils.js'; +import { isElseIf, isDefaultValuePattern, isJsxShortCircuit } from './cognitive/patterns.js'; +import { isRecursiveCall } from './cognitive/recursion.js'; + +interface CombinedComplexityScope extends FunctionScope { + nestingLevel: number; + nestingNodes: Set; + hasRecursiveCall: boolean; + cyclomaticPoints: ComplexityPoint[]; + cognitivePoints: ComplexityPoint[]; +} + +export interface CombinedComplexityResult { + cyclomatic: number; + cognitive: number; + cyclomaticPoints: ComplexityPoint[]; + cognitivePoints: ComplexityPoint[]; +} + +/** + * Create a combined visitor that calculates both cyclomatic and cognitive complexity + * in a single AST walk. + */ +// eslint-disable-next-line complexity/complexity -- Visitor factory pattern requires many nested handlers +export function createCombinedComplexityVisitor( + context: Context, + onComplexityCalculated: (result: CombinedComplexityResult, node: ESTreeNode) => void +): Visitor { + let globalFunctionNestingLevel = 0; + + const { context: visitorContext, baseVisitor } = createComplexityVisitor( + { + createScope: (node, name) => ({ + node, + name, + points: [], + cyclomaticPoints: [], + cognitivePoints: [], + nestingLevel: 0, + nestingNodes: new Set(), + hasRecursiveCall: false, + }), + + onEnterFunction(parentScope, node, _scope) { + // Only add nested function penalty for functions inside other functions + // (not for top-level arrow functions or callbacks) + // The penalty is added to the PARENT scope, not the nested function's own scope + if (parentScope && globalFunctionNestingLevel > 0) { + const functionType = + node.type === 'ArrowFunctionExpression' ? 'arrow function' : 'function'; + parentScope.cognitivePoints.push(createComplexityPoint(node, `nested ${functionType}`)); + } + globalFunctionNestingLevel++; + }, + + onExitFunction(scope, node) { + globalFunctionNestingLevel--; + + const cyclomatic = scope.cyclomaticPoints.reduce( + (sum, point) => sum + point.complexity, + BASE_FUNCTION_COMPLEXITY + ); + const cognitive = scope.cognitivePoints.reduce((sum, point) => sum + point.complexity, 0); + + onComplexityCalculated( + { + cyclomatic, + cognitive, + cyclomaticPoints: scope.cyclomaticPoints, + cognitivePoints: scope.cognitivePoints, + }, + node + ); + }, + + // Required by the base visitor interface; actual reporting is in onExitFunction + onComplexityCalculated() {}, + } + ); + + const { getCurrentScope } = visitorContext; + + function addCyclomatic(node: ESTreeNode, message: string, amount: number = 1): void { + const scope = getCurrentScope(); + if (scope) { + scope.cyclomaticPoints.push(createComplexityPoint(node, message, amount)); + } + } + + function addCognitive(node: ESTreeNode, message: string): void { + const scope = getCurrentScope(); + if (scope) { + scope.cognitivePoints.push(createComplexityPoint(node, message)); + } + } + + function addStructuralCognitive(node: ESTreeNode, message: string): void { + const scope = getCurrentScope(); + if (scope) { + scope.cognitivePoints.push( + createComplexityPoint(node, message, DEFAULT_COMPLEXITY_INCREMENT, scope.nestingLevel) + ); + } + } + + function addNestingNode(node: ESTreeNode): void { + const scope = getCurrentScope(); + if (scope) { + scope.nestingNodes.add(node); + } + } + + function handleNestingEnter(node: ESTreeNode): void { + const scope = getCurrentScope(); + if (!scope?.nestingNodes.has(node)) return; + scope.nestingLevel++; + } + + function handleNestingExit(node: ESTreeNode): void { + const scope = getCurrentScope(); + if (!scope?.nestingNodes.has(node)) return; + scope.nestingLevel--; + scope.nestingNodes.delete(node); + } + + function handleIfStatement(node: IfStatementNode): void { + addCyclomatic(node, 'if'); + + if (isElseIf(node)) { + addCognitive(node, 'else if'); + } else { + addStructuralCognitive(node, 'if'); + addNestingNode(node.consequent); + } + } + + function handleLogicalExpression(node: LogicalExpressionNode): void { + if (!getCurrentScope()) return; + + const operator = node.operator; + if (!includes(LOGICAL_OPERATORS, operator)) return; + + // Cyclomatic: always count logical operators + addCyclomatic(node, operator); + + // Cognitive: skip default value patterns and JSX short-circuit + if (isDefaultValuePattern(node, context) || isJsxShortCircuit(node)) { + return; + } + + // Cognitive: only count if NOT a continuation of the same operator + // (e.g., a && b && c counts as +1, not +3) + const parent = node.parent as LogicalExpressionNode | undefined; + const isContinuationOfSameOperator = + parent?.type === 'LogicalExpression' && parent.operator === operator; + + if (!isContinuationOfSameOperator) { + addCognitive(node, `logical operator '${operator}'`); + } + } + + function handleLabeledJump(node: ESTreeNode, keyword: string): void { + const stmt = node as LabeledJumpStatementNode; + if (stmt.label) { + addCognitive(node, `${keyword} to label '${stmt.label.name}'`); + } + } + + // Loop handler factory: all loop types follow the same enter/exit pattern + function createLoopHandlers(label: string): { + enter: (node: ESTreeNode) => void; + exit: (node: ESTreeNode) => void; + } { + return { + enter(node: ESTreeNode) { + addCyclomatic(node, label); + addStructuralCognitive(node, label); + addNestingNode((node as { body: ESTreeNode }).body); + }, + exit(node: ESTreeNode) { + handleNestingExit((node as { body: ESTreeNode }).body); + }, + }; + } + + const forLoop = createLoopHandlers('for'); + const forInLoop = createLoopHandlers('for-in'); + const forOfLoop = createLoopHandlers('for-of'); + const whileLoop = createLoopHandlers('while'); + const doWhileLoop = createLoopHandlers('do-while'); + + return { + ...baseVisitor, + + // Wildcard handlers to track nesting level for all nodes + '*'(node: ESTreeNode) { + handleNestingEnter(node); + }, + '*:exit'(node: ESTreeNode) { + handleNestingExit(node); + }, + + IfStatement(node: ESTreeNode) { + handleIfStatement(node as IfStatementNode); + }, + 'IfStatement:exit'(node: ESTreeNode) { + const ifNode = node as IfStatementNode; + handleNestingExit(ifNode.consequent); + + // Add 'else' complexity only for plain else blocks, not else-if chains + if (ifNode.alternate && ifNode.alternate.type !== 'IfStatement') { + addCognitive(node, 'else'); + addNestingNode(ifNode.alternate); + } + }, + 'IfStatement > .alternate:exit'(node: ESTreeNode) { + handleNestingExit(node); + }, + + ForStatement: forLoop.enter, + 'ForStatement:exit': forLoop.exit, + ForInStatement: forInLoop.enter, + 'ForInStatement:exit': forInLoop.exit, + ForOfStatement: forOfLoop.enter, + 'ForOfStatement:exit': forOfLoop.exit, + WhileStatement: whileLoop.enter, + 'WhileStatement:exit': whileLoop.exit, + DoWhileStatement: doWhileLoop.enter, + 'DoWhileStatement:exit': doWhileLoop.exit, + + SwitchCase(node: ESTreeNode) { + const switchCase = node as SwitchCaseNode; + if (switchCase.test !== null) { + addCyclomatic(node, 'case'); + } + }, + SwitchStatement(node: ESTreeNode) { + addStructuralCognitive(node, 'switch'); + addNestingNode(node); + }, + 'SwitchStatement:exit'(node: ESTreeNode) { + handleNestingExit(node); + }, + + CatchClause(node: ESTreeNode) { + addCyclomatic(node, 'catch'); + addStructuralCognitive(node, 'catch'); + addNestingNode((node as CatchClauseNode).body); + }, + 'CatchClause:exit'(node: ESTreeNode) { + handleNestingExit((node as CatchClauseNode).body); + }, + + ConditionalExpression(node: ESTreeNode) { + const ternary = node as ConditionalExpressionNode; + addCyclomatic(node, 'ternary'); + addStructuralCognitive(node, 'ternary operator'); + // Add nesting for both branches to properly track nested ternaries + addNestingNode(ternary.consequent as ESTreeNode); + addNestingNode(ternary.alternate as ESTreeNode); + }, + + LogicalExpression(node: ESTreeNode) { + handleLogicalExpression(node as LogicalExpressionNode); + }, + + AssignmentExpression(node: ESTreeNode) { + const assignment = node as AssignmentExpressionNode; + if (includes(LOGICAL_ASSIGNMENT_OPERATORS, assignment.operator)) { + addCyclomatic(node, assignment.operator); + } + }, + + BreakStatement(node: ESTreeNode) { + handleLabeledJump(node, 'break'); + }, + ContinueStatement(node: ESTreeNode) { + handleLabeledJump(node, 'continue'); + }, + + CallExpression(node: ESTreeNode) { + const scope = getCurrentScope(); + if (!scope?.name) return; + + if (!scope.hasRecursiveCall && isRecursiveCall(node as CallExpressionNode, scope.name)) { + scope.hasRecursiveCall = true; + addCognitive(node, 'recursive call'); + } + }, + } as Visitor; +} diff --git a/src/cyclomatic.ts b/src/cyclomatic.ts index 284f5fa..9b171cc 100644 --- a/src/cyclomatic.ts +++ b/src/cyclomatic.ts @@ -4,25 +4,16 @@ import type { FunctionScope, LogicalExpressionNode, SwitchCaseNode, - IfStatementNode, - ForStatementNode, - ForInStatementNode, - ForOfStatementNode, - WhileStatementNode, - DoWhileStatementNode, - CatchClauseNode, - ConditionalExpressionNode, AssignmentExpressionNode, ComplexityResult, } from './types.js'; import { createComplexityVisitor } from './visitor.js'; -import { BASE_FUNCTION_COMPLEXITY, LOGICAL_OPERATORS } from './utils.js'; - -/** - * Logical assignment operators (short-circuit assignment). - * Only used by cyclomatic complexity. - */ -const LOGICAL_ASSIGNMENT_OPERATORS = ['||=', '&&=', '??='] as const; +import { + BASE_FUNCTION_COMPLEXITY, + LOGICAL_OPERATORS, + LOGICAL_ASSIGNMENT_OPERATORS, + includes, +} from './utils.js'; /** * Calculate cyclomatic complexity for a function body. @@ -62,35 +53,35 @@ export function createCyclomaticVisitor( return { ...baseVisitor, - IfStatement(node: IfStatementNode): void { + IfStatement(node: ESTreeNode): void { addComplexity(node, 'if'); }, - ForStatement(node: ForStatementNode): void { + ForStatement(node: ESTreeNode): void { addComplexity(node, 'for'); }, - ForInStatement(node: ForInStatementNode): void { + ForInStatement(node: ESTreeNode): void { addComplexity(node, 'for...in'); }, - ForOfStatement(node: ForOfStatementNode): void { + ForOfStatement(node: ESTreeNode): void { addComplexity(node, 'for...of'); }, - WhileStatement(node: WhileStatementNode): void { + WhileStatement(node: ESTreeNode): void { addComplexity(node, 'while'); }, - DoWhileStatement(node: DoWhileStatementNode): void { + DoWhileStatement(node: ESTreeNode): void { addComplexity(node, 'do...while'); }, - CatchClause(node: CatchClauseNode): void { + CatchClause(node: ESTreeNode): void { addComplexity(node, 'catch'); }, - ConditionalExpression(node: ConditionalExpressionNode): void { + ConditionalExpression(node: ESTreeNode): void { addComplexity(node, 'ternary'); }, @@ -102,17 +93,13 @@ export function createCyclomaticVisitor( }, LogicalExpression(node: LogicalExpressionNode): void { - if (LOGICAL_OPERATORS.includes(node.operator as (typeof LOGICAL_OPERATORS)[number])) { + if (includes(LOGICAL_OPERATORS, node.operator)) { addComplexity(node, node.operator); } }, AssignmentExpression(node: AssignmentExpressionNode): void { - if ( - LOGICAL_ASSIGNMENT_OPERATORS.includes( - node.operator as (typeof LOGICAL_ASSIGNMENT_OPERATORS)[number] - ) - ) { + if (includes(LOGICAL_ASSIGNMENT_OPERATORS, node.operator)) { addComplexity(node, node.operator); } }, diff --git a/src/extraction/boundary-detector.ts b/src/extraction/boundary-detector.ts index 93632af..853b547 100644 --- a/src/extraction/boundary-detector.ts +++ b/src/extraction/boundary-detector.ts @@ -1,23 +1,15 @@ import type { ComplexityPoint } from '../types.js'; import type { ExtractionCandidate, ExtractionOptions } from './types.js'; +import { extractConstructFromMessage } from '../utils.js'; const DEFAULT_MIN_COMPLEXITY_PERCENTAGE = 30; const DEFAULT_MAX_COMPLEXITY_PERCENTAGE = 70; // Reject candidates covering too much of the function const DEFAULT_MAX_LINE_GAP = 2; const DEFAULT_MAX_CANDIDATES = 3; -/** - * Extract the construct type from a complexity point message. - * Messages are in format: "+N: construct" or "+N (incl. M for nesting): construct" - */ +/** Strip surrounding quotes from construct names like "'if'" */ function extractConstruct(message: string): string { - const match = message.match(/:\s*(.+)$/); - if (match) { - const construct = match[1].trim(); - // Remove quotes from constructs like "'if'" - return construct.replace(/^'|'$/g, ''); - } - return 'unknown'; + return extractConstructFromMessage(message).replace(/^'|'$/g, ''); } function getUniqueConstructs(points: ComplexityPoint[]): string[] { diff --git a/src/extraction/flow-analyzer.ts b/src/extraction/flow-analyzer.ts index 7a8cd80..73d3c9d 100644 --- a/src/extraction/flow-analyzer.ts +++ b/src/extraction/flow-analyzer.ts @@ -35,17 +35,18 @@ function getWritesInRange( .map((ref) => ({ line: ref.line, type: ref.type as 'write' | 'readwrite' })); } -function detectMutations( +function isDeclaredInRange(variable: VariableInfo, startLine: number, endLine: number): boolean { + return variable.declarationLine >= startLine && variable.declarationLine <= endLine; +} + +function detectDirectMutations( candidate: ExtractionCandidate, variables: Map ): MutationInfo[] { const mutations: MutationInfo[] = []; for (const variable of variables.values()) { - const declaredInRange = - variable.declarationLine >= candidate.startLine && - variable.declarationLine <= candidate.endLine; - if (declaredInRange) continue; + if (isDeclaredInRange(variable, candidate.startLine, candidate.endLine)) continue; const writes = getWritesInRange(variable, candidate.startLine, candidate.endLine); for (const write of writes) { @@ -60,12 +61,10 @@ function detectMutations( return mutations; } -/** Check if a node is a closure (function expression or arrow function) */ function isClosureNode(node: ESTreeNode): boolean { return node.type === 'FunctionExpression' || node.type === 'ArrowFunctionExpression'; } -/** Find the enclosing closure within the candidate range, if any */ function findEnclosingClosure( startNode: ESTreeNode | null | undefined, candidate: ExtractionCandidate @@ -84,11 +83,6 @@ function findEnclosingClosure( return null; } -/** Check if a reference is within the candidate range */ -function isRefInRange(ref: { line: number }, candidate: ExtractionCandidate): boolean { - return ref.line >= candidate.startLine && ref.line <= candidate.endLine; -} - function detectClosures( candidate: ExtractionCandidate, variables: Map @@ -99,7 +93,7 @@ function detectClosures( if (!variable.isMutable || variable.declarationLine >= candidate.startLine) continue; for (const ref of variable.references) { - if (!isRefInRange(ref, candidate)) continue; + if (ref.line < candidate.startLine || ref.line > candidate.endLine) continue; const closure = findEnclosingClosure(ref.node.parent, candidate); if (closure) { @@ -109,17 +103,12 @@ function detectClosures( closureEndLine: closure.endLine, issue: `Captures mutable variable '${variable.name}'`, }); + break; // One closure entry per variable is sufficient } } } - const seen = new Set(); - return closures.filter((c) => { - const key = c.variable.name; - if (seen.has(key)) return false; - seen.add(key); - return true; - }); + return closures; } const NESTED_FUNCTION_TYPES = new Set([ @@ -138,20 +127,31 @@ function isOutsideRange(n: ESTreeNode, startLine: number, endLine: number): bool return !!n.loc && (n.loc.end.line < startLine || n.loc.start.line > endLine); } -function isReturnInRange(n: ESTreeNode, startLine: number, endLine: number): boolean { - return ( - n.type === 'ReturnStatement' && - !!n.loc && - n.loc.start.line >= startLine && - n.loc.start.line <= endLine - ); +function getNodeProp(n: ESTreeNode, key: string): unknown { + return (n as unknown as Record)[key]; +} + +/** + * Walk a MemberExpression chain (e.g. `a.b.c.d`) to its root Identifier. + * Returns null for non-Identifier roots (e.g. `getObj().prop`). + */ +function resolveRootIdentifier(node: ESTreeNode): string | null { + let current: ESTreeNode = node; + while (current.type === 'MemberExpression') { + const obj = getNodeProp(current, 'object'); + if (!isNodeLike(obj)) return null; + current = obj; + } + if (current.type === 'Identifier') { + return getNodeProp(current, 'name') as string; + } + return null; } -/** Walk AST child properties, invoking `visit` on each child node. */ function walkChildren(n: ESTreeNode, visit: (child: ESTreeNode) => void): void { for (const key of Object.keys(n)) { if (SKIP_WALK_KEYS.has(key)) continue; - const child = (n as unknown as Record)[key]; + const child = getNodeProp(n, key); if (Array.isArray(child)) { for (const item of child) { if (isNodeLike(item)) visit(item); @@ -162,6 +162,127 @@ function walkChildren(n: ESTreeNode, visit: (child: ESTreeNode) => void): void { } } +function walkInRange( + root: ESTreeNode, + startLine: number, + endLine: number, + visitor: (n: ESTreeNode) => void +): void { + function walk(n: ESTreeNode): void { + if (isOutsideRange(n, startLine, endLine)) return; + if (n !== root && NESTED_FUNCTION_TYPES.has(n.type)) return; + visitor(n); + walkChildren(n, walk); + } + walk(root); +} + +/** + * Resolve a MemberExpression root to a variable declared outside the candidate range. + * Returns null if the root is not an identifier, unknown, or declared within range. + */ +function findExternalVariable( + memberExpr: ESTreeNode, + variables: Map, + startLine: number, + endLine: number +): VariableInfo | null { + const rootName = resolveRootIdentifier(memberExpr); + if (!rootName) return null; + const variable = variables.get(rootName); + if (!variable || isDeclaredInRange(variable, startLine, endLine)) return null; + return variable; +} + +const MUTATING_METHODS = new Set([ + // Array + 'push', + 'pop', + 'shift', + 'unshift', + 'splice', + 'sort', + 'reverse', + 'fill', + 'copyWithin', + // Map / Set + 'set', + 'add', + 'delete', + 'clear', +]); + +function checkPropertyAssignment( + n: ESTreeNode, + variables: Map, + startLine: number, + endLine: number +): MutationInfo | null { + const left = getNodeProp(n, 'left'); + if (!isNodeLike(left) || left.type !== 'MemberExpression') return null; + const variable = findExternalVariable(left, variables, startLine, endLine); + if (!variable) return null; + return { variable, mutationLine: n.loc?.start.line ?? 0, mutationType: 'assignment' }; +} + +function checkPropertyUpdate( + n: ESTreeNode, + variables: Map, + startLine: number, + endLine: number +): MutationInfo | null { + const arg = getNodeProp(n, 'argument'); + if (!isNodeLike(arg) || arg.type !== 'MemberExpression') return null; + const variable = findExternalVariable(arg, variables, startLine, endLine); + if (!variable) return null; + return { variable, mutationLine: n.loc?.start.line ?? 0, mutationType: 'increment' }; +} + +function checkMethodCallMutation( + n: ESTreeNode, + variables: Map, + startLine: number, + endLine: number +): MutationInfo | null { + const callee = getNodeProp(n, 'callee'); + if (!isNodeLike(callee) || callee.type !== 'MemberExpression') return null; + const prop = getNodeProp(callee, 'property'); + if (!isNodeLike(prop) || prop.type !== 'Identifier') return null; + const methodName = getNodeProp(prop, 'name') as string; + if (!MUTATING_METHODS.has(methodName)) return null; + const variable = findExternalVariable(callee, variables, startLine, endLine); + if (!variable) return null; + return { variable, mutationLine: n.loc?.start.line ?? 0, mutationType: 'method-call' }; +} + +const AST_MUTATION_CHECKERS: Record = { + AssignmentExpression: checkPropertyAssignment, + UpdateExpression: checkPropertyUpdate, + CallExpression: checkMethodCallMutation, +}; + +/** + * Detect property assignments (obj.x = ...), update expressions (obj.x++), + * and mutating method calls (arr.push(...)) on variables declared outside the candidate range. + */ +function detectAstMutations( + candidate: ExtractionCandidate, + variables: Map, + functionNode: ESTreeNode +): MutationInfo[] { + const mutations: MutationInfo[] = []; + const { startLine, endLine } = candidate; + + walkInRange(functionNode, startLine, endLine, (n) => { + const checker = AST_MUTATION_CHECKERS[n.type]; + if (!checker) return; + const mutation = checker(n, variables, startLine, endLine); + if (mutation) mutations.push(mutation); + }); + + return mutations; +} + function collectReturnStatements( node: ESTreeNode, startLine: number, @@ -169,19 +290,17 @@ function collectReturnStatements( ): ESTreeNode[] { const returns: ESTreeNode[] = []; - function walk(n: ESTreeNode | null | undefined): void { - if (!isNodeLike(n)) return; - if (isOutsideRange(n, startLine, endLine)) return; - if (n !== node && NESTED_FUNCTION_TYPES.has(n.type)) return; - - if (isReturnInRange(n, startLine, endLine)) { + walkInRange(node, startLine, endLine, (n) => { + if ( + n.type === 'ReturnStatement' && + n.loc && + n.loc.start.line >= startLine && + n.loc.start.line <= endLine + ) { returns.push(n); } + }); - walkChildren(n, walk); - } - - walk(node); return returns; } @@ -197,6 +316,26 @@ function hasEarlyReturn(candidate: ExtractionCandidate, functionNode: ESTreeNode return returnLine < candidate.endLine - 1; } +function detectThisReferences(candidate: ExtractionCandidate, functionNode: ESTreeNode): boolean { + let found = false; + walkInRange(functionNode, candidate.startLine, candidate.endLine, (n) => { + if (n.type === 'ThisExpression') { + found = true; + } + }); + return found; +} + +function deduplicateMutations(mutations: MutationInfo[]): MutationInfo[] { + const seen = new Set(); + return mutations.filter((m) => { + const key = `${m.variable.name}:${m.mutationLine}`; + if (seen.has(key)) return false; + seen.add(key); + return true; + }); +} + export function analyzeVariableFlow( candidate: ExtractionCandidate, variables: Map, @@ -208,16 +347,14 @@ export function analyzeVariableFlow( for (const variable of variables.values()) { const declaredBefore = variable.declarationLine < candidate.startLine; - const declaredInRange = - variable.declarationLine >= candidate.startLine && - variable.declarationLine <= candidate.endLine; + const declaredInside = isDeclaredInRange(variable, candidate.startLine, candidate.endLine); const readsInRange = hasReadsInRange(variable, candidate.startLine, candidate.endLine); const usedAfterRange = isUsedAfter(variable, candidate.endLine); if (declaredBefore && readsInRange) { inputs.push(variable); - } else if (declaredInRange) { + } else if (declaredInside) { if (usedAfterRange) { outputs.push(variable); } else { @@ -226,7 +363,11 @@ export function analyzeVariableFlow( } } - const mutations = detectMutations(candidate, variables); + const mutations = deduplicateMutations([ + ...detectDirectMutations(candidate, variables), + ...detectAstMutations(candidate, variables, functionNode), + ]); + const closures = detectClosures(candidate, variables); return { @@ -236,5 +377,6 @@ export function analyzeVariableFlow( mutations, closures, hasEarlyReturn: hasEarlyReturn(candidate, functionNode), + hasThisReference: detectThisReferences(candidate, functionNode), }; } diff --git a/src/extraction/index.ts b/src/extraction/index.ts index 56f3f23..43e4687 100644 --- a/src/extraction/index.ts +++ b/src/extraction/index.ts @@ -5,7 +5,6 @@ import type { ESTreeNode, ComplexityPoint } from '../types.js'; import type { ExtractionSuggestion, ExtractionOptions, VariableInfo } from './types.js'; import { findExtractionCandidates } from './boundary-detector.js'; import { analyzeVariableFlow } from './flow-analyzer.js'; -export { PLACEHOLDER_FUNCTION_NAME } from './suggestion-generator.js'; import { createExtractionSuggestion } from './suggestion-generator.js'; const DEFAULT_MIN_COMPLEXITY_MULTIPLIER = 1.5; @@ -17,7 +16,7 @@ export function analyzeExtractionOpportunities( variables: Map, options?: ExtractionOptions ): ExtractionSuggestion[] { - if (!variables || variables.size === 0) { + if (variables.size === 0) { return []; } @@ -26,15 +25,10 @@ export function analyzeExtractionOpportunities( return []; } - const suggestions: ExtractionSuggestion[] = []; - - for (const candidate of candidates) { + return candidates.map((candidate) => { const flow = analyzeVariableFlow(candidate, variables, functionNode); - const suggestion = createExtractionSuggestion(candidate, flow); - suggestions.push(suggestion); - } - - return suggestions; + return createExtractionSuggestion(candidate, flow); + }); } export function shouldAnalyzeExtraction( diff --git a/src/extraction/suggestion-generator.ts b/src/extraction/suggestion-generator.ts index f7fd1fa..d3720c4 100644 --- a/src/extraction/suggestion-generator.ts +++ b/src/extraction/suggestion-generator.ts @@ -34,7 +34,8 @@ function determineConfidence(flow: VariableFlowAnalysis): ExtractionConfidence { if ( inputCount > MAX_HIGH_CONFIDENCE_INPUTS || outputCount > MAX_HIGH_CONFIDENCE_OUTPUTS || - hasEarlyReturn + hasEarlyReturn || + flow.hasThisReference ) { return 'medium'; } @@ -66,9 +67,13 @@ function detectIssues(flow: VariableFlowAnalysis): ExtractionIssue[] { const issues: ExtractionIssue[] = []; for (const mutation of flow.mutations) { + const desc = + mutation.mutationType === 'method-call' + ? `Mutates external variable '${mutation.variable.name}' via method call` + : `Mutates external variable '${mutation.variable.name}'`; issues.push({ type: 'mutation', - description: `Mutates external variable '${mutation.variable.name}'`, + description: desc, line: mutation.mutationLine, variable: mutation.variable.name, }); @@ -104,6 +109,13 @@ function detectIssues(flow: VariableFlowAnalysis): ExtractionIssue[] { }); } + if (flow.hasThisReference) { + issues.push({ + type: 'this-reference', + description: 'Block references `this` which complicates extraction', + }); + } + return issues; } @@ -129,6 +141,11 @@ function generateSuggestions(issues: ExtractionIssue[]): string[] { 'Consider restructuring to avoid early returns, or handle them explicitly' ); break; + case 'this-reference': + suggestions.push( + 'Block references `this` — extracted function will need `.call(this)` or accept the instance as a parameter' + ); + break; } } diff --git a/src/extraction/types.ts b/src/extraction/types.ts index 65c4e1d..f0d5b10 100644 --- a/src/extraction/types.ts +++ b/src/extraction/types.ts @@ -37,6 +37,7 @@ export interface VariableFlowAnalysis { mutations: MutationInfo[]; closures: ClosureInfo[]; hasEarlyReturn: boolean; + hasThisReference: boolean; } export interface MutationInfo { @@ -53,7 +54,13 @@ export interface ClosureInfo { } export interface ExtractionIssue { - type: 'mutation' | 'closure' | 'early-return' | 'too-many-params' | 'multiple-outputs'; + type: + | 'mutation' + | 'closure' + | 'early-return' + | 'too-many-params' + | 'multiple-outputs' + | 'this-reference'; description: string; line?: number; variable?: string; diff --git a/src/extraction/variable-tracker.ts b/src/extraction/variable-tracker.ts index 42c64a8..64aa845 100644 --- a/src/extraction/variable-tracker.ts +++ b/src/extraction/variable-tracker.ts @@ -13,7 +13,7 @@ interface IdentifierNode { /** * Extract TypeScript type annotation string from an identifier node. */ -export function getTypeAnnotation(node: ESTreeNode): string | undefined { +function getTypeAnnotation(node: ESTreeNode): string | undefined { const typed = node as IdentifierNode; if (!typed.typeAnnotation) return undefined; @@ -30,8 +30,8 @@ export function getTypeAnnotation(node: ESTreeNode): string | undefined { /** * Convert a TypeScript type node to a human-readable string. */ -// eslint-disable-next-line complexity/max-cyclomatic -- type mapping switch -export function getTypeString(node: ESTreeNode): string { +// eslint-disable-next-line complexity/complexity -- type mapping switch +function getTypeString(node: ESTreeNode): string { switch (node.type) { case 'TSStringKeyword': return 'string'; diff --git a/src/index.ts b/src/index.ts index ef52cff..7a2c606 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,5 +1,7 @@ import { definePlugin } from 'oxlint/plugins'; -import { maxCyclomatic, maxCognitive } from './rules.js'; +import { maxCyclomatic } from './rules/max-cyclomatic.js'; +import { maxCognitive } from './rules/max-cognitive.js'; +import { complexity } from './rules/complexity.js'; // Re-export types for library users export type { @@ -25,20 +27,39 @@ export { createCognitiveVisitor } from './cognitive/visitor.js'; // Re-export utilities export { getFunctionName, createComplexityPoint, summarizeComplexity } from './utils.js'; +// Re-export extraction analysis +export type { + ExtractionSuggestion, + ExtractionOptions, + ExtractionCandidate, + ExtractionConfidence, + VariableFlowAnalysis, + VariableInfo, + TypedVariable, + ExtractionIssue, +} from './extraction/index.js'; +export { + analyzeExtractionOpportunities, + shouldAnalyzeExtraction, + formatExtractionSuggestions, +} from './extraction/index.js'; + /** * oxlint-plugin-complexity * * Provides cyclomatic and cognitive complexity rules for oxlint. * * Rules: - * - complexity/max-cyclomatic: Enforce maximum cyclomatic complexity - * - complexity/max-cognitive: Enforce maximum cognitive complexity + * - complexity/complexity: RECOMMENDED - Enforce both metrics in one pass (17% faster) + * - complexity/max-cyclomatic: DEPRECATED - Use complexity instead + * - complexity/max-cognitive: DEPRECATED - Use complexity instead */ const plugin = definePlugin({ meta: { name: 'complexity', }, rules: { + complexity, 'max-cyclomatic': maxCyclomatic, 'max-cognitive': maxCognitive, }, diff --git a/src/rules.ts b/src/rules.ts deleted file mode 100644 index 80e14a0..0000000 --- a/src/rules.ts +++ /dev/null @@ -1,260 +0,0 @@ -import { defineRule } from 'oxlint/plugins'; -import type { - Rule, - Context, - Visitor, - FunctionNode, - ComplexityResult, - VisitorWithHooks, - ESTreeNode, -} from './types.js'; -import { - getFunctionName, - summarizeComplexity, - formatBreakdown, - type BreakdownOptions, -} from './utils.js'; -import { createCyclomaticVisitor } from './cyclomatic.js'; -import { - createCognitiveVisitorWithTracking, - type ComplexityResultWithVariables, -} from './cognitive/visitor.js'; -import { - analyzeExtractionOpportunities, - shouldAnalyzeExtraction, - formatExtractionSuggestions, - type ExtractionOptions, -} from './extraction/index.js'; - -/** - * Configuration for creating a complexity rule. - */ -interface ComplexityRuleConfig { - metricName: string; - defaultMax: number; - schemaMinimum: number; - description: string; - url: string; - - /** Factory to create the visitor */ - createVisitor: ( - onComplexityCalculated: (result: ComplexityResult, node: ESTreeNode) => void - ) => Visitor; - - normalizeCategory?: (category: string) => string; -} - -function createComplexityRule(config: ComplexityRuleConfig): Rule { - return defineRule({ - meta: { - type: 'suggestion', - docs: { - description: config.description, - recommended: false, - url: config.url, - }, - schema: [ - { - type: 'object', - properties: { - max: { - type: 'integer', - minimum: config.schemaMinimum, - }, - }, - additionalProperties: false, - }, - ], - }, - - createOnce(context: Context) { - let maxComplexity = config.defaultMax; - - return { - before() { - const options = (context.options[0] || {}) as { max?: number }; - maxComplexity = options.max ?? config.defaultMax; - }, - - ...config.createVisitor((result, node) => { - if (result.total > maxComplexity) { - const funcNode = node as FunctionNode; - const name = getFunctionName(funcNode, funcNode.parent); - const summary = summarizeComplexity(result.points, config.normalizeCategory); - const breakdown = formatBreakdown(result.points); - - context.report({ - node, - message: `Function '${name}' has ${config.metricName} of ${result.total}. Maximum allowed is ${maxComplexity}.${summary}${breakdown}`, - }); - } - }), - } as VisitorWithHooks; - }, - }); -} - -function normalizeCognitiveCategory(category: string): string { - if (category.startsWith('logical operator')) return 'logical operators'; - if (category.startsWith('nested ')) return 'nested functions'; - if (category.startsWith('break to') || category.startsWith('continue to')) return 'labeled jumps'; - return category; -} - -/** - * Enforce a maximum cyclomatic complexity for functions. - * Default threshold: 20 - */ -export const maxCyclomatic = createComplexityRule({ - metricName: 'cyclomatic complexity', - defaultMax: 20, - schemaMinimum: 1, - description: 'Enforce a maximum cyclomatic complexity for functions', - url: 'https://github.com/itaymendel/oxlint-plugin-complexity#complexitymax-cyclomatic', - createVisitor: (onComplexityCalculated) => createCyclomaticVisitor(onComplexityCalculated), -}); - -/** - * Enforce a maximum cognitive complexity for functions. - * Default threshold: 15 - * - * For functions that significantly exceed the threshold (>150% of max), - * provides Smart Extraction Detection suggestions for refactoring. - */ -export const maxCognitive: Rule = defineRule({ - meta: { - type: 'suggestion', - docs: { - description: 'Cognitive Complexity of functions should not be too high', - recommended: false, - url: 'https://github.com/itaymendel/oxlint-plugin-complexity#complexitymax-cognitive', - }, - schema: [ - { - type: 'object', - properties: { - max: { - type: 'integer', - minimum: 0, - }, - extractionMultiplier: { - type: 'number', - minimum: 1, - description: - 'Multiplier for max complexity to trigger extraction suggestions (default: 1.5)', - }, - minExtractionPercentage: { - type: 'integer', - minimum: 1, - maximum: 100, - description: - 'Minimum percentage of total complexity for an extraction candidate (default: 30)', - }, - enableExtraction: { - type: 'boolean', - description: - 'Enable smart extraction suggestions for complex functions (default: false)', - }, - nestingTipThreshold: { - type: 'integer', - minimum: 0, - description: - 'Minimum nesting depth to show extraction tip on top offender (default: 3, set to 0 to disable)', - }, - elseIfChainThreshold: { - type: 'integer', - minimum: 0, - description: - 'Minimum else-if branches to show chain tip (default: 4, set to 0 to disable)', - }, - logicalOperatorThreshold: { - type: 'integer', - minimum: 0, - description: - 'Minimum logical operator sequences to show tip (default: 3, set to 0 to disable)', - }, - }, - additionalProperties: false, - }, - ], - }, - - createOnce(context: Context) { - const DEFAULT_MAX = 15; - let maxComplexity = DEFAULT_MAX; - let enableExtraction = false; - let extractionOptions: ExtractionOptions | undefined; - let breakdownOptions: BreakdownOptions | undefined; - - return { - before() { - const options = (context.options[0] || {}) as { - max?: number; - enableExtraction?: boolean; - extractionMultiplier?: number; - minExtractionPercentage?: number; - nestingTipThreshold?: number; - elseIfChainThreshold?: number; - logicalOperatorThreshold?: number; - }; - maxComplexity = options.max ?? DEFAULT_MAX; - enableExtraction = options.enableExtraction ?? false; - - // Build extraction options if any are provided - if ( - options.extractionMultiplier !== undefined || - options.minExtractionPercentage !== undefined - ) { - extractionOptions = { - minComplexityMultiplier: options.extractionMultiplier, - minComplexityPercentage: options.minExtractionPercentage, - }; - } - - // Build breakdown options if any tip thresholds are specified - if ( - options.nestingTipThreshold !== undefined || - options.elseIfChainThreshold !== undefined || - options.logicalOperatorThreshold !== undefined - ) { - breakdownOptions = { - nestingTipThreshold: options.nestingTipThreshold, - elseIfChainThreshold: options.elseIfChainThreshold, - logicalOperatorThreshold: options.logicalOperatorThreshold, - }; - } - }, - - ...createCognitiveVisitorWithTracking( - context, - (result: ComplexityResultWithVariables, node: ESTreeNode) => { - if (result.total > maxComplexity) { - const summary = summarizeComplexity(result.points, normalizeCognitiveCategory); - const breakdown = formatBreakdown(result.points, breakdownOptions); - - // Add extraction suggestions if enabled and complexity is significant (default: >150% of max) - let extractionOutput = ''; - if ( - enableExtraction && - shouldAnalyzeExtraction(result.total, maxComplexity, extractionOptions) - ) { - const suggestions = analyzeExtractionOpportunities( - node, - result.points, - result.total, - result.variables, - extractionOptions - ); - extractionOutput = formatExtractionSuggestions(suggestions); - } - - context.report({ - node, - message: `Function '${result.functionName}' has Cognitive Complexity of ${result.total}. Maximum allowed is ${maxComplexity}.${summary}${breakdown}${extractionOutput}`, - }); - } - } - ), - } as VisitorWithHooks; - }, -}); diff --git a/src/rules/complexity.ts b/src/rules/complexity.ts new file mode 100644 index 0000000..f797bd0 --- /dev/null +++ b/src/rules/complexity.ts @@ -0,0 +1,151 @@ +import { defineRule } from 'oxlint/plugins'; +import type { + Rule, + Context, + FunctionNode, + MaxCognitiveOptions, + VisitorWithHooks, + ESTreeNode, +} from '../types.js'; +import { getFunctionName, summarizeComplexity, formatBreakdown } from '../utils.js'; +import { + createCombinedComplexityVisitor, + type CombinedComplexityResult, +} from '../combined-visitor.js'; +import { + normalizeCognitiveCategory, + parseExtractionOptions, + getExtractionOutput, + EXTRACTION_SCHEMA_PROPERTIES, +} from './shared.js'; + +const DEFAULT_CYCLOMATIC = 20; +const DEFAULT_COGNITIVE = 15; +const DEFAULT_MIN_LINES = 10; + +interface CombinedComplexityOptions extends Omit { + cyclomatic?: number; + cognitive?: number; + minLines?: number; +} + +/** + * Enforce maximum cyclomatic and cognitive complexity (RECOMMENDED). + * + * This rule combines both complexity checks in a single AST walk, + * providing better performance than using separate rules. + * + * Default thresholds: + * - Cyclomatic: 20 + * - Cognitive: 15 + * - minLines: 10 (skip functions with fewer lines for better performance) + */ +export const complexity: Rule = defineRule({ + meta: { + type: 'suggestion', + docs: { + description: 'Enforce maximum cyclomatic and cognitive complexity', + recommended: true, + url: 'https://github.com/itaymendel/oxlint-plugin-complexity#complexitycomplexity', + }, + schema: [ + { + type: 'object', + properties: { + cyclomatic: { + type: 'integer', + minimum: 1, + description: 'Maximum cyclomatic complexity (default: 20)', + }, + cognitive: { + type: 'integer', + minimum: 0, + description: 'Maximum cognitive complexity (default: 15)', + }, + minLines: { + type: 'integer', + minimum: 0, + description: 'Minimum lines to analyze (default: 10, 0 = analyze all)', + }, + ...EXTRACTION_SCHEMA_PROPERTIES, + }, + additionalProperties: false, + }, + ], + }, + + createOnce(context: Context) { + let maxCyclomatic = DEFAULT_CYCLOMATIC; + let maxCognitive = DEFAULT_COGNITIVE; + let minLines = DEFAULT_MIN_LINES; + let parsed = parseExtractionOptions({}); + + function isBelowMinLines(node: ESTreeNode): boolean { + if (minLines <= 0 || !node.loc) return false; + const functionLines = node.loc.end.line - node.loc.start.line + 1; + return functionLines < minLines; + } + + function reportCyclomatic( + node: ESTreeNode, + functionName: string, + result: CombinedComplexityResult + ): void { + if (result.cyclomatic <= maxCyclomatic) return; + + const summary = summarizeComplexity(result.cyclomaticPoints); + const breakdown = formatBreakdown(result.cyclomaticPoints); + + context.report({ + node, + message: `Function '${functionName}' has cyclomatic complexity of ${result.cyclomatic}. Maximum allowed is ${maxCyclomatic}.${summary}${breakdown}`, + }); + } + + function reportCognitive( + node: ESTreeNode, + functionName: string, + result: CombinedComplexityResult + ): void { + if (result.cognitive <= maxCognitive) return; + + const summary = summarizeComplexity(result.cognitivePoints, normalizeCognitiveCategory); + const breakdown = formatBreakdown(result.cognitivePoints, parsed.breakdownOptions); + const extractionOutput = getExtractionOutput( + parsed, + context, + node, + result.cognitivePoints, + result.cognitive, + maxCognitive + ); + + context.report({ + node, + message: `Function '${functionName}' has Cognitive Complexity of ${result.cognitive}. Maximum allowed is ${maxCognitive}.${summary}${breakdown}${extractionOutput}`, + }); + } + + function handleComplexityResult(result: CombinedComplexityResult, node: ESTreeNode): void { + if (isBelowMinLines(node)) return; + + const funcNode = node as FunctionNode; + const functionName = getFunctionName(funcNode, funcNode.parent); + + reportCyclomatic(node, functionName, result); + reportCognitive(node, functionName, result); + } + + return { + before() { + const options = (context.options[0] ?? {}) as CombinedComplexityOptions; + maxCyclomatic = options.cyclomatic ?? DEFAULT_CYCLOMATIC; + maxCognitive = options.cognitive ?? DEFAULT_COGNITIVE; + minLines = options.minLines ?? DEFAULT_MIN_LINES; + parsed = parseExtractionOptions(options); + }, + + ...createCombinedComplexityVisitor(context, handleComplexityResult), + } as VisitorWithHooks; + }, +}); diff --git a/src/rules/max-cognitive.ts b/src/rules/max-cognitive.ts new file mode 100644 index 0000000..2d3e523 --- /dev/null +++ b/src/rules/max-cognitive.ts @@ -0,0 +1,91 @@ +import { defineRule } from 'oxlint/plugins'; +import type { + Rule, + Context, + FunctionNode, + ComplexityResult, + VisitorWithHooks, + ESTreeNode, + MaxCognitiveOptions, +} from '../types.js'; +import { getFunctionName, summarizeComplexity, formatBreakdown } from '../utils.js'; +import { createCognitiveVisitor } from '../cognitive/visitor.js'; +import { + normalizeCognitiveCategory, + warnDeprecated, + parseExtractionOptions, + getExtractionOutput, + EXTRACTION_SCHEMA_PROPERTIES, +} from './shared.js'; + +const DEFAULT_MAX = 15; + +/** + * Enforce a maximum cognitive complexity for functions. + * Default threshold: 15 + * + * For functions that significantly exceed the threshold (>150% of max), + * provides Smart Extraction Detection suggestions for refactoring. + * + * @deprecated Use 'complexity/complexity' instead for better performance. + */ +export const maxCognitive: Rule = defineRule({ + meta: { + type: 'suggestion', + docs: { + description: 'Cognitive Complexity of functions should not be too high', + recommended: false, + url: 'https://github.com/itaymendel/oxlint-plugin-complexity#complexitymax-cognitive', + }, + schema: [ + { + type: 'object', + properties: { + max: { + type: 'integer', + minimum: 0, + }, + ...EXTRACTION_SCHEMA_PROPERTIES, + }, + additionalProperties: false, + }, + ], + }, + + createOnce(context: Context) { + let maxComplexity = DEFAULT_MAX; + let parsed = parseExtractionOptions({}); + + return { + before() { + warnDeprecated('max-cognitive'); + + const options = (context.options[0] ?? {}) as MaxCognitiveOptions; + maxComplexity = options.max ?? DEFAULT_MAX; + parsed = parseExtractionOptions(options); + }, + + ...createCognitiveVisitor(context, (result: ComplexityResult, node: ESTreeNode) => { + if (result.total <= maxComplexity) return; + + const funcNode = node as FunctionNode; + const functionName = getFunctionName(funcNode, funcNode.parent); + const summary = summarizeComplexity(result.points, normalizeCognitiveCategory); + const breakdown = formatBreakdown(result.points, parsed.breakdownOptions); + const extractionOutput = getExtractionOutput( + parsed, + context, + node, + result.points, + result.total, + maxComplexity + ); + + context.report({ + node, + message: `Function '${functionName}' has Cognitive Complexity of ${result.total}. Maximum allowed is ${maxComplexity}.${summary}${breakdown}${extractionOutput}`, + }); + }), + } as VisitorWithHooks; + }, +}); diff --git a/src/rules/max-cyclomatic.ts b/src/rules/max-cyclomatic.ts new file mode 100644 index 0000000..54111ed --- /dev/null +++ b/src/rules/max-cyclomatic.ts @@ -0,0 +1,19 @@ +import type { Rule } from '../types.js'; +import { createCyclomaticVisitor } from '../cyclomatic.js'; +import { createComplexityRule, warnDeprecated } from './shared.js'; + +/** + * Enforce a maximum cyclomatic complexity for functions. + * Default threshold: 20 + * + * @deprecated Use 'complexity/complexity' instead for better performance. + */ +export const maxCyclomatic: Rule = createComplexityRule({ + metricName: 'cyclomatic complexity', + defaultMax: 20, + schemaMinimum: 1, + description: 'Enforce a maximum cyclomatic complexity for functions', + url: 'https://github.com/itaymendel/oxlint-plugin-complexity#complexitymax-cyclomatic', + before: () => warnDeprecated('max-cyclomatic'), + createVisitor: createCyclomaticVisitor, +}); diff --git a/src/rules/shared.ts b/src/rules/shared.ts new file mode 100644 index 0000000..fa3c7b7 --- /dev/null +++ b/src/rules/shared.ts @@ -0,0 +1,211 @@ +import { defineRule } from 'oxlint/plugins'; +import type { + Rule, + Context, + Visitor, + FunctionNode, + ComplexityPoint, + ComplexityResult, + VisitorWithHooks, + ESTreeNode, + MaxCognitiveOptions, +} from '../types.js'; +import { + getFunctionName, + summarizeComplexity, + formatBreakdown, + type BreakdownOptions, +} from '../utils.js'; +import { + analyzeExtractionOpportunities, + shouldAnalyzeExtraction, + formatExtractionSuggestions, + type ExtractionOptions, +} from '../extraction/index.js'; +import { getVariablesForFunction } from '../extraction/variable-tracker.js'; + +/** + * Configuration for creating a simple complexity rule + * (single metric, no extraction analysis). + */ +export interface ComplexityRuleConfig { + metricName: string; + defaultMax: number; + schemaMinimum: number; + description: string; + url: string; + + /** Factory to create the visitor */ + createVisitor: ( + onComplexityCalculated: (result: ComplexityResult, node: ESTreeNode) => void + ) => Visitor; + + /** Called once before the first file is processed */ + before?: () => void; + + normalizeCategory?: (category: string) => string; +} + +export function createComplexityRule(config: ComplexityRuleConfig): Rule { + return defineRule({ + meta: { + type: 'suggestion', + docs: { + description: config.description, + recommended: false, + url: config.url, + }, + schema: [ + { + type: 'object', + properties: { + max: { + type: 'integer', + minimum: config.schemaMinimum, + }, + }, + additionalProperties: false, + }, + ], + }, + + createOnce(context: Context) { + let maxComplexity = config.defaultMax; + + return { + before() { + const options = (context.options[0] ?? {}) as { max?: number }; + maxComplexity = options.max ?? config.defaultMax; + config.before?.(); + }, + + ...config.createVisitor((result, node) => { + if (result.total > maxComplexity) { + const funcNode = node as FunctionNode; + const name = getFunctionName(funcNode, funcNode.parent); + const summary = summarizeComplexity(result.points, config.normalizeCategory); + const breakdown = formatBreakdown(result.points); + + context.report({ + node, + message: `Function '${name}' has ${config.metricName} of ${result.total}. Maximum allowed is ${maxComplexity}.${summary}${breakdown}`, + }); + } + }), + } as VisitorWithHooks; + }, + }); +} + +export function normalizeCognitiveCategory(category: string): string { + if (category.startsWith('logical operator')) return 'logical operators'; + if (category.startsWith('nested ')) return 'nested functions'; + if (category.startsWith('break to') || category.startsWith('continue to')) return 'labeled jumps'; + return category; +} + +const MIGRATION_URL = 'https://github.com/itaymendel/oxlint-plugin-complexity#migration-to-v1'; +const warnedDeprecations = new Set(); + +export function warnDeprecated(ruleName: string): void { + if (warnedDeprecations.has(ruleName)) return; + warnedDeprecations.add(ruleName); + + // eslint-disable-next-line no-console -- Intentional deprecation warning + console.warn(` +DEPRECATION WARNING: complexity/${ruleName} + + Use "complexity/complexity" instead for better performance: + + "complexity/complexity": ["warn", { + "cyclomatic": 20, + "cognitive": 15 + }] + + See: ${MIGRATION_URL} +`); +} + +/** JSON Schema properties shared by rules that support extraction analysis and breakdown tips. */ +export const EXTRACTION_SCHEMA_PROPERTIES = { + extractionMultiplier: { + type: 'number', + minimum: 1, + description: 'Multiplier for max complexity to trigger extraction suggestions (default: 1.5)', + }, + minExtractionPercentage: { + type: 'integer', + minimum: 1, + maximum: 100, + description: 'Minimum percentage of total complexity for an extraction candidate (default: 30)', + }, + enableExtraction: { + type: 'boolean', + description: 'Enable smart extraction suggestions for complex functions (default: false)', + }, + nestingTipThreshold: { + type: 'integer', + minimum: 0, + description: + 'Minimum nesting depth to show extraction tip on top offender (default: 3, set to 0 to disable)', + }, + elseIfChainThreshold: { + type: 'integer', + minimum: 0, + description: 'Minimum else-if branches to show chain tip (default: 4, set to 0 to disable)', + }, + logicalOperatorThreshold: { + type: 'integer', + minimum: 0, + description: 'Minimum logical operator sequences to show tip (default: 3, set to 0 to disable)', + }, +} as const; + +type ExtractionSchemaOptions = Omit; + +export interface ParsedExtractionOptions { + enableExtraction: boolean; + extractionOptions: ExtractionOptions; + breakdownOptions: BreakdownOptions; +} + +export function parseExtractionOptions(options: ExtractionSchemaOptions): ParsedExtractionOptions { + return { + enableExtraction: options.enableExtraction ?? false, + extractionOptions: { + minComplexityMultiplier: options.extractionMultiplier, + minComplexityPercentage: options.minExtractionPercentage, + }, + breakdownOptions: { + nestingTipThreshold: options.nestingTipThreshold, + elseIfChainThreshold: options.elseIfChainThreshold, + logicalOperatorThreshold: options.logicalOperatorThreshold, + }, + }; +} + +/** + * Analyze extraction opportunities and return formatted output. + * Returns an empty string if extraction is disabled or complexity is below threshold. + */ +export function getExtractionOutput( + parsed: ParsedExtractionOptions, + context: Context, + node: ESTreeNode, + points: ComplexityPoint[], + total: number, + maxComplexity: number +): string { + if (!parsed.enableExtraction) return ''; + if (!shouldAnalyzeExtraction(total, maxComplexity, parsed.extractionOptions)) return ''; + + const variables = getVariablesForFunction(context, node); + const suggestions = analyzeExtractionOpportunities( + node, + points, + total, + variables, + parsed.extractionOptions + ); + return formatExtractionSuggestions(suggestions); +} diff --git a/src/types.ts b/src/types.ts index 7a970c0..b7dd6f0 100644 --- a/src/types.ts +++ b/src/types.ts @@ -14,14 +14,8 @@ export type CallExpressionNode = ESTree.CallExpression; export type AssignmentExpressionNode = ESTree.AssignmentExpression; export type IfStatementNode = ESTree.IfStatement; -export type ForStatementNode = ESTree.ForStatement; -export type ForInStatementNode = ESTree.ForInStatement; -export type ForOfStatementNode = ESTree.ForOfStatement; -export type WhileStatementNode = ESTree.WhileStatement; -export type DoWhileStatementNode = ESTree.DoWhileStatement; export type SwitchStatementNode = ESTree.SwitchStatement; export type SwitchCaseNode = ESTree.SwitchCase; -export type BlockStatementNode = ESTree.BlockStatement; export type LabeledJumpStatementNode = ESTree.BreakStatement | ESTree.ContinueStatement; @@ -53,4 +47,10 @@ export interface MaxCyclomaticOptions { export interface MaxCognitiveOptions { max?: number; + enableExtraction?: boolean; + extractionMultiplier?: number; + minExtractionPercentage?: number; + nestingTipThreshold?: number; + elseIfChainThreshold?: number; + logicalOperatorThreshold?: number; } diff --git a/src/utils.ts b/src/utils.ts index 495d965..e9e71b0 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,5 +1,10 @@ import type { ESTreeNode, FunctionNode, ComplexityPoint } from './types.js'; +/** Type-safe `includes` for readonly const arrays. */ +export function includes(array: readonly T[], value: string): value is T { + return (array as readonly string[]).includes(value); +} + /** Cyclomatic complexity formula starts at 1 (1 + decision_points). */ export const BASE_FUNCTION_COMPLEXITY = 1; @@ -7,11 +12,23 @@ export const DEFAULT_COMPLEXITY_INCREMENT = 1; export const LOGICAL_OPERATORS = ['&&', '||', '??'] as const; +/** Logical assignment operators (short-circuit assignment), used by cyclomatic complexity. */ +export const LOGICAL_ASSIGNMENT_OPERATORS = ['||=', '&&=', '??='] as const; + const DEFAULT_LOCATION = { start: { line: 0, column: 0 }, end: { line: 0, column: 0 }, } as const; +/** + * Extract the construct name from a complexity point message. + * Messages follow the format: "+N: construct" or "+N (incl. M for nesting): construct" + */ +export function extractConstructFromMessage(message: string): string { + const match = message.match(/:\s*(.+)$/); + return match ? match[1].trim() : 'unknown'; +} + export function createComplexityPoint( node: ESTreeNode, message: string, @@ -31,73 +48,54 @@ export function createComplexityPoint( }; } -function getNameFromId(node: FunctionNode): string | null { - if ('id' in node && node.id && 'name' in node.id) { - return String(node.id.name); - } - return null; -} - -function getNameFromKey(node: FunctionNode): string | null { - if ('key' in node && node.key && 'name' in node.key) { - return String(node.key.name); - } - return null; -} - -function getNameFromVariableDeclarator(parent?: ESTreeNode): string | null { - if (parent?.type !== 'VariableDeclarator') return null; - if ('id' in parent && parent.id && 'name' in parent.id) { - return String(parent.id.name); +/** + * Safely extract a `.name` string from a nested property of a node. + * Returns null if the property path doesn't exist or has no `.name`. + */ +function getNestedName(node: ESTreeNode | FunctionNode, prop: string): string | null { + if (!(prop in node)) return null; + const child = (node as unknown as Record)[prop]; + if (child && typeof child === 'object' && 'name' in child) { + return String((child as { name: unknown }).name); } return null; } -function getNameFromProperty(parent?: ESTreeNode): string | null { - if (parent?.type !== 'Property') return null; - if ('key' in parent && parent.key && 'name' in parent.key) { - return String(parent.key.name); - } - return null; -} - -function getNameFromAssignment(parent?: ESTreeNode): string | null { - if (parent?.type !== 'AssignmentExpression') return null; - if ('left' in parent && parent.left && 'name' in parent.left) { - return String(parent.left.name); - } - return null; -} - -function getNameFromMethodDefinition(parent?: ESTreeNode): string | null { - if (parent?.type !== 'MethodDefinition') return null; - if ('key' in parent && parent.key && 'name' in parent.key) { - return String(parent.key.name); - } - if ('kind' in parent && parent.kind === 'constructor') { - return 'constructor'; +/** + * Try to extract a name from the parent node based on its type. + * Each entry maps a parent type to the property that holds the name identifier. + */ +const PARENT_NAME_PROPERTIES: ReadonlyArray<{ type: string; prop: string }> = [ + { type: 'VariableDeclarator', prop: 'id' }, + { type: 'Property', prop: 'key' }, + { type: 'AssignmentExpression', prop: 'left' }, + { type: 'MethodDefinition', prop: 'key' }, + { type: 'PropertyDefinition', prop: 'key' }, +]; + +function getNameFromParent(parent?: ESTreeNode): string | null { + if (!parent) return null; + + for (const { type, prop } of PARENT_NAME_PROPERTIES) { + if (parent.type !== type) continue; + const name = getNestedName(parent, prop); + if (name) return name; + + // Special case: class constructor has no key name + if (type === 'MethodDefinition' && 'kind' in parent && parent.kind === 'constructor') { + return 'constructor'; + } } - return null; -} -function getNameFromPropertyDefinition(parent?: ESTreeNode): string | null { - if (parent?.type !== 'PropertyDefinition') return null; - if ('key' in parent && parent.key && 'name' in parent.key) { - return String(parent.key.name); - } return null; } export function getFunctionName(node: FunctionNode, parent?: ESTreeNode): string { - const fromNode = getNameFromId(node) ?? getNameFromKey(node); + const fromNode = + getNestedName(node as ESTreeNode, 'id') ?? getNestedName(node as ESTreeNode, 'key'); if (fromNode) return fromNode; - const fromParent = - getNameFromVariableDeclarator(parent) ?? - getNameFromProperty(parent) ?? - getNameFromAssignment(parent) ?? - getNameFromMethodDefinition(parent) ?? - getNameFromPropertyDefinition(parent); + const fromParent = getNameFromParent(parent); if (fromParent) return fromParent; return node.type === 'ArrowFunctionExpression' ? '' : ''; @@ -110,14 +108,12 @@ export function summarizeComplexity( const categories: Record = {}; for (const point of points) { - const match = point.message.match(/:\s*(.+)$/); - if (match) { - let category = match[1].trim(); - if (normalizeCategory) { - category = normalizeCategory(category); - } - categories[category] = (categories[category] || 0) + point.complexity; + let category = extractConstructFromMessage(point.message); + if (category === 'unknown') continue; + if (normalizeCategory) { + category = normalizeCategory(category); } + categories[category] = (categories[category] || 0) + point.complexity; } const sorted = Object.entries(categories) @@ -155,7 +151,7 @@ function countPatterns(points: ComplexityPoint[]): PatternCounts { let logicalOperatorCount = 0; for (const point of points) { - const construct = point.message.match(/:\s*(.+)$/)?.[1]?.trim() ?? ''; + const construct = extractConstructFromMessage(point.message); if (construct === 'else if') { elseIfCount++; @@ -197,8 +193,7 @@ export function formatBreakdown(points: ComplexityPoint[], options?: BreakdownOp const maxComplexity = Math.max(...sorted.map((p) => p.complexity)); const lines = sorted.map((point) => { - const constructMatch = point.message.match(/:\s*(.+)$/); - const construct = constructMatch ? constructMatch[1].trim() : 'unknown'; + const construct = extractConstructFromMessage(point.message); const nestingMatch = point.message.match(/\(incl\.\s*(\d+)\s*for nesting\)/); const nestingLevel = nestingMatch ? parseInt(nestingMatch[1], 10) : 0; diff --git a/src/visitor.ts b/src/visitor.ts index b110483..c2e8afa 100644 --- a/src/visitor.ts +++ b/src/visitor.ts @@ -55,25 +55,17 @@ export function createComplexityVisitor( const scope = config.createScope(node, name); scopeStack.push(scope); - - if (config.onEnterFunction) { - config.onEnterFunction(parentScope, node, scope); - } + config.onEnterFunction?.(parentScope, node, scope); } function exitFunction(node: ESTreeNode): void { - const scope = getCurrentScope(); + const scope = scopeStack.pop(); + if (!scope) return; - if (scope && config.onExitFunction) { - config.onExitFunction(scope, node); - } - - const poppedScope = scopeStack.pop(); + config.onExitFunction?.(scope, node); - if (poppedScope) { - const total = poppedScope.points.reduce((sum, point) => sum + point.complexity, 0); - config.onComplexityCalculated({ total, points: poppedScope.points }, node); - } + const total = scope.points.reduce((sum, point) => sum + point.complexity, 0); + config.onComplexityCalculated({ total, points: scope.points }, node); } const baseVisitor: Partial = { diff --git a/tests/complexity.test.ts b/tests/complexity.test.ts index 9aca08b..4aa08e3 100644 --- a/tests/complexity.test.ts +++ b/tests/complexity.test.ts @@ -5,6 +5,7 @@ import { loadFixturesFromDir, getParseFilename } from './utils/fixture-loader'; import { calculateCyclomaticComplexity, calculateCognitiveComplexity, + calculateCombinedComplexity, type ComplexityFunctionResult, } from './utils/test-helpers'; @@ -53,3 +54,47 @@ function createComplexityTestSuite( // Run both test suites createComplexityTestSuite('cyclomatic', calculateCyclomaticComplexity); createComplexityTestSuite('cognitive', calculateCognitiveComplexity); + +/** + * Test that the combined visitor produces identical results to running + * cyclomatic and cognitive visitors separately. + */ +describe('Combined Visitor (complexity/complexity rule)', () => { + const fixtures = loadFixturesFromDir(fixturesDir); + + describe.each(fixtures)('$relativePath', (fixture) => { + const parseFilename = getParseFilename(fixture); + + it('should match standalone cyclomatic results', () => { + const standalone = calculateCyclomaticComplexity(fixture.code, parseFilename); + const combined = calculateCombinedComplexity(fixture.code, parseFilename); + + // Check all functions exist in both results + expect(Array.from(combined.cyclomatic.keys()).sort()).toEqual( + Array.from(standalone.keys()).sort() + ); + + // Check all complexity values match + for (const [name, standaloneFn] of standalone) { + const combinedFn = combined.cyclomatic.get(name); + expect(combinedFn?.total).toBe(standaloneFn.total); + } + }); + + it('should match standalone cognitive results', () => { + const standalone = calculateCognitiveComplexity(fixture.code, parseFilename); + const combined = calculateCombinedComplexity(fixture.code, parseFilename); + + // Check all functions exist in both results + expect(Array.from(combined.cognitive.keys()).sort()).toEqual( + Array.from(standalone.keys()).sort() + ); + + // Check all complexity values match + for (const [name, standaloneFn] of standalone) { + const combinedFn = combined.cognitive.get(name); + expect(combinedFn?.total).toBe(standaloneFn.total); + } + }); + }); +}); diff --git a/tests/extraction-flow.test.ts b/tests/extraction-flow.test.ts new file mode 100644 index 0000000..5166b28 --- /dev/null +++ b/tests/extraction-flow.test.ts @@ -0,0 +1,265 @@ +import { describe, it, expect, beforeAll } from 'vitest'; +import { dirname, join } from 'path'; +import { fileURLToPath } from 'url'; +import type { ExtractionCandidate } from '#src/extraction/types.js'; +import { + analyzeExtractionOpportunities, + formatExtractionSuggestions, +} from '#src/extraction/index.js'; +import { analyzeVariableFlow } from '#src/extraction/flow-analyzer.js'; +import { createExtractionSuggestion } from '#src/extraction/suggestion-generator.js'; +import { loadFixture } from './utils/fixture-loader.js'; +import { + type ExtendedResult, + calculateCognitiveWithTracking, + analyzeFlowFromResult, + buildCandidateForRange, +} from './utils/extraction-helpers.js'; + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const fixturesDir = join(__dirname, 'fixtures'); + +describe('Smart Extraction Detection', () => { + describe('Early Return Detection', () => { + let earlyReturnResults: Map; + + beforeAll(() => { + const fixture = loadFixture(join(fixturesDir, 'js/early-returns.js'), fixturesDir); + earlyReturnResults = calculateCognitiveWithTracking(fixture.code, 'test.js'); + }); + + it('detects early returns in guard clause functions', () => { + const result = earlyReturnResults.get('guardClauses')!; + expect(result).toBeDefined(); + + const flow = analyzeFlowFromResult(result); + expect(flow.hasEarlyReturn).toBe(true); + }); + + it('detects early returns in functions with multiple exit points', () => { + const result = earlyReturnResults.get('multipleExitPoints')!; + expect(result).toBeDefined(); + + const flow = analyzeFlowFromResult(result); + expect(flow.hasEarlyReturn).toBe(true); + }); + + it('does not flag functions without return statements as having early returns', () => { + const code = ` + function noReturns(items) { + let count = 0; + for (const item of items) { + if (item.active) { + for (const child of item.children || []) { + if (child.valid) { + count++; + } + } + } + } + console.log(count); + } + `; + + const results = calculateCognitiveWithTracking(code, 'test.js'); + const result = results.get('noReturns')!; + const flow = analyzeFlowFromResult(result); + + expect(flow.hasEarlyReturn).toBe(false); + }); + + it('does not flag a single return on the last line as early return', () => { + const code = ` + function singleReturn(a, b) { + let result = 0; + if (a > 0) { + result = a + b; + } + return result; + } + `; + + const results = calculateCognitiveWithTracking(code, 'test.js'); + const result = results.get('singleReturn')!; + const flow = analyzeFlowFromResult(result); + + expect(flow.hasEarlyReturn).toBe(false); + }); + + it('shows early-return issues in formatted output', () => { + const code = ` + function processData(items, config) { + if (!items) return null; + let total = 0; + for (const item of items) { + if (item.valid) { + total += item.value; + } + } + + const factor = config.factor || 1; + const adjusted = total * factor; + const prefix = config.prefix || ''; + + let output = ''; + for (const entry of config.entries || []) { + if (entry.active) { + if (entry.value > adjusted) { + output += prefix + entry.label; + } + } + } + return output; + } + `; + + const results = calculateCognitiveWithTracking(code, 'test.js'); + const result = results.get('processData')!; + + const suggestions = analyzeExtractionOpportunities( + result.node, + result.points, + result.total, + result.variables, + 'processData' + ); + + expect(suggestions.length).toBe(2); + + const withEarlyReturn = suggestions.find((s) => + s.issues.some((i) => i.type === 'early-return') + ); + expect(withEarlyReturn).toBeDefined(); + + const formatted = formatExtractionSuggestions(suggestions); + expect(formatted).toContain('early return'); + }); + }); + + describe('This Reference Detection', () => { + it('detects this in function body', () => { + const code = ` + function outer(items) { + let count = 0; + for (const item of items) { + if (item.active) { + if (item.value > 0) { + this.total += item.value; + count++; + } + } + } + return count; + } + `; + + const results = calculateCognitiveWithTracking(code, 'test.js'); + const result = results.get('outer')!; + const candidate = buildCandidateForRange(result, 2, -1); + const flow = analyzeVariableFlow(candidate, result.variables, result.node); + + expect(flow.hasThisReference).toBe(true); + }); + + it('does not flag this inside nested function expression', () => { + const code = ` + function outer(items) { + let count = 0; + for (const item of items) { + if (item.active) { + if (item.value > 0) { + const fn = function() { return this.x; }; + count++; + } + } + } + return count; + } + `; + + const results = calculateCognitiveWithTracking(code, 'test.js'); + const result = results.get('outer')!; + const candidate = buildCandidateForRange(result, 2, -1); + const flow = analyzeVariableFlow(candidate, result.variables, result.node); + + expect(flow.hasThisReference).toBe(false); + }); + + it('does not flag this inside arrow function', () => { + const code = ` + function outer(items) { + let count = 0; + for (const item of items) { + if (item.active) { + if (item.value > 0) { + const fn = () => this.x; + count++; + } + } + } + return count; + } + `; + + const results = calculateCognitiveWithTracking(code, 'test.js'); + const result = results.get('outer')!; + const candidate = buildCandidateForRange(result, 2, -1); + const flow = analyzeVariableFlow(candidate, result.variables, result.node); + + expect(flow.hasThisReference).toBe(false); + }); + + it('reports no this references when none present', () => { + const code = ` + function outer(items) { + let count = 0; + for (const item of items) { + if (item.active) { + if (item.value > 0) { + count++; + } + } + } + return count; + } + `; + + const results = calculateCognitiveWithTracking(code, 'test.js'); + const result = results.get('outer')!; + const candidate = buildCandidateForRange(result, 2, -1); + const flow = analyzeVariableFlow(candidate, result.variables, result.node); + + expect(flow.hasThisReference).toBe(false); + }); + + it('emits this-reference issue with correct suggestion', () => { + const code = ` + function outer(items) { + let count = 0; + for (const item of items) { + if (item.active) { + if (item.value > 0) { + this.total += item.value; + count++; + } + } + } + return count; + } + `; + + const results = calculateCognitiveWithTracking(code, 'test.js'); + const result = results.get('outer')!; + const candidate = buildCandidateForRange(result, 2, -1); + const flow = analyzeVariableFlow(candidate, result.variables, result.node); + const suggestion = createExtractionSuggestion(candidate, flow); + + const thisIssue = suggestion.issues.find((i) => i.type === 'this-reference'); + expect(thisIssue).toBeDefined(); + expect(thisIssue!.description).toContain('this'); + + const thisSuggestion = suggestion.suggestions.find((s) => s.includes('.call(this)')); + expect(thisSuggestion).toBeDefined(); + }); + }); +}); diff --git a/tests/extraction-mutations.test.ts b/tests/extraction-mutations.test.ts new file mode 100644 index 0000000..66bfee6 --- /dev/null +++ b/tests/extraction-mutations.test.ts @@ -0,0 +1,448 @@ +import { describe, it, expect } from 'vitest'; +import type { ExtractionCandidate } from '#src/extraction/types.js'; +import { analyzeVariableFlow } from '#src/extraction/flow-analyzer.js'; +import { extractConstructFromMessage } from '#src/utils.js'; +import { calculateCognitiveWithTracking } from './utils/extraction-helpers.js'; + +describe('Smart Extraction Detection', () => { + describe('Property Mutation Detection', () => { + it('detects simple property assignment as mutation', () => { + // Outer function declares `data`, inner block is the extraction candidate + const code = ` + function outer(data) { + let total = 0; + for (const item of data.items) { + if (item.active) { + if (item.value > 0) { + total += item.value; + } + } + } + data.processed = true; + return total; + } + `; + + const results = calculateCognitiveWithTracking(code, 'test.js'); + const result = results.get('outer')!; + // Build a candidate for the loop+mutation block (lines inside the function body) + const funcStart = result.node.loc!.start.line; + const funcEnd = result.node.loc!.end.line; + const candidate: ExtractionCandidate = { + startLine: funcStart + 2, // skip param declaration line + endLine: funcEnd - 1, // before closing brace + complexity: result.total, + complexityPercentage: 50, + points: result.points, + constructs: result.points.map((p) => extractConstructFromMessage(p.message)), + }; + const flow = analyzeVariableFlow(candidate, result.variables, result.node); + + const propMutation = flow.mutations.find( + (m) => m.variable.name === 'data' && m.mutationType === 'assignment' + ); + expect(propMutation).toBeDefined(); + }); + + it('detects nested member chain mutation (state.nested.deep.value)', () => { + const code = ` + function outer(state) { + let count = 0; + for (const key of Object.keys(state)) { + if (state[key]) { + if (state[key].active) { + count++; + } + } + } + state.nested.deep.value = count; + return count; + } + `; + + const results = calculateCognitiveWithTracking(code, 'test.js'); + const result = results.get('outer')!; + const funcStart = result.node.loc!.start.line; + const funcEnd = result.node.loc!.end.line; + const candidate: ExtractionCandidate = { + startLine: funcStart + 2, + endLine: funcEnd - 1, + complexity: result.total, + complexityPercentage: 50, + points: result.points, + constructs: result.points.map((p) => extractConstructFromMessage(p.message)), + }; + const flow = analyzeVariableFlow(candidate, result.variables, result.node); + + const propMutation = flow.mutations.find( + (m) => m.variable.name === 'state' && m.mutationType === 'assignment' + ); + expect(propMutation).toBeDefined(); + }); + + it('detects computed property assignment as mutation', () => { + const code = ` + function outer(target, items) { + let count = 0; + for (const item of items) { + if (item.active) { + if (item.key) { + target[item.key] = item.value; + count++; + } + } + } + return count; + } + `; + + const results = calculateCognitiveWithTracking(code, 'test.js'); + const result = results.get('outer')!; + const funcStart = result.node.loc!.start.line; + const funcEnd = result.node.loc!.end.line; + const candidate: ExtractionCandidate = { + startLine: funcStart + 2, + endLine: funcEnd - 1, + complexity: result.total, + complexityPercentage: 50, + points: result.points, + constructs: result.points.map((p) => extractConstructFromMessage(p.message)), + }; + const flow = analyzeVariableFlow(candidate, result.variables, result.node); + + const propMutation = flow.mutations.find( + (m) => m.variable.name === 'target' && m.mutationType === 'assignment' + ); + expect(propMutation).toBeDefined(); + }); + + it('detects UpdateExpression on member expression (stats.count++)', () => { + const code = ` + function outer(stats, items) { + let sum = 0; + for (const item of items) { + if (item.valid) { + if (item.value > 0) { + stats.count++; + sum += item.value; + } + } + } + return sum; + } + `; + + const results = calculateCognitiveWithTracking(code, 'test.js'); + const result = results.get('outer')!; + const funcStart = result.node.loc!.start.line; + const funcEnd = result.node.loc!.end.line; + const candidate: ExtractionCandidate = { + startLine: funcStart + 2, + endLine: funcEnd - 1, + complexity: result.total, + complexityPercentage: 50, + points: result.points, + constructs: result.points.map((p) => extractConstructFromMessage(p.message)), + }; + const flow = analyzeVariableFlow(candidate, result.variables, result.node); + + const propMutation = flow.mutations.find( + (m) => m.variable.name === 'stats' && m.mutationType === 'increment' + ); + expect(propMutation).toBeDefined(); + }); + + it('does not flag property mutations on locally declared variables', () => { + const code = ` + function outer(items) { + const result = {}; + let count = 0; + for (const item of items) { + if (item.active) { + if (item.key) { + result[item.key] = item.value; + count++; + } + } + } + return result; + } + `; + + const results = calculateCognitiveWithTracking(code, 'test.js'); + const result = results.get('outer')!; + const funcStart = result.node.loc!.start.line; + const funcEnd = result.node.loc!.end.line; + // Candidate includes the `const result = {}` declaration (line after function header) + const candidate: ExtractionCandidate = { + startLine: funcStart + 1, + endLine: funcEnd - 1, + complexity: result.total, + complexityPercentage: 50, + points: result.points, + constructs: result.points.map((p) => extractConstructFromMessage(p.message)), + }; + const flow = analyzeVariableFlow(candidate, result.variables, result.node); + + const propMutation = flow.mutations.find( + (m) => m.variable.name === 'result' && m.mutationType === 'assignment' + ); + expect(propMutation).toBeUndefined(); + }); + }); + + describe('Method-Call Mutation Detection', () => { + it('detects arr.push(item) on external variable', () => { + const code = ` + function outer(arr) { + let count = 0; + for (const i of [1,2,3]) { + if (i > 0) { + if (i < 10) { + arr.push(i); + count++; + } + } + } + return count; + } + `; + + const results = calculateCognitiveWithTracking(code, 'test.js'); + const result = results.get('outer')!; + const funcStart = result.node.loc!.start.line; + const funcEnd = result.node.loc!.end.line; + const candidate: ExtractionCandidate = { + startLine: funcStart + 2, + endLine: funcEnd - 1, + complexity: result.total, + complexityPercentage: 50, + points: result.points, + constructs: result.points.map((p) => extractConstructFromMessage(p.message)), + }; + const flow = analyzeVariableFlow(candidate, result.variables, result.node); + + const methodMutation = flow.mutations.find( + (m) => m.variable.name === 'arr' && m.mutationType === 'method-call' + ); + expect(methodMutation).toBeDefined(); + }); + + it('detects arr.sort() on external variable', () => { + const code = ` + function outer(arr) { + let total = 0; + for (const item of arr) { + if (item > 0) { + if (item < 100) { + total += item; + } + } + } + arr.sort(); + return total; + } + `; + + const results = calculateCognitiveWithTracking(code, 'test.js'); + const result = results.get('outer')!; + const funcStart = result.node.loc!.start.line; + const funcEnd = result.node.loc!.end.line; + const candidate: ExtractionCandidate = { + startLine: funcStart + 2, + endLine: funcEnd - 1, + complexity: result.total, + complexityPercentage: 50, + points: result.points, + constructs: result.points.map((p) => extractConstructFromMessage(p.message)), + }; + const flow = analyzeVariableFlow(candidate, result.variables, result.node); + + const methodMutation = flow.mutations.find( + (m) => m.variable.name === 'arr' && m.mutationType === 'method-call' + ); + expect(methodMutation).toBeDefined(); + }); + + it('detects map.set(k, v) on external variable', () => { + const code = ` + function outer(map, items) { + let count = 0; + for (const item of items) { + if (item.key) { + if (item.value) { + map.set(item.key, item.value); + count++; + } + } + } + return count; + } + `; + + const results = calculateCognitiveWithTracking(code, 'test.js'); + const result = results.get('outer')!; + const funcStart = result.node.loc!.start.line; + const funcEnd = result.node.loc!.end.line; + const candidate: ExtractionCandidate = { + startLine: funcStart + 2, + endLine: funcEnd - 1, + complexity: result.total, + complexityPercentage: 50, + points: result.points, + constructs: result.points.map((p) => extractConstructFromMessage(p.message)), + }; + const flow = analyzeVariableFlow(candidate, result.variables, result.node); + + const methodMutation = flow.mutations.find( + (m) => m.variable.name === 'map' && m.mutationType === 'method-call' + ); + expect(methodMutation).toBeDefined(); + }); + + it('detects set.delete(x) on external variable', () => { + const code = ` + function outer(mySet, items) { + let removed = 0; + for (const item of items) { + if (item.expired) { + if (mySet.has(item.id)) { + mySet.delete(item.id); + removed++; + } + } + } + return removed; + } + `; + + const results = calculateCognitiveWithTracking(code, 'test.js'); + const result = results.get('outer')!; + const funcStart = result.node.loc!.start.line; + const funcEnd = result.node.loc!.end.line; + const candidate: ExtractionCandidate = { + startLine: funcStart + 2, + endLine: funcEnd - 1, + complexity: result.total, + complexityPercentage: 50, + points: result.points, + constructs: result.points.map((p) => extractConstructFromMessage(p.message)), + }; + const flow = analyzeVariableFlow(candidate, result.variables, result.node); + + const methodMutation = flow.mutations.find( + (m) => m.variable.name === 'mySet' && m.mutationType === 'method-call' + ); + expect(methodMutation).toBeDefined(); + }); + + it('detects chained a.b.push(x) where a is external', () => { + const code = ` + function outer(state) { + let count = 0; + for (const key of Object.keys(state)) { + if (state[key]) { + if (state[key].active) { + count++; + } + } + } + state.items.push(count); + return count; + } + `; + + const results = calculateCognitiveWithTracking(code, 'test.js'); + const result = results.get('outer')!; + const funcStart = result.node.loc!.start.line; + const funcEnd = result.node.loc!.end.line; + const candidate: ExtractionCandidate = { + startLine: funcStart + 2, + endLine: funcEnd - 1, + complexity: result.total, + complexityPercentage: 50, + points: result.points, + constructs: result.points.map((p) => extractConstructFromMessage(p.message)), + }; + const flow = analyzeVariableFlow(candidate, result.variables, result.node); + + const methodMutation = flow.mutations.find( + (m) => m.variable.name === 'state' && m.mutationType === 'method-call' + ); + expect(methodMutation).toBeDefined(); + }); + + it('does not flag method calls on locally declared variables', () => { + const code = ` + function outer(items) { + const localArr = []; + let count = 0; + for (const item of items) { + if (item.active) { + if (item.value > 0) { + localArr.push(item.value); + count++; + } + } + } + return localArr; + } + `; + + const results = calculateCognitiveWithTracking(code, 'test.js'); + const result = results.get('outer')!; + const funcStart = result.node.loc!.start.line; + const funcEnd = result.node.loc!.end.line; + const candidate: ExtractionCandidate = { + startLine: funcStart + 1, + endLine: funcEnd - 1, + complexity: result.total, + complexityPercentage: 50, + points: result.points, + constructs: result.points.map((p) => extractConstructFromMessage(p.message)), + }; + const flow = analyzeVariableFlow(candidate, result.variables, result.node); + + const methodMutation = flow.mutations.find( + (m) => m.variable.name === 'localArr' && m.mutationType === 'method-call' + ); + expect(methodMutation).toBeUndefined(); + }); + + it('does not flag non-mutating methods like arr.map()', () => { + const code = ` + function outer(arr) { + let total = 0; + for (const item of arr) { + if (item > 0) { + if (item < 100) { + total += item; + } + } + } + const mapped = arr.map(x => x * 2); + return mapped; + } + `; + + const results = calculateCognitiveWithTracking(code, 'test.js'); + const result = results.get('outer')!; + const funcStart = result.node.loc!.start.line; + const funcEnd = result.node.loc!.end.line; + const candidate: ExtractionCandidate = { + startLine: funcStart + 2, + endLine: funcEnd - 1, + complexity: result.total, + complexityPercentage: 50, + points: result.points, + constructs: result.points.map((p) => extractConstructFromMessage(p.message)), + }; + const flow = analyzeVariableFlow(candidate, result.variables, result.node); + + const methodMutation = flow.mutations.find( + (m) => m.variable.name === 'arr' && m.mutationType === 'method-call' + ); + expect(methodMutation).toBeUndefined(); + }); + }); +}); diff --git a/tests/extraction.test.ts b/tests/extraction.test.ts index b14002d..64f1c29 100644 --- a/tests/extraction.test.ts +++ b/tests/extraction.test.ts @@ -1,87 +1,17 @@ import { describe, it, expect, beforeAll } from 'vitest'; import { dirname, join } from 'path'; import { fileURLToPath } from 'url'; -import type { ESTreeNode, ComplexityResult } from '#src/types.js'; -import type { - VariableInfo, - ExtractionCandidate, - VariableFlowAnalysis, -} from '#src/extraction/types.js'; -import { - createCognitiveVisitorWithTracking, - type ComplexityResultWithVariables, -} from '#src/cognitive/visitor.js'; import { analyzeExtractionOpportunities, shouldAnalyzeExtraction, formatExtractionSuggestions, - PLACEHOLDER_FUNCTION_NAME, } from '#src/extraction/index.js'; -import { analyzeVariableFlow } from '#src/extraction/flow-analyzer.js'; import { loadFixture } from './utils/fixture-loader.js'; -import { createMockContext, walkWithVisitor, parseAndPrepareAst } from './utils/test-helpers.js'; +import { type ExtendedResult, calculateCognitiveWithTracking } from './utils/extraction-helpers.js'; const __dirname = dirname(fileURLToPath(import.meta.url)); const fixturesDir = join(__dirname, 'fixtures'); -interface ExtendedResult extends ComplexityResult { - functionName: string; - variables: Map; - node: ESTreeNode; -} - -/** - * Calculate cognitive complexity with variable tracking. - * Uses eslint-scope for scope analysis to match oxlint's behavior in tests. - */ -function calculateCognitiveWithTracking( - code: string, - filename: string -): Map { - const { program, errors } = parseAndPrepareAst(code, filename); - if (errors.length > 0) { - throw new Error(`Parse errors: ${errors.map((e) => e.message).join(', ')}`); - } - - const results = new Map(); - - // Create context with scope analysis enabled - const context = createMockContext(program); - - const listener = createCognitiveVisitorWithTracking( - context, - (result: ComplexityResultWithVariables, node: ESTreeNode) => { - results.set(result.functionName, { - total: result.total, - points: result.points, - functionName: result.functionName, - variables: result.variables, - node, - }); - } - ); - - walkWithVisitor(program, listener, code); - return results; -} - -function buildCandidateFromResult(result: ExtendedResult): ExtractionCandidate { - return { - startLine: result.node.loc!.start.line, - endLine: result.node.loc!.end.line, - complexity: result.total, - complexityPercentage: 50, - points: result.points, - constructs: result.points.map((p) => p.construct), - }; -} - -function analyzeFlowFromResult(result: ExtendedResult): VariableFlowAnalysis { - const candidate = buildCandidateFromResult(result); - const functionEndLine = result.node.loc!.end.line; - return analyzeVariableFlow(candidate, result.variables, result.node, functionEndLine); -} - describe('Smart Extraction Detection', () => { describe('shouldAnalyzeExtraction', () => { it('returns false when complexity is at threshold', () => { @@ -316,122 +246,6 @@ describe('Smart Extraction Detection', () => { }); }); - describe('Early Return Detection', () => { - let earlyReturnResults: Map; - - beforeAll(() => { - const fixture = loadFixture(join(fixturesDir, 'js/early-returns.js'), fixturesDir); - earlyReturnResults = calculateCognitiveWithTracking(fixture.code, 'test.js'); - }); - - it('detects early returns in guard clause functions', () => { - const result = earlyReturnResults.get('guardClauses')!; - expect(result).toBeDefined(); - - const flow = analyzeFlowFromResult(result); - expect(flow.hasEarlyReturn).toBe(true); - }); - - it('detects early returns in functions with multiple exit points', () => { - const result = earlyReturnResults.get('multipleExitPoints')!; - expect(result).toBeDefined(); - - const flow = analyzeFlowFromResult(result); - expect(flow.hasEarlyReturn).toBe(true); - }); - - it('does not flag functions without return statements as having early returns', () => { - const code = ` - function noReturns(items) { - let count = 0; - for (const item of items) { - if (item.active) { - for (const child of item.children || []) { - if (child.valid) { - count++; - } - } - } - } - console.log(count); - } - `; - - const results = calculateCognitiveWithTracking(code, 'test.js'); - const result = results.get('noReturns')!; - const flow = analyzeFlowFromResult(result); - - expect(flow.hasEarlyReturn).toBe(false); - }); - - it('does not flag a single return on the last line as early return', () => { - const code = ` - function singleReturn(a, b) { - let result = 0; - if (a > 0) { - result = a + b; - } - return result; - } - `; - - const results = calculateCognitiveWithTracking(code, 'test.js'); - const result = results.get('singleReturn')!; - const flow = analyzeFlowFromResult(result); - - expect(flow.hasEarlyReturn).toBe(false); - }); - - it('shows early-return issues in formatted output', () => { - const code = ` - function processData(items, config) { - if (!items) return null; - let total = 0; - for (const item of items) { - if (item.valid) { - total += item.value; - } - } - - const factor = config.factor || 1; - const adjusted = total * factor; - const prefix = config.prefix || ''; - - let output = ''; - for (const entry of config.entries || []) { - if (entry.active) { - if (entry.value > adjusted) { - output += prefix + entry.label; - } - } - } - return output; - } - `; - - const results = calculateCognitiveWithTracking(code, 'test.js'); - const result = results.get('processData')!; - - const suggestions = analyzeExtractionOpportunities( - result.node, - result.points, - result.total, - result.variables, - 'processData' - ); - - expect(suggestions.length).toBe(2); - - const withEarlyReturn = suggestions.find((s) => - s.issues.some((i) => i.type === 'early-return') - ); - expect(withEarlyReturn).toBeDefined(); - - const formatted = formatExtractionSuggestions(suggestions); - expect(formatted).toContain('early return'); - }); - }); - describe('Requirements Validation', () => { it('tracks variable declarations and references within function', () => { const code = ` @@ -563,12 +377,15 @@ describe('Smart Extraction Detection', () => { expect(suggestions.length).toBe(2); - const withSignature = suggestions.find((s) => s.suggestedSignature); - expect(withSignature).toBeDefined(); - expect(withSignature!.suggestedSignature).toMatch(/\w+\(/); - expect(withSignature!.suggestedSignature).toMatch( - new RegExp(`^${PLACEHOLDER_FUNCTION_NAME}\\(`) - ); + // The first candidate contains `processed.push(item.id)` where `processed` + // is declared before the candidate range — correctly flagged as method-call + // mutation. The second candidate mutates `output` directly. Both have + // mutations so neither gets a suggested signature (low confidence). + const withMutation = suggestions.filter((s) => s.issues.some((i) => i.type === 'mutation')); + expect(withMutation.length).toBeGreaterThan(0); + for (const s of suggestions) { + expect(s.confidence).toBe('low'); + } }); }); }); diff --git a/tests/minlines.test.ts b/tests/minlines.test.ts new file mode 100644 index 0000000..81ad87f --- /dev/null +++ b/tests/minlines.test.ts @@ -0,0 +1,377 @@ +import { describe, it, expect } from 'vitest'; +import { parseSync } from 'oxc-parser'; +import { + createCombinedComplexityVisitor, + type CombinedComplexityResult, +} from '#src/combined-visitor.js'; +import { createMockContext, walkWithVisitor } from './utils/test-helpers.js'; +import type { ESTreeNode } from '#src/types.js'; + +/** + * Test suite for minLines configuration option. + * Tests that functions below the minLines threshold are skipped. + */ +describe('minLines configuration', () => { + interface TestCase { + name: string; + code: string; + expectedLines: number; + expectedCyclomatic: number; + expectedCognitive: number; + } + + const testCases: TestCase[] = [ + { + name: 'smallFunction', + code: ` + function smallFunction(a) { + return a + 1; + } + `, + expectedLines: 3, + expectedCyclomatic: 1, + expectedCognitive: 0, + }, + { + name: 'mediumFunction', + code: ` + function mediumFunction(a, b) { + const sum = a + b; + const product = a * b; + return { sum, product }; + } + `, + expectedLines: 5, + expectedCyclomatic: 1, + expectedCognitive: 0, + }, + { + name: 'largeSimpleFunction', + code: ` + function largeSimpleFunction(x) { + const a = x + 1; + const b = x + 2; + const c = x + 3; + const d = x + 4; + const e = x + 5; + const f = x + 6; + return a + b + c + d + e + f; + } + `, + expectedLines: 9, + expectedCyclomatic: 1, + expectedCognitive: 0, + }, + { + name: 'complexShortFunction', + code: ` + function complexShortFunction(a, b, c, d, e, f, g, h, i) { + return a && b && c && d && e && f && g && h && i; + } + `, + expectedLines: 3, + expectedCyclomatic: 9, // 1 + 8 && operators + expectedCognitive: 1, // Logical operators in sequence add +1 total (not per operator) + }, + { + name: 'complexLongFunction', + code: ` + function complexLongFunction(items, mode, config) { + if (!items) return null; + + for (const item of items) { + if (item.active) { + if (mode === 'strict') { + if (config.validate) { + if (item.required) { + processItem(item); + } + } + } + } + } + } + `, + expectedLines: 15, + expectedCyclomatic: 7, // 1 + 1 for + 5 if (including the early return if) + expectedCognitive: 16, // Deep nesting + }, + ]; + + describe('Line counting', () => { + it.each(testCases)('should count $expectedLines lines for $name', ({ code, expectedLines }) => { + const { program } = parseSync('test.ts', code); + const ast = program as unknown as ESTreeNode; + + let actualLines = 0; + + const visitor = { + 'FunctionDeclaration:exit': (node: ESTreeNode) => { + if (node.loc) { + actualLines = node.loc.end.line - node.loc.start.line + 1; + } + }, + 'FunctionExpression:exit': (node: ESTreeNode) => { + if (node.loc) { + actualLines = node.loc.end.line - node.loc.start.line + 1; + } + }, + 'ArrowFunctionExpression:exit': (node: ESTreeNode) => { + if (node.loc) { + actualLines = node.loc.end.line - node.loc.start.line + 1; + } + }, + }; + + walkWithVisitor(ast, visitor, code); + + expect(actualLines).toBe(expectedLines); + }); + }); + + describe('minLines filtering behavior', () => { + interface FilterTestCase { + minLines: number; + expectedFunctionsAnalyzed: string[]; + } + + const filterTests: FilterTestCase[] = [ + { + minLines: 0, + expectedFunctionsAnalyzed: [ + 'smallFunction', + 'mediumFunction', + 'largeSimpleFunction', + 'complexShortFunction', + 'complexLongFunction', + ], + }, + { + minLines: 5, + expectedFunctionsAnalyzed: ['mediumFunction', 'largeSimpleFunction', 'complexLongFunction'], + }, + { + minLines: 10, + expectedFunctionsAnalyzed: ['complexLongFunction'], + }, + { + minLines: 20, + expectedFunctionsAnalyzed: [], + }, + ]; + + it.each(filterTests)( + 'with minLines: $minLines, should analyze: $expectedFunctionsAnalyzed', + ({ minLines, expectedFunctionsAnalyzed }) => { + const allCode = testCases.map((tc) => tc.code).join('\n'); + const { program } = parseSync('test.ts', allCode); + const ast = program as unknown as ESTreeNode; + + const analyzedFunctions: string[] = []; + + const onComplexityCalculated = (result: CombinedComplexityResult, node: ESTreeNode) => { + // Simulate the minLines check from complexity.ts + if (minLines > 0 && node.loc) { + const functionLines = node.loc.end.line - node.loc.start.line + 1; + if (functionLines < minLines) { + return; // Skip + } + } + + // Extract function name + if (node.type === 'FunctionDeclaration') { + const funcNode = node as { id?: { name: string } }; + if (funcNode.id?.name) { + analyzedFunctions.push(funcNode.id.name); + } + } + }; + + const listener = createCombinedComplexityVisitor( + createMockContext(ast), + onComplexityCalculated + ); + walkWithVisitor(ast, listener, allCode); + + expect(analyzedFunctions.sort()).toEqual(expectedFunctionsAnalyzed.sort()); + } + ); + }); + + describe('Complexity calculation with minLines', () => { + it('should correctly calculate complexity for functions that pass minLines threshold', () => { + const minLines = 10; + const code = testCases.map((tc) => tc.code).join('\n'); + const { program } = parseSync('test.ts', code); + const ast = program as unknown as ESTreeNode; + + const results = new Map(); + + const onComplexityCalculated = (result: CombinedComplexityResult, node: ESTreeNode) => { + // Apply minLines filter + if (node.loc) { + const functionLines = node.loc.end.line - node.loc.start.line + 1; + if (minLines > 0 && functionLines < minLines) { + return; // Skip + } + + // Get function name + if (node.type === 'FunctionDeclaration') { + const funcNode = node as { id?: { name: string } }; + if (funcNode.id?.name) { + results.set(funcNode.id.name, { + cyclomatic: result.cyclomatic, + cognitive: result.cognitive, + lines: functionLines, + }); + } + } + } + }; + + const listener = createCombinedComplexityVisitor( + createMockContext(ast), + onComplexityCalculated + ); + walkWithVisitor(ast, listener, code); + + // Only complexLongFunction should be analyzed (15 lines >= 10) + expect(results.size).toBe(1); + expect(results.has('complexLongFunction')).toBe(true); + + const complexLong = results.get('complexLongFunction')!; + expect(complexLong.lines).toBe(15); + expect(complexLong.cyclomatic).toBe(7); // Fixed: 1 + 6 (1 for + 5 if) + expect(complexLong.cognitive).toBe(16); + }); + + it('should analyze all functions when minLines is 0', () => { + const minLines = 0; + const code = testCases.map((tc) => tc.code).join('\n'); + const { program } = parseSync('test.ts', code); + const ast = program as unknown as ESTreeNode; + + const results = new Map(); + + const onComplexityCalculated = (result: CombinedComplexityResult, node: ESTreeNode) => { + // Apply minLines filter + if (minLines > 0 && node.loc) { + const functionLines = node.loc.end.line - node.loc.start.line + 1; + if (functionLines < minLines) { + return; // Skip + } + } + + // Get function name + if (node.type === 'FunctionDeclaration') { + const funcNode = node as { id?: { name: string } }; + if (funcNode.id?.name) { + results.set(funcNode.id.name, { + cyclomatic: result.cyclomatic, + cognitive: result.cognitive, + }); + } + } + }; + + const listener = createCombinedComplexityVisitor( + createMockContext(ast), + onComplexityCalculated + ); + walkWithVisitor(ast, listener, code); + + // All 5 functions should be analyzed + expect(results.size).toBe(5); + + // Verify specific complexities + const testCase = testCases.find((tc) => tc.name === 'complexShortFunction')!; + const complexShort = results.get('complexShortFunction'); + expect(complexShort?.cyclomatic).toBe(testCase.expectedCyclomatic); + expect(complexShort?.cognitive).toBe(testCase.expectedCognitive); + }); + }); + + describe('Edge cases', () => { + it('should handle one-liner arrow functions correctly', () => { + const code = 'const oneLineArrow = (x) => x + 1;'; + const { program } = parseSync('test.ts', code); + const ast = program as unknown as ESTreeNode; + + let functionLines = 0; + + const visitor = { + 'ArrowFunctionExpression:exit': (node: ESTreeNode) => { + if (node.loc) { + functionLines = node.loc.end.line - node.loc.start.line + 1; + } + }, + }; + + walkWithVisitor(ast, visitor, code); + + // One-liner arrow function should be 1 line + expect(functionLines).toBe(1); + }); + + it('should handle functions with comments correctly', () => { + const code = ` + function withComments(a, b) { + // This is a comment + const sum = a + b; + // Another comment + + return sum; + } + `; + const { program } = parseSync('test.ts', code); + const ast = program as unknown as ESTreeNode; + + let functionLines = 0; + + const visitor = { + 'FunctionDeclaration:exit': (node: ESTreeNode) => { + if (node.loc) { + functionLines = node.loc.end.line - node.loc.start.line + 1; + } + }, + }; + + walkWithVisitor(ast, visitor, code); + + // Should count comments and blank lines + expect(functionLines).toBe(7); + }); + + it('should handle functions without loc gracefully', () => { + const code = 'function test() { return 1; }'; + const { program } = parseSync('test.ts', code); + const ast = program as unknown as ESTreeNode; + + let analyzedCount = 0; + + const onComplexityCalculated = (result: CombinedComplexityResult, node: ESTreeNode) => { + // Simulate the minLines check - should handle missing loc gracefully + const minLines = 10; + if (minLines > 0 && node.loc) { + const functionLines = node.loc.end.line - node.loc.start.line + 1; + if (functionLines < minLines) { + return; // Skip + } + } + + // If loc is missing or function is long enough, it gets analyzed + analyzedCount++; + }; + + const listener = createCombinedComplexityVisitor( + createMockContext(ast), + onComplexityCalculated + ); + walkWithVisitor(ast, listener, code); + + // Function with 1 line should be skipped (< minLines: 10) + // But if loc is missing, the check is bypassed and function is analyzed + expect(analyzedCount).toBe(0); // Should be skipped because it has loc and is < 10 lines + }); + }); +}); diff --git a/tests/utils/extraction-helpers.ts b/tests/utils/extraction-helpers.ts new file mode 100644 index 0000000..bf9039e --- /dev/null +++ b/tests/utils/extraction-helpers.ts @@ -0,0 +1,94 @@ +import type { ESTreeNode, ComplexityResult } from '#src/types.js'; +import type { + VariableInfo, + ExtractionCandidate, + VariableFlowAnalysis, +} from '#src/extraction/types.js'; +import { + createCognitiveVisitorWithTracking, + type ComplexityResultWithVariables, +} from '#src/cognitive/visitor.js'; +import { analyzeVariableFlow } from '#src/extraction/flow-analyzer.js'; +import { extractConstructFromMessage } from '#src/utils.js'; +import { createMockContext, walkWithVisitor, parseAndPrepareAst } from './test-helpers.js'; + +export interface ExtendedResult extends ComplexityResult { + functionName: string; + variables: Map; + node: ESTreeNode; +} + +/** + * Calculate cognitive complexity with variable tracking. + * Uses eslint-scope for scope analysis to match oxlint's behavior in tests. + */ +export function calculateCognitiveWithTracking( + code: string, + filename: string +): Map { + const { program, errors } = parseAndPrepareAst(code, filename); + if (errors.length > 0) { + throw new Error(`Parse errors: ${errors.map((e) => e.message).join(', ')}`); + } + + const results = new Map(); + + // Create context with scope analysis enabled + const context = createMockContext(program); + + const listener = createCognitiveVisitorWithTracking( + context, + (result: ComplexityResultWithVariables, node: ESTreeNode) => { + results.set(result.functionName, { + total: result.total, + points: result.points, + functionName: result.functionName, + variables: result.variables, + node, + }); + } + ); + + walkWithVisitor(program, listener, code); + return results; +} + +export function buildCandidateFromResult(result: ExtendedResult): ExtractionCandidate { + return { + startLine: result.node.loc!.start.line, + endLine: result.node.loc!.end.line, + complexity: result.total, + complexityPercentage: 50, + points: result.points, + constructs: result.points.map((p) => extractConstructFromMessage(p.message)), + }; +} + +/** + * Build an ExtractionCandidate for a specific range within a function. + * @param result - The extended analysis result containing the function node and complexity data + * @param startOffset - Offset from function start line (e.g., 2 to skip first 2 lines) + * @param endOffset - Offset from function end line (e.g., -1 to exclude last line) + */ +export function buildCandidateForRange( + result: ExtendedResult, + startOffset: number, + endOffset: number +): ExtractionCandidate { + const funcStart = result.node.loc!.start.line; + const funcEnd = result.node.loc!.end.line; + + return { + startLine: funcStart + startOffset, + endLine: funcEnd + endOffset, + complexity: result.total, + complexityPercentage: 50, + points: result.points, + constructs: result.points.map((p) => extractConstructFromMessage(p.message)), + }; +} + +export function analyzeFlowFromResult(result: ExtendedResult): VariableFlowAnalysis { + const candidate = buildCandidateFromResult(result); + return analyzeVariableFlow(candidate, result.variables, result.node); +} diff --git a/tests/utils/test-helpers.ts b/tests/utils/test-helpers.ts index a62c2b0..6c95350 100644 --- a/tests/utils/test-helpers.ts +++ b/tests/utils/test-helpers.ts @@ -3,6 +3,10 @@ import { walk } from 'estree-walker'; import type { Node as EstreeWalkerNode } from 'estree-walker'; import { createCyclomaticVisitor } from '#src/cyclomatic.js'; import { createCognitiveVisitor } from '#src/cognitive/visitor.js'; +import { + createCombinedComplexityVisitor, + type CombinedComplexityResult, +} from '#src/combined-visitor.js'; import { getFunctionName as getProductionFunctionName } from '#src/utils.js'; import type { ESTreeNode, FunctionNode, ComplexityResult, Context } from '#src/types.js'; import type { ScopeManager } from 'oxlint/plugins'; @@ -253,3 +257,49 @@ export function calculateComplexity( cognitive: calculateCognitiveComplexity(code, filename), }; } + +/** + * Calculate both cyclomatic and cognitive complexity using the combined visitor. + * This should produce identical results to running them separately. + */ +export function calculateCombinedComplexity( + code: string, + filename = 'test.ts' +): { + cyclomatic: Map; + cognitive: Map; +} { + const { program, errors } = parseSync(filename, code); + + if (errors.length > 0) { + throw new Error(`Parse errors in "${filename}": ${errors.map((e) => e.message).join(', ')}`); + } + + const cyclomaticResults = new Map(); + const cognitiveResults = new Map(); + let functionIndex = 0; + + const onComplexityCalculated = (result: CombinedComplexityResult, node: ESTreeNode) => { + const name = getFunctionName(node, functionIndex++); + + cyclomaticResults.set(name, { + name, + total: result.cyclomatic, + points: result.cyclomaticPoints, + }); + + cognitiveResults.set(name, { + name, + total: result.cognitive, + points: result.cognitivePoints, + }); + }; + + const listener = createCombinedComplexityVisitor(createMockContext(), onComplexityCalculated); + walkWithVisitor(program as unknown as ESTreeNode, listener, code); + + return { + cyclomatic: cyclomaticResults, + cognitive: cognitiveResults, + }; +}