Enable nullable reference types in Microsoft.Android.Build.BaseTasks

[Copilot]

The Microsoft.Android.Build.BaseTasks project had no nullable reference type analysis, allowing null dereference bugs to slip through (e.g. #342).

Changes

• csproj: Added <Nullable>enable</Nullable> and <WarningsAsErrors>nullable</WarningsAsErrors>
• Files.cs: Fixed a real null-dereference bug in DeleteFile where an as cast was dereferenced without a null check:

  // Before: crashes if log is not a TaskLoggingHelper
  var helper = log as TaskLoggingHelper;
  helper.LogErrorFromException (ex);
  
  // After:
  helper?.LogErrorFromException (ex);

  Also: HashZip → string?, HasZipChanged out params → string?, CopyIfZipChanged now null-checks Path.GetDirectoryName
• AsyncTask.cs: Made code/file params nullable across LogCodedError/LogCodedWarning overloads; fixed IDictionary<string, string> → IDictionary<string, string?> for LogTelemetry
• AndroidRidAbiHelper.cs: GetNativeLibraryAbi → string? return
• AndroidRunToolTask.cs: BaseDirectory → string?, androidErrorRegex → Regex?
• LinePreservedXmlWriter.cs: nav → XPathNavigator?, Proceed param → IXmlLineInfo?
• MSBuildExtensions.cs: GetRegisteredTaskObjectAssemblyLocal<T> and UnregisterTaskObjectAssemblyLocal<T> → T? return (they use as T)

[jonathanpeppers]

Nullable Reference Types & Public API Impact

No binary-breaking changes

Nullable annotations are compile-time metadata only (NullableAttribute); at the IL level, string and string? are the same type. Existing compiled code will continue to work.

However, there are source-breaking changes for consumers with <Nullable>enable</Nullable>. The primary consumer — dotnet/android — has nullable enabled in Xamarin.Android.Build.Tasks.csproj, so bumping this package will surface new warnings or errors there.

Public APIs Changed

API	Change	Risk
GetNativeLibraryAbi (2 overloads)	string → string? return	⚠️ Source-breaking
HasZipChanged (2 overloads)	out string → out string?	⚠️ Source-breaking
GetRegisteredTaskObjectAssemblyLocal<T>	T → T? return	⚠️ Source-breaking
UnregisterTaskObjectAssemblyLocal<T>	T → T? return	⚠️ Source-breaking
LogCodedError / LogCodedWarning params	string → string?	✅ Safe (widening)
LogTelemetry param	IDictionary<string, string> → IDictionary<string, string?>	✅ Safe (widening)
ExtractAll delegates	non-nullable → nullable	✅ Safe (already had = null defaults)

Good news: dotnet/android already handles null

I checked the primary consumer — CollectNativeFilesForArchive.cs in dotnet/android already uses #nullable enable and treats the return from GetNativeLibraryAbi as nullable. The annotations are making the API honest about what it already does.

Potential concern: GetRegisteredTaskObjectAssemblyLocal<T>

This one has 14 call sites in dotnet/android. The return type changing from T to T? means every caller that assigns to a T variable (not T?) will get a new CS8600 warning. These callers may or may not already null-check the result. A companion PR in dotnet/android may be needed when bumping the package.

The bug fix is good 👍

The DeleteFile null-deref fix (helper?.LogErrorFromException) addresses a real bug where an as cast was dereferenced without a null check.

Recommendation

The changes are semantically correct — every method marked ? can already return null at runtime. When bumping the package in dotnet/android, expect to fix nullable warnings especially around GetRegisteredTaskObjectAssemblyLocal<T>.

[Copilot]

Pull request overview

Enables nullable reference type analysis in Microsoft.Android.Build.BaseTasks to catch null-related issues at compile time, and updates several utility/task APIs and implementations to be nullable-safe.

Changes:

• Enabled nullable analysis and made nullable warnings errors for the Microsoft.Android.Build.BaseTasks project.
• Updated nullability annotations and some null-handling in Files, AsyncTask, and various helpers/utilities.
• Adjusted a few APIs to more accurately reflect possible null values (e.g., as-casts and ABI detection).

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/Microsoft.Android.Build.BaseTasks/Microsoft.Android.Build.BaseTasks.csproj	Enables nullable reference types and treats nullable warnings as errors.
src/Microsoft.Android.Build.BaseTasks/MSBuildExtensions.cs	Marks as T-based task object helpers as returning nullable T?.
src/Microsoft.Android.Build.BaseTasks/LinePreservedXmlWriter.cs	Makes navigator/line-info handling nullable-aware.
src/Microsoft.Android.Build.BaseTasks/Files.cs	Adds null-safe directory creation for zip copies; updates zip hash APIs and extraction callbacks for nullability.
src/Microsoft.Android.Build.BaseTasks/AsyncTask.cs	Adjusts coded error/warning overloads for nullable code/file; updates telemetry property value nullability.
src/Microsoft.Android.Build.BaseTasks/AndroidRunToolTask.cs	Makes BaseDirectory and cached regex field nullable.
src/Microsoft.Android.Build.BaseTasks/AndroidRidAbiHelper.cs	Updates ABI detection helpers to return nullable.

Comments suppressed due to low confidence (1)

src/Microsoft.Android.Build.BaseTasks/AndroidRunToolTask.cs:44

• AndroidErrorRegex returns a Regex but the backing field is now Regex?. With nullable warnings treated as errors, returning the nullable field can still trigger CS8603/CS8602-style warnings because flow analysis doesn’t reliably prove non-null for fields. Consider returning a non-null local (or using ??= with a local) so the getter is provably non-null under nullable analysis.

		static Regex? androidErrorRegex;
		public static Regex AndroidErrorRegex {
			get {
				if (androidErrorRegex == null)
					androidErrorRegex = new Regex (@"