From 2a9c8b0d9cce52b29af45d1c92008b82f99adbd7 Mon Sep 17 00:00:00 2001 From: Victorien Elvinger Date: Thu, 31 Aug 2023 15:57:42 +0200 Subject: [PATCH] fix(lint/noDuplicateJsxProps): make the rule case-sensitive (#100) --- CHANGELOG.md | 9 ++ .../suspicious/no_duplicate_jsx_props.rs | 17 +-- .../noDuplicateJsxProps/invalid.jsx | 5 +- .../noDuplicateJsxProps/invalid.jsx.snap | 107 +++++++----------- .../suspicious/noDuplicateJsxProps/valid.jsx | 3 + .../noDuplicateJsxProps/valid.jsx.snap | 3 + website/src/pages/internals/changelog.mdx | 9 ++ 7 files changed, 72 insertions(+), 81 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 00a0aa8bb027..be19c65cb6b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,15 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom ### Bug fixes +- Fix [#80](https://github.com/biomejs/biome/issues/95), making [noDuplicateJsxProps](https://biomejs.dev/lint/rules/noDuplicateJsxProps/) case-insensitive. + + Some frameworks, such as Material UI, rely on the case-sensitivity of JSX properties. + For example, [TextField has two properties with the same name, but distinct cases](https://mui.com/material-ui/api/text-field/#TextField-prop-inputProps): + + ```jsx + + ``` + - Fix [rome#4713](https://github.com/rome/tools/issues/4713). Previously, [useTemplate](https://biomejs.dev/lint/rules/useTemplate/) made the following suggestion: diff --git a/crates/rome_js_analyze/src/analyzers/suspicious/no_duplicate_jsx_props.rs b/crates/rome_js_analyze/src/analyzers/suspicious/no_duplicate_jsx_props.rs index 9b3de6d3ab4d..4f5219301599 100644 --- a/crates/rome_js_analyze/src/analyzers/suspicious/no_duplicate_jsx_props.rs +++ b/crates/rome_js_analyze/src/analyzers/suspicious/no_duplicate_jsx_props.rs @@ -37,16 +37,6 @@ declare_rule! { } } -fn push_attribute( - attr: JsxAttribute, - attributes: &mut HashMap>, -) -> Option<()> { - let name = attr.name().ok()?.syntax().text_trimmed(); - let name = name.to_string().to_lowercase(); - attributes.entry(name).or_default().push(attr); - Some(()) -} - impl Rule for NoDuplicateJsxProps { type Query = Ast; type State = (String, Vec); @@ -59,7 +49,12 @@ impl Rule for NoDuplicateJsxProps { let mut defined_attributes: HashMap> = HashMap::new(); for attribute in node.attributes() { if let AnyJsxAttribute::JsxAttribute(attr) = attribute { - let _ = push_attribute(attr, &mut defined_attributes); + if let Ok(name) = attr.name() { + defined_attributes + .entry(name.text()) + .or_default() + .push(attr); + } } } diff --git a/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateJsxProps/invalid.jsx b/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateJsxProps/invalid.jsx index a79aa2005c56..190bd7d760d2 100644 --- a/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateJsxProps/invalid.jsx +++ b/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateJsxProps/invalid.jsx @@ -1,13 +1,10 @@ //Duplicate `name` property ; -//Case insensitive case -; - //Case where property is repeated more than twice ; //Case with two duplicates -; +; diff --git a/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateJsxProps/invalid.jsx.snap b/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateJsxProps/invalid.jsx.snap index 0605329ee8be..5dd6ca299adc 100644 --- a/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateJsxProps/invalid.jsx.snap +++ b/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateJsxProps/invalid.jsx.snap @@ -7,14 +7,11 @@ expression: invalid.jsx //Duplicate `name` property ; -//Case insensitive case -; - //Case where property is repeated more than twice ; //Case with two duplicates -; +; @@ -30,7 +27,7 @@ invalid.jsx:2:8 lint/suspicious/noDuplicateJsxProps ━━━━━━━━━ > 2 │ ; │ ^^^^^^^^^^^ 3 │ - 4 │ //Case insensitive case + 4 │ //Case where property is repeated more than twice i This attribute is assigned again here. @@ -38,7 +35,7 @@ invalid.jsx:2:8 lint/suspicious/noDuplicateJsxProps ━━━━━━━━━ > 2 │ ; │ ^^^^^^^^^^^ 3 │ - 4 │ //Case insensitive case + 4 │ //Case where property is repeated more than twice ``` @@ -48,115 +45,93 @@ invalid.jsx:5:8 lint/suspicious/noDuplicateJsxProps ━━━━━━━━━ ! This JSX property is assigned multiple times. - 4 │ //Case insensitive case - > 5 │ ; + 4 │ //Case where property is repeated more than twice + > 5 │ ; │ ^^^^^^^^^^^ 6 │ - 7 │ //Case where property is repeated more than twice + 7 │ //Case with two duplicates i This attribute is assigned again here. - 4 │ //Case insensitive case - > 5 │ ; + 4 │ //Case where property is repeated more than twice + > 5 │ ; │ ^^^^^^^^^^^ 6 │ - 7 │ //Case where property is repeated more than twice - - -``` - -``` -invalid.jsx:8:8 lint/suspicious/noDuplicateJsxProps ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - - ! This JSX property is assigned multiple times. - - 7 │ //Case where property is repeated more than twice - > 8 │ ; - │ ^^^^^^^^^^^ - 9 │ - 10 │ //Case with two duplicates - - i This attribute is assigned again here. - - 7 │ //Case where property is repeated more than twice - > 8 │ ; - │ ^^^^^^^^^^^ - 9 │ - 10 │ //Case with two duplicates + 7 │ //Case with two duplicates i This attribute is assigned again here. - 7 │ //Case where property is repeated more than twice - > 8 │ ; - │ ^^^^^^^^^^^ - 9 │ - 10 │ //Case with two duplicates + 4 │ //Case where property is repeated more than twice + > 5 │ ; + │ ^^^^^^^^^^^ + 6 │ + 7 │ //Case with two duplicates ``` ``` -invalid.jsx:11:8 lint/suspicious/noDuplicateJsxProps ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +invalid.jsx:8:8 lint/suspicious/noDuplicateJsxProps ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This JSX property is assigned multiple times. - 10 │ //Case with two duplicates - > 11 │ ; + 7 │ //Case with two duplicates + > 8 │ ; │ ^^^^^^^^^^^ - 12 │ - 13 │ + 9 │ + 10 │ i This attribute is assigned again here. - 10 │ //Case with two duplicates - > 11 │ ; + 7 │ //Case with two duplicates + > 8 │ ; │ ^^^^^^^^^^^ - 12 │ - 13 │ + 9 │ + 10 │ ``` ``` -invalid.jsx:11:32 lint/suspicious/noDuplicateJsxProps ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +invalid.jsx:8:32 lint/suspicious/noDuplicateJsxProps ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This JSX property is assigned multiple times. - 10 │ //Case with two duplicates - > 11 │ ; + 7 │ //Case with two duplicates + > 8 │ ; │ ^^^^^^^^^^^^^^ - 12 │ - 13 │ + 9 │ + 10 │ i This attribute is assigned again here. - 10 │ //Case with two duplicates - > 11 │ ; + 7 │ //Case with two duplicates + > 8 │ ; │ ^^^^^^^^^^^^^^ - 12 │ - 13 │ + 9 │ + 10 │ ``` ``` -invalid.jsx:13:8 lint/suspicious/noDuplicateJsxProps ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +invalid.jsx:10:8 lint/suspicious/noDuplicateJsxProps ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This JSX property is assigned multiple times. - 11 │ ; - 12 │ - > 13 │ + 8 │ ; + 9 │ + > 10 │ │ ^^^^^^^^^^^^^^^^ - 14 │ + 11 │ i This attribute is assigned again here. - 11 │ ; - 12 │ - > 13 │ + 8 │ ; + 9 │ + > 10 │ │ ^^^^^^^^^^^^^^^^ - 14 │ + 11 │ ``` diff --git a/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateJsxProps/valid.jsx b/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateJsxProps/valid.jsx index 8ef15778ee68..db00c97dceca 100644 --- a/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateJsxProps/valid.jsx +++ b/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateJsxProps/valid.jsx @@ -2,3 +2,6 @@ ; + +//Case insensitive case +; diff --git a/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateJsxProps/valid.jsx.snap b/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateJsxProps/valid.jsx.snap index 501ec7a4a838..1360d79bc9a1 100644 --- a/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateJsxProps/valid.jsx.snap +++ b/crates/rome_js_analyze/tests/specs/suspicious/noDuplicateJsxProps/valid.jsx.snap @@ -9,6 +9,9 @@ expression: valid.jsx +//Case insensitive case +; + ``` diff --git a/website/src/pages/internals/changelog.mdx b/website/src/pages/internals/changelog.mdx index e6324b41cfd3..c27fbe7eefff 100644 --- a/website/src/pages/internals/changelog.mdx +++ b/website/src/pages/internals/changelog.mdx @@ -47,6 +47,15 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom ### Bug fixes +- Fix [#80](https://github.com/biomejs/biome/issues/95), making [noDuplicateJsxProps](https://biomejs.dev/lint/rules/noDuplicateJsxProps/) case-insensitive. + + Some frameworks, such as Material UI, rely on the case-sensitivity of JSX properties. + For example, [TextField has two properties with the same name, but distinct cases](https://mui.com/material-ui/api/text-field/#TextField-prop-inputProps): + + ```html + + ``` + - Fix [rome#4713](https://github.com/rome/tools/issues/4713). Previously, [useTemplate](https://biomejs.dev/lint/rules/useTemplate/) made the following suggestion: