Skip to content

Commit

Permalink
refactor(rome_js_anaylze): introduce StaticValue enum for checking va…
Browse files Browse the repository at this point in the history
…lue of the JSX attributes or JS expressions (rome#4342)
  • Loading branch information
nissy-dev authored Apr 7, 2023
1 parent 6cb9585 commit f6ada3a
Show file tree
Hide file tree
Showing 24 changed files with 352 additions and 328 deletions.
3 changes: 2 additions & 1 deletion crates/rome_js_analyze/src/analyzers/a11y/no_blank_target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,8 @@ impl Rule for NoBlankTarget {
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_text = old_jsx_string.inner_string_text().ok()?;
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);
Expand Down
2 changes: 1 addition & 1 deletion crates/rome_js_analyze/src/analyzers/a11y/use_alt_text.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ fn input_has_type_image(element: &JsxSelfClosingElement) -> Option<bool> {
let initializer = prop.initializer()?.value().ok()?;
let initializer = initializer.as_jsx_string()?;

if initializer.inner_string_text().ok()? == "image" {
if initializer.inner_string_text().ok()?.text() == "image" {
return Some(true);
}
return None;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,8 @@ fn is_aria_hidden_truthy(aria_hidden_attribute: JsxAttribute) -> Option<bool> {
}
AnyJsxAttributeValue::AnyJsxTag(_) => false,
AnyJsxAttributeValue::JsxString(aria_hidden_string) => {
let aria_hidden_value = aria_hidden_string.inner_string_text().ok()?;
aria_hidden_value == "true"
let quoted_string = aria_hidden_string.inner_string_text().ok()?;
quoted_string.text() == "true"
}
})
}
Expand All @@ -243,8 +243,8 @@ fn is_expression_truthy(expression: AnyJsExpression) -> Option<bool> {
} else if let AnyJsLiteralExpression::JsStringLiteralExpression(string_literal) =
literal_expression
{
let text = string_literal.inner_string_text().ok()?;
text == "true"
let quoted_string = string_literal.inner_string_text().ok()?;
quoted_string.text() == "true"
} else {
false
}
Expand Down
2 changes: 1 addition & 1 deletion crates/rome_js_analyze/src/analyzers/a11y/use_html_lang.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ used by screen readers when no user default is specified."
}

fn is_valid_lang_attribute(attr: JsxAttribute) -> Option<()> {
if attr.is_value_undefined_or_null() {
if attr.is_value_null_or_undefined() {
return None;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,10 @@ impl Rule for UseKeyWithMouseEvents {
fn has_valid_focus_attributes(elem: &AnyJsxElement) -> bool {
if let Some(on_mouse_over_attribute) = elem.find_attribute_by_name("onMouseOver") {
if !elem.has_trailing_spread_prop(on_mouse_over_attribute) {
return elem
.find_attribute_by_name("onFocus")
.map_or(false, |it| !it.is_value_undefined_or_null());
return elem.find_attribute_by_name("onFocus").map_or(false, |it| {
!it.as_static_value()
.map_or(false, |value| value.is_null_or_undefined())
});
}
}
true
Expand All @@ -118,9 +119,10 @@ fn has_valid_focus_attributes(elem: &AnyJsxElement) -> bool {
fn has_valid_blur_attributes(elem: &AnyJsxElement) -> bool {
if let Some(on_mouse_attribute) = elem.find_attribute_by_name("onMouseOut") {
if !elem.has_trailing_spread_prop(on_mouse_attribute) {
return elem
.find_attribute_by_name("onBlur")
.map_or(false, |it| !it.is_value_undefined_or_null());
return elem.find_attribute_by_name("onBlur").map_or(false, |it| {
!it.as_static_value()
.map_or(false, |value| value.is_null_or_undefined())
});
}
}
true
Expand Down
7 changes: 4 additions & 3 deletions crates/rome_js_analyze/src/analyzers/a11y/use_valid_anchor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,8 @@ fn is_invalid_anchor(anchor_attribute: &JsxAttribute) -> Option<UseValidAnchorSt
AnyJsExpression::AnyJsLiteralExpression(
AnyJsLiteralExpression::JsStringLiteralExpression(string_literal),
) => {
let text = string_literal.inner_string_text().ok()?;
if text == "#" {
let quoted_string = string_literal.inner_string_text().ok()?;
if quoted_string.text() == "#" {
return Some(UseValidAnchorState::IncorrectHref(
string_literal.syntax().text_trimmed_range(),
));
Expand Down Expand Up @@ -327,7 +327,8 @@ fn is_invalid_anchor(anchor_attribute: &JsxAttribute) -> Option<UseValidAnchorSt
}
AnyJsxAttributeValue::AnyJsxTag(_) => {}
AnyJsxAttributeValue::JsxString(href_string) => {
let href_value = href_string.inner_string_text().ok()?;
let quoted_string = href_string.inner_string_text().ok()?;
let href_value = quoted_string.text();

// href="#" or href="javascript:void(0)"
if href_value == "#" || href_value.contains("javascript:") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,9 @@ impl CaseMismatchInfo {

fn compare_call_with_literal(call: JsCallExpression, literal: AnyJsExpression) -> Option<Self> {
let expected_case = ExpectedStringCase::from_call(&call)?;
let literal_value = literal.as_string_constant()?;
let expected_value = expected_case.convert(&literal_value);
let static_value = literal.as_static_value()?;
let literal_value = static_value.text();
let expected_value = expected_case.convert(literal_value);
if literal_value != expected_value {
Some(Self {
expected_case,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ impl Rule for NoRedundantAlt {
== "false"
}
AnyJsxAttributeValue::JsxString(aria_hidden) => {
aria_hidden.inner_string_text().ok()? == "false"
aria_hidden.inner_string_text().ok()?.text() == "false"
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,8 @@ fn is_valid_attribute_value(
let opening_element = jsx_element.opening_element().ok()?;
let maybe_attribute = opening_element.find_attribute_by_name("id").ok()?;
let child_attribute_value = maybe_attribute?.initializer()?.value().ok()?;
let is_valid = attribute_value.inner_text_value().ok()??.text()
== child_attribute_value.inner_text_value().ok()??.text();
let is_valid = attribute_value.as_static_value()?.text()
== child_attribute_value.as_static_value()?.text();
Some(is_valid)
})
.any(|x| x);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,9 @@ impl CallInfo {
.next()?
.ok()?
.as_any_js_expression()?
.as_string_constant()?;
.as_static_value()?
.as_string_constant()?
.to_string();
let radix = args
.next()?
.ok()?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ use rome_analyze::context::RuleContext;
use rome_analyze::{declare_rule, Rule, RuleDiagnostic};
use rome_console::markup;
use rome_js_syntax::jsx_ext::AnyJsxElement;
use rome_js_syntax::{AnyJsExpression, AnyJsLiteralExpression, AnyJsxAttributeValue};
use rome_rowan::{AstNode, AstNodeList, TextRange};
use rome_rowan::{AstNode, TextRange};

declare_rule! {
/// Enforce that interactive ARIA roles are not assigned to non-interactive HTML elements.
///
Expand Down Expand Up @@ -66,39 +66,11 @@ impl Rule for NoNoninteractiveElementToInteractiveRole {
let aria_roles = ctx.aria_roles();
if node.is_element() {
let role_attribute = node.find_attribute_by_name("role")?;
let role_attribute_value = role_attribute.initializer()?.value().ok()?;
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()
.and_then(|chunk| {
chunk
.as_js_template_chunk_element()
.and_then(|t| t.template_chunk_token().ok())
})
.map(|t| t.token_text_trimmed())
} else {
None
}
}
_ => None,
}
}

_ => None,
}?;
let role_attribute_static_value = role_attribute.as_static_value()?;
let role_attribute_value = role_attribute_static_value.text();
let element_name = node.name().ok()?.as_jsx_name()?.value_token().ok()?;
if aria_roles.is_not_interactive_element(element_name.text_trimmed())
&& aria_roles.is_role_interactive(role_attribute_value.text())
&& aria_roles.is_role_interactive(role_attribute_value)
{
// <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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ 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,
jsx_ext::AnyJsxElement, AnyJsxAttributeValue, JsNumberLiteralExpression,
JsStringLiteralExpression, JsUnaryExpression, TextRange,
};
use rome_rowan::{declare_node_union, AstNode, AstNodeList};
use rome_rowan::{declare_node_union, AstNode};

declare_rule! {
/// Enforce that `tabIndex` is not assigned to non-interactive HTML elements.
Expand Down Expand Up @@ -187,31 +187,5 @@ 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()))
Some(aria_roles.is_role_interactive(role_attribute_value.as_static_value()?.text()))
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@ use rome_aria::{roles::AriaRoleDefinition, AriaRoles};
use rome_console::markup;
use rome_diagnostics::Applicability;
use rome_js_syntax::{
jsx_ext::AnyJsxElement, AnyJsLiteralExpression, AnyJsxAttributeValue, JsxAttribute,
JsxAttributeList,
jsx_ext::AnyJsxElement, AnyJsxAttributeValue, JsxAttribute, JsxAttributeList,
};
use rome_rowan::{AstNode, BatchMutationExt};

Expand Down Expand Up @@ -88,7 +87,7 @@ impl Rule for NoRedundantRoles {
}

fn diagnostic(_: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
let binding = state.redundant_attribute_value.inner_text_value().ok()??;
let binding = state.redundant_attribute_value.as_static_value()?;
let role_attribute = binding.text();
let element = state.element_name.to_string();
Some(RuleDiagnostic::new(
Expand Down Expand Up @@ -133,27 +132,13 @@ fn get_explicit_role(
aria_roles: &AriaRoles,
role_attribute_value: &AnyJsxAttributeValue,
) -> Option<&'static dyn AriaRoleDefinition> {
let text = match role_attribute_value {
AnyJsxAttributeValue::JsxString(val) => val.inner_string_text().ok()?,
AnyJsxAttributeValue::JsxExpressionAttributeValue(val) => match val.expression().ok()? {
rome_js_syntax::AnyJsExpression::AnyJsLiteralExpression(
AnyJsLiteralExpression::JsStringLiteralExpression(expr),
) => expr.inner_string_text().ok()?,
rome_js_syntax::AnyJsExpression::JsTemplateExpression(expr) => {
let first_template_element = expr.elements().into_iter().next()?;
let first_element = first_template_element
.as_js_template_chunk_element()?
.template_chunk_token()
.ok()?;
first_element.token_text_trimmed()
}
_ => return None,
},
_ => return None,
};
let static_value = role_attribute_value.as_static_value()?;

// If a role attribute has multiple values, the first valid value (specified role) will be used.
// Check: https://www.w3.org/TR/2014/REC-wai-aria-implementation-20140320/#mapping_role
let explicit_role = text.split(' ').find_map(|role| aria_roles.get_role(role))?;
let explicit_role = static_value
.text()
.split(' ')
.find_map(|role| aria_roles.get_role(role))?;
Some(explicit_role)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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! {
Expand Down Expand Up @@ -76,42 +73,10 @@ 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()) {
let attribute_static_value = node.as_static_value()?;
let attribute_text = attribute_static_value.text();
if !aria_property.contains_correct_value(attribute_text) {
return Some(UseAriaProptypesState {
attribute_value_range,
allowed_values: aria_property.values(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,9 @@ impl Rule for UseValidLang {
if element_text.text_trimmed() == "html" {
let attribute = node.find_attribute_by_name("lang")?;
let attribute_value = attribute.initializer()?.value().ok()?;
let attribute_text = attribute_value.inner_text_value().ok()??;
let mut split_value = attribute_text.text().split('-');
let attribute_static_value = attribute_value.as_static_value()?;
let attribute_text = attribute_static_value.text();
let mut split_value = attribute_text.split('-');
match (split_value.next(), split_value.next()) {
(Some(language), Some(country)) => {
if !ctx.is_valid_iso_language(language) {
Expand Down
4 changes: 2 additions & 2 deletions crates/rome_js_analyze/src/aria_services.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ impl AriaServices {
continue
};
let initializer = initializer.value().ok()?;
let text = initializer.inner_text_value().ok()??;
let static_value = initializer.as_static_value()?;
// handle multiple values e.g. `<div class="wrapper document">`
let values = text.split(' ');
let values = static_value.text().split(' ');
let values = values.map(|s| s.to_string()).collect::<Vec<String>>();
defined_attributes.entry(name).or_insert(values);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ impl Rule for UseButtonType {
.as_js_string_literal_expression()?;

// case sensitive is important, <button> is different from <Button>
if first_argument.inner_string_text().ok()? == "button" {
if first_argument.inner_string_text().ok()?.text() == "button" {
return if let Some(props) = react_create_element.props.as_ref() {
let type_member = react_create_element.find_prop_by_name("type");
if let Some(member) = type_member {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ impl Rule for UseIframeTitle {

match attribute_value {
AnyJsxAttributeValue::JsxString(str) => {
let text = str.inner_string_text().ok()?;
let is_valid_string = !text.is_empty() && text != r#"``"#;
let quoted_string = str.inner_string_text().ok()?;
let is_valid_string = !quoted_string.is_empty() && quoted_string.text() != r#"``"#;
if is_valid_string {
return None;
}
Expand Down
Loading

0 comments on commit f6ada3a

Please sign in to comment.