Fix location for optional params in arrow functions (#9998)

* Fix location with optional params in arrow functions * add test * Ensure rollup replaces NODE_ENV and create sourcemap in dev * Ensure finishNod*() is never called twice on a node * Fix check for already finished nodes
babel · May 21, 2019 · 54d257c · 54d257c
1 parent 9c06e4e
commit 54d257c
Show file tree

Hide file tree

Showing 14 changed files with 361 additions and 51 deletions.
diff --git a/Gulpfile.js b/Gulpfile.js
@@ -15,6 +15,7 @@ const merge = require("merge-stream");
 const rollup = require("rollup");
 const rollupBabel = require("rollup-plugin-babel");
 const rollupNodeResolve = require("rollup-plugin-node-resolve");
+const rollupReplace = require("rollup-plugin-replace");
 const { registerStandalonePackageTask } = require("./scripts/gulp-tasks");
 
 const sources = ["codemods", "packages"];
@@ -92,6 +93,9 @@ function buildRollup(packages) {
         .rollup({
           input,
           plugins: [
+            rollupReplace({
+              "process.env.NODE_ENV": JSON.stringify(process.env.NODE_ENV),
+            }),
             rollupBabel({
               envName: "babel-parser",
             }),
@@ -103,6 +107,7 @@ function buildRollup(packages) {
             file: path.join(pkg, "lib/index.js"),
             format: "cjs",
             name: "babel-parser",
+            sourcemap: process.env.NODE_ENV !== "production",
           });
         });
     })

diff --git a/Makefile b/Makefile
@@ -124,7 +124,7 @@ prepublish-build:
 	rm -rf packages/babel-runtime/helpers
 	rm -rf packages/babel-runtime-corejs2/helpers
 	rm -rf packages/babel-runtime-corejs2/core-js
-	BABEL_ENV=production make build-dist
+	NODE_ENV=production BABEL_ENV=production make build-dist
 	make clone-license
 
 prepublish:

diff --git a/package.json b/package.json
@@ -62,6 +62,7 @@
     "rollup": "^1.6.0",
     "rollup-plugin-babel": "^4.0.0",
     "rollup-plugin-node-resolve": "^4.0.1",
+    "rollup-plugin-replace": "^2.2.0",
     "test262-stream": "^1.2.0",
     "through2": "^2.0.0",
     "warnings-to-errors-webpack-plugin": "^2.0.0",

