chore: fixes

rome · Dec 12, 2022 · 247d6cb · 247d6cb
1 parent d107878
commit 247d6cb
Show file tree

Hide file tree

Showing 7 changed files with 62 additions and 76 deletions.
diff --git a/crates/rome_aria/src/macros.rs b/crates/rome_aria/src/macros.rs
@@ -40,11 +40,11 @@ macro_rules! define_role {
         }
 
         impl $crate::AriaRoleDefinition for $id {
-            fn properties<'a>(&self) -> std::slice::Iter<'a, (&str, bool)> {
+            fn properties(&self) -> std::slice::Iter<(&str, bool)> {
                 $id::PROPS.iter()
             }
 
-            fn roles<'a>(&self) -> std::slice::Iter<'a, &str> {
+            fn roles(&self) -> std::slice::Iter<&str> {
                 $id::ROLES.iter()
             }
         }

diff --git a/crates/rome_aria/src/roles.rs b/crates/rome_aria/src/roles.rs
@@ -652,7 +652,15 @@ impl<'a> AriaRoles {
     }
 
     /// Given the name of element, the function tell whether it's interactive
-    pub fn is_element_interactive(&self, element_name: &str) -> bool {
+    pub fn is_not_interactive_element(&self, element_name: &str) -> bool {
+        // <header> elements do not technically have semantics, unless the
+        // element is a direct descendant of <body>, and this plugin cannot
+        // reliably test that.
+        //
+        // Check: https://www.w3.org/TR/wai-aria-practices/examples/landmarks/banner.html
+        if element_name == "header" {
+            return false;
+        }
         for element in Self::ROLE_WITH_CONCEPTS {
             let role = match *element {
                 "checkbox" => &CheckboxRole as &dyn AriaRoleDefinitionWithConcepts,
@@ -691,12 +699,14 @@ impl<'a> AriaRoles {
             };
             if let Some(mut concepts) = role.concepts_by_element_name(element_name) {
                 if concepts.any(|(name, _)| *name == element_name) {
-                    return role.is_interactive();
+                    if !role.is_interactive() {
+                        return true;
+                    }
                 }
             }
         }
 
-        true
+        false
     }
 }
 
@@ -720,12 +730,12 @@ mod test {
     fn should_be_interactive() {
         let aria_roles = AriaRoles {};
 
-        assert!(aria_roles.is_element_interactive("input"));
-        assert!(aria_roles.is_element_interactive("option"));
-        assert!(aria_roles.is_element_interactive("select"));
-        assert!(aria_roles.is_element_interactive("button"));
-        assert!(aria_roles.is_element_interactive("td"));
-        assert!(aria_roles.is_element_interactive("tr"));
-        assert!(aria_roles.is_element_interactive("hr"));
+        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"));
     }
 }
diff --git a/...me_js_analyze/src/aria_analyzers/nursery/no_noninteractive_element_to_interactive_role.rs b/...me_js_analyze/src/aria_analyzers/nursery/no_noninteractive_element_to_interactive_role.rs
@@ -91,7 +91,7 @@ impl Rule for NoNoninteractiveElementToInteractiveRole {
                 _ => None,
             }?;
             let element_name = node.name().ok()?.as_jsx_name()?.value_token().ok()?;
-            if !aria_roles.is_element_interactive(element_name.text_trimmed())
+            if aria_roles.is_not_interactive_element(element_name.text_trimmed())
                 && aria_roles.is_role_interactive(role_attribute_value.text())
             {
                 return Some(RuleState {

diff --git a/crates/rome_js_analyze/src/lib.rs b/crates/rome_js_analyze/src/lib.rs
@@ -10,7 +10,7 @@ use rome_analyze::{
     Phases, RuleAction, RuleRegistry, ServiceBag, SuppressionKind, SyntaxVisitor,
 };
 use rome_aria::{AriaProperties, AriaRoles};
-use rome_diagnostics::{category, FileId};
+use rome_diagnostics::{category, Diagnostic, FileId};
 use rome_js_syntax::suppression::SuppressionDiagnostic;
 use rome_js_syntax::{suppression::parse_suppression_comment, JsLanguage};
 use serde::{Deserialize, Serialize};
@@ -427,6 +427,8 @@ pub enum RuleError {
     },
 }
 
+impl Diagnostic for RuleError {}
+
 impl std::fmt::Display for RuleError {
     fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result {
         match self {

diff --git a/.../rome_js_analyze/tests/specs/nursery/noNoninteractiveElementToInteractiveRole/invalid.jsx b/.../rome_js_analyze/tests/specs/nursery/noNoninteractiveElementToInteractiveRole/invalid.jsx
@@ -5,3 +5,4 @@ var a = <h1 role="combobox"></h1>;
 var a = <h1 role="scrollbar"></h1>;
 var a = <h1 role={"scrollbar"}></h1>;
 var a = <h1 role={`scrollbar`}></h1>;
+	var a = <ol role="menuitem" />;
diff --git a/..._js_analyze/tests/specs/nursery/noNoninteractiveElementToInteractiveRole/invalid.jsx.snap b/..._js_analyze/tests/specs/nursery/noNoninteractiveElementToInteractiveRole/invalid.jsx.snap
@@ -11,6 +11,7 @@ var a = <h1 role="combobox"></h1>;
 var a = <h1 role="scrollbar"></h1>;
 var a = <h1 role={"scrollbar"}></h1>;
 var a = <h1 role={`scrollbar`}></h1>;
+	var a = <ol role="menuitem" />;
 
 ```
 
@@ -107,7 +108,7 @@ invalid.jsx:6:13 lint/nursery/noNoninteractiveElementToInteractiveRole ━━━
   > 6 │ var a = <h1 role={"scrollbar"}></h1>;
       │             ^^^^^^^^^^^^^^^^^^
     7 │ var a = <h1 role={`scrollbar`}></h1>;
-    8 │ 
+    8 │ 	var a = <ol role="menuitem" />;
   
   i Replace h1 with a div or a span.
   
@@ -123,7 +124,8 @@ invalid.jsx:7:13 lint/nursery/noNoninteractiveElementToInteractiveRole ━━━
     6 │ var a = <h1 role={"scrollbar"}></h1>;
   > 7 │ var a = <h1 role={`scrollbar`}></h1>;
       │             ^^^^^^^^^^^^^^^^^^
-    8 │ 
+    8 │ 	var a = <ol role="menuitem" />;
+    9 │ 
   
   i Replace h1 with a div or a span.
   

diff --git a/crates/rome_service/src/configuration/linter/rules.rs b/crates/rome_service/src/configuration/linter/rules.rs