Merge pull request #415 from manfred-brands/issues/414_NullableHasValueSuppressor

Issues/414 nullable has value suppressor

Author: mikkelbu

documentation/NUnit3001.md

@@ -24,6 +24,14 @@ It then checks the previous statements for one of:
 For the same expression as the one that raised the original compiler error.
 If found, the compiler error is suppressed.
 
+The rule also covers `CS8629: Nullable value type may be null`
+
+In this case, the previous statement is allowed to be one of:
+* `Assert.That(...HasValue)`
+* `Assert.That(...HasValue, Is.True)`
+* `Assert.True(...HasValue)`
+* `Assert.IsTrue(...HasValue)`
+
 The exception is that if the statement is part of an `Assert.Multiple`
 it is not suppressed, as in this case the statement containing the compiler error will be executed.
 

src/global.json

@@ -0,0 +1,7 @@
+{
+  "sdk": {
+    "version": "5.0.100",
+    "rollForward" : "latestFeature",
+    "allowPrerelease": false
+  }
+}

src/nunit.analyzers.tests/DiagnosticSuppressors/DereferencePossiblyNullReferenceSuppressorTests.cs

@@ -286,6 +286,50 @@ await DiagnosticsSuppressorAnalyzer.EnsureNotSuppressed(suppressor, testCode)
                                                .ConfigureAwait(false);
         }
 
+        [TestCase("Assert.True(nullable.HasValue)")]
+        [TestCase("Assert.IsTrue(nullable.HasValue)")]
+        [TestCase("Assert.That(nullable.HasValue, \"Ensure Value is set\")")]
+        [TestCase("Assert.That(nullable.HasValue)")]
+        [TestCase("Assert.That(nullable.HasValue, Is.True)")]
+        [TestCase("Assert.That(nullable, Is.Not.Null)")]
+        public async Task NullableWithValidAssert(string assert)
+        {
+            var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@$"
+                #nullable enable
+                [TestCase(42)]
+                public void Test(int? nullable)
+                {{
+                    {assert};
+                    Assert.That(nullable.Value, Is.EqualTo(42));
+                }}
+            ");
+
+            await DiagnosticsSuppressorAnalyzer.EnsureSuppressed(suppressor,
+                DereferencePossiblyNullReferenceSuppressor.SuppressionDescriptors["CS8629"], testCode)
+                .ConfigureAwait(false);
+        }
+
+        [TestCase("Assert.False(nullable.HasValue)")]
+        [TestCase("Assert.IsFalse(nullable.HasValue)")]
+        [TestCase("Assert.That(!nullable.HasValue)")]
+        [TestCase("Assert.That(nullable.HasValue, Is.False)")]
+        [TestCase("Assert.That(nullable, Is.Null)")]
+        public async Task NullableWithInvalidAssert(string assert)
+        {
+            var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@$"
+                #nullable enable
+                [TestCase(42)]
+                public void Test(int? nullable)
+                {{
+                    {assert};
+                    Assert.That(nullable.Value, Is.EqualTo(42));
+                }}
+            ");
+
+            await DiagnosticsSuppressorAnalyzer.EnsureNotSuppressed(suppressor, testCode)
+                .ConfigureAwait(false);
+        }
+
         [Test]
         public async Task WithIndexer()
         {

src/nunit.analyzers/DiagnosticSuppressors/DereferencePossiblyNullReferenceSuppressor.cs

@@ -122,26 +122,53 @@ private static bool IsValidatedNotNull(string possibleNullReference, BlockSyntax
                         }
                     }
 
-                    // Check if this is Assert.NotNull or Assert.IsNotNull for the same symbol
+                    // Check if this is an Assert for the same symbol
                     if (AssertHelper.IsAssert(expressionStatement.Expression, out string member, out ArgumentListSyntax? argumentList))
                     {
-                        if (member == NUnitFrameworkConstants.NameOfAssertNotNull ||
-                            member == NUnitFrameworkConstants.NameOfAssertIsNotNull ||
-                            member == NUnitFrameworkConstants.NameOfAssertThat)
+                        string firstArgument = argumentList.Arguments.First().Expression.ToString();
+
+                        if (member == NUnitFrameworkConstants.NameOfAssertThat)
                         {
-                            if (member == NUnitFrameworkConstants.NameOfAssertThat)
+                            string? secondArgument =
+                                argumentList.Arguments.ElementAtOrDefault(1)?.Expression.ToString();
+
+                            // If test is on <nullable>.HasValue
+                            if (IsHasValue(firstArgument, possibleNullReference))
                             {
-                                // We must check the 2nd argument for anything but "Is.Null"
-                                // E.g.: Is.Not.Null.And.Not.Empty.
-                                ArgumentSyntax? secondArgument = argumentList.Arguments.ElementAtOrDefault(1);
-                                if (secondArgument?.ToString() == "Is.Null")
+                                // Could be:
+                                // Assert.That(<nullable>.HasValue)
+                                // Assert.That(<nullable>.HasValue, "Ensure Value Set")
+                                // Assert.That(<nullable>.HasValue, Is.True)
+                                if (secondArgument != "Is.False")
                                 {
-                                    continue;
+                                    return true;
                                 }
                             }
-
-                            ArgumentSyntax firstArgument = argumentList.Arguments.First();
-                            if (CoveredBy(firstArgument.Expression.ToString(), possibleNullReference))
+                            else
+                            {
+                                // Null check, could be Is.Not.Null or more complex
+                                // like Is.Not.Null.And.Not.Empty.
+                                if (secondArgument != "Is.Null")
+                                {
+                                    if (CoveredBy(firstArgument, possibleNullReference))
+                                    {
+                                        return true;
+                                    }
+                                }
+                            }
+                        }
+                        else if (member == NUnitFrameworkConstants.NameOfAssertNotNull ||
+                                member == NUnitFrameworkConstants.NameOfAssertIsNotNull)
+                        {
+                            if (CoveredBy(firstArgument, possibleNullReference))
+                            {
+                                return true;
+                            }
+                        }
+                        else if (member == NUnitFrameworkConstants.NameOfAssertIsTrue ||
+                                member == NUnitFrameworkConstants.NameOfAssertTrue)
+                        {
+                            if (IsHasValue(firstArgument, possibleNullReference))
                             {
                                 return true;
                             }
@@ -224,6 +251,11 @@ private static bool CoveredBy(string assertedNotNull, string possibleNullReferen
             return false;
         }
 
+        private static bool IsHasValue(string argument, string possibleNullReference)
+        {
+            return argument == possibleNullReference + ".HasValue";
+        }
+
         private static ImmutableDictionary<string, SuppressionDescriptor> CreateSuppressionDescriptors(params string[] suppressionDiagnosticsIds)
         {
             var builder = new Dictionary<string, SuppressionDescriptor>();