Skip to content

Commit 7b7bb03

Browse files
mikkelbumikkelbu
authored
Merge pull request #813 from nunit/Issue801_NUnit2005
Optimize Are(Not)Equal when actually wanting to test for Empty.
2 parents 86f8c23 + 3095eae commit 7b7bb03

File tree

5 files changed

+199
-21
lines changed

5 files changed

+199
-21
lines changed

‎src/nunit.analyzers.tests/ClassicModelAssertUsage/AreEqualClassicModelAssertUsageCodeFixTests.cs‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1+
using System.Collections.Generic;
2+
using System.Collections.Immutable;
3+
using System.Linq;
14
using Gu.Roslyn.Asserts;
5+
using Microsoft.CodeAnalysis;
26
using Microsoft.CodeAnalysis.CodeFixes;
37
using Microsoft.CodeAnalysis.Diagnostics;
48
using NUnit.Analyzers.ClassicModelAssertUsage;
@@ -350,5 +354,60 @@ public void CodeFixMaintainsReasonableTriviaWithAllArgumentsOnSameLine([Values]
350354

351355
RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode, fixTitle: ClassicModelAssertUsageCodeFix.TransformToConstraintModelDescription);
352356
}
357+
358+
[TestCase("string.Empty")]
359+
[TestCase("String.Empty")]
360+
[TestCase("Guid.Empty")]
361+
[TestCase("\"\"")]
362+
[TestCase("Array.Empty<int>()")]
363+
[TestCase("Enumerable.Empty<int>()", "using System.Linq;")]
364+
public void CodeFixUsesIsEmpty(string expected, string? additionalUsings = null)
365+
{
366+
var code = TestUtility.WrapInTestMethod($@"
367+
string value = ""Value"";
368+
↓ClassicAssert.AreEqual({expected}, value);",
369+
additionalUsings);
370+
371+
var fixedCode = TestUtility.WrapInTestMethod($@"
372+
string value = ""Value"";
373+
Assert.That(value, Is.Empty);",
374+
additionalUsings);
375+
376+
IEnumerable<MetadataReference> existingReferences = Settings.Default.MetadataReferences ?? Enumerable.Empty<MetadataReference>();
377+
378+
Settings settings = Settings.Default
379+
.WithMetadataReferences(existingReferences.Concat(MetadataReferences.Transitive(typeof(ImmutableArray<>))));
380+
381+
RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode,
382+
fixTitle: ClassicModelAssertUsageCodeFix.TransformToConstraintModelDescription,
383+
settings: settings);
384+
}
385+
386+
[TestCase("ImmutableArray<int>.Empty")]
387+
[TestCase("ImmutableList<int>.Empty")]
388+
[TestCase("ImmutableHashSet<int>.Empty")]
389+
public void CodeFixUsesIsEmpty(string expected)
390+
{
391+
const string UsingSystemCollectionsImmutable = "using System.Collections.Immutable;";
392+
393+
var code = TestUtility.WrapInTestMethod($@"
394+
string value = ""Value"";
395+
↓ClassicAssert.AreEqual({expected}, value);",
396+
UsingSystemCollectionsImmutable);
397+
398+
var fixedCode = TestUtility.WrapInTestMethod($@"
399+
string value = ""Value"";
400+
Assert.That(value, Is.Empty);",
401+
UsingSystemCollectionsImmutable);
402+
403+
IEnumerable<MetadataReference> existingReferences = Settings.Default.MetadataReferences ?? Enumerable.Empty<MetadataReference>();
404+
405+
Settings settings = Settings.Default
406+
.WithMetadataReferences(existingReferences.Concat(MetadataReferences.Transitive(typeof(ImmutableArray<>))));
407+
408+
RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode,
409+
fixTitle: ClassicModelAssertUsageCodeFix.TransformToConstraintModelDescription,
410+
settings: settings);
411+
}
353412
}
354413
}

‎src/nunit.analyzers.tests/ClassicModelAssertUsage/AreNotEqualClassicModelAssertUsageCodeFixTests.cs‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1+
using System.Collections.Generic;
2+
using System.Collections.Immutable;
3+
using System.Linq;
14
using Gu.Roslyn.Asserts;
5+
using Microsoft.CodeAnalysis;
26
using Microsoft.CodeAnalysis.CodeFixes;
37
using Microsoft.CodeAnalysis.Diagnostics;
48
using NUnit.Analyzers.ClassicModelAssertUsage;
@@ -158,5 +162,60 @@ public void CodeFixMaintainsReasonableTriviaWithAllArgumentsOnSameLine([Values]
158162

159163
RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode, fixTitle: ClassicModelAssertUsageCodeFix.TransformToConstraintModelDescription);
160164
}
165+
166+
[TestCase("string.Empty")]
167+
[TestCase("String.Empty")]
168+
[TestCase("Guid.Empty")]
169+
[TestCase("\"\"")]
170+
[TestCase("Array.Empty<int>()")]
171+
[TestCase("Enumerable.Empty<int>()", "using System.Linq;")]
172+
public void CodeFixUsesIsEmpty(string expected, string? additionalUsings = null)
173+
{
174+
var code = TestUtility.WrapInTestMethod($@"
175+
string value = ""Value"";
176+
↓ClassicAssert.AreNotEqual({expected}, value);",
177+
additionalUsings);
178+
179+
var fixedCode = TestUtility.WrapInTestMethod($@"
180+
string value = ""Value"";
181+
Assert.That(value, Is.Not.Empty);",
182+
additionalUsings);
183+
184+
IEnumerable<MetadataReference> existingReferences = Settings.Default.MetadataReferences ?? Enumerable.Empty<MetadataReference>();
185+
186+
Settings settings = Settings.Default
187+
.WithMetadataReferences(existingReferences.Concat(MetadataReferences.Transitive(typeof(ImmutableArray<>))));
188+
189+
RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode,
190+
fixTitle: ClassicModelAssertUsageCodeFix.TransformToConstraintModelDescription,
191+
settings: settings);
192+
}
193+
194+
[TestCase("ImmutableArray<int>.Empty")]
195+
[TestCase("ImmutableList<int>.Empty")]
196+
[TestCase("ImmutableHashSet<int>.Empty")]
197+
public void CodeFixUsesIsEmpty(string expected)
198+
{
199+
const string UsingSystemCollectionsImmutable = "using System.Collections.Immutable;";
200+
201+
var code = TestUtility.WrapInTestMethod($@"
202+
string value = ""Value"";
203+
↓ClassicAssert.AreNotEqual({expected}, value);",
204+
UsingSystemCollectionsImmutable);
205+
206+
var fixedCode = TestUtility.WrapInTestMethod($@"
207+
string value = ""Value"";
208+
Assert.That(value, Is.Not.Empty);",
209+
UsingSystemCollectionsImmutable);
210+
211+
IEnumerable<MetadataReference> existingReferences = Settings.Default.MetadataReferences ?? Enumerable.Empty<MetadataReference>();
212+
213+
Settings settings = Settings.Default
214+
.WithMetadataReferences(existingReferences.Concat(MetadataReferences.Transitive(typeof(ImmutableArray<>))));
215+
216+
RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode,
217+
fixTitle: ClassicModelAssertUsageCodeFix.TransformToConstraintModelDescription,
218+
settings: settings);
219+
}
161220
}
162221
}

‎src/nunit.analyzers/ClassicModelAssertUsage/AreEqualClassicModelAssertUsageCodeFix.cs‎

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System;
12
using System.Collections.Generic;
23
using System.Collections.Immutable;
34
using System.Composition;
@@ -6,6 +7,7 @@
67
using Microsoft.CodeAnalysis.CSharp;
78
using Microsoft.CodeAnalysis.CSharp.Syntax;
89
using NUnit.Analyzers.Constants;
10+
using NUnit.Analyzers.Helpers;
911

