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

Add support for Assert.EnterMultipleScope #816

Merged
merged 2 commits into from
Jan 8, 2025
Merged

Add support for Assert.EnterMultipleScope #816

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
804395a
Detect `using (Assert.EnterMultipleScope())` as an `Assert.Multiple` …
manfred-brands Dec 23, 2024
cf7eacb
Add CodeFix to add `using (Assert.EnterMultipleScope())`
manfred-brands Dec 23, 2024
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
2 changes: 2 additions & 0 deletions src/nunit.analyzers.tests/Constants/NUnitFrameworkConstantsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,10 @@ public sealed class NUnitFrameworkConstantsTests
(nameof(NUnitFrameworkConstants.NameOfMultiple), nameof(Assert.Multiple)),
#if NUNIT4
(nameof(NUnitFrameworkConstants.NameOfMultipleAsync), nameof(Assert.MultipleAsync)),
(nameof(NUnitFrameworkConstants.NameOfEnterMultipleScope), nameof(Assert.EnterMultipleScope)),
#else
(nameof(NUnitFrameworkConstants.NameOfMultipleAsync), "MultipleAsync"),
(nameof(NUnitFrameworkConstants.NameOfEnterMultipleScope), "EnterMultipleScope"),
#endif

(nameof(NUnitFrameworkConstants.NameOfOut), nameof(TestContext.Out)),
Expand Down
32 changes: 32 additions & 0 deletions src/nunit.analyzers.tests/UseAssertMultiple/UseAssertMultipleAnalyzerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,38 @@ await Assert.MultipleAsync(async () =>
}");
RoslynAssert.Valid(this.analyzer, testCode);
}

#if WOULD_SOMEONE_ACTUALLY_USE_THIS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason for why we wouldn't want to run this test :) ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it won't work as we don't have support for it.
It would require tracking the declared variable and check if it is manually disposed somewhere within the scope.

Unless someone has a good reason for using this pattern, I don't see any, I won't spend my time on it.

[Test]
public void AnalyzeWhenMultipleScopeDeclarationIsUsed()
{
var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
public void Test()
{
using IDisposable disposable = Assert.EnterMultipleScope();

Assert.That(true, Is.True);
disposable.Dispose();
Assert.That(false, Is.False);
}");
RoslynAssert.Valid(this.analyzer, testCode);
}
#endif

[Test]
public void AnalyzeWhenMultipleScopeStatementIsUsed()
{
var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
public void Test()
{
using (Assert.EnterMultipleScope())
{
Assert.That(true, Is.True);
Assert.That(false, Is.False);
}
}");
RoslynAssert.Valid(this.analyzer, testCode);
}
#endif

[Test]
Expand Down
171 changes: 128 additions & 43 deletions src/nunit.analyzers.tests/UseAssertMultiple/UseAssertMultipleCodeFixTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public void TestMethod()
Assert.That(false, Is.False);
Console.WriteLine(""Next Statement"");
}");

var fixedCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
public void TestMethod()
{
Expand All @@ -44,12 +45,36 @@ public void TestMethod()
});
Console.WriteLine(""Next Statement"");
}");
RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode);

RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode, UseAssertMultipleCodeFix.WrapWithAssertMultiple);

#if NUNIT4
fixedCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
public void TestMethod()
{
using (Assert.EnterMultipleScope())
{
Assert.That(true, Is.True);
Assert.That(false, Is.False);
}
Console.WriteLine(""Next Statement"");
}");

RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode, UseAssertMultipleCodeFix.WrapWithAssertEnterMultipleScope);
#endif
}

[Test]
public void VerifyPartlyIndependent()
{
const string ConfigurationClass = @"
private sealed class Configuration
{
public int Value1 { get; set; }
public double Value2 { get; set; }
public string Value11 { get; set; } = string.Empty;
}";

var code = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
public void Test()
{
Expand All @@ -59,14 +84,8 @@ public void Test()
Assert.That(configuration.Value2, Is.EqualTo(0.0));
Assert.That(configuration.Value11, Is.EqualTo(string.Empty));
configuration = null;
}
}" + ConfigurationClass);

private sealed class Configuration
{
public int Value1 { get; set; }
public double Value2 { get; set; }
public string Value11 { get; set; } = string.Empty;
}");
var fixedCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
public void Test()
{
Expand All @@ -79,40 +98,54 @@ public void Test()
Assert.That(configuration.Value11, Is.EqualTo(string.Empty));
});
configuration = null;
}" + ConfigurationClass);

RoslynAssert.FixAll(analyzer, fix, expectedDiagnostic, code, fixedCode, UseAssertMultipleCodeFix.WrapWithAssertMultiple);

#if NUNIT4
fixedCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
public void Test()
{
var configuration = new Configuration();
Assert.That(configuration, Is.Not.Null);
using (Assert.EnterMultipleScope())
{
Assert.That(configuration.Value1, Is.EqualTo(0));
Assert.That(configuration.Value2, Is.EqualTo(0.0));
Assert.That(configuration.Value11, Is.EqualTo(string.Empty));
}
configuration = null;
}" + ConfigurationClass);

