fix(transformer): optional-catch-binding unused variable side effect (#2822)

Before this PR for this given case:

```javascript
const _unused = "It's a lie, They gonna use me:(";
try {
    throw 0;
} catch {
  console.log(_unused);
}
```

We would've generated this:

```javascript
const _unused = "It's a lie, They gonna use me:(";
try {
    throw 0;
} catch (_unused) {
  console.log(_unused);
}
```

This is incorrect, This PR aims to use the `CreateVars` trait in order
to ensure the variable uniqueness.

Now it would output this:

```javascript
const _unused = "It's a lie, They gonna use me:(";
try {
    throw 0;
} catch (_unused2) {
  console.log(_unused);
}
```
This commit is contained in:
Ali Rezvani 2024-03-27 09:23:02 +03:30 committed by GitHub
parent b76b02d019
commit 528744ca0a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 49 additions and 26 deletions

View file

@ -215,7 +215,7 @@ impl<'a> ExponentiationOperator<'a> {
expr: Expression<'a>, expr: Expression<'a>,
nodes: &mut Vec<'a, Expression<'a>>, nodes: &mut Vec<'a, Expression<'a>>,
) -> Expression<'a> { ) -> Expression<'a> {
let ident = self.create_new_var(&expr); let ident = self.create_new_var_with_expression(&expr);
// Add new reference `_name = name` to nodes // Add new reference `_name = name` to nodes
let target = self.ctx.ast.simple_assignment_target_identifier(ident.clone()); let target = self.ctx.ast.simple_assignment_target_identifier(ident.clone());
let op = AssignmentOperator::Assign; let op = AssignmentOperator::Assign;

View file

@ -1,9 +1,8 @@
use std::rc::Rc; use oxc_allocator::Vec;
use oxc_ast::ast::*;
use oxc_ast::{ast::*, AstBuilder};
use oxc_span::SPAN; use oxc_span::SPAN;
use crate::{context::TransformerCtx, options::TransformTarget}; use crate::{context::TransformerCtx, options::TransformTarget, utils::CreateVars};
/// ES2019: Optional Catch Binding /// ES2019: Optional Catch Binding
/// ///
@ -11,22 +10,34 @@ use crate::{context::TransformerCtx, options::TransformTarget};
/// * <https://babel.dev/docs/babel-plugin-transform-optional-catch-binding> /// * <https://babel.dev/docs/babel-plugin-transform-optional-catch-binding>
/// * <https://github.com/babel/babel/tree/main/packages/babel-plugin-transform-optional-catch-binding> /// * <https://github.com/babel/babel/tree/main/packages/babel-plugin-transform-optional-catch-binding>
pub struct OptionalCatchBinding<'a> { pub struct OptionalCatchBinding<'a> {
ast: Rc<AstBuilder<'a>>, ctx: TransformerCtx<'a>,
vars: Vec<'a, VariableDeclarator<'a>>,
}
impl<'a> CreateVars<'a> for OptionalCatchBinding<'a> {
fn ctx(&self) -> &TransformerCtx<'a> {
&self.ctx
}
fn vars_mut(&mut self) -> &mut Vec<'a, VariableDeclarator<'a>> {
&mut self.vars
}
} }
impl<'a> OptionalCatchBinding<'a> { impl<'a> OptionalCatchBinding<'a> {
pub fn new(ctx: TransformerCtx<'a>) -> Option<Self> { pub fn new(ctx: TransformerCtx<'a>) -> Option<Self> {
(ctx.options.target < TransformTarget::ES2019 || ctx.options.optional_catch_binding) (ctx.options.target < TransformTarget::ES2019 || ctx.options.optional_catch_binding)
.then_some(Self { ast: ctx.ast }) .then_some(Self { vars: ctx.ast.new_vec(), ctx })
} }
pub fn transform_catch_clause<'b>(&mut self, clause: &'b mut CatchClause<'a>) { pub fn transform_catch_clause<'b>(&mut self, clause: &'b mut CatchClause<'a>) {
if clause.param.is_some() { if clause.param.is_some() {
return; return;
} }
let binding_identifier = BindingIdentifier::new(SPAN, "_unused".into()); let unused = self.create_new_named_var("unused");
let binding_pattern_kind = self.ast.binding_pattern_identifier(binding_identifier); let binding_identifier = BindingIdentifier::new(SPAN, unused.name);
let binding_pattern = self.ast.binding_pattern(binding_pattern_kind, None, false); let binding_pattern_kind = self.ctx.ast.binding_pattern_identifier(binding_identifier);
let binding_pattern = self.ctx.ast.binding_pattern(binding_pattern_kind, None, false);
clause.param = Some(binding_pattern); clause.param = Some(binding_pattern);
} }
} }

