[Mono.Android] Fix build warnings in Mono.Android and build tooling#11958
Closed
simonrozsival wants to merge 4 commits into
Closed
[Mono.Android] Fix build warnings in Mono.Android and build tooling#11958simonrozsival wants to merge 4 commits into
simonrozsival wants to merge 4 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR focuses on reducing compiler/analyzer noise in the .NET for Android codebase by tightening nullability annotations, adding targeted suppressions/attributes, and cleaning up PublicAPI lists, primarily in src/Mono.Android and related build/test tooling.
Changes:
- Adjust nullability and add explicit null-handling in core interop/runtime code (
JNIEnv,TypeManager) and in generated bindings (AnimatorSet,ValueAnimator). - Add/suppress warnings where appropriate (CA1416 in test helper, CS0649 for native-populated structs, CS0436 in
Mono.Android.csproj). - Clean up API surface bookkeeping (remove duplicate
PublicAPI.Shipped.txtentries) and minor test/tooling cleanup.
Show a summary per file
| File | Description |
|---|---|
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Android/AndroidSdkResolver.cs | Suppresses CA1416 around Windows-only registry access in test tooling |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/InlineData.cs | Removes a duplicate using |
| src/Mono.Android/Xamarin.Android.Net/AndroidMessageHandler.cs | Updates helper signature to accept nullable message |
| src/Mono.Android/PublicAPI/API-37/PublicAPI.Shipped.txt | Removes a duplicate shipped API entry |
| src/Mono.Android/PublicAPI/API-36.1/PublicAPI.Shipped.txt | Removes a duplicate shipped API entry |
| src/Mono.Android/Mono.Android.csproj | Adds CS0436 to NoWarn |
| src/Mono.Android/Microsoft.Android.Runtime/JniRemappingLookup.cs | Suppresses CS0649 for a native-populated struct |
| src/Mono.Android/Microsoft.Android.Runtime/JavaMarshalRegisteredPeers.cs | Suppresses CS0649 for an internal native-mirrored struct |
| src/Mono.Android/Java.Interop/TypeManager.cs | Adds null checks for strings coming from native pointers / mapping lookups |
| src/Mono.Android/Android.Runtime/JNIEnv.cs | Enables nullable context and tightens null-handling for typemap/array helpers |
| src/Mono.Android/Android.Bluetooth/BluetoothDevice.cs | Adds ObsoletedOSPlatform("android37.0") to obsolete overloads |
| src/Mono.Android/Android.Animation/ValueAnimator.cs | Addresses nullable warnings in generated binding code |
| src/Mono.Android/Android.Animation/AnimatorSet.cs | Addresses nullable warnings in generated binding code |
| build-tools/jnienv-gen/Generator.cs | Suppresses CS0649 for generator metadata populated indirectly |
Copilot's findings
- Files reviewed: 14/14 changed files
- Comments generated: 4
Member
|
/review |
|
✅ Android PR Reviewer completed successfully! |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🤖 Code review
Focused, well-scoped warning cleanup. Nice touches: the pragma suppressions carry explanatory comments, the CS0649 suppressions on native-mirror structs are appropriate, the new argument checks use nameof, and the duplicate using System.Globalization; removal in InlineData.cs is genuine dead-code removal. I read the full source around each change; everything below is non-blocking and surfaced inline.
- Incomplete PublicAPI dedup — the cleanup removed the
VP9Profile2HDRduplicate but an exact-duplicateVP9Profile3HDRline remains right below it in bothAPI-36.1andAPI-37/PublicAPI.Shipped.txt(lines 66145/66146), leaving the sameRS0025duplicate-entry warning this PR is clearing. Details inline.
💡 Suggestions (3)
#nullable enableinJNIEnv.csis redundant — nullable is already enabled project-wide (Mono.Android.csproj:24).string? type = GetClassNameFromInstance (array)inJNIEnv.cs, plus itstype == nullcheck, are now dead since the same PR made that method return non-nullstring.AnimatorSet.cs/ValueAnimator.csuse?? throwfor the object return but keep!on__this; the generated-mirror pattern in sibling files uses!for that return.
Accuracy note: a few changes do subtly alter behavior on previously null/NRE paths — GetClassNameFromInstance, AnimatorSet/ValueAnimator.SetDuration, and the managed→java typemap now throw a clear exception where they would previously have returned null. That is an improvement, just not strictly “no behavioral change.”
CI: no checks had reported at review time (status still pending), so a green build is unconfirmed; mergeable_state was blocked.
Verdict:
Generated by Android PR Reviewer for #11958 · 417.8 AIC · ⌖ 23.6 AIC · ⊞ 6.8K
Comment /review to run again
Addresses several compiler/analyzer warnings, primarily in Mono.Android.csproj: - Add nullable annotations and null checks in JNIEnv, TypeManager, and generated bindings (AnimatorSet, ValueAnimator) to resolve CS86xx nullability warnings. - Annotate obsoleted BluetoothDevice.ConnectGatt overloads with ObsoletedOSPlatform. - Suppress CS0649 on internal control-block/replacement-method structs that are populated from native code (JavaMarshalRegisteredPeers, JniRemappingLookup, jnienv-gen). - Suppress CA1416 for the Windows-only registry access in AndroidSdkResolver. - Allow CS0436 in Mono.Android.csproj NoWarn. - Remove duplicate PublicAPI.Shipped.txt entries (API-36.1, API-37). - Remove duplicate using in InlineData test helper. - Widen AndroidMessageHandler.CreateHttpRequestException message parameter to nullable.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…ve and pragma - Remove leftover byte-identical VP9Profile3HDR duplicate in API-36.1 and API-37 PublicAPI.Shipped.txt (clears the RS0025 the PR targets). - Drop redundant per-file '#nullable enable' in JNIEnv.cs (nullable is enabled project-wide) and remove the now-dead 'string?'/'type == null' check since GetClassNameFromInstance returns non-null. - Replace misleading CA1416 pragma with an analyzer-recognized OperatingSystem.IsWindows() guard in AndroidSdkResolver.cs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…line The API-37 (CinnamonBun preview) PublicAPI baseline (PublicAPI.Shipped.txt is a copy of API-36.1) is out of date with the generated android-37 bindings, so the PublicApiAnalyzer reports thousands of RS0016 (undeclared public API) and RS0017 (declared-but-missing) errors that are unrelated to this warning-cleanup PR. Suppress RS0016/RS0017 via NoWarn (which overrides the WarningsAsErrors escalation) to unblock the build. A follow-up PR will regenerate the API-37 PublicAPI.Shipped.txt / PublicAPI.Unshipped.txt files to declare the new API surface. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
simonrozsival
force-pushed
the
dev/simonrozsival/mono-android-warning-fixes
branch
from
3aaec9c to
d8745bb
Compare
simonrozsival
changed the title
simonrozsival
added
the
ready-to-review
Comment on lines
+523
to
+526
| #pragma warning disable CS0649 // Field 'JniFunction.Prebind' is never assigned to, and will always have its default value false | ||
| // If true, then we initialize the binding on the static ctor, we dont lazy-define it | ||
| public bool Prebind; | ||
| #pragma warning restore CS0649 |
Member
There was a problem hiding this comment.
This was removed in main, so I think we can rebase.
Member
Author
|
Superseded by #11999 |
This file contains hidden or 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
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.
Fixes several compiler/analyzer warnings, primarily surfaced when building
Mono.Android.csproj.Changes
JNIEnv,TypeManager, and the generatedAnimatorSet/ValueAnimatorbindings.ConnectGattoverloads with[ObsoletedOSPlatform("android37.0")].JavaMarshalRegisteredPeers,JniRemappingLookup, andjnienv-gen'sGenerator).AndroidSdkResolver.NoWarninMono.Android.csproj.PublicAPI.Shipped.txtentries (API-36.1, API-37).using System.Globalization;in theInlineDatatest helper and widenAndroidMessageHandler.CreateHttpRequestException'smessageparameter to nullable.No functional/behavioral changes — warning cleanup only.