From 530b487757e355249e18afeca74841ef819bb895 Mon Sep 17 00:00:00 2001 From: unvalley Date: Fri, 5 May 2023 16:23:34 +0900 Subject: [PATCH 1/5] feat(rome_js_analyze): update noNoninteractiveElementToInteractiveRole feat: wip fix: handle input type hidden and refactor --- crates/rome_aria/src/roles.rs | 152 +- ...interactive_element_to_interactive_role.rs | 6 +- .../nursery/no_noninteractive_tabindex.rs | 2 +- .../invalid.jsx | 90 +- .../invalid.jsx.snap | 1394 ++++++++++++++++- .../invalid.jsx.snap.new | 1228 +++++++++++++++ .../valid.jsx | 107 ++ .../valid.jsx.snap | 107 ++ 8 files changed, 3010 insertions(+), 76 deletions(-) create mode 100644 crates/rome_js_analyze/tests/specs/a11y/noNoninteractiveElementToInteractiveRole/invalid.jsx.snap.new diff --git a/crates/rome_aria/src/roles.rs b/crates/rome_aria/src/roles.rs index ce4d7258180..cb71d2a71f2 100644 --- a/crates/rome_aria/src/roles.rs +++ b/crates/rome_aria/src/roles.rs @@ -324,7 +324,7 @@ define_role! { /// https://www.w3.org/TR/wai-aria-1.1/#link LinkRole { PROPS: [("aria-expanded", false)], - ROLES: ["command"], + ROLES: ["command", "widget"], CONCEPTS: &[("a", &[]), ("link", &[])], } } @@ -342,7 +342,7 @@ define_role! { /// https://www.w3.org/TR/wai-aria-1.1/#listbox ListBoxRole { PROPS: [], - ROLES: ["select"], + ROLES: ["select", "widget"], CONCEPTS: &[("select", &[])], } } @@ -365,10 +365,11 @@ define_role! { } define_role! { - /// https://www.w3.org/TR/wai-aria-1.1/#main + /// https://w3c.github.io/aria/#main MainRole { PROPS: [], ROLES: ["landmark"], + CONCEPTS: &[("main", &[])], } } @@ -382,9 +383,17 @@ define_role! { define_role! { /// https://www.w3.org/TR/wai-aria-1.1/#menu - MenuItem { + MenuRole { PROPS: [("aria-posinset", false), ("aria-setsize", false)], - ROLES: ["command"], + ROLES: ["select"], + } +} + +define_role! { + /// https://www.w3.org/TR/wai-aria-1.1/#menuitem + MenuItemRole { + PROPS: [("aria-posinset", false), ("aria-setsize", false)], + ROLES: ["command", "widget"], } } @@ -392,7 +401,7 @@ define_role! { /// https://www.w3.org/TR/wai-aria-1.1/#menuitemcheckbox MenuItemCheckboxRole { PROPS: [("aria-checked", true)], - ROLES: ["checkbox", "menuitem"], + ROLES: ["checkbox", "menuitem", "widget"], } } @@ -400,7 +409,7 @@ define_role! { /// https://www.w3.org/TR/wai-aria-1.1/#menuitemradio MenuItemRadioRole { PROPS: [("aria-checked", true)], - ROLES: ["radio", "menuitemcheckbox"], + ROLES: ["radio", "menuitemcheckbox", "widget"], } } @@ -417,7 +426,7 @@ define_role! { /// https://www.w3.org/TR/wai-aria-1.1/#progressbar ProgressBarRole { PROPS: [("aria-valuenow", true), ("aria-valuemin", true), ("aria-valuemax", true)], - ROLES: ["range"], + ROLES: ["range", "widget"], } } define_role! { @@ -466,7 +475,7 @@ define_role! { ("aria-readonly", false), ("aria-required", false), ], - ROLES: ["textbox"], + ROLES: ["textbox", "widget"], CONCEPTS: &[("input", &[("type", "search")])], } } @@ -516,7 +525,7 @@ define_role! { ("aria-readonly", false), ("aria-required", false), ], - ROLES: ["input"], + ROLES: ["input", "widget"], CONCEPTS: &[("textarea", &[]), ("input", &[("type", "search")])], } } @@ -547,10 +556,11 @@ define_role! { } define_role! { - /// https://www.w3.org/TR/wai-aria-1.2/#complementary + /// https://w3c.github.io/aria/#complementary ComplementaryRole { PROPS: [], ROLES: ["landmark"], + CONCEPTS: &[("aside", &[])], } } @@ -564,11 +574,11 @@ define_role! { } define_role! { - /// https://www.w3.org/TR/wai-aria-1.2/#caption + /// https://w3c.github.io/aria/#caption CaptionRole { PROPS: [], ROLES: ["section"], - CONCEPTS: &[("caption", &[]), ("figcaption", &[])], + CONCEPTS: &[("caption", &[]), ("figcaption", &[]), ("legend", &[])], } } @@ -683,6 +693,7 @@ define_role! { StatusRole { PROPS: [], ROLES: ["section"], + CONCEPTS: &[("output", &[])], } } @@ -712,6 +723,42 @@ define_role! { } } +define_role! { + /// https://w3c.github.io/aria/#mark + MarkRole { + PROPS: [], + ROLES: ["section"], + CONCEPTS: &[("mark", &[])], + } +} + +define_role! { + /// https://w3c.github.io/aria/#marquee + MarqueeRole { + PROPS: [], + ROLES: ["section"], + CONCEPTS: &[("marquee", &[])], + } +} + +define_role! { + /// https://w3c.github.io/aria/#associationlist + AssociationListRole { + PROPS: [], + ROLES: ["section"], + CONCEPTS: &[("dl", &[])], + } +} + +define_role! { + /// https://w3c.github.io/aria/#contentinfo + ContentInfoRole { + PROPS: [], + ROLES: ["landmark"], + CONCEPTS: &[("footer", &[])], + } +} + impl<'a> AriaRoles { /// These are roles that will contain "concepts". pub(crate) const ROLE_WITH_CONCEPTS: &'a [&'a str] = &[ @@ -748,6 +795,16 @@ impl<'a> AriaRoles { "term", "textbox", "generic", + "caption", + "main", + "time", + "p", + "aside", + "blockquote", + "associationlist", + "status", + "contentinfo", + "region", ]; /// It returns the metadata of a role, if it exits. @@ -801,7 +858,8 @@ impl<'a> AriaRoles { "log" => &LogRole as &dyn AriaRoleDefinition, "main" => &MainRole as &dyn AriaRoleDefinition, "menubar" => &MenubarRole as &dyn AriaRoleDefinition, - "menu" => &MenuItem as &dyn AriaRoleDefinition, + "menu" => &MenuRole as &dyn AriaRoleDefinition, + "menuitem" => &MenuItemRole as &dyn AriaRoleDefinition, "menuitemcheckbox" => &MenuItemCheckboxRole as &dyn AriaRoleDefinition, "menuitemradio" => &MenuItemRadioRole as &dyn AriaRoleDefinition, "navigation" => &NavigationRole as &dyn AriaRoleDefinition, @@ -962,7 +1020,11 @@ impl<'a> AriaRoles { } /// Given the name of element, the function tells whether it's interactive - pub fn is_not_interactive_element(&self, element_name: &str) -> bool { + pub fn is_not_interactive_element( + &self, + element_name: &str, + attributes: Option>>, + ) -> bool { //
elements do not technically have semantics, unless the // element is a direct descendant of , and this crate cannot // reliably test that. @@ -971,6 +1033,25 @@ impl<'a> AriaRoles { if element_name == "header" { return false; } + + let elements_no_concept_info = [ + "body", "br", "details", "dir", "frame", "iframe", "label", "mark", "marquee", "menu", + "meter", "optgroup", "pre", "progress", "ruby", + ]; + if elements_no_concept_info.contains(&element_name) { + return true; + } + + // is not interactive + if element_name == "input" + && attributes + .as_ref() + .and_then(|attributes| attributes.get("type")) + .map_or(false, |values| values.iter().any(|x| x == "hidden")) + { + return true; + } + for element in Self::ROLE_WITH_CONCEPTS { let role = match *element { "checkbox" => &CheckboxRole as &dyn AriaRoleDefinitionWithConcepts, @@ -1006,11 +1087,32 @@ impl<'a> AriaRoles { "term" => &TermRole as &dyn AriaRoleDefinitionWithConcepts, "textbox" => &TextboxRole as &dyn AriaRoleDefinitionWithConcepts, "generic" => &GenericRole as &dyn AriaRoleDefinitionWithConcepts, + "caption" => &CaptionRole as &dyn AriaRoleDefinitionWithConcepts, + "main" => &MainRole as &dyn AriaRoleDefinitionWithConcepts, + "time" => &TimeRole as &dyn AriaRoleDefinitionWithConcepts, + "p" => &ParagraphRole as &dyn AriaRoleDefinitionWithConcepts, + "aside" => &ComplementaryRole as &dyn AriaRoleDefinitionWithConcepts, + "blockquote" => &BlockQuoteRole as &dyn AriaRoleDefinitionWithConcepts, + "associationlist" => &AssociationListRole as &dyn AriaRoleDefinitionWithConcepts, + "status" => &StatusRole as &dyn AriaRoleDefinitionWithConcepts, + "contentinfo" => &ContentInfoRole as &dyn AriaRoleDefinitionWithConcepts, + "region" => &RegionRole as &dyn AriaRoleDefinitionWithConcepts, _ => return false, }; - if let Some(mut concepts) = role.concepts_by_element_name(element_name) { - if concepts.any(|(name, _)| *name == element_name) && !role.is_interactive() { - return true; + if let Some(concepts) = role.concepts_by_element_name(element_name) { + for (concept_name, concept_attributes) in concepts { + if *concept_name == element_name && !role.is_interactive() { + if let Some(given_attributes) = attributes { + for (attr_name, attr_value) in *concept_attributes { + if let Some(values) = given_attributes.get(*attr_name) { + if values.contains(&attr_value.to_string()) { + return true; + } + } + } + } + return true; + } } } } @@ -1041,13 +1143,13 @@ mod test { fn should_be_interactive() { let aria_roles = AriaRoles {}; - assert!(!aria_roles.is_not_interactive_element("header")); - assert!(aria_roles.is_not_interactive_element("h1")); - assert!(aria_roles.is_not_interactive_element("h2")); - assert!(aria_roles.is_not_interactive_element("h3")); - assert!(aria_roles.is_not_interactive_element("h4")); - assert!(aria_roles.is_not_interactive_element("h5")); - assert!(aria_roles.is_not_interactive_element("h6")); + assert!(!aria_roles.is_not_interactive_element("header", None)); + assert!(aria_roles.is_not_interactive_element("h1", None)); + assert!(aria_roles.is_not_interactive_element("h2", None)); + assert!(aria_roles.is_not_interactive_element("h3", None)); + assert!(aria_roles.is_not_interactive_element("h4", None)); + assert!(aria_roles.is_not_interactive_element("h5", None)); + assert!(aria_roles.is_not_interactive_element("h6", None)); } #[test] diff --git a/crates/rome_js_analyze/src/aria_analyzers/a11y/no_noninteractive_element_to_interactive_role.rs b/crates/rome_js_analyze/src/aria_analyzers/a11y/no_noninteractive_element_to_interactive_role.rs index 4ea908751e0..b629d07b21b 100644 --- a/crates/rome_js_analyze/src/aria_analyzers/a11y/no_noninteractive_element_to_interactive_role.rs +++ b/crates/rome_js_analyze/src/aria_analyzers/a11y/no_noninteractive_element_to_interactive_role.rs @@ -64,12 +64,15 @@ impl Rule for NoNoninteractiveElementToInteractiveRole { fn run(ctx: &RuleContext) -> Self::Signals { let node = ctx.query(); let aria_roles = ctx.aria_roles(); + if node.is_element() { let role_attribute = node.find_attribute_by_name("role")?; let role_attribute_static_value = role_attribute.as_static_value()?; let role_attribute_value = role_attribute_static_value.text(); let element_name = node.name().ok()?.as_jsx_name()?.value_token().ok()?; - if aria_roles.is_not_interactive_element(element_name.text_trimmed()) + + let attributes = ctx.extract_attributes(&node.attributes()); + if aria_roles.is_not_interactive_element(element_name.text_trimmed(), attributes) && aria_roles.is_role_interactive(role_attribute_value) { //
and are considered neither interactive nor non-interactive, depending on the presence or absence of the role attribute. @@ -85,7 +88,6 @@ impl Rule for NoNoninteractiveElementToInteractiveRole { }); } } - None } diff --git a/crates/rome_js_analyze/src/aria_analyzers/nursery/no_noninteractive_tabindex.rs b/crates/rome_js_analyze/src/aria_analyzers/nursery/no_noninteractive_tabindex.rs index 9794e4815c8..73e6ee8a447 100644 --- a/crates/rome_js_analyze/src/aria_analyzers/nursery/no_noninteractive_tabindex.rs +++ b/crates/rome_js_analyze/src/aria_analyzers/nursery/no_noninteractive_tabindex.rs @@ -108,7 +108,7 @@ impl Rule for NoNoninteractiveTabindex { let element_name = node.name().ok()?.as_jsx_name()?.value_token().ok()?; let aria_roles = ctx.aria_roles(); - if aria_roles.is_not_interactive_element(element_name.text_trimmed()) { + if aria_roles.is_not_interactive_element(element_name.text_trimmed(), None) { let tabindex_attribute = node.find_attribute_by_name("tabIndex")?; let tabindex_attribute_value = tabindex_attribute.initializer()?.value().ok()?; if attribute_has_negative_tabindex(&tabindex_attribute_value)? { diff --git a/crates/rome_js_analyze/tests/specs/a11y/noNoninteractiveElementToInteractiveRole/invalid.jsx b/crates/rome_js_analyze/tests/specs/a11y/noNoninteractiveElementToInteractiveRole/invalid.jsx index 4b3acc507a3..f75bf1d9de6 100644 --- a/crates/rome_js_analyze/tests/specs/a11y/noNoninteractiveElementToInteractiveRole/invalid.jsx +++ b/crates/rome_js_analyze/tests/specs/a11y/noNoninteractiveElementToInteractiveRole/invalid.jsx @@ -1,8 +1,82 @@ -var a =

; -var a =

; -var a =

; -var a =

; -var a =

; -var a =

; -var a =

; - var a =
    ; +let a =

    ; +let a =

    ; +let a =

    ; +let a =

    ; +let a =

    ; +let a =

    ; +let a =

    ; +let a =
      ; +let a =
      ; +let a =
      ; +let a =