From 247d6cb6a07d287d03f253e176f392fd94278c36 Mon Sep 17 00:00:00 2001 From: ematipico Date: Wed, 7 Dec 2022 16:07:16 +0000 Subject: [PATCH] chore: fixes --- crates/rome_aria/src/macros.rs | 4 +- crates/rome_aria/src/roles.rs | 30 ++++-- ...interactive_element_to_interactive_role.rs | 2 +- crates/rome_js_analyze/src/lib.rs | 4 +- .../invalid.jsx | 1 + .../invalid.jsx.snap | 6 +- .../src/configuration/linter/rules.rs | 91 +++++++------------ 7 files changed, 62 insertions(+), 76 deletions(-) diff --git a/crates/rome_aria/src/macros.rs b/crates/rome_aria/src/macros.rs index c7c506748b7e..e70954606497 100644 --- 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 index 18da08116ef4..7c5b7756045f 100644 --- 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 { + //
elements do not technically have semantics, unless the + // element is a direct descendant of , 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/crates/rome_js_analyze/src/aria_analyzers/nursery/no_noninteractive_element_to_interactive_role.rs b/crates/rome_js_analyze/src/aria_analyzers/nursery/no_noninteractive_element_to_interactive_role.rs index c118bfc5c69b..bf335de76561 100644 --- a/crates/rome_js_analyze/src/aria_analyzers/nursery/no_noninteractive_element_to_interactive_role.rs +++ b/crates/rome_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 index e4d5466521fc..cf5b832ddff5 100644 --- 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/crates/rome_js_analyze/tests/specs/nursery/noNoninteractiveElementToInteractiveRole/invalid.jsx b/crates/rome_js_analyze/tests/specs/nursery/noNoninteractiveElementToInteractiveRole/invalid.jsx index 4029bf84d099..4b3acc507a3f 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/noNoninteractiveElementToInteractiveRole/invalid.jsx +++ b/crates/rome_js_analyze/tests/specs/nursery/noNoninteractiveElementToInteractiveRole/invalid.jsx @@ -5,3 +5,4 @@ var a =

; var a =

; var a =

; var a =

; + var a =
    ; diff --git a/crates/rome_js_analyze/tests/specs/nursery/noNoninteractiveElementToInteractiveRole/invalid.jsx.snap b/crates/rome_js_analyze/tests/specs/nursery/noNoninteractiveElementToInteractiveRole/invalid.jsx.snap index b26cbd29a58c..de9ee6e8b46c 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/noNoninteractiveElementToInteractiveRole/invalid.jsx.snap +++ b/crates/rome_js_analyze/tests/specs/nursery/noNoninteractiveElementToInteractiveRole/invalid.jsx.snap @@ -11,6 +11,7 @@ var a =

    ; var a =

    ; var a =

    ; var a =

    ; + var a =
      ; ``` @@ -107,7 +108,7 @@ invalid.jsx:6:13 lint/nursery/noNoninteractiveElementToInteractiveRole ━━━ > 6 │ var a =

      ; │ ^^^^^^^^^^^^^^^^^^ 7 │ var a =

      ; - 8 │ + 8 │ var a =
        ; i Replace h1 with a div or a span. @@ -123,7 +124,8 @@ invalid.jsx:7:13 lint/nursery/noNoninteractiveElementToInteractiveRole ━━━ 6 │ var a =

        ; > 7 │ var a =

        ; │ ^^^^^^^^^^^^^^^^^^ - 8 │ + 8 │ var a =
          ; + 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 index 089aaec77bd7..dc5b23f7f635 100644 --- a/crates/rome_service/src/configuration/linter/rules.rs +++ b/crates/rome_service/src/configuration/linter/rules.rs @@ -184,9 +184,7 @@ impl Rules { None } } - pub(crate) fn is_recommended(&self) -> bool { - !matches!(self.recommended, Some(false)) - } + pub(crate) fn is_recommended(&self) -> bool { !matches!(self.recommended, Some(false)) } #[doc = r" It returns a tuple of filters. The first element of the tuple are the enabled rules,"] #[doc = r" while the second element are the disabled rules."] #[doc = r""] @@ -421,9 +419,7 @@ impl A11y { RuleFilter::Rule("a11y", Self::CATEGORY_RULES[7]), RuleFilter::Rule("a11y", Self::CATEGORY_RULES[8]), ]; - pub(crate) fn is_recommended(&self) -> bool { - !matches!(self.recommended, Some(false)) - } + pub(crate) fn is_recommended(&self) -> bool { !matches!(self.recommended, Some(false)) } pub(crate) fn get_enabled_rules(&self) -> IndexSet { IndexSet::from_iter(self.rules.iter().filter_map(|(key, conf)| { if conf.is_enabled() { @@ -443,9 +439,7 @@ impl A11y { })) } #[doc = r" Checks if, given a rule name, matches one of the rules contained in this category"] - pub(crate) fn has_rule(rule_name: &str) -> bool { - Self::CATEGORY_RULES.contains(&rule_name) - } + pub(crate) fn has_rule(rule_name: &str) -> bool { Self::CATEGORY_RULES.contains(&rule_name) } #[doc = r" Checks if, given a rule name, it is marked as recommended"] pub(crate) fn is_recommended_rule(rule_name: &str) -> bool { Self::RECOMMENDED_RULES.contains(&rule_name) @@ -531,9 +525,7 @@ impl Complexity { RuleFilter::Rule("complexity", Self::CATEGORY_RULES[4]), RuleFilter::Rule("complexity", Self::CATEGORY_RULES[5]), ]; - pub(crate) fn is_recommended(&self) -> bool { - !matches!(self.recommended, Some(false)) - } + pub(crate) fn is_recommended(&self) -> bool { !matches!(self.recommended, Some(false)) } pub(crate) fn get_enabled_rules(&self) -> IndexSet { IndexSet::from_iter(self.rules.iter().filter_map(|(key, conf)| { if conf.is_enabled() { @@ -553,9 +545,7 @@ impl Complexity { })) } #[doc = r" Checks if, given a rule name, matches one of the rules contained in this category"] - pub(crate) fn has_rule(rule_name: &str) -> bool { - Self::CATEGORY_RULES.contains(&rule_name) - } + pub(crate) fn has_rule(rule_name: &str) -> bool { Self::CATEGORY_RULES.contains(&rule_name) } #[doc = r" Checks if, given a rule name, it is marked as recommended"] pub(crate) fn is_recommended_rule(rule_name: &str) -> bool { Self::RECOMMENDED_RULES.contains(&rule_name) @@ -664,9 +654,7 @@ impl Correctness { RuleFilter::Rule("correctness", Self::CATEGORY_RULES[9]), RuleFilter::Rule("correctness", Self::CATEGORY_RULES[10]), ]; - pub(crate) fn is_recommended(&self) -> bool { - !matches!(self.recommended, Some(false)) - } + pub(crate) fn is_recommended(&self) -> bool { !matches!(self.recommended, Some(false)) } pub(crate) fn get_enabled_rules(&self) -> IndexSet { IndexSet::from_iter(self.rules.iter().filter_map(|(key, conf)| { if conf.is_enabled() { @@ -686,9 +674,7 @@ impl Correctness { })) } #[doc = r" Checks if, given a rule name, matches one of the rules contained in this category"] - pub(crate) fn has_rule(rule_name: &str) -> bool { - Self::CATEGORY_RULES.contains(&rule_name) - } + pub(crate) fn has_rule(rule_name: &str) -> bool { Self::CATEGORY_RULES.contains(&rule_name) } #[doc = r" Checks if, given a rule name, it is marked as recommended"] pub(crate) fn is_recommended_rule(rule_name: &str) -> bool { Self::RECOMMENDED_RULES.contains(&rule_name) @@ -761,6 +747,8 @@ struct NurserySchema { no_invalid_constructor_super: Option, #[doc = "Disallow non-null assertions using the ! postfix operator."] no_non_null_assertion: Option, + #[doc = "Enforce that interactive ARIA roles are not assigned to non-interactive HTML elements."] + no_noninteractive_element_to_interactive_role: Option, #[doc = "Disallow literal numbers that lose precision"] no_precision_loss: Option, #[doc = "Enforce img alt prop does not contain the word \"image\", \"picture\", or \"photo\"."] @@ -806,7 +794,7 @@ struct NurserySchema { } impl Nursery { const CATEGORY_NAME: &'static str = "nursery"; - pub(crate) const CATEGORY_RULES: [&'static str; 33] = [ + pub(crate) const CATEGORY_RULES: [&'static str; 34] = [ "noAccessKey", "noAssignInExpressions", "noBannedTypes", @@ -819,6 +807,7 @@ impl Nursery { "noHeaderScope", "noInvalidConstructorSuper", "noNonNullAssertion", + "noNoninteractiveElementToInteractiveRole", "noPrecisionLoss", "noRedundantAlt", "noRedundantUseStrict", @@ -841,7 +830,7 @@ impl Nursery { "useHookAtTopLevel", "useNumericLiterals", ]; - const RECOMMENDED_RULES: [&'static str; 24] = [ + const RECOMMENDED_RULES: [&'static str; 25] = [ "noAssignInExpressions", "noBannedTypes", "noConstEnum", @@ -852,6 +841,7 @@ impl Nursery { "noExtraNonNullAssertion", "noHeaderScope", "noInvalidConstructorSuper", + "noNoninteractiveElementToInteractiveRole", "noRedundantAlt", "noSetterReturn", "noStringCaseMismatch", @@ -867,7 +857,7 @@ impl Nursery { "useExhaustiveDependencies", "useNumericLiterals", ]; - const RECOMMENDED_RULES_AS_FILTERS: [RuleFilter<'static>; 24] = [ + const RECOMMENDED_RULES_AS_FILTERS: [RuleFilter<'static>; 25] = [ RuleFilter::Rule("nursery", Self::CATEGORY_RULES[1]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[2]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[3]), @@ -878,24 +868,23 @@ impl Nursery { RuleFilter::Rule("nursery", Self::CATEGORY_RULES[8]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[9]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[10]), - RuleFilter::Rule("nursery", Self::CATEGORY_RULES[13]), - RuleFilter::Rule("nursery", Self::CATEGORY_RULES[16]), + RuleFilter::Rule("nursery", Self::CATEGORY_RULES[12]), + RuleFilter::Rule("nursery", Self::CATEGORY_RULES[14]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[17]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[18]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[19]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[20]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[21]), - RuleFilter::Rule("nursery", Self::CATEGORY_RULES[23]), - RuleFilter::Rule("nursery", Self::CATEGORY_RULES[25]), + RuleFilter::Rule("nursery", Self::CATEGORY_RULES[22]), + RuleFilter::Rule("nursery", Self::CATEGORY_RULES[24]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[26]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[27]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[28]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[29]), - RuleFilter::Rule("nursery", Self::CATEGORY_RULES[32]), + RuleFilter::Rule("nursery", Self::CATEGORY_RULES[30]), + RuleFilter::Rule("nursery", Self::CATEGORY_RULES[33]), ]; - pub(crate) fn is_recommended(&self) -> bool { - !matches!(self.recommended, Some(false)) - } + pub(crate) fn is_recommended(&self) -> bool { !matches!(self.recommended, Some(false)) } pub(crate) fn get_enabled_rules(&self) -> IndexSet { IndexSet::from_iter(self.rules.iter().filter_map(|(key, conf)| { if conf.is_enabled() { @@ -915,14 +904,12 @@ impl Nursery { })) } #[doc = r" Checks if, given a rule name, matches one of the rules contained in this category"] - pub(crate) fn has_rule(rule_name: &str) -> bool { - Self::CATEGORY_RULES.contains(&rule_name) - } + pub(crate) fn has_rule(rule_name: &str) -> bool { Self::CATEGORY_RULES.contains(&rule_name) } #[doc = r" Checks if, given a rule name, it is marked as recommended"] pub(crate) fn is_recommended_rule(rule_name: &str) -> bool { Self::RECOMMENDED_RULES.contains(&rule_name) } - pub(crate) fn recommended_rules_as_filters() -> [RuleFilter<'static>; 24] { + pub(crate) fn recommended_rules_as_filters() -> [RuleFilter<'static>; 25] { Self::RECOMMENDED_RULES_AS_FILTERS } } @@ -975,9 +962,7 @@ impl Performance { const RECOMMENDED_RULES: [&'static str; 1] = ["noDelete"]; const RECOMMENDED_RULES_AS_FILTERS: [RuleFilter<'static>; 1] = [RuleFilter::Rule("performance", Self::CATEGORY_RULES[0])]; - pub(crate) fn is_recommended(&self) -> bool { - !matches!(self.recommended, Some(false)) - } + pub(crate) fn is_recommended(&self) -> bool { !matches!(self.recommended, Some(false)) } pub(crate) fn get_enabled_rules(&self) -> IndexSet { IndexSet::from_iter(self.rules.iter().filter_map(|(key, conf)| { if conf.is_enabled() { @@ -997,9 +982,7 @@ impl Performance { })) } #[doc = r" Checks if, given a rule name, matches one of the rules contained in this category"] - pub(crate) fn has_rule(rule_name: &str) -> bool { - Self::CATEGORY_RULES.contains(&rule_name) - } + pub(crate) fn has_rule(rule_name: &str) -> bool { Self::CATEGORY_RULES.contains(&rule_name) } #[doc = r" Checks if, given a rule name, it is marked as recommended"] pub(crate) fn is_recommended_rule(rule_name: &str) -> bool { Self::RECOMMENDED_RULES.contains(&rule_name) @@ -1067,9 +1050,7 @@ impl Security { RuleFilter::Rule("security", Self::CATEGORY_RULES[0]), RuleFilter::Rule("security", Self::CATEGORY_RULES[1]), ]; - pub(crate) fn is_recommended(&self) -> bool { - !matches!(self.recommended, Some(false)) - } + pub(crate) fn is_recommended(&self) -> bool { !matches!(self.recommended, Some(false)) } pub(crate) fn get_enabled_rules(&self) -> IndexSet { IndexSet::from_iter(self.rules.iter().filter_map(|(key, conf)| { if conf.is_enabled() { @@ -1089,9 +1070,7 @@ impl Security { })) } #[doc = r" Checks if, given a rule name, matches one of the rules contained in this category"] - pub(crate) fn has_rule(rule_name: &str) -> bool { - Self::CATEGORY_RULES.contains(&rule_name) - } + pub(crate) fn has_rule(rule_name: &str) -> bool { Self::CATEGORY_RULES.contains(&rule_name) } #[doc = r" Checks if, given a rule name, it is marked as recommended"] pub(crate) fn is_recommended_rule(rule_name: &str) -> bool { Self::RECOMMENDED_RULES.contains(&rule_name) @@ -1200,9 +1179,7 @@ impl Style { RuleFilter::Rule("style", Self::CATEGORY_RULES[11]), RuleFilter::Rule("style", Self::CATEGORY_RULES[12]), ]; - pub(crate) fn is_recommended(&self) -> bool { - !matches!(self.recommended, Some(false)) - } + pub(crate) fn is_recommended(&self) -> bool { !matches!(self.recommended, Some(false)) } pub(crate) fn get_enabled_rules(&self) -> IndexSet { IndexSet::from_iter(self.rules.iter().filter_map(|(key, conf)| { if conf.is_enabled() { @@ -1222,9 +1199,7 @@ impl Style { })) } #[doc = r" Checks if, given a rule name, matches one of the rules contained in this category"] - pub(crate) fn has_rule(rule_name: &str) -> bool { - Self::CATEGORY_RULES.contains(&rule_name) - } + pub(crate) fn has_rule(rule_name: &str) -> bool { Self::CATEGORY_RULES.contains(&rule_name) } #[doc = r" Checks if, given a rule name, it is marked as recommended"] pub(crate) fn is_recommended_rule(rule_name: &str) -> bool { Self::RECOMMENDED_RULES.contains(&rule_name) @@ -1362,9 +1337,7 @@ impl Suspicious { RuleFilter::Rule("suspicious", Self::CATEGORY_RULES[14]), RuleFilter::Rule("suspicious", Self::CATEGORY_RULES[15]), ]; - pub(crate) fn is_recommended(&self) -> bool { - !matches!(self.recommended, Some(false)) - } + pub(crate) fn is_recommended(&self) -> bool { !matches!(self.recommended, Some(false)) } pub(crate) fn get_enabled_rules(&self) -> IndexSet { IndexSet::from_iter(self.rules.iter().filter_map(|(key, conf)| { if conf.is_enabled() { @@ -1384,9 +1357,7 @@ impl Suspicious { })) } #[doc = r" Checks if, given a rule name, matches one of the rules contained in this category"] - pub(crate) fn has_rule(rule_name: &str) -> bool { - Self::CATEGORY_RULES.contains(&rule_name) - } + pub(crate) fn has_rule(rule_name: &str) -> bool { Self::CATEGORY_RULES.contains(&rule_name) } #[doc = r" Checks if, given a rule name, it is marked as recommended"] pub(crate) fn is_recommended_rule(rule_name: &str) -> bool { Self::RECOMMENDED_RULES.contains(&rule_name)