From d02f91452b81caff971f7895237cc4fb002e31da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Thu, 14 Nov 2024 05:21:49 -0500 Subject: [PATCH] feat: merge rule.meta.defaultOptions before validation (#166) * feat: merge rule.meta.defaultOptions before validation * Switch from ?. to && * Apply suggestions from code review Co-authored-by: Francesco Trotta * switch from undefined to void 0 in deep-merge-arrays --------- Co-authored-by: Francesco Trotta --- lib/shared/config-validator.js | 6 +- lib/shared/deep-merge-arrays.js | 58 +++++++++++ tests/lib/shared/config-validator.js | 30 ++++++ tests/lib/shared/deep-merge-arrays.js | 134 ++++++++++++++++++++++++++ 4 files changed, 227 insertions(+), 1 deletion(-) create mode 100644 lib/shared/deep-merge-arrays.js create mode 100644 tests/lib/shared/deep-merge-arrays.js diff --git a/lib/shared/config-validator.js b/lib/shared/config-validator.js index 531ccbf..0829bf9 100644 --- a/lib/shared/config-validator.js +++ b/lib/shared/config-validator.js @@ -19,6 +19,7 @@ import util from "util"; import * as ConfigOps from "./config-ops.js"; import { emitDeprecationWarning } from "./deprecation-warnings.js"; import ajvOrig from "./ajv.js"; +import { deepMergeArrays } from "./deep-merge-arrays.js"; import configSchema from "../../conf/config-schema.js"; import BuiltInEnvironments from "../../conf/environments.js"; @@ -148,7 +149,10 @@ export default class ConfigValidator { const validateRule = ruleValidators.get(rule); if (validateRule) { - validateRule(localOptions); + const mergedOptions = deepMergeArrays(rule.meta?.defaultOptions, localOptions); + + validateRule(mergedOptions); + if (validateRule.errors) { throw new Error(validateRule.errors.map( error => `\tValue ${JSON.stringify(error.data)} ${error.message}.\n` diff --git a/lib/shared/deep-merge-arrays.js b/lib/shared/deep-merge-arrays.js new file mode 100644 index 0000000..0c7ec34 --- /dev/null +++ b/lib/shared/deep-merge-arrays.js @@ -0,0 +1,58 @@ +/** + * @fileoverview Applies default rule options + * @author JoshuaKGoldberg + */ + +/** + * Check if the variable contains an object strictly rejecting arrays + * @param {unknown} value an object + * @returns {boolean} Whether value is an object + */ +function isObjectNotArray(value) { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +/** + * Deeply merges second on top of first, creating a new {} object if needed. + * @param {T} first Base, default value. + * @param {U} second User-specified value. + * @returns {T | U | (T & U)} Merged equivalent of second on top of first. + */ +function deepMergeObjects(first, second) { + if (second === void 0) { + return first; + } + + if (!isObjectNotArray(first) || !isObjectNotArray(second)) { + return second; + } + + const result = { ...first, ...second }; + + for (const key of Object.keys(second)) { + if (Object.prototype.propertyIsEnumerable.call(first, key)) { + result[key] = deepMergeObjects(first[key], second[key]); + } + } + + return result; +} + +/** + * Deeply merges second on top of first, creating a new [] array if needed. + * @param {T[] | undefined} first Base, default values. + * @param {U[] | undefined} second User-specified values. + * @returns {(T | U | (T & U))[]} Merged equivalent of second on top of first. + */ +function deepMergeArrays(first, second) { + if (!first || !second) { + return second || first || []; + } + + return [ + ...first.map((value, i) => deepMergeObjects(value, second[i])), + ...second.slice(first.length) + ]; +} + +export { deepMergeArrays }; diff --git a/tests/lib/shared/config-validator.js b/tests/lib/shared/config-validator.js index abed983..fbd92d9 100644 --- a/tests/lib/shared/config-validator.js +++ b/tests/lib/shared/config-validator.js @@ -73,6 +73,21 @@ const mockInvalidJSONSchemaRule = { } }; +const mockMaxPropertiesSchema = { + meta: { + defaultOptions: [{ + foo: 42 + }], + schema: [{ + type: "object", + maxProperties: 2 + }] + }, + create() { + return {}; + } +}; + //------------------------------------------------------------------------------ // Tests //------------------------------------------------------------------------------ @@ -253,4 +268,19 @@ describe("ConfigValidator", () => { }); }); + + describe("validateRuleSchema", () => { + + it("should throw when rule options are invalid after defaults are applied", () => { + const fn = validator.validateRuleSchema.bind(validator, mockMaxPropertiesSchema, [{ bar: 6, baz: 7 }]); + + nodeAssert.throws( + fn, + { + message: '\tValue {"foo":42,"bar":6,"baz":7} should NOT have more than 2 properties.\n' + } + ); + }); + + }); }); diff --git a/tests/lib/shared/deep-merge-arrays.js b/tests/lib/shared/deep-merge-arrays.js new file mode 100644 index 0000000..55cc272 --- /dev/null +++ b/tests/lib/shared/deep-merge-arrays.js @@ -0,0 +1,134 @@ +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +import assert from "node:assert"; + +import { deepMergeArrays } from "../../../lib/shared/deep-merge-arrays.js"; + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +/** + * Turns a value into its string equivalent for a test name. + * @param {unknown} value Value to be stringified. + * @returns {string} String equivalent of the value. + */ +function toTestCaseName(value) { + return typeof value === "object" ? JSON.stringify(value) : `${value}`; +} + +describe("deepMerge", () => { + for (const [first, second, result] of [ + [void 0, void 0, []], + [[], void 0, []], + [["abc"], void 0, ["abc"]], + [void 0, ["abc"], ["abc"]], + [[], ["abc"], ["abc"]], + [[void 0], ["abc"], ["abc"]], + [[void 0, void 0], ["abc"], ["abc", void 0]], + [[void 0, void 0], ["abc", "def"], ["abc", "def"]], + [[void 0, null], ["abc"], ["abc", null]], + [[void 0, null], ["abc", "def"], ["abc", "def"]], + [[null], ["abc"], ["abc"]], + [[123], [void 0], [123]], + [[123], [null], [null]], + [[123], [{ a: 0 }], [{ a: 0 }]], + [["abc"], [void 0], ["abc"]], + [["abc"], [null], [null]], + [["abc"], ["def"], ["def"]], + [["abc"], [{ a: 0 }], [{ a: 0 }]], + [[["abc"]], [null], [null]], + [[["abc"]], ["def"], ["def"]], + [[["abc"]], [{ a: 0 }], [{ a: 0 }]], + [[{ abc: true }], ["def"], ["def"]], + [[{ abc: true }], [["def"]], [["def"]]], + [[null], [{ abc: true }], [{ abc: true }]], + [[{ a: void 0 }], [{ a: 0 }], [{ a: 0 }]], + [[{ a: null }], [{ a: 0 }], [{ a: 0 }]], + [[{ a: null }], [{ a: { b: 0 } }], [{ a: { b: 0 } }]], + [[{ a: 0 }], [{ a: 1 }], [{ a: 1 }]], + [[{ a: 0 }], [{ a: null }], [{ a: null }]], + [[{ a: 0 }], [{ a: void 0 }], [{ a: 0 }]], + [[{ a: 0 }], ["abc"], ["abc"]], + [[{ a: 0 }], [123], [123]], + [[[{ a: 0 }]], [123], [123]], + [ + [{ a: ["b"] }], + [{ a: ["c"] }], + [{ a: ["c"] }] + ], + [ + [{ a: [{ b: "c" }] }], + [{ a: [{ d: "e" }] }], + [{ a: [{ d: "e" }] }] + ], + [ + [{ a: { b: "c" }, d: true }], + [{ a: { e: "f" } }], + [{ a: { b: "c", e: "f" }, d: true }] + ], + [ + [{ a: { b: "c" } }], + [{ a: { e: "f" }, d: true }], + [{ a: { b: "c", e: "f" }, d: true }] + ], + [ + [{ a: { b: "c" } }, { d: true }], + [{ a: { e: "f" } }, { f: 123 }], + [{ a: { b: "c", e: "f" } }, { d: true, f: 123 }] + ], + [ + [{ hasOwnProperty: true }], + [{}], + [{ hasOwnProperty: true }] + ], + [ + [{ hasOwnProperty: false }], + [{}], + [{ hasOwnProperty: false }] + ], + [ + [{ hasOwnProperty: null }], + [{}], + [{ hasOwnProperty: null }] + ], + [ + [{ hasOwnProperty: void 0 }], + [{}], + [{ hasOwnProperty: void 0 }] + ], + [ + [{}], + [{ hasOwnProperty: null }], + [{ hasOwnProperty: null }] + ], + [ + [{}], + [{ hasOwnProperty: void 0 }], + [{ hasOwnProperty: void 0 }] + ], + [ + [{ + allow: [], + ignoreDestructuring: false, + ignoreGlobals: false, + ignoreImports: false, + properties: "always" + }], + [], + [{ + allow: [], + ignoreDestructuring: false, + ignoreGlobals: false, + ignoreImports: false, + properties: "always" + }] + ] + ]) { + it(`${toTestCaseName(first)}, ${toTestCaseName(second)}`, () => { + assert.deepStrictEqual(deepMergeArrays(first, second), result); + }); + } +});