From c2fb14640e66aaf0b97b9a39d424071e5fa92806 Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Thu, 27 Sep 2018 15:01:46 +0200
Subject: [PATCH 01/20] JS: move isDefensiveInit to DefensiveProgramming.qll

---
 .../ql/src/Statements/UselessConditional.ql   | 26 ++----------------
 .../javascript/DefensiveProgramming.qll       | 27 +++++++++++++++++++
 2 files changed, 29 insertions(+), 24 deletions(-)
 create mode 100644 javascript/ql/src/semmle/javascript/DefensiveProgramming.qll

diff --git a/javascript/ql/src/Statements/UselessConditional.ql b/javascript/ql/src/Statements/UselessConditional.ql
index 6e71d6827134..5de56e292eec 100644
--- a/javascript/ql/src/Statements/UselessConditional.ql
+++ b/javascript/ql/src/Statements/UselessConditional.ql
@@ -15,29 +15,7 @@
 import javascript
 import semmle.javascript.RestrictedLocations
 import semmle.javascript.dataflow.Refinements
-
-/**
- * Holds if `va` is a defensive truthiness check that may be worth keeping, even if it
- * is strictly speaking useless.
- *
- * We currently recognize three patterns:
- *
- *   - the first `x` in `x || (x = e)`
- *   - the second `x` in `x = (x || e)`
- *   - the second `x` in `var x = x || e`
- */
-predicate isDefensiveInit(VarAccess va) {
-  exists (LogOrExpr o, VarRef va2 |
-    va = o.getLeftOperand().getUnderlyingReference() and va2.getVariable() = va.getVariable() |
-    exists (AssignExpr assgn | va2 = assgn.getTarget() |
-      assgn = o.getRightOperand().stripParens() or
-      o = assgn.getRhs().getUnderlyingValue()
-    ) or
-    exists (VariableDeclarator vd | va2 = vd.getBindingPattern() |
-      o = vd.getInit().getUnderlyingValue()
-    )
-  )
-}
+import semmle.javascript.DefensiveProgramming
 
 /**
  * Holds if variable `v` looks like a symbolic constant, that is, it is assigned
@@ -109,7 +87,7 @@ predicate isConstantBooleanReturnValue(Expr e) {
 predicate whitelist(Expr e) {
   isConstant(e) or
   isConstant(e.(LogNotExpr).getOperand()) or
-  isDefensiveInit(e) or
+  e.flow() instanceof DefensiveInit or
   isInitialParameterUse(e) or
   isConstantBooleanReturnValue(e)
 }
diff --git a/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll b/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
new file mode 100644
index 000000000000..2bd4f80bede1
--- /dev/null
+++ b/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
@@ -0,0 +1,27 @@
+import javascript
+
+/**
+ * A defensive truthiness check that may be worth keeping, even if it
+ * is strictly speaking useless.
+ *
+ * We currently recognize three patterns:
+ *
+ *   - the first `x` in `x || (x = e)`
+ *   - the second `x` in `x = (x || e)`
+ *   - the second `x` in `var x = x || e`
+ */
+class DefensiveInit extends DataFlow::ValueNode {
+  DefensiveInit() {
+    exists(VarAccess va, LogOrExpr o, VarRef va2 |
+      va = astNode and
+      va = o.getLeftOperand().stripParens() and va2.getVariable() = va.getVariable() |
+      exists(AssignExpr assgn | va2 = assgn.getTarget() |
+        assgn = o.getRightOperand().stripParens() or
+        o = assgn.getRhs().stripParens()
+      )
+      or
+      exists(VariableDeclarator vd | va2 = vd.getBindingPattern() | o = vd.getInit().stripParens())
+    )
+  }
+
+}
\ No newline at end of file

From a5eeba3c3a4996e9f76b3b008f754685c92948a6 Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Wed, 10 Oct 2018 14:45:06 +0200
Subject: [PATCH 02/20] JS: prepare DefensiveProgramming.qll for additions

---
 .../ql/src/Statements/UselessConditional.ql   |  2 +-
 .../javascript/DefensiveProgramming.qll       | 60 +++++++++++++------
 2 files changed, 42 insertions(+), 20 deletions(-)

diff --git a/javascript/ql/src/Statements/UselessConditional.ql b/javascript/ql/src/Statements/UselessConditional.ql
index 5de56e292eec..06e6e8601b91 100644
--- a/javascript/ql/src/Statements/UselessConditional.ql
+++ b/javascript/ql/src/Statements/UselessConditional.ql
@@ -87,7 +87,7 @@ predicate isConstantBooleanReturnValue(Expr e) {
 predicate whitelist(Expr e) {
   isConstant(e) or
   isConstant(e.(LogNotExpr).getOperand()) or
-  e.flow() instanceof DefensiveInit or
+  e.flow() instanceof Internal::DefensiveInit or
   isInitialParameterUse(e) or
   isConstantBooleanReturnValue(e)
 }
diff --git a/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll b/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
index 2bd4f80bede1..a14988661081 100644
--- a/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
+++ b/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
@@ -1,27 +1,49 @@
+/**
+ * Provides classes for working with defensive programming patterns.
+ */
+
 import javascript
+private import semmle.javascript.dataflow.InferredTypes
 
 /**
- * A defensive truthiness check that may be worth keeping, even if it
- * is strictly speaking useless.
- *
- * We currently recognize three patterns:
- *
- *   - the first `x` in `x || (x = e)`
- *   - the second `x` in `x = (x || e)`
- *   - the second `x` in `var x = x || e`
+ * The test in a defensive programming pattern.
  */
-class DefensiveInit extends DataFlow::ValueNode {
-  DefensiveInit() {
-    exists(VarAccess va, LogOrExpr o, VarRef va2 |
-      va = astNode and
-      va = o.getLeftOperand().stripParens() and va2.getVariable() = va.getVariable() |
-      exists(AssignExpr assgn | va2 = assgn.getTarget() |
-        assgn = o.getRightOperand().stripParens() or
-        o = assgn.getRhs().stripParens()
+abstract class DefensiveExpression extends DataFlow::ValueNode {
+  /** Gets the unique Boolean value that this test evaluates to, if any. */
+  abstract boolean getTheTestResult();
+}
+
+/**
+ * INTERNAL: Do not use directly; use `DefensiveExpression` instead.
+ */
+module Internal {
+  /**
+   * A defensive truthiness check that may be worth keeping, even if it
+   * is strictly speaking useless.
+   *
+   * We currently recognize three patterns:
+   *
+   *   - the first `x` in `x || (x = e)`
+   *   - the second `x` in `x = (x || e)`
+   *   - the second `x` in `var x = x || e`
+   */
+  class DefensiveInit extends DefensiveExpression {
+    DefensiveInit() {
+      exists(VarAccess va, LogOrExpr o, VarRef va2 |
+        va = astNode and
+        va = o.getLeftOperand().stripParens() and va2.getVariable() = va.getVariable() |
+        exists(AssignExpr assgn | va2 = assgn.getTarget() |
+          assgn = o.getRightOperand().stripParens() or
+          o = assgn.getRhs().stripParens()
+        )
+        or
+        exists(VariableDeclarator vd | va2 = vd.getBindingPattern() | o = vd.getInit().stripParens())
       )
-      or
-      exists(VariableDeclarator vd | va2 = vd.getBindingPattern() | o = vd.getInit().stripParens())
-    )
+    }
+
+    override boolean getTheTestResult() {
+      result = analyze().getTheBooleanValue()
+    }
   }
 
 }
\ No newline at end of file

From 8086e88587d5b9c333b4f988862c881796f16e85 Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Wed, 10 Oct 2018 14:53:26 +0200
Subject: [PATCH 03/20] JS: add utilities to DefensiveProgramming.qll

---
 javascript/ql/src/semmle/javascript/DefensiveProgramming.qll | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll b/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
index a14988661081..9c4a975dbcc5 100644
--- a/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
+++ b/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
@@ -6,7 +6,7 @@ import javascript
 private import semmle.javascript.dataflow.InferredTypes
 
 /**
- * The test in a defensive programming pattern.
+ * A test in a defensive programming pattern.
  */
 abstract class DefensiveExpression extends DataFlow::ValueNode {
   /** Gets the unique Boolean value that this test evaluates to, if any. */

From 2b6ef24bc2ce74436ef5fc9eedd80e8642550109 Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Wed, 10 Oct 2018 14:53:26 +0200
Subject: [PATCH 04/20] JS: add utilities to DefensiveProgramming.qll

---
 .../javascript/DefensiveProgramming.qll       | 82 +++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll b/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
index 9c4a975dbcc5..7c6baa87256e 100644
--- a/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
+++ b/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
@@ -46,4 +46,86 @@ module Internal {
     }
   }
 
+  /**
+   * Gets the inner expression of `e`, with any surrounding parentheses and boolean nots removed.
+   * `polarity` is true iff the inner expression is nested in an even number of negations.
+   */
+  private Expr stripNotsAndParens(Expr e, boolean polarity) {
+    exists (Expr inner |
+      inner = e.stripParens() |
+      if inner instanceof LogNotExpr then
+        (result = stripNotsAndParens(inner.(LogNotExpr).getOperand(), polarity.booleanNot()))
+      else
+        (result = inner and polarity = true)
+    )
+  }
+
+  /**
+   * An equality test for `null` and `undefined`.
+   */
+  private abstract class UndefinedNullTest extends EqualityTest {
+    /** Gets the unique Boolean value that this test evaluates to, if any. */
+    abstract boolean getTheTestResult();
+
+    /**
+     * Gets the expression that is tested for being `null` or `undefined`.
+     */
+    abstract Expr getOperand();
+  }
+
+  /**
+   * A dis- or conjunction that tests if an expression is `null` or `undefined` in either branch.
+   */
+  private class CompositeUndefinedNullTestPart extends DefensiveExpression {
+
+    UndefinedNullTest test;
+
+    boolean polarity;
+
+    CompositeUndefinedNullTestPart(){
+      exists (BinaryExpr composite, Variable v, Expr op, Expr opOther, UndefinedNullTest testOther |
+        composite instanceof LogAndExpr or
+        composite instanceof LogOrExpr |
+        composite.hasOperands(op, opOther) and
+        this = op.flow() and
+        test = stripNotsAndParens(op, polarity) and
+        testOther = stripNotsAndParens(opOther, _) and
+        test.getOperand().(VarRef).getVariable() = v and
+        testOther.getOperand().(VarRef).getVariable() = v
+      )
+    }
+
+    override boolean getTheTestResult() {
+      polarity = true and result = test.getTheTestResult()
+      or
+      polarity = false and result = test.getTheTestResult().booleanNot()
+    }
+
+  }
+
+  /**
+   * A test for `undefined` or `null` in an if-statement.
+   */
+  private class SanityCheckingUndefinedNullGuard extends DefensiveExpression {
+
+    UndefinedNullTest test;
+
+    boolean polarity;
+
+    SanityCheckingUndefinedNullGuard() {
+      exists (IfStmt c |
+        this = c.getCondition().flow() and
+        test = stripNotsAndParens(c.getCondition(), polarity) and
+        test.getOperand() instanceof VarRef
+      )
+    }
+
+    override boolean getTheTestResult() {
+      polarity = true and result = test.getTheTestResult()
+      or
+      polarity = false and result = test.getTheTestResult().booleanNot()
+    }
+
+  }
+
 }
\ No newline at end of file

From a2ecf408789bbe9e37ae30452c9f766fd479b4ef Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Wed, 10 Oct 2018 14:58:10 +0200
Subject: [PATCH 05/20] JS: recognize defensive expressions for null/undefined

---
 .../javascript/DefensiveProgramming.qll       | 56 +++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll b/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
index 7c6baa87256e..515f03d45b0e 100644
--- a/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
+++ b/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
@@ -128,4 +128,60 @@ module Internal {
 
   }
 
+  /**
+   * Holds if `t` is `null` or `undefined`.
+   */
+  private predicate isNullOrUndefined(InferredType t) {
+    t = TTNull() or
+    t = TTUndefined()
+  }
+
+  /**
+   * Holds if `t` is not `null` or `undefined`.
+   */
+  private predicate isNotNullOrUndefined(InferredType t) {
+    not isNullOrUndefined(t)
+  }
+
+  /**
+   * A value comparison for `null` and `undefined`.
+   */
+  private class NullUndefinedComparison extends UndefinedNullTest {
+
+    Expr operand;
+
+    InferredType op2type;
+
+    NullUndefinedComparison() {
+      exists (Expr op2 |
+        hasOperands(operand, op2) |
+        op2type = TTNull() and SyntacticConstants::isNull(op2)
+        or
+        op2type = TTUndefined() and SyntacticConstants::isUndefined(op2)
+      )
+    }
+
+    override boolean getTheTestResult() {
+      result = getPolarity() and
+      (
+        if this instanceof StrictEqualityTest then
+          operand.analyze().getTheType() = op2type
+        else
+          not isNotNullOrUndefined(operand.analyze().getAType())
+      )
+      or
+      result = getPolarity().booleanNot() and
+      (
+        if this instanceof StrictEqualityTest then
+          not operand.analyze().getAType() = op2type
+        else
+          not isNullOrUndefined(operand.analyze().getAType())
+      )
+    }
+
+    override Expr getOperand() {
+      result = operand
+    }
+  }
+
 }
\ No newline at end of file

From 6e77489a3b24dc2ae80aa4d7f3daa9646ff1115c Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Wed, 10 Oct 2018 15:00:21 +0200
Subject: [PATCH 06/20] JS: add utilities for expression guards to
 DefensiveProgramming.qll

---
 .../javascript/DefensiveProgramming.qll       | 66 +++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll b/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
index 515f03d45b0e..2d07a01f2177 100644
--- a/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
+++ b/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
@@ -184,4 +184,70 @@ module Internal {
     }
   }
 
+  /**
+   * An expression that throws an exception if one of its subexpressions evaluates to `null` or `undefined`.
+   */
+  private class UndefinedNullCrashUse extends Expr {
+
+    Expr target;
+
+    UndefinedNullCrashUse() {
+      this.(InvokeExpr).getCallee().stripParens() = target
+      or
+      this.(PropAccess).getBase().stripParens() = target
+      or
+      this.(MethodCallExpr).getReceiver().stripParens() = target
+    }
+
+    /**
+     * Gets the subexpression that will cause an exception to be thrown if it is `null` or `undefined`.
+     */
+    Expr getVulnerableSubexpression() {
+      result = target
+    }
+
+  }
+
+  /**
+   * An expression that throws an exception if one of its subexpressions is not a `function`.
+   */
+  private class NonFunctionCallCrashUse extends Expr {
+
+    Expr target;
+
+    NonFunctionCallCrashUse() {
+      this.(InvokeExpr).getCallee().stripParens() = target
+    }
+
+    /**
+     * Gets the subexpression that will cause an exception to be thrown if it is not a `function`.
+     */
+    Expr getVulnerableSubexpression() {
+      result = target
+    }
+
+  }
+
+  /**
+   * Gets the first expression that is guarded by `guard`.
+   */
+  private Expr getAGuardedExpr(Expr guard) {
+    exists(BinaryExpr op |
+      op.getLeftOperand() = guard and
+      (op instanceof LogAndExpr or op instanceof LogOrExpr) and
+      op.getRightOperand() = result
+    )
+    or
+    exists(IfStmt c |
+      c.getCondition() = guard |
+      result = c.getAControlledStmt().getChildExpr(0) or
+      result = c.getAControlledStmt().(BlockStmt).getStmt(0).getChildExpr(0)
+    )
+    or
+    exists (ConditionalExpr c |
+      c.getCondition() = guard |
+      result = c.getABranch()
+    )
+  }
+
 }
\ No newline at end of file

From c403416fef86ccca0322993d57ac86c20437139d Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Wed, 10 Oct 2018 15:02:42 +0200
Subject: [PATCH 07/20] JS: recognize defensive expressions that prevents
 exceptions

---
 .../javascript/DefensiveProgramming.qll       | 67 +++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll b/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
index 2d07a01f2177..e0f659fa186f 100644
--- a/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
+++ b/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
@@ -250,4 +250,71 @@ module Internal {
     )
   }
 
+  /**
+   * Holds if `t` is `string`, `number` or `boolean`.
+   */
+  private predicate isStringOrNumOrBool(InferredType t) {
+    t = TTString() or
+    t = TTNumber() or
+    t = TTBoolean()
+  }
+
+  /**
+   * A defensive expression that tests for `undefined` and `null` using a truthiness test.
+   */
+  private class UndefinedNullTruthinessGuard extends DefensiveExpression {
+
+    VarRef guardVar;
+
+    boolean polarity;
+
+    UndefinedNullTruthinessGuard() {
+      exists (VarRef useVar |
+        guardVar = stripNotsAndParens(this.asExpr(), polarity) and
+        guardVar.getVariable() = useVar.getVariable() |
+        getAGuardedExpr(this.asExpr()).stripParens().(UndefinedNullCrashUse).getVulnerableSubexpression() = useVar and
+        // exclude types whose truthiness depend on the value
+        not isStringOrNumOrBool(guardVar.analyze().getAType())
+      )
+    }
+
+    override boolean getTheTestResult() {
+      exists (boolean testResult |
+        testResult = guardVar.analyze().getTheBooleanValue() |
+        if polarity = true then
+          result = testResult
+        else
+          result = testResult.booleanNot()
+      )
+    }
+
+  }
+
+  /**
+   * A defensive expression that tests for `undefined` and `null`.
+   */
+  private class UndefinedNullTypeGuard extends DefensiveExpression {
+
+    UndefinedNullTest test;
+
+    boolean polarity;
+
+    UndefinedNullTypeGuard() {
+      exists (Expr guard, VarRef guardVar, VarRef useVar |
+        this = guard.flow() and
+        test = stripNotsAndParens(guard, polarity) and
+        test.getOperand() = guardVar and
+        guardVar.getVariable() = useVar.getVariable() |
+        getAGuardedExpr(guard).stripParens().(UndefinedNullCrashUse).getVulnerableSubexpression() = useVar
+      )
+    }
+
+    override boolean getTheTestResult() {
+      polarity = true and result = test.getTheTestResult()
+      or
+      polarity = false and result = test.getTheTestResult().booleanNot()
+    }
+
+  }
+
 }
\ No newline at end of file

From 7b215ecb2b621de8499914dd2d5a0787df40fe2a Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Wed, 10 Oct 2018 15:10:36 +0200
Subject: [PATCH 08/20] JS: recognize defensive programming patterns using
 `typeof`

---
 .../javascript/DefensiveProgramming.qll       | 92 +++++++++++++++++++
 1 file changed, 92 insertions(+)

diff --git a/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll b/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
index e0f659fa186f..ffa4246e1285 100644
--- a/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
+++ b/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
@@ -317,4 +317,96 @@ module Internal {
 
   }
 
+  /**
+   * A test for the value of a `typeof` expression.
+   */
+  private class TypeofTest extends EqualityTest {
+    Expr operand;
+
+    TypeofTag tag;
+
+    TypeofTest() {
+      exists (Expr op1, Expr op2 |
+        hasOperands(op1, op2) |
+        operand = op1.(TypeofExpr).getOperand() and
+        op2.mayHaveStringValue(tag)
+      )
+    }
+
+    boolean getTheTestResult() {
+      exists (boolean testResult |
+        testResult = true and operand.analyze().getTheType().getTypeofTag() = tag or
+        testResult = false and not operand.analyze().getAType().getTypeofTag() = tag |
+        if getPolarity() = true then
+          result = testResult
+        else
+          result = testResult.booleanNot()
+      )
+    }
+
+    /**
+     * Gets the operand used in the `typeof` expression.
+     */
+    Expr getOperand() {
+      result = operand
+    }
+
+    /**
+     * Gets the `typeof` tag that is tested.
+     */
+    TypeofTag getTag() {
+      result = tag
+    }
+  }
+
+  /**
+   * A defensive expression that tests if an expression has type `function`.
+   */
+  private class FunctionTypeGuard extends DefensiveExpression {
+
+    TypeofTest test;
+
+    boolean polarity;
+
+    FunctionTypeGuard() {
+      exists (Expr guard, VarRef guardVar, VarRef useVar |
+        this = guard.flow() and
+        test = stripNotsAndParens(guard, polarity) and
+        test.getOperand() = guardVar and
+        guardVar.getVariable() = useVar.getVariable() |
+        getAGuardedExpr(guard).stripParens().(NonFunctionCallCrashUse).getVulnerableSubexpression() = useVar
+      ) and
+      test.getTag() = "function"
+    }
+
+    override boolean getTheTestResult() {
+      polarity = true and result = test.getTheTestResult()
+      or
+      polarity = false and result = test.getTheTestResult().booleanNot()
+    }
+
+  }
+
+  /**
+   * A test for `undefined` using a `typeof` expression.
+   */
+  private class TypeofUndefinedTest extends UndefinedNullTest {
+
+    TypeofTest test;
+
+    TypeofUndefinedTest() {
+      this = test and
+      test.getTag() = "undefined"
+    }
+
+    override boolean getTheTestResult() {
+      result = test.getTheTestResult()
+    }
+
+    override Expr getOperand() {
+      result = test.getOperand()
+    }
+
+  }
+
 }
\ No newline at end of file

From b073fcfca25ea55c638c90c30dc955fc38509bd8 Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Wed, 10 Oct 2018 14:34:58 +0200
Subject: [PATCH 09/20] JS: add query: js/useless-defensive-code

---
 .../suites/javascript/maintainability-more    |   1 +
 .../UselessDefensiveProgramming.qhelp         |  91 +++++++++
 .../UselessDefensiveProgramming.ql            |  19 ++
 .../UselessDefensiveProgramming1_bad.js       |  10 +
 .../UselessDefensiveProgramming1_good.js      |   9 +
 .../UselessDefensiveProgramming2_bad.js       |   5 +
 .../UselessDefensiveProgramming2_good.js      |   5 +
 .../UselessDefensiveProgramming.expected      |  78 ++++++++
 .../UselessDefensiveProgramming.qlref         |   1 +
 .../global-module-definition.js               |  11 ++
 .../module-environment-detection.js           |  24 +++
 .../UselessDefensiveProgramming/tst.js        | 174 ++++++++++++++++++
 .../UselessDefensiveProgramming/tst2.js       |  10 +
 13 files changed, 438 insertions(+)
 create mode 100644 javascript/ql/src/Expressions/UselessDefensiveProgramming.qhelp
 create mode 100644 javascript/ql/src/Expressions/UselessDefensiveProgramming.ql
 create mode 100644 javascript/ql/src/Expressions/examples/UselessDefensiveProgramming1_bad.js
 create mode 100644 javascript/ql/src/Expressions/examples/UselessDefensiveProgramming1_good.js
 create mode 100644 javascript/ql/src/Expressions/examples/UselessDefensiveProgramming2_bad.js
 create mode 100644 javascript/ql/src/Expressions/examples/UselessDefensiveProgramming2_good.js
 create mode 100644 javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessDefensiveProgramming.expected
 create mode 100644 javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessDefensiveProgramming.qlref
 create mode 100644 javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/global-module-definition.js
 create mode 100644 javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/module-environment-detection.js
 create mode 100644 javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/tst.js
 create mode 100644 javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/tst2.js

