fix(linter): Enforce config options for react/forbid-dom-props rule.#19387
fix(linter): Enforce config options for react/forbid-dom-props rule.#19387graphite-app[bot] merged 1 commit intomainfrom
react/forbid-dom-props rule.#19387Conversation
react/forbid-dom-props rule.
There was a problem hiding this comment.
Pull request overview
This PR tightens and documents configuration handling for the react/forbid-dom-props lint rule by moving from ad-hoc serde_json::Value parsing to a typed, schema-backed configuration model that can be used for auto-generated docs.
Changes:
- Introduces typed config types (
ForbidDomPropsConfig,ForbidDomPropsItem) withDeserialize+JsonSchemafor documentation generation. - Simplifies config parsing by switching
from_configurationtoDefaultRuleConfig<Self>and converting into an internal lookup map viaFrom. - Removes the manual “Options” section from the rule doc block in favor of schema-driven docs.
Merging this PR will not alter performance
Comparing Footnotes
|
Merge activity
|
#19387) And add proper documentation for the config options so the docs page actually has details on how the config works. This also simplifies the code a bit. Part of #14743. Basically, the changes are: - Move the logic for deserializing the complex JSON config out of `from_configuration` into a derived `From`. - Remove "Options" doc section from the doc block. - Add an actual JsonSchema for the config on this rule so we can auto-generate the docs page. - Simplified the logic for deserialization a bit to remove extra, unnecessary code. Generated config docs: ````md ## Configuration Configuration for the `forbid-dom-props` rule. This rule accepts a configuration object with the following properties: ### forbid type: `array` An array of prop names or objects that are forbidden on DOM elements. Each array element can be a string with the property name, or an object with `propName`, an optional `disallowedFor` array of DOM node names, and an optional custom `message`. Examples: - `["error", { "forbid": ["id", "style"] }]` - `["error", { "forbid": [{ "propName": "className", "message": "Use class instead" }] }]` - `["error", { "forbid": [{ "propName": "style", "disallowedFor": ["div", "span"] }] }]` #### forbid[n] type: `object | string` A forbidden prop, either as a plain prop name string or with options. ##### forbid[n].disallowedFor type: `string[]` A list of DOM element names (e.g. `["div", "span"]`) on which this prop is forbidden. If empty or omitted, the prop is forbidden on all DOM elements. ##### forbid[n].message type: `string` A custom message to display when this prop is used. ##### forbid[n].propName type: `string` The name of the prop to forbid. ```` AI Disclosure: Built with help from Claude Code, Opus 4.6. Tested and reviewed by me, no changes to the test cases/snaps.
540da79 to
c2b1870
Compare
# Oxlint ### 💥 BREAKING CHANGES - 7711821 oxlint: [**BREAKING**] `no-shadow-restricted-names` report `globalThis` by default (#19407) (Sysix) ### 🚀 Features - ce1baa0 linter: Implement eslint/no-shadow (#18979) (Víctor Fernández) - 7a333c1 linter: Support dynamic configs via CLI arguments (#19384) (camc314) - 1bf569b linter: Implement typescript/unified-signatures (#19375) (camc314) - 6562a9b linter: Implement typescript/parameter-properties (#19374) (camc314) - 94d8d74 linter: Implement typescript/no-use-before-define (#19373) (camc314) - 80b002a linter: Implement fixer for unicorn/no-instanceof-builtins (#19371) (camc314) - 5c3784b linter: Implement eslint/no-unmodified-loop-condition (#19341) (Vincent R) - cc00a59 linter/const-comparisons: Improve diagnostics when mixing logical/comparison operators (#19370) (camc314) - ea2c401 linter: Add support for no constructed context values (#18067) (Jovi De Croock) - f2440eb linter: Mark eslint/no-return-assign as having no fixer (#19327) (camc314) - 8588670 linter/unicorn: Implement suggestion for `unicorn/no-await-in-promise-methods` rule (#19359) (Mikhail Baev) - f0af965 linter: Move `jsx-a11y/no-static-element-interactions` rule out of the nursery. (#19355) (connorshea) - be0ce50 linter/tsgolint: Add support for labeled ranges in tsgolint diagnostics (#19201) (camchenry) - b5bc900 linter: Implement fixer for unicorn/no-new-buffer (#19326) (camc314) - 1612932 linter: Add typescript/no-unnecessary-condition (#19130) (camc314) - 37dc6c5 linter: Implement fixer for unicorn/prefer-includes (#19323) (camc314) ### 🐛 Bug Fixes - c2b1870 linter: Enforce config options for `react/forbid-dom-props` rule. (#19387) (connorshea) - 3d24e44 linter: Honor no-empty-function allow getters/setters for object literals (#19393) (camc314) - bbced8d linter: Enforce config options for `eslint/no-empty-function` rule, improve docs. (#19390) (connorshea) - 6bc8aec linter: Fix the behavior of `import/extensions` rule for a file that has multiple extensions. (#18919) (connorshea) - c62a295 linter/img-redundant-alt: Enforce whole-word matching for redundant alt text (#19367) (camc314) - 98956fe linter/describe-function-title: Skip autofix for type-only imports (#19329) (camc314) - d96d26d linter/plugins: Provide `parser.version` (#19364) (overlookmotel) - 81f0039 linter_codegen: Compute rule IDs relative to previous rule (#19350) (camchenry) - b7ef0a8 linter/consistent-indexed-object-style: Avoid unsafe Record conversions for mapped types (#19320) (camc314) ### 📚 Documentation - 3a6059f linter: Improve docs for `eslint/require-await` rule. (#19361) (connorshea) - b069269 linter/plugins: Add comment about deprecated `ScopeManager` methods (#19363) (overlookmotel) - 2d8aaf9 linter: Disable formatting for `eslint/no-unsafe-negation` examples. (#19347) (connorshea) - fb87806 linter: Ensure that we do not auto-format the docs for `unicorn/number-literal-case` rule. (#19346) (connorshea) - 8d3ae27 linter/typescript: Skip docs for deprecated type aware rule options (#19324) (camc314) # Oxfmt ### 💥 BREAKING CHANGES - 00135b5 formatter/sort_imports: [**BREAKING**] Change default `groups` order (#19427) (leaysgur) - 9c34f72 formatter/sort_imports: [**BREAKING**] Report invalid group name with renaming `side-effect` > `side_effect` (#19416) (leaysgur) ### 🚀 Features - 4baebef formatter/sort_imports: Support `{ newlinesBetween: bool }` inside `groups` (#19358) (leaysgur) - d1c2fb6 formatter/sort_imports: Support `customGroups` attributes(`selector` and `modifiers`) (#19356) (leaysgur) ### 🐛 Bug Fixes - f084ea6 oxfmt: Explicitly pass `process.env` for the forked process (#19380) (Long Ho) - 2bc7a14 formatter: Arrow function body incorrectly broken when return type has comment (#19368) (Dunqing) - 90ec3d2 oxfmt: Update tailwind plugin which fixes crash on non-js file (#19353) (leaysgur) - e9c5b1e formatter: Treat `PrivateFieldExpression` as simple call argument (#19348) (Dunqing) - 80643d5 formatter: Match Prettier union indentation with leading comments (#19271) (Dunqing) ### ⚡ Performance - c169c77 syntax: Optimize `is_identifier_name_patched` (#19386) (sapphi-red)
|
@connorshea has something changed in config generation? It looks like actual docs are missing
same thing with generated config doc happens in #20005, it uses similar config for the rule |
|
ooh, good catch. It's not due to a change in the config parsing, it's due to a change I made to simplify the code that the config parsing apparently doesn't handle (where the prior version was handled properly and is what I had used for the PR description, apparently). I am very glad I have the unsquashed commit history of this branch on my laptop or I would not have figured this out. This is the diff that fixes it for me, although you do lose the ability to apply diff --git a/crates/oxc_linter/src/rules/react/forbid_dom_props.rs b/crates/oxc_linter/src/rules/react/forbid_dom_props.rs
index 690f9f05e5..b17041c0b6 100644
--- a/crates/oxc_linter/src/rules/react/forbid_dom_props.rs
+++ b/crates/oxc_linter/src/rules/react/forbid_dom_props.rs
@@ -45,11 +45,7 @@ impl From<ForbidDomPropsConfig> for ForbidDomProps {
ForbidDomPropsItem::PropName(prop_name) => {
forbid.insert(prop_name, ForbidPropOptions::default());
}
- ForbidDomPropsItem::PropWithOptions(PropWithOptions {
- prop_name,
- disallowed_for,
- message,
- }) => {
+ ForbidDomPropsItem::PropWithOptions { prop_name, disallowed_for, message } => {
forbid.insert(
prop_name,
ForbidPropOptions {
@@ -74,21 +70,18 @@ pub enum ForbidDomPropsItem {
/// A prop name to forbid on all DOM elements.
PropName(CompactStr),
/// A prop with optional `disallowedFor` DOM node list and custom `message`.
- PropWithOptions(PropWithOptions),
-}
-
-/// A prop with optional `disallowedFor` DOM node list and custom `message`.
-#[derive(Debug, Clone, Deserialize, JsonSchema)]
-#[serde(rename_all = "camelCase", deny_unknown_fields)]
-pub struct PropWithOptions {
- /// The name of the prop to forbid.
- prop_name: CompactStr,
- /// A list of DOM element names (e.g. `["div", "span"]`) on which this
- /// prop is forbidden. If empty or omitted, the prop is forbidden on all
- /// DOM elements.
- disallowed_for: Option<Vec<CompactStr>>,
- /// A custom message to display when this prop is used.
- message: Option<String>,
+ PropWithOptions {
+ /// The name of the prop to forbid.
+ #[serde(rename = "propName")]
+ prop_name: CompactStr,
+ /// A list of DOM element names (e.g. `["div", "span"]`) on which this
+ /// prop is forbidden. If empty or omitted, the prop is forbidden on all
+ /// DOM elements.
+ #[serde(rename = "disallowedFor")]
+ disallowed_for: Option<Vec<CompactStr>>,
+ /// A custom message to display when this prop is used.
+ message: Option<String>,
+ },
}
/// Configuration for the `forbid-dom-props` rule.Let me see if I can fix the config parsing to handle this properly without changing the config struct/enum, though... |
|
#20018 should fix it |
… configs (#20018) Claude's summary: - Enhanced `schema_markdown.rs` to properly handle `allOf` schemas created by the `RemoveRefSiblings` visitor - When a `$ref` has sibling properties (like `description`), schemars' draft-07 visitor wraps it in an `allOf` - The renderer now resolves through `allOf` structures to find the actual referenced schema ---- This fixes the problem discovered in #19387 (comment). This actually improves the generated docs for a few rules: - `typescript/no-deprecated` - `no-floating-promises` - `react/forbid-dom-props` - `typescript/no-misused-spread` - `typescript/only-throw-error` - `typescript/prefer-readonly-parameter-types` - `typescript/prefer-nullish-coalescing` - `typescript/restrict-template-expressions` Basically, any rule that has a struct nested inside an array has better docs now, as the docs generator actually handles their schema correctly now. AI Disclosure: Generated by Claude Code. Examples: no-deprecated ````md ## Configuration This rule accepts a configuration object with the following properties: ### allow type: `array` default: `[]` An array of type or value specifiers that are allowed to be used even if deprecated. Use this to allow specific deprecated APIs that you intentionally want to continue using. #### allow[n] type: `object | string` Type or value specifier for matching specific declarations Supports four types of specifiers: 1. **String specifier** (deprecated): Universal match by name ```json "Promise" ``` 2. **File specifier**: Match types/values declared in local files ```json { "from": "file", "name": "MyType" } { "from": "file", "name": ["Type1", "Type2"] } { "from": "file", "name": "MyType", "path": "./types.ts" } ``` 3. **Lib specifier**: Match TypeScript built-in lib types ```json { "from": "lib", "name": "Promise" } { "from": "lib", "name": ["Promise", "PromiseLike"] } ``` 4. **Package specifier**: Match types/values from npm packages ```json { "from": "package", "name": "Observable", "package": "rxjs" } { "from": "package", "name": ["Observable", "Subject"], "package": "rxjs" } ``` ##### allow[n].from type: `"file"` ##### allow[n].name type: `array | string` Name specifier that can be a single string or array of strings ###### allow[n].name[n] type: `string` ##### allow[n].path type: `string` Optional file path to specify where the types or values must be declared. If omitted, all files will be matched. ```` react/forbid-dom-props ````md ## Configuration Configuration for the `forbid-dom-props` rule. This rule accepts a configuration object with the following properties: ### forbid type: `array` An array of prop names or objects that are forbidden on DOM elements. Each array element can be a string with the property name, or an object with `propName`, an optional `disallowedFor` array of DOM node names, and an optional custom `message`. Examples: - `["error", { "forbid": ["id", "style"] }]` - `["error", { "forbid": [{ "propName": "className", "message": "Use class instead" }] }]` - `["error", { "forbid": [{ "propName": "style", "disallowedFor": ["div", "span"] }] }]` #### forbid[n] type: `object | string` A forbidden prop, either as a plain prop name string or with options. ##### forbid[n].disallowedFor type: `string[]` A list of DOM element names (e.g. `["div", "span"]`) on which this prop is forbidden. If empty or omitted, the prop is forbidden on all DOM elements. ##### forbid[n].message type: `string` A custom message to display when this prop is used. ##### forbid[n].propName type: `string` The name of the prop to forbid. ```` --------- Co-authored-by: Claude <noreply@anthropic.com>

And add proper documentation for the config options so the docs page actually has details on how the config works. This also simplifies the code a bit. Part of #14743.
Basically, the changes are:
from_configurationinto a derivedFrom.Generated config docs:
AI Disclosure: Built with help from Claude Code, Opus 4.6. Tested and reviewed by me, no changes to the test cases/snaps.