Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

feat(rome_js_analyze): noNoninteractiveTabindex #4312

Merged
merged 4 commits into from
Mar 31, 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#### New rules
- [`noConfusingArrow`](https://docs.rome.tools/lint/rules/noConfusingArrow/)
- [`noRedundantRoles`](https://docs.rome.tools/lint/rules/noRedundantRoles/)
- [`noNoninteractiveTabindex`](https://docs.rome.tools/lint/rules/noNoninteractiveTabindex/)
### Parser
### VSCode
### JavaScript APIs
Expand Down
3 changes: 3 additions & 0 deletions crates/rome_aria/src/roles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -747,6 +747,7 @@ impl<'a> AriaRoles {
"table",
"term",
"textbox",
"generic",
];

/// It returns the metadata of a role, if it exits.
Expand Down Expand Up @@ -820,6 +821,7 @@ impl<'a> AriaRoles {
"region" => &RegionRole as &dyn AriaRoleDefinition,
"presentation" => &PresentationRole as &dyn AriaRoleDefinition,
"document" => &DocumentRole as &dyn AriaRoleDefinition,
"generic" => &GenericRole as &dyn AriaRoleDefinition,
_ => return None,
};
Some(result)
Expand Down Expand Up @@ -1003,6 +1005,7 @@ impl<'a> AriaRoles {
"table" => &TableRole as &dyn AriaRoleDefinitionWithConcepts,
"term" => &TermRole as &dyn AriaRoleDefinitionWithConcepts,
"textbox" => &TextboxRole as &dyn AriaRoleDefinitionWithConcepts,
"generic" => &GenericRole as &dyn AriaRoleDefinitionWithConcepts,
_ => return false,
};
if let Some(mut concepts) = role.concepts_by_element_name(element_name) {
Expand Down
1 change: 1 addition & 0 deletions crates/rome_diagnostics_categories/src/categories.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ define_categories! {
"lint/nursery/noParameterAssign": "https://docs.rome.tools/lint/rules/noParameterAssign",
"lint/nursery/noNamespace": "https://docs.rome.tools/lint/rules/noNamespace",
"lint/nursery/noConfusingArrow": "https://docs.rome.tools/lint/rules/noConfusingArrow",
"lint/nursery/noNoninteractiveTabindex": "https://docs.rome.tools/lint/rules/noNoninteractiveTabindex",
// Insert new nursery rule here
"lint/nursery/noRedeclare": "https://docs.rome.tools/lint/rules/noRedeclare",
"lint/nursery/useNamespaceKeyword": "https://docs.rome.tools/lint/rules/useNamespaceKeyword",
Expand Down
3 changes: 2 additions & 1 deletion crates/rome_js_analyze/src/aria_analyzers/nursery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@

use rome_analyze::declare_group;
mod no_noninteractive_element_to_interactive_role;
mod no_noninteractive_tabindex;
mod no_redundant_roles;
mod use_aria_prop_types;
mod use_aria_props_for_role;
mod use_valid_aria_props;
mod use_valid_lang;
declare_group! { pub (crate) Nursery { name : "nursery" , rules : [self :: no_noninteractive_element_to_interactive_role :: NoNoninteractiveElementToInteractiveRole , self :: no_redundant_roles :: NoRedundantRoles , self :: use_aria_prop_types :: UseAriaPropTypes , self :: use_aria_props_for_role :: UseAriaPropsForRole , self :: use_valid_aria_props :: UseValidAriaProps , self :: use_valid_lang :: UseValidLang ,] } }
declare_group! { pub (crate) Nursery { name : "nursery" , rules : [self :: no_noninteractive_element_to_interactive_role :: NoNoninteractiveElementToInteractiveRole , self :: no_noninteractive_tabindex :: NoNoninteractiveTabindex , self :: no_redundant_roles :: NoRedundantRoles , self :: use_aria_prop_types :: UseAriaPropTypes , self :: use_aria_props_for_role :: UseAriaPropsForRole , self :: use_valid_aria_props :: UseValidAriaProps , self :: use_valid_lang :: UseValidLang ,] } }
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,13 @@ impl Rule for NoNoninteractiveElementToInteractiveRole {
if aria_roles.is_not_interactive_element(element_name.text_trimmed())
&& aria_roles.is_role_interactive(role_attribute_value.text())
{
// <div> and <span> are considered neither interactive nor non-interactive, depending on the presence or absence of the role attribute.
// We don't report <div> and <span> here, because we cannot determine whether they are interactive or non-interactive.
let role_sensitive_elements = ["div", "span"];
if role_sensitive_elements.contains(&element_name.text_trimmed()) {
return None;
}

return Some(RuleState {
attribute_range: role_attribute.range(),
element_name: element_name.text_trimmed().to_string(),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,217 @@
use crate::aria_services::Aria;
use rome_analyze::{context::RuleContext, declare_rule, Rule, RuleDiagnostic};
use rome_aria::AriaRoles;
use rome_console::markup;
use rome_js_syntax::{
jsx_ext::AnyJsxElement, AnyJsExpression, AnyJsLiteralExpression, AnyJsxAttributeValue,
JsNumberLiteralExpression, JsStringLiteralExpression, JsUnaryExpression, TextRange,
};
use rome_rowan::{declare_node_union, AstNode, AstNodeList};

declare_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.
///
/// ESLint (eslint-plugin-jsx-a11y) Equivalent: [no-noninteractive-tabindex](https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/main/docs/rules/no-noninteractive-tabindex.md)
///
/// ## Examples
///
/// ### Invalid
///
/// ```jsx,expect_diagnostic
/// <div tabIndex="0" />
/// ```
///
/// ```jsx,expect_diagnostic
/// <div role="article" tabIndex="0" />
/// ```
///
/// ```jsx,expect_diagnostic
/// <article tabIndex="0" />
/// ```
///
/// ## Valid
///
/// ```jsx
/// <div />
/// ```
///
/// ```jsx
/// <MyButton tabIndex={0} />
/// ```
///
/// ```jsx
/// <article tabIndex="-1" />
/// ```
///
pub(crate) NoNoninteractiveTabindex {
version: "next",
name: "noNoninteractiveTabindex",
recommended: false,
}
}

declare_node_union! {
/// Subset of expressions supported by this rule.
///
/// ## Examples
///
/// - `JsStringLiteralExpression` &mdash; `"5"`
/// - `JsNumberLiteralExpression` &mdash; `5`
/// - `JsUnaryExpression` &mdash; `+5` | `-5`
///
pub(crate) AnyNumberLikeExpression = JsStringLiteralExpression | JsNumberLiteralExpression | JsUnaryExpression
}

impl AnyNumberLikeExpression {
/// Returns the value of a number-like expression; it returns the expression
/// text for literal expressions. However, for unary expressions, it only
/// returns the value for signed numeric expressions.
pub(crate) fn value(&self) -> Option<String> {
match self {
AnyNumberLikeExpression::JsStringLiteralExpression(string_literal) => {
return Some(string_literal.inner_string_text().ok()?.to_string());
}
AnyNumberLikeExpression::JsNumberLiteralExpression(number_literal) => {
return Some(number_literal.value_token().ok()?.to_string());
}
AnyNumberLikeExpression::JsUnaryExpression(unary_expression) => {
if unary_expression.is_signed_numeric_literal().ok()? {
return Some(unary_expression.text());
}
}
}
None
}
}

pub(crate) struct RuleState {
attribute_range: TextRange,
element_name: String,
}

impl Rule for NoNoninteractiveTabindex {
type Query = Aria<AnyJsxElement>;
type State = RuleState;
type Signals = Option<Self::State>;
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let node = ctx.query();
if !node.is_element() {
return None;
}

let element_name = node.name().ok()?.as_jsx_name()?.value_token().ok()?;
let aria_roles = ctx.aria_roles();

if aria_roles.is_not_interactive_element(element_name.text_trimmed()) {
let tabindex_attribute = node.find_attribute_by_name("tabIndex")?;
let tabindex_attribute_value = tabindex_attribute.initializer()?.value().ok()?;
if attribute_has_negative_tabindex(&tabindex_attribute_value)? {
return None;
}

let role_attribute = node.find_attribute_by_name("role");
let Some(role_attribute) = role_attribute else {
return Some(RuleState {
attribute_range: tabindex_attribute.range(),
element_name: element_name.text_trimmed().to_string(),
})
};

let role_attribute_value = role_attribute.initializer()?.value().ok()?;
if attribute_has_interactive_role(&role_attribute_value, aria_roles)? {
return None;
}

return Some(RuleState {
attribute_range: tabindex_attribute.range(),
element_name: element_name.text_trimmed().to_string(),
});
}
None
}

fn diagnostic(_: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
Some(
RuleDiagnostic::new(
rule_category!(),
state.attribute_range,
markup! {
"The HTML element "<Emphasis>{{&state.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."
}),
)
}
}

/// Verifies if number string is an integer less than 0.
/// Non-integer numbers are considered valid.
fn is_negative_tabindex(number_like_string: &str) -> bool {
let number_string_result = number_like_string.trim().parse::<i32>();
match number_string_result {
Ok(number) => number < 0,
Err(_) => true,
}
}

/// Checks if the given tabindex attribute value has negative integer or not.
fn attribute_has_negative_tabindex(
tabindex_attribute_value: &AnyJsxAttributeValue,
) -> Option<bool> {
match tabindex_attribute_value {
AnyJsxAttributeValue::JsxString(jsx_string) => {
let value = jsx_string.inner_string_text().ok()?.to_string();
Some(is_negative_tabindex(&value))
}
AnyJsxAttributeValue::JsxExpressionAttributeValue(value) => {
let expression = value.expression().ok()?;
let expression_value =
AnyNumberLikeExpression::cast_ref(expression.syntax())?.value()?;
Some(is_negative_tabindex(&expression_value))
}
_ => None,
}
}

/// Checks if the given role attribute value is interactive or not based on ARIA roles.
fn attribute_has_interactive_role(
role_attribute_value: &AnyJsxAttributeValue,
aria_roles: &AriaRoles,
) -> Option<bool> {
let role_attribute_value = match role_attribute_value {
AnyJsxAttributeValue::JsxString(string) => string.inner_string_text().ok(),
AnyJsxAttributeValue::JsxExpressionAttributeValue(expression) => {
match expression.expression().ok()? {
AnyJsExpression::AnyJsLiteralExpression(
AnyJsLiteralExpression::JsStringLiteralExpression(string),
) => string.inner_string_text().ok(),
AnyJsExpression::JsTemplateExpression(template) => {
if template.elements().len() == 1 {
template
.elements()
.iter()
.next()?
.as_js_template_chunk_element()?
.template_chunk_token()
.ok()
.map(|t| t.token_text_trimmed())
} else {
None
}
}
_ => None,
}
}
_ => None,
}?;
Some(aria_roles.is_role_interactive(role_attribute_value.text()))
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<>
<div tabIndex="0"></div>
<div role="article" tabIndex="0"></div>
<article tabIndex={0} />
</>;
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
---
source: crates/rome_js_analyze/tests/spec_tests.rs
expression: invalid.jsx
---
# Input
```js
<>
<div tabIndex="0"></div>
<div role="article" tabIndex="0"></div>
<article tabIndex={0} />
</>;

```

# Diagnostics
```
invalid.jsx:2:7 lint/nursery/noNoninteractiveTabindex ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! The HTML element div is non-interactive. Do not use tabIndex.

1 │ <>
> 2 │ <div tabIndex="0"></div>
│ ^^^^^^^^^^^^
3 │ <div role="article" tabIndex="0"></div>
4 │ <article tabIndex={0} />

i Adding non-interactive elements to the keyboard navigation flow can confuse users.


```

```
invalid.jsx:3:22 lint/nursery/noNoninteractiveTabindex ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! The HTML element div is non-interactive. Do not use tabIndex.

1 │ <>
2 │ <div tabIndex="0"></div>
> 3 │ <div role="article" tabIndex="0"></div>
│ ^^^^^^^^^^^^
4 │ <article tabIndex={0} />
5 │ </>;

i Adding non-interactive elements to the keyboard navigation flow can confuse users.


```

```
invalid.jsx:4:11 lint/nursery/noNoninteractiveTabindex ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! The HTML element article is non-interactive. Do not use tabIndex.

2 │ <div tabIndex="0"></div>
3 │ <div role="article" tabIndex="0"></div>
> 4 │ <article tabIndex={0} />
│ ^^^^^^^^^^^^
5 │ </>;
6 │

i Adding non-interactive elements to the keyboard navigation flow can confuse users.


```


Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<>
<button tabIndex="0"></button>
<span role="button" tabIndex="0"></span>
<span role="article" tabIndex="-1"></span>
<MyButton tabIndex={0} />
<article tabIndex="-1"></article>
<div tabIndex="-1"></div>
<article tabIndex={-1}></article>
<div tabIndex={-1}></div>
<div></div>
<button tabIndex="-1"></button>
</>;
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
source: crates/rome_js_analyze/tests/spec_tests.rs
expression: valid.jsx
---
# Input
```js
<>
<button tabIndex="0"></button>
<span role="button" tabIndex="0"></span>
<span role="article" tabIndex="-1"></span>
<MyButton tabIndex={0} />
<article tabIndex="-1"></article>
<div tabIndex="-1"></div>
<article tabIndex={-1}></article>
<div tabIndex={-1}></div>
<div></div>
<button tabIndex="-1"></button>
</>;

```


Loading