[release/6.0.3xx] [NET Attribute Conversion][generator] Generate NET style attributes #14877
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- The integration with the existing generator code is not ideal in this commit, but given time pressures it will hopefully be improved in the future. - GetPlatformAttributesToPrint is repeating some of the logic that is #if !NET'ed out in places such as PrintPlatformAttributes because those checks were not robust enough for the NET6 attribute logic (we generate many more 'duplicate' attributes now)
…remove now unnecessary ignores on other tests
- This is an actual API inclusion, not just attribute change. - All of the other defines, and attributes imply that GetCurrentInputDevice() was expected to be on catalyst, but the define on 433 did not include it
Co-authored-by: Rolf Bjarne Kvinge <[email protected]>
Co-authored-by: Rolf Bjarne Kvinge <[email protected]>
…re lower cased in one place but not another)
- Generate Obsolete/Advice and Unsupported pairs for Deprecated/Obsoleted instead of a single attribute - A number of generator improvements and test/binding fixes
…mespaceNotIncluded always assuming we're core
vs-mobiletools-engineering-service2
requested review from
dalexsoto,
rolfbjarne,
mandel-macaque and
chamons
as code owners
vs-mobiletools-engineering-service2
added
backported
do-not-merge
rolfbjarne
approved these changes
May 3, 2022
📋 [PR Build] API Diff 📋API Current PR diffℹ️ API Diff (from PR only) (please review changes) View dotnet API diffView dotnet legacy API diffAPI diff✅ API Diff from stable View dotnet API diffView dotnet legacy API diffGenerator diffℹ️ Generator Diff (please review changes) Pipeline on Agent XAMBOT-1030.Monterey' |
dalexsoto
approved these changes
May 3, 2022
❌ [PR Build] Tests on macOS Mac Catalina (10.15) failed ❌Failed tests are:
Pipeline on Agent |
mandel-macaque
approved these changes
May 4, 2022
|
I just tested this against 4 NET ported sames TJ pointed out had warnings and the largest MAUI sample (https://github.com/microsoft/dotnet-podcasts) and there were no spurious availability warnings. This is good enough to go in. |
chamons
approved these changes
May 4, 2022
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
?? the final large piece of the saga that is #10170
We won't know until landing this and testing the result but it's the last known hole.....
Summary
This PR teaches our code generator to generate .NET 6 style availability attributes, adds a 4th Cecil test to verify our generated attributes, and a metric ton of API changes to satisfy that test.
Details
The generator work is the core of this PR, and includes:
FilterMinimumVersionandPrintPlatformAttributesIs64BitiOSOnly.GetPlatformAttributesToPrint, which synthesizes many attributes "out of thing air" from:PrintPlatformAttributesNoDuplicatesandGeneratePropertybecause the existing PrintPlatformAttributes did not pass down parent context down, and the refactor was dangerous/too time consuming given time pressure.There are two intended API changes introduced by the reviews in this PR:
GetCurrentInputDevicewas obviously intended by availability attributes to exist on Catalyst but due to define confusion was excluded. It is an addition inMicrosoft.MacCatalyst.dllonly.NEAppRuleconstructors were mis-marked on platforms, and were showing up incorrectly on Mac/Catalyst. I corrected the Catalyst one unconditionally, as we have not shipped Catalyst yet, but Mac is only fixed in NET6.Known API changes
Limitations
I could spend another week or two polishing this, at minimum. It is being landed in this state due to time pressure.
There are bugs in the generated attributes:
Each of these has cases hit that are Ignored in test code (see
IgnoreElementsThatDoNotExistInThatAssembly).Other things to note (and forgive) include:
#ifhacks in generator.cs, which are signs that my hacked in code did not gel well with the existing logic. I should in theory extend each check to better handle the NET6 availability attribute reality and then remove logic from GetPlatformAttributesToPrint.Despite this, given time pressure and test coverage I think it is ready to go in.
I've filed #14802 to track the cleanup work from this PR.
Backport of #14779