-
-
Notifications
You must be signed in to change notification settings - Fork 467
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(lint): useCollapsedElseIf (#160)
Co-authored-by: Victorien Elvinger <victorien@elvinger.fr>
- Loading branch information
Showing
15 changed files
with
996 additions
and
22 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
154 changes: 154 additions & 0 deletions
154
crates/rome_js_analyze/src/analyzers/nursery/use_collapsed_else_if.rs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,154 @@ | ||
use crate::JsRuleAction; | ||
use rome_analyze::{context::RuleContext, declare_rule, ActionCategory, Ast, Rule, RuleDiagnostic}; | ||
use rome_console::markup; | ||
use rome_diagnostics::Applicability; | ||
use rome_js_syntax::{AnyJsStatement, JsBlockStatement, JsElseClause, JsIfStatement}; | ||
use rome_rowan::{AstNode, AstNodeList, BatchMutationExt}; | ||
|
||
declare_rule! { | ||
/// Enforce using `else if` instead of nested `if` in `else` clauses. | ||
/// | ||
/// If an `if` statement is the only statement in the `else` block, it is often clearer to use an `else if` form. | ||
/// | ||
/// Source: https://eslint.org/docs/latest/rules/no-lonely-if | ||
/// | ||
/// ## Examples | ||
/// | ||
/// ### Invalid | ||
/// | ||
/// ```js,expect_diagnostic | ||
/// if (condition) { | ||
/// // ... | ||
/// } else { | ||
/// if (anotherCondition) { | ||
/// // ... | ||
/// } | ||
/// } | ||
/// ``` | ||
/// | ||
/// ```js,expect_diagnostic | ||
/// if (condition) { | ||
/// // ... | ||
/// } else { | ||
/// if (anotherCondition) { | ||
/// // ... | ||
/// } else { | ||
/// // ... | ||
/// } | ||
/// } | ||
/// ``` | ||
/// | ||
/// ### Valid | ||
/// | ||
/// ```js | ||
/// if (condition) { | ||
/// // ... | ||
/// } else if (anotherCondition) { | ||
/// // ... | ||
/// } | ||
/// ``` | ||
/// | ||
/// ```js | ||
/// if (condition) { | ||
/// // ... | ||
/// } else if (anotherCondition) { | ||
/// // ... | ||
/// } else { | ||
/// // ... | ||
/// } | ||
/// ``` | ||
/// | ||
/// ```js | ||
/// if (condition) { | ||
/// // ... | ||
/// } else { | ||
/// if (anotherCondition) { | ||
/// // ... | ||
/// } | ||
/// doSomething(); | ||
/// } | ||
/// ``` | ||
/// | ||
pub(crate) UseCollapsedElseIf { | ||
version: "next", | ||
name: "useCollapsedElseIf", | ||
recommended: false, | ||
} | ||
} | ||
|
||
pub(crate) struct RuleState { | ||
block_statement: JsBlockStatement, | ||
if_statement: JsIfStatement, | ||
} | ||
|
||
impl Rule for UseCollapsedElseIf { | ||
type Query = Ast<JsElseClause>; | ||
type State = RuleState; | ||
type Signals = Option<Self::State>; | ||
type Options = (); | ||
|
||
fn run(ctx: &RuleContext<Self>) -> Self::Signals { | ||
let else_clause = ctx.query(); | ||
let alternate = else_clause.alternate().ok()?; | ||
let AnyJsStatement::JsBlockStatement(block_statement) = alternate else { | ||
return None; | ||
}; | ||
let statements = block_statement.statements(); | ||
if statements.len() != 1 { | ||
return None; | ||
} | ||
if let AnyJsStatement::JsIfStatement(if_statement) = statements.first()? { | ||
Some(RuleState { | ||
block_statement, | ||
if_statement, | ||
}) | ||
} else { | ||
None | ||
} | ||
} | ||
|
||
fn diagnostic(_: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> { | ||
Some(RuleDiagnostic::new( | ||
rule_category!(), | ||
state.if_statement.syntax().text_range(), | ||
markup! { | ||
"This "<Emphasis>"if"</Emphasis>" statement can be collapsed into an "<Emphasis>"else if"</Emphasis>" statement." | ||
}, | ||
)) | ||
} | ||
|
||
fn action(ctx: &RuleContext<Self>, state: &Self::State) -> Option<JsRuleAction> { | ||
let RuleState { | ||
block_statement, | ||
if_statement, | ||
} = state; | ||
|
||
let has_comments = block_statement | ||
.l_curly_token() | ||
.ok()? | ||
.has_trailing_comments() | ||
|| if_statement.syntax().has_leading_comments() | ||
|| if_statement.syntax().has_trailing_comments() | ||
|| block_statement.r_curly_token().ok()?.has_leading_comments(); | ||
|
||
let applicability = if has_comments { | ||
Applicability::MaybeIncorrect | ||
} else { | ||
Applicability::Always | ||
}; | ||
|
||
let mut mutation = ctx.root().begin(); | ||
mutation.replace_node( | ||
AnyJsStatement::from(block_statement.clone()), | ||
if_statement.clone().into(), | ||
); | ||
|
||
Some(JsRuleAction { | ||
category: ActionCategory::QuickFix, | ||
applicability, | ||
message: markup! { "Use collapsed "<Emphasis>"else if"</Emphasis>" instead." } | ||
.to_owned(), | ||
mutation, | ||
}) | ||
} | ||
} |
101 changes: 101 additions & 0 deletions
101
crates/rome_js_analyze/tests/specs/nursery/useCollapsedElseIf/invalid.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
/** | ||
* Safe fixes: | ||
*/ | ||
|
||
if (condition) { | ||
// ... | ||
} else { | ||
if (anotherCondition) { | ||
// ... | ||
} | ||
} | ||
|
||
if (condition) { | ||
// ... | ||
} else { | ||
if (anotherCondition) { | ||
// ... | ||
} else { | ||
// ... | ||
} | ||
} | ||
|
||
/** | ||
* Suggested fixes: | ||
*/ | ||
|
||
if (condition) { | ||
// ... | ||
} else { // Comment | ||
if (anotherCondition) { | ||
// ... | ||
} | ||
} | ||
|
||
if (condition) { | ||
// ... | ||
} else { | ||
// Comment | ||
if (anotherCondition) { | ||
// ... | ||
} | ||
} | ||
|
||
if (condition) { | ||
// ... | ||
} else { | ||
if (anotherCondition) { | ||
// ... | ||
} // Comment | ||
} | ||
|
||
if (condition) { | ||
// ... | ||
} else { | ||
if (anotherCondition) { | ||
// ... | ||
} | ||
// Comment | ||
} | ||
|
||
if (condition) { | ||
// ... | ||
} else { // Comment | ||
if (anotherCondition) { | ||
// ... | ||
} else { | ||
// ... | ||
} | ||
} | ||
|
||
if (condition) { | ||
// ... | ||
} else { | ||
// Comment | ||
if (anotherCondition) { | ||
// ... | ||
} else { | ||
// ... | ||
} | ||
} | ||
|
||
if (condition) { | ||
// ... | ||
} else { | ||
if (anotherCondition) { | ||
// ... | ||
} else { | ||
// ... | ||
} // Comment | ||
} | ||
|
||
if (condition) { | ||
// ... | ||
} else { | ||
if (anotherCondition) { | ||
// ... | ||
} else { | ||
// ... | ||
} | ||
// Comment | ||
} |
Oops, something went wrong.