RoslynAssert.FixAll(analyzer, fix, expectedDiagnostic, code, fixedCode, UseAssertMultipleCodeFix.WrapWithAssertEnterMultipleScope);
#endif
}

[Test]
public void AddsAsyncWhenAwaitIsUsed()
{
const string ConfigurationClass = @"
private sealed class Configuration
{
public int Value1 { get; set; }
public double Value2 { get; set; }
public string Value11 { get; set; } = string.Empty;
}");
RoslynAssert.FixAll(analyzer, fix, expectedDiagnostic, code, fixedCode);
}
public Task<string> AsStringAsync() => Task.FromResult(Value11);
}";

[Test]
public void AddsAsyncWhenAwaitIsUsed()
{
var code = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
public void Test()
public async Task Test()
{
var configuration = new Configuration();
Assert.That(configuration, Is.Not.Null);
↓Assert.That(configuration.Value1, Is.EqualTo(0));
Assert.That(configuration.Value2, Is.EqualTo(0.0));
Assert.That(await configuration.AsStringAsync(), Is.EqualTo(string.Empty));
configuration = null;
}
}" + ConfigurationClass);

private sealed class Configuration
{
public int Value1 { get; set; }
public double Value2 { get; set; }
public string Value11 { get; set; } = string.Empty;
public Task<string> AsStringAsync() => Task.FromResult(Value11);
}");
var fixedCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
public void Test()
public async Task Test()
{
var configuration = new Configuration();
Assert.That(configuration, Is.Not.Null);
Expand All @@ -123,16 +156,30 @@ public void Test()
Assert.That(await configuration.AsStringAsync(), Is.EqualTo(string.Empty));
});
configuration = null;
}
}" + ConfigurationClass);

private sealed class Configuration
// The test method itself no longer awaits, so CS1998 is generated.
// Fixing this is outside the scope of this analyzer and there could be other non-touched statements that are waited.
RoslynAssert.FixAll(analyzer, fix, expectedDiagnostic, code, fixedCode, UseAssertMultipleCodeFix.WrapWithAssertMultiple,
Settings.Default.WithAllowedCompilerDiagnostics(AllowedCompilerDiagnostics.WarningsAndErrors));

#if NUNIT4
fixedCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
public async Task Test()
{
public int Value1 { get; set; }
public double Value2 { get; set; }
public string Value11 { get; set; } = string.Empty;
public Task<string> AsStringAsync() => Task.FromResult(Value11);
}");
RoslynAssert.FixAll(analyzer, fix, expectedDiagnostic, code, fixedCode);
var configuration = new Configuration();
Assert.That(configuration, Is.Not.Null);
using (Assert.EnterMultipleScope())
{
Assert.That(configuration.Value1, Is.EqualTo(0));
Assert.That(configuration.Value2, Is.EqualTo(0.0));
Assert.That(await configuration.AsStringAsync(), Is.EqualTo(string.Empty));
}
configuration = null;
}" + ConfigurationClass);

RoslynAssert.FixAll(analyzer, fix, expectedDiagnostic, code, fixedCode, UseAssertMultipleCodeFix.WrapWithAssertEnterMultipleScope);
#endif
}

[Test]
Expand All @@ -152,6 +199,7 @@ public void TestMethod()
Assert.That(False, Is.False);{newline}
{preComment}Console.WriteLine(""Next Statement"");{postComment}
}}");

var fixedCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@$"
public void TestMethod()
{{
Expand All @@ -166,30 +214,67 @@ public void TestMethod()
}});{newline}
{preComment}Console.WriteLine(""Next Statement"");{postComment}
}}");
RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode);

RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode, UseAssertMultipleCodeFix.WrapWithAssertMultiple);

#if NUNIT4
fixedCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@$"
public void TestMethod()
{{
const bool True = true;
const bool False = false;

using (Assert.EnterMultipleScope())
{{
// Verify that our bool constants are correct
Assert.That(True, Is.True);
Assert.That(False, Is.False);
}}{newline}
{preComment}Console.WriteLine(""Next Statement"");{postComment}
}}");

RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode, UseAssertMultipleCodeFix.WrapWithAssertEnterMultipleScope);
#endif
}

[Test]
public void VerifyKeepsTrivia()
{
var code = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@$"
var code = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
public void TestMethod()
{{
// Verify that boolean work as expected
{
// Verify that boolean work as expected
↓Assert.That(true, Is.True);
Assert.That(false, Is.False);
}}");
var fixedCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@$"
}");

var fixedCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
public void TestMethod()
{{
{
Assert.Multiple(() =>
{{
{
// Verify that boolean work as expected
Assert.That(true, Is.True);
Assert.That(false, Is.False);
}});
}}");
RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode);
});
}");

RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode, UseAssertMultipleCodeFix.WrapWithAssertMultiple);

#if NUNIT4
fixedCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
public void TestMethod()
{
using (Assert.EnterMultipleScope())
{
// Verify that boolean work as expected
Assert.That(true, Is.True);
Assert.That(false, Is.False);
}
}");

RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode, UseAssertMultipleCodeFix.WrapWithAssertEnterMultipleScope);
#endif
}
}
}
2 changes: 2 additions & 0 deletions src/nunit.analyzers/Constants/AnalyzerPropertyKeys.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,7 @@ internal static class AnalyzerPropertyKeys
internal const string ModelName = nameof(AnalyzerPropertyKeys.ModelName);
internal const string ArgsIsArray = nameof(AnalyzerPropertyKeys.ArgsIsArray);
internal const string MinimumNumberOfArguments = nameof(AnalyzerPropertyKeys.MinimumNumberOfArguments);

internal const string SupportsEnterMultipleScope = nameof(AnalyzerPropertyKeys.SupportsEnterMultipleScope);
}
}
1 change: 1 addition & 0 deletions src/nunit.analyzers/Constants/NUnitFrameworkConstants.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ public static class NUnitFrameworkConstants

public const string NameOfMultiple = "Multiple";
public const string NameOfMultipleAsync = "MultipleAsync";
public const string NameOfEnterMultipleScope = "EnterMultipleScope";

public const string NameOfOut = "Out";
public const string NameOfWrite = "Write";
Expand Down
27 changes: 22 additions & 5 deletions src/nunit.analyzers/Helpers/AssertHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,18 +95,35 @@ public static bool IsLiteralOperation(IOperation operation)
/// </summary>
public static bool IsInsideAssertMultiple(SyntaxNode node)
{
InvocationExpressionSyntax? possibleAssertMultiple;

while ((possibleAssertMultiple = node.Ancestors().OfType<InvocationExpressionSyntax>().FirstOrDefault()) is not null)
// Look for Assert.Multiple(delegate) invocation.
SyntaxNode currentNode = node;
InvocationExpressionSyntax? possibleAssertMultipleInvocation;
while ((possibleAssertMultipleInvocation = currentNode.Ancestors().OfType<InvocationExpressionSyntax>().FirstOrDefault()) is not null)
{
// Is the statement inside a Block which is part of an Assert.Multiple.
if (IsAssert(possibleAssertMultiple, NUnitFrameworkConstants.NameOfMultiple, NUnitFrameworkConstants.NameOfMultipleAsync))
if (IsAssert(possibleAssertMultipleInvocation, NUnitFrameworkConstants.NameOfMultiple, NUnitFrameworkConstants.NameOfMultipleAsync))
{
return true;
}

// Keep looking at possible parent nested expression.
currentNode = possibleAssertMultipleInvocation;
}

// Look for using (Assert.EnterMultipleScope()) invocation.
currentNode = node;
UsingStatementSyntax? usingStatement;
while ((usingStatement = currentNode.Ancestors().OfType<UsingStatementSyntax>().FirstOrDefault()) is not null)
{
// Is the using expression an Assert.EnterMultipleScope.
if (usingStatement.Expression is InvocationExpressionSyntax usingInvocation &&
IsAssert(usingInvocation, NUnitFrameworkConstants.NameOfEnterMultipleScope))
{
return true;
}

// Keep looking at possible parent nested expression.
node = possibleAssertMultiple;
currentNode = usingStatement;
}

return false;
Expand Down
10 changes: 8 additions & 2 deletions src/nunit.analyzers/UseAssertMultiple/UseAssertMultipleAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ namespace NUnit.Analyzers.UseAssertMultiple
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class UseAssertMultipleAnalyzer : BaseAssertionAnalyzer
{
private static readonly Version firstNUnitVersionWithEnterMultipleScope = new Version(4, 2);

private static readonly DiagnosticDescriptor descriptor = DiagnosticDescriptorCreator.Create(
id: AnalyzerIdentifiers.UseAssertMultiple,
title: UseAssertMultipleConstants.Title,
Expand Down Expand Up @@ -67,7 +69,7 @@ internal static void Add(HashSet<string> previousArguments, string argument)
}
}

protected override void AnalyzeAssertInvocation(OperationAnalysisContext context, IInvocationOperation assertOperation)
protected override void AnalyzeAssertInvocation(Version nunitVersion, OperationAnalysisContext context, IInvocationOperation assertOperation)
{
if (assertOperation.TargetMethod.Name != NUnitFrameworkConstants.NameOfAssertThat ||
AssertHelper.IsInsideAssertMultiple(assertOperation.Syntax))
Expand Down Expand Up @@ -134,7 +136,11 @@ protected override void AnalyzeAssertInvocation(OperationAnalysisContext context

if (lastAssert > firstAssert)
{
context.ReportDiagnostic(Diagnostic.Create(descriptor, assertOperation.Syntax.GetLocation()));
var properties = ImmutableDictionary.CreateBuilder<string, string?>();
properties.Add(AnalyzerPropertyKeys.SupportsEnterMultipleScope,
nunitVersion >= firstNUnitVersionWithEnterMultipleScope ?
NUnitFrameworkConstants.NameOfEnterMultipleScope : null);
context.ReportDiagnostic(Diagnostic.Create(descriptor, assertOperation.Syntax.GetLocation(), properties.ToImmutable()));
}
}
}
Expand Down
Loading
Loading