From ebf5cf8c82d59fb45e467049b284612b448a150d Mon Sep 17 00:00:00 2001 From: Ken-HH24 <62000888+Ken-HH24@users.noreply.github.com> Date: Thu, 23 Nov 2023 17:28:22 +0800 Subject: [PATCH] feat(linter): heading-has-content for eslint-plugin-jsx-a11y (#1501) Try to implement `heading-has-content` for #1141 . --- crates/oxc_linter/src/rules.rs | 4 +- .../oxc_linter/src/rules/jsx_a11y/alt_text.rs | 20 +-- .../src/rules/jsx_a11y/heading_has_content.rs | 166 ++++++++++++++++++ .../src/snapshots/heading_has_content.snap | 61 +++++++ crates/oxc_linter/src/utils/react.rs | 61 ++++++- 5 files changed, 290 insertions(+), 22 deletions(-) create mode 100644 crates/oxc_linter/src/rules/jsx_a11y/heading_has_content.rs create mode 100644 crates/oxc_linter/src/snapshots/heading_has_content.snap diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index b559c5e400e83..51b26cb69d51b 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -207,6 +207,7 @@ mod jsx_a11y { pub mod alt_text; pub mod anchor_has_content; pub mod anchor_is_valid; + pub mod heading_has_content; pub mod html_has_lang; } @@ -390,5 +391,6 @@ oxc_macros::declare_all_lint_rules! { jsx_a11y::alt_text, jsx_a11y::anchor_has_content, jsx_a11y::anchor_is_valid, - jsx_a11y::html_has_lang + jsx_a11y::html_has_lang, + jsx_a11y::heading_has_content } diff --git a/crates/oxc_linter/src/rules/jsx_a11y/alt_text.rs b/crates/oxc_linter/src/rules/jsx_a11y/alt_text.rs index d6d8a61bb8d99..a87dd7d8d7b7f 100644 --- a/crates/oxc_linter/src/rules/jsx_a11y/alt_text.rs +++ b/crates/oxc_linter/src/rules/jsx_a11y/alt_text.rs @@ -12,7 +12,7 @@ use oxc_diagnostics::{ use oxc_macros::declare_oxc_lint; use oxc_span::Span; -use crate::utils::has_jsx_prop_lowercase; +use crate::utils::{get_literal_prop_value, get_prop_value, has_jsx_prop_lowercase}; use crate::{context::LintContext, rule::Rule, AstNode}; #[derive(Debug, Error, Diagnostic)] @@ -193,24 +193,6 @@ impl Rule for AltText { } } -fn get_prop_value<'a, 'b>(item: &'b JSXAttributeItem<'a>) -> Option<&'b JSXAttributeValue<'a>> { - if let JSXAttributeItem::Attribute(attr) = item { - attr.0.value.as_ref() - } else { - None - } -} - -fn get_literal_prop_value<'a>(item: &'a JSXAttributeItem<'_>) -> Option<&'a str> { - get_prop_value(item).and_then(|v| { - if let JSXAttributeValue::StringLiteral(s) = v { - Some(s.value.as_str()) - } else { - None - } - }) -} - fn is_valid_alt_prop(item: &JSXAttributeItem<'_>) -> bool { match get_prop_value(item) { None => false, diff --git a/crates/oxc_linter/src/rules/jsx_a11y/heading_has_content.rs b/crates/oxc_linter/src/rules/jsx_a11y/heading_has_content.rs new file mode 100644 index 0000000000000..5bb312b2fb94e --- /dev/null +++ b/crates/oxc_linter/src/rules/jsx_a11y/heading_has_content.rs @@ -0,0 +1,166 @@ +use oxc_ast::{ast::JSXElementName, AstKind}; +use oxc_diagnostics::{ + miette::{self, Diagnostic}, + thiserror::Error, +}; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; + +use crate::{ + context::LintContext, + rule::Rule, + utils::{is_hidden_from_screen_reader, object_has_accessible_child}, + AstNode, +}; + +#[derive(Debug, Error, Diagnostic)] +#[error("eslint(heading-has-content): Headings must have content and the content must be accessible by a screen reader.")] +#[diagnostic( + severity(warning), + help("Provide screen reader accessible content when using heading elements.") +)] +struct HeadingHasContentDiagnostic(#[label] pub Span); + +#[derive(Debug, Default, Clone)] +pub struct HeadingHasContent { + components: Option>, +} + +declare_oxc_lint!( + /// ### What it does + /// + /// Enforce that heading elements (h1, h2, etc.) have content and + /// that the content is accessible to screen readers. + /// Accessible means that it is not hidden using the aria-hidden prop. + /// + /// ### Why is this bad? + /// + /// Screen readers alert users to the presence of a heading tag. + /// If the heading is empty or the text cannot be accessed, + /// this could either confuse users or even prevent them + /// from accessing information on the page's structure. + /// + /// ### Example + /// ```javascript + /// // Bad + ///

+ /// + /// // Good + ///

Foo

+ /// ``` + HeadingHasContent, + correctness +); + +// always including

thru

+const DEFAULT_COMPONENTS: [&str; 6] = ["h1", "h2", "h3", "h4", "h5", "h6"]; + +impl Rule for HeadingHasContent { + fn from_configuration(value: serde_json::Value) -> Self { + Self { + components: value + .get(0) + .and_then(|v| v.get("components")) + .and_then(serde_json::Value::as_array) + .map(|v| { + v.iter() + .filter_map(serde_json::Value::as_str) + .map(ToString::to_string) + .collect() + }), + } + } + + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + let AstKind::JSXOpeningElement(jsx_el) = node.kind() else { + return; + }; + + let JSXElementName::Identifier(iden) = &jsx_el.name else { + return; + }; + + let name = iden.name.as_str(); + + if !DEFAULT_COMPONENTS.iter().any(|&comp| comp == name) + && !self + .components + .as_ref() + .is_some_and(|components| components.iter().any(|comp| comp == name)) + { + return; + } + + let maybe_parent = ctx.nodes().parent_node(node.id()).map(oxc_semantic::AstNode::kind); + if let Some(AstKind::JSXElement(parent)) = maybe_parent { + if object_has_accessible_child(parent) { + return; + } + } + + if is_hidden_from_screen_reader(jsx_el) { + return; + } + + ctx.diagnostic(HeadingHasContentDiagnostic(jsx_el.span)); + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + fn components() -> serde_json::Value { + serde_json::json!([{ + "components": ["Heading", "Title"], + }]) + } + + let pass = vec![ + // DEFAULT ELEMENT TESTS + (r"

Foo

", None), + (r"

Foo

", None), + (r"

Foo

", None), + (r"

Foo

", None), + (r"
Foo
", None), + (r"
Foo
", None), + (r"
123
", None), + (r"

", None), + (r"

{foo}

", None), + (r"

{foo.bar}

", None), + (r#"

"#, None), + (r"

", None), + // CUSTOM ELEMENT TESTS FOR COMPONENTS OPTION + (r"Foo", Some(components())), + (r"Foo", Some(components())), + (r"", Some(components())), + (r"{foo}", Some(components())), + (r"{foo.bar}", Some(components())), + (r#""#, Some(components())), + (r"", Some(components())), + (r"

", Some(components())), + // TODO: When polymorphic components are supported + // CUSTOM ELEMENT TESTS FOR COMPONENTS SETTINGS + // (r"Foo", None), + // (r#"

"#, None), + ]; + + let fail = vec![ + // DEFAULT ELEMENT TESTS + (r"

", None), + (r"

", None), + (r"

{undefined}

", None), + (r"

<>

", None), + (r#"

"#, None), + // CUSTOM ELEMENT TESTS FOR COMPONENTS OPTION + (r"", Some(components())), + (r"", Some(components())), + (r"{undefined}", Some(components())), + // TODO: When polymorphic components are supported + // CUSTOM ELEMENT TESTS FOR COMPONENTS SETTINGS + // (r"", None), + // (r#"

"#, None), + ]; + + Tester::new(HeadingHasContent::NAME, pass, fail).with_jsx_a11y_plugin(true).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/heading_has_content.snap b/crates/oxc_linter/src/snapshots/heading_has_content.snap new file mode 100644 index 0000000000000..a33b34cb30263 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/heading_has_content.snap @@ -0,0 +1,61 @@ +--- +source: crates/oxc_linter/src/tester.rs +expression: heading_has_content +--- + ⚠ eslint(heading-has-content): Headings must have content and the content must be accessible by a screen reader. + ╭─[heading_has_content.tsx:1:1] + 1 │

+ · ────── + ╰──── + help: Provide screen reader accessible content when using heading elements. + + ⚠ eslint(heading-has-content): Headings must have content and the content must be accessible by a screen reader. + ╭─[heading_has_content.tsx:1:1] + 1 │

+ · ──── + ╰──── + help: Provide screen reader accessible content when using heading elements. + + ⚠ eslint(heading-has-content): Headings must have content and the content must be accessible by a screen reader. + ╭─[heading_has_content.tsx:1:1] + 1 │

{undefined}

+ · ──── + ╰──── + help: Provide screen reader accessible content when using heading elements. + + ⚠ eslint(heading-has-content): Headings must have content and the content must be accessible by a screen reader. + ╭─[heading_has_content.tsx:1:1] + 1 │

<>

+ · ──── + ╰──── + help: Provide screen reader accessible content when using heading elements. + + ⚠ eslint(heading-has-content): Headings must have content and the content must be accessible by a screen reader. + ╭─[heading_has_content.tsx:1:1] + 1 │

+ · ──── + ╰──── + help: Provide screen reader accessible content when using heading elements. + + ⚠ eslint(heading-has-content): Headings must have content and the content must be accessible by a screen reader. + ╭─[heading_has_content.tsx:1:1] + 1 │ + · ─────────── + ╰──── + help: Provide screen reader accessible content when using heading elements. + + ⚠ eslint(heading-has-content): Headings must have content and the content must be accessible by a screen reader. + ╭─[heading_has_content.tsx:1:1] + 1 │ + · ───────── + ╰──── + help: Provide screen reader accessible content when using heading elements. + + ⚠ eslint(heading-has-content): Headings must have content and the content must be accessible by a screen reader. + ╭─[heading_has_content.tsx:1:1] + 1 │ {undefined} + · ───────── + ╰──── + help: Provide screen reader accessible content when using heading elements. + + diff --git a/crates/oxc_linter/src/utils/react.rs b/crates/oxc_linter/src/utils/react.rs index af2e04c622fc6..e64fb894dd845 100644 --- a/crates/oxc_linter/src/utils/react.rs +++ b/crates/oxc_linter/src/utils/react.rs @@ -1,5 +1,9 @@ use oxc_ast::{ - ast::{CallExpression, Expression, JSXAttributeItem, JSXAttributeName, JSXOpeningElement}, + ast::{ + CallExpression, Expression, JSXAttributeItem, JSXAttributeName, JSXAttributeValue, + JSXChild, JSXElement, JSXElementName, JSXExpression, JSXExpressionContainer, + JSXOpeningElement, + }, AstKind, }; use oxc_semantic::AstNode; @@ -37,11 +41,64 @@ pub fn has_jsx_prop_lowercase<'a, 'b>( JSXAttributeItem::Attribute(attr) => { let JSXAttributeName::Identifier(name) = &attr.name else { return false }; - name.name.as_str().to_lowercase() == target_prop + name.name.as_str().to_lowercase() == target_prop.to_lowercase() } }) } +pub fn get_prop_value<'a, 'b>(item: &'b JSXAttributeItem<'a>) -> Option<&'b JSXAttributeValue<'a>> { + if let JSXAttributeItem::Attribute(attr) = item { + attr.0.value.as_ref() + } else { + None + } +} + +pub fn get_literal_prop_value<'a>(item: &'a JSXAttributeItem<'_>) -> Option<&'a str> { + get_prop_value(item).and_then(|v| { + 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 +pub fn is_hidden_from_screen_reader(node: &JSXOpeningElement) -> bool { + if let JSXElementName::Identifier(iden) = &node.name { + if iden.name.as_str().to_uppercase() == "INPUT" { + if let Some(item) = has_jsx_prop_lowercase(node, "type") { + let hidden = get_literal_prop_value(item); + + if hidden.is_some_and(|val| val.to_uppercase() == "HIDDEN") { + return true; + } + } + } + } + + has_jsx_prop_lowercase(node, "aria-hidden").map_or(false, |v| match get_prop_value(v) { + None => true, + Some(JSXAttributeValue::StringLiteral(s)) if s.value == "true" => true, + _ => false, + }) +} + +// ref: https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/main/src/util/hasAccessibleChild.js +pub fn object_has_accessible_child(node: &JSXElement<'_>) -> bool { + node.children.iter().any(|child| match child { + JSXChild::Text(text) => !text.value.is_empty(), + JSXChild::Element(el) => !is_hidden_from_screen_reader(&el.opening_element), + JSXChild::ExpressionContainer(JSXExpressionContainer { + expression: JSXExpression::Expression(expr), + .. + }) => !expr.is_undefined(), + _ => false, + }) || has_jsx_prop_lowercase(&node.opening_element, "dangerouslySetInnerHTML").is_some() + || has_jsx_prop_lowercase(&node.opening_element, "children").is_some() +} + const PRAGMA: &str = "React"; const CREATE_CLASS: &str = "createReactClass";