isSameAs fails with primitive type values outside the autobox cache

[scordio]

Describe the bug

isSameAs assertions on primitive types fail if the value being tested is outside the cached range of the corresponding wrapper type.

From JLS section 5.1.7:

If the value p being boxed is an integer literal of type int between -128 and 127 inclusive (§3.10.1), or the boolean literal true or false (§3.10.3), or a character literal between '\u0000' and '\u007f' inclusive (§3.10.4), then let a and b be the results of any two boxing conversions of p. It is always the case that a == b.

Test case reproducing the bug

assertThat(127 == 127).isTrue(); // succeeds
assertThat(127).isSameAs(127);   // succeeds

assertThat(128 == 128).isTrue(); // succeeds
assertThat(128).isSameAs(128);   // fails

[joel-costigliola]

Good catch!

[krzyk]

I would like to contribute by fixing this bug.
From my newbie eyes it looks rather easy, but I might be missing something.

My idea would be to add: isSame(A) (where A is in byte,short,int,long,char) - because we don't need to deal with float and doubles.

Unless we would like to reduce the number of added methods and go with Number and char.

[scordio]

Hi @krzyk, thanks for volunteering! I didn't label this as a good-first-issue as I'm not sure yet about the hidden complexity.

My suggestion is to start thinking about how we should store a primitive actual (i.e., the assertThat parameter) in the NumberAssert subtypes as right now actual gets stored in AbstractAssert but as an object, which requires the autoboxing.

Also, we might want to simplify how AbstractAssert::isSameAs is implemented today. It currently delegates to Objects::assertSame which is one more layer of complexity to deal with.

That's a common pattern in AssertJ, initially adopted for assertion reusability. However, certain assertions are quite specific and we could move everything to the assertion class, getting rid of the intermediate Objects layer.

Would you like to start designing a proposal?