From b757bd8d6910dc1b75f014eaec1f2d36fa38cf4d Mon Sep 17 00:00:00 2001 From: Anix Date: Fri, 24 Apr 2020 08:45:16 +0000 Subject: [PATCH 01/13] Fix: clone config data before validation (fixes #12592) --- lib/cli-engine/config-array-factory.js | 4 +++- tests/lib/rules/camelcase.js | 23 +++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/lib/cli-engine/config-array-factory.js b/lib/cli-engine/config-array-factory.js index 997a7e15318..74a8319217a 100644 --- a/lib/cli-engine/config-array-factory.js +++ b/lib/cli-engine/config-array-factory.js @@ -43,6 +43,7 @@ const fs = require("fs"); const path = require("path"); const importFresh = require("import-fresh"); const stripComments = require("strip-json-comments"); +const lodash = require("lodash"); const { validateConfigSchema } = require("../shared/config-validator"); const naming = require("../shared/naming"); const ModuleResolver = require("../shared/relative-module-resolver"); @@ -593,8 +594,9 @@ class ConfigArrayFactory { ? path.resolve(cwd, providedFilePath) : ""; const name = providedName || (filePath && path.relative(cwd, filePath)); + const clonedConfig = lodash.cloneDeep(configData); - validateConfigSchema(configData, name || filePath); + validateConfigSchema(clonedConfig, name || filePath); return this._normalizeObjectConfigData(configData, filePath, name); } diff --git a/tests/lib/rules/camelcase.js b/tests/lib/rules/camelcase.js index 21f280adb35..80dbe183c9d 100644 --- a/tests/lib/rules/camelcase.js +++ b/tests/lib/rules/camelcase.js @@ -225,6 +225,10 @@ ruleTester.run("camelcase", rule, { code: "foo = { [computedBar]: 0 };", options: [{ ignoreDestructuring: true }], parserOptions: { ecmaVersion: 6 } + }, + { + code: "var obj = { mem_var: 1 };", + options: [{ properties: "never" }] } ], invalid: [ @@ -736,6 +740,25 @@ ruleTester.run("camelcase", rule, { type: "Identifier" } ] + }, + { + code: "const {mem_var = 1} = obh", + options: [{ ignoreDestructuring: false }], + parserOptions: { ecmaVersion: 6 }, + errors: [{ + messageId: "notCamelCase", + data: { name: "mem_var" }, + type: "Identifier" + }] + }, + { + code: "var obj = { mem_var: 1 };", + options: [{}], + errors: [{ + messageId: "notCamelCase", + data: { name: "mem_var" }, + type: "Identifier" + }] } ] }); From ef60d4cae04c66f6eeb2445dc28b81160c9522cc Mon Sep 17 00:00:00 2001 From: Anix Date: Wed, 29 Apr 2020 06:44:53 +0000 Subject: [PATCH 02/13] Chore: added test and cloning before validation --- lib/cli-engine/config-array-factory.js | 4 +-- lib/shared/config-validator.js | 4 ++- .../config-file/cloned-config/eslintConfig.js | 4 +++ .../config-file/cloned-config/index.js | 1 + .../config-file/cloned-config/inlineText.js | 3 +++ tests/lib/cli.js | 26 +++++++++++++++++++ 6 files changed, 38 insertions(+), 4 deletions(-) create mode 100644 tests/fixtures/config-file/cloned-config/eslintConfig.js create mode 100644 tests/fixtures/config-file/cloned-config/index.js create mode 100644 tests/fixtures/config-file/cloned-config/inlineText.js diff --git a/lib/cli-engine/config-array-factory.js b/lib/cli-engine/config-array-factory.js index 48e03a08e10..67211f6e9f4 100644 --- a/lib/cli-engine/config-array-factory.js +++ b/lib/cli-engine/config-array-factory.js @@ -43,7 +43,6 @@ const fs = require("fs"); const path = require("path"); const importFresh = require("import-fresh"); const stripComments = require("strip-json-comments"); -const lodash = require("lodash"); const { validateConfigSchema } = require("../shared/config-validator"); const naming = require("../shared/naming"); const ModuleResolver = require("../shared/relative-module-resolver"); @@ -624,9 +623,8 @@ class ConfigArrayFactory { * @private */ _normalizeConfigData(configData, ctx) { - const clonedConfig = lodash.cloneDeep(configData); - validateConfigSchema(clonedConfig, ctx.name || ctx.filePath); + validateConfigSchema(configData, ctx.name || ctx.filePath); return this._normalizeObjectConfigData(configData, ctx); } diff --git a/lib/shared/config-validator.js b/lib/shared/config-validator.js index 458bd2a802b..9f6bb509b1f 100644 --- a/lib/shared/config-validator.js +++ b/lib/shared/config-validator.js @@ -15,6 +15,7 @@ const BuiltInEnvironments = require("../../conf/environments"), BuiltInRules = require("../rules"), ConfigOps = require("./config-ops"), + _ = require("lodash"), { emitDeprecationWarning } = require("./deprecation-warnings"); const ajv = require("./ajv")(); @@ -183,8 +184,9 @@ function validateRules( Object.keys(rulesConfig).forEach(id => { const rule = getAdditionalRule(id) || BuiltInRules.get(id) || null; + const clonedRulesConfigForId = _.cloneDeep(rulesConfig[id]); - validateRuleOptions(rule, id, rulesConfig[id], source); + validateRuleOptions(rule, id, clonedRulesConfigForId, source); }); } diff --git a/tests/fixtures/config-file/cloned-config/eslintConfig.js b/tests/fixtures/config-file/cloned-config/eslintConfig.js new file mode 100644 index 00000000000..8602e60b0fd --- /dev/null +++ b/tests/fixtures/config-file/cloned-config/eslintConfig.js @@ -0,0 +1,4 @@ +let errorConfig = ["error", {}]; +module.exports = { + rules: { camelcase: errorConfig, "default-case": errorConfig } +}; diff --git a/tests/fixtures/config-file/cloned-config/index.js b/tests/fixtures/config-file/cloned-config/index.js new file mode 100644 index 00000000000..b792f78cad3 --- /dev/null +++ b/tests/fixtures/config-file/cloned-config/index.js @@ -0,0 +1 @@ +var someValue = "bar"; diff --git a/tests/fixtures/config-file/cloned-config/inlineText.js b/tests/fixtures/config-file/cloned-config/inlineText.js new file mode 100644 index 00000000000..e000713abaf --- /dev/null +++ b/tests/fixtures/config-file/cloned-config/inlineText.js @@ -0,0 +1,3 @@ +/*eslint default-case: ["error", {}]*/ +/*eslint camelcase: ["error", {}]*/ +var someValue = "bar"; diff --git a/tests/lib/cli.js b/tests/lib/cli.js index 6af3b5c838e..ee082e50615 100644 --- a/tests/lib/cli.js +++ b/tests/lib/cli.js @@ -185,6 +185,7 @@ describe("cli", () => { }); }); + describe("when given a config with environment set to Node.js", () => { it("should execute without any errors", () => { const configPath = getFixturePath("configurations", "env-node.json"); @@ -1192,4 +1193,29 @@ describe("cli", () => { }); }); + describe("testing the cloned config", () => { + describe("config file and input file", () => { + it("should execute without any errors", () => { + const configPath = getFixturePath("config-file", "cloned-config", "eslintConfig.js"); + const filePath = getFixturePath("config-file", "cloned-config", "index.js"); + const code = `--config ${configPath} ${filePath}`; + + const exit = cli.execute(code); + + assert.strictEqual(exit, 0); + }); + }); + + describe("inline config and input file", () => { + it("should execute without any errors", () => { + const filePath = getFixturePath("config-file", "cloned-config", "inlineText.js"); + const code = `${filePath}`; + + const exit = cli.execute(code); + + assert.strictEqual(exit, 0); + }); + }); + }); + }); From ca2e7068140d7b1111f7efcafd648339cd1517d5 Mon Sep 17 00:00:00 2001 From: Anix Date: Wed, 29 Apr 2020 08:05:03 +0000 Subject: [PATCH 03/13] Chore: test fix --- tests/lib/cli.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/lib/cli.js b/tests/lib/cli.js index 093d6168c51..e1df3ec75a1 100644 --- a/tests/lib/cli.js +++ b/tests/lib/cli.js @@ -1163,23 +1163,23 @@ describe("cli", () => { describe("testing the cloned config", () => { describe("config file and input file", () => { - it("should execute without any errors", () => { + it("should execute without any errors", async () => { const configPath = getFixturePath("config-file", "cloned-config", "eslintConfig.js"); const filePath = getFixturePath("config-file", "cloned-config", "index.js"); const code = `--config ${configPath} ${filePath}`; - const exit = cli.execute(code); + const exit = await cli.execute(code); assert.strictEqual(exit, 0); }); }); describe("inline config and input file", () => { - it("should execute without any errors", () => { + it("should execute without any errors", async () => { const filePath = getFixturePath("config-file", "cloned-config", "inlineText.js"); const code = `${filePath}`; - const exit = cli.execute(code); + const exit = await cli.execute(code); assert.strictEqual(exit, 0); }); From df673322060b5b6e169ad89eed9aea5a0fd24192 Mon Sep 17 00:00:00 2001 From: Anix Date: Wed, 29 Apr 2020 08:46:15 +0000 Subject: [PATCH 04/13] Chore: rules tests fixes --- tests/lib/rules/computed-property-spacing.js | 4 ++-- tests/lib/rules/use-isnan.js | 10 ---------- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/tests/lib/rules/computed-property-spacing.js b/tests/lib/rules/computed-property-spacing.js index b0ecef6eb81..3514abc120f 100644 --- a/tests/lib/rules/computed-property-spacing.js +++ b/tests/lib/rules/computed-property-spacing.js @@ -782,7 +782,7 @@ ruleTester.run("computed-property-spacing", rule, { { code: "A = class { [ a ](){} get [ b ](){} set [ c ](foo){} static [ d ](){} static get [ e ](){} static set [ f ](bar){} }", output: "A = class { [a](){} get [b](){} set [c](foo){} static [d](){} static get [e](){} static set [f](bar){} }", - options: ["never", {}], + options: ["never"], parserOptions: { ecmaVersion: 6 }, errors: [ { @@ -1040,7 +1040,7 @@ ruleTester.run("computed-property-spacing", rule, { { code: "class A { [a](){} get [b](){} set [c](foo){} static [d](){} static get [e](){} static set [f](bar){} }", output: "class A { [ a ](){} get [ b ](){} set [ c ](foo){} static [ d ](){} static get [ e ](){} static set [ f ](bar){} }", - options: ["always", {}], + options: ["always"], parserOptions: { ecmaVersion: 6 }, errors: [ { diff --git a/tests/lib/rules/use-isnan.js b/tests/lib/rules/use-isnan.js index aa1b1746fcd..0f0ba8678fc 100644 --- a/tests/lib/rules/use-isnan.js +++ b/tests/lib/rules/use-isnan.js @@ -280,16 +280,6 @@ ruleTester.run("use-isnan", rule, { code: "switch(foo) { case NaN: break; }", errors: [{ messageId: "caseNaN", type: "SwitchCase", column: 15 }] }, - { - code: "switch(NaN) { case foo: break; }", - options: [{}], - errors: [{ messageId: "switchNaN", type: "SwitchStatement", column: 1 }] - }, - { - code: "switch(foo) { case NaN: break; }", - options: [{}], - errors: [{ messageId: "caseNaN", type: "SwitchCase", column: 15 }] - }, { code: "switch(NaN) {}", options: [{ enforceForSwitchCase: true }], From 2260565f3a129ab31cb6c70dd44b580338b6bc1f Mon Sep 17 00:00:00 2001 From: Anix Date: Thu, 7 May 2020 07:00:03 +0000 Subject: [PATCH 05/13] Update: cloning config using json parse and stringify --- lib/cli-engine/config-array-factory.js | 4 +++- lib/shared/config-validator.js | 4 +--- tests/lib/rules/computed-property-spacing.js | 4 ++-- tests/lib/rules/use-isnan.js | 10 ++++++++++ 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/lib/cli-engine/config-array-factory.js b/lib/cli-engine/config-array-factory.js index 96af54af93b..e5bf870d150 100644 --- a/lib/cli-engine/config-array-factory.js +++ b/lib/cli-engine/config-array-factory.js @@ -625,7 +625,9 @@ class ConfigArrayFactory { _normalizeConfigData(configData, ctx) { validateConfigSchema(configData, ctx.name || ctx.filePath); - return this._normalizeObjectConfigData(configData, ctx); + const clonedConfig = JSON.parse(JSON.stringify(configData)); + + return this._normalizeObjectConfigData(clonedConfig, ctx); } /** diff --git a/lib/shared/config-validator.js b/lib/shared/config-validator.js index 9f6bb509b1f..458bd2a802b 100644 --- a/lib/shared/config-validator.js +++ b/lib/shared/config-validator.js @@ -15,7 +15,6 @@ const BuiltInEnvironments = require("../../conf/environments"), BuiltInRules = require("../rules"), ConfigOps = require("./config-ops"), - _ = require("lodash"), { emitDeprecationWarning } = require("./deprecation-warnings"); const ajv = require("./ajv")(); @@ -184,9 +183,8 @@ function validateRules( Object.keys(rulesConfig).forEach(id => { const rule = getAdditionalRule(id) || BuiltInRules.get(id) || null; - const clonedRulesConfigForId = _.cloneDeep(rulesConfig[id]); - validateRuleOptions(rule, id, clonedRulesConfigForId, source); + validateRuleOptions(rule, id, rulesConfig[id], source); }); } diff --git a/tests/lib/rules/computed-property-spacing.js b/tests/lib/rules/computed-property-spacing.js index 3514abc120f..b7055461573 100644 --- a/tests/lib/rules/computed-property-spacing.js +++ b/tests/lib/rules/computed-property-spacing.js @@ -782,7 +782,7 @@ ruleTester.run("computed-property-spacing", rule, { { code: "A = class { [ a ](){} get [ b ](){} set [ c ](foo){} static [ d ](){} static get [ e ](){} static set [ f ](bar){} }", output: "A = class { [a](){} get [b](){} set [c](foo){} static [d](){} static get [e](){} static set [f](bar){} }", - options: ["never"], + options: ["never", {}], parserOptions: { ecmaVersion: 6 }, errors: [ { @@ -924,7 +924,7 @@ ruleTester.run("computed-property-spacing", rule, { { code: "A = class { [a](){} get [b](){} set [c](foo){} static [d](){} static get [e](){} static set [f](bar){} }", output: "A = class { [ a ](){} get [ b ](){} set [ c ](foo){} static [ d ](){} static get [ e ](){} static set [ f ](bar){} }", - options: ["always"], + options: ["always", {}], parserOptions: { ecmaVersion: 6 }, errors: [ { diff --git a/tests/lib/rules/use-isnan.js b/tests/lib/rules/use-isnan.js index 0f0ba8678fc..aa1b1746fcd 100644 --- a/tests/lib/rules/use-isnan.js +++ b/tests/lib/rules/use-isnan.js @@ -280,6 +280,16 @@ ruleTester.run("use-isnan", rule, { code: "switch(foo) { case NaN: break; }", errors: [{ messageId: "caseNaN", type: "SwitchCase", column: 15 }] }, + { + code: "switch(NaN) { case foo: break; }", + options: [{}], + errors: [{ messageId: "switchNaN", type: "SwitchStatement", column: 1 }] + }, + { + code: "switch(foo) { case NaN: break; }", + options: [{}], + errors: [{ messageId: "caseNaN", type: "SwitchCase", column: 15 }] + }, { code: "switch(NaN) {}", options: [{ enforceForSwitchCase: true }], From 024f2681bdec4667a380e5cb14f92108ce0a99e3 Mon Sep 17 00:00:00 2001 From: Anix Date: Thu, 7 May 2020 07:07:19 +0000 Subject: [PATCH 06/13] Update: changed test description --- tests/lib/cli.js | 12 ++++++------ tests/lib/rules/computed-property-spacing.js | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/lib/cli.js b/tests/lib/cli.js index e1df3ec75a1..c97c5495bee 100644 --- a/tests/lib/cli.js +++ b/tests/lib/cli.js @@ -1163,23 +1163,23 @@ describe("cli", () => { describe("testing the cloned config", () => { describe("config file and input file", () => { - it("should execute without any errors", async () => { + 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 code = `--config ${configPath} ${filePath}`; + const args = `--config ${configPath} ${filePath}`; - const exit = await cli.execute(code); + const exit = await cli.execute(args); assert.strictEqual(exit, 0); }); }); describe("inline config and input file", () => { - it("should execute without any errors", async () => { + it("should not modify original configuration object", async () => { const filePath = getFixturePath("config-file", "cloned-config", "inlineText.js"); - const code = `${filePath}`; + const args = `${filePath}`; - const exit = await cli.execute(code); + const exit = await cli.execute(args); assert.strictEqual(exit, 0); }); diff --git a/tests/lib/rules/computed-property-spacing.js b/tests/lib/rules/computed-property-spacing.js index b7055461573..b0ecef6eb81 100644 --- a/tests/lib/rules/computed-property-spacing.js +++ b/tests/lib/rules/computed-property-spacing.js @@ -924,7 +924,7 @@ ruleTester.run("computed-property-spacing", rule, { { code: "A = class { [a](){} get [b](){} set [c](foo){} static [d](){} static get [e](){} static set [f](bar){} }", output: "A = class { [ a ](){} get [ b ](){} set [ c ](foo){} static [ d ](){} static get [ e ](){} static set [ f ](bar){} }", - options: ["always", {}], + options: ["always"], parserOptions: { ecmaVersion: 6 }, errors: [ { @@ -1040,7 +1040,7 @@ ruleTester.run("computed-property-spacing", rule, { { code: "class A { [a](){} get [b](){} set [c](foo){} static [d](){} static get [e](){} static set [f](bar){} }", output: "class A { [ a ](){} get [ b ](){} set [ c ](foo){} static [ d ](){} static get [ e ](){} static set [ f ](bar){} }", - options: ["always"], + options: ["always", {}], parserOptions: { ecmaVersion: 6 }, errors: [ { From 0374e39ebf115f1691207cad40a125405cf61701 Mon Sep 17 00:00:00 2001 From: Anix Date: Thu, 7 May 2020 10:04:25 +0000 Subject: [PATCH 07/13] Chore: removed unnecessary tests --- tests/lib/rules/camelcase.js | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/tests/lib/rules/camelcase.js b/tests/lib/rules/camelcase.js index 0bceea81aae..1e3eecc2110 100644 --- a/tests/lib/rules/camelcase.js +++ b/tests/lib/rules/camelcase.js @@ -226,10 +226,6 @@ ruleTester.run("camelcase", rule, { options: [{ ignoreDestructuring: true }], parserOptions: { ecmaVersion: 6 } }, - { - code: "var obj = { mem_var: 1 };", - options: [{ properties: "never" }] - }, { code: "({ a: obj.fo_o } = bar);", options: [{ allow: ["fo_o"] }], @@ -794,25 +790,6 @@ ruleTester.run("camelcase", rule, { } ] }, - { - code: "const {mem_var = 1} = obh", - options: [{ ignoreDestructuring: false }], - parserOptions: { ecmaVersion: 6 }, - errors: [{ - messageId: "notCamelCase", - data: { name: "mem_var" }, - type: "Identifier" - }] - }, - { - code: "var obj = { mem_var: 1 };", - options: [{}], - errors: [{ - messageId: "notCamelCase", - data: { name: "mem_var" }, - type: "Identifier" - }] - }, { code: "({ a: obj.fo_o } = bar);", parserOptions: { ecmaVersion: 6 }, From 79a1a1915a721de75ec43e357cef7883a08c43e8 Mon Sep 17 00:00:00 2001 From: Anix Date: Fri, 15 May 2020 14:28:40 +0000 Subject: [PATCH 08/13] Update: changed clone logic --- lib/cli-engine/config-array-factory.js | 32 ++++++++++++++++++- .../cloned-config/eslintConfigFail.js | 13 ++++++++ tests/lib/cli.js | 15 +++++++++ 3 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 tests/fixtures/config-file/cloned-config/eslintConfigFail.js diff --git a/lib/cli-engine/config-array-factory.js b/lib/cli-engine/config-array-factory.js index e5bf870d150..bf68120daad 100644 --- a/lib/cli-engine/config-array-factory.js +++ b/lib/cli-engine/config-array-factory.js @@ -625,7 +625,37 @@ class ConfigArrayFactory { _normalizeConfigData(configData, ctx) { validateConfigSchema(configData, ctx.name || ctx.filePath); - const clonedConfig = JSON.parse(JSON.stringify(configData)); + + /** + * Function to clone a object ref https://stackoverflow.com/a/1042676/9339537 + * @param {Object} from object which needs to be cloned + * @returns {Object} returns the cloned object + */ + function clone(from) { + + if (from === null || typeof from !== "object") { + return from; + } + if (from.constructor !== Object && from.constructor !== Array) { + return from; + } + if (from.constructor === Date || from.constructor === RegExp || from.constructor === Function || + from.constructor === String || from.constructor === Number || from.constructor === Boolean) { + return new from.constructor(from); + } + + const clonedResult = new from.constructor(); + + for (const name in from) { + + if (typeof clonedResult[name] === "undefined") { + clonedResult[name] = clone(from[name]); + } + } + + return clonedResult; + } + const clonedConfig = clone(configData); return this._normalizeObjectConfigData(clonedConfig, ctx); } diff --git a/tests/fixtures/config-file/cloned-config/eslintConfigFail.js b/tests/fixtures/config-file/cloned-config/eslintConfigFail.js new file mode 100644 index 00000000000..b8702c6ffb8 --- /dev/null +++ b/tests/fixtures/config-file/cloned-config/eslintConfigFail.js @@ -0,0 +1,13 @@ +let errorConfig = ["error", {}]; +module.exports = { + rules: { + camelcase: errorConfig, + "default-case": errorConfig , + "camelcase" : [ + 'error', + { + "ignoreDestructuring": new Date().getUTCFullYear // returns a function + } + ] + } +}; diff --git a/tests/lib/cli.js b/tests/lib/cli.js index c97c5495bee..5488b896224 100644 --- a/tests/lib/cli.js +++ b/tests/lib/cli.js @@ -1174,6 +1174,21 @@ describe("cli", () => { }); }); + 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"); From a20065d3ee0294fc83b4bff90b78365b805e3ec7 Mon Sep 17 00:00:00 2001 From: Anix Date: Thu, 21 May 2020 15:19:12 +0000 Subject: [PATCH 09/13] Update: using lodash.cloneDeepWith for cloning --- lib/cli-engine/config-array-factory.js | 37 +++++++++++--------------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/lib/cli-engine/config-array-factory.js b/lib/cli-engine/config-array-factory.js index bf68120daad..db74a7925e7 100644 --- a/lib/cli-engine/config-array-factory.js +++ b/lib/cli-engine/config-array-factory.js @@ -43,6 +43,7 @@ const fs = require("fs"); const path = require("path"); const importFresh = require("import-fresh"); const stripComments = require("strip-json-comments"); +const lodash = require("lodash"); const { validateConfigSchema } = require("../shared/config-validator"); const naming = require("../shared/naming"); const ModuleResolver = require("../shared/relative-module-resolver"); @@ -627,35 +628,27 @@ class ConfigArrayFactory { validateConfigSchema(configData, ctx.name || ctx.filePath); /** - * Function to clone a object ref https://stackoverflow.com/a/1042676/9339537 - * @param {Object} from object which needs to be cloned + * Customizer for lodash.cloneDeepWith * @returns {Object} returns the cloned object */ - function clone(from) { + function getCustomizer() { + let root = true; - if (from === null || typeof from !== "object") { - return from; - } - if (from.constructor !== Object && from.constructor !== Array) { - return from; - } - if (from.constructor === Date || from.constructor === RegExp || from.constructor === Function || - from.constructor === String || from.constructor === Number || from.constructor === Boolean) { - return new from.constructor(from); - } - - const clonedResult = new from.constructor(); + return function customizer(value) { + if (!root && typeof value === "object" && value !== null) { - for (const name in from) { - - if (typeof clonedResult[name] === "undefined") { - clonedResult[name] = clone(from[name]); + // start new lodash.cloneDeepWith + return lodash.cloneDeepWith(value, getCustomizer()); } - } - return clonedResult; + root = false; + + // return `undefined` for the default lodash.cloneDeepWith behavior + return void 0; + }; } - const clonedConfig = clone(configData); + + const clonedConfig = lodash.cloneDeepWith(configData, getCustomizer()); return this._normalizeObjectConfigData(clonedConfig, ctx); } From 1453116c5e31338d747e0770a6032412538e2ede Mon Sep 17 00:00:00 2001 From: Anix Date: Sat, 23 May 2020 09:51:07 +0000 Subject: [PATCH 10/13] Update: handling circular ref cases --- lib/cli-engine/config-array-factory.js | 48 ++++++++++++++----- .../cloned-config/circularRefEslintConfig.js | 15 ++++++ tests/lib/cli.js | 15 ++++++ 3 files changed, 67 insertions(+), 11 deletions(-) create mode 100644 tests/fixtures/config-file/cloned-config/circularRefEslintConfig.js diff --git a/lib/cli-engine/config-array-factory.js b/lib/cli-engine/config-array-factory.js index db74a7925e7..b3eba8bfa44 100644 --- a/lib/cli-engine/config-array-factory.js +++ b/lib/cli-engine/config-array-factory.js @@ -628,27 +628,53 @@ class ConfigArrayFactory { validateConfigSchema(configData, ctx.name || ctx.filePath); /** - * Customizer for lodash.cloneDeepWith + * Cloning function using lodash.cloneDeepWith + * @param {Object} val Object which needs to be cloned * @returns {Object} returns the cloned object */ - function getCustomizer() { - let root = true; + function cloneDeep(val) { + const seenObjects = new Set(); + let forceDefault = false; + + /** + * Customizer for lodash.cloneDeepWith + * @param {Object} value value that needs to be clone with custom logic + * @returns {Object} returns the cloned object + */ + function customizer(value) { + if (typeof value === "object" && value !== null) { + if (forceDefault) { + forceDefault = false; + } else { + if (seenObjects.has(value)) { + throw new Error("Cannot clone circular structure"); + } - return function customizer(value) { - if (!root && typeof value === "object" && value !== null) { + seenObjects.add(value); + forceDefault = true; + const clonedValue = lodash.cloneDeepWith(value, customizer); - // start new lodash.cloneDeepWith - return lodash.cloneDeepWith(value, getCustomizer()); - } + seenObjects.delete(value); - root = false; + return clonedValue; + } + } // return `undefined` for the default lodash.cloneDeepWith behavior return void 0; - }; + } + + return lodash.cloneDeepWith(val, customizer); } - const clonedConfig = lodash.cloneDeepWith(configData, getCustomizer()); + let clonedConfig = configData; + + try { + clonedConfig = cloneDeep(clonedConfig); + } catch (error) { + debug("Failed to clone config object as it noticed some circular reference. Using the original config"); + clonedConfig = configData; + } return this._normalizeObjectConfigData(clonedConfig, ctx); } diff --git a/tests/fixtures/config-file/cloned-config/circularRefEslintConfig.js b/tests/fixtures/config-file/cloned-config/circularRefEslintConfig.js new file mode 100644 index 00000000000..cc9fcdbcc7c --- /dev/null +++ b/tests/fixtures/config-file/cloned-config/circularRefEslintConfig.js @@ -0,0 +1,15 @@ +let errorConfig = ["error", {}]; + +class CircularRef { + constructor() { + this.self = this; + } +} + +module.exports = { + settings: { + sharedData: new CircularRef() + }, + + rules: { camelcase: errorConfig, "default-case": errorConfig } +}; diff --git a/tests/lib/cli.js b/tests/lib/cli.js index 5488b896224..946db48f573 100644 --- a/tests/lib/cli.js +++ b/tests/lib/cli.js @@ -1199,6 +1199,21 @@ describe("cli", () => { 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); + assert.strictEqual(/Configuration for rule "default-case" is invalid/iu.test(error), true); + } + }); + }); }); }); From 949b3ee720e216ccdc0d23adb06bd5972a52f19a Mon Sep 17 00:00:00 2001 From: Anix Date: Sat, 13 Jun 2020 16:24:48 +0000 Subject: [PATCH 11/13] Update: using JSON method for cloning logic --- lib/cli-engine/config-array-factory.js | 50 +------------------------- tests/lib/cli.js | 14 -------- 2 files changed, 1 insertion(+), 63 deletions(-) diff --git a/lib/cli-engine/config-array-factory.js b/lib/cli-engine/config-array-factory.js index b3eba8bfa44..5e56aa5edf9 100644 --- a/lib/cli-engine/config-array-factory.js +++ b/lib/cli-engine/config-array-factory.js @@ -43,7 +43,6 @@ const fs = require("fs"); const path = require("path"); const importFresh = require("import-fresh"); const stripComments = require("strip-json-comments"); -const lodash = require("lodash"); const { validateConfigSchema } = require("../shared/config-validator"); const naming = require("../shared/naming"); const ModuleResolver = require("../shared/relative-module-resolver"); @@ -627,54 +626,7 @@ class ConfigArrayFactory { validateConfigSchema(configData, ctx.name || ctx.filePath); - /** - * Cloning function using lodash.cloneDeepWith - * @param {Object} val Object which needs to be cloned - * @returns {Object} returns the cloned object - */ - function cloneDeep(val) { - const seenObjects = new Set(); - let forceDefault = false; - - /** - * Customizer for lodash.cloneDeepWith - * @param {Object} value value that needs to be clone with custom logic - * @returns {Object} returns the cloned object - */ - function customizer(value) { - if (typeof value === "object" && value !== null) { - if (forceDefault) { - forceDefault = false; - } else { - if (seenObjects.has(value)) { - throw new Error("Cannot clone circular structure"); - } - - seenObjects.add(value); - forceDefault = true; - const clonedValue = lodash.cloneDeepWith(value, customizer); - - seenObjects.delete(value); - - return clonedValue; - } - } - - // return `undefined` for the default lodash.cloneDeepWith behavior - return void 0; - } - - return lodash.cloneDeepWith(val, customizer); - } - - let clonedConfig = configData; - - try { - clonedConfig = cloneDeep(clonedConfig); - } catch (error) { - debug("Failed to clone config object as it noticed some circular reference. Using the original config"); - clonedConfig = configData; - } + const clonedConfig = JSON.parse(JSON.stringify(configData)); return this._normalizeObjectConfigData(clonedConfig, ctx); } diff --git a/tests/lib/cli.js b/tests/lib/cli.js index 946db48f573..a44f51afb59 100644 --- a/tests/lib/cli.js +++ b/tests/lib/cli.js @@ -1200,20 +1200,6 @@ describe("cli", () => { }); }); - 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); - assert.strictEqual(/Configuration for rule "default-case" is invalid/iu.test(error), true); - } - }); - }); }); }); From a786d386507c44b34b52b1cf83069247c22613bf Mon Sep 17 00:00:00 2001 From: Anix Date: Sun, 14 Jun 2020 09:52:15 +0000 Subject: [PATCH 12/13] Update: moved clone logic to normalieObjectConfigData --- lib/cli-engine/config-array-factory.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/cli-engine/config-array-factory.js b/lib/cli-engine/config-array-factory.js index 5e56aa5edf9..8714fe0c517 100644 --- a/lib/cli-engine/config-array-factory.js +++ b/lib/cli-engine/config-array-factory.js @@ -626,9 +626,7 @@ class ConfigArrayFactory { validateConfigSchema(configData, ctx.name || ctx.filePath); - const clonedConfig = JSON.parse(JSON.stringify(configData)); - - return this._normalizeObjectConfigData(clonedConfig, ctx); + return this._normalizeObjectConfigData(configData, ctx); } /** @@ -701,6 +699,8 @@ class ConfigArrayFactory { ctx.matchBasePath ); + const clonedRulesConfig = rules && JSON.parse(JSON.stringify((rules))); + // Flatten `extends`. for (const extendName of extendList.filter(Boolean)) { yield* this._loadExtends(extendName, ctx); @@ -735,7 +735,7 @@ class ConfigArrayFactory { processor, reportUnusedDisableDirectives, root, - rules, + rules: clonedRulesConfig, settings }; From b030e8320ac637525c5de202680f2350996ae821 Mon Sep 17 00:00:00 2001 From: Anix Date: Sun, 14 Jun 2020 12:40:16 +0000 Subject: [PATCH 13/13] Chore: added comments for cloning reason --- lib/cli-engine/config-array-factory.js | 27 ++++++++++++++++++++++++-- tests/lib/cli.js | 13 +++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/lib/cli-engine/config-array-factory.js b/lib/cli-engine/config-array-factory.js index 8714fe0c517..ccb7b7d5490 100644 --- a/lib/cli-engine/config-array-factory.js +++ b/lib/cli-engine/config-array-factory.js @@ -623,9 +623,7 @@ class ConfigArrayFactory { * @private */ _normalizeConfigData(configData, ctx) { - validateConfigSchema(configData, ctx.name || ctx.filePath); - return this._normalizeObjectConfigData(configData, ctx); } @@ -699,6 +697,31 @@ 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`. diff --git a/tests/lib/cli.js b/tests/lib/cli.js index a44f51afb59..afb225f1f7c 100644 --- a/tests/lib/cli.js +++ b/tests/lib/cli.js @@ -1202,4 +1202,17 @@ describe("cli", () => { }); + 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); + } + }); + }); });