fix(linter): deny unknown options for some rules#22924
Conversation
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. This stack of pull requests is managed by Graphite. Learn more about stacking. |
There was a problem hiding this comment.
Pull request overview
This PR aims to make rule configuration stricter by rejecting unknown/typoed options for a set of linter rules (primarily by adding deny_unknown_fields to their config types).
Changes:
- Add
deny_unknown_fieldsto multiple rule config structs/enums to prevent silently-ignored unknown options during deserialization. - Tighten option schemas for several Jest/Vitest shared-rule configs and a few React/TypeScript/Import/ESLint-derived rules.
- Extend “deny unknown options” behavior across more rule plugins (jest/vitest, import, eslint, react, jsdoc, typescript).
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/oxc_linter/src/rules/typescript/no_restricted_types.rs | Adds deny_unknown_fields to an untagged ban-config enum to reject unknown object keys. |
| crates/oxc_linter/src/rules/shared/jest_vitest/valid_expect.rs | Adds deny_unknown_fields to the schema/metadata for ValidExpect options. |
| crates/oxc_linter/src/rules/shared/jest_vitest/prefer_expect_assertions.rs | Adds deny_unknown_fields to PreferExpectAssertions config. |
| crates/oxc_linter/src/rules/shared/jest_vitest/no_large_snapshots.rs | Adds deny_unknown_fields to NoLargeSnapshots config. |
| crates/oxc_linter/src/rules/shared/jest_vitest/expect_expect.rs | Adds deny_unknown_fields to the schema/metadata for ExpectExpect options. |
| crates/oxc_linter/src/rules/shared/jest_vitest/consistent_test_it.rs | Adds deny_unknown_fields to ConsistentTestIt config. |
| crates/oxc_linter/src/rules/react/forbid_elements.rs | Adds deny_unknown_fields to an untagged forbidden-element item enum. |
| crates/oxc_linter/src/rules/jsdoc/require_param.rs | Adds deny_unknown_fields to RequireParam rule config. |
| crates/oxc_linter/src/rules/jest/prefer_importing_jest_globals.rs | Adds deny_unknown_fields to PreferImportingJestGlobals config. |
| crates/oxc_linter/src/rules/jest/prefer_ending_with_an_expect.rs | Adds deny_unknown_fields to PreferEndingWithAnExpect config. |
| crates/oxc_linter/src/rules/jest/no_deprecated_functions.rs | Adds deny_unknown_fields to NoDeprecatedFunctions config. |
| crates/oxc_linter/src/rules/import/no_nodejs_modules.rs | Attempts to deny unknown fields for NoNodejsModules config (but currently via schemars). |
| crates/oxc_linter/src/rules/import/extensions.rs | Adds deny_unknown_fields to Extensions config metadata/schema. |
| crates/oxc_linter/src/rules/eslint/complexity.rs | Adds deny_unknown_fields to Complexity config. |
| crates/oxc_linter/src/rules/eslint/class_methods_use_this.rs | Adds deny_unknown_fields to ClassMethodsUseThis config. |
Comments suppressed due to low confidence (3)
crates/oxc_linter/src/rules/import/no_nodejs_modules.rs:31
deny_unknown_fieldsis applied under#[schemars(...)], which only affects the generated JSON schema. It does not make config deserialization reject unknown keys, so the rule still accepts unknown options at runtime. To actually deny unknown options (and to allow{}to deserialize via the struct default), this needs to be a#[serde(...)]container attribute instead.
#[derive(Debug, Default, Clone, Serialize, Deserialize, JsonSchema)]
#[schemars(rename_all = "camelCase", deny_unknown_fields)]
pub struct NoNodejsModulesConfig {
/// Array of names of allowed modules. Defaults to an empty array.
allow: FxHashSet<CompactStr>,
}
crates/oxc_linter/src/rules/shared/jest_vitest/expect_expect.rs:60
deny_unknown_fieldsonExpectExpectConfigonly affects the generated schema. Runtime config parsing is manual (ExpectExpectConfig::from_configuration) and ignores unknown keys, so unknown options are still accepted at runtime (contrary to the PR intent). To truly deny unknown options, parse viaserde_json::from_value(deriveDeserialize) or add an explicit unknown-key check infrom_configurationand return an error.
#[derive(Debug, Clone, JsonSchema)]
#[serde(rename_all = "camelCase", default, deny_unknown_fields)]
pub struct ExpectExpectConfig {
/// A list of function names that should be treated as assertion functions.
///
/// NOTE: The default value is `["expect"]` for Jest and
/// `["expect", "expectTypeOf", "assert", "assertType"]` for Vitest.
#[serde(rename = "assertFunctionNames")]
assert_function_names_jest: Vec<CompactStr>,
#[schemars(skip)] // Skipped because this field isn't exposed to the user.
assert_function_names_vitest: Vec<CompactStr>,
/// An array of function names that should also be treated as test blocks.
additional_test_block_functions: Vec<CompactStr>,
}
crates/oxc_linter/src/rules/typescript/no_restricted_types.rs:55
- This note is inaccurate:
falsedoes fail to deserialize (as intended), but it won’t be “ignored” — it will cause the rule configuration to be rejected with an error viaserde_json::from_value::<DefaultRuleConfig<_>>(). This is especially relevant now that config deserialization is becoming stricter.
/// `"TypeName": true` - ban with default message
/// Note: Only `true` is valid; `false` would fail to deserialize and be ignored.
Bool(True),
Merging this PR will not alter performance
Comparing Footnotes
|
fabbe93 to
020b891
Compare
Merge activity
|
camc314
left a comment
There was a problem hiding this comment.
nice!
we can merge this incrementally as, and when you find issues.
Just a start. I get out of hand dangling all the PRs :') This is a part of #22907 Found with #22907 (comment)
020b891 to
594ed86
Compare
# Oxlint ### 🚀 Features - e805174 linter: Add schema for `jest/vitest/max-expects` (#23105) (Sysix) - 7850577 linter: Add schema for `jest/vitest/expect-expect` (#23104) (Sysix) - 75f641a linter: Add schema for `jest/vitest/consistent-test-it` (#23103) (Sysix) - 5125f89 linter/unicorn: Support no-null `checkArguments` option (#23098) (camc314) - b8b9797 linter: Add schema for `import-max-dependencies` (#23096) (Sysix) - 65cb47a linter/eslint: Support no-unused-expressions `ignoreDirectives` option (#23097) (camc314) - f6c36d5 linter: Add schema for `import/prefer-default-export` (#23091) (Sysix) - 0d4a5d1 linter: Add schema for `eslint/sort-vars` (#23090) (Sysix) - fdb5bf5 linter: Add schema for `eslint/radix` (#23082) (Sysix) - 05b4dcf linter: Add schema for `eslint/prefer-const` (#23081) (Sysix) - 5a06c4d linter/vue: Implement next-tick-style rule (#23041) (Alex Peshkov) - e38a36a linter: Add schema for `eslint/operator-assignment` (#23080) (Sysix) - 907cee7 linter: Add schema for `eslint/no-warning-comments` (#23075) (Sysix) - 9470bb2 linter: Add schema for `eslint/no-unused-vars` (#23073) (Sysix) - 234b5cf linter: Add schema for `eslint/no-shadow` (#23072) (Sysix) - de0dd8b linter: Add schema for `eslint/no-restricted-exports` (#23020) (Sysix) - faa3e0d linter: Add schema for `eslint/no-param-reassign` (#23018) (Sysix) - dbc9c27 linter: Add schema for `eslint/no-magic-numbers` (#23017) (Sysix) - 38d3569 linter: Add schema for `eslint/no-inner-declarations` (#23016) (Sysix) - 008fa41 linter: Add schema for `eslint/no-constant-condition` (#22991) (Sysix) - ca44623 linter: Add schema for `eslint/no-empty-function` (#22988) (Sysix) - 43eb04d linter: Add schema for `eslint/id-match` (#22987) (Sysix) - a800f27 linter: Add schema for `eslint/capitalized-comments` (#22984) (Sysix) - 96e2d32 linter: Add schema for `eslint/id-length` (#22963) (Sysix) - 545493f linter: Add schema for `eslint/complexity` (#22960) (Sysix) - 5f0b558 linter: Add schema for `eslint/class-methods-use-this` (#22959) (Sysix) - 719b720 linter: Add schema for simple rule configurations (#22948) (Sysix) - fd00966 linter: Add right schema for `eslint/max-*` rules (#22923) (Sysix) - 1226d78 linter: Fill schema with rule configurations (#22907) (Sysix) - 8f423c1 linter/vue: Implement `require-direct-export` rule (#17623) (yefan) - 78e915b linter/vue: Implement no-reserved-props rule (#22914) (bab) - 0f200a9 linter/vue: Implement require-prop-types rule (#22083) (Alex Peshkov) - 5da9da9 linter/vue: Implement no-reserved-keys rule (#21780) (bab) - 75e14a8 linter/vue: Implement prop-name-casing rule (#22892) (bab) - 85efabf semantic: Make building the class table optional, off by default (#22862) (Boshen) ### 🐛 Bug Fixes - a49b0cf linter/no-map-spread: Remove ineffective autofix (#22956) (camc314) - cf53285 parser: Report reserved type-declaration names in the parser (#23035) (Boshen) - 0383e61 linter: Fix schema for rules without a config (#22946) (Sysix) - 4d722e0 parser: Report duplicate switch `default` clause in the parser (#23012) (Boshen) - 6cb34b8 linter/plugins: Make spreading `Token` instances keep `loc` property (#22947) (Nicolas Le Cam) - 27de044 linter/plugins: Make spreading `Comment` instances keep `loc` property (#22238) (Nicolas Le Cam) - 742fd0b linter/double-comparisons: Make fixer a suggestion (#22968) (camc314) - 93f4494 linter: Respect default child config plugin when extending parent config (#22903) (Sysix) - 594ed86 linter: Deny unknown options for some rules (#22924) (Sysix) - 3253038 linter/expect-expect: Align default rule options (#22890) (camc314) - bbe44ea linter: Respect default plugins from extended config (#22896) (Sysix) ### ⚡ Performance - 0b7ce7e linter/plugins: Create global prop vars at top level of modules (#22928) (overlookmotel) - 0f7c319 linter/plugins: Define class `#loc` setter functions as `const`s (#22919) (overlookmotel) ### 📚 Documentation - 7b0380d linter: Remove preserve-caught-error note (#22994) (camc314) - dadafe3 oxlint, oxfmt: Mention migrate skills in npm READMEs (#22965) (Boshen) # Oxfmt ### 🚀 Features - 3da77e0 oxfmt: Format `parser:json5` files by `oxc_formatter_json` (#22990) (leaysgur) - c786f0d oxfmt: Format `parser:jsonc` files by `oxc_formatter_json` (#22913) (leaysgur) - 27a6db8 formatter_json: Implement jsonc variant (#22912) (leaysgur) ### 🐛 Bug Fixes - 2aedd52 oxfmt: Avoid JS promise rejects for all TSFN call sites (#23107) (leaysgur) - 01e0871 formatter,formatter_json: Handle PS/LS as line terminator (#22978) (leaysgur) - 23902d9 formatter_json: Handle CR only line breaks (#22977) (leaysgur) - 136b72b formatter_json: Use line_suffix for line comment outside array (#22931) (leaysgur) - 44e40fa formatter_json: Expand line comment inside array (#22911) (leaysgur) - 2c86896 formatter_json: Avoid example binary name collision (#22904) (camc314) ### 📚 Documentation - cc69d8d formatter_json: Update AGENTS.md (#22981) (leaysgur) - 0490721 formatter_json: Update AGENTS.md (#22976) (leaysgur) - dadafe3 oxlint, oxfmt: Mention migrate skills in npm READMEs (#22965) (Boshen) - f88961a oxfmt: Annotate each config option with supported languages (#22953) (leaysgur) - 7e514bf formatter_json: Update AGENTS.md (#22930) (leaysgur) Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>

Just a start. I get out of hand dangling all the PRs :')
This is a part of #22907
Found with #22907 (comment)