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

Commit

Permalink
refactor: update a11y rules with no prefix
Browse files Browse the repository at this point in the history
  • Loading branch information
@nissy-dev
nissy-dev committed Apr 15, 2023
1 parent 7cfaf39 commit 55a5cc4
Show file tree
Hide file tree
Showing 19 changed files with 640 additions and 279 deletions.
71 changes: 21 additions & 50 deletions crates/rome_js_analyze/src/analyzers/a11y/no_access_key.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@ use crate::JsRuleAction;
use rome_analyze::{context::RuleContext, declare_rule, ActionCategory, Ast, Rule, RuleDiagnostic};
use rome_console::markup;
use rome_diagnostics::Applicability;
use rome_js_syntax::{
AnyJsxAttributeValue, JsxAttribute, JsxAttributeList, JsxOpeningElement, JsxSelfClosingElement,
};
use rome_rowan::{declare_node_union, AstNode, BatchMutationExt};
use rome_js_syntax::{jsx_ext::AnyJsxElement, JsxAttribute, JsxAttributeList};
use rome_rowan::{AstNode, BatchMutationExt};

declare_rule! {
/// Enforce that the `accessKey` attribute is not used on any HTML element.
Expand Down Expand Up @@ -43,24 +41,6 @@ declare_rule! {
}
}

declare_node_union! {
pub(crate) AnyJsxElement = JsxOpeningElement | JsxSelfClosingElement
}

impl AnyJsxElement {
/// Check if the given element is a HTML element, not a user-created React component
fn is_html_element(&self) -> Option<bool> {
Some(match self {
AnyJsxElement::JsxOpeningElement(element) => {
element.name().ok()?.as_jsx_name().is_some()
}
AnyJsxElement::JsxSelfClosingElement(element) => {
element.name().ok()?.as_jsx_name().is_some()
}
})
}
}

impl Rule for NoAccessKey {
type Query = Ast<JsxAttribute>;
type State = ();
Expand All @@ -69,10 +49,7 @@ impl Rule for NoAccessKey {

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let node = ctx.query();
let attribute_name = node.name().ok()?;
let name = attribute_name.as_jsx_name()?;
let name_token = name.value_token().ok()?;
if name_token.text_trimmed() != "accessKey" {
if node.name_value_token()?.text_trimmed() != "accessKey" {
return None;
}

Expand All @@ -82,19 +59,13 @@ impl Rule for NoAccessKey {

// We do not know if the `accessKey` prop is used for HTML elements
// or for user-created React components
if !element.is_html_element()? {
if element.is_custom_component() {
return None;
}

let attribute_value = node.initializer()?.value().ok();
if let Some(AnyJsxAttributeValue::JsxExpressionAttributeValue(expression)) = attribute_value
{
let expression = expression.expression().ok()?;
let name = expression.as_js_identifier_expression()?.name().ok()?;
let name_token = name.value_token().ok()?;
if name_token.text_trimmed() == "undefined" {
return None;
}
let attribute_value = node.initializer()?.value().ok()?;
if attribute_value.is_value_null_or_undefined() {
return None;
}

Some(())
Expand All @@ -103,20 +74,20 @@ impl Rule for NoAccessKey {
fn diagnostic(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<RuleDiagnostic> {
let node = ctx.query();
Some(
RuleDiagnostic::new(
rule_category!(),
node.syntax().text_trimmed_range(),
markup! {
"Avoid the "<Emphasis>"accessKey"</Emphasis>" attribute to reduce inconsistencies between \
keyboard shortcuts and screen reader keyboard comments."
},
).note(
markup! {
"Assigning keyboard shortcuts using the "<Emphasis>"accessKey"</Emphasis>" attribute leads to \
inconsistent keyboard actions across applications."
},
)
)
RuleDiagnostic::new(
rule_category!(),
node.syntax().text_trimmed_range(),
markup! {
"Avoid the "<Emphasis>"accessKey"</Emphasis>" attribute to reduce inconsistencies between \
keyboard shortcuts and screen reader keyboard comments."
},
).note(
markup! {
"Assigning keyboard shortcuts using the "<Emphasis>"accessKey"</Emphasis>" attribute leads to \
inconsistent keyboard actions across applications."
},
)
)
}

fn action(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<JsRuleAction> {
Expand Down
23 changes: 13 additions & 10 deletions crates/rome_js_analyze/src/analyzers/a11y/no_auto_focus.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ use rome_js_syntax::{jsx_ext::AnyJsxElement, JsxAttribute};
use rome_rowan::{AstNode, BatchMutationExt};

declare_rule! {
/// Avoid the `autoFocus` attribute
/// Enforce that autoFocus prop is not used on elements.
///
/// Autofocusing elements can cause usability issues for sighted and non-sighted users, alike.
///
/// ## Examples
///
Expand Down Expand Up @@ -46,6 +48,12 @@ declare_rule! {
/// // `autoFocus` prop in user created component is valid
/// <MyComponent autoFocus={true} />
///```
///
/// ## Resources
///
/// - [WHATWG HTML Standard, The autofocus attribute](https://html.spec.whatwg.org/multipage/interaction.html#attr-fe-autofocus)
/// - [The accessibility of HTML 5 autofocus](https://brucelawson.co.uk/2009/the-accessibility-of-html-5-autofocus/)
///
pub(crate) NoAutoFocus {
version: "10.0.0",
name: "noAutofocus",
Expand All @@ -61,16 +69,11 @@ impl Rule for NoAutoFocus {

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let node = ctx.query();
match node {
AnyJsxElement::JsxOpeningElement(element) => {
element.name().ok()?.as_jsx_name()?;
element.find_attribute_by_name("autoFocus").ok()?
}
AnyJsxElement::JsxSelfClosingElement(element) => {
element.name().ok()?.as_jsx_name()?;
element.find_attribute_by_name("autoFocus").ok()?
}
if node.is_custom_component() {
return None;
}

node.find_attribute_by_name("autoFocus")
}

fn diagnostic(_ctx: &RuleContext<Self>, attr: &Self::State) -> Option<RuleDiagnostic> {
Expand Down
124 changes: 36 additions & 88 deletions crates/rome_js_analyze/src/analyzers/a11y/no_blank_target.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ use rome_js_factory::make::{
jsx_attribute, jsx_attribute_initializer_clause, jsx_attribute_list, jsx_ident, jsx_name,
jsx_string, token,
};
use rome_js_syntax::jsx_ext::AnyJsxElement;
use rome_js_syntax::{
AnyJsxAttribute, AnyJsxAttributeName, AnyJsxAttributeValue, JsxAttribute, JsxAttributeList,
JsxElement, JsxSelfClosingElement, T,
AnyJsxAttribute, AnyJsxAttributeName, AnyJsxAttributeValue, JsxAttribute, JsxAttributeList, T,
};
use rome_rowan::{declare_node_union, AstNode, AstNodeList, BatchMutationExt, TriviaPieceKind};
use rome_rowan::{AstNode, AstNodeList, BatchMutationExt, TriviaPieceKind};

declare_rule! {
/// Disallow `target="_blank"` attribute without `rel="noreferrer"`
Expand Down Expand Up @@ -39,15 +39,14 @@ declare_rule! {
/// <a {...props} href='http://external.link' target='_blank' rel="noopener">child</a>
/// ```
///
/// ```jsx,expect_diagnostic
/// // case-insensitive
/// <a href='http://external.link' target='_BlaNk'>child</a>
/// ```
/// ### Valid
///
/// ```jsx
/// let a = <a href='http://external.link' rel='noreferrer' target='_blank'>child</a>;
/// let a = <a href='http://external.link' target='_blank' rel="noopener" {...props}>child</a>;
/// <a href='http://external.link' rel='noreferrer' target='_blank'>child</a>
/// ```
///
/// ```jsx
/// <a href='http://external.link' target='_blank' rel="noopener" {...props}>child</a>
/// ```
pub(crate) NoBlankTarget {
version: "10.0.0",
Expand All @@ -56,29 +55,8 @@ declare_rule! {
}
}

declare_node_union! {
pub(crate) NoBlankTargetQuery = JsxElement | JsxSelfClosingElement
}

impl NoBlankTargetQuery {
fn has_trailing_spread_attribute(
&self,
current_attribute: impl Into<AnyJsxAttribute>,
) -> Option<bool> {
Some(match self {
NoBlankTargetQuery::JsxElement(element) => element
.opening_element()
.ok()?
.has_trailing_spread_prop(current_attribute),
NoBlankTargetQuery::JsxSelfClosingElement(element) => {
element.has_trailing_spread_prop(current_attribute)
}
})
}
}

impl Rule for NoBlankTarget {
type Query = Ast<NoBlankTargetQuery>;
type Query = Ast<AnyJsxElement>;
/// Two attributes:
/// 1. The attribute `target=`
/// 2. The attribute `rel=`, if present
Expand All @@ -88,64 +66,34 @@ impl Rule for NoBlankTarget {

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let node = ctx.query();
let (target_attribute, rel_attribute) = match node {
NoBlankTargetQuery::JsxElement(element) => {
let opening_element = element.opening_element().ok()?;
if opening_element.name().ok()?.text() != "a"
|| opening_element
.find_attribute_by_name("href")
.ok()
.is_none()
{
return None;
}

(
opening_element.find_attribute_by_name("target").ok()?,
opening_element.find_attribute_by_name("rel").ok()?,
)
}
NoBlankTargetQuery::JsxSelfClosingElement(element) => {
if element.name().ok()?.text() != "a"
|| element.find_attribute_by_name("href").ok().is_none()
{
return None;
}
(
element.find_attribute_by_name("target").ok()?,
element.find_attribute_by_name("rel").ok()?,
)
}
};

let target_attribute = target_attribute?;
if node.name_value_token()?.text_trimmed() != "a"
|| node.find_attribute_by_name("href").is_none()
{
return None;
}

let text = target_attribute
.initializer()?
.value()
.ok()?
.as_jsx_string()?
.inner_string_text()
.ok()?;
let target_attribute = node.find_attribute_by_name("target")?;
let rel_attribute = node.find_attribute_by_name("rel");

if text.to_lowercase() == "_blank" {
if target_attribute
.as_static_value()?
.is_string_constant("_blank")
{
match rel_attribute {
None => {
if !node.has_trailing_spread_attribute(target_attribute.clone())? {
if !node.has_trailing_spread_prop(target_attribute.clone()) {
return Some((target_attribute, None));
}
}
Some(rel_attribute) => {
let rel_text = rel_attribute
.initializer()?
.value()
.ok()?
.as_jsx_string()?
.inner_string_text()
.ok()?;
if !rel_text.text().contains("noreferrer")
&& !node.has_trailing_spread_attribute(target_attribute.clone())?
&& !node.has_trailing_spread_attribute(rel_attribute.clone())?
if rel_attribute.initializer().is_none()
|| (!rel_attribute
.as_static_value()?
.text()
.split_ascii_whitespace()
.any(|f| f == "noreferrer")
&& !node.has_trailing_spread_prop(target_attribute.clone())
&& !node.has_trailing_spread_prop(rel_attribute.clone()))
{
return Some((target_attribute, Some(rel_attribute)));
}
Expand All @@ -162,13 +110,13 @@ impl Rule for NoBlankTarget {
) -> Option<JsRuleAction> {
let mut mutation = ctx.root().begin();
let message = if let Some(rel_attribute) = rel_attribute {
let old_jsx_string = rel_attribute.initializer()?.value().ok()?;
let old_jsx_string = old_jsx_string.as_jsx_string()?;
let rel_quoted_string = old_jsx_string.inner_string_text().ok()?;
let rel_text = rel_quoted_string.text();
let new_text = format!("\"noreferrer {rel_text}\"");
let new_jsx_string = jsx_string(jsx_ident(&new_text));
mutation.replace_node(old_jsx_string.clone(), new_jsx_string);
let prev_jsx_attribute = rel_attribute.initializer()?.value().ok()?;
let prev_jsx_string = prev_jsx_attribute.as_jsx_string()?;
let new_text = format!(
"\"noreferrer {}\"",
prev_jsx_string.inner_string_text().ok()?.text()
);
mutation.replace_node(prev_jsx_string.clone(), jsx_string(jsx_ident(&new_text)));

(markup! {
"Add the "<Emphasis>"\"noreferrer\""</Emphasis>" to the existing attribute."
Expand Down
11 changes: 8 additions & 3 deletions crates/rome_js_analyze/src/analyzers/a11y/no_distracting_elements.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,23 @@ declare_rule! {
/// ### Invalid
///
/// ```jsx,expect_diagnostic
/// <marquee/>
/// <marquee />
/// ```
///
/// ```jsx,expect_diagnostic
/// <blink/>
/// <blink />
/// ```
///
/// ### Valid
///
/// ```jsx
/// <div/>
/// <div />
/// ```
///
/// ## Accessibility guidelines
///
/// - [WCAG 2.2.2](https://www.w3.org/WAI/WCAG21/Understanding/pause-stop-hide)
///
pub(crate) NoDistractingElements {
version: "11.0.0",
name: "noDistractingElements",
Expand Down
12 changes: 8 additions & 4 deletions crates/rome_js_analyze/src/analyzers/a11y/no_header_scope.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ use rome_rowan::{AstNode, BatchMutationExt};
use crate::JsRuleAction;

declare_rule! {
/// Check that the scope attribute is only used on `th` elements.
///
/// ESLint Equivalent: [scope](https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/master/docs/rules/scope.md)
/// The scope prop should be used only on `<th>` elements.
///
/// ## Examples
///
Expand All @@ -34,6 +32,12 @@ declare_rule! {
/// ```jsx
/// <th scope="col"></th>
/// ```
///
/// ## Accessibility guidelines
///
/// - [WCAG 1.3.1](https://www.w3.org/WAI/WCAG21/Understanding/info-and-relationships)
/// - [WCAG 4.1.1](https://www.w3.org/WAI/WCAG21/Understanding/parsing)
///
pub(crate) NoHeaderScope {
version: "11.0.0",
name: "noHeaderScope",
Expand All @@ -50,7 +54,7 @@ impl Rule for NoHeaderScope {
fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let attr = ctx.query();

if attr.name().ok()?.syntax().text_trimmed() != "scope" {
if attr.name_value_token()?.text_trimmed() != "scope" {
return None;
}

Expand Down
8 changes: 8 additions & 0 deletions crates/rome_js_analyze/tests/specs/a11y/noAccessKey/invalid.jsx
View file
Original file line number Diff line number Diff line change
@@ -1,2 +1,10 @@
<input type="submit" accessKey="s" value="Submit" />;
<button accessKey="n">Next</button>;
<div accessKey="h" {...props} />;
<div accessKey={"y"} />;
<div accessKey={`${y}`} />;
<div accessKey={`${undefined}y${undefined}`} />;
<div accessKey={`This is ${bad}`} />;
<div accessKey={accessKey} />;
<div accessKey={`${undefined}`} />;
<div accessKey={`${undefined}${undefined}`} />;
Loading

0 comments on commit 55a5cc4

Please sign in to comment.