Skip to content

Commit

Permalink
fix(transformer): optional-catch-binding unused variable side effect (#…
Browse files Browse the repository at this point in the history
…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);
}
```
  • Loading branch information
rzvxa committed Mar 27, 2024
1 parent b76b02d commit 528744c
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 27 deletions.
Expand Up @@ -215,7 +215,7 @@ impl<'a> ExponentiationOperator<'a> {
expr: Expression<'a>,
nodes: &mut Vec<'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
let target = self.ctx.ast.simple_assignment_target_identifier(ident.clone());
let op = AssignmentOperator::Assign;
Expand Down
29 changes: 20 additions & 9 deletions crates/oxc_transformer/src/es2019/optional_catch_binding.rs
@@ -1,32 +1,43 @@
use std::rc::Rc;

use oxc_ast::{ast::*, AstBuilder};
use oxc_allocator::Vec;
use oxc_ast::ast::*;
use oxc_span::SPAN;

use crate::{context::TransformerCtx, options::TransformTarget};
use crate::{context::TransformerCtx, options::TransformTarget, utils::CreateVars};

/// ES2019: Optional Catch Binding
///
/// References:
/// * <https://babel.dev/docs/babel-plugin-transform-optional-catch-binding>
/// * <https://github.com/babel/babel/tree/main/packages/babel-plugin-transform-optional-catch-binding>
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> {
pub fn new(ctx: TransformerCtx<'a>) -> Option<Self> {
(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>) {
if clause.param.is_some() {
return;
}
let binding_identifier = BindingIdentifier::new(SPAN, "_unused".into());
let binding_pattern_kind = self.ast.binding_pattern_identifier(binding_identifier);
let binding_pattern = self.ast.binding_pattern(binding_pattern_kind, None, false);
let unused = self.create_new_named_var("unused");
let binding_identifier = BindingIdentifier::new(SPAN, unused.name);
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);
}
}
Expand Up @@ -63,7 +63,7 @@ impl<'a> NullishCoalescingOperator<'a> {
reference = self.ctx.ast.copy(&logical_expr.left);
assignment = self.ctx.ast.copy(&logical_expr.left);
} 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());
let left = self.ctx.ast.simple_assignment_target_identifier(ident);
let right = self.ctx.ast.copy(&logical_expr.left);
Expand Down
44 changes: 28 additions & 16 deletions crates/oxc_transformer/src/utils.rs
Expand Up @@ -2,7 +2,7 @@ use std::mem;

use oxc_allocator::Vec;
use oxc_ast::ast::*;
use oxc_span::Span;
use oxc_span::{CompactStr, Span};
use oxc_syntax::identifier::is_identifier_name;

use crate::context::TransformerCtx;
Expand All @@ -25,21 +25,14 @@ pub trait CreateVars<'a> {
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);
self.ctx().add_binding(name.clone());

// Add `var name` to scope
// TODO: hookup symbol id
let name = self.ctx().ast.new_atom(name.as_str());
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)
create_new_var(self, name)
}

fn create_new_named_var(&mut self, name: &'a str) -> IdentifierReference<'a> {
let name = self.ctx().scopes().generate_uid(name);
create_new_var(self, name)
}

/// Possibly generate a memoised identifier if it is not static and has consequences.
Expand All @@ -51,7 +44,7 @@ pub trait CreateVars<'a> {
if self.ctx().symbols().is_static(expr) {
None
} else {
Some(self.create_new_var(expr))
Some(self.create_new_var_with_expression(expr))
}
}
}
Expand Down Expand Up @@ -152,3 +145,22 @@ pub fn is_reserved_word(name: &str, in_module: bool) -> bool {
pub fn is_valid_es3_identifier(name: &str) -> bool {
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)
}

0 comments on commit 528744c

Please sign in to comment.