Improve module expression parsing/printing (#14980)

* fix: several ModuleExpression printing edgecases * fix: end module expression's program before } * fix: expect braceL after module * refactor: introduce ensureNoLineTerminator * use this.ensureNoLineTerminator * update source-map fixtures * address review comments * consume eof when end is eof
babel · Sep 30, 2022 · 98c3bb9 · 98c3bb9
1 parent 5729ce9
commit 98c3bb9
Show file tree

Hide file tree

Showing 29 changed files with 80 additions and 33 deletions.
diff --git a/packages/babel-generator/src/generators/expressions.ts b/packages/babel-generator/src/generators/expressions.ts
@@ -351,16 +351,19 @@ export function V8IntrinsicIdentifier(
 
 export function ModuleExpression(this: Printer, node: t.ModuleExpression) {
   this.word("module");
-  this.space();
+  // ensure no line terminator between `module` and `{`
+  this.ensureNoLineTerminator(() => {
+    this.printInnerComments(node);
+    this.space();
+  });
   this.token("{");
-  if (node.body.body.length === 0) {
-    this.token("}");
-  } else {
+  this.indent();
+  const { body } = node;
+  if (body.body.length || body.directives.length) {
     this.newline();
-    this.printSequence(node.body.body, node, { indent: true });
-
-    this.sourceWithOffset("end", node.loc, 0, -1);
-
-    this.rightBrace();
   }
+  this.print(body, node);
+  this.dedent();
+  this.sourceWithOffset("end", node.loc, 0, -1);
+  this.rightBrace();
 }
diff --git a/packages/babel-generator/src/printer.ts b/packages/babel-generator/src/printer.ts
@@ -495,6 +495,13 @@ class Printer {
     return this._indentRepeat * this._indent;
   }
 
+  ensureNoLineTerminator(fn: () => void) {
+    const { _noLineTerminator } = this;
+    this._noLineTerminator = true;
+    fn();
+    this._noLineTerminator = _noLineTerminator;
+  }
+
   printTerminatorless(node: t.Node, parent: t.Node, isLabel: boolean) {
     /**
      * Set some state that will be modified if a newline has been inserted before any
@@ -512,9 +519,9 @@ class Printer {
      *  `undefined` will be returned and not `foo` due to the terminator.
      */
     if (isLabel) {
-      this._noLineTerminator = true;
-      this.print(node, parent);
-      this._noLineTerminator = false;
+      this.ensureNoLineTerminator(() => {
+        this.print(node, parent);
+      });
     } else {
       const terminatorState = {
         printed: false,

diff --git a/packages/babel-generator/test/fixtures/comments/module-expression-inner-comment/input.js b/packages/babel-generator/test/fixtures/comments/module-expression-inner-comment/input.js
@@ -0,0 +1 @@
+module /* 1 */ { /* 2 */ }
diff --git a/packages/babel-generator/test/fixtures/comments/module-expression-inner-comment/options.json b/packages/babel-generator/test/fixtures/comments/module-expression-inner-comment/options.json
@@ -0,0 +1,3 @@
+{
+  "plugins": ["moduleBlocks"]
+}
diff --git a/packages/babel-generator/test/fixtures/comments/module-expression-inner-comment/output.js b/packages/babel-generator/test/fixtures/comments/module-expression-inner-comment/output.js
@@ -0,0 +1,3 @@
+module /* 1 */ {
+  /* 2 */
+};
diff --git a/packages/babel-generator/test/fixtures/sourcemaps/ModuleExpression/source-map.json b/packages/babel-generator/test/fixtures/sourcemaps/ModuleExpression/source-map.json
@@ -11,5 +11,5 @@
   "sourcesContent": [
     "const m = module { export const foo = \"foo\" };\nmodule {\n  foo;\n  bar;\n};\nfoo(module {});"
   ],
-  "mappings": "AAAA,MAAMA,CAAC,GAAG;EAAS,OAAO,MAAMC,GAAG,GAAG,KAAK;AAAC,CAAC;AAC7C;EACEA,GAAG;EACHC,GAAG;AACL,CAAC;AACDD,GAAG,CAAC,SAAS,CAAC"
+  "mappings": "AAAA,MAAMA,CAAC,GAAG;EAAS,OAAO,MAAMC,GAAG,GAAG,KAAK;AAAC,CAAC;AAC7C;EACEA,GAAG;EACHC,GAAG;AACL,CAAC;AACDD,GAAG,CAAC,QAAQ,CAAC,CAAC"
 }
diff --git a/packages/babel-generator/test/fixtures/types/ModuleExpression-directives/input.js b/packages/babel-generator/test/fixtures/types/ModuleExpression-directives/input.js
@@ -0,0 +1,4 @@
+module {
+  "hide source";
+  secret;
+}
diff --git a/packages/babel-generator/test/fixtures/types/ModuleExpression-directives/options.json b/packages/babel-generator/test/fixtures/types/ModuleExpression-directives/options.json
@@ -0,0 +1,4 @@
+{
+  "plugins": ["moduleBlocks"],
+  "sourceType": "module"
+}
diff --git a/packages/babel-generator/test/fixtures/types/ModuleExpression-directives/output.js b/packages/babel-generator/test/fixtures/types/ModuleExpression-directives/output.js
@@ -0,0 +1,5 @@
+module {
+  "hide source";
+
+  secret;
+};
diff --git a/packages/babel-parser/src/parser/expression.ts b/packages/babel-parser/src/parser/expression.ts
@@ -3233,7 +3233,7 @@ export default abstract class ExpressionParser extends LValParser {
     this.expectPlugin("moduleBlocks");
     const node = this.startNode<N.ModuleExpression>();
     this.next(); // eat "module"
-    this.eat(tt.braceL);
+    this.expect(tt.braceL);
 
     const revertScopes = this.initializeScopes(/** inModule */ true);
     this.enterInitialScopes();

diff --git a/packages/babel-parser/src/parser/statement.ts b/packages/babel-parser/src/parser/statement.ts
@@ -222,6 +222,9 @@ export default abstract class StatementParser extends ExpressionParser {
         this.raise(Errors.ModuleExportUndefined, { at, localName });
       }
     }
+    if (end === tt.eof) {
+      this.next(); // consume eof for the top level program
+    }
     return this.finishNode(program, "Program");
   }
 
@@ -1123,6 +1126,7 @@ export default abstract class StatementParser extends ExpressionParser {
       tt.braceR,
       afterBlockParse,
     );
+    this.next(); // eat tt.braceR
     if (createNewLexicalScope) {
       this.scope.exit();
     }
@@ -1204,8 +1208,6 @@ export default abstract class StatementParser extends ExpressionParser {
     if (!oldStrict) {
       this.setStrict(false);
     }
-
-    this.next();
   }
 
   // Parse a regular `for` loop. The disambiguation code in
@@ -1881,6 +1883,7 @@ export default abstract class StatementParser extends ExpressionParser {
     this.prodParam.enter(PARAM);
     const body: N.Node[] = (member.body = []);
     this.parseBlockOrModuleBlockBody(body, undefined, false, tt.braceR);
+    this.next(); // eat tt.braceR
     this.prodParam.exit();
     this.scope.exit();
     this.state.labels = oldLabels;

diff --git a/packages/babel-parser/src/plugins/typescript/index.ts b/packages/babel-parser/src/plugins/typescript/index.ts
@@ -1869,6 +1869,7 @@ export default (superClass: ClassWithMixin<typeof Parser, IJSXParserMixin>) =>
         /* topLevel */ true,
         /* end */ tt.braceR,
       );
+      this.next(); // eat tt.braceR
       this.scope.exit();
       return this.finishNode(node, "TSModuleBlock");
     }

diff --git a/...arser/test/fixtures/experimental/module-blocks/invalid-class-in-module-blocks/output.json b/...arser/test/fixtures/experimental/module-blocks/invalid-class-in-module-blocks/output.json
@@ -53,7 +53,7 @@
                       "start":23,"end":66,"loc":{"start":{"line":3,"column":4,"index":23},"end":{"line":5,"column":5,"index":66}},
                       "body": {
                         "type": "Program",
-                        "start":38,"end":66,"loc":{"start":{"line":4,"column":6,"index":38},"end":{"line":5,"column":5,"index":66}},
+                        "start":38,"end":60,"loc":{"start":{"line":4,"column":6,"index":38},"end":{"line":4,"column":28,"index":60}},
                         "sourceType": "module",
                         "interpreter": null,
                         "body": [

diff --git a/...test/fixtures/experimental/module-blocks/invalid-duplicated-exports-in-blocks/output.json b/...test/fixtures/experimental/module-blocks/invalid-duplicated-exports-in-blocks/output.json
@@ -27,7 +27,7 @@
               "start":10,"end":66,"loc":{"start":{"line":1,"column":10,"index":10},"end":{"line":4,"column":1,"index":66}},
               "body": {
                 "type": "Program",
-                "start":21,"end":66,"loc":{"start":{"line":2,"column":2,"index":21},"end":{"line":4,"column":1,"index":66}},
+                "start":21,"end":64,"loc":{"start":{"line":2,"column":2,"index":21},"end":{"line":3,"column":17,"index":64}},
                 "sourceType": "module",
                 "interpreter": null,
                 "body": [

diff --git a/...babel-parser/test/fixtures/experimental/module-blocks/invalid-module-bracketBarL/input.js b/...babel-parser/test/fixtures/experimental/module-blocks/invalid-module-bracketBarL/input.js
@@ -0,0 +1 @@
+module{|foo:42|}}
diff --git a/...l-parser/test/fixtures/experimental/module-blocks/invalid-module-bracketBarL/options.json b/...l-parser/test/fixtures/experimental/module-blocks/invalid-module-bracketBarL/options.json
@@ -0,0 +1,12 @@
+{
+  "plugins": [
+    "moduleBlocks",
+    [
+      "recordAndTuple",
+      {
+        "syntaxType": "bar"
+      }
+    ]
+  ],
+  "throws": "Unexpected token, expected \"{\" (1:6)"
+}
diff --git a/...xtures/experimental/module-blocks/invalid-undefined-export-in-module-blocks01/output.json b/...xtures/experimental/module-blocks/invalid-undefined-export-in-module-blocks01/output.json
@@ -18,7 +18,7 @@
           "start":0,"end":27,"loc":{"start":{"line":1,"column":0,"index":0},"end":{"line":3,"column":1,"index":27}},
           "body": {
             "type": "Program",
-            "start":11,"end":27,"loc":{"start":{"line":2,"column":2,"index":11},"end":{"line":3,"column":1,"index":27}},
+            "start":11,"end":25,"loc":{"start":{"line":2,"column":2,"index":11},"end":{"line":2,"column":16,"index":25}},
             "sourceType": "module",
             "interpreter": null,
             "body": [

diff --git a/...xtures/experimental/module-blocks/invalid-undefined-export-in-module-blocks02/output.json b/...xtures/experimental/module-blocks/invalid-undefined-export-in-module-blocks02/output.json
@@ -35,7 +35,7 @@
           "start":9,"end":37,"loc":{"start":{"line":2,"column":0,"index":9},"end":{"line":4,"column":1,"index":37}},
           "body": {
             "type": "Program",
-            "start":20,"end":37,"loc":{"start":{"line":3,"column":2,"index":20},"end":{"line":4,"column":1,"index":37}},
+            "start":20,"end":35,"loc":{"start":{"line":3,"column":2,"index":20},"end":{"line":3,"column":17,"index":35}},
             "sourceType": "module",
             "interpreter": null,
             "body": [

diff --git a/...er/test/fixtures/experimental/module-blocks/module-block-in-member-expression/output.json b/...er/test/fixtures/experimental/module-blocks/module-block-in-member-expression/output.json
@@ -18,7 +18,7 @@
             "start":0,"end":9,"loc":{"start":{"line":1,"column":0,"index":0},"end":{"line":1,"column":9,"index":9}},
             "body": {
               "type": "Program",
-              "start":8,"end":9,"loc":{"start":{"line":1,"column":8,"index":8},"end":{"line":1,"column":9,"index":9}},
+              "start":8,"end":8,"loc":{"start":{"line":1,"column":8,"index":8},"end":{"line":1,"column":8,"index":8}},
               "sourceType": "module",
               "interpreter": null,
               "body": [],

diff --git a/...t/fixtures/experimental/module-blocks/valid-assign-module-blocks-to-variables/output.json b/...t/fixtures/experimental/module-blocks/valid-assign-module-blocks-to-variables/output.json
@@ -24,7 +24,7 @@
               "start":10,"end":48,"loc":{"start":{"line":1,"column":10,"index":10},"end":{"line":3,"column":1,"index":48}},
               "body": {
                 "type": "Program",
-                "start":21,"end":48,"loc":{"start":{"line":2,"column":2,"index":21},"end":{"line":3,"column":1,"index":48}},
+                "start":21,"end":46,"loc":{"start":{"line":2,"column":2,"index":21},"end":{"line":2,"column":27,"index":46}},
                 "sourceType": "module",
                 "interpreter": null,
                 "body": [

diff --git a/...-parser/test/fixtures/experimental/module-blocks/valid-class-in-module-blocks/output.json b/...-parser/test/fixtures/experimental/module-blocks/valid-class-in-module-blocks/output.json
@@ -50,7 +50,7 @@
                       "start":23,"end":96,"loc":{"start":{"line":3,"column":4,"index":23},"end":{"line":8,"column":5,"index":96}},
                       "body": {
                         "type": "Program",
-                        "start":38,"end":96,"loc":{"start":{"line":4,"column":6,"index":38},"end":{"line":8,"column":5,"index":96}},
+                        "start":38,"end":90,"loc":{"start":{"line":4,"column":6,"index":38},"end":{"line":7,"column":7,"index":90}},
                         "sourceType": "module",
                         "interpreter": null,
                         "body": [

diff --git a/...res/experimental/module-blocks/valid-duplicated-exports-between-other-modules/output.json b/...res/experimental/module-blocks/valid-duplicated-exports-between-other-modules/output.json
@@ -55,7 +55,7 @@
               "start":37,"end":72,"loc":{"start":{"line":2,"column":11,"index":37},"end":{"line":2,"column":46,"index":72}},
               "body": {
                 "type": "Program",
-                "start":46,"end":72,"loc":{"start":{"line":2,"column":20,"index":46},"end":{"line":2,"column":46,"index":72}},
+                "start":46,"end":70,"loc":{"start":{"line":2,"column":20,"index":46},"end":{"line":2,"column":44,"index":70}},
                 "sourceType": "module",
                 "interpreter": null,
                 "body": [
@@ -115,7 +115,7 @@
               "start":85,"end":120,"loc":{"start":{"line":3,"column":11,"index":85},"end":{"line":3,"column":46,"index":120}},
               "body": {
                 "type": "Program",
-                "start":94,"end":120,"loc":{"start":{"line":3,"column":20,"index":94},"end":{"line":3,"column":46,"index":120}},
+                "start":94,"end":118,"loc":{"start":{"line":3,"column":20,"index":94},"end":{"line":3,"column":44,"index":118}},
                 "sourceType": "module",
                 "interpreter": null,
                 "body": [

diff --git a/...xtures/experimental/module-blocks/valid-duplicated-variables-in-module-blocks/output.json b/...xtures/experimental/module-blocks/valid-duplicated-variables-in-module-blocks/output.json
@@ -49,7 +49,7 @@
               "start":29,"end":60,"loc":{"start":{"line":2,"column":10,"index":29},"end":{"line":4,"column":1,"index":60}},
               "body": {
                 "type": "Program",
-                "start":40,"end":60,"loc":{"start":{"line":3,"column":2,"index":40},"end":{"line":4,"column":1,"index":60}},
+                "start":40,"end":58,"loc":{"start":{"line":3,"column":2,"index":40},"end":{"line":3,"column":20,"index":58}},
                 "sourceType": "module",
                 "interpreter": null,
                 "body": [

diff --git a/...bel-parser/test/fixtures/experimental/module-blocks/valid-empty-module-blocks/output.json b/...bel-parser/test/fixtures/experimental/module-blocks/valid-empty-module-blocks/output.json
@@ -15,7 +15,7 @@
           "start":0,"end":9,"loc":{"start":{"line":1,"column":0,"index":0},"end":{"line":1,"column":9,"index":9}},
           "body": {
             "type": "Program",
-            "start":8,"end":9,"loc":{"start":{"line":1,"column":8,"index":8},"end":{"line":1,"column":9,"index":9}},
+            "start":8,"end":8,"loc":{"start":{"line":1,"column":8,"index":8},"end":{"line":1,"column":8,"index":8}},
             "sourceType": "module",
             "interpreter": null,
             "body": [],

diff --git a/...el-parser/test/fixtures/experimental/module-blocks/valid-import-module-blocks/output.json b/...el-parser/test/fixtures/experimental/module-blocks/valid-import-module-blocks/output.json
@@ -24,7 +24,7 @@
               "start":10,"end":48,"loc":{"start":{"line":1,"column":10,"index":10},"end":{"line":3,"column":1,"index":48}},
               "body": {
                 "type": "Program",
-                "start":21,"end":48,"loc":{"start":{"line":2,"column":2,"index":21},"end":{"line":3,"column":1,"index":48}},
+                "start":21,"end":46,"loc":{"start":{"line":2,"column":2,"index":21},"end":{"line":2,"column":27,"index":46}},
                 "sourceType": "module",
                 "interpreter": null,
                 "body": [

diff --git a/...ser/test/fixtures/experimental/module-blocks/valid-module-blocks-as-arguments/output.json b/...ser/test/fixtures/experimental/module-blocks/valid-module-blocks-as-arguments/output.json
@@ -33,7 +33,7 @@
                   "start":26,"end":66,"loc":{"start":{"line":1,"column":26,"index":26},"end":{"line":3,"column":1,"index":66}},
                   "body": {
                     "type": "Program",
-                    "start":39,"end":66,"loc":{"start":{"line":2,"column":4,"index":39},"end":{"line":3,"column":1,"index":66}},
+                    "start":39,"end":64,"loc":{"start":{"line":2,"column":4,"index":39},"end":{"line":2,"column":29,"index":64}},
                     "sourceType": "module",
                     "interpreter": null,
                     "body": [

diff --git a/...el-parser/test/fixtures/experimental/module-blocks/valid-nested-module-blocks/output.json b/...el-parser/test/fixtures/experimental/module-blocks/valid-nested-module-blocks/output.json
@@ -24,7 +24,7 @@
               "start":8,"end":62,"loc":{"start":{"line":1,"column":8,"index":8},"end":{"line":5,"column":1,"index":62}},
               "body": {
                 "type": "Program",
-                "start":19,"end":62,"loc":{"start":{"line":2,"column":2,"index":19},"end":{"line":5,"column":1,"index":62}},
+                "start":19,"end":60,"loc":{"start":{"line":2,"column":2,"index":19},"end":{"line":4,"column":4,"index":60}},
                 "sourceType": "module",
                 "interpreter": null,
                 "body": [
@@ -36,7 +36,7 @@
                       "start":19,"end":59,"loc":{"start":{"line":2,"column":2,"index":19},"end":{"line":4,"column":3,"index":59}},
                       "body": {
                         "type": "Program",
-                        "start":32,"end":59,"loc":{"start":{"line":3,"column":4,"index":32},"end":{"line":4,"column":3,"index":59}},
+                        "start":32,"end":55,"loc":{"start":{"line":3,"column":4,"index":32},"end":{"line":3,"column":27,"index":55}},
                         "sourceType": "module",
                         "interpreter": null,
                         "body": [

diff --git a/...arser/test/fixtures/experimental/module-blocks/valid-singleline-module-blocks/output.json b/...arser/test/fixtures/experimental/module-blocks/valid-singleline-module-blocks/output.json
@@ -24,7 +24,7 @@
               "start":10,"end":45,"loc":{"start":{"line":1,"column":10,"index":10},"end":{"line":1,"column":45,"index":45}},
               "body": {
                 "type": "Program",
-                "start":19,"end":45,"loc":{"start":{"line":1,"column":19,"index":19},"end":{"line":1,"column":45,"index":45}},
+                "start":19,"end":43,"loc":{"start":{"line":1,"column":19,"index":19},"end":{"line":1,"column":43,"index":43}},
                 "sourceType": "module",
                 "interpreter": null,
                 "body": [

diff --git a/...st/fixtures/experimental/module-blocks/valid-top-level-await-in-module-blocks/output.json b/...st/fixtures/experimental/module-blocks/valid-top-level-await-in-module-blocks/output.json
@@ -15,7 +15,7 @@
           "start":0,"end":18,"loc":{"start":{"line":1,"column":0,"index":0},"end":{"line":1,"column":18,"index":18}},
           "body": {
             "type": "Program",
-            "start":9,"end":18,"loc":{"start":{"line":1,"column":9,"index":9},"end":{"line":1,"column":18,"index":18}},
+            "start":9,"end":16,"loc":{"start":{"line":1,"column":9,"index":9},"end":{"line":1,"column":16,"index":16}},
             "sourceType": "module",
             "interpreter": null,
             "body": [