AssertJ recipes should not change `assertThat(null== a).isEqualTo(true)` to `assertThat(null).isSameAs(a)`

[ChenyuWang98]

Compilation error

assertThat(null== a).isEqualTo(true) => assertThat(null).isSameAs(a);

Better

assertThat(null== a).isEqualTo(true) => assertThat(a).isNull()

[timtebeek]

hi @ChenyuWang98 ; It looks like both the left and right side here are using AssertJ already, is that correct? What was the original in Hamcrest before any migration?

Or would you want to see the left hand side optimized towards the right hand alternative when already using AssertJ?

[ChenyuWang98]

hi @ChenyuWang98 ; It looks like both the left and right side here are using AssertJ already, is that correct? What was the original in Hamcrest before any migration?

Or would you want to see the left hand side optimized towards the right hand alternative when already using AssertJ?

My problem is that when the first line using assertJ is updated again using this recipe, it becomes the statement on the right side of the first line. This causes a syntax problem, and I think it should be changed to the second line or left unchanged.

[timtebeek]

Thanks for the clarification; that helps.

[timtebeek]

It looks like this might be coming from the order in which the recipes generated from these lines are processed: https://github.com/PicnicSupermarket/error-prone-support/blob/6d7d9a4b77621ef3bc2719a56f443cb713d6dce3/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJObjectRules.java#L98-L163

[Stephan202]

Probably not helpful in the OpenRewrite context, but in case you're wondering why this works with Refaster: only because we implemented a custom heuristic.