mirror of
https://github.com/danbulant/oxc
synced 2026-05-24 12:21:58 +00:00
feat(linter): add fixer for no-empty (#5276)
adds a suggestion for `no-empty` eslint lint rule
This commit is contained in:
parent
7fa2fa386c
commit
6633972663
1 changed files with 96 additions and 9 deletions
|
|
@ -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();
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue