Skip to content

Commit

Permalink
Revert "Fix: clone config before validating (fixes #12592) (#13034)"
Browse files Browse the repository at this point in the history
This reverts commit 7fb45cf.
  • Loading branch information
aladdin-add committed Jun 30, 2020
1 parent 4e679b1 commit 11aa42b
Show file tree
Hide file tree
Showing 7 changed files with 1 addition and 119 deletions.
29 changes: 1 addition & 28 deletions lib/cli-engine/config-array-factory.js
Expand Up @@ -697,33 +697,6 @@ class ConfigArrayFactory {
ctx.matchBasePath
);

/**
* Cloning the rule's config as we are setting `useDefaults` to true`
* which mutates the config with its default value if present. And when we
* refer to a same variable for config for different rules, that referred variable will
* be mutated and it will be used for both.
*
* Example:
*
* const commonRuleConfig = ['error', {}];
*
* Now if we use this variable as a config for rules like this
*
* {
* rules: {
* "a" : commonRuleConfig,
* "b" : commonRuleConfig,
* }
* }
*
* And if these rules have default values in their schema, their
* config will be mutated with default values, the mutated `commonRuleConfig` will be used for `b` as well and it probably
* throw schema voilation errors.
*
* Refer https://github.com/eslint/eslint/issues/12592
*/
const clonedRulesConfig = rules && JSON.parse(JSON.stringify((rules)));

// Flatten `extends`.
for (const extendName of extendList.filter(Boolean)) {
yield* this._loadExtends(extendName, ctx);
Expand Down Expand Up @@ -758,7 +731,7 @@ class ConfigArrayFactory {
processor,
reportUnusedDisableDirectives,
root,
rules: clonedRulesConfig,
rules,
settings
};

Expand Down

This file was deleted.

4 changes: 0 additions & 4 deletions tests/fixtures/config-file/cloned-config/eslintConfig.js

This file was deleted.

13 changes: 0 additions & 13 deletions tests/fixtures/config-file/cloned-config/eslintConfigFail.js

This file was deleted.

1 change: 0 additions & 1 deletion tests/fixtures/config-file/cloned-config/index.js

This file was deleted.

3 changes: 0 additions & 3 deletions tests/fixtures/config-file/cloned-config/inlineText.js

This file was deleted.

55 changes: 0 additions & 55 deletions tests/lib/cli.js
Expand Up @@ -187,7 +187,6 @@ describe("cli", () => {
});
});


describe("when given a config with environment set to Node.js", () => {
it("should execute without any errors", async () => {
const configPath = getFixturePath("configurations", "env-node.json");
Expand Down Expand Up @@ -1161,58 +1160,4 @@ describe("cli", () => {
});
});

describe("testing the cloned config", () => {
describe("config file and input file", () => {
it("should not modify original configuration object", async () => {
const configPath = getFixturePath("config-file", "cloned-config", "eslintConfig.js");
const filePath = getFixturePath("config-file", "cloned-config", "index.js");
const args = `--config ${configPath} ${filePath}`;

const exit = await cli.execute(args);

assert.strictEqual(exit, 0);
});
});

describe("config file and input file", () => {
it("should exit with 1 as camelcase has wrong property type", async () => {
const configPath = getFixturePath("config-file", "cloned-config", "eslintConfigFail.js");
const filePath = getFixturePath("config-file", "cloned-config", "index.js");
const args = `--config ${configPath} ${filePath}`;

try {
await cli.execute(args);
} catch (error) {
assert.strictEqual(/Configuration for rule "camelcase" is invalid:/u.test(error), true);
}

});
});

describe("inline config and input file", () => {
it("should not modify original configuration object", async () => {
const filePath = getFixturePath("config-file", "cloned-config", "inlineText.js");
const args = `${filePath}`;

const exit = await cli.execute(args);

assert.strictEqual(exit, 0);
});
});

});

describe("handling circular reference while cloning", () => {
it("should handle circular ref", async () => {
const configPath = getFixturePath("config-file", "cloned-config", "circularRefEslintConfig.js");
const filePath = getFixturePath("config-file", "cloned-config", "index.js");
const args = `--config ${configPath} ${filePath}`;

try {
await cli.execute(args);
} catch (error) {
assert.instanceOf(error, Error);
}
});
});
});

0 comments on commit 11aa42b

Please sign in to comment.