Skip to content

Commit

Permalink
feat(transformer/nullish-coalescing-operator): handles nullish coales…
Browse files Browse the repository at this point in the history 
…cing expression in the FormalParamter (#4975)

### What I did in this PR
1. Replace `self.clone_identifier_reference` with `ctx.clone_identifier.reference`
2. Remove the usage of `ast.copy`
3. Handle below example correctly

### Example

```js
// Input
var foo = object.foo ?? "default";

// Output
var _object$foo;
var foo =
(_object$foo = object.foo) !== null && _object$foo !== void 0
  ? _object$foo
  : "default";
```
  • Loading branch information
@Dunqing
Dunqing committed Aug 20, 2024
1 parent f794870 commit f51d3f9
Show file tree
Hide file tree
Showing 2 changed files with 137 additions and 68 deletions.
191 changes: 135 additions & 56 deletions crates/oxc_transformer/src/es2020/nullish_coalescing_operator.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@
use std::cell::Cell;

use oxc_semantic::{ReferenceFlag, SymbolFlags};
use oxc_traverse::{Traverse, TraverseCtx};
use oxc_semantic::{ReferenceFlag, ScopeFlags, ScopeId, SymbolFlags};
use oxc_traverse::{Ancestor, Traverse, TraverseCtx};

use oxc_allocator::{CloneIn, Vec};
use oxc_ast::ast::*;
Expand All @@ -50,52 +50,72 @@ impl<'a> NullishCoalescingOperator<'a> {
Self { _ctx: ctx, var_declarations: vec![] }
}

fn clone_identifier_reference(
ident: &IdentifierReference<'a>,
ctx: &mut TraverseCtx<'a>,
) -> IdentifierReference<'a> {
let reference = ctx.symbols().get_reference(ident.reference_id.get().unwrap());
let symbol_id = reference.symbol_id();
let flag = reference.flag();
ctx.create_reference_id(ident.span, ident.name.clone(), symbol_id, *flag)
}

fn clone_expression(expr: &Expression<'a>, ctx: &mut TraverseCtx<'a>) -> Expression<'a> {
match expr {
Expression::Identifier(ident) => ctx
.ast
.expression_from_identifier_reference(Self::clone_identifier_reference(ident, ctx)),
Expression::Identifier(ident) => ctx.ast.expression_from_identifier_reference(
ctx.clone_identifier_reference(ident, ReferenceFlag::Read),
),
_ => expr.clone_in(ctx.ast.allocator),
}
}

fn create_new_var_with_expression(
&mut self,
expr: &Expression<'a>,
current_scope_id: ScopeId,
ctx: &mut TraverseCtx<'a>,
) -> IdentifierReference<'a> {
) -> (BindingPattern<'a>, IdentifierReference<'a>) {
// Add `var name` to scope
let symbol_id = ctx
.generate_uid_in_current_scope_based_on_node(expr, SymbolFlags::FunctionScopedVariable);
let symbol_id = ctx.generate_uid_based_on_node(
expr,
current_scope_id,
SymbolFlags::FunctionScopedVariable,
);
let symbol_name = ctx.ast.atom(ctx.symbols().get_name(symbol_id));

{
// var _name;
let binding_identifier = BindingIdentifier {
span: SPAN,
name: symbol_name.clone(),
symbol_id: Cell::new(Some(symbol_id)),
};
let kind = VariableDeclarationKind::Var;
let id = ctx.ast.binding_pattern_kind_from_binding_identifier(binding_identifier);
let id = ctx.ast.binding_pattern(id, None::<TSTypeAnnotation<'_>>, false);
self.var_declarations
.last_mut()
.unwrap()
.push(ctx.ast.variable_declarator(SPAN, kind, id, None, false));
// var _name;
let binding_identifier = BindingIdentifier {
span: SPAN,
name: symbol_name.clone(),
symbol_id: Cell::new(Some(symbol_id)),
};
let id = ctx.ast.binding_pattern_kind_from_binding_identifier(binding_identifier);
let id = ctx.ast.binding_pattern(id, None::<TSTypeAnnotation<'_>>, false);
let reference =
ctx.create_reference_id(SPAN, symbol_name, Some(symbol_id), ReferenceFlag::Read);

ctx.create_reference_id(SPAN, symbol_name, Some(symbol_id), ReferenceFlag::Read)
(id, reference)
}

/// Create a conditional expression
///
/// ```js
/// // Input
/// bar ?? "qux"
///
/// // Output
/// qux = bar !== null && bar !== void 0 ? bar : "qux"
/// // ^^^ assignment ^^^ reference ^^^ default
/// ```
///
/// reference and assignment are the same in this case, but they can be different
fn create_conditional_expression(
reference: Expression<'a>,
assignment: Expression<'a>,
default: Expression<'a>,
ctx: &mut TraverseCtx<'a>,
) -> Expression<'a> {
let op = BinaryOperator::StrictInequality;
let null = ctx.ast.expression_null_literal(SPAN);
let left = ctx.ast.expression_binary(SPAN, assignment, op, null);
let right = ctx.ast.expression_binary(
SPAN,
Self::clone_expression(&reference, ctx),
op,
ctx.ast.void_0(),
);
let test = ctx.ast.expression_logical(SPAN, left, LogicalOperator::And, right);

ctx.ast.expression_conditional(SPAN, test, reference, default)
}
}

Expand Down Expand Up @@ -137,33 +157,92 @@ impl<'a> Traverse<'a> for NullishCoalescingOperator<'a> {
};

