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

refactor(rome_syntax): improve inner_string_text #4731

Merged
merged 1 commit into from
Jul 28, 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
5 changes: 1 addition & 4 deletions crates/rome_js_analyze/src/analyzers/a11y/no_blank_target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,7 @@ impl Rule for NoBlankTarget {
let target_attribute = node.find_attribute_by_name("target")?;
let rel_attribute = node.find_attribute_by_name("rel");

if target_attribute
.as_static_value()?
.is_string_constant("_blank")
{
if target_attribute.as_static_value()?.text() == "_blank" {
match rel_attribute {
None => {
if !node.has_trailing_spread_prop(target_attribute.clone()) {
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 @@ -146,7 +146,7 @@ fn has_type_image_attribute(element: &AnyJsxElement) -> bool {
.map_or(false, |attribute| {
attribute
.as_static_value()
.map_or(false, |value| value.is_string_constant("image"))
.map_or(false, |value| value.text() == "image")
})
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,12 +175,7 @@ fn collect_control_characters_from_expression(
callee: &AnyJsExpression,
js_call_arguments: &JsCallArguments,
) -> Option<Vec<String>> {
let js_identifier = match callee {
AnyJsExpression::JsIdentifierExpression(js_identifier) => js_identifier,
_ => return None,
};

if js_identifier.name().ok()?.has_name("RegExp") {
if callee.as_js_reference_identifier()?.has_name("RegExp") {
let mut args = js_call_arguments.args().iter();
let raw_pattern = args
.next()
Expand Down
27 changes: 13 additions & 14 deletions crates/rome_js_analyze/src/analyzers/nursery/no_self_assign.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use rome_analyze::{context::RuleContext, declare_rule, Ast, Rule, RuleDiagnostic};
use rome_console::markup;
use rome_js_syntax::{
AnyJsArrayAssignmentPatternElement, AnyJsArrayElement, AnyJsAssignment, AnyJsAssignmentPattern,
AnyJsExpression, AnyJsLiteralExpression, AnyJsName, AnyJsObjectAssignmentPatternMember,
AnyJsObjectMember, JsAssignmentExpression, JsAssignmentOperator, JsCallExpression,
JsComputedMemberAssignment, JsComputedMemberExpression, JsIdentifierAssignment,
JsIdentifierExpression, JsLanguage, JsName, JsPrivateName, JsReferenceIdentifier,
inner_string_text, AnyJsArrayAssignmentPatternElement, AnyJsArrayElement, AnyJsAssignment,
AnyJsAssignmentPattern, AnyJsExpression, AnyJsLiteralExpression, AnyJsName,
AnyJsObjectAssignmentPatternMember, AnyJsObjectMember, JsAssignmentExpression,
JsAssignmentOperator, JsCallExpression, JsComputedMemberAssignment, JsComputedMemberExpression,
JsIdentifierAssignment, JsLanguage, JsName, JsPrivateName, JsReferenceIdentifier,
JsStaticMemberAssignment, JsStaticMemberExpression, JsSyntaxToken,
};
use rome_rowan::{
Expand Down Expand Up @@ -421,7 +421,9 @@ impl AnyJsAssignmentExpressionLikeIterator {
fn from_computed_member_assignment(source: JsComputedMemberAssignment) -> SyntaxResult<Self> {
Ok(Self {
source_member: source.member().and_then(|expression| match expression {
AnyJsExpression::JsIdentifierExpression(node) => Ok(AnyNameLike::from(node)),
AnyJsExpression::JsIdentifierExpression(node) => {
Ok(AnyNameLike::from(node.name()?))
}
_ => Err(SyntaxError::MissingRequiredChild),
})?,
source_object: source.object()?,
Expand All @@ -433,7 +435,9 @@ impl AnyJsAssignmentExpressionLikeIterator {
fn from_computed_member_expression(source: JsComputedMemberExpression) -> SyntaxResult<Self> {
Ok(Self {
source_member: source.member().and_then(|expression| match expression {
AnyJsExpression::JsIdentifierExpression(node) => Ok(AnyNameLike::from(node)),
AnyJsExpression::JsIdentifierExpression(node) => {
Ok(AnyNameLike::from(node.name()?))
}
_ => Err(SyntaxError::MissingRequiredChild),
})?,
source_object: source.object()?,
Expand Down Expand Up @@ -539,7 +543,7 @@ enum AnyAssignmentLike {
}

declare_node_union! {
pub(crate) AnyNameLike = AnyJsName | JsReferenceIdentifier | JsIdentifierExpression | AnyJsLiteralExpression
pub(crate) AnyNameLike = AnyJsName | JsReferenceIdentifier | AnyJsLiteralExpression
}

declare_node_union! {
Expand Down Expand Up @@ -715,11 +719,6 @@ impl TryFrom<(AnyNameLike, AnyNameLike)> for IdentifiersLike {
AnyNameLike::JsReferenceIdentifier(right),
) => Ok(Self::References(left, right)),

(
AnyNameLike::JsIdentifierExpression(left),
AnyNameLike::JsIdentifierExpression(right),
) => Ok(Self::References(left.name()?, right.name()?)),

(
AnyNameLike::AnyJsLiteralExpression(left),
AnyNameLike::AnyJsLiteralExpression(right),
Expand Down Expand Up @@ -808,7 +807,7 @@ fn with_same_identifiers(identifiers_like: &IdentifiersLike) -> Option<()> {
},
};

if left_value.text_trimmed() == right_value.text_trimmed() {
if inner_string_text(&left_value) == inner_string_text(&right_value) {
Some(())
} else {
None
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ impl Rule for UseLiteralEnumMembers {
};
if let Ok(name) = enum_member.name() {
if let Some(name) = name.name() {
enum_member_names.insert(name.text().to_string());
enum_member_names.insert(name.to_string());
}
}
}
Expand Down Expand Up @@ -208,11 +208,8 @@ fn is_enum_member_reference(
(move || {
// Allow reference to previous member name namespaced by the enum name
let object = expr.object().ok()?.omit_parentheses();
let object = object.as_js_identifier_expression()?;
Some(
object.name().ok()?.has_name(enum_name)
&& enum_member_names.contains(expr.member_name()?.text()),
)
let object = object.as_js_reference_identifier()?;
Some(object.has_name(enum_name) && enum_member_names.contains(expr.member_name()?.text()))
})()
.unwrap_or_default()
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ impl Rule for UseLiteralKeys {
if member.value().ok()?.kind() == JsSyntaxKind::JS_STRING_LITERAL {
let name = member.name().ok()?;
if is_js_ident(&name) {
return Some((member.range(), name));
return Some((member.range(), name.to_string()));
}
}
return None;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@ use rome_analyze::{declare_rule, ActionCategory, Ast, Rule, RuleDiagnostic};
use rome_console::markup;
use rome_diagnostics::Applicability;
use rome_js_factory::make;
use rome_js_syntax::{
inner_text, AnyJsExpression, AnyJsLiteralExpression, JsSyntaxKind, TsEnumMember,
};
use rome_js_syntax::{AnyJsExpression, AnyJsLiteralExpression, JsSyntaxKind, TsEnumMember};
use rome_rowan::{AstNode, BatchMutationExt, Direction};

declare_rule! {
Expand Down Expand Up @@ -119,9 +117,8 @@ impl Rule for UseEnumInitializers {
))
}
AnyJsLiteralExpression::JsStringLiteralExpression(expr) => {
let prev_enum_delim_val = expr.value_token().ok()?;
let prev_enum_val = inner_text(&prev_enum_delim_val);
if prev_name.text() == prev_enum_val {
let prev_enum_val = expr.inner_string_text().ok()?;
if prev_name.name() == Some(prev_enum_val) {
let enum_name = enum_member.name().ok()?.text();
Some(AnyJsLiteralExpression::JsStringLiteralExpression(
make::js_string_literal_expression(make::js_string_literal(&enum_name)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ use rome_js_syntax::{
AnyJsClassMemberName, JsClassMemberList, JsGetterClassMember, JsMethodClassMember,
JsPropertyClassMember, JsSetterClassMember, JsStaticModifier, JsSyntaxList, TextRange,
};
use rome_rowan::AstNodeList;
use rome_rowan::{declare_node_union, AstNode};
use rome_rowan::{AstNodeList, TokenText};

declare_rule! {
/// Disallow duplicate class members.
Expand Down Expand Up @@ -91,7 +91,7 @@ declare_rule! {
}
}

fn get_member_name(node: &AnyJsClassMemberName) -> Option<String> {
fn get_member_name(node: &AnyJsClassMemberName) -> Option<TokenText> {
match node {
AnyJsClassMemberName::JsLiteralMemberName(node) => node.name().ok(),
_ => None,
Expand Down Expand Up @@ -186,7 +186,7 @@ impl Rule for NoDuplicateClassMembers {
let member_definition = AnyClassMemberDefinition::cast_ref(member.syntax())?;
let member_name_node = member_definition.name()?;
let member_state = MemberState {
name: get_member_name(&member_name_node)?,
name: get_member_name(&member_name_node)?.to_string(),
is_static: is_static_member(member_definition.modifiers_list()),
};

Expand Down Expand Up @@ -215,7 +215,7 @@ impl Rule for NoDuplicateClassMembers {
state.range(),
format!(
"Duplicate class member name {:?}",
get_member_name(&state.name()?)?
get_member_name(&state.name()?)?.text()
),
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use rome_js_syntax::{
use rome_js_syntax::{
JsMethodObjectMember, JsPropertyObjectMember, JsShorthandPropertyObjectMember, TextRange,
};
use rome_rowan::{AstNode, BatchMutationExt};
use rome_rowan::{AstNode, BatchMutationExt, TokenText};
use std::cmp::Ordering;
use std::collections::HashMap;
use std::fmt::Display;
Expand Down Expand Up @@ -69,7 +69,7 @@ enum MemberDefinition {
ShorthandProperty(JsShorthandPropertyObjectMember),
}
impl MemberDefinition {
fn name(&self) -> Option<String> {
fn name(&self) -> Option<TokenText> {
match self {
MemberDefinition::Getter(getter) => {
getter.name().ok()?.as_js_literal_member_name()?.name().ok()
Expand All @@ -86,9 +86,14 @@ impl MemberDefinition {
.as_js_literal_member_name()?
.name()
.ok(),
MemberDefinition::ShorthandProperty(shorthand_property) => {
Some(shorthand_property.name().ok()?.text())
}
MemberDefinition::ShorthandProperty(shorthand_property) => Some(
shorthand_property
.name()
.ok()?
.value_token()
.ok()?
.token_text_trimmed(),
),
}
}

Expand Down Expand Up @@ -210,7 +215,7 @@ impl Rule for NoDuplicateObjectKeys {
fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let node = ctx.query();

let mut defined_properties: HashMap<String, DefinedProperty> = HashMap::new();
let mut defined_properties: HashMap<TokenText, DefinedProperty> = HashMap::new();
let mut signals: Self::Signals = Vec::new();

for member_definition in node
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ impl Rule for NoPrototypeBuiltins {
let member_name_text = member_name.text();
return is_prototype_builtins(member_name_text).then_some(RuleState {
prototype_builtins_method_name: member_name_text.to_string(),
text_range: member_name.token().text_trimmed_range(),
text_range: member_name.range(),
});
}
None
Expand Down
10 changes: 5 additions & 5 deletions crates/rome_js_analyze/src/aria_services.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ mod tests {
use crate::aria_services::AriaServices;
use rome_aria::{AriaProperties, AriaRoles};
use rome_js_factory::make::{
ident, jsx_attribute, jsx_attribute_initializer_clause, jsx_attribute_list, jsx_ident,
jsx_name, jsx_string, token,
ident, jsx_attribute, jsx_attribute_initializer_clause, jsx_attribute_list, jsx_name,
jsx_string, jsx_string_literal, token,
};
use rome_js_syntax::{AnyJsxAttribute, AnyJsxAttributeName, AnyJsxAttributeValue, T};
use std::sync::Arc;
Expand All @@ -88,8 +88,8 @@ mod tests {
jsx_attribute(AnyJsxAttributeName::JsxName(jsx_name(ident("class"))))
.with_initializer(jsx_attribute_initializer_clause(
token(T![=]),
AnyJsxAttributeValue::JsxString(jsx_string(jsx_ident(
"\"wrapper document\"",
AnyJsxAttributeValue::JsxString(jsx_string(jsx_string_literal(
"wrapper document",
))),
))
.build(),
Expand All @@ -98,7 +98,7 @@ mod tests {
jsx_attribute(AnyJsxAttributeName::JsxName(jsx_name(ident("role"))))
.with_initializer(jsx_attribute_initializer_clause(
token(T![=]),
AnyJsxAttributeValue::JsxString(jsx_string(jsx_ident("\"article\""))),
AnyJsxAttributeValue::JsxString(jsx_string(jsx_string_literal("article"))),
))
.build(),
),
Expand Down
4 changes: 2 additions & 2 deletions crates/rome_js_analyze/src/react.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ pub(crate) fn is_react_call_api(
let expr = expression.omit_parentheses();
if let Some(callee) = AnyJsMemberExpression::cast_ref(expr.syntax()) {
let Some(object) = callee.object().ok() else { return false };
let Some(reference) = object.omit_parentheses().as_reference_identifier() else { return false };
let Some(reference) = object.omit_parentheses().as_js_reference_identifier() else { return false };
let Some(member_name) = callee.member_name() else { return false };
if member_name.text() != api_name {
return false;
Expand All @@ -203,7 +203,7 @@ pub(crate) fn is_react_call_api(
};
}

if let Some(ident) = expr.as_reference_identifier() {
if let Some(ident) = expr.as_js_reference_identifier() {
return model
.binding(&ident)
.and_then(|it| is_named_react_export(it, lib, api_name))
Expand Down
10 changes: 4 additions & 6 deletions crates/rome_js_analyze/src/react/hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::collections::{HashMap, HashSet};

use rome_js_semantic::{Capture, Closure, ClosureExtensions, SemanticModel};
use rome_js_syntax::{
binding_ext::AnyJsIdentifierBinding, static_value::StaticStringValue, AnyJsExpression,
binding_ext::AnyJsIdentifierBinding, static_value::StaticValue, AnyJsExpression,
AnyJsMemberExpression, JsArrayBindingPattern, JsArrayBindingPatternElementList,
JsArrowFunctionExpression, JsCallExpression, JsFunctionExpression, JsVariableDeclarator,
TextRange,
Expand Down Expand Up @@ -140,16 +140,14 @@ pub(crate) fn react_hook_with_dependency(
model: &SemanticModel,
) -> Option<ReactCallWithDependencyResult> {
let expression = call.callee().ok()?;
let name = if let AnyJsExpression::JsIdentifierExpression(identifier) = expression.clone() {
Some(StaticStringValue::Unquoted(
identifier.name().ok()?.value_token().ok()?,
))
let name = if let Some(identifier) = expression.as_js_reference_identifier() {
Some(StaticValue::String(identifier.value_token().ok()?))
} else if let Some(member_expr) = AnyJsMemberExpression::cast_ref(expression.syntax()) {
Some(member_expr.member_name()?)
} else {
None
}?;
let function_name_range = name.token().text_trimmed_range();
let function_name_range = name.range();
let name = name.text();

// check if the hooks api is imported from the react library
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ impl Rule for NoGlobalObjectCalls {
model
.binding(&reference)
.is_none()
.then_some((non_callable, name.token().text_trimmed_range()))
.then_some((non_callable, name.range()))
}

fn diagnostic(
Expand Down
Loading
Loading