Skip to content

Commit

Permalink
Require JDK 17 rather than JDK 11 (#603)
Browse files Browse the repository at this point in the history
By raising this baseline the project can now use Java 17 language features such
as text blocks, switch expressions and `instanceof` pattern matching. The code
has been updated to make use of these constructs.

Note that the project can still be used by builds that target an older version
of Java, as long as those builds are executed using JDK 17+.
  • Loading branch information
Stephan202 authored Feb 11, 2024
1 parent 1f50772 commit 433b8b9
Show file tree
Hide file tree
Showing 46 changed files with 283 additions and 315 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ jobs:
strategy:
matrix:
os: [ ubuntu-22.04 ]
jdk: [ 11.0.20, 17.0.8, 21.0.0 ]
jdk: [ 17.0.8, 21.0.0 ]
distribution: [ temurin ]
experimental: [ false ]
include:
Expand Down
5 changes: 4 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ high-quality and consistent Java code_][picnic-blog-ep-post].
### Installation

This library is built on top of [Error Prone][error-prone-orig-repo]. To use
it, read the installation guide for Maven or Gradle below.
it, read the installation guide for Maven or Gradle below. The library requires
that your build is executed using JDK 17 or above, but supports builds that
[target][baeldung-java-source-target-options] older versions of Java.

#### Maven

Expand Down Expand Up @@ -263,6 +265,7 @@ guidelines][contributing].
If you want to report a security vulnerability, please do so through a private
channel; please see our [security policy][security] for details.

[baeldung-java-source-target-options]: https://www.baeldung.com/java-source-target-options
[bug-checks]: https://github.com/PicnicSupermarket/error-prone-support/blob/master/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/
[bug-checks-identity-conversion]: https://github.com/PicnicSupermarket/error-prone-support/blob/master/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IdentityConversion.java
[codeql-badge]: https://github.com/PicnicSupermarket/error-prone-support/actions/workflows/codeql.yml/badge.svg?branch=master&event=push
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,8 @@ private static Optional<String> getClassUnderTest(
}

ExpressionTree receiver = ASTHelpers.getReceiver(tree);
return receiver instanceof MethodInvocationTree
? getClassUnderTest((MethodInvocationTree) receiver, state)
return receiver instanceof MethodInvocationTree methodInvocation
? getClassUnderTest(methodInvocation, state)
: Optional.empty();
}

Expand All @@ -154,8 +154,8 @@ private static void extractIdentificationTestCases(
}

ExpressionTree receiver = ASTHelpers.getReceiver(tree);
if (receiver instanceof MethodInvocationTree) {
extractIdentificationTestCases((MethodInvocationTree) receiver, sink, state);
if (receiver instanceof MethodInvocationTree methodInvocation) {
extractIdentificationTestCases(methodInvocation, sink, state);
}
}

Expand Down Expand Up @@ -184,8 +184,8 @@ private static void extractReplacementTestCases(
}

ExpressionTree receiver = ASTHelpers.getReceiver(tree);
if (receiver instanceof MethodInvocationTree) {
extractReplacementTestCases((MethodInvocationTree) receiver, sink, state);
if (receiver instanceof MethodInvocationTree methodInvocation) {
extractReplacementTestCases(methodInvocation, sink, state);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,20 @@ void extraction(@TempDir Path outputDirectory) {
"DocumentationGeneratorTaskListenerTestClass.java",
"class DocumentationGeneratorTaskListenerTestClass {}");

// XXX: Once we support only JDK 15+, use a text block for the `expected` string.
assertThat(
outputDirectory.resolve(
"documentation-generator-task-listener-test-DocumentationGeneratorTaskListenerTestClass.json"))
.content(UTF_8)
.isEqualToIgnoringWhitespace(
"{\"className\":\"DocumentationGeneratorTaskListenerTestClass\",\"path\":[\"CLASS: DocumentationGeneratorTaskListenerTestClass\",\"COMPILATION_UNIT\"]}");
"""
{
"className": "DocumentationGeneratorTaskListenerTestClass",
"path": [
"CLASS: DocumentationGeneratorTaskListenerTestClass",
"COMPILATION_UNIT"
]
}
""");
}

@Immutable
Expand All @@ -125,8 +132,8 @@ public Optional<ExtractionParameters> tryExtract(ClassTree tree, VisitorState st
}

