Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Descend into local asynchronous methods. #586

Closed
wants to merge 2 commits into from
Closed

Descend into local asynchronous methods. #586

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cfe5e9d
Descend into local asynchronous methods.
manfred-brands Sep 7, 2023
3d13a40
DereferencePossiblyNullReferenceSuppressor: Search inside Assert.Mult…
manfred-brands Sep 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions ....analyzers.tests/DiagnosticSuppressors/DereferencePossiblyNullReferenceSuppressorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -932,5 +932,37 @@ 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 record class Extra(string Info, int Value);
");

RoslynAssert.Suppressed(suppressor,
ExpectedDiagnostic.Create(DereferencePossiblyNullReferenceSuppressor.SuppressionDescriptors["CS8602"]),
testCode);
}
}
}
33 changes: 33 additions & 0 deletions ...s.tests/DiagnosticSuppressors/NonNullableFieldOrPropertyIsUninitializedSuppressorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
34 changes: 34 additions & 0 deletions ...DisposeFieldsAndPropertiesInTearDown/DisposeFieldsAndPropertiesInTearDownAnalyzerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
133 changes: 81 additions & 52 deletions src/nunit.analyzers/DiagnosticSuppressors/DereferencePossiblyNullReferenceSuppressor.cs
View file
Open in desktop
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.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;
}
}
}
Expand Down
18 changes: 18 additions & 0 deletions ...it.analyzers/DiagnosticSuppressors/NonNullableFieldOrPropertyIsUninitializedSuppressor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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) &&
Expand Down
18 changes: 18 additions & 0 deletions ...zers/DisposeFieldsAndPropertiesInTearDown/DisposeFieldsAndPropertiesInTearDownAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,17 @@ private static void AssignedIn(Parameters parameters, HashSet<string> assignment

private static void AssignedIn(Parameters parameters, HashSet<string> 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
Expand All @@ -241,6 +252,13 @@ private static void AssignedIn(Parameters parameters, HashSet<string> 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)
{
Expand Down
Loading