fix(lint/noSvgWithoutTitle): fix false-positives for aria-label and…

… report cases where the `role` is implicit (#788)
biomejs · Nov 19, 2023 · bb16142 · bb16142
1 parent 6ce1352
commit bb16142
Show file tree

Hide file tree

Showing 8 changed files with 62 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -54,6 +54,8 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom
 
 - Fix [#727](https://github.com/biomejs/biome/issues/727). [noInferrableTypes](https://biomejs.dev/linter/rules/no-inferrable-types) now correctly keeps type annotations when the initialization expression is `null`. Contributed by @Conaclos
 
+- Fix [#784](https://github.com/biomejs/biome/issues/784), [noSvgWithoutTitle](https://biomejs.dev/linter/rules/no-svg-without-title) fixes false-positives to `aria-label` and reports svg's role attribute is implicit. Contributed by @unvalley
+
 ### Parser
 
 ### VSCode

diff --git a/crates/biome_js_analyze/src/analyzers/a11y/no_svg_without_title.rs b/crates/biome_js_analyze/src/analyzers/a11y/no_svg_without_title.rs
@@ -62,6 +62,12 @@ declare_rule! {
     /// ```
     ///
     /// ```js
+    /// <svg role="img" aria-labelledby="title">
+    ///     <span id="title">Pass</span>
+    /// </svg>
+    /// ```
+    ///
+    /// ```js
     /// <svg role="img" aria-label="title">
     ///     <span id="title">Pass</span>
     /// </svg>
@@ -108,29 +114,31 @@ impl Rule for NoSvgWithoutTitle {
         };
 
         let role_attribute_value = role_attribute.initializer()?.value().ok()?;
-        let Some(text) = role_attribute_value
+        let Some(role_attribute_text) = role_attribute_value
             .as_jsx_string()?
             .inner_string_text()
             .ok()
         else {
             return Some(());
         };
 
-        if text.to_lowercase() == "img" {
-            let [aria_label, aria_labelledby] = node
-                .attributes()
-                .find_by_names(["aria-label", "aria-labelledby"]);
-
-            let jsx_child_list = jsx_element.children();
-            let is_valid = is_valid_attribute_value(aria_label, &jsx_child_list).unwrap_or(false)
-                || is_valid_attribute_value(aria_labelledby, &jsx_child_list).unwrap_or(false);
-
-            if !is_valid {
-                return Some(());
+        match role_attribute_text.to_lowercase().as_str() {
+            "img" => {
+                let [aria_label, aria_labelledby] = node
+                    .attributes()
+                    .find_by_names(["aria-label", "aria-labelledby"]);
+                let is_valid_a11y_attribute = aria_label.is_some()
+                    || is_valid_attribute_value(aria_labelledby, &jsx_element.children())
+                        .unwrap_or(false);
+                if is_valid_a11y_attribute {
+                    return None;
+                }
+                Some(())
             }
-        };
-
-        None
+            // if role attribute is empty, the svg element should have title element
+            "" => Some(()),
+            _ => None,
+        }
     }
 
     fn diagnostic(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<RuleDiagnostic> {

diff --git a/crates/biome_js_analyze/tests/specs/a11y/noSvgWithoutTitle/invalid.jsx b/crates/biome_js_analyze/tests/specs/a11y/noSvgWithoutTitle/invalid.jsx
@@ -4,13 +4,13 @@
 		<title></title>
 		<circle />
 	</svg>
-	<svg role="img" aria-label="title">
-		<span id="">foo</span>
-	</svg>
 	<svg role="img" title="title">
 		<span id="">foo</span>
 	</svg>
 	<svg role="img" aria-labelledby="title">
 		<span id="">foo</span>
 	</svg>
+	<svg role="">
+		<span>implicit role</span>
+	</svg>
 </>;
diff --git a/crates/biome_js_analyze/tests/specs/a11y/noSvgWithoutTitle/invalid.jsx.snap b/crates/biome_js_analyze/tests/specs/a11y/noSvgWithoutTitle/invalid.jsx.snap
@@ -10,15 +10,15 @@ expression: invalid.jsx
 		<title></title>
 		<circle />
 	</svg>
-	<svg role="img" aria-label="title">
-		<span id="">foo</span>
-	</svg>
 	<svg role="img" title="title">
 		<span id="">foo</span>
 	</svg>
 	<svg role="img" aria-labelledby="title">
 		<span id="">foo</span>
 	</svg>
+	<svg role="">
+		<span>implicit role</span>
+	</svg>
 </>;
 
 ```
@@ -64,8 +64,8 @@ invalid.jsx:7:2 lint/a11y/noSvgWithoutTitle ━━━━━━━━━━━━
   
     5 │ 		<circle />
     6 │ 	</svg>
-  > 7 │ 	<svg role="img" aria-label="title">
-      │ 	^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  > 7 │ 	<svg role="img" title="title">
+      │ 	^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     8 │ 		<span id="">foo</span>
     9 │ 	</svg>
   
@@ -81,8 +81,8 @@ invalid.jsx:10:2 lint/a11y/noSvgWithoutTitle ━━━━━━━━━━━
   
      8 │ 		<span id="">foo</span>
      9 │ 	</svg>
-  > 10 │ 	<svg role="img" title="title">
-       │ 	^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  > 10 │ 	<svg role="img" aria-labelledby="title">
+       │ 	^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     11 │ 		<span id="">foo</span>
     12 │ 	</svg>
   
@@ -98,9 +98,9 @@ invalid.jsx:13:2 lint/a11y/noSvgWithoutTitle ━━━━━━━━━━━
   
     11 │ 		<span id="">foo</span>
     12 │ 	</svg>
-  > 13 │ 	<svg role="img" aria-labelledby="title">
-       │ 	^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-    14 │ 		<span id="">foo</span>
+  > 13 │ 	<svg role="">
+       │ 	^^^^^^^^^^^^^
+    14 │ 		<span>implicit role</span>
     15 │ 	</svg>
   
   i For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.

diff --git a/crates/biome_js_analyze/tests/specs/a11y/noSvgWithoutTitle/valid.jsx b/crates/biome_js_analyze/tests/specs/a11y/noSvgWithoutTitle/valid.jsx
@@ -22,12 +22,19 @@
 		<title id="title">Pass</title>
 	</svg>
 	<svg role="img" aria-label="title">
-		<span id="title">Pass</span>
+		<span>Pass</span>
+	</svg>
+	<svg role="img" aria-label="title">
+		<span id="sample">Pass</span>
 	</svg>
 	<svg role="img" aria-labelledby="title">
 		<title id="title">Pass</title>
 	</svg>
 	<svg role="img" aria-labelledby="title">
 		<span id="title">Pass</span>
 	</svg>
+	<svg role="">
+		<title>implicit role</title>
+		<span>Pass</span>
+	</svg>
 </>;
diff --git a/crates/biome_js_analyze/tests/specs/a11y/noSvgWithoutTitle/valid.jsx.snap b/crates/biome_js_analyze/tests/specs/a11y/noSvgWithoutTitle/valid.jsx.snap
@@ -28,14 +28,21 @@ expression: valid.jsx
 		<title id="title">Pass</title>
 	</svg>
 	<svg role="img" aria-label="title">
-		<span id="title">Pass</span>
+		<span>Pass</span>
+	</svg>
+	<svg role="img" aria-label="title">
+		<span id="sample">Pass</span>
 	</svg>
 	<svg role="img" aria-labelledby="title">
 		<title id="title">Pass</title>
 	</svg>
 	<svg role="img" aria-labelledby="title">
 		<span id="title">Pass</span>
 	</svg>
+	<svg role="">
+		<title>implicit role</title>
+		<span>Pass</span>
+	</svg>
 </>;
 
 ```

diff --git a/website/src/content/docs/internals/changelog.mdx b/website/src/content/docs/internals/changelog.mdx
@@ -60,6 +60,8 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom
 
 - Fix [#727](https://github.com/biomejs/biome/issues/727). [noInferrableTypes](https://biomejs.dev/linter/rules/no-inferrable-types) now correctly keeps type annotations when the initialization expression is `null`. Contributed by @Conaclos
 
+- Fix [#784](https://github.com/biomejs/biome/issues/784), [noSvgWithoutTitle](https://biomejs.dev/linter/rules/no-svg-without-title) fixes false-positives to `aria-label` and reports svg's role attribute is implicit. Contributed by @unvalley
+
 ### Parser
 
 ### VSCode

diff --git a/website/src/content/docs/linter/rules/no-svg-without-title.md b/website/src/content/docs/linter/rules/no-svg-without-title.md
@@ -92,6 +92,12 @@ To make svg accessible, the following methods are available:
 </svg>
 ```
 
+```jsx
+<svg role="img" aria-labelledby="title">
+    <span id="title">Pass</span>
+</svg>
+```
+
 ```jsx
 <svg role="img" aria-label="title">
     <span id="title">Pass</span>