Skip to content

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

Closed
simonrozsival wants to merge 15 commits into
dotnet:mainfrom
simonrozsival:dev/simonrozsival/fix-mono-android-build-warnings
Closed

[Mono.Android] Fix build warnings in hand-written code#11999
simonrozsival wants to merge 15 commits into
dotnet:mainfrom
simonrozsival:dev/simonrozsival/fix-mono-android-build-warnings

Conversation

@simonrozsival

Copy link
Copy Markdown
Member

Summary

This PR reduces the C# build warnings in src/Mono.Android by fixing the warnings that originate in hand-written source and by completing 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. The warnings that remain all originate in generated code, and fixing them requires changes to the code generators. Those are called out as follow-ups below.

What is fixed (hand-written code)

Each warning code was addressed in its own commit:

  • CS8765 – parameter nullability of overrides now matches the base member (TypeConverter subclasses in System.Drawing, SyncContext, the Java stream adapters, Color/AndroidBitmapInfo equality, AdapterView.RawAdapter).
  • CS8764JavaObject.ToString() returns a non-null string to match Java.Lang.Object.ToString().
  • CS8767 – interface-implementation parameter nullability now matches the interface (IComparer, ISpliterator, ISetCookie, IX509TrustManager/ISSLSession, IXmlPullParser).
  • CS0114 / CS0108 – added the new keyword to members that intentionally hide an inherited member (generic collection specializations, static FromJniHandle/ToLocalJniHandle/CreateFromResource helpers, IXmlResourceParser interface 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 above change the annotations of public members, and the API-37 baseline was also missing a large number of generated binding members. PublicAPI/API-37/PublicAPI.Unshipped.txt has been regenerated (via the RS0016 code fix, plus *REMOVED* markers for stale signatures) so that RS0016/RS0017 are satisfied and remain enforced (they are not in NoWarn).

Why NoWarn and _AllowProjectWarnings are kept

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

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

Because these files are regenerated on every build, hand-editing them does not stick — the fix has to happen in the generator. Until those follow-ups land we must:

  • keep the corresponding codes in <NoWarn> in Mono.Android.csproj, and
  • keep <_AllowProjectWarnings Condition=" '$(MSBuildProjectFile)' == 'Mono.Android.csproj' ">true</_AllowProjectWarnings> in Directory.Build.props.

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.
Copilot AI review requested due to automatic review settings July 7, 2026 09:09

Copilot AI 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 aims to reduce build warnings in src/Mono.Android by aligning nullability annotations with overridden/interface members, adding new where members intentionally hide inherited members, and updating PublicAPI baselines so the API analyzers (RS0016/RS0017) pass.

Changes:

  • Adjusted nullability and argument validation across various hand-written runtime, networking, and System.Drawing converter code paths to satisfy CS8764/CS8765/CS8767.
  • Added new on members that intentionally hide inherited members to address CS0108/CS0114.
  • Updated API shipped baselines and warning suppression configuration (NoWarn) to keep analyzers enforced while reducing warning noise.
Show a summary per file
File Description
src/Mono.Android/Xamarin.Android.Net/ServerCertificateCustomValidator.cs Nullability fixes for trust manager / SSL session APIs.
src/Mono.Android/Xamarin.Android.Net/AndroidMessageHandler.cs Nullability alignment for exception factory helper.
src/Mono.Android/System.Drawing/SizeFConverter.cs TypeConverter override nullability + culture handling.
src/Mono.Android/System.Drawing/SizeConverter.cs TypeConverter override nullability alignment.
src/Mono.Android/System.Drawing/RectangleConverter.cs TypeConverter override nullability + culture handling.
src/Mono.Android/System.Drawing/PointConverter.cs TypeConverter override nullability alignment.
src/Mono.Android/PublicAPI/API-37/PublicAPI.Shipped.txt Shipped API baseline cleanup (duplicate entries removed).
src/Mono.Android/PublicAPI/API-36.1/PublicAPI.Shipped.txt Shipped API baseline cleanup (duplicate entries removed).
src/Mono.Android/Org.Apache.Http.Impl.Cookie/BasicClientCookie.cs Cookie setter nullability aligned to interface expectations.
src/Mono.Android/Mono.Android.csproj Updates warning suppression list (notably removing IL3050 from NoWarn).
src/Mono.Android/Microsoft.Android.Runtime/TrimmableTypeMapTypeManager.cs Refactors runtime branching based on dynamic-code support; adds RequiresDynamicCode annotation.
src/Mono.Android/Microsoft.Android.Runtime/ManagedTypeManager.cs Adds IL3050 suppression for reflection-based type manager.
src/Mono.Android/Microsoft.Android.Runtime/JniRemappingLookup.cs Suppresses CS0649 for struct fields used for native data.
src/Mono.Android/Microsoft.Android.Runtime/JavaMarshalRegisteredPeers.cs Suppresses CS0649 for mirrored native control-block struct.
src/Mono.Android/Java.Util/Spliterators.cs Nullability alignment for TryAdvance consumer parameter.
src/Mono.Android/Java.Lang/StringBuilder.cs Adds new on Append overload to acknowledge member hiding.
src/Mono.Android/Java.Lang/StringBuffer.cs Adds new on Append overload to acknowledge member hiding.
src/Mono.Android/Java.Interop/TypeManager.cs Nullability alignment for comparer + additional null checks in typemap lookup.
src/Mono.Android/Java.Interop/JavaPeerProxy.cs Changes proxy-attribute ctor accessibility to protected.
src/Mono.Android/Java.Interop/JavaConvert.cs Adjusts dynamic/AOT generic collection factory logic; adds RequiresDynamicCode and suppression path.
src/Mono.Android/Android.Widget/BaseAdapter.cs Adds new to acknowledge static member hiding (class_ref).
src/Mono.Android/Android.Widget/ArrayAdapter.cs Adds new to acknowledge member hiding (CreateFromResource, GetItem, Sort).
src/Mono.Android/Android.Widget/AdapterView.cs Nullability alignment for RawAdapter override.
src/Mono.Android/Android.Util/SparseArray.cs Adds new to acknowledge member hiding (Get, ValueAt).
src/Mono.Android/Android.Runtime/XmlReaderPullParser.cs Adds new and fixes parameter nullability for XML parser adapter members.
src/Mono.Android/Android.Runtime/XmlPullParserReader.cs Adds new to acknowledge member hiding (FromJniHandle).
src/Mono.Android/Android.Runtime/RuntimeNativeMethods.cs Adjusts P/Invoke signature nullability (clr_typemap_managed_to_java).
src/Mono.Android/Android.Runtime/OutputStreamAdapter.cs Stream override nullability alignment + explicit argument validation.
src/Mono.Android/Android.Runtime/JNIEnv.cs Refines array creation logic for dynamic/AOT paths; propagates updated nullability for helpers.
src/Mono.Android/Android.Runtime/JavaSet.cs Adds new for enumerator / FromJniHandle member hiding.
src/Mono.Android/Android.Runtime/JavaProxyThrowable.cs Adds new keyword for intentionally hidden InnerException.
src/Mono.Android/Android.Runtime/JavaObject.cs Ensures ToString() is non-null to match base contract.
src/Mono.Android/Android.Runtime/JavaList.cs Adds new for hidden members (Equals/indexer/enumerator/FromJniHandle).
src/Mono.Android/Android.Runtime/JavaDictionary.cs Adds new for hidden members (Keys/Values/enumerator/FromJniHandle).
src/Mono.Android/Android.Runtime/JavaCollection.cs Adds new for hidden members (enumerator/FromJniHandle).
src/Mono.Android/Android.Runtime/InputStreamAdapter.cs Stream override nullability alignment + explicit argument validation.
src/Mono.Android/Android.OS/AsyncTask.cs Adds new to acknowledge static member hiding (class_ref).
src/Mono.Android/Android.Graphics/Color.cs Nullability alignment for overrides + marshaler signature updates.
src/Mono.Android/Android.Graphics/AndroidBitmapInfo.cs Nullability alignment for Equals override.
src/Mono.Android/Android.Content.Res/IXmlResourceParser.cs Adds new to disambiguate interface members and avoid hiding warnings.
src/Mono.Android/Android.Bluetooth/BluetoothDevice.cs Adds ObsoletedOSPlatform("android37.0") to obsolete overloads.
src/Mono.Android/Android.App/SyncContext.cs Nullability alignment for SynchronizationContext overrides.
src/Mono.Android/Android.App/AlertDialog.cs Switches to EventHelper for event add/remove helper calls.
src/Mono.Android/Android.Animation/ValueAnimator.cs Nullability tweaks in binding glue (Delegate?, GetObject(...)).
src/Mono.Android/Android.Animation/AnimatorSet.cs Nullability tweaks in binding glue (Delegate?, GetObject(...)).

Copilot's findings

  • Files reviewed: 45/46 changed files
  • Comments generated: 7

Comment thread src/Mono.Android/Xamarin.Android.Net/ServerCertificateCustomValidator.cs Outdated
Comment thread src/Mono.Android/Android.Animation/ValueAnimator.cs Outdated
Comment thread src/Mono.Android/Android.Animation/ValueAnimator.cs Outdated
Comment thread src/Mono.Android/Android.Animation/AnimatorSet.cs Outdated
Comment thread src/Mono.Android/Android.Animation/AnimatorSet.cs Outdated
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)!; }
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The getter no longer uses the null-forgiving operator now that RawAdapter is nullable.

The setter keeps it intentionally: Adapter is a non-nullable generic T, but null is a valid input here (Android's setAdapter(null) clears the adapter). Removing ! would require either throwing on a legitimate null or changing the public Adapter signature to T?, so the operator is retained to preserve the clear-adapter behavior. (3886367)

Comment thread src/Mono.Android/Android.Runtime/RuntimeNativeMethods.cs
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.
@simonrozsival

Copy link
Copy Markdown
Member Author

Thanks for the review — addressed in 388636743:

  • ServerCertificateCustomValidator.CheckClientTrusted — now guards chain/authType with ArgumentNullException.ThrowIfNull, matching CheckServerTrusted.
  • RuntimeNativeMethods.clr_typemap_managed_to_java — kept the P/Invoke parameter non-nullable (string fullName) and added a guard at the call site in JNIEnv.TypemapManagedToJava that returns null when Type.FullName is null, so a null never flows into native code.
  • ValueAnimator / AnimatorSet — replaced the null-forgiving operators in the marshaling stubs with explicit null checks that throw a clear InvalidOperationException (the n_ throws are reported through OnUserUnhandledException).
  • AdapterView.RawAdapter getter — dropped the redundant ! now that the property is nullable.

One intentional exception: the AdapterView.RawAdapter setter keeps FromJavaObject<T>(value)!. Adapter is a non-nullable generic T, but a null value is a legitimate input (Android's setAdapter(null) clears the adapter). Removing the ! would either require throwing on a valid null or changing the public Adapter signature to T?, so the operator is retained here on purpose.

@simonrozsival

Copy link
Copy Markdown
Member Author

Superseded by #12003, which is rebased on the latest main and pushed to an origin branch (so it runs the full CI). Closing in favor of that PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants