Skip to content
This repository has been archived by the owner on Aug 18, 2021. It is now read-only.

RFC: Do not set a default ecmaVersion or sourceType #458

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 33 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ $ npm install [email protected] babel-eslint@6 --save-dev
```json
{
"parser": "babel-eslint",
"parserOptions": {
"ecmaVersion": 8,
"sourceType": "module"
},
"rules": {
"strict": 0
}
Expand All @@ -76,19 +80,42 @@ Check out the [ESLint docs](http://eslint.org/docs/rules/) for all possible rule

### Configuration

`sourceType` can be set to `'module'`(default) or `'script'` if your code isn't using ECMAScript modules.
`allowImportExportEverywhere` can be set to true to allow import and export declarations to appear anywhere a statement is allowed if your build environment supports that. By default, import and export declarations can only appear at a program's top level.
`codeFrame` can be set to false to disable the code frame in the reporter. This is useful since some eslint formatters don't play well with it.
* `sourceType`, `ecmaVersion`, `ecmaFeatures.globalReturn` and `ecmaFeatures.impliedStrict` should be set in accordance with http://eslint.org/docs/user-guide/configuring#specifying-parser-options.
- `ecmaVersion` is disregarded by `babel-eslint`, however, some ESlint rules depend on this value being set correctly.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would it take to make the first part not true?

- If `sourceType` is not set to `"module"`, then using `import`/`export` will error.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, files will not automatically be in strict mode.

* `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. By default, import and export declarations can only appear at a program's top level.
* `allowSuperOutsideMethod` (default `true`) can be set to `false` to disallow use of `super` outside of methods. **Note:** Babel's default is `false`.
* `codeFrame` can be set to false to disable the code frame in the reporter. This is useful since some ESlint formatters don't play well with it.

**.eslintrc**
#### `.eslintrc`

**Recommended:**

```json
{
"parser": "babel-eslint",
"parserOptions": {
"ecmaVersion": 8,
"sourceType": "module"
}
}
```

**Defaults:**

```json
{
"parser": "babel-eslint",
"parserOptions": {
"sourceType": "module",
"ecmaVersion": 5,
"sourceType": "script",
"ecmaFeatures": {
"globalReturn": false,
"impliedStrict": false
},
"allowImportExportEverywhere": false,
"codeFrame": false
"allowSuperOutsideMethod": true,
"codeFrame": true
}
}
```
Expand Down
43 changes: 19 additions & 24 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ var traverse = require("babel-traverse").default;
var codeFrame = require("babel-code-frame");

var hasPatched = false;
var eslintOptions = {};

