feat(linter): add fixer for no-empty (#5276)

adds a suggestion for `no-empty` eslint lint rule
This commit is contained in:
camc314 2024-08-28 15:06:29 +00:00
parent 7fa2fa386c
commit 6633972663

View file

@ -1,4 +1,4 @@
use oxc_ast::AstKind; use oxc_ast::{ast::BlockStatement, AstKind};
use oxc_diagnostics::OxcDiagnostic; use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint; use oxc_macros::declare_oxc_lint;
use oxc_span::Span; use oxc_span::Span;
@ -32,7 +32,7 @@ declare_oxc_lint!(
/// ``` /// ```
NoEmpty, NoEmpty,
restriction, restriction,
pending // a suggestion could be added to remove the empty statement suggestion
); );
impl Rule for NoEmpty { impl Rule for NoEmpty {
@ -49,16 +49,25 @@ impl Rule for NoEmpty {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
match node.kind() { match node.kind() {
AstKind::BlockStatement(block) if block.body.is_empty() => { AstKind::BlockStatement(block) if block.body.is_empty() => {
if self.allow_empty_catch let parent = ctx.nodes().parent_kind(node.id());
&& matches!(ctx.nodes().parent_kind(node.id()), Some(AstKind::CatchClause(_))) if self.allow_empty_catch && matches!(parent, Some(AstKind::CatchClause(_))) {
{
return; return;
} }
if ctx.semantic().trivias().has_comments_between(block.span) { if ctx.semantic().trivias().has_comments_between(block.span) {
return; 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`. // The visitor does not visit the `BlockStatement` inside the `FinallyClause`.
// See `Visit::visit_finally_clause`. // See `Visit::visit_finally_clause`.
@ -66,16 +75,70 @@ impl Rule for NoEmpty {
if ctx.semantic().trivias().has_comments_between(finally_clause.span) { if ctx.semantic().trivias().has_comments_between(finally_clause.span) {
return; 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() => { 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] #[test]
fn test() { fn test() {
use serde_json::json; use serde_json::json;
@ -125,5 +188,29 @@ fn test() {
("try { foo(); } catch (ex) {} finally {}", None), ("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();
} }