From 5aa3efa97cc366612cf84ca6c117e706765295fc Mon Sep 17 00:00:00 2001 From: Victorien ELVINGER Date: Fri, 28 Apr 2023 18:29:23 +0200 Subject: [PATCH] fix(rome_js_analyze): #4410 (#4418) --- CHANGELOG.md | 1 + .../a11y/use_button_type.rs | 108 +++++++++------ .../tests/specs/a11y/useButtonType/inJsx.jsx | 8 ++ .../specs/a11y/useButtonType/inJsx.jsx.snap | 129 +++++++++++++++++- .../specs/a11y/useButtonType/inObject.js | 3 + .../specs/a11y/useButtonType/inObject.js.snap | 51 +++++-- .../a11y/useButtonType/withBindingInvalid.js | 4 + .../useButtonType/withBindingInvalid.js.snap | 23 ++++ .../withDefaultNamespaceInvalid.js | 3 + .../withDefaultNamespaceInvalid.js.snap | 23 ++++ .../useButtonType/withRenamedImportInvalid.js | 4 + .../withRenamedImportInvalid.js.snap | 23 ++++ website/src/playground/tabs/SettingsTab.tsx | 17 ++- 13 files changed, 331 insertions(+), 66 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 50d3ea3698e..e8cbf402b28 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/crates/rome_js_analyze/src/semantic_analyzers/a11y/use_button_type.rs b/crates/rome_js_analyze/src/semantic_analyzers/a11y/use_button_type.rs index adaa082efb5..4b9cd6cab21 100644 --- a/crates/rome_js_analyze/src/semantic_analyzers/a11y/use_button_type.rs +++ b/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::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, + + + + + + + + │ ^^^^^^^^ 4 │ - 5 │ + 5 │ 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 │ > 4 │ │ ^^^^^ - 5 │ - 6 │ + 5 │ + 6 │ + 4 │ + > 5 │ + │ ^^^^ + 6 │ + 5 │ + > 6 │ + 6 │ + + 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 │ + 10 │ + │ ^^^^^^^^^^^^^^^^^^^^^^^ + 10 │ + > 10 │ - @@ -249,7 +253,7 @@ function FileView({

Files - @@ -342,12 +346,12 @@ function FileViewItem({
  • {filename} - {canDelete && ( - @@ -667,6 +671,7 @@ function LineWidthInput({