diff --git a/crates/perry-hir/src/lower/expr_assign.rs b/crates/perry-hir/src/lower/expr_assign.rs index bd457f7fc..35b57df09 100644 --- a/crates/perry-hir/src/lower/expr_assign.rs +++ b/crates/perry-hir/src/lower/expr_assign.rs @@ -70,7 +70,7 @@ fn lower_rhs_with_assignment_name( result } -fn throw_type_error_const_assignment(name: &str) -> Expr { +pub(crate) fn throw_type_error_const_assignment(name: &str) -> Expr { Expr::Call { callee: Box::new(Expr::ExternFuncRef { name: "js_throw_type_error_const_assignment".to_string(), @@ -413,6 +413,97 @@ pub(super) fn lower_assign(ctx: &mut LoweringContext, assign: &ast::AssignExpr) lower_assignment_target(ctx, &assign.left, value) } +/// Lower ` = value` where `` is an identifier assignment target. +/// +/// #6300: this is the ONE place identifier stores are resolved, so the +/// `const`-immutability (and class-inner-name) checks can't be routed around. +/// It is shared by the bare-`Ident` `AssignTarget` arm below and by +/// `lower_expr_assignment`'s `Ident` arm, which is what the parenthesized / +/// TS-cast targets (`(c) = 9`, `(c as any) = 9`, `(c satisfies T) = 9`, +/// `(c!) = 9`) unwrap into. Before the extraction, only the bare-`Ident` arm +/// checked immutability, so any wrapper on the LHS silently mutated a `const`. +pub(crate) fn lower_ident_assignment( + ctx: &mut LoweringContext, + name: String, + value: Box, +) -> Result { + if let Some(env_id) = ctx.active_with_envs_for_ident(&name).into_iter().next() { + let fallback = with_set_fallback_for_ident(ctx, &name); + return Ok(Expr::WithSet { + object: Box::new(Expr::LocalGet(env_id)), + property: name, + value, + fallback, + strict: ctx.current_strict, + }); + } + if let Some(id) = ctx.lookup_local(&name) { + if ctx.is_local_immutable(id) { + // `const c = 1; c = 9` (and every wrapped spelling of the same + // target) evaluates the RHS for side effects, then throws + // `TypeError: Assignment to constant variable.` + return Ok(Expr::Sequence(vec![ + *value, + throw_type_error_const_assignment(&name), + ])); + } + Ok(Expr::LocalSet(id, value)) + } else if ctx.current_class_inner_name.as_deref() == Some(name.as_str()) { + // Assigning to the class own-name binding from inside the class + // body targets the immutable inner `const` binding -> TypeError + // (test262 language/statements/class/name-binding/const). Evaluate + // the RHS for side effects first, then throw. A local/param that + // shadows the name was already handled by the `lookup_local` arm + // above, so this only fires for the genuine class binding. + Ok(Expr::Sequence(vec![ + *value, + throw_type_error_const_assignment(&name), + ])) + } else if ctx.lookup_class(&name).is_some() || ctx.lookup_func(&name).is_some() { + // v0.5.757: don't shadow a class/function binding with an + // implicit local for ` = X` patterns. Drizzle's + // sql.js uses `((sql2) => { ... })(sql || (sql = {}))` + // (and the same for SQL) — since the binding exists + // (truthy), the OR short-circuits and the assignment is + // dead. Pre-fix the implicit local hid the original + // binding from later reads. Just evaluate the RHS for + // side effects. Refs #420. + Ok(*value) + } else { + if ctx.current_strict { + // #5989: strict-mode assignment to an existing global + // builtin is a property write, not a ReferenceError. See + // `strict_global_assign_existing_or_throw` for the full + // rationale. + return Ok(strict_global_assign_existing_or_throw(name, value)); + } + eprintln!( + " Warning: Assignment to undeclared variable '{}', creating sloppy global", + name + ); + // Sloppy implicit global: the binding IS a property of globalThis + // (spec CreateGlobalVarBinding on the global object), so `foo = 1` + // must be visible as `globalThis.foo`, write through to a + // pre-existing global property, and observe a later + // `delete globalThis.foo`. Reads of the name resolve through the + // `js_global_get_or_throw_unresolved` fallback, so no module-local + // shadow may be created here (a stale local would keep serving + // deleted/overwritten values). + // NOTE: `GlobalGet(0)` alone is a by-name routing SENTINEL in + // codegen (bare reads lower to 0.0) — the write must target + // the VALUE globalThis, which the `PropertyGet { GlobalGet(0), + // "globalThis" }` shape resolves to the real global object. + Ok(Expr::PropertySet { + object: Box::new(Expr::PropertyGet { + object: Box::new(Expr::GlobalGet(0)), + property: "globalThis".to_string(), + }), + property: name, + value, + }) + } +} + fn lower_assignment_target( ctx: &mut LoweringContext, target: &ast::AssignTarget, @@ -420,74 +511,7 @@ fn lower_assignment_target( ) -> Result { match target { ast::AssignTarget::Simple(ast::SimpleAssignTarget::Ident(ident)) => { - let name = ident.id.sym.to_string(); - if let Some(env_id) = ctx.active_with_envs_for_ident(&name).into_iter().next() { - let fallback = with_set_fallback_for_ident(ctx, &name); - return Ok(Expr::WithSet { - object: Box::new(Expr::LocalGet(env_id)), - property: name, - value, - fallback, - strict: ctx.current_strict, - }); - } - if let Some(id) = ctx.lookup_local(&name) { - if ctx.is_local_immutable(id) { - return Ok(Expr::Sequence(vec![ - *value, - throw_type_error_const_assignment(&name), - ])); - } - Ok(Expr::LocalSet(id, value)) - } else if ctx.current_class_inner_name.as_deref() == Some(name.as_str()) { - // Assigning to the class own-name binding from inside the class - // body targets the immutable inner `const` binding -> TypeError - // (test262 language/statements/class/name-binding/const). Evaluate - // the RHS for side effects first, then throw. A local/param that - // shadows the name was already handled by the `lookup_local` arm - // above, so this only fires for the genuine class binding. - Ok(Expr::Sequence(vec![ - *value, - throw_type_error_const_assignment(&name), - ])) - } else if ctx.lookup_class(&name).is_some() || ctx.lookup_func(&name).is_some() { - // v0.5.757: don't shadow a class/function binding with an - // implicit local for ` = X` patterns. Drizzle's - // sql.js uses `((sql2) => { ... })(sql || (sql = {}))` - // (and the same for SQL) — since the binding exists - // (truthy), the OR short-circuits and the assignment is - // dead. Pre-fix the implicit local hid the original - // binding from later reads. Just evaluate the RHS for - // side effects. Refs #420. - Ok(*value) - } else { - if ctx.current_strict { - // #5989: strict-mode assignment to an existing global - // builtin is a property write, not a ReferenceError. See - // `strict_global_assign_existing_or_throw` for the full - // rationale (shared with the sibling arm in - // lower_expr/assignment.rs). - return Ok(strict_global_assign_existing_or_throw(name, value)); - } - eprintln!( - " Warning: Assignment to undeclared variable '{}', creating sloppy global", - name - ); - // Sloppy implicit global — a real globalThis property, not - // a module local (see the sibling arm in lower_expr.rs). - // NOTE: `GlobalGet(0)` alone is a by-name routing SENTINEL in - // codegen (bare reads lower to 0.0) — the write must target - // the VALUE globalThis, which the `PropertyGet { GlobalGet(0), - // "globalThis" }` shape resolves to the real global object. - Ok(Expr::PropertySet { - object: Box::new(Expr::PropertyGet { - object: Box::new(Expr::GlobalGet(0)), - property: "globalThis".to_string(), - }), - property: name, - value, - }) - } + lower_ident_assignment(ctx, ident.id.sym.to_string(), value) } ast::AssignTarget::Simple(ast::SimpleAssignTarget::Member(member)) => { // Proxy set: `proxy.foo = v` / `proxy[k] = v` diff --git a/crates/perry-hir/src/lower/expr_misc.rs b/crates/perry-hir/src/lower/expr_misc.rs index 319b10c08..ccd9bcbea 100644 --- a/crates/perry-hir/src/lower/expr_misc.rs +++ b/crates/perry-hir/src/lower/expr_misc.rs @@ -26,6 +26,7 @@ use anyhow::{anyhow, Result}; use swc_ecma_ast as ast; use crate::ir::{BinaryOp, Expr, UpdateOp}; +use crate::lower::expr_assign::throw_type_error_const_assignment; use crate::lower_patterns::unescape_template; use super::{lower_expr, LoweringContext}; @@ -130,13 +131,19 @@ pub(super) fn lower_update(ctx: &mut LoweringContext, update: &ast::UpdateExpr) ast::UpdateOp::MinusMinus => BinaryOp::Sub, }; - // Unwrap compile-time-only wrappers: `obj.x!++` (TS non-null assertion) and - // `(obj.x)++` (parenthesized) are transparent to update-expression lowering. + // Unwrap compile-time-only wrappers: `obj.x!++` (TS non-null assertion), + // `(obj.x)++` (parenthesized) and the TS casts (`(x as any)++`, + // `(x)++`, `(x satisfies number)++`) are all transparent to + // update-expression lowering (#6300 — the cast forms previously fell + // through to the "only identifiers and member expressions" error). let mut arg = update.arg.as_ref(); loop { match arg { ast::Expr::TsNonNull(inner) => arg = inner.expr.as_ref(), ast::Expr::Paren(inner) => arg = inner.expr.as_ref(), + ast::Expr::TsAs(inner) => arg = inner.expr.as_ref(), + ast::Expr::TsTypeAssertion(inner) => arg = inner.expr.as_ref(), + ast::Expr::TsSatisfies(inner) => arg = inner.expr.as_ref(), _ => break, } } @@ -174,6 +181,36 @@ pub(super) fn lower_update(ctx: &mut LoweringContext, update: &ast::UpdateExpr) byte_offset: 0, }); }; + if ctx.is_local_immutable(id) { + // #6300: `const c = 1; c++` (and `(c as any)--`, `(c!)++`, …) is + // a PutValue on an immutable binding → + // `TypeError: Assignment to constant variable.` Emitting + // `Expr::Update` here silently mutated the `const` instead. + // + // The throw is NOT unconditional-first: per ES `++`/`--` + // (13.4.2.1 / 13.4.3.1) the operand is run through + // `ToNumeric(GetValue(lhs))` *before* `PutValue` rejects the + // immutable binding, and that coercion is itself observable — + // `const s = Symbol(); s++` throws "Cannot convert a Symbol + // value to a number", not the const TypeError (a BigInt, by + // contrast, passes ToNumeric cleanly and does get the const + // TypeError). Sequence the same `js_to_numeric` the normal + // update path uses in front of the throw so both orders match + // V8. + return Ok(Expr::Sequence(vec![ + Expr::Call { + callee: Box::new(Expr::ExternFuncRef { + name: "js_to_numeric".to_string(), + param_types: vec![perry_types::Type::Any], + return_type: perry_types::Type::Any, + }), + args: vec![Expr::LocalGet(id)], + type_args: vec![], + byte_offset: 0, + }, + throw_type_error_const_assignment(&name), + ])); + } let op = match update.op { ast::UpdateOp::PlusPlus => UpdateOp::Increment, ast::UpdateOp::MinusMinus => UpdateOp::Decrement, diff --git a/crates/perry-hir/src/lower/lower_expr/assignment.rs b/crates/perry-hir/src/lower/lower_expr/assignment.rs index f592c6a58..74f57b8c6 100644 --- a/crates/perry-hir/src/lower/lower_expr/assignment.rs +++ b/crates/perry-hir/src/lower/lower_expr/assignment.rs @@ -17,63 +17,12 @@ pub(crate) fn lower_expr_assignment( value: Box, ) -> Result { match expr { + // #6300: identifier stores — including the ones reached by unwrapping a + // parenthesized / TS-cast assignment target below — go through the one + // shared helper, so the `const`-immutability check can't be bypassed by + // writing `(c as any) = 9` instead of `c = 9`. ast::Expr::Ident(ident) => { - let name = ident.sym.to_string(); - if let Some(env_id) = ctx.active_with_envs_for_ident(&name).into_iter().next() { - let fallback = with_set_fallback_for_ident(ctx, &name); - return Ok(Expr::WithSet { - object: Box::new(Expr::LocalGet(env_id)), - property: name, - value, - fallback, - strict: ctx.current_strict, - }); - } - if let Some(id) = ctx.lookup_local(&name) { - Ok(Expr::LocalSet(id, value)) - } else if ctx.lookup_class(&name).is_some() || ctx.lookup_func(&name).is_some() { - // v0.5.757: don't shadow a class/function binding with an - // implicit local for ` = X` patterns. Drizzle's - // sql.js uses `((sql2) => { ... })(sql || (sql = {}))` — - // the binding exists (truthy), the OR short-circuits, and - // the assignment is dead. Pre-fix the implicit local hid - // the original binding from later reads. Just evaluate - // the RHS for side effects. Refs #420. - Ok(*value) - } else { - if ctx.current_strict { - // #5989: strict-mode assignment to an existing global - // builtin is a property write, not a ReferenceError. See - // `strict_global_assign_existing_or_throw` for the full - // rationale (shared with the sibling arm in expr_assign.rs). - return Ok(strict_global_assign_existing_or_throw(name, value)); - } - eprintln!( - " Warning: Assignment to undeclared variable '{}', creating sloppy global", - name - ); - // Sloppy implicit global: the binding IS a property of - // globalThis (spec CreateGlobalVarBinding on the global - // object), so `foo = 1` must be visible as - // `globalThis.foo`, write through to a pre-existing global - // property, and observe a later `delete globalThis.foo`. - // Reads of the name resolve through the - // `js_global_get_or_throw_unresolved` fallback, so no - // module-local shadow may be created here (a stale local - // would keep serving deleted/overwritten values). - // NOTE: `GlobalGet(0)` alone is a by-name routing SENTINEL in - // codegen (bare reads lower to 0.0) — the write must target - // the VALUE globalThis, which the `PropertyGet { GlobalGet(0), - // "globalThis" }` shape resolves to the real global object. - Ok(Expr::PropertySet { - object: Box::new(Expr::PropertyGet { - object: Box::new(Expr::GlobalGet(0)), - property: "globalThis".to_string(), - }), - property: name, - value, - }) - } + crate::lower::expr_assign::lower_ident_assignment(ctx, ident.sym.to_string(), value) } ast::Expr::Member(member) => { if let ast::Expr::Ident(obj_ident) = member.obj.as_ref() { diff --git a/test-files/test_gap_6300_const_cast_assign.ts b/test-files/test_gap_6300_const_cast_assign.ts new file mode 100644 index 000000000..427c3b125 --- /dev/null +++ b/test-files/test_gap_6300_const_cast_assign.ts @@ -0,0 +1,197 @@ +// #6300: a parenthesized / TS-cast assignment target must NOT bypass the +// `const` immutability check. `(c as any) = 9` used to silently mutate the +// binding with no diagnostic — only the bare-identifier store path checked +// immutability, and every wrapper spelling (parens, `as`, `satisfies`, `!`, +// ``) routed around it into a second, unchecked identifier-store arm. +// +// Perry appends the binding name to the message (`... variable 'c'.`) where +// V8 does not, so normalize that away to stay byte-for-byte identical to +// `node --experimental-strip-types`. +function norm(e: unknown): string { + const err = e as Error; + const isTypeError = err instanceof TypeError; + const msg = err.message.replace(/ '[^']*'\.$/, "."); + return `${isTypeError ? "TypeError" : "Error"}: ${msg}`; +} + +function expectThrow(label: string, fn: () => void): void { + try { + fn(); + console.log(`${label}: NO THROW`); + } catch (e) { + console.log(`${label}: ${norm(e)}`); + } +} + +// --- const targets: every wrapper must throw and leave the binding intact --- + +const c1 = 1; +expectThrow("bare", () => { + // @ts-expect-error assignment to const + c1 = 9; +}); +console.log("c1 =", c1); + +const c2 = 2; +expectThrow("paren", () => { + // @ts-expect-error assignment to const + (c2) = 9; +}); +console.log("c2 =", c2); + +const c3 = 3; +expectThrow("as-cast", () => { + (c3 as any) = 9; +}); +console.log("c3 =", c3); + +const c4 = 4; +expectThrow("satisfies", () => { + (c4 satisfies number) = 9; +}); +console.log("c4 =", c4); + +const c5 = 5; +expectThrow("non-null", () => { + // @ts-expect-error assignment to const + (c5!) = 9; +}); +console.log("c5 =", c5); + +const c6 = 6; +expectThrow("nested-wrappers", () => { + ((c6 as any)!) = 9; +}); +console.log("c6 =", c6); + +// --- compound / update / logical assignment through a cast --- + +const c7 = 7; +expectThrow("compound-add", () => { + (c7 as any) += 1; +}); +console.log("c7 =", c7); + +const c8 = 8; +expectThrow("compound-shift", () => { + (c8 as any) <<= 1; +}); +console.log("c8 =", c8); + +const c9 = 9; +expectThrow("postfix-incr", () => { + (c9 as any)++; +}); +console.log("c9 =", c9); + +const c10 = 10; +expectThrow("prefix-decr", () => { + --(c10 as any); +}); +console.log("c10 =", c10); + +const c11 = 11; +expectThrow("bare-incr", () => { + // @ts-expect-error update of const + c11++; +}); +console.log("c11 =", c11); + +// A BigInt const passes ToNumeric cleanly, so `++` still reaches PutValue and +// reports the const TypeError. +const c12 = 12n; +expectThrow("bigint-incr", () => { + (c12 as any)++; +}); +console.log("c12 =", c12); + +const c13: number | null = null; +expectThrow("nullish-assign", () => { + (c13 as any) ??= 13; +}); +console.log("c13 =", c13); + +const c14 = 0; +expectThrow("or-assign", () => { + (c14 as any) ||= 14; +}); +console.log("c14 =", c14); + +// The RHS is evaluated before PutValue rejects the binding, so its side +// effects are observable even though the assignment throws. +const c15 = 15; +let rhsRuns = 0; +expectThrow("rhs-side-effect", () => { + (c15 as any) = (rhsRuns++, 99); +}); +console.log("c15 =", c15, "rhsRuns =", rhsRuns); + +// A short-circuiting logical assignment never reaches PutValue, so a truthy +// const with `||=` must NOT throw. +const c16 = 16; +expectThrow("or-assign-shortcircuit", () => { + (c16 as any) ||= 99; +}); +console.log("c16 =", c16); + +// A const inside a function body behaves the same. +function inFunction(): void { + const local = 1; + expectThrow("fn-local", () => { + (local as any) = 9; + }); + console.log("fn-local =", local); +} +inFunction(); + +// `for (const x of ...)` binds a fresh immutable binding per iteration. +for (const item of [1]) { + expectThrow("for-of-const", () => { + (item as any) = 9; + }); + console.log("item =", item); +} + +// --- legitimate writes must keep working --- + +let d1 = 1; +(d1 as any) = 9; +console.log("let as-cast:", d1); + +let d2 = 1; +(d2) = 9; +console.log("let paren:", d2); + +let d3 = 1; +(d3 as any) += 4; +console.log("let compound:", d3); + +let d4 = 1; +(d4 as any)++; +++(d4 as any); +console.log("let update:", d4); + +let d5: number | null = null; +(d5 as any) ??= 7; +console.log("let nullish:", d5); + +let d6 = 1; +(d6 satisfies number) = 6; +(d6!) = d6 + 1; +console.log("let satisfies/non-null:", d6); + +// A cast MEMBER expression target is a property write, not a binding write — +// it stays legal even when the object itself is `const`. +const obj: any = { x: 0 }; +(obj as any).x = 1; +console.log("obj.x =", obj.x); +(obj as any).x++; +console.log("obj.x =", obj.x); +(obj as any)["x"] += 10; +console.log("obj.x =", obj.x); + +// Mutating a `const` object/array is fine — only rebinding is not. +const arr: number[] = [1]; +arr.push(2); +(arr as any)[0] = 5; +console.log("arr =", arr.join(","));