mirror of
https://github.com/oxc-project/oxc.git
synced 2026-11-04 10:29:58 +00:00
fix(transformer_plugins): unwrap ChainExpression after define replacement removes optional markers (#20058)
## Summary
- Fix panic when `ReplaceGlobalDefines` removes all `optional` markers
from a `ChainExpression`, causing the optional chaining lowering
transformer to hit `unreachable!()`
- Fix incorrect test expectation for `process?.env[0]` with `process.env
-> {}`
## Root Cause
When `ReplaceGlobalDefines` replaces a member expression inside a
`ChainExpression` (e.g., `process?.env[0]` with define `process.env ->
{}`), the inner `StaticMemberExpression` that carried `optional: true`
is replaced by `{}`. This leaves a `ChainExpression` wrapper with no
optional elements. When the optional chaining transformer later tries to
lower this (target < ES2020), it panics at
`transform_chain_expression_impl` in
`crates/oxc_transformer/src/es2020/optional_chaining.rs` because it
expects at least one optional element in the chain.
This affects the playground pipeline where define runs before the
transformer.
## Fix
After `walk_expression` in `visit_expression`, check if the current
expression is a `ChainExpression` with no remaining `optional` markers.
If so, unwrap it to a plain expression (e.g.,
`ChainExpression(ComputedMemberExpression(...))` becomes
`ComputedMemberExpression(...)`), producing a valid AST.
## Test plan
- [x] Added integration test `define_then_transform_optional_chain` that
reproduces the exact panic (define first, then transformer with ES2019
target)
- [x] `cargo test -p oxc_transformer_plugins` — all 42 integration tests
pass
- [x] `just lint` passes clean
---------
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Dunqing <dengqing0821@gmail.com>
This commit is contained in:
2 files changed
+159
-3
No files matched your search
@@ -2,7 +2,7 @@ use std::{cmp::Ordering, sync::Arc};
|
||||
|
||||
use rustc_hash::FxHashSet;
|
||||
|
||||
use oxc_allocator::{Address, Allocator, GetAddress, UnstableAddress};
|
||||
use oxc_allocator::{Address, Allocator, GetAddress, TakeIn, UnstableAddress};
|
||||
use oxc_ast::ast::*;
|
||||
use oxc_ast_visit::{VisitMut, walk_mut};
|
||||
use oxc_diagnostics::OxcDiagnostic;
|
||||
@@ -259,6 +259,13 @@ impl<'a> VisitMut<'a> for ReplaceGlobalDefines<'a> {
|
||||
if self.ast_node_lock == Some(expr.address()) {
|
||||
self.ast_node_lock = None;
|
||||
}
|
||||
// A define replacement inside a `ChainExpression` may remove the node that
|
||||
// carried `optional: true` (e.g. `process?.env[0]` with define `process.env -> {}`),
|
||||
// leaving an invalid `ChainExpression` with no optional elements.
|
||||
// Unwrap it to a plain expression to produce a valid AST.
|
||||
if matches!(expr, Expression::ChainExpression(_)) {
|
||||
Self::unwrap_chain_expression_if_no_optional(self.allocator, expr);
|
||||
}
|
||||
}
|
||||
|
||||
fn visit_assignment_expression(&mut self, node: &mut AssignmentExpression<'a>) {
|
||||
@@ -687,6 +694,62 @@ impl<'a> ReplaceGlobalDefines<'a> {
|
||||
if self.non_arrow_function_depth > 0 { ScopeFlags::Function } else { ScopeFlags::Top }
|
||||
}
|
||||
|
||||
/// If `expr` is a `ChainExpression` whose chain no longer contains any
|
||||
/// `optional: true` markers (because a define replacement removed them),
|
||||
/// unwrap it to a plain expression.
|
||||
fn unwrap_chain_expression_if_no_optional(allocator: &'a Allocator, expr: &mut Expression<'a>) {
|
||||
let Expression::ChainExpression(chain) = &*expr else { return };
|
||||
|
||||
// Check the chain element's optional flag and get the first object/callee to walk.
|
||||
let (optional, mut current) = match &chain.expression {
|
||||
ChainElement::CallExpression(c) => (c.optional, Some(&c.callee)),
|
||||
ChainElement::TSNonNullExpression(ts) => (false, Some(&ts.expression)),
|
||||
_ => match chain.expression.as_member_expression() {
|
||||
Some(m) => (m.optional(), Some(m.object())),
|
||||
None => return,
|
||||
},
|
||||
};
|
||||
if optional {
|
||||
return;
|
||||
}
|
||||
|
||||
// Walk down the object/callee chain. If any node has `optional: true`, keep the chain.
|
||||
while let Some(e) = current {
|
||||
match e {
|
||||
Expression::StaticMemberExpression(m) => {
|
||||
if m.optional {
|
||||
return;
|
||||
}
|
||||
current = Some(&m.object);
|
||||
}
|
||||
Expression::ComputedMemberExpression(m) => {
|
||||
if m.optional {
|
||||
return;
|
||||
}
|
||||
current = Some(&m.object);
|
||||
}
|
||||
Expression::PrivateFieldExpression(m) => {
|
||||
if m.optional {
|
||||
return;
|
||||
}
|
||||
current = Some(&m.object);
|
||||
}
|
||||
Expression::CallExpression(c) => {
|
||||
if c.optional {
|
||||
return;
|
||||
}
|
||||
current = Some(&c.callee);
|
||||
}
|
||||
_ => break,
|
||||
}
|
||||
}
|
||||
|
||||
// No optional markers remain — unwrap the chain to a plain expression.
|
||||
let chain_expr = expr.take_in(allocator);
|
||||
let Expression::ChainExpression(chain) = chain_expr else { unreachable!() };
|
||||
*expr = Expression::from(chain.unbox().expression);
|
||||
}
|
||||
|
||||
pub fn is_dot_define<'b>(
|
||||
scoping: &Scoping,
|
||||
scope_flags: ScopeFlags,
|
||||
|
||||
@@ -151,7 +151,7 @@ fn dot_with_postfix_mixed() {
|
||||
|
||||
#[test]
|
||||
fn optional_chain() {
|
||||
let config = config(&[("a.b.c", "1")]);
|
||||
let config = config(&[("a.b.c", "1"), ("process.env", "{}")]);
|
||||
test("foo(a.b.c)", "foo(1)", &config);
|
||||
test("foo(a?.b.c)", "foo(1)", &config);
|
||||
test("foo(a.b?.c)", "foo(1)", &config);
|
||||
@@ -159,9 +159,13 @@ fn optional_chain() {
|
||||
test("foo(a?.['b']['c'])", "foo(1)", &config);
|
||||
test("foo(a['b']?.['c'])", "foo(1)", &config);
|
||||
|
||||
test_same("a[b][c]", &config);
|
||||
// `process?.env` replaced by `{}`, ChainExpression unwrapped since no optional markers remain.
|
||||
test("process?.env[0]", "({})[0]", &config);
|
||||
|
||||
// Chains where optional markers remain should NOT be unwrapped.
|
||||
test_same("a?.[b][c]", &config);
|
||||
test_same("a[b]?.[c]", &config);
|
||||
test_same("a[b][c]", &config);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -323,3 +327,92 @@ log(__MEMBER__);
|
||||
let snapshot = visualizer.get_text();
|
||||
insta::assert_snapshot!("test_sourcemap", snapshot);
|
||||
}
|
||||
|
||||
/// Run ReplaceGlobalDefines then Transformer (like the playground pipeline).
|
||||
/// This reproduces the panic when define replaces the member expression that
|
||||
/// carries `optional: true`, leaving a `ChainExpression` with no optional markers.
|
||||
#[track_caller]
|
||||
fn test_define_then_transform(
|
||||
source_text: &str,
|
||||
expected: &str,
|
||||
define_config: &ReplaceGlobalDefinesConfig,
|
||||
) {
|
||||
test_define_then_transform_impl(source_text, expected, define_config, SourceType::mjs());
|
||||
}
|
||||
|
||||
#[track_caller]
|
||||
fn test_define_then_transform_ts(
|
||||
source_text: &str,
|
||||
expected: &str,
|
||||
define_config: &ReplaceGlobalDefinesConfig,
|
||||
) {
|
||||
test_define_then_transform_impl(
|
||||
source_text,
|
||||
expected,
|
||||
define_config,
|
||||
SourceType::ts().with_module(true),
|
||||
);
|
||||
}
|
||||
|
||||
#[track_caller]
|
||||
fn test_define_then_transform_impl(
|
||||
source_text: &str,
|
||||
expected: &str,
|
||||
define_config: &ReplaceGlobalDefinesConfig,
|
||||
source_type: SourceType,
|
||||
) {
|
||||
use oxc_transformer::{TransformOptions, Transformer};
|
||||
use std::path::Path;
|
||||
|
||||
let allocator = Allocator::default();
|
||||
let ret = Parser::new(&allocator, source_text, source_type).parse();
|
||||
assert!(ret.errors.is_empty());
|
||||
let mut program = ret.program;
|
||||
|
||||
// Step 1: Run define plugin first (like the playground does)
|
||||
let scoping = SemanticBuilder::new().build(&program).semantic.into_scoping();
|
||||
let _ret =
|
||||
ReplaceGlobalDefines::new(&allocator, define_config.clone()).build(scoping, &mut program);
|
||||
|
||||
// Step 2: Rebuild semantic for transformer
|
||||
let scoping =
|
||||
SemanticBuilder::new().with_excess_capacity(2.0).build(&program).semantic.into_scoping();
|
||||
|
||||
// Step 3: Run transformer with ES2019 target (lowers optional chaining)
|
||||
let options = TransformOptions::from_target("es2019").unwrap();
|
||||
let filename = if source_type.is_typescript() { "test.ts" } else { "test.mjs" };
|
||||
let ret = Transformer::new(&allocator, Path::new(filename), &options)
|
||||
.build_with_scoping(scoping, &mut program);
|
||||
assert!(ret.errors.is_empty());
|
||||
|
||||
let result = Codegen::new()
|
||||
.with_options(CodegenOptions { single_quote: true, ..CodegenOptions::default() })
|
||||
.build(&program)
|
||||
.code;
|
||||
let expected = codegen(expected, source_type);
|
||||
assert_eq!(result, expected, "for source {source_text}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn define_then_transform_optional_chain() {
|
||||
let c = config(&[("process.env", "{}")]);
|
||||
|
||||
// All optional markers removed → ChainExpression unwrapped, no panic.
|
||||
test_define_then_transform("console.log(process?.env[0]);", "console.log({}[0])", &c);
|
||||
test_define_then_transform("process?.env[0]", "({})[0]", &c);
|
||||
|
||||
// Optional markers hidden behind TS non-null assertion should still be detected.
|
||||
// `process?.env!` — TSNonNullExpression wraps the optional member.
|
||||
test_define_then_transform_ts("process?.env!", "({})", &c);
|
||||
|
||||
// Parenthesized expression wrapping an optional chain.
|
||||
test_define_then_transform("(process?.env)[0]", "({})[0]", &c);
|
||||
|
||||
// Nested chain: the inner `a?.b` is replaced by `'replaced'`, but outer `?.c` keeps optional.
|
||||
let c2 = config(&[("a.b", "'replaced'")]);
|
||||
test_define_then_transform(
|
||||
"a?.b?.c",
|
||||
"var _replaced; (_replaced = 'replaced') === null || _replaced === void 0 ? void 0 : _replaced.c",
|
||||
&c2,
|
||||
);
|
||||
}
|
||||
Reference in new issue
Block a user