Skip to content

Commit 0597d43

Browse files
Vibe Botfrancois-mora-sonarsource
authored andcommitted
Fix SonarQube issues in S2310 implementation
Reduce cognitive complexity (S3776) and nesting depth (S134) in the checkCounter function by extracting the skip-ahead detection logic into a standalone isIntentionalSkipAhead helper function. This moves the UpdateExpression and compound assignment checks out of deeply nested control flow, lowering checkCounter's cognitive complexity from 17 to ~8 (allowed: 15) and eliminating nesting beyond 3 levels. No behavioral changes; all existing tests pass.
1 parent b7eaec7 commit 0597d43

File tree

2 files changed

+44
-12
lines changed

2 files changed

+44
-12
lines changed

package-lock.json

Lines changed: 23 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/jsts/src/rules/S2310/rule.ts

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -51,17 +51,8 @@ export const rule: Rule.RuleModule = {
5151
}
5252
for (const ref of variable.references) {
5353
if (ref.isWrite() && isUsedInsideBody(ref.identifier, block)) {
54-
// Allow UpdateExpression (i++, i--, ++i, --i) and compound assignments (i+=n, i-=n, etc.)
55-
// Only report simple assignments (i = value) which indicate early-exit patterns
56-
// Exception: modifications in nested for-loop update clauses should still be reported
57-
const parent = getNodeParent(ref.identifier);
58-
if (!isInNestedForLoopUpdate(ref.identifier, block)) {
59-
if (parent?.type === 'UpdateExpression') {
60-
continue;
61-
}
62-
if (parent?.type === 'AssignmentExpression' && parent.operator !== '=') {
63-
continue;
64-
}
54+
if (isIntentionalSkipAhead(ref.identifier, block)) {
55+
continue;
6556
}
6657
report(
6758
context,
@@ -108,6 +99,25 @@ function collectCountersFor(updateExpression: estree.Expression, counters: estre
10899
}
109100
}
110101

102+
/**
103+
* Checks if a loop counter modification is an intentional skip-ahead pattern
104+
* (UpdateExpression or compound assignment) rather than a simple assignment.
105+
* Modifications in nested for-loop update clauses are not considered skip-ahead.
106+
*/
107+
function isIntentionalSkipAhead(id: estree.Identifier, outerLoopBody: estree.Node): boolean {
108+
if (isInNestedForLoopUpdate(id, outerLoopBody)) {
109+
return false;
110+
}
111+
const parent = getNodeParent(id);
112+
if (parent?.type === 'UpdateExpression') {
113+
return true;
114+
}
115+
if (parent?.type === 'AssignmentExpression' && parent.operator !== '=') {
116+
return true;
117+
}
118+
return false;
119+
}
120+
111121
function isUsedInsideBody(id: estree.Identifier, loopBody: estree.Node) {
112122
const bodyRange = loopBody.range;
113123
return id.range && bodyRange && id.range[0] > bodyRange[0] && id.range[1] < bodyRange[1];

0 commit comments

Comments
 (0)