// skip creating extra reference when `left` is static
let (reference, assignment) = if ctx.symbols().is_static(&logical_expr.left) {
(Self::clone_expression(&logical_expr.left, ctx), logical_expr.left)
if ctx.symbols().is_static(&logical_expr.left) {
*expr = Self::create_conditional_expression(
Self::clone_expression(&logical_expr.left, ctx),
logical_expr.left,
logical_expr.right,
ctx,
);
return;
}

// ctx.ancestor(1) is AssignmentPattern
// ctx.ancestor(2) is BindingPattern;
// ctx.ancestor(3) is FormalParameter
let is_parent_formal_parameter = ctx
.ancestor(3)
.is_some_and(|ancestor| matches!(ancestor, Ancestor::FormalParameterPattern(_)));

let current_scope_id = if is_parent_formal_parameter {
ctx.create_child_scope_of_current(ScopeFlags::Arrow | ScopeFlags::Function)
} else {
let ident = self.create_new_var_with_expression(&logical_expr.left, ctx);
let left =
AssignmentTarget::from(ctx.ast.simple_assignment_target_from_identifier_reference(
Self::clone_identifier_reference(&ident, ctx),
));
let right = logical_expr.left;
(
ctx.ast.expression_from_identifier_reference(ident),
ctx.ast.expression_assignment(SPAN, AssignmentOperator::Assign, left, right),
)
ctx.current_scope_id()
};

let op = BinaryOperator::StrictInequality;
let null = ctx.ast.expression_null_literal(SPAN);
let left = ctx.ast.expression_binary(SPAN, assignment, op, null);
let right = ctx.ast.expression_binary(
let (id, ident) =
Self::create_new_var_with_expression(&logical_expr.left, current_scope_id, ctx);

let left =
AssignmentTarget::from(ctx.ast.simple_assignment_target_from_identifier_reference(
ctx.clone_identifier_reference(&ident, ReferenceFlag::Write),
));

let reference = ctx.ast.expression_from_identifier_reference(ident);
let assignment = ctx.ast.expression_assignment(
SPAN,
// SAFETY: `ast.copy` is unsound! We need to fix.
unsafe { ctx.ast.copy(&reference) },
op,
ctx.ast.void_0(),
AssignmentOperator::Assign,
left,
logical_expr.left,
);
let test = ctx.ast.expression_logical(SPAN, left, LogicalOperator::And, right);

*expr = ctx.ast.expression_conditional(SPAN, test, reference, logical_expr.right);
let mut new_expr =
Self::create_conditional_expression(reference, assignment, logical_expr.right, ctx);

if is_parent_formal_parameter {
// Replace `function (a, x = a.b ?? c) {}` to `function (a, x = (() => a.b ?? c)() ){}`
// so the temporary variable can be injected in correct scope
let param = ctx.ast.formal_parameter(SPAN, ctx.ast.vec(), id, None, false, false);
let params = ctx.ast.formal_parameters(
SPAN,
FormalParameterKind::ArrowFormalParameters,
ctx.ast.vec1(param),
None::<BindingRestElement>,
);
let body = ctx.ast.function_body(
SPAN,
ctx.ast.vec(),
ctx.ast.vec1(ctx.ast.statement_expression(SPAN, new_expr)),
);
let type_parameters = None::<TSTypeParameterDeclaration>;
let type_annotation = None::<TSTypeAnnotation>;
let arrow_function = ctx.ast.arrow_function_expression(
SPAN,
true,
false,
type_parameters,
params,
type_annotation,
body,
);
arrow_function.scope_id.set(Some(current_scope_id));
let arrow_function = ctx.ast.expression_from_arrow_function(arrow_function);
// `(x) => x;` -> `((x) => x)();`
new_expr = ctx.ast.expression_call(
SPAN,
ctx.ast.vec(),
arrow_function,
None::<TSTypeParameterInstantiation>,
false,
);
} else {
let kind = VariableDeclarationKind::Var;
self.var_declarations
.last_mut()
.unwrap()
.push(ctx.ast.variable_declarator(SPAN, kind, id, None, false));
}

*expr = new_expr;
}
}
14 changes: 2 additions & 12 deletions tasks/transform_conformance/babel.snap.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1308,12 +1308,7 @@ Symbols mismatch after transform
ReferenceId mismatch after transform

* assumption-noDocumentAll/transform-in-default-param/input.js
Bindings Mismatch:
previous scope ScopeId(0): ["bar", "foo"]
current scope ScopeId(0): ["_foo$bar", "bar", "foo"]
Bindings Mismatch:
previous scope ScopeId(1): ["_foo$bar", "foo", "qux"]
current scope ScopeId(1): ["foo", "qux"]
Scopes mismatch after transform
Symbols mismatch after transform
ReferenceId mismatch after transform

Expand All @@ -1332,12 +1327,7 @@ Symbols mismatch after transform
ReferenceId mismatch after transform

* nullish-coalescing/transform-in-default-param/input.js
Bindings Mismatch:
previous scope ScopeId(0): ["bar", "foo"]
current scope ScopeId(0): ["_foo$bar", "bar", "foo"]
Bindings Mismatch:
previous scope ScopeId(1): ["_foo$bar", "foo", "qux"]
current scope ScopeId(1): ["foo", "qux"]
Scopes mismatch after transform
Symbols mismatch after transform
ReferenceId mismatch after transform

Expand Down

0 comments on commit f51d3f9

Please sign in to comment.