[babel 8] Align allow* parser options with ESLint behavior (#13921)

nicolo-ribaudo · web-flow · commit de28707dfebd · 2021-11-04T07:56:06.000+01:00
* [babel 8] Align `allow*` parser options with ESLint behavior

- The `@babel/eslint-parser` `allowImportExportEverywhere` option is removed; users can pass it to `parserOpts`
- `allowSuperOutsideMethod` is disabled
- `allowReturnOutsideFunction` is inferred from `ecmaFeatures.globalReturn`

* Update failing tests
diff --git a/eslint/babel-eslint-parser/README.md b/eslint/babel-eslint-parser/README.md
@@ -51,6 +51,7 @@ Additional configuration options can be set in your ESLint configuration under t
 
 - `requireConfigFile` (default `true`) can be set to `false` to allow @babel/eslint-parser to run on files that do not have a Babel configuration associated with them. This can be useful for linting files that are not transformed by Babel (such as tooling configuration files), though we recommend using the default parser via [glob-based configuration](https://eslint.org/docs/user-guide/configuring/configuration-files#configuration-based-on-glob-patterns). Note: @babel/eslint-parser will not parse any experimental syntax when no configuration file is found.
 - `sourceType` can be set to `"module"`(default) or `"script"` if your code isn't using ECMAScript modules.
+<!-- TODO(Babel 8): Remove this -->
 - `allowImportExportEverywhere` (default `false`) can be set to `true` to allow import and export declarations to appear anywhere a statement is allowed if your build environment supports that. Otherwise import and export declarations can only appear at a program's top level.
 - `ecmaFeatures.globalReturn` (default `false`) allow return statements in the global scope when used with `sourceType: "script"`.
 - `babelOptions` is an object containing Babel configuration [options](https://babeljs.io/docs/en/options) that are passed to Babel's parser at runtime. For cases where users might not want to use a Babel configuration file or are running Babel through another tool (such as Webpack with `babel-loader`).
@@ -97,13 +98,13 @@ This configuration is useful for monorepo, when you are running ESLint on every
 
 ```js
 module.exports = {
-  "parser": "@babel/eslint-parser",
-  "parserOptions": {
-    "babelOptions": {
-      "rootMode": "upward"
-    }
-  }
-}
+  parser: "@babel/eslint-parser",
+  parserOptions: {
+    babelOptions: {
+      rootMode: "upward",
+    },
+  },
+};
 ```
 
 ### Run
diff --git a/eslint/babel-eslint-parser/src/analyze-scope.cjs b/eslint/babel-eslint-parser/src/analyze-scope.cjs
@@ -340,8 +340,7 @@ module.exports = function analyzeScope(ast, parserOptions, client) {
     directive: false,
     nodejsScope:
       ast.sourceType === "script" &&
-      (parserOptions.ecmaFeatures &&
-        parserOptions.ecmaFeatures.globalReturn) === true,
+      parserOptions.ecmaFeatures?.globalReturn === true,
     impliedStrict: false,
     sourceType: ast.sourceType,
     ecmaVersion: parserOptions.ecmaVersion,
diff --git a/eslint/babel-eslint-parser/src/configuration.cjs b/eslint/babel-eslint-parser/src/configuration.cjs
@@ -4,7 +4,6 @@ exports.normalizeESLintConfig = function (options) {
     // ESLint sets ecmaVersion: undefined when ecmaVersion is not set in the config.
     ecmaVersion = 2020,
     sourceType = "module",
-    allowImportExportEverywhere = false,
     requireConfigFile = true,
     ...otherOptions
   } = options;
@@ -13,7 +12,6 @@ exports.normalizeESLintConfig = function (options) {
     babelOptions: { cwd: process.cwd(), ...babelOptions },
     ecmaVersion: ecmaVersion === "latest" ? 1e8 : ecmaVersion,
     sourceType,
-    allowImportExportEverywhere,
     requireConfigFile,
     ...otherOptions,
   };
diff --git a/eslint/babel-eslint-parser/src/worker/configuration.cjs b/eslint/babel-eslint-parser/src/worker/configuration.cjs
@@ -26,9 +26,16 @@ function normalizeParserOptions(options) {
     filename: options.filePath,
     ...options.babelOptions,
     parserOpts: {
-      allowImportExportEverywhere: options.allowImportExportEverywhere,
-      allowReturnOutsideFunction: true,
-      allowSuperOutsideMethod: true,
+      ...(process.env.BABEL_8_BREAKING
+        ? {}
+        : {
+            allowImportExportEverywhere:
+              options.allowImportExportEverywhere ?? false,
+            allowSuperOutsideMethod: true,
+          }),
+      allowReturnOutsideFunction:
+        options.ecmaFeatures?.globalReturn ??
+        (process.env.BABEL_8_BREAKING ? false : true),
       ...options.babelOptions.parserOpts,
       plugins: getParserPlugins(options.babelOptions),
       // skip comment attaching for parsing performance
diff --git a/eslint/babel-eslint-parser/test/index.js b/eslint/babel-eslint-parser/test/index.js
@@ -68,8 +68,6 @@ describe("Babel and Espree", () => {
       globalReturn: true,
       // enable implied strict mode (if ecmaVersion >= 5)
       impliedStrict: true,
-      // allow experimental object rest/spread
-      experimentalObjectRestSpread: true,
     },
     tokens: true,
     loc: true,
@@ -78,7 +76,11 @@ describe("Babel and Espree", () => {
     sourceType: "module",
   };
 
-  function parseAndAssertSame(code, /* optional */ eslintVersion) {
+  function parseAndAssertSame(
+    code,
+    eslintVersion = undefined,
+    babelEcmaFeatures = null,
+  ) {
     code = unpad(code);
 
     if (eslintVersion !== 8) {
@@ -91,6 +93,7 @@ describe("Babel and Espree", () => {
         eslintVisitorKeys: true,
         eslintScopeManager: true,
         babelOptions: BABEL_OPTIONS,
+        ecmaFeatures: babelEcmaFeatures,
       }).ast;
 
       deeplyRemoveProperties(babelAST, PROPS_TO_REMOVE);
@@ -113,6 +116,7 @@ describe("Babel and Espree", () => {
         eslintVisitorKeys: true,
         eslintScopeManager: true,
         babelOptions: BABEL_OPTIONS,
+        ecmaFeatures: babelEcmaFeatures,
       }).ast;
 
       deeplyRemoveProperties(babelAST, PROPS_TO_REMOVE);
@@ -851,24 +855,122 @@ describe("Babel and Espree", () => {
 
     it("do not allow import export everywhere", () => {
       expect(() => {
-        parseAndAssertSame('function F() { import a from "a"; }');
-      }).toThrow(
-        new SyntaxError(
-          "'import' and 'export' may only appear at the top level",
-        ),
-      );
-    });
-
-    it("return outside function", () => {
-      parseAndAssertSame("return;");
+        parseForESLint('function F() { import a from "a"; }', {
+          babelOptions: BABEL_OPTIONS,
+        });
+      }).toThrow(/'import' and 'export' may only appear at the top level/);
     });
 
-    it("super outside method", () => {
+    it("allowImportExportEverywhere", () => {
       expect(() => {
-        parseAndAssertSame("function F() { super(); }");
-      }).toThrow(new SyntaxError("'super' keyword outside a method"));
+        parseForESLint('function F() { import a from "a"; }', {
+          babelOptions: {
+            ...BABEL_OPTIONS,
+            parserOpts: {
+              allowImportExportEverywhere: true,
+            },
+          },
+        });
+      }).not.toThrow();
     });
 
+    if (!process.env.BABEL_8_BREAKING) {
+      it("top-level allowImportExportEverywhere", () => {
+        expect(() => {
+          parseForESLint('function F() { import a from "a"; }', {
+            babelOptions: BABEL_OPTIONS,
+            allowImportExportEverywhere: true,
+          });
+        }).not.toThrow();
+      });
+    }
+
+    if (process.env.BABEL_8_BREAKING) {
+      it("return outside function with ecmaFeatures.globalReturn: true", () => {
+        parseAndAssertSame("return;", /* version */ undefined, {
+          globalReturn: true,
+        });
+      });
+
+      it("return outside function with ecmaFeatures.globalReturn: false", () => {
+        expect(() =>
+          parseForESLint("return;", {
+            babelOptions: BABEL_OPTIONS,
+            ecmaVersion: { globalReturn: false },
+          }),
+        ).toThrow(new SyntaxError("'return' outside of function. (1:0)"));
+
+        expect(() =>
+          parseForESLint8("return;", {
+            babelOptions: BABEL_OPTIONS,
+            ecmaVersion: { globalReturn: false },
+          }),
+        ).toThrow(new SyntaxError("'return' outside of function. (1:0)"));
+      });
+
+      it("return outside function without ecmaFeatures.globalReturn", () => {
+        expect(() =>
+          parseForESLint("return;", { babelOptions: BABEL_OPTIONS }),
+        ).toThrow(new SyntaxError("'return' outside of function. (1:0)"));
+
+        expect(() =>
+          parseForESLint8("return;", { babelOptions: BABEL_OPTIONS }),
+        ).toThrow(new SyntaxError("'return' outside of function. (1:0)"));
+      });
+    } else {
+      it("return outside function", () => {
+        parseAndAssertSame("return;");
+      });
+    }
+
+    if (process.env.BABEL_8_BREAKING) {
+      it("super outside method", () => {
+        expect(() => {
+          parseForESLint("function F() { super(); }", {
+            babelOptions: BABEL_OPTIONS,
+          });
+        }).toThrow(
+          /`super\(\)` is only valid inside a class constructor of a subclass\./,
+        );
+      });
+
+      it("super outside method - enabled", () => {
+        expect(() => {
+          parseForESLint("function F() { super(); }", {
+            babelOptions: {
+              ...BABEL_OPTIONS,
+              parserOpts: {
+                allowSuperOutsideMethod: true,
+              },
+            },
+          });
+        }).not.toThrow();
+      });
+    } else {
+      it("super outside method", () => {
+        expect(() => {
+          parseForESLint("function F() { super(); }", {
+            babelOptions: BABEL_OPTIONS,
+          });
+        }).not.toThrow();
+      });
+
+      it("super outside method - disabled", () => {
+        expect(() => {
+          parseForESLint("function F() { super(); }", {
+            babelOptions: {
+              ...BABEL_OPTIONS,
+              parserOpts: {
+                allowSuperOutsideMethod: false,
+              },
+            },
+          });
+        }).toThrow(
+          /`super\(\)` is only valid inside a class constructor of a subclass\./,
+        );
+      });
+    }
+
     it("StringLiteral", () => {
       parseAndAssertSame("");
       parseAndAssertSame("");
diff --git a/eslint/babel-eslint-tests/test/integration/eslint/verify.js b/eslint/babel-eslint-tests/test/integration/eslint/verify.js
@@ -45,9 +45,10 @@ describe("verify", () => {
   });
 
   it("super keyword in class (issue #10)", () => {
-    verifyAndAssertMessages("class Foo { constructor() { super() } }", {
-      "no-undef": 1,
-    });
+    verifyAndAssertMessages(
+      "class Foo extends class {} { constructor() { super() } }",
+      { "no-undef": 1 },
+    );
   });
 
   it("Rest parameter in destructuring assignment (issue #11)", () => {
@@ -1549,7 +1550,9 @@ describe("verify", () => {
     );
   });
 
-  it("allowImportExportEverywhere option (#327)", () => {
+  const babel7 = process.env.BABEL_8_BREAKING ? it.skip : it;
+
+  babel7("allowImportExportEverywhere option (#327)", () => {
     verifyAndAssertMessages(
       `
         if (true) { import Foo from 'foo'; }
@@ -1570,6 +1573,29 @@ describe("verify", () => {
     );
   });
 
+  it("allowImportExportEverywhere @babel/parser option (#327)", () => {
+    verifyAndAssertMessages(
+      `
+        if (true) { import Foo from 'foo'; }
+        function foo() { import Bar from 'bar'; }
+        switch (a) { case 1: import FooBar from 'foobar'; }
+      `,
+      {},
+      [],
+      "module",
+      {
+        env: {},
+        parserOptions: {
+          ecmaVersion: 6,
+          sourceType: "module",
+          babelOptions: {
+            parserOpts: { allowImportExportEverywhere: true },
+          },
+        },
+      },
+    );
+  });
+
   it("with does not crash parsing in script mode (strict off) #171", () => {
     verifyAndAssertMessages("with (arguments) { length; }", {}, [], "script");
   });
