JS: Remove parenthesized expressions from AST

Author: asgerf

javascript/downgrades/462ace12909fd8fe7abf468003dba4457c91ad76/old.dbscheme

@@ -0,0 +1,2 @@
+description: Remove parenthesized expression from AST, add has_parentheses relation
+compatibility: full

javascript/downgrades/462ace12909fd8fe7abf468003dba4457c91ad76/semmlecode.javascript.dbscheme

@@ -1473,9 +1473,11 @@ 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.
+      Label innerLabel = nd.getExpression().accept(this, c);
+      trapwriter.addTuple("has_parentheses", innerLabel);
+      return innerLabel;
     }
 
     @Override

javascript/downgrades/462ace12909fd8fe7abf468003dba4457c91ad76/upgrade.properties

@@ -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/extractor/src/com/semmle/js/extractor/ASTExtractor.java

@@ -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/extractor/src/com/semmle/js/extractor/CFGExtractor.java

@@ -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,9 @@ 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 constant integer value this expression evaluates to, if any. */
   int getIntValue() { none() }
 
@@ -235,9 +236,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 +349,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/Expressions/ExprHasNoEffect.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/Expr.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/dataflow/DataFlow.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.