Skip to content

[Mono.Android] Fix build warnings in hand-written code - #12003

Merged
jonathanpeppers merged 20 commits into
mainfrom
dev/simonrozsival/fix-mono-android-build-warnings
Jul 13, 2026
Merged

[Mono.Android] Fix build warnings in hand-written code#12003
jonathanpeppers merged 20 commits into
mainfrom
dev/simonrozsival/fix-mono-android-build-warnings

Conversation

@simonrozsival

Copy link
Copy Markdown
Member

Supersedes #11999 (that PR was from a fork; this one is rebased on latest main and pushed to an origin branch so it runs the full CI). #11999 will be closed.

Summary

Reduces the C# build warnings in src/Mono.Android by fixing the warnings that originate in hand-written source, and completes the API-37 public API baseline so the public API analyzer passes.

It intentionally does not remove every NoWarn suppression, nor the Mono.Android.csproj entry in _AllowProjectWarnings (in Directory.Build.props). The warnings that remain all originate in generated code and require changes to the code generators — see "Why suppressions remain" below.

What is fixed (hand-written code)

  • CS8765 - override parameter nullability aligned to the base member (System.DrawingTypeConverter subclasses, SyncContext, Java stream adapters, Color/AndroidBitmapInfo equality, AdapterView.RawAdapter).
  • CS8764 - JavaObject.ToString() returns non-null to match Java.Lang.Object.ToString().
  • CS8767 - interface-implementation parameter nullability aligned to the interface (IComparer, ISpliterator, ISetCookie, IX509TrustManager/ISSLSession, IXmlPullParser).
  • CS0114 / CS0108 - new added to members that intentionally hide an inherited member (generic collection specializations, static helpers, IXmlResourceParser disambiguation, StringBuffer/StringBuilder.Append, AsyncTask.class_ref, JavaProxyThrowable.InnerException, SparseArray<E>, ArrayAdapter<T>, JavaList.Equals).

Public API baseline (RS0016 / RS0017)

Some of the nullability fixes change the annotations of public members, and the API-37 baseline was missing a large number of generated binding members. PublicAPI/API-37/PublicAPI.Unshipped.txt is regenerated (via the RS0016 code fix, plus *REMOVED* markers for stale signatures) so RS0016/RS0017 are satisfied and remain enforced (they are not in NoWarn).

Why suppressions remain

