From 9519380e09726cb4aa0f24a4aeb54a8792945cc3 Mon Sep 17 00:00:00 2001 From: camc314 <18101008+camc314@users.noreply.github.com> Date: Thu, 19 Mar 2026 23:44:07 +0000 Subject: [PATCH] fix(linter/no-noninteractive-tabindex): handle numeric tabIndex values on non-interactive elements (#20538) fixes #20471 --- .../jsx_a11y/no_noninteractive_tabindex.rs | 78 ++++++++++++++----- .../jsx_a11y_no_noninteractive_tabindex.snap | 35 +++++++++ crates/oxc_linter/src/utils/react.rs | 12 +++ 3 files changed, 106 insertions(+), 19 deletions(-) diff --git a/crates/oxc_linter/src/rules/jsx_a11y/no_noninteractive_tabindex.rs b/crates/oxc_linter/src/rules/jsx_a11y/no_noninteractive_tabindex.rs index e7e759e17b74d..61ab2e19aade9 100644 --- a/crates/oxc_linter/src/rules/jsx_a11y/no_noninteractive_tabindex.rs +++ b/crates/oxc_linter/src/rules/jsx_a11y/no_noninteractive_tabindex.rs @@ -11,8 +11,9 @@ use serde::Deserialize; use crate::{ AstNode, context::LintContext, + globals::HTML_TAG, rule::Rule, - utils::{get_element_type, has_jsx_prop_ignore_case, is_interactive_element}, + utils::{get_element_type, has_jsx_prop_ignore_case, is_interactive_element, parse_jsx_value}, }; fn no_noninteractive_tabindex_diagnostic(span: Span) -> OxcDiagnostic { @@ -124,41 +125,65 @@ impl Rule for NoNoninteractiveTabindex { return; }; - let Some(JSXAttributeValue::StringLiteral(tabindex)) = &tabindex_attr.value else { + let Some(tabindex_value) = &tabindex_attr.value else { return; }; - if tabindex.value == "-1" { + let Ok(tabindex) = parse_jsx_value(tabindex_value) else { + if matches!(tabindex_value, JSXAttributeValue::ExpressionContainer(_)) + && !self.0.allow_expression_values + { + ctx.diagnostic(no_noninteractive_tabindex_diagnostic(tabindex_attr.span)); + } + return; + }; + + if tabindex < 0.0 || tabindex.fract() != 0.0 { return; } let component = &get_element_type(ctx, jsx_el); - if is_interactive_element(component, jsx_el) { + if self.0.tags.iter().any(|tag| tag == component.as_ref()) { return; } - let Some(JSXAttributeItem::Attribute(role_attr)) = has_jsx_prop_ignore_case(jsx_el, "role") - else { - // if the component is not an interactive element and has no role, the tabindex is invalid. - ctx.diagnostic(no_noninteractive_tabindex_diagnostic(tabindex_attr.span)); + if !HTML_TAG.contains(component.as_ref()) { return; - }; + } - if self.0.allow_expression_values { + if is_interactive_element(component, jsx_el) { return; } - let Some(JSXAttributeValue::StringLiteral(role)) = &role_attr.value else { + let Some(JSXAttributeItem::Attribute(role_attr)) = has_jsx_prop_ignore_case(jsx_el, "role") + else { + // if the component is not an interactive element and has no role, the tabindex is invalid. ctx.diagnostic(no_noninteractive_tabindex_diagnostic(tabindex_attr.span)); return; }; - if !INTERACTIVE_HTML_ROLES.contains(&role.value.as_str()) - && !self.0.roles.contains(&CompactStr::new(role.value.as_str())) - { - ctx.diagnostic(no_noninteractive_tabindex_diagnostic(tabindex_attr.span)); + if let Some(role) = role_attr.value.as_ref() { + match role { + JSXAttributeValue::StringLiteral(role) => { + let is_interactive_role = + role.value.split_whitespace().next().is_some_and(|role| { + INTERACTIVE_HTML_ROLES.contains(&role) + || self.0.roles.iter().any(|allowed_role| allowed_role == role) + }); + + if is_interactive_role { + return; + } + } + JSXAttributeValue::ExpressionContainer(_) if self.0.allow_expression_values => { + return; + } + _ => {} + } } + + ctx.diagnostic(no_noninteractive_tabindex_diagnostic(tabindex_attr.span)); } fn from_configuration(value: serde_json::Value) -> Result { @@ -218,6 +243,16 @@ fn test() { (r"", None, Some(settings())), (r#"
"#, None, None), (r#"
{}} tabIndex="0" />;"#, None, None), + ( + r"
", + Some(serde_json::json!([{ "allowExpressionValues": true }])), + None, + ), + ( + r"
", + Some(serde_json::json!([{ "allowExpressionValues": false }])), + None, + ), ( r#"
{}} tabIndex="0" />;"#, Some(serde_json::json!([{ "allowExpressionValues": true }])), @@ -242,10 +277,10 @@ fn test() { let fail = vec![ (r#"
"#, None, None), - // TODO: Fix the rule for these tests. - // (r#"
"#, None, None), - // (r"
", None, None), - // (r"
", None, Some(settings())), + (r"
", None, None), + (r#"
"#, None, None), + (r"
", None, None), + (r"
", None, Some(settings())), (r#"
"#, None, None), ( r#"
"#, @@ -267,6 +302,11 @@ fn test() { Some(serde_json::json!([{ "allowExpressionValues": false }])), None, ), + ( + r"
", + Some(serde_json::json!([{ "allowExpressionValues": false }])), + None, + ), ]; Tester::new(NoNoninteractiveTabindex::NAME, NoNoninteractiveTabindex::PLUGIN, pass, fail) diff --git a/crates/oxc_linter/src/snapshots/jsx_a11y_no_noninteractive_tabindex.snap b/crates/oxc_linter/src/snapshots/jsx_a11y_no_noninteractive_tabindex.snap index 548388d47db11..d08d111c0e25c 100644 --- a/crates/oxc_linter/src/snapshots/jsx_a11y_no_noninteractive_tabindex.snap +++ b/crates/oxc_linter/src/snapshots/jsx_a11y_no_noninteractive_tabindex.snap @@ -9,6 +9,34 @@ source: crates/oxc_linter/src/tester.rs ╰──── help: The `tabIndex` attribute should be removed. + ⚠ eslint-plugin-jsx-a11y(no-noninteractive-tabindex): `tabIndex` should only be declared on interactive elements. + ╭─[no_noninteractive_tabindex.tsx:1:6] + 1 │
+ · ──────────── + ╰──── + help: The `tabIndex` attribute should be removed. + + ⚠ eslint-plugin-jsx-a11y(no-noninteractive-tabindex): `tabIndex` should only be declared on interactive elements. + ╭─[no_noninteractive_tabindex.tsx:1:21] + 1 │
+ · ──────────── + ╰──── + help: The `tabIndex` attribute should be removed. + + ⚠ eslint-plugin-jsx-a11y(no-noninteractive-tabindex): `tabIndex` should only be declared on interactive elements. + ╭─[no_noninteractive_tabindex.tsx:1:10] + 1 │
+ · ──────────── + ╰──── + help: The `tabIndex` attribute should be removed. + + ⚠ eslint-plugin-jsx-a11y(no-noninteractive-tabindex): `tabIndex` should only be declared on interactive elements. + ╭─[no_noninteractive_tabindex.tsx:1:10] + 1 │
+ · ──────────── + ╰──── + help: The `tabIndex` attribute should be removed. + ⚠ eslint-plugin-jsx-a11y(no-noninteractive-tabindex): `tabIndex` should only be declared on interactive elements. ╭─[no_noninteractive_tabindex.tsx:1:10] 1 │
@@ -43,3 +71,10 @@ source: crates/oxc_linter/src/tester.rs · ──────────── ╰──── help: The `tabIndex` attribute should be removed. + + ⚠ eslint-plugin-jsx-a11y(no-noninteractive-tabindex): `tabIndex` should only be declared on interactive elements. + ╭─[no_noninteractive_tabindex.tsx:1:6] + 1 │
+ · ────────────────── + ╰──── + help: The `tabIndex` attribute should be removed. diff --git a/crates/oxc_linter/src/utils/react.rs b/crates/oxc_linter/src/utils/react.rs index 8e7c263b03643..f34503874b5f8 100644 --- a/crates/oxc_linter/src/utils/react.rs +++ b/crates/oxc_linter/src/utils/react.rs @@ -12,6 +12,7 @@ use oxc_ast::{ use oxc_ast_visit::{Visit, walk}; use oxc_ecmascript::{ToBoolean, WithoutGlobalReferenceInformation}; use oxc_semantic::AstNode; +use oxc_syntax::operator::UnaryOperator; use oxc_syntax::scope::ScopeFlags; use crate::globals::HTML_TAG; @@ -474,6 +475,17 @@ pub fn parse_jsx_value(value: &JSXAttributeValue) -> Result { tmpl.quasis.first().unwrap().value.raw.parse().or(Err(())) } JSXExpression::NumericLiteral(num) => Ok(num.value), + JSXExpression::UnaryExpression(expr) => { + let Expression::NumericLiteral(num) = &expr.argument else { + return Err(()); + }; + + match expr.operator { + UnaryOperator::UnaryPlus => Ok(num.value), + UnaryOperator::UnaryNegation => Ok(-num.value), + _ => Err(()), + } + } _ => Err(()), }, _ => Err(()),