private static String describeTree(Tree tree) {
return (tree instanceof ClassTree)
? String.join(": ", String.valueOf(tree.getKind()), ((ClassTree) tree).getSimpleName())
return (tree instanceof ClassTree clazz)
? String.join(": ", String.valueOf(tree.getKind()), clazz.getSimpleName())
: tree.getKind().toString();
}
}
Expand Down
1 change: 0 additions & 1 deletion error-prone-contrib/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,6 @@
</path>
</annotationProcessorPaths>
<compilerArgs combine.children="append">
<arg>-Xplugin:RefasterRuleCompiler</arg>
<arg>-Xplugin:DocumentationGenerator -XoutputDirectory=${project.build.directory}/docs</arg>
</compilerArgs>
</configuration>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.Tree;
import com.sun.source.tree.Tree.Kind;
import com.sun.tools.javac.code.Symbol;
import java.util.Map;
import javax.lang.model.element.AnnotationValue;
Expand Down Expand Up @@ -46,7 +46,7 @@ public Description matchAnnotation(AnnotationTree tree, VisitorState state) {
}

ClassTree clazz = state.findEnclosing(ClassTree.class);
if (clazz == null || clazz.getKind() != Tree.Kind.ENUM) {
if (clazz == null || clazz.getKind() != Kind.ENUM) {
return Description.NO_MATCH;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import com.sun.source.tree.AssignmentTree;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.NewArrayTree;
import com.sun.source.tree.Tree.Kind;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
Expand Down Expand Up @@ -119,7 +118,7 @@ private static Optional<Fix> dropRedundantCurlies(AnnotationTree tree, VisitorSt
* the expression as a whole.
*/
ExpressionTree value =
(arg.getKind() == Kind.ASSIGNMENT) ? ((AssignmentTree) arg).getExpression() : arg;
(arg instanceof AssignmentTree assignment) ? assignment.getExpression() : arg;

/* Store a fix for each expression that was successfully simplified. */
simplifyAttributeValue(value, state)
Expand All @@ -130,13 +129,10 @@ private static Optional<Fix> dropRedundantCurlies(AnnotationTree tree, VisitorSt
}

private static Optional<String> simplifyAttributeValue(ExpressionTree expr, VisitorState state) {
if (expr.getKind() != Kind.NEW_ARRAY) {
/* There are no curly braces or commas to be dropped here. */
return Optional.empty();
}

NewArrayTree array = (NewArrayTree) expr;
return simplifySingletonArray(array, state).or(() -> dropTrailingComma(array, state));
/* Drop curly braces or commas if possible. */
return expr instanceof NewArrayTree newArray
? simplifySingletonArray(newArray, state).or(() -> dropTrailingComma(newArray, state))
: Optional.empty();
}

/** Returns the expression describing the array's sole element, if any. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,8 @@ private static boolean isPassedToCanonicalNameUsingType(VisitorState state) {
path = path.getParentPath();
}

return path.getLeaf() instanceof MethodInvocationTree
&& isOwnedByCanonicalNameUsingType(
ASTHelpers.getSymbol((MethodInvocationTree) path.getLeaf()));
return path.getLeaf() instanceof MethodInvocationTree methodInvocation
&& isOwnedByCanonicalNameUsingType(ASTHelpers.getSymbol(methodInvocation));
}

private static boolean isOwnedByCanonicalNameUsingType(MethodSymbol symbol) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,19 +101,17 @@ public Description matchBlock(BlockTree tree, VisitorState state) {
}

private static Optional<ExpressionTree> tryMatchAssignment(Symbol targetSymbol, Tree tree) {
if (tree instanceof ExpressionStatementTree) {
return tryMatchAssignment(targetSymbol, ((ExpressionStatementTree) tree).getExpression());
if (tree instanceof ExpressionStatementTree expressionStatement) {
return tryMatchAssignment(targetSymbol, expressionStatement.getExpression());
}

if (tree instanceof AssignmentTree) {
AssignmentTree assignment = (AssignmentTree) tree;
if (tree instanceof AssignmentTree assignment) {
return targetSymbol.equals(ASTHelpers.getSymbol(assignment.getVariable()))
? Optional.of(assignment.getExpression())
: Optional.empty();
}

if (tree instanceof VariableTree) {
VariableTree declaration = (VariableTree) tree;
if (tree instanceof VariableTree declaration) {
return declaration.getModifiers().getAnnotations().isEmpty()
&& targetSymbol.equals(ASTHelpers.getSymbol(declaration))
? Optional.ofNullable(declaration.getInitializer())
Expand Down Expand Up @@ -151,11 +149,11 @@ private static boolean isIdentifierSymbolReferencedInAssociatedFinallyBlock(
Streams.stream(state.getPath()).skip(1),
Streams.stream(state.getPath()),
(tree, child) -> {
if (!(tree instanceof TryTree)) {
if (!(tree instanceof TryTree tryTree)) {
return null;
}

BlockTree finallyBlock = ((TryTree) tree).getFinallyBlock();
BlockTree finallyBlock = tryTree.getFinallyBlock();
return !child.equals(finallyBlock) ? finallyBlock : null;
})
.anyMatch(finallyBlock -> referencesIdentifierSymbol(symbol, finallyBlock));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@
@AutoService(BugChecker.class)
@BugPattern(
summary =
"`Flux#flatMap` and `Flux#flatMapSequential` have subtle semantics; "
+ "please use `Flux#concatMap` or explicitly specify the desired amount of concurrency",
"""
`Flux#flatMap` and `Flux#flatMapSequential` have subtle semantics; please use \
`Flux#concatMap` or explicitly specify the desired amount of concurrency""",
link = BUG_PATTERNS_BASE_URL + "FluxFlatMapUsage",
linkType = CUSTOM,
severity = ERROR,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,8 @@ private static class ReplacementArgumentsConstructor
}

private void appendExpression(Tree tree) {
if (tree instanceof LiteralTree) {
formatString.append(((LiteralTree) tree).getValue());
if (tree instanceof LiteralTree literal) {
formatString.append(literal.getValue());
} else {
formatString.append(formatSpecifier);
formatArguments.add(tree);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,9 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState

return buildDescription(tree)
.setMessage(
"This method invocation appears redundant; remove it or suppress this warning and "
+ "add a comment explaining its purpose")
"""
This method invocation appears redundant; remove it or suppress this warning and add a \
comment explaining its purpose""")
.addFix(SuggestedFix.replace(tree, SourceCode.treeToString(sourceTree, state)))
.addFix(SuggestedFixes.addSuppressWarnings(state, canonicalName()))
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,9 @@
@AutoService(BugChecker.class)
@BugPattern(
summary =
"`SortedSet` properties of a `@Value.Immutable` or `@Value.Modifiable` type must be "
+ "annotated with `@Value.NaturalOrder` or `@Value.ReverseOrder`",
"""
`SortedSet` properties of a `@Value.Immutable` or `@Value.Modifiable` type must be \
annotated with `@Value.NaturalOrder` or `@Value.ReverseOrder`""",
link = BUG_PATTERNS_BASE_URL + "ImmutablesSortedSetComparator",
linkType = CUSTOM,
severity = ERROR,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.InstanceOfTree;
import com.sun.source.tree.LambdaExpressionTree;
import com.sun.source.tree.Tree.Kind;
import com.sun.source.tree.VariableTree;
import tech.picnic.errorprone.utils.SourceCode;

Expand All @@ -41,12 +40,12 @@ public IsInstanceLambdaUsage() {}

@Override
public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState state) {
if (tree.getParameters().size() != 1 || tree.getBody().getKind() != Kind.INSTANCE_OF) {
if (tree.getParameters().size() != 1
|| !(tree.getBody() instanceof InstanceOfTree instanceOf)) {
return Description.NO_MATCH;
}

VariableTree param = Iterables.getOnlyElement(tree.getParameters());
InstanceOfTree instanceOf = (InstanceOfTree) tree.getBody();
if (!ASTHelpers.getSymbol(param).equals(ASTHelpers.getSymbol(instanceOf.getExpression()))) {
return Description.NO_MATCH;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -265,8 +265,8 @@ private static Optional<String> tryExtractValueSourceAttributeValue(
arguments.stream()
.map(
arg ->
arg instanceof MethodInvocationTree
? Iterables.getOnlyElement(((MethodInvocationTree) arg).getArguments())
arg instanceof MethodInvocationTree methodInvocation
? Iterables.getOnlyElement(methodInvocation.getArguments())
: arg)
.map(argument -> SourceCode.treeToString(argument, state))
.collect(joining(", ")))
Expand All @@ -276,16 +276,12 @@ private static Optional<String> tryExtractValueSourceAttributeValue(
private static String toValueSourceAttributeName(Type type) {
String typeString = type.tsym.name.toString();

switch (typeString) {
case "Class":
return "classes";
case "Character":
return "chars";
case "Integer":
return "ints";
default:
return typeString.toLowerCase(Locale.ROOT) + 's';
}
return switch (typeString) {
case "Class" -> "classes";
case "Character" -> "chars";
case "Integer" -> "ints";
default -> typeString.toLowerCase(Locale.ROOT) + 's';
};
}

private static <T> Optional<T> getElementIfSingleton(Collection<T> collection) {
Expand All @@ -297,11 +293,10 @@ private static <T> Optional<T> getElementIfSingleton(Collection<T> collection) {
private static Matcher<ExpressionTree> isSingleDimensionArrayCreationWithAllElementsMatching(
Matcher<? super ExpressionTree> elementMatcher) {
return (tree, state) -> {
if (!(tree instanceof NewArrayTree)) {
if (!(tree instanceof NewArrayTree newArray)) {
return false;
}

NewArrayTree newArray = (NewArrayTree) tree;
return newArray.getDimensions().isEmpty()
&& !newArray.getInitializers().isEmpty()
&& newArray.getInitializers().stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import com.sun.source.tree.NewArrayTree;
import com.sun.source.tree.PrimitiveTypeTree;
import com.sun.source.tree.Tree;
import com.sun.source.tree.Tree.Kind;
import com.sun.source.util.TreeScanner;
import com.sun.tools.javac.code.Symtab;
import com.sun.tools.javac.code.Type;
Expand Down Expand Up @@ -122,13 +121,9 @@ private Optional<Fix> sortArrayElements(AnnotationTree tree, VisitorState state)
}

private static Optional<NewArrayTree> extractArray(ExpressionTree expr) {
if (expr.getKind() == Kind.ASSIGNMENT) {
return extractArray(((AssignmentTree) expr).getExpression());
}

return Optional.of(expr)
.filter(e -> e.getKind() == Kind.NEW_ARRAY)
.map(NewArrayTree.class::cast);
return expr instanceof AssignmentTree assignment
? extractArray(assignment.getExpression())
: Optional.of(expr).filter(NewArrayTree.class::isInstance).map(NewArrayTree.class::cast);
}

private static Optional<SuggestedFix.Builder> suggestSorting(
Expand Down Expand Up @@ -200,8 +195,8 @@ private static ImmutableList<ImmutableList<String>> getStructure(ExpressionTree
public @Nullable Void visitLiteral(LiteralTree node, @Nullable Void unused) {
Object value = ASTHelpers.constValue(node);
nodes.add(
value instanceof String
? STRING_ARGUMENT_SPLITTER.splitToStream((String) value).collect(toImmutableList())
value instanceof String str
? STRING_ARGUMENT_SPLITTER.splitToStream(str).collect(toImmutableList())
: ImmutableList.of(String.valueOf(value)));

return super.visitLiteral(node, unused);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,20 +67,19 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
return describeMatch(tree, SuggestedFixes.removeElement(arguments.get(0), arguments, state));
}

// XXX: Use switch pattern matching once the targeted JDK supports this.
private static boolean isTypeDerivableFromContext(MethodInvocationTree tree, VisitorState state) {
Tree parent = state.getPath().getParentPath().getLeaf();
switch (parent.getKind()) {
case VARIABLE:
return !ASTHelpers.hasImplicitType((VariableTree) parent, state)
&& MoreASTHelpers.areSameType(tree, parent, state);
case ASSIGNMENT:
return MoreASTHelpers.areSameType(tree, parent, state);
case RETURN:
return MoreASTHelpers.findMethodExitedOnReturn(state)
.filter(m -> MoreASTHelpers.areSameType(tree, m.getReturnType(), state))
.isPresent();
default:
return false;
}
return switch (parent.getKind()) {
case VARIABLE ->
!ASTHelpers.hasImplicitType((VariableTree) parent, state)
&& MoreASTHelpers.areSameType(tree, parent, state);
case ASSIGNMENT -> MoreASTHelpers.areSameType(tree, parent, state);
case RETURN ->
MoreASTHelpers.findMethodExitedOnReturn(state)
.filter(m -> MoreASTHelpers.areSameType(tree, m.getReturnType(), state))
.isPresent();
default -> false;
};
}
}
Loading

0 comments on commit 433b8b9

Please sign in to comment.