1012
namespace NUnit.Analyzers.ClassicModelAssertUsage
1113
{
@@ -21,32 +23,45 @@ protected override (ArgumentSyntax ActualArgument, ArgumentSyntax? ConstraintArg
2123
IReadOnlyDictionary<string, ArgumentSyntax> argumentNamesToArguments)
2224
{
2325
var expectedArgument = argumentNamesToArguments[NUnitFrameworkConstants.NameOfExpectedParameter].WithNameColon(null);
24-
var equalToInvocationNode = SyntaxFactory.InvocationExpression(
25-
SyntaxFactory.MemberAccessExpression(
26+
27+
ExpressionSyntax constraint;
28+
29+
if (CodeFixHelper.IsEmpty(expectedArgument.Expression))
30+
{
31+
constraint = SyntaxFactory.MemberAccessExpression(
2632
SyntaxKind.SimpleMemberAccessExpression,
2733
SyntaxFactory.IdentifierName(NUnitFrameworkConstants.NameOfIs),
28-
SyntaxFactory.IdentifierName(NUnitFrameworkConstants.NameOfIsEqualTo)))
29-
.WithArgumentList(SyntaxFactory.ArgumentList(
30-
SyntaxFactory.SingletonSeparatedList(expectedArgument.WithoutTrivia())));
31-
32-
// The tolerance argument has to be added to the "Is.EqualTo(expected)" as ".Within(tolerance)"
33-
if (argumentNamesToArguments.TryGetValue(NUnitFrameworkConstants.NameOfDeltaParameter, out var toleranceArgument))
34+
SyntaxFactory.IdentifierName(NUnitFrameworkConstants.NameOfIsEmpty));
35+
}
36+
else
3437
{
35-
// The tolerance argument should be renamed from 'delta' to 'amount' but with the model constraint the
36-
// argument is moved to Within which makes it way more explicit so we can just drop the name colon.
37-
var toleranceArgumentNoColon = toleranceArgument.WithNameColon(null);
38-
39-
equalToInvocationNode = SyntaxFactory.InvocationExpression(
38+
constraint = SyntaxFactory.InvocationExpression(
4039
SyntaxFactory.MemberAccessExpression(
4140
SyntaxKind.SimpleMemberAccessExpression,
42-
equalToInvocationNode,
43-
SyntaxFactory.IdentifierName(NUnitFrameworkConstants.NameOfEqualConstraintWithin)))
41+
SyntaxFactory.IdentifierName(NUnitFrameworkConstants.NameOfIs),
42+
SyntaxFactory.IdentifierName(NUnitFrameworkConstants.NameOfIsEqualTo)))
4443
.WithArgumentList(SyntaxFactory.ArgumentList(
45-
SyntaxFactory.SingletonSeparatedList(toleranceArgumentNoColon.WithoutTrivia())));
44+
SyntaxFactory.SingletonSeparatedList(expectedArgument.WithoutTrivia())));
45+
46+
// The tolerance argument has to be added to the "Is.EqualTo(expected)" as ".Within(tolerance)"
47+
if (argumentNamesToArguments.TryGetValue(NUnitFrameworkConstants.NameOfDeltaParameter, out var toleranceArgument))
48+
{
49+
// The tolerance argument should be renamed from 'delta' to 'amount' but with the model constraint the
50+
// argument is moved to Within which makes it way more explicit so we can just drop the name colon.
51+
var toleranceArgumentNoColon = toleranceArgument.WithNameColon(null);
52+
53+
constraint = SyntaxFactory.InvocationExpression(
54+
SyntaxFactory.MemberAccessExpression(
55+
SyntaxKind.SimpleMemberAccessExpression,
56+
constraint,
57+
SyntaxFactory.IdentifierName(NUnitFrameworkConstants.NameOfEqualConstraintWithin)))
58+
.WithArgumentList(SyntaxFactory.ArgumentList(
59+
SyntaxFactory.SingletonSeparatedList(toleranceArgumentNoColon.WithoutTrivia())));
60+
}
4661
}
4762

4863
var actualArgument = argumentNamesToArguments[NUnitFrameworkConstants.NameOfActualParameter].WithNameColon(null);
49-
return (actualArgument, SyntaxFactory.Argument(equalToInvocationNode));
64+
return (actualArgument, SyntaxFactory.Argument(constraint));
5065
}
5166
}
5267
}

‎src/nunit.analyzers/ClassicModelAssertUsage/AreNotEqualClassicModelAssertUsageCodeFix.cs‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using Microsoft.CodeAnalysis.CSharp;
77
using Microsoft.CodeAnalysis.CSharp.Syntax;
88
using NUnit.Analyzers.Constants;
9+
using NUnit.Analyzers.Helpers;
910

