Skip to content
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
9 changes: 9 additions & 0 deletions .changeset/html-no-noninteractive-tab-index.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@biomejs/biome": minor
---

Added the [`noNoninteractiveTabindex`](https://biomejs.dev/linter/rules/no-noninteractive-tabindex/) lint rule for HTML. This rule enforces that `tabindex` is not used on non-interactive elements, as it can cause usability issues for keyboard users.

```html
<div tabindex="0">Invalid: non-interactive element</div>`
```
3 changes: 3 additions & 0 deletions crates/biome_html_analyze/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ mod suppression_action;
mod utils;

pub use crate::registry::visit_registry;
pub use crate::services::aria::{Aria, AriaServices};
pub use crate::services::module_graph::{HtmlModuleGraph, HtmlModuleGraphService};
use crate::suppression_action::HtmlSuppressionAction;

Expand All @@ -24,6 +25,7 @@ use biome_analyze::{
LanguageRoot, MatchQueryParams, MetadataRegistry, RuleAction, RuleRegistry,
to_analyzer_suppressions,
};
use biome_aria::AriaRoles;
use biome_deserialize::TextRange;
use biome_diagnostics::Error;
use biome_html_syntax::{HtmlFileSource, HtmlLanguage};
Expand Down Expand Up @@ -124,6 +126,7 @@ where
}

services.insert_service(source_type);
services.insert_service(Arc::new(AriaRoles));
if let Some(module_graph) = html_services.module_graph {
services.insert_service(module_graph);
}
Expand Down
146 changes: 146 additions & 0 deletions crates/biome_html_analyze/src/lint/a11y/no_noninteractive_tabindex.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
use biome_analyze::context::RuleContext;
use biome_analyze::{FixKind, Rule, RuleDiagnostic, RuleSource, declare_lint_rule};
use biome_aria_metadata::AriaRole;
use biome_console::markup;
use biome_diagnostics::Severity;
use biome_html_syntax::{HtmlAttribute, element_ext::AnyHtmlTagElement};
use biome_rowan::{AstNode, BatchMutationExt, TextRange, TokenText};
use biome_rule_options::no_noninteractive_tabindex::NoNoninteractiveTabindexOptions;

use crate::{Aria, HtmlRuleAction};

declare_lint_rule! {
/// Enforce that `tabindex` is not assigned to non-interactive HTML elements.
///
/// When using the tab key to navigate a webpage, limit it to interactive elements.
/// You don't need to add tabindex to items in an unordered list as assistive technology can navigate through the HTML.
/// Keep the tab ring small, which is the order of elements when tabbing, for a more efficient and accessible browsing experience.
///
/// ## Examples
///
/// ### Invalid
///
/// ```html,expect_diagnostic
/// <div tabindex="0"></div>
/// ```
///
/// ```html,expect_diagnostic
/// <div role="article" tabindex="0"></div>
/// ```
///
/// ```html,expect_diagnostic
/// <article tabindex="0"></article>
/// ```
///
/// ### Valid
///
/// ```html
/// <div></div>
/// ```
///
/// ```html
/// <button tabindex="0"></button>
/// ```
///
/// ```html
/// <article tabindex="-1"></article>
/// ```
///
pub NoNoninteractiveTabindex {
version: "next",
name: "noNoninteractiveTabindex",
language: "html",
sources: &[RuleSource::EslintJsxA11y("no-noninteractive-tabindex").same()],
recommended: true,
severity: Severity::Error,
fix_kind: FixKind::Unsafe,
}
}

pub struct RuleState {
attribute_range: TextRange,
attribute: HtmlAttribute,
element_name: TokenText,
}