diff --git a/javascript/config/suites/javascript/maintainability-more b/javascript/config/suites/javascript/maintainability-more
index 6cf17736f532..822a055b2e7b 100644
--- a/javascript/config/suites/javascript/maintainability-more
+++ b/javascript/config/suites/javascript/maintainability-more
@@ -6,6 +6,7 @@
 + semmlecode-javascript-queries/Declarations/DuplicateVarDecl.ql: /Maintainability/Declarations
 + semmlecode-javascript-queries/Declarations/UnusedParameter.ql: /Maintainability/Declarations
 + semmlecode-javascript-queries/Declarations/UnusedVariable.ql: /Maintainability/Declarations
++ semmlecode-javascript-queries/Expressions/UselessDefensiveProgramming.ql: /Maintainability/Expressions
 + semmlecode-javascript-queries/LanguageFeatures/ArgumentsCallerCallee.ql: /Maintainability/Language Features
 + semmlecode-javascript-queries/LanguageFeatures/ConditionalComments.ql: /Maintainability/Language Features
 + semmlecode-javascript-queries/LanguageFeatures/Eval.ql: /Maintainability/Language Features
diff --git a/javascript/ql/src/Expressions/UselessDefensiveProgramming.qhelp b/javascript/ql/src/Expressions/UselessDefensiveProgramming.qhelp
new file mode 100644
index 000000000000..870bc26da177
--- /dev/null
+++ b/javascript/ql/src/Expressions/UselessDefensiveProgramming.qhelp
@@ -0,0 +1,91 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+	<overview>
+		<p>
+
+			Defensive code can prevent unforeseen circumstances from
+			causing fatal program behaviors.
+
+			A common defensive code pattern is to guard
+			against dereferencing the values <code>null</code> or
+			<code>undefined</code>.
+
+			However, if the situation that some defensive code guards
+			against never can occur, then the defensive code serves no purpose and
+			it can safely be removed.
+
+		</p>
+
+	</overview>
+	<recommendation>
+
+		<p>
+
+			Examine the surrounding code to determine if the defensive
+			code is worth keeping despite providing no practical use. If it is no
+			longer needed, remove it.
+
+		</p>
+
+	</recommendation>
+	<example>
+
+		<p>
+
+			The following example shows a <code>cleanupLater</code>
+			function that asynchronously will perform a cleanup task after some
+			delay.  When the cleanup task completes, the function invokes the
+			provided callback parameter <code>cb</code>.
+
+			To prevent a crash by invoking <code>cb</code> when it
+			has the value <code>undefined</code>, defensive code guards
+			the invocation by checking if <code>cb</code> is truthy.
+
+		</p>
+
+		<sample src="examples/UselessDefensiveProgramming1_bad.js" />
+
+		<p>
+
+			However, the <code>cleanupLater</code> function is always
+			invoked with a callback argument, so the defensive code condition
+			always holds, and it is therefore not
+			required. The function can therefore be simplified to:
+
+		</p>
+
+		<sample src="examples/UselessDefensiveProgramming1_good.js" />
+
+		<p>
+
+			Guarding against the same situation multiple times is
+			another example of defensive code that provides no practical use. The
+			example below shows a function that assigns a value to a property of
+			an object, where defensive code ensures that the assigned value is not
+			<code>undefined</code> or <code>null</code>.
+
+		</p>
+
+		<sample src="examples/UselessDefensiveProgramming2_bad.js" />
+
+		<p>
+
+			However, due to coercion rules, <code>v ==
+			undefined</code> holds for both the situation where <code>v</code>
+			is<code>undefined</code> and the situation where <code>v</code>
+			is<code>null</code>, so the <code>v == null</code>
+			guard serves no purpose, and it can be removed:
+
+		</p>
+
+		<sample src="examples/UselessDefensiveProgramming2_good.js" />
+
+	</example>
+	<references>
+
+		<li>Wikipedia: <a href="https://en.wikipedia.org/wiki/Defensive_programming">Defensive programming</a>.</li>
+
+	</references>
+</qhelp>
diff --git a/javascript/ql/src/Expressions/UselessDefensiveProgramming.ql b/javascript/ql/src/Expressions/UselessDefensiveProgramming.ql
new file mode 100644
index 000000000000..9dd76ffec130
--- /dev/null
+++ b/javascript/ql/src/Expressions/UselessDefensiveProgramming.ql
@@ -0,0 +1,19 @@
+/**
+ * @name Useless defensive code
+ * @description If the situation some defensive code guards against never
+ *              happens, then the defensive code is not needed.
+ * @kind problem
+ * @problem.severity recommendation
+ * @id js/useless-defensive-code
+ * @tags correctness
+ *       external/cwe/cwe-570
+ *       external/cwe/cwe-571
+ * @precision very-high
+ */
+
+import javascript
+import semmle.javascript.DefensiveProgramming
+
+from DefensiveExpression e, boolean cv
+where e.getTheTestResult() = cv
+select e, "This guard always evaluates to " + cv + "."
diff --git a/javascript/ql/src/Expressions/examples/UselessDefensiveProgramming1_bad.js b/javascript/ql/src/Expressions/examples/UselessDefensiveProgramming1_bad.js
new file mode 100644
index 000000000000..d4816efdabb3
--- /dev/null
+++ b/javascript/ql/src/Expressions/examples/UselessDefensiveProgramming1_bad.js
@@ -0,0 +1,10 @@
+function cleanupLater(delay, cb) {
+    setTimeout(function() {
+        cleanup();
+        if (cb) { // BAD: useless check, `cb` is always truthy
+            cb();
+        }
+    }, delay)
+}
+
+cleanupLater(1000, function(){console.log("Cleanup done")});
diff --git a/javascript/ql/src/Expressions/examples/UselessDefensiveProgramming1_good.js b/javascript/ql/src/Expressions/examples/UselessDefensiveProgramming1_good.js
new file mode 100644
index 000000000000..3a67bd563f88
--- /dev/null
+++ b/javascript/ql/src/Expressions/examples/UselessDefensiveProgramming1_good.js
@@ -0,0 +1,9 @@
+function cleanupLater(delay, cb) {
+    setTimeout(function() {
+        cleanupNow();
+        // GOOD: no need to guard the invocation
+        cb();
+    }, delay)
+}
+
+cleanupLater(function(){console.log("Cleanup done")});
diff --git a/javascript/ql/src/Expressions/examples/UselessDefensiveProgramming2_bad.js b/javascript/ql/src/Expressions/examples/UselessDefensiveProgramming2_bad.js
new file mode 100644
index 000000000000..087a19497c9d
--- /dev/null
+++ b/javascript/ql/src/Expressions/examples/UselessDefensiveProgramming2_bad.js
@@ -0,0 +1,5 @@
+function setSafeStringProp(o, prop, v) {
+    // BAD: `v == null` is useless
+    var safe = v == undefined || v == null? '': v;
+    o[prop] = safe;
+}
diff --git a/javascript/ql/src/Expressions/examples/UselessDefensiveProgramming2_good.js b/javascript/ql/src/Expressions/examples/UselessDefensiveProgramming2_good.js
new file mode 100644
index 000000000000..80734c2391ff
--- /dev/null
+++ b/javascript/ql/src/Expressions/examples/UselessDefensiveProgramming2_good.js
@@ -0,0 +1,5 @@
+function setSafeStringProp(o, prop, v) {
+    // GOOD: `v == undefined` handles both `undefined` and `null`
+    var safe = v == undefined? '': v;
+    o[prop] = safe;
+}
diff --git a/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessDefensiveProgramming.expected b/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessDefensiveProgramming.expected
new file mode 100644
index 000000000000..65a25659f1a5
--- /dev/null
+++ b/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessDefensiveProgramming.expected
@@ -0,0 +1,78 @@
+| global-module-definition.js:10:8:10:11 | Mod2 | This guard always evaluates to false. |
+| module-environment-detection.js:3:13:3:41 | typeof  ... efined' | This guard always evaluates to true. |
+| module-environment-detection.js:15:9:15:37 | typeof  ... efined' | This guard always evaluates to true. |
+| module-environment-detection.js:23:8:23:36 | typeof  ... efined' | This guard always evaluates to true. |
+| tst2.js:4:12:4:35 | typeof  ... efined" | This guard always evaluates to true. |
+| tst.js:13:10:13:11 | u_ | This guard always evaluates to false. |
+| tst.js:14:10:14:11 | n_ | This guard always evaluates to false. |
+| tst.js:15:10:15:11 | o_ | This guard always evaluates to true. |
+| tst.js:18:5:18:5 | u | This guard always evaluates to false. |
+| tst.js:19:5:19:5 | n | This guard always evaluates to false. |
+| tst.js:20:5:20:5 | o | This guard always evaluates to true. |
+| tst.js:23:5:23:5 | u | This guard always evaluates to false. |
+| tst.js:24:5:24:5 | n | This guard always evaluates to false. |
+| tst.js:25:5:25:5 | o | This guard always evaluates to true. |
+| tst.js:28:5:28:6 | !u | This guard always evaluates to true. |
+| tst.js:29:5:29:6 | !n | This guard always evaluates to true. |
+| tst.js:30:5:30:6 | !o | This guard always evaluates to false. |
+| tst.js:33:5:33:7 | !!u | This guard always evaluates to false. |
+| tst.js:34:5:34:7 | !!n | This guard always evaluates to false. |
+| tst.js:35:5:35:7 | !!o | This guard always evaluates to true. |
+| tst.js:38:5:38:18 | u != undefined | This guard always evaluates to false. |
+| tst.js:39:5:39:18 | n != undefined | This guard always evaluates to false. |
+| tst.js:40:5:40:18 | o != undefined | This guard always evaluates to true. |
+| tst.js:43:5:43:18 | u == undefined | This guard always evaluates to true. |
+| tst.js:44:5:44:18 | n == undefined | This guard always evaluates to true. |
+| tst.js:45:5:45:18 | o == undefined | This guard always evaluates to false. |
+| tst.js:48:5:48:19 | u === undefined | This guard always evaluates to true. |
+| tst.js:49:5:49:19 | n === undefined | This guard always evaluates to false. |
+| tst.js:50:5:50:19 | o === undefined | This guard always evaluates to false. |
+| tst.js:53:9:53:9 | u | This guard always evaluates to false. |
+| tst.js:56:9:56:9 | n | This guard always evaluates to false. |
+| tst.js:59:9:59:9 | o | This guard always evaluates to true. |
+| tst.js:66:5:66:5 | u | This guard always evaluates to false. |
+| tst.js:67:5:67:5 | n | This guard always evaluates to false. |
+| tst.js:68:5:68:5 | o | This guard always evaluates to true. |
+| tst.js:71:9:71:23 | u !== undefined | This guard always evaluates to false. |
+| tst.js:74:9:74:23 | n !== undefined | This guard always evaluates to true. |
+| tst.js:77:9:77:23 | o !== undefined | This guard always evaluates to true. |
+| tst.js:84:9:84:22 | u == undefined | This guard always evaluates to true. |
+| tst.js:85:9:85:22 | n == undefined | This guard always evaluates to true. |
+| tst.js:86:9:86:22 | o == undefined | This guard always evaluates to false. |
+| tst.js:89:9:89:22 | u != undefined | This guard always evaluates to false. |
+| tst.js:90:9:90:22 | n != undefined | This guard always evaluates to false. |
+| tst.js:91:9:91:22 | o != undefined | This guard always evaluates to true. |
+| tst.js:94:9:94:32 | typeof  ... efined" | This guard always evaluates to true. |
+| tst.js:95:9:95:32 | typeof  ... efined" | This guard always evaluates to false. |
+| tst.js:96:9:96:32 | typeof  ... efined" | This guard always evaluates to false. |
+| tst.js:100:5:100:27 | typeof  ... nction" | This guard always evaluates to true. |
+| tst.js:101:5:101:27 | typeof  ... nction" | This guard always evaluates to false. |
+| tst.js:114:5:114:15 | empty_array | This guard always evaluates to true. |
+| tst.js:115:5:115:22 | pseudo_empty_array | This guard always evaluates to true. |
+| tst.js:116:5:116:19 | non_empty_array | This guard always evaluates to true. |
+| tst.js:124:6:124:20 | u !== undefined | This guard always evaluates to false. |
+| tst.js:124:25:124:34 | u !== null | This guard always evaluates to true. |
+| tst.js:125:5:125:19 | u !== undefined | This guard always evaluates to false. |
+| tst.js:125:24:125:33 | u !== null | This guard always evaluates to true. |
+| tst.js:127:5:127:18 | u != undefined | This guard always evaluates to false. |
+| tst.js:127:23:127:31 | u != null | This guard always evaluates to false. |
+| tst.js:127:23:127:31 | u != null | This guard always evaluates to true. |
+| tst.js:128:5:128:18 | u == undefined | This guard always evaluates to true. |
+| tst.js:128:23:128:31 | u == null | This guard always evaluates to false. |
+| tst.js:128:23:128:31 | u == null | This guard always evaluates to true. |
+| tst.js:129:5:129:19 | u !== undefined | This guard always evaluates to false. |
+| tst.js:129:24:129:33 | u !== null | This guard always evaluates to true. |
+| tst.js:130:5:130:22 | !(u === undefined) | This guard always evaluates to false. |
+| tst.js:130:27:130:39 | !(u === null) | This guard always evaluates to true. |
+| tst.js:131:5:131:19 | u === undefined | This guard always evaluates to true. |
+| tst.js:131:24:131:33 | u === null | This guard always evaluates to false. |
+| tst.js:132:7:132:21 | u === undefined | This guard always evaluates to true. |
+| tst.js:132:26:132:35 | u === null | This guard always evaluates to false. |
+| tst.js:133:5:133:22 | !(u === undefined) | This guard always evaluates to false. |
+| tst.js:133:27:133:36 | u !== null | This guard always evaluates to true. |
+| tst.js:135:5:135:18 | u == undefined | This guard always evaluates to true. |
+| tst.js:135:23:135:31 | u == null | This guard always evaluates to true. |
+| tst.js:138:24:138:33 | x === null | This guard always evaluates to false. |
+| tst.js:140:13:140:22 | x === null | This guard always evaluates to false. |
+| tst.js:156:23:156:31 | x != null | This guard always evaluates to true. |
+| tst.js:158:13:158:21 | x != null | This guard always evaluates to true. |
diff --git a/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessDefensiveProgramming.qlref b/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessDefensiveProgramming.qlref
new file mode 100644
index 000000000000..9a5222a05d0e
--- /dev/null
+++ b/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessDefensiveProgramming.qlref
@@ -0,0 +1 @@
+Expressions/UselessDefensiveProgramming.ql
diff --git a/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/global-module-definition.js b/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/global-module-definition.js
new file mode 100644
index 000000000000..8c61f38d1132
--- /dev/null
+++ b/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/global-module-definition.js
@@ -0,0 +1,11 @@
+var Mod1;
+(function (Mod1) {
+    Mod1.p = 42;
+})(Mod1 || (Mod1 = {}));
+
+(function(){
+    var Mod2;
+    (function (Mod2) {
+        Mod2.p = 42;
+    })(Mod2 || (Mod2 = {})); // NOT OK
+});
diff --git a/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/module-environment-detection.js b/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/module-environment-detection.js
new file mode 100644
index 000000000000..913684d6f6aa
--- /dev/null
+++ b/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/module-environment-detection.js
@@ -0,0 +1,24 @@
+var _ = (function() {
+    if (typeof exports !== 'undefined') {
+        if (typeof module !== 'undefined' && module.exports) {
+            exports = module.exports = _;
+        }
+        exports._ = _;
+    }
+    return {
+        define: function(name, factory) {
+        }
+    };
+})(this);
+
+if (typeof exports !== 'undefined') {
+    if (typeof module !== 'undefined' && module.exports) {
+        exports = module.exports = emmet;
+    }
+    exports.emmet = emmet;
+}
+
+(function(){
+    var module;
+    if(typeof module === 'undefined'); // NOT OK
+});
diff --git a/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/tst.js b/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/tst.js
new file mode 100644
index 000000000000..4993c1e32c00
--- /dev/null
+++ b/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/tst.js
@@ -0,0 +1,174 @@
+(function(){
+    var v;
+    var u = undefined;
+    var n = null;
+    var o = {};
+    var x = functionWithUnknownReturnValue();
+
+    var u_ = u;
+    var n_ = n;
+    var o_ = o;
+    var x_ = x;
+
+    u_ = u_ || e;
+    n_ = n_ || e;
+    o_ = o_ || e;
+    x_ = x_ || e;
+
+    u && u.p;
+    n && n.p;
+    o && o.p;
+    x && x.p;
+
+    u && u();
+    n && n();
+    o && o();
+    x && x();
+
+    !u || u.p;
+    !n || n.p;
+    !o || o.p;
+    !x || x.p;
+
+    !!u && u.p;
+    !!n && n.p;
+    !!o && o.p;
+    !!x && x.p;
+
+    u != undefined && u.p;
+    n != undefined && n.p;
+    o != undefined && o.p;
+    x != undefined && x.p;
+
+    u == undefined || u.p;
+    n == undefined || n.p;
+    o == undefined || o.p;
+    x == undefined || x.p;
+
+    u === undefined || u.p;
+    n === undefined || n.p;
+    o === undefined || o.p; // T, D
+    x === undefined || x.p;
+
+    if (u) {
+        u.p;
+    }
+    if (n) {
+        n.p;
+    }
+    if (o) {
+        o.p;
+    }
+    if (x) {
+        x.p;
+    }
+
+    u? u():_;
+    n? n(): _;
+    o? o(): _;
+    x? x(): _;
+
+    if (u !== undefined) {
+        u.p;
+    }
+    if (n !== undefined) {
+        n.p;
+    }
+    if (o !== undefined) {
+        o.p;
+    }
+    if (x !== undefined) {
+        x.p;
+    }
+
+    if (u == undefined){}
+    if (n == undefined){}
+    if (o == undefined){}
+    if (x == undefined){}
+
+    if (u != undefined){}
+    if (n != undefined){}
+    if (o != undefined){}
+    if (x != undefined){}
+
+    if (typeof u === "undefined"){}
+    if (typeof n === "undefined"){}
+    if (typeof o === "undefined"){}
+    if (typeof x === "undefined"){}
+
+    function f() { }
+    typeof f === "function" && f();
+    typeof u === "function" && u();
+    typeof x === "function" && x();
+
+    var empty_array = [];
+    var pseudo_empty_array = [''];
+    var non_empty_array = ['foo'];
+    var empty_string = "";
+    var non_empty_string = "foo";
+    var zero = 0;
+    var neg = -1;
+    var _true = true;
+    var _false = false;
+
+    empty_array && empty_array.pop();
+    pseudo_empty_array && pseudo_empty_array.pop();
+    non_empty_array && non_empty_array.pop();
+    empty_string && empty_string.charAt(0);
+    non_empty_string && non_empty_string.charAt(0);
+    zero && zero();
+    neg && neg();
+    _true && _true();
+    _false && _false();D
+
+    (u !== undefined && u !== null) && u.p;
+    u !== undefined && u !== null && u.p;
+
+    u != undefined && u != null;
+    u == undefined || u == null;
+    u !== undefined && u !== null;
+    !(u === undefined) && !(u === null);
+    u === undefined || u === null;
+    !(u === undefined || u === null);
+    !(u === undefined) && u !== null;
+    u !== undefined && n !== null;
+    u == undefined && u == null;
+    x == undefined && x == null;
+
+    x === undefined && x === null;
+    if (x === undefined) {
+        if (x === null) {
+        }
+    }
+
+    x !== undefined && x !== null;
+    if (x !== undefined) {
+        if (x !== null) {
+        }
+    }
+
+    x == undefined && x == null;
+    if (x == undefined) {
+        if (x == null) {
+        }
+    }
+
+    x != undefined && x != null;
+    if (x != undefined) {
+        if (x != null) {
+        }
+    }
+
+    if (typeof x !== undefined);
+    if (typeof window !== undefined);
+    if (typeof x !== x);
+    if (typeof x !== u);
+
+    if (typeof window !== "undefined");
+    if (typeof module !== "undefined");
+    if (typeof global !== "undefined");
+
+    if (typeof window !== "undefined" && window.document);
+    if (typeof module !== "undefined" && module.exports);
+    if (typeof global !== "undefined" && global.process);
+});
diff --git a/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/tst2.js b/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/tst2.js
new file mode 100644
index 000000000000..4c6ad02a6bfb
--- /dev/null
+++ b/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/tst2.js
@@ -0,0 +1,10 @@
+(function(){
+    var v;
+    (function(){
+        if(typeof v === "undefined"){ // NOT OK
+            v = 42;
+        }
+        for(var v in x){
+        }
+    });
+});

From e29c57a58ee1387b30ccf04af13122594e9f89a6 Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Wed, 10 Oct 2018 14:37:36 +0200
Subject: [PATCH 10/20] JS: add whitelist to js/useless-defensive-code

---
 .../Expressions/UselessDefensiveProgramming.ql    | 15 ++++++++++++++-
 .../semmle/javascript/DefensiveProgramming.qll    |  2 +-
 .../UselessDefensiveProgramming.expected          |  6 ------
 3 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/javascript/ql/src/Expressions/UselessDefensiveProgramming.ql b/javascript/ql/src/Expressions/UselessDefensiveProgramming.ql
index 9dd76ffec130..3ca0a5fdafbc 100644
--- a/javascript/ql/src/Expressions/UselessDefensiveProgramming.ql
+++ b/javascript/ql/src/Expressions/UselessDefensiveProgramming.ql
@@ -15,5 +15,18 @@ import javascript
 import semmle.javascript.DefensiveProgramming
 
 from DefensiveExpression e, boolean cv
-where e.getTheTestResult() = cv
+where e.getTheTestResult() = cv and
+      // whitelist
+      not (
+        // module environment detection
+        exists (VarAccess access, string name |
+          name = "exports" or name = "module" |
+          e.asExpr().(Internal::TypeofUndefinedTest).getOperand() = access and
+          access.getName() = name and
+          not exists (access.getVariable().getADeclaration())
+        )
+        or
+        // too benign in practice
+        e instanceof Internal::DefensiveInit
+      )
 select e, "This guard always evaluates to " + cv + "."