1011
namespace NUnit.Analyzers.ClassicModelAssertUsage
1112
{
@@ -21,8 +22,21 @@ protected override (ArgumentSyntax ActualArgument, ArgumentSyntax? ConstraintArg
2122
IReadOnlyDictionary<string, ArgumentSyntax> argumentNamesToArguments)
2223
{
2324
var expectedArgument = argumentNamesToArguments[NUnitFrameworkConstants.NameOfExpectedParameter].WithNameColon(null);
24-
var constraintArgument = SyntaxFactory.Argument(
25-
SyntaxFactory.InvocationExpression(
25+
ExpressionSyntax constraint;
26+
27+
if (CodeFixHelper.IsEmpty(expectedArgument.Expression))
28+
{
29+
constraint = SyntaxFactory.MemberAccessExpression(
30+
SyntaxKind.SimpleMemberAccessExpression,
31+
SyntaxFactory.MemberAccessExpression(
32+
SyntaxKind.SimpleMemberAccessExpression,
33+
SyntaxFactory.IdentifierName(NUnitFrameworkConstants.NameOfIs),
34+
SyntaxFactory.IdentifierName(NUnitFrameworkConstants.NameOfIsNot)),
35+
SyntaxFactory.IdentifierName(NUnitFrameworkConstants.NameOfIsEmpty));
36+
}
37+
else
38+
{
39+
constraint = SyntaxFactory.InvocationExpression(
2640
SyntaxFactory.MemberAccessExpression(
2741
SyntaxKind.SimpleMemberAccessExpression,
2842
SyntaxFactory.MemberAccessExpression(
@@ -31,10 +45,11 @@ protected override (ArgumentSyntax ActualArgument, ArgumentSyntax? ConstraintArg
3145
SyntaxFactory.IdentifierName(NUnitFrameworkConstants.NameOfIsNot)),
3246
SyntaxFactory.IdentifierName(NUnitFrameworkConstants.NameOfIsEqualTo)))
3347
.WithArgumentList(SyntaxFactory.ArgumentList(
34-
SyntaxFactory.SingletonSeparatedList(expectedArgument))));
48+
SyntaxFactory.SingletonSeparatedList(expectedArgument)));
49+
}
3550

3651
var actualArgument = argumentNamesToArguments[NUnitFrameworkConstants.NameOfActualParameter].WithNameColon(null);
37-
return (actualArgument, constraintArgument);
52+
return (actualArgument, SyntaxFactory.Argument(constraint));
3853
}
3954
}
4055
}

‎src/nunit.analyzers/Helpers/CodeFixHelper.cs‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System;
12
using System.Collections.Generic;
23
using System.Globalization;
34
using System.Linq;
@@ -129,6 +130,35 @@ public static void UpdateStringFormatToFormattableString(List<ArgumentSyntax> ar
129130
arguments.RemoveRange(nextArgument, arguments.Count - nextArgument);
130131
}
131132

133+
/// <summary>
134+
/// Checks if the given expression is something that is covered by the EmptyConstraint.
135+
/// </summary>
136+
internal static bool IsEmpty(ExpressionSyntax expression)
137+
{
138+
if (expression is LiteralExpressionSyntax literalExpression && literalExpression.Token.ValueText.Length == 0)
139+
{
140+
return true;
141+
}
142+
143+
if (expression is MemberAccessExpressionSyntax memberProperty &&
144+
memberProperty.Name.Identifier.Text == nameof(string.Empty))
145+
{
146+
// Intends to covers string.Empty, ImmutableXXX<T>.Empty and other similar cases.
147+
return true;
148+
}
149+
150+
if (expression is InvocationExpressionSyntax invocationExpression &&
151+
invocationExpression.ArgumentList.Arguments.Count == 0 &&
152+
invocationExpression.Expression is MemberAccessExpressionSyntax memberMethod &&
153+
memberMethod.Name is GenericNameSyntax genericName && genericName.Identifier.Text == nameof(Array.Empty))
154+
{
155+
// Intends to covers Array.Empty<T>(), Enumerable.Empty<T> and other similar cases.
156+
return true;
157+
}
158+
159+
return false;
160+
}
161+
132162
internal static IEnumerable<InterpolatedStringContentSyntax> UpdateStringFormatToFormattableString(string formatSpecification, ExpressionSyntax[] formatArguments)
133163
{
134164
int startIndex = 0;

0 commit comments

Comments
 (0)