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 de1e88090..b352bec67 100644 --- a/crates/oxc_linter/src/rules/jsx_a11y/alt_text.rs +++ b/crates/oxc_linter/src/rules/jsx_a11y/alt_text.rs @@ -1,7 +1,7 @@ use oxc_ast::{ ast::{ - JSXAttributeItem, JSXAttributeValue, JSXChild, JSXElement, JSXExpression, - JSXExpressionContainer, JSXOpeningElement, + JSXAttributeItem, JSXAttributeValue, JSXElement, JSXExpression, JSXExpressionContainer, + JSXOpeningElement, }, AstKind, }; @@ -14,6 +14,7 @@ use oxc_span::Span; use crate::utils::{ get_element_type, get_prop_value, get_string_literal_prop_value, has_jsx_prop_lowercase, + object_has_accessible_child, }; use crate::{context::LintContext, rule::Rule, AstNode}; @@ -236,31 +237,6 @@ fn aria_label_has_value<'a>(item: &'a JSXAttributeItem<'a>) -> bool { } } -// ref: https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/main/src/util/hasAccessibleChild.js -fn object_has_accessible_child(node: &JSXElement<'_>) -> bool { - node.children.iter().any(|child| match child { - JSXChild::Text(text) => !text.value.is_empty(), - JSXChild::Fragment(_) => true, - JSXChild::Element(el) => { - let is_hidden_from_screen_reader = - has_jsx_prop_lowercase(&el.opening_element, "aria-hidden").map_or(false, |v| { - match get_prop_value(v) { - None => true, - Some(JSXAttributeValue::StringLiteral(s)) if s.value == "true" => true, - _ => false, - } - }); - !is_hidden_from_screen_reader - } - 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() -} - fn img_rule<'a>(node: &'a JSXOpeningElement<'a>, ctx: &LintContext<'a>) { if let Some(alt_prop) = has_jsx_prop_lowercase(node, "alt") { if !is_valid_alt_prop(alt_prop) { @@ -305,7 +281,7 @@ fn object_rule<'a>( .and_then(get_string_literal_prop_value) .map_or(false, |v| !v.is_empty()); - if has_label || has_title_attr || object_has_accessible_child(parent) { + if has_label || has_title_attr || object_has_accessible_child(ctx, parent) { return; } ctx.diagnostic(AltTextDiagnostic::Object(node.span)); diff --git a/crates/oxc_linter/src/rules/jsx_a11y/anchor_has_content.rs b/crates/oxc_linter/src/rules/jsx_a11y/anchor_has_content.rs index 65c9a5b1a..860a267dd 100644 --- a/crates/oxc_linter/src/rules/jsx_a11y/anchor_has_content.rs +++ b/crates/oxc_linter/src/rules/jsx_a11y/anchor_has_content.rs @@ -1,10 +1,4 @@ -use oxc_ast::{ - ast::{ - Expression, JSXAttributeItem, JSXAttributeValue, JSXChild, JSXElement, JSXElementName, - JSXExpression, - }, - AstKind, -}; +use oxc_ast::AstKind; use oxc_diagnostics::{ miette::{self, Diagnostic}, thiserror::Error, @@ -12,12 +6,13 @@ use oxc_diagnostics::{ use oxc_macros::declare_oxc_lint; use oxc_span::Span; -use oxc_allocator::Vec; - use crate::{ context::LintContext, rule::Rule, - utils::{get_element_type, get_prop_value, has_jsx_prop_lowercase}, + utils::{ + get_element_type, has_jsx_prop_lowercase, is_hidden_from_screen_reader, + object_has_accessible_child, + }, AstNode, }; @@ -77,82 +72,24 @@ impl Rule for AnchorHasContent { if let AstKind::JSXElement(jsx_el) = node.kind() { let Some(name) = &get_element_type(ctx, &jsx_el.opening_element) else { return }; if name == "a" { - // check self attr - if has_jsx_prop_lowercase(&jsx_el.opening_element, "aria-hidden").is_some() { + if is_hidden_from_screen_reader(ctx, &jsx_el.opening_element) { ctx.diagnostic(AnchorHasContentDiagnostic::RemoveAriaHidden(jsx_el.span)); return; } - // check if self attr has title/aria-label - if (has_jsx_prop_lowercase(&jsx_el.opening_element, "title").is_some() - || has_jsx_prop_lowercase(&jsx_el.opening_element, "aria-label").is_some() - || has_jsx_prop_lowercase(&jsx_el.opening_element, "children").is_some() - || has_jsx_prop_lowercase(&jsx_el.opening_element, "dangerouslysetinnerhtml") - .is_some()) - && match_valid_prop(&jsx_el.opening_element.attributes) - { - // pass + if object_has_accessible_child(ctx, jsx_el) { return; } - // check content accessible - check_has_accessible_child(jsx_el, ctx); - } - } - - // custom component - } -} - -fn match_valid_prop(attr_items: &Vec) -> bool { - attr_items - .into_iter() - .any(|attr| matches!(get_prop_value(attr), Some(JSXAttributeValue::ExpressionContainer(_)))) -} - -fn check_has_accessible_child(jsx: &JSXElement, ctx: &LintContext) { - let children = &jsx.children; - if children.len() == 0 { - if let JSXElementName::Identifier(ident) = &jsx.opening_element.name { - ctx.diagnostic(AnchorHasContentDiagnostic::MissingContent(ident.span)); - return; - } - } - - // If each child is inaccessible, an error is reported - let mut diagnostic = AnchorHasContentDiagnostic::MissingContent(jsx.span); - let all_not_has_content = children.into_iter().all(|child| match child { - JSXChild::Text(text) => { - if text.value.trim() == "" { - return true; - } - false - } - JSXChild::ExpressionContainer(exp) => { - if let JSXExpression::Expression(jsexp) = &exp.expression { - if let Expression::Identifier(ident) = jsexp { - if ident.name == "undefined" { - return true; - } - } else if let Expression::NullLiteral(_) = jsexp { - return true; + for attr in ["title", "aria-label"] { + if has_jsx_prop_lowercase(&jsx_el.opening_element, attr).is_some() { + return; + }; } - }; - false - } - JSXChild::Element(ele) => { - let is_hidden = has_jsx_prop_lowercase(&ele.opening_element, "aria-hidden").is_some(); - if is_hidden { - diagnostic = AnchorHasContentDiagnostic::RemoveAriaHidden(jsx.span); - return true; - } - false - } - _ => false, - }); - if all_not_has_content { - ctx.diagnostic(diagnostic); + ctx.diagnostic(AnchorHasContentDiagnostic::MissingContent(jsx_el.span)); + } + } } } @@ -169,6 +106,7 @@ fn test() { (r"{foo.bar}", None, None), (r#""#, None, None), (r"", None, None), + (r"", None, None), ( r"foo", None, @@ -177,7 +115,6 @@ fn test() { (r"", None, None), (r"", None, None), (r"", None, None), - (r"Foo", None, None), ]; let fail = vec![ @@ -189,9 +126,6 @@ fn test() { None, Some(serde_json::json!({ "jsx-a11y": { "components": { "Link": "a" } } })), ), - (r"", None, None), - (r"{null}", None, None), - (r"", None, None), ]; Tester::new(AnchorHasContent::NAME, pass, fail).test_and_snapshot(); diff --git a/crates/oxc_linter/src/rules/jsx_a11y/click_events_have_key_events.rs b/crates/oxc_linter/src/rules/jsx_a11y/click_events_have_key_events.rs index 951117bdf..cdac7d54b 100644 --- a/crates/oxc_linter/src/rules/jsx_a11y/click_events_have_key_events.rs +++ b/crates/oxc_linter/src/rules/jsx_a11y/click_events_have_key_events.rs @@ -65,7 +65,8 @@ impl Rule for ClickEventsHaveKeyEvents { return; }; - if is_hidden_from_screen_reader(jsx_opening_el) || is_presentation_role(jsx_opening_el) { + if is_hidden_from_screen_reader(ctx, jsx_opening_el) || is_presentation_role(jsx_opening_el) + { return; } 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 index dc5f92a16..c9d48e330 100644 --- a/crates/oxc_linter/src/rules/jsx_a11y/heading_has_content.rs +++ b/crates/oxc_linter/src/rules/jsx_a11y/heading_has_content.rs @@ -107,12 +107,12 @@ impl Rule for HeadingHasContent { 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) { + if object_has_accessible_child(ctx, parent) { return; } } - if is_hidden_from_screen_reader(jsx_el) { + if is_hidden_from_screen_reader(ctx, jsx_el) { return; } diff --git a/crates/oxc_linter/src/rules/jsx_a11y/img_redundant_alt.rs b/crates/oxc_linter/src/rules/jsx_a11y/img_redundant_alt.rs index d296a6f69..65a34cf29 100644 --- a/crates/oxc_linter/src/rules/jsx_a11y/img_redundant_alt.rs +++ b/crates/oxc_linter/src/rules/jsx_a11y/img_redundant_alt.rs @@ -117,7 +117,7 @@ impl Rule for ImgRedundantAlt { return; } - if is_hidden_from_screen_reader(jsx_el) { + if is_hidden_from_screen_reader(ctx, jsx_el) { return; } diff --git a/crates/oxc_linter/src/rules/react/no_unknown_property.rs b/crates/oxc_linter/src/rules/react/no_unknown_property.rs index 009ed5ea8..504b06b35 100644 --- a/crates/oxc_linter/src/rules/react/no_unknown_property.rs +++ b/crates/oxc_linter/src/rules/react/no_unknown_property.rs @@ -1,7 +1,7 @@ use itertools::Itertools; use once_cell::sync::Lazy; use oxc_ast::{ - ast::{JSXAttributeItem, JSXAttributeName}, + ast::{JSXAttributeItem, JSXAttributeName, JSXElementName}, AstKind, }; use oxc_diagnostics::{ @@ -16,12 +16,7 @@ use serde::Deserialize; use std::collections::hash_map::HashMap; use std::collections::hash_set::HashSet; -use crate::{ - context::LintContext, - rule::Rule, - utils::{get_element_type, get_jsx_attribute_name}, - AstNode, -}; +use crate::{context::LintContext, rule::Rule, utils::get_jsx_attribute_name, AstNode}; #[derive(Debug, Error, Diagnostic)] enum NoUnknownPropertyDiagnostic { @@ -455,15 +450,17 @@ impl Rule for NoUnknownProperty { let AstKind::JSXOpeningElement(el) = &node.kind() else { return; }; - let Some(el_type) = get_element_type(ctx, el) else { + let JSXElementName::Identifier(ident) = &el.name else { return; }; + let el_type = ident.name.as_str(); + // fbt/fbs nodes are bonkers, let's not go there if !el_type.starts_with(char::is_lowercase) || el_type == "fbt" || el_type == "fbs" { return; } - let is_valid_html_tag = HTML_TAG_CONVENTION.is_match(el_type.as_str()) + let is_valid_html_tag = HTML_TAG_CONVENTION.is_match(el_type) && el.attributes.iter().all(|attr| { let JSXAttributeItem::Attribute(jsx_attr) = attr else { return true; @@ -500,7 +497,7 @@ impl Rule for NoUnknownProperty { }; let name = normalize_attribute_case(actual_name.as_str()); if let Some(tags) = ATTRIBUTE_TAGS_MAP.get(name) { - if !tags.contains(el_type.as_str()) { + if !tags.contains(el_type) { ctx.diagnostic(NoUnknownPropertyDiagnostic::InvalidPropOnTag( span, actual_name.to_string(), diff --git a/crates/oxc_linter/src/snapshots/anchor_has_content.snap b/crates/oxc_linter/src/snapshots/anchor_has_content.snap index c51337155..ce7d6b469 100644 --- a/crates/oxc_linter/src/snapshots/anchor_has_content.snap +++ b/crates/oxc_linter/src/snapshots/anchor_has_content.snap @@ -4,9 +4,9 @@ expression: anchor_has_content --- ⚠ eslint-plugin-jsx-a11y(anchor-has-content): Missing accessible content when using `a` elements. - ╭─[anchor_has_content.tsx:1:2] + ╭─[anchor_has_content.tsx:1:1] 1 │ - · ─ + · ───── ╰──── help: Provide screen reader accessible content when using `a` elements. @@ -15,7 +15,7 @@ expression: anchor_has_content 1 │ · ────────────────────────── ╰──── - help: Remove the `aria-hidden` attribute to allow the anchor element and its content visible to assistive technologies. + help: Provide screen reader accessible content when using `a` elements. ⚠ eslint-plugin-jsx-a11y(anchor-has-content): Missing accessible content when using `a` elements. ╭─[anchor_has_content.tsx:1:1] @@ -25,30 +25,9 @@ expression: anchor_has_content help: Provide screen reader accessible content when using `a` elements. ⚠ eslint-plugin-jsx-a11y(anchor-has-content): Missing accessible content when using `a` elements. - ╭─[anchor_has_content.tsx:1:2] + ╭─[anchor_has_content.tsx:1:1] 1 │ - · ──── - ╰──── - help: Provide screen reader accessible content when using `a` elements. - - ⚠ eslint-plugin-jsx-a11y(anchor-has-content): Missing accessible content when using `a` elements. - ╭─[anchor_has_content.tsx:1:1] - 1 │ - · ──────────────────── - ╰──── - help: Remove the `aria-hidden` attribute to allow the anchor element and its content visible to assistive technologies. - - ⚠ eslint-plugin-jsx-a11y(anchor-has-content): Missing accessible content when using `a` elements. - ╭─[anchor_has_content.tsx:1:1] - 1 │ {null} - · ───────────── - ╰──── - help: Provide screen reader accessible content when using `a` elements. - - ⚠ eslint-plugin-jsx-a11y(anchor-has-content): Missing accessible content when using `a` elements. - ╭─[anchor_has_content.tsx:1:2] - 1 │ - · ─ + · ──────── ╰──── help: Provide screen reader accessible content when using `a` elements. diff --git a/crates/oxc_linter/src/utils/react.rs b/crates/oxc_linter/src/utils/react.rs index 8be9ecde5..7c56329a8 100644 --- a/crates/oxc_linter/src/utils/react.rs +++ b/crates/oxc_linter/src/utils/react.rs @@ -74,9 +74,9 @@ pub fn get_string_literal_prop_value<'a>(item: &'a JSXAttributeItem<'_>) -> Opti } // 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" { +pub fn is_hidden_from_screen_reader(ctx: &LintContext, node: &JSXOpeningElement) -> bool { + if let Some(name) = get_element_type(ctx, node) { + if name.as_str().to_uppercase() == "INPUT" { if let Some(item) = has_jsx_prop_lowercase(node, "type") { let hidden = get_string_literal_prop_value(item); @@ -99,14 +99,14 @@ pub fn is_hidden_from_screen_reader(node: &JSXOpeningElement) -> bool { } // 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 { +pub fn object_has_accessible_child(ctx: &LintContext, 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::Element(el) => !is_hidden_from_screen_reader(ctx, &el.opening_element), JSXChild::ExpressionContainer(JSXExpressionContainer { expression: JSXExpression::Expression(expr), .. - }) => !expr.is_undefined(), + }) => !expr.is_undefined() && !expr.is_null(), _ => false, }) || has_jsx_prop_lowercase(&node.opening_element, "dangerouslySetInnerHTML").is_some() || has_jsx_prop_lowercase(&node.opening_element, "children").is_some() @@ -219,7 +219,8 @@ pub fn get_parent_es6_component<'a, 'b>(ctx: &'b LintContext<'a>) -> Option<&'b }) } -// ref: https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/main/src/util/getElementType.js +/// Resolve element type(name) using jsx-a11y settings +/// ref: https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/main/src/util/getElementType.js pub fn get_element_type(context: &LintContext, element: &JSXOpeningElement) -> Option { let JSXElementName::Identifier(ident) = &element.name else { return None;