fix(linter): Fix JSON schema to deny additional properties for plugins enum.#15259
fix(linter): Fix JSON schema to deny additional properties for plugins enum.#15259
Conversation
…s enum. This ensures that VS Code and other editors will not accept unknown values for the plugins field. I believe `string` was allowed initially because of the way jsPlugins were going to be implemented, but then the jsPlugins plans changed to use a separate config field? I'm not 100% sure, but that appears to be the case. Anyway, the schema was never updated after that pivot, and so this fixes the problem. We could probably remove the `any_of` to simplify things, but eh I just want to fix this for now.
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
There was a problem hiding this comment.
Pull Request Overview
This PR simplifies the JSON schema for lint plugin options by removing the arbitrary string type, restricting plugin specifications to only predefined enum values.
- Removes the
"type": "string"option from the plugin schema'sanyOfarray - Restricts plugin names to the enumerated values defined in
LintPluginOptionsSchema - Updates both the generated schema JSON file and the snapshot test
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| npm/oxlint/configuration_schema.json | Removes string type from plugin options schema |
| crates/oxc_linter/src/snapshots/schema_json.snap | Updates snapshot to reflect schema change |
| crates/oxc_linter/src/config/plugins.rs | Removes code that generated string schema option |
Comments suppressed due to low confidence (2)
crates/oxc_linter/src/config/plugins.rs:258
- The
anyOfsubschema validation with a single item is unnecessary. When there's only one schema option, it should be used directly as theitem_schemainstead of wrapping it in ananyOfarray. Replace lines 252-258 withlet item_schema = enum_schema;to simplify the code.
let item_schema = Schema::Object(schemars::schema::SchemaObject {
subschemas: Some(Box::new(schemars::schema::SubschemaValidation {
any_of: Some(vec![enum_schema]),
..Default::default()
})),
..Default::default()
});
crates/oxc_linter/src/config/plugins.rs:248
- The schema enum only accepts the kebab-case canonical names for each plugin (e.g., 'typescript'), but the deserialization logic in
LintPlugins::try_from(lines 130-154) accepts multiple aliases for plugins. For example, 'typescript-eslint', 'typescript_eslint', and '@typescript-eslint' are all valid for the TypeScript plugin, but the schema will reject them. This creates a mismatch between what the schema allows and what the code accepts, potentially breaking existing configurations that use these valid aliases. Consider either: (1) keeping the string type in the schema to allow validation at runtime, or (2) documenting that only canonical names are supported and updating error messages to suggest the canonical form.
enum LintPluginOptionsSchema {
Eslint,
React,
Unicorn,
Typescript,
Oxc,
Import,
Jsdoc,
Jest,
Vitest,
JsxA11y,
Nextjs,
ReactPerf,
Promise,
Node,
Regex,
Vue,
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
CodSpeed Performance ReportMerging #15259 will not alter performanceComparing Summary
Footnotes
|
This ensures that VS Code and other editors will not accept unknown values for the plugins field.
I believe
stringwas allowed initially because of the way jsPlugins were going to be implemented (see #12117), but then the jsPlugins plans changed to use a separate config field? I'm not 100% sure, but that appears to be the case. Anyway, the schema was never updated after that pivot, and so this fixes the problem. We could probably remove theany_ofto simplify things, but eh I just want to fix this for now.With this change, jsPlugins still allows arbitrary values, while
pluginsonly allows known values:In the future, it'd probably be good to add tests to do basic schema validation (e.g. here are 10 JSON blobs that should pass, here are 10 JSON blobs that should fail) so we can ensure regressions don't occur for this kind of developer experience nice-to-have.
Part of #15247.