Skip to content

Commit

Permalink
feat(linter) oxc: only used in recurion
Browse files Browse the repository at this point in the history
  • Loading branch information
camc314 authored and Boshen committed Dec 27, 2023
1 parent ea8a59b commit 6fefe3e
Show file tree
Hide file tree
Showing 3 changed files with 347 additions and 0 deletions.
2 changes: 2 additions & 0 deletions crates/oxc_linter/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ mod oxc {
pub mod double_comparisons;
pub mod misrefactored_assign_op;
pub mod no_accumulating_spread;
pub mod only_used_in_recursion;
}

oxc_macros::declare_all_lint_rules! {
Expand Down Expand Up @@ -480,4 +481,5 @@ oxc_macros::declare_all_lint_rules! {
oxc::double_comparisons,
oxc::misrefactored_assign_op,
oxc::no_accumulating_spread,
oxc::only_used_in_recursion,
}
277 changes: 277 additions & 0 deletions crates/oxc_linter/src/rules/oxc/only_used_in_recursion.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,277 @@
use oxc_ast::{
ast::{BindingIdentifier, BindingPatternKind, Expression},
AstKind,
};
use oxc_diagnostics::{
miette::{self, Diagnostic},
thiserror::{self, Error},
};
use oxc_macros::declare_oxc_lint;
use oxc_span::{Atom, GetSpan, Span};

use crate::{context::LintContext, rule::Rule, AstNode};

#[derive(Debug, Error, Diagnostic)]
#[error("oxc(only-used-in-recursion): Parameter `{1}` is only used in recursive calls")]
#[diagnostic(
severity(warning),
help(
"Remove the argument and its usage. Alternatively, use the argument in the function body."
)
)]
struct OnlyUsedInRecursionDiagnostic(#[label] pub Span, pub Atom);

#[derive(Debug, Default, Clone)]
pub struct OnlyUsedInRecursion;

declare_oxc_lint!(
/// ### What it does
///
/// Checks for arguments that are only used in recursion with no side-effects.
///
/// Inspired by https://rust-lang.github.io/rust-clippy/master/#/only_used_in_recursion
///
/// ### Why is this bad?
///
/// Supplying an argument that is only used in recursive calls is likely a mistake.
///
/// It increase cognitive complexity and may impact performance.
///
/// ### Example
/// ```javascript
/// // Bad - the argument `b` is only used in recursive calls
/// function f(a: number, b: number): number {
/// if a == 0 {
/// return 1
/// } else {
/// return f(a - 1, b + 1)
/// }
/// }
///
/// // Good - the argument `b` is omitted
/// function f(a: number): number {
/// if a == 0 {
/// return 1
/// } else {
/// return f(a - 1)
/// }
/// }
/// ```
OnlyUsedInRecursion,
correctness
);

impl Rule for OnlyUsedInRecursion {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
let AstKind::Function(function) = node.kind() else { return };

let Some(function_id) = &function.id else { return };

if function.body.is_none() {
return;
}

if is_function_maybe_reassigned(function_id, ctx) {
return;
}

for (arg_index, arg) in function.params.items.iter().enumerate() {
let BindingPatternKind::BindingIdentifier(arg) = &arg.pattern.kind else { continue };

if is_argument_only_used_in_recursion(function_id, arg, arg_index, ctx) {
ctx.diagnostic(OnlyUsedInRecursionDiagnostic(arg.span, arg.name.clone()));
}
}
}
}

fn is_argument_only_used_in_recursion<'a>(
function_id: &'a BindingIdentifier,
arg: &'a BindingIdentifier,
arg_index: usize,
ctx: &'a LintContext<'_>,
) -> bool {
let mut is_used_only_in_recursion = true;
let mut has_references = false;

for reference in
ctx.semantic().symbol_references(arg.symbol_id.get().expect("`symbol_id` should be set"))
{
has_references = true;
if let Some(AstKind::Argument(argument)) = ctx.nodes().parent_kind(reference.node_id()) {
if let Some(AstKind::CallExpression(call_expr)) =
ctx.nodes().parent_kind(ctx.nodes().parent_node(reference.node_id()).unwrap().id())
{
if !call_expr.arguments.iter().enumerate().any(|(index, arg)| {
index == arg_index
&& arg.span() == argument.span()
&& if let Expression::Identifier(identifier) = &call_expr.callee {
identifier.name == function_id.name
} else {
false
}
}) {
is_used_only_in_recursion = false;
break;
}
} else {
is_used_only_in_recursion = false;
break;
}
} else {
is_used_only_in_recursion = false;
break;
}
}

has_references && is_used_only_in_recursion
}

fn is_function_maybe_reassigned<'a>(
function_id: &'a BindingIdentifier,
ctx: &'a LintContext<'_>,
) -> bool {
let mut is_maybe_reassigned = false;

for reference in ctx
.semantic()
.symbol_references(function_id.symbol_id.get().expect("`symbol_id` should be set"))
{
if let Some(AstKind::SimpleAssignmentTarget(_)) =
ctx.nodes().parent_kind(reference.node_id())
{
is_maybe_reassigned = true;
}
}

is_maybe_reassigned
}

#[test]
fn test() {
use crate::tester::Tester;

let pass = vec![
// no args, no recursion
"
function test() {
// some code
}
",
// unused arg, no recursion
"
function test(arg0) {
// arg0 not used
}
",
"
function test(arg0) {
anotherTest(arg0);
}
function anotherTest(arg) { }
",
// conditional recursion
"
function test(arg0) {
if (arg0 > 0) {
test(arg0 - 1);
}
}
",
"
function test(arg0, arg1) {
// only arg0 used in recursion
arg0
test(arg0);
}
",
// allowed case
"
function test() {
test()
}
",
// arg not passed to recursive call
"
function test(arg0) {
arg0()
}
",
"function test(arg0) { }",
// args in wrong order
"
function test(arg0, arg1) {
test(arg1, arg0)
}
",
// Arguments Swapped in Recursion
r"
function test(arg0, arg1) {
test(arg1, arg0);
}
",
// https://github.com/swc-project/swc/blob/3ca954b9f9622ed400308f2af35242583a4bdc3d/crates/swc_ecma_transforms_base/src/helpers/_get.js#L1-L16
r#"
function _get(target, property, receiver) {
if (typeof Reflect !== "undefined" && Reflect.get) {
_get = Reflect.get;
} else {
_get = function get(target, property, receiver) {
var base = _super_prop_base(target, property);
if (!base) return;
var desc = Object.getOwnPropertyDescriptor(base, property);
if (desc.get) {
return desc.get.call(receiver || target);
}
return desc.value;
};
}
return _get(target, property, receiver || target);
}
"#,
];

let fail = vec![
"
function test(arg0) {
return test(arg0);
}
",
r#"
function test(arg0, arg1) {
return test("", arg1);
}
"#,
// Argument Not Altered in Recursion
r"
function test(arg0) {
test(arg0);
}
",
// Wrong Number of Arguments in Recursion
r"
function test(arg0, arg1) {
test(arg0);
}
",
// Unused Argument in Recursion
r"
function test(arg0, arg1) {
test(arg0);
}
",
r"
module.exports = function test(a) {
test(a)
}
",
r"
export function test(a) {
test(a)
}
",
];

Tester::new_without_config(OnlyUsedInRecursion::NAME, pass, fail).test_and_snapshot();
}
68 changes: 68 additions & 0 deletions crates/oxc_linter/src/snapshots/only_used_in_recursion.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
---
source: crates/oxc_linter/src/tester.rs
expression: only_used_in_recursion
---
oxc(only-used-in-recursion): Parameter `arg0` is only used in recursive calls
╭─[only_used_in_recursion.tsx:1:1]
1
2function test(arg0) {
· ────
3return test(arg0);
╰────
help: Remove the argument and its usage. Alternatively, use the argument in the function body.

⚠ oxc(only-used-in-recursion): Parameter `arg1` is only used in recursive calls
╭─[only_used_in_recursion.tsx:1:1]
1 │
2 │ function test(arg0, arg1) {
· ────
3return test("", arg1);
╰────
help: Remove the argument and its usage. Alternatively, use the argument in the function body.

⚠ oxc(only-used-in-recursion): Parameter `arg0` is only used in recursive calls
╭─[only_used_in_recursion.tsx:1:1]
1 │
2 │ function test(arg0) {
· ────
3test(arg0);
╰────
help: Remove the argument and its usage. Alternatively, use the argument in the function body.

⚠ oxc(only-used-in-recursion): Parameter `arg0` is only used in recursive calls
╭─[only_used_in_recursion.tsx:1:1]
1 │
2 │ function test(arg0, arg1) {
· ────
3test(arg0);
╰────
help: Remove the argument and its usage. Alternatively, use the argument in the function body.

⚠ oxc(only-used-in-recursion): Parameter `arg0` is only used in recursive calls
╭─[only_used_in_recursion.tsx:1:1]
1 │
2 │ function test(arg0, arg1) {
· ────
3test(arg0);
╰────
help: Remove the argument and its usage. Alternatively, use the argument in the function body.

⚠ oxc(only-used-in-recursion): Parameter `a` is only used in recursive calls
╭─[only_used_in_recursion.tsx:1:1]
1 │
2 │ module.exports = function test(a) {
· ─
3test(a)
╰────
help: Remove the argument and its usage. Alternatively, use the argument in the function body.

⚠ oxc(only-used-in-recursion): Parameter `a` is only used in recursive calls
╭─[only_used_in_recursion.tsx:1:1]
1 │
2 │ export function test(a) {
· ─
3test(a)
╰────
help: Remove the argument and its usage. Alternatively, use the argument in the function body.


0 comments on commit 6fefe3e

Please sign in to comment.