From 3095eaea42eb33f4a44200a5ab86bd1523aa1ee0 Mon Sep 17 00:00:00 2001 From: Manfred Brands Date: Sun, 22 Dec 2024 13:43:24 +0800 Subject: [PATCH] Optimize Are(Not)Equal when actually wanting to test for Empty. --- ...qualClassicModelAssertUsageCodeFixTests.cs | 59 +++++++++++++++++++ ...qualClassicModelAssertUsageCodeFixTests.cs | 59 +++++++++++++++++++ .../AreEqualClassicModelAssertUsageCodeFix.cs | 49 +++++++++------ ...eNotEqualClassicModelAssertUsageCodeFix.cs | 23 ++++++-- src/nunit.analyzers/Helpers/CodeFixHelper.cs | 30 ++++++++++ 5 files changed, 199 insertions(+), 21 deletions(-) diff --git a/src/nunit.analyzers.tests/ClassicModelAssertUsage/AreEqualClassicModelAssertUsageCodeFixTests.cs b/src/nunit.analyzers.tests/ClassicModelAssertUsage/AreEqualClassicModelAssertUsageCodeFixTests.cs index 88dbb6d9..df670876 100644 --- a/src/nunit.analyzers.tests/ClassicModelAssertUsage/AreEqualClassicModelAssertUsageCodeFixTests.cs +++ b/src/nunit.analyzers.tests/ClassicModelAssertUsage/AreEqualClassicModelAssertUsageCodeFixTests.cs @@ -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; @@ -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()")] + [TestCase("Enumerable.Empty()", "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 existingReferences = Settings.Default.MetadataReferences ?? Enumerable.Empty(); + + 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.Empty")] + [TestCase("ImmutableList.Empty")] + [TestCase("ImmutableHashSet.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 existingReferences = Settings.Default.MetadataReferences ?? Enumerable.Empty(); + + Settings settings = Settings.Default + .WithMetadataReferences(existingReferences.Concat(MetadataReferences.Transitive(typeof(ImmutableArray<>)))); + + RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode, + fixTitle: ClassicModelAssertUsageCodeFix.TransformToConstraintModelDescription, + settings: settings); + } } } diff --git a/src/nunit.analyzers.tests/ClassicModelAssertUsage/AreNotEqualClassicModelAssertUsageCodeFixTests.cs b/src/nunit.analyzers.tests/ClassicModelAssertUsage/AreNotEqualClassicModelAssertUsageCodeFixTests.cs index bfeb92a1..3b1a3596 100644 --- a/src/nunit.analyzers.tests/ClassicModelAssertUsage/AreNotEqualClassicModelAssertUsageCodeFixTests.cs +++ b/src/nunit.analyzers.tests/ClassicModelAssertUsage/AreNotEqualClassicModelAssertUsageCodeFixTests.cs @@ -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; @@ -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()")] + [TestCase("Enumerable.Empty()", "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 existingReferences = Settings.Default.MetadataReferences ?? Enumerable.Empty(); + + 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.Empty")] + [TestCase("ImmutableList.Empty")] + [TestCase("ImmutableHashSet.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 existingReferences = Settings.Default.MetadataReferences ?? Enumerable.Empty(); + + Settings settings = Settings.Default + .WithMetadataReferences(existingReferences.Concat(MetadataReferences.Transitive(typeof(ImmutableArray<>)))); + + RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode, + fixTitle: ClassicModelAssertUsageCodeFix.TransformToConstraintModelDescription, + settings: settings); + } } } diff --git a/src/nunit.analyzers/ClassicModelAssertUsage/AreEqualClassicModelAssertUsageCodeFix.cs b/src/nunit.analyzers/ClassicModelAssertUsage/AreEqualClassicModelAssertUsageCodeFix.cs index c52e099f..58fae6e6 100644 --- a/src/nunit.analyzers/ClassicModelAssertUsage/AreEqualClassicModelAssertUsageCodeFix.cs +++ b/src/nunit.analyzers/ClassicModelAssertUsage/AreEqualClassicModelAssertUsageCodeFix.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using System.Collections.Immutable; using System.Composition; @@ -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 { @@ -21,32 +23,45 @@ protected override (ArgumentSyntax ActualArgument, ArgumentSyntax? ConstraintArg IReadOnlyDictionary 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)); } } } diff --git a/src/nunit.analyzers/ClassicModelAssertUsage/AreNotEqualClassicModelAssertUsageCodeFix.cs b/src/nunit.analyzers/ClassicModelAssertUsage/AreNotEqualClassicModelAssertUsageCodeFix.cs index 55c2f91d..a0041769 100644 --- a/src/nunit.analyzers/ClassicModelAssertUsage/AreNotEqualClassicModelAssertUsageCodeFix.cs +++ b/src/nunit.analyzers/ClassicModelAssertUsage/AreNotEqualClassicModelAssertUsageCodeFix.cs @@ -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 { @@ -21,8 +22,21 @@ protected override (ArgumentSyntax ActualArgument, ArgumentSyntax? ConstraintArg IReadOnlyDictionary 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( @@ -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)); } } } diff --git a/src/nunit.analyzers/Helpers/CodeFixHelper.cs b/src/nunit.analyzers/Helpers/CodeFixHelper.cs index ad3f71c8..0fb1d683 100644 --- a/src/nunit.analyzers/Helpers/CodeFixHelper.cs +++ b/src/nunit.analyzers/Helpers/CodeFixHelper.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using System.Globalization; using System.Linq; @@ -129,6 +130,35 @@ public static void UpdateStringFormatToFormattableString(List ar arguments.RemoveRange(nextArgument, arguments.Count - nextArgument); } + /// + /// Checks if the given expression is something that is covered by the EmptyConstraint. + /// + 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.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(), Enumerable.Empty and other similar cases. + return true; + } + + return false; + } + internal static IEnumerable UpdateStringFormatToFormattableString(string formatSpecification, ExpressionSyntax[] formatArguments) { int startIndex = 0;