diff --git a/javascript/ql/integration-tests/query-suite/javascript-code-quality-extended.qls.expected b/javascript/ql/integration-tests/query-suite/javascript-code-quality-extended.qls.expected index 0ee4ff2cf9c5..6894a776b379 100644 --- a/javascript/ql/integration-tests/query-suite/javascript-code-quality-extended.qls.expected +++ b/javascript/ql/integration-tests/query-suite/javascript-code-quality-extended.qls.expected @@ -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 diff --git a/javascript/ql/integration-tests/query-suite/javascript-code-quality.qls.expected b/javascript/ql/integration-tests/query-suite/javascript-code-quality.qls.expected index 0ee4ff2cf9c5..6894a776b379 100644 --- a/javascript/ql/integration-tests/query-suite/javascript-code-quality.qls.expected +++ b/javascript/ql/integration-tests/query-suite/javascript-code-quality.qls.expected @@ -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 diff --git a/javascript/ql/src/Statements/LoopIterationSkippedDueToShifting.ql b/javascript/ql/src/Statements/LoopIterationSkippedDueToShifting.ql index 08ed90777467..d36884e15a32 100644 --- a/javascript/ql/src/Statements/LoopIterationSkippedDueToShifting.ql +++ b/javascript/ql/src/Statements/LoopIterationSkippedDueToShifting.ql @@ -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 */ @@ -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 + ) } } diff --git a/javascript/ql/src/change-notes/2025-06-12-loop-iteration-fix.md b/javascript/ql/src/change-notes/2025-06-12-loop-iteration-fix.md new file mode 100644 index 000000000000..2716069fb711 --- /dev/null +++ b/javascript/ql/src/change-notes/2025-06-12-loop-iteration-fix.md @@ -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. diff --git a/javascript/ql/src/change-notes/2025-06-12-loop-iteration.md b/javascript/ql/src/change-notes/2025-06-12-loop-iteration.md new file mode 100644 index 000000000000..13b9fcf592aa --- /dev/null +++ b/javascript/ql/src/change-notes/2025-06-12-loop-iteration.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The `js/loop-iteration-skipped-due-to-shifting` query now has the `reliability` tag. diff --git a/javascript/ql/test/query-tests/Statements/LoopIterationSkippedDueToShifting/LoopIterationSkippedDueToShifting.expected b/javascript/ql/test/query-tests/Statements/LoopIterationSkippedDueToShifting/LoopIterationSkippedDueToShifting.expected index 4b7becd8e163..cfe2b5f4f58d 100644 --- a/javascript/ql/test/query-tests/Statements/LoopIterationSkippedDueToShifting/LoopIterationSkippedDueToShifting.expected +++ b/javascript/ql/test/query-tests/Statements/LoopIterationSkippedDueToShifting/LoopIterationSkippedDueToShifting.expected @@ -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. | diff --git a/javascript/ql/test/query-tests/Statements/LoopIterationSkippedDueToShifting/tst.js b/javascript/ql/test/query-tests/Statements/LoopIterationSkippedDueToShifting/tst.js index 68c50516da04..01f046d1c1e2 100644 --- a/javascript/ql/test/query-tests/Statements/LoopIterationSkippedDueToShifting/tst.js +++ b/javascript/ql/test/query-tests/Statements/LoopIterationSkippedDueToShifting/tst.js @@ -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--; + } + } + } +}