From a718e5903de2157021a12d4c63ef5512cf401d41 Mon Sep 17 00:00:00 2001 From: Don Isaac Date: Tue, 16 Jul 2024 23:25:57 -0400 Subject: [PATCH] feat(linter): add suggestion for no-console --- crates/oxc_ast/src/ast/js.rs | 2 + .../oxc_linter/src/rules/eslint/no_console.rs | 66 ++++++++++++- .../src/snapshots/no_console.snap.new | 94 +++++++++++++++++++ crates/oxc_linter/src/tester.rs | 6 +- crates/oxc_semantic/src/node.rs | 7 ++ 5 files changed, 169 insertions(+), 6 deletions(-) create mode 100644 crates/oxc_linter/src/snapshots/no_console.snap.new diff --git a/crates/oxc_ast/src/ast/js.rs b/crates/oxc_ast/src/ast/js.rs index 3cc7b53cd29b7d..5e19c55a6e7b1d 100644 --- a/crates/oxc_ast/src/ast/js.rs +++ b/crates/oxc_ast/src/ast/js.rs @@ -581,6 +581,8 @@ pub struct LogicalExpression<'a> { } /// Conditional Expression +/// +/// This is a ternary #[visited_node] #[derive(Debug, Hash)] #[cfg_attr(feature = "serialize", derive(Serialize, Tsify))] diff --git a/crates/oxc_linter/src/rules/eslint/no_console.rs b/crates/oxc_linter/src/rules/eslint/no_console.rs index 54b63c785f11b6..6a86e8a20fd55d 100644 --- a/crates/oxc_linter/src/rules/eslint/no_console.rs +++ b/crates/oxc_linter/src/rules/eslint/no_console.rs @@ -1,12 +1,15 @@ +use crate::fixer::{RuleFix, RuleFixer}; use oxc_ast::{ast::Expression, AstKind}; use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; -use oxc_span::Span; +use oxc_span::{GetSpan, Span}; use crate::{context::LintContext, rule::Rule, AstNode}; fn no_console_diagnostic(span0: Span) -> OxcDiagnostic { - OxcDiagnostic::warn("eslint(no-console): Unexpected console statement.").with_label(span0) + OxcDiagnostic::warn("eslint(no-console): Unexpected console statement.") + .with_label(span0) + .with_help("Delete this console statement.") } #[derive(Debug, Default, Clone)] @@ -78,8 +81,12 @@ impl Rule for NoConsole { .iter() .any(|s| mem.static_property_name().is_some_and(|f| f == s)) { - if let Some(mem) = mem.static_property_info() { - ctx.diagnostic(no_console_diagnostic(mem.0)); + if let Some((mem_span, _)) = mem.static_property_info() { + let diagnostic_span = ident.span().merge(&mem_span); + ctx.diagnostic_with_suggestion( + no_console_diagnostic(diagnostic_span), + |fixer| remove_console(fixer, ctx, node), + ); } } } @@ -88,6 +95,41 @@ impl Rule for NoConsole { } } +fn remove_console<'c, 'a: 'c>( + fixer: RuleFixer<'c, 'a>, + ctx: &'c LintContext<'a>, + node: &AstNode<'a>, +) -> RuleFix<'a> { + let mut node_to_delete = node; + for parent in ctx.nodes().iter_parents(node.id()).skip(1) { + match parent.kind() { + AstKind::ParenthesizedExpression(_) + | AstKind::ExpressionStatement(_) + => node_to_delete = parent, + AstKind::IfStatement(_) + | AstKind::WhileStatement(_) + | AstKind::ForStatement(_) + | AstKind::ForInStatement(_) + | AstKind::ForOfStatement(_) + | AstKind::ArrowFunctionExpression(_) => { + return fixer.replace(node_to_delete.span(), "{}") + } + | AstKind::FunctionBody(body) if !fixer.source_range(body.span).starts_with('{') => { + return fixer.replace(node_to_delete.span(), "{}") + } + // Marked as dangerous until we're sure this is safe + AstKind::ConditionalExpression(_) + // from: const x = (console.log("foo"), 5); + // to: const x = (undefined, 5); + | AstKind::SequenceExpression(_) => { + return fixer.replace(node_to_delete.span(), "undefined").dangerously() + } + _ => break, + } + } + fixer.delete(node_to_delete) +} + #[test] fn test() { use crate::tester::Tester; @@ -122,5 +164,19 @@ fn test() { ("console.warn(foo)", Some(serde_json::json!([{ "allow": ["info", "log"] }]))), ]; - Tester::new(NoConsole::NAME, pass, fail).test_and_snapshot(); + let fix = vec![ + ("function foo() { console.log(bar); }", "function foo() { }", None), + ("function foo() { console.log(bar) }", "function foo() { }", None), + ("const x = () => console.log(foo)", "const x = () => {}", None), + ("const x = () => { console.log(foo) }", "const x = () => { }", None), + ("const x = () => { console.log(foo); }", "const x = () => { }", None), + ("const x = () => { ((console.log(foo))); }", "const x = () => { }", None), + ("const x = () => { console.log(foo); return 5 }", "const x = () => { return 5 }", None), + ("if (foo) { console.log(foo) }", "if (foo) { }", None), + ("foo ? console.log(foo) : 5", "foo ? undefined : 5", None), + ("(console.log(foo), 5)", "(undefined, 5)", None), + ("(5, console.log(foo))", "(5, undefined)", None), + ]; + + Tester::new(NoConsole::NAME, pass, fail).expect_fix(fix).test_and_snapshot(); } diff --git a/crates/oxc_linter/src/snapshots/no_console.snap.new b/crates/oxc_linter/src/snapshots/no_console.snap.new new file mode 100644 index 00000000000000..83970dc3576eda --- /dev/null +++ b/crates/oxc_linter/src/snapshots/no_console.snap.new @@ -0,0 +1,94 @@ +--- +source: crates/oxc_linter/src/tester.rs +assertion_line: 216 +--- + ⚠ eslint(no-console): Unexpected console statement. + ╭─[no_console.tsx:1:1] + 1 │ console.log() + · ─────────── + ╰──── + help: Delete this console statement. + + ⚠ eslint(no-console): Unexpected console statement. + ╭─[no_console.tsx:1:1] + 1 │ console.log(foo) + · ─────────── + ╰──── + help: Delete this console statement. + + ⚠ eslint(no-console): Unexpected console statement. + ╭─[no_console.tsx:1:1] + 1 │ console.error(foo) + · ───────────── + ╰──── + help: Delete this console statement. + + ⚠ eslint(no-console): Unexpected console statement. + ╭─[no_console.tsx:1:1] + 1 │ console.info(foo) + · ──────────── + ╰──── + help: Delete this console statement. + + ⚠ eslint(no-console): Unexpected console statement. + ╭─[no_console.tsx:1:1] + 1 │ console.warn(foo) + · ──────────── + ╰──── + help: Delete this console statement. + + ⚠ eslint(no-console): Unexpected console statement. + ╭─[no_console.tsx:1:1] + 1 │ console.log(foo) + · ─────────── + ╰──── + help: Delete this console statement. + + ⚠ eslint(no-console): Unexpected console statement. + ╭─[no_console.tsx:1:1] + 1 │ console.error(foo) + · ───────────── + ╰──── + help: Delete this console statement. + + ⚠ eslint(no-console): Unexpected console statement. + ╭─[no_console.tsx:1:1] + 1 │ console.info(foo) + · ──────────── + ╰──── + help: Delete this console statement. + + ⚠ eslint(no-console): Unexpected console statement. + ╭─[no_console.tsx:1:1] + 1 │ console.warn(foo) + · ──────────── + ╰──── + help: Delete this console statement. + + ⚠ eslint(no-console): Unexpected console statement. + ╭─[no_console.tsx:1:1] + 1 │ console.log(foo) + · ─────────── + ╰──── + help: Delete this console statement. + + ⚠ eslint(no-console): Unexpected console statement. + ╭─[no_console.tsx:1:1] + 1 │ console.error(foo) + · ───────────── + ╰──── + help: Delete this console statement. + + ⚠ eslint(no-console): Unexpected console statement. + ╭─[no_console.tsx:1:1] + 1 │ console.info(foo) + · ──────────── + ╰──── + help: Delete this console statement. + + ⚠ eslint(no-console): Unexpected console statement. + ╭─[no_console.tsx:1:1] + 1 │ console.warn(foo) + · ──────────── + ╰──── + help: Delete this console statement. diff --git a/crates/oxc_linter/src/tester.rs b/crates/oxc_linter/src/tester.rs index 1ad9c32e669345..7de203d6ec8864 100644 --- a/crates/oxc_linter/src/tester.rs +++ b/crates/oxc_linter/src/tester.rs @@ -259,7 +259,11 @@ impl Tester { let allocator = Allocator::default(); let rule = self.find_rule().read_json(rule_config.unwrap_or_default()); let options = LintOptions::default() - .with_fix(is_fix.then_some(FixKind::DangerousFix).unwrap_or_default()) + .with_fix( + is_fix + .then_some(FixKind::DangerousFix.union(FixKind::Suggestion)) + .unwrap_or_default(), + ) .with_import_plugin(self.import_plugin) .with_jest_plugin(self.jest_plugin) .with_vitest_plugin(self.vitest_plugin) diff --git a/crates/oxc_semantic/src/node.rs b/crates/oxc_semantic/src/node.rs index 8c4805b68a2430..a90dd56de1071c 100644 --- a/crates/oxc_semantic/src/node.rs +++ b/crates/oxc_semantic/src/node.rs @@ -1,6 +1,7 @@ use oxc_ast::AstKind; use oxc_cfg::BasicBlockId; use oxc_index::IndexVec; +use oxc_span::GetSpan; pub use oxc_syntax::node::{AstNodeId, NodeFlags}; use crate::scope::ScopeId; @@ -57,6 +58,12 @@ impl<'a> AstNode<'a> { } } +impl GetSpan for AstNode<'_> { + fn span(&self) -> oxc_span::Span { + self.kind.span() + } +} + /// Untyped AST nodes flattened into an vec #[derive(Debug, Default)] pub struct AstNodes<'a> {