From be6e93ba7db0dab191d5a810fbde94c337ed4777 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Sun, 22 Jan 2023 18:39:15 +0100 Subject: [PATCH] WIP --- .../RedundantStringConversion.java | 3 +++ .../refasterrules/AssertJObjectRules.java | 27 +++++++++++++++++++ .../AssertJObjectRulesTestInput.java | 6 +++++ .../AssertJObjectRulesTestOutput.java | 7 +++++ pom.xml | 4 +++ 5 files changed, 47 insertions(+) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RedundantStringConversion.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RedundantStringConversion.java index 4b6ba6ce..14a02e20 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RedundantStringConversion.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RedundantStringConversion.java @@ -147,6 +147,9 @@ public final class RedundantStringConversion extends BugChecker instanceMethod() .onDescendantOf("org.slf4j.Logger") .namedAnyOf("trace", "debug", "info", "warn", "error"); + // XXX: Also add support for: + // org.springframework.web.util.UriBuilder#queryParam(String name, Object... values) + // org.springframework.web.util.UriBuilder#replaceQueryParam(String name, Object... values) private final Matcher conversionMethodMatcher; diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJObjectRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJObjectRules.java index 41b1a51c..c17e4b7c 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJObjectRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJObjectRules.java @@ -2,11 +2,14 @@ package tech.picnic.errorprone.refasterrules; import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.InstanceOfAssertFactories.type; import com.google.errorprone.refaster.Refaster; import com.google.errorprone.refaster.annotation.AfterTemplate; import com.google.errorprone.refaster.annotation.BeforeTemplate; +import com.google.errorprone.refaster.annotation.Placeholder; import com.google.errorprone.refaster.annotation.UseImportPolicy; +import org.assertj.core.api.AbstractAssert; import org.assertj.core.api.AbstractBooleanAssert; import org.assertj.core.api.AbstractStringAssert; import org.assertj.core.api.ObjectAssert; @@ -80,4 +83,28 @@ final class AssertJObjectRules { return assertThat(object).hasToString(str); } } + + // XXX: This rule could produce non-compilable code. Possible improvements: + // Instead of `type(clazz)` suggest a more suitable `InstanceOfAssertFactory`. (Perhaps have + // separate rules which e.g. replace `.asInstanceOf(type(clazz))` with + // `.asInstanceOf(throwable(clazz))`.) + // Next to `matches(Predicate)`, this rule applies to several other functional interface-accepting + // assertion methods. + // Arguably this rule should be split in two. + @SuppressWarnings("unchecked") + abstract static class AbstractAssertAsInstanceOfMatches { + @Placeholder + abstract boolean test(S value); + + @BeforeTemplate + AbstractAssert before(AbstractAssert abstractAssert, Class clazz) { + return abstractAssert.isInstanceOf(clazz).matches(v -> test((S) v)); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + AbstractAssert after(AbstractAssert abstractAssert, Class clazz) { + return abstractAssert.asInstanceOf(type(clazz)).matches(v -> test(v)); + } + } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJObjectRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJObjectRulesTestInput.java index ce373065..68dce184 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJObjectRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJObjectRulesTestInput.java @@ -25,4 +25,10 @@ final class AssertJObjectRulesTest implements RefasterRuleCollectionTestCase { AbstractAssert testAssertThatHasToString() { return assertThat(new Object().toString()).isEqualTo("foo"); } + + AbstractAssert testAbstractAssertAsInstanceOfMatches() { + return assertThat((Object) new IllegalStateException()) + .isInstanceOf(RuntimeException.class) + .matches(t -> !((Exception) t).toString().isEmpty()); + } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJObjectRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJObjectRulesTestOutput.java index 0af2474f..41b5be06 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJObjectRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJObjectRulesTestOutput.java @@ -1,6 +1,7 @@ package tech.picnic.errorprone.refasterrules; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.InstanceOfAssertFactories.type; import org.assertj.core.api.AbstractAssert; import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; @@ -25,4 +26,10 @@ final class AssertJObjectRulesTest implements RefasterRuleCollectionTestCase { AbstractAssert testAssertThatHasToString() { return assertThat(new Object()).hasToString("foo"); } + + AbstractAssert testAbstractAssertAsInstanceOfMatches() { + return assertThat((Object) new IllegalStateException()) + .asInstanceOf(type(RuntimeException.class)) + .matches(t -> !(t).toString().isEmpty()); + } } diff --git a/pom.xml b/pom.xml index 071a890f..a5f94206 100644 --- a/pom.xml +++ b/pom.xml @@ -662,6 +662,10 @@ +