Skip to content

Commit

Permalink
fix(linter/no-unused-vars): panic on UsingDeclarations (#5206)
Browse files Browse the repository at this point in the history
Closes #5202
  • Loading branch information
DonIsaac committed Aug 25, 2024
1 parent 982bd6e commit 8ff6f2c
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,26 @@ impl NoUnusedVars {
return fixer.noop();
}

let Some(AstKind::VariableDeclaration(declaration)) =
symbol.nodes().parent_node(decl_id).map(AstNode::kind)
else {
panic!("VariableDeclarator nodes should always be direct children of VariableDeclaration nodes");
let Some(parent) = symbol.nodes().parent_node(decl_id).map(AstNode::kind) else {
#[cfg(debug_assertions)]
panic!("VariableDeclarator nodes should always have a parent node");
#[cfg(not(debug_assertions))]
return fixer.noop();
};
let (span, declarations) = match parent {
AstKind::VariableDeclaration(decl) => (decl.span, &decl.declarations),
AstKind::UsingDeclaration(decl) => {
if decl.is_await {
return fixer.noop();
}
(decl.span, &decl.declarations)
}
_ => {
#[cfg(debug_assertions)]
panic!("VariableDeclarator nodes should always be direct children of VariableDeclaration or UsingDeclaration nodes");
#[cfg(not(debug_assertions))]
return fixer.noop();
}
};

// `true` even if references aren't considered a usage.
Expand All @@ -48,18 +64,16 @@ impl NoUnusedVars {
// for `let x = 1;` or `const { x } = obj; the whole declaration can
// be removed, but for `const { x, y } = obj;` or `let x = 1, y = 2`
// we need to keep the other declarations
let has_neighbors = declaration.declarations.len() > 1;
debug_assert!(!declaration.declarations.is_empty());
let has_neighbors = declarations.len() > 1;
debug_assert!(!declarations.is_empty());
let binding_info = symbol.get_binding_info(&decl.id.kind);

match binding_info {
BindingInfo::SingleDestructure | BindingInfo::NotDestructure => {
if has_neighbors {
return symbol
.delete_from_list(fixer, &declaration.declarations, decl)
.dangerously();
return symbol.delete_from_list(fixer, declarations, decl).dangerously();
}
return fixer.delete(declaration).dangerously();
return fixer.delete_range(span).dangerously();
}
BindingInfo::MultiDestructure(mut span, is_object, is_last) => {
let source_after = &fixer.source_text()[(span.end as usize)..];
Expand Down
10 changes: 10 additions & 0 deletions crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/oxc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,16 @@ fn test_vars_catch() {
.test_and_snapshot();
}

#[test]
fn test_vars_using() {
let pass = vec![("using a = 1; console.log(a)", None)];

let fail = vec![("using a = 1;", None)];

Tester::new(NoUnusedVars::NAME, pass, fail)
.with_snapshot_suffix("oxc-vars-using")
.test_and_snapshot();
}
#[test]
fn test_functions() {
let pass = vec![
Expand Down
10 changes: 10 additions & 0 deletions crates/oxc_linter/src/snapshots/no_unused_vars@oxc-vars-using.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
source: crates/oxc_linter/src/tester.rs
---
eslint(no-unused-vars): Variable 'a' is declared but never used.
╭─[no_unused_vars.tsx:1:7]
1using a = 1;
· ┬
· ╰── 'a' is declared here
╰────
help: Consider removing this declaration.

0 comments on commit 8ff6f2c

Please sign in to comment.