Merged
Conversation
📝 WalkthroughWalkthroughAdds a single-pass combined complexity visitor and new complexity rule, refactors rule/shared helpers, enhances extraction (AST mutation detection and this-reference), exposes extraction APIs, removes React JSX detection, updates utilities and docs, and adds/adjusts extensive tests and test helpers. Changes
Sequence Diagram(s)sequenceDiagram
participant Analyzer as Analyzer
participant Combined as CombinedVisitor
participant Scope as FunctionScope
participant Points as PointsCollector
Analyzer->>Combined: walk AST
Combined->>Scope: enter function (push scope)
loop traverse nodes
Combined->>Combined: detect control-flow / logical ops / recursion
Combined->>Points: add cyclomatic point
Combined->>Points: add cognitive point
Combined->>Scope: update nesting / record points
end
Combined->>Scope: exit function (pop scope)
Combined->>Analyzer: report CombinedComplexityResult (cyclomatic, cognitive, points)
sequenceDiagram
participant FlowAnalyzer as Flow Analyzer
participant AST as AST Walker
participant Direct as DirectMutationChecker
participant Prop as PropertyMutationChecker
participant Dedup as Deduplicator
FlowAnalyzer->>FlowAnalyzer: prepare candidate range
par detect paths
FlowAnalyzer->>Direct: check direct reassignments in range
FlowAnalyzer->>AST: walk range (skip nested functions)
AST->>Prop: detect property updates & mutating method calls
Prop->>FlowAnalyzer: return AST-based mutations
and
Direct->>FlowAnalyzer: return direct mutations
end
FlowAnalyzer->>Dedup: combine and deduplicate mutations + detect this-reference
Dedup->>FlowAnalyzer: return unique mutations + hasThisReference
FlowAnalyzer->>Client: return VariableFlowAnalysis
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
39-52:⚠️ Potential issue | 🟡 MinorAdd a language identifier to the fenced output block.
This addresses the MD040 warning and improves tooling support.📝 Suggested fix
-``` +```text ```
🤖 Fix all issues with AI agents
In `@package.json`:
- Line 48: The "benchmark" npm script currently tries to run a TypeScript file
with node ("benchmark": "node benchmarks/run-benchmark.ts"), which won't work;
update the "benchmark" script to invoke a TypeScript runner (e.g., use "tsx"
like the other profile scripts) so benchmarks/run-benchmark.ts is executed via
tsx (or another configured TS runner) instead of node.
In `@README.md`:
- Around line 148-155: Change the heading for "Known Limitations" to the correct
level to avoid a markdown heading-level jump (MD001); locate the "Known
Limitations" heading (currently "##### Known Limitations") and adjust the number
of '#' characters so it follows the previous section's level (e.g., change to
"#### Known Limitations" or "### Known Limitations" as appropriate) so heading
depth increments/decrements by only one level relative to its surrounding
headings.
- Around line 74-123: Remove the outdated React component exclusion from the
"Cognitive Complexity" docs: edit the "Cognitive Complexity" section (the header
and its bullets) to delete the phrase "React components (PascalCase + returns
JSX)" from the "Excluded" line and ensure the remaining exclusion examples
(e.g., default value patterns like `a || []`) are kept accurate; update any
adjacent explanatory text or examples that reference React components so the
docs match the current implementation.
In `@src/rules/shared.ts`:
- Around line 109-123: warnDeprecated currently prints a deprecation warning
each time createOnce's before() runs (once per file); add a module-level
deduplication flag (e.g., let warned = false) and update warnDeprecated to
check/set that flag so it only logs the message the first time it's invoked;
reference the existing warnDeprecated function and the createOnce/before() usage
when making the change.
In `@tests/extraction-flow.test.ts`:
- Around line 159-166: Tests build ExtractionCandidate objects using
result.points.map(p => p.construct) but ComplexityPoint has no construct field;
replace that pattern by mapping the messages through
extractConstructFromMessage. Extract this into a helper (e.g.,
buildCandidateForRange) that accepts an ExtendedResult and start/end offsets,
computes funcStart/funcEnd from result.node.loc, sets startLine/endLine,
complexity, complexityPercentage, points: result.points, and constructs:
result.points.map(p => extractConstructFromMessage(p.message)); update all
occurrences (the repeated blocks around lines mentioned) to call this helper.
In `@tests/utils/extraction-helpers.ts`:
- Around line 66-69: The call in analyzeFlowFromResult passes a fourth argument
functionEndLine to analyzeVariableFlow but analyzeVariableFlow’s signature only
accepts (candidate, variables, functionNode); update the call (in
analyzeFlowFromResult) to remove the extra argument and call
analyzeVariableFlow(candidate, result.variables, result.node) or alternatively,
if functionEndLine is required, update the analyzeVariableFlow function
signature in src/extraction/flow-analyzer.ts to accept a fourth parameter (e.g.,
functionEndLine) and wire it through its implementation; reference symbols:
analyzeFlowFromResult, analyzeVariableFlow, buildCandidateFromResult,
ExtendedResult, VariableFlowAnalysis, functionEndLine.
- Around line 55-64: The buildCandidateFromResult function is using p.construct
(which doesn't exist on ComplexityPoint), so update it to derive constructs via
extractConstructFromMessage: import extractConstructFromMessage from
src/utils.ts and replace constructs: result.points.map(p => p.construct) with
constructs: result.points.map(p => extractConstructFromMessage(p.message)),
ensuring you reference buildCandidateFromResult and ComplexityPoint when making
the change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
thisreferences).Deprecated
max-cyclomaticandmax-cognitivesuperseded bycomplexity.Documentation
Tests
Chores