[RGen] Use the PlatformSupportVersion for the supported and unsupported versions. #23842
Conversation
When adding a platform to a symbol it might be the case that the addition has an implication for another platform. For example, when we add support for iOS we are adding support for Catalyst. This new struct keeps track of the reason why a version was added and will allow to choose always the Explicit verison when two versions are compared. This will later can be used with the Builder class to add Implicit versions for a platform and be able to calculate which version to use.
Do not change the behaviour just yet but use the new struct in the builder to ensure that we will later be able to add support to implicit/explicit versions.
…ed versions.
This commit has not behaviour change yet, we are keeping the same
behaviour but we are using the PlatformSupportVersion to later be able
to perform better decisions for situations such as:
```
pulib class Test {
[SupportedOSPlatform ("ios")]
pulic int Count { get; set; }
```
By using PlatformSupportVersion we will be able to check that Count is
only supported on iOS because the attr was added and its addition to the
other platforms was implicit. The decision making will be added in
comming PRs.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the platform availability system to use PlatformSupportVersion instead of raw Version objects for tracking supported and unsupported platform versions. This change enables better decision-making for platform availability scenarios by differentiating between explicit and implicit version declarations.
Key changes:
- Updated
PlatformAvailabilityclass to usePlatformSupportVersioninstead ofVersionfor supported/unsupported versions - Enhanced
PlatformSupportVersionwith comparison operators and nullable-aware static methods - Refactored test assertions to work with the new type structure
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
PlatformAvailabilityTests.cs |
Updated test assertions to access version through PlatformSupportVersion.Value.Version and use ExplicitDefault constant |
PlatformAvailabilityMergeTests.cs |
Modified test assertions to compare PlatformSupportVersion objects directly instead of extracting versions |
TabbedStringBuilderAvailability.cs |
Updated code generation to access version through PlatformSupportVersion.Value.Version property |
PlatformSupportVersion.cs |
Added IComparable interface and comparison operators, made static methods nullable-aware |
PlatformAvailabilityBuilder.cs |
Simplified ToImmutable() method to pass through PlatformSupportVersion objects directly |
PlatformAvailability.cs |
Changed property types from Version to PlatformSupportVersion and updated related logic |
|
|
||
| var availability = builder.ToImmutable (); | ||
| // get all the versions we added | ||
| var unsupported= availability.UnsupportedVersions.Keys |
There was a problem hiding this comment.
Missing space after assignment operator. Should be var unsupported = availability.UnsupportedVersions.Keys
| var unsupported= availability.UnsupportedVersions.Keys | |
| var unsupported = availability.UnsupportedVersions.Keys |
Copilot uses AI. Check for mistakes.
| /// <returns><c>true</c> if the left instance is greater than or equal to the right instance; otherwise, <c>false</c>.</returns> | ||
| public static bool operator >=(PlatformSupportVersion left, PlatformSupportVersion right) | ||
| { | ||
| return left.CompareTo(right) >= 0; |
There was a problem hiding this comment.
Inconsistent spacing around method call parentheses. Should be left.CompareTo (right) to match the pattern used in other comparison operators in this file.
| return left.CompareTo(right) >= 0; | |
| return left.CompareTo (right) >= 0; |
Copilot uses AI. Check for mistakes.
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.
✅ [CI Build #47f3c28] Build passed (Build macOS tests) ✅Pipeline on Agent |
❌ [CI Build #47f3c28] Tests on macOS X64 - Mac Sonoma (14) failed ❌Tests on macOS X64 - Mac Sonoma (14) failed for unknown reasons. Pipeline on Agent |
💻 [CI Build #47f3c28] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
💻 [CI Build #47f3c28] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
💻 [CI Build #47f3c28] 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.
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.
🚀 [CI Build #47f3c28] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 117 tests passed 🎉 Tests counts✅ cecil: All 1 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
🔥 [CI Build #06314f8] Build failed (Build packages) 🔥Build failed for the job 'Build packages' (with job status 'Failed') Pipeline on Agent |
🔥 [PR Build #06314f8] Build failed (Detect API changes) 🔥Build failed for the job 'Detect API changes' (with job status 'Failed') Pipeline on Agent |
|
🔥 Unable to find the contents for the comment: D:\a\1\s\change-detection\results\gh-comment.md does not exist :fire Pipeline on Agent |
This commit has not behaviour change yet, we are keeping the same behaviour but we are using the PlatformSupportVersion to later be able to perform better decisions for situations such as:
By using PlatformSupportVersion we will be able to check that Count is only supported on iOS because the attr was added and its addition to the other platforms was implicit. The decision making will be added in comming PRs.