`assertThat(boolean)` is harmful - violates fluent API principles

[clehene]

This was raised before by @cpovirk #48 and IMO the suggestion that error prone would catch it is irrelevant since truth doesn't require error prone.

Truth is a fluent API so the "contract" is that grammatically correct things would work as expected: "assert that something is true" is correct, yet Truth requires a double boolean check.

This looks natural:

assertThat(e.getStatus().getCode() == Status.ALREADY_EXISTS.getCode());

It's also consistent with how Guava preconditions work, however it's wrong and will always pass.

The correct (boolean) version is not fluent

assertThat(e.getStatus().getCode() == Status.ALREADY_EXISTS.getCode()).isTrue();

// and yes, it should be

assertThat(e.getStatus().getCode()).isEqualTo(Status.ALREADY_EXISTS.getCode());

Worse it's terribly difficult to spot and that's a sign of a bad API.

[jbduncan]

@clehene I'm not a Googler or on the Truth team, but I'm unclear on what you mean by assertThat(e.getStatus().getCode()).isEqualTo(Status.ALREADY_EXISTS.getCode()); not being fluent.

Would you mind clarifying? 🤔

[jbduncan]

I agree with you that the first example is more natural, but IIUC, the reason why Truth (and AssertJ) adopt the less intuitive approach over the more natural one is because these assertion libraries have very limited access to code e.getStatus().getCode() == Status.ALREADY_EXISTS.getCode(), and thus they can't provide a more helpful message than expected: <true> but was: <false> (taken from JUnit 5's assertTrue).

Thus I think you're asking for something that's very difficult to achieve in Java, but I could be wrong about this, and I'd love to be corrected. :)

[clehene]

@jbduncan that is fluent, the first one (assertThat(e.getStatus().getCode() == Status.ALREADY_EXISTS.getCode()).isTrue()) is not fluent.
I was stating that normally it should be assert(something).isEqualsTo(expected) (which is fluent and works) rather than (assert(a == b)) but still ..

I should have had two code snippets there rather than a single one with a comment in it to make it clear what I wanted to say, sorry.

[jbduncan]

@clehene Oh oops, sorry, my mistake! Thanks for clarifying what you meant. 👍

[jbduncan]

...for some reason I hadn't seen the .isTrue() at the end of your first example, so sorry for missing that!

[cpovirk]

This is quite possibly the biggest footgun in the Truth API. We once batted around some crazy ideas to try to catch it at runtime, but we couldn't find anything that seemed like an improvement. At this point, we're committed to the existing API. We might want to recommend Error Prone or similar static analysis somewhere prominent.