fix(linter/no-noninteractive-tabindex): handle numeric tabIndex values on non-interactive elements

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<Self, serde_json::error::Error> {
@@ -218,6 +243,16 @@ fn test() {
         (r"<MyButton tabIndex={0} />", None, Some(settings())),
         (r#"<div role="tabpanel" tabIndex="0" />"#, None, None),
         (r#"<div role={ROLE_BUTTON} onClick={() => {}} tabIndex="0" />;"#, None, None),
+        (
+            r"<div tabIndex={someVar} />",
+            Some(serde_json::json!([{ "allowExpressionValues": true }])),
+            None,
+        ),
+        (
+            r"<div tabIndex={-1} />",
+            Some(serde_json::json!([{ "allowExpressionValues": false }])),
+            None,
+        ),
         (
             r#"<div role={BUTTON} onClick={() => {}} tabIndex="0" />;"#,
             Some(serde_json::json!([{ "allowExpressionValues": true }])),
@@ -242,10 +277,10 @@ fn test() {
 
     let fail = vec![
         (r#"<div tabIndex="0" />"#, None, None),
-        // TODO: Fix the rule for these tests.
-        // (r#"<div role="article" tabIndex="0" />"#, None, None),
-        // (r"<article tabIndex={0} />", None, None),
-        // (r"<Article tabIndex={0} />", None, Some(settings())),
+        (r"<div tabIndex={0} />", None, None),
+        (r#"<div role="article" tabIndex="0" />"#, None, None),
+        (r"<article tabIndex={0} />", None, None),
+        (r"<Article tabIndex={0} />", None, Some(settings())),
         (r#"<article tabIndex="0" />"#, None, None),
         (
             r#"<div role="tabpanel" tabIndex="0" />"#,
@@ -267,6 +302,11 @@ fn test() {
             Some(serde_json::json!([{ "allowExpressionValues": false }])),
             None,
         ),
+        (
+            r"<div tabIndex={someVar} />",
+            Some(serde_json::json!([{ "allowExpressionValues": false }])),
+            None,
+        ),
     ];
 
     Tester::new(NoNoninteractiveTabindex::NAME, NoNoninteractiveTabindex::PLUGIN, pass, fail)

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 │ <div tabIndex={0} />
+   ·      ────────────
+   ╰────
+  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 │ <div role="article" tabIndex="0" />
+   ·                     ────────────
+   ╰────
+  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 │ <article tabIndex={0} />
+   ·          ────────────
+   ╰────
+  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 │ <Article tabIndex={0} />
+   ·          ────────────
+   ╰────
+  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 │ <article tabIndex="0" />
@@ -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 │ <div tabIndex={someVar} />
+   ·      ──────────────────
+   ╰────
+  help: The `tabIndex` attribute should be removed.

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<f64, ()> {
                 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(()),