Skip to content

Commit

Permalink
NullReferenceSuppressor: Keep looking outside Assert.Multiple for oth…
Browse files Browse the repository at this point in the history
…er reason to suppress.
  • Loading branch information
manfred-brands committed Mar 29, 2022
1 parent 742361e commit 84deb52
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -616,5 +616,28 @@ public void Test()
await DiagnosticsSuppressorAnalyzer.EnsureNotSuppressed(suppressor, testCode)
.ConfigureAwait(false);
}

[Test]
public async Task TestIssue436()
{
var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
#nullable enable
[TestCase(null)]
public void HasCountShouldNotAffectNullabilitySuppression(List<int>? maybeNull)
{
Assert.That(maybeNull, Is.Not.Null);
Assert.Multiple(() =>
{
Assert.That(maybeNull, Has.Count.EqualTo(2));
Assert.That(maybeNull[1], Is.EqualTo(1));
});
}
", @"using System.Collections.Generic;");

await DiagnosticsSuppressorAnalyzer.EnsureSuppressed(suppressor,
DereferencePossiblyNullReferenceSuppressor.SuppressionDescriptors["CS8604"], testCode)
.ConfigureAwait(false);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,71 +46,59 @@ public override void ReportSuppressions(SuppressionAnalysisContext context)
continue;
}

// Was the offending variable assigned or verified to be not null inside an Assert.Multiple?
// NUnit doesn't throw on failures and therefore the compiler is correct.
if (ShouldBeSuppressed(node, out SyntaxNode? suppressionCause) && !AssertHelper.IsInsideAssertMultiple(suppressionCause))
if (ShouldBeSuppressed(node))
{
context.ReportSuppression(Suppression.Create(SuppressionDescriptors[diagnostic.Id], diagnostic));
}
}
}

private static bool ShouldBeSuppressed(SyntaxNode node, [NotNullWhen(true)] out SyntaxNode? suppressionCause)
private static bool ShouldBeSuppressed(SyntaxNode node)
{
suppressionCause = default(SyntaxNode);

if (IsKnownToBeNotNull(node))
{
// Known to be not null value assigned or passed to non-nullable type.
suppressionCause = node;
return true;
}

string possibleNullReference = node.ToString();
if (node is CastExpressionSyntax castExpression)
{
// Drop the cast.
possibleNullReference = castExpression.Expression.ToString();
}

bool validatedNotNull;

do
for (SyntaxNode? currentNode = node; currentNode is not null;)
{
BlockSyntax? parent = node.Ancestors().OfType<BlockSyntax>().FirstOrDefault();
StatementSyntax? statement = currentNode.Ancestors().OfType<StatementSyntax>().FirstOrDefault();

if (parent is null)
if (statement is null)
{
return false;
break;
}

validatedNotNull = IsValidatedNotNull(possibleNullReference, parent, node, out suppressionCause);
if (parent.Parent is null)
BlockSyntax? parent = statement.Ancestors().OfType<BlockSyntax>().FirstOrDefault();

// If the statement is inside an Assert.Multiple NUnit doesn't throw
// flow continues with actual null values and therefore these shouldn't be suppressed.
if (!AssertHelper.IsInsideAssertMultiple(statement))
{
return false;
if (IsKnownToBeNotNull(currentNode) ||
(parent is not null && IsValidatedNotNullByPreviousStatementInSameBlock(possibleNullReference, parent, statement)))
{
return true;
}
}

node = parent.Parent;
currentNode = parent;
}
while (!validatedNotNull);

return validatedNotNull;
return false;
}

private static bool IsValidatedNotNull(string possibleNullReference, BlockSyntax parent, SyntaxNode node,
[NotNullWhen(true)] out SyntaxNode? suppressionCause)
private static bool IsValidatedNotNullByPreviousStatementInSameBlock(string possibleNullReference, BlockSyntax parent, StatementSyntax statement)
{
suppressionCause = default(SyntaxNode);

StatementSyntax? statement = node?.AncestorsAndSelf().OfType<StatementSyntax>().FirstOrDefault();
var siblings = parent.ChildNodes().ToList();

// Look in earlier statements to see if the variable was previously checked for null.
for (int nodeIndex = siblings.FindIndex(x => x == statement); --nodeIndex >= 0;)
{
SyntaxNode previous = siblings[nodeIndex];

suppressionCause = previous;
if (previous is ExpressionStatementSyntax expressionStatement)
{
if (expressionStatement.Expression is AssignmentExpressionSyntax assignmentExpression)
Expand Down

0 comments on commit 84deb52

Please sign in to comment.