impl Rule for NoNoninteractiveTabindex {
type Query = Aria<AnyHtmlTagElement>;
type State = RuleState;
type Signals = Option<Self::State>;
type Options = NoNoninteractiveTabindexOptions;

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let node = ctx.query();

if !ctx.aria_roles().is_not_interactive_element(node) {
return None;
}

let tabindex_attribute = node.find_attribute_by_name("tabindex")?;

let is_negative = tabindex_attribute
.initializer()
.and_then(|init| init.value().ok())
.and_then(|value| value.string_value())
.is_some_and(|value| is_negative_tabindex(&value));

if is_negative {
return None;
}

let role_attribute = node.find_attribute_by_name("role");
if let Some(role_attr) = role_attribute {
let role_value = role_attr.initializer()?.value().ok()?.string_value()?;
let role = AriaRole::from_roles(role_value.trim());

if let Some(aria_role) = role
&& aria_role.is_interactive()
{
return None;
}
}
Comment on lines +91 to +101
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n crates/biome_html_analyze/src/lint/a11y/no_noninteractive_tabindex.rs | sed -n '75,110p'

Repository: biomejs/biome

Length of output: 1387


🏁 Script executed:

rg "fn from_roles" --type rs -A 2

Repository: biomejs/biome

Length of output: 83


🏁 Script executed:

fd "Cargo.toml" | head -1 | xargs grep -A 10 "^\[package\]" | grep -E "(rust-version|edition)"

Repository: biomejs/biome

Length of output: 39


🏁 Script executed:

rg "fn from_roles" --type rust -A 3

Repository: biomejs/biome

Length of output: 363


🏁 Script executed:

rg "struct AriaRole|enum AriaRole" --type rust -A 5

Repository: biomejs/biome

Length of output: 1835


🏁 Script executed:

find . -name "Cargo.toml" -path "*/biome_html_analyze/*" | head -1 | xargs cat | grep -E "rust-version|edition"

Repository: biomejs/biome

Length of output: 84


🏁 Script executed:

find . -name "Cargo.toml" -path "./Cargo.toml" | xargs cat | grep -A 5 "^\[workspace\]"

Repository: biomejs/biome

Length of output: 300


🏁 Script executed:

grep -A 20 "^\[workspace\]" Cargo.toml | grep -E "edition|rust-version"

Repository: biomejs/biome

Length of output: 76


🏁 Script executed:

rg "is_some_and" --type rust | head -5

Repository: biomejs/biome

Length of output: 598


Avoid exiting early on malformed role attributes.

At line 91, the ? chain exits the function when a role attribute exists but lacks a usable value, suppressing the violation report. Since malformed roles aren't interactive roles, the lint should still fire. Use and_then to keep role as an Option, then only skip when it's both present and interactive.

Suggested fix
-        let role_attribute = node.find_attribute_by_name("role");
-        if let Some(role_attr) = role_attribute {
-            let role_value = role_attr.initializer()?.value().ok()?.string_value()?;
-            let role = AriaRole::from_roles(role_value.trim());
-
-            if let Some(aria_role) = role {
-                if aria_role.is_interactive() {
-                    return None;
-                }
-            }
-        }
+        if let Some(role_attr) = node.find_attribute_by_name("role") {
+            let role = role_attr
+                .initializer()
+                .and_then(|init| init.value().ok())
+                .and_then(|value| value.string_value())
+                .and_then(|value| AriaRole::from_roles(value.trim()));
+
+            if role.is_some_and(|aria_role| aria_role.is_interactive()) {
+                return None;
+            }
+        }

Remember to run just gen-rules before opening the PR.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let role_attribute = node.find_attribute_by_name("role");
if let Some(role_attr) = role_attribute {
let role_value = role_attr.initializer()?.value().ok()?.string_value()?;
let role = AriaRole::from_roles(role_value.trim());
if let Some(aria_role) = role {
if aria_role.is_interactive() {
return None;
}
}
}
if let Some(role_attr) = node.find_attribute_by_name("role") {
let role = role_attr
.initializer()
.and_then(|init| init.value().ok())
.and_then(|value| value.string_value())
.and_then(|value| AriaRole::from_roles(value.trim()));
if role.is_some_and(|aria_role| aria_role.is_interactive()) {
return None;
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/biome_html_analyze/src/lint/a11y/no_noninteractive_tabindex.rs` around
lines 89 - 99, The current early-return `?` chain when reading a `role`
attribute (in the block using role_attribute,
role_attr.initializer()?.value().ok()?.string_value()?) causes the lint to exit
on malformed `role` values; change this to use option-preserving combinators
(e.g., replace the `?` chain with and_then/map calls) so you get an
Option<String> from role_attr.initializer()/value()/string_value() instead of
returning, then call AriaRole::from_roles on that Option and only return None
when an aria_role is present and aria_role.is_interactive(); otherwise allow the
lint to continue reporting. Ensure you update the block around role_attribute,
role_attr, AriaRole::from_roles, and aria_role.is_interactive(), and run `just
gen-rules` before the PR.


let element_name = node.tag_name()?;
let attribute_range = tabindex_attribute.range();

Some(RuleState {
attribute_range,
attribute: tabindex_attribute,
element_name,
})
}

fn diagnostic(_ctx: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
let element_name = state.element_name.text();
Some(
RuleDiagnostic::new(
rule_category!(),
state.attribute_range,
markup! {
"The HTML element "<Emphasis>{element_name}</Emphasis>" is non-interactive. Do not use "<Emphasis>"tabindex"</Emphasis>"."
},
)
.note(markup! {
"Adding non-interactive elements to the keyboard navigation flow can confuse users."
}),
)
}

fn action(ctx: &RuleContext<Self>, state: &Self::State) -> Option<HtmlRuleAction> {
let mut mutation = ctx.root().begin();
mutation.remove_node(state.attribute.clone());

Some(HtmlRuleAction::new(
ctx.metadata().action_category(ctx.category(), ctx.group()),
ctx.metadata().applicability(),
markup! { "Remove the "<Emphasis>"tabindex"</Emphasis>" attribute." }.to_owned(),
mutation,
))
}
}

/// Returns `true` only for valid integers strictly less than 0.
fn is_negative_tabindex(number_like_string: &str) -> bool {
matches!(number_like_string.trim().parse::<i64>(), Ok(n) if n < 0)
}

35 changes: 4 additions & 31 deletions crates/biome_html_analyze/src/lint/a11y/no_svg_without_title.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
use biome_analyze::{Ast, Rule, RuleDiagnostic, context::RuleContext, declare_lint_rule};
use biome_analyze::{Rule, RuleDiagnostic, context::RuleContext, declare_lint_rule};
use biome_console::markup;
use biome_diagnostics::Severity;
use biome_html_syntax::{AnyHtmlElement, HtmlAttribute, HtmlElementList, HtmlFileSource};
use biome_rowan::AstNode;
use biome_rule_options::no_svg_without_title::NoSvgWithoutTitleOptions;
use biome_string_case::StrLikeExtension;

use crate::{a11y::is_aria_hidden_true, utils::is_html_tag};

const NAME_REQUIRED_ROLES: &[&str] = &["img", "image", "graphics-document", "graphics-symbol"];
use crate::Aria;

declare_lint_rule! {
/// Enforces the usage of the `title` element for the `svg` element.
Expand Down Expand Up @@ -121,7 +119,7 @@ declare_lint_rule! {
}

impl Rule for NoSvgWithoutTitle {
type Query = Ast<AnyHtmlElement>;
type Query = Aria<AnyHtmlElement>;
type State = ();
type Signals = Option<Self::State>;
type Options = NoSvgWithoutTitleOptions;
Expand All @@ -148,32 +146,7 @@ impl Rule for NoSvgWithoutTitle {
}
}

// TODO: use `aria_roles.has_name_required_image_role` of aria crate
// Checks if a `svg` element has role='img' and title/aria-label/aria-labelledby attribute
let Some(role_attribute) = node.find_attribute_by_name("role") else {
return Some(());
};

let role_attribute_value = role_attribute.initializer()?.value().ok()?;
let Some(role_attribute_text) = role_attribute_value
.as_html_string()?
.inner_string_text()
.ok()
else {
return Some(());
};

let role_text = role_attribute_text.to_ascii_lowercase_cow();
if role_text.trim().is_empty() {
return Some(());
}

// Check if any of the space-separated roles are valid
let has_name_required_role = role_text
.split_whitespace()
.any(|role| NAME_REQUIRED_ROLES.contains(&role));

if has_name_required_role {
if ctx.aria_roles().has_name_required_image_role(node) {
let aria_label = node.find_attribute_by_name("aria-label");
let aria_labelledby = node.find_attribute_by_name("aria-labelledby");
let is_valid_a11y_attribute = aria_label.is_some()
Expand Down
66 changes: 66 additions & 0 deletions crates/biome_html_analyze/src/services/aria.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
use biome_analyze::{
AddVisitor, FromServices, Phase, Phases, QueryKey, Queryable, RuleKey, RuleMetadata,
ServiceBag, ServicesDiagnostic, SyntaxVisitor,
};
use biome_aria::AriaRoles;
use biome_html_syntax::{HtmlLanguage, HtmlRoot, HtmlSyntaxNode};
use biome_rowan::AstNode;
use std::sync::Arc;

#[derive(Debug, Clone)]
pub struct AriaServices {
pub(crate) roles: Arc<AriaRoles>,
}

impl AriaServices {
pub fn aria_roles(&self) -> &AriaRoles {
&self.roles
}
}

impl FromServices for AriaServices {
fn from_services(
rule_key: &RuleKey,
_rule_metadata: &RuleMetadata,
services: &ServiceBag,
) -> Result<Self, ServicesDiagnostic> {
let roles: &Arc<AriaRoles> = services
.get_service()
.ok_or_else(|| ServicesDiagnostic::new(rule_key.rule_name(), &["AriaRoles"]))?;
Ok(Self {
roles: roles.clone(),
})
}
}

impl Phase for AriaServices {
fn phase() -> Phases {
Phases::Syntax
}
}

#[derive(Clone)]
pub struct Aria<N>(pub N);

impl<N> Queryable for Aria<N>
where
N: AstNode<Language = HtmlLanguage> + 'static,
{
type Input = HtmlSyntaxNode;
type Output = N;

type Language = HtmlLanguage;
type Services = AriaServices;

fn build_visitor(analyzer: &mut impl AddVisitor<HtmlLanguage>, _: &HtmlRoot) {
analyzer.add_visitor(Phases::Syntax, SyntaxVisitor::default);
}

fn key() -> QueryKey<Self::Language> {
QueryKey::Syntax(N::KIND_SET)
}

fn unwrap_match(_: &ServiceBag, node: &Self::Input) -> Self::Output {
N::unwrap_cast(node.clone())
}
}
1 change: 1 addition & 0 deletions crates/biome_html_analyze/src/services/mod.rs
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
pub mod aria;
pub mod module_graph;
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<!-- should generate diagnostics -->
<div tabindex="0"></div>
<div role="article" tabindex="0"></div>
<article tabindex="0"></article>
<span tabindex="1"></span>
Loading
Loading