From e9ad03bb62e9c3dc710644334a2e8db4baaf4967 Mon Sep 17 00:00:00 2001 From: DonIsaac <22823424+DonIsaac@users.noreply.github.com> Date: Thu, 11 Jul 2024 04:33:43 +0000 Subject: [PATCH] fix(linter): fixer for no-debugger creates incorrect code (#4184) Part of #4179. Fixes cases like: ```js if (foo) debugger // got fixed into if (foo) // but now gets fixed to if (foo) {} ``` --- .../src/rules/eslint/no_debugger.rs | 33 +++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_debugger.rs b/crates/oxc_linter/src/rules/eslint/no_debugger.rs index 0e6650a3dacad..a0c19195b6332 100644 --- a/crates/oxc_linter/src/rules/eslint/no_debugger.rs +++ b/crates/oxc_linter/src/rules/eslint/no_debugger.rs @@ -35,7 +35,28 @@ impl Rule for NoDebugger { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { if let AstKind::DebuggerStatement(stmt) = node.kind() { ctx.diagnostic_with_fix(no_debugger_diagnostic(stmt.span), |fixer| { - fixer.delete(&stmt.span) + let Some(parent) = ctx + .nodes() + .iter_parents(node.id()) + .skip(1) + .find(|p| !matches!(p.kind(), AstKind::ParenthesizedExpression(_))) + else { + return fixer.delete(&stmt.span); + }; + + // For statements like `if (foo) debugger;`, we can't just + // delete the `debugger` statement; we need to replace it with an empty block. + match parent.kind() { + AstKind::IfStatement(_) + | AstKind::WhileStatement(_) + | AstKind::ForStatement(_) + | AstKind::ForInStatement(_) + | AstKind::ForOfStatement(_) => return fixer.replace(stmt.span, "{}"), + // NOTE: no need to check for + // AstKind::ArrowFunctionExpression because + // `const x = () => debugger` is a parse error + _ => fixer.delete(&stmt.span), + } }); } } @@ -48,6 +69,14 @@ fn test() { let pass = vec![("var test = { debugger: 1 }; test.debugger;", None)]; let fail = vec![("if (foo) debugger", None)]; + let fix = vec![ + ("let x; debugger; let y;", "let x; let y;", None), + ("if (foo) debugger", "if (foo) {}", None), + ("for (;;) debugger", "for (;;) {}", None), + ("while (i > 0) debugger", "while (i > 0) {}", None), + ("if (foo) { debugger; }", "if (foo) { }", None), + ("if (foo) { debugger }", "if (foo) { }", None), + ]; - Tester::new(NoDebugger::NAME, pass, fail).test_and_snapshot(); + Tester::new(NoDebugger::NAME, pass, fail).expect_fix(fix).test_and_snapshot(); }