View file

@ -63,7 +63,7 @@ impl<'a> NullishCoalescingOperator<'a> {
reference = self.ctx.ast.copy(&logical_expr.left); reference = self.ctx.ast.copy(&logical_expr.left);
assignment = self.ctx.ast.copy(&logical_expr.left); assignment = self.ctx.ast.copy(&logical_expr.left);
} else { } else {
let ident = self.create_new_var(&logical_expr.left); let ident = self.create_new_var_with_expression(&logical_expr.left);
reference = self.ctx.ast.identifier_reference_expression(ident.clone()); reference = self.ctx.ast.identifier_reference_expression(ident.clone());
let left = self.ctx.ast.simple_assignment_target_identifier(ident); let left = self.ctx.ast.simple_assignment_target_identifier(ident);
let right = self.ctx.ast.copy(&logical_expr.left); let right = self.ctx.ast.copy(&logical_expr.left);

View file

@ -2,7 +2,7 @@ use std::mem;
use oxc_allocator::Vec; use oxc_allocator::Vec;
use oxc_ast::ast::*; use oxc_ast::ast::*;
use oxc_span::Span; use oxc_span::{CompactStr, Span};
use oxc_syntax::identifier::is_identifier_name; use oxc_syntax::identifier::is_identifier_name;
use crate::context::TransformerCtx; use crate::context::TransformerCtx;
@ -25,21 +25,14 @@ pub trait CreateVars<'a> {
stmts.insert(0, stmt); stmts.insert(0, stmt);
} }
fn create_new_var(&mut self, expr: &Expression<'a>) -> IdentifierReference<'a> { fn create_new_var_with_expression(&mut self, expr: &Expression<'a>) -> IdentifierReference<'a> {
let name = self.ctx().scopes().generate_uid_based_on_node(expr); let name = self.ctx().scopes().generate_uid_based_on_node(expr);
self.ctx().add_binding(name.clone()); create_new_var(self, name)
}
// Add `var name` to scope fn create_new_named_var(&mut self, name: &'a str) -> IdentifierReference<'a> {
// TODO: hookup symbol id let name = self.ctx().scopes().generate_uid(name);
let name = self.ctx().ast.new_atom(name.as_str()); create_new_var(self, name)
let binding_identifier = BindingIdentifier::new(Span::default(), name.clone());
let binding_pattern_kind = self.ctx().ast.binding_pattern_identifier(binding_identifier);
let binding = self.ctx().ast.binding_pattern(binding_pattern_kind, None, false);
let kind = VariableDeclarationKind::Var;
let decl = self.ctx().ast.variable_declarator(Span::default(), kind, binding, None, false);
self.vars_mut().push(decl);
// TODO: add reference id and flag
IdentifierReference::new(Span::default(), name)
} }
/// Possibly generate a memoised identifier if it is not static and has consequences. /// Possibly generate a memoised identifier if it is not static and has consequences.
@ -51,7 +44,7 @@ pub trait CreateVars<'a> {
if self.ctx().symbols().is_static(expr) { if self.ctx().symbols().is_static(expr) {
None None
} else { } else {
Some(self.create_new_var(expr)) Some(self.create_new_var_with_expression(expr))
} }
} }
} }
@ -152,3 +145,22 @@ pub fn is_reserved_word(name: &str, in_module: bool) -> bool {
pub fn is_valid_es3_identifier(name: &str) -> bool { pub fn is_valid_es3_identifier(name: &str) -> bool {
is_valid_identifier(name, true) && !RESERVED_WORDS_ES3_ONLY.contains(name) is_valid_identifier(name, true) && !RESERVED_WORDS_ES3_ONLY.contains(name)
} }
fn create_new_var<'a, V: CreateVars<'a> + ?Sized>(
create_vars: &mut V,
name: CompactStr,
) -> IdentifierReference<'a> {
// Add `var name` to scope
// TODO: hookup symbol id
let atom = create_vars.ctx().ast.new_atom(name.as_str());
let binding_identifier = BindingIdentifier::new(Span::default(), atom.clone());
let binding_pattern_kind = create_vars.ctx().ast.binding_pattern_identifier(binding_identifier);
let binding = create_vars.ctx().ast.binding_pattern(binding_pattern_kind, None, false);
let kind = VariableDeclarationKind::Var;
let decl =
create_vars.ctx().ast.variable_declarator(Span::default(), kind, binding, None, false);
create_vars.ctx().add_binding(name);
create_vars.vars_mut().push(decl);
// TODO: add reference id and flag
IdentifierReference::new(Span::default(), atom)
}