From 478d894dfd66e0cdd30c9c956dc1b5ec48b276b7 Mon Sep 17 00:00:00 2001 From: ematipico Date: Mon, 19 Dec 2022 16:41:22 +0000 Subject: [PATCH] refactor(rome_js_analyze): `has_truthy_value` --- .../src/analyzers/a11y/use_anchor_content.rs | 88 +++------------ .../src/analyzers/nursery/no_redundant_alt.rs | 22 +--- .../nursery/use_aria_prop_types.rs | 48 ++------ crates/rome_js_syntax/src/jsx_ext.rs | 103 +++++++++++++++++- 4 files changed, 129 insertions(+), 132 deletions(-) diff --git a/crates/rome_js_analyze/src/analyzers/a11y/use_anchor_content.rs b/crates/rome_js_analyze/src/analyzers/a11y/use_anchor_content.rs index 4770cb28dfe..c040a93d47b 100644 --- a/crates/rome_js_analyze/src/analyzers/a11y/use_anchor_content.rs +++ b/crates/rome_js_analyze/src/analyzers/a11y/use_anchor_content.rs @@ -2,10 +2,10 @@ use rome_analyze::context::RuleContext; use rome_analyze::{declare_rule, Ast, Rule, RuleDiagnostic}; use rome_console::markup; use rome_js_syntax::{ - AnyJsExpression, AnyJsLiteralExpression, AnyJsTemplateElement, AnyJsxAttributeValue, - AnyJsxChild, JsxAttribute, JsxElement, JsxReferenceIdentifier, JsxSelfClosingElement, + AnyJsExpression, AnyJsLiteralExpression, AnyJsxChild, JsxElement, JsxReferenceIdentifier, + JsxSelfClosingElement, }; -use rome_rowan::{declare_node_union, AstNode, AstNodeList}; +use rome_rowan::{declare_node_union, AstNode}; declare_rule! { /// Enforce that anchor elements have content and that the content is accessible to screen readers. @@ -86,9 +86,10 @@ impl UseAnchorContentNode { UseAnchorContentNode::JsxElement(element) => { if let Ok(opening_element) = element.opening_element() { match opening_element.find_attribute_by_name("aria-hidden") { - Ok(Some(aria_hidden_attribute)) => { - is_aria_hidden_truthy(aria_hidden_attribute).unwrap_or(false) - } + Ok(Some(aria_hidden_attribute)) => aria_hidden_attribute + .has_truthy_value(|value| value == "true") + .ok() + .unwrap_or(false), _ => false, } } else { @@ -97,9 +98,10 @@ impl UseAnchorContentNode { } UseAnchorContentNode::JsxSelfClosingElement(element) => { match element.find_attribute_by_name("aria-hidden") { - Ok(Some(aria_hidden_attribute)) => { - is_aria_hidden_truthy(aria_hidden_attribute).unwrap_or(false) - } + Ok(Some(aria_hidden_attribute)) => aria_hidden_attribute + .has_truthy_value(|value| value.text() == "true") + .ok() + .unwrap_or(false), _ => false, } } @@ -180,7 +182,9 @@ fn is_accessible_to_screen_reader(element: AnyJsxChild) -> Option { let aria_hidden_attribute = opening_element .find_attribute_by_name("aria-hidden") .ok()??; - !is_aria_hidden_truthy(aria_hidden_attribute)? + !aria_hidden_attribute + .has_truthy_value(|value| value == "true") + .ok()? } AnyJsxChild::JsxSelfClosingElement(element) => { // We don't check if a component (e.g. ) is using the `aria-hidden` property, @@ -191,7 +195,9 @@ fn is_accessible_to_screen_reader(element: AnyJsxChild) -> Option { } let aria_hidden_attribute = element.find_attribute_by_name("aria-hidden").ok()??; - !is_aria_hidden_truthy(aria_hidden_attribute)? + !aria_hidden_attribute + .has_truthy_value(|value| value == "true") + .ok()? } AnyJsxChild::JsxExpressionChild(expression) => { let expression = expression.expression()?; @@ -209,63 +215,3 @@ fn is_accessible_to_screen_reader(element: AnyJsxChild) -> Option { _ => true, }) } - -/// Check if the `aria-hidden` attribute is present or the value is true. -fn is_aria_hidden_truthy(aria_hidden_attribute: JsxAttribute) -> Option { - let initializer = aria_hidden_attribute.initializer(); - if initializer.is_none() { - return Some(true); - } - let attribute_value = initializer?.value().ok()?; - Some(match attribute_value { - AnyJsxAttributeValue::JsxExpressionAttributeValue(attribute_value) => { - let expression = attribute_value.expression().ok()?; - is_expression_truthy(expression)? - } - AnyJsxAttributeValue::AnyJsxTag(_) => false, - AnyJsxAttributeValue::JsxString(aria_hidden_string) => { - let aria_hidden_value = aria_hidden_string.inner_string_text().ok()?; - aria_hidden_value == "true" - } - }) -} - -/// Check if the expression contains only one boolean literal `true` -/// or one string literal `"true"` -fn is_expression_truthy(expression: AnyJsExpression) -> Option { - Some(match expression { - AnyJsExpression::AnyJsLiteralExpression(literal_expression) => { - if let AnyJsLiteralExpression::JsBooleanLiteralExpression(boolean_literal) = - literal_expression - { - let text = boolean_literal.value_token().ok()?; - text.text_trimmed() == "true" - } else if let AnyJsLiteralExpression::JsStringLiteralExpression(string_literal) = - literal_expression - { - let text = string_literal.inner_string_text().ok()?; - text == "true" - } else { - false - } - } - AnyJsExpression::JsTemplateExpression(template) => { - let mut iter = template.elements().iter(); - if iter.len() != 1 { - return None; - } - match iter.next() { - Some(AnyJsTemplateElement::JsTemplateChunkElement(element)) => { - let template_token = element.template_chunk_token().ok()?; - template_token.text_trimmed() == "true" - } - Some(AnyJsTemplateElement::JsTemplateElement(element)) => { - let expression = element.expression().ok()?; - is_expression_truthy(expression)? - } - _ => false, - } - } - _ => false, - }) -} diff --git a/crates/rome_js_analyze/src/analyzers/nursery/no_redundant_alt.rs b/crates/rome_js_analyze/src/analyzers/nursery/no_redundant_alt.rs index 6ef9abef814..8a797ec9be6 100644 --- a/crates/rome_js_analyze/src/analyzers/nursery/no_redundant_alt.rs +++ b/crates/rome_js_analyze/src/analyzers/nursery/no_redundant_alt.rs @@ -59,25 +59,11 @@ impl Rule for NoRedundantAlt { } let aria_hidden_attribute = node.find_attribute_by_name("aria-hidden"); if let Some(aria_hidden) = aria_hidden_attribute { - let is_false = match aria_hidden.initializer()?.value().ok()? { - AnyJsxAttributeValue::AnyJsxTag(_) => false, - AnyJsxAttributeValue::JsxExpressionAttributeValue(aria_hidden) => { - aria_hidden - .expression() - .ok()? - .as_any_js_literal_expression()? - .as_js_boolean_literal_expression()? - .value_token() - .ok()? - .text_trimmed() - == "false" - } - AnyJsxAttributeValue::JsxString(aria_hidden) => { - aria_hidden.inner_string_text().ok()? == "false" - } - }; + let has_truthy_value = aria_hidden + .has_truthy_value(|value| value.text() == "true") + .ok()?; - if !is_false { + if has_truthy_value { return None; } } diff --git a/crates/rome_js_analyze/src/aria_analyzers/nursery/use_aria_prop_types.rs b/crates/rome_js_analyze/src/aria_analyzers/nursery/use_aria_prop_types.rs index 0acc342889f..a30fcc3820b 100644 --- a/crates/rome_js_analyze/src/aria_analyzers/nursery/use_aria_prop_types.rs +++ b/crates/rome_js_analyze/src/aria_analyzers/nursery/use_aria_prop_types.rs @@ -3,11 +3,8 @@ use rome_analyze::context::RuleContext; use rome_analyze::{declare_rule, Rule, RuleDiagnostic}; use rome_aria::AriaPropertyTypeEnum; use rome_console::markup; -use rome_js_syntax::{ - AnyJsExpression, AnyJsLiteralExpression, AnyJsxAttributeValue, JsSyntaxToken, JsxAttribute, - TextRange, -}; -use rome_rowan::{AstNode, AstNodeList}; +use rome_js_syntax::{JsSyntaxToken, JsxAttribute, TextRange}; +use rome_rowan::AstNode; use std::slice::Iter; declare_rule! { @@ -76,42 +73,13 @@ impl Rule for UseAriaPropTypes { let attribute_name = node.name().ok()?.as_jsx_name()?.value_token().ok()?; if let Some(aria_property) = aria_properties.get_property(attribute_name.text_trimmed()) { - let attribute_value = node.initializer()?.value().ok()?; let attribute_value_range = node.range(); - let attribute_text = match attribute_value { - AnyJsxAttributeValue::JsxString(string) => Some(string.inner_string_text().ok()?), - AnyJsxAttributeValue::JsxExpressionAttributeValue(expression) => { - match expression.expression().ok()? { - AnyJsExpression::JsTemplateExpression(template) => { - if template.elements().is_empty() { - // Early error, the template literal is empty - return Some(UseAriaProptypesState { - attribute_value_range, - allowed_values: aria_property.values(), - attribute_name, - property_type: aria_property.property_type(), - }); - } - template.elements().iter().next().and_then(|chunk| { - Some( - chunk - .as_js_template_chunk_element()? - .template_chunk_token() - .ok()? - .token_text_trimmed(), - ) - }) - } - AnyJsExpression::AnyJsLiteralExpression( - AnyJsLiteralExpression::JsStringLiteralExpression(string), - ) => Some(string.inner_string_text().ok()?), - _ => None, - } - } - _ => return None, - }?; - - if !aria_property.contains_correct_value(attribute_text.text()) { + if !node + .has_truthy_value(|value| { + !value.text().is_empty() && aria_property.contains_correct_value(value.text()) + }) + .ok()? + { return Some(UseAriaProptypesState { attribute_value_range, allowed_values: aria_property.values(), diff --git a/crates/rome_js_syntax/src/jsx_ext.rs b/crates/rome_js_syntax/src/jsx_ext.rs index 9f468b8e391..771215a26d2 100644 --- a/crates/rome_js_syntax/src/jsx_ext.rs +++ b/crates/rome_js_syntax/src/jsx_ext.rs @@ -1,9 +1,9 @@ use std::collections::HashSet; use crate::{ - AnyJsExpression, AnyJsLiteralExpression, AnyJsxAttribute, AnyJsxAttributeValue, - AnyJsxElementName, JsSyntaxToken, JsxAttribute, JsxAttributeList, JsxName, JsxOpeningElement, - JsxSelfClosingElement, JsxString, TextSize, + AnyJsExpression, AnyJsLiteralExpression, AnyJsTemplateElement, AnyJsxAttribute, + AnyJsxAttributeValue, AnyJsxElementName, JsSyntaxToken, JsxAttribute, JsxAttributeList, + JsxName, JsxOpeningElement, JsxSelfClosingElement, JsxString, TextSize, }; use rome_rowan::{declare_node_union, AstNode, AstNodeList, SyntaxResult, SyntaxTokenText}; @@ -409,6 +409,103 @@ impl JsxAttribute { }) .unwrap_or(false) } + + /// This function checks the value of the attribute contains "truthy" value. + /// + /// ### Examples + /// + /// The following examples represents attributes that **don't have** truthy values: + /// + /// ```jsx + /// content + /// content + /// content + /// content + /// content + /// content + /// content + /// ``` + /// + /// The function attempts to extract the value of the attributes. + /// + /// Fhe function accepts a `closure` that will be used to for further check the value. + pub fn has_truthy_value(&self, closure: F) -> SyntaxResult + where + F: FnOnce(SyntaxTokenText) -> bool, + { + if self.is_value_undefined_or_null() { + return Ok(false); + } + + if let Some(initializer) = self.initializer() { + let value = initializer.value()?; + + let result = match value { + AnyJsxAttributeValue::JsxExpressionAttributeValue(expression) => { + let expression = expression.expression()?; + match expression { + AnyJsExpression::AnyJsLiteralExpression(literal_expression) => { + if let AnyJsLiteralExpression::JsBooleanLiteralExpression( + boolean_literal, + ) = literal_expression + { + let text = boolean_literal.value_token()?; + Some(text.token_text_trimmed()) + } else if let AnyJsLiteralExpression::JsStringLiteralExpression( + string_literal, + ) = literal_expression + { + let text = string_literal.inner_string_text()?; + Some(text) + } else { + None + } + } + AnyJsExpression::JsTemplateExpression(template) => { + let mut iter = template.elements().iter(); + if iter.len() != 1 { + return Ok(false); + } + match iter.next() { + Some(AnyJsTemplateElement::JsTemplateChunkElement(element)) => { + let template_token = element.template_chunk_token()?; + Some(template_token.token_text()) + } + Some(AnyJsTemplateElement::JsTemplateElement(element)) => { + let expression = element.expression()?; + if let AnyJsExpression::AnyJsLiteralExpression( + AnyJsLiteralExpression::JsBooleanLiteralExpression( + boolean_expression, + ), + ) = expression + { + let value = boolean_expression.value_token()?; + Some(value.token_text_trimmed()) + } else { + None + } + } + _ => None, + } + } + _ => None, + } + } + AnyJsxAttributeValue::JsxString(string) => { + let value = string.inner_string_text()?; + Some(value) + } + + _ => None, + }; + + Ok(result.map(|token| closure(token)).unwrap_or(true)) + } else { + // values that don't have initializer get a `true` at compile time, making them + // truthy + Ok(true) + } + } } impl AnyJsxAttributeValue {