[RGen] Add validations for Category methods. #23791
Conversation
Add a new analyzer error that will ensure that the classes that inherit from UIView or NSView do have a initWithFrame: constructor exposed.
Add a validation that will ensure that category methods are: 1. Partial 2. Static 3. Extension methods 4. Extension methods for the correct type. Tests have been added to make sure that we indeed get the correct warnings/errors.
There was a problem hiding this comment.
Pull Request Overview
This PR adds validation to ensure category methods follow proper C# extension method conventions for Objective-C category bindings. It validates that category methods are partial, static, extension methods with the correct first parameter type matching the category's extended type.
- Added comprehensive validation logic for category method structure and type safety
- Created new diagnostic descriptors (RBI0042-RBI0044) for specific category method errors
- Added test coverage for all validation scenarios
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
CategoryAnalyzerTests.cs |
New test file with comprehensive test cases for category method validation scenarios |
CategoryValidator.cs |
Added ValidMethods validation logic and registered new diagnostic strategies |
RgenDiagnostics.cs |
Added three new diagnostic descriptors for category method validation errors |
Resources.resx |
Added resource strings for new diagnostic messages and descriptions |
Files not reviewed (1)
- src/rgen/Microsoft.Macios.Bindings.Analyzer/Resources.Designer.cs: Language not supported
| ); | ||
|
|
||
| /// <summary> | ||
| /// Disgnostic descriptor for when a category method is not partial |
There was a problem hiding this comment.
Typo in XML documentation comments: 'Disgnostic' should be 'Diagnostic' in all three instances.
Copilot uses AI. Check for mistakes.
| ); | ||
|
|
||
| /// <summary> | ||
| /// Disgnostic descriptor for when a category method is not and extension |
There was a problem hiding this comment.
Typo in XML documentation comments: 'Disgnostic' should be 'Diagnostic' in all three instances.
Copilot uses AI. Check for mistakes.
| ); | ||
|
|
||
| /// <summary> | ||
| /// Disgnostic descriptor for when a category method is not and extension of the category's target type |
There was a problem hiding this comment.
Typo in XML documentation comments: 'Disgnostic' should be 'Diagnostic' in all three instances.
Copilot uses AI. Check for mistakes.
| <value>Category methods have to be partial.</value> | ||
| </data> | ||
| <data name="RBI0042MessageFormat" xml:space="preserve"> | ||
| <value>The method '{0}' must me partial</value> |
There was a problem hiding this comment.
Two typos in resource strings: 'must me partial' should be 'must be partial' and 'categoyr' should be 'category'.
Copilot uses AI. Check for mistakes.
| </data> | ||
| <data name="RBI0044MessageFormat" xml:space="preserve"> | ||
| <value>Extension method '{0}' in category '{1}' must have the first parameter type match the category's extended type '{2}' found '{3}'</value> | ||
| <comment>{0} is the name of the method, {1} the name of the categoyr, {2} is the category type and {3} is the type found</comment> |
There was a problem hiding this comment.
Two typos in resource strings: 'must me partial' should be 'must be partial' and 'categoyr' should be 'category'.
Copilot uses AI. Check for mistakes.
🔥 [CI Build #a39e9ae] Test results 🔥Test results❌ Tests failed on VSTS: test results 1 tests crashed, 0 tests failed, 116 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 |
✅ [PR Build #775591f] Build passed (Detect API changes) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
🚀 [CI Build #e7bdc2e] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 119 tests passed 🎉 Tests counts✅ cecil: All 1 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
✅ [PR Build #d5747f1] Build passed (Detect API changes) ✅Pipeline on Agent |
✅ [CI Build #775591f] Build passed (Build packages) ✅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 |
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.
✅ [CI Build #775591f] Build passed (Build macOS tests) ✅Pipeline on Agent |
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 #775591f] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
💻 [CI Build #775591f] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
💻 [CI Build #775591f] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
💻 [CI Build #775591f] Tests on macOS arm64 - Mac Sequoia (15) passed 💻✅ All tests on macOS arm64 - Mac Sequoia (15) passed. Pipeline on Agent |
🚀 [CI Build #d5747f1] 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 validation that will ensure that category methods are:
Tests have been added to make sure that we indeed get the correct warnings/errors.