Skip to content

Clear (some) nullability warnings#577

Merged
notfood merged 3 commits intorwmt:devfrom
ianleeder:clear-warnings-nullability
Jul 19, 2025
Merged

Clear (some) nullability warnings#577
notfood merged 3 commits intorwmt:devfrom
ianleeder:clear-warnings-nullability

Conversation

@ianleeder
Copy link

Overview

This resolves 16 build warnings, including (🤞) 10/11 of the GitHub build action checks, eg:
image

Changes

Layouter.cs

The /Source/Client/Multiplayer.csproj doesn't include <Nullable>enable</Nullable> like a lot of the other csproj files do, and I'm not 100% of the ramifications of enabling it project-wide, so I only enabled it for this file (which was clearly trying to use nullable types).

Native.cs

The best definition for the Mono entrypoint I could find is in the Mono repo. Since we're already passing null for some of the strings, and their example code literally says:

* <code>mono_dllmap_insert (NULL, "i:libdemo.dll", NULL, relocated_demo_path, NULL);</code>

It seems safe to switch these to string? to resolve warnings.

LoggingByteWriter.cs

The base class ByteWriter accepts and handles null for both methods.

ServerLog.cs

Everywhere these info and error actions are used, a null check is already performed.

@ianleeder ianleeder marked this pull request as draft July 18, 2025 15:06
@ianleeder
Copy link
Author

Everything compiles and looks good, but I want to run some tests on this before merging.

@ianleeder
Copy link
Author

The last GitHub Action warning is a bit of a doozy:

\Source\Common\Util\Endpoints.cs(10,66): warning CS0436: The type NotNullWhenAttribute in \Source\Common\Util\CompilerTypes.cs conflicts with the imported type NotNullWhenAttribute in mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089. Using the type defined in \Source\Common\Util\CompilerTypes.cs.

The .NET Core (3+) System.Diagnostics.CodeAnalysis namespace contains a class named NotNullWhenAttribute (source link). CompilerTypes creates the exact same class in the exact same namespace to support .NET Framework. Interestingly the compiler warning is on the attribute usage, not definition.

Currently this attribute is only used in one location. We can disable the warning in EndPoints.cs:

#pragma warning disable CS0436 // Type conflicts with imported type
        // The NotNullWhen attribute will raise a warning when compiled with .NET Core 3 or greater, as it conflicts with the one in System.Diagnostics.CodeAnalysis.
        // Disable the warning.
        public static bool TryParse(string s, uint defaultPort, [NotNullWhen(true)] out IPEndPoint? result)
#pragma warning restore CS0436 // Type conflicts with imported type

Given that there are already a bunch of Resharper suppressions in code (// ReSharper disable once xxx), it's probably not too bad to put another compiler one in. Thoughts?

@ianleeder ianleeder marked this pull request as ready for review July 18, 2025 15:45
@ianleeder ianleeder force-pushed the clear-warnings-nullability branch from a118f19 to c308a82 Compare July 18, 2025 22:50
@notfood notfood merged commit 66e6c86 into rwmt:dev Jul 19, 2025
1 check passed
@ianleeder ianleeder deleted the clear-warnings-nullability branch July 19, 2025 05:07
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