refactor(linter): clean up jsx-a11y/anchor-is-valid (#4831)

crates/oxc_ast/src/ast_impl/jsx.rs

@@ -24,6 +24,15 @@ impl<'a> fmt::Display for JSXNamespacedName<'a> {
     }
 }
 
+impl<'a> JSXElementName<'a> {
+    pub fn as_identifier(&self) -> Option<&JSXIdentifier<'a>> {
+        match self {
+            Self::Identifier(id) => Some(id.as_ref()),
+            _ => None,
+        }
+    }
+}
+
 impl<'a> JSXMemberExpression<'a> {
     pub fn get_object_identifier(&self) -> &JSXIdentifier {
         let mut member_expr = self;
@@ -91,11 +100,62 @@ impl<'a> JSXAttribute<'a> {
         matches!(&self.name, JSXAttributeName::Identifier(ident) if ident.name == name)
     }
 
+    pub fn is_identifier_ignore_case(&self, name: &str) -> bool {
+        matches!(&self.name, JSXAttributeName::Identifier(ident) if ident.name.eq_ignore_ascii_case(name))
+    }
+
     pub fn is_key(&self) -> bool {
         self.is_identifier("key")
     }
 }
 
+impl<'a> JSXAttributeName<'a> {
+    pub fn as_identifier(&self) -> Option<&JSXIdentifier<'a>> {
+        match self {
+            Self::Identifier(ident) => Some(ident.as_ref()),
+            Self::NamespacedName(_) => None,
+        }
+    }
+    pub fn get_identifier(&self) -> &JSXIdentifier<'a> {
+        match self {
+            Self::Identifier(ident) => ident.as_ref(),
+            Self::NamespacedName(namespaced) => &namespaced.property,
+        }
+    }
+}
+impl<'a> JSXAttributeValue<'a> {
+    pub fn as_string_literal(&self) -> Option<&StringLiteral<'a>> {
+        match self {
+            Self::StringLiteral(lit) => Some(lit.as_ref()),
+            _ => None,
+        }
+    }
+}
+
+impl<'a> JSXAttributeItem<'a> {
+    /// Get the contained [`JSXAttribute`] if it is an attribute item, otherwise
+    /// returns [`None`].
+    ///
+    /// This is the inverse of [`JSXAttributeItem::as_spread`].
+    pub fn as_attribute(&self) -> Option<&JSXAttribute<'a>> {
+        match self {
+            Self::Attribute(attr) => Some(attr),
+            Self::SpreadAttribute(_) => None,
+        }
+    }
+
+    /// Get the contained [`JSXSpreadAttribute`] if it is a spread attribute item,
+    /// otherwise returns [`None`].
+    ///
+    /// This is the inverse of [`JSXAttributeItem::as_attribute`].
+    pub fn as_spread(&self) -> Option<&JSXSpreadAttribute<'a>> {
+        match self {
+            Self::Attribute(_) => None,
+            Self::SpreadAttribute(spread) => Some(spread),
+        }
+    }
+}
+
 impl<'a> JSXChild<'a> {
     pub const fn is_expression_container(&self) -> bool {
         matches!(self, Self::ExpressionContainer(_))

crates/oxc_linter/src/rules/jsx_a11y/anchor_is_valid.rs

@@ -1,10 +1,13 @@
+use std::ops::Deref;
+
 use oxc_ast::{
     ast::{JSXAttributeItem, JSXAttributeValue, JSXElementName, JSXExpression},
     AstKind,
 };
 use oxc_diagnostics::OxcDiagnostic;
 use oxc_macros::declare_oxc_lint;
-use oxc_span::Span;
+use oxc_span::{CompactStr, Span};
+use serde_json::Value;
 
 use crate::{
     context::LintContext,
@@ -35,8 +38,17 @@ fn cant_be_anchor(span0: Span) -> OxcDiagnostic {
 pub struct AnchorIsValid(Box<AnchorIsValidConfig>);
 
 #[derive(Debug, Default, Clone)]
-struct AnchorIsValidConfig {
+pub struct AnchorIsValidConfig {
-    valid_hrefs: Vec<String>,
+    /// Unique and sorted list of valid hrefs
+    valid_hrefs: Vec<CompactStr>,
+}
+
+impl Deref for AnchorIsValid {
+    type Target = AnchorIsValidConfig;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
 }
 
 declare_oxc_lint!(
@@ -109,11 +121,10 @@ declare_oxc_lint!(
 
 impl Rule for AnchorIsValid {
     fn from_configuration(value: serde_json::Value) -> Self {
-        let valid_hrefs =
+        let Some(valid_hrefs) = value.get("validHrefs").and_then(Value::as_array) else {
-            value.get("validHrefs").and_then(|v| v.as_array()).map_or_else(Vec::new, |array| {
+            return Self::default();
-                array.iter().filter_map(|v| v.as_str().map(String::from)).collect::<Vec<String>>()
+        };
-            });
+        Self(Box::new(valid_hrefs.iter().collect()))
-        Self(Box::new(AnchorIsValidConfig { valid_hrefs }))
     }
 
     fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
@@ -124,77 +135,83 @@ impl Rule for AnchorIsValid {
             let Some(name) = &get_element_type(ctx, &jsx_el.opening_element) else {
                 return;
             };
-            if name == "a" {
+            if name != "a" {
-                if let Option::Some(herf_attr) =
+                return;
+            };
+            if let Option::Some(href_attr) =
                 has_jsx_prop_ignore_case(&jsx_el.opening_element, "href")
             {
+                let JSXAttributeItem::Attribute(attr) = href_attr else {
+                    return;
+                };
+
                 // Check if the 'a' element has a correct href attribute
-                    match herf_attr {
+                let Some(value) = attr.value.as_ref() else {
-                        JSXAttributeItem::Attribute(attr) => match &attr.value {
+                    ctx.diagnostic(incorrect_href(ident.span));
-                            Some(value) => {
+                    return;
-                                let is_empty = check_value_is_empty(value, &self.0.valid_hrefs);
+                };
+
+                let is_empty = self.check_value_is_empty(value);
                 if is_empty {
-                                    if has_jsx_prop_ignore_case(&jsx_el.opening_element, "onclick")
+                    if has_jsx_prop_ignore_case(&jsx_el.opening_element, "onclick").is_some() {
-                                        .is_some()
-                                    {
                         ctx.diagnostic(cant_be_anchor(ident.span));
                         return;
                     }
                     ctx.diagnostic(incorrect_href(ident.span));
                     return;
                 }
-                            }
-                            None => {
-                                ctx.diagnostic(incorrect_href(ident.span));
-                                return;
-                            }
-                        },
-                        JSXAttributeItem::SpreadAttribute(_) => {
-                            // pass
-                            return;
-                        }
-                    }
                 return;
             }
             // Exclude '<a {...props} />' case
-                let has_spreed_attr =
+            let has_spread_attr = jsx_el.opening_element.attributes.iter().any(|attr| match attr {
-                    jsx_el.opening_element.attributes.iter().any(|attr| match attr {
                 JSXAttributeItem::SpreadAttribute(_) => true,
                 JSXAttributeItem::Attribute(_) => false,
             });
-                if has_spreed_attr {
+            if has_spread_attr {
                 return;
             }
             ctx.diagnostic(missing_href_attribute(ident.span));
         }
     }
-    }
 }
 
-fn check_value_is_empty(value: &JSXAttributeValue, valid_hrefs: &[String]) -> bool {
+impl AnchorIsValid {
+    fn check_value_is_empty(&self, value: &JSXAttributeValue) -> bool {
         match value {
             JSXAttributeValue::Element(_) => false,
-        JSXAttributeValue::StringLiteral(str_lit) => {
+            JSXAttributeValue::StringLiteral(str_lit) => self.is_invalid_href(&str_lit.value),
-            let href_value = str_lit.value.to_string(); // Assuming Atom implements ToString
-            href_value.is_empty()
-                || href_value == "#"
-                || href_value == "javascript:void(0)"
-                || !valid_hrefs.contains(&href_value)
-        }
             JSXAttributeValue::ExpressionContainer(exp) => match &exp.expression {
                 JSXExpression::Identifier(ident) => ident.name == "undefined",
                 JSXExpression::NullLiteral(_) => true,
-            JSXExpression::StringLiteral(str_lit) => {
+                JSXExpression::StringLiteral(str_lit) => self.is_invalid_href(&str_lit.value),
-                let href_value = str_lit.value.to_string();
-                href_value.is_empty()
-                    || href_value == "#"
-                    || href_value == "javascript:void(0)"
-                    || !valid_hrefs.contains(&href_value)
-            }
                 _ => false,
             },
             JSXAttributeValue::Fragment(_) => true,
         }
+    }
+}
+
+impl AnchorIsValidConfig {
+    fn new(mut valid_hrefs: Vec<CompactStr>) -> Self {
+        valid_hrefs.sort_unstable();
+        valid_hrefs.dedup();
+        Self { valid_hrefs }
+    }
+
+    fn is_invalid_href(&self, href: &str) -> bool {
+        href.is_empty() || href == "#" || href == "javascript:void(0)" || !self.contains(href)
+    }
+
+    fn contains(&self, href: &str) -> bool {
+        self.valid_hrefs.binary_search_by(|valid_href| valid_href.as_str().cmp(href)).is_ok()
+    }
+}
+
+impl<'v> FromIterator<&'v Value> for AnchorIsValidConfig {
+    fn from_iter<T: IntoIterator<Item = &'v Value>>(iter: T) -> Self {
+        let hrefs = iter.into_iter().filter_map(Value::as_str).map(CompactStr::from).collect();
+        Self::new(hrefs)
+    }
 }
 
 #[test]

crates/oxc_linter/src/utils/react.rs

@@ -3,7 +3,7 @@ use std::borrow::Cow;
 use oxc_ast::{
     ast::{
         CallExpression, Expression, JSXAttributeItem, JSXAttributeName, JSXAttributeValue,
-        JSXChild, JSXElement, JSXElementName, JSXExpression, JSXOpeningElement, MemberExpression,
+        JSXChild, JSXElement, JSXExpression, JSXOpeningElement, MemberExpression,
     },
     match_member_expression, AstKind,
 };
@@ -28,40 +28,22 @@ pub fn has_jsx_prop<'a, 'b>(
     node: &'b JSXOpeningElement<'a>,
     target_prop: &'b str,
 ) -> Option<&'b JSXAttributeItem<'a>> {
-    node.attributes.iter().find(|attr| match attr {
+    node.attributes
-        JSXAttributeItem::SpreadAttribute(_) => false,
+        .iter()
-        JSXAttributeItem::Attribute(attr) => {
+        .find(|attr| attr.as_attribute().is_some_and(|attr| attr.is_identifier(target_prop)))
-            let JSXAttributeName::Identifier(name) = &attr.name else {
-                return false;
-            };
-
-            name.name == target_prop
-        }
-    })
 }
 
 pub fn has_jsx_prop_ignore_case<'a, 'b>(
     node: &'b JSXOpeningElement<'a>,
     target_prop: &'b str,
 ) -> Option<&'b JSXAttributeItem<'a>> {
-    node.attributes.iter().find(|attr| match attr {
+    node.attributes.iter().find(|attr| {
-        JSXAttributeItem::SpreadAttribute(_) => false,
+        attr.as_attribute().is_some_and(|attr| attr.is_identifier_ignore_case(target_prop))
-        JSXAttributeItem::Attribute(attr) => {
-            let JSXAttributeName::Identifier(name) = &attr.name else {
-                return false;
-            };
-
-            name.name.eq_ignore_ascii_case(target_prop)
-        }
     })
 }
 
 pub fn get_prop_value<'a, 'b>(item: &'b JSXAttributeItem<'a>) -> Option<&'b JSXAttributeValue<'a>> {
-    if let JSXAttributeItem::Attribute(attr) = item {
+    item.as_attribute().and_then(|item| item.value.as_ref())
-        attr.value.as_ref()
-    } else {
-        None
-    }
 }
 
 pub fn get_jsx_attribute_name<'a>(attr: &JSXAttributeName<'a>) -> Cow<'a, str> {
@@ -74,13 +56,7 @@ pub fn get_jsx_attribute_name<'a>(attr: &JSXAttributeName<'a>) -> Cow<'a, str> {
 }
 
 pub fn get_string_literal_prop_value<'a>(item: &'a JSXAttributeItem<'_>) -> Option<&'a str> {
-    get_prop_value(item).and_then(|v| {
+    get_prop_value(item).and_then(JSXAttributeValue::as_string_literal).map(|s| s.value.as_str())
-        if let JSXAttributeValue::StringLiteral(s) = v {
-            Some(s.value.as_str())
-        } else {
-            None
-        }
-    })
 }
 
 // ref: https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/main/src/util/isHiddenFromScreenReader.js
@@ -132,11 +108,8 @@ pub fn is_presentation_role(jsx_opening_el: &JSXOpeningElement) -> bool {
     let Some(role) = has_jsx_prop(jsx_opening_el, "role") else {
         return false;
     };
-    let Some("presentation" | "none") = get_string_literal_prop_value(role) else {
-        return false;
-    };
 
-    true
+    matches!(get_string_literal_prop_value(role), Some("presentation" | "none"))
 }
 
 // TODO: Should re-implement
@@ -246,9 +219,7 @@ pub fn get_element_type<'c, 'a>(
     context: &'c LintContext<'a>,
     element: &JSXOpeningElement<'a>,
 ) -> Option<Cow<'c, str>> {
-    let JSXElementName::Identifier(ident) = &element.name else {
+    let name = element.name.as_identifier()?;
-        return None;
-    };
 
     let OxlintSettings { jsx_a11y, .. } = context.settings();
 
@@ -259,17 +230,14 @@ pub fn get_element_type<'c, 'a>(
             has_jsx_prop_ignore_case(element, polymorphic_prop_name_value)
         })
         .and_then(get_prop_value)
-        .and_then(|prop_value| match prop_value {
+        .and_then(JSXAttributeValue::as_string_literal)
-            JSXAttributeValue::StringLiteral(str) => Some(str.value.as_str()),
+        .map(|s| s.value.as_str());
-            _ => None,
-        });
 
-    let raw_type = polymorphic_prop.unwrap_or_else(|| ident.name.as_str());
+    let raw_type = polymorphic_prop.unwrap_or_else(|| name.name.as_str());
     match jsx_a11y.components.get(raw_type) {
         Some(component) => Some(Cow::Borrowed(component)),
         None => Some(Cow::Borrowed(raw_type)),
     }
-    // Some(String::from(jsx_a11y.components.get(raw_type).map_or(raw_type, |c| c)))
 }
 
 pub fn parse_jsx_value(value: &JSXAttributeValue) -> Result<f64, ()> {

crates/oxc_span/src/atom.rs

@@ -198,7 +198,7 @@ impl<'a> fmt::Display for Atom<'a> {
 ///
 /// Currently implemented as just a wrapper around [`compact_str::CompactString`],
 /// but will be reduced in size with a custom implementation later.
-#[derive(Clone, Eq)]
+#[derive(Clone, Eq, PartialOrd, Ord)]
 #[cfg_attr(feature = "serialize", derive(serde::Deserialize))]
 pub struct CompactStr(CompactString);