Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Excavator: Format Java files #973

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 0 additions & 4 deletions .baseline/checkstyle/checkstyle.xml
Original file line number Diff line number Diff line change
Expand Up @@ -199,10 +199,6 @@
<property name="separated" value="true"/>
<property name="sortStaticImportsAlphabetically" value="true"/>
</module>
<module name="Indentation"> <!-- Java Style Guide: Block indentation: +4 spaces -->
<property name="arrayInitIndent" value="8"/>
<property name="lineWrappingIndentation" value="8"/>
</module>
<module name="InnerAssignment"/> <!-- Java Coding Guidelines: Inner assignments: Not used -->
<module name="LeftCurly"/> <!-- Java Style Guide: Nonempty blocks: K & R style -->
<module name="LineLength"> <!-- Java Style Guide: No line-wrapping -->
Expand Down
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,5 @@
.settings
build
out/
gradle.properties
docs/node_modules/
generated_src
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,14 @@ public final class CatchBlockLogException extends BugChecker implements BugCheck
.onDescendantOf("org.slf4j.Logger")
.withNameMatching(Pattern.compile("trace|debug|info|warn|error"));

private static final Matcher<Tree> containslogMethod = Matchers.contains(
Matchers.toType(ExpressionTree.class, logMethod));
private static final Matcher<Tree> containslogMethod =
Matchers.contains(Matchers.toType(ExpressionTree.class, logMethod));

private static final Matcher<ExpressionTree> logException = Matchers.methodInvocation(
logMethod, ChildMultiMatcher.MatchType.LAST, Matchers.isSubtypeOf(Throwable.class));

private static final Matcher<Tree> containslogException = Matchers.contains(Matchers.toType(
ExpressionTree.class, logException));
private static final Matcher<Tree> containslogException =
Matchers.contains(Matchers.toType(ExpressionTree.class, logException));

