-
Notifications
You must be signed in to change notification settings - Fork 547
[RGen] Add an analyzer error for views that miss initWithFrame. #23788
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Add a new analyzer error that will ensure that the classes that inherit from UIView or NSView do have a initWithFrame: constructor exposed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a new analyzer error (RBI0041) to ensure that classes inheriting from UIView or NSView expose the required initWithFrame:
constructor. This is a binding requirement for view classes to properly support initialization with frame rectangles.
Key changes include:
- Extended type inheritance tracking to identify view classes
- Added new analyzer validation for required constructors
- Comprehensive test coverage for both UIView and NSView scenarios
Reviewed Changes
Copilot reviewed 12 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
TypeSymbolExtensions.Core.cs |
Added view detection logic and updated inheritance tracking method signature |
SemanticModelExtensions.Generator.cs |
Extended symbol data extraction to include TypeInfo parameter |
TypeInfo.cs & TypeInfo.Transformer.cs |
Added IsView property to track view inheritance |
Binding.cs & Binding.Generator.cs |
Added TypeInfo field to binding data model |
ClassValidator.cs |
Implemented validation logic for required constructors |
RgenDiagnostics.cs & Resources.resx |
Added RBI0041 diagnostic descriptor and messages |
Test files | Added comprehensive test coverage for the new analyzer functionality |
Files not reviewed (1)
- src/rgen/Microsoft.Macios.Bindings.Analyzer/Resources.Designer.cs: Language not supported
/// </summary> | ||
public bool IsView { | ||
get => isView; | ||
init => isView= value; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing space before assignment operator. Should be isView = value;
Copilot uses AI. Check for mistakes.
diagnostic: analyzerDiagnotics [0], | ||
diagnosticId: "RBI0041", | ||
severity: DiagnosticSeverity.Error, | ||
message: "The class 'TestClass' inherits from UIView but does not expose a 'initWithFrame:' constructor"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message is incorrect for NSView test case. It should say 'inherits from NSView' instead of 'inherits from UIView' for the NSViewInitWithFrameMissing test.
message: "The class 'TestClass' inherits from UIView but does not expose a 'initWithFrame:' constructor"); | |
message: "The class 'TestClass' inherits from NSView but does not expose a 'initWithFrame:' constructor"); |
Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <[email protected]>
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.
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.
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.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did we miss this in our existing bindings somewhere?
Also I'm not sure this is correct: there's no reason a UIView subclass can't offer a different constructor (say "initWithFrame:someOtherOption:" and then call UIView's "initWithFrame:" ctor)
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.
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.
1. Remove old comment and provide a correct one 2. Provide the protocol name in the error RI00041
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ [CI Build #769e2f2] Build passed (Build packages) ✅Pipeline on Agent |
✅ [PR Build #769e2f2] Build passed (Detect API changes) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ 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 #769e2f2] Build passed (Build macOS tests) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
💻 [CI Build #769e2f2] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
💻 [CI Build #769e2f2] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
💻 [CI Build #769e2f2] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
💻 [CI Build #769e2f2] Tests on macOS arm64 - Mac Sequoia (15) passed 💻✅ All tests on macOS arm64 - Mac Sequoia (15) passed. Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
🚀 [CI Build #769e2f2] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 115 tests passed 🎉 Tests counts✅ cecil: All 1 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
Add a new analyzer error that will ensure that the classes that inherit from UIView or NSView do have a initWithFrame: constructor exposed.