Skip to content

chore(linter): Ensure that no unknown fields are allowed in OxlintOverride struct.#16870

Merged
graphite-app[bot] merged 1 commit intomainfrom
overrides-limit
Dec 15, 2025
Merged

chore(linter): Ensure that no unknown fields are allowed in OxlintOverride struct.#16870
graphite-app[bot] merged 1 commit intomainfrom
overrides-limit

Conversation

@connorshea
Copy link
Member

Ensure that extra fields are not allowed inside overrides, to make it clear to users when they have added an invalid field.

Pulled out of #16822 since I think it's worth including regardless of that PR.

@connorshea connorshea requested a review from camc314 as a code owner December 15, 2025 05:33
Copilot AI review requested due to automatic review settings December 15, 2025 05:33
@github-actions github-actions bot added A-linter Area - Linter C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior labels Dec 15, 2025
Copy link

@charliecreates charliecreates bot left a comment

Choose a reason for hiding this comment

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

OxlintOverride now rejects unknown fields at both deserialization and schema-validation time, which is good for catching typos but can be behavior-breaking for any configs relying on ignored extra keys. The main risk is user experience: ensure there’s a regression test verifying the failure mode and that the surfaced error is actionable (field name + override index/path). Consider adding a brief schema/doc note clarifying that unknown keys are not permitted in overrides.

Additional notes (1)
  • Readability | npm/oxlint/configuration_schema.json:505-510
    With additionalProperties: false added, schema consumers will now hard-fail on unknown keys. That’s desirable, but it often triggers churn if the schema is used by editors/validators that might add metadata (or if oxlint plans to add new override keys in the future). Consider whether you want to explicitly document this stricter behavior in the schema’s top-level markdownDescription (or relevant docs) so users know unknown keys are rejected rather than ignored.
Summary of changes

What changed

  • Disallowed unknown fields in override configs by adding #[serde(deny_unknown_fields)] to OxlintOverride in crates/oxc_linter/src/config/overrides.rs.
  • Updated the generated JSON Schema for OxlintOverride to include "additionalProperties": false.
    • Reflected in both the Rust snapshot crates/oxc_linter/src/snapshots/schema_json.snap and the published schema npm/oxlint/configuration_schema.json.

Result

Override objects now fail validation/deserialization when users provide extra/typoed keys, making configuration errors more visible and explicit.

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 enhances configuration validation by ensuring that the OxlintOverride struct rejects unknown/invalid fields during deserialization. This helps users catch typos or invalid configuration options early by providing clear error messages.

Key Changes

  • Added #[serde(deny_unknown_fields)] attribute to the OxlintOverride struct in Rust
  • Added "additionalProperties": false to the corresponding JSON schema definition
  • Updated the schema snapshot to reflect the JSON schema change

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
crates/oxc_linter/src/config/overrides.rs Added #[serde(deny_unknown_fields)] to enforce strict field validation during deserialization
npm/oxlint/configuration_schema.json Added "additionalProperties": false to the OxlintOverride schema for JSON validation
crates/oxc_linter/src/snapshots/schema_json.snap Updated snapshot to match the JSON schema changes

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codspeed-hq
Copy link

codspeed-hq bot commented Dec 15, 2025

CodSpeed Performance Report

Merging #16870 will not alter performance

Comparing overrides-limit (b2273d7) with main (aafcf3e)1

Summary

✅ 4 untouched
⏩ 41 skipped2

Footnotes

  1. No successful run was found on main (e0d8001) during the generation of this report, so aafcf3e was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

  2. 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.

@camc314 camc314 self-assigned this Dec 15, 2025
@camc314 camc314 added the 0-merge Merge with Graphite Merge Queue label Dec 15, 2025
Copy link
Contributor

camc314 commented Dec 15, 2025

Merge activity

…rride struct. (#16870)

Ensure that extra fields are not allowed inside overrides, to make it clear to users when they have added an invalid field.

Pulled out of #16822 since I think it's worth including regardless of that PR.
@graphite-app graphite-app bot merged commit 7510414 into main Dec 15, 2025
20 checks passed
@graphite-app graphite-app bot deleted the overrides-limit branch December 15, 2025 09:50
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Dec 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-linter Area - Linter C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants