fix(rome_js_analyze): #4410

CHANGELOG.md

@@ -22,6 +22,7 @@ output. [#4405](https:/rome/tools/pull/4405)
 - Add new command `rome migrate` the transform the configuration file `rome.json`
 when there are breaking changes.
 - Fix [#4348](https:/rome/tools/issues/4348) that caused [`noNonNullAssertion`](https://docs.rome.tools/lint/rules/nononnullassertion/) to emit incorrect code action
+- Fix [#4410](https:/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"
 });