JavaScript: Introduce new library predicate for computing whitespace around binary operators.

change-notes/1.19/analysis-javascript.md

@@ -28,5 +28,6 @@
 | Remote property injection | Fewer results | The precision of this rule has been revised to "medium". Results are no longer shown on LGTM by default. |
 | Missing CSRF middleware | Fewer false-positive results | This rule now recognizes additional CSRF protection middlewares. |
 | Server-side URL redirect | More results | This rule now recognizes redirection calls in more cases. |
+| Whitespace contradicts operator precedence | Fewer false-positive results | This rule no longer flags operators with asymmetric whitespace. |
 
 ## Changes to QL libraries

javascript/ql/src/Expressions/WhitespaceContradictsPrecedence.ql

@@ -54,29 +54,6 @@ class HarmlessNestedExpr extends BinaryExpr {
   }
 }
 
-/** Holds if the right operand of `expr` starts on line `line`, at column `col`. */
-predicate startOfBinaryRhs(BinaryExpr expr, int line, int col) {
-  exists(Location rloc | rloc = expr.getRightOperand().getLocation() |
-    rloc.getStartLine() = line and rloc.getStartColumn() = col
-  )
-}
-
-/** Holds if the left operand of `expr` ends on line `line`, at column `col`. */
-predicate endOfBinaryLhs(BinaryExpr expr, int line, int col) {
-  exists(Location lloc | lloc = expr.getLeftOperand().getLocation() |
-    lloc.getEndLine() = line and lloc.getEndColumn() = col
-  )
-}
-
-/** Gets the number of whitespace characters around the operator of `expr`. */
-int operatorWS(BinaryExpr expr) {
-  exists(int line, int lcol, int rcol |
-    endOfBinaryLhs(expr, line, lcol) and
-    startOfBinaryRhs(expr, line, rcol) and
-    result = rcol - lcol + 1 - expr.getOperator().length()
-  )
-}
-
 /**
  * Holds if `inner` is an operand of `outer`, and the relative precedence
  * may not be immediately clear, but is important for the semantics of
@@ -88,10 +65,8 @@ predicate interestingNesting(BinaryExpr inner, BinaryExpr outer) {
   not inner instanceof HarmlessNestedExpr
 }
 
-from BinaryExpr inner, BinaryExpr outer, int wsouter, int wsinner
+from BinaryExpr inner, BinaryExpr outer
 where interestingNesting(inner, outer) and
-      wsinner = operatorWS(inner) and wsouter = operatorWS(outer) and
-      wsinner % 2 = 0 and wsouter % 2 = 0 and
-      wsinner > wsouter and
+      inner.getWhitespaceAroundOperator() > outer.getWhitespaceAroundOperator() and
       not outer.getTopLevel().isMinified()
 select outer, "Whitespace around nested operators contradicts precedence."

javascript/ql/src/semmle/javascript/Expr.qll

@@ -1008,6 +1008,25 @@ class BinaryExpr extends @binaryexpr, Expr {
   override ControlFlowNode getFirstControlFlowNode() {
     result = getLeftOperand().getFirstControlFlowNode()
   }
+
+  /**
+   * Gets the number of whitespace characters around the operator of this expression.
+   *
+   * This predicate is only defined if both operands are on the same line, and if the
+   * amount of whitespace before and after the operator are the same.
+   */
+  int getWhitespaceAroundOperator() {
+    exists (Token lastLeft, Token operator, Token firstRight, int l, int c1, int c2, int c3, int c4 |
+      lastLeft = getLeftOperand().getLastToken() and
+      operator = lastLeft.getNextToken() and
+      firstRight = operator.getNextToken() and
+      lastLeft.getLocation().hasLocationInfo(_, _, _, l, c1) and
+      operator.getLocation().hasLocationInfo(_, l, c2, l, c3) and
+      firstRight.getLocation().hasLocationInfo(_, l, c4, _, _) and
+      result = c2-c1-1 and
+      result = c4-c3-1
+    )
+  }
 }
 
 /**

javascript/ql/test/query-tests/Expressions/WhitespaceContradictsPrecedence/WhitespaceContradictsPrecedence.expected

@@ -1,2 +1,3 @@
 | tst.js:2:9:2:16 | x + x>>1 | Whitespace around nested operators contradicts precedence. |
 | tst.js:42:9:42:20 | p in o&&o[p] | Whitespace around nested operators contradicts precedence. |
+| tst.js:49:1:49:12 | x  +  x >> 1 | Whitespace around nested operators contradicts precedence. |

javascript/ql/test/query-tests/Expressions/WhitespaceContradictsPrecedence/tst.js

@@ -44,3 +44,9 @@ function ok10(o, p) {
 
 // OK
 x==y ** 2;
+
+// NOT OK
+x  +  x >> 1
+
+// OK
+x +   x >> 1