feat(linter/jsx-a11y): add fixer for anchor-has-content (#4852)

Add a conditional fix that removes `aria-hidden` from an anchor's child if there is only a single child. This PR also fixes a false positive on hidden anchors. It should report visible anchors with hidden content, not hidden anchors.
oxc-project · Aug 13, 2024 · a6195a6 · a6195a6
1 parent a81ce3a
commit a6195a6
Show file tree

Hide file tree

Showing 3 changed files with 97 additions and 11 deletions.
diff --git a/crates/oxc_linter/src/fixer/fix.rs b/crates/oxc_linter/src/fixer/fix.rs
@@ -145,6 +145,16 @@ macro_rules! impl_from {
 // but this breaks when implementing `From<RuleFix<'a>> for CompositeFix<'a>`.
 impl_from!(CompositeFix<'a>, Fix<'a>, Option<Fix<'a>>, Vec<Fix<'a>>);
 
+impl<'a> FromIterator<Fix<'a>> for RuleFix<'a> {
+    fn from_iter<T: IntoIterator<Item = Fix<'a>>>(iter: T) -> Self {
+        Self {
+            kind: FixKind::SafeFix,
+            message: None,
+            fix: iter.into_iter().collect::<Vec<_>>().into(),
+        }
+    }
+}
+
 impl<'a> From<RuleFix<'a>> for CompositeFix<'a> {
     #[inline]
     fn from(val: RuleFix<'a>) -> Self {

diff --git a/crates/oxc_linter/src/rules/jsx_a11y/anchor_has_content.rs b/crates/oxc_linter/src/rules/jsx_a11y/anchor_has_content.rs
@@ -1,10 +1,14 @@
-use oxc_ast::AstKind;
+use oxc_ast::{
+    ast::{JSXAttributeItem, JSXChild, JSXElement},
+    AstKind,
+};
 use oxc_diagnostics::OxcDiagnostic;
 use oxc_macros::declare_oxc_lint;
 use oxc_span::Span;
 
 use crate::{
     context::LintContext,
+    fixer::{Fix, RuleFix},
     rule::Rule,
     utils::{
         get_element_type, has_jsx_prop_ignore_case, is_hidden_from_screen_reader,
@@ -19,12 +23,6 @@ fn missing_content(span0: Span) -> OxcDiagnostic {
         .with_label(span0)
 }
 
-fn remove_aria_hidden(span0: Span) -> OxcDiagnostic {
-    OxcDiagnostic::warn("Missing accessible content when using `a` elements.")
-        .with_help("Remove the `aria-hidden` attribute to allow the anchor element and its content visible to assistive technologies.")
-        .with_label(span0)
-}
-
 #[derive(Debug, Default, Clone)]
 pub struct AnchorHasContent;
 
@@ -59,7 +57,8 @@ declare_oxc_lint!(
     /// ```
     ///
     AnchorHasContent,
-    correctness
+    correctness,
+    conditional_suggestion
 );
 
 impl Rule for AnchorHasContent {
@@ -70,7 +69,6 @@ impl Rule for AnchorHasContent {
             };
             if name == "a" {
                 if is_hidden_from_screen_reader(ctx, &jsx_el.opening_element) {
-                    ctx.diagnostic(remove_aria_hidden(jsx_el.span));
                     return;
                 }
 
@@ -84,12 +82,43 @@ impl Rule for AnchorHasContent {
                     };
                 }
 
-                ctx.diagnostic(missing_content(jsx_el.span));
+                let diagnostic = missing_content(jsx_el.span);
+                if jsx_el.children.len() == 1 {
+                    let child = &jsx_el.children[0];
+                    if let JSXChild::Element(child) = child {
+                        ctx.diagnostic_with_suggestion(diagnostic, |_fixer| {
+                            remove_hidden_attributes(child)
+                        });
+                        return;
+                    }
+                }
+
+                ctx.diagnostic(diagnostic);
             }
         }
     }
 }
 
