What do you think @jnyrup ? Is it going to hurt or improve the API?

I'm not sure what to think, but here are my thoughts.

Pros:

• It doesn't hurt any functionality.
• It's allow you to write grammatically more correct code.

Cons:

• It adds more ways to do the same thing (which I think might be bad for an API).

For now I'm inconclusive.
Usually it means, that the idea will smoulder in the back of me head for some time.

It's allow you to write grammatically more correct code.

Which is kind of what FA tries to do ;-).

If we're being honest here, 99% of developers wouldn't stop and think about which alias to use, or lookup grammar rules. Unit tests will still not be 100% grammatically correct, but now there's going to be all these different aliases used everywhere, possibly creating more confusion by bloating the API and IntelliSense, than doing good. But that's my two cents.

possibly creating more confusion by bloating the API

Yeah, that's my concern as well.

Looking this from a library consumer perspective, when I press dot, and IntelliSense shows me a few different properties, I will have to stop and read what each one does. Then I will learn that they are all aliases, and do the same thing. If I'm not the most proficient of English speakers I will then have to look up the proper grammar and do that for every test, which is tedious. Even though this PR has good intentions, I just don't see how this will benefit the majority of users in practice.

Well, lets say that you have an advanced level in writing and you know that's a "fluent" API so you would probably first start typing what you would expect in this scenario and if it doesn't exist try to look for other words. If you are more of a classical English writer then yes you will probably have to lookup to understand that they are all aliases (at the same time rather than just saying that Whose is an alias of Which, the same for Subject and for That we could list them all in the doc + doc comment).

Anyway I honestly don't mind if we are closing this PR.

How about putting this in a separate package? That way everything stays the same, and if you're using it you explicitly chose to use it and know about the aliases and other consequences.

How about putting this in a separate package? That way everything stays the same, and if you're using it you explicitly chose to use it and know about the aliases and other consequences.

I was also thinking this may be a good compromise. Maybe some other optional enhancements of this sort can go there.

I am not a fan because I prefer to have one clearly defined way to do things and agree with this by daniel:

Looking this from a library consumer perspective, when I press dot, and IntelliSense shows me a few different properties, I will have to stop and read what each one does. Then I will learn that they are all aliases, and do the same thing. If I'm not the most proficient of English speakers I will then have to look up the proper grammar and do that for every test, which is tedious. Even though this PR has good intentions, I just don't see how this will benefit the majority of users in practice.

Could you make it into extension methods in an extension namespace? - a separate package (as in nuget) seems like overkill and cumbersome to me.

[2c]

@dennisdoomen, @jnyrup What's your point of view on this? To sump up the expressed options:

1/ Keep it as-is with the risk of "confusion" for the user.

2/ Close the PR and issue as won't fix

3/ Move this to a separate namespace so that you don't see those overloads by default.

4/ Create a separate package.

The problem is that you can't do this in any other way than directly adding this to the framework. But looking at the feedback, I think it's best to close the PR.

Folks, would it be possible to come with separate AndWhichConstraint, AndWhoseConstraint and AndThatConstraint and leave it to the assertion extension to decide which one to return?

It sounds to me (and I might be completely wrong here) that the value to use really depends on the assertion that was called and the fact that something is "contained" or not in something else. Thus, depending on the assertion extension, it would be possible to decide which one of these to return, since there is enough context there to select the more appropriate English term.

This does introduce breaking changes to existing assertions of course, but those could be easily deprecated over time.

You're suggesting that depending on the specific assertion, it will return either Which, Whose or That, based on what makes most sense to that specific assertion?

I kind of like that idea. @jnyrup what do you think? Maybe we can consider this for 6.0.

I don't think I get the picture, maybe just due a long night's celebration of New Year.

Could you perhaps come up with an example to illustrate the proposal?

I think the proposal is to allow only one of Which, Whose or That per assertion method, but that for each method we choose the most appropriate word. So some methods will return an AndWhichConstraint, another AndWhoseConstraint, etc.

So if the API currently is:

AndWhichConstraint Foo();
AndWhichConstraint Bar();
AndWhichConstraint Baz();

the proposal is to change it into

AndWhichConstraint Foo();
AndWhoseConstraint Bar();
AndThatConstraint Baz();

I'm no grammar expert, but the correct relative clause (which, whose, that) might depend on the word succeeding the clause.

Both of these two assertions should be grammatically correct, but it requires the return type of Contain to support both Which and Whose.

subject.Should().Contain(...)
  .Which.Should().Be(...)

subject.Should().Contain(...)
  .Whose.Name.Should().Be(...)

Two other problems of introducing AndWhoseConstraint and AndThatConstraint:

• Major API breakage and harder transitioning to v6, you cannot simply search/replace Which.
• When using the API one cannot just write .Which, but either have to:
  • Know the return type of the assertion
  • Depend on Intellisense
  • Try to see which of Which, Whose or That is available.

I acknowledge the desire to write grammatically correct code, but I'm hesitating in complicating the API for those who don't care (that much).

Another way to this could be to introduce two extension methods under a sub-namespace.

namespace FluentAssertions.Grammar
{
    public static TResult Whose<TAssertion, TResult>(this AndWhichConstraint<TAssertion, TResult> constraint) => constraint.Which;

    public static TResult That<TAssertion, TResult>(this AndWhichConstraint<TAssertion, TResult> constraint) => constraint.Which;
}

I'm no grammar expert, but the correct relative clause (which, whose, that) might depend on the word succeeding the clause.

This is correct. So we need to look at case by case to determine the right word. Of course And is always available.

Major API breakage and harder transitioning to v6, you cannot simply search/replace Which.

I don't think it'll be that bad. I have not spoken to a lot of folks that even know about this Which thing, even at my clients.

@dennisdoomen My point was that Contain cannot know how you intent to chain the returned And...Constraint and therefore cannot just be either AndWhichConstraint or AndWhoseConstraint.