This commit is contained in:
Stephan Schroevers
2023-01-22 18:39:15 +01:00
parent 0aa612073f
commit be6e93ba7d
5 changed files with 47 additions and 0 deletions

View File

@@ -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<MethodInvocationTree> conversionMethodMatcher;

View File

@@ -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<R, S, T extends S> {
@Placeholder
abstract boolean test(S value);
@BeforeTemplate
AbstractAssert<?, R> before(AbstractAssert<?, R> abstractAssert, Class<T> clazz) {
return abstractAssert.isInstanceOf(clazz).matches(v -> test((S) v));
}
@AfterTemplate
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
AbstractAssert<?, S> after(AbstractAssert<?, R> abstractAssert, Class<S> clazz) {
return abstractAssert.asInstanceOf(type(clazz)).matches(v -> test(v));
}
}
}

View File

@@ -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());
}
}

View File

@@ -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());
}
}

View File

@@ -662,6 +662,10 @@
<property name="allowNonPrintableEscapes" value="true" />
</module>
<module name="AvoidNoArgumentSuperConstructorCall" />
<!-- XXX: Review whether to enable this rule:
<module name="CyclomaticComplexity">
<property name="switchBlockAsSingleDecisionPoint" value="true" />
</module>-->
<module name="DeclarationOrder">
<!-- We don't enforce sorting fields by
their visibility modifier, for two