+fn remove_hidden_attributes<'a>(element: &JSXElement<'a>) -> RuleFix<'a> {
+    element
+        .opening_element
+        .attributes
+        .iter()
+        .filter_map(JSXAttributeItem::as_attribute)
+        .filter_map(|attr| {
+            attr.name.as_identifier().and_then(|name| {
+                if name.name.eq_ignore_ascii_case("aria-hidden")
+                    || name.name.eq_ignore_ascii_case("hidden")
+                {
+                    Some(Fix::delete(attr.span))
+                } else {
+                    None
+                }
+            })
+        })
+        .collect()
+}
+
 #[test]
 fn test() {
     use crate::tester::Tester;
@@ -114,12 +143,28 @@ fn test() {
         (r"<a title={title} />", None, None),
         (r"<a aria-label={ariaLabel} />", None, None),
         (r"<a title={title} aria-label={ariaLabel} />", None, None),
+        (r#"<a><Bar aria-hidden="false" /></a>"#, None, None),
+        // anchors can be hidden
+        (r"<a aria-hidden>Foo</a>", None, None),
+        (r#"<a aria-hidden="true">Foo</a>"#, None, None),
+        (r"<a hidden>Foo</a>", None, None),
+        (r"<a aria-hidden><span aria-hidden>Foo</span></a>", None, None),
+        (r#"<a hidden="true">Foo</a>"#, None, None),
+        (r#"<a hidden="">Foo</a>"#, None, None),
+        // TODO: should these be failing?
+        (r"<a><div hidden /></a>", None, None),
+        (r"<a><Bar hidden /></a>", None, None),
+        (r#"<a><Bar hidden="" /></a>"#, None, None),
+        (r#"<a><Bar hidden="until-hidden" /></a>"#, None, None),
     ];
 
     let fail = vec![
         (r"<a />", None, None),
         (r"<a><Bar aria-hidden /></a>", None, None),
+        (r#"<a><Bar aria-hidden="true" /></a>"#, None, None),
+        (r#"<a><input type="hidden" /></a>"#, None, None),
         (r"<a>{undefined}</a>", None, None),
+        (r"<a>{null}</a>", None, None),
         (
             r"<Link />",
             None,
@@ -129,5 +174,15 @@ fn test() {
         ),
     ];
 
-    Tester::new(AnchorHasContent::NAME, pass, fail).test_and_snapshot();
+    let fix = vec![
+        (r"<a><Bar aria-hidden /></a>", "<a><Bar  /></a>"),
+        (r"<a><Bar aria-hidden>Can't see me</Bar></a>", r"<a><Bar >Can't see me</Bar></a>"),
+        (r"<a><Bar aria-hidden={true}>Can't see me</Bar></a>", r"<a><Bar >Can't see me</Bar></a>"),
+        (
+            r#"<a><Bar aria-hidden="true">Can't see me</Bar></a>"#,
+            r"<a><Bar >Can't see me</Bar></a>",
+        ),
+    ];
+
+    Tester::new(AnchorHasContent::NAME, pass, fail).expect_fix(fix).test_and_snapshot();
 }
diff --git a/crates/oxc_linter/src/snapshots/anchor_has_content.snap b/crates/oxc_linter/src/snapshots/anchor_has_content.snap
@@ -15,13 +15,34 @@ source: crates/oxc_linter/src/tester.rs
    ╰────
   help: Provide screen reader accessible content when using `a` elements.
 
+  ⚠ eslint-plugin-jsx-a11y(anchor-has-content): Missing accessible content when using `a` elements.
+   ╭─[anchor_has_content.tsx:1:1]
+ 1 │ <a><Bar aria-hidden="true" /></a>
+   · ─────────────────────────────────
+   ╰────
+  help: Provide screen reader accessible content when using `a` elements.
+
+  ⚠ eslint-plugin-jsx-a11y(anchor-has-content): Missing accessible content when using `a` elements.
+   ╭─[anchor_has_content.tsx:1:1]
+ 1 │ <a><input type="hidden" /></a>
+   · ──────────────────────────────
+   ╰────
+  help: Provide screen reader accessible content when using `a` elements.
+
   ⚠ eslint-plugin-jsx-a11y(anchor-has-content): Missing accessible content when using `a` elements.
    ╭─[anchor_has_content.tsx:1:1]
  1 │ <a>{undefined}</a>
    · ──────────────────
    ╰────
   help: Provide screen reader accessible content when using `a` elements.
 
+  ⚠ eslint-plugin-jsx-a11y(anchor-has-content): Missing accessible content when using `a` elements.
+   ╭─[anchor_has_content.tsx:1:1]
+ 1 │ <a>{null}</a>
+   · ─────────────
+   ╰────
+  help: Provide screen reader accessible content when using `a` elements.
+
   ⚠ eslint-plugin-jsx-a11y(anchor-has-content): Missing accessible content when using `a` elements.
    ╭─[anchor_has_content.tsx:1:1]
  1 │ <Link />