From ad20d26f688c2a8e6a8f639154da28e84f844435 Mon Sep 17 00:00:00 2001 From: Manfred Brands Date: Sat, 16 Sep 2023 09:32:11 +0800 Subject: [PATCH] DereferencePossiblyNullReferenceSuppressor: Search inside Assert.Multiple (#588) --- ...ncePossiblyNullReferenceSuppressorTests.cs | 42 ++++++ ...eferencePossiblyNullReferenceSuppressor.cs | 133 +++++++++++------- 2 files changed, 123 insertions(+), 52 deletions(-) diff --git a/src/nunit.analyzers.tests/DiagnosticSuppressors/DereferencePossiblyNullReferenceSuppressorTests.cs b/src/nunit.analyzers.tests/DiagnosticSuppressors/DereferencePossiblyNullReferenceSuppressorTests.cs index 1791baf7..21dc9056 100644 --- a/src/nunit.analyzers.tests/DiagnosticSuppressors/DereferencePossiblyNullReferenceSuppressorTests.cs +++ b/src/nunit.analyzers.tests/DiagnosticSuppressors/DereferencePossiblyNullReferenceSuppressorTests.cs @@ -932,5 +932,47 @@ private static void DoNothing(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); + } } } diff --git a/src/nunit.analyzers/DiagnosticSuppressors/DereferencePossiblyNullReferenceSuppressor.cs b/src/nunit.analyzers/DiagnosticSuppressors/DereferencePossiblyNullReferenceSuppressor.cs index 34f830ea..bec23d11 100644 --- a/src/nunit.analyzers/DiagnosticSuppressors/DereferencePossiblyNullReferenceSuppressor.cs +++ b/src/nunit.analyzers/DiagnosticSuppressors/DereferencePossiblyNullReferenceSuppressor.cs @@ -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; @@ -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]; @@ -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 .HasValue - if (IsHasValue(firstArgument, possibleNullReference)) - { - // Could be: - // Assert.That(.HasValue) - // Assert.That(.HasValue, "Ensure Value Set") - // Assert.That(.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 .HasValue + if (IsHasValue(firstArgument, possibleNullReference)) + { + // Could be: + // Assert.That(.HasValue) + // Assert.That(.HasValue, "Ensure Value Set") + // Assert.That(.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; } } }