Merge pull request #3464 from github/robertbrignull/token-not-used

Create token-not-used.ql to catch cases where we aren't using the progress bar token correctly

Author: robertbrignull

.github/codeql/queries/ProgressBar.qll

@@ -0,0 +1,36 @@
+import javascript
+
+abstract class ProgressBar extends CallExpr {
+  ProgressBar() { any() }
+
+  abstract Function getCallback();
+
+  abstract ObjectExpr getOptions();
+
+  predicate usesToken() { exists(this.getTokenParameter()) }
+
+  Parameter getTokenParameter() { result = this.getCallback().getParameter(1) }
+
+  Property getCancellableProperty() { result = this.getOptions().getPropertyByName("cancellable") }
+
+  predicate isCancellable() {
+    this.getCancellableProperty().getInit().(BooleanLiteral).getBoolValue() =
+      true
+  }
+}
+
+class WithProgressCall extends ProgressBar {
+  WithProgressCall() { this.getCalleeName() = "withProgress" }
+
+  override Function getCallback() { result = this.getArgument(0) }
+
+  override ObjectExpr getOptions() { result = this.getArgument(1) }
+}
+
+class WithInheritedProgressCall extends ProgressBar {
+  WithInheritedProgressCall() { this.getCalleeName() = "withInheritedProgress" }
+
+  override Function getCallback() { result = this.getArgument(1) }
+
+  override ObjectExpr getOptions() { result = this.getArgument(2) }
+}

.github/codeql/queries/progress-not-cancellable.ql

@@ -0,0 +1,20 @@
+/**
+ * @name Using token for non-cancellable progress bar
+ * @kind problem
+ * @problem.severity warning
+ * @id vscode-codeql/progress-not-cancellable
+ * @description If we call `withProgress` without `cancellable: true` then the
+ * token that is given to us should be ignored because it won't ever be cancelled.
+ * This makes the code more confusing as it tries to account for cases that can't
+ * happen. The fix is to either not use the token or make the progress bar cancellable.
+ */
+
+import javascript
+import ProgressBar
+
+from ProgressBar t
+where not t.isCancellable() and t.usesToken()
+select t,
+  "The $@ should not be used when the progress bar is not cancellable. Either stop using the $@ or mark the progress bar as cancellable.",
+  t.getTokenParameter(), t.getTokenParameter().getName(), t.getTokenParameter(),
+  t.getTokenParameter().getName()

.github/codeql/queries/token-not-used.ql

@@ -0,0 +1,18 @@
+/**
+ * @name Don't ignore the token for a cancellable progress bar
+ * @kind problem
+ * @problem.severity warning
+ * @id vscode-codeql/token-not-used
+ * @description If we call `withProgress` with `cancellable: true` but then
+ * ignore the token that is given to us, it will lead to a poor user experience
+ * because the progress bar will appear to be canceled but it will not actually
+ * affect the background process. Either check the token and respect when it
+ * has been cancelled, or mark the progress bar as not cancellable.
+ */
+
+import javascript
+import ProgressBar
+
+from ProgressBar t
+where t.isCancellable() and not t.usesToken()
+select t, "This progress bar is $@ but the token is not used. Either use the token or mark the progress bar as not cancellable.", t.getCancellableProperty(), "cancellable"