From 915cb4d5a36db34a2ace5fa708c7f378cd29d656 Mon Sep 17 00:00:00 2001 From: camc314 <18101008+camc314@users.noreply.github.com> Date: Sun, 18 Aug 2024 01:19:37 +0000 Subject: [PATCH] feat(linter): add dangerous fixer for oxc only used in recursion (#4805) --- .../src/rules/oxc/only_used_in_recursion.rs | 158 +++++++++++++++++- 1 file changed, 150 insertions(+), 8 deletions(-) diff --git a/crates/oxc_linter/src/rules/oxc/only_used_in_recursion.rs b/crates/oxc_linter/src/rules/oxc/only_used_in_recursion.rs index 8a727808936f3..4d6a9b2e58d4f 100644 --- a/crates/oxc_linter/src/rules/oxc/only_used_in_recursion.rs +++ b/crates/oxc_linter/src/rules/oxc/only_used_in_recursion.rs @@ -6,7 +6,9 @@ use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; use oxc_span::{GetSpan, Span}; -use crate::{ast_util::outermost_paren_parent, context::LintContext, rule::Rule, AstNode}; +use crate::{ + ast_util::outermost_paren_parent, context::LintContext, fixer::Fix, rule::Rule, AstNode, +}; fn only_used_in_recursion_diagnostic(span0: Span, x1: &str) -> OxcDiagnostic { OxcDiagnostic::warn(format!( @@ -55,28 +57,29 @@ declare_oxc_lint!( /// } /// ``` OnlyUsedInRecursion, - correctness + correctness, + dangerous_fix ); impl Rule for OnlyUsedInRecursion { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { - let (function_id, function_parameters) = match node.kind() { + let (function_id, function_parameters, function_span) = match node.kind() { AstKind::Function(function) => { if function.is_typescript_syntax() { return; } if let Some(binding_ident) = get_function_like_declaration(node, ctx) { - (binding_ident, &function.params) + (binding_ident, &function.params, function.span) } else if let Some(function_id) = &function.id { - (function_id, &function.params) + (function_id, &function.params, function.span) } else { return; } } AstKind::ArrowFunctionExpression(arrow_function) => { if let Some(binding_ident) = get_function_like_declaration(node, ctx) { - (binding_ident, &arrow_function.params) + (binding_ident, &arrow_function.params, arrow_function.span) } else { return; } @@ -94,7 +97,66 @@ impl Rule for OnlyUsedInRecursion { }; if is_argument_only_used_in_recursion(function_id, arg, arg_index, ctx) { - ctx.diagnostic(only_used_in_recursion_diagnostic(arg.span, arg.name.as_str())); + if arg_index == function_parameters.items.len() - 1 + && !ctx + .semantic() + .symbols() + .get_flag(function_id.symbol_id.get().expect("`symbol_id` should be set")) + .is_export() + { + ctx.diagnostic_with_dangerous_fix( + only_used_in_recursion_diagnostic(arg.span, arg.name.as_str()), + |fixer| { + let mut fix = fixer.new_fix_with_capacity( + ctx.semantic() + .symbol_references( + arg.symbol_id.get().expect("`symbol_id` should be set"), + ) + .count() + + 1, + ); + fix.push(Fix::delete(arg.span())); + + for reference in ctx.semantic().symbol_references( + arg.symbol_id.get().expect("`symbol_id` should be set"), + ) { + let node = ctx.nodes().get_node(reference.node_id()); + + fix.push(Fix::delete(node.span())); + } + + // search for references to the function and remove the argument + for reference in ctx.semantic().symbol_references( + function_id.symbol_id.get().expect("`symbol_id` should be set"), + ) { + let node = ctx.nodes().get_node(reference.node_id()); + + if let Some(AstKind::CallExpression(call_expr)) = + ctx.nodes().parent_kind(node.id()) + { + // check if the number of arguments is the same + if call_expr.arguments.len() != function_parameters.items.len() + || function_span.contains_inclusive(call_expr.span) + { + continue; + } + + // remove the argument + let arg_to_delete = call_expr.arguments[arg_index].span(); + + fix.push(Fix::delete(Span::new( + arg_to_delete.start, + skip_to_next_char(ctx.source_text(), arg_to_delete.end), + ))); + } + } + + fix + }, + ); + } else { + ctx.diagnostic(only_used_in_recursion_diagnostic(arg.span, arg.name.as_str())); + } } } } @@ -184,6 +246,20 @@ fn is_function_maybe_reassigned<'a>( is_maybe_reassigned } +// skipping whitespace, commas, finds the next character (exclusive) +#[allow(clippy::cast_possible_truncation)] +fn skip_to_next_char(s: &str, start: u32) -> u32 { + let mut i = start as usize; + while i < s.len() { + let c = s.chars().nth(i).unwrap(); + if !c.is_whitespace() && c != ',' { + break; + } + i += 1; + } + i as u32 +} + #[test] fn test() { use crate::tester::Tester; @@ -342,5 +418,71 @@ fn test() { ", ]; - Tester::new(OnlyUsedInRecursion::NAME, pass, fail).test_and_snapshot(); + let fix = vec![ + ( + r#"function test(a) { + test(a) + } + + test("") + "#, + r"function test() { + test() + } + + test() + ", + ), + ( + r#" + test(foo, bar); + function test(arg0, arg1) { + return test("", arg1); + } + "#, + r#" + test(foo, ); + function test(arg0, ) { + return test("", ); + } + "#, + ), + // Expecting no fix: function is exported + ( + r"export function test(a) { + test(a) + } + ", + r"export function test(a) { + test(a) + } + ", + ), + ( + r"function test(a) { + test(a) + } + export { test }; + ", + r"function test(a) { + test(a) + } + export { test }; + ", + ), + ( + r"function test(a) { + test(a) + } + export default test; + ", + r"function test(a) { + test(a) + } + export default test; + ", + ), + ]; + + Tester::new(OnlyUsedInRecursion::NAME, pass, fail).expect_fix(fix).test_and_snapshot(); }