[RGen] Ensure that a protocol is correctly generated when it has its name changed. #23521
Conversation
Remove the need to create a heap Nullable object for the FieldInfo attribute in properties, this is done to reduce the GC pressure and improve performance. A follow up PR will do the same for the ExportData attr.
Co-authored-by: Copilot <[email protected]>
Remove the boxing that happens when we use Nullable in the ExportData structure. This reduces the GC preasure by esnuring that there are no refs to the struct from a ref object.
Add the implementation for the protocol members. We do this by adding a new transformer in the Method class that will return a extension method to be use for a protocol and then rely in the logic used to write category extension methods.
…struct. When implementing the protocol wrappers we need to implement ALL properties and methods from the interface, to do so we need access to the parent protocols data. This commit adds two new properties in the Binding struct that will be used to keep track of the properties and methods of the parents. We also provide a method that returns and IEnumerable of the parent members and the needed upedats to construct the Method and Property structs.
…r.cs Co-authored-by: Copilot <[email protected]>
Add the Protocol Wrapper class definition. Do not generate methods or properties yet, do that in follow up PRs because we need changes in the property and method emitters.
We need to add the following changes: - Add helper methods to convert a protocol method to a wrapper method, mainly remove not needed modifiers. - Add export attributes just for wrapper methods context is a protocol and method is not a extension. - EmitMethod will use the send directly when we are generating a wrapper method. - Add missing protocol member attrs for methods.
Add the emittion of the default constructor for the protocol wrappers.
Refactor the code that emits the properties so that it can later be reused for the protocol wrapper classes. This is the same path we took with the method emition.
Add the following changes to reuse the current property emitter: - Add export if we are generating the property for a protocol. - Add direct send generation if we are generating a property for a protocol. - Add a helper method that will return a property for a wrapper by removing the not needed modifiers. - Add a helper method to add the export attr for a property.
…nsions.cs Co-authored-by: Copilot <[email protected]>
If a protocol has been marked as needing a model the generator now creates the model class and provides the model implementation for the methods and properties. Two protocol examples have been added to the tests, one for properties and a second one for methods.
Refactor the code to reuse the class emitter to generate the NSObject default constructors in a model.
Allow to choose the name of the model that will be generated for a protocol.
…name changed. Ensure that if we set the name of a protocol not to match that one in objc that we correcly generate and register the protocol and its wrapper/model. - Ensure that the correct name is used in the protocl attr. - The protocol wrapper should use the C# internface name as a prefix, not the name of the objc protocol. - The model should use the name of the C# interface without the I. We have new tests that make sure that the code works when: - We have a Name set, no model. - We have a Name set with a model.
mandel-macaque
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR fixes protocol generation issues when a protocol has a custom name that doesn't match the Objective-C protocol name. It ensures correct naming is used for the protocol attribute, wrapper class, and model class.
Key Changes
- Added extension method to get protocol wrapper name using C# interface name as prefix
- Fixed model name generation to use C# interface name without 'I' prefix instead of Objective-C protocol name
- Added comprehensive test coverage for protocols with custom names both with and without models
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
ProtocolGenerationTests.cs |
Added test cases for custom-named protocols with and without models |
INSUrlSessionDelegate.cs |
New test input file for custom-named protocol without model |
INSUrlSessionDelegateWithModel.cs |
New test input file for custom-named protocol with model |
ExpectedINSUrlSessionDelegate.cs |
Expected output for custom-named protocol without model |
ExpectedINSUrlSessionDelegateWithModel.cs |
Expected output for custom-named protocol with model |
ProtocolWrapperEmitter.cs |
Updated to use new extension method for wrapper name |
ProtocolEmitter.cs |
Updated to use new extension method for wrapper name |
ModelEmitter.cs |
Fixed model name generation logic |
BindingContext.cs |
Added extension method to get protocol wrapper name |
✅ [CI Build #f9471e7] Build passed (Build packages) ✅Pipeline on Agent |
✅ [PR Build #f9471e7] Build passed (Detect API changes) ✅Pipeline on Agent |
✅ [CI Build #f9471e7] Build passed (Build macOS tests) ✅Pipeline on Agent |
💻 [CI Build #f9471e7] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
💻 [CI Build #f9471e7] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
💻 [CI Build #f9471e7] Tests on macOS arm64 - Mac Sequoia (15) passed 💻✅ All tests on macOS arm64 - Mac Sequoia (15) passed. Pipeline on Agent |
💻 [CI Build #f9471e7] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
✅ API diff for current PR / commit.NET ( No breaking changes )✅ API diff vs stable.NET ( No breaking changes )ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
🔥 [CI Build #f9471e7] Test results 🔥Test results❌ Tests failed on VSTS: test results 1 tests crashed, 0 tests failed, 110 tests passed. Failures❌ monotouch tests (macOS)🔥 Failed catastrophically on VSTS: test results - monotouch_macos (no summary found). Html Report (VSDrops) Download Successes✅ cecil: All 1 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
|
Failing tests are unrelated to code changes. |
Ensure that if we set the name of a protocol not to match that one in objc that we correcly generate and register the protocol and its wrapper/model.
We have new tests that make sure that the code works when: