From 23ac6b195d633d3bff564e85f09fcb65c64ae82c Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Tue, 16 Dec 2025 23:03:44 +0000 Subject: [PATCH] fix(linter/plugins): apply defaults from `meta.schema` to options (#16930) ESLint has 2 different ways to define default options for a rule - `rule.meta.defaultOptions`, or `rule.meta.schema` containing a schema which defines default values. We previously supported the former, but not the latter. This PR implements support for defining defaults via `rule.meta.schema`. We compile the schema and apply it to options on JS side, using [`ajv`](https://www.npmjs.com/package/ajv), which is what ESLint uses. But this process only happens once before the start of linting, not once for every file, so the perf hit should be small. This also prepares the way for validating options against the schema (#15631). --- apps/oxlint/conformance/snapshot.md | 318 +----------------- apps/oxlint/package.json | 2 + apps/oxlint/src-js/plugins/load.ts | 23 +- apps/oxlint/src-js/plugins/options.ts | 244 +++++++++++--- .../test/fixtures/options/.oxlintrc.json | 30 +- .../test/fixtures/options/output.snap.md | 103 +++++- apps/oxlint/test/fixtures/options/plugin.ts | 96 ++++++ pnpm-lock.yaml | 6 + 8 files changed, 440 insertions(+), 382 deletions(-) diff --git a/apps/oxlint/conformance/snapshot.md b/apps/oxlint/conformance/snapshot.md index 0a587ad81e8fc..f1bd122144c16 100644 --- a/apps/oxlint/conformance/snapshot.md +++ b/apps/oxlint/conformance/snapshot.md @@ -7,8 +7,8 @@ | Status | Count | % | | ----------------- | ----- | ------ | | Total rules | 277 | 100.0% | -| Fully passing | 217 | 78.3% | -| Partially passing | 59 | 21.3% | +| Fully passing | 218 | 78.7% | +| Partially passing | 58 | 20.9% | | Fully failing | 1 | 0.4% | | Load errors | 0 | 0.0% | | No tests run | 0 | 0.0% | @@ -18,8 +18,8 @@ | Status | Count | % | | ----------- | ----- | ------ | | Total tests | 29765 | 100.0% | -| Passing | 28671 | 96.3% | -| Failing | 987 | 3.3% | +| Passing | 28673 | 96.3% | +| Failing | 985 | 3.3% | | Skipped | 107 | 0.4% | ## Fully Passing Rules @@ -38,6 +38,7 @@ - `class-methods-use-this` (156 tests) - `comma-spacing` (173 tests) - `comma-style` (96 tests) +- `computed-property-spacing` (125 tests) - `curly` (216 tests) - `default-case-last` (37 tests) - `default-case` (23 tests) @@ -247,7 +248,6 @@ - `block-scoped-var` - 97 / 106 (91.5%) - `camelcase` - 187 / 204 (91.7%) - `comma-dangle` - 265 / 267 (99.3%) -- `computed-property-spacing` - 123 / 125 (98.4%) - `consistent-this` - 23 / 26 (88.5%) - `func-call-spacing` - 137 / 151 (90.7%) - `id-blacklist` - 120 / 131 (91.6%) @@ -1585,314 +1585,6 @@ AssertionError [ERR_ASSERTION]: Should have 2 errors but had 1: [ at apps/oxlint/dist/index.js -### `computed-property-spacing` - -Pass: 123 / 125 (98.4%) -Fail: 2 / 125 (1.6%) -Skip: 0 / 125 (0.0%) - -#### computed-property-spacing > invalid - -```js -A = class { [ a ](){} get [ b ](){} set [ c ](foo){} static [ d ](){} static get [ e ](){} static set [ f ](bar){} } -``` - -```json -{ - "output": "A = class { [a](){} get [b](){} set [c](foo){} static [d](){} static get [e](){} static set [f](bar){} }", - "options": [ - "never", - {} - ], - "languageOptions": { - "ecmaVersion": 6 - }, - "errors": [ - { - "messageId": "unexpectedSpaceAfter", - "data": { - "tokenValue": "[" - }, - "line": 1, - "column": 14, - "endLine": 1, - "endColumn": 15 - }, - { - "messageId": "unexpectedSpaceBefore", - "data": { - "tokenValue": "]" - }, - "line": 1, - "column": 16, - "endLine": 1, - "endColumn": 17 - }, - { - "messageId": "unexpectedSpaceAfter", - "data": { - "tokenValue": "[" - }, - "line": 1, - "column": 28, - "endLine": 1, - "endColumn": 29 - }, - { - "messageId": "unexpectedSpaceBefore", - "data": { - "tokenValue": "]" - }, - "line": 1, - "column": 30, - "endLine": 1, - "endColumn": 31 - }, - { - "messageId": "unexpectedSpaceAfter", - "data": { - "tokenValue": "[" - }, - "line": 1, - "column": 42, - "endLine": 1, - "endColumn": 43 - }, - { - "messageId": "unexpectedSpaceBefore", - "data": { - "tokenValue": "]" - }, - "line": 1, - "column": 44, - "endLine": 1, - "endColumn": 45 - }, - { - "messageId": "unexpectedSpaceAfter", - "data": { - "tokenValue": "[" - }, - "line": 1, - "column": 62, - "endLine": 1, - "endColumn": 63 - }, - { - "messageId": "unexpectedSpaceBefore", - "data": { - "tokenValue": "]" - }, - "line": 1, - "column": 64, - "endLine": 1, - "endColumn": 65 - }, - { - "messageId": "unexpectedSpaceAfter", - "data": { - "tokenValue": "[" - }, - "line": 1, - "column": 83, - "endLine": 1, - "endColumn": 84 - }, - { - "messageId": "unexpectedSpaceBefore", - "data": { - "tokenValue": "]" - }, - "line": 1, - "column": 85, - "endLine": 1, - "endColumn": 86 - }, - { - "messageId": "unexpectedSpaceAfter", - "data": { - "tokenValue": "[" - }, - "line": 1, - "column": 104, - "endLine": 1, - "endColumn": 105 - }, - { - "messageId": "unexpectedSpaceBefore", - "data": { - "tokenValue": "]" - }, - "line": 1, - "column": 106, - "endLine": 1, - "endColumn": 107 - } - ] -} -``` - -AssertionError [ERR_ASSERTION]: Should have 12 errors but had 0: [] - -0 !== 12 - - at assertErrorCountIsCorrect (apps/oxlint/dist/index.js) - at assertInvalidTestCasePasses (apps/oxlint/dist/index.js) - at runInvalidTestCase (apps/oxlint/dist/index.js) - at apps/oxlint/dist/index.js - - -#### computed-property-spacing > invalid - -```js -class A { [a](){} get [b](){} set [c](foo){} static [d](){} static get [e](){} static set [f](bar){} } -``` - -```json -{ - "output": "class A { [ a ](){} get [ b ](){} set [ c ](foo){} static [ d ](){} static get [ e ](){} static set [ f ](bar){} }", - "options": [ - "always", - {} - ], - "languageOptions": { - "ecmaVersion": 6 - }, - "errors": [ - { - "messageId": "missingSpaceAfter", - "data": { - "tokenValue": "[" - }, - "line": 1, - "column": 11, - "endLine": 1, - "endColumn": 12 - }, - { - "messageId": "missingSpaceBefore", - "data": { - "tokenValue": "]" - }, - "line": 1, - "column": 13, - "endLine": 1, - "endColumn": 14 - }, - { - "messageId": "missingSpaceAfter", - "data": { - "tokenValue": "[" - }, - "line": 1, - "column": 23, - "endLine": 1, - "endColumn": 24 - }, - { - "messageId": "missingSpaceBefore", - "data": { - "tokenValue": "]" - }, - "line": 1, - "column": 25, - "endLine": 1, - "endColumn": 26 - }, - { - "messageId": "missingSpaceAfter", - "data": { - "tokenValue": "[" - }, - "line": 1, - "column": 35, - "endLine": 1, - "endColumn": 36 - }, - { - "messageId": "missingSpaceBefore", - "data": { - "tokenValue": "]" - }, - "line": 1, - "column": 37, - "endLine": 1, - "endColumn": 38 - }, - { - "messageId": "missingSpaceAfter", - "data": { - "tokenValue": "[" - }, - "line": 1, - "column": 53, - "endLine": 1, - "endColumn": 54 - }, - { - "messageId": "missingSpaceBefore", - "data": { - "tokenValue": "]" - }, - "line": 1, - "column": 55, - "endLine": 1, - "endColumn": 56 - }, - { - "messageId": "missingSpaceAfter", - "data": { - "tokenValue": "[" - }, - "line": 1, - "column": 72, - "endLine": 1, - "endColumn": 73 - }, - { - "messageId": "missingSpaceBefore", - "data": { - "tokenValue": "]" - }, - "line": 1, - "column": 74, - "endLine": 1, - "endColumn": 75 - }, - { - "messageId": "missingSpaceAfter", - "data": { - "tokenValue": "[" - }, - "line": 1, - "column": 91, - "endLine": 1, - "endColumn": 92 - }, - { - "messageId": "missingSpaceBefore", - "data": { - "tokenValue": "]" - }, - "line": 1, - "column": 93, - "endLine": 1, - "endColumn": 94 - } - ] -} -``` - -AssertionError [ERR_ASSERTION]: Should have 12 errors but had 0: [] - -0 !== 12 - - at assertErrorCountIsCorrect (apps/oxlint/dist/index.js) - at assertInvalidTestCasePasses (apps/oxlint/dist/index.js) - at runInvalidTestCase (apps/oxlint/dist/index.js) - at apps/oxlint/dist/index.js - - ### `consistent-this` Pass: 23 / 26 (88.5%) diff --git a/apps/oxlint/package.json b/apps/oxlint/package.json index 5674f415450f3..35a2e586e9aa5 100644 --- a/apps/oxlint/package.json +++ b/apps/oxlint/package.json @@ -25,8 +25,10 @@ "devDependencies": { "@types/esquery": "^1.5.4", "@types/estree": "^1.0.8", + "@types/json-schema": "^7.0.15", "@types/json-stable-stringify-without-jsonify": "^1.0.2", "@typescript-eslint/scope-manager": "8.48.1", + "ajv": "^6.12.6", "cross-env": "catalog:", "eslint": "^9.36.0", "esquery": "^1.6.0", diff --git a/apps/oxlint/src-js/plugins/load.ts b/apps/oxlint/src-js/plugins/load.ts index 12eff678c4f64..96da044660d99 100644 --- a/apps/oxlint/src-js/plugins/load.ts +++ b/apps/oxlint/src-js/plugins/load.ts @@ -1,11 +1,11 @@ import { createContext } from "./context.ts"; import { deepFreezeJsonArray } from "./json.ts"; -import { DEFAULT_OPTIONS } from "./options.ts"; +import { compileSchema, DEFAULT_OPTIONS } from "./options.ts"; import { getErrorMessage } from "../utils/utils.ts"; import type { Writable } from "type-fest"; import type { Context } from "./context.ts"; -import type { Options } from "./options.ts"; +import type { Options, SchemaValidator } from "./options.ts"; import type { RuleMeta } from "./rule_meta.ts"; import type { AfterHook, BeforeHook, Visitor, VisitorWithHooks } from "./types.ts"; import type { SetNullable } from "../utils/types.ts"; @@ -56,6 +56,7 @@ interface RuleDetailsBase { readonly isFixable: boolean; readonly messages: Readonly> | null; readonly defaultOptions: Readonly; + readonly optionsSchemaValidator: SchemaValidator | null; // Updated for each file ruleIndex: number; options: Readonly | null; // Initially `null`, set to options object before linting a file @@ -146,7 +147,8 @@ export function registerPlugin(plugin: Plugin, packageName: string | null): Plug // Validate `rule.meta` and convert to vars with standardized shape let isFixable = false, messages: Record | null = null, - defaultOptions: Readonly = DEFAULT_OPTIONS; + defaultOptions: Readonly = DEFAULT_OPTIONS, + schemaValidator: SchemaValidator | null = null; const ruleMeta = rule.meta; if (ruleMeta != null) { if (typeof ruleMeta !== "object") throw new TypeError("Invalid `rule.meta`"); @@ -159,13 +161,18 @@ export function registerPlugin(plugin: Plugin, packageName: string | null): Plug isFixable = true; } + // If `schema` provided, compile schema to validator for applying schema defaults to options + schemaValidator = compileSchema(ruleMeta.schema); + + // Get default options const inputDefaultOptions = ruleMeta.defaultOptions; if (inputDefaultOptions != null) { - // TODO: Validate against provided options schema if (!Array.isArray(inputDefaultOptions)) { throw new TypeError("`rule.meta.defaultOptions` must be an array if provided"); } + // ESLint treats empty `defaultOptions` the same as no `defaultOptions`, + // and does not validate against schema if (inputDefaultOptions.length !== 0) { // Serialize to JSON and deserialize again. // This is the simplest way to make sure that `defaultOptions` does not contain any `undefined` values, @@ -179,6 +186,13 @@ export function registerPlugin(plugin: Plugin, packageName: string | null): Plug ); } + // Apply defaults from schema, if schema was provided. + // `schemaValidator` can mutate original `rule.meta.defaultOptions` object in place, + // but that's what ESLint does, so it's OK. + // TODO: Throw if `rule.meta.schema == null` (only `false` is allowed when `defaultOptions` is provided) + // TODO: Throw if validation errors + if (schemaValidator !== null) schemaValidator(defaultOptions); + deepFreezeJsonArray(defaultOptions as Writable); } } @@ -200,6 +214,7 @@ export function registerPlugin(plugin: Plugin, packageName: string | null): Plug isFixable, messages, defaultOptions, + optionsSchemaValidator: schemaValidator, ruleIndex: 0, options: null, visitor: null, diff --git a/apps/oxlint/src-js/plugins/options.ts b/apps/oxlint/src-js/plugins/options.ts index 29f3c7bbe23c3..dcdf1f730514e 100644 --- a/apps/oxlint/src-js/plugins/options.ts +++ b/apps/oxlint/src-js/plugins/options.ts @@ -3,16 +3,23 @@ */ import assert from "node:assert"; +import Ajv from "ajv"; +import ajvPackageJson from "ajv/package.json" with { type: "json" }; +import metaSchema from "ajv/lib/refs/json-schema-draft-04.json" with { type: "json" }; import { registeredRules } from "./load.ts"; -import { - deepFreezeJsonValue as deepFreezeValue, - deepFreezeJsonArray as deepFreezeArray, - deepFreezeJsonObject as deepFreezeObject, -} from "./json.ts"; -import { debugAssertIsNonNull } from "../utils/asserts.ts"; +import { deepCloneJsonValue, deepFreezeJsonArray } from "./json.ts"; +import { debugAssert, debugAssertIsNonNull } from "../utils/asserts.ts"; +import type { JSONSchema4 } from "json-schema"; import type { Writable } from "type-fest"; import type { JsonValue } from "./json.ts"; +import type { RuleDetails } from "./load.ts"; + +export type SchemaValidator = Ajv.ValidateFunction; + +// ================================================================================================= +// Options types and constants +// ================================================================================================= /** * Options for a rule on a file. @@ -23,11 +30,12 @@ export type Options = JsonValue[]; * Schema describing valid options for a rule. * `schema` property of `RuleMeta`. * - * `false` opts out of schema validation. This is not recommended, as it increases the chance of bugs and mistakes. + * Can be one of: + * - `JSONSchema4` - Full JSON Schema object (must have `type: "array"` at root). + * - `JSONSchema4[]` - Array shorthand where each element describes corresponding options element. + * - `false` - Opts out of schema validation (not recommended). */ -// TODO: Make this more precise. -// TODO: Use this to validate options in configs. -export type RuleOptionsSchema = Record | unknown[] | false; +export type RuleOptionsSchema = JSONSchema4 | JSONSchema4[] | false; // Default rule options export const DEFAULT_OPTIONS: Readonly = Object.freeze([]); @@ -40,6 +48,89 @@ export let allOptions: Readonly[] | null = null; // Index into `allOptions` for default options export const DEFAULT_OPTIONS_ID = 0; +// ================================================================================================= +// Schema compilation +// ================================================================================================= + +// ESLint uses AJV v6. +// AJV v7 removed support for JSON Schema draft-04, which ESLint rule schemas use. +// We must stay on v6 to match ESLint's behavior. +debugAssert( + ajvPackageJson.version.startsWith("6."), + `AJV must be v6.x for JSON Schema draft-04 support, got ${ajvPackageJson.version}`, +); + +// AJV instance configured to match ESLint's behavior. +// `useDefaults: true` applies schema `default` values to options during validation. +// +// Based on ESLint's AJV configuration: +// https://github.com/eslint/eslint/blob/v9.39.2/lib/config/config.js#L15-L16 +// https://github.com/eslint/eslint/blob/v9.39.2/lib/shared/ajv.js#L18-L34 +const AJV = new Ajv({ + meta: false, + useDefaults: true, + validateSchema: false, + missingRefs: "ignore", + verbose: true, + schemaId: "auto", +}); +AJV.addMetaSchema(metaSchema); +// Ajv internal API +(AJV._opts as { defaultMeta: string }).defaultMeta = metaSchema.id; + +// Schema for rules with no options. +// Currently this is unused because we don't support options validation, but leaving it here for when we do. +// oxlint-disable-next-line no-unused-vars +const NO_OPTIONS_SCHEMA: JSONSchema4 = { + type: "array", + minItems: 0, + maxItems: 0, +}; + +/** + * Compile a rule's schema into a validator function. + * + * This should be called once when loading a rule, and the returned validator stored in `RuleDetails`. + * + * ESLint allows array shorthand: `schema: [item1, item2]` which means options[0] must match `item1`, etc. + * This function converts that to a proper JSON Schema, before compiling it. + * + * Based on ESLint's `getRuleOptionsSchema`: + * https://github.com/eslint/eslint/blob/v9.39.2/lib/config/config.js#L177-L210 + * + * @param schema - Rule's schema from `meta.schema` + * @returns Compiled AJV validator, or `null` if no schema + */ +export function compileSchema( + schema: RuleOptionsSchema | null | undefined, +): SchemaValidator | null { + // `null`, `undefined`, or `false` means no schema validation + if (schema == null || schema === false) return null; + + // TODO: Does AJV already do this validation? + if (typeof schema !== "object") { + throw new TypeError("`rule.meta.schema` must be an array, object, or `false` if provided"); + } + + if (Array.isArray(schema)) { + // TODO: Once we support options validation, we should return `NO_OPTIONS_SCHEMA` instead of `null` + if (schema.length === 0) return null; + + schema = { + type: "array", + items: schema, + minItems: 0, + maxItems: schema.length, + }; + } + + return AJV.compile(schema); +} + +// ================================================================================================= +// Options processing +// ================================================================================================= + /** * Set all external rule options. * Called once from Rust after config building, before any linting occurs. @@ -74,28 +165,38 @@ export function setOptions(optionsJson: string): void { } } - // Merge each options array with default options for their corresponding rule. + // Process each options array. + // For each options array, merge with default options and apply schema defaults for the corresponding rule. // Skip the first, as index 0 is a sentinel value meaning default options. First element is never accessed. - // `mergeOptions` also deep-freezes the options. + // `processOptions` also deep-freezes the options. for (let i = 1, len = allOptions.length; i < len; i++) { - allOptions[i] = mergeOptions( + allOptions[i] = processOptions( // `allOptions`' type is `Readonly`, but the array is mutable at present allOptions[i] as Writable<(typeof allOptions)[number]>, - registeredRules[ruleIds[i]].defaultOptions, + registeredRules[ruleIds[i]], ); } } /** - * Merge user-provided options from config with rule's default options. + * Process user-provided options for a rule by applying the rule's defaults + * - merging with default options and applying schema defaults for the rule. * - * Config options take precedence over default options. + * Order of operations (matching ESLint's behavior): + * 1. Merge with `defaultOptions` (config options take precedence). + * 2. Apply schema defaults via AJV (fills in remaining gaps). + * + * This order ensures precedence: config > defaultOptions > schema defaults. + * + * ESLint calls `#normalizeRulesConfig()` first (merges `defaultOptions`), then `validateRulesConfig()` (AJV): + * https://github.com/eslint/eslint/blob/v9.39.2/lib/config/config.js#L483-L484 + * https://github.com/eslint/eslint/blob/v9.39.2/lib/config/config.js#L532-L637 * * Returned options are deep frozen. - * `configOptions` may be frozen in place (or partially frozen) too. * `defaultOptions` must already be deep frozen before calling this function. + * `configOption` may be mutated. * - * Follows the same merging logic as ESLint's `getRuleOptions`. + * Default options merging follows the same logic as ESLint's `getRuleOptions`: * https://github.com/eslint/eslint/blob/0f5a94a84beee19f376025c74f703f275d52c94b/lib/linter/linter.js#L443-L454 * https://github.com/eslint/eslint/blob/0f5a94a84beee19f376025c74f703f275d52c94b/lib/shared/deep-merge-arrays.js * @@ -104,86 +205,123 @@ export function setOptions(optionsJson: string): void { * - Default options: [ [2, 3], 4 ] * - Merged options: [ [1], 4 ] * + * @param configOptions - Options from config (may be mutated by AJV) + * @param ruleDetails - Rule details + * @returns Processed options (deep frozen) + */ +function processOptions(configOptions: Options, ruleDetails: RuleDetails): Readonly { + // Merge with `defaultOptions` first + const { defaultOptions } = ruleDetails; + + const options = + defaultOptions === DEFAULT_OPTIONS + ? configOptions + : mergeOptions(configOptions, defaultOptions); + + // Apply schema defaults (mutates `options` in place). + // + // AJV validation with `useDefaults: true` fills in default values from the schema. + // `mergeOptions` cloned `defaultOptions`, so mutations made by AJV validation won't affect `defaultOptions` + // (and `defaultOptions` is frozen anyway, so it can't be mutated). + // `configOptions` may be mutated, but that's OK, because we only use it once. + // + // We ignore validation errors - we only care about applying defaults. + // TODO: Pass validation errors back to Rust. + const validator = ruleDetails.optionsSchemaValidator; + if (validator !== null) validator(options); + + deepFreezeJsonArray(options); + return options; +} + +/** + * Merge user-provided options from config with rule's default options. + * + * Config options take precedence over default options. + * + * Options returned are entirely mutable. No parts are frozen, even parts which come from default options. + * Any parts of `defaultOptions` which are included are deep cloned. + * Any parts of `configOptions` which are included in return value are *not* cloned. + * * @param configOptions - Options from config * @param defaultOptions - Default options from `rule.meta.defaultOptions` - * @returns Merged options + * @returns Merged options (mutable, not frozen) */ -function mergeOptions( - configOptions: Options, - defaultOptions: Readonly, -): Readonly { - if (defaultOptions === DEFAULT_OPTIONS) { - deepFreezeArray(configOptions); - return configOptions; - } - - // Both are defined - merge them - const merged = []; +function mergeOptions(configOptions: Options, defaultOptions: Readonly): Options { + const merged: Options = []; const defaultOptionsLength = defaultOptions.length, configOptionsLength = configOptions.length, bothLength = Math.min(defaultOptionsLength, configOptionsLength); + // Merge elements shared by both arrays let i = 0; for (; i < bothLength; i++) { merged.push(mergeValues(configOptions[i], defaultOptions[i])); } + // Take remaining elements from whichever array is longer if (defaultOptionsLength > configOptionsLength) { + // `defaultOptions` has more elements - deep clone remaining elements for (; i < defaultOptionsLength; i++) { - merged.push(defaultOptions[i]); + merged.push(deepCloneJsonValue(defaultOptions[i])); } } else { + // `configOptions` has more elements - just copy references (will be frozen later) for (; i < configOptionsLength; i++) { - const prop = configOptions[i]; - deepFreezeValue(prop); - merged.push(prop); + merged.push(configOptions[i]); } } - return Object.freeze(merged); + return merged; } /** * Merge value from user-provided options with value from default options. * + * Config value takes precedence over default value. + * Returns a mutable value (not frozen) - caller is responsible for freezing. + * `configValue` is mutated in place. + * * @param configValue - Value from config * @param defaultValue - Value from default options - * @returns Merged value + * @returns Merged value (mutable) */ function mergeValues(configValue: JsonValue, defaultValue: JsonValue): JsonValue { - // If config value is a primitive, it wins - if (configValue === null || typeof configValue !== "object") return configValue; - - // If config value is an array, it wins - if (Array.isArray(configValue)) { - deepFreezeArray(configValue); + // If config value is a primitive or array, it wins + if (configValue === null || typeof configValue !== "object" || Array.isArray(configValue)) { return configValue; } // If default value is a primitive or an array, config value wins (it's an object) if (defaultValue === null || typeof defaultValue !== "object" || Array.isArray(defaultValue)) { - deepFreezeObject(configValue); return configValue; } - // Both are objects (not arrays) - const merged = { ...defaultValue, ...configValue }; - + // Both are objects (not arrays) - merge `defaultValue` into `configValue`. + // // Symbol properties and circular references are not possible in JSON, so no need to handle them here. - // `configValue` is from JSON, so we can use a simple `for..in` loop over `configValue`. - for (const key in configValue) { + // `defaultValue` is from JSON, so we can use a simple `for..in` loop over `defaultValue`. + for (const key in defaultValue) { // `Object.hasOwn` not `in`, in case `key` is `"__proto__"` - if (Object.hasOwn(defaultValue, key)) { - // `key` is an own property of both `configValue` and `defaultValue`, so must be an own property of `merged` too. + if (Object.hasOwn(configValue, key)) { + // Both have this key - recursively merge. + // `key` is an own property of both `configValue` and `defaultValue`. // Therefore, we don't need special handling for if `key` is `"__proto__"`. // All the property reads and writes here will affect only the owned properties of these objects, // (including if those properties are named `"__proto__"`). - merged[key] = mergeValues(configValue[key], defaultValue[key]); + configValue[key] = mergeValues(configValue[key], defaultValue[key]); } else { - deepFreezeValue(configValue[key]); + // Only `defaultValue` has this key - deep clone and add to `configValue`. + // `Object.defineProperty` in case `key` is `"__proto__"`. + Object.defineProperty(configValue, key, { + value: deepCloneJsonValue(defaultValue[key]), + writable: true, + enumerable: true, + configurable: true, + }); } } - return Object.freeze(merged); + return configValue; } diff --git a/apps/oxlint/test/fixtures/options/.oxlintrc.json b/apps/oxlint/test/fixtures/options/.oxlintrc.json index 642931b27261c..70ec3db2d1e03 100644 --- a/apps/oxlint/test/fixtures/options/.oxlintrc.json +++ b/apps/oxlint/test/fixtures/options/.oxlintrc.json @@ -30,7 +30,26 @@ [], 16 ], - "options-plugin/empty-default-options": ["error", "from-config", 42] + "options-plugin/empty-default-options": ["error", "from-config", 42], + "options-plugin/schema-defaults": "error", + "options-plugin/schema-and-default-options": [ + "error", + { + "fromConfig": 72, + "overrideSchemaByConfig": 32, + "overrideDefaultOptionsByConfig": 62, + "overrideBothByConfig": 42 + } + ], + "options-plugin/schema-and-empty-default-options": [ + "error", + { + "fromConfig": 72, + "overrideSchemaByConfig": 32, + "overrideDefaultOptionsByConfig": 62, + "overrideBothByConfig": 42 + } + ] }, "overrides": [ { @@ -38,7 +57,14 @@ "rules": { "options-plugin/options": ["error", { "somethingElse": true }], "options-plugin/merge-options": ["error", { "fromConfig": 21 }, { "fromConfig": 22 }], - "options-plugin/empty-default-options": "error" + "options-plugin/empty-default-options": "error", + "options-plugin/schema-defaults": [ + "error", + { "overrideSchema": 21, "fromConfig": 31 }, + "config-string" + ], + "options-plugin/schema-and-default-options": "error", + "options-plugin/schema-and-empty-default-options": "error" } } ] diff --git a/apps/oxlint/test/fixtures/options/output.snap.md b/apps/oxlint/test/fixtures/options/output.snap.md index f70d2a8f3ae27..4e9bb14a53964 100644 --- a/apps/oxlint/test/fixtures/options/output.snap.md +++ b/apps/oxlint/test/fixtures/options/output.snap.md @@ -48,17 +48,17 @@ x options-plugin(merge-options): | options: [ | { - | "fromDefault": 1, + | "fromConfig": 11, | "overrideDefault": 12, | "nested": { - | "fromDefault": 3, - | "overrideDefault": 14, | "fromConfig": 13, + | "overrideDefault": 14, | "fromConfigObject": { | "objectKey": 17 - | } + | }, + | "fromDefault": 3 | }, - | "fromConfig": 11 + | "fromDefault": 1 | }, | 15, | true, @@ -99,6 +99,49 @@ : ^ `---- + x options-plugin(schema-and-default-options): + | options: [ + | { + | "fromConfig": 72, + | "overrideSchemaByConfig": 32, + | "overrideDefaultOptionsByConfig": 62, + | "overrideBothByConfig": 42, + | "fromDefaultOptions": 51, + | "overrideSchemaByDefaultOptions": 21, + | "fromSchema": 10 + | } + | ] + | isDeepFrozen: true + ,-[files/index.js:1:1] + 1 | debugger; + : ^ + `---- + + x options-plugin(schema-and-empty-default-options): + | options: [ + | { + | "fromConfig": 72, + | "overrideSchemaByConfig": 32, + | "overrideDefaultOptionsByConfig": 62, + | "overrideBothByConfig": 42, + | "fromSchema": 10, + | "overrideSchemaByDefaultOptions": 20 + | } + | ] + | isDeepFrozen: true + ,-[files/index.js:1:1] + 1 | debugger; + : ^ + `---- + + x options-plugin(schema-defaults): + | options: [] + | isDeepFrozen: true + ,-[files/index.js:1:1] + 1 | debugger; + : ^ + `---- + x options-plugin(default-options): | options: [ | "string", @@ -141,17 +184,17 @@ x options-plugin(merge-options): | options: [ | { + | "fromConfig": 21, | "fromDefault": 1, | "overrideDefault": 2, | "nested": { | "fromDefault": 3, | "overrideDefault": 4 - | }, - | "fromConfig": 21 + | } | }, | { - | "fromDefault": 5, - | "fromConfig": 22 + | "fromConfig": 22, + | "fromDefault": 5 | }, | { | "fromDefault": 6 @@ -184,7 +227,47 @@ : ^ `---- -Found 0 warnings and 10 errors. + x options-plugin(schema-and-default-options): + | options: [ + | { + | "fromDefaultOptions": 51, + | "overrideDefaultOptionsByConfig": 61, + | "overrideSchemaByDefaultOptions": 21, + | "overrideBothByConfig": 41, + | "fromSchema": 10, + | "overrideSchemaByConfig": 30 + | } + | ] + | isDeepFrozen: true + ,-[files/nested/index.js:1:1] + 1 | let x; + : ^ + `---- + + x options-plugin(schema-and-empty-default-options): + | options: [] + | isDeepFrozen: true + ,-[files/nested/index.js:1:1] + 1 | let x; + : ^ + `---- + + x options-plugin(schema-defaults): + | options: [ + | { + | "overrideSchema": 21, + | "fromConfig": 31, + | "fromSchema": 10 + | }, + | "config-string" + | ] + | isDeepFrozen: true + ,-[files/nested/index.js:1:1] + 1 | let x; + : ^ + `---- + +Found 0 warnings and 16 errors. Finished in Xms on 2 files using X threads. ``` diff --git a/apps/oxlint/test/fixtures/options/plugin.ts b/apps/oxlint/test/fixtures/options/plugin.ts index 67cbf1d135feb..4adf48ee15dd4 100644 --- a/apps/oxlint/test/fixtures/options/plugin.ts +++ b/apps/oxlint/test/fixtures/options/plugin.ts @@ -94,6 +94,102 @@ const plugin: Plugin = { return {}; }, }, + + // Rule with schema defaults only (no `defaultOptions`) + "schema-defaults": { + meta: { + schema: [ + { + type: "object", + default: {}, + properties: { + fromSchema: { type: "number", default: 10 }, + overrideSchema: { type: "number", default: 20 }, + }, + additionalProperties: false, + }, + { + type: "string", + default: "schema-default-string", + }, + ], + }, + create(context) { + context.report({ + message: + `\noptions: ${JSON.stringify(context.options, null, 2)}\n` + + `isDeepFrozen: ${isDeepFrozen(context.options)}`, + node: SPAN, + }); + return {}; + }, + }, + + // Rule with both schema defaults AND `defaultOptions`. + // Order: `defaultOptions` merged first, then schema defaults applied after. + "schema-and-default-options": { + meta: { + schema: [ + { + type: "object", + default: {}, + properties: { + fromSchema: { type: "number", default: 10 }, + overrideSchemaByDefaultOptions: { type: "number", default: 20 }, + overrideSchemaByConfig: { type: "number", default: 30 }, + overrideBothByConfig: { type: "number", default: 40 }, + }, + additionalProperties: true, + }, + ], + defaultOptions: [ + { + fromDefaultOptions: 51, + overrideDefaultOptionsByConfig: 61, + overrideSchemaByDefaultOptions: 21, + overrideBothByConfig: 41, + }, + ], + }, + create(context) { + context.report({ + message: + `\noptions: ${JSON.stringify(context.options, null, 2)}\n` + + `isDeepFrozen: ${isDeepFrozen(context.options)}`, + node: SPAN, + }); + return {}; + }, + }, + + // Rule with both schema defaults AND `defaultOptions`, with `defaultOptions` empty + "schema-and-empty-default-options": { + meta: { + schema: [ + { + type: "object", + default: {}, + properties: { + fromSchema: { type: "number", default: 10 }, + overrideSchemaByDefaultOptions: { type: "number", default: 20 }, + overrideSchemaByConfig: { type: "number", default: 30 }, + overrideBothByConfig: { type: "number", default: 40 }, + }, + additionalProperties: true, + }, + ], + defaultOptions: [], + }, + create(context) { + context.report({ + message: + `\noptions: ${JSON.stringify(context.options, null, 2)}\n` + + `isDeepFrozen: ${isDeepFrozen(context.options)}`, + node: SPAN, + }); + return {}; + }, + }, }, }; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a783bc86f4099..ca59122d425cd 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -107,12 +107,18 @@ importers: '@types/estree': specifier: ^1.0.8 version: 1.0.8 + '@types/json-schema': + specifier: ^7.0.15 + version: 7.0.15 '@types/json-stable-stringify-without-jsonify': specifier: ^1.0.2 version: 1.0.2 '@typescript-eslint/scope-manager': specifier: 8.48.1 version: 8.48.1(patch_hash=08ab34b5f27480602bc518186b717a8915aa4d6b2b5df1adc747320ba25b1f8b) + ajv: + specifier: ^6.12.6 + version: 6.12.6 cross-env: specifier: 'catalog:' version: 10.1.0