Skip to content

Commit

Permalink
Fix: not set ecmaVersion to 6 when sourceType=module (fixes #9687) (#…
Browse files Browse the repository at this point in the history 
…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.
  • Loading branch information
@aladdin-add @not-an-aardvark
aladdin-add authored and not-an-aardvark committed May 11, 2019
1 parent 9484e9e commit f47d72c
Show file tree
Hide file tree
Showing 41 changed files with 438 additions and 428 deletions.
26 changes: 12 additions & 14 deletions lib/linter.js
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
32 changes: 24 additions & 8 deletions tests/lib/linter.js
View file
Expand Up @@ -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 => {
Expand All @@ -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 => {
Expand Down Expand Up @@ -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);
});

Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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) - ()");
});
Expand Down Expand Up @@ -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 }
});

Expand Down
24 changes: 12 additions & 12 deletions tests/lib/rules/block-scoped-var.js
View file
Expand Up @@ -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; }",
Expand All @@ -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 } },
Expand All @@ -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 } },
Expand Down Expand Up @@ -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" }] },
Expand Down Expand Up @@ -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" }]
},
{
Expand Down

0 comments on commit f47d72c

Please sign in to comment.