Skip to content

Commit

Permalink
feat(linter): heading-has-content for eslint-plugin-jsx-a11y (#1501)
Browse files Browse the repository at this point in the history
Try to implement `heading-has-content` for #1141 .
  • Loading branch information
Ken-HH24 authored Nov 23, 2023
1 parent 635008a commit ebf5cf8
Show file tree
Hide file tree
Showing 5 changed files with 290 additions and 22 deletions.
4 changes: 3 additions & 1 deletion crates/oxc_linter/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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
}
20 changes: 1 addition & 19 deletions crates/oxc_linter/src/rules/jsx_a11y/alt_text.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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,
Expand Down
166 changes: 166 additions & 0 deletions crates/oxc_linter/src/rules/jsx_a11y/heading_has_content.rs
Original file line number Diff line number Diff line change
@@ -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<Vec<String>>,
}

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
/// <h1 />
///
/// // Good
/// <h1>Foo</h1>
/// ```
HeadingHasContent,
correctness
);

// always including <h1> thru <h6>
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"<h1>Foo</h1>", None),
(r"<h2>Foo</h2>", None),
(r"<h3>Foo</h3>", None),
(r"<h4>Foo</h4>", None),
(r"<h5>Foo</h5>", None),
(r"<h6>Foo</h6>", None),
(r"<h6>123</h6>", None),
(r"<h1><Bar /></h1>", None),
(r"<h1>{foo}</h1>", None),
(r"<h1>{foo.bar}</h1>", None),
(r#"<h1 dangerouslySetInnerHTML={{ __html: "foo" }} />"#, None),
(r"<h1 children={children} />", None),
// CUSTOM ELEMENT TESTS FOR COMPONENTS OPTION
(r"<Heading>Foo</Heading>", Some(components())),
(r"<Title>Foo</Title>", Some(components())),
(r"<Heading><Bar /></Heading>", Some(components())),
(r"<Heading>{foo}</Heading>", Some(components())),
(r"<Heading>{foo.bar}</Heading>", Some(components())),
(r#"<Heading dangerouslySetInnerHTML={{ __html: "foo" }} />"#, Some(components())),
(r"<Heading children={children} />", Some(components())),
(r"<h1 aria-hidden />", Some(components())),
// TODO: When polymorphic components are supported
// CUSTOM ELEMENT TESTS FOR COMPONENTS SETTINGS
// (r"<Heading>Foo</Heading>", None),
// (r#"<h1><CustomInput type="hidden" /></h1>"#, None),
];

let fail = vec![
// DEFAULT ELEMENT TESTS
(r"<h1 />", None),
(r"<h1><Bar aria-hidden /></h1>", None),
(r"<h1>{undefined}</h1>", None),
(r"<h1><></></h1>", None),
(r#"<h1><input type="hidden" /></h1>"#, None),
// CUSTOM ELEMENT TESTS FOR COMPONENTS OPTION
(r"<Heading />", Some(components())),
(r"<Heading><Bar aria-hidden /></Heading>", Some(components())),
(r"<Heading>{undefined}</Heading>", Some(components())),
// TODO: When polymorphic components are supported
// CUSTOM ELEMENT TESTS FOR COMPONENTS SETTINGS
// (r"<Heading />", None),
// (r#"<h1><CustomInput type="hidden" /></h1>"#, None),
];

Tester::new(HeadingHasContent::NAME, pass, fail).with_jsx_a11y_plugin(true).test_and_snapshot();
}
61 changes: 61 additions & 0 deletions crates/oxc_linter/src/snapshots/heading_has_content.snap
Original file line number Diff line number Diff line change
@@ -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<h1 />
· ──────
╰────
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 │ <h1><Bar aria-hidden /></h1>
· ────
╰────
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 │ <h1>{undefined}</h1>
· ────
╰────
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 │ <h1><></></h1>
· ────
╰────
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 │ <h1><input type="hidden" /></h1>
· ────
╰────
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<Heading />
· ───────────
╰────
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 │ <Heading><Bar aria-hidden /></Heading>
· ─────────
╰────
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 │ <Heading>{undefined}</Heading>
· ─────────
╰────
help: Provide screen reader accessible content when using heading elements.


61 changes: 59 additions & 2 deletions crates/oxc_linter/src/utils/react.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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";

Expand Down

0 comments on commit ebf5cf8

Please sign in to comment.