JS: Fix parsing of destructuring rest patterns

javascript/extractor/src/com/semmle/jcorn/CustomParser.java

@@ -321,7 +321,8 @@ protected ComprehensionExpression parseComprehension(
   }
 
   @Override
-  protected Expression parseParenAndDistinguishExpression(boolean canBeArrow) {
+  protected Expression parseParenAndDistinguishExpression(
+      boolean canBeArrow, DestructuringErrors refDestructuringErrors) {
     if (options.mozExtensions()) {
       // check whether next token is `for`, suggesting a generator comprehension
       Position startLoc = this.startLoc;
@@ -338,7 +339,7 @@ protected Expression parseParenAndDistinguishExpression(boolean canBeArrow) {
       }
     }
 
-    Expression res = super.parseParenAndDistinguishExpression(canBeArrow);
+    Expression res = super.parseParenAndDistinguishExpression(canBeArrow, refDestructuringErrors);
     if (res instanceof ParenthesizedExpression) {
       ParenthesizedExpression p = (ParenthesizedExpression) res;
       if (p.getExpression() instanceof ComprehensionExpression) {

javascript/extractor/src/com/semmle/jcorn/ESNextParser.java

@@ -69,10 +69,18 @@ protected Property parseProperty(
 
     Property prop = null;
     if (this.type == TokenType.ellipsis) {
-      SpreadElement spread = this.parseSpread(null);
+      DestructuringErrors errors = new DestructuringErrors();
+      SpreadElement spread = this.parseSpread(errors);
+      if (this.type == TokenType.comma) errors.recordTrailingComma(this.startLoc);
       Expression val;
-      if (isPattern) val = new RestElement(spread.getLoc(), spread.getArgument());
-      else val = spread;
+      if (isPattern) {
+        this.checkPatternErrors(errors, true);
+        val = (Expression) this.toAssignable(spread, true);
+      } else {
+        if (refDestructuringErrors != null) refDestructuringErrors.merge(errors);
+        else this.checkExpressionErrors(errors, true);
+        val = spread;
+      }
       prop =
           this.finishNode(
               new Property(
@@ -88,8 +96,14 @@ protected Property parseProperty(
 
   @Override
   protected INode toAssignable(INode node, boolean isBinding) {
-    if (node instanceof SpreadElement)
-      return new RestElement(node.getLoc(), ((SpreadElement) node).getArgument());
+    if (node instanceof SpreadElement) {
+      Expression arg =
+          (Expression) this.toAssignable(((SpreadElement) node).getArgument(), isBinding);
+      Expression target = arg.stripParens();
+      if (!(target instanceof Identifier || target instanceof MemberExpression))
+        this.unexpected(arg.getLoc().getStart());
+      return new RestElement(node.getLoc(), arg);
+    }
     return super.toAssignable(node, isBinding);
   }
 

javascript/extractor/tests/errors/output/trap/invalid-assignment-pattern.js.trap

@@ -9,111 +9,20 @@ hasLocation(#10000,#10002)
 #20000=@"global_scope"
 scopes(#20000,0)
 #20001=@"script;{#10000},1,1"
-#20002=*
-lines(#20002,#20001,"(x = 0) = y","
-")
-#20003=@"loc,{#10000},1,1,1,11"
-locations_default(#20003,#10000,1,1,1,11)
-hasLocation(#20002,#20003)
-numlines(#20001,1,1,0)
-#20004=*
-tokeninfo(#20004,8,#20001,0,"(")
-#20005=@"loc,{#10000},1,1,1,1"
-locations_default(#20005,#10000,1,1,1,1)
-hasLocation(#20004,#20005)
-#20006=*
-tokeninfo(#20006,6,#20001,1,"x")
-#20007=@"loc,{#10000},1,2,1,2"
-locations_default(#20007,#10000,1,2,1,2)
-hasLocation(#20006,#20007)
-#20008=*
-tokeninfo(#20008,8,#20001,2,"=")
-#20009=@"loc,{#10000},1,4,1,4"
-locations_default(#20009,#10000,1,4,1,4)
-hasLocation(#20008,#20009)
-#20010=*
-tokeninfo(#20010,3,#20001,3,"0")
-#20011=@"loc,{#10000},1,6,1,6"
-locations_default(#20011,#10000,1,6,1,6)
-hasLocation(#20010,#20011)
-#20012=*
-tokeninfo(#20012,8,#20001,4,")")
-#20013=@"loc,{#10000},1,7,1,7"
-locations_default(#20013,#10000,1,7,1,7)
-hasLocation(#20012,#20013)
-#20014=*
-tokeninfo(#20014,8,#20001,5,"=")
-#20015=@"loc,{#10000},1,9,1,9"
-locations_default(#20015,#10000,1,9,1,9)
-hasLocation(#20014,#20015)
-#20016=*
-tokeninfo(#20016,6,#20001,6,"y")
-#20017=@"loc,{#10000},1,11,1,11"
-locations_default(#20017,#10000,1,11,1,11)
-hasLocation(#20016,#20017)
-#20018=*
-tokeninfo(#20018,0,#20001,7,"")
-#20019=@"loc,{#10000},2,1,2,0"
-locations_default(#20019,#10000,2,1,2,0)
-hasLocation(#20018,#20019)
 toplevels(#20001,0)
-#20020=@"loc,{#10000},1,1,2,0"
-locations_default(#20020,#10000,1,1,2,0)
-hasLocation(#20001,#20020)
-#20021=@"var;{this};{#20000}"
-variables(#20021,"this",#20000)
-#20022=*
-stmts(#20022,2,#20001,0,"(x = 0) = y")
-hasLocation(#20022,#20003)
-stmt_containers(#20022,#20001)
-#20023=*
-exprs(#20023,47,#20022,0,"(x = 0) = y")
-hasLocation(#20023,#20003)
-enclosing_stmt(#20023,#20022)
-expr_containers(#20023,#20001)
-#20024=*
-exprs(#20024,63,#20023,0,"(x = 0)")
-#20025=@"loc,{#10000},1,1,1,7"
-locations_default(#20025,#10000,1,1,1,7)
-hasLocation(#20024,#20025)
-enclosing_stmt(#20024,#20022)
-expr_containers(#20024,#20001)
-#20026=*
-exprs(#20026,79,#20024,0,"x")
-hasLocation(#20026,#20007)
-enclosing_stmt(#20026,#20022)
-expr_containers(#20026,#20001)
-#20027=*
-exprs(#20027,79,#20023,1,"y")
-hasLocation(#20027,#20017)
-enclosing_stmt(#20027,#20022)
-expr_containers(#20027,#20001)
-literals("y","y",#20027)
-#20028=@"var;{y};{#20000}"
-variables(#20028,"y",#20000)
-bind(#20027,#20028)
-#20029=*
-entry_cfg_node(#20029,#20001)
-#20030=@"loc,{#10000},1,1,1,0"
-locations_default(#20030,#10000,1,1,1,0)
-hasLocation(#20029,#20030)
-#20031=*
-exit_cfg_node(#20031,#20001)
-hasLocation(#20031,#20019)
-successor(#20022,#20024)
-successor(#20027,#20023)
-successor(#20024,#20026)
-successor(#20026,#20027)
-successor(#20023,#20031)
-successor(#20029,#20022)
-#20032=*
-js_parse_errors(#20032,#20001,"Error: Unexpected assignment pattern.","(x = 0) = y
+#20002=@"loc,{#10000},1,1,1,1"
+locations_default(#20002,#10000,1,1,1,1)
+hasLocation(#20001,#20002)
+#20003=*
+js_parse_errors(#20003,#20001,"Error: Parenthesized pattern","(x = 0) = y
 ")
-hasLocation(#20032,#20007)
-#20033=*
-lines(#20033,#20001,"(x = 0) = y","
+hasLocation(#20003,#20002)
+#20004=*
+lines(#20004,#20001,"(x = 0) = y","
 ")
-hasLocation(#20033,#20003)
+#20005=@"loc,{#10000},1,1,1,11"
+locations_default(#20005,#10000,1,1,1,11)
+hasLocation(#20004,#20005)
 numlines(#20001,1,0,0)
-numlines(#10000,1,1,0)
+numlines(#10000,1,0,0)
 filetype(#10000,"javascript")

javascript/extractor/tests/es2015/output/trap/restparms2.js.trap

@@ -9,27 +9,178 @@ hasLocation(#10000,#10002)
 #20000=@"global_scope"
 scopes(#20000,0)
 #20001=@"script;{#10000},1,1"
-toplevels(#20001,0)
-#20002=@"loc,{#10000},1,1,1,1"
-locations_default(#20002,#10000,1,1,1,1)
-hasLocation(#20001,#20002)
-#20003=*
-js_parse_errors(#20003,#20001,"Error: Unexpected token","function f(x, ...[y, z]) {
-")
-#20004=@"loc,{#10000},1,18,1,18"
-locations_default(#20004,#10000,1,18,1,18)
-hasLocation(#20003,#20004)
-#20005=*
-lines(#20005,#20001,"function f(x, ...[y, z]) {","
+#20002=*
+lines(#20002,#20001,"function f(x, ...[y, z]) {","
 ")
-#20006=@"loc,{#10000},1,1,1,26"
-locations_default(#20006,#10000,1,1,1,26)
-hasLocation(#20005,#20006)
-#20007=*
-lines(#20007,#20001,"}","")
-#20008=@"loc,{#10000},2,1,2,1"
-locations_default(#20008,#10000,2,1,2,1)
-hasLocation(#20007,#20008)
-numlines(#20001,2,0,0)
-numlines(#10000,2,0,0)
+#20003=@"loc,{#10000},1,1,1,26"
+locations_default(#20003,#10000,1,1,1,26)
+hasLocation(#20002,#20003)
+#20004=*
+lines(#20004,#20001,"}","")
+#20005=@"loc,{#10000},2,1,2,1"
+locations_default(#20005,#10000,2,1,2,1)
+hasLocation(#20004,#20005)
+numlines(#20001,2,2,0)
+#20006=*
+tokeninfo(#20006,7,#20001,0,"function")
+#20007=@"loc,{#10000},1,1,1,8"
+locations_default(#20007,#10000,1,1,1,8)
+hasLocation(#20006,#20007)
+#20008=*
+tokeninfo(#20008,6,#20001,1,"f")
+#20009=@"loc,{#10000},1,10,1,10"
+locations_default(#20009,#10000,1,10,1,10)
+hasLocation(#20008,#20009)
+#20010=*
+tokeninfo(#20010,8,#20001,2,"(")
+#20011=@"loc,{#10000},1,11,1,11"
+locations_default(#20011,#10000,1,11,1,11)
+hasLocation(#20010,#20011)
+#20012=*
+tokeninfo(#20012,6,#20001,3,"x")
+#20013=@"loc,{#10000},1,12,1,12"
+locations_default(#20013,#10000,1,12,1,12)
+hasLocation(#20012,#20013)
+#20014=*
+tokeninfo(#20014,8,#20001,4,",")
+#20015=@"loc,{#10000},1,13,1,13"
+locations_default(#20015,#10000,1,13,1,13)
+hasLocation(#20014,#20015)
+#20016=*
+tokeninfo(#20016,8,#20001,5,"...")
+#20017=@"loc,{#10000},1,15,1,17"
+locations_default(#20017,#10000,1,15,1,17)
+hasLocation(#20016,#20017)
+#20018=*
+tokeninfo(#20018,8,#20001,6,"[")
+#20019=@"loc,{#10000},1,18,1,18"
+locations_default(#20019,#10000,1,18,1,18)
+hasLocation(#20018,#20019)
+#20020=*
+tokeninfo(#20020,6,#20001,7,"y")
+#20021=@"loc,{#10000},1,19,1,19"
+locations_default(#20021,#10000,1,19,1,19)
+hasLocation(#20020,#20021)
+#20022=*
+tokeninfo(#20022,8,#20001,8,",")
+#20023=@"loc,{#10000},1,20,1,20"
+locations_default(#20023,#10000,1,20,1,20)
+hasLocation(#20022,#20023)
+#20024=*
+tokeninfo(#20024,6,#20001,9,"z")
+#20025=@"loc,{#10000},1,22,1,22"
+locations_default(#20025,#10000,1,22,1,22)
+hasLocation(#20024,#20025)
+#20026=*
+tokeninfo(#20026,8,#20001,10,"]")
+#20027=@"loc,{#10000},1,23,1,23"
+locations_default(#20027,#10000,1,23,1,23)
+hasLocation(#20026,#20027)
+#20028=*
+tokeninfo(#20028,8,#20001,11,")")
+#20029=@"loc,{#10000},1,24,1,24"
+locations_default(#20029,#10000,1,24,1,24)
+hasLocation(#20028,#20029)
+#20030=*
+tokeninfo(#20030,8,#20001,12,"{")
+#20031=@"loc,{#10000},1,26,1,26"
+locations_default(#20031,#10000,1,26,1,26)
+hasLocation(#20030,#20031)
+#20032=*
+tokeninfo(#20032,8,#20001,13,"}")
+hasLocation(#20032,#20005)
+#20033=*
+tokeninfo(#20033,0,#20001,14,"")
+#20034=@"loc,{#10000},2,2,2,1"
+locations_default(#20034,#10000,2,2,2,1)
+hasLocation(#20033,#20034)
+toplevels(#20001,0)
+#20035=@"loc,{#10000},1,1,2,1"
+locations_default(#20035,#10000,1,1,2,1)
+hasLocation(#20001,#20035)
+#20036=@"var;{f};{#20000}"
+variables(#20036,"f",#20000)
+#20037=@"var;{this};{#20000}"
+variables(#20037,"this",#20000)
+#20038=*
+stmts(#20038,17,#20001,0,"functio ... z]) {\n}")
+hasLocation(#20038,#20035)
+stmt_containers(#20038,#20001)
+#20039=*
+exprs(#20039,78,#20038,-1,"f")
+hasLocation(#20039,#20009)
+expr_containers(#20039,#20038)
+literals("f","f",#20039)
+decl(#20039,#20036)
+#20040=*
+scopes(#20040,1)
+scopenodes(#20038,#20040)
+scopenesting(#20040,#20000)
+#20041=@"var;{this};{#20040}"
+variables(#20041,"this",#20040)
+#20042=@"var;{x};{#20040}"
+variables(#20042,"x",#20040)
+#20043=*
+exprs(#20043,78,#20038,0,"x")
+hasLocation(#20043,#20013)
+expr_containers(#20043,#20038)
+literals("x","x",#20043)
+decl(#20043,#20042)
+#20044=@"var;{y};{#20040}"
+variables(#20044,"y",#20040)
+#20045=@"var;{z};{#20040}"
+variables(#20045,"z",#20040)
+#20046=*
+exprs(#20046,67,#20038,1,"[y, z]")
+#20047=@"loc,{#10000},1,18,1,23"
+locations_default(#20047,#10000,1,18,1,23)
+hasLocation(#20046,#20047)
+expr_containers(#20046,#20038)
+#20048=*
+exprs(#20048,78,#20046,0,"y")
+hasLocation(#20048,#20021)
+expr_containers(#20048,#20038)
+literals("y","y",#20048)
+decl(#20048,#20044)
+#20049=*
+exprs(#20049,78,#20046,1,"z")
+hasLocation(#20049,#20025)
+expr_containers(#20049,#20038)
+literals("z","z",#20049)
+decl(#20049,#20045)
+array_size(#20046,2)
+#20050=@"var;{arguments};{#20040}"
+variables(#20050,"arguments",#20040)
+is_arguments_object(#20050)
+has_rest_parameter(#20038)
+#20051=*
+stmts(#20051,1,#20038,-2,"{\n}")
+#20052=@"loc,{#10000},1,26,2,1"
+locations_default(#20052,#10000,1,26,2,1)
+hasLocation(#20051,#20052)
+stmt_containers(#20051,#20038)
+#20053=*
+entry_cfg_node(#20053,#20001)
+#20054=@"loc,{#10000},1,1,1,0"
+locations_default(#20054,#10000,1,1,1,0)
+hasLocation(#20053,#20054)
+#20055=*
+exit_cfg_node(#20055,#20001)
+hasLocation(#20055,#20034)
+successor(#20038,#20055)
+#20056=*
+entry_cfg_node(#20056,#20038)
+hasLocation(#20056,#20054)
+#20057=*
+exit_cfg_node(#20057,#20038)
+hasLocation(#20057,#20034)
+successor(#20051,#20057)
+successor(#20046,#20048)
+successor(#20049,#20051)
+successor(#20048,#20049)
+successor(#20043,#20046)
+successor(#20056,#20043)
+successor(#20039,#20038)
+successor(#20053,#20039)
+numlines(#10000,2,2,0)
 filetype(#10000,"javascript")