JS: Remove parenthesized expressions from AST

Author: asgerf

javascript/extractor/src/com/semmle/js/extractor/ASTExtractor.java

@@ -1473,9 +1473,18 @@ public Label visit(SequenceExpression nd, Context c) {
 
     @Override
     public Label visit(ParenthesizedExpression nd, Context c) {
-      Label key = super.visit(nd, c);
-      visit(nd.getExpression(), key, 0, IdContext.VAR_BIND);
-      return key;
+      // Bypass the parenthesized expression node: visit the inner expression
+      // with the parent's context so it takes the place of the ParExpr in the tree.
+      // Count nested parentheses so that ((x)) results in depth 2.
+      int depth = 1;
+      Expression inner = nd.getExpression();
+      while (inner instanceof ParenthesizedExpression) {
+        depth++;
+        inner = ((ParenthesizedExpression) inner).getExpression();
+      }
+      Label innerLabel = inner.accept(this, c);
+      trapwriter.addTuple("has_parentheses", innerLabel, depth);
+      return innerLabel;
     }
 
     @Override

javascript/extractor/src/com/semmle/js/extractor/CFGExtractor.java

@@ -537,6 +537,11 @@ public Node visit(XMLDotDotExpression nd, Void c) {
       return nd.getLeft().accept(this, c);
     }
 
+    @Override
+    public Node visit(ParenthesizedExpression nd, Void c) {
+      return nd.getExpression().accept(this, c);
+    }
+
     public static Node of(Node nd) {
       return nd.accept(new First(), null);
     }
@@ -1299,7 +1304,7 @@ public Void visit(ExpressionStatement nd, SuccessorInfo i) {
 
     @Override
     public Void visit(ParenthesizedExpression nd, SuccessorInfo i) {
-      writeSuccessor(nd, First.of(nd.getExpression()));
+      // Bypass parenthesized expression - it has no DB entry, so just delegate to the inner expression
       return nd.getExpression().accept(this, i);
     }
 

javascript/ql/lib/Expressions/ExprHasNoEffect.qll

@@ -24,9 +24,6 @@ predicate inVoidContext(Expr e) {
     )
   )
   or
-  // propagate void context through parenthesized expressions
-  inVoidContext(e.getParent().(ParExpr))
-  or
   exists(SeqExpr seq, int i, int n |
     e = seq.getOperand(i) and
     n = seq.getNumOperands()
@@ -146,8 +143,6 @@ predicate isCompoundExpression(Expr e) {
   e instanceof LogicalBinaryExpr
   or
   e instanceof SeqExpr
-  or
-  e instanceof ParExpr
 }
 
 /**
@@ -180,8 +175,8 @@ predicate hasNoEffect(Expr e) {
     not exists(fe.getName())
   ) and
   // exclude block-level flow type annotations. For example: `(name: empty)`.
-  not exists(ParExpr parent |
-    e.getParent() = parent and
+  not (
+    e.isParenthesized() and
     e.getLastToken().getNextToken().getValue() = ":"
   ) and
   // exclude expressions that are part of a conditional expression

javascript/ql/lib/semmle/javascript/Expr.qll

@@ -45,8 +45,6 @@ class ExprOrType extends @expr_or_type, Documentable {
       exists(DotExpr dot | this = dot.getProperty() | result = dot.getDocumentation())
       or
       exists(AssignExpr e | this = e.getRhs() | result = e.getDocumentation())
-      or
-      exists(ParExpr p | this = p.getExpression() | result = p.getDocumentation())
     )
   }
 
@@ -109,6 +107,12 @@ class Expr extends @expr, ExprOrStmt, ExprOrType, AST::ValueNode {
   /** Gets this expression, with any surrounding parentheses removed. */
   override Expr stripParens() { result = this }
 
+  /** Holds if this expression is wrapped in parentheses. */
+  predicate isParenthesized() { has_parentheses(this, _) }
+
+  /** Gets the number of enclosing parentheses around this expression. */
+  int getParenthesisDepth() { has_parentheses(this, result) }
+
   /** Gets the constant integer value this expression evaluates to, if any. */
   int getIntValue() { none() }
 
@@ -235,9 +239,6 @@ class Expr extends @expr, ExprOrStmt, ExprOrType, AST::ValueNode {
       this = ctx.(ForOfStmt).getIterationDomain()
       or
       // recursive cases
-      this = ctx.(ParExpr).getExpression() and
-      ctx.(ParExpr).inNullSensitiveContext()
-      or
       this = ctx.(SeqExpr).getLastOperand() and
       ctx.(SeqExpr).inNullSensitiveContext()
       or
@@ -351,6 +352,11 @@ class Literal extends @literal, Expr {
 /**
  * A parenthesized expression.
  *
+ * Note: in new databases, parenthesized expressions are not represented as separate
+ * AST nodes. Instead, the inner expression takes the place of the parenthesized
+ * expression and `Expr.isParenthesized()` indicates whether it was wrapped in
+ * parentheses. This class is retained for backwards compatibility with old databases.
+ *
  * Example:
  *
  * ```

javascript/ql/lib/semmle/javascript/dataflow/DataFlow.qll

@@ -1641,8 +1641,6 @@ module DataFlow {
     exists(Expr predExpr, Expr succExpr |
       pred = TValueNode(predExpr) and succ = TValueNode(succExpr)
     |
-      predExpr = succExpr.(ParExpr).getExpression()
-      or
       predExpr = succExpr.(SeqExpr).getLastOperand()
       or
       predExpr = succExpr.(AssignExpr).getRhs()

javascript/ql/lib/semmle/javascript/dataflow/Refinements.qll

@@ -154,20 +154,6 @@ private class VariableRefinement extends RefinementCandidate, VarUse {
   }
 }
 
-/** A parenthesized refinement expression. */
-private class ParRefinement extends RefinementCandidate, ParExpr {
-  ParRefinement() { this.getExpression() instanceof RefinementCandidate }
-
-  override SsaSourceVariable getARefinedVar() {
-    result = this.getExpression().(RefinementCandidate).getARefinedVar()
-  }
-
-  overlay[global]
-  override RefinementValue eval(RefinementContext ctxt) {
-    result = this.getExpression().(RefinementCandidate).eval(ctxt)
-  }
-}
-
 /** A `typeof` refinement expression. */
 private class TypeofRefinement extends RefinementCandidate, TypeofExpr {
   TypeofRefinement() { this.getOperand() instanceof RefinementCandidate }

javascript/ql/lib/semmle/javascript/internal/NameResolution.qll

@@ -147,11 +147,6 @@ module NameResolution {
       node2 = type
     )
     or
-    exists(ParenthesisExpr expr |
-      node1 = expr.getExpression() and
-      node2 = expr
-    )
-    or
     exists(NonNullAssertion assertion |
       // For the time being we don't use this for nullness analysis, so just
       // propagate through these assertions.

javascript/ql/lib/semmlecode.javascript.dbscheme

@@ -202,6 +202,9 @@ array_size (unique int ae: @arraylike ref,
 
 is_delegating (int yield: @yield_expr ref);
 
+has_parentheses (unique int expr: @expr ref,
+       int depth: int ref);
+
 @expr_or_stmt = @expr | @stmt;
 @expr_or_type = @expr | @typeexpr;
 @expr_parent = @expr_or_stmt | @property | @function_typeexpr;

javascript/ql/lib/semmlecode.javascript.dbscheme.stats

@@ -7969,6 +7969,21 @@
 <dependencies/>
 </relation>
 <relation>
+<name>has_parentheses</name>
+<cardinality>1</cardinality>
+<columnsizes>
+<e>
+<k>expr</k>
+<v>1</v>
+</e>
+<e>
+<k>depth</k>
+<v>1</v>
+</e>
+</columnsizes>
+<dependencies/>
+</relation>
+<relation>
 <name>expr_contains_template_tag_location</name>
 <cardinality>31</cardinality>
 <columnsizes>

javascript/ql/src/Expressions/CompareIdenticalValues.ql

@@ -17,15 +17,13 @@
 import Clones
 
 /**
- * Holds if `e` is a reference to variable `v`, possibly with parentheses or
+ * Holds if `e` is a reference to variable `v`, possibly with
  * numeric conversions (that is, the unary operators `+` or `-` or a call to `Number`)
  * applied.
  */
 predicate accessWithConversions(Expr e, Variable v) {
   e = v.getAnAccess()
   or
-  accessWithConversions(e.(ParExpr).getExpression(), v)
-  or
   exists(UnaryExpr ue | ue instanceof NegExpr or ue instanceof PlusExpr |
     ue = e and accessWithConversions(ue.getOperand(), v)
   )