fix(rome_js_analyze): #4410 (#4418)

CHANGELOG.md

@@ -22,6 +22,7 @@ output. [#4405](https://github.com/rome/tools/pull/4405)
 - Add new command `rome migrate` the transform the configuration file `rome.json`
 when there are breaking changes.
 - Fix [#4348](https://github.com/rome/tools/issues/4348) that caused [`noNonNullAssertion`](https://docs.rome.tools/lint/rules/nononnullassertion/) to emit incorrect code action
+- Fix [#4410](https://github.com/rome/tools/issues/4410) that caused [`useButtonType`](https://docs.rome.tools/lint/rules/usebuttontype/) to miss some cases
 
 ### Configuration
 ### Editors

crates/rome_js_analyze/src/semantic_analyzers/a11y/use_button_type.rs

@@ -3,10 +3,10 @@ use crate::semantic_services::Semantic;
 use rome_analyze::{context::RuleContext, declare_rule, Rule, RuleDiagnostic};
 use rome_console::markup;
 use rome_js_syntax::{
-    AnyJsxElementName, JsCallExpression, JsObjectExpression, JsStringLiteralExpression,
-    JsxOpeningElement, JsxString,
+    AnyJsxElementName, JsCallExpression, JsxAttribute, JsxOpeningElement, JsxSelfClosingElement,
+    TextRange,
 };
-use rome_rowan::{declare_node_union, AstNode, AstNodeList};
+use rome_rowan::{declare_node_union, AstNode};
 
 declare_rule! {
     /// Enforces the usage of the attribute `type` for the element `button`
@@ -45,15 +45,11 @@ declare_rule! {
 const ALLOWED_BUTTON_TYPES: [&str; 3] = ["submit", "button", "reset"];
 
 declare_node_union! {
-    pub(crate) UseButtonTypeQuery = JsxOpeningElement | JsCallExpression
-}
-
-declare_node_union! {
-    pub(crate) UseButtonTypeNode = JsxString | JsxOpeningElement | JsStringLiteralExpression | JsObjectExpression
+    pub(crate) UseButtonTypeQuery = JsxSelfClosingElement | JsxOpeningElement | JsCallExpression
 }
 
 pub(crate) struct UseButtonTypeState {
-    node: UseButtonTypeNode,
+    range: TextRange,
     missing_prop: bool,
 }
 
@@ -65,36 +61,34 @@ impl Rule for UseButtonType {
 
     fn run(ctx: &RuleContext<Self>) -> Self::Signals {
         let node = ctx.query();
-
         match node {
-            UseButtonTypeQuery::JsxOpeningElement(opening_element) => {
-                let name = opening_element.name().ok()?;
-                // we bail early the current tag is not a button; case sensitive is important
-                if is_button(&name)? {
-                    let attributes = opening_element.attributes();
-                    if attributes.is_empty() {
-                        return Some(UseButtonTypeState {
-                            node: UseButtonTypeNode::from(opening_element.clone()),
-                            missing_prop: true,
-                        });
-                    } else {
-                        let type_attribute = opening_element.find_attribute_by_name("type").ok()?;
-
-                        if let Some(attribute) = type_attribute {
-                            let initializer = attribute.initializer()?.value().ok()?;
-                            let initializer = initializer.as_jsx_string()?;
-                            if !ALLOWED_BUTTON_TYPES
-                                .contains(&&*initializer.inner_string_text().ok()?)
-                            {
-                                return Some(UseButtonTypeState {
-                                    node: UseButtonTypeNode::from(initializer.clone()),
-                                    missing_prop: false,
-                                });
-                            }
-                        }
-                    }
+            UseButtonTypeQuery::JsxSelfClosingElement(element) => {
+                let name = element.name().ok()?;
+                if !is_button(&name)? {
+                    return None;
                 }
-                None
+                let type_attribute = element.find_attribute_by_name("type").ok()?;
+                let Some(attribute) = type_attribute else {
+                    return Some(UseButtonTypeState {
+                        range: element.range(),
+                        missing_prop: true,
+                    });
+                };
+                inspect_jsx_type_attribute(attribute)
+            }
+            UseButtonTypeQuery::JsxOpeningElement(element) => {
+                let name = element.name().ok()?;
+                if !is_button(&name)? {
+                    return None;
+                }
+                let type_attribute = element.find_attribute_by_name("type").ok()?;
+                let Some(attribute) = type_attribute else {
+                    return Some(UseButtonTypeState {
+                        range: element.range(),
+                        missing_prop: true,
+                    });
+                };
+                inspect_jsx_type_attribute(attribute)
             }
             UseButtonTypeQuery::JsCallExpression(call_expression) => {
                 let model = ctx.model();
@@ -108,19 +102,23 @@ impl Rule for UseButtonType {
                     .as_any_js_literal_expression()?
                     .as_js_string_literal_expression()?;
 
-                // case sensitive is important, <button> is different from <Button>
                 if first_argument.inner_string_text().ok()?.text() == "button" {
                     return if let Some(props) = react_create_element.props.as_ref() {
                         let type_member = react_create_element.find_prop_by_name("type");
                         if let Some(member) = type_member {
                             let property_value = member.value().ok()?;
-                            let value = property_value
+                            let Some(value) = property_value
                                 .as_any_js_literal_expression()?
-                                .as_js_string_literal_expression()?;
+                                .as_js_string_literal_expression() else {
+                                    return Some(UseButtonTypeState {
+                                        range: property_value.range(),
+                                        missing_prop: false,
+                                    });
+                                };
 
                             if !ALLOWED_BUTTON_TYPES.contains(&&*value.inner_string_text().ok()?) {
                                 return Some(UseButtonTypeState {
-                                    node: UseButtonTypeNode::from(value.clone()),
+                                    range: value.range(),
                                     missing_prop: false,
                                 });
                             }
@@ -129,12 +127,12 @@ impl Rule for UseButtonType {
                         // if we are here, it means that we haven't found the property "type" and
                         // we have to return a diagnostic
                         Some(UseButtonTypeState {
-                            node: UseButtonTypeNode::from(props.clone()),
+                            range: props.range(),
                             missing_prop: false,
                         })
                     } else {
                         Some(UseButtonTypeState {
-                            node: UseButtonTypeNode::from(first_argument.clone()),
+                            range: first_argument.range(),
                             missing_prop: true,
                         })
                     };
@@ -156,7 +154,7 @@ impl Rule for UseButtonType {
             }).to_owned()
         };
         Some(RuleDiagnostic::new(rule_category!(),
-            state.node.syntax().text_trimmed_range(),
+            state.range,
             message
         )
             .note(markup! {
@@ -172,10 +170,32 @@ impl Rule for UseButtonType {
     }
 }
 
+fn inspect_jsx_type_attribute(attribute: JsxAttribute) -> Option<UseButtonTypeState> {
+    let Some(initializer) = attribute.initializer() else {
+        return Some(UseButtonTypeState {
+            range: attribute.range(),
+            missing_prop: false,
+        });
+    };
+    let value = initializer.value().ok()?;
+    let Some(value) = value.as_jsx_string() else {
+        // computed value
+        return None;
+    };
+    if ALLOWED_BUTTON_TYPES.contains(&&*value.inner_string_text().ok()?) {
+        return None;
+    }
+    Some(UseButtonTypeState {
+        range: value.range(),
+        missing_prop: false,
+    })
+}
+
 /// Checks whether the current element is a button
 ///
 /// Case sensitive is important, `<button>` is different from `<Button>`
 fn is_button(name: &AnyJsxElementName) -> Option<bool> {
+    // case sensitive is important, <button> is different from <Button>
     Some(match name {
         AnyJsxElementName::JsxName(name) => {
             let name = name.value_token().ok()?;

crates/rome_js_analyze/tests/specs/a11y/useButtonType/inJsx.jsx

@@ -2,11 +2,19 @@
 <>
     <button>do something</button>
     <button type="bar">do something</button>
+    <button type>do something</button>
+    <button/>
+    <button type="bar"/>
+    <button type/>
+    <button onClick={null}>test</button>
+    <button onClick={null}/>
 </>
 
 
 // valid
 <>
     <button type="button">do something</button>
     <button type={dynamic_value}>do something</button>
+    <button type="button"/>
+    <button type={dynamic_value}/>
 </>

crates/rome_js_analyze/tests/specs/a11y/useButtonType/inJsx.jsx.snap

@@ -1,5 +1,6 @@
 ---
 source: crates/rome_js_analyze/tests/spec_tests.rs
+assertion_line: 96
 expression: inJsx.jsx
 ---
 # Input
@@ -8,13 +9,21 @@ expression: inJsx.jsx
 <>
     <button>do something</button>
     <button type="bar">do something</button>
+    <button type>do something</button>
+    <button/>
+    <button type="bar"/>
+    <button type/>
+    <button onClick={null}>test</button>
+    <button onClick={null}/>
 </>
 
 
 // valid
 <>
     <button type="button">do something</button>
     <button type={dynamic_value}>do something</button>
+    <button type="button"/>
+    <button type={dynamic_value}/>
 </>
 ```
 
@@ -29,7 +38,7 @@ inJsx.jsx:3:5 lint/a11y/useButtonType ━━━━━━━━━━━━━━
   > 3 │     <button>do something</button>
       │     ^^^^^^^^
     4 │     <button type="bar">do something</button>
-    5 │ </>
+    5 │     <button type>do something</button>
   
   i The default  type of a button is submit, which causes the submission of a form when placed inside a `form` element. This is likely not the behaviour that you want inside a React application.
   
@@ -47,8 +56,122 @@ inJsx.jsx:4:18 lint/a11y/useButtonType ━━━━━━━━━━━━━
     3 │     <button>do something</button>
   > 4 │     <button type="bar">do something</button>
       │                  ^^^^^
-    5 │ </>
-    6 │ 
+    5 │     <button type>do something</button>
+    6 │     <button/>
+  
+  i The default  type of a button is submit, which causes the submission of a form when placed inside a `form` element. This is likely not the behaviour that you want inside a React application.
+  
+  i Allowed button types are: submit, button or reset
+  
+
+```
+
+```
+inJsx.jsx:5:13 lint/a11y/useButtonType ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Provide a valid type prop for the button element.
+  
+    3 │     <button>do something</button>
+    4 │     <button type="bar">do something</button>
+  > 5 │     <button type>do something</button>
+      │             ^^^^
+    6 │     <button/>
+    7 │     <button type="bar"/>
+  
+  i The default  type of a button is submit, which causes the submission of a form when placed inside a `form` element. This is likely not the behaviour that you want inside a React application.
+  
+  i Allowed button types are: submit, button or reset
+  
+
+```
+
+```
+inJsx.jsx:6:5 lint/a11y/useButtonType ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Provide an explicit type prop for the button element.
+  
+    4 │     <button type="bar">do something</button>
+    5 │     <button type>do something</button>
+  > 6 │     <button/>
+      │     ^^^^^^^^^
+    7 │     <button type="bar"/>
+    8 │     <button type/>
+  
+  i The default  type of a button is submit, which causes the submission of a form when placed inside a `form` element. This is likely not the behaviour that you want inside a React application.
+  
+  i Allowed button types are: submit, button or reset
+  
+
+```
+
+```
+inJsx.jsx:7:18 lint/a11y/useButtonType ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Provide a valid type prop for the button element.
+  
+    5 │     <button type>do something</button>
+    6 │     <button/>
+  > 7 │     <button type="bar"/>
+      │                  ^^^^^
+    8 │     <button type/>
+    9 │     <button onClick={null}>test</button>
+  
+  i The default  type of a button is submit, which causes the submission of a form when placed inside a `form` element. This is likely not the behaviour that you want inside a React application.
+  
+  i Allowed button types are: submit, button or reset
+  
+
+```
+
+```
+inJsx.jsx:8:13 lint/a11y/useButtonType ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Provide a valid type prop for the button element.
+  
+     6 │     <button/>
+     7 │     <button type="bar"/>
+   > 8 │     <button type/>
+       │             ^^^^
+     9 │     <button onClick={null}>test</button>
+    10 │     <button onClick={null}/>
+  
+  i The default  type of a button is submit, which causes the submission of a form when placed inside a `form` element. This is likely not the behaviour that you want inside a React application.
+  
+  i Allowed button types are: submit, button or reset
+  
+
+```
+
+```
+inJsx.jsx:9:5 lint/a11y/useButtonType ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Provide an explicit type prop for the button element.
+  
+     7 │     <button type="bar"/>
+     8 │     <button type/>
+   > 9 │     <button onClick={null}>test</button>
+       │     ^^^^^^^^^^^^^^^^^^^^^^^
+    10 │     <button onClick={null}/>
+    11 │ </>
+  
+  i The default  type of a button is submit, which causes the submission of a form when placed inside a `form` element. This is likely not the behaviour that you want inside a React application.
+  
+  i Allowed button types are: submit, button or reset
+  
+
+```
+
+```
+inJsx.jsx:10:5 lint/a11y/useButtonType ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Provide an explicit type prop for the button element.
+  
+     8 │     <button type/>
+     9 │     <button onClick={null}>test</button>
+  > 10 │     <button onClick={null}/>
+       │     ^^^^^^^^^^^^^^^^^^^^^^^^
+    11 │ </>
+    12 │ 
   
   i The default  type of a button is submit, which causes the submission of a form when placed inside a `form` element. This is likely not the behaviour that you want inside a React application.
   

crates/rome_js_analyze/tests/specs/a11y/useButtonType/inObject.js

@@ -3,6 +3,9 @@ React.createElement('button');
 React.createElement('button', {
     "type": "bar"
 });
+React.createElement('button', {
+    "type": 1
+});
 React.createElement('button', {
     "style": "background: red"
 });