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

Commit

Permalink
Fix clippy warning. Rename helpers
Browse files Browse the repository at this point in the history
  • Loading branch information
@MichaReiser
MichaReiser committed Nov 10, 2022
1 parent 44c8712 commit 7e8dfa2
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 106 deletions.
118 changes: 56 additions & 62 deletions crates/rome_js_analyze/src/analyzers/a11y/use_key_with_click_events.rs
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
use rome_analyze::{context::RuleContext, declare_rule, Ast, Rule, RuleDiagnostic};
use rome_console::markup;
use rome_js_syntax::{JsxAnyAttribute, JsxOpeningElement, JsxSelfClosingElement};
use rome_rowan::{declare_node_union, AstNode, AstNodeList};
use rome_js_syntax::{
JsxAnyAttribute, JsxAnyElementName, JsxAttributeList, JsxOpeningElement, JsxSelfClosingElement,
};
use rome_rowan::{declare_node_union, AstNode, SyntaxResult};

declare_rule! {
/// Enforce to have the `onClick` mouse event with the `onKeyUp`, the `onKeyDown`, or the `noKeyPress` keyboard event.
/// Enforce to have the `onClick` mouse event with the `onKeyUp`, the `onKeyDown`, or the `onKeyPress` keyboard event.
///
/// ## Examples
///
Expand Down Expand Up @@ -44,6 +46,10 @@ declare_rule! {
/// ```jsx
/// <div {...spread} onClick={() => {}} ></div>
/// ```
///
/// ```jsx
/// <button onClick={() => console.log("test")}>Submit</button>
/// ```
pub(crate) UseKeyWithClickEvents {
version: "10.0.0",
name: "useKeyWithClickEvents",
Expand All @@ -56,22 +62,19 @@ declare_node_union! {
}

impl JsxAnyElement {
fn has_spread_attribute(&self) -> bool {
fn attributes(&self) -> JsxAttributeList {
match self {
JsxAnyElement::JsxOpeningElement(element) => element
.attributes()
.iter()
.any(|attribute| matches!(attribute, JsxAnyAttribute::JsxSpreadAttribute(_))),
JsxAnyElement::JsxSelfClosingElement(element) => element
.attributes()
.iter()
.any(|attribute| matches!(attribute, JsxAnyAttribute::JsxSpreadAttribute(_))),
JsxAnyElement::JsxOpeningElement(element) => element.attributes(),
JsxAnyElement::JsxSelfClosingElement(element) => element.attributes(),
}
}
}

impl UseKeyWithClickEvents {
const REQUIRED_PROPS: [&'static str; 3] = ["onKeyDown", "onKeyUp", "onKeyPress"];
fn name(&self) -> SyntaxResult<JsxAnyElementName> {
match self {
JsxAnyElement::JsxOpeningElement(element) => element.name(),
JsxAnyElement::JsxSelfClosingElement(element) => element.name(),
}
}
}

impl Rule for UseKeyWithClickEvents {
Expand All @@ -81,64 +84,55 @@ impl Rule for UseKeyWithClickEvents {
type Options = ();

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

match element.name() {
Ok(JsxAnyElementName::JsxName(name)) => {
let element_name = name.value_token().ok()?.text_trimmed().to_lowercase();

match node {
JsxAnyElement::JsxOpeningElement(element) => {
let on_click_attribute = element.find_attribute_by_name("onClick").ok()?;
if element.name().ok()?.as_jsx_name().is_none()
|| on_click_attribute.is_none()
|| node.has_spread_attribute()
{
// Don't handle interactive roles
// TODO Support aria roles https://github.com/rome/tools/issues/3640
if matches!(
element_name.as_str(),
"button" | "checkbox" | "combobox" | "a" | "input"
) {
return None;
}
}
_ => {
return None;
}
}

for attribute in element.attributes().into_iter() {
if let JsxAnyAttribute::JsxAttribute(attribute) = attribute {
let name = attribute
.name()
.ok()?
.as_jsx_name()?
.syntax()
.text_trimmed()
.to_string();

if Self::REQUIRED_PROPS.contains(&name.as_str()) {
return None;
}
}
}
let attributes = element.attributes();
let on_click_attribute = attributes.find_by_name("onClick").ok()?;

Some(())
}
JsxAnyElement::JsxSelfClosingElement(element) => {
let on_click_attribute = element.find_attribute_by_name("onClick").ok()?;
if element.name().ok()?.as_jsx_name().is_none()
|| on_click_attribute.is_none()
|| node.has_spread_attribute()
{
return None;
}
#[allow(clippy::question_mark)]
if on_click_attribute.is_none() {
return None;
}

for attribute in element.attributes().into_iter() {
if let JsxAnyAttribute::JsxAttribute(attribute) = attribute {
let name = attribute
.name()
.ok()?
.as_jsx_name()?
.syntax()
.text_trimmed()
.to_string();
for attribute in attributes {
match attribute {
JsxAnyAttribute::JsxAttribute(attribute) => {
let attribute_name = attribute.name().ok()?;
let name = attribute_name.as_jsx_name()?;
let name_token = name.value_token().ok()?;

if Self::REQUIRED_PROPS.contains(&name.as_str()) {
return None;
}
if matches!(
name_token.text_trimmed(),
"onKeyDown" | "onKeyUp" | "onKeyPress"
) {
return None;
}
}

Some(())
JsxAnyAttribute::JsxSpreadAttribute(_) => {
return None;
}
}
}

Some(())
}

fn diagnostic(ctx: &RuleContext<Self>, _: &Self::State) -> Option<RuleDiagnostic> {
Expand Down
2 changes: 1 addition & 1 deletion crates/rome_js_analyze/src/utils/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ pub fn ok_find_attributes_by_name() {
.filter_map(rome_js_syntax::JsxAttributeList::cast)
.next()
.unwrap();
let [a, c, d] = list.find_attributes_by_name(["a", "c", "d"]);
let [a, c, d] = list.find_by_names(["a", "c", "d"]);
assert_eq!(
a.unwrap()
.initializer()
Expand Down
78 changes: 37 additions & 41 deletions crates/rome_js_syntax/src/jsx_ext.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ impl JsxOpeningElement {
&self,
name_to_lookup: &str,
) -> SyntaxResult<Option<JsxAttribute>> {
find_attribute_by_name(self.attributes(), name_to_lookup)
self.attributes().find_by_name(name_to_lookup)
}

/// It checks if current attribute has a trailing spread props
Expand Down Expand Up @@ -131,7 +131,8 @@ impl JsxOpeningElement {
/// assert!(opening_element.has_trailing_spread_prop(div.clone()));
/// ```
pub fn has_trailing_spread_prop(&self, current_attribute: impl Into<JsxAnyAttribute>) -> bool {
has_trailing_spread_prop(self.attributes(), current_attribute)
self.attributes()
.has_trailing_spread_prop(current_attribute)
}
}

Expand Down Expand Up @@ -182,7 +183,7 @@ impl JsxSelfClosingElement {
&self,
name_to_lookup: &str,
) -> SyntaxResult<Option<JsxAttribute>> {
find_attribute_by_name(self.attributes(), name_to_lookup)
self.attributes().find_by_name(name_to_lookup)
}

/// It checks if current attribute has a trailing spread props
Expand Down Expand Up @@ -230,21 +231,22 @@ impl JsxSelfClosingElement {
/// assert!(opening_element.has_trailing_spread_prop(div.clone()));
/// ```
pub fn has_trailing_spread_prop(&self, current_attribute: impl Into<JsxAnyAttribute>) -> bool {
has_trailing_spread_prop(self.attributes(), current_attribute)
self.attributes()
.has_trailing_spread_prop(current_attribute)
}
}

impl JsxAttributeList {
/// Find and return the `JsxAttribute` that matches the given name like [find_attribute_by_name].
/// Only attributes with name as [JsxName] can be returned.
/// Finds and returns attributes `JsxAttribute` that matches the given names like [Self::find_by_name].
/// Only attributes with name as [JsxName] can be returned.
///
/// Each name of "names_to_lookup" should be unique.
/// Each name of "names_to_lookup" should be unique.
///
/// Supports maximum of 16 names to avoid stack overflow. Eeach attribute will consume:
/// Supports maximum of 16 names to avoid stack overflow. Each attribute will consume:
///
/// - 8 bytes for the [Option<JsxAttribute>] result;
/// - plus 16 bytes for the [&str] argument.
pub fn find_attributes_by_name<const N: usize>(
pub fn find_by_names<const N: usize>(
&self,
names_to_lookup: [&str; N],
) -> [Option<JsxAttribute>; N] {
Expand All @@ -257,7 +259,7 @@ impl JsxAttributeList {

let mut missing = N;

'attributes: for att in self.iter() {
'attributes: for att in self {
if let Some(attribute) = att.as_jsx_attribute() {
if let Some(name) = attribute
.name()
Expand All @@ -282,40 +284,34 @@ impl JsxAttributeList {

results
}
}

pub fn find_attribute_by_name(
attributes: JsxAttributeList,
name_to_lookup: &str,
) -> SyntaxResult<Option<JsxAttribute>> {
let attribute = attributes.iter().find_map(|attribute| {
let attribute = JsxAttribute::cast_ref(attribute.syntax())?;
let name = attribute.name().ok()?;
let name = JsxName::cast_ref(name.syntax())?;
if name.value_token().ok()?.text_trimmed() == name_to_lookup {
Some(attribute)
} else {
None
}
});
pub fn find_by_name(&self, name_to_lookup: &str) -> SyntaxResult<Option<JsxAttribute>> {
let attribute = self.iter().find_map(|attribute| {
let attribute = JsxAttribute::cast_ref(attribute.syntax())?;
let name = attribute.name().ok()?;
let name = JsxName::cast_ref(name.syntax())?;
if name.value_token().ok()?.text_trimmed() == name_to_lookup {
Some(attribute)
} else {
None
}
});

Ok(attribute)
}
Ok(attribute)
}

pub fn has_trailing_spread_prop(
attributes: JsxAttributeList,
current_attribute: impl Into<JsxAnyAttribute>,
) -> bool {
let mut current_attribute_found = false;
let current_attribute = current_attribute.into();
for attribute in attributes {
if attribute == current_attribute {
current_attribute_found = true;
continue;
}
if current_attribute_found && attribute.as_jsx_spread_attribute().is_some() {
return true;
pub fn has_trailing_spread_prop(&self, current_attribute: impl Into<JsxAnyAttribute>) -> bool {
let mut current_attribute_found = false;
let current_attribute = current_attribute.into();
for attribute in self {
if attribute == current_attribute {
current_attribute_found = true;
continue;
}
if current_attribute_found && attribute.as_jsx_spread_attribute().is_some() {
return true;
}
}
false
}
false
}
2 changes: 1 addition & 1 deletion website/docs/src/lint/rules/index.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ Enforces the usage of the attribute <code>type</code> for the element <code>butt
<a href="/lint/rules/useKeyWithClickEvents">useKeyWithClickEvents</a>
<span class="recommended">recommended</span>
</h3>
Enforce to have the <code>onClick</code> mouse event with the <code>onKeyUp</code>, the <code>onKeyDown</code>, or the <code>noKeyPress</code> keyboard event.
Enforce to have the <code>onClick</code> mouse event with the <code>onKeyUp</code>, the <code>onKeyDown</code>, or the <code>onKeyPress</code> keyboard event.
</section>
<section class="rule">
<h3 data-toc-exclude id="useKeyWithMouseEvents">
Expand Down
6 changes: 5 additions & 1 deletion website/docs/src/lint/rules/useKeyWithClickEvents.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ title: Lint Rule useKeyWithClickEvents

> This rule is recommended by Rome.
Enforce to have the `onClick` mouse event with the `onKeyUp`, the `onKeyDown`, or the `noKeyPress` keyboard event.
Enforce to have the `onClick` mouse event with the `onKeyUp`, the `onKeyDown`, or the `onKeyPress` keyboard event.

## Examples

Expand Down Expand Up @@ -71,3 +71,7 @@ Enforce to have the `onClick` mouse event with the `onKeyUp`, the `onKeyDown`, o
<div {...spread} onClick={() => {}} ></div>
```

```jsx
<button onClick={() => console.log("test")}>Submit</button>
```

0 comments on commit 7e8dfa2

Please sign in to comment.