Skip to content

Commit

Permalink
Appease SonarCloud
Browse files Browse the repository at this point in the history
  • Loading branch information
@Stephan202
Stephan202 committed Dec 27, 2024
1 parent edb6cc7 commit 992e5fb
Showing 1 changed file with 74 additions and 58 deletions.
132 changes: 74 additions & 58 deletions ...prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/EagerStringFormatting.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import static com.google.errorprone.matchers.Matchers.isSubtypeOf;
import static com.google.errorprone.matchers.Matchers.staticMethod;
import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
import static java.util.Objects.requireNonNull;
import static java.util.stream.Collectors.joining;
import static tech.picnic.errorprone.utils.Documentation.BUG_PATTERNS_BASE_URL;

Expand Down Expand Up @@ -65,7 +66,7 @@ public final class EagerStringFormatting extends BugChecker implements MethodInv
private static final Matcher<ExpressionTree> LOCALE = isSubtypeOf(Locale.class);
private static final Matcher<ExpressionTree> SLF4J_MARKER = isSubtypeOf("org.slf4j.Marker");
private static final Matcher<ExpressionTree> THROWABLE = isSubtypeOf(Throwable.class);
private static final Matcher<ExpressionTree> REQUIRE_NOT_NULL_INVOCATION =
private static final Matcher<ExpressionTree> REQUIRE_NON_NULL_INVOCATION =
staticMethod().onClass(Objects.class.getCanonicalName()).named("requireNonNull");
private static final Matcher<ExpressionTree> GUAVA_GUARD_INVOCATION =
anyOf(
Expand Down Expand Up @@ -110,68 +111,16 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState

private Description analyzeFormatStringContext(
StringFormatExpression stringFormat, MethodInvocationTree context, VisitorState state) {
List<? extends ExpressionTree> arguments = context.getArguments();

if (REQUIRE_NOT_NULL_INVOCATION.matches(context, state)) {
if (arguments.size() != 2 || arguments.get(0).equals(stringFormat.expression())) {
/* Vacuous validation that string formatting doesn't yield `null`. */
return buildDescription(context).setMessage(MESSAGE_NEVER_NULL_ARGUMENT).build();
}

if (stringFormat.arguments().stream()
.anyMatch(EagerStringFormatting::isNonFinalLocalVariable)) {
/*
* The format operation depends on a variable that isn't final or effectively final; moving
* it into a lambda expression would cause a compilation error.
*/
return buildDescription(context)
.setMessage(
message() + " (but this requires introducing an effectively final variable)")
.build();
}

/* Suggest that the string formatting is deferred. */
return describeMatch(context, SuggestedFix.prefixWith(stringFormat.expression(), "() -> "));
if (REQUIRE_NON_NULL_INVOCATION.matches(context, state)) {
return analyzeRequireNonNullStringFormatContext(stringFormat, context);
}

if (GUAVA_GUARD_INVOCATION.matches(context, state)) {
if (arguments.get(0).equals(stringFormat.expression())) {
/*
* Vacuous `checkNotNull` or `verifyNotNull` validation that string formatting doesn't yield
* `null`.
*/
return buildDescription(context).setMessage(MESSAGE_NEVER_NULL_ARGUMENT).build();
}

if (stringFormat.simplifiableFormatString().isEmpty() || arguments.size() > 2) {
/*
* The format string cannot be simplified, or the format string produces a format string
* itself, or its result is the input to another format operation. These are complex cases
* that we'll only flag.
*/
return createSimplificationSuggestion(context, "Guava");
}

return describeMatch(context, stringFormat.suggestFlattening("%s", state));
return analyzeGuavaGuardStringFormatContext(stringFormat, context, state);
}

if (SLF4J_LOGGER_INVOCATION.matches(context, state)) {
if (stringFormat.simplifiableFormatString().isEmpty()) {
/* We can't simplify this case; only flag it. */
return createSimplificationSuggestion(context, "SLF4J");
}

int leftOffset = SLF4J_MARKER.matches(arguments.get(0), state) ? 1 : 0;
int rightOffset = THROWABLE.matches(arguments.get(arguments.size() - 1), state) ? 1 : 0;
if (arguments.size() != leftOffset + 1 + rightOffset) {
/*
* The format string produces a format string itself, or its result is the input to another
* format operation. This is a complex case that we'll only flag.
*/
return createSimplificationSuggestion(context, "SLF4J");
}

return describeMatch(context, stringFormat.suggestFlattening("{}", state));
return analyzeSlf4jLoggerStringFormatContext(stringFormat, context, state);
}

/*
Expand All @@ -181,6 +130,73 @@ private Description analyzeFormatStringContext(
return Description.NO_MATCH;

Check warning on line 130 in error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/EagerStringFormatting.java

View workflow job for this annotation

GitHub Actions / pitest

A change can be made to line 130 without causing a test to fail 

replaced return value with null for analyzeFormatStringContext (covered by 1 tests NullReturnValsMutator)
}

private Description analyzeRequireNonNullStringFormatContext(
StringFormatExpression stringFormat, MethodInvocationTree context) {
List<? extends ExpressionTree> arguments = context.getArguments();
if (arguments.size() != 2 || arguments.get(0).equals(stringFormat.expression())) {
/* Vacuous validation that string formatting doesn't yield `null`. */
return buildDescription(context).setMessage(MESSAGE_NEVER_NULL_ARGUMENT).build();
}

if (stringFormat.arguments().stream()
.anyMatch(EagerStringFormatting::isNonFinalLocalVariable)) {
/*
* The format operation depends on a variable that isn't final or effectively final; moving
* it into a lambda expression would cause a compilation error.
*/
return buildDescription(context)
.setMessage(message() + " (but this requires introducing an effectively final variable)")
.build();
}

/* Suggest that the string formatting is deferred. */
return describeMatch(context, SuggestedFix.prefixWith(stringFormat.expression(), "() -> "));
}

private Description analyzeGuavaGuardStringFormatContext(
StringFormatExpression stringFormat, MethodInvocationTree context, VisitorState state) {
List<? extends ExpressionTree> arguments = context.getArguments();
if (arguments.get(0).equals(stringFormat.expression())) {
/*
* Vacuous `checkNotNull` or `verifyNotNull` validation that string formatting doesn't yield
* `null`.
*/
return buildDescription(context).setMessage(MESSAGE_NEVER_NULL_ARGUMENT).build();
}

if (stringFormat.simplifiableFormatString().isEmpty() || arguments.size() > 2) {
/*
* The format string cannot be simplified, or the format string produces a format string
* itself, or its result is the input to another format operation. These are complex cases
* that we'll only flag.
*/
return createSimplificationSuggestion(context, "Guava");
}

return describeMatch(context, stringFormat.suggestFlattening("%s", state));
}

private Description analyzeSlf4jLoggerStringFormatContext(
StringFormatExpression stringFormat, MethodInvocationTree context, VisitorState state) {
if (stringFormat.simplifiableFormatString().isEmpty()) {
/* We can't simplify this case; only flag it. */
return createSimplificationSuggestion(context, "SLF4J");
}

List<? extends ExpressionTree> arguments = context.getArguments();
int leftOffset = SLF4J_MARKER.matches(arguments.get(0), state) ? 1 : 0;
int rightOffset = THROWABLE.matches(arguments.get(arguments.size() - 1), state) ? 1 : 0;
if (arguments.size() != leftOffset + 1 + rightOffset) {
/*
* The format string produces a format string itself, or its result is the input to another
* format operation. This is a complex case that we'll only flag.
*/
return createSimplificationSuggestion(context, "SLF4J");
}

return describeMatch(context, stringFormat.suggestFlattening("{}", state));
}

private static boolean isNonFinalLocalVariable(Tree tree) {
Symbol symbol = ASTHelpers.getSymbol(tree);
return symbol instanceof VarSymbol
Expand Down Expand Up @@ -235,7 +251,7 @@ private static Optional<StringFormatExpression> tryCreate(
return Optional.of(
create(
tree,
ASTHelpers.getReceiver(tree),
requireNonNull(ASTHelpers.getReceiver(tree), "Receiver unexpectedly absent"),
ImmutableList.copyOf(tree.getArguments()),
state));
}
Expand Down

0 comments on commit 992e5fb

Please sign in to comment.