mirror of
https://github.com/danbulant/oxc
synced 2026-05-19 12:19:15 +00:00
refactor(linter/no-unused-vars): improve implementation to remove using SymbolFlags::Export (#7412)
part of #7414
This commit is contained in:
parent
7ff9f13973
commit
c8adc46a08
6 changed files with 31 additions and 114 deletions
|
|
@ -2,84 +2,35 @@
|
|||
//! consider variables ignored by name pattern, but by where they are declared.
|
||||
use oxc_ast::{ast::*, AstKind};
|
||||
use oxc_semantic::{NodeId, Semantic};
|
||||
use oxc_span::GetSpan;
|
||||
|
||||
use super::{options::ArgsOption, NoUnusedVars, Symbol};
|
||||
use crate::rules::eslint::no_unused_vars::binding_pattern::{BindingContext, HasAnyUsedBinding};
|
||||
|
||||
impl<'s, 'a> Symbol<'s, 'a> {
|
||||
/// Returns `true` if this function is use.
|
||||
/// Check if the declaration of this [`Symbol`] is use.
|
||||
///
|
||||
/// Checks for these cases
|
||||
/// 1. passed as a callback to another [`CallExpression`] or [`NewExpression`]
|
||||
/// 2. invoked as an IIFE
|
||||
/// 3. Returned from another function
|
||||
/// 4. Used as an attribute in a JSX element
|
||||
/// If it's an expression, then it's always passed in as an argument
|
||||
/// or assigned to a variable, so it's always used.
|
||||
///
|
||||
/// ```js
|
||||
/// // True:
|
||||
/// const a = class Name{}
|
||||
/// export default (function Name() {})
|
||||
/// console.log(function Name() {})
|
||||
///
|
||||
/// // False
|
||||
/// function foo() {}
|
||||
/// {
|
||||
/// class Foo {}
|
||||
/// }
|
||||
/// ```
|
||||
#[inline]
|
||||
pub fn is_function_or_class_declaration_used(&self) -> bool {
|
||||
#[cfg(debug_assertions)]
|
||||
{
|
||||
let kind = self.declaration().kind();
|
||||
assert!(kind.is_function_like() || matches!(kind, AstKind::Class(_)));
|
||||
pub(crate) fn is_function_or_class_declaration_used(&self) -> bool {
|
||||
match self.declaration().kind() {
|
||||
AstKind::Class(class) => class.is_expression(),
|
||||
AstKind::Function(func) => func.is_expression(),
|
||||
_ => false,
|
||||
}
|
||||
|
||||
for parent in self.iter_relevant_parents() {
|
||||
match parent.kind() {
|
||||
AstKind::MemberExpression(_) | AstKind::ParenthesizedExpression(_)
|
||||
// e.g. `const x = [function foo() {}]`
|
||||
// Only considered used if the array containing the symbol is used.
|
||||
| AstKind::ArrayExpressionElement(_)
|
||||
| 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;
|
||||
}
|
||||
// Returned from another function. Definitely won't be the same
|
||||
// function because we're walking up from its declaration
|
||||
AstKind::ReturnStatement(_)
|
||||
// <Component onClick={function onClick(e) { }} />
|
||||
| AstKind::JSXExpressionContainer(_)
|
||||
// Function declaration is passed as an argument to another function.
|
||||
| AstKind::CallExpression(_) | AstKind::Argument(_)
|
||||
// e.g. `const x = { foo: function foo() {} }`
|
||||
// Allowed off-the-bat since objects being the only child of an
|
||||
// ExpressionStatement is rare, since you would need to wrap the
|
||||
// object in parentheses to avoid creating a block statement.
|
||||
| AstKind::ObjectProperty(_)
|
||||
// e.g. var foo = function bar() { }
|
||||
// we don't want to check for violations on `bar`, just `foo`
|
||||
| AstKind::VariableDeclarator(_)
|
||||
// new (class CustomRenderer{})
|
||||
// new (function() {})
|
||||
| AstKind::NewExpression(_)
|
||||
=> {
|
||||
return true;
|
||||
}
|
||||
// !function() {}; is an IIFE
|
||||
AstKind::UnaryExpression(expr) => return expr.operator.is_not(),
|
||||
// function is used as a value for an assignment
|
||||
// e.g. Array.prototype.sort ||= function sort(a, b) { }
|
||||
AstKind::AssignmentExpression(assignment) if assignment.right.span().contains_inclusive(self.span()) => {
|
||||
return self != &assignment.left;
|
||||
}
|
||||
AstKind::ExpressionStatement(_) => {
|
||||
// implicit return in arrow function expression
|
||||
let Some(AstKind::FunctionBody(body)) = self.nodes().parent_kind(parent.id()) else {
|
||||
return false;
|
||||
};
|
||||
return body.span.contains_inclusive(self.span()) && body.statements.len() == 1 && !self.get_snippet(body.span).starts_with('{')
|
||||
}
|
||||
_ => {
|
||||
parent.kind().debug_name();
|
||||
return false;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
false
|
||||
}
|
||||
|
||||
fn is_declared_in_for_of_loop(&self) -> bool {
|
||||
|
|
@ -124,12 +75,6 @@ fn is_ambient_namespace(namespace: &TSModuleDeclaration) -> bool {
|
|||
}
|
||||
|
||||
impl NoUnusedVars {
|
||||
#[allow(clippy::unused_self)]
|
||||
pub(super) fn is_allowed_class_or_function(&self, symbol: &Symbol<'_, '_>) -> bool {
|
||||
symbol.is_function_or_class_declaration_used()
|
||||
// || symbol.is_function_or_class_assigned_to_same_name_variable()
|
||||
}
|
||||
|
||||
#[allow(clippy::unused_self)]
|
||||
pub(super) fn is_allowed_ts_namespace<'a>(
|
||||
&self,
|
||||
|
|
|
|||
|
|
@ -232,8 +232,7 @@ impl NoUnusedVars {
|
|||
}
|
||||
|
||||
// Order matters. We want to call cheap/high "yield" functions first.
|
||||
let is_exported = symbol.is_exported();
|
||||
let is_used = is_exported || symbol.has_usages(self);
|
||||
let is_used = symbol.is_exported() || symbol.has_usages(self);
|
||||
|
||||
match (is_used, is_ignored) {
|
||||
(true, true) => {
|
||||
|
|
@ -306,12 +305,6 @@ impl NoUnusedVars {
|
|||
}
|
||||
ctx.diagnostic(diagnostic::declared(symbol, &self.vars_ignore_pattern));
|
||||
}
|
||||
AstKind::Class(_) | AstKind::Function(_) => {
|
||||
if self.is_allowed_class_or_function(symbol) {
|
||||
return;
|
||||
}
|
||||
ctx.diagnostic(diagnostic::declared(symbol, &IgnorePattern::<&str>::None));
|
||||
}
|
||||
AstKind::TSModuleDeclaration(namespace) => {
|
||||
if self.is_allowed_ts_namespace(symbol, namespace) {
|
||||
return;
|
||||
|
|
|
|||
|
|
@ -105,11 +105,6 @@ impl<'s, 'a> Symbol<'s, 'a> {
|
|||
self.nodes().ancestors(self.declaration_id())
|
||||
}
|
||||
|
||||
#[inline]
|
||||
pub fn iter_relevant_parents(&self) -> impl Iterator<Item = &AstNode<'a>> + Clone + '_ {
|
||||
self.iter_relevant_parents_of(self.declaration_id())
|
||||
}
|
||||
|
||||
pub fn iter_relevant_parents_of(
|
||||
&self,
|
||||
node_id: NodeId,
|
||||
|
|
@ -176,10 +171,9 @@ impl<'s, 'a> Symbol<'s, 'a> {
|
|||
/// NOTE: does not support CJS right now.
|
||||
pub fn is_exported(&self) -> bool {
|
||||
let is_in_exportable_scope = self.is_root() || self.is_in_ts_namespace();
|
||||
(is_in_exportable_scope
|
||||
&& (self.flags.contains(SymbolFlags::Export)
|
||||
|| self.semantic.module_record().exported_bindings.contains_key(self.name())))
|
||||
|| self.in_export_node()
|
||||
is_in_exportable_scope
|
||||
&& (self.semantic.module_record().exported_bindings.contains_key(self.name())
|
||||
|| self.in_export_node())
|
||||
}
|
||||
|
||||
#[inline]
|
||||
|
|
@ -194,7 +188,7 @@ impl<'s, 'a> Symbol<'s, 'a> {
|
|||
AstKind::ModuleDeclaration(module) => {
|
||||
return module.is_export();
|
||||
}
|
||||
AstKind::ExportDefaultDeclaration(_) => {
|
||||
AstKind::ExportNamedDeclaration(_) | AstKind::ExportDefaultDeclaration(_) => {
|
||||
return true;
|
||||
}
|
||||
AstKind::VariableDeclaration(_)
|
||||
|
|
|
|||
|
|
@ -819,11 +819,7 @@ fn test_used_declarations() {
|
|||
"export const Foo = class Bar {}",
|
||||
"export const Foo = @SomeDecorator() class Foo {}",
|
||||
];
|
||||
let fail = vec![
|
||||
// array is not used, so the function is not used
|
||||
";[function foo() {}]",
|
||||
";[class Foo {}]",
|
||||
];
|
||||
let fail = vec![];
|
||||
|
||||
Tester::new(NoUnusedVars::NAME, pass, fail)
|
||||
.intentionally_allow_no_fix_tests()
|
||||
|
|
|
|||
|
|
@ -92,6 +92,10 @@ impl<'s, 'a> Symbol<'s, 'a> {
|
|||
|
||||
/// Check if this [`Symbol`] has any [`Reference`]s that are considered a usage.
|
||||
pub fn has_usages(&self, options: &NoUnusedVars) -> bool {
|
||||
if self.is_function_or_class_declaration_used() {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Use symbol flags to skip the usage checks we are certain don't need
|
||||
// to be run.
|
||||
let do_reassignment_checks = self.is_possibly_reassignable();
|
||||
|
|
|
|||
|
|
@ -1,19 +1,4 @@
|
|||
---
|
||||
source: crates/oxc_linter/src/tester.rs
|
||||
snapshot_kind: text
|
||||
---
|
||||
⚠ eslint(no-unused-vars): Function 'foo' is declared but never used.
|
||||
╭─[no_unused_vars.tsx:1:12]
|
||||
1 │ ;[function foo() {}]
|
||||
· ─┬─
|
||||
· ╰── 'foo' is declared here
|
||||
╰────
|
||||
help: Consider removing this declaration.
|
||||
|
||||
⚠ eslint(no-unused-vars): Class 'Foo' is declared but never used.
|
||||
╭─[no_unused_vars.tsx:1:9]
|
||||
1 │ ;[class Foo {}]
|
||||
· ─┬─
|
||||
· ╰── 'Foo' is declared here
|
||||
╰────
|
||||
help: Consider removing this declaration.
|
||||
|
|
|
|||
Loading…
Reference in a new issue