Questions
If community developers have similar questions or suggested changes, please reply to this Issue! A Maintainer or I will will append them to this post with a a quote and link to the reply.
Problem
anegostudios/vsapi has many source files with the #nullable disable preprocessor directive, thereby disabling nullability analysis for large swaths of the project. This means symbols that should be null-aware are instead null-_un_aware—They appear to be never-null, but are actually maybe-null. This leads to a high risk of null-dereferences (NullReferenceException) throughout the library. Because of this, API users are also at risk of null-dereferences.
Solution
#nullable disable preprocessor directives should be removed or used very sparingly.- Maybe-null cases should be handled with either a null-conditional operator (
?., ?[]) or the occasional not-null assertion (!) where it makes sense e.g. in classes whose static members are null until Initialize is called (consider using a lazy singleton, instead!).
Side effects may include encouraging mod developers to write nullable-aware code.
Background
I've been seeing several mods with NullReferenceException stack traces pointing to GuiElementItemSlotGridBase.ComposeSlotOverlays. And so I began investigating them.
Mario90900/Toolsmith#35
dizzyd/packrat#2
dizzyd/packrat#3
At first, I thought it was isolated to ToolSmith's Reflection-based IL patch of the method, but the ComposeSlotOverlays NullReferenceException reportedly occurs even without ToolSmith's IL patches. So, I read the body of ComposeSlotOverlays, found that the stack traces' :Line### indicated the wrong lines (bad symbols?), and then wondered why nullability analysis is explicitly disabled in
...and many other source files.
And so...
(See Questions)
Questions
NullReferenceExceptions. What other null de-reference protections (aside from nullable types) are planned?If community developers have similar questions or suggested changes, please reply to this Issue! A Maintainer or I will will append them to this post with a a quote and link to the reply.
Problem
anegostudios/vsapihas many source files with the#nullable disablepreprocessor directive, thereby disabling nullability analysis for large swaths of the project. This means symbols that should be null-aware are instead null-_un_aware—They appear to be never-null, but are actually maybe-null. This leads to a high risk of null-dereferences (NullReferenceException) throughout the library. Because of this, API users are also at risk of null-dereferences.Solution
#nullable disablepreprocessor directives should be removed or used very sparingly.?.,?[]) or the occasional not-null assertion (!) where it makes sense e.g. in classes whose static members are null untilInitializeis called (consider using a lazy singleton, instead!).Side effects may include encouraging mod developers to write nullable-aware code.
Background
I've been seeing several mods with
NullReferenceExceptionstack traces pointing toGuiElementItemSlotGridBase.ComposeSlotOverlays. And so I began investigating them.Mario90900/Toolsmith#35
dizzyd/packrat#2
dizzyd/packrat#3
At first, I thought it was isolated to ToolSmith's Reflection-based IL patch of the method, but the
ComposeSlotOverlaysNullReferenceExceptionreportedly occurs even without ToolSmith's IL patches. So, I read the body ofComposeSlotOverlays, found that the stack traces':Line###indicated the wrong lines (bad symbols?), and then wondered why nullability analysis is explicitly disabled invsapi/Client/UI/Elements/Impl/Interactive/Inventory/GuiElementItemSlotGridBase.cs
Line 10 in 409af70
And so...
(See Questions)