Transform collection.size() < 1 to isEmpty()

[Bananeweizen]

Not only size comparisons with zero can be optimized to isEmpty() calls. Additionally we can do (plus order changes):

• size() < 1 to isEmpty()
• size() >= 1 to !isEmpty()

What's your motivation?

real life code

Anything in particular you'd like reviewers to focus on?

I've changed one of the existing unit tests. It was obviously meant to test size() == 0 and 0 == size() (to create all 8 variants of positive and negative tests).

Checklist

• I've added unit tests to cover both positive and negative cases

[timtebeek]

Thanks for this addition! Wanted to give a quick heads up that we're slowly transitioning to a different way to define recipes for relatively straightforward replacements, as seen here: https://github.com/openrewrite/rewrite-migrate-java/blob/4b1cf028c52298968ff40590bd9af9e452eec096/src/main/java/org/openrewrite/java/migrate/lang/UseStringIsEmpty.java#L24

Not sure if that would fully work here, but it might be worth comparing the two approaches . Documentation is still ongoing on that form of recipe development, but it's a promising approach going forward.

[Bananeweizen]

I know refaster (and also how horrible complicated it is to set it up in other tech stacks than what the Google people use). Even with a refaster template, this second template (comparing with 1) would be needed. My impression was that the refaster part is not yet production ready, or am I wrong there?

[Stephan202]

(Sorry for going off-topic.)

I know refaster (and also how horrible complicated it is to set it up in other tech stacks than what the Google people use).

OOTB that's very true. At Picnic we built a Refaster compiler and runner, such that Refaster templates are effectively exposed as a regular Error Prone check. (We should probably further improve the documentation, but all the pieces are in that repo.)

W.r.t. the rule being discussed: we have this one.

[knutwannheden]

My impression was that the refaster part is not yet production ready, or am I wrong there?

No, it is not quite ready yet. And I think this particular recipe would indeed be problematic, because we don't properly support generics yet in the Refaster-based recipes. Also, the very nice Refaster.anyOf() and @AlsoNegation features are not supported yet.

[timtebeek]

Much appreciate this addition, and how the test covers every aspect without much duplicated setup.