Skip to content

Fix null-forgiving operator usage in MarshalMethodsAssemblyRewriter #10362

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
jonathanpeppers merged 8 commits into from
Aug 1, 2025
Merged

Fix null-forgiving operator usage in MarshalMethodsAssemblyRewriter #10362

jonathanpeppers merged 8 commits into from
Aug 1, 2025

Conversation

Copy link
Contributor

Copilot AI commented Jul 31, 2025 •
edited by jonathanpeppers
Loading

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:

// 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.

Context #10326.

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@jonathanpeppers
Changes before error encountered
9a2eeb1 
Co-authored-by: jonathanpeppers <[email protected]>
jonathanpeppers
jonathanpeppers reviewed Jul 31, 2025
View reviewed changes
Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

@copilot continue your work/checklist.

Copy link
Contributor Author

Copilot AI commented Jul 31, 2025

@copilot continue your work/checklist.

I'm continuing the work on fixing NRT annotations in the Utilities folder. I've successfully converted 2 files so far:

  • MarshalMethodsAssemblyRewriter.cs: Fixed nullable reference type annotations for class properties, method return types, and added appropriate null-forgiving operators
  • LlvmIrStringManager.cs: Converted String.IsNullOrEmpty() calls to extension methods and fixed nullable casting issue

Some files have complex interdependencies that require converting multiple files together. I'm working through them systematically, starting with the simpler standalone files first. Progress is in commit d9bcb70.

Copilot AI and others added 2 commits July 31, 2025 18:04
jonathanpeppers
jonathanpeppers reviewed Jul 31, 2025
View reviewed changes
Copilot AI changed the title [WIP] Fix NRT annotations in src\Xamarin.Android.Build.Tasks\ Fix null-forgiving operator usage in MarshalMethodsAssemblyRewriter Jul 31, 2025
Copilot AI requested a review from jonathanpeppers July 31, 2025 18:59
@jonathanpeppers jonathanpeppers marked this pull request as ready for review July 31, 2025 19:06
@jonathanpeppers jonathanpeppers enabled auto-merge (squash) July 31, 2025 19:08
jonathanpeppers
jonathanpeppers requested changes Jul 31, 2025
View reviewed changes
Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

@copilot fix the errors:

(CoreCompile target) -> 
         /Users/builder/azdo/_work/7/s/android/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsAssemblyRewriter.cs(50,29): error CS8600: Converting null literal or possible null value to non-nullable type. [/Users/builder/azdo/_work/7/s/android/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj]
         /Users/builder/azdo/_work/7/s/android/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsAssemblyRewriter.cs(53,53): error CS8600: Converting null literal or possible null value to non-nullable type. [/Users/builder/azdo/_work/7/s/android/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj]
         /Users/builder/azdo/_work/7/s/android/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsAssemblyRewriter.cs(57,40): error CS8600: Converting null literal or possible null value to non-nullable type. [/Users/builder/azdo/_work/7/s/android/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj]
         /Users/builder/azdo/_work/7/s/android/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsAssemblyRewriter.cs(60,48): error CS8600: Converting null literal or possible null value to non-nullable type. [/Users/builder/azdo/_work/7/s/android/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj]
         /Users/builder/azdo/_work/7/s/android/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsAssemblyRewriter.cs(64,42): error CS8600: Converting null literal or possible null value to non-nullable type. [/Users/builder/azdo/_work/7/s/android/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj]
         /Users/builder/azdo/_work/7/s/android/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsAssemblyRewriter.cs(67,52): error CS8600: Converting null literal or possible null value to non-nullable type. [/Users/builder/azdo/_work/7/s/android/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj]
         /Users/builder/azdo/_work/7/s/android/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsAssemblyRewriter.cs(74,37): error CS8600: Converting null literal or possible null value to non-nullable type. [/Users/builder/azdo/_work/7/s/android/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj]

auto-merge was automatically disabled July 31, 2025 20:26

Head branch was pushed to by a user without write access

Copy link
Contributor Author

Copilot AI commented Jul 31, 2025

@copilot fix the errors:

(CoreCompile target) ->

...

Fixed the CS8600 compilation errors by making all variables that receive return values from FindType and FindMethod nullable to match their return types. The variables are now properly declared as nullable (TypeDefinition?, MethodDefinition?) and include appropriate null checks with ArgumentNullException throws. Build completed successfully with no errors in commit 500fd3c.

Copilot AI requested a review from jonathanpeppers July 31, 2025 20:28
@jonathanpeppers jonathanpeppers merged commit c24828c into main Aug 1, 2025
59 checks passed
@jonathanpeppers jonathanpeppers deleted the copilot/fix-10326 branch August 1, 2025 16:12
@github-actions github-actions bot locked and limited conversation to collaborators Sep 1, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@grendello grendello Awaiting requested review from grendello grendello is a code owner

1 more reviewer

@jonathanpeppers jonathanpeppers jonathanpeppers approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jonathanpeppers jonathanpeppers

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jonathanpeppers