Skip to content

Commit

Permalink
feat(linter): add dangerous fixer for oxc only used in recursion (#4805)
Browse files Browse the repository at this point in the history
  • Loading branch information
camc314 committed Aug 18, 2024
1 parent 4cb8c37 commit 915cb4d
Showing 1 changed file with 150 additions and 8 deletions.
158 changes: 150 additions & 8 deletions crates/oxc_linter/src/rules/oxc/only_used_in_recursion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down Expand Up @@ -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;
}
Expand All @@ -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()));
}
}
}
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}

0 comments on commit 915cb4d

Please sign in to comment.