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.

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

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

Closed
JeppeSN opened this issue Jan 11, 2024 · 1 comment · Fixed by #292
Closed

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

JeppeSN opened this issue Jan 11, 2024 · 1 comment · Fixed by #292
Labels

Comments

@JeppeSN
Copy link

JeppeSN commented Jan 11, 2024

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 Meir017 added the bug label Jan 11, 2024
Meir017 added a commit that referenced this issue Jan 15, 2024
Meir017 added a commit that referenced this issue Jan 17, 2024
Meir017 added a commit that referenced this issue Jan 17, 2024
@Meir017
Copy link
Member

Meir017 commented Jan 17, 2024

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

Meir017 added a commit that referenced this issue Jan 17, 2024
* tests: sanity test for issue #290

* bugfix: fix issue #290

* bugfix: fix issue #290
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants