Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(linter): heading-has-content for eslint-plugin-jsx-a11y #1501

Merged
merged 6 commits into from
Nov 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion crates/oxc_linter/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,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 @@ -386,5 +387,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
Loading