only flag unused array-destructs if it is the last variable

Author: erik-krogh

javascript/ql/src/Statements/SuspiciousUnusedLoopIterationVariable.ql

@@ -45,11 +45,28 @@ predicate countingLoop(EnhancedForLoop efl) {
   )
 }
 
+/**
+ * Holds if `iter` is the non-last variable of array-destructuring in a for-loop.
+ * E.g. `for(const [foo, iter, bar] of array) {..}`.
+ */
+predicate isNonLastDestructedArrayElement(PurelyLocalVariable iter) {
+  exists(ArrayPattern pattern | pattern = any(EnhancedForLoop loop).getLValue() |
+    iter = pattern.getAVariable() and
+    iter =
+      pattern
+          .getElement([0 .. pattern.getSize() - 2])
+          .(BindingPattern)
+          .getABindingVarRef()
+          .getVariable()
+  )
+}
+
 from EnhancedForLoop efl, PurelyLocalVariable iter
 where
   iter = efl.getAnIterationVariable() and
   not exists(SsaExplicitDefinition ssa | ssa.defines(efl.getIteratorExpr(), iter)) and
   exists(ReachableBasicBlock body | body.getANode() = efl.getBody() | body.getASuccessor+() = body) and
   not countingLoop(efl) and
+  not isNonLastDestructedArrayElement(iter) and
   not iter.getName().toLowerCase().regexpMatch("(_|dummy|unused).*")
 select efl.getIterator(), "For loop variable " + iter.getName() + " is not used in the loop body."

javascript/ql/test/query-tests/Statements/SuspiciousUnusedLoopIterationVariable/SuspiciousUnusedLoopIterationVariable.expected

@@ -1 +1,4 @@
 | tst.js:4:7:4:11 | let x | For loop variable x is not used in the loop body. |
+| tst.js:138:6:138:23 | const [key, value] | For loop variable value is not used in the loop body. |
+| tst.js:151:6:151:35 | const [ ...  value] | For loop variable value is not used in the loop body. |
+| tst.js:152:6:152:10 | let i | For loop variable i is not used in the loop body. |

javascript/ql/test/query-tests/Statements/SuspiciousUnusedLoopIterationVariable/tst.js

@@ -133,3 +133,20 @@ function countOccurrencesDead(xs, p) {
 		a;
 	}
 });
+
+// NOT OK
+for (const [key, value] of array) {}
+
+// OK: for array-destructurings we only flag the last element
+for (const [key, value] of array) {
+	console.log(value)
+}
+
+// OK: for array-destructurings we only flag the last element
+for (const [key, key2, key3, value] of array) {
+	console.log(value)
+}
+
+// NOT OK
+for (const [key, key2, key3, value] of array) {}
+for (let i of [1, 2]) {}