Skip to content

feat(linter): Implement rule option validation/error handling for jest/no-hooks and eslint/no-return-assign rules.#17600

Merged
graphite-app[bot] merged 1 commit intomainfrom
rule-deserialization-validation
Jan 16, 2026
Merged

feat(linter): Implement rule option validation/error handling for jest/no-hooks and eslint/no-return-assign rules.#17600
graphite-app[bot] merged 1 commit intomainfrom
rule-deserialization-validation

Conversation

@connorshea
Copy link
Member

@connorshea connorshea commented Jan 3, 2026

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.

Copilot AI review requested due to automatic review settings January 3, 2026 05:28
@connorshea connorshea requested a review from camc314 as a code owner January 3, 2026 05:28
@github-actions github-actions bot added A-linter Area - Linter A-cli Area - CLI A-linter-plugins Area - Linter JS plugins C-enhancement Category - New feature or request labels Jan 3, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 DefaultRuleConfig deserialization to properly validate and report configuration errors with detailed error messages
  • Updated jest/no-hooks and eslint/no-return-assign rules to use strict validation with deny_unknown_fields
  • Added comprehensive error handling infrastructure with OverrideRulesError type 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-hq
Copy link

codspeed-hq bot commented Jan 3, 2026

CodSpeed Performance Report

Merging this PR will not alter performance

Comparing rule-deserialization-validation (005a865) with main (6be97f9)

Summary

✅ 4 untouched benchmarks
⏩ 41 skipped benchmarks1

Footnotes

  1. 41 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@connorshea
Copy link
Member Author

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

@connorshea connorshea changed the title feat(linter): Proof-of-concept for rule option validation/error handling feat(linter): Implement rule option validation/error handling for jest/no-hooks and eslint/no-return-assign rules. Jan 3, 2026
@camc314 camc314 self-assigned this Jan 5, 2026
@camc314 camc314 force-pushed the rule-deserialization-validation branch from 06e5c29 to 1765a9b Compare January 7, 2026 14:55
@connorshea
Copy link
Member Author

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.

@camc314
Copy link
Contributor

camc314 commented Jan 11, 2026

Anything we need to finish up for this before merging?

nothing - I just ran out of time last week to get it merged 🙂

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.

This is more for a fix than a breaking change 😉

@camc314 camc314 force-pushed the rule-deserialization-validation branch from 8b9228c to d9d2f20 Compare January 16, 2026 11:06
@camc314 camc314 added the 0-merge Merge with Graphite Merge Queue label Jan 16, 2026
Copy link
Contributor

@camc314 camc314 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ncie work!

Copy link
Contributor

camc314 commented Jan 16, 2026

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.
@graphite-app graphite-app bot force-pushed the rule-deserialization-validation branch from 005a865 to b516088 Compare January 16, 2026 11:33
@graphite-app graphite-app bot merged commit b516088 into main Jan 16, 2026
21 checks passed
@graphite-app graphite-app bot deleted the rule-deserialization-validation branch January 16, 2026 11:39
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Jan 16, 2026
connorshea added a commit that referenced this pull request Jan 16, 2026
… 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.
connorshea added a commit that referenced this pull request Jan 17, 2026
… 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.
graphite-app bot pushed a commit that referenced this pull request Jan 17, 2026
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.
connorshea added a commit that referenced this pull request Jan 21, 2026
… 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.
graphite-app bot pushed a commit that referenced this pull request Jan 21, 2026
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-cli Area - CLI A-linter Area - Linter A-linter-plugins Area - Linter JS plugins C-enhancement Category - New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

linter: Convert one or two rules to return a Result with actual error handling

2 participants

Comments