The remaining warnings are all in generated code and cannot be fixed here without changing the generators:

  • mcw/*.cs (binding generator): CS0108, CS0114, CS0618, CS0809, CS8613, CS8764, CS8765, CS8766, CS8767.
  • JNIEnv.g.cs (jnienv-gen, // Generated file; DO NOT EDIT!): RS0041 (public members using oblivious reference types).

These files are regenerated on every build, so the fix must happen in the generator. Until those follow-ups land, we keep the corresponding codes in <NoWarn> and keep <_AllowProjectWarnings Condition=" '$(MSBuildProjectFile)' == 'Mono.Android.csproj' ">true</_AllowProjectWarnings>. Removing either now would turn hundreds of generated-code warnings into build errors under TreatWarningsAsErrors.

Follow-ups

Once those land, the remaining NoWarn codes and the _AllowProjectWarnings opt-in for Mono.Android.csproj can be removed.

Match the nullability of overridden base-class parameters (TypeConverter, SynchronizationContext, Java stream adapters, Color/AndroidBitmapInfo equality, AdapterView.RawAdapter) so overrides no longer widen non-nullable base parameters, guarding newly-nullable arguments where the body dereferences them.
JavaObject.ToString() overrides Java.Lang.Object.ToString() (non-null), so return a non-null string instead of string?.
Match the parameter nullability of the implemented interface members (IComparer, ISpliterator, ISetCookie, IX509TrustManager/ISSLSession, IXmlPullParser). Guard TrustManager callbacks with ArgumentNullException.ThrowIfNull and return null early from GetNamespace for a null prefix.
Add the 'new' keyword to members that intentionally hide an inherited virtual member (SparseArray<E>.Get/ValueAt, ArrayAdapter<T>.GetItem/Sort, JavaList.Equals(Java.Lang.Object)).
Add the 'new' keyword to members that intentionally hide an inherited member: generic collection specializations (JavaCollection/JavaList/JavaSet/JavaDictionary/ArrayAdapter/BaseAdapter), static helpers (FromJniHandle/ToLocalJniHandle/CreateFromResource), IXmlResourceParser members disambiguating IAttributeSet/IXmlPullParser, StringBuffer/StringBuilder.Append, AsyncTask.class_ref, and JavaProxyThrowable.InnerException.
The API-37 public API baseline was missing ~4570 members (generated bindings plus a few hand-written types), and the nullability fixes in earlier commits changed the annotations of several public members. Populate PublicAPI.Unshipped.txt via the RS0016 code fix and add *REMOVED* markers for the stale signatures (RS0017) so the PublicApiAnalyzer passes with RS0016/RS0017 enforced.
Guard CheckClientTrusted params (consistency with CheckServerTrusted); keep the clr_typemap_managed_to_java P/Invoke param non-nullable and guard the call site when Type.FullName is null; drop the redundant null-forgiving operator in AdapterView.RawAdapter getter; replace null-forgiving operators in ValueAnimator/AnimatorSet marshaling with explicit null checks that throw a clear exception.
main removed JavaPeerProxy's invokerType constructor parameter and InvokerType property and added Java.Interop.BaseExportAttribute; the rebased constructor-visibility change also exposes the JavaPeerProxy constructors as protected. Update PublicAPI.Unshipped.txt (API 37) accordingly so RS0016/RS0017 stay clean.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR reduces build warnings in src/Mono.Android by aligning nullability annotations in hand-written code with base members/interfaces, tightening up a few marshaling paths, and updating the API-37 public API baselines so the public API analyzer passes.

Changes:

  • Updated a number of overrides/interface implementations to match expected nullable signatures and added targeted null checks where appropriate.
  • Adjusted dynamic-code vs AOT code paths (notably typemap/collection conversion) and replaced/added specific suppressions so IL3050 can be removed from project-wide NoWarn.
  • Regenerated/cleaned PublicAPI baseline files (API-36.1/API-37) to keep RS0016/RS0017 enforced.
Show a summary per file
FileDescription
src/Mono.Android/Xamarin.Android.Net/ServerCertificateCustomValidator.csAlign trust manager / SSL session method nullability with interfaces and add argument validation.
src/Mono.Android/Xamarin.Android.Net/AndroidMessageHandler.csAlign helper method parameter nullability with HttpRequestException/WebException usage.
src/Mono.Android/System.Drawing/SizeFConverter.csAlign TypeConverter override nullability and handle null CultureInfo.
src/Mono.Android/System.Drawing/SizeConverter.csAlign TypeConverter override nullability and handle null CultureInfo.
src/Mono.Android/System.Drawing/RectangleConverter.csAlign TypeConverter override nullability and handle null CultureInfo.
src/Mono.Android/System.Drawing/PointConverter.csAlign TypeConverter override nullability.
src/Mono.Android/PublicAPI/API-37/PublicAPI.Shipped.txtRemove duplicate baseline entries.
src/Mono.Android/PublicAPI/API-36.1/PublicAPI.Shipped.txtRemove duplicate baseline entries.
src/Mono.Android/Org.Apache.Http.Impl.Cookie/BasicClientCookie.csAlign cookie setter nullability with interface expectations.
src/Mono.Android/Mono.Android.csprojRemove IL3050 from NoWarn now that code is suppressed/guarded more precisely.
src/Mono.Android/Microsoft.Android.Runtime/TrimmableTypeMapTypeManager.csRoute array-type generation based on dynamic-code support vs pre-generated typemap proxies; add RequiresDynamicCode.
src/Mono.Android/Microsoft.Android.Runtime/ManagedTypeManager.csAdd IL3050 suppression for reflection-based type manager base.
src/Mono.Android/Microsoft.Android.Runtime/JniRemappingLookup.csSuppress CS0649 for struct fields used as interop shapes.
src/Mono.Android/Microsoft.Android.Runtime/JavaMarshalRegisteredPeers.csSuppress CS0649 for interop struct fields.
src/Mono.Android/Java.Util/Spliterators.csAlign ISpliterator-related nullability in TryAdvance.
src/Mono.Android/Java.Lang/StringBuilder.csMark intentional member hiding with new.
src/Mono.Android/Java.Lang/StringBuffer.csMark intentional member hiding with new.
src/Mono.Android/Java.Interop/TypeManager.csImprove nullability handling in typemap paths (e.g., pointer-to-string conversion, class-name lookup).
src/Mono.Android/Java.Interop/JavaPeerProxy.csAdjust proxy attribute ctor accessibility and validate ctor arguments.
src/Mono.Android/Java.Interop/JavaConvert.csGuard reflection-based generic collection conversion behind dynamic-code support and add RequiresDynamicCode.
src/Mono.Android/Android.Widget/BaseAdapter.csMark intentional member hiding with new.
src/Mono.Android/Android.Widget/ArrayAdapter.csMark intentional member hiding with new for members that shadow base APIs.
src/Mono.Android/Android.Widget/AdapterView.csUpdate RawAdapter nullability and remove null-forgiving in getter.
src/Mono.Android/Android.Util/SparseArray.csMark intentional member hiding with new.
src/Mono.Android/Android.Runtime/XmlReaderPullParser.csAlign nullability to IXmlPullParser expectations; add defensive null handling.
src/Mono.Android/Android.Runtime/XmlPullParserReader.csMark intentional member hiding with new.
src/Mono.Android/Android.Runtime/OutputStreamAdapter.csAlign override nullability and add argument validation.
src/Mono.Android/Android.Runtime/JNIEnv.csRefine dynamic-code/AOT array creation behavior and use targeted IL3050 suppression only when required.
src/Mono.Android/Android.Runtime/JavaSet.csMark intentional member hiding with new.
src/Mono.Android/Android.Runtime/JavaProxyThrowable.csMark intentional member hiding with new.
src/Mono.Android/Android.Runtime/JavaObject.csEnsure ToString() returns non-null to match Java expectations.
src/Mono.Android/Android.Runtime/JavaList.csMark intentional member hiding with new.
src/Mono.Android/Android.Runtime/JavaDictionary.csMark intentional member hiding with new.
src/Mono.Android/Android.Runtime/JavaCollection.csMark intentional member hiding with new.
src/Mono.Android/Android.Runtime/InputStreamAdapter.csAlign override nullability and add argument validation.
src/Mono.Android/Android.OS/AsyncTask.csMark intentional member hiding with new.
src/Mono.Android/Android.Graphics/Color.csAlign override nullability for marshaler APIs.
src/Mono.Android/Android.Graphics/AndroidBitmapInfo.csAlign Equals(object) nullability.
src/Mono.Android/Android.Content.Res/IXmlResourceParser.csDisambiguate interface conflicts with new members.
src/Mono.Android/Android.Bluetooth/BluetoothDevice.csAdd OS-platform obsoletion metadata for API-37.
src/Mono.Android/Android.App/SyncContext.csAlign SynchronizationContext override nullability.
src/Mono.Android/Android.App/AlertDialog.csSwitch to EventHelper helper to align event wiring nullability.
src/Mono.Android/Android.Animation/ValueAnimator.csMake delegate field nullable and add null checks for marshaling results.
src/Mono.Android/Android.Animation/AnimatorSet.csMake delegate field nullable and add null checks for marshaling results.

Copilot's findings

Comments suppressed due to low confidence (1)

src/Mono.Android/Android.Graphics/Color.cs:451

  • Avoid the null-forgiving operator when reflecting for the Color(int) constructor. Even if it "should" exist, trimming/linking (or future refactors) can make this return null; an explicit check keeps the failure mode clear and avoids !.
 public override Expression CreateParameterToManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue, ParameterAttributes synchronize, Type? targetType)
{
var c = typeof (Color).GetConstructor (new[]{typeof (int)})!;
var v = Expression.Variable (typeof (Color), sourceValue.Name + "_val");
context.LocalVariables.Add (v);
context.CreationStatements.Add (Expression.Assign (v, Expression.New (c, sourceValue)));
  • Files reviewed: 44/45 changed files
  • Comments generated: 4

Comment on lines +132 to 135
protected override Java.Lang.Object? RawAdapter {
get { return JavaObjectExtensions.JavaCast<Java.Lang.Object>(JavaConvert.ToJavaObject (Adapter)); }
set { Adapter = JavaConvert.FromJavaObject<T>(value)!; }
}
Comment threadsrc/Mono.Android/System.Drawing/SizeFConverter.cs
Comment threadsrc/Mono.Android/Java.Interop/JavaPeerProxy.cs
@simonrozsival

Copy link
Copy Markdown
MemberAuthor

@copilot fix this warning:

Xamarin.Android.Build.Tests.BuildTest2.BuildHasNoWarnings(True,False,"apk",NativeAOT)
build.log should have exactly 2 MSBuild warnings for NativeAOT, but found 0.
Expected: 2
But was: 0

…ore expected ILC warnings
Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>

CopilotAI commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

@copilot fix this warning:

...

Fixed in the latest commit. The class-level [UnconditionalSuppressMessage ("Trimming", "IL3050")] I added to ManagedTypeManager was silencing the ILC warnings at the AOT analysis step, so the test found 0 instead of the expected 2. Removing that attribute lets the ILC emit the 2 IL3050 warnings (from ManagedTypeManager inheriting [RequiresDynamicCode]ReflectionJniTypeManager) that the test expects. The build stays clean because _AllowProjectWarnings=true for Mono.Android.csproj.

@simonrozsival

Copy link
Copy Markdown
MemberAuthor

@copilot what's still left to do?

CopilotAI commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

@copilot what's still left to do?

One open reviewer thread remains: the ! in the RawAdapter setter in AdapterView.cs (JavaConvert.FromJavaObject<T>(value)!). That ! was already present before this PR — I only changed the property from set to override to fix CS8765. If you'd like me to address it I can, otherwise the PR is otherwise complete:

  • ✅ CS8765/CS8764/CS8767/CS0114/CS0108 warnings fixed in hand-written code
  • ✅ API-37 public API baseline regenerated (RS0016/RS0017 passing)
  • ✅ IL3050 removed from NoWarn (replaced with targeted suppressions)
  • BuildHasNoWarnings(NativeAOT) test fix (restored expected 2 IL3050 ILC warnings)
  • ✅ All other reviewer threads resolved

simonrozsivaland others added 2 commits July 8, 2026 00:07
Use the AOT warning category for the ManagedTypeManager IL3050 suppression so NativeAOT analysis recognizes it.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep the ManagedTypeManager AOT IL3050 suppression needed for the NativeAOT build.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@simonrozsival

Copy link
Copy Markdown
MemberAuthor

/review

@github-actions

github-actionsBot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Android PR Reviewer completed successfully!

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 Automated review — PR #12003

Verdict: ✅ Clean, well-scoped change. This is a mechanical build-warnings cleanup by a maintainer; I found no bugs — only 3 minor 💡 suggestions posted inline (non-blocking).

What this PR does (independent read)

It removes IL3050 from <NoWarn> in Mono.Android.csproj and instead eliminates or precisely suppresses the underlying warnings in hand-written code:

  • Replaces the old IsCoreClrRuntime / IsNativeAotRuntime branch logic in JNIEnv.ArrayCreateInstance, JavaConvert.GetJniHandleConverter, and TrimmableTypeMapTypeManager.GetArrayTypes with RuntimeFeature.IsDynamicCodeSupported as the primary guard (a BCL [FeatureGuard(typeof(RequiresDynamicCodeAttribute))], so [RequiresDynamicCode] calls inside those blocks no longer trip IL3050).
  • Adds targeted [UnconditionalSuppressMessage] / [RequiresDynamicCode] where a suppression is genuinely required (e.g. the ManagedTypeMap path).
  • Silences a batch of nullability (CS8764/CS8765/CS8767) and member-hiding (CS0108/CS0114) warnings via signature alignment and the new keyword.
  • Correspondingly removes the NativeAOT "expect 2 IL3050 warnings" special-case in BuildTest2.BuildHasNoWarnings, which now asserts zero warnings for every runtime — a strengthened regression guard.

Verification notes

  • Behavior-preserving restructures: I traced the three rewritten methods across CoreCLR-JIT (IsDynamicCodeSupported == true), Mono (true), and NativeAOT (falseTrimmableTypeMap proxy path). Each new fail-fast throw NotSupportedException is only reachable in genuinely-unsupported configurations. ✔️
  • EventHelper rename in AlertDialog.cs is correct — AndroidEventHelper is [Obsolete("Use EventHelper")], and this matches the established pattern (MediaController, ViewTreeObserver, AccountManager, and the generator's InterfaceListenerEvent). ✔️
  • GetClassNameFromInstancestring? (public API): every product caller uses it in string interpolation/concatenation (null-safe); the 3 test callers in JnienvArrayMarshaling.cs assign to string, but that file has no #nullable enable and the test csproj only sets WarningsAsErrors=IL2037, so no CS8600 is produced. No downstream break. ✔️
  • Stream adapters / converters / new-hidden members: all mechanical alignments to base/interface signatures; ArgumentNullException.ThrowIfNull on the stream adapters matches Java NPE semantics. ✔️

CI

At review time the full Azure DevOps (dnceng-public) build — which actually validates the zero-warnings claim behind removing IL3050 from <NoWarn> — was still in progress with no failures observed (Java.Interop Tests on Windows/Mac already green; license/CLA green). Mergeability depends on those build stages completing green.

Inline suggestions (all 💡, non-blocking)

  1. JavaConvert.cs L109 — NotSupportedException message says "array element type" for a generic-collection branch.
  2. JavaProxyThrowable.cs L14 — awareness note on the now-explicit InnerException shadow (behavior unchanged).
  3. PointConverter.cs L67 — continuation-line indentation drifted out of alignment (recurs across the System.Drawing converters).

Nice, tidy cleanup overall. 👍

Generated by Android PR Reviewer for #12003 · 367.8 AIC · ⌖ 23.2 AIC · ⊞ 6.8K
Comment /review to run again

Comment threadsrc/Mono.Android/Java.Interop/JavaConvert.cs
Comment threadsrc/Mono.Android/Android.Runtime/JavaProxyThrowable.cs
Comment threadsrc/Mono.Android/System.Drawing/PointConverter.cs
The ManagedTypeManager AOT warning is now suppressed, so NativeAOT builds should be warning-free in these tests.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@simonrozsivalsimonrozsival added the ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable). label Jul 8, 2026
@jonathanpeppers
jonathanpeppers merged commit a1266b2 into mainJul 13, 2026
42 checks passed
@jonathanpeppers
jonathanpeppers deleted the dev/simonrozsival/fix-mono-android-build-warnings branch July 13, 2026 13:17
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Aug 13, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

ready-to-reviewThis PR is ready to review/merge, I think any CI failures are just flaky (ignorable).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@simonrozsival@jonathanpeppers