Disallow setters to have RestElement (#7498)

packages/babylon/src/parser/expression.js

@@ -1383,17 +1383,24 @@ export default class ExpressionParser extends LValParser {
   }
 
   // get methods aren't allowed to have any parameters
-  // set methods must have exactly 1 parameter
+  // set methods must have exactly 1 parameter which is not a rest parameter
-  checkGetterSetterParamCount(method: N.ObjectMethod | N.ClassMethod): void {
+  checkGetterSetterParams(method: N.ObjectMethod | N.ClassMethod): void {
     const paramCount = method.kind === "get" ? 0 : 1;
-    if (method.params.length !== paramCount) {
     const start = method.start;
+    if (method.params.length !== paramCount) {
       if (method.kind === "get") {
-        this.raise(start, "getter should have no params");
+        this.raise(start, "getter must not have any formal parameters");
       } else {
-        this.raise(start, "setter should have exactly one param");
+        this.raise(start, "setter must have exactly one formal parameter");
       }
     }
+
+    if (method.kind === "set" && method.params[0].type === "RestElement") {
+      this.raise(
+        start,
+        "setter function argument must not be a rest parameter",
+      );
+    }
   }
 
   parseObjectMethod(
@@ -1426,7 +1433,7 @@ export default class ExpressionParser extends LValParser {
         /* isConstructor */ false,
         "ObjectMethod",
       );
-      this.checkGetterSetterParamCount(prop);
+      this.checkGetterSetterParams(prop);
       return prop;
     }
   }

packages/babylon/src/parser/statement.js

@@ -1161,7 +1161,7 @@ export default class StatementParser extends ExpressionParser {
         );
       }
 
-      this.checkGetterSetterParamCount(publicMethod);
+      this.checkGetterSetterParams(publicMethod);
     } else if (this.isLineTerminator()) {
       // an uninitialized class property (due to ASI, since we don't otherwise recognize the next token)
       if (isPrivate) {

packages/babylon/src/plugins/estree.js

@@ -82,17 +82,24 @@ export default (superClass: Class<Parser>): Class<Parser> =>
       }
     }
 
-    checkGetterSetterParamCount(prop: N.ObjectMethod | N.ClassMethod): void {
+    checkGetterSetterParams(method: N.ObjectMethod | N.ClassMethod): void {
+      const prop = ((method: any): N.EstreeProperty | N.EstreeMethodDefinition);
       const paramCount = prop.kind === "get" ? 0 : 1;
-      // $FlowFixMe (prop.value present for ObjectMethod, but for ClassMethod should use prop.params?)
-      if (prop.value.params.length !== paramCount) {
       const start = prop.start;
+      if (prop.value.params.length !== paramCount) {
         if (prop.kind === "get") {
-          this.raise(start, "getter should have no params");
+          this.raise(start, "getter must not have any formal parameters");
         } else {
-          this.raise(start, "setter should have exactly one param");
+          this.raise(start, "setter must have exactly one formal parameter");
         }
       }
+
+      if (prop.kind === "set" && prop.value.params[0].type === "RestElement") {
+        this.raise(
+          start,
+          "setter function argument must not be a rest parameter",
+        );
+      }
     }
 
     checkLVal(

packages/babylon/src/plugins/flow.js

@@ -763,7 +763,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
             this.startNodeAt(node.start, node.loc.start),
           );
           if (kind === "get" || kind === "set") {
-            this.flowCheckGetterSetterParamCount(node);
+            this.flowCheckGetterSetterParams(node);
           }
         } else {
           if (kind !== "init") this.unexpected();
@@ -783,20 +783,29 @@ export default (superClass: Class<Parser>): Class<Parser> =>
       }
     }
 
-    // This is similar to checkGetterSetterParamCount, but as
+    // This is similar to checkGetterSetterParams, but as
     // babylon uses non estree properties we cannot reuse it here
-    flowCheckGetterSetterParamCount(
+    flowCheckGetterSetterParams(
       property: N.FlowObjectTypeProperty | N.FlowObjectTypeSpreadProperty,
     ): void {
       const paramCount = property.kind === "get" ? 0 : 1;
-      if (property.value.params.length !== paramCount) {
       const start = property.start;
+      const length =
+        property.value.params.length + (property.value.rest ? 1 : 0);
+      if (length !== paramCount) {
         if (property.kind === "get") {
-          this.raise(start, "getter should have no params");
+          this.raise(start, "getter must not have any formal parameters");
         } else {
-          this.raise(start, "setter should have exactly one param");
+          this.raise(start, "setter must have exactly one formal parameter");
         }
       }
+
+      if (property.kind === "set" && property.value.rest) {
+        this.raise(
+          start,
+          "setter function argument must not be a rest parameter",
+        );
+      }
     }
 
     flowObjectTypeSemicolon(): void {

packages/babylon/src/types.js

@@ -941,6 +941,18 @@ export type EstreeProperty = NodeBase & {
   variance?: ?FlowVariance,
 };
 
+export type EstreeMethodDefinition = NodeBase & {
+  type: "MethodDefinition",
+  static: boolean,
+  key: Expression,
+  computed: boolean,
+  value: Expression,
+  decorators: $ReadOnlyArray<Decorator>,
+  kind?: "get" | "set" | "method",
+
+  variance?: ?FlowVariance,
+};
+
 // === === === ===
 // TypeScript
 // === === === ===

packages/babylon/test/fixtures/es2015/class-methods/getter-signature/options.json

@@ -1,3 +1,3 @@
 {
-  "throws": "getter should have no params (2:2)"
+  "throws": "getter must not have any formal parameters (2:2)"
 }

packages/babylon/test/fixtures/es2015/uncategorised/347/options.json

@@ -1,3 +1,3 @@
 {
-  "throws": "setter should have exactly one param (1:10)"
+  "throws": "setter must have exactly one formal parameter (1:10)"
 }

packages/babylon/test/fixtures/esprima/rest-parameter/invalid-setter-rest/options.json

@@ -0,0 +1,3 @@
+{
+    "throws": "setter function argument must not be a rest parameter (1:6)"
+  }

packages/babylon/test/fixtures/esprima/rest-parameter/invalid-setter-rest/output.json

@@ -1,188 +0,0 @@
-{
-  "type": "File",
-  "start": 0,
-  "end": 22,
-  "loc": {
-    "start": {
-      "line": 1,
-      "column": 0
-    },
-    "end": {
-      "line": 1,
-      "column": 22
-    }
-  },
-  "program": {
-    "type": "Program",
-    "start": 0,
-    "end": 22,
-    "loc": {
-      "start": {
-        "line": 1,
-        "column": 0
-      },
-      "end": {
-        "line": 1,
-        "column": 22
-      }
-    },
-    "sourceType": "script",
-    "body": [
-      {
-        "type": "ExpressionStatement",
-        "start": 0,
-        "end": 22,
-        "loc": {
-          "start": {
-            "line": 1,
-            "column": 0
-          },
-          "end": {
-            "line": 1,
-            "column": 22
-          }
-        },
-        "expression": {
-          "type": "AssignmentExpression",
-          "start": 0,
-          "end": 22,
-          "loc": {
-            "start": {
-              "line": 1,
-              "column": 0
-            },
-            "end": {
-              "line": 1,
-              "column": 22
-            }
-          },
-          "operator": "=",
-          "left": {
-            "type": "Identifier",
-            "start": 0,
-            "end": 1,
-            "loc": {
-              "start": {
-                "line": 1,
-                "column": 0
-              },
-              "end": {
-                "line": 1,
-                "column": 1
-              },
-              "identifierName": "x"
-            },
-            "name": "x"
-          },
-          "right": {
-            "type": "ObjectExpression",
-            "start": 4,
-            "end": 22,
-            "loc": {
-              "start": {
-                "line": 1,
-                "column": 4
-              },
-              "end": {
-                "line": 1,
-                "column": 22
-              }
-            },
-            "properties": [
-              {
-                "type": "ObjectMethod",
-                "start": 6,
-                "end": 20,
-                "loc": {
-                  "start": {
-                    "line": 1,
-                    "column": 6
-                  },
-                  "end": {
-                    "line": 1,
-                    "column": 20
-                  }
-                },
-                "method": false,
-                "key": {
-                  "type": "Identifier",
-                  "start": 10,
-                  "end": 11,
-                  "loc": {
-                    "start": {
-                      "line": 1,
-                      "column": 10
-                    },
-                    "end": {
-                      "line": 1,
-                      "column": 11
-                    },
-                    "identifierName": "f"
-                  },
-                  "name": "f"
-                },
-                "computed": false,
-                "kind": "set",
-                "id": null,
-                "generator": false,
-                "async": false,
-                "params": [
-                  {
-                    "type": "RestElement",
-                    "start": 12,
-                    "end": 16,
-                    "loc": {
-                      "start": {
-                        "line": 1,
-                        "column": 12
-                      },
-                      "end": {
-                        "line": 1,
-                        "column": 16
-                      }
-                    },
-                    "argument": {
-                      "type": "Identifier",
-                      "start": 15,
-                      "end": 16,
-                      "loc": {
-                        "start": {
-                          "line": 1,
-                          "column": 15
-                        },
-                        "end": {
-                          "line": 1,
-                          "column": 16
-                        },
-                        "identifierName": "y"
-                      },
-                      "name": "y"
-                    }
-                  }
-                ],
-                "body": {
-                  "type": "BlockStatement",
-                  "start": 18,
-                  "end": 20,
-                  "loc": {
-                    "start": {
-                      "line": 1,
-                      "column": 18
-                    },
-                    "end": {
-                      "line": 1,
-                      "column": 20
-                    }
-                  },
-                  "body": [],
-                  "directives": []
-                }
-              }
-            ]
-          }
-        }
-      }
-    ],
-    "directives": []
-  }
-}

packages/babylon/test/fixtures/flow/object-types/invalid-getter-param-count-rest/input.js

@@ -0,0 +1,3 @@
+type B = {
+  get a(...foo): number;
+}

packages/babylon/test/fixtures/flow/object-types/invalid-getter-param-count-rest/options.json

@@ -0,0 +1,3 @@
+{
+  "throws": "getter must not have any formal parameters (2:2)"
+}

packages/babylon/test/fixtures/flow/object-types/invalid-getter-param-count/options.json

@@ -1,3 +1,3 @@
 {
-  "throws": "getter should have no params (2:2)"
+  "throws": "getter must not have any formal parameters (2:2)"
 }

packages/babylon/test/fixtures/flow/object-types/invalid-setter-param-count/options.json

@@ -1,3 +1,3 @@
 {
-  "throws": "setter should have exactly one param (2:2)"
+  "throws": "setter must have exactly one formal parameter (2:2)"
 }

packages/babylon/test/fixtures/flow/object-types/invalid-setter-param-type/input.js

@@ -0,0 +1,3 @@
+type B = {
+  set a(...w): void;
+}

packages/babylon/test/fixtures/flow/object-types/invalid-setter-param-type/options.json

@@ -0,0 +1,3 @@
+{
+  "throws": "setter function argument must not be a rest parameter (2:2)"
+}