From 1beb640086102c1d3e53a0689329826363992d6e Mon Sep 17 00:00:00 2001 From: Brian Donovan <1938+eventualbuddha@users.noreply.github.com> Date: Mon, 14 Oct 2024 05:46:04 -0700 Subject: [PATCH 1/3] fix(no-unused-vars): consider functions within conditional expressions usable Previously, a function declared directly within an `AstKind::ConditionalExpression` would be considered unused. This happened because `is_function_or_class_declaration_used` did not directly handle conditional expression parents, letting such cases fall into the default branch which just returned `false`, marking the function as unused. This change fixes that by considering such functions used when the conditional expression itself is used. --- .../oxc_linter/src/rules/eslint/no_unused_vars/allowed.rs | 6 ++++++ .../src/rules/eslint/no_unused_vars/tests/eslint.rs | 3 +++ 2 files changed, 9 insertions(+) diff --git a/crates/oxc_linter/src/rules/eslint/no_unused_vars/allowed.rs b/crates/oxc_linter/src/rules/eslint/no_unused_vars/allowed.rs index 375c851782db0..590d2ebd373d8 100644 --- a/crates/oxc_linter/src/rules/eslint/no_unused_vars/allowed.rs +++ b/crates/oxc_linter/src/rules/eslint/no_unused_vars/allowed.rs @@ -70,6 +70,12 @@ impl<'s, 'a> Symbol<'s, 'a> { }; return body.span.contains_inclusive(self.span()) && body.statements.len() == 1 && !self.get_snippet(body.span).starts_with('{') } + // a ? b : function foo() {} + // Only considered used if the function is the test or the selected branch, + // but we can't determine that here. + AstKind::ConditionalExpression(_) => { + continue; + } _ => { parent.kind().debug_name(); return false; diff --git a/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/eslint.rs b/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/eslint.rs index 5177e18d61d96..731a999dfeede 100644 --- a/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/eslint.rs +++ b/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/eslint.rs @@ -364,6 +364,9 @@ fn test() { None, ), ("function* foo(cb) { cb = yield function(a) { cb(1 + a); }; } foo();", None), // { "ecmaVersion": 6 }, + ("console.log(function a() {} ? b : c)", None), // { "ecmaVersion": 6 } + ("console.log(a ? function b() {} : c)", None), // { "ecmaVersion": 6 } + ("console.log(a ? b : function c() {})", None), // { "ecmaVersion": 6 } ("function foo(cb) { cb = tag`hello${function(a) { cb(1 + a); }}`; } foo();", None), // { "ecmaVersion": 6 }, ("function foo(cb) { var b; cb = b = function(a) { cb(1 + a); }; b(); } foo();", None), ( From 1b886b1eb1875f93895e291f59827afd270175a0 Mon Sep 17 00:00:00 2001 From: Brian Donovan <1938+eventualbuddha@users.noreply.github.com> Date: Mon, 14 Oct 2024 05:56:19 -0700 Subject: [PATCH 2/3] style(no-unused-vars): fix clippy lints --- .../src/rules/eslint/no_unused_vars/allowed.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_unused_vars/allowed.rs b/crates/oxc_linter/src/rules/eslint/no_unused_vars/allowed.rs index 590d2ebd373d8..efc2441387dc1 100644 --- a/crates/oxc_linter/src/rules/eslint/no_unused_vars/allowed.rs +++ b/crates/oxc_linter/src/rules/eslint/no_unused_vars/allowed.rs @@ -32,6 +32,10 @@ impl<'s, 'a> Symbol<'s, 'a> { | AstKind::ArrayExpressionElement(_) | AstKind::ExpressionArrayElement(_) | AstKind::ArrayExpression(_) + // a ? b : function foo() {} + // Only considered used if the function is the test or the selected branch, + // but we can't determine that here. + | AstKind::ConditionalExpression(_) => { continue; } @@ -70,12 +74,6 @@ impl<'s, 'a> Symbol<'s, 'a> { }; return body.span.contains_inclusive(self.span()) && body.statements.len() == 1 && !self.get_snippet(body.span).starts_with('{') } - // a ? b : function foo() {} - // Only considered used if the function is the test or the selected branch, - // but we can't determine that here. - AstKind::ConditionalExpression(_) => { - continue; - } _ => { parent.kind().debug_name(); return false; From a387d5755e5d7b96628737a8fa55131e61b97eeb Mon Sep 17 00:00:00 2001 From: Brian Donovan <1938+eventualbuddha@users.noreply.github.com> Date: Mon, 14 Oct 2024 06:25:24 -0700 Subject: [PATCH 3/3] test(no-unused-vars): move to `oxc` from `eslint` suite Per https://github.com/oxc-project/oxc/pull/6553#discussion_r1799496110, the `eslint` suite is for tests ported directly from ESLint. --- .../oxc_linter/src/rules/eslint/no_unused_vars/tests/eslint.rs | 3 --- crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/oxc.rs | 3 +++ 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/eslint.rs b/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/eslint.rs index 731a999dfeede..5177e18d61d96 100644 --- a/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/eslint.rs +++ b/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/eslint.rs @@ -364,9 +364,6 @@ fn test() { None, ), ("function* foo(cb) { cb = yield function(a) { cb(1 + a); }; } foo();", None), // { "ecmaVersion": 6 }, - ("console.log(function a() {} ? b : c)", None), // { "ecmaVersion": 6 } - ("console.log(a ? function b() {} : c)", None), // { "ecmaVersion": 6 } - ("console.log(a ? b : function c() {})", None), // { "ecmaVersion": 6 } ("function foo(cb) { cb = tag`hello${function(a) { cb(1 + a); }}`; } foo();", None), // { "ecmaVersion": 6 }, ("function foo(cb) { var b; cb = b = function(a) { cb(1 + a); }; b(); } foo();", None), ( diff --git a/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/oxc.rs b/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/oxc.rs index e8ed79af2ca70..d5c4a3e342658 100644 --- a/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/oxc.rs +++ b/crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/oxc.rs @@ -54,6 +54,9 @@ fn test_vars_simple() { ", None, ), + ("console.log(function a() {} ? b : c)", None), + ("console.log(a ? function b() {} : c)", None), + ("console.log(a ? b : function c() {})", None), ]; let fail = vec![ ("let a = 1", None),