diff --git a/packages/babel-parser/src/parser/node.js b/packages/babel-parser/src/parser/node.js
@@ -83,6 +83,12 @@ export class NodeUtils extends UtilParser {
     pos: number,
     loc: Position,
   ): T {
+    if (process.env.NODE_ENV !== "production" && node.end > 0) {
+      throw new Error(
+        "Do not call finishNode*() twice on the same node." +
+          " Instead use resetEndLocation() or change type directly.",
+      );
+    }
     node.type = type;
     node.end = pos;
     node.loc.end = loc;
@@ -97,6 +103,16 @@ export class NodeUtils extends UtilParser {
     if (this.options.ranges) node.range[0] = start;
   }
 
+  resetEndLocation(
+    node: NodeBase,
+    end?: number = this.state.lastTokEnd,
+    endLoc?: Position = this.state.lastTokEndLoc,
+  ): void {
+    node.end = end;
+    node.loc.end = endLoc;
+    if (this.options.ranges) node.range[1] = end;
+  }
+
   /**
    * Reset the start location of node to the start location of locationNode
    */

diff --git a/packages/babel-parser/src/plugins/flow.js b/packages/babel-parser/src/plugins/flow.js
@@ -222,8 +222,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
 
       id.typeAnnotation = this.finishNode(typeContainer, "TypeAnnotation");
 
-      this.finishNode(id, id.type);
-
+      this.resetEndLocation(id);
       this.semicolon();
 
       return this.finishNode(node, "DeclareFunction");
@@ -432,15 +431,19 @@ export default (superClass: Class<Parser>): Class<Parser> =>
     ): N.FlowDeclareTypeAlias {
       this.next();
       this.flowParseTypeAlias(node);
-      return this.finishNode(node, "DeclareTypeAlias");
+      // Don't do finishNode as we don't want to process comments twice
+      node.type = "DeclareTypeAlias";
+      return node;
     }
 
     flowParseDeclareOpaqueType(
       node: N.FlowDeclareOpaqueType,
     ): N.FlowDeclareOpaqueType {
       this.next();
       this.flowParseOpaqueType(node, true);
-      return this.finishNode(node, "DeclareOpaqueType");
+      // Don't do finishNode as we don't want to process comments twice
+      node.type = "DeclareOpaqueType";
+      return node;
     }
 
     flowParseDeclareInterface(
@@ -1520,20 +1523,21 @@ export default (superClass: Class<Parser>): Class<Parser> =>
         : this.flowParseRestrictedIdentifier();
       if (this.match(tt.colon)) {
         ident.typeAnnotation = this.flowParseTypeAnnotation();
-        this.finishNode(ident, ident.type);
+        this.resetEndLocation(ident);
       }
       return ident;
     }
 
     typeCastToParameter(node: N.Node): N.Node {
       node.expression.typeAnnotation = node.typeAnnotation;
 
-      return this.finishNodeAt(
+      this.resetEndLocation(
         node.expression,
-        node.expression.type,
         node.typeAnnotation.end,
         node.typeAnnotation.loc.end,
       );
+
+      return node.expression;
     }
 
     flowParseVariance(): ?N.FlowVariance {
@@ -1848,6 +1852,10 @@ export default (superClass: Class<Parser>): Class<Parser> =>
       node = super.parseParenItem(node, startPos, startLoc);
       if (this.eat(tt.question)) {
         node.optional = true;
+        // Include questionmark in location of node
+        // Don't use this.finishNode() as otherwise we might process comments twice and
+        // include already consumed parens
+        this.resetEndLocation(node);
       }
 
       if (this.match(tt.colon)) {
@@ -2205,7 +2213,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
       if (this.match(tt.colon)) {
         param.typeAnnotation = this.flowParseTypeAnnotation();
       }
-      this.finishNode(param, param.type);
+      this.resetEndLocation(param);
       return param;
     }
 
@@ -2393,7 +2401,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
       super.parseVarId(decl, kind);
       if (this.match(tt.colon)) {
         decl.id.typeAnnotation = this.flowParseTypeAnnotation();
-        this.finishNode(decl.id, decl.id.type);
+        this.resetEndLocation(decl.id); // set end position to end of type
       }
     }
 

diff --git a/packages/babel-parser/src/plugins/placeholders.js b/packages/babel-parser/src/plugins/placeholders.js
@@ -71,8 +71,10 @@ export default (superClass: Class<Parser>): Class<Parser> =>
       node: N.Node,
       expectedNode: T,
     ): /*N.Placeholder<T>*/ MaybePlaceholder<T> {
+      const isFinished = !!(node.expectedNode && node.type === "Placeholder");
       node.expectedNode = expectedNode;
-      return this.finishNode(node, "Placeholder");
+
+      return isFinished ? node : this.finishNode(node, "Placeholder");
     }
 
     /* ============================================================ *

diff --git a/packages/babel-parser/src/plugins/typescript/index.js b/packages/babel-parser/src/plugins/typescript/index.js
@@ -422,7 +422,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
       this.expect(tt.bracketL);
       const id = this.parseIdentifier();
       id.typeAnnotation = this.tsParseTypeAnnotation();
-      this.finishNode(id, "Identifier"); // set end position to end of type
+      this.resetEndLocation(id); // set end position to end of type
 
       this.expect(tt.bracketR);
       node.parameters = [id];
@@ -1961,6 +1961,10 @@ export default (superClass: Class<Parser>): Class<Parser> =>
       node = super.parseParenItem(node, startPos, startLoc);
       if (this.eat(tt.question)) {
         node.optional = true;
+        // Include questionmark in location of node
+        // Don't use this.finishNode() as otherwise we might process comments twice and
+        // include already consumed parens
+        this.resetEndLocation(node);
       }
 
       if (this.match(tt.colon)) {
@@ -1974,7 +1978,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
         return this.finishNode(typeCastNode, "TSTypeCastExpression");
       }
 
-      return this.finishNode(node, node.type);
+      return node;
     }
 
     parseExportDeclaration(node: N.ExportNamedDeclaration): ?N.Declaration {
@@ -2095,7 +2099,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
       const type = this.tsTryParseTypeAnnotation();
       if (type) {
         decl.id.typeAnnotation = type;
-        this.finishNode(decl.id, decl.id.type); // set end position to end of type
+        this.resetEndLocation(decl.id); // set end position to end of type
       }
     }
 
@@ -2239,7 +2243,9 @@ export default (superClass: Class<Parser>): Class<Parser> =>
       }
       const type = this.tsTryParseTypeAnnotation();
       if (type) param.typeAnnotation = type;
-      return this.finishNode(param, param.type);
+      this.resetEndLocation(param);
+
+      return param;
     }
 
     toAssignable(
@@ -2401,12 +2407,13 @@ export default (superClass: Class<Parser>): Class<Parser> =>
     typeCastToParameter(node: N.TsTypeCastExpression): N.Node {
       node.expression.typeAnnotation = node.typeAnnotation;
 
-      return this.finishNodeAt(
+      this.resetEndLocation(
         node.expression,
-        node.expression.type,
         node.typeAnnotation.end,
         node.typeAnnotation.loc.end,
       );
+
+      return node.expression;
     }
 
     toReferencedList(

diff --git a/packages/babel-parser/test/fixtures/flow/optional-type/1/output.json b/packages/babel-parser/test/fixtures/flow/optional-type/1/output.json
@@ -96,15 +96,15 @@
                 {
                   "type": "Identifier",
                   "start": 11,
-                  "end": 12,
+                  "end": 13,
                   "loc": {
                     "start": {
                       "line": 1,
                       "column": 11
                     },
                     "end": {
                       "line": 1,
-                      "column": 12
+                      "column": 13
                     },
                     "identifierName": "x"
                   },

diff --git a/packages/babel-parser/test/fixtures/flow/optional-type/3/output.json b/packages/babel-parser/test/fixtures/flow/optional-type/3/output.json
@@ -96,15 +96,15 @@
                 {
                   "type": "Identifier",
                   "start": 11,
-                  "end": 12,
+                  "end": 13,
                   "loc": {
                     "start": {
                       "line": 1,
                       "column": 11
                     },
                     "end": {
                       "line": 1,
-                      "column": 12
+                      "column": 13
                     },
                     "identifierName": "x"
                   },

diff --git a/packages/babel-parser/test/fixtures/flow/optional-type/4/output.json b/packages/babel-parser/test/fixtures/flow/optional-type/4/output.json
@@ -96,15 +96,15 @@
                 {
                   "type": "RestElement",
                   "start": 11,
-                  "end": 15,
+                  "end": 16,
                   "loc": {
                     "start": {
                       "line": 1,
                       "column": 11
                     },
                     "end": {
                       "line": 1,
-                      "column": 15
+                      "column": 16
                     }
                   },
                   "argument": {

diff --git a/packages/babel-parser/test/fixtures/typescript/cast/as-const/output.json b/packages/babel-parser/test/fixtures/typescript/cast/as-const/output.json
@@ -3951,15 +3951,15 @@
               "expression": {
                 "type": "UnaryExpression",
                 "start": 858,
-                "end": 862,
+                "end": 861,
                 "loc": {
                   "start": {
                     "line": 29,
                     "column": 11
                   },
                   "end": {
                     "line": 29,
-                    "column": 15
+                    "column": 14
                   }
                 },
                 "operator": "-",
@@ -4104,15 +4104,15 @@
                   {
                     "type": "NumericLiteral",
                     "start": 886,
-                    "end": 889,
+                    "end": 888,
                     "loc": {
                       "start": {
                         "line": 30,
                         "column": 12
                       },
                       "end": {
                         "line": 30,
-                        "column": 15
+                        "column": 14
                       }
                     },
                     "extra": {

diff --git a/packages/babel-parser/test/fixtures/typescript/regression/comments-disappearing/input.ts b/packages/babel-parser/test/fixtures/typescript/regression/comments-disappearing/input.ts
@@ -0,0 +1,6 @@
+let fun = () => {
+    // one
+    // two
+    // three
+    return (1);
+}