Skip to content

Commit

Permalink
feat(linter): add fixer for no-empty (#5276)
Browse files Browse the repository at this point in the history
adds a suggestion for `no-empty` eslint lint rule
  • Loading branch information
camc314 committed Aug 28, 2024
1 parent 7fa2fa3 commit 6633972
Showing 1 changed file with 96 additions and 9 deletions.
105 changes: 96 additions & 9 deletions crates/oxc_linter/src/rules/eslint/no_empty.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use oxc_ast::AstKind;
use oxc_ast::{ast::BlockStatement, AstKind};
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;
Expand Down Expand Up @@ -32,7 +32,7 @@ declare_oxc_lint!(
/// ```
NoEmpty,
restriction,
pending // a suggestion could be added to remove the empty statement
suggestion
);

impl Rule for NoEmpty {
Expand All @@ -49,33 +49,96 @@ impl Rule for NoEmpty {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
match node.kind() {
AstKind::BlockStatement(block) if block.body.is_empty() => {
if self.allow_empty_catch
&& matches!(ctx.nodes().parent_kind(node.id()), Some(AstKind::CatchClause(_)))
{
let parent = ctx.nodes().parent_kind(node.id());
if self.allow_empty_catch && matches!(parent, Some(AstKind::CatchClause(_))) {
return;
}

if ctx.semantic().trivias().has_comments_between(block.span) {
return;
}
ctx.diagnostic(no_empty_diagnostic("block", block.span));
ctx.diagnostic_with_suggestion(no_empty_diagnostic("block", block.span), |fixer| {
if let Some(parent) = parent {
if matches!(parent, AstKind::CatchClause(_)) {
fixer.noop()
} else {
fixer.delete(&parent)
}
} else {
fixer.noop()
}
});
}
// The visitor does not visit the `BlockStatement` inside the `FinallyClause`.
// See `Visit::visit_finally_clause`.
AstKind::FinallyClause(finally_clause) if finally_clause.body.is_empty() => {
if ctx.semantic().trivias().has_comments_between(finally_clause.span) {
return;
}
ctx.diagnostic(no_empty_diagnostic("block", finally_clause.span));
ctx.diagnostic_with_suggestion(
no_empty_diagnostic("block", finally_clause.span),
|fixer| {
let parent = ctx
.nodes()
.parent_kind(node.id())
.expect("finally clauses must have a parent node");

let AstKind::TryStatement(parent) = parent else {
unreachable!("finally clauses must be children of a try statement");
};

// if there's no `catch`, we can't remove the `finally` block
if parent.handler.is_none() {
return fixer.noop();
}

if let Some(finally_kw_start) = find_finally_start(ctx, finally_clause) {
fixer.delete_range(Span::new(finally_kw_start, finally_clause.span.end))
} else {
fixer.noop()
}
},
);
}
AstKind::SwitchStatement(switch) if switch.cases.is_empty() => {
ctx.diagnostic(no_empty_diagnostic("switch", switch.span));
ctx.diagnostic_with_suggestion(
no_empty_diagnostic("switch", switch.span),
|fixer| fixer.delete(switch),
);
}
_ => {}
}
}
}

fn find_finally_start(ctx: &LintContext, finally_clause: &BlockStatement) -> Option<u32> {
let src = ctx.source_text();
let finally_start = finally_clause.span.start as usize - 1;
let mut start = finally_start;

let src_chars: Vec<char> = src.chars().collect();

while start > 0 {
if let Some(&ch) = src_chars.get(start) {
if !ch.is_whitespace() {
if ch == 'y'
&& "finally".chars().rev().skip(1).all(|c| {
start -= 1;
src_chars.get(start) == Some(&c)
})
{
#[allow(clippy::cast_possible_truncation)]
return Some(start as u32);
}
return None;
}
}
start = start.saturating_sub(1);
}

None
}

#[test]
fn test() {
use serde_json::json;
Expand Down Expand Up @@ -125,5 +188,29 @@ fn test() {
("try { foo(); } catch (ex) {} finally {}", None),
];

Tester::new(NoEmpty::NAME, pass, fail).test_and_snapshot();
let fix = vec![
("try {} catch (ex) {throw ex}", "", None),
(
"try { foo() } catch (ex) {throw ex} finally {}",
"try { foo() } catch (ex) {throw ex} ",
None,
),
// we can't fix this because removing the `catch` block would change the semantics of the code
("try { foo() } catch (ex) {}", "try { foo() } catch (ex) {}", None),
("if (foo) {}", "", None),
("while (foo) {}", "", None),
("for (;foo;) {}", "", None),
("switch(foo) {}", "", None),
("switch (foo) { /* empty */ }", "", None),
("try {} catch (ex) {}", "", Some(json!([ { "allowEmptyCatch": true }]))),
(
"try { foo(); } catch (ex) {} finally {}",
"try { foo(); } catch (ex) {} ",
Some(json!([ { "allowEmptyCatch": true }])),
),
("try {} catch (ex) {} finally {}", "", Some(json!([ { "allowEmptyCatch": true }]))),
("try { foo(); } catch (ex) {} finally {}", "try { foo(); } catch (ex) {} ", None),
];

Tester::new(NoEmpty::NAME, pass, fail).expect_fix(fix).test_and_snapshot();
}

0 comments on commit 6633972

Please sign in to comment.