diff --git a/src/nunit.analyzers.tests/DiagnosticSuppressors/DereferencePossiblyNullReferenceSuppressorTests.cs b/src/nunit.analyzers.tests/DiagnosticSuppressors/DereferencePossiblyNullReferenceSuppressorTests.cs index 1791baf7..cf713134 100644 --- a/src/nunit.analyzers.tests/DiagnosticSuppressors/DereferencePossiblyNullReferenceSuppressorTests.cs +++ b/src/nunit.analyzers.tests/DiagnosticSuppressors/DereferencePossiblyNullReferenceSuppressorTests.cs @@ -932,5 +932,37 @@ 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 record class Extra(string Info, int Value); + "); + + RoslynAssert.Suppressed(suppressor, + ExpectedDiagnostic.Create(DereferencePossiblyNullReferenceSuppressor.SuppressionDescriptors["CS8602"]), + testCode); + } } } diff --git a/src/nunit.analyzers.tests/DiagnosticSuppressors/NonNullableFieldOrPropertyIsUninitializedSuppressorTests.cs b/src/nunit.analyzers.tests/DiagnosticSuppressors/NonNullableFieldOrPropertyIsUninitializedSuppressorTests.cs index 22703c88..f6c43527 100644 --- a/src/nunit.analyzers.tests/DiagnosticSuppressors/NonNullableFieldOrPropertyIsUninitializedSuppressorTests.cs +++ b/src/nunit.analyzers.tests/DiagnosticSuppressors/NonNullableFieldOrPropertyIsUninitializedSuppressorTests.cs @@ -246,6 +246,39 @@ private void SetFields() RoslynAssert.Suppressed(suppressor, testCode); } + [TestCase("async Task", "await ", "", "")] + [TestCase("async Task", "await ", "this.", "")] + [TestCase("async Task", "await ", "", ".ConfigureAwait(false)")] + [TestCase("async Task", "await ", "this.", ".ConfigureAwait(false)")] + [TestCase("void", "", "", ".Wait()")] + [TestCase("void", "", "this.", ".Wait()")] + public void FieldAssignedInAsyncCalledMethod(string returnType, string awaitKeyWord, string accessor, string suffix) + { + var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@$" + private string ↓field; + + [SetUp] + public {returnType} Initialize() + {{ + {awaitKeyWord}{accessor}SetFields(){suffix}; + }} + + [Test] + public void Test() + {{ + Assert.That({accessor}field, Is.Not.Null); + }} + + private Task SetFields() + {{ + {accessor}field = string.Empty; + return Task.CompletedTask; + }} + "); + + RoslynAssert.Suppressed(suppressor, testCode); + } + [TestCase("SetUp", "")] [TestCase("OneTimeSetUp", "this.")] public void FieldAssignedInCalledMethods(string attribute, string prefix) diff --git a/src/nunit.analyzers.tests/DisposeFieldsAndPropertiesInTearDown/DisposeFieldsAndPropertiesInTearDownAnalyzerTests.cs b/src/nunit.analyzers.tests/DisposeFieldsAndPropertiesInTearDown/DisposeFieldsAndPropertiesInTearDownAnalyzerTests.cs index 78da5790..3ee6728b 100644 --- a/src/nunit.analyzers.tests/DisposeFieldsAndPropertiesInTearDown/DisposeFieldsAndPropertiesInTearDownAnalyzerTests.cs +++ b/src/nunit.analyzers.tests/DisposeFieldsAndPropertiesInTearDown/DisposeFieldsAndPropertiesInTearDownAnalyzerTests.cs @@ -225,6 +225,40 @@ public void TearDownMethod() RoslynAssert.Diagnostics(analyzer, expectedDiagnostic, testCode); } + [Test] + public void FieldConditionallyAssignedInCalledLocalMethod() + { + var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings($@" + private object? ↓field; + private bool initializeField; + + public TestClass(bool initializeField) => this.initializeField = initializeField; + + [SetUp] + public async Task SetUp() + {{ + if (initializeField) + await InitializeField().ConfigureAwait(false); + }} + + [TearDown] + public void TearDownMethod() + {{ + field = null; + }} + + private Task InitializeField() + {{ + field = new DummyDisposable(); + return Task.CompletedTask; + }} + + {DummyDisposable} + "); + + RoslynAssert.Diagnostics(analyzer, expectedDiagnostic, testCode); + } + [Test] public void AnalyzeWhenPropertyBackingFieldIsDisposed( [Values("field", "Property")] string fieldOrProperty) diff --git a/src/nunit.analyzers/DiagnosticSuppressors/DereferencePossiblyNullReferenceSuppressor.cs b/src/nunit.analyzers/DiagnosticSuppressors/DereferencePossiblyNullReferenceSuppressor.cs index 34f830ea..c4c09531 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.First().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; } } } diff --git a/src/nunit.analyzers/DiagnosticSuppressors/NonNullableFieldOrPropertyIsUninitializedSuppressor.cs b/src/nunit.analyzers/DiagnosticSuppressors/NonNullableFieldOrPropertyIsUninitializedSuppressor.cs index 0c67bbae..9e82444b 100644 --- a/src/nunit.analyzers/DiagnosticSuppressors/NonNullableFieldOrPropertyIsUninitializedSuppressor.cs +++ b/src/nunit.analyzers/DiagnosticSuppressors/NonNullableFieldOrPropertyIsUninitializedSuppressor.cs @@ -202,6 +202,17 @@ private static bool IsAssignedIn( ExpressionSyntax? expressionStatement, string fieldOrPropertyName) { + if (expressionStatement is AwaitExpressionSyntax awaitExpression) + { + expressionStatement = awaitExpression.Expression; + if (expressionStatement is InvocationExpressionSyntax awaitInvocationExpression && + awaitInvocationExpression.Expression is MemberAccessExpressionSyntax awaitMemberAccessExpression && + awaitMemberAccessExpression.Name.Identifier.Text == "ConfigureAwait") + { + expressionStatement = awaitMemberAccessExpression.Expression; + } + } + if (expressionStatement is AssignmentExpressionSyntax assignmentExpression) { if (assignmentExpression.Left is TupleExpressionSyntax tupleExpression) @@ -224,6 +235,13 @@ private static bool IsAssignedIn( } else if (expressionStatement is InvocationExpressionSyntax invocationExpression) { + if (invocationExpression.Expression is MemberAccessExpressionSyntax memberAccessExpression && + memberAccessExpression.Expression is InvocationExpressionSyntax awaitedInvocationExpression && + memberAccessExpression.Name.Identifier.Text == "Wait") + { + invocationExpression = awaitedInvocationExpression; + } + string? identifier = GetIdentifier(invocationExpression.Expression); if (!string.IsNullOrEmpty(identifier) && diff --git a/src/nunit.analyzers/DisposeFieldsAndPropertiesInTearDown/DisposeFieldsAndPropertiesInTearDownAnalyzer.cs b/src/nunit.analyzers/DisposeFieldsAndPropertiesInTearDown/DisposeFieldsAndPropertiesInTearDownAnalyzer.cs index 59ffe7c0..d0bad819 100644 --- a/src/nunit.analyzers/DisposeFieldsAndPropertiesInTearDown/DisposeFieldsAndPropertiesInTearDownAnalyzer.cs +++ b/src/nunit.analyzers/DisposeFieldsAndPropertiesInTearDown/DisposeFieldsAndPropertiesInTearDownAnalyzer.cs @@ -227,6 +227,17 @@ private static void AssignedIn(Parameters parameters, HashSet assignment private static void AssignedIn(Parameters parameters, HashSet assignments, ExpressionSyntax expression) { + if (expression is AwaitExpressionSyntax awaitExpression) + { + expression = awaitExpression.Expression; + if (expression is InvocationExpressionSyntax awaitInvocationExpression && + awaitInvocationExpression.Expression is MemberAccessExpressionSyntax awaitMemberAccessExpression && + awaitMemberAccessExpression.Name.Identifier.Text == "ConfigureAwait") + { + expression = awaitMemberAccessExpression.Expression; + } + } + if (expression is AssignmentExpressionSyntax assignmentExpression) { // We only deal with simple assignments, not tuple or deconstruct @@ -241,6 +252,13 @@ private static void AssignedIn(Parameters parameters, HashSet assignment } else if (expression is InvocationExpressionSyntax invocationExpression) { + if (invocationExpression.Expression is MemberAccessExpressionSyntax memberAccessExpression && + memberAccessExpression.Expression is InvocationExpressionSyntax awaitedInvocationExpression && + memberAccessExpression.Name.Identifier.Text == "Wait") + { + invocationExpression = awaitedInvocationExpression; + } + string? method = GetIdentifier(invocationExpression.Expression); if (method is not null) {