Skip to content

Commit

Permalink
feat: merge rule.meta.defaultOptions before validation (#166)
Browse files Browse the repository at this point in the history
* feat: merge rule.meta.defaultOptions before validation

* Switch from ?. to &&

* Apply suggestions from code review

Co-authored-by: Francesco Trotta <[email protected]>

* switch from undefined to void 0 in deep-merge-arrays

---------

Co-authored-by: Francesco Trotta <[email protected]>
  • Loading branch information
JoshuaKGoldberg and fasttime authored Nov 14, 2024
1 parent 2b0aa3a commit d02f914
Show file tree
Hide file tree
Showing 4 changed files with 227 additions and 1 deletion.
6 changes: 5 additions & 1 deletion lib/shared/config-validator.js
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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`
Expand Down
58 changes: 58 additions & 0 deletions lib/shared/deep-merge-arrays.js
Original file line number Diff line number Diff line change
@@ -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 };
30 changes: 30 additions & 0 deletions tests/lib/shared/config-validator.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,21 @@ const mockInvalidJSONSchemaRule = {
}
};

const mockMaxPropertiesSchema = {
meta: {
defaultOptions: [{
foo: 42
}],
schema: [{
type: "object",
maxProperties: 2
}]
},
create() {
return {};
}
};

//------------------------------------------------------------------------------
// Tests
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -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'
}
);
});

});
});
134 changes: 134 additions & 0 deletions tests/lib/shared/deep-merge-arrays.js
Original file line number Diff line number Diff line change
@@ -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);
});
}
});

0 comments on commit d02f914

Please sign in to comment.