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

fix(rome_js_analyze): handle default clause in useSingleCaseStetement #4069

Merged
merged 1 commit into from
Feb 15, 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
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,14 @@ use rome_analyze::{context::RuleContext, declare_rule, ActionCategory, Ast, Rule
use rome_console::markup;
use rome_diagnostics::Applicability;
use rome_js_factory::make;
use rome_js_syntax::{
AnyJsStatement, JsCaseClause, JsCaseClauseFields, JsSyntaxToken, TriviaPieceKind, T,
};
use rome_rowan::{AstNode, AstNodeList, BatchMutationExt, TriviaPiece};
use rome_js_syntax::{AnyJsStatement, AnyJsSwitchClause, TriviaPieceKind, T};
use rome_rowan::{AstNode, AstNodeList, BatchMutationExt};

use crate::JsRuleAction;

declare_rule! {
/// Enforces case clauses have a single statement, emits a quick fix wrapping
/// the statements in a block
/// Enforces switch clauses have a single statement, emits a quick fix wrapping
/// the statements in a block.
///
/// ## Examples
///
Expand Down Expand Up @@ -47,104 +45,53 @@ declare_rule! {
}

impl Rule for UseSingleCaseStatement {
type Query = Ast<JsCaseClause>;
type Query = Ast<AnyJsSwitchClause>;
type State = ();
type Signals = Option<Self::State>;
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Option<Self::State> {
let n = ctx.query();
if n.consequent().len() > 1 {
let switch_clause = ctx.query();
if switch_clause.consequent().len() > 1 {
Some(())
} else {
None
}
}

fn diagnostic(ctx: &RuleContext<Self>, _: &Self::State) -> Option<RuleDiagnostic> {
let n = ctx.query();

let switch_clause = ctx.query();
Some(RuleDiagnostic::new(
rule_category!(),
n.consequent().range(),
switch_clause.consequent().range(),
markup! {
"A switch case should only have a single statement. If you want more, then wrap it in a block."
"A "<Emphasis>"switch clause"</Emphasis>" should only have a single statement."
},
))
}

fn action(ctx: &RuleContext<Self>, _: &Self::State) -> Option<JsRuleAction> {
let n = ctx.query();
let switch_clause = ctx.query();
let clause_token = switch_clause.clause_token().ok()?;
let colon_token = switch_clause.colon_token().ok()?;
let consequent = switch_clause.consequent();
let new_colon_token = colon_token.with_trailing_trivia(iter::empty());
let new_consequent = make::js_statement_list(Some(AnyJsStatement::JsBlockStatement(
make::js_block_statement(
make::token(T!['{'])
.with_leading_trivia(Some((TriviaPieceKind::Whitespace, " ")))
.with_trailing_trivia_pieces(colon_token.trailing_trivia().pieces()),
consequent.clone(),
make::token(T!['}']).with_leading_trivia_pieces(clause_token.indentation_trivia()),
Conaclos marked this conversation as resolved.
Show resolved Hide resolved
),
)));
let mut mutation = ctx.root().begin();

let JsCaseClauseFields {
case_token,
colon_token,
consequent,
..
} = n.as_fields();

// Move the trailing trivia of the colon token to the opening bracket token,
// this ensure comments stay in the right place
let mut opening_token = String::from(" {");
let mut trailing = Vec::new();

if let Ok(token) = colon_token {
for piece in token.trailing_trivia().pieces() {
opening_token.push_str(piece.text());
trailing.push(TriviaPiece::new(piece.kind(), piece.text_len()));
}
}

// Copy the leading trivia of the case token on the closing bracket token
// up to the first newline to align the indentation level
let mut closing_token = String::new();
let mut leading = Vec::new();

if let Ok(token) = case_token {
let leading_trivia = token.leading_trivia().pieces();
let num_pieces = leading_trivia.len();
let skip_count = leading_trivia
.rev()
.position(|piece| piece.is_newline())
.and_then(|index| num_pieces.checked_sub(index + 1))
.unwrap_or(0);

for piece in token.leading_trivia().pieces().skip(skip_count) {
closing_token.push_str(piece.text());
leading.push(TriviaPiece::new(piece.kind(), piece.text_len()));
}
}

closing_token.push('}');

let node = n
.clone()
.with_consequent(make::js_statement_list(iter::once(
AnyJsStatement::JsBlockStatement(make::js_block_statement(
JsSyntaxToken::new_detached(
T!['{'],
&opening_token,
[TriviaPiece::new(TriviaPieceKind::Whitespace, 1)],
trailing,
),
consequent,
JsSyntaxToken::new_detached(T!['}'], &closing_token, leading, []),
)),
)));

let node = if let Ok(colon_token) = n.colon_token() {
node.with_colon_token(colon_token.with_trailing_trivia(iter::empty()))
} else {
node
};

mutation.replace_node(n.clone(), node);

mutation.replace_token_discard_trivia(colon_token, new_colon_token);
mutation.replace_node_discard_trivia(consequent, new_consequent);
Some(JsRuleAction {
category: ActionCategory::QuickFix,
applicability: Applicability::MaybeIncorrect,
message: markup! { "Wrap the statements in a block" }.to_owned(),
message: markup! { "Wrap the statements in a block." }.to_owned(),
mutation,
})
}
Expand Down
49 changes: 47 additions & 2 deletions crates/rome_js_analyze/tests/specs/style/useSingleCaseStatement.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,17 @@ switch (foo) {
foo;
}

switch (foo) { case false: let foo = ''; foo; }

switch (foo) {
// comment
case false :
case false:
let foo = '';
foo;
}

switch (foo) {
case false : // comment
case false: // comment
let foo = '';
foo;
}
Expand All @@ -40,3 +42,46 @@ switch (foo) {
switch (foo) {
case true:
}

switch (foo) {
case true:
default:
let foo = '';
foo;
}

switch (foo) {
// comment
default:
let foo = '';
foo;
}

switch (foo) {
default: // comment
let foo = '';
foo;
}

switch (foo) {
default
/* comment */ :
let foo = '';
foo;
}

switch (foo) {
case true:
default:
'yes';
}

switch (foo) {
default: {
// empty
}
}

switch (foo) {
default:
}
Loading