diff --git a/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll b/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
index ffa4246e1285..3de4f2121ef6 100644
--- a/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
+++ b/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
@@ -390,7 +390,7 @@ module Internal {
   /**
    * A test for `undefined` using a `typeof` expression.
    */
-  private class TypeofUndefinedTest extends UndefinedNullTest {
+  class TypeofUndefinedTest extends UndefinedNullTest {
 
     TypeofTest test;
 
diff --git a/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessDefensiveProgramming.expected b/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessDefensiveProgramming.expected
index 65a25659f1a5..f726f0a0b5df 100644
--- a/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessDefensiveProgramming.expected
+++ b/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessDefensiveProgramming.expected
@@ -1,11 +1,5 @@
-| global-module-definition.js:10:8:10:11 | Mod2 | This guard always evaluates to false. |
-| module-environment-detection.js:3:13:3:41 | typeof  ... efined' | This guard always evaluates to true. |
-| module-environment-detection.js:15:9:15:37 | typeof  ... efined' | This guard always evaluates to true. |
 | module-environment-detection.js:23:8:23:36 | typeof  ... efined' | This guard always evaluates to true. |
 | tst2.js:4:12:4:35 | typeof  ... efined" | This guard always evaluates to true. |
-| tst.js:13:10:13:11 | u_ | This guard always evaluates to false. |
-| tst.js:14:10:14:11 | n_ | This guard always evaluates to false. |
-| tst.js:15:10:15:11 | o_ | This guard always evaluates to true. |
 | tst.js:18:5:18:5 | u | This guard always evaluates to false. |
 | tst.js:19:5:19:5 | n | This guard always evaluates to false. |
 | tst.js:20:5:20:5 | o | This guard always evaluates to true. |

From 358e6188d98ed43449a29a0c54d3da9c31546b5c Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Wed, 10 Oct 2018 12:52:52 +0200
Subject: [PATCH 11/20] JS: downgrade other alerts to js/useless-defensive-code

---
 javascript/ql/src/Expressions/HeterogeneousComparison.ql       | 2 ++
 javascript/ql/src/Statements/UselessConditional.ql             | 2 +-
 .../HeterogeneousComparison.expected                           | 3 +++
 .../UselessDefensiveProgramming/HeterogeneousComparison.qlref  | 1 +
 .../UselessDefensiveProgramming/UselessConditional.expected    | 0
 .../UselessDefensiveProgramming/UselessConditional.qlref       | 1 +
 6 files changed, 8 insertions(+), 1 deletion(-)
 create mode 100644 javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/HeterogeneousComparison.expected
 create mode 100644 javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/HeterogeneousComparison.qlref
 create mode 100644 javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessConditional.expected
 create mode 100644 javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessConditional.qlref

diff --git a/javascript/ql/src/Expressions/HeterogeneousComparison.ql b/javascript/ql/src/Expressions/HeterogeneousComparison.ql
index fd524466ddf4..6e20e3acaa4c 100644
--- a/javascript/ql/src/Expressions/HeterogeneousComparison.ql
+++ b/javascript/ql/src/Expressions/HeterogeneousComparison.ql
@@ -15,6 +15,7 @@
 
 import javascript
 private import semmle.javascript.dataflow.InferredTypes
+private import semmle.javascript.DefensiveProgramming
 
 /**
  * Holds if `left` and `right` are the left and right operands, respectively, of `nd`, which is
@@ -198,6 +199,7 @@ from ASTNode cmp,
      int leftTypeCount, int rightTypeCount ,
      string leftTypeDescription, string rightTypeDescription
 where isHeterogeneousComparison(cmp, left, right, leftTypes, rightTypes) and
+      not exists (cmp.(Expr).flow().(DefensiveExpression).getTheTestResult()) and
       not whitelist(left.asExpr()) and
       not whitelist(right.asExpr()) and
       leftExprDescription = capitalize(getDescription(left.asExpr(), "this expression")) and
diff --git a/javascript/ql/src/Statements/UselessConditional.ql b/javascript/ql/src/Statements/UselessConditional.ql
index 06e6e8601b91..d4dbebbb6f86 100644
--- a/javascript/ql/src/Statements/UselessConditional.ql
+++ b/javascript/ql/src/Statements/UselessConditional.ql
@@ -87,7 +87,7 @@ predicate isConstantBooleanReturnValue(Expr e) {
 predicate whitelist(Expr e) {
   isConstant(e) or
   isConstant(e.(LogNotExpr).getOperand()) or
-  e.flow() instanceof Internal::DefensiveInit or
+  exists (e.flow().(DefensiveExpression).getTheTestResult()) or
   isInitialParameterUse(e) or
   isConstantBooleanReturnValue(e)
 }
diff --git a/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/HeterogeneousComparison.expected b/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/HeterogeneousComparison.expected
new file mode 100644
index 000000000000..534048b0d15f
--- /dev/null
+++ b/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/HeterogeneousComparison.expected
@@ -0,0 +1,3 @@
+| tst.js:162:9:162:16 | typeof x | This expression is of type string, but it is compared to $@ of type undefined. | tst.js:162:22:162:30 | undefined | 'undefined' |
+| tst.js:163:9:163:21 | typeof window | This expression is of type string, but it is compared to $@ of type undefined. | tst.js:163:27:163:35 | undefined | 'undefined' |
+| tst.js:165:9:165:16 | typeof x | This expression is of type string, but it is compared to $@ of type undefined. | tst.js:165:22:165:22 | u | variable 'u' |
diff --git a/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/HeterogeneousComparison.qlref b/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/HeterogeneousComparison.qlref
new file mode 100644
index 000000000000..13b0e2a181cb
--- /dev/null
+++ b/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/HeterogeneousComparison.qlref
@@ -0,0 +1 @@
+Expressions/HeterogeneousComparison.ql
\ No newline at end of file
diff --git a/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessConditional.expected b/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessConditional.expected
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessConditional.qlref b/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessConditional.qlref
new file mode 100644
index 000000000000..d29916245d6e
--- /dev/null
+++ b/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessConditional.qlref
@@ -0,0 +1 @@
+Statements/UselessConditional.ql
\ No newline at end of file

From f440c9221ac686b4e3c34de865fe946872a9f54e Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Tue, 16 Oct 2018 10:40:58 +0200
Subject: [PATCH 12/20] JS: replace some `Expr.stripParens` with
 `Expr.getUnderlyingValue`

---
 .../semmle/javascript/DefensiveProgramming.qll   | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll b/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
index 3de4f2121ef6..def68da6824f 100644
--- a/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
+++ b/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
@@ -52,7 +52,7 @@ module Internal {
    */
   private Expr stripNotsAndParens(Expr e, boolean polarity) {
     exists (Expr inner |
-      inner = e.stripParens() |
+      inner = e.getUnderlyingValue() |
       if inner instanceof LogNotExpr then
         (result = stripNotsAndParens(inner.(LogNotExpr).getOperand(), polarity.booleanNot()))
       else
@@ -192,11 +192,11 @@ module Internal {
     Expr target;
 
     UndefinedNullCrashUse() {
-      this.(InvokeExpr).getCallee().stripParens() = target
+      this.(InvokeExpr).getCallee().getUnderlyingValue() = target
       or
-      this.(PropAccess).getBase().stripParens() = target
+      this.(PropAccess).getBase().getUnderlyingValue() = target
       or
-      this.(MethodCallExpr).getReceiver().stripParens() = target
+      this.(MethodCallExpr).getReceiver().getUnderlyingValue() = target
     }
 
     /**
@@ -216,7 +216,7 @@ module Internal {
     Expr target;
 
     NonFunctionCallCrashUse() {
-      this.(InvokeExpr).getCallee().stripParens() = target
+      this.(InvokeExpr).getCallee().getUnderlyingValue() = target
     }
 
     /**
@@ -272,7 +272,7 @@ module Internal {
       exists (VarRef useVar |
         guardVar = stripNotsAndParens(this.asExpr(), polarity) and
         guardVar.getVariable() = useVar.getVariable() |
-        getAGuardedExpr(this.asExpr()).stripParens().(UndefinedNullCrashUse).getVulnerableSubexpression() = useVar and
+        getAGuardedExpr(this.asExpr()).getUnderlyingValue().(UndefinedNullCrashUse).getVulnerableSubexpression() = useVar and
         // exclude types whose truthiness depend on the value
         not isStringOrNumOrBool(guardVar.analyze().getAType())
       )
@@ -305,7 +305,7 @@ module Internal {
         test = stripNotsAndParens(guard, polarity) and
         test.getOperand() = guardVar and
         guardVar.getVariable() = useVar.getVariable() |
-        getAGuardedExpr(guard).stripParens().(UndefinedNullCrashUse).getVulnerableSubexpression() = useVar
+        getAGuardedExpr(guard).getUnderlyingValue().(UndefinedNullCrashUse).getVulnerableSubexpression() = useVar
       )
     }
 
@@ -374,7 +374,7 @@ module Internal {
         test = stripNotsAndParens(guard, polarity) and
         test.getOperand() = guardVar and
         guardVar.getVariable() = useVar.getVariable() |
-        getAGuardedExpr(guard).stripParens().(NonFunctionCallCrashUse).getVulnerableSubexpression() = useVar
+        getAGuardedExpr(guard).getUnderlyingValue().(NonFunctionCallCrashUse).getVulnerableSubexpression() = useVar
       ) and
       test.getTag() = "function"
     }

From 7d4cf495451d7138a2bf94f88709a0c9ae044d7c Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Wed, 31 Oct 2018 09:33:49 +0100
Subject: [PATCH 13/20] JS: fixup double reporting of alerts

---
 .../ql/src/Statements/UselessConditional.ql       | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/javascript/ql/src/Statements/UselessConditional.ql b/javascript/ql/src/Statements/UselessConditional.ql
index d4dbebbb6f86..e682a56b0064 100644
--- a/javascript/ql/src/Statements/UselessConditional.ql
+++ b/javascript/ql/src/Statements/UselessConditional.ql
@@ -73,6 +73,19 @@ predicate isConstantBooleanReturnValue(Expr e) {
   isConstantBooleanReturnValue(e.(LogNotExpr).getOperand())
 }
 
+/**
+ * Holds if `e` is a defensive expression with a fixed outcome.
+ */
+predicate isConstantDefensive(Expr e) {
+  exists(Expr defensive |
+    defensive = e or
+    // traverse negations
+    defensive.(LogNotExpr).getOperand+() = e
+    |
+    exists(defensive.flow().(DefensiveExpression).getTheTestResult())
+  )
+}
+
 /**
  * Holds if `e` is an expression that should not be flagged as a useless condition.
  *
@@ -87,7 +100,7 @@ predicate isConstantBooleanReturnValue(Expr e) {
 predicate whitelist(Expr e) {
   isConstant(e) or
   isConstant(e.(LogNotExpr).getOperand()) or
-  exists (e.flow().(DefensiveExpression).getTheTestResult()) or
+  isConstantDefensive(e) or // flagged by js/useless-defensive-code
   isInitialParameterUse(e) or
   isConstantBooleanReturnValue(e)
 }

From a636319c97b64264fff32eeede7929d9de5ed4e7 Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Thu, 1 Nov 2018 08:47:35 +0100
Subject: [PATCH 14/20] JS: change notes for js/useless-defensive-code

---
 change-notes/1.19/analysis-javascript.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/change-notes/1.19/analysis-javascript.md b/change-notes/1.19/analysis-javascript.md
index 5630452ded37..e79bae13009f 100644
--- a/change-notes/1.19/analysis-javascript.md
+++ b/change-notes/1.19/analysis-javascript.md
@@ -24,6 +24,7 @@
 | Stored cross-site scripting (`js/stored-xss`) | security, external/cwe/cwe-079, external/cwe/cwe-116 | Highlights uncontrolled stored values flowing into HTML content, indicating a violation of [CWE-079](https://cwe.mitre.org/data/definitions/79.html). Results shown on LGTM by default. |
 | Unclear precedence of nested operators (`js/unclear-operator-precedence`) | maintainability, correctness, external/cwe/cwe-783 | Highlights nested binary operators whose relative precedence is easy to misunderstand. Results shown on LGTM by default. |
 | Useless assignment to property | maintainability | Highlights property assignments whose value is always overwritten. Results are shown on LGTM by default. |
+| Useless defensive code | correctness, external/cwe/cwe-570, external/cwe/cwe-571 | Highlights locations where defensive code serves no purpose. Results are shown on LGTM by default. |
 | User-controlled data in file | security, external/cwe/cwe-912 | Highlights locations where user-controlled data is written to a file. Results are not shown on LGTM by default. |
 
 ## Changes to existing queries

From 8b71b25a2a05c26a1681cb660c9725a47a0e1a12 Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Thu, 1 Nov 2018 09:48:16 +0100
Subject: [PATCH 15/20] JS: annotate test file with expected results

---
 .../UselessDefensiveProgramming/tst.js        | 130 +++++++++---------
 1 file changed, 65 insertions(+), 65 deletions(-)

diff --git a/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/tst.js b/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/tst.js
index 4993c1e32c00..1e03c4e3ba1a 100644
--- a/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/tst.js
+++ b/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/tst.js
@@ -10,95 +10,95 @@
     var o_ = o;
     var x_ = x;
 
-    u_ = u_ || e;
-    n_ = n_ || e;
-    o_ = o_ || e;
+    u_ = u_ || e; // NOT OK
+    n_ = n_ || e; // NOT OK
+    o_ = o_ || e; // NOT OK
     x_ = x_ || e;
 
-    u && u.p;
-    n && n.p;
-    o && o.p;
+    u && u.p; // NOT OK
+    n && n.p; // NOT OK
+    o && o.p; // NOT OK
     x && x.p;
 
-    u && u();
-    n && n();
-    o && o();
+    u && u(); // NOT OK
+    n && n(); // NOT OK
+    o && o(); // NOT OK
     x && x();
 
-    !u || u.p;
-    !n || n.p;
-    !o || o.p;
+    !u || u.p; // NOT OK
+    !n || n.p; // NOT OK
+    !o || o.p; // NOT OK
     !x || x.p;
 
-    !!u && u.p;
-    !!n && n.p;
-    !!o && o.p;
+    !!u && u.p; // NOT OK
+    !!n && n.p; // NOT OK
+    !!o && o.p; // NOT OK
     !!x && x.p;
 
-    u != undefined && u.p;
-    n != undefined && n.p;
-    o != undefined && o.p;
+    u != undefined && u.p; // NOT OK
+    n != undefined && n.p; // NOT OK
+    o != undefined && o.p; // NOT OK
     x != undefined && x.p;
 
-    u == undefined || u.p;
-    n == undefined || n.p;
-    o == undefined || o.p;
+    u == undefined || u.p; // NOT OK
+    n == undefined || n.p; // NOT OK
+    o == undefined || o.p; // NOT OK
     x == undefined || x.p;
 
-    u === undefined || u.p;
-    n === undefined || n.p;
-    o === undefined || o.p; // T, D
+    u === undefined || u.p; // NOT OK
+    n === undefined || n.p; // NOT OK
+    o === undefined || o.p; // NOT OK
     x === undefined || x.p;
 
-    if (u) {
+    if (u) { // NOT OK
         u.p;
     }
-    if (n) {
+    if (n) { // NOT OK
         n.p;
     }
-    if (o) {
+    if (o) { // NOT OK
         o.p;
     }
     if (x) {
         x.p;
     }
 
-    u? u():_;
-    n? n(): _;
-    o? o(): _;
+    u? u():_; // NOT OK
+    n? n(): _; // NOT OK
+    o? o(): _; // NOT OK
     x? x(): _;
 
-    if (u !== undefined) {
+    if (u !== undefined) { // NOT OK
         u.p;
     }
-    if (n !== undefined) {
+    if (n !== undefined) { // NOT OK
         n.p;
     }
-    if (o !== undefined) {
+    if (o !== undefined) { // NOT OK
         o.p;
     }
     if (x !== undefined) {
         x.p;
     }
 
-    if (u == undefined){}
-    if (n == undefined){}
-    if (o == undefined){}
+    if (u == undefined){} // NOT OK
+    if (n == undefined){} // NOT OK
+    if (o == undefined){} // NOT OK
     if (x == undefined){}
 
-    if (u != undefined){}
-    if (n != undefined){}
-    if (o != undefined){}
+    if (u != undefined){} // NOT OK
+    if (n != undefined){} // NOT OK
+    if (o != undefined){} // NOT OK
     if (x != undefined){}
 
-    if (typeof u === "undefined"){}
-    if (typeof n === "undefined"){}
-    if (typeof o === "undefined"){}
+    if (typeof u === "undefined"){} // NOT OK
+    if (typeof n === "undefined"){} // NOT OK
+    if (typeof o === "undefined"){} // NOT OK
     if (typeof x === "undefined"){}
 
     function f() { }
-    typeof f === "function" && f();
-    typeof u === "function" && u();
+    typeof f === "function" && f(); // NOT OK
+    typeof u === "function" && u(); // NOT OK
     typeof x === "function" && x();
 
     var empty_array = [];
@@ -111,33 +111,33 @@
     var _true = true;
     var _false = false;
 
-    empty_array && empty_array.pop();
-    pseudo_empty_array && pseudo_empty_array.pop();
-    non_empty_array && non_empty_array.pop();
+    empty_array && empty_array.pop(); // NOT OK
+    pseudo_empty_array && pseudo_empty_array.pop(); // NOT OK
+    non_empty_array && non_empty_array.pop(); // NOT OK
     empty_string && empty_string.charAt(0);
     non_empty_string && non_empty_string.charAt(0);
     zero && zero();
     neg && neg();
     _true && _true();
-    _false && _false();D
-
-    (u !== undefined && u !== null) && u.p;
-    u !== undefined && u !== null && u.p;
-
-    u != undefined && u != null;
-    u == undefined || u == null;
-    u !== undefined && u !== null;
-    !(u === undefined) && !(u === null);
-    u === undefined || u === null;
-    !(u === undefined || u === null);
-    !(u === undefined) && u !== null;
+    _false && _false();
+
+    (u !== undefined && u !== null) && u.p; // NOT OK
+    u !== undefined && u !== null && u.p; // NOT OK
+
+    u != undefined && u != null; // NOT OK
+    u == undefined || u == null; // NOT OK
+    u !== undefined && u !== null; // NOT OK
+    !(u === undefined) && !(u === null); // NOT OK
+    u === undefined || u === null; // NOT OK
+    !(u === undefined || u === null); // NOT OK
+    !(u === undefined) && u !== null; // NOT OK
     u !== undefined && n !== null;
-    u == undefined && u == null;
+    u == undefined && u == null; // NOT OK
     x == undefined && x == null;
 
-    x === undefined && x === null;
+    x === undefined && x === null; // NOT OK
     if (x === undefined) {
-        if (x === null) {
+        if (x === null) { // NOT OK
         }
     }
 
@@ -153,16 +153,16 @@
         }
     }
 
-    x != undefined && x != null;
+    x != undefined && x != null; // NOT OK
     if (x != undefined) {
-        if (x != null) {
+        if (x != null) { // NOT OK
         }
     }
 
     if (typeof x !== undefined);
     if (typeof window !== undefined);
     if (typeof x !== x);
-    if (typeof x !== u);
+    if (typeof x !== u); // NOT OK
 
     if (typeof window !== "undefined");
     if (typeof module !== "undefined");

From 8ea9fd4ccac8415a6ec6d3e4f4a5ad56001008bb Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Mon, 5 Nov 2018 11:27:09 +0100
Subject: [PATCH 16/20] JS: address review comments

---
 .../Expressions/HeterogeneousComparison.ql    |  2 +-
 .../UselessDefensiveProgramming.ql            |  2 +-
 .../ql/src/Statements/UselessConditional.ql   |  2 +-
 .../javascript/DefensiveProgramming.qll       | 27 ++++++++++---------
 4 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/javascript/ql/src/Expressions/HeterogeneousComparison.ql b/javascript/ql/src/Expressions/HeterogeneousComparison.ql
index 6e20e3acaa4c..ae1be5a7a989 100644
--- a/javascript/ql/src/Expressions/HeterogeneousComparison.ql
+++ b/javascript/ql/src/Expressions/HeterogeneousComparison.ql
@@ -199,7 +199,7 @@ from ASTNode cmp,
      int leftTypeCount, int rightTypeCount ,
      string leftTypeDescription, string rightTypeDescription
 where isHeterogeneousComparison(cmp, left, right, leftTypes, rightTypes) and
-      not exists (cmp.(Expr).flow().(DefensiveExpression).getTheTestResult()) and
+      not exists (cmp.(Expr).flow().(DefensiveExpressionTest).getTheTestResult()) and
       not whitelist(left.asExpr()) and
       not whitelist(right.asExpr()) and
       leftExprDescription = capitalize(getDescription(left.asExpr(), "this expression")) and
diff --git a/javascript/ql/src/Expressions/UselessDefensiveProgramming.ql b/javascript/ql/src/Expressions/UselessDefensiveProgramming.ql
index 3ca0a5fdafbc..ce6dea98ecfb 100644
--- a/javascript/ql/src/Expressions/UselessDefensiveProgramming.ql
+++ b/javascript/ql/src/Expressions/UselessDefensiveProgramming.ql
@@ -14,7 +14,7 @@
 import javascript
 import semmle.javascript.DefensiveProgramming
 
-from DefensiveExpression e, boolean cv
+from DefensiveExpressionTest e, boolean cv
 where e.getTheTestResult() = cv and
       // whitelist
       not (
diff --git a/javascript/ql/src/Statements/UselessConditional.ql b/javascript/ql/src/Statements/UselessConditional.ql
index e682a56b0064..3051ee3448e7 100644
--- a/javascript/ql/src/Statements/UselessConditional.ql
+++ b/javascript/ql/src/Statements/UselessConditional.ql
@@ -82,7 +82,7 @@ predicate isConstantDefensive(Expr e) {
     // traverse negations
     defensive.(LogNotExpr).getOperand+() = e
     |
-    exists(defensive.flow().(DefensiveExpression).getTheTestResult())
+    exists(defensive.flow().(DefensiveExpressionTest).getTheTestResult())
   )
 }
 
diff --git a/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll b/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
index def68da6824f..08bfe6702c51 100644
--- a/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
+++ b/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
@@ -8,13 +8,13 @@ private import semmle.javascript.dataflow.InferredTypes
 /**
  * A test in a defensive programming pattern.
  */
-abstract class DefensiveExpression extends DataFlow::ValueNode {
+abstract class DefensiveExpressionTest extends DataFlow::ValueNode {
   /** Gets the unique Boolean value that this test evaluates to, if any. */
   abstract boolean getTheTestResult();
 }
 
 /**
- * INTERNAL: Do not use directly; use `DefensiveExpression` instead.
+ * INTERNAL: Do not use directly; use `DefensiveExpressionTest` instead.
  */
 module Internal {
   /**
@@ -27,7 +27,7 @@ module Internal {
    *   - the second `x` in `x = (x || e)`
    *   - the second `x` in `var x = x || e`
    */
-  class DefensiveInit extends DefensiveExpression {
+  class DefensiveInit extends DefensiveExpressionTest {
     DefensiveInit() {
       exists(VarAccess va, LogOrExpr o, VarRef va2 |
         va = astNode and
@@ -76,16 +76,14 @@ module Internal {
   /**
    * A dis- or conjunction that tests if an expression is `null` or `undefined` in either branch.
    */
-  private class CompositeUndefinedNullTestPart extends DefensiveExpression {
+  private class CompositeUndefinedNullTestPart extends DefensiveExpressionTest {
 
     UndefinedNullTest test;
 
     boolean polarity;
 
     CompositeUndefinedNullTestPart(){
-      exists (BinaryExpr composite, Variable v, Expr op, Expr opOther, UndefinedNullTest testOther |
-        composite instanceof LogAndExpr or
-        composite instanceof LogOrExpr |
+      exists (LogicalBinaryExpr composite, Variable v, Expr op, Expr opOther, UndefinedNullTest testOther |
         composite.hasOperands(op, opOther) and
         this = op.flow() and
         test = stripNotsAndParens(op, polarity) and
@@ -106,7 +104,7 @@ module Internal {
   /**
    * A test for `undefined` or `null` in an if-statement.
    */
-  private class SanityCheckingUndefinedNullGuard extends DefensiveExpression {
+  private class SanityCheckingUndefinedNullGuard extends DefensiveExpressionTest {
 
     UndefinedNullTest test;
 
@@ -165,16 +163,20 @@ module Internal {
       result = getPolarity() and
       (
         if this instanceof StrictEqualityTest then
+          // case: `operand === null` or `operand === undefined`
           operand.analyze().getTheType() = op2type
         else
+          // case: `operand == null` or `operand == undefined`
           not isNotNullOrUndefined(operand.analyze().getAType())
       )
       or
       result = getPolarity().booleanNot() and
       (
         if this instanceof StrictEqualityTest then
+          // case: `operand !== null` or `operand !== undefined`
           not operand.analyze().getAType() = op2type
         else
+          // case: `operand != null` or `operand != undefined`
           not isNullOrUndefined(operand.analyze().getAType())
       )
     }
@@ -232,9 +234,8 @@ module Internal {
    * Gets the first expression that is guarded by `guard`.
    */
   private Expr getAGuardedExpr(Expr guard) {
-    exists(BinaryExpr op |
+    exists(LogicalBinaryExpr op |
       op.getLeftOperand() = guard and
-      (op instanceof LogAndExpr or op instanceof LogOrExpr) and
       op.getRightOperand() = result
     )
     or
@@ -262,7 +263,7 @@ module Internal {
   /**
    * A defensive expression that tests for `undefined` and `null` using a truthiness test.
    */
-  private class UndefinedNullTruthinessGuard extends DefensiveExpression {
+  private class UndefinedNullTruthinessGuard extends DefensiveExpressionTest {
 
     VarRef guardVar;
 
@@ -293,7 +294,7 @@ module Internal {
   /**
    * A defensive expression that tests for `undefined` and `null`.
    */
-  private class UndefinedNullTypeGuard extends DefensiveExpression {
+  private class UndefinedNullTypeGuard extends DefensiveExpressionTest {
 
     UndefinedNullTest test;
 
@@ -362,7 +363,7 @@ module Internal {
   /**
    * A defensive expression that tests if an expression has type `function`.
    */
-  private class FunctionTypeGuard extends DefensiveExpression {
+  private class FunctionTypeGuard extends DefensiveExpressionTest {
 
     TypeofTest test;
 

From 15123da0b7f728f7ecc327f7d4d809159347ea32 Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Mon, 5 Nov 2018 11:33:14 +0100
Subject: [PATCH 17/20] JS: minor fixup: only traverse `LogNotExpr`s

---
 javascript/ql/src/Statements/UselessConditional.ql | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/javascript/ql/src/Statements/UselessConditional.ql b/javascript/ql/src/Statements/UselessConditional.ql
index 3051ee3448e7..27b703f652ba 100644
--- a/javascript/ql/src/Statements/UselessConditional.ql
+++ b/javascript/ql/src/Statements/UselessConditional.ql
@@ -73,16 +73,18 @@ predicate isConstantBooleanReturnValue(Expr e) {
   isConstantBooleanReturnValue(e.(LogNotExpr).getOperand())
 }
 
+private Expr maybeStripLogNot(Expr e) {
+  result = maybeStripLogNot(e.(LogNotExpr).getOperand()) or
+  result = e
+}
+
 /**
  * Holds if `e` is a defensive expression with a fixed outcome.
  */
 predicate isConstantDefensive(Expr e) {
-  exists(Expr defensive |
-    defensive = e or
-    // traverse negations
-    defensive.(LogNotExpr).getOperand+() = e
-    |
-    exists(defensive.flow().(DefensiveExpressionTest).getTheTestResult())
+  exists(DefensiveExpressionTest defensive |
+    maybeStripLogNot(defensive.asExpr()) = e and
+    exists(defensive.getTheTestResult())
   )
 }
 

From 3aae1d17dbcb3d7b8b42806279d331bf39f19894 Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Mon, 5 Nov 2018 11:34:34 +0100
Subject: [PATCH 18/20] JS: avoid two uses of `getChildExpr(0)`

---
 .../ql/src/semmle/javascript/DefensiveProgramming.qll    | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll b/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
index 08bfe6702c51..6b224fe586b2 100644
--- a/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
+++ b/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
@@ -239,10 +239,11 @@ module Internal {
       op.getRightOperand() = result
     )
     or
-    exists(IfStmt c |
-      c.getCondition() = guard |
-      result = c.getAControlledStmt().getChildExpr(0) or
-      result = c.getAControlledStmt().(BlockStmt).getStmt(0).getChildExpr(0)
+    exists(IfStmt c, ExprStmt guardedStmt |
+      c.getCondition() = guard and
+      result = guardedStmt.getExpr() |
+      guardedStmt = c.getAControlledStmt() or
+      guardedStmt = c.getAControlledStmt().(BlockStmt).getStmt(0)
     )
     or
     exists (ConditionalExpr c |

From 1db2e6ca55777501ae048696782b28f321a411e9 Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Mon, 5 Nov 2018 11:49:54 +0100
Subject: [PATCH 19/20] JS: add source code examples to docstrings

---
 .../javascript/DefensiveProgramming.qll       | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll b/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
index 6b224fe586b2..6529566495b6 100644
--- a/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
+++ b/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
@@ -62,6 +62,8 @@ module Internal {
 
   /**
    * An equality test for `null` and `undefined`.
+   *
+   * Examples: `e === undefined` or `typeof e !== undefined`.
    */
   private abstract class UndefinedNullTest extends EqualityTest {
     /** Gets the unique Boolean value that this test evaluates to, if any. */
@@ -75,6 +77,8 @@ module Internal {
 
   /**
    * A dis- or conjunction that tests if an expression is `null` or `undefined` in either branch.
+   *
+   * Example: a branch in `x === null || x === undefined`.
    */
   private class CompositeUndefinedNullTestPart extends DefensiveExpressionTest {
 
@@ -103,6 +107,8 @@ module Internal {
 
   /**
    * A test for `undefined` or `null` in an if-statement.
+   *
+   * Example: `if (x === null) ...`.
    */
   private class SanityCheckingUndefinedNullGuard extends DefensiveExpressionTest {
 
@@ -143,6 +149,8 @@ module Internal {
 
   /**
    * A value comparison for `null` and `undefined`.
+   *
+   * Examples: `x === null` or `x != undefined`.
    */
   private class NullUndefinedComparison extends UndefinedNullTest {
 
@@ -188,6 +196,8 @@ module Internal {
 
   /**
    * An expression that throws an exception if one of its subexpressions evaluates to `null` or `undefined`.
+   *
+   * Examples: `sub.p` or `sub()`.
    */
   private class UndefinedNullCrashUse extends Expr {
 
@@ -212,6 +222,8 @@ module Internal {
 
   /**
    * An expression that throws an exception if one of its subexpressions is not a `function`.
+   *
+   * Example: `sub()`.
    */
   private class NonFunctionCallCrashUse extends Expr {
 
@@ -263,6 +275,8 @@ module Internal {
 
   /**
    * A defensive expression that tests for `undefined` and `null` using a truthiness test.
+   *
+   * Examples: The condition in `if(x) { x.p; }` or `!x || x.m()`.
    */
   private class UndefinedNullTruthinessGuard extends DefensiveExpressionTest {
 
@@ -294,6 +308,8 @@ module Internal {
 
   /**
    * A defensive expression that tests for `undefined` and `null`.
+   *
+   * Example: the condition in `if(x !== null) { x.p; }`.
    */
   private class UndefinedNullTypeGuard extends DefensiveExpressionTest {
 
@@ -321,6 +337,8 @@ module Internal {
 
   /**
    * A test for the value of a `typeof` expression.
+   *
+   * Example: `typeof x === 'undefined'`.
    */
   private class TypeofTest extends EqualityTest {
     Expr operand;
@@ -363,6 +381,8 @@ module Internal {
 
   /**
    * A defensive expression that tests if an expression has type `function`.
+   *
+   * Example: the condition in `if(typeof x === 'function') x()`.
    */
   private class FunctionTypeGuard extends DefensiveExpressionTest {
 
@@ -391,6 +411,8 @@ module Internal {
 
   /**
    * A test for `undefined` using a `typeof` expression.
+   *
+   * Example: `typeof x === undefined'.
    */
   class TypeofUndefinedTest extends UndefinedNullTest {
 

From 5666deac14bee439f529cc238767af74f3851427 Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esben@semmle.com>
Date: Thu, 8 Nov 2018 12:08:32 +0100
Subject: [PATCH 20/20] JS: rename js/useless-defensive-code to
 js/unneeded-defensive-code

---
 change-notes/1.19/analysis-javascript.md             |  2 +-
 ...ming.qhelp => UnneededDefensiveProgramming.qhelp} | 12 ++++++------
 ...rogramming.ql => UnneededDefensiveProgramming.ql} |  7 +++----
 ...1_bad.js => UnneededDefensiveProgramming1_bad.js} |  0
 ...good.js => UnneededDefensiveProgramming1_good.js} |  0
 ...2_bad.js => UnneededDefensiveProgramming2_bad.js} |  0
 ...good.js => UnneededDefensiveProgramming2_good.js} |  0
 .../HeterogeneousComparison.expected                 |  0
 .../HeterogeneousComparison.qlref                    |  0
 .../UnneededDefensiveProgramming.expected}           |  0
 .../UnneededDefensiveProgramming.qlref               |  1 +
 .../UselessConditional.expected                      |  0
 .../UselessConditional.qlref                         |  0
 .../global-module-definition.js                      |  0
 .../module-environment-detection.js                  |  0
 .../tst.js                                           |  0
 .../tst2.js                                          |  0
 .../UselessDefensiveProgramming.qlref                |  1 -
 18 files changed, 11 insertions(+), 12 deletions(-)
 rename javascript/ql/src/Expressions/{UselessDefensiveProgramming.qhelp => UnneededDefensiveProgramming.qhelp} (86%)
 rename javascript/ql/src/Expressions/{UselessDefensiveProgramming.ql => UnneededDefensiveProgramming.ql} (80%)
 rename javascript/ql/src/Expressions/examples/{UselessDefensiveProgramming1_bad.js => UnneededDefensiveProgramming1_bad.js} (100%)
 rename javascript/ql/src/Expressions/examples/{UselessDefensiveProgramming1_good.js => UnneededDefensiveProgramming1_good.js} (100%)
 rename javascript/ql/src/Expressions/examples/{UselessDefensiveProgramming2_bad.js => UnneededDefensiveProgramming2_bad.js} (100%)
 rename javascript/ql/src/Expressions/examples/{UselessDefensiveProgramming2_good.js => UnneededDefensiveProgramming2_good.js} (100%)
 rename javascript/ql/test/query-tests/Expressions/{UselessDefensiveProgramming => UnneededDefensiveProgramming}/HeterogeneousComparison.expected (100%)
 rename javascript/ql/test/query-tests/Expressions/{UselessDefensiveProgramming => UnneededDefensiveProgramming}/HeterogeneousComparison.qlref (100%)
 rename javascript/ql/test/query-tests/Expressions/{UselessDefensiveProgramming/UselessDefensiveProgramming.expected => UnneededDefensiveProgramming/UnneededDefensiveProgramming.expected} (100%)
 create mode 100644 javascript/ql/test/query-tests/Expressions/UnneededDefensiveProgramming/UnneededDefensiveProgramming.qlref
 rename javascript/ql/test/query-tests/Expressions/{UselessDefensiveProgramming => UnneededDefensiveProgramming}/UselessConditional.expected (100%)
 rename javascript/ql/test/query-tests/Expressions/{UselessDefensiveProgramming => UnneededDefensiveProgramming}/UselessConditional.qlref (100%)
 rename javascript/ql/test/query-tests/Expressions/{UselessDefensiveProgramming => UnneededDefensiveProgramming}/global-module-definition.js (100%)
 rename javascript/ql/test/query-tests/Expressions/{UselessDefensiveProgramming => UnneededDefensiveProgramming}/module-environment-detection.js (100%)
 rename javascript/ql/test/query-tests/Expressions/{UselessDefensiveProgramming => UnneededDefensiveProgramming}/tst.js (100%)
 rename javascript/ql/test/query-tests/Expressions/{UselessDefensiveProgramming => UnneededDefensiveProgramming}/tst2.js (100%)
 delete mode 100644 javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessDefensiveProgramming.qlref

diff --git a/change-notes/1.19/analysis-javascript.md b/change-notes/1.19/analysis-javascript.md
index e79bae13009f..4071deb9cb9d 100644
--- a/change-notes/1.19/analysis-javascript.md
+++ b/change-notes/1.19/analysis-javascript.md
@@ -23,8 +23,8 @@
 | Replacement of a substring with itself (`js/identity-replacement`) | correctness, security, external/cwe/cwe-116 | Highlights string replacements that replace a string with itself, which usually indicates a mistake. Results shown on LGTM by default. |
 | Stored cross-site scripting (`js/stored-xss`) | security, external/cwe/cwe-079, external/cwe/cwe-116 | Highlights uncontrolled stored values flowing into HTML content, indicating a violation of [CWE-079](https://cwe.mitre.org/data/definitions/79.html). Results shown on LGTM by default. |
 | Unclear precedence of nested operators (`js/unclear-operator-precedence`) | maintainability, correctness, external/cwe/cwe-783 | Highlights nested binary operators whose relative precedence is easy to misunderstand. Results shown on LGTM by default. |
+| Unneeded defensive code | correctness, external/cwe/cwe-570, external/cwe/cwe-571 | Highlights locations where defensive code is not needed. Results are shown on LGTM by default. |
 | Useless assignment to property | maintainability | Highlights property assignments whose value is always overwritten. Results are shown on LGTM by default. |
-| Useless defensive code | correctness, external/cwe/cwe-570, external/cwe/cwe-571 | Highlights locations where defensive code serves no purpose. Results are shown on LGTM by default. |
 | User-controlled data in file | security, external/cwe/cwe-912 | Highlights locations where user-controlled data is written to a file. Results are not shown on LGTM by default. |
 
 ## Changes to existing queries
diff --git a/javascript/ql/src/Expressions/UselessDefensiveProgramming.qhelp b/javascript/ql/src/Expressions/UnneededDefensiveProgramming.qhelp
similarity index 86%
rename from javascript/ql/src/Expressions/UselessDefensiveProgramming.qhelp
rename to javascript/ql/src/Expressions/UnneededDefensiveProgramming.qhelp
index 870bc26da177..fd7191dd7fa8 100644
--- a/javascript/ql/src/Expressions/UselessDefensiveProgramming.qhelp
+++ b/javascript/ql/src/Expressions/UnneededDefensiveProgramming.qhelp
@@ -14,7 +14,7 @@
 
 			However, if the situation that some defensive code guards
 			against never can occur, then the defensive code serves no purpose and
-			it can safely be removed.
+			can safely be removed.
 
 		</p>
 
@@ -45,7 +45,7 @@
 
 		</p>
 
-		<sample src="examples/UselessDefensiveProgramming1_bad.js" />
+		<sample src="examples/UnneededDefensiveProgramming1_bad.js" />
 
 		<p>
 
@@ -56,7 +56,7 @@
 
 		</p>
 
-		<sample src="examples/UselessDefensiveProgramming1_good.js" />
+		<sample src="examples/UnneededDefensiveProgramming1_good.js" />
 
 		<p>
 
@@ -68,7 +68,7 @@
 
 		</p>
 
-		<sample src="examples/UselessDefensiveProgramming2_bad.js" />
+		<sample src="examples/UnneededDefensiveProgramming2_bad.js" />
 
 		<p>
 
@@ -76,11 +76,11 @@
 			undefined</code> holds for both the situation where <code>v</code>
 			is<code>undefined</code> and the situation where <code>v</code>
 			is<code>null</code>, so the <code>v == null</code>
-			guard serves no purpose, and it can be removed:
+			guard serves no purpose, and can be removed:
 
 		</p>
 
-		<sample src="examples/UselessDefensiveProgramming2_good.js" />
+		<sample src="examples/UnneededDefensiveProgramming2_good.js" />
 
 	</example>
 	<references>
diff --git a/javascript/ql/src/Expressions/UselessDefensiveProgramming.ql b/javascript/ql/src/Expressions/UnneededDefensiveProgramming.ql
similarity index 80%
rename from javascript/ql/src/Expressions/UselessDefensiveProgramming.ql
rename to javascript/ql/src/Expressions/UnneededDefensiveProgramming.ql
index ce6dea98ecfb..bb36d94aef33 100644
--- a/javascript/ql/src/Expressions/UselessDefensiveProgramming.ql
+++ b/javascript/ql/src/Expressions/UnneededDefensiveProgramming.ql
@@ -1,10 +1,9 @@
 /**
- * @name Useless defensive code
- * @description If the situation some defensive code guards against never
- *              happens, then the defensive code is not needed.
+ * @name Unneeded defensive code
+ * @description Defensive code that guards against a situation that never happens is not needed.
  * @kind problem
  * @problem.severity recommendation
- * @id js/useless-defensive-code
+ * @id js/unneeded-defensive-code
  * @tags correctness
  *       external/cwe/cwe-570
  *       external/cwe/cwe-571
diff --git a/javascript/ql/src/Expressions/examples/UselessDefensiveProgramming1_bad.js b/javascript/ql/src/Expressions/examples/UnneededDefensiveProgramming1_bad.js
similarity index 100%
rename from javascript/ql/src/Expressions/examples/UselessDefensiveProgramming1_bad.js
rename to javascript/ql/src/Expressions/examples/UnneededDefensiveProgramming1_bad.js
diff --git a/javascript/ql/src/Expressions/examples/UselessDefensiveProgramming1_good.js b/javascript/ql/src/Expressions/examples/UnneededDefensiveProgramming1_good.js
similarity index 100%
rename from javascript/ql/src/Expressions/examples/UselessDefensiveProgramming1_good.js
rename to javascript/ql/src/Expressions/examples/UnneededDefensiveProgramming1_good.js
diff --git a/javascript/ql/src/Expressions/examples/UselessDefensiveProgramming2_bad.js b/javascript/ql/src/Expressions/examples/UnneededDefensiveProgramming2_bad.js
similarity index 100%
rename from javascript/ql/src/Expressions/examples/UselessDefensiveProgramming2_bad.js
rename to javascript/ql/src/Expressions/examples/UnneededDefensiveProgramming2_bad.js
diff --git a/javascript/ql/src/Expressions/examples/UselessDefensiveProgramming2_good.js b/javascript/ql/src/Expressions/examples/UnneededDefensiveProgramming2_good.js
similarity index 100%
rename from javascript/ql/src/Expressions/examples/UselessDefensiveProgramming2_good.js
rename to javascript/ql/src/Expressions/examples/UnneededDefensiveProgramming2_good.js
diff --git a/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/HeterogeneousComparison.expected b/javascript/ql/test/query-tests/Expressions/UnneededDefensiveProgramming/HeterogeneousComparison.expected
similarity index 100%
rename from javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/HeterogeneousComparison.expected
rename to javascript/ql/test/query-tests/Expressions/UnneededDefensiveProgramming/HeterogeneousComparison.expected
diff --git a/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/HeterogeneousComparison.qlref b/javascript/ql/test/query-tests/Expressions/UnneededDefensiveProgramming/HeterogeneousComparison.qlref
similarity index 100%
rename from javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/HeterogeneousComparison.qlref
rename to javascript/ql/test/query-tests/Expressions/UnneededDefensiveProgramming/HeterogeneousComparison.qlref
diff --git a/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessDefensiveProgramming.expected b/javascript/ql/test/query-tests/Expressions/UnneededDefensiveProgramming/UnneededDefensiveProgramming.expected
similarity index 100%
rename from javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessDefensiveProgramming.expected
rename to javascript/ql/test/query-tests/Expressions/UnneededDefensiveProgramming/UnneededDefensiveProgramming.expected
diff --git a/javascript/ql/test/query-tests/Expressions/UnneededDefensiveProgramming/UnneededDefensiveProgramming.qlref b/javascript/ql/test/query-tests/Expressions/UnneededDefensiveProgramming/UnneededDefensiveProgramming.qlref
new file mode 100644
index 000000000000..13c05f1e74bd
--- /dev/null
+++ b/javascript/ql/test/query-tests/Expressions/UnneededDefensiveProgramming/UnneededDefensiveProgramming.qlref
@@ -0,0 +1 @@
+Expressions/UnneededDefensiveProgramming.ql
diff --git a/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessConditional.expected b/javascript/ql/test/query-tests/Expressions/UnneededDefensiveProgramming/UselessConditional.expected
similarity index 100%
rename from javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessConditional.expected
rename to javascript/ql/test/query-tests/Expressions/UnneededDefensiveProgramming/UselessConditional.expected
diff --git a/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessConditional.qlref b/javascript/ql/test/query-tests/Expressions/UnneededDefensiveProgramming/UselessConditional.qlref
similarity index 100%
rename from javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessConditional.qlref
rename to javascript/ql/test/query-tests/Expressions/UnneededDefensiveProgramming/UselessConditional.qlref
diff --git a/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/global-module-definition.js b/javascript/ql/test/query-tests/Expressions/UnneededDefensiveProgramming/global-module-definition.js
similarity index 100%
rename from javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/global-module-definition.js
rename to javascript/ql/test/query-tests/Expressions/UnneededDefensiveProgramming/global-module-definition.js
diff --git a/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/module-environment-detection.js b/javascript/ql/test/query-tests/Expressions/UnneededDefensiveProgramming/module-environment-detection.js
similarity index 100%
rename from javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/module-environment-detection.js
rename to javascript/ql/test/query-tests/Expressions/UnneededDefensiveProgramming/module-environment-detection.js
diff --git a/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/tst.js b/javascript/ql/test/query-tests/Expressions/UnneededDefensiveProgramming/tst.js
similarity index 100%
rename from javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/tst.js
rename to javascript/ql/test/query-tests/Expressions/UnneededDefensiveProgramming/tst.js
diff --git a/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/tst2.js b/javascript/ql/test/query-tests/Expressions/UnneededDefensiveProgramming/tst2.js
similarity index 100%
rename from javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/tst2.js
rename to javascript/ql/test/query-tests/Expressions/UnneededDefensiveProgramming/tst2.js
diff --git a/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessDefensiveProgramming.qlref b/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessDefensiveProgramming.qlref
deleted file mode 100644
index 9a5222a05d0e..000000000000
--- a/javascript/ql/test/query-tests/Expressions/UselessDefensiveProgramming/UselessDefensiveProgramming.qlref
+++ /dev/null
@@ -1 +0,0 @@
-Expressions/UselessDefensiveProgramming.ql