Skip to content

Commit

Permalink
update code and fix conflicts
Browse files Browse the repository at this point in the history
  • Loading branch information
ematipico committed Jan 3, 2025
1 parent af00d40 commit df1d58b
Show file tree
Hide file tree
Showing 10 changed files with 231 additions and 223 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/biome_aria/src/roles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ impl AriaRoles {

/// Given an element name and attributes, it returns the role associated with that element.
/// If no explicit role attribute is present, an implicit role is returned.
fn get_role_by_element_name(&self, element: &impl Element) -> Option<AriaRole> {
pub fn get_role_by_element_name(&self, element: &impl Element) -> Option<AriaRole> {
element
.find_attribute_by_name(|name| name == "role")
.as_ref()
Expand Down
2 changes: 1 addition & 1 deletion crates/biome_aria_metadata/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ fn is_valid_html_id(id: &str) -> bool {
}

impl AriaRole {
/// Returns the first valid role from `values`, a space-separated list of roles.
/// Returns the first valid role from `roles`, a space-separated list of roles.
///
/// If a role attribute has multiple values, the first valid role (specified role) will be used.
/// See <https://www.w3.org/TR/2014/REC-wai-aria-implementation-20140320/#mapping_role>
Expand Down
184 changes: 102 additions & 82 deletions crates/biome_configuration/src/analyzer/linter/rules.rs

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
use crate::services::aria::Aria;
use biome_analyze::{context::RuleContext, declare_lint_rule, Rule, RuleDiagnostic, RuleSource};
use biome_aria::AriaRoles;
use biome_console::markup;
use biome_js_syntax::jsx_ext::AnyJsxElement;
use biome_rowan::AstNode;
use rustc_hash::FxHashMap;

declare_lint_rule! {
/// Disallow use event handlers on non-interactive elements.
Expand Down Expand Up @@ -78,6 +76,58 @@ declare_lint_rule! {
}
}

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

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

// Custom components are not checked because we do not know what DOM will be used.
if element.is_custom_component() {
return None;
}

let aria_roles = ctx.aria_roles();
let role = aria_roles.get_role_by_element_name(element);
let has_interactive_role = role.is_some_and(|role| role.is_interactive());

if !has_handler_props(element)
|| is_content_editable(element)
|| has_presentation_role(element)
|| is_hidden_from_screen_reader(element)?
|| has_interactive_role
{
return None;
}

// Non-interactive elements what contains event handler should be reported.
if has_handler_props(element) && aria_roles.is_not_interactive_element(element) {
return Some(());
};

None
}

fn diagnostic(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<RuleDiagnostic> {
let node = ctx.query();
Some(
RuleDiagnostic::new(
rule_category!(),
node.range(),
markup! {
"Non-interactive element should not have event handler."
},
)
.note(markup! {
"Consider replace semantically interactive element like "<Emphasis>"<button/>"</Emphasis>" or "<Emphasis>"<a href/>"</Emphasis>"."
})
)
}
}

/// Ref: https://github.com/jsx-eslint/jsx-ast-utils/blob/v3.3.5/src/eventHandlers.js
const INTERACTIVE_HANDLERS: &[&str] = &[
"onClick",
Expand Down Expand Up @@ -107,30 +157,22 @@ const INTERACTIVE_HANDLERS: &[&str] = &[
"onError",
];

type AttributesRef<'a> = Option<&'a FxHashMap<String, Vec<String>>>;

/// Check if the element contains event handler
fn has_handler_props(attributes: AttributesRef) -> bool {
attributes.is_some_and(|m| {
INTERACTIVE_HANDLERS
.iter()
.any(|handler| m.contains_key(*handler))
})
fn has_handler_props(element: &AnyJsxElement) -> bool {
INTERACTIVE_HANDLERS
.iter()
.any(|handler| element.find_attribute_by_name(handler).is_some())
}

/// Check if the element's implicit ARIA semantics have been removed.
/// See https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/presentation_role
///
/// Ref: https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/v6.10.0/src/util/isPresentationRole.js
fn has_presentation_role(attributes: AttributesRef) -> bool {
const PRESENTATION_ROLE: &[&str] = &["presentation", "none"];

if let Some(attributes) = attributes {
if let Some(values) = attributes.get("role") {
let is_presentation = values
.iter()
.any(|v| PRESENTATION_ROLE.contains(&v.as_str()));
return is_presentation;
fn has_presentation_role(element: &AnyJsxElement) -> bool {
if let Some(attribute) = element.find_attribute_by_name("role") {
let value = attribute.as_static_value();
if let Some(value) = value {
return matches!(value.as_string_constant(), Some("presentation" | "none"));
}
}
false
Expand All @@ -142,101 +184,32 @@ fn has_presentation_role(attributes: AttributesRef) -> bool {
/// - https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/hidden
///
/// Ref: https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/v6.10.0/src/util/isHiddenFromScreenReader.js
fn is_hidden_from_screen_reader(element_name: &str, attributes: AttributesRef) -> bool {
if let Some(attributes) = attributes {
let aria_hidden = attributes
.get("aria-hidden")
.map_or(false, |attr| attr.contains(&"true".to_string()));

let input_hidden = element_name == "input"
&& attributes
.get("type")
.map_or(false, |attr| attr.contains(&"hidden".to_string()));

return aria_hidden || input_hidden;
}

false
fn is_hidden_from_screen_reader(element_name: &AnyJsxElement) -> Option<bool> {
let is_aria_hidden = element_name.has_truthy_attribute("aria-hidden");

let name = element_name.name_value_token().ok()?;

let is_input_hidden = if name.text_trimmed() == "input" {
element_name
.find_attribute_by_name("type")
.and_then(|attribute| attribute.as_static_value())
.and_then(|value| value.as_string_constant().map(|value| value == "hidden"))
.unwrap_or_default()
} else {
false
};

Some(is_aria_hidden || is_input_hidden)
}

/// Check if the element is `contentEditable`
/// See https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/contenteditable
///
/// Ref: https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/v6.10.0/src/util/isContentEditable.js
fn is_content_editable(attributes: AttributesRef) -> bool {
if let Some(attributes) = attributes {
if let Some(values) = attributes.get("contentEditable") {
return values.contains(&"true".to_string());
}
}
false
}

/// Check if the element has interactive role
///
/// Ref: https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/v6.10.0/src/util/isInteractiveRole.js
fn is_interactive_role(attributes: AttributesRef, aria_roles: &AriaRoles) -> bool {
if let Some(attributes) = attributes {
if let Some(roles) = attributes.get("role") {
return roles
.iter()
.any(|role| aria_roles.is_role_interactive(role.as_str()));
}
}
false
}

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

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

// Custom components are not checked because we do not know what DOM will be used.
if element.is_custom_component() {
return None;
}

let attributes = ctx.extract_attributes(&element.attributes());
let attributes = ctx.convert_all_attribute_values(attributes);
let attributes_ref = attributes.as_ref();
let element_name = element.name().ok()?.as_jsx_name()?.value_token().ok()?;
let element_name = element_name.text_trimmed();
let aria_roles = ctx.aria_roles();

if !has_handler_props(attributes_ref)
|| is_content_editable(attributes_ref)
|| has_presentation_role(attributes_ref)
|| is_hidden_from_screen_reader(element_name, attributes_ref)
|| is_interactive_role(attributes_ref, aria_roles)
{
return None;
}

// Non-interactive elements what contains event handler should be reported.
if aria_roles.is_not_interactive_element(element_name, attributes) {
return Some(());
};

None
}

fn diagnostic(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<RuleDiagnostic> {
let node = ctx.query();
Some(
RuleDiagnostic::new(
rule_category!(),
node.range(),
markup! {
"Non-interactive element should not have event handler."
},
)
.note(markup! {
"Consider replace semantically interactive element like "<Emphasis>"<button/>"</Emphasis>" or "<Emphasis>"<a href/>"</Emphasis>"."
})
)
}
fn is_content_editable(element: &AnyJsxElement) -> bool {
element
.find_attribute_by_name("contentEditable")
.and_then(|attribute| attribute.as_static_value())
.and_then(|value| value.as_string_constant().map(|value| value == "true"))
.unwrap_or_default()
}
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,5 @@
<div role="term" onClick={() => { }} />
<div role="timer" onClick={() => { }} />
<div role="tooltip" onClick={() => { }} />
</>
<div role="progressbar" onClick={() => { }} />
</>
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: invalid.jsx
snapshot_kind: text
---
# Input
```jsx
Expand Down Expand Up @@ -92,7 +93,9 @@ expression: invalid.jsx
<div role="term" onClick={() => { }} />
<div role="timer" onClick={() => { }} />
<div role="tooltip" onClick={() => { }} />
<div role="progressbar" onClick={() => { }} />
</>

```

# Diagnostics
Expand Down Expand Up @@ -280,23 +283,6 @@ invalid.jsx:12:5 lint/nursery/noNoninteractiveElementInteractions ━━━━
i Consider replace semantically interactive element like <button/> or <a href/>.
```

```
invalid.jsx:13:5 lint/nursery/noNoninteractiveElementInteractions ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Non-interactive element should not have event handler.
11 │ <details onClick={() => { }} />
12 │ <dfn onClick={() => { }} />
> 13 │ <dl onClick={() => { }} />
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^
14 │ <dir onClick={() => { }} />
15 │ <dt onClick={() => { }} />
i Consider replace semantically interactive element like <button/> or <a href/>.
```

```
Expand Down Expand Up @@ -1550,7 +1536,7 @@ invalid.jsx:87:5 lint/nursery/noNoninteractiveElementInteractions ━━━━
> 87 │ <div role="timer" onClick={() => { }} />
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
88 │ <div role="tooltip" onClick={() => { }} />
89 │ </>
89 │ <div role="progressbar" onClick={() => { }} />
i Consider replace semantically interactive element like <button/> or <a href/>.
Expand All @@ -1566,7 +1552,25 @@ invalid.jsx:88:5 lint/nursery/noNoninteractiveElementInteractions ━━━━
87 │ <div role="timer" onClick={() => { }} />
> 88 │ <div role="tooltip" onClick={() => { }} />
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
89 │ </>
89 │ <div role="progressbar" onClick={() => { }} />
90 │ </>
i Consider replace semantically interactive element like <button/> or <a href/>.
```

```
invalid.jsx:89:4 lint/nursery/noNoninteractiveElementInteractions ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Non-interactive element should not have event handler.
87 │ <div role="timer" onClick={() => { }} />
88 │ <div role="tooltip" onClick={() => { }} />
> 89 │ <div role="progressbar" onClick={() => { }} />
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
90 │ </>
91 │
i Consider replace semantically interactive element like <button/> or <a href/>.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
<div role="menuitemcheckbox" onClick={() => { }} />
<div role="menuitemradio" onClick={() => { }} />
<div role="option" onClick={() => { }} />
<div role="progressbar" onClick={() => { }} />
<div role="radio" onClick={() => { }} />
<div role="row" onClick={() => { }} />
<div role="scrollbar" onClick={() => { }} />
Expand All @@ -44,4 +43,8 @@

{/* Presentation role */}
<div role="presentation" onClick={() => { }} />
</>

<div onClick={() => {}} aria-hidden />
<div onClick={() => {}} aria-hidden="true" />

</>
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: valid.jsx
snapshot_kind: text
---
# Input
```jsx
Expand Down Expand Up @@ -37,7 +38,6 @@ expression: valid.jsx
<div role="menuitemcheckbox" onClick={() => { }} />
<div role="menuitemradio" onClick={() => { }} />
<div role="option" onClick={() => { }} />
<div role="progressbar" onClick={() => { }} />
<div role="radio" onClick={() => { }} />
<div role="row" onClick={() => { }} />
<div role="scrollbar" onClick={() => { }} />
Expand All @@ -50,5 +50,10 @@ expression: valid.jsx

{/* Presentation role */}
<div role="presentation" onClick={() => { }} />

<div onClick={() => {}} aria-hidden />
<div onClick={() => {}} aria-hidden="true" />

</>

```
Loading

0 comments on commit df1d58b

Please sign in to comment.