EnsureArg.IsNotNull for nullable value type

[hannasm]

Uri? a = null;
EnsureArg.IsNotNull(a);
var x = a.ToString();

On line 3, Visual studio generates a CS8602, dereference of a possibly null reference

[ndrwrbgs]

From dotnet/roslyn-analyzers#2578 it looks like this shouldn't be the case. But I can confirm that in a csproj like so it does (please include in future reports for expediency).

Not seeing much online talking about it, though, seems nullable might not have wide adoption yet?

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>netcoreapp3.1</TargetFramework>
    <Nullable>enable</Nullable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Ensure.That" Version="10.0.0" />
  </ItemGroup>

</Project>

[ndrwrbgs]

Brute force testing seems the current SA from roslyn team requires EXPLICITLY the https://docs.microsoft.com/en-us/dotnet/api/system.diagnostics.codeanalysis.notnullattribute?view=net-5.0 attribute, without compatibility with the existing R# attributes.

Sadly, the old ValidatedNotNull will still be required for projects using those versions, and NotNull will be required for new projects, while ContractAnnotation is required for CodeContracts based projects. Microsoft has quite the repertoire of static analysis approaches. I apologize in advance for how attribute-y the code is getting @danielwertheim!

It does appear the nice JetBrains team made compat with the C#8 attributes though, so we can simply replace in some places.

[ndrwrbgs]

@hannasm will you be able to confirm the preview package when published?

[hannasm]

Sure i'd be glad to check it

…

On Tue, Mar 30, 2021 at 2:42 PM ndrwrbgs ***@***.***> wrote: @hannasm <https://github.com/hannasm> will you be able to confirm the preview package when published? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#154 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AASHZJFX3GZXK6RM2T5I6KDTGILRJANCNFSM4YWI5HSA> .

[danielwertheim]

There's now a pre-release out regarding this: https://www.nuget.org/packages/Ensure.That/10.1.0-p1

[danielwertheim]

@hannasm did the preview release solve your issue?

[hannasm]

I can confirm this issue is resolved for me. There is a substantial API surface to validate and I only tested a few use cases. I was thinking that perhaps you could create a project with 'warnings as errors' around these use cases to automate the validation. I'm sure multiple build targets and varying toolchains will complicate that setup, and it's a bit unconventional.

…

On Fri, Apr 2, 2021 at 12:27 PM Daniel Wertheim ***@***.***> wrote: @hannasm <https://github.com/hannasm> did the preview release solve your issue? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#154 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AASHZJE5TMSROIMDH3JRCDLTGXV5PANCNFSM4YWI5HSA> .

[ndrwrbgs]

The hard part is that most of the impact for other toolchains will be authoring time only and not a build break. I'd ship it, myself, since the issues it can cause aren't program validity issues but one of { someone upgrading's build will outright break, someone's IDE/tools will false warn about something }. Neither is ideal, but I see both as low impact to a product and easy to fix if reported.

[danielwertheim]

Thanks @hannasm for verifying.

@ndrwrbgs Agreed. It's now released as v10.1.0. Thanks for all the work fixing this.