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

refactor(rome_js_anaylze): introduce StaticValue enum for checking value of the JSX attributes or JS expressions #4342

Merged
merged 7 commits into from
Apr 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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