Skip to content

Commit

Permalink
DereferencePossiblyNullReferenceSuppressor: Search inside Assert.Mult…
Browse files Browse the repository at this point in the history 
…iple (#588)
  • Loading branch information
@manfred-brands
manfred-brands authored Sep 16, 2023
1 parent c652c64 commit ad20d26
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 52 deletions.
42 changes: 42 additions & 0 deletions ....analyzers.tests/DiagnosticSuppressors/DereferencePossiblyNullReferenceSuppressorTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -932,5 +932,47 @@ private static void DoNothing<T>(params T[] p)
ExpectedDiagnostic.Create(DereferencePossiblyNullReferenceSuppressor.SuppressionDescriptors[diagnostic]),
testCode);
}

[Test]
public void TestIssue587SuppressedInsideAssertMultiple()
{
var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
[Test]
public void Test()
{
Extra? oldExtra = GetResult();
Extra? extra = GetResult();
Assert.Multiple(() =>
{
Assert.That(oldExtra, Is.SameAs(extra));
Assert.That(extra, Is.Not.Null);
});
Assert.Multiple(() =>
{
Assert.That(↓extra.Value, Is.EqualTo(8));
Assert.That(extra.Info, Is.EqualTo(""Hi""));
});
}
private static Extra? GetResult() => new("".NET"", 8);
private sealed class Extra
{
public Extra(string info, int value)
{
Info = info;
Value = value;
}
public string Info { get; }
public int Value { get; }
}
");

RoslynAssert.Suppressed(suppressor,
ExpectedDiagnostic.Create(DereferencePossiblyNullReferenceSuppressor.SuppressionDescriptors["CS8602"]),
testCode);
}
}
}
133 changes: 81 additions & 52 deletions src/nunit.analyzers/DiagnosticSuppressors/DereferencePossiblyNullReferenceSuppressor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using System.Linq.Expressions;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
Expand Down Expand Up @@ -257,12 +258,13 @@ arrayType.ElementType is ITypeParameterSymbol typeParameter &&
return null;
}

private static bool IsValidatedNotNullByPreviousStatementInSameBlock(string possibleNullReference, BlockSyntax block, StatementSyntax statement)
private static bool IsValidatedNotNullByPreviousStatementInSameBlock(string possibleNullReference, BlockSyntax block, StatementSyntax? statement)
{
var siblings = block.ChildNodes().ToList();
int nodeIndex = statement is null ? siblings.Count : siblings.FindIndex(x => x == statement);

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

Expand All @@ -277,71 +279,98 @@ private static bool IsValidatedNotNullByPreviousStatementInSameBlock(string poss
ShouldBeSuppressed(assignmentExpression.Right);
}
}

// Check if this is an Assert for the same symbol
if (AssertHelper.IsAssert(expressionStatement.Expression, out string member, out ArgumentListSyntax? argumentList))
else
{
string firstArgument = argumentList.Arguments.First().Expression.ToString();

if (member == NUnitFrameworkConstants.NameOfAssertThat)
if (IsValidatedNotNullByExpression(possibleNullReference, expressionStatement.Expression))
return true;
}
}
else if (previous is LocalDeclarationStatementSyntax localDeclarationStatement)
{
VariableDeclarationSyntax declaration = localDeclarationStatement.Declaration;
foreach (var variable in declaration.Variables)
{
if (variable.Identifier.ToString() == possibleNullReference)
{
string? secondArgument =
argumentList.Arguments.ElementAtOrDefault(1)?.Expression.ToString();

// If test is on <nullable>.HasValue
if (IsHasValue(firstArgument, possibleNullReference))
{
// Could be:
// Assert.That(<nullable>.HasValue)
// Assert.That(<nullable>.HasValue, "Ensure Value Set")
// Assert.That(<nullable>.HasValue, Is.True)
if (secondArgument != "Is.False")
{
return true;
}
}
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;
}
}
}
ExpressionSyntax? initializer = variable.Initializer?.Value;
return initializer is not null &&
(IsKnownToBeNotNull(initializer) ||
ShouldBeSuppressed(initializer));
}
else if (member == NUnitFrameworkConstants.NameOfAssertNotNull ||
member == NUnitFrameworkConstants.NameOfAssertIsNotNull)
}
}
}

return false;
}

private static bool IsValidatedNotNullByExpression(string possibleNullReference, ExpressionSyntax expression)
{
// Check if this is an Assert for the same symbol
if (AssertHelper.IsAssert(expression, out string member, out ArgumentListSyntax? argumentList))
{
string firstArgument = argumentList.Arguments.First().Expression.ToString();

if (member == NUnitFrameworkConstants.NameOfAssertThat)
{
string? secondArgument =
argumentList.Arguments.ElementAtOrDefault(1)?.Expression.ToString();

// If test is on <nullable>.HasValue
if (IsHasValue(firstArgument, possibleNullReference))
{
// Could be:
// Assert.That(<nullable>.HasValue)
// Assert.That(<nullable>.HasValue, "Ensure Value Set")
// Assert.That(<nullable>.HasValue, Is.True)
if (secondArgument != "Is.False")
{
if (CoveredBy(firstArgument, possibleNullReference))
{
return true;
}
return true;
}
else if (member == NUnitFrameworkConstants.NameOfAssertIsTrue ||
member == NUnitFrameworkConstants.NameOfAssertTrue)
}
else
{
// Null check, could be Is.Not.Null or more complex
// like Is.Not.Null.And.Not.Empty.
if (secondArgument != "Is.Null")
{
if (IsHasValue(firstArgument, possibleNullReference))
if (CoveredBy(firstArgument, possibleNullReference))
{
return true;
}
}
}
}
else if (previous is LocalDeclarationStatementSyntax localDeclarationStatement)
else if (member == NUnitFrameworkConstants.NameOfAssertNotNull ||
member == NUnitFrameworkConstants.NameOfAssertIsNotNull)
{
VariableDeclarationSyntax declaration = localDeclarationStatement.Declaration;
foreach (var variable in declaration.Variables)
if (CoveredBy(firstArgument, possibleNullReference))
{
if (variable.Identifier.ToString() == possibleNullReference)
return true;
}
}
else if (member == NUnitFrameworkConstants.NameOfAssertIsTrue ||
member == NUnitFrameworkConstants.NameOfAssertTrue)
{
if (IsHasValue(firstArgument, possibleNullReference))
{
return true;
}
}
else if (member == NUnitFrameworkConstants.NameOfMultiple)
{
// Look up into the actual asserted parameter
if (argumentList.Arguments.FirstOrDefault()?.Expression is AnonymousFunctionExpressionSyntax anonymousFunction)
{
if (anonymousFunction.Block is not null)
{
ExpressionSyntax? initializer = variable.Initializer?.Value;
return initializer is not null &&
(IsKnownToBeNotNull(initializer) ||
ShouldBeSuppressed(initializer));
if (IsValidatedNotNullByPreviousStatementInSameBlock(possibleNullReference, anonymousFunction.Block, null))
return true;
}
else if (anonymousFunction.ExpressionBody is not null)
{
if (IsValidatedNotNullByExpression(possibleNullReference, anonymousFunction.ExpressionBody))
return true;
}
}
}
Expand Down

0 comments on commit ad20d26

Please sign in to comment.