feat(linter): Implement rule option validation/error handling for jest/no-hooks and eslint/no-return-assign rules.#17600
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements a proof-of-concept for rule option validation and error handling in the oxlint linter. The changes enable proper validation of rule configurations and surface meaningful errors when users provide invalid options, replacing the previous behavior of silently falling back to defaults with unwrap_or_default().
Key Changes:
- Enhanced
DefaultRuleConfigdeserialization to properly validate and report configuration errors with detailed error messages - Updated
jest/no-hooksandeslint/no-return-assignrules to use strict validation withdeny_unknown_fields - Added comprehensive error handling infrastructure with
OverrideRulesErrortype for better error propagation through the configuration system
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
crates/oxc_linter/src/rule.rs |
Modified DefaultRuleConfig deserialization to validate configurations and return detailed error messages instead of silently using defaults |
crates/oxc_linter/src/config/rules.rs |
Added OverrideRulesError enum to properly categorize and format configuration errors |
crates/oxc_linter/src/config/config_builder.rs |
Updated error handling to use new OverrideRulesError and added RuleConfigurationError variant to ConfigBuilderError |
crates/oxc_linter/src/rules/jest/no_hooks.rs |
Fixed typo in diagnostic function name, added period to error message, enabled deny_unknown_fields, updated from_configuration to propagate errors, removed invalid test cases, added new test cases and validation tests |
crates/oxc_linter/src/rules/eslint/no_return_assign.rs |
Updated from_configuration to properly propagate errors and added comprehensive validation tests |
crates/oxc_linter/src/snapshots/jest_no_hooks.snap |
Updated snapshot with corrected diagnostic message and additional test cases |
apps/oxlint/test/fixtures/invalid_config_* |
Added snapshot tests demonstrating error handling for various invalid configuration scenarios (unknown fields, type mismatches, invalid enum values, configurations in overrides) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
CodSpeed Performance ReportMerging this PR will not alter performanceComparing Summary
Footnotes
|
apps/oxlint/test/fixtures/invalid_config_multiple_rules/output.snap.md
Outdated
Show resolved
Hide resolved
|
Did a run of ecosystem-ci and it did not cause any new failures, so that's good: https://github.com/oxc-project/oxc-ecosystem-ci/actions/runs/20672971798 |
apps/oxlint/test/fixtures/invalid_config_in_override/files/index.js
Outdated
Show resolved
Hide resolved
06e5c29 to
1765a9b
Compare
|
Anything we need to finish up for this before merging? Only real concern I have is that this could technically count as a breaking change in our versioning rules, but I would argue against it really warranting a major version bump. |
nothing - I just ran out of time last week to get it merged 🙂
This is more for a fix than a breaking change 😉 |
8b9228c to
d9d2f20
Compare
Merge activity
|
…t/no-hooks and eslint/no-return-assign rules. (#17600) AI Disclosure: The changes to `config_builder.rs` and DefaultRuleConfig were heavily assisted with AI (Copilot + Raptor mini + Claude Opus 4.5), but generally seem reasonable to me. I went through a few iterations to get a simpler implementation of the changeset and I'm relatively happy with where we're at now. I am unsure about how performant it is for a large/complex configuration, though. Closes #17506. Follow-up on #17479. Basically, this updates two rules (`jest/no-hooks` and `eslint/no-return-assign`) to surface errors when the user provides invalid configuration options. This will surface all config errors it finds and then bail without doing any linting. This will hard-error and not continue linting when it hits an invalid config object. There are valid arguments for not doing this and instead just logging a warning and falling back to default values, but I think this is probably the correct option? I have added direct tests to the rule files themselves for valid/invalid config options, it may be a good idea to update the Tester to accept array of invalid and valid config shapes, but that's something we can figure out later. I've also added some snapshot tests to ensure that we have snapshots of what it looks like when an invalid config option is hit, and can ensure we don't regress on this behavior in the future.
005a865 to
b516088
Compare
… object. This was done using a script that looks for specific usages of DefaultRuleConfig to ensure that this only changes rules which are safe to enforce this on in-bulk. This builds on top of the changes from #17600. Note that I was not able to make this change for `jest/prefer-lowercase-title` or `import/no-cycle`, as both have technically-incorrect types for the values they accept in their tests. This should probably be fixed by removing the relevant tests that allow those values, or by updating the config struct for the rules to allow the currently-invalid values.
… object. This was done using a script that looks for specific usages of DefaultRuleConfig to ensure that this only changes rules which are safe to enforce this on in-bulk. This builds on top of the changes from #17600. Note that I was not able to make this change for `jest/prefer-lowercase-title` or `import/no-cycle`, as both have technically-incorrect types for the values they accept in their tests. This should probably be fixed by removing the relevant tests that allow those values, or by updating the config struct for the rules to allow the currently-invalid values.
We want to test the output that results from multiple invalid rules, so we are doing that now. This was a mistake made in #17600. Also add a test for rule alias support, to make sure vitest/no-hooks has the same config option validation.
… object. This was done using a script that looks for specific usages of DefaultRuleConfig to ensure that this only changes rules which are safe to enforce this on in-bulk. This builds on top of the changes from #17600. Note that I was not able to make this change for `jest/prefer-lowercase-title` or `import/no-cycle`, as both have technically-incorrect types for the values they accept in their tests. This should probably be fixed by removing the relevant tests that allow those values, or by updating the config struct for the rules to allow the currently-invalid values.
…alid config options. (#18104) Ran oxc-ecosystem-ci and it passed fine, no diff vs. main: https://github.com/oxc-project/oxc-ecosystem-ci/actions/runs/21199291772 Trial by fire, I guess. If this is too big of a change, we can split it up into batches of 25 each. This builds on top of the changes from #17600 and various changes we've made over the last few months to adopt DefaultRuleConfig and other validations for the rule configuration logic. Note that I was not able to make this change for `jest/prefer-lowercase-title` or `import/no-cycle`, as both have some tests using incorrect types in their tests, and so those tests begin failing if I try to expose invalid config options. This should probably be fixed by removing the relevant tests that allow those values, as the values already are treated as invalid, or by updating the config struct for the rules to allow the currently-invalid values if we really want to support them. But that is a separate concern. Note that `eslint/prefer-promise-reject-errors` is also not updated by this PR, as that is already covered by #18103. Part of the goal of this change is to implement some of #17854, and also to prevent users from shooting themselves in the foot with a bad config hallucinated by AI tooling. I used AI to create a quick little script and a bunch of invalid oxlint configs for some rules from this PR (and other PRs I opened today), see https://github.com/connorshea/oxlint-issue-repro-template/tree/invalid-configs. The result is printed below. <details> <summary>Invalid configs output</summary> ``` ============================================================ Config: <code-dir>/oxlint-invalid-configs/configs/eslint-no-bitwise.json Command: node <code-dir>/oxc/apps/oxlint/dist/cli.js -c=<code-dir>/oxlint-invalid-configs/configs/eslint-no-bitwise.json ------------------------------------------------------------ Failed to parse oxlint configuration file. x Invalid configuration for rule `no-bitwise`: invalid type: string "not-an-array", expected a sequence, received `{ "allow": "not-an-array" }` ------------------------------------------------------------ Exit code: 1 ============================================================ Config: <code-dir>/oxlint-invalid-configs/configs/eslint-no-cond-assign.json Command: node <code-dir>/oxc/apps/oxlint/dist/cli.js -c=<code-dir>/oxlint-invalid-configs/configs/eslint-no-cond-assign.json ------------------------------------------------------------ Failed to parse oxlint configuration file. x Invalid configuration for rule `no-cond-assign`: unknown variant `exceptions`, expected `except-parens` or `always`, received `{ "exceptions": 123 }` ------------------------------------------------------------ Exit code: 1 ============================================================ Config: <code-dir>/oxlint-invalid-configs/configs/eslint-no-unused-expressions.json Command: node <code-dir>/oxc/apps/oxlint/dist/cli.js -c=<code-dir>/oxlint-invalid-configs/configs/eslint-no-unused-expressions.json ------------------------------------------------------------ Failed to parse oxlint configuration file. x Invalid configuration for rule `no-unused-expressions`: invalid type: string "not-boolean", expected a boolean, received `{ "allowShortCircuit": "not-boolean" }` ------------------------------------------------------------ Exit code: 1 ============================================================ Config: <code-dir>/oxlint-invalid-configs/configs/eslint-unicode-bom.json Command: node <code-dir>/oxc/apps/oxlint/dist/cli.js -c=<code-dir>/oxlint-invalid-configs/configs/eslint-unicode-bom.json ------------------------------------------------------------ Failed to parse oxlint configuration file. x Invalid configuration for rule `unicode-bom`: unknown variant `require`, expected `always` or `never`, received `{ "require": 0 }` ------------------------------------------------------------ Exit code: 1 ============================================================ Config: <code-dir>/oxlint-invalid-configs/configs/import-consistent-type-specifier-style.json Command: node <code-dir>/oxc/apps/oxlint/dist/cli.js -c=<code-dir>/oxlint-invalid-configs/configs/import-consistent-type-specifier-style.json ------------------------------------------------------------ Failed to parse oxlint configuration file. x Invalid configuration for rule `import/consistent-type-specifier-style`: unknown variant `specifierStyle`, expected `prefer-top-level` or `prefer-inline`, received `{ "specifierStyle": 42 }` ------------------------------------------------------------ Exit code: 1 ============================================================ Config: <code-dir>/oxlint-invalid-configs/configs/import-no-absolute-path.json Command: node <code-dir>/oxc/apps/oxlint/dist/cli.js -c=<code-dir>/oxlint-invalid-configs/configs/import-no-absolute-path.json ------------------------------------------------------------ Failed to parse oxlint configuration file. x Invalid configuration for rule `import/no-absolute-path`: unknown field `allow`, expected one of `esmodule`, `commonjs`, `amd`, received `{ "allow": "yes" }` ------------------------------------------------------------ Exit code: 1 ============================================================ Config: <code-dir>/oxlint-invalid-configs/configs/import-no-duplicates.json Command: node <code-dir>/oxc/apps/oxlint/dist/cli.js -c=<code-dir>/oxlint-invalid-configs/configs/import-no-duplicates.json ------------------------------------------------------------ Failed to parse oxlint configuration file. x Invalid configuration for rule `import/no-duplicates`: unknown field `preferInlineType`, expected `prefer-inline` or `preferInline`, received `{ "preferInlineType": "maybe" }` ------------------------------------------------------------ Exit code: 1 ============================================================ Config: <code-dir>/oxlint-invalid-configs/configs/import-no-unassigned-import.json Command: node <code-dir>/oxc/apps/oxlint/dist/cli.js -c=<code-dir>/oxlint-invalid-configs/configs/import-no-unassigned-import.json ------------------------------------------------------------ Failed to parse oxlint configuration file. x Invalid configuration for rule `import/no-unassigned-import`: invalid type: integer `123`, expected a sequence, received `{ "allow": 123 }` ------------------------------------------------------------ Exit code: 1 ============================================================ Config: <code-dir>/oxlint-invalid-configs/configs/import-prefer-default-export.json Command: node <code-dir>/oxc/apps/oxlint/dist/cli.js -c=<code-dir>/oxlint-invalid-configs/configs/import-prefer-default-export.json ------------------------------------------------------------ Failed to parse oxlint configuration file. x Invalid configuration for rule `import/prefer-default-export`: invalid type: integer `123`, expected string or map, received `{ "target": 123 }` ------------------------------------------------------------ Exit code: 1 ============================================================ Config: <code-dir>/oxlint-invalid-configs/configs/jsdoc-require-returns.json Command: node <code-dir>/oxc/apps/oxlint/dist/cli.js -c=<code-dir>/oxlint-invalid-configs/configs/jsdoc-require-returns.json ------------------------------------------------------------ Failed to parse oxlint configuration file. x Invalid configuration for rule `jsdoc/require-returns`: invalid type: integer `999`, expected a sequence, received `{ "exemptedBy": 999 }` ------------------------------------------------------------ Exit code: 1 ============================================================ Config: <code-dir>/oxlint-invalid-configs/configs/jsx-fragments.json Command: node <code-dir>/oxc/apps/oxlint/dist/cli.js -c=<code-dir>/oxlint-invalid-configs/configs/jsx-fragments.json ------------------------------------------------------------ Failed to parse oxlint configuration file. x Invalid configuration for rule `react/jsx-fragments`: data did not match any variant of untagged enum JsxFragments, received `{ "eventHandlerPrefix": 123 }` ------------------------------------------------------------ Exit code: 1 ============================================================ Config: <code-dir>/oxlint-invalid-configs/configs/react-jsx-no-target-blank.json Command: node <code-dir>/oxc/apps/oxlint/dist/cli.js -c=<code-dir>/oxlint-invalid-configs/configs/react-jsx-no-target-blank.json ------------------------------------------------------------ Failed to parse oxlint configuration file. x Invalid configuration for rule `react/jsx-no-target-blank`: unknown field `enforce_dynamic_links`, expected one of `enforceDynamicLinks`, `warnOnSpreadAttributes`, `allowReferrer`, `links`, `forms`, received `{ "enforce_dynamic_links": 123 }` ------------------------------------------------------------ Exit code: 1 ============================================================ Config: <code-dir>/oxlint-invalid-configs/configs/react-prefer-es6-class.json Command: node <code-dir>/oxc/apps/oxlint/dist/cli.js -c=<code-dir>/oxlint-invalid-configs/configs/react-prefer-es6-class.json ------------------------------------------------------------ Failed to parse oxlint configuration file. x Invalid configuration for rule `react/prefer-es6-class`: unknown variant `allow`, expected `always` or `never`, received `{ "allow": 1 }` ------------------------------------------------------------ Exit code: 1 ============================================================ Config: <code-dir>/oxlint-invalid-configs/configs/react-state-in-constructor.json Command: node <code-dir>/oxc/apps/oxlint/dist/cli.js -c=<code-dir>/oxlint-invalid-configs/configs/react-state-in-constructor.json ------------------------------------------------------------ Failed to parse oxlint configuration file. x Invalid configuration for rule `react/state-in-constructor`: unknown variant `mode`, expected `always` or `never`, received `{ "mode": 123 }` ------------------------------------------------------------ Exit code: 1 ============================================================ Config: <code-dir>/oxlint-invalid-configs/configs/typescript-consistent-indexed-object-style.json Command: node <code-dir>/oxc/apps/oxlint/dist/cli.js -c=<code-dir>/oxlint-invalid-configs/configs/typescript-consistent-indexed-object-style.json ------------------------------------------------------------ Failed to parse oxlint configuration file. x Invalid configuration for rule `typescript/consistent-indexed-object-style`: unknown variant `style`, expected `record` or `index-signature`, received `{ "style": true }` ------------------------------------------------------------ Exit code: 1 ============================================================ Config: <code-dir>/oxlint-invalid-configs/configs/typescript-consistent-type-definitions.json Command: node <code-dir>/oxc/apps/oxlint/dist/cli.js -c=<code-dir>/oxlint-invalid-configs/configs/typescript-consistent-type-definitions.json ------------------------------------------------------------ Failed to parse oxlint configuration file. x Invalid configuration for rule `typescript/consistent-type-definitions`: unknown variant `style`, expected `interface` or `type`, received `{ "style": 456 }` ------------------------------------------------------------ Exit code: 1 ============================================================ Config: <code-dir>/oxlint-invalid-configs/configs/typescript-no-explicit-any.json Command: node <code-dir>/oxc/apps/oxlint/dist/cli.js -c=<code-dir>/oxlint-invalid-configs/configs/typescript-no-explicit-any.json ------------------------------------------------------------ Failed to parse oxlint configuration file. x Invalid configuration for rule `typescript/no-explicit-any`: invalid type: string "maybe", expected a boolean, received `{ "fixToUnknown": "maybe" }` ------------------------------------------------------------ Exit code: 1 ============================================================ Config: <code-dir>/oxlint-invalid-configs/configs/typescript-prefer-nullish-coalescing.json Command: node <code-dir>/oxc/apps/oxlint/dist/cli.js -c=<code-dir>/oxlint-invalid-configs/configs/typescript-prefer-nullish-coalescing.json ------------------------------------------------------------ Failed to parse oxlint configuration file. x Invalid configuration for rule `typescript/prefer-nullish-coalescing`: data did not match any variant of untagged enum IgnorePrimitives, received `{ "ignorePrimitives": "not-an-object" }` ------------------------------------------------------------ Exit code: 1 ============================================================ Config: <code-dir>/oxlint-invalid-configs/configs/typescript-return-await.json Command: node <code-dir>/oxc/apps/oxlint/dist/cli.js -c=<code-dir>/oxlint-invalid-configs/configs/typescript-return-await.json ------------------------------------------------------------ Failed to parse oxlint configuration file. x Invalid configuration for rule `typescript/return-await`: unknown variant `allowIn`, expected one of `in-try-catch`, `always`, `error-handling-correctness-only`, `never`, received `{ "allowIn": 1 }` ------------------------------------------------------------ Exit code: 1 ============================================================ Config: <code-dir>/oxlint-invalid-configs/configs/unicorn-explicit-length-check.json Command: node <code-dir>/oxc/apps/oxlint/dist/cli.js -c=<code-dir>/oxlint-invalid-configs/configs/unicorn-explicit-length-check.json ------------------------------------------------------------ Failed to parse oxlint configuration file. x Invalid configuration for rule `unicorn/explicit-length-check`: unknown field `allowedTypes`, expected `non-zero`, received `{ "allowedTypes": 456 }` ------------------------------------------------------------ Exit code: 1 ============================================================ Config: <code-dir>/oxlint-invalid-configs/configs/unicorn-no-array-reverse.json Command: node <code-dir>/oxc/apps/oxlint/dist/cli.js -c=<code-dir>/oxlint-invalid-configs/configs/unicorn-no-array-reverse.json ------------------------------------------------------------ Failed to parse oxlint configuration file. x Invalid configuration for rule `unicorn/no-array-reverse`: unknown field `allow_as_expression`, expected `allowExpressionStatement`, received `{ "allow_as_expression": "nope" }` ------------------------------------------------------------ Exit code: 1 ============================================================ Config: <code-dir>/oxlint-invalid-configs/configs/unicorn-switch-case-braces.json Command: node <code-dir>/oxc/apps/oxlint/dist/cli.js -c=<code-dir>/oxlint-invalid-configs/configs/unicorn-switch-case-braces.json ------------------------------------------------------------ Failed to parse oxlint configuration file. x Invalid configuration for rule `unicorn/switch-case-braces`: unknown variant `caseBraces`, expected `always` or `avoid`, received `{ "caseBraces": 123 }` ------------------------------------------------------------ Exit code: 1 ============================================================ Config: <code-dir>/oxlint-invalid-configs/configs/vue-define-emits-declaration.json Command: node <code-dir>/oxc/apps/oxlint/dist/cli.js -c=<code-dir>/oxlint-invalid-configs/configs/vue-define-emits-declaration.json ------------------------------------------------------------ Failed to parse oxlint configuration file. x Invalid configuration for rule `vue/define-emits-declaration`: unknown variant `declaration`, expected one of `type-based`, `type-literal`, `runtime`, received `{ "declaration": 0 }` ------------------------------------------------------------ Exit code: 1 ============================================================ Config: <code-dir>/oxlint-invalid-configs/configs/vue-define-props-declaration.json Command: node <code-dir>/oxc/apps/oxlint/dist/cli.js -c=<code-dir>/oxlint-invalid-configs/configs/vue-define-props-declaration.json ------------------------------------------------------------ Failed to parse oxlint configuration file. x Invalid configuration for rule `vue/define-props-declaration`: unknown variant `declaration`, expected `type-based` or `runtime`, received `{ "declaration": false }` ------------------------------------------------------------ Exit code: 1 Done. ``` </details> ------ AI Disclosure: This changeset was produced using Node.js scripts which I generated using AI, to find specific patterns in the codebase and then perform the replacements in the code accordingly, the two scripts are included below. I have gone through the 125 changes and confirmed that all 125 are identical to one another, and are the intended end-result code. Basically, they look for rules which have code like this, and then perform the updates only on those rules: ```rs fn from_configuration(value: serde_json::Value) -> Result<Self, serde_json::error::Error> { Ok(serde_json::from_value::<DefaultRuleConfig<Self>>(value) .unwrap_or_default() .into_inner()) } ``` This avoids needing to go through manually and decide which of these rules to update, and also ensures that all 125 of these rules were safe to update - as the configuration has already been updated to use DefaultRuleConfig, which means the unit tests have validated that the shape of the JSON schema for the rule matches the actual values ingested by the rule. <details> <summary>Node scripts for editing the rule files</summary> `add-deny-unknown-fields.js`: ```js #!/usr/bin/env node /* Add `deny_unknown_fields` to `#[serde(...)]` attributes in files under `rules/`. Usage: node scripts/add_deny_unknown_fields.js [--apply] [--base PATH] - Scans for Rust files under any `rules/` directory. - For files that contain the token `DefaultRuleConfig` and a `#[serde(...)]` attribute containing `rename_all = "camelCase"` and `default`, it will add `deny_unknown_fields` to that serde attribute if missing. This is conservative and only modifies attributes that match those conditions and currently do not include `deny_unknown_fields`. Examples: # Preview changes (dry-run): node scripts/add_deny_unknown_fields.js # Apply changes in-place: node scripts/add_deny_unknown_fields.js --apply */ const fs = require("fs").promises; const path = require("path"); const ATTR_RE = /(#\s*\[\s*serde\(([^)]*)\)\s*\])/gs; // s flag to allow newlines (node 12+) const RENAME_RE = /rename_all\s*=\s*"camelCase"/; async function findRuleFiles(base) { const results = []; const rulesDir = path.join(base, "crates", "oxc_linter", "src", "rules"); try { const stat = await fs.stat(rulesDir); if (!stat.isDirectory()) return results; } catch (e) { // No rules directory found; return empty list return results; } async function walk(dir) { const entries = await fs.readdir(dir, { withFileTypes: true }); for (const ent of entries) { const res = path.join(dir, ent.name); if (ent.isDirectory()) { // skip node_modules and target for speed if (ent.name === "node_modules" || ent.name === "target") continue; await walk(res); } else if (ent.isFile() && res.endsWith(".rs")) { results.push(res); } } } await walk(rulesDir); return results; } function updateSerdeAttribute(full, inner) { // inner is the content inside serde(...) const hasRename = RENAME_RE.test(inner); const hasDefault = /\bdefault\b/.test(inner); const hasDeny = /deny_unknown_fields/.test(inner); if (hasRename && hasDefault && !hasDeny) { // normalize tokens by splitting on commas const tokens = inner .split(",") .map((t) => t.trim()) .filter(Boolean); if (!tokens.includes("deny_unknown_fields")) tokens.push("deny_unknown_fields"); const newInner = tokens.join(", "); const newFull = full.replace("(" + inner + ")", "(" + newInner + ")"); return { newFull, newInner }; } return null; } async function processFile(filePath, apply) { const text = await fs.readFile(filePath, "utf8"); const FROM_CONF_SNIPPET = `fn from_configuration(value: serde_json::Value) -> Result<Self, serde_json::error::Error> { Ok(serde_json::from_value::<DefaultRuleConfig<Self>>(value) .unwrap_or_default() .into_inner()) }`; const FROM_CONF_SNIPPET_ALT = `fn from_configuration(value: Value) -> Result<Self, serde_json::error::Error> { Ok(serde_json::from_value::<DefaultRuleConfig<Self>>(value) .unwrap_or_default() .into_inner()) }`; // Only process files that contain the exact from_configuration method above if (!text.includes(FROM_CONF_SNIPPET) && !text.includes(FROM_CONF_SNIPPET_ALT)) return false; let changed = false; let out = text; const edits = []; let m; while ((m = ATTR_RE.exec(text)) !== null) { const full = m[1]; const inner = m[2]; const upd = updateSerdeAttribute(full, inner); if (upd) { edits.push({ full, newFull: upd.newFull }); } } if (edits.length > 0) { for (const e of edits) { out = out.replace(e.full, e.newFull); changed = true; console.log(`Will update: ${filePath}:`); console.log(` - ${e.full.trim()}`); console.log(` + ${e.newFull.trim()}`); console.log(""); } if (apply) { await fs.writeFile(filePath, out, "utf8"); console.log(`Applied changes to ${filePath}\n`); } } return changed; } async function main() { const args = process.argv.slice(2); const apply = args.includes("--apply"); const quiet = args.includes("--quiet"); const baseIndex = args.findIndex((a) => a === "--base"); let base = process.cwd(); if (baseIndex !== -1 && args.length > baseIndex + 1) base = path.resolve(args[baseIndex + 1]); try { const files = await findRuleFiles(base); if (!files.length) { if (!quiet) console.log("No files found in crates/oxc_linter/src/rules/."); return; } let changedAny = false; const editedFiles = []; for (const f of files) { try { const changed = await processFile(f, apply); if (changed) { changedAny = true; if (apply) editedFiles.push(f); } } catch (err) { console.error(`Error processing ${f}: ${err}`); } } if (!changedAny && !quiet) console.log("No matching serde attributes found that needed changes."); if (changedAny && !apply) console.log("\nRun with --apply to make the changes."); if (apply && editedFiles.length > 0) { if (!quiet) { console.log("\nEdited files:"); for (const ef of editedFiles) console.log(` - ${ef}`); } } else if (apply && !quiet) { console.log("\nNo files were changed."); } } catch (err) { console.error(err); process.exit(2); } } if (require.main === module) main(); ``` `update-from-configuration.js`: ```js #!/usr/bin/env node /* Update `from_configuration` implementations in rule files that include `deny_unknown_fields`. Usage: node scripts/update_from_configuration.js [--apply] [--base PATH] - Scans for Rust files under `crates/oxc_linter/src/rules`. - For files that contain the token `deny_unknown_fields` and the exact `from_configuration` method form that uses `unwrap_or_default().into_inner()`, it replaces that method body with a shorter `map(DefaultRuleConfig::into_inner)` call as described in the repo's style. Examples: # Preview changes (dry-run): node scripts/update_from_configuration.js # Apply changes in-place: node scripts/update_from_configuration.js --apply */ const fs = require("fs").promises; const path = require("path"); async function findRuleFiles(base) { const results = []; const rulesDir = path.join(base, "crates", "oxc_linter", "src", "rules"); try { const stat = await fs.stat(rulesDir); if (!stat.isDirectory()) return results; } catch (e) { return results; } async function walk(dir) { const entries = await fs.readdir(dir, { withFileTypes: true }); for (const ent of entries) { const res = path.join(dir, ent.name); if (ent.isDirectory()) { if (ent.name === "node_modules" || ent.name === "target") continue; await walk(res); } else if (ent.isFile() && res.endsWith(".rs")) { results.push(res); } } } await walk(rulesDir); return results; } async function processFile(filePath, apply) { const text = await fs.readFile(filePath, "utf8"); // Only handle files that mention deny_unknown_fields if (!text.includes("deny_unknown_fields")) return false; const FROM_CONF_SNIPPET = `fn from_configuration(value: serde_json::Value) -> Result<Self, serde_json::error::Error> { Ok(serde_json::from_value::<DefaultRuleConfig<Self>>(value) .unwrap_or_default() .into_inner()) }`; const FROM_CONF_SNIPPET_ALT = `fn from_configuration(value: Value) -> Result<Self, serde_json::error::Error> { Ok(serde_json::from_value::<DefaultRuleConfig<Self>>(value) .unwrap_or_default() .into_inner()) }`; const REPLACEMENT = `fn from_configuration(value: serde_json::Value) -> Result<Self, serde_json::error::Error> { serde_json::from_value::<DefaultRuleConfig<Self>>(value).map(DefaultRuleConfig::into_inner) }`; const REPLACEMENT_ALT = `fn from_configuration(value: Value) -> Result<Self, serde_json::error::Error> { serde_json::from_value::<DefaultRuleConfig<Self>>(value).map(DefaultRuleConfig::into_inner) }`; if (!text.includes(FROM_CONF_SNIPPET) && !text.includes(FROM_CONF_SNIPPET_ALT)) return false; let out = text; let changed = false; if (out.includes(FROM_CONF_SNIPPET)) { out = out.replace(FROM_CONF_SNIPPET, REPLACEMENT); changed = true; console.log(`Will update: ${filePath} - replace serde_json::Value form`); } if (out.includes(FROM_CONF_SNIPPET_ALT)) { out = out.replace(FROM_CONF_SNIPPET_ALT, REPLACEMENT_ALT); changed = true; console.log(`Will update: ${filePath} - replace Value form`); } if (changed && apply) { await fs.writeFile(filePath, out, "utf8"); console.log(`Applied changes to ${filePath}\n`); } return changed; } async function main() { const args = process.argv.slice(2); const apply = args.includes("--apply"); const quiet = args.includes("--quiet"); const baseIndex = args.findIndex((a) => a === "--base"); let base = process.cwd(); if (baseIndex !== -1 && args.length > baseIndex + 1) base = path.resolve(args[baseIndex + 1]); try { const files = await findRuleFiles(base); if (!files.length) { if (!quiet) console.log("No files found in crates/oxc_linter/src/rules/."); return; } let changedAny = false; const editedFiles = []; for (const f of files) { try { const changed = await processFile(f, apply); if (changed) { changedAny = true; if (apply) editedFiles.push(f); } } catch (err) { console.error(`Error processing ${f}: ${err}`); } } if (!changedAny && !quiet) console.log("No matching from_configuration methods found that needed changes."); if (changedAny && !apply) console.log("\nRun with --apply to make the changes."); if (apply && editedFiles.length > 0) { if (!quiet) { console.log("\nEdited files:"); for (const ef of editedFiles) console.log(` - ${ef}`); } } else if (apply && !quiet) { console.log("\nNo files were changed."); } } catch (err) { console.error(err); process.exit(2); } } if (require.main === module) main(); ``` </details>
AI Disclosure: The changes to
config_builder.rsand DefaultRuleConfig were heavily assisted with AI (Copilot + Raptor mini + Claude Opus 4.5), but generally seem reasonable to me. I went through a few iterations to get a simpler implementation of the changeset and I'm relatively happy with where we're at now. I am unsure about how performant it is for a large/complex configuration, though.Closes #17506. Follow-up on #17479.
Basically, this updates two rules (
jest/no-hooksandeslint/no-return-assign) to surface errors when the user provides invalid configuration options.This will surface all config errors it finds and then bail without doing any linting.
This will hard-error and not continue linting when it hits an invalid config object. There are valid arguments for not doing this and instead just logging a warning and falling back to default values, but I think this is probably the correct option?
I have added direct tests to the rule files themselves for valid/invalid config options, it may be a good idea to update the Tester to accept array of invalid and valid config shapes, but that's something we can figure out later.
I've also added some snapshot tests to ensure that we have snapshots of what it looks like when an invalid config option is hit, and can ensure we don't regress on this behavior in the future.