fluentassertions / fluentassertions.analyzers

Analyzers based on the FluentAssertions best practices docs
https://www.fluentassertions.com/tips
MIT License
136 stars 21 forks source link

Saying `.Select(x => x.Name).Should().Equal` should not be a code smell #290

Closed JeppeSN closed 7 months ago

JeppeSN commented 7 months ago

Description

When I use the pattern .Select(x => x.Name).Should().Equal, an analyzer FAA0001 claims I should use .BeEquivalentTo.

Complete minimal example reproducing the issue

Consider the following complete C# program:

using FluentAssertions;
using System;
using System.Collections.Generic;
using System.Linq;

record Item(string Name, Guid Id) {
}

static class Program {
    static void Main() => TestGetSortedItems();

    static void TestGetSortedItems() {
        IEnumerable<string> expectedOrderedNames = ["Alpha", "Bravo", "Charlie",];

        var actualItems = GetSortedItems();

        actualItems.Select(x => x.Name).Should().Equal(expectedOrderedNames);
    }

    static IEnumerable<Item> GetSortedItems() {
        yield return new("Bravo", Guid.NewGuid());
        yield return new("Charlie", Guid.NewGuid());
        yield return new("Alpha", Guid.NewGuid());
    }
}

This program compiles, and when run, FluentAssertions will correctly complain that the expectation is not met (order of names is incorrect).

Expected behavior:

No "code smell" from any FluentAssertion analyzer.

Actual behavior:

The analyzer with ID FAA0001 and name "Simplify Assertion" says Use .Should().BeEquivalentTo() without further explanation. This analysis does not seem helpful!

The code is clear as it is. If the line in question is blindly changed to:

        actualItems.Select(x => x.Name).Should().BeEquivalentTo(expectedOrderedNames);

then the program behavior is broken (now FluentAssertions does not report the problem anymore).

There does not seem to be any suggestion of how the selection of the property .Name could be moved into the FluentAssertions framework?

I am aware I can write:

        actualItems.Select(x => x.Name).Should().BeEquivalentTo(expectedOrderedNames, options => options.WithStrictOrdering());

but this does certainly not seem to be an improvement over the code I had (and if Name had possessed a more complex type than just string, this would still not be equivalent to .Equal, so in that case even more bloat would be needed to get rid of the analyzer diagnosis).

Versions

Using FluentAssertions.Analyzers version 0.29.0 (and FluentAssertions" version 6.12.0)

Targeting .NET 8, using C# 12.

Meir017 commented 7 months ago

@JeppeSN nice catch! the description of the code-fix was indeed incorrect