Expand StringRules to cover CharSequence as well

[timtebeek]

Problem

Right now StringRules is able to do replacements for Strings, but skips StringBuilder for instance, as identified here:

• Use StringBuilder#isEmpty() instead of comparing size() openrewrite/rewrite-migrate-java#336 (comment)

There's a note at the top of this recipe that goes into somewhat more detail on what's needed to cover any CharSequence.

error-prone-support/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StringRules.java

Lines 31 to 46 in 4b458e0

	/** Prefer {@link String#isEmpty()} over alternatives that consult the string's length. */
	// XXX: Now that we build with JDK 15+, this rule can be generalized to cover all `CharSequence`
	// subtypes. This does require a mechanism (perhaps an annotation, or a separate Maven module) to
	// make sure that non-String expressions are rewritten only if client code also targets JDK 15+.
	static final class StringIsEmpty {
	@BeforeTemplate
	boolean before(String str) {
	return Refaster.anyOf(str.length() == 0, str.length() <= 0, str.length() < 1);
	}
	@AfterTemplate
	@AlsoNegation
	boolean after(String str) {
	return str.isEmpty();
	}
	}

Description of the proposed new feature

Implement whatever's needed to rewrite any CharSequence, especially since this project now targets Java 17+.

Considerations

If we're to support Java 8-15 still (please do 😇) then any recipe targeting Java 15+ CharSequence.isEmpty() might need to be in addition to the recipe for String.isEmpty(), which is still available on older versions of Java, and matches as such.

Participation

• I am willing to submit a pull request to implement this improvement.

Would this need anything more than just an additional CharSequenceRule ? Or what approach should be taken here?

[Stephan202]

Thanks for filing an issue for this @timtebeek! Off the top of my head there are (at least) ~two ways forward:

1. Solve the problem using multiple Maven modules. E.g. by having multiple refaster-rules-java-X modules, plus an aggregate refaster-rules module with a runtime dependency on the others. There may be several variations on this, and we'd have to assess whether the refaster-rules-java-X modules should target Java X. That would validate compliance by the @AfterTemplate methods, but would be too restrictive for the @BeforeTemplate methods, likely leading to duplication.
2. Express the constraint using an annotation (analogous to JUnit 5's @EnabledForJreRange), and have the Refaster check respect said annotation. "For reasons" this may require some reflection that (I think) we should be able to lift from Speed up Refaster bug checker #261. I need to give some thought to how we'd validate that the proposed annotation is applied correctly.

While (2) comes with more unknowns, it feels more maintainable, and the annotation idea matches what I wrote here (that is: we may add custom annotation support for Refaster anyway). Something to sleep on.

(Managing expectations: also "for reasons", it may be quite a while before I find time to dig into this more deeply.)