@Override
public Description matchCatch(CatchTree tree, VisitorState state) {
Expand All @@ -91,17 +91,16 @@ private static Optional<SuggestedFix> attemptFix(CatchTree tree, VisitorState st
// There are no valid log invocations without at least a single argument.
ExpressionTree lastArgument = loggingArguments.get(loggingArguments.size() - 1);
return Optional.of(SuggestedFix.builder()
.replace(lastArgument, lastArgument.accept(ThrowableFromArgVisitor.INSTANCE, state)
.orElseGet(() -> state.getSourceForNode(lastArgument) + ", " + tree.getParameter().getName()))
.replace(lastArgument, lastArgument.accept(ThrowableFromArgVisitor.INSTANCE, state).orElseGet(() ->
state.getSourceForNode(lastArgument) + ", " + tree.getParameter().getName()))
.build());
}

private static final class ThrowableFromArgVisitor extends SimpleTreeVisitor<Optional<String>, VisitorState> {
private static final ThrowableFromArgVisitor INSTANCE = new ThrowableFromArgVisitor();

private static final Matcher<ExpressionTree> throwableMessageInvocation = Matchers.instanceMethod()
.onDescendantOf(Throwable.class.getName())
.named("getMessage");
private static final Matcher<ExpressionTree> throwableMessageInvocation =
Matchers.instanceMethod().onDescendantOf(Throwable.class.getName()).named("getMessage");

ThrowableFromArgVisitor() {
super(Optional.empty());
Expand Down Expand Up @@ -152,19 +151,15 @@ public List<MethodInvocationTree> visitCatch(CatchTree node, VisitorState state)

@Override
public List<MethodInvocationTree> reduce(
@Nullable List<MethodInvocationTree> left,
@Nullable List<MethodInvocationTree> right) {
@Nullable List<MethodInvocationTree> left, @Nullable List<MethodInvocationTree> right) {
// Unfortunately there's no way to provide default initial values, so we must handle nulls.
if (left == null) {
return right;
}
if (right == null) {
return left;
}
return ImmutableList.<MethodInvocationTree>builder()
.addAll(left)
.addAll(right)
.build();
return ImmutableList.<MethodInvocationTree>builder().addAll(left).addAll(right).build();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@
linkType = BugPattern.LinkType.CUSTOM,
severity = BugPattern.SeverityLevel.ERROR,
summary = "Disallow CompletableFuture asynchronous operations without an Executor.")
public final class DangerousCompletableFutureUsage
extends BugChecker implements BugChecker.MethodInvocationTreeMatcher {
public final class DangerousCompletableFutureUsage extends BugChecker
implements BugChecker.MethodInvocationTreeMatcher {
private static final long serialVersionUID = 1L;

private static final String ERROR_MESSAGE = "Should not use CompletableFuture methods without specifying a "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,21 +35,19 @@
link = "https://github.com/palantir/gradle-baseline#baseline-error-prone-checks",
linkType = BugPattern.LinkType.CUSTOM,
severity = SeverityLevel.ERROR,
summary = "Disallow usage of Jackson's JsonTypeInfo.Id.CLASS annotation for security reasons, "
+ "cf. https://github.com/FasterXML/jackson-databind/issues/1599")
summary =
"Disallow usage of Jackson's JsonTypeInfo.Id.CLASS annotation for security reasons, "
+ "cf. https://github.com/FasterXML/jackson-databind/issues/1599")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not great

public final class DangerousJsonTypeInfoUsage extends BugChecker implements BugChecker.AnnotationTreeMatcher {

private static final long serialVersionUID = 1L;

private static final Matcher<AnnotationTree> matcher = new AnnotationHasArgumentWithValue("use",
Matchers.allOf(
new IsSameType<>("com.fasterxml.jackson.annotation.JsonTypeInfo$Id"),
Matchers.anyOf(
treeEqualsStringMatcher("JsonTypeInfo.Id.CLASS"),
treeEqualsStringMatcher("JsonTypeInfo.Id.MINIMAL_CLASS"),
treeEqualsStringMatcher("com.fasterxml.jackson.annotation.JsonTypeInfo.Id.CLASS"),
treeEqualsStringMatcher("com.fasterxml.jackson.annotation.JsonTypeInfo.Id.MINIMAL_CLASS")
)));
private static final Matcher<AnnotationTree> matcher = new AnnotationHasArgumentWithValue(
"use", Matchers.allOf(new IsSameType<>("com.fasterxml.jackson.annotation.JsonTypeInfo$Id"), Matchers.anyOf(
treeEqualsStringMatcher("JsonTypeInfo.Id.CLASS"),
treeEqualsStringMatcher("JsonTypeInfo.Id.MINIMAL_CLASS"),
treeEqualsStringMatcher("com.fasterxml.jackson.annotation.JsonTypeInfo.Id.CLASS"),
treeEqualsStringMatcher("com.fasterxml.jackson.annotation.JsonTypeInfo.Id.MINIMAL_CLASS"))));

private static Matcher<ExpressionTree> treeEqualsStringMatcher(String value) {
return (expressionTree, state) -> state.getSourceForNode(expressionTree).equals(value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ public final class DangerousParallelStreamUsage extends BugChecker implements Bu
+ "blob/1.9.1/src/main/java/com/palantir/common/streams/MoreStreams.java#L53";
private static final String ERROR_MESSAGE = "Should not use .parallel() on a Java stream. "
+ "Doing so has very subtle and potentially severe performance implications, so in general you're better "
+ "off using " + MORE_STREAMS_URL + " which allows you to provide your own executor service and "
+ "off using "
+ MORE_STREAMS_URL
+ " which allows you to provide your own executor service and "
+ "specify the desired parallelism (i.e. the max number of concurrent tasks that will be submitted).\n"
+ "On the contrary the implementation of Java parallel streams uses a globally shared ForkJoinPool and "
+ "does not allow you to provide your own pool. Fork/join pools implement work-stealing, where any thread "
Expand All @@ -56,9 +58,7 @@ public final class DangerousParallelStreamUsage extends BugChecker implements Bu
+ "You can find more info here: https://stackoverflow.com/a/54581148/7182570";

private static final Matcher<ExpressionTree> PARALLEL_CALL_ON_JAVA_STREAM_MATCHER =
MethodMatchers.instanceMethod()
.onDescendantOf("java.util.stream.Stream")
.named("parallel");
MethodMatchers.instanceMethod().onDescendantOf("java.util.stream.Stream").named("parallel");

@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,17 +54,12 @@ public final class DangerousStringInternUsage extends BugChecker implements BugC
private static final long serialVersionUID = 1L;

private static final Matcher<ExpressionTree> STRING_INTERN_METHOD_MATCHER =
MethodMatchers.instanceMethod()
.onExactClass(String.class.getName())
.named("intern")
.withParameters();
MethodMatchers.instanceMethod().onExactClass(String.class.getName()).named("intern").withParameters();

@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (STRING_INTERN_METHOD_MATCHER.matches(tree, state)) {
return buildDescription(tree)
.setMessage(MESSAGE)
.build();
return buildDescription(tree).setMessage(MESSAGE).build();
}
return Description.NO_MATCH;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,11 @@ public final class DangerousThrowableMessageSafeArg extends BugChecker

private static final long serialVersionUID = 1L;

private static final Matcher<ExpressionTree> SAFEARG_FACTORY_METHOD = MethodMatchers.staticMethod()
.onClass("com.palantir.logsafe.SafeArg")
.named("of");
private static final Matcher<ExpressionTree> SAFEARG_FACTORY_METHOD =
MethodMatchers.staticMethod().onClass("com.palantir.logsafe.SafeArg").named("of");

private static final Matcher<ExpressionTree> THROWABLE_MESSAGE_METHOD = MethodMatchers.instanceMethod()
.onDescendantOf(Throwable.class.getName())
.named("getMessage");
private static final Matcher<ExpressionTree> THROWABLE_MESSAGE_METHOD =
MethodMatchers.instanceMethod().onDescendantOf(Throwable.class.getName()).named("getMessage");

@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,11 @@
linkType = BugPattern.LinkType.CUSTOM,
providesFix = BugPattern.ProvidesFix.REQUIRES_HUMAN_ATTENTION,
severity = BugPattern.SeverityLevel.ERROR,
summary = "Uncaught exceptions from ExecutorService.submit are not logged by the uncaught exception handler "
+ "because it is assumed that the returned future is used to watch for failures.\n"
+ "When the returned future is ignored, using ExecutorService.execute is preferred because "
+ "failures are recorded.")
summary =
"Uncaught exceptions from ExecutorService.submit are not logged by the uncaught exception handler "
+ "because it is assumed that the returned future is used to watch for failures.\n"
+ "When the returned future is ignored, using ExecutorService.execute is preferred because "
+ "failures are recorded.")
public final class ExecutorSubmitRunnableFutureIgnored extends AbstractReturnValueIgnored {

private static final Matcher<ExpressionTree> MATCHER = MethodMatchers.instanceMethod()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ protected Description matchMessageFormat(MethodInvocationTree tree, String messa
return Description.NO_MATCH;
}

return buildDescription(tree).setMessage(
"Use printf-style formatting in Guava Preconditions, not '{}' style formatting.").build();
return buildDescription(tree)
.setMessage("Use printf-style formatting in Guava Preconditions, not '{}' style formatting.")
.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,18 +53,16 @@ public Description matchClass(ClassTree tree, VisitorState state) {
if (!hasMigrationSupport.matches(tree, state)
&& hasJunit5TestCases.matches(tree, state)
&& hasJunit4Rules.matches(tree, state)) {
return buildDescription(tree)
.setMessage("Do not use Rule/ClassRule with junit-jupiter")
.build();
return buildDescription(tree).setMessage("Do not use Rule/ClassRule with junit-jupiter").build();
}

return Description.NO_MATCH;
}

static Matcher<ClassTree> hasVariable(Matcher<VariableTree> matcher) {
return (classTree, state) -> classTree.getMembers().stream()
.filter(tree -> tree instanceof VariableTree)
.anyMatch(tree -> matcher.matches((VariableTree) tree, state));
return (classTree, state) ->
classTree.getMembers().stream().filter(tree -> tree instanceof VariableTree).anyMatch(tree ->
matcher.matches((VariableTree) tree, state));
}

static Matcher<VariableTree> hasAnnotationOnVariable(String annotation) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,6 @@ private static List<Type> getReferencedClasses(AnnotationTree tree, VisitorState
}

throw new UnsupportedOperationException(String.format(
"Unable to get referenced classes for %s of type %s",
state.getSourceForNode(tree),
value.getClass()));
"Unable to get referenced classes for %s of type %s", state.getSourceForNode(tree), value.getClass()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,7 @@ public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState

private Description checkMethodInvocation(
MethodInvocationTree methodInvocation, LambdaExpressionTree root, VisitorState state) {
if (!methodInvocation.getArguments().isEmpty()
|| !methodInvocation.getTypeArguments().isEmpty()) {
if (!methodInvocation.getArguments().isEmpty() || !methodInvocation.getTypeArguments().isEmpty()) {
return Description.NO_MATCH;
}
Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(methodInvocation);
Expand All @@ -104,16 +103,11 @@ private Description checkMethodInvocation(
// which executes 'doWork' eagerly, even when the supplier is not used.
return Description.NO_MATCH;
}
return buildDescription(root)
.setMessage(MESSAGE)
.addFix(buildFix(methodSymbol, root, state))
.build();
return buildDescription(root).setMessage(MESSAGE).addFix(buildFix(methodSymbol, root, state)).build();
}

private static Optional<SuggestedFix> buildFix(
Symbol.MethodSymbol symbol,
LambdaExpressionTree root,
VisitorState state) {
Symbol.MethodSymbol symbol, LambdaExpressionTree root, VisitorState state) {
SuggestedFix.Builder builder = SuggestedFix.builder();
return toMethodReference(SuggestedFixes.qualifyType(state, builder, symbol))
.map(qualified -> builder.replace(root, qualified).build());
Expand All @@ -122,8 +116,8 @@ private static Optional<SuggestedFix> buildFix(
private static Optional<String> toMethodReference(String qualifiedMethodName) {
int index = qualifiedMethodName.lastIndexOf('.');
if (index > 0) {
return Optional.of(qualifiedMethodName.substring(0, index)
+ "::" + qualifiedMethodName.substring(index + 1));
return Optional.of(
qualifiedMethodName.substring(0, index) + "::" + qualifiedMethodName.substring(index + 1));
}
return Optional.empty();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,7 @@ public final class NonComparableStreamSort extends BugChecker implements BugChec
private static final long serialVersionUID = 1L;

private static final Matcher<ExpressionTree> SORTED_CALL_ON_JAVA_STREAM_MATCHER =
MethodMatchers.instanceMethod()
.onDescendantOf(Stream.class.getName())
.named("sorted")
.withParameters();
MethodMatchers.instanceMethod().onDescendantOf(Stream.class.getName()).named("sorted").withParameters();

@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,8 @@ public final class OptionalOrElseMethodInvocation extends BugChecker implements

private static final long serialVersionUID = 1L;

private static final Matcher<ExpressionTree> OR_ELSE_METHOD = MethodMatchers.instanceMethod()
.onExactClass("java.util.Optional")
.named("orElse");
private static final Matcher<ExpressionTree> OR_ELSE_METHOD =
MethodMatchers.instanceMethod().onExactClass("java.util.Optional").named("orElse");

private static final Matcher<ExpressionTree> METHOD_INVOCATIONS = Matchers.anyOf(
MethodInvocationMatcher.INSTANCE,
Expand Down Expand Up @@ -73,14 +72,12 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
}

private enum MethodInvocationMatcher implements Matcher<ExpressionTree> {

INSTANCE;

@Override
public boolean matches(ExpressionTree tree, VisitorState state) {
Kind kind = tree.getKind();
return kind == Kind.NEW_CLASS
|| kind == Kind.METHOD_INVOCATION;
return kind == Kind.NEW_CLASS || kind == Kind.METHOD_INVOCATION;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,7 @@ public final class OptionalOrElseThrowThrows extends BugChecker implements BugCh
Optional.class.getName(),
OptionalDouble.class.getName(),
OptionalInt.class.getName(),
OptionalLong.class.getName()
)))
OptionalLong.class.getName())))
.named("orElseThrow")
.withParameters(Supplier.class.getName());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,9 @@ public final class PreconditionsConstantMessage extends BugChecker implements Bu

private static final long serialVersionUID = 1L;

private static final Matcher<ExpressionTree> PRECONDITIONS_METHOD =
MethodMatchers.staticMethod()
.onClass("com.google.common.base.Preconditions")
.withNameMatching(Pattern.compile("checkArgument|checkState|checkNotNull"));
private static final Matcher<ExpressionTree> PRECONDITIONS_METHOD = MethodMatchers.staticMethod()
.onClass("com.google.common.base.Preconditions")
.withNameMatching(Pattern.compile("checkArgument|checkState|checkNotNull"));

private final Matcher<ExpressionTree> compileTimeConstExpressionMatcher =
new CompileTimeConstantExpressionMatcher();
Expand All @@ -66,8 +65,9 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
return Description.NO_MATCH;
}

return buildDescription(tree).setMessage(
"Preconditions.checkX() statement uses a non-constant message. "
+ "Consider using a template string with '%s'.").build();
return buildDescription(tree)
.setMessage("Preconditions.checkX() statement uses a non-constant message. "
+ "Consider using a template string with '%s'.")
.build();
}
}