Skip to content

Commit

Permalink
Update: re-enable experimentalObjectRestSpread (fixes #9990) (#10230)
Browse files Browse the repository at this point in the history
* Update: re-enable experimentalObjectRestSpread (fixes #9990)

And it generates a deprecation warning to show that the option has been
deprecated.

* move warning to config-validator

* revert some unnecessary changes

* move ecmaVersion=9 to resolveParserOptions function

* change error code of deprecation warnings

* make tests robuster

* fix Node 6.x problem.

* fix Node 6.x problem.
  • Loading branch information
mysticatea authored and not-an-aardvark committed Apr 28, 2018
1 parent f9c7371 commit a82cbea
Show file tree
Hide file tree
Showing 11 changed files with 192 additions and 17 deletions.
42 changes: 29 additions & 13 deletions lib/config/config-validator.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
// Requirements
//------------------------------------------------------------------------------

const ajv = require("../util/ajv"),
const path = require("path"),
ajv = require("../util/ajv"),
lodash = require("lodash"),
configSchema = require("../../conf/config-schema.js"),
util = require("util");
Expand All @@ -21,6 +22,12 @@ const ruleValidators = new WeakMap();
//------------------------------------------------------------------------------
let validateSchema;

// Defitions for deprecation warnings.
const deprecationWarningMessages = Object.freeze({
ESLINT_LEGACY_ECMAFEATURES: "The 'ecmaFeatures' config file property is deprecated, and has no effect.",
ESLINT_LEGACY_OBJECT_REST_SPREAD: "The 'parserOptions.ecmaFeatures.experimentalObjectRestSpread' option is deprecated. Use 'parserOptions.ecmaVersion' instead."
});

/**
* Gets a complete options schema for a rule.
* @param {{create: Function, schema: (Array|null)}} rule A new-style rule object
Expand Down Expand Up @@ -194,18 +201,18 @@ function formatErrors(errors) {
* for each unique file path, but repeated invocations with the same file path have no effect.
* No warnings are emitted if the `--no-deprecation` or `--no-warnings` Node runtime flags are active.
* @param {string} source The name of the configuration source to report the warning for.
* @param {string} errorCode The warning message to show.
* @returns {void}
*/
const emitEcmaFeaturesWarning = lodash.memoize(source => {

/*
* util.deprecate seems to be the only way to emit a warning in Node 4.x while respecting the --no-warnings flag.
* (In Node 6+, process.emitWarning could be used instead.)
*/
util.deprecate(
() => {},
`[eslint] The 'ecmaFeatures' config file property is deprecated, and has no effect. (found in ${source})`
)();
const emitDeprecationWarning = lodash.memoize((source, errorCode) => {
const rel = path.relative(process.cwd(), source);
const message = deprecationWarningMessages[errorCode];

process.emitWarning(
`${message} (found in "${rel}")`,
"DeprecationWarning",
errorCode
);
});

/**
Expand All @@ -221,8 +228,17 @@ function validateConfigSchema(config, source) {
throw new Error(`ESLint configuration in ${source} is invalid:\n${formatErrors(validateSchema.errors)}`);
}

if (Object.prototype.hasOwnProperty.call(config, "ecmaFeatures")) {
emitEcmaFeaturesWarning(source);
if (Object.hasOwnProperty.call(config, "ecmaFeatures")) {
emitDeprecationWarning(source, "ESLINT_LEGACY_ECMAFEATURES");
}

if (
(config.parser || "espree") === "espree" &&
config.parserOptions &&
config.parserOptions.ecmaFeatures &&
config.parserOptions.ecmaFeatures.experimentalObjectRestSpread
) {
emitDeprecationWarning(source, "ESLINT_LEGACY_OBJECT_REST_SPREAD");
}
}

Expand Down
17 changes: 14 additions & 3 deletions lib/linter.js
Original file line number Diff line number Diff line change
Expand Up @@ -410,11 +410,12 @@ function normalizeVerifyOptions(providedOptions) {

/**
* Combines the provided parserOptions with the options from environments
* @param {string} parserName The parser name which uses this options.
* @param {Object} providedOptions The provided 'parserOptions' key in a config
* @param {Environment[]} enabledEnvironments The environments enabled in configuration and with inline comments
* @returns {Object} Resulting parser options after merge
*/
function resolveParserOptions(providedOptions, enabledEnvironments) {
function resolveParserOptions(parserName, providedOptions, enabledEnvironments) {
const parserOptionsFromEnv = enabledEnvironments
.filter(env => env.parserOptions)
.reduce((parserOptions, env) => ConfigOps.merge(parserOptions, env.parserOptions), {});
Expand All @@ -431,6 +432,16 @@ function resolveParserOptions(providedOptions, enabledEnvironments) {

mergedParserOptions.ecmaVersion = normalizeEcmaVersion(mergedParserOptions.ecmaVersion, isModule);

// TODO: For backward compatibility. Will remove on v6.0.0.
if (
parserName === DEFAULT_PARSER_NAME &&
mergedParserOptions.ecmaFeatures &&
mergedParserOptions.ecmaFeatures.experimentalObjectRestSpread &&
(!mergedParserOptions.ecmaVersion || mergedParserOptions.ecmaVersion < 9)
) {
mergedParserOptions.ecmaVersion = 9;
}

return mergedParserOptions;
}

Expand Down Expand Up @@ -923,9 +934,9 @@ module.exports = class Linter {
.map(envName => this.environments.get(envName))
.filter(env => env);

const parserOptions = resolveParserOptions(config.parserOptions || {}, enabledEnvs);
const configuredGlobals = resolveGlobals(config.globals || {}, enabledEnvs);
const parserName = config.parser || DEFAULT_PARSER_NAME;
const parserOptions = resolveParserOptions(parserName, config.parserOptions || {}, enabledEnvs);
const configuredGlobals = resolveGlobals(config.globals || {}, enabledEnvs);
const settings = config.settings || {};

if (!lastSourceCodes.get(this)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
root: true

parser: ./parser.js
parserOptions:
ecmaFeatures:
experimentalObjectRestSpread: true
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports.parse = () => ({ type: "Program", body: [] })
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
root: true

parserOptions:
ecmaFeatures:
experimentalObjectRestSpread: true
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
root: true

extends:
- ./common.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
parserOptions:
ecmaFeatures:
experimentalObjectRestSpread: true
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
root: true

parserOptions:
ecmaFeatures:
experimentalObjectRestSpread: true
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
rules:
no-undef: off
115 changes: 114 additions & 1 deletion tests/lib/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,14 @@ function assertConfigsEqual(actual, expected) {
}
}

/**
* Wait for the next tick.
* @returns {Promise<void>} -
*/
function nextTick() {
return new Promise(resolve => process.nextTick(resolve));
}

//------------------------------------------------------------------------------
// Tests
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -1086,7 +1094,6 @@ describe("Config", () => {
});
});


describe("with overrides", () => {

/**
Expand Down Expand Up @@ -1359,6 +1366,112 @@ describe("Config", () => {
assertConfigsEqual(actual, expected);
});
});

describe("deprecation warnings", () => {
let warning = null;

function onWarning(w) { // eslint-disable-line require-jsdoc

// Node.js 6.x does not have 'w.code' property.
if (!w.hasOwnProperty("code") || typeof w.code === "string" && w.code.startsWith("ESLINT_")) {
warning = w;
}
}

beforeEach(() => {
warning = null;
process.on("warning", onWarning);
});
afterEach(() => {
process.removeListener("warning", onWarning);
});

it("should emit a deprecation warning if 'ecmaFeatures' is given.", () => Promise.resolve()
.then(() => {
const cwd = path.resolve(__dirname, "../fixtures/config-file/ecma-features/");
const config = new Config({ cwd }, linter);

config.getConfig("test.js");

// Wait for "warning" event.
return nextTick();
})
.then(() => {
assert.notStrictEqual(warning, null);
assert.strictEqual(
warning.message,
`The 'ecmaFeatures' config file property is deprecated, and has no effect. (found in "tests${path.sep}fixtures${path.sep}config-file${path.sep}ecma-features${path.sep}.eslintrc.yml")`
);
}));

it("should emit a deprecation warning if 'parserOptions.ecmaFeatures.experimentalObjectRestSpread' is given.", () => Promise.resolve()
.then(() => {
const cwd = path.resolve(__dirname, "../fixtures/config-file/experimental-object-rest-spread/basic/");
const config = new Config({ cwd }, linter);

config.getConfig("test.js");

// Wait for "warning" event.
return nextTick();
})
.then(() => {
assert.notStrictEqual(warning, null);
assert.strictEqual(
warning.message,
`The 'parserOptions.ecmaFeatures.experimentalObjectRestSpread' option is deprecated. Use 'parserOptions.ecmaVersion' instead. (found in "tests${path.sep}fixtures${path.sep}config-file${path.sep}experimental-object-rest-spread${path.sep}basic${path.sep}.eslintrc.yml")`
);
}));

it("should emit a deprecation warning if 'parserOptions.ecmaFeatures.experimentalObjectRestSpread' is given in a parent config.", () => Promise.resolve()
.then(() => {
const cwd = path.resolve(__dirname, "../fixtures/config-file/experimental-object-rest-spread/subdir/");
const config = new Config({ cwd }, linter);

config.getConfig("lib/test.js");

// Wait for "warning" event.
return nextTick();
})
.then(() => {
assert.notStrictEqual(warning, null);
assert.strictEqual(
warning.message,
`The 'parserOptions.ecmaFeatures.experimentalObjectRestSpread' option is deprecated. Use 'parserOptions.ecmaVersion' instead. (found in "tests${path.sep}fixtures${path.sep}config-file${path.sep}experimental-object-rest-spread${path.sep}subdir${path.sep}.eslintrc.yml")`
);
}));

it("should emit a deprecation warning if 'parserOptions.ecmaFeatures.experimentalObjectRestSpread' is given in a shareable config.", () => Promise.resolve()
.then(() => {
const cwd = path.resolve(__dirname, "../fixtures/config-file/experimental-object-rest-spread/extends/");
const config = new Config({ cwd }, linter);

config.getConfig("test.js");

// Wait for "warning" event.
return nextTick();
})
.then(() => {
assert.notStrictEqual(warning, null);
assert.strictEqual(
warning.message,
`The 'parserOptions.ecmaFeatures.experimentalObjectRestSpread' option is deprecated. Use 'parserOptions.ecmaVersion' instead. (found in "tests${path.sep}fixtures${path.sep}config-file${path.sep}experimental-object-rest-spread${path.sep}extends${path.sep}common.yml")`
);
}));

it("should NOT emit a deprecation warning even if 'parserOptions.ecmaFeatures.experimentalObjectRestSpread' is given, if parser is not espree.", () => Promise.resolve()
.then(() => {
const cwd = path.resolve(__dirname, "../fixtures/config-file/experimental-object-rest-spread/another-parser/");
const config = new Config({ cwd }, linter);

config.getConfig("test.js");

// Wait for "warning" event.
return nextTick();
})
.then(() => {
assert.strictEqual(warning, null);
}));
});
});

describe("Plugin Environments", () => {
Expand Down
9 changes: 9 additions & 0 deletions tests/lib/linter.js
Original file line number Diff line number Diff line change
Expand Up @@ -3492,6 +3492,15 @@ describe("Linter", () => {
linter.verify("x", { rules: { "foo-bar-baz": "error" } });
assert(spy.calledOnce);
});

it("should parse ES2018 code for backward compatibility if 'parserOptions.ecmaFeatures.experimentalObjectRestSpread' is given.", () => {
const messages = linter.verify(
"async function* f() { let {a, ...rest} = { a, ...obj }; }",
{ parserOptions: { ecmaFeatures: { experimentalObjectRestSpread: true } } }
);

assert(messages.length === 0);
});
});

describe("Variables and references", () => {
Expand Down

0 comments on commit a82cbea

Please sign in to comment.