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

Automatically add NullableAttributes support to netstandard2.0 projects that enable NRT. #1188

Merged
merged 1 commit into from
Mar 11, 2024

Conversation

jpobst
Copy link
Contributor

@jpobst jpobst commented Feb 8, 2024

netstandard2.0 doesn't have "built-in" compiler support for NRT attributes. Thus when we enable NRT on netstandard2.0 projects we are responsible for providing the attributes ourselves.

Rather than manually repeat this info in each netstandard2.0 .csproj, we can use Directory.Build.targets to automatically set the attributes up in projects that need it.

Additionally, we can mark it as Visible="false" so the file doesn't show up in Visual Studio Solution Explorer, just like the "real" "built-in" attributes.

@jpobst jpobst force-pushed the auto-nrt-attributes branch from b5fb7cb to 2a45727 Compare February 8, 2024 21:07
@jpobst jpobst force-pushed the auto-nrt-attributes branch from 2a45727 to 11ff8d0 Compare March 11, 2024 18:05
@jpobst jpobst force-pushed the auto-nrt-attributes branch from 11ff8d0 to 4636ad2 Compare March 11, 2024 18:08
@jpobst jpobst marked this pull request as ready for review March 11, 2024 18:30
@jpobst jpobst requested a review from jonpryor March 11, 2024 18:30
@jonpryor jonpryor merged commit 5bca8ad into main Mar 11, 2024
4 checks passed
@jonpryor jonpryor deleted the auto-nrt-attributes branch March 11, 2024 19:17
jonpryor pushed a commit to dotnet/android that referenced this pull request Mar 13, 2024
Change: dotnet/java-interop@3436a30...5bca8ad

  * dotnet/java-interop@5bca8ad6: [build] Automatically add NullableAttributes.cs for netstandard2.0 (dotnet/java-interop#1188)
  * dotnet/java-interop@45437e22: [Java.Interop] suppress IL3050 with `#pragma` (dotnet/java-interop#1201)
  * dotnet/java-interop@1c9c8c9c: [Java.Interop.Tools.Maven] Initial commit. (dotnet/java-interop#1179)

With dotnet/java-interop@5bca8ad6, all instances of nullable
attributes types defined in `NullableAttributes.cs` are now `internal`.

However, `Xamarin.Android.Build.Tasks` was consuming the `public`
types from its reference to `Java.Interop.Tools.JavaCallableWrappers`,
so this change resulted in a build break:

	MavenExtensions.cs(26,32): error CS0122: 'NotNullWhenAttribute' is inaccessible due to its protection level
	MamJsonParser.cs(92,43): error CS0122: 'NotNullWhenAttribute' is inaccessible due to its protection level
	MamJsonParser.cs(92,81): error CS0122: 'NotNullWhenAttribute' is inaccessible due to its protection level

We can apply almost the same logic from dotnet/java-interop@5bca8ad6
to `xamarin-android`; the difference is that neither of the
`netstandard2.0` projects in xamarin-android that need the attributes
have `$(Nullable)='enabled'` and thus our `Condition` fails.  

(`Xamarin.Android.Build.Tasks.csproj` and
`Xamarin.Android.Tools.JavadocImporter.csproj` include `.cs` files from
`external/Java.Interop` that have been NRT annotated and thus need the
nullable attribute types.)

We add `$(Nullable)='annotations'` to the `@(Compile)` which
allows one to use NRT syntax but does not emit any NRT warnings since
these assemblies have not been converted.  We then modify the
`NullableAttributes.cs` inclusion logic to additionally key off the
`$(Nullable)=annotations` value.

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Jonathan Pobst <jonathan.pobst@microsoft.com>
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants