Skip to content
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ ql/javascript/ql/src/Statements/DanglingElse.ql
ql/javascript/ql/src/Statements/IgnoreArrayResult.ql
ql/javascript/ql/src/Statements/InconsistentLoopOrientation.ql
ql/javascript/ql/src/Statements/LabelInCase.ql
ql/javascript/ql/src/Statements/LoopIterationSkippedDueToShifting.ql
ql/javascript/ql/src/Statements/MisleadingIndentationAfterControlStmt.ql
ql/javascript/ql/src/Statements/ReturnAssignsLocal.ql
ql/javascript/ql/src/Statements/SuspiciousUnusedLoopIterationVariable.ql
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ ql/javascript/ql/src/Statements/DanglingElse.ql
ql/javascript/ql/src/Statements/IgnoreArrayResult.ql
ql/javascript/ql/src/Statements/InconsistentLoopOrientation.ql
ql/javascript/ql/src/Statements/LabelInCase.ql
ql/javascript/ql/src/Statements/LoopIterationSkippedDueToShifting.ql
ql/javascript/ql/src/Statements/MisleadingIndentationAfterControlStmt.ql
ql/javascript/ql/src/Statements/ReturnAssignsLocal.ql
ql/javascript/ql/src/Statements/SuspiciousUnusedLoopIterationVariable.ql
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
* @kind problem
* @problem.severity warning
* @id js/loop-iteration-skipped-due-to-shifting
* @tags correctness
* @tags quality
* reliability
* correctness
* @precision high
*/

Expand Down Expand Up @@ -146,7 +148,12 @@ class ArrayIterationLoop extends ForStmt {
or
this.hasPathThrough(splice, cfg.getAPredecessor()) and
this.getLoopEntry().dominates(cfg.getBasicBlock()) and
not this.hasIndexingManipulation(cfg)
not this.hasIndexingManipulation(cfg) and
// Don't continue through a branch that tests the splice call's return value
not exists(ConditionGuardNode guard | cfg = guard |
guard.getTest() = splice.asExpr() and
guard.getOutcome() = false
)
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Fixed false positives in the `js/loop-iteration-skipped-due-to-shifting` query when the return value of `splice` is used to decide whether to adjust the loop counter.
4 changes: 4 additions & 0 deletions javascript/ql/src/change-notes/2025-06-12-loop-iteration.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The `js/loop-iteration-skipped-due-to-shifting` query now has the `reliability` tag.
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
| tst.js:4:27:4:44 | parts.splice(i, 1) | Removing an array item without adjusting the loop index 'i' causes the subsequent array item to be skipped. |
| tst.js:13:29:13:46 | parts.splice(i, 1) | Removing an array item without adjusting the loop index 'i' causes the subsequent array item to be skipped. |
| tst.js:24:9:24:26 | parts.splice(i, 1) | Removing an array item without adjusting the loop index 'i' causes the subsequent array item to be skipped. |
| tst.js:128:11:128:33 | pending ... e(i, 1) | Removing an array item without adjusting the loop index 'i' causes the subsequent array item to be skipped. |
| tst.js:153:11:153:26 | toc.splice(i, 1) | Removing an array item without adjusting the loop index 'i' causes the subsequent array item to be skipped. |
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,38 @@ function inspectNextElement(string) {
}
return parts.join('/');
}

function withTryCatch(pendingCSS) {
for (let i = 0; i < pendingCSS.length; ++i) {
try {
pendingCSS.splice(i, 1); // $ SPURIOUS:Alert
i -= 1;
} catch (ex) {}
}
}

function andOperand(toc) {
for (let i = 0; i < toc.length; i++) {
toc[i].ignoreSubHeading && toc.splice(i, 1) && i--;
}
}

function ifStatement(toc) {
for (let i = 0; i < toc.length; i++) {
if(toc[i].ignoreSubHeading){
if(toc.splice(i, 1)){
i--;
}
}
}
}

function ifStatement2(toc) {
for (let i = 0; i < toc.length; i++) {
if(toc[i].ignoreSubHeading){
if(!toc.splice(i, 1)){ // $Alert
i--;
}
}
}
}