diff --git a/TODO.md b/TODO.md index 253ac49..1353a82 100644 --- a/TODO.md +++ b/TODO.md @@ -40,7 +40,7 @@ - [x] **IAsyncOperationWithProgress IID computation**: Enum-in-struct now emits `enum(Namespace.Name;i4)` in both runtime IID signature (`metadata_table/iid.rs:77-80`) and codegen (`ts_dynwinrt_type` / `py_dynwinrt_type` recurse into struct fields and emit `enumType('FullName', [names], [values])`). Parameterized IID now matches QI for async-of-struct-with-enum. - ~~Also: `StructEntry.name` uses `Option` but WinRT structs are always named — should be `String`, deprecate `define_struct` in favor of `define_named_struct`~~ (done — `StructEntry.name` is now `String`) - [ ] **Nullable / IReference\ return handling**: Null COM pointer returns `Null` variant; JS side needs better null-check patterns -- [ ] **Composable class derived-constructor `.ctor` not implemented**: Unsealed WinRT runtime classes expose a derived-from constructor on their default instance interface, whose CLR method name is literally `.ctor`. Semantically it follows the COM aggregation pattern (`IInspectable* baseInterface, IInspectable** innerInterface`), meant to be invoked by a host framework performing subclassing — in practice this is almost exclusively XAML (WinUI controls, event args, framework base classes). Current codegen handles the other two `.ctor` flavors correctly: factory-interface `.ctor(args)` generates a class constructor, and delegate `.ctor + Invoke` is recognized as a delegate. But composable `.ctor` appearing alone on an instance interface falls through to the generic instance-method path and emits invalid syntax: `def .ctor(self) -> None:` in Python and `.ctor(): void { … }` in TypeScript. Full-WinAppSDK smoke test (PR #18) reported 62 affected files, all in `Microsoft.UI.Xaml.*`; non-XAML WinAppSDK namespaces are unaffected. Minimal fix: in `codegen/method.rs` + TS/Py generators, skip `method.name == ".ctor"` on instance interfaces (the `add_method(".ctor", …)` vtable entry can stay for IID computation). Full fix requires XAML support, which entails implementing the COM aggregation subclassing pattern — out of scope until we take on XAML hosting. +- [x] **Composable class derived-constructor `.ctor` skip**: Unsealed WinRT runtime classes expose a derived-from constructor on their default instance interface whose CLR method name is literally `.ctor`. Semantically it follows the COM aggregation pattern (`IInspectable* baseInterface, IInspectable** innerInterface`), meant to be invoked by a host framework performing subclassing — in practice this is almost exclusively XAML. Codegen used to fall through to the generic instance-method path and emit invalid syntax: `def .ctor(self) -> None:` in Python and `.ctor(): void { … }` in TypeScript. **Now suppressed at code emission**: `project_instance_method` (TS), `generate_iface_instance_method` (Py), and `emit_method_stub` (Py stub) all early-return for `method.name == ".ctor"`. The `.add_method(".ctor", …)` vtable entry is **still recorded** in interface registration so IIDs and indices remain correct — pinned by `tests/composable_ctor_test.rs::interface_registration_still_records_ctor_vtable_slot`. Full WinAppSDK XAML namespace (Microsoft.UI.Xaml.Controls.Button + transitive deps, 383 files) now compiles cleanly via `python -m py_compile` / `tsc --noEmit`. Full fix for actually invoking the composable constructor requires implementing the COM aggregation subclassing pattern — still out of scope until we take on XAML hosting. - [x] **Struct codegen deduplication**: `generate_struct_helpers()` now generates shared TS interface + pack/unpack functions once per struct, reused across methods - [x] **Exclusive interface codegen**: `all_interfaces()` resolves default + required interfaces; codegen generates wrapper classes for all interfaces a class implements - [x] **Codegen missing dependency warning**: `resolve_named_type` now emits warnings when types are not found in loaded .winmd files, plus `assert!(!iid.is_empty(), ...)` catches empty GUIDs at generation time diff --git a/tools/dynwinrt-codegen/src/codegen/project.rs b/tools/dynwinrt-codegen/src/codegen/project.rs index 3f9ec46..d4a354b 100644 --- a/tools/dynwinrt-codegen/src/codegen/project.rs +++ b/tools/dynwinrt-codegen/src/codegen/project.rs @@ -986,6 +986,17 @@ fn project_instance_method( delegate_sigs: &HashMap, delegate_param_wraps: &HashMap>, ) -> Option { + // Skip composable `.ctor` on instance interfaces: WinRT marks the derived-from + // constructor of unsealed runtime classes with the literal CLR method name `.ctor` + // on the default/required interface. It follows the COM aggregation pattern, is + // only meant to be invoked by a host framework (in practice XAML), and emitting it + // here would produce invalid syntax (`.ctor(): void { ... }`). The `.ctor` entry is + // still kept in the interface registration / vtable so IIDs and indices stay correct. + // Factory `.ctor(args)` (legitimate class construction) goes through + // `project_factory_method`, and delegate `.ctor` is short-circuited by `project_delegate`. + if method.name == ".ctor" { + return None; + } let in_params = get_in_params(method); let return_type_meta = method.return_type.as_ref(); let is_with_progress = return_type_meta.is_some_and(|rt| matches!(rt, diff --git a/tools/dynwinrt-codegen/src/codegen/py_method.rs b/tools/dynwinrt-codegen/src/codegen/py_method.rs index 2db0a5a..a2fc9de 100644 --- a/tools/dynwinrt-codegen/src/codegen/py_method.rs +++ b/tools/dynwinrt-codegen/src/codegen/py_method.rs @@ -277,6 +277,11 @@ pub(crate) fn generate_iface_instance_method( known_types: &HashSet, delegate_type_names: &HashSet, ) -> String { + // Skip composable `.ctor` on instance interfaces (see project.rs for full rationale). + // Emitting it would produce `def .ctor(self) -> None:` which is a syntax error. + if method.name == ".ctor" { + return String::new(); + } generate_method_body(iface_var, "self._obj", method, known_types, delegate_type_names, None) } diff --git a/tools/dynwinrt-codegen/src/codegen/python_stub.rs b/tools/dynwinrt-codegen/src/codegen/python_stub.rs index be66c70..8369656 100644 --- a/tools/dynwinrt-codegen/src/codegen/python_stub.rs +++ b/tools/dynwinrt-codegen/src/codegen/python_stub.rs @@ -104,6 +104,10 @@ fn emit_method_stub( delegate_type_names: &HashSet, indent_spaces: usize, ) -> String { + // Skip composable `.ctor` on instance interfaces (see project.rs for full rationale). + if method.name == ".ctor" { + return String::new(); + } let indent = " ".repeat(indent_spaces); let in_params = get_in_params(method); let return_type = method.return_type.as_ref(); diff --git a/tools/dynwinrt-codegen/tests/composable_ctor_test.rs b/tools/dynwinrt-codegen/tests/composable_ctor_test.rs new file mode 100644 index 0000000..92f5697 --- /dev/null +++ b/tools/dynwinrt-codegen/tests/composable_ctor_test.rs @@ -0,0 +1,181 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Regression test for the composable runtime-class constructor. +//! +//! Unsealed WinRT runtime classes expose a derived-from constructor on the +//! default/required instance interface, whose CLR method name is literally +//! `.ctor`. It implements the COM aggregation pattern and is only meant to be +//! invoked by a host framework (in practice, XAML). Codegen used to fall +//! through to the regular instance-method path for these methods, emitting +//! `.ctor(): void { ... }` (TS) and `def .ctor(self) -> None:` (Python) — both +//! syntax errors. This test pins the fix: `.ctor` on a non-delegate interface +//! must be skipped at code emission, while still being kept in the interface +//! registration / vtable so IIDs and indices remain correct. + +use std::collections::{HashMap, HashSet}; + +use dynwinrt_codegen::codegen::{project, python, python_stub, render_dts, render_js}; +use dynwinrt_codegen::meta::{InterfaceMeta, MethodMeta, ParamDirection, ParamMeta}; +use dynwinrt_codegen::types::TypeMeta; + +/// Build a synthetic instance interface containing a composable `.ctor` plus +/// one ordinary property-getter so we can verify the test interface was +/// actually projected (i.e. we're not just generating an empty class). +fn make_composable_iface() -> InterfaceMeta { + InterfaceMeta { + name: "ITestInstance".into(), + namespace: "Test.Sample".into(), + iid: "11111111-2222-3333-4444-555555555555".into(), + methods: vec![ + // Ordinary property — must appear in output. + MethodMeta { + name: "get_Name".into(), + vtable_index: 6, + params: vec![], + return_type: Some(TypeMeta::String), + is_property_getter: true, + ..Default::default() + }, + // Composable `.ctor(IInspectable* base, IInspectable** inner)` — + // must NOT appear in output. + MethodMeta { + name: ".ctor".into(), + vtable_index: 7, + params: vec![ + ParamMeta { + name: "baseInterface".into(), + typ: TypeMeta::Object, + direction: ParamDirection::In, + }, + ParamMeta { + name: "innerInterface".into(), + typ: TypeMeta::Object, + direction: ParamDirection::Out, + }, + ], + return_type: None, + ..Default::default() + }, + ], + ..Default::default() + } +} + +fn assert_no_ctor(label: &str, output: &str) { + // The legitimate appearance of `.ctor` is inside the interface-registration + // string literal (`.addMethod(".ctor", ...)` / `.add_method(".ctor", ...)`). + // We must NOT see it anywhere else — particularly not as a method + // declaration, which would be a syntax error in both TS and Python. + let bad_patterns = [ + ".ctor(", // TS method/declaration syntax: `.ctor(): void { ... }` + ".ctor:", // DTS field syntax: `.ctor: () => void` + "def .ctor", // Python method definition + "get .ctor", // TS getter + "set .ctor", // TS setter + "@.ctor", // TS decorator (defensive) + ]; + for pat in bad_patterns { + assert!( + !output.contains(pat), + "{label}: emitted output contains forbidden pattern `{pat}`:\n{output}", + ); + } + // Sanity: the test interface must have been projected — otherwise an + // empty/skipped output would trivially pass. + assert!( + output.contains("ITestInstance"), + "{label}: emitted output is missing the interface name `ITestInstance`:\n{output}", + ); +} + +#[test] +fn typescript_codegen_skips_composable_ctor() { + let iface = make_composable_iface(); + let known_types: HashSet = std::iter::once("ITestInstance".to_string()).collect(); + let delegate_type_names: HashSet = HashSet::new(); + let delegate_sigs: HashMap = HashMap::new(); + let delegate_sig_refs: HashMap> = HashMap::new(); + let delegate_param_wraps: HashMap> = HashMap::new(); + + let projected = project::project_interface( + &iface, + &known_types, + &delegate_type_names, + &delegate_sigs, + &delegate_sig_refs, + &delegate_param_wraps, + ); + + // The interface must NOT be classified as a delegate (no Invoke method), + // otherwise this whole test scenario wouldn't apply. + assert!( + projected.ifaces.iter().all(|i| !i.is_delegate), + "synthetic interface should not be classified as a delegate", + ); + + let js = render_js::render(&projected); + let dts = render_dts::render(&projected); + + assert_no_ctor("render_js", &js); + assert_no_ctor("render_dts", &dts); + + // The legitimate property must still be there — so we know the interface + // was actually emitted and not skipped wholesale. + assert!(js.contains("get name()"), "render_js missing `name` getter:\n{js}"); + assert!(dts.contains("name"), "render_dts missing `name` declaration:\n{dts}"); +} + +#[test] +fn python_codegen_skips_composable_ctor() { + let iface = make_composable_iface(); + let known_types: HashSet = std::iter::once("ITestInstance".to_string()).collect(); + let delegate_type_names: HashSet = HashSet::new(); + + let py = python::generate_interface(&iface, &known_types, &delegate_type_names); + assert_no_ctor("python::generate_interface", &py); + + let pyi = python_stub::generate_interface_stub(&iface, &known_types, &delegate_type_names); + assert_no_ctor("python_stub::generate_interface_stub", &pyi); + + // Snake-case `name` property must still be emitted. + assert!(py.contains("def name"), "python output missing `name` property:\n{py}"); + assert!(pyi.contains("def name"), "pyi output missing `name` property:\n{pyi}"); +} + +#[test] +fn interface_registration_still_records_ctor_vtable_slot() { + // Even though `.ctor` is suppressed in the emitted public surface, the + // interface-registration / vtable description must still account for it + // so IID computation and vtable indices stay correct for downstream + // method calls. We assert this by checking that the registration block + // emitted by render_js mentions the vtable slot ABI for `.ctor`'s + // parameters (two `Object`-shaped slots) — i.e. methods after the `.ctor` + // would otherwise have wrong vtable indices. + let iface = make_composable_iface(); + let known_types: HashSet = std::iter::once("ITestInstance".to_string()).collect(); + let projected = project::project_interface( + &iface, + &known_types, + &HashSet::new(), + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + ); + let js = render_js::render(&projected); + + // The `.ctor` registration entry must reserve a vtable slot. Implementations + // typically express this via `.addMethod(...)` calls; the count must equal + // the number of methods in the interface (including `.ctor`), so that + // subsequent vtable indices line up. + let add_method_count = js.matches(".addMethod").count(); + assert_eq!( + add_method_count, + iface.methods.len(), + "registration must reserve a vtable slot for every method (including `.ctor`); \ + got {} addMethod calls for {} methods:\n{}", + add_method_count, + iface.methods.len(), + js, + ); +}