Skip to content

Commit ccbba31

Browse files
DiagnosticSuppressor: Add support for Nullable.Hasvalue checks
1 parent aae3936 commit ccbba31

3 files changed

Lines changed: 97 additions & 13 deletions

File tree

documentation/NUnit3001.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,14 @@ It then checks the previous statements for one of:
2424
For the same expression as the one that raised the original compiler error.
2525
If found, the compiler error is suppressed.
2626

27+
The rule also covers `CS8629: Nullable value type may be null`
28+
29+
In this case, the previous statement is allowed to be one of:
30+
* `Assert.That(...HasValue)`
31+
* `Assert.That(...HasValue, Is.True)`
32+
* `Assert.True(...HasValue)`
33+
* `Assert.IsTrue(...HasValue)`
34+
2735
The exception is that if the statement is part of an `Assert.Multiple`
2836
it is not suppressed, as in this case the statement containing the compiler error will be executed.
2937

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,50 @@ await DiagnosticsSuppressorAnalyzer.EnsureNotSuppressed(suppressor, testCode)
286286
.ConfigureAwait(false);
287287
}
288288

289+
[TestCase("Assert.True(nullable.HasValue)")]
290+
[TestCase("Assert.IsTrue(nullable.HasValue)")]
291+
[TestCase("Assert.That(nullable.HasValue, \"Ensure Value is set\")")]
292+
[TestCase("Assert.That(nullable.HasValue)")]
293+
[TestCase("Assert.That(nullable.HasValue, Is.True)")]
294+
[TestCase("Assert.That(nullable, Is.Not.Null)")]
295+
public async Task NullableWithValidAssert(string assert)
296+
{
297+
var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@$"
298+
#nullable enable
299+
[TestCase(42)]
300+
public void Test(int? nullable)
301+
{{
302+
{assert};
303+
Assert.That(nullable.Value, Is.EqualTo(42));
304+
}}
305+
");
306+
307+
await DiagnosticsSuppressorAnalyzer.EnsureSuppressed(suppressor,
308+
DereferencePossiblyNullReferenceSuppressor.SuppressionDescriptors["CS8629"], testCode)
309+
.ConfigureAwait(false);
310+
}
311+
312+
[TestCase("Assert.False(nullable.HasValue)")]
313+
[TestCase("Assert.IsFalse(nullable.HasValue)")]
314+
[TestCase("Assert.That(!nullable.HasValue)")]
315+
[TestCase("Assert.That(nullable.HasValue, Is.False)")]
316+
[TestCase("Assert.That(nullable, Is.Null)")]
317+
public async Task NullableWithInvalidAssert(string assert)
318+
{
319+
var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@$"
320+
#nullable enable
321+
[TestCase(42)]
322+
public void Test(int? nullable)
323+
{{
324+
{assert};
325+
Assert.That(nullable.Value, Is.EqualTo(42));
326+
}}
327+
");
328+
329+
await DiagnosticsSuppressorAnalyzer.EnsureNotSuppressed(suppressor, testCode)
330+
.ConfigureAwait(false);
331+
}
332+
289333
[Test]
290334
public async Task WithIndexer()
291335
{

src/nunit.analyzers/DiagnosticSuppressors/DereferencePossiblyNullReferenceSuppressor.cs

Lines changed: 45 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -122,26 +122,53 @@ private static bool IsValidatedNotNull(string possibleNullReference, BlockSyntax
122122
}
123123
}
124124

125-
// Check if this is Assert.NotNull or Assert.IsNotNull for the same symbol
125+
// Check if this is an Assert for the same symbol
126126
if (AssertHelper.IsAssert(expressionStatement.Expression, out string member, out ArgumentListSyntax? argumentList))
127127
{
128-
if (member == NUnitFrameworkConstants.NameOfAssertNotNull ||
129-
member == NUnitFrameworkConstants.NameOfAssertIsNotNull ||
130-
member == NUnitFrameworkConstants.NameOfAssertThat)
128+
string firstArgument = argumentList.Arguments.First().Expression.ToString();
129+
130+
if (member == NUnitFrameworkConstants.NameOfAssertThat)
131131
{
132-
if (member == NUnitFrameworkConstants.NameOfAssertThat)
132+
string? secondArgument =
133+
argumentList.Arguments.ElementAtOrDefault(1)?.Expression.ToString();
134+
135+
// If test is on <nullable>.HasValue
136+
if (IsHasValue(firstArgument, possibleNullReference))
133137
{
134-
// We must check the 2nd argument for anything but "Is.Null"
135-
// E.g.: Is.Not.Null.And.Not.Empty.
136-
ArgumentSyntax? secondArgument = argumentList.Arguments.ElementAtOrDefault(1);
137-
if (secondArgument?.ToString() == "Is.Null")
138+
// Could be:
139+
// Assert.That(<nullable>.HasValue)
140+
// Assert.That(<nullable>.HasValue, "Ensure Value Set")
141+
// Assert.That(<nullable>.HasValue, Is.True)
142+
if (secondArgument != "Is.False")
138143
{
139-
continue;
144+
return true;
140145
}
141146
}
142-
143-
ArgumentSyntax firstArgument = argumentList.Arguments.First();
144-
if (CoveredBy(firstArgument.Expression.ToString(), possibleNullReference))
147+
else
148+
{
149+
// Null check, could be Is.Not.Null or more complex
150+
// like Is.Not.Null.And.Not.Empty.
151+
if (secondArgument != "Is.Null")
152+
{
153+
if (CoveredBy(firstArgument, possibleNullReference))
154+
{
155+
return true;
156+
}
157+
}
158+
}
159+
}
160+
else if (member == NUnitFrameworkConstants.NameOfAssertNotNull ||
161+
member == NUnitFrameworkConstants.NameOfAssertIsNotNull)
162+
{
163+
if (CoveredBy(firstArgument, possibleNullReference))
164+
{
165+
return true;
166+
}
167+
}
168+
else if (member == NUnitFrameworkConstants.NameOfAssertIsTrue ||
169+
member == NUnitFrameworkConstants.NameOfAssertTrue)
170+
{
171+
if (IsHasValue(firstArgument, possibleNullReference))
145172
{
146173
return true;
147174
}
@@ -224,6 +251,11 @@ private static bool CoveredBy(string assertedNotNull, string possibleNullReferen
224251
return false;
225252
}
226253

254+
private static bool IsHasValue(string argument, string possibleNullReference)
255+
{
256+
return argument == possibleNullReference + ".HasValue";
257+
}
258+
227259
private static ImmutableDictionary<string, SuppressionDescriptor> CreateSuppressionDescriptors(params string[] suppressionDiagnosticsIds)
228260
{
229261
var builder = new Dictionary<string, SuppressionDescriptor>();

0 commit comments

Comments
 (0)