From 35ebe3fa53ce28d574e1c64806826cf659616fe9 Mon Sep 17 00:00:00 2001 From: Meir Blachman Date: Mon, 25 Dec 2023 19:51:29 +0200 Subject: [PATCH] feat: Migrate equals to IOperation --- .../Tips/ShouldEqualsTests.cs | 32 +++---- .../Tips/DiagnosticMetadata.cs | 4 + .../Tips/FluentAssertionsCodeFix.cs | 21 ++++- ...FluentAssertionsOperationAnalyzer.Utils.cs | 8 ++ .../Tips/FluentAssertionsOperationAnalyzer.cs | 23 +++++ .../Tips/ShouldEquals.cs | 90 ------------------- .../FluentAssertionsCodeFixProvider.cs | 4 +- 7 files changed, 73 insertions(+), 109 deletions(-) delete mode 100644 src/FluentAssertions.Analyzers/Tips/ShouldEquals.cs diff --git a/src/FluentAssertions.Analyzers.Tests/Tips/ShouldEqualsTests.cs b/src/FluentAssertions.Analyzers.Tests/Tips/ShouldEqualsTests.cs index 9ee2b858..6d3083e8 100644 --- a/src/FluentAssertions.Analyzers.Tests/Tips/ShouldEqualsTests.cs +++ b/src/FluentAssertions.Analyzers.Tests/Tips/ShouldEqualsTests.cs @@ -1,5 +1,4 @@ using FluentAssertions.Analyzers.TestUtils; -using FluentAssertions.Analyzers.Tips; using Microsoft.CodeAnalysis; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -11,7 +10,7 @@ public class ShouldEqualsTests [TestMethod] [Implemented] public void ShouldEquals_TestAnalyzer() - => VerifyCSharpDiagnosticExpressionBody("actual.Should().Equals(expected);"); + => VerifyCSharpDiagnosticExpressionBody("actual.Should().Equals(expected);", DiagnosticMetadata.ShouldBe_ShouldEquals); [TestMethod] [Implemented] @@ -20,13 +19,13 @@ public void ShouldEquals_ShouldBe_ObjectType_TestCodeFix() var oldSource = GenerateCode.ObjectStatement("actual.Should().Equals(expected);"); var newSource = GenerateCode.ObjectStatement("actual.Should().Be(expected);"); - DiagnosticVerifier.VerifyCSharpFix(oldSource, newSource); + DiagnosticVerifier.VerifyCSharpFix(oldSource, newSource); } [TestMethod] [Implemented] public void ShouldEquals_NestedInsideIfBlock_TestAnalyzer() - => VerifyCSharpDiagnosticExpressionBody("if(true) { actual.Should().Equals(expected); }", 10, 24); + => VerifyCSharpDiagnosticExpressionBody("if(true) { actual.Should().Equals(expected); }", 10, 24, DiagnosticMetadata.ShouldBe_ShouldEquals); [TestMethod] [Implemented] @@ -35,13 +34,13 @@ public void ShouldEquals_NestedInsideIfBlock_ShouldBe_ObjectType_TestCodeFix() var oldSource = GenerateCode.ObjectStatement("if(true) { actual.Should().Equals(expected); }"); var newSource = GenerateCode.ObjectStatement("if(true) { actual.Should().Be(expected); }"); - DiagnosticVerifier.VerifyCSharpFix(oldSource, newSource); + DiagnosticVerifier.VerifyCSharpFix(oldSource, newSource); } [TestMethod] [Implemented] public void ShouldEquals_NestedInsideWhileBlock_TestAnalyzer() - => VerifyCSharpDiagnosticExpressionBody("while(true) { actual.Should().Equals(expected); }", 10, 27); + => VerifyCSharpDiagnosticExpressionBody("while(true) { actual.Should().Equals(expected); }", 10, 27, DiagnosticMetadata.ShouldBe_ShouldEquals); [TestMethod] [Implemented] @@ -50,14 +49,14 @@ public void ShouldEquals_NestedInsideWhileBlock_ShouldBe_ObjectType_TestCodeFix( var oldSource = GenerateCode.ObjectStatement("while(true) { actual.Should().Equals(expected); }"); var newSource = GenerateCode.ObjectStatement("while(true) { actual.Should().Be(expected); }"); - DiagnosticVerifier.VerifyCSharpFix(oldSource, newSource); + DiagnosticVerifier.VerifyCSharpFix(oldSource, newSource); } [TestMethod] [Implemented] public void ShouldEquals_ActualIsMethodInvoaction_TestAnalyzer() => VerifyCSharpDiagnosticExpressionBody("object ResultSupplier() { return null; } \n" - + "ResultSupplier().Should().Equals(expected);", 11, 0); + + "ResultSupplier().Should().Equals(expected);", 11, 0, DiagnosticMetadata.ShouldBe_ShouldEquals); [TestMethod] [Implemented] @@ -67,7 +66,7 @@ public void ShouldEquals_ActualIsMethodInvoaction_ShouldBe_ObjectType_TestCodeFi var oldSource = GenerateCode.ObjectStatement(methodInvocation + "ResultSupplier().Should().Equals(expected);"); var newSource = GenerateCode.ObjectStatement(methodInvocation + "ResultSupplier().Should().Be(expected);"); - DiagnosticVerifier.VerifyCSharpFix(oldSource, newSource); + DiagnosticVerifier.VerifyCSharpFix(oldSource, newSource); } [TestMethod] @@ -77,7 +76,7 @@ public void ShouldEquals_ShouldBe_NumberType_TestCodeFix() var oldSource = GenerateCode.DoubleAssertion("actual.Should().Equals(expected);"); var newSource = GenerateCode.DoubleAssertion("actual.Should().Be(expected);"); - DiagnosticVerifier.VerifyCSharpFix(oldSource, newSource); + DiagnosticVerifier.VerifyCSharpFix(oldSource, newSource); } [TestMethod] @@ -87,7 +86,7 @@ public void ShouldEquals_ShouldBe_StringType_TestCodeFix() var oldSource = GenerateCode.StringAssertion("actual.Should().Equals(expected);"); var newSource = GenerateCode.StringAssertion("actual.Should().Be(expected);"); - DiagnosticVerifier.VerifyCSharpFix(oldSource, newSource); + DiagnosticVerifier.VerifyCSharpFix(oldSource, newSource); } [TestMethod] @@ -97,17 +96,18 @@ public void ShouldEquals_ShouldEqual_EnumerableType_TestCodeFix() var oldSource = GenerateCode.GenericIListCodeBlockAssertion("actual.Should().Equals(expected);"); var newSource = GenerateCode.GenericIListCodeBlockAssertion("actual.Should().Equal(expected);"); - DiagnosticVerifier.VerifyCSharpFix(oldSource, newSource); + DiagnosticVerifier.VerifyCSharpFix(oldSource, newSource); } - private void VerifyCSharpDiagnosticExpressionBody(string sourceAssertion) => VerifyCSharpDiagnosticExpressionBody(sourceAssertion, 10, 13); - private void VerifyCSharpDiagnosticExpressionBody(string sourceAssertion, int line, int column) + private void VerifyCSharpDiagnosticExpressionBody(string sourceAssertion, DiagnosticMetadata metadata) => VerifyCSharpDiagnosticExpressionBody(sourceAssertion, 10, 13, metadata); + private void VerifyCSharpDiagnosticExpressionBody(string sourceAssertion, int line, int column, DiagnosticMetadata metadata) { var source = GenerateCode.ObjectStatement(sourceAssertion); DiagnosticVerifier.VerifyCSharpDiagnosticUsingAllAnalyzers(source, new DiagnosticResult { - Id = ShouldEqualsAnalyzer.DiagnosticId, - Message = ShouldEqualsAnalyzer.Message, + Id = FluentAssertionsOperationAnalyzer.DiagnosticId, + Message = metadata.Message, + VisitorName = metadata.Name, Locations = new DiagnosticResultLocation[] { new DiagnosticResultLocation("Test0.cs", line, column) diff --git a/src/FluentAssertions.Analyzers/Tips/DiagnosticMetadata.cs b/src/FluentAssertions.Analyzers/Tips/DiagnosticMetadata.cs index 6561625b..2cdbd573 100644 --- a/src/FluentAssertions.Analyzers/Tips/DiagnosticMetadata.cs +++ b/src/FluentAssertions.Analyzers/Tips/DiagnosticMetadata.cs @@ -104,6 +104,10 @@ private DiagnosticMetadata(string message, string helpLink, [CallerMemberName] s public static DiagnosticMetadata ExceptionShouldThrowExactlyWithInnerException_ShouldThrowExactlyAndInnerExceptionShouldBeAssignableTo = new("Use .Should().ThrowExactly().WithInnerException()", string.Empty); public static DiagnosticMetadata ExceptionShouldThrowExactlyWithInnerException_ShouldThrowExactlyWhichInnerExceptionShouldBeAssignableTo = new("Use .Should().ThrowExactly().WithInnerException()", string.Empty); + public static DiagnosticMetadata StringShouldBe_StringShouldEquals = new("Use .Should().Be()", string.Empty); + public static DiagnosticMetadata CollectionShouldEqual_CollectionShouldEquals = new("Use .Should().Equal()", string.Empty); + public static DiagnosticMetadata ShouldEquals = new("Use .Should().Be() or .Should().BeEquivalentTo or .Should().Equal() or other equivalency assertion", string.Empty); + public static DiagnosticMetadata ShouldBe_ShouldEquals = new("Use .Should().Be()", string.Empty); private static string GetHelpLink(string id) => $"https://fluentassertions.com/tips/#{id}"; } \ No newline at end of file diff --git a/src/FluentAssertions.Analyzers/Tips/FluentAssertionsCodeFix.cs b/src/FluentAssertions.Analyzers/Tips/FluentAssertionsCodeFix.cs index 46bc0712..37836a20 100644 --- a/src/FluentAssertions.Analyzers/Tips/FluentAssertionsCodeFix.cs +++ b/src/FluentAssertions.Analyzers/Tips/FluentAssertionsCodeFix.cs @@ -1,6 +1,7 @@ using System.Collections.Immutable; using System.Composition; using System.Linq; +using System.Threading.Tasks; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp; @@ -13,6 +14,20 @@ public partial class FluentAssertionsCodeFix : FluentAssertionsCodeFixProvider { public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(FluentAssertionsOperationAnalyzer.DiagnosticId); + protected override Task CanRewriteAssertion(ExpressionSyntax expression, CodeFixContext context, Diagnostic diagnostic) + { + if (diagnostic.Properties.TryGetValue(Constants.DiagnosticProperties.VisitorName, out var visitorName)) + { + return visitorName switch + { + nameof(DiagnosticMetadata.ShouldEquals) => Task.FromResult(false), + _ => Task.FromResult(true), + }; + } + + return base.CanRewriteAssertion(expression, context, diagnostic); + } + protected override ExpressionSyntax GetNewExpression(ExpressionSyntax expression, FluentAssertionsDiagnosticProperties properties) { // oldAssertion: subject.(arg1).Should().(arg2, {reasonArgs}); @@ -337,7 +352,11 @@ ExpressionSyntax GetCombinedAssertionsWithArgumentsReversedOrder(string remove, case nameof(DiagnosticMetadata.ExceptionShouldThrowWithMessage_ShouldThrowAndMessageShouldEndWith): case nameof(DiagnosticMetadata.ExceptionShouldThrowExactlyWithMessage_ShouldThrowExactlyAndMessageShouldEndWith): return ReplaceEndWithMessage(expression, "And"); - + case nameof(DiagnosticMetadata.CollectionShouldEqual_CollectionShouldEquals): + return GetNewExpression(expression, NodeReplacement.Rename("Equals", "Equal")); + case nameof(DiagnosticMetadata.StringShouldBe_StringShouldEquals): + case nameof(DiagnosticMetadata.ShouldBe_ShouldEquals): + return GetNewExpression(expression, NodeReplacement.Rename("Equals", "Be")); default: throw new System.InvalidOperationException($"Invalid visitor name - {properties.VisitorName}"); }; } diff --git a/src/FluentAssertions.Analyzers/Tips/FluentAssertionsOperationAnalyzer.Utils.cs b/src/FluentAssertions.Analyzers/Tips/FluentAssertionsOperationAnalyzer.Utils.cs index f49159e0..bb8fee3d 100644 --- a/src/FluentAssertions.Analyzers/Tips/FluentAssertionsOperationAnalyzer.Utils.cs +++ b/src/FluentAssertions.Analyzers/Tips/FluentAssertionsOperationAnalyzer.Utils.cs @@ -1,7 +1,9 @@ using System; using System.Collections; using System.Collections.Generic; +using System.IO; using System.Linq; +using System.Threading.Tasks; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Operations; @@ -52,7 +54,10 @@ public FluentAssertionsMetadata(Compilation compilation) DictionaryOfT2 = compilation.GetTypeByMetadataName(typeof(Dictionary<,>).FullName); IReadonlyDictionaryOfT2 = compilation.GetTypeByMetadataName(typeof(IReadOnlyDictionary<,>).FullName); Enumerable = compilation.GetTypeByMetadataName(typeof(Enumerable).FullName); + IEnumerable = compilation.GetTypeByMetadataName(typeof(IEnumerable).FullName); Math = compilation.GetTypeByMetadataName(typeof(Math).FullName); + TaskCompletionSourceOfT1 = compilation.GetTypeByMetadataName(typeof(TaskCompletionSource<>).FullName); + Stream = compilation.GetTypeByMetadataName(typeof(Stream).FullName); } public INamedTypeSymbol AssertionExtensions { get; } public INamedTypeSymbol ReferenceTypeAssertionsOfT2 { get; } @@ -68,6 +73,9 @@ public FluentAssertionsMetadata(Compilation compilation) public INamedTypeSymbol BooleanAssertionsOfT1 { get; } public INamedTypeSymbol NumericAssertionsOfT2 { get; } public INamedTypeSymbol Enumerable { get; } + public INamedTypeSymbol IEnumerable { get; } public INamedTypeSymbol Math { get; } + public INamedTypeSymbol TaskCompletionSourceOfT1 { get; } + public INamedTypeSymbol Stream { get; } } } \ No newline at end of file diff --git a/src/FluentAssertions.Analyzers/Tips/FluentAssertionsOperationAnalyzer.cs b/src/FluentAssertions.Analyzers/Tips/FluentAssertionsOperationAnalyzer.cs index 142e6daf..771a6570 100644 --- a/src/FluentAssertions.Analyzers/Tips/FluentAssertionsOperationAnalyzer.cs +++ b/src/FluentAssertions.Analyzers/Tips/FluentAssertionsOperationAnalyzer.cs @@ -54,6 +54,29 @@ private static void AnalyzeInvocation(OperationAnalysisContext context, FluentAs switch (assertion.TargetMethod.Name) { + case nameof(object.Equals): + + if (subject.Type.SpecialType is SpecialType.System_String) + { + context.ReportDiagnostic(CreateDiagnostic(assertion, DiagnosticMetadata.StringShouldBe_StringShouldEquals)); + return; + } + + if (subject.Type.EqualsSymbol(metadata.TaskCompletionSourceOfT1) + || subject.Type.AllInterfaces.Contains(metadata.Stream)) + { + context.ReportDiagnostic(CreateDiagnostic(assertion, DiagnosticMetadata.ShouldEquals)); + return; + } + + if (subject.Type.ImplementsOrIsInterface(metadata.IEnumerable)) + { + context.ReportDiagnostic(CreateDiagnostic(assertion, DiagnosticMetadata.CollectionShouldEqual_CollectionShouldEquals)); + return; + } + + context.ReportDiagnostic(CreateDiagnostic(assertion, DiagnosticMetadata.ShouldBe_ShouldEquals)); + return; case "NotBeEmpty" when assertion.IsContainedInType(metadata.GenericCollectionAssertionsOfT3): { if (assertion.TryGetChainedInvocationAfterAndConstraint("NotBeNull", out var chainedInvocation)) diff --git a/src/FluentAssertions.Analyzers/Tips/ShouldEquals.cs b/src/FluentAssertions.Analyzers/Tips/ShouldEquals.cs deleted file mode 100644 index f2477be6..00000000 --- a/src/FluentAssertions.Analyzers/Tips/ShouldEquals.cs +++ /dev/null @@ -1,90 +0,0 @@ -using FluentAssertions.Analyzers.Utilities; -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CodeFixes; -using Microsoft.CodeAnalysis.CSharp.Syntax; -using Microsoft.CodeAnalysis.Diagnostics; -using System.Collections; -using System.Collections.Generic; -using System.Collections.Immutable; -using System.Composition; -using System.IO; -using System.Threading; -using System.Threading.Tasks; - -namespace FluentAssertions.Analyzers.Tips; - -[DiagnosticAnalyzer(LanguageNames.CSharp)] -public class ShouldEqualsAnalyzer : FluentAssertionsAnalyzer -{ - public const string DiagnosticId = Constants.CodeSmell.ShouldEquals; - public const string Category = Constants.CodeSmell.Category; - public const string Message = ".Should().Equals() is not an assertion, it just calls the native Object.Equals method.\n" - + "Use .Should().Equal() for collections or .Should().Be() for other types"; - - protected override DiagnosticDescriptor Rule => new DiagnosticDescriptor(DiagnosticId, Title, Message, Category, DiagnosticSeverity.Info, true); - protected override IEnumerable Visitors - { - get - { - yield return new ShouldEqualsSyntaxVisitor(); - } - } - - public class ShouldEqualsSyntaxVisitor : FluentAssertionsCSharpSyntaxVisitor - { - public ShouldEqualsSyntaxVisitor() : base(MemberValidator.Should, new MemberValidator("Equals")) - { - } - } -} - -[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(ShouldEqualsCodeFix)), Shared] -public class ShouldEqualsCodeFix : FluentAssertionsCodeFixProvider -{ - public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(ShouldEqualsAnalyzer.DiagnosticId); - - protected override async Task CanRewriteAssertion(ExpressionSyntax expression, CodeFixContext context) - { - var model = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false); - - var visitor = new MemberAccessExpressionsCSharpSyntaxVisitor(); - expression.Accept(visitor); - - var member = visitor.Members[visitor.Members.Count - 1]; - var info = model.GetTypeInfo(member.Expression); - - var taskCompletionSourceInfo = model.Compilation.GetTypeByMetadataName(typeof(TaskCompletionSource<>).FullName); - if (info.Type.EqualsSymbol(taskCompletionSourceInfo)) return false; - - var streamInfo = model.Compilation.GetTypeByMetadataName(typeof(Stream).FullName); - if (info.Type.AllInterfaces.Contains(streamInfo)) return false; - - return true; - } - - protected override async Task GetNewExpressionAsync(ExpressionSyntax expression, Document document, FluentAssertionsDiagnosticProperties properties, CancellationToken cancellationToken) - { - var model = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); - - var visitor = new MemberAccessExpressionsCSharpSyntaxVisitor(); - expression.Accept(visitor); - - var member = visitor.Members[visitor.Members.Count - 1]; - var info = model.GetTypeInfo(member.Expression); - - var stringInfo = model.Compilation.GetTypeByMetadataName(typeof(string).FullName); - - if (info.Type.EqualsSymbol(stringInfo)) - { - return GetNewExpression(expression, NodeReplacement.Rename("Equals", "Be")); - } - - var ienumerableInfo = model.Compilation.GetTypeByMetadataName(typeof(IEnumerable).FullName); - if (info.Type.AllInterfaces.Contains(ienumerableInfo)) - { - return GetNewExpression(expression, NodeReplacement.Rename("Equals", "Equal")); - } - - return GetNewExpression(expression, NodeReplacement.Rename("Equals", "Be")); - } -} diff --git a/src/FluentAssertions.Analyzers/Utilities/FluentAssertionsCodeFixProvider.cs b/src/FluentAssertions.Analyzers/Utilities/FluentAssertionsCodeFixProvider.cs index f90dd2ea..93d8274e 100644 --- a/src/FluentAssertions.Analyzers/Utilities/FluentAssertionsCodeFixProvider.cs +++ b/src/FluentAssertions.Analyzers/Utilities/FluentAssertionsCodeFixProvider.cs @@ -23,7 +23,7 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) foreach (var diagnostic in context.Diagnostics) { var expression = (ExpressionSyntax)root.FindNode(diagnostic.Location.SourceSpan); - if (await CanRewriteAssertion(expression, context).ConfigureAwait(false)) + if (await CanRewriteAssertion(expression, context, diagnostic).ConfigureAwait(false)) { context.RegisterCodeFix(CodeAction.Create( title: diagnostic.Properties[Constants.DiagnosticProperties.Title], @@ -33,7 +33,7 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) } } - protected virtual Task CanRewriteAssertion(ExpressionSyntax expression, CodeFixContext context) => Task.FromResult(true); + protected virtual Task CanRewriteAssertion(ExpressionSyntax expression, CodeFixContext context, Diagnostic diagnostic) => Task.FromResult(true); protected async Task RewriteAssertion(Document document, ExpressionSyntax expression, ImmutableDictionary properties, CancellationToken cancellationToken) {