diff --git a/changelog.d/10258-imported-default-ctor-arity.md b/changelog.d/10258-imported-default-ctor-arity.md new file mode 100644 index 0000000000..1cc6dba602 --- /dev/null +++ b/changelog.d/10258-imported-default-ctor-arity.md @@ -0,0 +1 @@ +Fix `new ImportedClass(args)` for an exported class with no own constructor whose `extends` clause is a runtime value (for example effect v4's `class SystemError extends Data.Error {}`). The defining module synthesizes a forwarding constructor with a fixed parameter band because the parent's arity is unknown there, but the imported-class metadata counted only the class's own (absent) constructor and declared it with zero parameters, so the importing module called it with `this` alone and the parent constructor never saw the arguments. Importers now derive the arity from the same rule as the synthesized constructor. This unblocks OpenCode's config loading, where effect's file-not-found `PlatformError` lost its reason and turned a missing optional config file into a fatal error (#10107). diff --git a/crates/perry-codegen/src/codegen/ctor_arity.rs b/crates/perry-codegen/src/codegen/ctor_arity.rs index efc3d68469..73917e63e2 100644 --- a/crates/perry-codegen/src/codegen/ctor_arity.rs +++ b/crates/perry-codegen/src/codegen/ctor_arity.rs @@ -6,6 +6,33 @@ use std::collections::HashMap; +/// Positional forwarding band for a synthesized default ctor whose parent arity +/// cannot be resolved while compiling the defining module (see the tail of +/// [`synthesized_ctor_param_count`]). +pub const UNRESOLVED_PARENT_FWD_ARITY: usize = 8; + +/// The standalone-constructor arity of `class` when it can be decided from the +/// class definition alone, without the defining module's class table or +/// imports. Importers use this to describe a class they did not compile, so it +/// MUST agree with [`synthesized_ctor_param_count`] for every case it answers: +/// an own constructor, a native parent, no heritage, and a heritage that is only +/// a runtime value (`extends_expr` with no resolvable `extends_name`), which +/// always synthesizes the fixed forwarding band. `None` means the arity depends +/// on the ancestor walk and callers keep their existing behaviour (#10258). +pub fn context_free_ctor_param_count(class: &perry_hir::Class) -> Option { + if let Some(c) = class.constructor.as_ref() { + return Some(c.params.len()); + } + if class.native_extends.is_some() { + return Some(0); + } + match (&class.extends_name, &class.extends_expr) { + (None, None) => Some(0), + (None, Some(_)) => Some(UNRESOLVED_PARENT_FWD_ARITY), + _ => None, + } +} + /// The standalone-constructor arity Perry emits for `class`, accounting for the /// JS spec default ctor `constructor(...args) { super(...args) }` that a class /// with NO own constructor but WITH heritage inherits. Walks the ancestor chain @@ -78,6 +105,5 @@ pub(super) fn synthesized_ctor_param_count( // positional params: the `new` site pads missing slots with `undefined`, // and a parent ctor reading fewer params ignores the trailing `undefined`s, // so over-declaring is correct for any (non-native) parent up to this band. - const UNRESOLVED_PARENT_FWD_ARITY: usize = 8; UNRESOLVED_PARENT_FWD_ARITY } diff --git a/crates/perry-codegen/src/codegen/mod.rs b/crates/perry-codegen/src/codegen/mod.rs index df28e079e1..8b09ca39a5 100644 --- a/crates/perry-codegen/src/codegen/mod.rs +++ b/crates/perry-codegen/src/codegen/mod.rs @@ -188,6 +188,7 @@ mod clone_suffix_tests; mod closure; mod closure_collect; mod ctor_arity; +pub use ctor_arity::{context_free_ctor_param_count, UNRESOLVED_PARENT_FWD_ARITY}; #[cfg(test)] mod declared_string_add_tests; #[cfg(test)] diff --git a/crates/perry-codegen/src/lib.rs b/crates/perry-codegen/src/lib.rs index 9eb72e7496..dd085e8618 100644 --- a/crates/perry-codegen/src/lib.rs +++ b/crates/perry-codegen/src/lib.rs @@ -75,7 +75,7 @@ pub mod types; pub mod unit_cache; pub use codegen::{ - compile_module, namespace_member_class_key, namespace_member_func_key, + compile_module, context_free_ctor_param_count, namespace_member_class_key, namespace_member_func_key, namespace_member_var_key, resolve_target_triple, short_spread_method_capabilities, user_function_symbol, AppMetadata, CompileOptions, ExportedObjectLiteralCapability, FpContractMode, ImportedClass, ImportedObjectLiteral, ImportedObjectLiteralMethod, diff --git a/crates/perry/src/commands/compile/run_pipeline.rs b/crates/perry/src/commands/compile/run_pipeline.rs index a71acbbcd2..1016b348c3 100644 --- a/crates/perry/src/commands/compile/run_pipeline.rs +++ b/crates/perry/src/commands/compile/run_pipeline.rs @@ -293,11 +293,20 @@ fn imported_class_from_hir( local_alias, namespace: None, source_prefix, - constructor_param_count: class - .constructor - .as_ref() - .map(|ctor| ctor.params.len()) - .unwrap_or(0), + // #10258: must match the arity of the standalone constructor the + // defining module emits. A class with no own constructor whose parent + // is only a runtime value synthesizes a fixed forwarding band; counting + // just the (absent) own constructor declared it as 0 params here, so + // `new ImportedClass(args)` passed only `this` and the parent ran + // without its arguments. + constructor_param_count: perry_codegen::context_free_ctor_param_count(class) + .unwrap_or_else(|| { + class + .constructor + .as_ref() + .map(|ctor| ctor.params.len()) + .unwrap_or(0) + }), has_own_constructor: class.constructor.is_some(), constructor_has_rest: class .constructor diff --git a/crates/perry/tests/source_graph_export_regressions.rs b/crates/perry/tests/source_graph_export_regressions.rs index ea4df499cb..4797a6c6a9 100644 --- a/crates/perry/tests/source_graph_export_regressions.rs +++ b/crates/perry/tests/source_graph_export_regressions.rs @@ -963,3 +963,5 @@ mod issue_10160; mod issue_10180; #[path = "source_graph_export_regressions/issue_10197.rs"] mod issue_10197; +#[path = "source_graph_export_regressions/issue_10258.rs"] +mod issue_10258; diff --git a/crates/perry/tests/source_graph_export_regressions/issue_10258.rs b/crates/perry/tests/source_graph_export_regressions/issue_10258.rs new file mode 100644 index 0000000000..e2c790d879 --- /dev/null +++ b/crates/perry/tests/source_graph_export_regressions/issue_10258.rs @@ -0,0 +1,55 @@ +//! `new ImportedClass(args)` for an exported class with no own constructor whose +//! parent is a runtime value must forward the arguments (#10258). The defining +//! module synthesizes a fixed-arity forwarding constructor; the importer used to +//! declare it with zero parameters and dropped every argument. effect v4's +//! `PlatformError.SystemError extends Data.Error {}` has this shape. + +use super::{compile_and_run, write}; + +#[test] +fn imported_default_ctor_with_runtime_parent_forwards_args() { + let dir = tempfile::tempdir().unwrap(); + write( + dir.path(), + "core.ts", + "export const YieldableError = (function () { class YieldableError extends globalThis.Error {}; return YieldableError })()\n\ + export const Error = (function () {\n\ + \x20 return class Base extends YieldableError {\n\ + \x20 constructor(args?: any) { super(args?.message); if (args) Object.assign(this, args) }\n\ + \x20 }\n\ + })()\n\ + export const Plain = (function () { return class P { constructor(a?: any, b?: any, c?: any) { (this as any).sum = [a, b, c] } } })()\n", + ); + write(dir.path(), "data.ts", "import * as core from \"./core\"\nexport const Error = core.Error\nexport const Plain = core.Plain\n"); + write( + dir.path(), + "platform.ts", + "import * as Data from \"./data\"\n\ + export class Empty extends (Data.Error as any) {}\n\ + export class WithGetter extends (Data.Error as any) { get message() { return \"G:\" + (this as any)._tag } }\n\ + export class WithMethod extends (Data.Error as any) { describe() { return (this as any).module } }\n\ + export class Explicit extends (Data.Error as any) { constructor(a: any) { super(a) } }\n\ + export class ThreeArgs extends (Data.Plain as any) {}\n\ + export const makeLocal = (o: any) => new WithGetter(o)\n", + ); + write( + dir.path(), + "main.ts", + "import { Empty, WithGetter, WithMethod, Explicit, ThreeArgs, makeLocal } from \"./platform\"\n\ + import * as P from \"./platform\"\n\ + const o = () => ({ _tag: \"NotFound\", module: \"FS\" })\n\ + const show = (e: any) => [e._tag, e.module, e instanceof Error].join(\",\")\n\ + console.log(show(new Empty(o())), show(new WithGetter(o())), new WithGetter(o()).message)\n\ + console.log(show(new WithMethod(o())), new WithMethod(o()).describe(), show(new Explicit(o())))\n\ + console.log(show(new P.Empty(o())), show(makeLocal(o())), JSON.stringify((new ThreeArgs(1, 2, 3) as any).sum))\n\ + class Sub extends Empty {}\n\ + console.log(show(new Sub(o())))\n", + ); + assert_eq!( + compile_and_run(dir.path(), "main.ts"), + "NotFound,FS,true NotFound,FS,true G:NotFound\n\ + NotFound,FS,true FS NotFound,FS,true\n\ + NotFound,FS,true NotFound,FS,true [1,2,3]\n\ + NotFound,FS,true\n" + ); +}