C#: Enable nullable environment for GodotTools
#87137
Conversation
Repiteo
force-pushed
the
dotnet/nullable-godot-tools
branch
from
4bdd367
to
2b4a68e
Compare
|
Updated to include |
|
Could you please rebase on the current master? We had an issue with macOS builds which failed your CI. |
Repiteo
force-pushed
the
dotnet/nullable-godot-tools
branch
from
2b4a68e
to
a04fc4e
Compare
|
Done and done! |
modules/mono/editor/GodotTools/GodotTools/Export/ExportPlugin.cs
Outdated
Show resolved
Hide resolved
modules/mono/editor/GodotTools/GodotTools/Internals/GodotSharpDirs.cs
Outdated
Show resolved
Hide resolved
modules/mono/editor/GodotTools/GodotTools.IdeMessaging/IHandshake.cs
Outdated
Show resolved
Hide resolved
modules/mono/editor/GodotTools/GodotTools.IdeMessaging/Client.cs
Outdated
Show resolved
Hide resolved
modules/mono/editor/GodotTools/GodotTools.IdeMessaging/Utils/NotifyAwaiter.cs
Outdated
Show resolved
Hide resolved
modules/mono/editor/GodotTools/GodotTools.IdeMessaging/Utils/NotifyAwaiter.cs
Outdated
Show resolved
Hide resolved
Repiteo
force-pushed
the
dotnet/nullable-godot-tools
branch
4 times, most recently
from
968988c
to
0edadd7
Compare
modules/mono/editor/GodotTools/GodotTools/Utils/CollectionExtensions.cs
Outdated
Show resolved
Hide resolved
modules/mono/editor/GodotTools/GodotTools/Utils/CollectionExtensions.cs
Outdated
Show resolved
Hide resolved
| @@ -20,36 +20,36 @@ public JetBrains.Rider.PathLocator.OS CurrentOS | |||
| } | |||
| } | |||
|
|
|||
| public T FromJson<T>(string json) | |||
| public T? FromJson<T>(string json) | |||
There was a problem hiding this comment.
These methods implement the IRiderLocatorEnvironment interface that comes from https://github.com/JetBrains/resharper-unity/blob/net241/unity/PathLocator/PathLocator.csproj. This project doesn't enable nullability so the interface is not annotated.
Usually the implementation should match the interface, but in this case I'm not aware of any Microsoft guidelines that cover the case of a null-oblivious interface implemented by a null-aware class.
| @@ -7,11 +7,11 @@ namespace GodotTools.BuildLogger | |||
| { | |||
| public class GodotBuildLogger : ILogger | |||
| { | |||
| public string Parameters { get; set; } | |||
| public string? Parameters { get; set; } | |||
There was a problem hiding this comment.
This property implements the ILogger interface which is not annotated for nullability: https://source.dot.net/#Microsoft.Build.Framework/ILogger.cs
There was a problem hiding this comment.
Nullability is disabled for that linked file overall, but the description of the property is: "The parameter string (can be null)". This certainly makes it seem like making it nullable is the correct call in a null-aware context
There was a problem hiding this comment.
I agree. Like I said in #87137 (comment) I'm not aware of any guidelines to follow in this case so I was only bringing attention to it, but it's probably fine to leave it as-is.
Repiteo
force-pushed
the
dotnet/nullable-godot-tools
branch
2 times, most recently
from
2c2b612
to
2964218
Compare
Repiteo
force-pushed
the
dotnet/nullable-godot-tools
branch
from
2964218
to
3314f8c
Compare
|
Thanks! |
Enables nullability in majority of
GodotToolsprojects. At first I didn't think this would be possible withoutGodotSharpgetting nullable first (see #83117), but it turns out there was quite a lot that could be setup as-is! The only real area that didn't get expanded upon were certainGodotObjectfields (mirroring the selectively disabled nullability seen on a handful of files beforehand), which is likely to be more feasible whenGodotSharpgets full-project nullability.