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

feat(rome_js_analyze): Implement noStringCaseMismatch #3794

Merged
merged 11 commits into from
Nov 22, 2022
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
1 change: 1 addition & 0 deletions crates/rome_diagnostics_categories/src/categories.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ define_dategories! {
"lint/nursery/noHeaderScope": "https://docs.rome.tools/lint/rules/noHeaderScope",
"lint/nursery/noInvalidConstructorSuper": "https://docs.rome.tools/lint/rules/noInvalidConstructorSuper",
"lint/nursery/noPrecisionLoss": "https://docs.rome.tools/lint/rules/noPrecisionLoss",
"lint/nursery/noStringCaseMismatch": "https://docs.rome.tools/lint/rules/noStringCaseMismatch",
"lint/nursery/noUnsafeFinally": "https://docs.rome.tools/lint/rules/noUnsafeFinally",
"lint/nursery/useCamelCase": "https://docs.rome.tools/lint/rules/useCamelCase",
"lint/nursery/useConst":"https://docs.rome.tools/lint/rules/useConst",
Expand Down
3 changes: 2 additions & 1 deletion crates/rome_js_analyze/src/analyzers/nursery.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,207 @@
use rome_analyze::context::RuleContext;
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::*;
use rome_rowan::{declare_node_union, AstNode, AstSeparatedList, BatchMutationExt};

use crate::JsRuleAction;

declare_rule! {
/// Disallow comparison of expressions modifying the string case with non-compliant value.
///
/// ## Examples
///
/// ### Invalid
///
/// ```js,expect_diagnostic
/// if (s.toUpperCase() === "Abc") {}
/// ```
///
/// ```js,expect_diagnostic
/// while (s.toLowerCase() === "Abc") {}
/// ```
/// ### Valid
///
/// ```js
/// if (s.toUpperCase() === "ABC") {}
/// while (s.toLowerCase() === "abc") {}
/// for (;s.toLocaleLowerCase() === "ABC";) {}
/// while (s.toLocaleUpperCase() === "abc") {}
/// for (let s = "abc"; s === "abc"; s = s.toUpperCase()) {}
/// ```
pub(crate) NoStringCaseMismatch {
version: "11.0.0",
name: "noStringCaseMismatch",
recommended: false,
}
}

impl Rule for NoStringCaseMismatch {
type Query = Ast<QueryCandidate>;
type State = CaseMismatchInfo;
type Signals = Vec<Self::State>;
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Vec<Self::State> {
let query = ctx.query();
match query {
QueryCandidate::JsBinaryExpression(expr) => CaseMismatchInfo::from_binary_expr(expr)
.into_iter()
.collect(),
QueryCandidate::JsSwitchStatement(stmt) => CaseMismatchInfo::from_switch_stmt(stmt),
}
}

fn diagnostic(ctx: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
let query = ctx.query();
let mut diagnostic = match query {
QueryCandidate::JsBinaryExpression(expr) => RuleDiagnostic::new(
rule_category!(),
expr.range(),
markup! { "This expression always returns false." },
),
QueryCandidate::JsSwitchStatement(..) => RuleDiagnostic::new(
rule_category!(),
state.literal.range(),
markup! { "This case will never match." },
),
};
diagnostic = diagnostic
.description("This expression always returns false, because the string is converted and will never match")
.detail(
state.call.range(),
markup! {
"This call convert the string to " { state.expected_case.description() }
},
)
.detail(
state.literal.range(),
markup! {
"... but this value is not in " { state.expected_case.description() }
},
);
Some(diagnostic)
}

fn action(ctx: &RuleContext<Self>, state: &Self::State) -> Option<JsRuleAction> {
let mut mutation = ctx.root().begin();
mutation.replace_node(
state.literal.clone(),
JsAnyExpression::JsAnyLiteralExpression(
JsAnyLiteralExpression::JsStringLiteralExpression(
make::js_string_literal_expression(make::js_string_literal(
&state.expected_value,
)),
),
),
);
Some(JsRuleAction {
mutation,
message: markup! {"Use "<Emphasis>{state.expected_case.description()}</Emphasis>" string value."}.to_owned(),
category: ActionCategory::QuickFix,
applicability: Applicability::MaybeIncorrect,
})
}
}

declare_node_union! {
pub(crate) QueryCandidate = JsBinaryExpression | JsSwitchStatement
}

pub(crate) struct CaseMismatchInfo {
expected_case: ExpectedStringCase,
expected_value: String,
call: JsCallExpression,
literal: JsAnyExpression,
}

impl CaseMismatchInfo {
fn from_binary_expr(expr: &JsBinaryExpression) -> Option<Self> {
let (left, right) = match expr.as_fields() {
JsBinaryExpressionFields {
left: Ok(left),
right: Ok(right),
operator_token: Ok(op),
} if matches!(op.kind(), JsSyntaxKind::EQ2 | JsSyntaxKind::EQ3) => (left, right),
_ => return None,
};
let (call, literal) = match (left, right) {
(JsAnyExpression::JsCallExpression(call), other)
| (other, JsAnyExpression::JsCallExpression(call)) => (call, other),
_ => return None,
};
Self::compare_call_with_literal(call, literal)
}

fn from_switch_stmt(stmt: &JsSwitchStatement) -> Vec<Self> {
match stmt.as_fields() {
JsSwitchStatementFields {
discriminant: Ok(JsAnyExpression::JsCallExpression(call)),
cases,
..
} => cases
.into_iter()
.flat_map(|case| case.as_js_case_clause().and_then(|case| case.test().ok()))
.flat_map(|test| Self::compare_call_with_literal(call.clone(), test))
.collect(),
_ => Vec::new(),
}
}

fn compare_call_with_literal(call: JsCallExpression, literal: JsAnyExpression) -> Option<Self> {
let expected_case = ExpectedStringCase::from_call(&call)?;
let literal_value = literal.as_string_constant()?;
let expected_value = expected_case.convert(&literal_value);
if literal_value != expected_value {
Some(Self {
expected_case,
expected_value,
call,
literal,
})
} else {
None
}
}
}

enum ExpectedStringCase {
Upper,
Lower,
}

impl ExpectedStringCase {
fn from_call(call: &JsCallExpression) -> Option<Self> {
if call.arguments().ok()?.args().len() != 0 {
return None;
}

let callee = call.callee().ok()?;
let member_expr = JsAnyMemberExpression::cast_ref(callee.syntax())?;
if member_expr.has_member_name("toLowerCase") {
return Some(Self::Lower);
}

if member_expr.has_member_name("toUpperCase") {
return Some(Self::Upper);
}

None
}

fn convert(&self, s: &str) -> String {
match self {
ExpectedStringCase::Upper => s.to_uppercase(),
ExpectedStringCase::Lower => s.to_lowercase(),
}
}

fn description(&self) -> &str {
match self {
ExpectedStringCase::Upper => "upper case",
ExpectedStringCase::Lower => "lower case",
}
}
}
35 changes: 35 additions & 0 deletions crates/rome_js_analyze/tests/specs/nursery/noStringCaseMismatch.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// valid
s.toUpperCase() === "ABC";
s.toLowerCase() === "abc";
s.toUpperCase() === "20";
s.toLowerCase() === "20";
s.toLowerCase() === `eFg${12}`;
s.toLowerCase() == `eFg${12}`;

// invalid
const a = s.toUpperCase() === "abc";
const c = s.toUpperCase() == "abc";
const a2 = "abc" === s.toUpperCase();
if (s.toUpperCase() === "abc" && c == d && e == f) {}
while (s.toUpperCase() === "abc" && c == d && e == f) {}
while (s.toUpperCase() === "abc") {}
let b = s.toLowerCase() === `eFg`;
do {} while (s.toLowerCase() === "ABC");
for (; s.toLowerCase() === "ABC"; ) {}

switch (s.toUpperCase()) {
case "ABC":
case "abc":
case "aBc":
default:
}

for (; s["toLowerCase"]() === "ABC"; ) {}
for (; s[`toUpperCase`]() === "abc"; ) {}

switch (s["toLowerCase"]()) {
case "Abc":
case "aBc":
case "abC":
default:
}
Loading