function createModule(filename) {
var mod = new Module(filename);
Expand Down Expand Up @@ -49,12 +48,6 @@ function monkeypatch() {
var escope = require(escopeLoc);
var analyze = escope.analyze;
escope.analyze = function (ast, opts) {
opts.ecmaVersion = eslintOptions.ecmaVersion;
opts.sourceType = eslintOptions.sourceType;
if (eslintOptions.globalReturn !== undefined) {
opts.nodejsScope = eslintOptions.globalReturn;
}

var results = analyze.call(this, ast, opts);
return results;
};
Expand Down Expand Up @@ -358,33 +351,23 @@ function monkeypatch() {
}

exports.parse = function (code, options) {
options = options || {};
eslintOptions.ecmaVersion = options.ecmaVersion = options.ecmaVersion || 6;
eslintOptions.sourceType = options.sourceType = options.sourceType || "module";
eslintOptions.allowImportExportEverywhere = options.allowImportExportEverywhere = options.allowImportExportEverywhere || false;
if (options.sourceType === "module") {
eslintOptions.globalReturn = false;
} else {
delete eslintOptions.globalReturn;
}

try {
monkeypatch();
} catch (err) {
console.error(err.stack);
process.exit(1);
}

return exports.parseNoPatch(code, options);
return exports.parseNoPatch(code, options || {});
};

exports.parseNoPatch = function (code, options) {
// Some ESlint APIs (like `getScope()`) and some rules change behavior based
// on the values of "ecmaVersion", "sourceType", "globalReturn" and
// "impliedStrict" as specified in "parserOptions". For this reason, no
// defaults are set that differ from ESlint's own defaults.
var opts = {
codeFrame: options.hasOwnProperty("codeFrame") ? options.codeFrame : true,
codeFrame: "codeFrame" in options ? options.codeFrame : true,
sourceType: options.sourceType,
allowImportExportEverywhere: options.allowImportExportEverywhere, // consistent with espree
allowReturnOutsideFunction: true,
allowSuperOutsideMethod: true,
plugins: [
"flow",
"jsx",
Expand All @@ -401,7 +384,19 @@ exports.parseNoPatch = function (code, options) {
"objectRestSpread",
"trailingFunctionCommas",
"dynamicImport"
]
],
// espree configurable via "parserOptions":
allowReturnOutsideFunction:
options.ecmaFeatures &&
options.ecmaFeatures.globalReturn || false,
strictMode:
options.ecmaFeatures &&
options.ecmaFeatures.impliedStrict || false,
// defaults consistent with espree behavior:
allowImportExportEverywhere:
options.allowImportExportEverywhere || false,
allowSuperOutsideMethod: "allowSuperOutsideMethod" in options ?
options.allowSuperOutsideMethod : true,
};

var ast;
Expand Down
41 changes: 34 additions & 7 deletions test/babel-eslint.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ function lookup(obj, keypath, backwardsDepth) {
.reduce((base, segment) => { return base && base[segment], obj; });
}

function parseAndAssertSame(code) {
var esAST = espree.parse(code, {
function parseAndAssertSame(code, captureParseErrors) {
var options = {
ecmaFeatures: {
// enable JSX parsing
jsx: true,
Expand All @@ -59,8 +59,27 @@ function parseAndAssertSame(code) {
attachComment: true,
ecmaVersion: 8,
sourceType: "module"
});
var babylonAST = babelEslint.parse(code);
};
var errors = { espree: null, babylon: null };
try {
var esAST = espree.parse(code, options);
errors.espree = false;
} catch (err) {
errors.espree = err;
}
try {
var babylonAST = babelEslint.parse(code, options);
errors.babylon = false;
} catch (err) {
errors.babylon = err;
}
if (captureParseErrors) {
return errors;
} else if (errors.espree) {
throw errors.espree;
} else if (errors.babylon) {
throw errors.babylon;
}
try {
assertImplementsAST(esAST, babylonAST);
} catch (err) {
Expand Down Expand Up @@ -416,10 +435,18 @@ describe("babylon-to-esprima", () => {
);
});

it("do not allow import export everywhere", () => {
it("do not allow import export everywhere (espree)", () => {
assert.throws(() => {
var errors = parseAndAssertSame("function F() { import a from \"a\"; }", true);
throw errors.espree;
}, /'import' and 'export' may only appear at the top level/);
});

it("do not allow import export everywhere (babylon)", () => {
assert.throws(() => {
parseAndAssertSame("function F() { import a from \"a\"; }");
}, /SyntaxError: 'import' and 'export' may only appear at the top level/);
var errors = parseAndAssertSame("function F() { import a from \"a\"; }", true);
throw errors.babylon;
}, /'import' and 'export' may only appear at the top level/);
});

it("return outside function", () => {
Expand Down
21 changes: 2 additions & 19 deletions test/non-regression.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ function verifyAndAssertMessages(code, rules, expectedMessages, sourceType, over
experimentalObjectRestSpread: true,
globalReturn: true
},
sourceType
sourceType: sourceType || "module"
}
};

Expand Down Expand Up @@ -1566,7 +1566,7 @@ describe("verify", () => {
verifyAndAssertMessages(
"var leakedGlobal = 1;",
{ "no-implicit-globals": 1 },
[],
[ "1:5 Implicit global variable, assign as global property instead. no-implicit-globals" ],
null,
{
env: {},
Expand All @@ -1575,23 +1575,6 @@ describe("verify", () => {
);
});

it("allowImportExportEverywhere option (#327)", () => {
verifyAndAssertMessages(
unpad(`
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", allowImportExportEverywhere: true }
}
);
});

it("with does not crash parsing in script mode (strict off) #171", () => {
verifyAndAssertMessages(
"with (arguments) { length; }",
Expand Down