From 2052a5eac18e81fbbaa442cefa4a86670e687723 Mon Sep 17 00:00:00 2001 From: "Leilei Zhang (from Dev Box)" Date: Tue, 11 Aug 2026 18:24:36 +0800 Subject: [PATCH 1/8] Share generated interface members Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 84688dea-d965-4933-a2d9-41f8f6ad2f2e --- README.md | 28 + tools/dynwinrt-codegen/npm/README.md | 18 + .../src/codegen/winrt/javascript/ir.rs | 13 + .../codegen/winrt/javascript/project/mod.rs | 210 +++++++- .../winrt/javascript/render/javascript/mod.rs | 63 ++- tools/dynwinrt-codegen/src/main.rs | 17 +- .../tests/shared_interface_members_test.rs | 479 ++++++++++++++++++ 7 files changed, 822 insertions(+), 6 deletions(-) create mode 100644 tools/dynwinrt-codegen/tests/shared_interface_members_test.rs diff --git a/README.md b/README.md index a37b702c..c673260d 100644 --- a/README.md +++ b/README.md @@ -171,10 +171,38 @@ For deployment, see | `--lang LANG` | No | `js` (default, emits `.js` + `.d.ts`) or `py` (emits `.py` + `.pyi` and `py.typed`) | | `--no-pyi` | No | With `--lang py`, emit implementation files without type stubs | | `--output DIR` | No | Output directory (default `./generated`) | +| `--shared-interface-members` | No | JS opt-in: reuse shared required-interface prototype descriptors instead of duplicating inherited member bodies in every concrete class | | `--dry-run` | No | Validate input, don't write files | For each WinRT class the codegen emits a typed wrapper, factory, interface registration, async + progress support, generic collections, structs, enums, delegates, and an `index.js` / `index.d.ts` that re-exports every emitted symbol. +### Shared interface members + +Large JavaScript projections can opt into shared inherited-interface +implementations without changing concrete class declarations or member names: + +```powershell +dynwinrt-codegen generate ` + --winmd-list .winapp\winmds.txt ` + --class-name Microsoft.UI.Xaml.Controls.Button,Microsoft.UI.Xaml.Controls.TextBlock ` + --output .winapp\bindings ` + --shared-interface-members +``` + +The generated concrete prototypes receive the same method and accessor +descriptors from standalone shared interface prototypes. Overloaded or +conflicting members remain class-local, raw interface wrapper classes remain +available, and the option does not change generated `.d.ts` files. Only +required interfaces already canonicalized as standalone shared wrappers +participate; one-off inline required interfaces remain class-local. Generation +without this flag is unchanged. + +Focused validation: + +```powershell +cargo test -p dynwinrt-codegen --test shared_interface_members_test +``` + ## Local development — fix import paths in generated files Generated files import from `'@microsoft/dynwinrt'`. When iterating against a locally-built runtime, rewrite imports to the relative path: diff --git a/tools/dynwinrt-codegen/npm/README.md b/tools/dynwinrt-codegen/npm/README.md index d4e74b3a..61895631 100644 --- a/tools/dynwinrt-codegen/npm/README.md +++ b/tools/dynwinrt-codegen/npm/README.md @@ -56,8 +56,26 @@ npx dynwinrt-codegen generate \ | `--ref PATH` | Additional `.winmd` files for type resolution only (no code emitted) | | `--lang LANG` | `js` (default, emits `.js` + `.d.ts`) or `py` (Python) | | `--output DIR` | Output directory (default `./generated`) | +| `--shared-interface-members` | JS opt-in that shares inherited interface member descriptors across concrete classes | | `--dry-run` | Validate input, don't write files | +### Shared interface members + +```powershell +npx dynwinrt-codegen generate ` + --winmd-list .winapp\winmds.txt ` + --class-name Microsoft.UI.Xaml.Controls.Button,Microsoft.UI.Xaml.Controls.TextBlock ` + --output .winapp\bindings ` + --shared-interface-members +``` + +The generated concrete prototypes reuse method and accessor descriptors from +the standalone required-interface prototypes. Overloaded or conflicting +members remain class-local, raw interface wrappers remain available, generated +`.d.ts` files are unchanged, and generation without the flag is unchanged. +Only interfaces already emitted as canonical standalone shared wrappers +participate; one-off inline required interfaces remain class-local. + ## What gets generated For each WinRT class, the codegen emits: diff --git a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/ir.rs b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/ir.rs index 919a0ce5..ddb8a48a 100644 --- a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/ir.rs +++ b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/ir.rs @@ -257,18 +257,31 @@ pub struct ProjectedClass { pub doc: Option, pub members: Vec, pub required_ifaces: Vec, + /// Required-interface members whose implementation descriptors are copied + /// from a shared standalone interface prototype. + pub shared_interface_members: Vec, /// Static factory/static interface cache field declarations (JS only) pub static_cache_fields: Vec, /// Static factory/static interface accessor methods (JS only) pub static_accessors: Vec, } +pub struct ProjectedSharedInterfaceMembers { + pub interface_name: String, + /// Projection-level member keys used to suppress duplicate class bodies. + pub member_keys: Vec, + /// JavaScript property-key expressions copied from the interface prototype. + pub descriptor_keys: Vec, +} + pub struct ProjectedIface { pub name: String, pub doc: Option, pub iid_const: Option, pub has_static_from: bool, pub has_parameterized_cast: bool, + /// The interface prototype may be reused by concrete runtime classes. + pub shared_member_source: bool, pub members: Vec, pub is_delegate: bool, } diff --git a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/project/mod.rs b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/project/mod.rs index edce23f4..ab0be29e 100644 --- a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/project/mod.rs +++ b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/project/mod.rs @@ -23,6 +23,7 @@ use crate::types::{TypeKind, TypeMeta}; thread_local! { static RUNTIME_IMPORT_NAME: RefCell = RefCell::new("@microsoft/dynwinrt".into()); + static SHARED_INTERFACE_MEMBERS: RefCell = const { RefCell::new(false) }; } /// Set the runtime package import name used in generated JS/TS files. @@ -35,6 +36,17 @@ pub fn get_import_name() -> String { RUNTIME_IMPORT_NAME.with(|n| n.borrow().clone()) } +/// Opt into reusing standalone interface prototype descriptors from concrete +/// runtime classes instead of rendering inherited member bodies repeatedly. +pub fn set_shared_interface_members(enabled: bool) { + SHARED_INTERFACE_MEMBERS.with(|value| *value.borrow_mut() = enabled); +} + +/// Return whether concrete-class shared interface member projection is enabled. +pub fn shared_interface_members_enabled() -> bool { + SHARED_INTERFACE_MEMBERS.with(|value| *value.borrow()) +} + use crate::codegen::winrt::shared::imports::{ NO_DEFERRED, collect_iface_type_imports, collect_type_imports, collect_used_generics_from_class, collect_used_generics_from_methods, fill_array_output_index, @@ -878,6 +890,9 @@ pub fn project_class( // Required interface inline wrappers let mut required_ifaces = Vec::new(); + let mut shared_member_candidates: Vec<(String, String, Vec)> = Vec::new(); + let mut conflicting_shared_members = HashSet::new(); + let share_interface_members = shared_interface_members_enabled(); // Track names already on the main class to avoid conflicts let mut main_member_names: HashSet = members .iter() @@ -897,6 +912,8 @@ pub fn project_class( continue; } let is_imported = imported_names.contains(&req_iface.name); + let is_shared_member_source = + share_interface_members && shared_iids.contains(&req_iface.iid) && is_imported; let reg_var = format!("_{}", req_iface.name); let mut ri_members = Vec::new(); @@ -987,9 +1004,25 @@ pub fn project_class( let sig_key = format!("{}#{}", name, pm.params.len()); if main_member_names.insert(sig_key) { members.push(member.clone()); + if is_shared_member_source { + record_shared_member_candidate( + &mut shared_member_candidates, + &mut conflicting_shared_members, + req_iface, + member, + ); + } } } else if main_member_names.insert(name) { members.push(member.clone()); + if is_shared_member_source { + record_shared_member_candidate( + &mut shared_member_candidates, + &mut conflicting_shared_members, + req_iface, + member, + ); + } } } @@ -1012,6 +1045,48 @@ pub fn project_class( // Merge overloaded method names: rename `foo2`, `foo3` to `foo` when `foo` exists. // Must happen after flatten so required-interface methods are included. merge_overload_names(&mut members); + let mut final_descriptor_counts = HashMap::new(); + for member in &members { + for descriptor_key in shared_member_descriptor_keys(member) { + *final_descriptor_counts + .entry(descriptor_key) + .or_insert(0usize) += 1; + } + } + + let mut shared_interface_members: Vec = Vec::new(); + if share_interface_members { + for (member_key, interface_name, descriptor_keys) in shared_member_candidates { + if conflicting_shared_members.contains(&member_key) + || descriptor_keys.iter().any(|descriptor_key| { + final_descriptor_counts + .get(descriptor_key) + .is_some_and(|count| *count > 1) + }) + { + continue; + } + let group = shared_interface_members + .iter_mut() + .find(|group| group.interface_name == interface_name); + if let Some(group) = group { + if !group.member_keys.contains(&member_key) { + group.member_keys.push(member_key); + } + for descriptor_key in descriptor_keys { + if !group.descriptor_keys.contains(&descriptor_key) { + group.descriptor_keys.push(descriptor_key); + } + } + } else { + shared_interface_members.push(ProjectedSharedInterfaceMembers { + interface_name, + member_keys: vec![member_key], + descriptor_keys, + }); + } + } + } // Check if _unwrap is used let needs_unwrap = check_needs_unwrap(&members, &required_ifaces); @@ -1029,6 +1104,7 @@ pub fn project_class( doc, members, required_ifaces, + shared_interface_members, static_cache_fields, static_accessors, }], @@ -1048,6 +1124,28 @@ pub fn project_interface( delegate_sigs: &HashMap, delegate_sig_refs: &HashMap>, delegate_param_wraps: &HashMap>, +) -> ProjectedFile { + project_interface_with_shared_member_source( + iface, + known_types, + delegate_type_names, + delegate_sigs, + delegate_sig_refs, + delegate_param_wraps, + false, + ) +} + +/// Project an interface and optionally make its prototype a canonical source +/// for concrete-class shared member descriptors. +pub fn project_interface_with_shared_member_source( + iface: &InterfaceMeta, + known_types: &HashSet, + delegate_type_names: &HashSet, + delegate_sigs: &HashMap, + delegate_sig_refs: &HashMap>, + delegate_param_wraps: &HashMap>, + shared_member_source: bool, ) -> ProjectedFile { // Check if delegate let is_delegate = iface.methods.iter().any(|m| m.name == ".ctor") @@ -1173,6 +1271,12 @@ pub fn project_interface( // Members let iface_var = format!("_{}", iface.name); + let shared_member_source = shared_member_source && !iface.iid.is_empty(); + let obj_expr = if shared_member_source { + "__interfaceValue(this)" + } else { + "this._obj" + }; let mut members = Vec::new(); for method in &iface.methods { if should_skip_raw_collection_method(iface, &method.name) { @@ -1180,7 +1284,7 @@ pub fn project_interface( } if let Some(m) = project_instance_method( &iface_var, - "this._obj", + obj_expr, method, known_types, &delegate_names, @@ -1193,7 +1297,7 @@ pub fn project_interface( } // Collection helpers - project_collection_helpers(iface, known_types, &mut members, &mut imports, "this._obj"); + project_collection_helpers(iface, known_types, &mut members, &mut imports, obj_expr); // Static create() for IVector / IMap project_collection_create(iface, known_types, &mut members, &mut imports); @@ -1269,6 +1373,7 @@ pub fn project_interface( iid_const: None, // already in file-level iid_consts has_static_from: !iface.iid.is_empty(), has_parameterized_cast, + shared_member_source, members, is_delegate: false, }], @@ -1411,6 +1516,107 @@ pub fn project_delegate( // Utility helpers // ====================================================================== +fn record_shared_member_candidate( + candidates: &mut Vec<(String, String, Vec)>, + conflicts: &mut HashSet, + interface: &InterfaceMeta, + member: &ProjectedMember, +) { + let Some(member_key) = shared_member_key(member) else { + return; + }; + if matches!( + member, + ProjectedMember::Method(ProjectedMethod { + overload_of: Some(_), + .. + }) + ) { + conflicts.insert(member_key); + return; + } + let descriptor_keys = shared_member_descriptor_keys(member); + if descriptor_keys.is_empty() { + return; + } + if let Some((_, existing_interface, existing_descriptors)) = candidates + .iter_mut() + .find(|(existing_key, _, _)| existing_key == &member_key) + { + if existing_interface != &interface.name || matches!(member, ProjectedMember::Method(_)) { + conflicts.insert(member_key); + } else { + for descriptor_key in descriptor_keys { + if !existing_descriptors.contains(&descriptor_key) { + existing_descriptors.push(descriptor_key); + } + } + } + return; + } + candidates.push((member_key, interface.name.clone(), descriptor_keys)); +} + +fn shared_member_key(member: &ProjectedMember) -> Option { + match member { + ProjectedMember::Method(method) => { + let has_numeric_suffix = method + .name + .chars() + .last() + .is_some_and(|character| character.is_ascii_digit()); + (!has_numeric_suffix).then(|| method.name.clone()) + } + ProjectedMember::Property(property) => Some(property.name.clone()), + ProjectedMember::Event(event) if !event.subscribe_name.is_empty() => { + Some(event.subscribe_name.clone()) + } + ProjectedMember::Symbol(symbol) => Some(symbol_dedup_key(&symbol.kind)), + _ => None, + } +} + +fn shared_member_descriptor_keys(member: &ProjectedMember) -> Vec { + let quoted = |name: &str| format!("'{name}'"); + match member { + ProjectedMember::Constructor(_) => vec![quoted("constructor")], + ProjectedMember::Method(method) => vec![quoted(&method.name)], + ProjectedMember::Property(property) => vec![quoted(&property.name)], + ProjectedMember::Event(event) => { + if !event.subscribe_name.is_empty() { + let event_name = event + .subscribe_name + .strip_prefix("on") + .unwrap_or(&event.subscribe_name); + let mut names = vec![ + quoted(&event.subscribe_name), + quoted(&format!("once{event_name}")), + ]; + if event.remove_vtable_index.is_some() { + names.push(quoted(&format!("off{event_name}"))); + } + names + } else if !event.unsubscribe_name.is_empty() { + vec![quoted(&event.unsubscribe_name)] + } else { + Vec::new() + } + } + ProjectedMember::Symbol(symbol) => vec![match &symbol.kind { + SymbolKind::ToString { .. } => quoted("toString"), + SymbolKind::ToPrimitive => "Symbol.toPrimitive".into(), + SymbolKind::ToStringTag { .. } => "Symbol.toStringTag".into(), + SymbolKind::Iterator { .. } => "Symbol.iterator".into(), + SymbolKind::CollectionLength => quoted("length"), + SymbolKind::CollectionAt { .. } => quoted("at"), + SymbolKind::CollectionToArray { .. } => quoted("toArray"), + SymbolKind::IteratorNext { .. } => quoted("next"), + }], + ProjectedMember::AsCast => vec![quoted("as")], + ProjectedMember::Close => vec![quoted("close")], + } +} + /// Returns a dedup key for a SymbolKind so flatten can detect duplicate symbols. pub fn symbol_dedup_key(kind: &SymbolKind) -> String { match kind { diff --git a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/render/javascript/mod.rs b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/render/javascript/mod.rs index a7658161..2b47cce8 100644 --- a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/render/javascript/mod.rs +++ b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/render/javascript/mod.rs @@ -113,6 +113,21 @@ fn render_esm(file: &ProjectedFile) -> String { } // Classes + if file + .classes + .iter() + .any(|class| !class.shared_interface_members.is_empty()) + { + out.push_str( + "const __copyInterfaceMembers = (target, source, keys) => {\n\ + for (const key of keys) {\n\ + const descriptor = Object.getOwnPropertyDescriptor(source.prototype, key);\n\ + if (descriptor === undefined) throw new Error(`Missing shared interface member ${String(key)}`);\n\ + Object.defineProperty(target.prototype, key, descriptor);\n\ + }\n\ +};\n\n", + ); + } for class in &file.classes { render_class_js(&mut out, class); @@ -251,6 +266,11 @@ fn render_class_js(out: &mut String, class: &ProjectedClass) { // Members — handle same-name overloads (from OverloadAttribute merging) out.push('\n'); + let shared_member_keys: std::collections::HashSet<&str> = class + .shared_interface_members + .iter() + .flat_map(|group| group.member_keys.iter().map(String::as_str)) + .collect(); let mut emitted_names: std::collections::HashSet = std::collections::HashSet::new(); // Group same-name methods within class.members let mut same_class_groups: std::collections::HashMap> = @@ -267,6 +287,12 @@ fn render_class_js(out: &mut String, class: &ProjectedClass) { } for (i, member) in class.members.iter().enumerate() { + if member_render_key(member) + .as_deref() + .is_some_and(|key| shared_member_keys.contains(key)) + { + continue; + } if let ProjectedMember::Method(method) = member { // Skip if already emitted as part of a group if !emitted_names.insert(method.name.clone()) { @@ -302,12 +328,27 @@ fn render_class_js(out: &mut String, class: &ProjectedClass) { render_member_js(out, member, &class.name); } out.push_str("}\n"); + for shared in &class.shared_interface_members { + out.push_str(&format!( + "__copyInterfaceMembers({}, {}, [{}]);\n", + class.name, + ref_marker(&shared.interface_name), + shared.descriptor_keys.join(", "), + )); + } } fn render_iface_js(out: &mut String, iface: &ProjectedIface, _file: &ProjectedFile) { if let Some(ref doc) = iface.doc { out.push_str(&render_jsdoc(doc, "")); } + if iface.shared_member_source { + out.push_str("const __interfaceInstances = new WeakSet();\n"); + out.push_str(&format!( + "const __interfaceValue = (value) => __interfaceInstances.has(value) ? value._obj : value._obj.cast(IID_{});\n", + iface.name + )); + } out.push_str(&format!("export class {} {{\n", iface.name)); out.push_str(" _obj;\n\n"); @@ -318,12 +359,14 @@ fn render_iface_js(out: &mut String, iface: &ProjectedIface, _file: &ProjectedFi " this._obj = obj.cast(IID_{});\n", iface.name )); - out.push_str(" }\n"); } else { out.push_str(" constructor(obj) {\n"); out.push_str(" this._obj = obj;\n"); - out.push_str(" }\n"); } + if iface.shared_member_source { + out.push_str(" __interfaceInstances.add(this);\n"); + } + out.push_str(" }\n"); // static from() if iface.has_static_from { @@ -363,6 +406,22 @@ fn render_required_iface_js(out: &mut String, ri: &ProjectedRequiredIface) { out.push_str("}\n"); } +fn member_render_key(member: &ProjectedMember) -> Option { + match member { + ProjectedMember::Method(method) => Some(method.name.clone()), + ProjectedMember::Property(property) => Some(property.name.clone()), + ProjectedMember::Event(event) if !event.subscribe_name.is_empty() => { + Some(event.subscribe_name.clone()) + } + ProjectedMember::Symbol(symbol) => { + Some(crate::codegen::winrt::javascript::project::symbol_dedup_key(&symbol.kind)) + } + ProjectedMember::Close => Some("close".into()), + ProjectedMember::AsCast => Some("as".into()), + _ => None, + } +} + fn render_member_js(out: &mut String, member: &ProjectedMember, _class_name: &str) { match member { ProjectedMember::Constructor(ctor) => { diff --git a/tools/dynwinrt-codegen/src/main.rs b/tools/dynwinrt-codegen/src/main.rs index bfe2b110..f75797c0 100644 --- a/tools/dynwinrt-codegen/src/main.rs +++ b/tools/dynwinrt-codegen/src/main.rs @@ -125,6 +125,11 @@ enum Commands { #[arg(long, default_value = "@microsoft/dynwinrt", value_name = "NAME")] import_name: String, + /// Reuse standalone interface prototype implementations for inherited + /// concrete-class members. Opt-in while compatibility data is gathered. + #[arg(long)] + shared_interface_members: bool, + /// Validate metadata and resolve dependencies without writing files #[arg(long)] dry_run: bool, @@ -257,6 +262,7 @@ fn run() -> Result<(), String> { lang, output, import_name, + shared_interface_members, dry_run, pyi, no_pyi, @@ -264,6 +270,9 @@ fn run() -> Result<(), String> { if lang != "py" && (pyi || no_pyi) { return Err("--pyi and --no-pyi require --lang py".into()); } + if lang != "js" && shared_interface_members { + return Err("--shared-interface-members requires --lang js".into()); + } let pyi = lang == "py" && !no_pyi; // Collect winmd paths from --folder and/or --winmd let mut winmd_parts: Vec = Vec::new(); @@ -370,6 +379,7 @@ fn run() -> Result<(), String> { let output_dir = effective_output_dir.as_path(); if lang == "js" { project::set_import_name(&import_name); + project::set_shared_interface_members(shared_interface_members); } if !dry_run { fs::create_dir_all(output_dir).map_err(|e| { @@ -1118,13 +1128,14 @@ fn generate_js_files( if !is_emittable_interface(iface) { continue; } - let projected = project::project_interface( + let projected = project::project_interface_with_shared_member_source( iface, known_types, delegate_type_names, delegate_sigs, delegate_sig_refs, delegate_param_wraps, + project::shared_interface_members_enabled() && shared_iids.contains(&iface.iid), ); let js = render_js::render(&projected); let dts = render_dts::render(&projected); @@ -1137,13 +1148,14 @@ fn generate_js_files( if !is_emittable_interface(iface) { continue; } - let projected = project::project_interface( + let projected = project::project_interface_with_shared_member_source( iface, known_types, delegate_type_names, delegate_sigs, delegate_sig_refs, delegate_param_wraps, + project::shared_interface_members_enabled() && shared_iids.contains(&iface.iid), ); let js = render_js::render(&projected); let dts = render_dts::render(&projected); @@ -3269,6 +3281,7 @@ fn print_capabilities() { "input.winmd-list", "input.ref-list", "selector.namespace-class", + "layout.shared-interface-members", ] { println!("{}", capability); } diff --git a/tools/dynwinrt-codegen/tests/shared_interface_members_test.rs b/tools/dynwinrt-codegen/tests/shared_interface_members_test.rs new file mode 100644 index 00000000..046a3aa4 --- /dev/null +++ b/tools/dynwinrt-codegen/tests/shared_interface_members_test.rs @@ -0,0 +1,479 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +use std::collections::{HashMap, HashSet}; +use std::fs; +use std::process::Command; + +use dynwinrt_codegen::codegen::{project, render_dts, render_js}; +use dynwinrt_codegen::meta::{ClassMeta, InterfaceMeta, MethodMeta, ParamDirection, ParamMeta}; +use dynwinrt_codegen::types::TypeMeta; + +fn value_interface() -> InterfaceMeta { + InterfaceMeta { + name: "IValue".into(), + namespace: "Contoso".into(), + iid: "11111111-1111-1111-1111-111111111111".into(), + methods: vec![ + MethodMeta { + name: "get_Value".into(), + raw_name: "get_Value".into(), + vtable_index: 6, + return_type: Some(TypeMeta::I32), + is_property_getter: true, + ..Default::default() + }, + MethodMeta { + name: "put_Value".into(), + raw_name: "put_Value".into(), + vtable_index: 7, + params: vec![ParamMeta { + name: "value".into(), + typ: TypeMeta::I32, + direction: ParamDirection::In, + }], + is_property_setter: true, + ..Default::default() + }, + ], + ..Default::default() + } +} + +fn widget_class(interface: &InterfaceMeta) -> ClassMeta { + ClassMeta { + name: "Widget".into(), + namespace: "Contoso".into(), + full_name: "Contoso.Widget".into(), + required_interfaces: vec![interface.clone()], + ..Default::default() + } +} + +#[test] +fn opt_in_reuses_shared_interface_descriptors_without_changing_dts() { + let interface = value_interface(); + let class = widget_class(&interface); + let known_types = HashSet::from(["Widget".into(), "IValue".into()]); + let shared_iids = HashSet::from([interface.iid.clone()]); + + project::set_shared_interface_members(true); + let interface_file = project::project_interface_with_shared_member_source( + &interface, + &known_types, + &HashSet::new(), + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + true, + ); + let class_file = project::project_class( + &class, + &known_types, + &HashSet::new(), + &shared_iids, + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + ); + project::set_shared_interface_members(false); + + let interface_js = render_js::render(&interface_file); + let class_js = render_js::render(&class_file); + let class_dts = render_dts::render(&class_file); + + assert!(interface_js.contains("const __interfaceInstances = new WeakSet();")); + assert!(interface_js.contains("__interfaceValue(this)")); + assert!(class_js.contains("__copyInterfaceMembers(Widget, (__get_IValue()), ['value']);")); + assert!(!class_js.contains("_IValue.method(6).invoke(this._obj.cast(IID_IValue)")); + assert!(class_dts.contains("get value(): number;")); + assert!(class_dts.contains("set value(value: number);")); + + let default_class_js = render_js::render(&project::project_class( + &class, + &known_types, + &HashSet::new(), + &shared_iids, + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + )); + assert!(!default_class_js.contains("__copyInterfaceMembers")); + assert!(default_class_js.contains("_IValue.method(6).invoke(this._obj.cast(IID_IValue)")); +} + +#[test] +fn shared_interface_members_preserve_overload_dispatch_and_declarations() { + let interface = InterfaceMeta { + name: "IOverloaded".into(), + namespace: "Contoso".into(), + iid: "22222222-2222-2222-2222-222222222222".into(), + methods: vec![ + MethodMeta { + name: "DoThing".into(), + raw_name: "DoThing".into(), + vtable_index: 6, + params: vec![ParamMeta { + name: "value".into(), + typ: TypeMeta::I32, + direction: ParamDirection::In, + }], + ..Default::default() + }, + MethodMeta { + name: "DoThing2".into(), + raw_name: "DoThing".into(), + vtable_index: 7, + params: vec![ + ParamMeta { + name: "value".into(), + typ: TypeMeta::I32, + direction: ParamDirection::In, + }, + ParamMeta { + name: "other".into(), + typ: TypeMeta::I32, + direction: ParamDirection::In, + }, + ], + ..Default::default() + }, + ], + ..Default::default() + }; + let class = widget_class(&interface); + let known_types = HashSet::from(["Widget".into(), "IOverloaded".into()]); + let shared_iids = HashSet::from([interface.iid.clone()]); + + project::set_shared_interface_members(true); + let interface_js = render_js::render(&project::project_interface_with_shared_member_source( + &interface, + &known_types, + &HashSet::new(), + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + true, + )); + let class_file = project::project_class( + &class, + &known_types, + &HashSet::new(), + &shared_iids, + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + ); + project::set_shared_interface_members(false); + let class_js = render_js::render(&class_file); + let class_dts = render_dts::render(&class_file); + + assert!(interface_js.contains("doThing(value)"), "{interface_js}"); + assert!( + interface_js.contains("doThing2(value, other)"), + "{interface_js}" + ); + assert!(!class_js.contains("__copyInterfaceMembers")); + assert!(class_js.contains("_doThing_1(value)"), "{class_js}"); + assert!(class_js.contains("_doThing_2(value, other)"), "{class_js}"); + assert!(class_js.contains("doThing(...args)"), "{class_js}"); + assert_eq!(class_dts.matches("doThing(").count(), 2); +} + +#[test] +fn shared_interface_members_preserve_cross_interface_overload_dispatch() { + let default_interface = InterfaceMeta { + name: "IWidget".into(), + namespace: "Contoso".into(), + iid: "33333333-3333-3333-3333-333333333333".into(), + methods: vec![MethodMeta { + name: "DoThing2".into(), + raw_name: "DoThing2".into(), + vtable_index: 6, + params: vec![ + ParamMeta { + name: "value".into(), + typ: TypeMeta::I32, + direction: ParamDirection::In, + }, + ParamMeta { + name: "other".into(), + typ: TypeMeta::I32, + direction: ParamDirection::In, + }, + ], + ..Default::default() + }], + ..Default::default() + }; + let required_interface = InterfaceMeta { + name: "IRequired".into(), + namespace: "Contoso".into(), + iid: "44444444-4444-4444-4444-444444444444".into(), + methods: vec![MethodMeta { + name: "DoThing".into(), + raw_name: "DoThing".into(), + vtable_index: 6, + params: vec![ParamMeta { + name: "value".into(), + typ: TypeMeta::I32, + direction: ParamDirection::In, + }], + ..Default::default() + }], + ..Default::default() + }; + let class = ClassMeta { + default_interface: Some(default_interface), + required_interfaces: vec![required_interface.clone()], + ..widget_class(&required_interface) + }; + let known_types = HashSet::from(["Widget".into(), "IWidget".into(), "IRequired".into()]); + let shared_iids = HashSet::from([required_interface.iid.clone()]); + + project::set_shared_interface_members(true); + let class_file = project::project_class( + &class, + &known_types, + &HashSet::new(), + &shared_iids, + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + ); + project::set_shared_interface_members(false); + let class_js = render_js::render(&class_file); + let class_dts = render_dts::render(&class_file); + + assert!(!class_js.contains("__copyInterfaceMembers")); + assert!(class_js.contains("_doThing_1(value)"), "{class_js}"); + assert!(class_js.contains("_doThing_2(value, other)"), "{class_js}"); + assert!(class_js.contains("doThing(...args)"), "{class_js}"); + assert_eq!(class_dts.matches("doThing(").count(), 2); +} + +#[test] +fn shared_interface_event_alias_conflicts_remain_class_local() { + let default_interface = InterfaceMeta { + name: "IWidget".into(), + namespace: "Contoso".into(), + iid: "55555555-5555-5555-5555-555555555555".into(), + methods: vec![MethodMeta { + name: "OnceChanged".into(), + raw_name: "OnceChanged".into(), + vtable_index: 6, + ..Default::default() + }], + ..Default::default() + }; + let required_interface = InterfaceMeta { + name: "IChanged".into(), + namespace: "Contoso".into(), + iid: "66666666-6666-6666-6666-666666666666".into(), + methods: vec![ + MethodMeta { + name: "add_Changed".into(), + raw_name: "add_Changed".into(), + vtable_index: 6, + params: vec![ParamMeta { + name: "handler".into(), + typ: TypeMeta::Object, + direction: ParamDirection::In, + }], + is_event_add: true, + ..Default::default() + }, + MethodMeta { + name: "remove_Changed".into(), + raw_name: "remove_Changed".into(), + vtable_index: 7, + params: vec![ParamMeta { + name: "token".into(), + typ: TypeMeta::I64, + direction: ParamDirection::In, + }], + is_event_remove: true, + ..Default::default() + }, + ], + ..Default::default() + }; + let class = ClassMeta { + default_interface: Some(default_interface), + required_interfaces: vec![required_interface.clone()], + ..widget_class(&required_interface) + }; + let known_types = HashSet::from(["Widget".into(), "IWidget".into(), "IChanged".into()]); + let shared_iids = HashSet::from([required_interface.iid.clone()]); + + project::set_shared_interface_members(true); + let class_file = project::project_class( + &class, + &known_types, + &HashSet::new(), + &shared_iids, + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + ); + project::set_shared_interface_members(false); + let class_js = render_js::render(&class_file); + + assert!(!class_js.contains("__copyInterfaceMembers")); + assert!(class_js.contains("onChanged(callback)"), "{class_js}"); + assert!( + class_js.contains("_IChanged.method(6).invoke(this._obj.cast(IID_IChanged)"), + "{class_js}" + ); +} + +#[test] +fn noncanonical_required_interfaces_remain_class_local() { + let interface = value_interface(); + let class = widget_class(&interface); + let known_types = HashSet::from(["Widget".into(), "IValue".into()]); + + project::set_shared_interface_members(true); + let class_file = project::project_class( + &class, + &known_types, + &HashSet::new(), + &HashSet::new(), + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + ); + project::set_shared_interface_members(false); + assert_eq!(class_file.classes[0].required_ifaces.len(), 1); + let interface_file = project::project_interface_with_shared_member_source( + &interface, + &known_types, + &HashSet::new(), + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + false, + ); + let class_js = render_js::render(&class_file); + let interface_js = render_js::render(&interface_file); + + assert!(!class_js.contains("__copyInterfaceMembers")); + assert!(class_js.contains("_IValue.method(6).invoke(this._obj.cast(IID_IValue)")); + assert!(!interface_js.contains("__interfaceValue")); +} + +#[test] +fn shared_interface_descriptor_executes_for_raw_and_concrete_views() { + if Command::new("node").arg("--version").output().is_err() { + eprintln!("Skipping shared-interface runtime test: node is unavailable"); + return; + } + + let interface = value_interface(); + let class = widget_class(&interface); + let known_types = HashSet::from(["Widget".into(), "IValue".into()]); + let shared_iids = HashSet::from([interface.iid.clone()]); + + project::set_import_name("./runtime.js"); + project::set_shared_interface_members(true); + let interface_js = render_js::render(&project::project_interface_with_shared_member_source( + &interface, + &known_types, + &HashSet::new(), + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + true, + )); + let class_js = render_js::render(&project::project_class( + &class, + &known_types, + &HashSet::new(), + &shared_iids, + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + )); + project::set_shared_interface_members(false); + project::set_import_name("@microsoft/dynwinrt"); + + let directory = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) + .join("target") + .join(format!("shared-interface-runtime-{}", std::process::id())); + let _ = fs::remove_dir_all(&directory); + fs::create_dir_all(&directory).unwrap(); + fs::write(directory.join("IValue.js"), interface_js).unwrap(); + fs::write(directory.join("Widget.js"), class_js).unwrap(); + fs::write( + directory.join("lifetime.js"), + "\ +exports.castProjectedValueBorrowed = (value) => value;\n\ +exports.castProjectedValueOwned = (value) => value;\n\ +exports.trackProjectedValue = (value) => value;\n", + ) + .unwrap(); + fs::write( + directory.join("runtime.js"), + "\ +class DynWinRtMethodSig {\n\ + addIn() { return this; }\n\ + addOut() { return this; }\n\ +}\n\ +const DynWinRtType = {\n\ + registerInterface() {\n\ + return {\n\ + addMethod() { return this; },\n\ + method(index) { return { invoke: (obj, args) => obj.invoke(index, args) }; },\n\ + };\n\ + },\n\ + i32() { return {}; },\n\ +};\n\ +const DynWinRtValue = { i32: (value) => value };\n\ +const DynWinRtArray = {};\n\ +const DynWinRtDelegate = {};\n\ +const WinGuid = { parse: (value) => value };\n\ +module.exports = { DynWinRtType, DynWinRtMethodSig, DynWinRtValue, DynWinRtArray, DynWinRtDelegate, WinGuid };\n", + ) + .unwrap(); + fs::write( + directory.join("test.js"), + "\ +const assert = require('node:assert/strict');\n\ +const { IValue } = require('./IValue.js');\n\ +const { Widget } = require('./Widget.js');\n\ +let current = 41;\n\ +let casts = 0;\n\ +const interfaceValue = {\n\ + invoke(index, args) {\n\ + if (index === 6) return { toNumber: () => current };\n\ + if (index === 7) { current = args[0]; return undefined; }\n\ + throw new Error(`unexpected slot ${index}`);\n\ + },\n\ +};\n\ +const raw = { cast() { casts++; return interfaceValue; } };\n\ +const widget = Object.assign(Object.create(Widget.prototype), { _obj: raw });\n\ +const concreteDescriptor = Object.getOwnPropertyDescriptor(Widget.prototype, 'value');\n\ +const interfaceDescriptor = Object.getOwnPropertyDescriptor(IValue.prototype, 'value');\n\ +assert.equal(concreteDescriptor.get, interfaceDescriptor.get);\n\ +assert.equal(widget.value, 41);\n\ +widget.value = 52;\n\ +assert.equal(widget.value, 52);\n\ +const view = IValue.from(raw);\n\ +assert.equal(view.value, 52);\n\ +assert.equal(casts, 4);\n", + ) + .unwrap(); + + let output = Command::new("node") + .arg("test.js") + .current_dir(&directory) + .output() + .unwrap(); + let _ = fs::remove_dir_all(&directory); + assert!( + output.status.success(), + "node failed:\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + ); +} From ab3015681cdfb242bc4082e71af037125cea565d Mon Sep 17 00:00:00 2001 From: "Leilei Zhang (from Dev Box)" Date: Tue, 11 Aug 2026 20:15:49 +0800 Subject: [PATCH 2/8] Fix canonical shared interface sources Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b485b75e-2292-44e1-98c8-3f7bb6489bff --- README.md | 4 +- tools/dynwinrt-codegen/npm/README.md | 4 +- .../codegen/winrt/javascript/project/mod.rs | 99 ++++++- .../winrt/javascript/render/javascript/mod.rs | 19 +- tools/dynwinrt-codegen/src/main.rs | 140 ++++++--- .../tests/consistency_test.rs | 6 +- .../tests/shared_interface_members_test.rs | 276 +++++++++++++++++- tools/dynwinrt-codegen/tests/snapshot_test.rs | 4 +- 8 files changed, 493 insertions(+), 59 deletions(-) diff --git a/README.md b/README.md index c673260d..02ab250d 100644 --- a/README.md +++ b/README.md @@ -195,7 +195,9 @@ conflicting members remain class-local, raw interface wrapper classes remain available, and the option does not change generated `.d.ts` files. Only required interfaces already canonicalized as standalone shared wrappers participate; one-off inline required interfaces remain class-local. Generation -without this flag is unchanged. +without this flag is unchanged. If a shared source filename is ambiguous +between distinct interface identities, generation fails instead of choosing +an unsafe descriptor source. Focused validation: diff --git a/tools/dynwinrt-codegen/npm/README.md b/tools/dynwinrt-codegen/npm/README.md index 61895631..b9d9caa1 100644 --- a/tools/dynwinrt-codegen/npm/README.md +++ b/tools/dynwinrt-codegen/npm/README.md @@ -74,7 +74,9 @@ the standalone required-interface prototypes. Overloaded or conflicting members remain class-local, raw interface wrappers remain available, generated `.d.ts` files are unchanged, and generation without the flag is unchanged. Only interfaces already emitted as canonical standalone shared wrappers -participate; one-off inline required interfaces remain class-local. +participate; one-off inline required interfaces remain class-local. Generation +rejects a shared source filename that is ambiguous between distinct interface +identities. ## What gets generated diff --git a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/project/mod.rs b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/project/mod.rs index ab0be29e..4bd605ac 100644 --- a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/project/mod.rs +++ b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/project/mod.rs @@ -47,6 +47,93 @@ pub fn shared_interface_members_enabled() -> bool { SHARED_INTERFACE_MEMBERS.with(|value| *value.borrow()) } +#[derive(Clone, Debug, Eq, Hash, PartialEq)] +pub struct StandaloneInterfaceIdentity { + pub namespace: String, + pub name: String, + pub iid: String, +} + +impl StandaloneInterfaceIdentity { + fn describe(&self) -> String { + format!("{}.{} ({})", self.namespace, self.name, self.iid) + } +} + +pub fn standalone_interface_identity(iface: &InterfaceMeta) -> Option { + if iface.iid.is_empty() { + return None; + } + Some(StandaloneInterfaceIdentity { + namespace: iface.namespace.clone(), + name: iface.name.clone(), + iid: iface.iid.to_ascii_lowercase(), + }) +} + +#[derive(Clone, Debug)] +pub struct CanonicalInterfaceSource { + pub interface: InterfaceMeta, + pub identity: StandaloneInterfaceIdentity, + pub shared_member_source: bool, +} + +pub fn canonical_interface_sources( + interfaces: &[InterfaceMeta], + shared_candidates: &[InterfaceMeta], + class_names: &HashSet, +) -> Result, String> { + let shared_identities = shared_candidates + .iter() + .filter_map(standalone_interface_identity) + .collect::>(); + let mut sources: Vec = Vec::new(); + let mut source_by_name: HashMap = HashMap::new(); + + for iface in shared_candidates.iter().chain(interfaces) { + if class_names.contains(&iface.name) { + continue; + } + let Some(identity) = standalone_interface_identity(iface) else { + continue; + }; + let shared_member_source = shared_identities.contains(&identity); + if let Some(existing_index) = source_by_name.get(&iface.name).copied() { + let existing = &mut sources[existing_index]; + if existing.identity != identity { + if existing.shared_member_source || shared_member_source { + return Err(format!( + "Cannot use `{0}.js` as a shared interface member source because `{1}` and \ + `{2}` have different interface identities. Generate them separately or \ + select only one type.", + iface.name, + existing.identity.describe(), + identity.describe(), + )); + } + *existing = CanonicalInterfaceSource { + interface: iface.clone(), + identity, + shared_member_source: false, + }; + continue; + } + existing.shared_member_source |= shared_member_source; + continue; + } + + source_by_name.insert(iface.name.clone(), sources.len()); + sources.push(CanonicalInterfaceSource { + interface: iface.clone(), + identity, + shared_member_source, + }); + } + + sources.sort_by(|left, right| left.interface.name.cmp(&right.interface.name)); + Ok(sources) +} + use crate::codegen::winrt::shared::imports::{ NO_DEFERRED, collect_iface_type_imports, collect_type_imports, collect_used_generics_from_class, collect_used_generics_from_methods, fill_array_output_index, @@ -189,7 +276,7 @@ pub fn project_class( class: &ClassMeta, known_types: &HashSet, delegate_type_names: &HashSet, - shared_iids: &HashSet, + shared_interface_sources: &HashSet, delegate_sigs: &HashMap, delegate_sig_refs: &HashMap>, delegate_param_wraps: &HashMap>, @@ -326,7 +413,8 @@ pub fn project_class( for req_iface in &class.required_interfaces { if req_iface.generic_piid.is_none() && !req_iface.iid.is_empty() - && shared_iids.contains(&req_iface.iid) + && standalone_interface_identity(req_iface) + .is_some_and(|identity| shared_interface_sources.contains(&identity)) && !imported_names.contains(&req_iface.name) { imports.push(format_type_import_projected( @@ -912,8 +1000,11 @@ pub fn project_class( continue; } let is_imported = imported_names.contains(&req_iface.name); - let is_shared_member_source = - share_interface_members && shared_iids.contains(&req_iface.iid) && is_imported; + let is_shared_member_source = share_interface_members + && req_iface.generic_piid.is_none() + && standalone_interface_identity(req_iface) + .is_some_and(|identity| shared_interface_sources.contains(&identity)) + && is_imported; let reg_var = format!("_{}", req_iface.name); let mut ri_members = Vec::new(); diff --git a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/render/javascript/mod.rs b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/render/javascript/mod.rs index 2b47cce8..5387d093 100644 --- a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/render/javascript/mod.rs +++ b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/render/javascript/mod.rs @@ -113,13 +113,20 @@ fn render_esm(file: &ProjectedFile) -> String { } // Classes - if file + let copies_shared_interface_members = file .classes .iter() - .any(|class| !class.shared_interface_members.is_empty()) - { + .any(|class| !class.shared_interface_members.is_empty()); + let is_shared_interface_source = file.ifaces.iter().any(|iface| iface.shared_member_source); + if copies_shared_interface_members || is_shared_interface_source { + out.push_str( + "const __sharedInterfaceMemberSource = Symbol.for('dynwinrt.sharedInterfaceMemberSource');\n\n", + ); + } + if copies_shared_interface_members { out.push_str( "const __copyInterfaceMembers = (target, source, keys) => {\n\ + if (source[__sharedInterfaceMemberSource] !== true) throw new Error(`Interface ${source.name} is not a shared member source`);\n\ for (const key of keys) {\n\ const descriptor = Object.getOwnPropertyDescriptor(source.prototype, key);\n\ if (descriptor === undefined) throw new Error(`Missing shared interface member ${String(key)}`);\n\ @@ -387,6 +394,12 @@ fn render_iface_js(out: &mut String, iface: &ProjectedIface, _file: &ProjectedFi // so we just need the file.iid_consts to contain it out.push_str("}\n"); + if iface.shared_member_source { + out.push_str(&format!( + "Object.defineProperty({}, __sharedInterfaceMemberSource, {{ value: true }});\n", + iface.name + )); + } } fn render_required_iface_js(out: &mut String, ri: &ProjectedRequiredIface) { diff --git a/tools/dynwinrt-codegen/src/main.rs b/tools/dynwinrt-codegen/src/main.rs index f75797c0..b928f714 100644 --- a/tools/dynwinrt-codegen/src/main.rs +++ b/tools/dynwinrt-codegen/src/main.rs @@ -995,6 +995,50 @@ fn generate_for_types( let (delegate_signatures, delegate_sig_refs, delegate_param_wraps) = project::build_delegate_signatures(&all_interfaces, &delegate_type_names, &known_types); + let shared_interface_members_enabled = + lang == "js" && project::shared_interface_members_enabled(); + let canonical_shared_interfaces = if shared_interface_members_enabled { + let mut required_interface_count: HashMap< + project::StandaloneInterfaceIdentity, + (&meta::InterfaceMeta, usize), + > = HashMap::new(); + for class in &all_classes { + for req_iface in &class.required_interfaces { + let Some(identity) = project::standalone_interface_identity(req_iface) else { + continue; + }; + required_interface_count + .entry(identity) + .and_modify(|(_, count)| *count += 1) + .or_insert((req_iface, 1)); + } + } + required_interface_count + .values() + .filter(|(_, count)| *count >= 2) + .map(|(iface, _)| (*iface).clone()) + .collect::>() + } else { + Vec::new() + }; + for iface in &canonical_shared_interfaces { + known_types.insert(iface.name.clone()); + } + let canonical_interface_sources = if shared_interface_members_enabled { + Some(project::canonical_interface_sources( + &all_interfaces, + &canonical_shared_interfaces, + &class_names_all, + )?) + } else { + None + }; + let shared_interface_source_identities = canonical_interface_sources + .iter() + .flatten() + .filter(|source| source.shared_member_source) + .map(|source| source.identity.clone()) + .collect::>(); if !dry_run { if lang == "py" { @@ -1018,7 +1062,8 @@ fn generate_for_types( &shared_interfaces, &known_types, &delegate_type_names, - &shared_iids, + canonical_interface_sources.as_deref(), + &shared_interface_source_identities, &delegate_signatures, &delegate_sig_refs, &delegate_param_wraps, @@ -1091,7 +1136,8 @@ fn generate_js_files( shared_interfaces: &[meta::InterfaceMeta], known_types: &HashSet, delegate_type_names: &HashSet, - shared_iids: &HashSet, + canonical_interface_sources: Option<&[project::CanonicalInterfaceSource]>, + shared_interface_source_identities: &HashSet, delegate_sigs: &HashMap, delegate_sig_refs: &HashMap>, delegate_param_wraps: &HashMap>, @@ -1121,45 +1167,61 @@ fn generate_js_files( !iface.iid.is_empty() } - for iface in shared_interfaces { - if class_names.contains(iface.name.as_str()) { - continue; - } - if !is_emittable_interface(iface) { - continue; + if let Some(canonical_sources) = canonical_interface_sources { + for source in canonical_sources { + let iface = &source.interface; + let projected = project::project_interface_with_shared_member_source( + iface, + known_types, + delegate_type_names, + delegate_sigs, + delegate_sig_refs, + delegate_param_wraps, + source.shared_member_source, + ); + let js = render_js::render(&projected); + let dts = render_dts::render(&projected); + emit(&iface.name, &js, &dts)?; } - let projected = project::project_interface_with_shared_member_source( - iface, - known_types, - delegate_type_names, - delegate_sigs, - delegate_sig_refs, - delegate_param_wraps, - project::shared_interface_members_enabled() && shared_iids.contains(&iface.iid), - ); - let js = render_js::render(&projected); - let dts = render_dts::render(&projected); - emit(&iface.name, &js, &dts)?; - } - for iface in all_interfaces { - if class_names.contains(iface.name.as_str()) { - continue; + } else { + for iface in shared_interfaces { + if class_names.contains(iface.name.as_str()) { + continue; + } + if !is_emittable_interface(iface) { + continue; + } + let projected = project::project_interface( + iface, + known_types, + delegate_type_names, + delegate_sigs, + delegate_sig_refs, + delegate_param_wraps, + ); + let js = render_js::render(&projected); + let dts = render_dts::render(&projected); + emit(&iface.name, &js, &dts)?; } - if !is_emittable_interface(iface) { - continue; + for iface in all_interfaces { + if class_names.contains(iface.name.as_str()) { + continue; + } + if !is_emittable_interface(iface) { + continue; + } + let projected = project::project_interface( + iface, + known_types, + delegate_type_names, + delegate_sigs, + delegate_sig_refs, + delegate_param_wraps, + ); + let js = render_js::render(&projected); + let dts = render_dts::render(&projected); + emit(&iface.name, &js, &dts)?; } - let projected = project::project_interface_with_shared_member_source( - iface, - known_types, - delegate_type_names, - delegate_sigs, - delegate_sig_refs, - delegate_param_wraps, - project::shared_interface_members_enabled() && shared_iids.contains(&iface.iid), - ); - let js = render_js::render(&projected); - let dts = render_dts::render(&projected); - emit(&iface.name, &js, &dts)?; } for en in all_enums { if let TypeMeta::Enum { name, .. } = en { @@ -1206,7 +1268,7 @@ fn generate_js_files( class, known_types, delegate_type_names, - shared_iids, + shared_interface_source_identities, delegate_sigs, delegate_sig_refs, delegate_param_wraps, diff --git a/tools/dynwinrt-codegen/tests/consistency_test.rs b/tools/dynwinrt-codegen/tests/consistency_test.rs index 9430cf80..eab40656 100644 --- a/tools/dynwinrt-codegen/tests/consistency_test.rs +++ b/tools/dynwinrt-codegen/tests/consistency_test.rs @@ -154,7 +154,7 @@ fn setup_metadata( Vec, HashSet, HashSet, - HashSet, + HashSet, HashMap, HashMap>, HashMap>, @@ -189,7 +189,7 @@ fn setup_metadata( }) .map(|i| i.name.clone()) .collect(); - let shared_iids: HashSet = HashSet::new(); + let shared_iids: HashSet = HashSet::new(); let (delegate_sigs, delegate_sig_refs, delegate_param_wraps) = project::build_delegate_signatures(&all_interfaces, &delegate_type_names, &known_types); @@ -436,7 +436,7 @@ fn js_dts_structural_consistency_user_watcher() { }) .map(|i| i.name.clone()) .collect(); - let shared_iids: HashSet = HashSet::new(); + let shared_iids: HashSet = HashSet::new(); let (delegate_sigs, delegate_sig_refs, delegate_param_wraps) = project::build_delegate_signatures(&all_interfaces, &delegate_type_names, &known_types); diff --git a/tools/dynwinrt-codegen/tests/shared_interface_members_test.rs b/tools/dynwinrt-codegen/tests/shared_interface_members_test.rs index 046a3aa4..5dba1261 100644 --- a/tools/dynwinrt-codegen/tests/shared_interface_members_test.rs +++ b/tools/dynwinrt-codegen/tests/shared_interface_members_test.rs @@ -40,6 +40,23 @@ fn value_interface() -> InterfaceMeta { } } +fn items_control_interface() -> InterfaceMeta { + InterfaceMeta { + name: "IItemsControl".into(), + namespace: "Contoso.Controls".into(), + iid: "77777777-7777-7777-7777-777777777777".into(), + methods: vec![MethodMeta { + name: "get_Items".into(), + raw_name: "get_Items".into(), + vtable_index: 6, + return_type: Some(TypeMeta::Object), + is_property_getter: true, + ..Default::default() + }], + ..Default::default() + } +} + fn widget_class(interface: &InterfaceMeta) -> ClassMeta { ClassMeta { name: "Widget".into(), @@ -50,12 +67,16 @@ fn widget_class(interface: &InterfaceMeta) -> ClassMeta { } } +fn shared_sources(interface: &InterfaceMeta) -> HashSet { + HashSet::from([project::standalone_interface_identity(interface).unwrap()]) +} + #[test] fn opt_in_reuses_shared_interface_descriptors_without_changing_dts() { let interface = value_interface(); let class = widget_class(&interface); let known_types = HashSet::from(["Widget".into(), "IValue".into()]); - let shared_iids = HashSet::from([interface.iid.clone()]); + let shared_iids = shared_sources(&interface); project::set_shared_interface_members(true); let interface_file = project::project_interface_with_shared_member_source( @@ -84,7 +105,9 @@ fn opt_in_reuses_shared_interface_descriptors_without_changing_dts() { assert!(interface_js.contains("const __interfaceInstances = new WeakSet();")); assert!(interface_js.contains("__interfaceValue(this)")); + assert!(interface_js.contains("Object.defineProperty(IValue, __sharedInterfaceMemberSource")); assert!(class_js.contains("__copyInterfaceMembers(Widget, (__get_IValue()), ['value']);")); + assert!(class_js.contains("source[__sharedInterfaceMemberSource] !== true")); assert!(!class_js.contains("_IValue.method(6).invoke(this._obj.cast(IID_IValue)")); assert!(class_dts.contains("get value(): number;")); assert!(class_dts.contains("set value(value: number);")); @@ -102,6 +125,94 @@ fn opt_in_reuses_shared_interface_descriptors_without_changing_dts() { assert!(default_class_js.contains("_IValue.method(6).invoke(this._obj.cast(IID_IValue)")); } +#[test] +fn canonical_interface_source_survives_equivalent_duplicate_emission() { + let shared = value_interface(); + let mut duplicate = shared.clone(); + duplicate.iid = shared.iid.to_ascii_uppercase(); + let sources = project::canonical_interface_sources( + &[duplicate.clone()], + std::slice::from_ref(&shared), + &HashSet::new(), + ) + .unwrap(); + + assert_eq!(sources.len(), 1); + assert!(sources[0].shared_member_source); + assert_eq!(sources[0].interface.iid, shared.iid); + + project::set_shared_interface_members(true); + let interface_file = project::project_interface_with_shared_member_source( + &sources[0].interface, + &HashSet::from(["Widget".into(), "IValue".into()]), + &HashSet::new(), + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + sources[0].shared_member_source, + ); + let class_file = project::project_class( + &widget_class(&duplicate), + &HashSet::from(["Widget".into(), "IValue".into()]), + &HashSet::new(), + &HashSet::from([sources[0].identity.clone()]), + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + ); + project::set_shared_interface_members(false); + + assert!(render_js::render(&interface_file).contains("__interfaceValue(this)")); + assert!(render_js::render(&class_file).contains("__copyInterfaceMembers")); + assert_eq!( + render_dts::render(&interface_file), + render_dts::render(&project::project_interface( + &duplicate, + &HashSet::from(["Widget".into(), "IValue".into()]), + &HashSet::new(), + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + )) + ); +} + +#[test] +fn ambiguous_standalone_interface_identity_fails_explicitly() { + let first = value_interface(); + let mut second = first.clone(); + second.namespace = "Fabrikam".into(); + second.iid = "88888888-8888-8888-8888-888888888888".into(); + + let error = project::canonical_interface_sources( + std::slice::from_ref(&second), + std::slice::from_ref(&first), + &HashSet::new(), + ) + .expect_err("ambiguous shared source identities must fail"); + assert!(error.contains("IValue.js"), "{error}"); + assert!(error.contains("Contoso.IValue"), "{error}"); + assert!(error.contains("Fabrikam.IValue"), "{error}"); +} + +#[test] +fn nonshared_duplicate_interface_keeps_legacy_final_source() { + let first = value_interface(); + let mut second = first.clone(); + second.namespace = "Fabrikam".into(); + second.iid = "88888888-8888-8888-8888-888888888888".into(); + + let sources = + project::canonical_interface_sources(&[first, second.clone()], &[], &HashSet::new()) + .unwrap(); + assert_eq!(sources.len(), 1); + assert!(!sources[0].shared_member_source); + assert_eq!( + sources[0].identity, + project::standalone_interface_identity(&second).unwrap() + ); +} + #[test] fn shared_interface_members_preserve_overload_dispatch_and_declarations() { let interface = InterfaceMeta { @@ -143,7 +254,7 @@ fn shared_interface_members_preserve_overload_dispatch_and_declarations() { }; let class = widget_class(&interface); let known_types = HashSet::from(["Widget".into(), "IOverloaded".into()]); - let shared_iids = HashSet::from([interface.iid.clone()]); + let shared_iids = shared_sources(&interface); project::set_shared_interface_members(true); let interface_js = render_js::render(&project::project_interface_with_shared_member_source( @@ -229,7 +340,7 @@ fn shared_interface_members_preserve_cross_interface_overload_dispatch() { ..widget_class(&required_interface) }; let known_types = HashSet::from(["Widget".into(), "IWidget".into(), "IRequired".into()]); - let shared_iids = HashSet::from([required_interface.iid.clone()]); + let shared_iids = shared_sources(&required_interface); project::set_shared_interface_members(true); let class_file = project::project_class( @@ -304,7 +415,7 @@ fn shared_interface_event_alias_conflicts_remain_class_local() { ..widget_class(&required_interface) }; let known_types = HashSet::from(["Widget".into(), "IWidget".into(), "IChanged".into()]); - let shared_iids = HashSet::from([required_interface.iid.clone()]); + let shared_iids = shared_sources(&required_interface); project::set_shared_interface_members(true); let class_file = project::project_class( @@ -363,16 +474,169 @@ fn noncanonical_required_interfaces_remain_class_local() { } #[test] -fn shared_interface_descriptor_executes_for_raw_and_concrete_views() { +fn collection_getter_casts_concrete_view_and_rejects_unmarked_source() { if Command::new("node").arg("--version").output().is_err() { eprintln!("Skipping shared-interface runtime test: node is unavailable"); return; } + let interface = items_control_interface(); + let class = ClassMeta { + name: "ListView".into(), + namespace: "Contoso.Controls".into(), + full_name: "Contoso.Controls.ListView".into(), + required_interfaces: vec![interface.clone()], + ..Default::default() + }; + let known_types = HashSet::from(["ListView".into(), "IItemsControl".into()]); + let shared_sources = shared_sources(&interface); + + project::set_import_name("./runtime.js"); + project::set_shared_interface_members(true); + let interface_js = render_js::render(&project::project_interface_with_shared_member_source( + &interface, + &known_types, + &HashSet::new(), + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + true, + )); + let unsafe_interface_js = + render_js::render(&project::project_interface_with_shared_member_source( + &interface, + &known_types, + &HashSet::new(), + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + false, + )); + let class_js = render_js::render(&project::project_class( + &class, + &known_types, + &HashSet::new(), + &shared_sources, + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + )); + project::set_shared_interface_members(false); + project::set_import_name("@microsoft/dynwinrt"); + + let directory = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) + .join("target") + .join(format!("shared-collection-runtime-{}", std::process::id())); + let _ = fs::remove_dir_all(&directory); + fs::create_dir_all(&directory).unwrap(); + fs::write(directory.join("IItemsControl.js"), &interface_js).unwrap(); + fs::write(directory.join("ListView.js"), class_js).unwrap(); + fs::write( + directory.join("lifetime.js"), + "\ + exports.castProjectedValueBorrowed = (value) => value;\n\ + exports.castProjectedValueOwned = (value) => value;\n\ + exports.trackProjectedValue = (value) => value;\n", + ) + .unwrap(); + fs::write( + directory.join("runtime.js"), + "\ + class DynWinRtMethodSig {\n\ + addIn() { return this; }\n\ + addOut() { return this; }\n\ + }\n\ + const DynWinRtType = {\n\ + registerInterface() {\n\ + return {\n\ + addMethod() { return this; },\n\ + method(index) { return { invoke: (obj, args) => obj.invoke(index, args) }; },\n\ + };\n\ + },\n\ + object() { return {}; },\n\ + };\n\ + const DynWinRtValue = {};\n\ + const DynWinRtArray = {};\n\ + const DynWinRtDelegate = {};\n\ + const WinGuid = { parse: (value) => value };\n\ + module.exports = { DynWinRtType, DynWinRtMethodSig, DynWinRtValue, DynWinRtArray, DynWinRtDelegate, WinGuid };\n", + ) + .unwrap(); + fs::write( + directory.join("test.js"), + "\ + const assert = require('node:assert/strict');\n\ + const { IItemsControl } = require('./IItemsControl.js');\n\ + const { ListView } = require('./ListView.js');\n\ + const items = { isNull: () => false, kind: 'ItemCollection' };\n\ + const missing = { isNull: () => true };\n\ + let casts = 0;\n\ + const interfaceValue = {\n\ + invoke(index) {\n\ + assert.equal(index, 6);\n\ + return items;\n\ + },\n\ + };\n\ + const raw = {\n\ + invoke(index) {\n\ + assert.equal(index, 6);\n\ + return missing;\n\ + },\n\ + cast() {\n\ + casts++;\n\ + return interfaceValue;\n\ + },\n\ + };\n\ + const list = Object.assign(Object.create(ListView.prototype), { _obj: raw });\n\ + const concreteDescriptor = Object.getOwnPropertyDescriptor(ListView.prototype, 'items');\n\ + const interfaceDescriptor = Object.getOwnPropertyDescriptor(IItemsControl.prototype, 'items');\n\ + assert.equal(concreteDescriptor.get, interfaceDescriptor.get);\n\ + assert.equal(list.items, items);\n\ + assert.equal(IItemsControl.from(raw).items, items);\n\ + assert.equal(casts, 2);\n", + ) + .unwrap(); + + let output = Command::new("node") + .arg("test.js") + .current_dir(&directory) + .output() + .unwrap(); + assert!( + output.status.success(), + "node failed:\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + ); + + fs::write(directory.join("IItemsControl.js"), unsafe_interface_js).unwrap(); + let fail_closed = Command::new("node") + .args([ + "-e", + "require('node:assert/strict').throws(() => require('./ListView.js'), /not a shared member source/)", + ]) + .current_dir(&directory) + .output() + .unwrap(); + let _ = fs::remove_dir_all(&directory); + assert!( + fail_closed.status.success(), + "fail-closed node check failed:\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&fail_closed.stdout), + String::from_utf8_lossy(&fail_closed.stderr), + ); +} + +#[test] +fn shared_interface_descriptor_executes_for_raw_and_concrete_views() { + if Command::new("node").arg("--version").output().is_err() { + eprintln!("Skipping shared-interface runtime test: node is unavailable"); + return; + } let interface = value_interface(); let class = widget_class(&interface); let known_types = HashSet::from(["Widget".into(), "IValue".into()]); - let shared_iids = HashSet::from([interface.iid.clone()]); + let shared_iids = shared_sources(&interface); project::set_import_name("./runtime.js"); project::set_shared_interface_members(true); diff --git a/tools/dynwinrt-codegen/tests/snapshot_test.rs b/tools/dynwinrt-codegen/tests/snapshot_test.rs index 1bdd1d50..b7fd28db 100644 --- a/tools/dynwinrt-codegen/tests/snapshot_test.rs +++ b/tools/dynwinrt-codegen/tests/snapshot_test.rs @@ -60,7 +60,7 @@ fn snapshot_uri_class() { .map(|i| i.name.clone()) .collect(); - let shared_iids: HashSet = HashSet::new(); + let shared_iids: HashSet = HashSet::new(); let (delegate_sigs, delegate_sig_refs, delegate_param_wraps) = project::build_delegate_signatures(&all_interfaces, &delegate_type_names, &known_types); @@ -370,7 +370,7 @@ fn ts_async_methods_emit_abort_signal_scaffolding() { }) .map(|i| i.name.clone()) .collect(); - let shared: HashSet = HashSet::new(); + let shared: HashSet = HashSet::new(); let (dw_delegate_sigs, dw_delegate_sig_refs, dw_delegate_param_wraps) = project::build_delegate_signatures(&dw_ifaces, &delegates, &known); From eb64555a24d78be7b2b5f0a6a3184f28399d53aa Mon Sep 17 00:00:00 2001 From: "Leilei Zhang (from Dev Box)" Date: Tue, 11 Aug 2026 22:29:36 +0800 Subject: [PATCH 3/8] Fallback ambiguous shared interfaces Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b485b75e-2292-44e1-98c8-3f7bb6489bff --- README.md | 6 +- tools/dynwinrt-codegen/npm/README.md | 5 +- .../src/codegen/winrt/javascript/ir.rs | 12 + .../codegen/winrt/javascript/project/mod.rs | 158 ++++++++-- .../winrt/javascript/render/javascript/mod.rs | 37 ++- tools/dynwinrt-codegen/src/main.rs | 208 ++++++++++++- tools/dynwinrt-codegen/src/meta.rs | 40 ++- .../tests/shared_interface_members_test.rs | 284 ++++++++++++++++-- 8 files changed, 684 insertions(+), 66 deletions(-) diff --git a/README.md b/README.md index 02ab250d..99d59b0b 100644 --- a/README.md +++ b/README.md @@ -195,9 +195,9 @@ conflicting members remain class-local, raw interface wrapper classes remain available, and the option does not change generated `.d.ts` files. Only required interfaces already canonicalized as standalone shared wrappers participate; one-off inline required interfaces remain class-local. Generation -without this flag is unchanged. If a shared source filename is ambiguous -between distinct interface identities, generation fails instead of choosing -an unsafe descriptor source. +without this flag is unchanged. If a standalone interface filename is +ambiguous between distinct interface identities, none of those identities +participate in sharing and their inherited members remain class-local. Focused validation: diff --git a/tools/dynwinrt-codegen/npm/README.md b/tools/dynwinrt-codegen/npm/README.md index b9d9caa1..5d377bda 100644 --- a/tools/dynwinrt-codegen/npm/README.md +++ b/tools/dynwinrt-codegen/npm/README.md @@ -75,8 +75,9 @@ members remain class-local, raw interface wrappers remain available, generated `.d.ts` files are unchanged, and generation without the flag is unchanged. Only interfaces already emitted as canonical standalone shared wrappers participate; one-off inline required interfaces remain class-local. Generation -rejects a shared source filename that is ambiguous between distinct interface -identities. +excludes every identity behind a standalone interface filename that is +ambiguous between distinct interface identities; those inherited members +remain class-local. ## What gets generated diff --git a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/ir.rs b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/ir.rs index ddb8a48a..7511e532 100644 --- a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/ir.rs +++ b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/ir.rs @@ -257,6 +257,9 @@ pub struct ProjectedClass { pub doc: Option, pub members: Vec, pub required_ifaces: Vec, + /// Standalone shared sources imported by this class, including interfaces + /// whose descriptors remain class-local after conflict filtering. + pub shared_interface_sources: Vec, /// Required-interface members whose implementation descriptors are copied /// from a shared standalone interface prototype. pub shared_interface_members: Vec, @@ -266,8 +269,15 @@ pub struct ProjectedClass { pub static_accessors: Vec, } +pub struct ProjectedSharedInterfaceSource { + pub interface_name: String, + pub interface_identity: String, +} + pub struct ProjectedSharedInterfaceMembers { pub interface_name: String, + /// Normalized metadata identity expected from the standalone source file. + pub interface_identity: String, /// Projection-level member keys used to suppress duplicate class bodies. pub member_keys: Vec, /// JavaScript property-key expressions copied from the interface prototype. @@ -282,6 +292,8 @@ pub struct ProjectedIface { pub has_parameterized_cast: bool, /// The interface prototype may be reused by concrete runtime classes. pub shared_member_source: bool, + /// Normalized metadata identity stored on a shared member source. + pub interface_identity: String, pub members: Vec, pub is_delegate: bool, } diff --git a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/project/mod.rs b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/project/mod.rs index 4bd605ac..0966df61 100644 --- a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/project/mod.rs +++ b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/project/mod.rs @@ -55,8 +55,8 @@ pub struct StandaloneInterfaceIdentity { } impl StandaloneInterfaceIdentity { - fn describe(&self) -> String { - format!("{}.{} ({})", self.namespace, self.name, self.iid) + pub fn source_marker(&self) -> String { + format!("{}.{}:{}", self.namespace, self.name, self.iid) } } @@ -78,15 +78,44 @@ pub struct CanonicalInterfaceSource { pub shared_member_source: bool, } +pub fn ambiguous_standalone_interface_names<'a>( + interfaces: impl IntoIterator, +) -> HashSet { + let mut identities_by_name: HashMap> = + HashMap::new(); + for iface in interfaces { + let Some(identity) = standalone_interface_identity(iface) else { + continue; + }; + identities_by_name + .entry(iface.name.clone()) + .or_default() + .insert(identity); + } + identities_by_name + .into_iter() + .filter_map(|(name, identities)| (identities.len() > 1).then_some(name)) + .collect() +} + pub fn canonical_interface_sources( interfaces: &[InterfaceMeta], shared_candidates: &[InterfaceMeta], class_names: &HashSet, + excluded_shared_source_names: &HashSet, + forced_shared_source_identities: &HashSet, ) -> Result, String> { - let shared_identities = shared_candidates + let mut shared_identities = shared_candidates .iter() .filter_map(standalone_interface_identity) .collect::>(); + shared_identities.extend(forced_shared_source_identities.iter().cloned()); + let ambiguous_names = ambiguous_standalone_interface_names( + shared_candidates + .iter() + .chain(interfaces) + .filter(|iface| !class_names.contains(&iface.name)), + ); let mut sources: Vec = Vec::new(); let mut source_by_name: HashMap = HashMap::new(); @@ -97,20 +126,20 @@ pub fn canonical_interface_sources( let Some(identity) = standalone_interface_identity(iface) else { continue; }; - let shared_member_source = shared_identities.contains(&identity); + let ambiguous = ambiguous_names.contains(&iface.name) + || excluded_shared_source_names.contains(&iface.name); + let shared_member_source = !ambiguous && shared_identities.contains(&identity); if let Some(existing_index) = source_by_name.get(&iface.name).copied() { let existing = &mut sources[existing_index]; + if ambiguous { + *existing = CanonicalInterfaceSource { + interface: iface.clone(), + identity, + shared_member_source: false, + }; + continue; + } if existing.identity != identity { - if existing.shared_member_source || shared_member_source { - return Err(format!( - "Cannot use `{0}.js` as a shared interface member source because `{1}` and \ - `{2}` have different interface identities. Generate them separately or \ - select only one type.", - iface.name, - existing.identity.describe(), - identity.describe(), - )); - } *existing = CanonicalInterfaceSource { interface: iface.clone(), identity, @@ -280,6 +309,28 @@ pub fn project_class( delegate_sigs: &HashMap, delegate_sig_refs: &HashMap>, delegate_param_wraps: &HashMap>, +) -> ProjectedFile { + project_class_with_excluded_interface_imports( + class, + known_types, + delegate_type_names, + shared_interface_sources, + &HashSet::new(), + delegate_sigs, + delegate_sig_refs, + delegate_param_wraps, + ) +} + +pub fn project_class_with_excluded_interface_imports( + class: &ClassMeta, + known_types: &HashSet, + delegate_type_names: &HashSet, + shared_interface_sources: &HashSet, + excluded_interface_import_names: &HashSet, + delegate_sigs: &HashMap, + delegate_sig_refs: &HashMap>, + delegate_param_wraps: &HashMap>, ) -> ProjectedFile { let used_structs = collect_used_structs_from_class(class); let winui_bootstrap = winui::resolve_application_bootstrap(class, known_types); @@ -361,7 +412,14 @@ pub fn project_class( // Type imports let mut imported_names: HashSet = HashSet::new(); + let mut imported_interface_names: HashSet = HashSet::new(); let type_imports = collect_type_imports(class); + let local_required_interface_names = class + .required_interfaces + .iter() + .filter(|iface| excluded_interface_import_names.contains(&iface.name)) + .map(|iface| iface.name.as_str()) + .collect::>(); let mut sorted_imports: Vec<_> = type_imports.iter().collect(); sorted_imports .sort_by(|a, b| (&a.namespace, &a.name, &a.kind).cmp(&(&b.namespace, &b.name, &b.kind))); @@ -374,11 +432,16 @@ pub fn project_class( if r.name == class.name { continue; } + if r.kind == TypeKind::Interface && local_required_interface_names.contains(r.name.as_str()) + { + continue; + } if known_types.contains(&r.name) && !all_delegate_names.contains(&r.name) { imports.push(format_type_import_projected(&r.name, r.kind)); imported_names.insert(r.name.clone()); if r.kind == TypeKind::Interface { imported_names.insert(format!("IID_{}", r.name)); + imported_interface_names.insert(r.name.clone()); } } else if all_delegate_names.contains(&r.name) && delegate_sigs.contains_key(&r.name) @@ -415,6 +478,7 @@ pub fn project_class( && !req_iface.iid.is_empty() && standalone_interface_identity(req_iface) .is_some_and(|identity| shared_interface_sources.contains(&identity)) + && !excluded_interface_import_names.contains(&req_iface.name) && !imported_names.contains(&req_iface.name) { imports.push(format_type_import_projected( @@ -423,6 +487,7 @@ pub fn project_class( )); imported_names.insert(req_iface.name.clone()); imported_names.insert(format!("IID_{}", req_iface.name)); + imported_interface_names.insert(req_iface.name.clone()); } } @@ -448,13 +513,17 @@ pub fn project_class( .required_interfaces .iter() .any(|ri| ri.iid == ICLOSABLE_IID); - if needs_iclosable && !imported_names.contains("IClosable") { + if needs_iclosable + && !excluded_interface_import_names.contains("IClosable") + && !imported_names.contains("IClosable") + { imports.push(format_type_import_projected( "IClosable", TypeKind::Interface, )); imported_names.insert("IClosable".into()); imported_names.insert("IID_IClosable".into()); + imported_interface_names.insert("IClosable".into()); } // IID consts(private, for class-internal use) @@ -978,7 +1047,8 @@ pub fn project_class( // Required interface inline wrappers let mut required_ifaces = Vec::new(); - let mut shared_member_candidates: Vec<(String, String, Vec)> = Vec::new(); + let mut verified_shared_interface_sources = Vec::new(); + let mut shared_member_candidates: Vec<(String, String, String, Vec)> = Vec::new(); let mut conflicting_shared_members = HashSet::new(); let share_interface_members = shared_interface_members_enabled(); // Track names already on the main class to avoid conflicts @@ -999,12 +1069,28 @@ pub fn project_class( if req_iface.iid.is_empty() { continue; } - let is_imported = imported_names.contains(&req_iface.name); + let is_imported = imported_interface_names.contains(&req_iface.name); let is_shared_member_source = share_interface_members && req_iface.generic_piid.is_none() && standalone_interface_identity(req_iface) .is_some_and(|identity| shared_interface_sources.contains(&identity)) && is_imported; + if is_shared_member_source { + let interface_identity = standalone_interface_identity(req_iface) + .expect("shared interface source must have an identity") + .source_marker(); + if !verified_shared_interface_sources.iter().any( + |source: &ProjectedSharedInterfaceSource| { + source.interface_name == req_iface.name + && source.interface_identity == interface_identity + }, + ) { + verified_shared_interface_sources.push(ProjectedSharedInterfaceSource { + interface_name: req_iface.name.clone(), + interface_identity, + }); + } + } let reg_var = format!("_{}", req_iface.name); let mut ri_members = Vec::new(); @@ -1147,7 +1233,9 @@ pub fn project_class( let mut shared_interface_members: Vec = Vec::new(); if share_interface_members { - for (member_key, interface_name, descriptor_keys) in shared_member_candidates { + for (member_key, interface_name, interface_identity, descriptor_keys) in + shared_member_candidates + { if conflicting_shared_members.contains(&member_key) || descriptor_keys.iter().any(|descriptor_key| { final_descriptor_counts @@ -1157,9 +1245,10 @@ pub fn project_class( { continue; } - let group = shared_interface_members - .iter_mut() - .find(|group| group.interface_name == interface_name); + let group = shared_interface_members.iter_mut().find(|group| { + group.interface_name == interface_name + && group.interface_identity == interface_identity + }); if let Some(group) = group { if !group.member_keys.contains(&member_key) { group.member_keys.push(member_key); @@ -1172,6 +1261,7 @@ pub fn project_class( } else { shared_interface_members.push(ProjectedSharedInterfaceMembers { interface_name, + interface_identity, member_keys: vec![member_key], descriptor_keys, }); @@ -1195,6 +1285,7 @@ pub fn project_class( doc, members, required_ifaces, + shared_interface_sources: verified_shared_interface_sources, shared_interface_members, static_cache_fields, static_accessors, @@ -1465,6 +1556,9 @@ pub fn project_interface_with_shared_member_source( has_static_from: !iface.iid.is_empty(), has_parameterized_cast, shared_member_source, + interface_identity: standalone_interface_identity(iface) + .map(|identity| identity.source_marker()) + .unwrap_or_default(), members, is_delegate: false, }], @@ -1608,7 +1702,7 @@ pub fn project_delegate( // ====================================================================== fn record_shared_member_candidate( - candidates: &mut Vec<(String, String, Vec)>, + candidates: &mut Vec<(String, String, String, Vec)>, conflicts: &mut HashSet, interface: &InterfaceMeta, member: &ProjectedMember, @@ -1630,11 +1724,18 @@ fn record_shared_member_candidate( if descriptor_keys.is_empty() { return; } - if let Some((_, existing_interface, existing_descriptors)) = candidates + let Some(interface_identity) = standalone_interface_identity(interface) else { + return; + }; + let interface_identity = interface_identity.source_marker(); + if let Some((_, existing_interface, existing_identity, existing_descriptors)) = candidates .iter_mut() - .find(|(existing_key, _, _)| existing_key == &member_key) + .find(|(existing_key, _, _, _)| existing_key == &member_key) { - if existing_interface != &interface.name || matches!(member, ProjectedMember::Method(_)) { + if existing_interface != &interface.name + || existing_identity != &interface_identity + || matches!(member, ProjectedMember::Method(_)) + { conflicts.insert(member_key); } else { for descriptor_key in descriptor_keys { @@ -1645,7 +1746,12 @@ fn record_shared_member_candidate( } return; } - candidates.push((member_key, interface.name.clone(), descriptor_keys)); + candidates.push(( + member_key, + interface.name.clone(), + interface_identity, + descriptor_keys, + )); } fn shared_member_key(member: &ProjectedMember) -> Option { diff --git a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/render/javascript/mod.rs b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/render/javascript/mod.rs index 5387d093..d8a02f35 100644 --- a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/render/javascript/mod.rs +++ b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/render/javascript/mod.rs @@ -117,16 +117,27 @@ fn render_esm(file: &ProjectedFile) -> String { .classes .iter() .any(|class| !class.shared_interface_members.is_empty()); + let verifies_shared_interface_sources = file + .classes + .iter() + .any(|class| !class.shared_interface_sources.is_empty()); let is_shared_interface_source = file.ifaces.iter().any(|iface| iface.shared_member_source); - if copies_shared_interface_members || is_shared_interface_source { + if verifies_shared_interface_sources || is_shared_interface_source { out.push_str( "const __sharedInterfaceMemberSource = Symbol.for('dynwinrt.sharedInterfaceMemberSource');\n\n", ); } + if verifies_shared_interface_sources { + out.push_str( + "const __verifyInterfaceSource = (source, identity) => {\n\ + if (source[__sharedInterfaceMemberSource] !== identity) throw new Error(`Interface ${source.name} is not a shared member source for ${identity}`);\n\ +};\n\n", + ); + } if copies_shared_interface_members { out.push_str( - "const __copyInterfaceMembers = (target, source, keys) => {\n\ - if (source[__sharedInterfaceMemberSource] !== true) throw new Error(`Interface ${source.name} is not a shared member source`);\n\ + "const __copyInterfaceMembers = (target, source, identity, keys) => {\n\ + __verifyInterfaceSource(source, identity);\n\ for (const key of keys) {\n\ const descriptor = Object.getOwnPropertyDescriptor(source.prototype, key);\n\ if (descriptor === undefined) throw new Error(`Missing shared interface member ${String(key)}`);\n\ @@ -335,11 +346,25 @@ fn render_class_js(out: &mut String, class: &ProjectedClass) { render_member_js(out, member, &class.name); } out.push_str("}\n"); + for source in &class.shared_interface_sources { + let copied = class.shared_interface_members.iter().any(|shared| { + shared.interface_name == source.interface_name + && shared.interface_identity == source.interface_identity + }); + if !copied { + out.push_str(&format!( + "__verifyInterfaceSource({}, '{}');\n", + ref_marker(&source.interface_name), + source.interface_identity, + )); + } + } for shared in &class.shared_interface_members { out.push_str(&format!( - "__copyInterfaceMembers({}, {}, [{}]);\n", + "__copyInterfaceMembers({}, {}, '{}', [{}]);\n", class.name, ref_marker(&shared.interface_name), + shared.interface_identity, shared.descriptor_keys.join(", "), )); } @@ -396,8 +421,8 @@ fn render_iface_js(out: &mut String, iface: &ProjectedIface, _file: &ProjectedFi out.push_str("}\n"); if iface.shared_member_source { out.push_str(&format!( - "Object.defineProperty({}, __sharedInterfaceMemberSource, {{ value: true }});\n", - iface.name + "Object.defineProperty({}, __sharedInterfaceMemberSource, {{ value: '{}' }});\n", + iface.name, iface.interface_identity, )); } } diff --git a/tools/dynwinrt-codegen/src/main.rs b/tools/dynwinrt-codegen/src/main.rs index b928f714..512b1e1b 100644 --- a/tools/dynwinrt-codegen/src/main.rs +++ b/tools/dynwinrt-codegen/src/main.rs @@ -381,6 +381,12 @@ fn run() -> Result<(), String> { project::set_import_name(&import_name); project::set_shared_interface_members(shared_interface_members); } + let (metadata_excluded_shared_source_names, metadata_shared_source_identities) = + if lang == "js" && shared_interface_members { + loaded_metadata_interface_plan(&winmd) + } else { + (HashSet::new(), HashSet::new()) + }; if !dry_run { fs::create_dir_all(output_dir).map_err(|e| { format!("Failed to create output directory '{}': {}", output, e) @@ -591,6 +597,26 @@ fn run() -> Result<(), String> { winui::add_implicit_classes(&winmd, &mut classes); let mut implicit_interfaces = Vec::new(); winui::add_implicit_interfaces(&winmd, &classes, &mut implicit_interfaces); + let ( + mut excluded_shared_source_names, + mut shared_interface_source_identities, + reserved_non_interface_output_names, + ) = if lang == "js" && project::shared_interface_members_enabled() { + shared_interface_plan_for_batches( + &winmd, + &[(classes.clone(), implicit_interfaces.clone(), Vec::new())], + ) + } else { + (HashSet::new(), HashSet::new(), HashSet::new()) + }; + excluded_shared_source_names + .extend(metadata_excluded_shared_source_names.iter().cloned()); + shared_interface_source_identities.extend( + metadata_shared_source_identities + .iter() + .filter(|identity| !excluded_shared_source_names.contains(&identity.name)) + .cloned(), + ); generate_for_types( &winmd, output_dir, @@ -601,6 +627,9 @@ fn run() -> Result<(), String> { &lang, pyi, &doc_table, + &excluded_shared_source_names, + &shared_interface_source_identities, + &reserved_non_interface_output_names, )?; // Write (or append to) the index file for the output directory @@ -649,6 +678,8 @@ fn run() -> Result<(), String> { )) } else { !class_names.contains(&interface.name) + && !reserved_non_interface_output_names + .contains(&interface.name) } }); all_enums.retain(|e| match e { @@ -717,10 +748,7 @@ fn run() -> Result<(), String> { } }; - let mut total_classes = 0usize; - let mut total_interfaces = 0usize; - let mut total_enums = 0usize; - + let mut namespace_batches = Vec::new(); for ns in &namespaces { if let Some(interface) = com_metadata::first_classic_com_interface_in_namespace(&winmd, ns) @@ -734,9 +762,34 @@ fn run() -> Result<(), String> { } let mut classes = meta::parse_namespace(&winmd, ns); let mut interfaces = meta::parse_interfaces(&winmd, ns); - let mut enums = meta::parse_enums(&winmd, ns); + let enums = meta::parse_enums(&winmd, ns); winui::add_implicit_classes(&winmd, &mut classes); winui::add_implicit_interfaces(&winmd, &classes, &mut interfaces); + namespace_batches.push((classes, interfaces, enums)); + } + let ( + mut excluded_shared_source_names, + mut shared_interface_source_identities, + reserved_non_interface_output_names, + ) = if lang == "js" && project::shared_interface_members_enabled() { + shared_interface_plan_for_batches(&winmd, &namespace_batches) + } else { + (HashSet::new(), HashSet::new(), HashSet::new()) + }; + excluded_shared_source_names + .extend(metadata_excluded_shared_source_names.iter().cloned()); + shared_interface_source_identities.extend( + metadata_shared_source_identities + .iter() + .filter(|identity| !excluded_shared_source_names.contains(&identity.name)) + .cloned(), + ); + + let mut total_classes = 0usize; + let mut total_interfaces = 0usize; + let mut total_enums = 0usize; + + for (mut classes, mut interfaces, mut enums) in namespace_batches { for c in classes.iter_mut() { doc_table.apply_to_class(c); } @@ -748,8 +801,18 @@ fn run() -> Result<(), String> { } let (nc, ni, ne) = generate_for_types( - &winmd, output_dir, classes, interfaces, enums, dry_run, &lang, pyi, + &winmd, + output_dir, + classes, + interfaces, + enums, + dry_run, + &lang, + pyi, &doc_table, + &excluded_shared_source_names, + &shared_interface_source_identities, + &reserved_non_interface_output_names, )?; total_classes += nc; total_interfaces += ni; @@ -804,6 +867,8 @@ fn run() -> Result<(), String> { )) } else { !class_names.contains(&interface.name) + && !reserved_non_interface_output_names + .contains(&interface.name) } }); all_enums.retain(|e| match e { @@ -876,6 +941,9 @@ fn generate_for_types( lang: &str, pyi: bool, doc_table: &DocTable, + excluded_shared_source_names: &HashSet, + forced_shared_source_identities: &HashSet, + reserved_non_interface_output_names: &HashSet, ) -> Result<(usize, usize, usize), String> { let deps = meta::resolve_dependencies(winmd, &classes, &interfaces, &enums); let mut all_classes = classes; @@ -1024,11 +1092,22 @@ fn generate_for_types( for iface in &canonical_shared_interfaces { known_types.insert(iface.name.clone()); } + let mut effective_excluded_shared_source_names = excluded_shared_source_names.clone(); + for en in &all_enums { + if let TypeMeta::Enum { name, .. } = en + && !name.contains('<') + && !class_names_all.contains(name) + { + effective_excluded_shared_source_names.insert(name.clone()); + } + } let canonical_interface_sources = if shared_interface_members_enabled { Some(project::canonical_interface_sources( &all_interfaces, &canonical_shared_interfaces, &class_names_all, + &effective_excluded_shared_source_names, + forced_shared_source_identities, )?) } else { None @@ -1064,17 +1143,120 @@ fn generate_for_types( &delegate_type_names, canonical_interface_sources.as_deref(), &shared_interface_source_identities, + &effective_excluded_shared_source_names, + reserved_non_interface_output_names, &delegate_signatures, &delegate_sig_refs, &delegate_param_wraps, )?; } + drop(python_layout); } Ok((all_classes.len(), all_interfaces.len(), all_enums.len())) } +fn shared_interface_plan_for_batches( + winmd: &str, + batches: &[( + Vec, + Vec, + Vec, + )], +) -> ( + HashSet, + HashSet, + HashSet, +) { + let mut standalone_sources = Vec::new(); + let mut shared_source_identities = HashSet::new(); + let mut non_interface_writer_names = HashSet::new(); + for (classes, interfaces, enums) in batches { + let deps = meta::resolve_dependencies(winmd, classes, interfaces, enums); + let all_classes = classes.iter().chain(&deps.classes).collect::>(); + let class_names = all_classes + .iter() + .map(|class| class.name.clone()) + .collect::>(); + non_interface_writer_names.extend(class_names.iter().cloned()); + non_interface_writer_names.extend(enums.iter().chain(&deps.enums).filter_map( + |en| match en { + TypeMeta::Enum { name, .. } + if !name.contains('<') && !class_names.contains(name) => + { + Some(name.clone()) + } + _ => None, + }, + )); + standalone_sources.extend( + interfaces + .iter() + .chain(&deps.interfaces) + .filter(|iface| !class_names.contains(&iface.name)) + .cloned(), + ); + + let mut required_interface_count: HashMap< + project::StandaloneInterfaceIdentity, + (&meta::InterfaceMeta, usize), + > = HashMap::new(); + for class in all_classes { + for req_iface in &class.required_interfaces { + let Some(identity) = project::standalone_interface_identity(req_iface) else { + continue; + }; + required_interface_count + .entry(identity) + .and_modify(|(_, count)| *count += 1) + .or_insert((req_iface, 1)); + } + } + standalone_sources.extend( + required_interface_count + .into_values() + .filter(|(iface, count)| *count >= 2 && !class_names.contains(&iface.name)) + .map(|(iface, _)| { + shared_source_identities.insert( + project::standalone_interface_identity(iface) + .expect("counted required interface must have an identity"), + ); + iface.clone() + }), + ); + } + + let mut excluded_names = project::ambiguous_standalone_interface_names(&standalone_sources); + excluded_names.extend(non_interface_writer_names.iter().cloned()); + shared_source_identities.retain(|identity| !excluded_names.contains(&identity.name)); + ( + excluded_names, + shared_source_identities, + non_interface_writer_names, + ) +} + +fn loaded_metadata_interface_plan( + winmd: &str, +) -> ( + HashSet, + HashSet, +) { + let interfaces = meta::parse_all_interfaces_including_exclusive(winmd); + let excluded_names = project::ambiguous_standalone_interface_names(&interfaces); + let shared_source_identities = interfaces + .iter() + .filter(|iface| { + iface.generic_piid.is_none() + && !iface.methods.iter().any(|method| method.name == ".ctor") + && !excluded_names.contains(&iface.name) + }) + .filter_map(project::standalone_interface_identity) + .collect(); + (excluded_names, shared_source_identities) +} + fn python_type_identities( classes: &[meta::ClassMeta], interfaces: &[meta::InterfaceMeta], @@ -1138,6 +1320,8 @@ fn generate_js_files( delegate_type_names: &HashSet, canonical_interface_sources: Option<&[project::CanonicalInterfaceSource]>, shared_interface_source_identities: &HashSet, + excluded_interface_import_names: &HashSet, + reserved_non_interface_output_names: &HashSet, delegate_sigs: &HashMap, delegate_sig_refs: &HashMap>, delegate_param_wraps: &HashMap>, @@ -1170,6 +1354,9 @@ fn generate_js_files( if let Some(canonical_sources) = canonical_interface_sources { for source in canonical_sources { let iface = &source.interface; + if reserved_non_interface_output_names.contains(&iface.name) { + continue; + } let projected = project::project_interface_with_shared_member_source( iface, known_types, @@ -1188,6 +1375,9 @@ fn generate_js_files( if class_names.contains(iface.name.as_str()) { continue; } + if reserved_non_interface_output_names.contains(&iface.name) { + continue; + } if !is_emittable_interface(iface) { continue; } @@ -1207,6 +1397,9 @@ fn generate_js_files( if class_names.contains(iface.name.as_str()) { continue; } + if reserved_non_interface_output_names.contains(&iface.name) { + continue; + } if !is_emittable_interface(iface) { continue; } @@ -1264,11 +1457,12 @@ fn generate_js_files( if !class_is_usable(class) { continue; } - let projected = project::project_class( + let projected = project::project_class_with_excluded_interface_imports( class, known_types, delegate_type_names, shared_interface_source_identities, + excluded_interface_import_names, delegate_sigs, delegate_sig_refs, delegate_param_wraps, diff --git a/tools/dynwinrt-codegen/src/meta.rs b/tools/dynwinrt-codegen/src/meta.rs index a05cc373..8b1b1132 100644 --- a/tools/dynwinrt-codegen/src/meta.rs +++ b/tools/dynwinrt-codegen/src/meta.rs @@ -259,14 +259,48 @@ pub fn parse_namespace(winmd_paths: &str, namespace: &str) -> Vec { /// Exclusive interfaces (prefixed with I and paired with a RuntimeClass) are skipped /// since they are implementation details. We only generate public-facing interfaces. pub fn parse_interfaces(winmd_paths: &str, namespace: &str) -> Vec { + parse_interfaces_with_exclusive(winmd_paths, namespace, false) +} + +/// Parse every non-generic interface in a namespace, including interfaces +/// marked ExclusiveTo for generation-wide identity planning. +pub fn parse_interfaces_including_exclusive( + winmd_paths: &str, + namespace: &str, +) -> Vec { + parse_interfaces_with_exclusive(winmd_paths, namespace, true) +} + +/// Parse every non-generic interface across the loaded metadata in one pass, +/// including interfaces marked ExclusiveTo. +pub fn parse_all_interfaces_including_exclusive(winmd_paths: &str) -> Vec { let index = match load_index(winmd_paths) { Some(idx) => idx, None => return Vec::new(), }; + parse_interfaces_from_index(&index, None, true) +} +fn parse_interfaces_with_exclusive( + winmd_paths: &str, + namespace: &str, + include_exclusive: bool, +) -> Vec { + let index = match load_index(winmd_paths) { + Some(idx) => idx, + None => return Vec::new(), + }; + parse_interfaces_from_index(&index, Some(namespace), include_exclusive) +} + +fn parse_interfaces_from_index( + index: &reader::Index, + namespace: Option<&str>, + include_exclusive: bool, +) -> Vec { let mut interfaces = Vec::new(); for def in index.all() { - if def.namespace() != namespace { + if namespace.is_some_and(|namespace| def.namespace() != namespace) { continue; } // Skip CLR projection types @@ -287,10 +321,10 @@ pub fn parse_interfaces(winmd_paths: &str, namespace: &str) -> Vec>(); + assert!(shared_source_identities.is_empty()); + + let interface_file = project::project_interface_with_shared_member_source( + &sources[0].interface, + &known_types, + &HashSet::new(), + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + sources[0].shared_member_source, + ); + assert!(!render_js::render(&interface_file).contains("__interfaceValue")); + + let class_file = project::project_class_with_excluded_interface_imports( + class, + &known_types, + &HashSet::new(), + &shared_source_identities, + &ambiguous_names, + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + ); + assert_eq!(class_file.classes[0].required_ifaces.len(), 1); + assert!( + class_file + .iid_consts + .iter() + .any(|iid| { iid.name == "IID_IValue" && iid.rhs_expr.contains(&interface.iid) }) + ); + let class_js = render_js::render(&class_file); + assert!(!class_js.contains("__copyInterfaceMembers")); + assert!(!class_js.contains("require(\"./IValue.js\")"), "{class_js}"); + assert!( + class_js.contains("_IValue.method(6).invoke(this._obj.cast(IID_IValue)"), + "{class_js}" + ); + } + project::set_shared_interface_members(false); } #[test] @@ -202,9 +367,14 @@ fn nonshared_duplicate_interface_keeps_legacy_final_source() { second.namespace = "Fabrikam".into(); second.iid = "88888888-8888-8888-8888-888888888888".into(); - let sources = - project::canonical_interface_sources(&[first, second.clone()], &[], &HashSet::new()) - .unwrap(); + let sources = project::canonical_interface_sources( + &[first, second.clone()], + &[], + &HashSet::new(), + &HashSet::new(), + &HashSet::new(), + ) + .unwrap(); assert_eq!(sources.len(), 1); assert!(!sources[0].shared_member_source); assert_eq!( @@ -285,6 +455,9 @@ fn shared_interface_members_preserve_overload_dispatch_and_declarations() { "{interface_js}" ); assert!(!class_js.contains("__copyInterfaceMembers")); + assert!(class_js.contains( + "__verifyInterfaceSource((__get_IOverloaded()), 'Contoso.IOverloaded:22222222-2222-2222-2222-222222222222');" + )); assert!(class_js.contains("_doThing_1(value)"), "{class_js}"); assert!(class_js.contains("_doThing_2(value, other)"), "{class_js}"); assert!(class_js.contains("doThing(...args)"), "{class_js}"); @@ -357,6 +530,9 @@ fn shared_interface_members_preserve_cross_interface_overload_dispatch() { let class_dts = render_dts::render(&class_file); assert!(!class_js.contains("__copyInterfaceMembers")); + assert!(class_js.contains( + "__verifyInterfaceSource((__get_IRequired()), 'Contoso.IRequired:44444444-4444-4444-4444-444444444444');" + )); assert!(class_js.contains("_doThing_1(value)"), "{class_js}"); assert!(class_js.contains("_doThing_2(value, other)"), "{class_js}"); assert!(class_js.contains("doThing(...args)"), "{class_js}"); @@ -474,7 +650,48 @@ fn noncanonical_required_interfaces_remain_class_local() { } #[test] -fn collection_getter_casts_concrete_view_and_rejects_unmarked_source() { +fn excluded_iclosable_uses_local_wrapper() { + let interface = InterfaceMeta { + name: "IClosable".into(), + namespace: "Windows.Foundation".into(), + iid: "30d5a829-7fa4-4026-83bb-d75bae4ea99e".into(), + methods: vec![MethodMeta { + name: "Close".into(), + raw_name: "Close".into(), + vtable_index: 6, + ..Default::default() + }], + ..Default::default() + }; + let class = widget_class(&interface); + + let class_file = project::project_class_with_excluded_interface_imports( + &class, + &HashSet::from(["Widget".into(), "IClosable".into()]), + &HashSet::new(), + &HashSet::new(), + &HashSet::from(["IClosable".into()]), + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + ); + assert_eq!(class_file.classes[0].required_ifaces.len(), 1); + assert!( + class_file + .iid_consts + .iter() + .any(|iid| iid.name == "IID_IClosable" && iid.rhs_expr.contains(&interface.iid)) + ); + let class_js = render_js::render(&class_file); + assert!( + !class_js.contains("require(\"./IClosable.js\")"), + "{class_js}" + ); + assert!(class_js.contains("close()"), "{class_js}"); +} + +#[test] +fn collection_getter_casts_concrete_view_and_rejects_invalid_sources() { if Command::new("node").arg("--version").output().is_err() { eprintln!("Skipping shared-interface runtime test: node is unavailable"); return; @@ -512,6 +729,19 @@ fn collection_getter_casts_concrete_view_and_rejects_unmarked_source() { &HashMap::new(), false, )); + let mut mismatched_interface = interface.clone(); + mismatched_interface.namespace = "Fabrikam.Controls".into(); + mismatched_interface.iid = "99999999-9999-9999-9999-999999999999".into(); + let mismatched_interface_js = + render_js::render(&project::project_interface_with_shared_member_source( + &mismatched_interface, + &known_types, + &HashSet::new(), + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + true, + )); let class_js = render_js::render(&project::project_class( &class, &known_types, @@ -618,13 +848,29 @@ fn collection_getter_casts_concrete_view_and_rejects_unmarked_source() { .current_dir(&directory) .output() .unwrap(); - let _ = fs::remove_dir_all(&directory); assert!( fail_closed.status.success(), - "fail-closed node check failed:\nstdout:\n{}\nstderr:\n{}", + "unmarked fail-closed node check failed:\nstdout:\n{}\nstderr:\n{}", String::from_utf8_lossy(&fail_closed.stdout), String::from_utf8_lossy(&fail_closed.stderr), ); + + fs::write(directory.join("IItemsControl.js"), mismatched_interface_js).unwrap(); + let mismatched_fail_closed = Command::new("node") + .args([ + "-e", + "require('node:assert/strict').throws(() => require('./ListView.js'), /not a shared member source/)", + ]) + .current_dir(&directory) + .output() + .unwrap(); + let _ = fs::remove_dir_all(&directory); + assert!( + mismatched_fail_closed.status.success(), + "mismatched fail-closed node check failed:\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&mismatched_fail_closed.stdout), + String::from_utf8_lossy(&mismatched_fail_closed.stderr), + ); } #[test] From cb53ef3237e0ec229b9f607ffcafca0423caf58b Mon Sep 17 00:00:00 2001 From: "Leilei Zhang (from Dev Box)" Date: Tue, 11 Aug 2026 23:36:11 +0800 Subject: [PATCH 4/8] Preserve shared interface compatibility Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b485b75e-2292-44e1-98c8-3f7bb6489bff --- .../codegen/winrt/javascript/project/mod.rs | 45 ++++- tools/dynwinrt-codegen/src/main.rs | 183 +++++++++++++++++- .../tests/consistency_test.rs | 21 +- .../tests/shared_interface_members_test.rs | 106 +++++++++- tools/dynwinrt-codegen/tests/snapshot_test.rs | 19 +- 5 files changed, 352 insertions(+), 22 deletions(-) diff --git a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/project/mod.rs b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/project/mod.rs index 0966df61..ba05784a 100644 --- a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/project/mod.rs +++ b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/project/mod.rs @@ -305,7 +305,31 @@ pub fn project_class( class: &ClassMeta, known_types: &HashSet, delegate_type_names: &HashSet, - shared_interface_sources: &HashSet, + standalone_interface_iids: &HashSet, + delegate_sigs: &HashMap, + delegate_sig_refs: &HashMap>, + delegate_param_wraps: &HashMap>, +) -> ProjectedFile { + project_class_with_shared_member_sources( + class, + known_types, + delegate_type_names, + standalone_interface_iids, + &HashSet::new(), + delegate_sigs, + delegate_sig_refs, + delegate_param_wraps, + ) +} + +/// Project a class with explicit opt-in shared descriptor source identities in +/// addition to the legacy standalone-interface IID set. +pub fn project_class_with_shared_member_sources( + class: &ClassMeta, + known_types: &HashSet, + delegate_type_names: &HashSet, + standalone_interface_iids: &HashSet, + shared_member_source_identities: &HashSet, delegate_sigs: &HashMap, delegate_sig_refs: &HashMap>, delegate_param_wraps: &HashMap>, @@ -314,7 +338,8 @@ pub fn project_class( class, known_types, delegate_type_names, - shared_interface_sources, + standalone_interface_iids, + shared_member_source_identities, &HashSet::new(), delegate_sigs, delegate_sig_refs, @@ -322,11 +347,14 @@ pub fn project_class( ) } +/// Project a class while keeping legacy standalone-interface imports separate +/// from opt-in shared descriptor source identities. pub fn project_class_with_excluded_interface_imports( class: &ClassMeta, known_types: &HashSet, delegate_type_names: &HashSet, - shared_interface_sources: &HashSet, + standalone_interface_iids: &HashSet, + shared_member_source_identities: &HashSet, excluded_interface_import_names: &HashSet, delegate_sigs: &HashMap, delegate_sig_refs: &HashMap>, @@ -336,6 +364,7 @@ pub fn project_class_with_excluded_interface_imports( let winui_bootstrap = winui::resolve_application_bootstrap(class, known_types); let supports_unpackaged_xaml = winui_bootstrap.is_some_and(|bootstrap| bootstrap.supports_unpackaged_resources); + let share_interface_members = shared_interface_members_enabled(); // Collect delegate names only from interfaces of THIS class (not the entire batch) // for delegate imports; but also include global delegate_type_names for type filtering @@ -476,8 +505,11 @@ pub fn project_class_with_excluded_interface_imports( for req_iface in &class.required_interfaces { if req_iface.generic_piid.is_none() && !req_iface.iid.is_empty() - && standalone_interface_identity(req_iface) - .is_some_and(|identity| shared_interface_sources.contains(&identity)) + && (standalone_interface_iids.contains(&req_iface.iid) + || (share_interface_members + && standalone_interface_identity(req_iface).is_some_and(|identity| { + shared_member_source_identities.contains(&identity) + }))) && !excluded_interface_import_names.contains(&req_iface.name) && !imported_names.contains(&req_iface.name) { @@ -1050,7 +1082,6 @@ pub fn project_class_with_excluded_interface_imports( let mut verified_shared_interface_sources = Vec::new(); let mut shared_member_candidates: Vec<(String, String, String, Vec)> = Vec::new(); let mut conflicting_shared_members = HashSet::new(); - let share_interface_members = shared_interface_members_enabled(); // Track names already on the main class to avoid conflicts let mut main_member_names: HashSet = members .iter() @@ -1073,7 +1104,7 @@ pub fn project_class_with_excluded_interface_imports( let is_shared_member_source = share_interface_members && req_iface.generic_piid.is_none() && standalone_interface_identity(req_iface) - .is_some_and(|identity| shared_interface_sources.contains(&identity)) + .is_some_and(|identity| shared_member_source_identities.contains(&identity)) && is_imported; if is_shared_member_source { let interface_identity = standalone_interface_identity(req_iface) diff --git a/tools/dynwinrt-codegen/src/main.rs b/tools/dynwinrt-codegen/src/main.rs index 512b1e1b..1e5c2616 100644 --- a/tools/dynwinrt-codegen/src/main.rs +++ b/tools/dynwinrt-codegen/src/main.rs @@ -1142,6 +1142,7 @@ fn generate_for_types( &known_types, &delegate_type_names, canonical_interface_sources.as_deref(), + &shared_iids, &shared_interface_source_identities, &effective_excluded_shared_source_names, reserved_non_interface_output_names, @@ -1310,6 +1311,54 @@ fn validate_unique_class_output_names(classes: &[meta::ClassMeta]) -> Result<(), Ok(()) } +fn generated_shared_interface_source_identity(contents: &str) -> Option<&str> { + const PREFIX: &str = ", __sharedInterfaceMemberSource, { value: '"; + const SUFFIX: &str = "' });"; + let start = contents.find(PREFIX)? + PREFIX.len(); + let end = contents[start..].find(SUFFIX)? + start; + Some(&contents[start..end]) +} + +fn plan_existing_shared_interface_sources( + output_dir: &Path, + sources: &[project::CanonicalInterfaceSource], + reserved_non_interface_output_names: &HashSet, +) -> Result, String> { + let mut preserved_source_names = HashSet::new(); + for source in sources { + let interface = &source.interface; + if reserved_non_interface_output_names.contains(&interface.name) { + continue; + } + let path = output_dir.join(format!("{}.js", interface.name)); + let existing = match fs::read_to_string(&path) { + Ok(existing) => existing, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => continue, + Err(error) => { + return Err(format!("Failed to read {}: {}", path.display(), error)); + } + }; + let Some(existing_identity) = generated_shared_interface_source_identity(&existing) else { + continue; + }; + let requested_identity = source.identity.source_marker(); + if source.shared_member_source && existing_identity != requested_identity { + return Err(format!( + "Refusing to overwrite shared interface source `{}`: existing generated identity \ + `{}` does not match requested identity `{}`. Use a separate output directory or \ + remove and regenerate the conflicting file.", + path.display(), + existing_identity, + requested_identity, + )); + } + if !source.shared_member_source { + preserved_source_names.insert(interface.name.clone()); + } + } + Ok(preserved_source_names) +} + fn generate_js_files( output_dir: &Path, all_classes: &[meta::ClassMeta], @@ -1319,7 +1368,8 @@ fn generate_js_files( known_types: &HashSet, delegate_type_names: &HashSet, canonical_interface_sources: Option<&[project::CanonicalInterfaceSource]>, - shared_interface_source_identities: &HashSet, + standalone_interface_iids: &HashSet, + shared_member_source_identities: &HashSet, excluded_interface_import_names: &HashSet, reserved_non_interface_output_names: &HashSet, delegate_sigs: &HashMap, @@ -1351,12 +1401,26 @@ fn generate_js_files( !iface.iid.is_empty() } + let preserved_shared_source_names = canonical_interface_sources + .map(|canonical_sources| { + plan_existing_shared_interface_sources( + output_dir, + canonical_sources, + reserved_non_interface_output_names, + ) + }) + .transpose()? + .unwrap_or_default(); + if let Some(canonical_sources) = canonical_interface_sources { for source in canonical_sources { let iface = &source.interface; if reserved_non_interface_output_names.contains(&iface.name) { continue; } + if preserved_shared_source_names.contains(&iface.name) { + continue; + } let projected = project::project_interface_with_shared_member_source( iface, known_types, @@ -1461,7 +1525,8 @@ fn generate_js_files( class, known_types, delegate_type_names, - shared_interface_source_identities, + standalone_interface_iids, + shared_member_source_identities, excluded_interface_import_names, delegate_sigs, delegate_sig_refs, @@ -3619,6 +3684,56 @@ mod tests { )) } + fn shared_interface_source( + namespace: &str, + name: &str, + iid: &str, + ) -> project::CanonicalInterfaceSource { + let interface = meta::InterfaceMeta { + namespace: namespace.into(), + name: name.into(), + iid: iid.into(), + ..Default::default() + }; + project::CanonicalInterfaceSource { + identity: project::standalone_interface_identity(&interface).unwrap(), + interface, + shared_member_source: true, + } + } + + fn generate_shared_interface_sources( + output: &Path, + sources: &[project::CanonicalInterfaceSource], + ) -> Result<(), String> { + let known_types = sources + .iter() + .map(|source| source.interface.name.clone()) + .collect(); + let shared_source_identities = sources + .iter() + .filter(|source| source.shared_member_source) + .map(|source| source.identity.clone()) + .collect(); + generate_js_files( + output, + &[], + &[], + &[], + &[], + &known_types, + &HashSet::new(), + Some(sources), + &HashSet::new(), + &shared_source_identities, + &HashSet::new(), + &HashSet::new(), + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + ) + } + #[test] fn com_barrel_deduplicates_only_identical_pod_factories() { let descriptor = @@ -3724,6 +3839,70 @@ mod tests { .expect("identical metadata does not create an ambiguous output"); } + #[test] + fn incremental_shared_interface_same_identity_is_idempotent() { + let output = test_directory("shared-interface-same-identity"); + fs::create_dir_all(&output).unwrap(); + let source = + shared_interface_source("Contoso", "IValue", "11111111-1111-1111-1111-111111111111"); + + generate_shared_interface_sources(&output, std::slice::from_ref(&source)).unwrap(); + let first_js = fs::read(output.join("IValue.js")).unwrap(); + let first_dts = fs::read(output.join("IValue.d.ts")).unwrap(); + + generate_shared_interface_sources(&output, std::slice::from_ref(&source)).unwrap(); + + assert_eq!(fs::read(output.join("IValue.js")).unwrap(), first_js); + assert_eq!(fs::read(output.join("IValue.d.ts")).unwrap(), first_dts); + fs::remove_dir_all(output).unwrap(); + } + + #[test] + fn incremental_shared_interface_identity_mismatch_is_atomic() { + let output = test_directory("shared-interface-identity-mismatch"); + fs::create_dir_all(&output).unwrap(); + let original = + shared_interface_source("Contoso", "IValue", "11111111-1111-1111-1111-111111111111"); + generate_shared_interface_sources(&output, std::slice::from_ref(&original)).unwrap(); + let original_js = fs::read(output.join("IValue.js")).unwrap(); + let original_dts = fs::read(output.join("IValue.d.ts")).unwrap(); + + let preceding = + shared_interface_source("Contoso", "IOther", "22222222-2222-2222-2222-222222222222"); + let conflicting = + shared_interface_source("Fabrikam", "IValue", "33333333-3333-3333-3333-333333333333"); + let error = generate_shared_interface_sources(&output, &[preceding, conflicting]) + .expect_err("different flat-file identity must be rejected before writes"); + + assert!(error.contains("Refusing to overwrite shared interface source")); + assert!(error.contains("Contoso.IValue:11111111-1111-1111-1111-111111111111")); + assert!(error.contains("Fabrikam.IValue:33333333-3333-3333-3333-333333333333")); + assert_eq!(fs::read(output.join("IValue.js")).unwrap(), original_js); + assert_eq!(fs::read(output.join("IValue.d.ts")).unwrap(), original_dts); + assert!(!output.join("IOther.js").exists()); + assert!(!output.join("IOther.d.ts").exists()); + fs::remove_dir_all(output).unwrap(); + } + + #[test] + fn incremental_ambiguous_interface_fallback_does_not_abort() { + let output = test_directory("shared-interface-ambiguous-fallback"); + fs::create_dir_all(&output).unwrap(); + let original = + shared_interface_source("Contoso", "IValue", "11111111-1111-1111-1111-111111111111"); + generate_shared_interface_sources(&output, std::slice::from_ref(&original)).unwrap(); + let original_js = fs::read(output.join("IValue.js")).unwrap(); + + let mut fallback = + shared_interface_source("Fabrikam", "IValue", "33333333-3333-3333-3333-333333333333"); + fallback.shared_member_source = false; + generate_shared_interface_sources(&output, std::slice::from_ref(&fallback)) + .expect("ambiguous class-local fallback must not be rejected"); + + assert_eq!(fs::read(output.join("IValue.js")).unwrap(), original_js); + fs::remove_dir_all(output).unwrap(); + } + #[test] fn python_duplicate_short_names_use_namespace_facades() { let output = test_directory("namespace-facades"); diff --git a/tools/dynwinrt-codegen/tests/consistency_test.rs b/tools/dynwinrt-codegen/tests/consistency_test.rs index eab40656..33e73b7c 100644 --- a/tools/dynwinrt-codegen/tests/consistency_test.rs +++ b/tools/dynwinrt-codegen/tests/consistency_test.rs @@ -143,6 +143,21 @@ fn extract_exports(code: &str) -> Vec { .collect() } +fn repeated_required_interface_iids(classes: &[meta::ClassMeta]) -> HashSet { + let mut counts = HashMap::new(); + for class in classes { + for interface in &class.required_interfaces { + if !interface.iid.is_empty() { + *counts.entry(interface.iid.clone()).or_insert(0usize) += 1; + } + } + } + counts + .into_iter() + .filter_map(|(iid, count)| (count >= 2).then_some(iid)) + .collect() +} + /// Helper: build known_types, delegate_type_names, shared_iids from parsed metadata. fn setup_metadata( winmd: &str, @@ -154,7 +169,7 @@ fn setup_metadata( Vec, HashSet, HashSet, - HashSet, + HashSet, HashMap, HashMap>, HashMap>, @@ -189,7 +204,7 @@ fn setup_metadata( }) .map(|i| i.name.clone()) .collect(); - let shared_iids: HashSet = HashSet::new(); + let shared_iids = repeated_required_interface_iids(&all_classes); let (delegate_sigs, delegate_sig_refs, delegate_param_wraps) = project::build_delegate_signatures(&all_interfaces, &delegate_type_names, &known_types); @@ -436,7 +451,7 @@ fn js_dts_structural_consistency_user_watcher() { }) .map(|i| i.name.clone()) .collect(); - let shared_iids: HashSet = HashSet::new(); + let shared_iids = repeated_required_interface_iids(&all_classes); let (delegate_sigs, delegate_sig_refs, delegate_param_wraps) = project::build_delegate_signatures(&all_interfaces, &delegate_type_names, &known_types); diff --git a/tools/dynwinrt-codegen/tests/shared_interface_members_test.rs b/tools/dynwinrt-codegen/tests/shared_interface_members_test.rs index 02e3c4a6..07016ac0 100644 --- a/tools/dynwinrt-codegen/tests/shared_interface_members_test.rs +++ b/tools/dynwinrt-codegen/tests/shared_interface_members_test.rs @@ -71,6 +71,87 @@ fn shared_sources(interface: &InterfaceMeta) -> HashSet u64 { + contents.bytes().fold(0xcbf29ce484222325, |hash, byte| { + (hash ^ u64::from(byte)).wrapping_mul(0x100000001b3) + }) +} + +#[test] +fn default_projection_preserves_legacy_standalone_interface_imports() { + let interface = value_interface(); + let class = widget_class(&interface); + let known_types = HashSet::from(["Widget".into(), "IValue".into()]); + let standalone_interface_iids = HashSet::from([interface.iid.clone()]); + + project::set_shared_interface_members(false); + let projected = project::project_class( + &class, + &known_types, + &HashSet::new(), + &standalone_interface_iids, + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + ); + let separated = project::project_class_with_shared_member_sources( + &class, + &known_types, + &HashSet::new(), + &standalone_interface_iids, + &HashSet::new(), + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + ); + let descriptor_only = project::project_class_with_shared_member_sources( + &class, + &known_types, + &HashSet::new(), + &HashSet::new(), + &shared_sources(&interface), + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + ); + + let interface_imports = projected + .imports + .iter() + .filter(|import| import.from == "./IValue.js") + .map(|import| (import.symbols.clone(), import.runtime_only, import.dts_only)) + .collect::>(); + assert_eq!( + interface_imports, + vec![(vec!["IID_IValue".into(), "IValue".into()], false, false)] + ); + assert!(projected.classes[0].required_ifaces.is_empty()); + assert_eq!(descriptor_only.classes[0].required_ifaces.len(), 1); + assert!( + descriptor_only + .imports + .iter() + .all(|import| import.from != "./IValue.js") + ); + assert_eq!(render_js::render(&projected), render_js::render(&separated)); + assert_eq!( + render_dts::render(&projected), + render_dts::render(&separated) + ); + + let class_js = render_js::render(&projected); + let class_dts = render_dts::render(&projected); + assert_eq!( + (snapshot_hash(&class_js), snapshot_hash(&class_dts)), + (14855624038810436456, 13331642975496716118), + "flag-off output changed from the origin/main compatibility snapshot" + ); + assert!(class_js.contains("require('./IValue.js')")); + assert!(class_js.contains("_IValue.method(6).invoke(this._obj.cast(IID_IValue)")); + assert!(!class_js.contains("__copyInterfaceMembers")); + assert!(!class_js.contains("export class IValue")); +} + #[test] fn opt_in_reuses_shared_interface_descriptors_without_changing_dts() { let interface = value_interface(); @@ -88,10 +169,11 @@ fn opt_in_reuses_shared_interface_descriptors_without_changing_dts() { &HashMap::new(), true, ); - let class_file = project::project_class( + let class_file = project::project_class_with_shared_member_sources( &class, &known_types, &HashSet::new(), + &HashSet::new(), &shared_iids, &HashMap::new(), &HashMap::new(), @@ -120,7 +202,7 @@ fn opt_in_reuses_shared_interface_descriptors_without_changing_dts() { &class, &known_types, &HashSet::new(), - &shared_iids, + &HashSet::from([interface.iid.clone()]), &HashMap::new(), &HashMap::new(), &HashMap::new(), @@ -157,10 +239,11 @@ fn canonical_interface_source_survives_equivalent_duplicate_emission() { &HashMap::new(), sources[0].shared_member_source, ); - let class_file = project::project_class( + let class_file = project::project_class_with_shared_member_sources( &widget_class(&duplicate), &HashSet::from(["Widget".into(), "IValue".into()]), &HashSet::new(), + &HashSet::new(), &HashSet::from([sources[0].identity.clone()]), &HashMap::new(), &HashMap::new(), @@ -336,6 +419,7 @@ fn ambiguous_standalone_interface_identities_remain_class_local() { class, &known_types, &HashSet::new(), + &HashSet::new(), &shared_source_identities, &ambiguous_names, &HashMap::new(), @@ -436,10 +520,11 @@ fn shared_interface_members_preserve_overload_dispatch_and_declarations() { &HashMap::new(), true, )); - let class_file = project::project_class( + let class_file = project::project_class_with_shared_member_sources( &class, &known_types, &HashSet::new(), + &HashSet::new(), &shared_iids, &HashMap::new(), &HashMap::new(), @@ -516,10 +601,11 @@ fn shared_interface_members_preserve_cross_interface_overload_dispatch() { let shared_iids = shared_sources(&required_interface); project::set_shared_interface_members(true); - let class_file = project::project_class( + let class_file = project::project_class_with_shared_member_sources( &class, &known_types, &HashSet::new(), + &HashSet::new(), &shared_iids, &HashMap::new(), &HashMap::new(), @@ -594,10 +680,11 @@ fn shared_interface_event_alias_conflicts_remain_class_local() { let shared_iids = shared_sources(&required_interface); project::set_shared_interface_members(true); - let class_file = project::project_class( + let class_file = project::project_class_with_shared_member_sources( &class, &known_types, &HashSet::new(), + &HashSet::new(), &shared_iids, &HashMap::new(), &HashMap::new(), @@ -670,6 +757,7 @@ fn excluded_iclosable_uses_local_wrapper() { &HashSet::from(["Widget".into(), "IClosable".into()]), &HashSet::new(), &HashSet::new(), + &HashSet::new(), &HashSet::from(["IClosable".into()]), &HashMap::new(), &HashMap::new(), @@ -742,10 +830,11 @@ fn collection_getter_casts_concrete_view_and_rejects_invalid_sources() { &HashMap::new(), true, )); - let class_js = render_js::render(&project::project_class( + let class_js = render_js::render(&project::project_class_with_shared_member_sources( &class, &known_types, &HashSet::new(), + &HashSet::new(), &shared_sources, &HashMap::new(), &HashMap::new(), @@ -895,10 +984,11 @@ fn shared_interface_descriptor_executes_for_raw_and_concrete_views() { &HashMap::new(), true, )); - let class_js = render_js::render(&project::project_class( + let class_js = render_js::render(&project::project_class_with_shared_member_sources( &class, &known_types, &HashSet::new(), + &HashSet::new(), &shared_iids, &HashMap::new(), &HashMap::new(), diff --git a/tools/dynwinrt-codegen/tests/snapshot_test.rs b/tools/dynwinrt-codegen/tests/snapshot_test.rs index b7fd28db..2e39437e 100644 --- a/tools/dynwinrt-codegen/tests/snapshot_test.rs +++ b/tools/dynwinrt-codegen/tests/snapshot_test.rs @@ -20,6 +20,21 @@ use dynwinrt_codegen::types::TypeMeta; const WINDOWS_WINMD: &str = r"C:\Program Files (x86)\Windows Kits\10\UnionMetadata\10.0.26100.0\Windows.winmd"; +fn repeated_required_interface_iids(classes: &[meta::ClassMeta]) -> HashSet { + let mut counts = HashMap::new(); + for class in classes { + for interface in &class.required_interfaces { + if !interface.iid.is_empty() { + *counts.entry(interface.iid.clone()).or_insert(0usize) += 1; + } + } + } + counts + .into_iter() + .filter_map(|(iid, count)| (count >= 2).then_some(iid)) + .collect() +} + /// Generate TypeScript for Uri and compare every file against the snapshot. #[test] fn snapshot_uri_class() { @@ -60,7 +75,7 @@ fn snapshot_uri_class() { .map(|i| i.name.clone()) .collect(); - let shared_iids: HashSet = HashSet::new(); + let shared_iids = repeated_required_interface_iids(&all_classes); let (delegate_sigs, delegate_sig_refs, delegate_param_wraps) = project::build_delegate_signatures(&all_interfaces, &delegate_type_names, &known_types); @@ -370,7 +385,7 @@ fn ts_async_methods_emit_abort_signal_scaffolding() { }) .map(|i| i.name.clone()) .collect(); - let shared: HashSet = HashSet::new(); + let shared = repeated_required_interface_iids(&dw_all_classes); let (dw_delegate_sigs, dw_delegate_sig_refs, dw_delegate_param_wraps) = project::build_delegate_signatures(&dw_ifaces, &delegates, &known); From f3c68167f60dce0c4da65d7c2cc82599b68c74cb Mon Sep 17 00:00:00 2001 From: "Leilei Zhang (from Dev Box)" Date: Wed, 12 Aug 2026 22:34:26 +0800 Subject: [PATCH 5/8] Fix shared interface compatibility regressions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b485b75e-2292-44e1-98c8-3f7bb6489bff --- .../src/codegen/winrt/javascript/ir.rs | 7 + .../codegen/winrt/javascript/project/mod.rs | 40 ++- .../winrt/javascript/render/declarations.rs | 6 + .../winrt/javascript/render/javascript/mod.rs | 17 +- tools/dynwinrt-codegen/src/main.rs | 300 ++++++++++++++++-- tools/dynwinrt-codegen/src/meta.rs | 104 +++++- .../tests/shared_interface_members_test.rs | 218 +++++++++++++ 7 files changed, 652 insertions(+), 40 deletions(-) diff --git a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/ir.rs b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/ir.rs index 7511e532..43a1f7a9 100644 --- a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/ir.rs +++ b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/ir.rs @@ -213,6 +213,11 @@ pub struct ProjectedImport { pub is_runtime_package: bool, } +pub struct ProjectedReExport { + pub name: String, + pub from: String, +} + /// Disposition of a required interface. pub enum RequiredIfaceDisposition { /// Imported from its own generated file @@ -320,6 +325,8 @@ pub struct ProjectedDelegate { pub struct ProjectedFile { pub name: String, pub imports: Vec, + /// Public symbols preserved from a canonical sibling module. + pub re_exports: Vec, /// IID constants (rendered as `const` in JS, `declare const` in DTS) pub iid_consts: Vec, /// Interface registration blocks (JS only) diff --git a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/project/mod.rs b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/project/mod.rs index ba05784a..f09d9e43 100644 --- a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/project/mod.rs +++ b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/project/mod.rs @@ -442,6 +442,7 @@ pub fn project_class_with_excluded_interface_imports( // Type imports let mut imported_names: HashSet = HashSet::new(); let mut imported_interface_names: HashSet = HashSet::new(); + let mut shared_only_interface_imports: HashSet = HashSet::new(); let type_imports = collect_type_imports(class); let local_required_interface_names = class .required_interfaces @@ -501,18 +502,30 @@ pub fn project_class_with_excluded_interface_imports( } } + let needs_iclosable = class.name != "IClosable" + && class + .required_interfaces + .iter() + .any(|ri| ri.iid == ICLOSABLE_IID); + // Import shared required interfaces for req_iface in &class.required_interfaces { + let is_legacy_standalone = standalone_interface_iids.contains(&req_iface.iid); + let is_shared_member_source = share_interface_members + && standalone_interface_identity(req_iface) + .is_some_and(|identity| shared_member_source_identities.contains(&identity)); if req_iface.generic_piid.is_none() && !req_iface.iid.is_empty() - && (standalone_interface_iids.contains(&req_iface.iid) - || (share_interface_members - && standalone_interface_identity(req_iface).is_some_and(|identity| { - shared_member_source_identities.contains(&identity) - }))) + && (is_legacy_standalone || is_shared_member_source) && !excluded_interface_import_names.contains(&req_iface.name) && !imported_names.contains(&req_iface.name) { + if is_shared_member_source + && !is_legacy_standalone + && !(needs_iclosable && req_iface.name == "IClosable") + { + shared_only_interface_imports.insert(req_iface.name.clone()); + } imports.push(format_type_import_projected( &req_iface.name, TypeKind::Interface, @@ -540,11 +553,6 @@ pub fn project_class_with_excluded_interface_imports( // IClosable by name. Register the import here so the IID-const loop below // sees `IID_IClosable` in `imported_names` and skips declaring it, // avoiding a duplicate identifier in single-class emission. - let needs_iclosable = class.name != "IClosable" - && class - .required_interfaces - .iter() - .any(|ri| ri.iid == ICLOSABLE_IID); if needs_iclosable && !excluded_interface_import_names.contains("IClosable") && !imported_names.contains("IClosable") @@ -1304,10 +1312,19 @@ pub fn project_class_with_excluded_interface_imports( let needs_unwrap = check_needs_unwrap(&members, &required_ifaces); let doc = build_doc_info(class.doc.as_deref(), class.deprecated.as_deref(), None, &[]); + let mut re_exports = shared_only_interface_imports + .into_iter() + .map(|name| ProjectedReExport { + from: format!("./{}.js", name), + name, + }) + .collect::>(); + re_exports.sort_by(|left, right| left.name.cmp(&right.name)); ProjectedFile { name: class.name.clone(), imports, + re_exports, iid_consts, registrations, structs, @@ -1575,6 +1592,7 @@ pub fn project_interface_with_shared_member_source( ProjectedFile { name: iface.name.clone(), imports, + re_exports: vec![], iid_consts, registrations, structs, @@ -1626,6 +1644,7 @@ pub fn project_enum(en: &TypeMeta) -> Option { Some(ProjectedFile { name: name.clone(), imports: vec![], + re_exports: vec![], iid_consts: vec![], registrations: vec![], structs: vec![], @@ -1709,6 +1728,7 @@ pub fn project_delegate( ProjectedFile { name: iface.name.clone(), imports, + re_exports: vec![], iid_consts: vec![], registrations: vec![], structs: vec![], diff --git a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/render/declarations.rs b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/render/declarations.rs index 27edbcee..6eed198d 100644 --- a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/render/declarations.rs +++ b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/render/declarations.rs @@ -102,6 +102,12 @@ pub fn render(file: &ProjectedFile) -> String { )); } } + for re_export in &file.re_exports { + out.push_str(&format!( + "export {{ {} }} from '{}';\n", + re_export.name, re_export.from, + )); + } if !out.ends_with('\n') || out.len() > 50 { out.push('\n'); } diff --git a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/render/javascript/mod.rs b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/render/javascript/mod.rs index d8a02f35..9255272c 100644 --- a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/render/javascript/mod.rs +++ b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/render/javascript/mod.rs @@ -33,7 +33,17 @@ pub fn render(file: &ProjectedFile) -> String { runtime_sources.insert(imp.from.clone()); } } - convert_to_cjs_with_lazy(&esm, &runtime_sources) + let mut cjs = convert_to_cjs_with_lazy(&esm, &runtime_sources); + for re_export in &file.re_exports { + if !cjs.ends_with('\n') { + cjs.push('\n'); + } + cjs.push_str(&format!( + "Object.defineProperty(exports, '{}', {{ enumerable: true, get: () => require('{}').{} }});\n", + re_export.name, re_export.from, re_export.name, + )); + } + cjs } /// Render a projected file as ESM JS (internal — post-processed to CJS by render()). @@ -645,7 +655,10 @@ fn render_member_js(out: &mut String, member: &ProjectedMember, _class_name: &st ProjectedMember::Symbol(symbol) => { match &symbol.kind { SymbolKind::ToString { iface_name } => { - out.push_str(&format!(" toString() {{\n return {}.from(this._obj).toString();\n }}\n", iface_name)); + out.push_str(&format!( + " toString() {{\n return {}.from(this._obj).toString();\n }}\n", + ref_marker(iface_name), + )); } SymbolKind::ToPrimitive => { out.push_str(" [Symbol.toPrimitive](_hint) {\n return this.toString();\n }\n"); diff --git a/tools/dynwinrt-codegen/src/main.rs b/tools/dynwinrt-codegen/src/main.rs index 1e5c2616..a0a17924 100644 --- a/tools/dynwinrt-codegen/src/main.rs +++ b/tools/dynwinrt-codegen/src/main.rs @@ -482,6 +482,42 @@ fn run() -> Result<(), String> { )); } + winui::add_implicit_classes(&winmd, &mut classes); + let mut implicit_interfaces = Vec::new(); + winui::add_implicit_interfaces(&winmd, &classes, &mut implicit_interfaces); + let ( + mut excluded_shared_source_names, + mut shared_interface_source_identities, + reserved_non_interface_output_names, + ) = if lang == "js" && project::shared_interface_members_enabled() { + shared_interface_plan_for_batches( + &winmd, + &[(classes.clone(), implicit_interfaces.clone(), Vec::new())], + ) + } else { + (HashSet::new(), HashSet::new(), HashSet::new()) + }; + excluded_shared_source_names + .extend(metadata_excluded_shared_source_names.iter().cloned()); + shared_interface_source_identities.extend( + metadata_shared_source_identities + .iter() + .filter(|identity| !excluded_shared_source_names.contains(&identity.name)) + .cloned(), + ); + if lang == "js" && !dry_run && !classes.is_empty() { + preflight_js_generation_batch( + &winmd, + output_dir, + &classes, + &implicit_interfaces, + &[], + &excluded_shared_source_names, + &shared_interface_source_identities, + &reserved_non_interface_output_names, + )?; + } + // Classic COM occupies its own ESM subpackage so its symbols // cannot collide with or leak into the WinRT root barrel. if !com_interfaces.is_empty() || !com_coclasses.is_empty() { @@ -594,29 +630,6 @@ fn run() -> Result<(), String> { } } - winui::add_implicit_classes(&winmd, &mut classes); - let mut implicit_interfaces = Vec::new(); - winui::add_implicit_interfaces(&winmd, &classes, &mut implicit_interfaces); - let ( - mut excluded_shared_source_names, - mut shared_interface_source_identities, - reserved_non_interface_output_names, - ) = if lang == "js" && project::shared_interface_members_enabled() { - shared_interface_plan_for_batches( - &winmd, - &[(classes.clone(), implicit_interfaces.clone(), Vec::new())], - ) - } else { - (HashSet::new(), HashSet::new(), HashSet::new()) - }; - excluded_shared_source_names - .extend(metadata_excluded_shared_source_names.iter().cloned()); - shared_interface_source_identities.extend( - metadata_shared_source_identities - .iter() - .filter(|identity| !excluded_shared_source_names.contains(&identity.name)) - .cloned(), - ); generate_for_types( &winmd, output_dir, @@ -784,6 +797,20 @@ fn run() -> Result<(), String> { .filter(|identity| !excluded_shared_source_names.contains(&identity.name)) .cloned(), ); + if lang == "js" && !dry_run { + for (classes, interfaces, enums) in &namespace_batches { + preflight_js_generation_batch( + &winmd, + output_dir, + classes, + interfaces, + enums, + &excluded_shared_source_names, + &shared_interface_source_identities, + &reserved_non_interface_output_names, + )?; + } + } let mut total_classes = 0usize; let mut total_interfaces = 0usize; @@ -1359,6 +1386,166 @@ fn plan_existing_shared_interface_sources( Ok(preserved_source_names) } +fn plan_unflagged_existing_shared_interface_sources<'a>( + output_dir: &Path, + interfaces: impl IntoIterator, +) -> Result, String> { + let mut preserved_identities = HashSet::new(); + for interface in interfaces { + let Some(requested_identity) = project::standalone_interface_identity(interface) else { + continue; + }; + let path = output_dir.join(format!("{}.js", interface.name)); + let existing = match fs::read_to_string(&path) { + Ok(existing) => existing, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => continue, + Err(error) => { + return Err(format!("Failed to read {}: {}", path.display(), error)); + } + }; + let Some(existing_identity) = generated_shared_interface_source_identity(&existing) else { + continue; + }; + let requested_marker = requested_identity.source_marker(); + if existing_identity != requested_marker { + return Err(format!( + "Refusing to overwrite shared interface source `{}`: existing generated identity \ + `{}` does not match requested identity `{}`. Use a separate output directory or \ + remove and regenerate the conflicting file.", + path.display(), + existing_identity, + requested_marker, + )); + } + preserved_identities.insert(requested_identity); + } + Ok(preserved_identities) +} + +fn reject_non_interface_overwrites_of_shared_sources<'a>( + output_dir: &Path, + names: impl IntoIterator, +) -> Result<(), String> { + let mut checked = HashSet::new(); + for name in names { + if !checked.insert(name) { + continue; + } + let path = output_dir.join(format!("{name}.js")); + let existing = match fs::read_to_string(&path) { + Ok(existing) => existing, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => continue, + Err(error) => { + return Err(format!("Failed to read {}: {}", path.display(), error)); + } + }; + let Some(existing_identity) = generated_shared_interface_source_identity(&existing) else { + continue; + }; + return Err(format!( + "Refusing to overwrite shared interface source `{}` with non-interface output `{}`: \ + existing generated identity is `{}`. Use a separate output directory or remove and \ + regenerate the conflicting file.", + path.display(), + name, + existing_identity, + )); + } + Ok(()) +} + +fn preflight_js_generation_batch( + winmd: &str, + output_dir: &Path, + classes: &[meta::ClassMeta], + interfaces: &[meta::InterfaceMeta], + enums: &[TypeMeta], + excluded_shared_source_names: &HashSet, + forced_shared_source_identities: &HashSet, + reserved_non_interface_output_names: &HashSet, +) -> Result<(), String> { + let deps = meta::resolve_dependencies(winmd, classes, interfaces, enums); + let all_classes = classes.iter().chain(&deps.classes).collect::>(); + let all_interfaces = interfaces + .iter() + .chain(&deps.interfaces) + .collect::>(); + let all_enums = enums.iter().chain(&deps.enums).collect::>(); + let class_names = all_classes + .iter() + .map(|class| class.name.clone()) + .collect::>(); + let enum_names = all_enums + .iter() + .filter_map(|typ| match typ { + TypeMeta::Enum { name, .. } if !name.contains('<') && !class_names.contains(name) => { + Some(name.as_str()) + } + _ => None, + }) + .collect::>(); + reject_non_interface_overwrites_of_shared_sources( + output_dir, + all_classes + .iter() + .map(|class| class.name.as_str()) + .chain(enum_names), + )?; + + if project::shared_interface_members_enabled() { + let mut required_interface_count: HashMap< + project::StandaloneInterfaceIdentity, + (&meta::InterfaceMeta, usize), + > = HashMap::new(); + for class in &all_classes { + for req_iface in &class.required_interfaces { + let Some(identity) = project::standalone_interface_identity(req_iface) else { + continue; + }; + required_interface_count + .entry(identity) + .and_modify(|(_, count)| *count += 1) + .or_insert((req_iface, 1)); + } + } + let canonical_shared_interfaces = required_interface_count + .into_values() + .filter(|(_, count)| *count >= 2) + .map(|(iface, _)| iface.clone()) + .collect::>(); + let mut effective_excluded_names = excluded_shared_source_names.clone(); + effective_excluded_names.extend(all_enums.iter().filter_map(|typ| match typ { + TypeMeta::Enum { name, .. } if !name.contains('<') && !class_names.contains(name) => { + Some(name.clone()) + } + _ => None, + })); + let all_interfaces = all_interfaces.into_iter().cloned().collect::>(); + let canonical_sources = project::canonical_interface_sources( + &all_interfaces, + &canonical_shared_interfaces, + &class_names, + &effective_excluded_names, + forced_shared_source_identities, + )?; + plan_existing_shared_interface_sources( + output_dir, + &canonical_sources, + reserved_non_interface_output_names, + )?; + } else { + plan_unflagged_existing_shared_interface_sources( + output_dir, + all_interfaces.into_iter().filter(|iface| { + !class_names.contains(&iface.name) + && !reserved_non_interface_output_names.contains(&iface.name) + && !iface.iid.is_empty() + }), + )?; + } + Ok(()) +} + fn generate_js_files( output_dir: &Path, all_classes: &[meta::ClassMeta], @@ -1401,6 +1588,22 @@ fn generate_js_files( !iface.iid.is_empty() } + let enum_output_names = all_enums.iter().filter_map(|typ| match typ { + TypeMeta::Enum { name, .. } + if !name.contains('<') && !class_names.contains(name.as_str()) => + { + Some(name.as_str()) + } + _ => None, + }); + reject_non_interface_overwrites_of_shared_sources( + output_dir, + all_classes + .iter() + .map(|class| class.name.as_str()) + .chain(enum_output_names), + )?; + let preserved_shared_source_names = canonical_interface_sources .map(|canonical_sources| { plan_existing_shared_interface_sources( @@ -1411,6 +1614,21 @@ fn generate_js_files( }) .transpose()? .unwrap_or_default(); + let unflagged_preserved_shared_source_identities = if canonical_interface_sources.is_none() { + plan_unflagged_existing_shared_interface_sources( + output_dir, + shared_interfaces + .iter() + .chain(all_interfaces) + .filter(|iface| { + !class_names.contains(iface.name.as_str()) + && !reserved_non_interface_output_names.contains(&iface.name) + && is_emittable_interface(iface) + }), + )? + } else { + HashSet::new() + }; if let Some(canonical_sources) = canonical_interface_sources { for source in canonical_sources { @@ -1445,13 +1663,18 @@ fn generate_js_files( if !is_emittable_interface(iface) { continue; } - let projected = project::project_interface( + let preserve_shared_source = + project::standalone_interface_identity(iface).is_some_and(|identity| { + unflagged_preserved_shared_source_identities.contains(&identity) + }); + let projected = project::project_interface_with_shared_member_source( iface, known_types, delegate_type_names, delegate_sigs, delegate_sig_refs, delegate_param_wraps, + preserve_shared_source, ); let js = render_js::render(&projected); let dts = render_dts::render(&projected); @@ -1467,13 +1690,18 @@ fn generate_js_files( if !is_emittable_interface(iface) { continue; } - let projected = project::project_interface( + let preserve_shared_source = + project::standalone_interface_identity(iface).is_some_and(|identity| { + unflagged_preserved_shared_source_identities.contains(&identity) + }); + let projected = project::project_interface_with_shared_member_source( iface, known_types, delegate_type_names, delegate_sigs, delegate_sig_refs, delegate_param_wraps, + preserve_shared_source, ); let js = render_js::render(&projected); let dts = render_dts::render(&projected); @@ -3884,6 +4112,28 @@ mod tests { fs::remove_dir_all(output).unwrap(); } + #[test] + fn incremental_shared_interface_rejects_non_interface_overwrite() { + let output = test_directory("shared-interface-non-interface-overwrite"); + fs::create_dir_all(&output).unwrap(); + let original = + shared_interface_source("Contoso", "IValue", "11111111-1111-1111-1111-111111111111"); + generate_shared_interface_sources(&output, std::slice::from_ref(&original)).unwrap(); + let original_js = fs::read(output.join("IValue.js")).unwrap(); + let original_dts = fs::read(output.join("IValue.d.ts")).unwrap(); + + let error = + reject_non_interface_overwrites_of_shared_sources(&output, ["IOther", "IValue"]) + .expect_err("class or enum output must not replace a shared interface source"); + + assert!(error.contains("Refusing to overwrite shared interface source")); + assert!(error.contains("non-interface output `IValue`")); + assert_eq!(fs::read(output.join("IValue.js")).unwrap(), original_js); + assert_eq!(fs::read(output.join("IValue.d.ts")).unwrap(), original_dts); + assert!(!output.join("IOther.js").exists()); + fs::remove_dir_all(output).unwrap(); + } + #[test] fn incremental_ambiguous_interface_fallback_does_not_abort() { let output = test_directory("shared-interface-ambiguous-fallback"); diff --git a/tools/dynwinrt-codegen/src/meta.rs b/tools/dynwinrt-codegen/src/meta.rs index 8b1b1132..558c2e7e 100644 --- a/tools/dynwinrt-codegen/src/meta.rs +++ b/tools/dynwinrt-codegen/src/meta.rs @@ -324,7 +324,7 @@ fn parse_interfaces_from_index( if !include_exclusive && def.has_attribute("ExclusiveToAttribute") { continue; } - if let Some(iface) = parse_interface(index, def.namespace(), def.name()) { + if let Some(iface) = parse_interface_def(index, &def) { interfaces.push(iface); } } @@ -1068,8 +1068,12 @@ fn split_full_name(full_name: &str) -> Option<(&str, &str)> { fn parse_interface(index: &reader::Index, namespace: &str, name: &str) -> Option { let def = index.get(namespace, name).next()?; - let iid = extract_iid(&def); - parse_interface_methods(index, &def, name, namespace, &iid, &[]) + parse_interface_def(index, &def) +} + +fn parse_interface_def(index: &reader::Index, def: &reader::TypeDef) -> Option { + let iid = extract_iid(def); + parse_interface_methods(index, def, def.name(), def.namespace(), &iid, &[]) } fn parse_interface_type( @@ -1660,6 +1664,100 @@ fn resolve_named_type( #[cfg(test)] mod tests { use super::*; + use windows_metadata::writer::{AttributeType, HasAttribute, MemberRefParent, TypeDefOrRef}; + + fn write_interface_winmd(path: &Path, assembly_name: &str, iid: (u32, u16, u16, [u8; 8])) { + let mut file = windows_metadata::writer::File::new(assembly_name); + let interface = file.TypeDef( + "Contoso.Duplicates", + "IRepeated", + TypeDefOrRef::default(), + windows_metadata::TypeAttributes::Public + | windows_metadata::TypeAttributes::Interface + | windows_metadata::TypeAttributes::Abstract + | windows_metadata::TypeAttributes::WindowsRuntime, + ); + let guid_attribute = file.TypeRef("Windows.Foundation.Metadata", "GuidAttribute"); + let constructor = file.MemberRef( + ".ctor", + &windows_metadata::Signature { + flags: windows_metadata::MethodCallAttributes::HASTHIS, + return_type: windows_metadata::Type::Void, + types: vec![ + windows_metadata::Type::U32, + windows_metadata::Type::U16, + windows_metadata::Type::U16, + windows_metadata::Type::U8, + windows_metadata::Type::U8, + windows_metadata::Type::U8, + windows_metadata::Type::U8, + windows_metadata::Type::U8, + windows_metadata::Type::U8, + windows_metadata::Type::U8, + windows_metadata::Type::U8, + ], + }, + MemberRefParent::TypeRef(guid_attribute), + ); + let mut values = vec![ + ("".into(), windows_metadata::Value::U32(iid.0)), + ("".into(), windows_metadata::Value::U16(iid.1)), + ("".into(), windows_metadata::Value::U16(iid.2)), + ]; + values.extend( + iid.3 + .into_iter() + .map(|value| ("".into(), windows_metadata::Value::U8(value))), + ); + file.Attribute( + HasAttribute::TypeDef(interface), + AttributeType::MemberRef(constructor), + &values, + ); + std::fs::write(path, file.into_stream()).unwrap(); + } + + #[test] + fn interface_enumeration_parses_each_duplicate_typedef_directly() { + let directory = Path::new(env!("CARGO_MANIFEST_DIR")) + .join("target") + .join(format!( + "duplicate-interface-metadata-{}", + std::process::id() + )); + let _ = std::fs::remove_dir_all(&directory); + std::fs::create_dir_all(&directory).unwrap(); + let first = directory.join("First.winmd"); + let second = directory.join("Second.winmd"); + write_interface_winmd(&first, "First", (0x11111111, 0x1111, 0x1111, [0x11; 8])); + write_interface_winmd(&second, "Second", (0x22222222, 0x2222, 0x2222, [0x22; 8])); + + let paths = format!("{};{}", first.display(), second.display()); + let interfaces = parse_all_interfaces_including_exclusive(&paths) + .into_iter() + .filter(|interface| { + interface.namespace == "Contoso.Duplicates" && interface.name == "IRepeated" + }) + .collect::>(); + let mut iids = interfaces + .iter() + .map(|interface| interface.iid.clone()) + .collect::>(); + iids.sort(); + + assert_eq!( + iids, + vec![ + "11111111-1111-1111-1111-111111111111", + "22222222-2222-2222-2222-222222222222", + ] + ); + assert_eq!( + crate::codegen::project::ambiguous_standalone_interface_names(&interfaces), + HashSet::from(["IRepeated".into()]) + ); + std::fs::remove_dir_all(directory).unwrap(); + } #[test] fn make_parameterized_name_single_arg() { diff --git a/tools/dynwinrt-codegen/tests/shared_interface_members_test.rs b/tools/dynwinrt-codegen/tests/shared_interface_members_test.rs index 07016ac0..f3b04fbf 100644 --- a/tools/dynwinrt-codegen/tests/shared_interface_members_test.rs +++ b/tools/dynwinrt-codegen/tests/shared_interface_members_test.rs @@ -3,12 +3,16 @@ use std::collections::{HashMap, HashSet}; use std::fs; +use std::path::{Path, PathBuf}; use std::process::Command; use dynwinrt_codegen::codegen::{project, render_dts, render_js}; use dynwinrt_codegen::meta::{ClassMeta, InterfaceMeta, MethodMeta, ParamDirection, ParamMeta}; use dynwinrt_codegen::types::TypeMeta; +const WINDOWS_WINMD: &str = + r"C:\Program Files (x86)\Windows Kits\10\UnionMetadata\10.0.26100.0\Windows.winmd"; + fn value_interface() -> InterfaceMeta { InterfaceMeta { name: "IValue".into(), @@ -77,6 +81,220 @@ fn snapshot_hash(contents: &str) -> u64 { }) } +fn cli_test_directory(name: &str) -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("target") + .join(format!("{name}-{}", std::process::id())) +} + +fn run_codegen(output: &Path, class_names: &str, shared: bool) -> std::process::Output { + let mut command = Command::new(env!("CARGO_BIN_EXE_dynwinrt-codegen")); + command.args([ + "generate", + "--winmd", + WINDOWS_WINMD, + "--namespace", + "Windows.Foundation", + "--class-name", + class_names, + "--output", + ]); + command.arg(output); + command.args(["--import-name", "./runtime.js"]); + if shared { + command.arg("--shared-interface-members"); + } + command.output().expect("run dynwinrt-codegen") +} + +fn write_runtime_stub(output: &Path) { + fs::write( + output.join("runtime.js"), + "\ +class DynWinRtMethodSig { addIn() { return this; } addOut() { return this; } }\n\ +const registration = { addMethod() { return this; }, method() { return {}; } };\n\ +const DynWinRtType = new Proxy({\n\ + registerInterface() { return registration; },\n\ + parameterized() { return { iid() { return 'iid'; } }; },\n\ +}, { get(target, key) { return target[key] ?? (() => ({})); } });\n\ +const callable = new Proxy({}, { get() { return () => ({}); } });\n\ +module.exports = {\n\ + DynWinRtType,\n\ + DynWinRtMethodSig,\n\ + DynWinRtValue: callable,\n\ + DynWinRtArray: callable,\n\ + DynWinRtDelegate: callable,\n\ + WinGuid: { parse(value) { return value; } },\n\ +};\n", + ) + .unwrap(); +} + +fn assert_node_script(output: &Path, name: &str, script: &str) { + if Command::new("node").arg("--version").output().is_err() { + eprintln!("Skipping generated module-load assertion: node is unavailable"); + return; + } + let script_path = output.join(name); + fs::write(&script_path, script).unwrap(); + let result = Command::new("node").arg(&script_path).output().unwrap(); + assert!( + result.status.success(), + "node failed:\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&result.stdout), + String::from_utf8_lossy(&result.stderr), + ); +} + +fn snapshot_directory(output: &Path) -> HashMap> { + fs::read_dir(output) + .unwrap() + .map(|entry| entry.unwrap()) + .filter(|entry| entry.file_type().unwrap().is_file()) + .map(|entry| { + ( + entry.file_name().to_string_lossy().into_owned(), + fs::read(entry.path()).unwrap(), + ) + }) + .collect() +} + +#[test] +fn cli_shared_uri_preserves_deep_raw_interface_exports() { + if !Path::new(WINDOWS_WINMD).exists() { + eprintln!("Skipping shared Uri CLI test: Windows.winmd not found"); + return; + } + + let root = cli_test_directory("shared-uri-deep-interface"); + let baseline = root.join("baseline"); + let shared = root.join("shared"); + let _ = fs::remove_dir_all(&root); + + let baseline_result = run_codegen(&baseline, "Uri", false); + assert!( + baseline_result.status.success(), + "{}", + String::from_utf8_lossy(&baseline_result.stderr), + ); + let shared_result = run_codegen(&shared, "Uri", true); + assert!( + shared_result.status.success(), + "{}", + String::from_utf8_lossy(&shared_result.stderr), + ); + + let baseline_js = fs::read_to_string(baseline.join("Uri.js")).unwrap(); + let baseline_dts = fs::read_to_string(baseline.join("Uri.d.ts")).unwrap(); + let shared_js = fs::read_to_string(shared.join("Uri.js")).unwrap(); + let shared_dts = fs::read_to_string(shared.join("Uri.d.ts")).unwrap(); + let canonical_js = fs::read_to_string(shared.join("IStringable.js")).unwrap(); + + assert!(baseline_js.contains("exports.IStringable = IStringable;")); + assert!(baseline_dts.contains("export declare class IStringable")); + assert!(shared_js.contains( + "Object.defineProperty(exports, 'IStringable', { enumerable: true, get: () => require('./IStringable.js').IStringable });" + )); + assert!(shared_dts.contains("export { IStringable } from './IStringable.js';")); + assert!(shared_js.contains("return (__get_IStringable()).from(this._obj).toString();")); + assert!( + canonical_js + .contains("Windows.Foundation.IStringable:96369f54-8eb6-48f0-abce-c1b211e627c3") + ); + + write_runtime_stub(&shared); + assert_node_script( + &shared, + "verify-deep-interface.cjs", + "\ +const assert = require('node:assert/strict');\n\ +const deep = require('./Uri.js');\n\ +const canonical = require('./IStringable.js');\n\ +assert.equal(deep.IStringable, canonical.IStringable);\n", + ); + assert_node_script( + &shared, + "verify-deep-interface.mjs", + "\ +import assert from 'node:assert/strict';\n\ +import { IStringable as Deep } from './Uri.js';\n\ +import { IStringable as Canonical } from './IStringable.js';\n\ +assert.equal(Deep, Canonical);\n", + ); + + fs::remove_dir_all(root).unwrap(); +} + +#[test] +fn cli_mixed_flag_incremental_generation_preserves_shared_sources() { + if !Path::new(WINDOWS_WINMD).exists() { + eprintln!("Skipping mixed-flag CLI test: Windows.winmd not found"); + return; + } + + let output = cli_test_directory("mixed-flag-shared-source"); + let _ = fs::remove_dir_all(&output); + + let shared_result = run_codegen(&output, "Deferral,MemoryBuffer", true); + assert!( + shared_result.status.success(), + "{}", + String::from_utf8_lossy(&shared_result.stderr), + ); + write_runtime_stub(&output); + let deferral_before = fs::read(output.join("Deferral.js")).unwrap(); + let shared_iclosable = fs::read_to_string(output.join("IClosable.js")).unwrap(); + assert!( + shared_iclosable + .contains("Windows.Foundation.IClosable:30d5a829-7fa4-4026-83bb-d75bae4ea99e") + ); + assert!(shared_iclosable.contains("value._obj.cast(IID_IClosable)")); + assert_node_script( + &output, + "verify-deferral.cjs", + "require('./Deferral.js');\n", + ); + + let unflagged_result = run_codegen(&output, "MemoryBuffer", false); + assert!( + unflagged_result.status.success(), + "{}", + String::from_utf8_lossy(&unflagged_result.stderr), + ); + assert_eq!( + fs::read(output.join("Deferral.js")).unwrap(), + deferral_before + ); + let preserved_iclosable = fs::read_to_string(output.join("IClosable.js")).unwrap(); + assert!( + preserved_iclosable + .contains("Windows.Foundation.IClosable:30d5a829-7fa4-4026-83bb-d75bae4ea99e") + ); + assert!(preserved_iclosable.contains("value._obj.cast(IID_IClosable)")); + assert_node_script( + &output, + "verify-deferral-after-unflagged.cjs", + "require('./Deferral.js');\n", + ); + + let mismatched_iclosable = preserved_iclosable.replace( + "Windows.Foundation.IClosable:30d5a829-7fa4-4026-83bb-d75bae4ea99e", + "Fabrikam.IClosable:11111111-1111-1111-1111-111111111111", + ); + fs::write(output.join("IClosable.js"), mismatched_iclosable).unwrap(); + let before_rejection = snapshot_directory(&output); + let rejected = run_codegen(&output, "MemoryBuffer", false); + assert!(!rejected.status.success()); + assert!( + String::from_utf8_lossy(&rejected.stderr) + .contains("Refusing to overwrite shared interface source") + ); + assert_eq!(snapshot_directory(&output), before_rejection); + + fs::remove_dir_all(output).unwrap(); +} + #[test] fn default_projection_preserves_legacy_standalone_interface_imports() { let interface = value_interface(); From 602c329ae7d03659b02b94bc6ea12e374a08182d Mon Sep 17 00:00:00 2001 From: "Leilei Zhang (from Dev Box)" Date: Wed, 12 Aug 2026 22:47:22 +0800 Subject: [PATCH 6/8] Fix Node 22 shared interface re-exports Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 84688dea-d965-4933-a2d9-41f8f6ad2f2e --- .../src/codegen/winrt/javascript/render/javascript/mod.rs | 2 +- .../tests/shared_interface_members_test.rs | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/render/javascript/mod.rs b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/render/javascript/mod.rs index 9255272c..c8cf78be 100644 --- a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/render/javascript/mod.rs +++ b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/render/javascript/mod.rs @@ -39,7 +39,7 @@ pub fn render(file: &ProjectedFile) -> String { cjs.push('\n'); } cjs.push_str(&format!( - "Object.defineProperty(exports, '{}', {{ enumerable: true, get: () => require('{}').{} }});\n", + "exports.{} = require('{}').{};\n", re_export.name, re_export.from, re_export.name, )); } diff --git a/tools/dynwinrt-codegen/tests/shared_interface_members_test.rs b/tools/dynwinrt-codegen/tests/shared_interface_members_test.rs index f3b04fbf..a73e097e 100644 --- a/tools/dynwinrt-codegen/tests/shared_interface_members_test.rs +++ b/tools/dynwinrt-codegen/tests/shared_interface_members_test.rs @@ -193,9 +193,7 @@ fn cli_shared_uri_preserves_deep_raw_interface_exports() { assert!(baseline_js.contains("exports.IStringable = IStringable;")); assert!(baseline_dts.contains("export declare class IStringable")); - assert!(shared_js.contains( - "Object.defineProperty(exports, 'IStringable', { enumerable: true, get: () => require('./IStringable.js').IStringable });" - )); + assert!(shared_js.contains("exports.IStringable = require('./IStringable.js').IStringable;")); assert!(shared_dts.contains("export { IStringable } from './IStringable.js';")); assert!(shared_js.contains("return (__get_IStringable()).from(this._obj).toString();")); assert!( @@ -253,7 +251,7 @@ fn cli_mixed_flag_incremental_generation_preserves_shared_sources() { assert_node_script( &output, "verify-deferral.cjs", - "require('./Deferral.js');\n", + "Object.assign(globalThis, require('./runtime.js'));\nrequire('./Deferral.js');\n", ); let unflagged_result = run_codegen(&output, "MemoryBuffer", false); @@ -275,7 +273,7 @@ fn cli_mixed_flag_incremental_generation_preserves_shared_sources() { assert_node_script( &output, "verify-deferral-after-unflagged.cjs", - "require('./Deferral.js');\n", + "Object.assign(globalThis, require('./runtime.js'));\nrequire('./Deferral.js');\n", ); let mismatched_iclosable = preserved_iclosable.replace( From 25ec773d5c98c21fb41ed07de0c0b4c2d6d37df2 Mon Sep 17 00:00:00 2001 From: "Leilei Zhang (from Dev Box)" Date: Thu, 13 Aug 2026 12:59:21 +0800 Subject: [PATCH 7/8] Clarify shared interface declarations Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b485b75e-2292-44e1-98c8-3f7bb6489bff --- README.md | 17 ++++++++++------- tools/dynwinrt-codegen/npm/README.md | 16 +++++++++------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 99d59b0b..bb073c10 100644 --- a/README.md +++ b/README.md @@ -179,7 +179,7 @@ For each WinRT class the codegen emits a typed wrapper, factory, interface regis ### Shared interface members Large JavaScript projections can opt into shared inherited-interface -implementations without changing concrete class declarations or member names: +implementations while preserving the public declaration API and import paths: ```powershell dynwinrt-codegen generate ` @@ -192,12 +192,15 @@ dynwinrt-codegen generate ` The generated concrete prototypes receive the same method and accessor descriptors from standalone shared interface prototypes. Overloaded or conflicting members remain class-local, raw interface wrapper classes remain -available, and the option does not change generated `.d.ts` files. Only -required interfaces already canonicalized as standalone shared wrappers -participate; one-off inline required interfaces remain class-local. Generation -without this flag is unchanged. If a standalone interface filename is -ambiguous between distinct interface identities, none of those identities -participate in sharing and their inherited members remain class-local. +available, and the public declaration API and import paths are preserved. Class +deep modules may replace inline required-interface declarations with equivalent +re-exports from canonical standalone interface declarations. Only required +interfaces already canonicalized as standalone shared wrappers participate; +one-off inline required interfaces remain class-local. Generation without this +flag remains byte-compatible with the default output. If a standalone +interface filename is ambiguous between distinct interface identities, none of +those identities participate in sharing and their inherited members remain +class-local. Focused validation: diff --git a/tools/dynwinrt-codegen/npm/README.md b/tools/dynwinrt-codegen/npm/README.md index 5d377bda..a3f9d4af 100644 --- a/tools/dynwinrt-codegen/npm/README.md +++ b/tools/dynwinrt-codegen/npm/README.md @@ -71,13 +71,15 @@ npx dynwinrt-codegen generate ` The generated concrete prototypes reuse method and accessor descriptors from the standalone required-interface prototypes. Overloaded or conflicting -members remain class-local, raw interface wrappers remain available, generated -`.d.ts` files are unchanged, and generation without the flag is unchanged. -Only interfaces already emitted as canonical standalone shared wrappers -participate; one-off inline required interfaces remain class-local. Generation -excludes every identity behind a standalone interface filename that is -ambiguous between distinct interface identities; those inherited members -remain class-local. +members remain class-local, and raw interface wrappers remain available. The +public declaration API and import paths are preserved. Class deep modules may +replace inline required-interface declarations with equivalent re-exports from +canonical standalone interface declarations. Only interfaces already emitted +as canonical standalone shared wrappers participate; one-off inline required +interfaces remain class-local. Generation without the flag remains +byte-compatible with the default output. Generation excludes every identity +behind a standalone interface filename that is ambiguous between distinct +interface identities; those inherited members remain class-local. ## What gets generated From e4b5512f692760ad1b39bb70f6e910afcb631ac3 Mon Sep 17 00:00:00 2001 From: "Leilei Zhang (from Dev Box)" Date: Thu, 13 Aug 2026 13:22:07 +0800 Subject: [PATCH 8/8] Preserve shared interface module layout Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b485b75e-2292-44e1-98c8-3f7bb6489bff --- .../codegen/winrt/javascript/project/mod.rs | 44 +- tools/dynwinrt-codegen/src/main.rs | 468 ++++++++++-------- .../tests/shared_interface_members_test.rs | 8 +- 3 files changed, 299 insertions(+), 221 deletions(-) diff --git a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/project/mod.rs b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/project/mod.rs index f09d9e43..bc06d201 100644 --- a/tools/dynwinrt-codegen/src/codegen/winrt/javascript/project/mod.rs +++ b/tools/dynwinrt-codegen/src/codegen/winrt/javascript/project/mod.rs @@ -98,20 +98,23 @@ pub fn ambiguous_standalone_interface_names<'a>( .collect() } +/// Build the flat interface-module plan using the legacy emission order, then +/// annotate only safe final winners as descriptor-sharing sources. pub fn canonical_interface_sources( interfaces: &[InterfaceMeta], - shared_candidates: &[InterfaceMeta], + standalone_candidates: &[InterfaceMeta], + shared_member_candidates: &[InterfaceMeta], class_names: &HashSet, excluded_shared_source_names: &HashSet, forced_shared_source_identities: &HashSet, ) -> Result, String> { - let mut shared_identities = shared_candidates + let mut shared_identities = shared_member_candidates .iter() .filter_map(standalone_interface_identity) .collect::>(); shared_identities.extend(forced_shared_source_identities.iter().cloned()); let ambiguous_names = ambiguous_standalone_interface_names( - shared_candidates + standalone_candidates .iter() .chain(interfaces) .filter(|iface| !class_names.contains(&iface.name)), @@ -119,35 +122,19 @@ pub fn canonical_interface_sources( let mut sources: Vec = Vec::new(); let mut source_by_name: HashMap = HashMap::new(); - for iface in shared_candidates.iter().chain(interfaces) { + for iface in standalone_candidates.iter().chain(interfaces) { if class_names.contains(&iface.name) { continue; } let Some(identity) = standalone_interface_identity(iface) else { continue; }; - let ambiguous = ambiguous_names.contains(&iface.name) - || excluded_shared_source_names.contains(&iface.name); - let shared_member_source = !ambiguous && shared_identities.contains(&identity); if let Some(existing_index) = source_by_name.get(&iface.name).copied() { - let existing = &mut sources[existing_index]; - if ambiguous { - *existing = CanonicalInterfaceSource { - interface: iface.clone(), - identity, - shared_member_source: false, - }; - continue; - } - if existing.identity != identity { - *existing = CanonicalInterfaceSource { - interface: iface.clone(), - identity, - shared_member_source: false, - }; - continue; - } - existing.shared_member_source |= shared_member_source; + sources[existing_index] = CanonicalInterfaceSource { + interface: iface.clone(), + identity, + shared_member_source: false, + }; continue; } @@ -155,10 +142,15 @@ pub fn canonical_interface_sources( sources.push(CanonicalInterfaceSource { interface: iface.clone(), identity, - shared_member_source, + shared_member_source: false, }); } + for source in &mut sources { + source.shared_member_source = !ambiguous_names.contains(&source.interface.name) + && !excluded_shared_source_names.contains(&source.interface.name) + && shared_identities.contains(&source.identity); + } sources.sort_by(|left, right| left.interface.name.cmp(&right.interface.name)); Ok(sources) } diff --git a/tools/dynwinrt-codegen/src/main.rs b/tools/dynwinrt-codegen/src/main.rs index a0a17924..f91bbabc 100644 --- a/tools/dynwinrt-codegen/src/main.rs +++ b/tools/dynwinrt-codegen/src/main.rs @@ -956,6 +956,57 @@ fn run() -> Result<(), String> { Ok(()) } +struct RequiredInterfacePlans { + standalone_iids: HashSet, + standalone_interfaces: Vec, + shared_member_candidates: Vec, +} + +fn required_interface_plans<'a>( + classes: impl IntoIterator, +) -> RequiredInterfacePlans { + let mut legacy_counts: HashMap = HashMap::new(); + let mut identity_counts: HashMap< + project::StandaloneInterfaceIdentity, + (&meta::InterfaceMeta, usize), + > = HashMap::new(); + for class in classes { + for interface in &class.required_interfaces { + if interface.iid.is_empty() { + continue; + } + legacy_counts + .entry(interface.iid.clone()) + .and_modify(|(_, count)| *count += 1) + .or_insert((interface, 1)); + if let Some(identity) = project::standalone_interface_identity(interface) { + identity_counts + .entry(identity) + .and_modify(|(_, count)| *count += 1) + .or_insert((interface, 1)); + } + } + } + + RequiredInterfacePlans { + standalone_iids: legacy_counts + .iter() + .filter(|(_, (_, count))| *count >= 2) + .map(|(iid, _)| iid.clone()) + .collect(), + standalone_interfaces: legacy_counts + .into_values() + .filter(|(_, count)| *count >= 2) + .map(|(interface, _)| interface.clone()) + .collect(), + shared_member_candidates: identity_counts + .into_values() + .filter(|(_, count)| *count >= 2) + .map(|(interface, _)| interface.clone()) + .collect(), + } +} + /// Generate files for a set of types plus their transitive dependencies. /// When `dry_run` is true, all parsing/resolution runs but no files are written. fn generate_for_types( @@ -1061,29 +1112,9 @@ fn generate_for_types( .map(|i| i.name.clone()) .collect(); - let mut req_iface_count: HashMap = HashMap::new(); - for class in &all_classes { - for ri in &class.required_interfaces { - if ri.iid.is_empty() { - continue; - } - req_iface_count - .entry(ri.iid.clone()) - .and_modify(|(_, c)| *c += 1) - .or_insert((ri, 1)); - } - } - let shared_iids: HashSet = req_iface_count - .iter() - .filter(|(_, (_, count))| *count >= 2) - .map(|(iid, _)| iid.clone()) - .collect(); - - let shared_interfaces: Vec = req_iface_count - .iter() - .filter(|(_, (_, count))| *count >= 2) - .map(|(_, (iface, _))| (*iface).clone()) - .collect(); + let required_interface_plans = required_interface_plans(&all_classes); + let shared_iids = required_interface_plans.standalone_iids; + let shared_interfaces = required_interface_plans.standalone_interfaces; for iface in &shared_interfaces { known_types.insert(iface.name.clone()); } @@ -1093,26 +1124,7 @@ fn generate_for_types( let shared_interface_members_enabled = lang == "js" && project::shared_interface_members_enabled(); let canonical_shared_interfaces = if shared_interface_members_enabled { - let mut required_interface_count: HashMap< - project::StandaloneInterfaceIdentity, - (&meta::InterfaceMeta, usize), - > = HashMap::new(); - for class in &all_classes { - for req_iface in &class.required_interfaces { - let Some(identity) = project::standalone_interface_identity(req_iface) else { - continue; - }; - required_interface_count - .entry(identity) - .and_modify(|(_, count)| *count += 1) - .or_insert((req_iface, 1)); - } - } - required_interface_count - .values() - .filter(|(_, count)| *count >= 2) - .map(|(iface, _)| (*iface).clone()) - .collect::>() + required_interface_plans.shared_member_candidates } else { Vec::new() }; @@ -1120,31 +1132,40 @@ fn generate_for_types( known_types.insert(iface.name.clone()); } let mut effective_excluded_shared_source_names = excluded_shared_source_names.clone(); + let mut effective_reserved_non_interface_output_names = + reserved_non_interface_output_names.clone(); for en in &all_enums { if let TypeMeta::Enum { name, .. } = en && !name.contains('<') && !class_names_all.contains(name) { effective_excluded_shared_source_names.insert(name.clone()); + effective_reserved_non_interface_output_names.insert(name.clone()); } } - let canonical_interface_sources = if shared_interface_members_enabled { - Some(project::canonical_interface_sources( - &all_interfaces, - &canonical_shared_interfaces, - &class_names_all, - &effective_excluded_shared_source_names, - forced_shared_source_identities, - )?) + let empty_forced_shared_source_identities = HashSet::new(); + let effective_forced_shared_source_identities = if shared_interface_members_enabled { + forced_shared_source_identities } else { - None + &empty_forced_shared_source_identities + }; + let interface_emission_plan = project::canonical_interface_sources( + &all_interfaces, + &shared_interfaces, + &canonical_shared_interfaces, + &class_names_all, + &effective_excluded_shared_source_names, + effective_forced_shared_source_identities, + )?; + let shared_interface_source_identities = if shared_interface_members_enabled { + interface_emission_plan + .iter() + .filter(|source| source.shared_member_source) + .map(|source| source.identity.clone()) + .collect::>() + } else { + HashSet::new() }; - let shared_interface_source_identities = canonical_interface_sources - .iter() - .flatten() - .filter(|source| source.shared_member_source) - .map(|source| source.identity.clone()) - .collect::>(); if !dry_run { if lang == "py" { @@ -1163,16 +1184,13 @@ fn generate_for_types( generate_js_files( output_dir, &all_classes, - &all_interfaces, &all_enums, - &shared_interfaces, &known_types, &delegate_type_names, - canonical_interface_sources.as_deref(), + &interface_emission_plan, &shared_iids, &shared_interface_source_identities, - &effective_excluded_shared_source_names, - reserved_non_interface_output_names, + &effective_reserved_non_interface_output_names, &delegate_signatures, &delegate_sig_refs, &delegate_param_wraps, @@ -1484,35 +1502,21 @@ fn preflight_js_generation_batch( _ => None, }) .collect::>(); + let mut effective_reserved_non_interface_output_names = + reserved_non_interface_output_names.clone(); + effective_reserved_non_interface_output_names + .extend(enum_names.iter().map(|name| (*name).to_string())); reject_non_interface_overwrites_of_shared_sources( output_dir, all_classes .iter() .map(|class| class.name.as_str()) - .chain(enum_names), + .chain(enum_names.iter().copied()), )?; - if project::shared_interface_members_enabled() { - let mut required_interface_count: HashMap< - project::StandaloneInterfaceIdentity, - (&meta::InterfaceMeta, usize), - > = HashMap::new(); - for class in &all_classes { - for req_iface in &class.required_interfaces { - let Some(identity) = project::standalone_interface_identity(req_iface) else { - continue; - }; - required_interface_count - .entry(identity) - .and_modify(|(_, count)| *count += 1) - .or_insert((req_iface, 1)); - } - } - let canonical_shared_interfaces = required_interface_count - .into_values() - .filter(|(_, count)| *count >= 2) - .map(|(iface, _)| iface.clone()) - .collect::>(); + let required_interface_plans = required_interface_plans(all_classes.iter().copied()); + let all_interfaces = all_interfaces.into_iter().cloned().collect::>(); + let interface_emission_plan = if project::shared_interface_members_enabled() { let mut effective_excluded_names = excluded_shared_source_names.clone(); effective_excluded_names.extend(all_enums.iter().filter_map(|typ| match typ { TypeMeta::Enum { name, .. } if !name.contains('<') && !class_names.contains(name) => { @@ -1520,27 +1524,42 @@ fn preflight_js_generation_batch( } _ => None, })); - let all_interfaces = all_interfaces.into_iter().cloned().collect::>(); - let canonical_sources = project::canonical_interface_sources( + project::canonical_interface_sources( &all_interfaces, - &canonical_shared_interfaces, + &required_interface_plans.standalone_interfaces, + &required_interface_plans.shared_member_candidates, &class_names, &effective_excluded_names, forced_shared_source_identities, - )?; + )? + } else { + project::canonical_interface_sources( + &all_interfaces, + &required_interface_plans.standalone_interfaces, + &[], + &class_names, + &HashSet::new(), + &HashSet::new(), + )? + }; + + if project::shared_interface_members_enabled() { plan_existing_shared_interface_sources( output_dir, - &canonical_sources, - reserved_non_interface_output_names, + &interface_emission_plan, + &effective_reserved_non_interface_output_names, )?; } else { plan_unflagged_existing_shared_interface_sources( output_dir, - all_interfaces.into_iter().filter(|iface| { - !class_names.contains(&iface.name) - && !reserved_non_interface_output_names.contains(&iface.name) - && !iface.iid.is_empty() - }), + interface_emission_plan + .iter() + .map(|source| &source.interface) + .filter(|iface| { + !class_names.contains(&iface.name) + && !effective_reserved_non_interface_output_names.contains(&iface.name) + && !iface.iid.is_empty() + }), )?; } Ok(()) @@ -1549,15 +1568,12 @@ fn preflight_js_generation_batch( fn generate_js_files( output_dir: &Path, all_classes: &[meta::ClassMeta], - all_interfaces: &[meta::InterfaceMeta], all_enums: &[TypeMeta], - shared_interfaces: &[meta::InterfaceMeta], known_types: &HashSet, delegate_type_names: &HashSet, - canonical_interface_sources: Option<&[project::CanonicalInterfaceSource]>, + interface_emission_plan: &[project::CanonicalInterfaceSource], standalone_interface_iids: &HashSet, shared_member_source_identities: &HashSet, - excluded_interface_import_names: &HashSet, reserved_non_interface_output_names: &HashSet, delegate_sigs: &HashMap, delegate_sig_refs: &HashMap>, @@ -1604,22 +1620,22 @@ fn generate_js_files( .chain(enum_output_names), )?; - let preserved_shared_source_names = canonical_interface_sources - .map(|canonical_sources| { - plan_existing_shared_interface_sources( - output_dir, - canonical_sources, - reserved_non_interface_output_names, - ) - }) - .transpose()? - .unwrap_or_default(); - let unflagged_preserved_shared_source_identities = if canonical_interface_sources.is_none() { + let shared_interface_members_enabled = project::shared_interface_members_enabled(); + let preserved_shared_source_names = if shared_interface_members_enabled { + plan_existing_shared_interface_sources( + output_dir, + interface_emission_plan, + reserved_non_interface_output_names, + )? + } else { + HashSet::new() + }; + let unflagged_preserved_shared_source_identities = if !shared_interface_members_enabled { plan_unflagged_existing_shared_interface_sources( output_dir, - shared_interfaces + interface_emission_plan .iter() - .chain(all_interfaces) + .map(|source| &source.interface) .filter(|iface| { !class_names.contains(iface.name.as_str()) && !reserved_non_interface_output_names.contains(&iface.name) @@ -1630,83 +1646,33 @@ fn generate_js_files( HashSet::new() }; - if let Some(canonical_sources) = canonical_interface_sources { - for source in canonical_sources { - let iface = &source.interface; - if reserved_non_interface_output_names.contains(&iface.name) { - continue; - } - if preserved_shared_source_names.contains(&iface.name) { - continue; - } - let projected = project::project_interface_with_shared_member_source( - iface, - known_types, - delegate_type_names, - delegate_sigs, - delegate_sig_refs, - delegate_param_wraps, - source.shared_member_source, - ); - let js = render_js::render(&projected); - let dts = render_dts::render(&projected); - emit(&iface.name, &js, &dts)?; + for source in interface_emission_plan { + let iface = &source.interface; + if reserved_non_interface_output_names.contains(&iface.name) { + continue; } - } else { - for iface in shared_interfaces { - if class_names.contains(iface.name.as_str()) { - continue; - } - if reserved_non_interface_output_names.contains(&iface.name) { - continue; - } - if !is_emittable_interface(iface) { - continue; - } - let preserve_shared_source = - project::standalone_interface_identity(iface).is_some_and(|identity| { - unflagged_preserved_shared_source_identities.contains(&identity) - }); - let projected = project::project_interface_with_shared_member_source( - iface, - known_types, - delegate_type_names, - delegate_sigs, - delegate_sig_refs, - delegate_param_wraps, - preserve_shared_source, - ); - let js = render_js::render(&projected); - let dts = render_dts::render(&projected); - emit(&iface.name, &js, &dts)?; + if preserved_shared_source_names.contains(&iface.name) { + continue; } - for iface in all_interfaces { - if class_names.contains(iface.name.as_str()) { - continue; - } - if reserved_non_interface_output_names.contains(&iface.name) { - continue; - } - if !is_emittable_interface(iface) { - continue; - } - let preserve_shared_source = - project::standalone_interface_identity(iface).is_some_and(|identity| { - unflagged_preserved_shared_source_identities.contains(&identity) - }); - let projected = project::project_interface_with_shared_member_source( - iface, - known_types, - delegate_type_names, - delegate_sigs, - delegate_sig_refs, - delegate_param_wraps, - preserve_shared_source, - ); - let js = render_js::render(&projected); - let dts = render_dts::render(&projected); - emit(&iface.name, &js, &dts)?; + if !is_emittable_interface(iface) { + continue; } + let preserve_shared_source = + project::standalone_interface_identity(iface).is_some_and(|identity| { + unflagged_preserved_shared_source_identities.contains(&identity) + }); + let projected = project::project_interface_with_shared_member_source( + iface, + known_types, + delegate_type_names, + delegate_sigs, + delegate_sig_refs, + delegate_param_wraps, + source.shared_member_source || preserve_shared_source, + ); + let js = render_js::render(&projected); + let dts = render_dts::render(&projected); + emit(&iface.name, &js, &dts)?; } for en in all_enums { if let TypeMeta::Enum { name, .. } = en { @@ -1755,7 +1721,7 @@ fn generate_js_files( delegate_type_names, standalone_interface_iids, shared_member_source_identities, - excluded_interface_import_names, + reserved_non_interface_output_names, delegate_sigs, delegate_sig_refs, delegate_param_wraps, @@ -3943,23 +3909,23 @@ mod tests { .filter(|source| source.shared_member_source) .map(|source| source.identity.clone()) .collect(); - generate_js_files( + project::set_shared_interface_members(true); + let result = generate_js_files( output, &[], &[], - &[], - &[], &known_types, &HashSet::new(), - Some(sources), + sources, &HashSet::new(), &shared_source_identities, &HashSet::new(), - &HashSet::new(), &HashMap::new(), &HashMap::new(), &HashMap::new(), - ) + ); + project::set_shared_interface_members(false); + result } #[test] @@ -4153,6 +4119,120 @@ mod tests { fs::remove_dir_all(output).unwrap(); } + #[test] + fn legacy_standalone_layout_survives_nonshareable_identity_planning() { + let iid = "11111111-1111-1111-1111-111111111111"; + let first_interface = meta::InterfaceMeta { + name: "IValue".into(), + namespace: "Contoso".into(), + iid: iid.into(), + methods: vec![meta::MethodMeta { + name: "get_Value".into(), + raw_name: "get_Value".into(), + vtable_index: 6, + return_type: Some(TypeMeta::I32), + is_property_getter: true, + ..Default::default() + }], + ..Default::default() + }; + let mut second_interface = first_interface.clone(); + second_interface.namespace = "Fabrikam".into(); + let classes = vec![ + meta::ClassMeta { + name: "Widget".into(), + namespace: "Contoso".into(), + full_name: "Contoso.Widget".into(), + required_interfaces: vec![first_interface], + ..Default::default() + }, + meta::ClassMeta { + name: "Gadget".into(), + namespace: "Fabrikam".into(), + full_name: "Fabrikam.Gadget".into(), + required_interfaces: vec![second_interface], + ..Default::default() + }, + ]; + let required_interfaces = required_interface_plans(&classes); + assert_eq!( + required_interfaces.standalone_iids, + HashSet::from([iid.into()]) + ); + assert_eq!(required_interfaces.standalone_interfaces.len(), 1); + assert!(required_interfaces.shared_member_candidates.is_empty()); + + let class_names = classes + .iter() + .map(|class| class.name.clone()) + .collect::>(); + let excluded_shared_sources = HashSet::from(["IValue".into()]); + let emission_plan = project::canonical_interface_sources( + &[], + &required_interfaces.standalone_interfaces, + &required_interfaces.shared_member_candidates, + &class_names, + &excluded_shared_sources, + &HashSet::new(), + ) + .unwrap(); + assert_eq!(emission_plan.len(), 1); + assert_eq!(emission_plan[0].interface.name, "IValue"); + assert!(!emission_plan[0].shared_member_source); + + let root = Path::new(env!("CARGO_MANIFEST_DIR")) + .join("target") + .join(format!("legacy-standalone-layout-{}", std::process::id())); + let _ = fs::remove_dir_all(&root); + let baseline = root.join("baseline"); + let shared = root.join("shared"); + fs::create_dir_all(&baseline).unwrap(); + fs::create_dir_all(&shared).unwrap(); + let known_types = HashSet::from(["Widget".into(), "Gadget".into(), "IValue".into()]); + for (output, enabled) in [(&baseline, false), (&shared, true)] { + project::set_shared_interface_members(enabled); + generate_js_files( + output, + &classes, + &[], + &known_types, + &HashSet::new(), + &emission_plan, + &required_interfaces.standalone_iids, + &HashSet::new(), + &HashSet::new(), + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + ) + .unwrap(); + } + project::set_shared_interface_members(false); + + for class_name in ["Widget", "Gadget"] { + let baseline_js = + fs::read_to_string(baseline.join(format!("{class_name}.js"))).unwrap(); + let shared_js = fs::read_to_string(shared.join(format!("{class_name}.js"))).unwrap(); + let baseline_dts = + fs::read_to_string(baseline.join(format!("{class_name}.d.ts"))).unwrap(); + let shared_dts = fs::read_to_string(shared.join(format!("{class_name}.d.ts"))).unwrap(); + assert_eq!(shared_js, baseline_js); + assert_eq!(shared_dts, baseline_dts); + assert!(shared_js.contains("require('./IValue.js')")); + assert!(!shared_js.contains("class IValue")); + assert!(!shared_dts.contains("export declare class IValue")); + } + assert_eq!( + fs::read(shared.join("IValue.js")).unwrap(), + fs::read(baseline.join("IValue.js")).unwrap() + ); + assert_eq!( + fs::read(shared.join("IValue.d.ts")).unwrap(), + fs::read(baseline.join("IValue.d.ts")).unwrap() + ); + fs::remove_dir_all(root).unwrap(); + } + #[test] fn python_duplicate_short_names_use_namespace_facades() { let output = test_directory("namespace-facades"); diff --git a/tools/dynwinrt-codegen/tests/shared_interface_members_test.rs b/tools/dynwinrt-codegen/tests/shared_interface_members_test.rs index a73e097e..ca4e30e2 100644 --- a/tools/dynwinrt-codegen/tests/shared_interface_members_test.rs +++ b/tools/dynwinrt-codegen/tests/shared_interface_members_test.rs @@ -435,6 +435,7 @@ fn canonical_interface_source_survives_equivalent_duplicate_emission() { let sources = project::canonical_interface_sources( &[duplicate.clone()], std::slice::from_ref(&shared), + std::slice::from_ref(&shared), &HashSet::new(), &HashSet::new(), &HashSet::new(), @@ -443,7 +444,7 @@ fn canonical_interface_source_survives_equivalent_duplicate_emission() { assert_eq!(sources.len(), 1); assert!(sources[0].shared_member_source); - assert_eq!(sources[0].interface.iid, shared.iid); + assert_eq!(sources[0].interface.iid, duplicate.iid); project::set_shared_interface_members(true); let interface_file = project::project_interface_with_shared_member_source( @@ -491,6 +492,7 @@ fn equivalent_cross_batch_emission_preserves_shared_source_marker() { let sources = project::canonical_interface_sources( std::slice::from_ref(&shared), &[], + &[], &HashSet::new(), &HashSet::new(), &forced_shared_sources, @@ -518,6 +520,7 @@ fn non_interface_output_collision_disables_forced_shared_source() { let interface = value_interface(); let identity = project::standalone_interface_identity(&interface).unwrap(); let sources = project::canonical_interface_sources( + std::slice::from_ref(&interface), std::slice::from_ref(&interface), std::slice::from_ref(&interface), &HashSet::new(), @@ -577,6 +580,7 @@ fn ambiguous_standalone_interface_identities_remain_class_local() { assert_eq!(ambiguous_names, HashSet::from(["IValue".into()])); let combined_sources = project::canonical_interface_sources( + &[first.clone(), second.clone()], &[first.clone(), second.clone()], &[first.clone(), second.clone()], &HashSet::new(), @@ -604,6 +608,7 @@ fn ambiguous_standalone_interface_identities_remain_class_local() { project::set_shared_interface_members(true); for (interface, class) in [(&first, &first_class), (&second, &second_class)] { let sources = project::canonical_interface_sources( + std::slice::from_ref(interface), std::slice::from_ref(interface), std::slice::from_ref(interface), &HashSet::new(), @@ -670,6 +675,7 @@ fn nonshared_duplicate_interface_keeps_legacy_final_source() { let sources = project::canonical_interface_sources( &[first, second.clone()], &[], + &[], &HashSet::new(), &HashSet::new(), &HashSet::new(),