Skip to content

Commit

Permalink
Introduce CharSequenceRules Refaster rule collection (#1480)
Browse files Browse the repository at this point in the history
Resolves #1394.
  • Loading branch information
rickie authored Dec 26, 2024
1 parent 83f3f8b commit 5fc7bc2
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package tech.picnic.errorprone.refasterrules;

import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.AlsoNegation;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;

/** Refaster rules related to expressions dealing with {@link CharSequence}s. */
@OnlineDocumentation
final class CharSequenceRules {
private CharSequenceRules() {}

/**
* Prefer {@link CharSequence#isEmpty()} over alternatives that consult the char sequence's
* length.
*/
// XXX: Drop this rule once we (and OpenRewrite) no longer support projects targeting Java 14 or
// below.
static final class CharSequenceIsEmpty {
@BeforeTemplate
boolean before(CharSequence charSequence) {
return Refaster.anyOf(
charSequence.length() == 0, charSequence.length() <= 0, charSequence.length() < 1);
}

@AfterTemplate
@AlsoNegation
boolean after(CharSequence charSequence) {
return charSequence.isEmpty();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,14 @@ final class StringRules {
private StringRules() {}

/** 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+.
// XXX: Drop this rule once we (and OpenRewrite) no longer support projects targeting Java 14 or
// below. The `CharSequenceIsEmpty` rule then suffices. (This rule exists so that e.g. projects
// that target JDK 11 can disable `CharSequenceIsEmpty` without losing a valuable rule.)
// XXX: Look into a more general approach to supporting different Java language levels, such as
// rule selection based on some annotation, or a separate Maven module.
static final class StringIsEmpty {
@BeforeTemplate
@SuppressWarnings("CharSequenceIsEmpty" /* This is a more specific template. */)
boolean before(String str) {
return Refaster.anyOf(str.length() == 0, str.length() <= 0, str.length() < 1);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ final class RefasterRulesTest {
AssortedRules.class,
BigDecimalRules.class,
BugCheckerRules.class,
CharSequenceRules.class,
ClassRules.class,
CollectionRules.class,
ComparatorRules.class,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package tech.picnic.errorprone.refasterrules;

import com.google.common.collect.ImmutableSet;
import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase;

final class CharSequenceRulesTest implements RefasterRuleCollectionTestCase {
ImmutableSet<Boolean> testCharSequenceIsEmpty() {
return ImmutableSet.of(
new StringBuilder("foo").length() == 0,
new StringBuilder("bar").length() <= 0,
new StringBuilder("baz").length() < 1,
new StringBuilder("qux").length() != 0,
new StringBuilder("quux").length() > 0,
new StringBuilder("corge").length() >= 1);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package tech.picnic.errorprone.refasterrules;

import com.google.common.collect.ImmutableSet;
import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase;

final class CharSequenceRulesTest implements RefasterRuleCollectionTestCase {
ImmutableSet<Boolean> testCharSequenceIsEmpty() {
return ImmutableSet.of(
new StringBuilder("foo").isEmpty(),
new StringBuilder("bar").isEmpty(),
new StringBuilder("baz").isEmpty(),
!new StringBuilder("qux").isEmpty(),
!new StringBuilder("quux").isEmpty(),
!new StringBuilder("corge").isEmpty());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ ImmutableSet<Boolean> testStringIsEmpty() {
"foo".length() == 0,
"bar".length() <= 0,
"baz".length() < 1,
"foo".length() != 0,
"bar".length() > 0,
"baz".length() >= 1);
"qux".length() != 0,
"quux".length() > 0,
"corge".length() >= 1);
}

boolean testStringIsEmptyPredicate() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ ImmutableSet<Boolean> testStringIsEmpty() {
"foo".isEmpty(),
"bar".isEmpty(),
"baz".isEmpty(),
!"foo".isEmpty(),
!"bar".isEmpty(),
!"baz".isEmpty());
!"qux".isEmpty(),
!"quux".isEmpty(),
!"corge".isEmpty());
}

boolean testStringIsEmptyPredicate() {
Expand Down

0 comments on commit 5fc7bc2

Please sign in to comment.