Fix: not set ecmaVersion to 6 when sourceType=module (fixes #9687) (#…

…11649) PR #11610 was aimed to fix the issue #9687, however it did not work, since ecmaVersion has been normalized to 6 (when sourceType is setting to 'module'). this PR removed the normalize behaviour, and fixed failing tests.
eslint · May 11, 2019 · f47d72c · f47d72c
1 parent 9484e9e
commit f47d72c
Show file tree

Hide file tree

Showing 41 changed files with 438 additions and 428 deletions.
diff --git a/lib/linter.js b/lib/linter.js
@@ -298,25 +298,15 @@ function getDirectiveComments(filename, ast, ruleMapper) {
 /**
  * Normalize ECMAScript version from the initial config
  * @param  {number} ecmaVersion ECMAScript version from the initial config
- * @param  {boolean} isModule Whether the source type is module or not
  * @returns {number} normalized ECMAScript version
  */
-function normalizeEcmaVersion(ecmaVersion, isModule) {
-
-    // Need at least ES6 for modules
-    if (isModule && (!ecmaVersion || ecmaVersion < 6)) {
-        return 6;
-    }
+function normalizeEcmaVersion(ecmaVersion) {
 
     /*
      * Calculate ECMAScript edition number from official year version starting with
      * ES2015, which corresponds with ES6 (or a difference of 2009).
      */
-    if (ecmaVersion >= 2015) {
-        return ecmaVersion - 2009;
-    }
-
-    return ecmaVersion;
+    return ecmaVersion >= 2015 ? ecmaVersion - 2009 : ecmaVersion;
 }
 
 const eslintEnvPattern = /\/\*\s*eslint-env\s(.+?)\*\//gu;
@@ -372,11 +362,19 @@ function resolveParserOptions(parserName, providedOptions, enabledEnvironments)
 
     if (isModule) {
 
-        // can't have global return inside of modules
+        /*
+         * can't have global return inside of modules
+         * TODO: espree validate parserOptions.globalReturn when sourceType is setting to module.(@aladdin-add)
+         */
         mergedParserOptions.ecmaFeatures = Object.assign({}, mergedParserOptions.ecmaFeatures, { globalReturn: false });
     }
 
-    mergedParserOptions.ecmaVersion = normalizeEcmaVersion(mergedParserOptions.ecmaVersion, isModule);
+    /*
+     * TODO: @aladdin-add
+     * 1. for a 3rd-party parser, do not normalize parserOptions
+     * 2. for espree, no need to do this (espree will do it)
+     */
+    mergedParserOptions.ecmaVersion = normalizeEcmaVersion(mergedParserOptions.ecmaVersion);
 
     return mergedParserOptions;
 }

diff --git a/tests/lib/linter.js b/tests/lib/linter.js
@@ -667,7 +667,7 @@ describe("Linter", () => {
         });
 
         it("should retrieve module scope correctly from an ES6 module", () => {
-            const config = { rules: { checker: "error" }, parserOptions: { sourceType: "module" } };
+            const config = { rules: { checker: "error" }, parserOptions: { ecmaVersion: 6, sourceType: "module" } };
             let spy;
 
             linter.defineRule("checker", context => {
@@ -685,7 +685,7 @@ describe("Linter", () => {
         });
 
         it("should retrieve function scope correctly when globalReturn is true", () => {
-            const config = { rules: { checker: "error" }, parserOptions: { ecmaFeatures: { globalReturn: true } } };
+            const config = { rules: { checker: "error" }, parserOptions: { ecmaVersion: 6, ecmaFeatures: { globalReturn: true } } };
             let spy;
 
             linter.defineRule("checker", context => {
@@ -807,7 +807,7 @@ describe("Linter", () => {
                 return { "Program:exit": spy };
             });
 
-            linter.verify(code, { rules: { checker: "error" }, parserOptions: { sourceType: "module" } }, filename, true);
+            linter.verify(code, { rules: { checker: "error" }, parserOptions: { ecmaVersion: 6, sourceType: "module" } }, filename, true);
             assert(spy && spy.calledOnce);
         });
 
@@ -1327,7 +1327,7 @@ describe("Linter", () => {
 
         it("variables should not be exported in the es6 module environment", () => {
             const code = "/* exported horse */\nvar horse = 'circus'";
-            const config = { rules: { checker: "error" }, parserOptions: { sourceType: "module" } };
+            const config = { rules: { checker: "error" }, parserOptions: { ecmaVersion: 6, sourceType: "module" } };
             let spy;
 
             linter.defineRule("checker", context => {
@@ -4409,25 +4409,41 @@ describe("Linter", () => {
 
         it("should properly parse import statements when sourceType is module", () => {
             const code = "import foo from 'foo';";
-            const messages = linter.verify(code, { parserOptions: { sourceType: "module" } });
+            const messages = linter.verify(code, { parserOptions: { ecmaVersion: 6, sourceType: "module" } });
 
             assert.strictEqual(messages.length, 0);
         });
 
         it("should properly parse import all statements when sourceType is module", () => {
             const code = "import * as foo from 'foo';";
-            const messages = linter.verify(code, { parserOptions: { sourceType: "module" } });
+            const messages = linter.verify(code, { parserOptions: { ecmaVersion: 6, sourceType: "module" } });
 
             assert.strictEqual(messages.length, 0);
         });
 
         it("should properly parse default export statements when sourceType is module", () => {
             const code = "export default function initialize() {}";
-            const messages = linter.verify(code, { parserOptions: { sourceType: "module" } });
+            const messages = linter.verify(code, { parserOptions: { ecmaVersion: 6, sourceType: "module" } });
 
             assert.strictEqual(messages.length, 0);
         });
 
+        // https://github.com/eslint/eslint/issues/9687
+        it("should report an error when invalid parserOptions found", () => {
+            let messages = linter.verify("", { parserOptions: { ecmaVersion: 222 } });
+
+            assert.deepStrictEqual(messages.length, 1);
+            assert.ok(messages[0].message.includes("Invalid ecmaVersion"));
+
+            messages = linter.verify("", { parserOptions: { sourceType: "foo" } });
+            assert.deepStrictEqual(messages.length, 1);
+            assert.ok(messages[0].message.includes("Invalid sourceType"));
+
+            messages = linter.verify("", { parserOptions: { ecmaVersion: 5, sourceType: "module" } });
+            assert.deepStrictEqual(messages.length, 1);
+            assert.ok(messages[0].message.includes("sourceType 'module' is not supported when ecmaVersion < 2015"));
+        });
+
         it("should not crash when invalid parentheses syntax is encountered", () => {
             linter.verify("left = (aSize.width/2) - ()");
         });
@@ -4456,7 +4472,7 @@ describe("Linter", () => {
             linter.defineRule("test", () => ({}));
             linter.verify("var a = 0;", {
                 env: { node: true },
-                parserOptions: { sourceType: "module" },
+                parserOptions: { ecmaVersion: 6, sourceType: "module" },
                 rules: { test: 2 }
             });
 

diff --git a/tests/lib/rules/block-scoped-var.js b/tests/lib/rules/block-scoped-var.js
@@ -23,7 +23,7 @@ ruleTester.run("block-scoped-var", rule, {
 
         // See issue https://github.com/eslint/eslint/issues/2242
         { code: "function f() { } f(); var exports = { f: f };", parserOptions: { ecmaVersion: 6 } },
-        { code: "var f = () => {}; f(); var exports = { f: f };", parserOptions: { sourceType: "module" } },
+        { code: "var f = () => {}; f(); var exports = { f: f };", parserOptions: { ecmaVersion: 6, sourceType: "module" } },
         "!function f(){ f; }",
         "function f() { } f(); var exports = { f: f };",
         "function f() { var a, b; { a = true; } b = a; }",
@@ -46,7 +46,7 @@ ruleTester.run("block-scoped-var", rule, {
         { code: "function myFunc(foo) {  \"use strict\";  var [ bar ]  = foo;  bar.hello();}", parserOptions: { ecmaVersion: 6 } },
         { code: "function myFunc(...foo) {  return foo;}", parserOptions: { ecmaVersion: 6 } },
         { code: "var f = () => { var g = f; }", parserOptions: { ecmaVersion: 6 } },
-        { code: "class Foo {}\nexport default Foo;", parserOptions: { sourceType: "module" } },
+        { code: "class Foo {}\nexport default Foo;", parserOptions: { ecmaVersion: 6, sourceType: "module" } },
         { code: "new Date", globals: { Date: false } },
         { code: "new Date", globals: {} },
         { code: "var eslint = require('eslint');", globals: { require: false } },
@@ -67,15 +67,15 @@ ruleTester.run("block-scoped-var", rule, {
         "a:;",
         "foo: while (true) { bar: for (var i = 0; i < 13; ++i) {if (i === 7) break foo; } }",
         "foo: while (true) { bar: for (var i = 0; i < 13; ++i) {if (i === 7) continue foo; } }",
-        { code: "const React = require(\"react/addons\");const cx = React.addons.classSet;", parserOptions: { sourceType: "module" }, globals: { require: false } },
+        { code: "const React = require(\"react/addons\");const cx = React.addons.classSet;", parserOptions: { ecmaVersion: 6, sourceType: "module" }, globals: { require: false } },
         { code: "var v = 1;  function x() { return v; };", parserOptions: { parserOptions: { ecmaVersion: 6 } } },
-        { code: "import * as y from \"./other.js\"; y();", parserOptions: { sourceType: "module" } },
-        { code: "import y from \"./other.js\"; y();", parserOptions: { sourceType: "module" } },
-        { code: "import {x as y} from \"./other.js\"; y();", parserOptions: { sourceType: "module" } },
-        { code: "var x; export {x};", parserOptions: { sourceType: "module" } },
-        { code: "var x; export {x as v};", parserOptions: { sourceType: "module" } },
-        { code: "export {x} from \"./other.js\";", parserOptions: { sourceType: "module" } },
-        { code: "export {x as v} from \"./other.js\";", parserOptions: { sourceType: "module" } },
+        { code: "import * as y from \"./other.js\"; y();", parserOptions: { ecmaVersion: 6, sourceType: "module" } },
+        { code: "import y from \"./other.js\"; y();", parserOptions: { ecmaVersion: 6, sourceType: "module" } },
+        { code: "import {x as y} from \"./other.js\"; y();", parserOptions: { ecmaVersion: 6, sourceType: "module" } },
+        { code: "var x; export {x};", parserOptions: { ecmaVersion: 6, sourceType: "module" } },
+        { code: "var x; export {x as v};", parserOptions: { ecmaVersion: 6, sourceType: "module" } },
+        { code: "export {x} from \"./other.js\";", parserOptions: { ecmaVersion: 6, sourceType: "module" } },
+        { code: "export {x as v} from \"./other.js\";", parserOptions: { ecmaVersion: 6, sourceType: "module" } },
         { code: "class Test { myFunction() { return true; }}", parserOptions: { ecmaVersion: 6 } },
         { code: "class Test { get flag() { return true; }}", parserOptions: { ecmaVersion: 6 } },
         { code: "var Test = class { myFunction() { return true; }}", parserOptions: { ecmaVersion: 6 } },
@@ -110,7 +110,7 @@ ruleTester.run("block-scoped-var", rule, {
 
         // https://github.com/eslint/eslint/issues/2967
         "(function () { foo(); })(); function foo() {}",
-        { code: "(function () { foo(); })(); function foo() {}", parserOptions: { sourceType: "module" } }
+        { code: "(function () { foo(); })(); function foo() {}", parserOptions: { ecmaVersion: 6, sourceType: "module" } }
     ],
     invalid: [
         { code: "function f(){ x; { var x; } }", errors: [{ messageId: "outOfScope", data: { name: "x" }, type: "Identifier" }] },
@@ -145,7 +145,7 @@ ruleTester.run("block-scoped-var", rule, {
         },
         {
             code: "{ var a = 0; } a;",
-            parserOptions: { sourceType: "module" },
+            parserOptions: { ecmaVersion: 6, sourceType: "module" },
             errors: [{ messageId: "outOfScope", data: { name: "a" }, type: "Identifier" }]
         },
         {