Skip to content

Commit c8adc46

Browse files
DunqingDunqing
committed
refactor(linter/no-unused-vars): improve implementation to remove using SymbolFlags::Export (#7412)
part of #7414
1 parent 7ff9f13 commit c8adc46

6 files changed

Lines changed: 31 additions & 114 deletions

File tree

‎crates/oxc_linter/src/rules/eslint/no_unused_vars/allowed.rs‎

Lines changed: 21 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -2,84 +2,35 @@
22
//! consider variables ignored by name pattern, but by where they are declared.
33
use oxc_ast::{ast::*, AstKind};
44
use oxc_semantic::{NodeId, Semantic};
5-
use oxc_span::GetSpan;
65

76
use super::{options::ArgsOption, NoUnusedVars, Symbol};
87
use crate::rules::eslint::no_unused_vars::binding_pattern::{BindingContext, HasAnyUsedBinding};
98

109
impl<'s, 'a> Symbol<'s, 'a> {
11-
/// Returns `true` if this function is use.
10+
/// Check if the declaration of this [`Symbol`] is use.
1211
///
13-
/// Checks for these cases
14-
/// 1. passed as a callback to another [`CallExpression`] or [`NewExpression`]
15-
/// 2. invoked as an IIFE
16-
/// 3. Returned from another function
17-
/// 4. Used as an attribute in a JSX element
12+
/// If it's an expression, then it's always passed in as an argument
13+
/// or assigned to a variable, so it's always used.
14+
///
15+
/// ```js
16+
/// // True:
17+
/// const a = class Name{}
18+
/// export default (function Name() {})
19+
/// console.log(function Name() {})
20+
///
21+
/// // False
22+
/// function foo() {}
23+
/// {
24+
/// class Foo {}
25+
/// }
26+
/// ```
1827
#[inline]
19-
pub fn is_function_or_class_declaration_used(&self) -> bool {
20-
#[cfg(debug_assertions)]
21-
{
22-
let kind = self.declaration().kind();
23-
assert!(kind.is_function_like() || matches!(kind, AstKind::Class(_)));
24-
}
25-
26-
for parent in self.iter_relevant_parents() {
27-
match parent.kind() {
28-
AstKind::MemberExpression(_) | AstKind::ParenthesizedExpression(_)
29-
// e.g. `const x = [function foo() {}]`
30-
// Only considered used if the array containing the symbol is used.
31-
| AstKind::ArrayExpressionElement(_)
32-
| AstKind::ArrayExpression(_)
33-
// a ? b : function foo() {}
34-
// Only considered used if the function is the test or the selected branch,
35-
// but we can't determine that here.
36-
| AstKind::ConditionalExpression(_)
37-
=> {
38-
continue;
39-
}
40-
// Returned from another function. Definitely won't be the same
41-
// function because we're walking up from its declaration
42-
AstKind::ReturnStatement(_)
43-
// <Component onClick={function onClick(e) { }} />
44-
| AstKind::JSXExpressionContainer(_)
45-
// Function declaration is passed as an argument to another function.
46-
| AstKind::CallExpression(_) | AstKind::Argument(_)
47-
// e.g. `const x = { foo: function foo() {} }`
48-
// Allowed off-the-bat since objects being the only child of an
49-
// ExpressionStatement is rare, since you would need to wrap the
50-
// object in parentheses to avoid creating a block statement.
51-
| AstKind::ObjectProperty(_)
52-
// e.g. var foo = function bar() { }
53-
// we don't want to check for violations on `bar`, just `foo`
54-
| AstKind::VariableDeclarator(_)
55-
// new (class CustomRenderer{})
56-
// new (function() {})
57-
| AstKind::NewExpression(_)
58-
=> {
59-
return true;
60-
}
61-
// !function() {}; is an IIFE
62-
AstKind::UnaryExpression(expr) => return expr.operator.is_not(),
63-
// function is used as a value for an assignment
64-
// e.g. Array.prototype.sort ||= function sort(a, b) { }
65-
AstKind::AssignmentExpression(assignment) if assignment.right.span().contains_inclusive(self.span()) => {
66-
return self != &assignment.left;
67-
}
68-
AstKind::ExpressionStatement(_) => {
69-
// implicit return in arrow function expression
70-
let Some(AstKind::FunctionBody(body)) = self.nodes().parent_kind(parent.id()) else {
71-
return false;
72-
};
73-
return body.span.contains_inclusive(self.span()) && body.statements.len() == 1 && !self.get_snippet(body.span).starts_with('{')
74-
}
75-
_ => {
76-
parent.kind().debug_name();
77-
return false;
78-
}
79-
}
28+
pub(crate) fn is_function_or_class_declaration_used(&self) -> bool {
29+
match self.declaration().kind() {
30+
AstKind::Class(class) => class.is_expression(),
31+
AstKind::Function(func) => func.is_expression(),
32+
_ => false,
8033
}
81-
82-
false
8334
}
8435

8536
fn is_declared_in_for_of_loop(&self) -> bool {
@@ -124,12 +75,6 @@ fn is_ambient_namespace(namespace: &TSModuleDeclaration) -> bool {
12475
}
12576

12677
impl NoUnusedVars {
127-
#[allow(clippy::unused_self)]
128-
pub(super) fn is_allowed_class_or_function(&self, symbol: &Symbol<'_, '_>) -> bool {
129-
symbol.is_function_or_class_declaration_used()
130-
// || symbol.is_function_or_class_assigned_to_same_name_variable()
131-
}
132-
13378
#[allow(clippy::unused_self)]
13479
pub(super) fn is_allowed_ts_namespace<'a>(
13580
&self,

‎crates/oxc_linter/src/rules/eslint/no_unused_vars/mod.rs‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -232,8 +232,7 @@ impl NoUnusedVars {
232232
}
233233

234234
// Order matters. We want to call cheap/high "yield" functions first.
235-
let is_exported = symbol.is_exported();
236-
let is_used = is_exported || symbol.has_usages(self);
235+
let is_used = symbol.is_exported() || symbol.has_usages(self);
237236

238237
match (is_used, is_ignored) {
239238
(true, true) => {
@@ -306,12 +305,6 @@ impl NoUnusedVars {
306305
}
307306
ctx.diagnostic(diagnostic::declared(symbol, &self.vars_ignore_pattern));
308307
}
309-
AstKind::Class(_) | AstKind::Function(_) => {
310-
if self.is_allowed_class_or_function(symbol) {
311-
return;
312-
}
313-
ctx.diagnostic(diagnostic::declared(symbol, &IgnorePattern::<&str>::None));
314-
}
315308
AstKind::TSModuleDeclaration(namespace) => {
316309
if self.is_allowed_ts_namespace(symbol, namespace) {
317310
return;

‎crates/oxc_linter/src/rules/eslint/no_unused_vars/symbol.rs‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -105,11 +105,6 @@ impl<'s, 'a> Symbol<'s, 'a> {
105105
self.nodes().ancestors(self.declaration_id())
106106
}
107107

108-
#[inline]
109-
pub fn iter_relevant_parents(&self) -> impl Iterator<Item = &AstNode<'a>> + Clone + '_ {
110-
self.iter_relevant_parents_of(self.declaration_id())
111-
}
112-
113108
pub fn iter_relevant_parents_of(
114109
&self,
115110
node_id: NodeId,
@@ -176,10 +171,9 @@ impl<'s, 'a> Symbol<'s, 'a> {
176171
/// NOTE: does not support CJS right now.
177172
pub fn is_exported(&self) -> bool {
178173
let is_in_exportable_scope = self.is_root() || self.is_in_ts_namespace();
179-
(is_in_exportable_scope
180-
&& (self.flags.contains(SymbolFlags::Export)
181-
|| self.semantic.module_record().exported_bindings.contains_key(self.name())))
182-
|| self.in_export_node()
174+
is_in_exportable_scope
175+
&& (self.semantic.module_record().exported_bindings.contains_key(self.name())
176+
|| self.in_export_node())
183177
}
184178

185179
#[inline]
@@ -194,7 +188,7 @@ impl<'s, 'a> Symbol<'s, 'a> {
194188
AstKind::ModuleDeclaration(module) => {
195189
return module.is_export();
196190
}
197-
AstKind::ExportDefaultDeclaration(_) => {
191+
AstKind::ExportNamedDeclaration(_) | AstKind::ExportDefaultDeclaration(_) => {
198192
return true;
199193
}
200194
AstKind::VariableDeclaration(_)

‎crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/oxc.rs‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -819,11 +819,7 @@ fn test_used_declarations() {
819819
"export const Foo = class Bar {}",
820820
"export const Foo = @SomeDecorator() class Foo {}",
821821
];
822-
let fail = vec![
823-
// array is not used, so the function is not used
824-
";[function foo() {}]",
825-
";[class Foo {}]",
826-
];
822+
let fail = vec![];
827823

828824
Tester::new(NoUnusedVars::NAME, pass, fail)
829825
.intentionally_allow_no_fix_tests()

‎crates/oxc_linter/src/rules/eslint/no_unused_vars/usage.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,10 @@ impl<'s, 'a> Symbol<'s, 'a> {
9292

9393
/// Check if this [`Symbol`] has any [`Reference`]s that are considered a usage.
9494
pub fn has_usages(&self, options: &NoUnusedVars) -> bool {
95+
if self.is_function_or_class_declaration_used() {
96+
return true;
97+
}
98+
9599
// Use symbol flags to skip the usage checks we are certain don't need
96100
// to be run.
97101
let do_reassignment_checks = self.is_possibly_reassignable();

‎crates/oxc_linter/src/snapshots/no_unused_vars@oxc-used-declarations.snap‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,4 @@
11
---
22
source: crates/oxc_linter/src/tester.rs
3-
snapshot_kind: text
43
---
5-
⚠ eslint(no-unused-vars): Function 'foo' is declared but never used.
6-
╭─[no_unused_vars.tsx:1:12]
7-
1 │ ;[function foo() {}]
8-
· ─┬─
9-
· ╰── 'foo' is declared here
10-
╰────
11-
help: Consider removing this declaration.
124

13-
⚠ eslint(no-unused-vars): Class 'Foo' is declared but never used.
14-
╭─[no_unused_vars.tsx:1:9]
15-
1 │ ;[class Foo {}]
16-
· ─┬─
17-
· ╰── 'Foo' is declared here
18-
╰────
19-
help: Consider removing this declaration.

0 commit comments

Comments
 (0)