[bgen] Add [FactoryMethod] support for failable initializers. Fixes #26167. - #26196
Conversation
…26167. Add a new [FactoryMethod] binding attribute that instructs bgen to generate a static factory method (instead of a public constructor) from a failable Objective-C initializer. This replaces the manual boilerplate previously written by hand for such initializers. When [FactoryMethod] is applied to a constructor binding, bgen: 1. Emits the constructor as internal (hiding it from the public API). 2. Emits a public static factory method with the same parameters. The method name defaults to "Create" and can be customized via [FactoryMethod ("SomeName")]. The nullability of the factory method is derived from the nullability of the constructor's return value: * If the return value is nullable ([return: NullAllowed]), the factory method returns a nullable value, passes throwOnInitFailure: false to InitializeHandle, and returns null when the native initializer returns nil. * Otherwise the factory method returns a non-nullable value and just returns the newly created instance. If a [FactoryMethod] constructor has an 'out NSError' parameter but its return value isn't nullable, bgen emits a new warning (BI1125), since such a factory can't return null on failure. The 3-argument InitializeHandle overload in NSObject was internal, so it was only usable from the platform assembly. Generated factory methods (including in third-party bindings) need to call it with throwOnInitFailure: false, so make it protected internal. This matches the existing manual pattern (e.g. WKWebExtensionMatchPattern.Create). The xml documentation on a [FactoryMethod] constructor is emitted on the generated public factory method rather than the hidden internal constructor. Also add xml documentation to the new attribute and to the InitializeHandle overload, document the attribute and the new warning in docs/website, and add tests (including an xml-docs scenario and a BI1125 warning test). Fixes #26167 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1a8496e-64f7-444a-a436-7fa103b1c7cd
…selectors Extend the [FactoryMethod] attribute so it can be applied to a binding init method that is not a `Constructor` (a method that returns NativeHandle and is exported to an `init` selector). This is needed when a native class exposes two failable initializers with the same managed signature: they can't both be bound as constructors (C# doesn't allow two constructors with identical parameter types), so they're bound as named factory methods instead. For a named factory method the generator now emits an internal instance helper (prefixed with an underscore) that performs the `init` message send, plus a public static factory method (named after the binding method, or the attribute's MethodName) that allocates the instance, calls the helper via InitializeHandle and returns null when the native initializer fails (for nullable initializers). Also add error BI1126 when [FactoryMethod] is applied to a member whose selector isn't an Objective-C `init` selector (either `init` or `init` followed by an uppercase letter). Verified that [FactoryMethod] + [Internal] generates valid `internal static` code (no duplicate modifiers), and added a regression test for it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1a8496e-64f7-444a-a436-7fa103b1c7cd
…rameInterpolationConfiguration Two follow-ups for the [FactoryMethod] feature (#26167): * Error BI1127: specifying an explicit name (e.g. [FactoryMethod ("X")]) on a non-Constructor method is confusing/redundant, because the factory method is always named after the binding method itself. Report it as an error instead of silently ignoring the name. * Converted a real manual binding to use [FactoryMethod]: VTLowLatencyFrameInterpolationConfiguration had two hand-written static factory methods (CreateWithNumberOfInterpolatedFrames / CreateWithSpatialScaleFactor) wrapping two failable-shaped initializers with identical managed signatures. These are now generated from the binding via [FactoryMethod] on named methods (the exact scenario the non-Constructor support was added for), and the manual file was removed. Verified the generated API is identical to the previous manual code (same names, signatures, selectors, non-nullable return) and that it compiles into Microsoft.iOS.dll. This binding has monotouch-test coverage (VTLowLatencyFrameInterpolationConfigurationTest). Also augmented the factory-method-multiple bgen test with a non-nullable named factory (CreateWithBaz) to cover the non-failable init path used by the converted binding. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1a8496e-64f7-444a-a436-7fa103b1c7cd
There was a problem hiding this comment.
Pull request overview
Adds a new bgen [FactoryMethod] attribute to generate static factory methods from Objective-C initializers (especially failable init*), avoiding public constructors that can’t represent failure. Also converts an existing manual factory implementation in VideoToolbox to the new generator feature and extends bgen test coverage and documentation accordingly.
Changes:
- Implement
[FactoryMethod]in bgen, generating an internal backing initializer plus a public/static factory that can returnnullwhen[return: NullAllowed]is present. - Expand bgen tests to cover factory method generation scenarios, XML doc emission, and new diagnostics BI1125/BI1126/BI1127.
- Update VideoToolbox bindings to use
[FactoryMethod]and remove the now-redundant handwrittenVTLowLatencyFrameInterpolationConfigurationfactory partial.
Reviewed changes
Copilot reviewed 24 out of 25 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/bgen/tests/xmldocs.cs | Adds a [FactoryMethod] case to validate XML doc generation for the produced factory method. |
| tests/bgen/tests/factory-method.cs | New binding test input covering default/custom factory names and nullable vs non-nullable initializers. |
| tests/bgen/tests/factory-method-noninit.cs | New negative test input for BI1126 (non-init selector). |
| tests/bgen/tests/factory-method-named-error.cs | New negative test input for BI1127 (explicit name on non-constructor factory). |
| tests/bgen/tests/factory-method-multiple.cs | New test input for multiple same-signature initializers bound as named factories. |
| tests/bgen/tests/factory-method-internal.cs | New test input ensuring [Internal] produces an internal static factory method. |
| tests/bgen/tests/factory-method-error.cs | New test input for BI1125 (out NSError + non-nullable return). |
| tests/bgen/tests/ExpectedXmlDocs.tvOS.xml | Updates expected XML docs to include the generated T1.Create(...) member. |
| tests/bgen/tests/ExpectedXmlDocs.macOS.xml | Same as above for macOS. |
| tests/bgen/tests/ExpectedXmlDocs.MacCatalyst.xml | Same as above for Mac Catalyst. |
| tests/bgen/tests/ExpectedXmlDocs.iOS.xml | Same as above for iOS. |
| tests/bgen/ErrorTests.cs | Adds BI1126/BI1127 error assertions. |
| tests/bgen/BGenTests.cs | Adds IL-level assertions for factory method emission and BI1125 warning behavior. |
| src/VideoToolbox/VTLowLatencyFrameInterpolationConfiguration.cs | Removes manual factory implementation (now generated). |
| src/videotoolbox.cs | Converts VTLowLatencyFrameInterpolationConfiguration init bindings to [FactoryMethod]. |
| src/frameworks.sources | Removes the deleted manual partial file from the build input list. |
| src/Foundation/NSObject2.cs | Makes InitializeHandle(handle, selector, throwOnInitFailure) protected internal for generated/third-party factory patterns. |
| src/bgen/Models/MemberInformation.cs | Tracks factory-method-related rendering state (name, nullability, static-ness). |
| src/bgen/Generator.cs | Implements factory method generation, selector validation, and BI1125/BI1126/BI1127 diagnostics. |
| src/bgen/Attributes.cs | Introduces the [FactoryMethod] attribute and its documentation. |
| src/bgen/AttributeManager.cs | Registers FactoryMethodAttribute for attribute lookup. |
| src/Resources.resx | Adds BI1125/BI1126/BI1127 resource strings. |
| src/Resources.Designer.cs | Updates strongly-typed accessors for new BI1125-1127 resources. |
| docs/website/generator-errors.md | Documents new diagnostics BI1125/BI1126/BI1127. |
| docs/website/binding_types_reference_guide.md | Documents FactoryMethodAttribute usage and behavior. |
Files not reviewed (1)
- src/Resources.Designer.cs: Generated file
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Reword the BI1125 warning to refer to the "binding method" instead of a "constructor": the warning is also emitted for named init methods annotated with [FactoryMethod], not just constructors. Updated the message in Resources.resx/Resources.Designer.cs, the generator-errors.md documentation, and the BGenTests assertion. Also fixed the FactoryMethodAttribute xml docs: for a named (non-constructor) init method the factory method is always named after the binding method, and setting MethodName is an error (BI1127) — the docs previously claimed the name could be overridden via MethodName. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1a8496e-64f7-444a-a436-7fa103b1c7cd
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…l, no [Export]
The '_Create...' instance helper backing a named [FactoryMethod] init method
was generated as an internal, virtual method carrying the native 'init...'
[Export]. Exporting the selector registered it with the Objective-C runtime
and made introspection's ApiSelectorTest flag it ("init selector used on a
non-constructor"), and there was no reason for it to be overridable or
callable from outside the generated static factory method.
Generate these helpers as private (default accessibility), non-virtual, and
without an [Export] attribute. The helper is only ever called from the
generated static factory method on a freshly allocated instance, and the
message send uses the cached selector handle directly, so none of these are
needed. This also fixes the introspection failure at the source, so no
introspection-test change is required.
Backing constructors (for constructor-based factory methods) are unchanged
and remain internal so the factory method can reach them.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: bc7e84a3-7e86-47e8-aeda-216d8e2c8327
…bgen-add-support-for-generating-nullabl-b756c7
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…bgen-add-support-for-generating-nullabl-b756c7
This comment has been minimized.
This comment has been minimized.
Mark generated factory methods with BindingImpl metadata and teach xtro to read initializer selectors from their IL without exporting the methods. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6fa3ba12-13c8-4199-a6df-9a5cec00098b
…bgen-add-support-for-generating-nullabl-b756c7
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
🚀 [CI Build #1373c9c] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 203 tests passed 🎉 Tests counts✅ assembly-processing: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Monterey (12): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
Convert five hand-written failable-initializer factory helpers to use the new bgen `[FactoryMethod]` attribute (added in #26196). Each conversion replaces an `[Internal] NativeHandle _InitWith...` helper plus a manual partial-class factory method with a `[FactoryMethod] [return: NullAllowed] NativeHandle Constructor (...)` in the binding definition, letting the generator emit the `public static T? Create (...)` factory. The manual partial-class files are deleted and their entries removed from `frameworks.sources`. 🤖 Pull request created by Copilot --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Add a new
[FactoryMethod]binding attribute to bgen (the legacy generator)so a failable Objective-C initializer can be exposed as a
public staticfactory method instead of a public constructor. A public constructor can't
express failure — it always returns a non-null instance — so a nil result
from a failable
initwould otherwise throw. The factory method can insteadreturn
null.What the attribute does
Applied to a binding constructor, the generator emits the constructor as
internaland generates apublic staticfactory method alongside it(named
Createby default, or a name passed to the attribute):It can also be applied to a named init method (one that returns
NativeHandleand is exported to aninitselector) rather than aConstructor. This is required when a native class has two initializerswith the same managed signature — they can't both be bound as constructors
(C# forbids two constructors with identical parameter types):
In the named-init case the binding method becomes a private, non-virtual
backing helper (with no
[Export]) that performs theinitmessage send on afreshly allocated instance, and the
public staticfactory method is namedafter the binding method (setting a name via the attribute is an error, BI1127).
When the initializer's return value is nullable (
[return: NullAllowed]),the factory method returns a nullable value and yields
nullwhen the nativeinitializer fails; otherwise it returns a non-nullable value.
Diagnostics
[FactoryMethod]with anout NSErrorparameterbut a non-nullable return value (the factory can't return null on failure).
[FactoryMethod]applied to a non-initselector.[FactoryMethod](the binding method's own name is used, so a name is redundant/confusing).
Real-world conversion
Converted
VTLowLatencyFrameInterpolationConfigurationto use the attribute:its two hand-written static factories (
CreateWithNumberOfInterpolatedFrames/CreateWithSpatialScaleFactor, two same-signature initializers) are nowgenerated. The generated API is identical to the previous manual code, and the
binding has existing monotouch-test coverage.
Other changes
NSObject.InitializeHandle (handle, sel, throwOnInitFailure)changed frominternaltoprotected internalso generated bindings in other assemblies(including third-party) can call it — matching the existing manual pattern.
BindingImplOptions.FactoryMethodflag, and xtro-sharpie'sSelectorChecknow recognizes factory methods and extracts the
initselector from thefactory's IL. Because the named-init backing helper no longer carries an
[Export], this keeps extrospection validating the selector against Apple'sheaders.
docs/website/binding_types_reference_guide.mdand the new error/warning codes in
docs/website/generator-errors.md.internal, multiple, xml docs, and the BI1125/BI1126/BI1127 diagnostics).
Fixes #26167
🤖 Pull request created by Copilot