mirror of
https://github.com/jlengrand/error-prone-support.git
synced 2026-03-10 08:11:25 +00:00
Introduce PatternRules Refaster rule collection (#771)
While there, configure `StaticImport` to not require static importing of `com.google.common.base.Predicates.contains`. The new rules triggered cleanup of some `CompilationTestHelper` usages. See google/guava#6483.
This commit is contained in:
committed by
GitHub
parent
f1882caf88
commit
75679396df
@@ -155,6 +155,7 @@ public final class StaticImport extends BugChecker implements MemberSelectTreeMa
|
||||
@VisibleForTesting
|
||||
static final ImmutableSetMultimap<String, String> STATIC_IMPORT_EXEMPTED_MEMBERS =
|
||||
ImmutableSetMultimap.<String, String>builder()
|
||||
.put("com.google.common.base.Predicates", "contains")
|
||||
.put("com.mongodb.client.model.Filters", "empty")
|
||||
.putAll(
|
||||
"java.util.Collections",
|
||||
|
||||
@@ -0,0 +1,46 @@
|
||||
package tech.picnic.errorprone.refasterrules;
|
||||
|
||||
import static com.google.common.base.Predicates.containsPattern;
|
||||
|
||||
import com.google.common.base.Predicates;
|
||||
import com.google.errorprone.refaster.annotation.AfterTemplate;
|
||||
import com.google.errorprone.refaster.annotation.BeforeTemplate;
|
||||
import java.util.function.Predicate;
|
||||
import java.util.regex.Pattern;
|
||||
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
|
||||
|
||||
/** Refaster rules related to code dealing with regular expressions. */
|
||||
@OnlineDocumentation
|
||||
final class PatternRules {
|
||||
private PatternRules() {}
|
||||
|
||||
/** Prefer {@link Pattern#asPredicate()} over non-JDK alternatives. */
|
||||
// XXX: This rule could also replace `s -> pattern.matcher(s).find()`, though the lambda
|
||||
// expression may match functional interfaces other than `Predicate`. If we do add such a rule, we
|
||||
// should also add a rule that replaces `s -> pattern.matcher(s).matches()` with
|
||||
// `pattern.asMatchPredicate()`.
|
||||
static final class PatternAsPredicate {
|
||||
@BeforeTemplate
|
||||
Predicate<CharSequence> before(Pattern pattern) {
|
||||
return Predicates.contains(pattern);
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
Predicate<String> after(Pattern pattern) {
|
||||
return pattern.asPredicate();
|
||||
}
|
||||
}
|
||||
|
||||
/** Prefer {@link Pattern#asPredicate()} over non-JDK alternatives. */
|
||||
static final class PatternCompileAsPredicate {
|
||||
@BeforeTemplate
|
||||
Predicate<CharSequence> before(String pattern) {
|
||||
return containsPattern(pattern);
|
||||
}
|
||||
|
||||
@AfterTemplate
|
||||
Predicate<String> after(String pattern) {
|
||||
return Pattern.compile(pattern).asPredicate();
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1,7 +1,5 @@
|
||||
package tech.picnic.errorprone.bugpatterns;
|
||||
|
||||
import static com.google.common.base.Predicates.containsPattern;
|
||||
|
||||
import com.google.errorprone.BugCheckerRefactoringTestHelper;
|
||||
import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
|
||||
import com.google.errorprone.CompilationTestHelper;
|
||||
@@ -12,7 +10,7 @@ final class AmbiguousJsonCreatorTest {
|
||||
void identification() {
|
||||
CompilationTestHelper.newInstance(AmbiguousJsonCreator.class, getClass())
|
||||
.expectErrorMessage(
|
||||
"X", containsPattern("`JsonCreator.Mode` should be set for single-argument creators"))
|
||||
"X", m -> m.contains("`JsonCreator.Mode` should be set for single-argument creators"))
|
||||
.addSourceLines(
|
||||
"Container.java",
|
||||
"import com.fasterxml.jackson.annotation.JsonCreator;",
|
||||
|
||||
@@ -1,14 +1,12 @@
|
||||
package tech.picnic.errorprone.bugpatterns;
|
||||
|
||||
import static com.google.common.base.Predicates.and;
|
||||
import static com.google.common.base.Predicates.containsPattern;
|
||||
import static com.google.common.base.Predicates.not;
|
||||
import static com.google.errorprone.BugCheckerRefactoringTestHelper.FixChoosers.SECOND;
|
||||
import static com.google.errorprone.BugCheckerRefactoringTestHelper.FixChoosers.THIRD;
|
||||
|
||||
import com.google.errorprone.BugCheckerRefactoringTestHelper;
|
||||
import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
|
||||
import com.google.errorprone.CompilationTestHelper;
|
||||
import java.util.stream.Stream;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.reactivestreams.Publisher;
|
||||
import reactor.core.CorePublisher;
|
||||
@@ -20,10 +18,7 @@ final class FluxImplicitBlockTest {
|
||||
CompilationTestHelper.newInstance(FluxImplicitBlock.class, getClass())
|
||||
.expectErrorMessage(
|
||||
"X",
|
||||
and(
|
||||
containsPattern("SuppressWarnings"),
|
||||
containsPattern("toImmutableList"),
|
||||
containsPattern("toList")))
|
||||
m -> Stream.of("SuppressWarnings", "toImmutableList", "toList").allMatch(m::contains))
|
||||
.addSourceLines(
|
||||
"A.java",
|
||||
"import com.google.common.collect.ImmutableList;",
|
||||
@@ -63,7 +58,7 @@ final class FluxImplicitBlockTest {
|
||||
void identificationWithoutGuavaOnClasspath() {
|
||||
CompilationTestHelper.newInstance(FluxImplicitBlock.class, getClass())
|
||||
.withClasspath(CorePublisher.class, Flux.class, Publisher.class)
|
||||
.expectErrorMessage("X", not(containsPattern("toImmutableList")))
|
||||
.expectErrorMessage("X", m -> !m.contains("toImmutableList"))
|
||||
.addSourceLines(
|
||||
"A.java",
|
||||
"import reactor.core.publisher.Flux;",
|
||||
|
||||
@@ -1,7 +1,5 @@
|
||||
package tech.picnic.errorprone.bugpatterns;
|
||||
|
||||
import static com.google.common.base.Predicates.containsPattern;
|
||||
|
||||
import com.google.errorprone.BugCheckerRefactoringTestHelper;
|
||||
import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
|
||||
import com.google.errorprone.CompilationTestHelper;
|
||||
@@ -12,8 +10,8 @@ final class StringJoinTest {
|
||||
void identification() {
|
||||
CompilationTestHelper.newInstance(StringJoin.class, getClass())
|
||||
.expectErrorMessage(
|
||||
"valueOf", containsPattern("Prefer `String#valueOf` over `String#format`"))
|
||||
.expectErrorMessage("join", containsPattern("Prefer `String#join` over `String#format`"))
|
||||
"valueOf", m -> m.contains("Prefer `String#valueOf` over `String#format`"))
|
||||
.expectErrorMessage("join", m -> m.contains("Prefer `String#join` over `String#format`"))
|
||||
.addSourceLines(
|
||||
"A.java",
|
||||
"import java.util.Formattable;",
|
||||
|
||||
@@ -61,6 +61,7 @@ final class RefasterRulesTest {
|
||||
MultimapRules.class,
|
||||
NullRules.class,
|
||||
OptionalRules.class,
|
||||
PatternRules.class,
|
||||
PreconditionsRules.class,
|
||||
PrimitiveRules.class,
|
||||
ReactorRules.class,
|
||||
|
||||
@@ -0,0 +1,22 @@
|
||||
package tech.picnic.errorprone.refasterrules;
|
||||
|
||||
import com.google.common.base.Predicates;
|
||||
import com.google.common.collect.ImmutableSet;
|
||||
import java.util.function.Predicate;
|
||||
import java.util.regex.Pattern;
|
||||
import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase;
|
||||
|
||||
final class PatternRulesTest implements RefasterRuleCollectionTestCase {
|
||||
@Override
|
||||
public ImmutableSet<Object> elidedTypesAndStaticImports() {
|
||||
return ImmutableSet.of(Predicates.class);
|
||||
}
|
||||
|
||||
Predicate<?> testPatternAsPredicate() {
|
||||
return Predicates.contains(Pattern.compile("foo"));
|
||||
}
|
||||
|
||||
Predicate<?> testPatternCompileAsPredicate() {
|
||||
return Predicates.containsPattern("foo");
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,22 @@
|
||||
package tech.picnic.errorprone.refasterrules;
|
||||
|
||||
import com.google.common.base.Predicates;
|
||||
import com.google.common.collect.ImmutableSet;
|
||||
import java.util.function.Predicate;
|
||||
import java.util.regex.Pattern;
|
||||
import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase;
|
||||
|
||||
final class PatternRulesTest implements RefasterRuleCollectionTestCase {
|
||||
@Override
|
||||
public ImmutableSet<Object> elidedTypesAndStaticImports() {
|
||||
return ImmutableSet.of(Predicates.class);
|
||||
}
|
||||
|
||||
Predicate<?> testPatternAsPredicate() {
|
||||
return Pattern.compile("foo").asPredicate();
|
||||
}
|
||||
|
||||
Predicate<?> testPatternCompileAsPredicate() {
|
||||
return Pattern.compile("foo").asPredicate();
|
||||
}
|
||||
}
|
||||
@@ -1,6 +1,5 @@
|
||||
package tech.picnic.errorprone.refaster.runner;
|
||||
|
||||
import static com.google.common.base.Predicates.containsPattern;
|
||||
import static com.google.common.collect.ImmutableList.toImmutableList;
|
||||
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
|
||||
import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
|
||||
@@ -24,35 +23,33 @@ import org.junit.jupiter.params.provider.Arguments;
|
||||
import org.junit.jupiter.params.provider.MethodSource;
|
||||
|
||||
final class RefasterTest {
|
||||
private final CompilationTestHelper compilationHelper =
|
||||
CompilationTestHelper.newInstance(Refaster.class, getClass())
|
||||
.matchAllDiagnostics()
|
||||
.expectErrorMessage(
|
||||
"StringOfSizeZeroRule",
|
||||
containsPattern(
|
||||
"\\[Refaster Rule\\] FooRules\\.StringOfSizeZeroRule: Refactoring opportunity\\s+.+\\s+"))
|
||||
.expectErrorMessage(
|
||||
"StringOfSizeOneRule",
|
||||
containsPattern(
|
||||
"\\[Refaster Rule\\] FooRules\\.StringOfSizeOneRule: "
|
||||
+ "A custom description about matching single-char strings\\s+.+\\s+"
|
||||
+ "\\(see https://error-prone.picnic.tech/refasterrules/FooRules#StringOfSizeOneRule\\)"))
|
||||
.expectErrorMessage(
|
||||
"StringOfSizeTwoRule",
|
||||
containsPattern(
|
||||
"\\[Refaster Rule\\] FooRules\\.ExtraGrouping\\.StringOfSizeTwoRule: "
|
||||
+ "A custom subgroup description\\s+.+\\s+"
|
||||
+ "\\(see https://example.com/rule/FooRules#ExtraGrouping.StringOfSizeTwoRule\\)"))
|
||||
.expectErrorMessage(
|
||||
"StringOfSizeThreeRule",
|
||||
containsPattern(
|
||||
"\\[Refaster Rule\\] FooRules\\.ExtraGrouping\\.StringOfSizeThreeRule: "
|
||||
+ "A custom description about matching three-char strings\\s+.+\\s+"
|
||||
+ "\\(see https://example.com/custom\\)"));
|
||||
private static final Pattern DIAGNOSTIC_STRING_OF_SIZE_ZERO =
|
||||
Pattern.compile(
|
||||
"\\[Refaster Rule\\] FooRules\\.StringOfSizeZeroRule: Refactoring opportunity\\s+.+\\s+");
|
||||
private static final Pattern DIAGNOSTIC_STRING_OF_SIZE_ONE =
|
||||
Pattern.compile(
|
||||
"\\[Refaster Rule\\] FooRules\\.StringOfSizeOneRule: "
|
||||
+ "A custom description about matching single-char strings\\s+.+\\s+"
|
||||
+ "\\(see https://error-prone.picnic.tech/refasterrules/FooRules#StringOfSizeOneRule\\)");
|
||||
private static final Pattern DIAGNOSTIC_STRING_OF_SIZE_TWO =
|
||||
Pattern.compile(
|
||||
"\\[Refaster Rule\\] FooRules\\.ExtraGrouping\\.StringOfSizeTwoRule: "
|
||||
+ "A custom subgroup description\\s+.+\\s+"
|
||||
+ "\\(see https://example.com/rule/FooRules#ExtraGrouping.StringOfSizeTwoRule\\)");
|
||||
private static final Pattern DIAGNOSTIC_STRING_OF_SIZE_THREE =
|
||||
Pattern.compile(
|
||||
"\\[Refaster Rule\\] FooRules\\.ExtraGrouping\\.StringOfSizeThreeRule: "
|
||||
+ "A custom description about matching three-char strings\\s+.+\\s+"
|
||||
+ "\\(see https://example.com/custom\\)");
|
||||
|
||||
@Test
|
||||
void identification() {
|
||||
compilationHelper
|
||||
CompilationTestHelper.newInstance(Refaster.class, getClass())
|
||||
.matchAllDiagnostics()
|
||||
.expectErrorMessage("StringOfSizeZeroRule", DIAGNOSTIC_STRING_OF_SIZE_ZERO.asPredicate())
|
||||
.expectErrorMessage("StringOfSizeOneRule", DIAGNOSTIC_STRING_OF_SIZE_ONE.asPredicate())
|
||||
.expectErrorMessage("StringOfSizeTwoRule", DIAGNOSTIC_STRING_OF_SIZE_TWO.asPredicate())
|
||||
.expectErrorMessage("StringOfSizeThreeRule", DIAGNOSTIC_STRING_OF_SIZE_THREE.asPredicate())
|
||||
.addSourceLines(
|
||||
"A.java",
|
||||
"class A {",
|
||||
@@ -153,7 +150,7 @@ final class RefasterTest {
|
||||
void severityAssignment(
|
||||
ImmutableList<String> arguments, ImmutableList<SeverityLevel> expectedSeverities) {
|
||||
CompilationTestHelper compilationTestHelper =
|
||||
compilationHelper
|
||||
CompilationTestHelper.newInstance(Refaster.class, getClass())
|
||||
.setArgs(arguments)
|
||||
.addSourceLines(
|
||||
"A.java",
|
||||
@@ -167,13 +164,18 @@ final class RefasterTest {
|
||||
" };",
|
||||
" }",
|
||||
"}");
|
||||
assertThatThrownBy(compilationTestHelper::doTest)
|
||||
.isInstanceOf(AssertionError.class)
|
||||
.message()
|
||||
.satisfies(
|
||||
message ->
|
||||
assertThat(extractRefasterSeverities("A.java", message))
|
||||
.containsExactlyElementsOf(expectedSeverities));
|
||||
|
||||
if (expectedSeverities.isEmpty()) {
|
||||
compilationTestHelper.doTest();
|
||||
} else {
|
||||
assertThatThrownBy(compilationTestHelper::doTest)
|
||||
.isInstanceOf(AssertionError.class)
|
||||
.message()
|
||||
.satisfies(
|
||||
message ->
|
||||
assertThat(extractRefasterSeverities("A.java", message))
|
||||
.containsExactlyElementsOf(expectedSeverities));
|
||||
}
|
||||
}
|
||||
|
||||
private static ImmutableList<SeverityLevel> extractRefasterSeverities(
|
||||
|
||||
Reference in New Issue
Block a user