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

Analyzer incorrectly recommends ContainsSingle for HaveCount(1).And.Contains() #96

Closed
AquilaSands opened this issue May 25, 2021 · 3 comments · Fixed by #289
Closed
Labels

Comments

@AquilaSands
Copy link

AquilaSands commented May 25, 2021

Description

Analyzer incorrectly recommends ContainsSingle() for HaveCount(1).And.Contains(predicate) but looking at the tests for ContaiansSingle they are not equivalent as shown by this test https://github.com/fluentassertions/fluentassertions/blob/501cf353ecc872154fae54960ee1b692851bfb8a/Tests/FluentAssertions.Specs/Collections/GenericCollectionAssertionsSpecs.cs#L302-L314 which passes when a collection has multiple elements with only one matching the predicate.

Complete minimal example reproducing the issue

Complete means the code snippet can be copied into a unit test method in a fresh C# project and run.
Minimal means it is stripped from code not related to reproducing the issue.

// Arrange
IEnumerable<int> collection = new[] { 1, 2, 3 };

// Assert
collection.Should().HaveCount(1).And.Contain(i => i == 1 ); // This would fail
collection.Should().ContainSingle(i => i == 1 ); // This would pass

Expected behavior:

No analyzer recommendation

or suggest:

collection.Should().ContainSingle().Which.Should().Be(1);

Actual behavior:

Analyzer recommends using ContainsSingle

Versions

Fluent Assertions 0.11.4
.Net 5

@Meir017 Meir017 added the bug label May 25, 2021
@Meir017
Copy link
Member

Meir017 commented Oct 12, 2021

@AquilaSands is this an actual use-case?
I think a better syntax would be

collection.Should().ContainSingle().Which.Should().Be(1);

@AquilaSands
Copy link
Author

It was an actual use case for me but I agree the Which().Should().Be() is better, would it be possible for the analyzer to detect the And.Contain() and recommend the better syntax?

@Meir017
Copy link
Member

Meir017 commented Oct 17, 2021

@AquilaSands I think so. I'll take a look

Meir017 added a commit that referenced this issue Jan 10, 2024
Meir017 added a commit that referenced this issue Jan 10, 2024
Meir017 added a commit that referenced this issue Jan 10, 2024
* bugfix: and test for #96

* bugfix: and test for #96
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants