Skip to content
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

[dotnet][generator] Fix creating 3rd party bindings #11522

Merged
merged 1 commit into from
May 12, 2021

Conversation

spouliot
Copy link
Contributor

Internally the generator uses AvailabilityBaseAttribute to make its
decisions. For dotnet we generated the newer [SupportedOSPlatform]
and [UnsupportedOSPlatform].

A 3rd-party (dotnet) binding might refer to members decorated with the
newer attributes and fail the build with an error [1]. Avoiding the error
is easy (first block of the diff) but it does not make the right
decisions (e.g. if a member is unavailable for the platform) when
generating the code.

To fix this we need to be able to convert the new attributes into the
well know AvailabilityBaseAttribute subclasses. We have not spotted
such cases yet because

  • the bindings pass our tests (good but extra code would not fail)
  • API diff across legacy and dotnet is not yet done [2]

[1] #11497
[2] #10210

Internally the generator uses `AvailabilityBaseAttribute` to make its
decisions. For `dotnet` we generated the newer `[SupportedOSPlatform]`
and `[UnsupportedOSPlatform]`.

A 3rd-party (dotnet) binding might refer to members decorated with the
newer attributes and fail the build with an error [1]. Avoiding the error
is easy (first block of the diff) but it does not make the _right_
decisions (e.g. if a member is unavailable for the platform) when
generating the code.

To fix this we need to be able to convert the new attributes into the
well know `AvailabilityBaseAttribute` subclasses. We have not spotted
such cases yet because

* the bindings pass our tests (good but extra code would not fail)
* API diff across legacy and dotnet is not yet done [2]

[1] dotnet#11497
[2] dotnet#10210
@spouliot spouliot requested a review from dalexsoto as a code owner May 12, 2021 17:42
@spouliot spouliot added the not-notes-worthy Ignore for release notes label May 12, 2021
Copy link
Member

@dalexsoto dalexsoto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

default:
return Enumerable.Empty<System.Attribute> ();
}
}

#if NET
static (PlatformName, Version) ParseOSPlatformAttribute (string arg)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@vs-mobiletools-engineering-service2
Copy link
Collaborator

✅ [PR Build] Tests passed on Build. ✅

Tests passed on Build.

API diff

✅ API Diff from stable

View API diff

API & Generator diff

API Diff (from PR only) (no change)
ℹ️ Generator Diff (please review changes)

🎉 All 84 tests passed 🎉

Pipeline on Agent XAMBOT-1099.BigSur
Merge 995d993 into dc320a3

@spouliot spouliot merged commit 1026b3f into dotnet:main May 12, 2021
@spouliot spouliot deleted the gh11497 branch May 12, 2021 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
not-notes-worthy Ignore for release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BI1055 error is thrown for iOS Binding project after migrating to .NET6
4 participants