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

Optimize Are(Not)Equal when actually wanting to test for Empty. #813

Merged
merged 1 commit into from
Dec 22, 2024
Merged

Optimize Are(Not)Equal when actually wanting to test for Empty. #813

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
59 changes: 59 additions & 0 deletions ...it.analyzers.tests/ClassicModelAssertUsage/AreEqualClassicModelAssertUsageCodeFixTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using Gu.Roslyn.Asserts;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.Diagnostics;
using NUnit.Analyzers.ClassicModelAssertUsage;
Expand Down Expand Up @@ -350,5 +354,60 @@ public void CodeFixMaintainsReasonableTriviaWithAllArgumentsOnSameLine([Values]

RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode, fixTitle: ClassicModelAssertUsageCodeFix.TransformToConstraintModelDescription);
}

[TestCase("string.Empty")]
[TestCase("String.Empty")]
[TestCase("Guid.Empty")]
[TestCase("\"\"")]
[TestCase("Array.Empty<int>()")]
[TestCase("Enumerable.Empty<int>()", "using System.Linq;")]
public void CodeFixUsesIsEmpty(string expected, string? additionalUsings = null)
{
var code = TestUtility.WrapInTestMethod($@"
string value = ""Value"";
↓ClassicAssert.AreEqual({expected}, value);",
additionalUsings);

var fixedCode = TestUtility.WrapInTestMethod($@"
string value = ""Value"";
Assert.That(value, Is.Empty);",
additionalUsings);

IEnumerable<MetadataReference> existingReferences = Settings.Default.MetadataReferences ?? Enumerable.Empty<MetadataReference>();

Settings settings = Settings.Default
.WithMetadataReferences(existingReferences.Concat(MetadataReferences.Transitive(typeof(ImmutableArray<>))));

RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode,
fixTitle: ClassicModelAssertUsageCodeFix.TransformToConstraintModelDescription,
settings: settings);
}

[TestCase("ImmutableArray<int>.Empty")]
[TestCase("ImmutableList<int>.Empty")]
[TestCase("ImmutableHashSet<int>.Empty")]
public void CodeFixUsesIsEmpty(string expected)
{
const string UsingSystemCollectionsImmutable = "using System.Collections.Immutable;";

var code = TestUtility.WrapInTestMethod($@"
string value = ""Value"";
↓ClassicAssert.AreEqual({expected}, value);",
UsingSystemCollectionsImmutable);

var fixedCode = TestUtility.WrapInTestMethod($@"
string value = ""Value"";
Assert.That(value, Is.Empty);",
UsingSystemCollectionsImmutable);

IEnumerable<MetadataReference> existingReferences = Settings.Default.MetadataReferences ?? Enumerable.Empty<MetadataReference>();

Settings settings = Settings.Default
.WithMetadataReferences(existingReferences.Concat(MetadataReferences.Transitive(typeof(ImmutableArray<>))));

RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode,
fixTitle: ClassicModelAssertUsageCodeFix.TransformToConstraintModelDescription,
settings: settings);
}
}
}
59 changes: 59 additions & 0 deletions ...analyzers.tests/ClassicModelAssertUsage/AreNotEqualClassicModelAssertUsageCodeFixTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using Gu.Roslyn.Asserts;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.Diagnostics;
using NUnit.Analyzers.ClassicModelAssertUsage;
Expand Down Expand Up @@ -158,5 +162,60 @@ public void CodeFixMaintainsReasonableTriviaWithAllArgumentsOnSameLine([Values]

RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode, fixTitle: ClassicModelAssertUsageCodeFix.TransformToConstraintModelDescription);
}

[TestCase("string.Empty")]
[TestCase("String.Empty")]
[TestCase("Guid.Empty")]
[TestCase("\"\"")]
[TestCase("Array.Empty<int>()")]
[TestCase("Enumerable.Empty<int>()", "using System.Linq;")]
public void CodeFixUsesIsEmpty(string expected, string? additionalUsings = null)
{
var code = TestUtility.WrapInTestMethod($@"
string value = ""Value"";
↓ClassicAssert.AreNotEqual({expected}, value);",
additionalUsings);

var fixedCode = TestUtility.WrapInTestMethod($@"
string value = ""Value"";
Assert.That(value, Is.Not.Empty);",
additionalUsings);

IEnumerable<MetadataReference> existingReferences = Settings.Default.MetadataReferences ?? Enumerable.Empty<MetadataReference>();

Settings settings = Settings.Default
.WithMetadataReferences(existingReferences.Concat(MetadataReferences.Transitive(typeof(ImmutableArray<>))));

RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode,
fixTitle: ClassicModelAssertUsageCodeFix.TransformToConstraintModelDescription,
settings: settings);
}

[TestCase("ImmutableArray<int>.Empty")]
[TestCase("ImmutableList<int>.Empty")]
[TestCase("ImmutableHashSet<int>.Empty")]
public void CodeFixUsesIsEmpty(string expected)
{
const string UsingSystemCollectionsImmutable = "using System.Collections.Immutable;";

var code = TestUtility.WrapInTestMethod($@"
string value = ""Value"";
↓ClassicAssert.AreNotEqual({expected}, value);",
UsingSystemCollectionsImmutable);

var fixedCode = TestUtility.WrapInTestMethod($@"
string value = ""Value"";
Assert.That(value, Is.Not.Empty);",
UsingSystemCollectionsImmutable);

IEnumerable<MetadataReference> existingReferences = Settings.Default.MetadataReferences ?? Enumerable.Empty<MetadataReference>();

Settings settings = Settings.Default
.WithMetadataReferences(existingReferences.Concat(MetadataReferences.Transitive(typeof(ImmutableArray<>))));

RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode,
fixTitle: ClassicModelAssertUsageCodeFix.TransformToConstraintModelDescription,
settings: settings);
}
}
}
49 changes: 32 additions & 17 deletions src/nunit.analyzers/ClassicModelAssertUsage/AreEqualClassicModelAssertUsageCodeFix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Composition;
Expand All @@ -6,6 +7,7 @@
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using NUnit.Analyzers.Constants;
using NUnit.Analyzers.Helpers;

namespace NUnit.Analyzers.ClassicModelAssertUsage
{
Expand All @@ -21,32 +23,45 @@ protected override (ArgumentSyntax ActualArgument, ArgumentSyntax? ConstraintArg
IReadOnlyDictionary<string, ArgumentSyntax> argumentNamesToArguments)
{
var expectedArgument = argumentNamesToArguments[NUnitFrameworkConstants.NameOfExpectedParameter].WithNameColon(null);
var equalToInvocationNode = SyntaxFactory.InvocationExpression(
SyntaxFactory.MemberAccessExpression(

ExpressionSyntax constraint;

if (CodeFixHelper.IsEmpty(expectedArgument.Expression))
{
constraint = SyntaxFactory.MemberAccessExpression(
SyntaxKind.SimpleMemberAccessExpression,
SyntaxFactory.IdentifierName(NUnitFrameworkConstants.NameOfIs),
SyntaxFactory.IdentifierName(NUnitFrameworkConstants.NameOfIsEqualTo)))
.WithArgumentList(SyntaxFactory.ArgumentList(
SyntaxFactory.SingletonSeparatedList(expectedArgument.WithoutTrivia())));

// The tolerance argument has to be added to the "Is.EqualTo(expected)" as ".Within(tolerance)"
if (argumentNamesToArguments.TryGetValue(NUnitFrameworkConstants.NameOfDeltaParameter, out var toleranceArgument))
SyntaxFactory.IdentifierName(NUnitFrameworkConstants.NameOfIsEmpty));
}
else
{
// The tolerance argument should be renamed from 'delta' to 'amount' but with the model constraint the
// argument is moved to Within which makes it way more explicit so we can just drop the name colon.
var toleranceArgumentNoColon = toleranceArgument.WithNameColon(null);

equalToInvocationNode = SyntaxFactory.InvocationExpression(
constraint = SyntaxFactory.InvocationExpression(
SyntaxFactory.MemberAccessExpression(
SyntaxKind.SimpleMemberAccessExpression,
equalToInvocationNode,
SyntaxFactory.IdentifierName(NUnitFrameworkConstants.NameOfEqualConstraintWithin)))
SyntaxFactory.IdentifierName(NUnitFrameworkConstants.NameOfIs),
SyntaxFactory.IdentifierName(NUnitFrameworkConstants.NameOfIsEqualTo)))
.WithArgumentList(SyntaxFactory.ArgumentList(
SyntaxFactory.SingletonSeparatedList(toleranceArgumentNoColon.WithoutTrivia())));
SyntaxFactory.SingletonSeparatedList(expectedArgument.WithoutTrivia())));

// The tolerance argument has to be added to the "Is.EqualTo(expected)" as ".Within(tolerance)"
if (argumentNamesToArguments.TryGetValue(NUnitFrameworkConstants.NameOfDeltaParameter, out var toleranceArgument))
{
// The tolerance argument should be renamed from 'delta' to 'amount' but with the model constraint the
// argument is moved to Within which makes it way more explicit so we can just drop the name colon.
var toleranceArgumentNoColon = toleranceArgument.WithNameColon(null);

constraint = SyntaxFactory.InvocationExpression(
SyntaxFactory.MemberAccessExpression(
SyntaxKind.SimpleMemberAccessExpression,
constraint,
SyntaxFactory.IdentifierName(NUnitFrameworkConstants.NameOfEqualConstraintWithin)))
.WithArgumentList(SyntaxFactory.ArgumentList(
SyntaxFactory.SingletonSeparatedList(toleranceArgumentNoColon.WithoutTrivia())));
}
}

var actualArgument = argumentNamesToArguments[NUnitFrameworkConstants.NameOfActualParameter].WithNameColon(null);
return (actualArgument, SyntaxFactory.Argument(equalToInvocationNode));
return (actualArgument, SyntaxFactory.Argument(constraint));
}
}
}
23 changes: 19 additions & 4 deletions src/nunit.analyzers/ClassicModelAssertUsage/AreNotEqualClassicModelAssertUsageCodeFix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using NUnit.Analyzers.Constants;
using NUnit.Analyzers.Helpers;

namespace NUnit.Analyzers.ClassicModelAssertUsage
{
Expand All @@ -21,8 +22,21 @@ protected override (ArgumentSyntax ActualArgument, ArgumentSyntax? ConstraintArg
IReadOnlyDictionary<string, ArgumentSyntax> argumentNamesToArguments)
{
var expectedArgument = argumentNamesToArguments[NUnitFrameworkConstants.NameOfExpectedParameter].WithNameColon(null);
var constraintArgument = SyntaxFactory.Argument(
SyntaxFactory.InvocationExpression(
ExpressionSyntax constraint;

if (CodeFixHelper.IsEmpty(expectedArgument.Expression))
{
constraint = SyntaxFactory.MemberAccessExpression(
SyntaxKind.SimpleMemberAccessExpression,
SyntaxFactory.MemberAccessExpression(
SyntaxKind.SimpleMemberAccessExpression,
SyntaxFactory.IdentifierName(NUnitFrameworkConstants.NameOfIs),
SyntaxFactory.IdentifierName(NUnitFrameworkConstants.NameOfIsNot)),
SyntaxFactory.IdentifierName(NUnitFrameworkConstants.NameOfIsEmpty));
}
else
{
constraint = SyntaxFactory.InvocationExpression(
SyntaxFactory.MemberAccessExpression(
SyntaxKind.SimpleMemberAccessExpression,
SyntaxFactory.MemberAccessExpression(
Expand All @@ -31,10 +45,11 @@ protected override (ArgumentSyntax ActualArgument, ArgumentSyntax? ConstraintArg
SyntaxFactory.IdentifierName(NUnitFrameworkConstants.NameOfIsNot)),
SyntaxFactory.IdentifierName(NUnitFrameworkConstants.NameOfIsEqualTo)))
.WithArgumentList(SyntaxFactory.ArgumentList(
SyntaxFactory.SingletonSeparatedList(expectedArgument))));
SyntaxFactory.SingletonSeparatedList(expectedArgument)));
}

var actualArgument = argumentNamesToArguments[NUnitFrameworkConstants.NameOfActualParameter].WithNameColon(null);
return (actualArgument, constraintArgument);
return (actualArgument, SyntaxFactory.Argument(constraint));
}
}
}
30 changes: 30 additions & 0 deletions src/nunit.analyzers/Helpers/CodeFixHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
Expand Down Expand Up @@ -129,6 +130,35 @@ public static void UpdateStringFormatToFormattableString(List<ArgumentSyntax> ar
arguments.RemoveRange(nextArgument, arguments.Count - nextArgument);
}

/// <summary>
/// Checks if the given expression is something that is covered by the EmptyConstraint.
/// </summary>
internal static bool IsEmpty(ExpressionSyntax expression)
{
if (expression is LiteralExpressionSyntax literalExpression && literalExpression.Token.ValueText.Length == 0)
{
return true;
}

if (expression is MemberAccessExpressionSyntax memberProperty &&
memberProperty.Name.Identifier.Text == nameof(string.Empty))
{
// Intends to covers string.Empty, ImmutableXXX<T>.Empty and other similar cases.
return true;
}

if (expression is InvocationExpressionSyntax invocationExpression &&
invocationExpression.ArgumentList.Arguments.Count == 0 &&
invocationExpression.Expression is MemberAccessExpressionSyntax memberMethod &&
memberMethod.Name is GenericNameSyntax genericName && genericName.Identifier.Text == nameof(Array.Empty))
{
// Intends to covers Array.Empty<T>(), Enumerable.Empty<T> and other similar cases.
return true;
}

return false;
}

internal static IEnumerable<InterpolatedStringContentSyntax> UpdateStringFormatToFormattableString(string formatSpecification, ExpressionSyntax[] formatArguments)
{
int startIndex = 0;
Expand Down
Loading