From a2e098de6644c00f571dee34ca4be8dc75f1ca27 Mon Sep 17 00:00:00 2001 From: unvalley Date: Mon, 20 Nov 2023 01:45:12 +0900 Subject: [PATCH 1/6] fix(lint): false-positives for aria-label in no-svg-without-title and report cases where the role is other than img --- .../analyzers/a11y/no_svg_without_title.rs | 37 +++++---- .../specs/a11y/noSvgWithoutTitle/invalid.jsx | 15 +++- .../a11y/noSvgWithoutTitle/invalid.jsx.snap | 75 +++++++++++++++---- .../specs/a11y/noSvgWithoutTitle/valid.jsx | 17 ++++- .../a11y/noSvgWithoutTitle/valid.jsx.snap | 17 ++++- 5 files changed, 125 insertions(+), 36 deletions(-) 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 index 53379237edeb..1e3181f1cbcb 100644 --- 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 + /// + /// Pass + /// + /// ``` + /// + /// ```js /// /// Pass /// @@ -108,7 +114,7 @@ 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() @@ -116,21 +122,22 @@ impl Rule for NoSvgWithoutTitle { 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 not "img" role, the svg element should have a valid title element + _ => Some(()), + } } fn diagnostic(ctx: &RuleContext, _state: &Self::State) -> Option { diff --git a/crates/biome_js_analyze/tests/specs/a11y/noSvgWithoutTitle/invalid.jsx b/crates/biome_js_analyze/tests/specs/a11y/noSvgWithoutTitle/invalid.jsx index 36efeeb02595..7ac3cfae36af 100644 --- 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,22 @@ - - foo - foo + + aria-label + foo + + implicit role + + + presentation role without title + + + button role without title + ; 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 index 72d53852b46c..44824bab860e 100644 --- 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,24 @@ expression: invalid.jsx - - foo - foo + + aria-label + foo + + implicit role + + + presentation role without title + + + button role without title + ; ``` @@ -64,8 +73,8 @@ invalid.jsx:7:2 lint/a11y/noSvgWithoutTitle ━━━━━━━━━━━━ 5 │ 6 │ - > 7 │ - │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + > 7 │ + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 8 │ foo 9 │ @@ -75,16 +84,16 @@ invalid.jsx:7:2 lint/a11y/noSvgWithoutTitle ━━━━━━━━━━━━ ``` ``` -invalid.jsx:10:2 lint/a11y/noSvgWithoutTitle ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +invalid.jsx:13:2 lint/a11y/noSvgWithoutTitle ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! Alternative text title element cannot be empty - 8 │ foo - 9 │ - > 10 │ - │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - 11 │ foo + 11 │ aria-label 12 │ + > 13 │ + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 14 │ foo + 15 │ 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. @@ -92,16 +101,50 @@ invalid.jsx:10:2 lint/a11y/noSvgWithoutTitle ━━━━━━━━━━━ ``` ``` -invalid.jsx:13:2 lint/a11y/noSvgWithoutTitle ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +invalid.jsx:16:2 lint/a11y/noSvgWithoutTitle ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! Alternative text title element cannot be empty - 11 │ foo - 12 │ - > 13 │ - │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 14 │ foo 15 │ + > 16 │ + │ ^^^^^^^^^^^^^ + 17 │ implicit role + 18 │ + + 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. + + +``` + +``` +invalid.jsx:19:2 lint/a11y/noSvgWithoutTitle ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Alternative text title element cannot be empty + + 17 │ implicit role + 18 │ + > 19 │ + │ ^^^^^^^^^^^^^^^^^^^^^^^^^ + 20 │ presentation role without title + 21 │ + + 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. + + +``` + +``` +invalid.jsx:22:2 lint/a11y/noSvgWithoutTitle ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Alternative text title element cannot be empty + + 20 │ presentation role without title + 21 │ + > 22 │ + │ ^^^^^^^^^^^^^^^^^^^ + 23 │ button role without title + 24 │ 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 index f41f3baafc14..b239998ffacd 100644 --- a/crates/biome_js_analyze/tests/specs/a11y/noSvgWithoutTitle/valid.jsx +++ b/crates/biome_js_analyze/tests/specs/a11y/noSvgWithoutTitle/valid.jsx @@ -22,7 +22,10 @@ Pass - Pass + Pass + + + Pass Pass @@ -30,4 +33,16 @@ Pass + + implicit role + Pass + + + presentation role with title + Pass + + + button role with title + Pass + ; 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 index d899da044161..38547922b604 100644 --- 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,7 +28,10 @@ expression: valid.jsx Pass - Pass + Pass + + + Pass Pass @@ -36,6 +39,18 @@ expression: valid.jsx Pass + + implicit role + Pass + + + presentation role with title + Pass + + + button role with title + Pass + ; ``` From c4b37ef228b6f3914102c524a4d3df7692253a4b Mon Sep 17 00:00:00 2001 From: unvalley Date: Mon, 20 Nov 2023 01:52:04 +0900 Subject: [PATCH 2/6] docs: update lint example --- .../src/content/docs/linter/rules/no-svg-without-title.md | 6 ++++++ 1 file changed, 6 insertions(+) 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 index 3d398a96faf6..dd180031156a 100644 --- 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: ``` +```jsx + + Pass + +``` + ```jsx Pass From 07b40726cc980c2e5b401fe06be1361993da2696 Mon Sep 17 00:00:00 2001 From: unvalley Date: Mon, 20 Nov 2023 01:58:29 +0900 Subject: [PATCH 3/6] chore: update the CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f571055a7fd..b3e8258c26f6 100644 --- 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 other than `img`. Contributed by @unvalley + ### Parser ### VSCode From 06b8160afc41e783b28e9c0305e698669bd2ad14 Mon Sep 17 00:00:00 2001 From: unvalley Date: Mon, 20 Nov 2023 02:02:02 +0900 Subject: [PATCH 4/6] chore: fix waste space and codegen for changelog --- .../biome_js_analyze/src/analyzers/a11y/no_svg_without_title.rs | 2 +- website/src/content/docs/internals/changelog.mdx | 2 ++ website/src/content/docs/linter/rules/no-svg-without-title.md | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) 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 index 1e3181f1cbcb..6f67a4ff11c8 100644 --- 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,7 +62,7 @@ declare_rule! { /// ``` /// /// ```js - /// + /// /// Pass /// /// ``` diff --git a/website/src/content/docs/internals/changelog.mdx b/website/src/content/docs/internals/changelog.mdx index 2f263143fd45..7c58b6685ac0 100644 --- 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 other than `img`. 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 index dd180031156a..53e45defda6c 100644 --- a/website/src/content/docs/linter/rules/no-svg-without-title.md +++ b/website/src/content/docs/linter/rules/no-svg-without-title.md @@ -93,7 +93,7 @@ To make svg accessible, the following methods are available: ``` ```jsx - + Pass ``` From 3c23440b406055e358bfcf5b2116ec24778b011a Mon Sep 17 00:00:00 2001 From: unvalley Date: Mon, 20 Nov 2023 02:29:17 +0900 Subject: [PATCH 5/6] fix: do not report to specific role other than img --- CHANGELOG.md | 2 +- .../analyzers/a11y/no_svg_without_title.rs | 5 +- .../specs/a11y/noSvgWithoutTitle/invalid.jsx | 9 --- .../a11y/noSvgWithoutTitle/invalid.jsx.snap | 67 ++++--------------- .../specs/a11y/noSvgWithoutTitle/valid.jsx | 8 --- .../a11y/noSvgWithoutTitle/valid.jsx.snap | 8 --- 6 files changed, 16 insertions(+), 83 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b3e8258c26f6..6bb80042a7c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,7 +54,7 @@ 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 other than `img`. Contributed by @unvalley +- 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 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 index 6f67a4ff11c8..7a179c1d6a11 100644 --- 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 @@ -135,8 +135,9 @@ impl Rule for NoSvgWithoutTitle { } Some(()) } - // if not "img" role, the svg element should have a valid title element - _ => Some(()), + // if role attribute is empty, the svg element should have title element + "" => Some(()), + _ => None, } } diff --git a/crates/biome_js_analyze/tests/specs/a11y/noSvgWithoutTitle/invalid.jsx b/crates/biome_js_analyze/tests/specs/a11y/noSvgWithoutTitle/invalid.jsx index 7ac3cfae36af..26cbecc5480c 100644 --- a/crates/biome_js_analyze/tests/specs/a11y/noSvgWithoutTitle/invalid.jsx +++ b/crates/biome_js_analyze/tests/specs/a11y/noSvgWithoutTitle/invalid.jsx @@ -7,19 +7,10 @@ foo - - aria-label - foo implicit role - - presentation role without title - - - button role without title - ; 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 index 44824bab860e..5e58ed4d36d0 100644 --- a/crates/biome_js_analyze/tests/specs/a11y/noSvgWithoutTitle/invalid.jsx.snap +++ b/crates/biome_js_analyze/tests/specs/a11y/noSvgWithoutTitle/invalid.jsx.snap @@ -13,21 +13,12 @@ expression: invalid.jsx foo - - aria-label - foo implicit role - - presentation role without title - - - button role without title - ; ``` @@ -84,16 +75,16 @@ invalid.jsx:7:2 lint/a11y/noSvgWithoutTitle ━━━━━━━━━━━━ ``` ``` -invalid.jsx:13:2 lint/a11y/noSvgWithoutTitle ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +invalid.jsx:10:2 lint/a11y/noSvgWithoutTitle ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! Alternative text title element cannot be empty - 11 │ aria-label - 12 │ - > 13 │ + 8 │ foo + 9 │ + > 10 │ │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - 14 │ foo - 15 │ + 11 │ foo + 12 │ 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. @@ -101,50 +92,16 @@ invalid.jsx:13:2 lint/a11y/noSvgWithoutTitle ━━━━━━━━━━━ ``` ``` -invalid.jsx:16:2 lint/a11y/noSvgWithoutTitle ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +invalid.jsx:13:2 lint/a11y/noSvgWithoutTitle ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! Alternative text title element cannot be empty - 14 │ foo - 15 │ - > 16 │ + 11 │ foo + 12 │ + > 13 │ │ ^^^^^^^^^^^^^ - 17 │ implicit role - 18 │ - - 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. - - -``` - -``` -invalid.jsx:19:2 lint/a11y/noSvgWithoutTitle ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - - ! Alternative text title element cannot be empty - - 17 │ implicit role - 18 │ - > 19 │ - │ ^^^^^^^^^^^^^^^^^^^^^^^^^ - 20 │ presentation role without title - 21 │ - - 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. - - -``` - -``` -invalid.jsx:22:2 lint/a11y/noSvgWithoutTitle ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - - ! Alternative text title element cannot be empty - - 20 │ presentation role without title - 21 │ - > 22 │ - │ ^^^^^^^^^^^^^^^^^^^ - 23 │ button role without title - 24 │ + 14 │ implicit role + 15 │ 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 index b239998ffacd..f9975cf3b859 100644 --- a/crates/biome_js_analyze/tests/specs/a11y/noSvgWithoutTitle/valid.jsx +++ b/crates/biome_js_analyze/tests/specs/a11y/noSvgWithoutTitle/valid.jsx @@ -37,12 +37,4 @@ implicit role Pass - - presentation role with title - Pass - - - button role with title - Pass - ; 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 index 38547922b604..0cdbf0578c18 100644 --- a/crates/biome_js_analyze/tests/specs/a11y/noSvgWithoutTitle/valid.jsx.snap +++ b/crates/biome_js_analyze/tests/specs/a11y/noSvgWithoutTitle/valid.jsx.snap @@ -43,14 +43,6 @@ expression: valid.jsx implicit role Pass - - presentation role with title - Pass - - - button role with title - Pass - ; ``` From c2c94edd5e0e4718031963d7ce673b5229024862 Mon Sep 17 00:00:00 2001 From: unvalley Date: Mon, 20 Nov 2023 02:31:59 +0900 Subject: [PATCH 6/6] chore: codegen CHANGELOG.md --- website/src/content/docs/internals/changelog.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/src/content/docs/internals/changelog.mdx b/website/src/content/docs/internals/changelog.mdx index 7c58b6685ac0..17028cb936cc 100644 --- a/website/src/content/docs/internals/changelog.mdx +++ b/website/src/content/docs/internals/changelog.mdx @@ -60,7 +60,7 @@ 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 other than `img`. Contributed by @unvalley +- 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