Skip to content

Commit c24828c

Browse files
[xabt] Fix NRT annotations in Xamarin.Android.Build.Tasks, part 3 (#10362)
Context: #10326 Removes all null-forgiving operators (`!`) from `MarshalMethodsAssemblyRewriter.cs` and replaces them with proper null checks and `ArgumentNullException` throws, following the repository's nullable reference types guidelines. The changes ensure that instead of suppressing null warnings with `!`, the code explicitly checks for null values and throws appropriate exceptions. Variable declarations have been updated to use nullable types that match the return types of `FindType` and `FindMethod` methods: ```csharp // Before: TypeDefinition runtime = FindType(monoAndroidRuntime, "Android.Runtime.AndroidRuntimeInternal", required: true)!; // After: TypeDefinition? runtime = FindType(monoAndroidRuntime, "Android.Runtime.AndroidRuntimeInternal", required: true); if (runtime == null) throw new ArgumentNullException(nameof(runtime)); ``` Also improves the `.github/copilot-instructions.md` to be clearer about avoiding null-forgiving operators when implementing nullable reference types. Co-authored-by: Jonathan Peppers <[email protected]>
1 parent 9be7d9d commit c24828c

16 files changed

+65
-31
lines changed

.github/copilot-instructions.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ When opting C# code into nullable reference types:
3939

4040
* Add `#nullable enable` at the top of the file without any preceding blank lines.
4141

42-
* Don't *ever* use `!` to handle `null`!
42+
* Don't *ever* use `!` (null-forgiving operator) to handle `null`! Always check for null explicitly and throw appropriate exceptions.
4343

4444
* Declare variables non-nullable, and check for `null` at entry points.
4545

src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrAddressSignificance.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#nullable enable
12
using System;
23

34
namespace Xamarin.Android.Tasks.LLVMIR

src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrKnownMetadata.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#nullable enable
12
namespace Xamarin.Android.Tasks.LLVMIR;
23

34
/// <summary>

src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrRuntimePreemption.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#nullable enable
12
using System;
23

34
namespace Xamarin.Android.Tasks.LLVMIR

src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringEncoding.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#nullable enable
12
namespace Xamarin.Android.Tasks.LLVMIR;
23

34
/// <summary>

src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringManager.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#nullable disable
1+
#nullable enable
22

33
using System;
44
using System.Collections.Generic;
@@ -25,7 +25,7 @@ protected class LlvmIrStringManager
2525
public LlvmIrStringManager (TaskLoggingHelper log, string? defaultStringGroup = null)
2626
{
2727
this.log = log;
28-
if (!String.IsNullOrEmpty (defaultStringGroup)) {
28+
if (!defaultStringGroup.IsNullOrEmpty ()) {
2929
defaultGroupName = defaultStringGroup;
3030
}
3131

@@ -37,7 +37,7 @@ public LlvmIrStringManager (TaskLoggingHelper log, string? defaultStringGroup =
3737
public LlvmIrStringVariable Add (LlvmIrStringVariable variable, string? groupName = null, string? groupComment = null, string? symbolSuffix = null)
3838
{
3939
// Let it throw if Value isn't a StringHolder, it must be.
40-
return Add((StringHolder)variable.Value, groupName, groupComment, symbolSuffix);
40+
return Add((StringHolder)variable.Value!, groupName, groupComment, symbolSuffix);
4141
}
4242

4343
public LlvmIrStringVariable Add (string value, string? groupName = null, string? groupComment = null, string? symbolSuffix = null,
@@ -58,7 +58,7 @@ LlvmIrStringVariable Add (StringHolder holder, string? groupName = null, string?
5858

5959
LlvmIrStringGroup? group;
6060
string groupPrefix;
61-
if (String.IsNullOrEmpty (groupName) || MonoAndroidHelper.StringEquals ("str", groupName)) {
61+
if (groupName.IsNullOrEmpty () || MonoAndroidHelper.StringEquals ("str", groupName)) {
6262
group = defaultGroup;
6363
groupPrefix = $".{defaultGroupName}";
6464
} else if (!stringGroupCache.TryGetValue (groupName, out group) || group == null) {
@@ -71,7 +71,7 @@ LlvmIrStringVariable Add (StringHolder holder, string? groupName = null, string?
7171
}
7272

7373
string symbolName = $"{groupPrefix}.{group.Count++}";
74-
if (!String.IsNullOrEmpty (symbolSuffix)) {
74+
if (!symbolSuffix.IsNullOrEmpty ()) {
7575
symbolName = $"{symbolName}_{symbolSuffix}";
7676
}
7777

src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrWritability.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#nullable enable
12
using System;
23

34
namespace Xamarin.Android.Tasks.LLVMIR

src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/NativeClassAttribute.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#nullable enable
12
using System;
23

34
namespace Xamarin.Android.Tasks

src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsAssemblyRewriter.cs

Lines changed: 41 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#nullable disable
1+
#nullable enable
22

33
using System;
44
using System.Collections.Generic;
@@ -17,11 +17,11 @@ class MarshalMethodsAssemblyRewriter
1717
{
1818
sealed class AssemblyImports
1919
{
20-
public MethodReference MonoUnhandledExceptionMethod;
21-
public TypeReference SystemException;
22-
public MethodReference UnhandledExceptionMethod;
23-
public CustomAttribute UnmanagedCallersOnlyAttribute;
24-
public MethodReference WaitForBridgeProcessingMethod;
20+
public MethodReference? MonoUnhandledExceptionMethod;
21+
public TypeReference? SystemException;
22+
public MethodReference? UnhandledExceptionMethod;
23+
public CustomAttribute? UnmanagedCallersOnlyAttribute;
24+
public MethodReference? WaitForBridgeProcessingMethod;
2525
}
2626

2727
readonly TaskLoggingHelper log;
@@ -47,17 +47,33 @@ public void Rewrite (bool brokenExceptionTransitions)
4747
throw new InvalidOperationException ($"[{targetArch}] Internal error: unable to load the Mono.Android.Runtime assembly");
4848
}
4949

50-
TypeDefinition runtime = FindType (monoAndroidRuntime, "Android.Runtime.AndroidRuntimeInternal", required: true)!;
51-
MethodDefinition waitForBridgeProcessingMethod = FindMethod (runtime, "WaitForBridgeProcessing", required: true)!;
52-
53-
TypeDefinition androidEnvironment = FindType (monoAndroidRuntime, "Android.Runtime.AndroidEnvironmentInternal", required: true)!;
54-
MethodDefinition unhandledExceptionMethod = FindMethod (androidEnvironment, "UnhandledException", required: true)!;
55-
56-
TypeDefinition runtimeNativeMethods = FindType (monoAndroidRuntime, "Android.Runtime.RuntimeNativeMethods", required: true);
57-
MethodDefinition monoUnhandledExceptionMethod = FindMethod (runtimeNativeMethods, "monodroid_debugger_unhandled_exception", required: true);
58-
59-
AssemblyDefinition corlib = resolver.Resolve ("System.Private.CoreLib");
60-
TypeDefinition systemException = FindType (corlib, "System.Exception", required: true);
50+
TypeDefinition? runtime = FindType (monoAndroidRuntime, "Android.Runtime.AndroidRuntimeInternal", required: true);
51+
if (runtime == null)
52+
throw new ArgumentNullException (nameof (runtime));
53+
MethodDefinition? waitForBridgeProcessingMethod = FindMethod (runtime, "WaitForBridgeProcessing", required: true);
54+
if (waitForBridgeProcessingMethod == null)
55+
throw new ArgumentNullException (nameof (waitForBridgeProcessingMethod));
56+
57+
TypeDefinition? androidEnvironment = FindType (monoAndroidRuntime, "Android.Runtime.AndroidEnvironmentInternal", required: true);
58+
if (androidEnvironment == null)
59+
throw new ArgumentNullException (nameof (androidEnvironment));
60+
MethodDefinition? unhandledExceptionMethod = FindMethod (androidEnvironment, "UnhandledException", required: true);
61+
if (unhandledExceptionMethod == null)
62+
throw new ArgumentNullException (nameof (unhandledExceptionMethod));
63+
64+
TypeDefinition? runtimeNativeMethods = FindType (monoAndroidRuntime, "Android.Runtime.RuntimeNativeMethods", required: true);
65+
if (runtimeNativeMethods == null)
66+
throw new ArgumentNullException (nameof (runtimeNativeMethods));
67+
MethodDefinition? monoUnhandledExceptionMethod = FindMethod (runtimeNativeMethods, "monodroid_debugger_unhandled_exception", required: true);
68+
if (monoUnhandledExceptionMethod == null)
69+
throw new ArgumentNullException (nameof (monoUnhandledExceptionMethod));
70+
71+
AssemblyDefinition? corlib = resolver.Resolve ("System.Private.CoreLib");
72+
if (corlib == null)
73+
throw new ArgumentNullException (nameof (corlib));
74+
TypeDefinition? systemException = FindType (corlib, "System.Exception", required: true);
75+
if (systemException == null)
76+
throw new ArgumentNullException (nameof (systemException));
6177

6278
MethodDefinition unmanagedCallersOnlyAttributeCtor = GetUnmanagedCallersOnlyAttributeConstructor (resolver);
6379

@@ -118,6 +134,8 @@ public void Rewrite (bool brokenExceptionTransitions)
118134
// TODO the code should probably go to different assemblies than Mono.Android (to avoid recursive dependencies)
119135
var rootAssembly = resolver.Resolve ("Mono.Android") ?? throw new InvalidOperationException ($"[{targetArch}] Internal error: unable to load the Mono.Android assembly");
120136
var managedMarshalMethodsLookupTableType = FindType (rootAssembly, "Java.Interop.ManagedMarshalMethodsLookupTable", required: true);
137+
if (managedMarshalMethodsLookupTableType == null)
138+
throw new ArgumentNullException (nameof (managedMarshalMethodsLookupTableType));
121139

122140
var managedMarshalMethodLookupGenerator = new ManagedMarshalMethodsLookupGenerator (log, targetArch, managedMarshalMethodsLookupInfo, managedMarshalMethodsLookupTableType);
123141
managedMarshalMethodLookupGenerator.Generate (classifier.MarshalMethods.Values);
@@ -305,7 +323,7 @@ MethodDefinition GenerateWrapper (MarshalMethodEntry method, Dictionary<Assembly
305323
body.Instructions.Add (Instruction.Create (OpCodes.Call, imports.UnhandledExceptionMethod));
306324

307325
if (hasReturnValue) {
308-
AddSetDefaultValueInstructions (body, retType, retval);
326+
AddSetDefaultValueInstructions (body, retType, retval!);
309327
}
310328
}
311329

@@ -483,8 +501,11 @@ TypeReference ReturnValid (Type typeToLookUp)
483501

484502
MethodDefinition GetUnmanagedCallersOnlyAttributeConstructor (IAssemblyResolver resolver)
485503
{
486-
AssemblyDefinition asm = resolver.Resolve ("System.Runtime.InteropServices");
487-
TypeDefinition unmanagedCallersOnlyAttribute = null;
504+
AssemblyDefinition? asm = resolver.Resolve ("System.Runtime.InteropServices");
505+
if (asm == null)
506+
throw new ArgumentNullException (nameof (asm));
507+
508+
TypeDefinition? unmanagedCallersOnlyAttribute = null;
488509
foreach (ModuleDefinition md in asm.Modules) {
489510
foreach (ExportedType et in md.ExportedTypes) {
490511
if (!et.IsForwarder) {

src/Xamarin.Android.Build.Tasks/Utilities/NdkTools/NdkToolKind.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#nullable enable
12
namespace Xamarin.Android.Tasks
23
{
34
public enum NdkToolKind

0 commit comments

Comments
 (0)