Merge pull request github#5939 from max-schaefer/js/set-constructor-args

Approved by esbena

Author: codeql-ci

javascript/ql/src/experimental/StandardLibrary/MultipleArgumentsToSetConstructor.qhelp

@@ -0,0 +1,43 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+The <code>Set</code> constructor accepts an arbitrary number of arguments, but only the first one
+is used to construct the set. The remaining arguments are ignored.
+Code that invokes the <code>Set</code> constructor with multiple arguments is therefore likely to
+be incorrect.
+</p>
+</overview>
+
+<recommendation>
+<p>
+Only pass a single argument to the <code>Set</code> constructor, which should be an iterable object
+(such as an array).
+</p>
+</recommendation>
+
+<example>
+<p>
+The following example creates a set containing the vowels in the English language, and defines
+a function that returns a boolean indicating whether a given character is a vowel:
+</p>
+<sample src="examples/MultipleArgumentsToSetConstructorBad.js"/>
+
+<p>
+However, this code does not work as intended: the <code>Set</code> constructor ignores all but
+the first argument, so the <code>vowels</code> set only contains the letter <code>a</code>.
+</p>
+
+<p>
+Instead, the list of vowels should be wrapped into an array:
+</p>
+<sample src="examples/MultipleArgumentsToSetConstructorGood.js"/>
+</example>
+
+<references>
+<li><a href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set/Set">MDN Web Docs: Set() constructor</a></li>
+</references>
+</qhelp>

javascript/ql/src/experimental/StandardLibrary/MultipleArgumentsToSetConstructor.ql

@@ -0,0 +1,22 @@
+/**
+ * @name Multiple arguments to `Set` constructor
+ * @description The `Set` constructor ignores all but the first argument, so passing multiple
+ *              arguments may indicate a mistake.
+ * @kind problem
+ * @problem.severity warning
+ * @precision high
+ * @id js/multiple-arguments-to-set-constructor
+ * @tags correctness
+ */
+
+import javascript
+
+from DataFlow::NewNode newSet, DataFlow::Node ignoredArg
+where
+  newSet = DataFlow::globalVarRef("Set").getAnInstantiation() and
+  (
+    ignoredArg = newSet.getArgument(any(int n | n > 0))
+    or
+    ignoredArg = newSet.getASpreadArgument()
+  )
+select ignoredArg, "All but the first argument to the Set constructor are ignored."

javascript/ql/src/experimental/StandardLibrary/examples/MultipleArgumentsToSetConstructorBad.js

@@ -0,0 +1,5 @@
+const vowels = new Set('a', 'e', 'i', 'o', 'u');
+
+function isVowel(char) {
+  return vowels.has(char.toLowerCase());
+}

javascript/ql/src/experimental/StandardLibrary/examples/MultipleArgumentsToSetConstructorGood.js

@@ -0,0 +1,5 @@
+const vowels = new Set(['a', 'e', 'i', 'o', 'u']);
+
+function isVowel(char) {
+  return vowels.has(char.toLowerCase());
+}

javascript/ql/test/experimental/StandardLibrary/MultipleArgumentsToSetConstructor/MultipleArgumentsToSetConstructor.expected

@@ -0,0 +1,6 @@
+| MultipleArgumentsToSetConstructorBad.js:1:29:1:31 | 'e' | All but the first argument to the Set constructor are ignored. |
+| MultipleArgumentsToSetConstructorBad.js:1:34:1:36 | 'i' | All but the first argument to the Set constructor are ignored. |
+| MultipleArgumentsToSetConstructorBad.js:1:39:1:41 | 'o' | All but the first argument to the Set constructor are ignored. |
+| MultipleArgumentsToSetConstructorBad.js:1:44:1:46 | 'u' | All but the first argument to the Set constructor are ignored. |
+| tst.js:3:12:3:13 | xs | All but the first argument to the Set constructor are ignored. |
+| tst.js:3:19:3:20 | ys | All but the first argument to the Set constructor are ignored. |

javascript/ql/test/experimental/StandardLibrary/MultipleArgumentsToSetConstructor/MultipleArgumentsToSetConstructor.qlref

@@ -0,0 +1 @@
+experimental/StandardLibrary/MultipleArgumentsToSetConstructor.ql

javascript/ql/test/experimental/StandardLibrary/MultipleArgumentsToSetConstructor/MultipleArgumentsToSetConstructorBad.js

@@ -0,0 +1,5 @@
+const vowels = new Set('a', 'e', 'i', 'o', 'u');
+
+function isVowel(char) {
+  return vowels.has(char.toLowerCase());
+}

javascript/ql/test/experimental/StandardLibrary/MultipleArgumentsToSetConstructor/MultipleArgumentsToSetConstructorGood.js

@@ -0,0 +1,5 @@
+const vowels = new Set(['a', 'e', 'i', 'o', 'u']);
+
+function isVowel(char) {
+  return vowels.has(char.toLowerCase());
+}

javascript/ql/test/experimental/StandardLibrary/MultipleArgumentsToSetConstructor/tst.js

@@ -0,0 +1,6 @@
+let xs = [1, 2, 3];
+let ys = [4, 5, 6];
+new Set(...xs, ...ys); // NOT OK
+new Set([...xs, ...ys]); // OK
+new Set(xs); // OK
+new Set(); // OK