Introduce Refaster rules that resolve EnumOrdinal violations (#1104)

This commit is contained in:
Stephan Schroevers
2024-03-27 15:44:19 +01:00
committed by GitHub
parent 9d8a5af44a
commit 769779cf21
6 changed files with 93 additions and 2 deletions

View File

@@ -16,8 +16,11 @@ import com.google.common.collect.ImmutableSet;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
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 com.google.errorprone.refaster.annotation.Matches;
import com.google.errorprone.refaster.annotation.MayOptionallyUse;
import com.google.errorprone.refaster.annotation.Placeholder;
import com.google.errorprone.refaster.annotation.Repeated;
import com.google.errorprone.refaster.annotation.UseImportPolicy;
import java.util.Arrays;
@@ -92,6 +95,24 @@ final class ComparatorRules {
}
}
/** Don't explicitly compare enums by their ordinal. */
abstract static class ComparingEnum<E extends Enum<E>, T> {
@Placeholder(allowsIdentity = true)
abstract E toEnumFunction(@MayOptionallyUse T value);
@BeforeTemplate
@SuppressWarnings("EnumOrdinal" /* This violation will be rewritten. */)
Comparator<T> before() {
return comparingInt(v -> toEnumFunction(v).ordinal());
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
Comparator<T> after() {
return comparing(v -> toEnumFunction(v));
}
}
/** Don't explicitly create {@link Comparator}s unnecessarily. */
static final class ThenComparing<S, T extends Comparable<? super T>> {
@BeforeTemplate
@@ -419,4 +440,34 @@ final class ComparatorRules {
return maxBy(naturalOrder());
}
}
/** Don't explicitly compare enums by their ordinal. */
static final class IsLessThan<E extends Enum<E>> {
@BeforeTemplate
@SuppressWarnings("EnumOrdinal" /* This violation will be rewritten. */)
boolean before(E value1, E value2) {
return value1.ordinal() < value2.ordinal();
}
@AfterTemplate
@AlsoNegation
boolean after(E value1, E value2) {
return value1.compareTo(value2) < 0;
}
}
/** Don't explicitly compare enums by their ordinal. */
static final class IsLessThanOrEqualTo<E extends Enum<E>> {
@BeforeTemplate
@SuppressWarnings("EnumOrdinal" /* This violation will be rewritten. */)
boolean before(E value1, E value2) {
return value1.ordinal() <= value2.ordinal();
}
@AfterTemplate
@AlsoNegation
boolean after(E value1, E value2) {
return value1.compareTo(value2) <= 0;
}
}
}

View File

@@ -30,8 +30,9 @@ final class EqualityRules {
// XXX: This Refaster rule is the topic of https://github.com/google/error-prone/issues/559. We
// work around the issue by selecting the "largest replacements". See the `Refaster` check.
@BeforeTemplate
@SuppressWarnings("EnumOrdinal" /* This violation will be rewritten. */)
boolean before(T a, T b) {
return Refaster.anyOf(a.equals(b), Objects.equals(a, b));
return Refaster.anyOf(a.equals(b), Objects.equals(a, b), a.ordinal() == b.ordinal());
}
@AfterTemplate

View File

@@ -9,6 +9,7 @@ import static java.util.stream.Collectors.minBy;
import com.google.common.collect.Comparators;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import java.math.RoundingMode;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
@@ -54,6 +55,10 @@ final class ComparatorRulesTest implements RefasterRuleCollectionTestCase {
Comparator.comparing(s -> "foo", Comparator.comparingInt(String::length)));
}
Comparator<String> testComparingEnum() {
return Comparator.comparingInt(s -> RoundingMode.valueOf(s).ordinal());
}
Comparator<String> testThenComparing() {
return Comparator.<String>naturalOrder().thenComparing(Comparator.comparing(String::isEmpty));
}
@@ -173,4 +178,16 @@ final class ComparatorRulesTest implements RefasterRuleCollectionTestCase {
Collector<Integer, ?, Optional<Integer>> testMaxByNaturalOrder() {
return minBy(reverseOrder());
}
ImmutableSet<Boolean> testIsLessThan() {
return ImmutableSet.of(
RoundingMode.UP.ordinal() < RoundingMode.DOWN.ordinal(),
RoundingMode.UP.ordinal() >= RoundingMode.DOWN.ordinal());
}
ImmutableSet<Boolean> testIsLessThanOrEqualTo() {
return ImmutableSet.of(
RoundingMode.UP.ordinal() <= RoundingMode.DOWN.ordinal(),
RoundingMode.UP.ordinal() > RoundingMode.DOWN.ordinal());
}
}

View File

@@ -1,5 +1,6 @@
package tech.picnic.errorprone.refasterrules;
import static java.util.Comparator.comparing;
import static java.util.Comparator.naturalOrder;
import static java.util.Comparator.reverseOrder;
import static java.util.function.Function.identity;
@@ -9,6 +10,7 @@ import static java.util.stream.Collectors.minBy;
import com.google.common.collect.Comparators;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import java.math.RoundingMode;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
@@ -52,6 +54,10 @@ final class ComparatorRulesTest implements RefasterRuleCollectionTestCase {
Comparator.comparing(s -> "foo", Comparator.comparingInt(String::length)));
}
Comparator<String> testComparingEnum() {
return comparing(s -> RoundingMode.valueOf(s));
}
Comparator<String> testThenComparing() {
return Comparator.<String>naturalOrder().thenComparing(String::isEmpty);
}
@@ -163,4 +169,16 @@ final class ComparatorRulesTest implements RefasterRuleCollectionTestCase {
Collector<Integer, ?, Optional<Integer>> testMaxByNaturalOrder() {
return maxBy(naturalOrder());
}
ImmutableSet<Boolean> testIsLessThan() {
return ImmutableSet.of(
RoundingMode.UP.compareTo(RoundingMode.DOWN) < 0,
RoundingMode.UP.compareTo(RoundingMode.DOWN) >= 0);
}
ImmutableSet<Boolean> testIsLessThanOrEqualTo() {
return ImmutableSet.of(
RoundingMode.UP.compareTo(RoundingMode.DOWN) <= 0,
RoundingMode.UP.compareTo(RoundingMode.DOWN) > 0);
}
}

View File

@@ -21,8 +21,10 @@ final class EqualityRulesTest implements RefasterRuleCollectionTestCase {
return ImmutableSet.of(
RoundingMode.UP.equals(RoundingMode.DOWN),
Objects.equals(RoundingMode.UP, RoundingMode.DOWN),
RoundingMode.UP.ordinal() == RoundingMode.DOWN.ordinal(),
!RoundingMode.UP.equals(RoundingMode.DOWN),
!Objects.equals(RoundingMode.UP, RoundingMode.DOWN));
!Objects.equals(RoundingMode.UP, RoundingMode.DOWN),
RoundingMode.UP.ordinal() != RoundingMode.DOWN.ordinal());
}
boolean testEqualsPredicate() {

View File

@@ -21,6 +21,8 @@ final class EqualityRulesTest implements RefasterRuleCollectionTestCase {
return ImmutableSet.of(
RoundingMode.UP == RoundingMode.DOWN,
RoundingMode.UP == RoundingMode.DOWN,
RoundingMode.UP == RoundingMode.DOWN,
RoundingMode.UP != RoundingMode.DOWN,
RoundingMode.UP != RoundingMode.DOWN,
RoundingMode.UP != RoundingMode.DOWN);
}