Skip to content

SimplifyChainedAssertJAssertion: also simplify "literal-first" assertions where the expression is in isEqualTo(...) #1031

Description

@juherr

What problem are you trying to solve?

The dedicated-assertion simplifications (SimplifyChainedAssertJAssertion, SimplifyHasSizeAssertion, …) only inspect the assertThat(...) argument. When an assertion is written "literal-first" — assertThat(<constant>).isEqualTo(<expression>), with the meaningful expression on the isEqualTo(...) side — nothing fires. Such assertions also pass through TestNgToAssertj unchanged (it's a faithful 1:1 port).

There are two sub-cases, with different safety:

Case A — structural size checks (always safe)

assertThat(<int literal>).isEqualTo(coll.size()) involves no equals() at all — it's an int comparison — so reversing it into hasSize(...) is unconditionally behaviour-preserving:

// before
assertThat(4).isEqualTo(predecessors.size());
assertThat(1).isEqualTo(g.getIndependentNodes().size());
// after
assertThat(predecessors).hasSize(4);
assertThat(g.getIndependentNodes()).hasSize(1);

Same for .length()hasSize / isEmpty() etc. This is the main request.

Case B — object equality (NOT safe to swap in general)

assertThat(<constant>).isEqualTo(<expression>) cannot simply be reversed to assertThat(<expression>).isEqualTo(<constant>), because AssertJ's isEqualTo invokes actual.equals(expected): swapping changes which equals() is called, and an asymmetric or broken equals() can make a.equals(b) != b.equals(a).

assertThat("1").isEqualTo(l.get(0));   // reversing relies on String.equals symmetry — fine here,
                                       // but NOT safe for arbitrary types

So a reversal here should be restricted to constants of types with a well-defined symmetric equals (e.g. String, boxed primitives, enums), or skipped entirely. I'm flagging it mostly so the structural Case A isn't implemented in a way that also blindly reverses object equality.

Root cause

SimplifyChainedAssertJAssertion requires assertThat.getArguments().get(0) to be the J.MethodInvocation (e.g. .size()); when that call is the isEqualTo argument instead, the precondition fails and the assertion is left as-is.

Suggested approach

Handle Case A: when assertThat(...) holds an int/long literal and isEqualTo(...) holds a .size()/.length() call, emit hasSize(...) directly (the underlying comparison is primitive, so no equals concern). Treat Case B conservatively, if at all.

Any additional context

Are you interested in contributing this recipe?

Happy to provide more before/after samples and review a PR.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Fields

No fields configured for issues without a type.

Projects

Status
Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions