From 91ad613ae0484d347f6bf054cd8856ae7639dc6b Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Thu, 13 Oct 2022 10:47:16 -0400 Subject: [PATCH 1/5] Upgrade error-prone to 2.16 --- versions.lock | 20 ++++++++++---------- versions.props | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/versions.lock b/versions.lock index 57c679f5f..af1d65e9f 100644 --- a/versions.lock +++ b/versions.lock @@ -14,13 +14,13 @@ com.google.auto.service:auto-service-annotations:1.0.1 (3 constraints: 2a3379a0) com.google.auto.value:auto-value:1.7.4 (1 constraints: 1f1221fb) com.google.auto.value:auto-value-annotations:1.9 (3 constraints: 802d5ac8) com.google.code.findbugs:jsr305:3.0.2 (6 constraints: 66626968) -com.google.errorprone:error_prone_annotation:2.15.0 (3 constraints: c43898ed) -com.google.errorprone:error_prone_annotations:2.15.0 (12 constraints: 85bda891) -com.google.errorprone:error_prone_check_api:2.15.0 (2 constraints: b22516ff) -com.google.errorprone:error_prone_core:2.15.0 (2 constraints: 1118d166) -com.google.errorprone:error_prone_refaster:2.15.0 (1 constraints: 3a053e3b) -com.google.errorprone:error_prone_test_helpers:2.15.0 (1 constraints: 3a053e3b) -com.google.errorprone:error_prone_type_annotations:2.15.0 (1 constraints: 251151c9) +com.google.errorprone:error_prone_annotation:2.16 (3 constraints: ad370ed6) +com.google.errorprone:error_prone_annotations:2.16 (12 constraints: 6ebc6da5) +com.google.errorprone:error_prone_check_api:2.16 (2 constraints: f8242c7e) +com.google.errorprone:error_prone_core:2.16 (2 constraints: 5717731a) +com.google.errorprone:error_prone_refaster:2.16 (1 constraints: dd04fb30) +com.google.errorprone:error_prone_test_helpers:2.16 (1 constraints: dd04fb30) +com.google.errorprone:error_prone_type_annotations:2.16 (1 constraints: c81038a7) com.google.googlejavaformat:google-java-format:1.13.0 (1 constraints: 8b149d75) com.google.guava:failureaccess:1.0.1 (1 constraints: 140ae1b4) com.google.guava:guava:31.1-jre (14 constraints: d6e9749b) @@ -69,9 +69,9 @@ org.apache.maven.resolver:maven-resolver-util:1.6.3 (3 constraints: 5930fd65) org.apache.maven.shared:maven-dependency-analyzer:1.13.0 (1 constraints: 3705323b) org.apache.maven.shared:maven-shared-utils:3.3.4 (1 constraints: e60b61f3) org.assertj:assertj-core:3.23.1 (3 constraints: e42af49e) -org.checkerframework:checker-qual:3.23.0 (3 constraints: 08256088) -org.checkerframework:dataflow-errorprone:3.23.0 (4 constraints: fa3d685c) -org.checkerframework:dataflow-nullaway:3.23.0 (2 constraints: 671166f1) +org.checkerframework:checker-qual:3.24.0 (3 constraints: 08256088) +org.checkerframework:dataflow-errorprone:3.24.0 (4 constraints: 003ec05d) +org.checkerframework:dataflow-nullaway:3.24.0 (2 constraints: 671166f1) org.codehaus.groovy:groovy:3.0.10 (3 constraints: e32879d6) org.codehaus.groovy:groovy-xml:3.0.10 (1 constraints: 791161da) org.codehaus.plexus:plexus-cipher:2.0 (1 constraints: 641174c7) diff --git a/versions.props b/versions.props index 43639e4ca..0a2ffc103 100644 --- a/versions.props +++ b/versions.props @@ -44,7 +44,7 @@ com.palantir.javaformat:gradle-palantir-java-format = 1.1.0 com.diffplug.spotless:spotless-plugin-gradle = 6.6.0 # Newer releases of checkerframework do not support jdk15. These dependencies may # be moved back to hte upgradable block once we've removed uses of jdk-15 MTS -com.google.errorprone:error_prone_* = 2.15.0 +com.google.errorprone:error_prone_* = 2.16 com.uber.nullaway:nullaway = 0.9.9 org.checkerframework:* = 3.23.0 # Groovy versions must be compatible with gradle From fee86b5f263bab6c2f4cb45c11344c142b2883b6 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Thu, 13 Oct 2022 11:01:37 -0400 Subject: [PATCH 2/5] Fix ASTHelpersSuggestions --- .../baseline/errorprone/ClassInitializationDeadlock.java | 6 +++--- .../palantir/baseline/errorprone/StrictUnusedVariable.java | 4 ++-- .../palantir/baseline/errorprone/ZeroWarmupRateLimiter.java | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ClassInitializationDeadlock.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ClassInitializationDeadlock.java index 7323045c8..9aadf6dcb 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ClassInitializationDeadlock.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ClassInitializationDeadlock.java @@ -171,7 +171,7 @@ public Void visitCompoundAssignment(CompoundAssignmentTree node, VisitorState st private boolean handleAssignment(ExpressionTree variable, VisitorState state) { // Matches if a subtype static field assignment occurs Symbol symbol = ASTHelpers.getSymbol(variable); - if (symbol != null && symbol.isStatic() && symbol instanceof VarSymbol) { + if (symbol != null && ASTHelpers.isStatic(symbol) && symbol instanceof VarSymbol) { VarSymbol varSymbol = (VarSymbol) symbol; if (isSubtype(varSymbol.owner.type, baseType, state)) { state.reportMatch(describeMatch(variable)); @@ -184,7 +184,7 @@ private boolean handleAssignment(ExpressionTree variable, VisitorState state) { @Override public Void visitMemberSelect(MemberSelectTree node, VisitorState state) { Symbol symbol = ASTHelpers.getSymbol(node); - if (symbol != null && symbol.isStatic() && symbol instanceof VarSymbol) { + if (symbol != null && ASTHelpers.isStatic(symbol) && symbol instanceof VarSymbol) { VarSymbol varSymbol = (VarSymbol) symbol; // Constant values may be accessed without forcing initialization if (varSymbol.getConstValue() == null @@ -231,7 +231,7 @@ private static boolean canBeExternallyInitialized(Symbol symbol) { case FIELD: // fall through case METHOD: - if (symbol.isStatic() && !symbol.isPrivate()) { + if (ASTHelpers.isStatic(symbol) && !symbol.isPrivate()) { return true; } break; diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/StrictUnusedVariable.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/StrictUnusedVariable.java index 25b34175b..47e094108 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/StrictUnusedVariable.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/StrictUnusedVariable.java @@ -41,6 +41,7 @@ import static com.google.errorprone.BugPattern.SeverityLevel.ERROR; import static com.google.errorprone.util.ASTHelpers.getSymbol; import static com.google.errorprone.util.ASTHelpers.getType; +import static com.google.errorprone.util.ASTHelpers.isStatic; import static com.google.errorprone.util.ASTHelpers.isSubtype; import static com.google.errorprone.util.SideEffectAnalysis.hasSideEffect; import static com.sun.source.tree.Tree.Kind.POSTFIX_DECREMENT; @@ -449,8 +450,7 @@ private static ImmutableList buildUnusedVarFixes( if (hasSideEffect(initializer) && TOP_LEVEL_EXPRESSIONS.contains(initializer.getKind())) { if (varKind == ElementKind.FIELD) { String newContent = String.format( - "%s{ %s; }", - varSymbol.isStatic() ? "static " : "", state.getSourceForNode(initializer)); + "%s{ %s; }", isStatic(varSymbol) ? "static " : "", state.getSourceForNode(initializer)); fix.merge(SuggestedFixes.replaceIncludingComments(usagePath, newContent, state)); } else { fix.replace(statement, String.format("%s;", state.getSourceForNode(initializer))); diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ZeroWarmupRateLimiter.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ZeroWarmupRateLimiter.java index 5cfa69cbe..46dfdf8b2 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ZeroWarmupRateLimiter.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ZeroWarmupRateLimiter.java @@ -81,7 +81,7 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState */ private static boolean isDurationZero(ExpressionTree expressionTree, VisitorState state) { Symbol symbol = ASTHelpers.getSymbol(expressionTree); - if (symbol != null && symbol.isStatic() && symbol instanceof VarSymbol) { + if (symbol != null && ASTHelpers.isStatic(symbol) && symbol instanceof VarSymbol) { VarSymbol varSymbol = (VarSymbol) symbol; return varSymbol.name.contentEquals("ZERO") && state.getTypes() From 770c400b9e7091c7415c070bed798c714a218a96 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Thu, 13 Oct 2022 14:38:45 -0400 Subject: [PATCH 3/5] update JooqResultStreamLeak --- .../errorprone/JooqResultStreamLeak.java | 31 +++++++++---------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/JooqResultStreamLeak.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/JooqResultStreamLeak.java index 6c8379333..6fe0d1cbb 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/JooqResultStreamLeak.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/JooqResultStreamLeak.java @@ -23,11 +23,12 @@ import com.google.errorprone.bugpatterns.StreamResourceLeak; import com.google.errorprone.matchers.Description; import com.google.errorprone.matchers.Matcher; +import com.google.errorprone.matchers.Matchers; import com.google.errorprone.matchers.method.MethodMatchers; import com.google.errorprone.suppliers.Supplier; import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.ExpressionTree; -import com.sun.source.tree.MethodInvocationTree; +import com.sun.source.tree.MethodTree; import com.sun.tools.javac.code.Type; @AutoService(BugChecker.class) @@ -39,32 +40,28 @@ + " try-with-resources. Not doing so can result in leaked database resources (such as connections" + " or cursors) in code paths that throw an exception or fail to call #close().") public final class JooqResultStreamLeak extends StreamResourceLeak { - private static final Matcher MATCHER = MethodMatchers.instanceMethod() - .onDescendantOf("org.jooq.ResultQuery") - .withAnyName(); + private static final Matcher MATCHER = Matchers.allOf( + MethodMatchers.instanceMethod() + .onDescendantOf("org.jooq.ResultQuery") + .withAnyName(), + JooqResultStreamLeak::shouldBeAutoClosed); private static final Supplier JOOQ_QUERY_PART = VisitorState.memoize(state -> state.getTypeFromString("org.jooq.QueryPart")); - @Override - public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { - if (!MATCHER.matches(tree, state)) { - return Description.NO_MATCH; - } - - if (!shouldBeAutoClosed(tree, state)) { - return Description.NO_MATCH; - } + private static final Supplier AUTO_CLOSEABLE = + VisitorState.memoize(state -> state.getTypeFromString(AutoCloseable.class.getName())); - return matchNewClassOrMethodInvocation(tree, state, findingPerSite()); + @Override + public Description matchMethod(MethodTree tree, VisitorState state) { + return scanEntireMethodFor(MATCHER, tree, state); } - private static boolean shouldBeAutoClosed(MethodInvocationTree tree, VisitorState state) { + private static boolean shouldBeAutoClosed(ExpressionTree tree, VisitorState state) { Type returnType = ASTHelpers.getReturnType(tree); // Most auto-closeable returns should be auto-closed. - boolean isAutoCloseable = - ASTHelpers.isSubtype(returnType, state.getTypeFromString(AutoCloseable.class.getName()), state); + boolean isAutoCloseable = ASTHelpers.isSubtype(returnType, AUTO_CLOSEABLE.get(state), state); // QueryParts can hold resources but usually don't, so auto-tripping and trying to fix on things // like SelectConditionStep is unnecessary. From b89f80b4de50bbaeb8356d52ef58395eb28f1afa Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Thu, 13 Oct 2022 15:45:40 -0400 Subject: [PATCH 4/5] no jdk15 support --- .../baseline/plugins/BaselineNullAway.java | 45 ------------------- .../BaselineNullAwayIntegrationTest.groovy | 9 ---- versions.lock | 8 ++-- versions.props | 8 ++-- 4 files changed, 7 insertions(+), 63 deletions(-) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineNullAway.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineNullAway.java index 0db9611cb..49d35ccd5 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineNullAway.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineNullAway.java @@ -16,12 +16,10 @@ package com.palantir.baseline.plugins; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import java.util.Optional; import net.ltgt.gradle.errorprone.ErrorProneOptions; import org.gradle.api.Action; -import org.gradle.api.JavaVersion; import org.gradle.api.Plugin; import org.gradle.api.Project; import org.gradle.api.artifacts.Configuration; @@ -38,14 +36,6 @@ public final class BaselineNullAway implements Plugin { /** We may add a gradle extension in a future release allowing custom additional packages. */ private static final ImmutableSet DEFAULT_ANNOTATED_PACKAGES = ImmutableSet.of("com.palantir"); - private static final ImmutableList NON_15_DEPS = ImmutableList.of( - "com.uber.nullaway:nullaway:0.10.1", - // align 'org.checkerframework' dependency versions on - // the current latest version. - "org.checkerframework:dataflow-errorprone:3.25.0", - "org.checkerframework:dataflow-nullaway:3.25.0", - "org.checkerframework:checker-qual:3.25.0"); - @Override public void apply(Project project) { project.getPluginManager().withPlugin("com.palantir.baseline-error-prone", _unused0 -> { @@ -88,7 +78,6 @@ public void execute(ErrorProneOptions options) { } } }); - newerNullAwayInNonJdk15Projects(project); } private static void configureErrorProneOptions(Project proj, Action action) { @@ -106,38 +95,4 @@ public void execute(JavaCompile javaCompile) { } }); } - - // Workaround for nullaway bugs in the last release to support jdk-15. Projects that don't use jdk-15 - // resolve a newer nullaway version with bug-fixes. This may be deleted after we've rolled everything - // off jdk-15 MTS. - private static void newerNullAwayInNonJdk15Projects(Project project) { - project.getConfigurations() - .matching(new Spec() { - @Override - public boolean isSatisfiedBy(Configuration config) { - return "errorprone".equals(config.getName()); - } - }) - .configureEach(new Action() { - @Override - public void execute(Configuration _files) { - for (String dep : NON_15_DEPS) { - project.getDependencies().addProvider("errorprone", project.provider(() -> { - // Cannot upgrade dependencies in this case because newer nullaway/checkerframework - // are not compatible with java 15, but fully support jdk11 and jdk17. - return anyProjectUsesJava15(project) ? null : dep; - })); - } - } - }); - } - - private static boolean anyProjectUsesJava15(Project proj) { - return proj.getAllprojects().stream() - .anyMatch(project -> ImmutableList.copyOf(project.getTasks().withType(JavaCompile.class)).stream() - .anyMatch(comp -> { - JavaVersion javaVersion = JavaVersion.toVersion(comp.getTargetCompatibility()); - return javaVersion == JavaVersion.VERSION_15; - })); - } } diff --git a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineNullAwayIntegrationTest.groovy b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineNullAwayIntegrationTest.groovy index 805130b62..5f6c42f9b 100644 --- a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineNullAwayIntegrationTest.groovy +++ b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineNullAwayIntegrationTest.groovy @@ -115,15 +115,6 @@ class BaselineNullAwayIntegrationTest extends IntegrationSpec { runTasksSuccessfully('compileJava') } - def 'compileJava succeeds when null-away finds no errors on jdk15'() { - when: - buildFile << standardBuildFile.replace('libraryTarget = 17', 'libraryTarget = 15') - writeJavaSourceFile(validJavaFile) - - then: - runTasksSuccessfully('compileJava') - } - def 'compileJava succeeds when null-away finds no errors on jdk11'() { when: buildFile << standardBuildFile.replace('libraryTarget = 17', 'libraryTarget = 11') diff --git a/versions.lock b/versions.lock index af1d65e9f..c7c0d7273 100644 --- a/versions.lock +++ b/versions.lock @@ -40,7 +40,7 @@ com.palantir.javaformat:palantir-java-format-spi:1.1.0 (1 constraints: 711560be) com.palantir.safe-logging:preconditions:3.1.0 (6 constraints: 445723f0) com.palantir.safe-logging:safe-logging:3.1.0 (8 constraints: 587745d3) com.palantir.tritium:tritium-registry:0.57.0 (1 constraints: 3e05483b) -com.uber.nullaway:nullaway:0.9.9 (1 constraints: 14050f36) +com.uber.nullaway:nullaway:0.10.2 (1 constraints: 3505253b) commons-io:commons-io:2.11.0 (1 constraints: 57119bcb) commons-lang:commons-lang:2.6 (1 constraints: ac04232c) io.dropwizard.metrics:metrics-core:4.1.1 (1 constraints: 901088a5) @@ -69,9 +69,9 @@ org.apache.maven.resolver:maven-resolver-util:1.6.3 (3 constraints: 5930fd65) org.apache.maven.shared:maven-dependency-analyzer:1.13.0 (1 constraints: 3705323b) org.apache.maven.shared:maven-shared-utils:3.3.4 (1 constraints: e60b61f3) org.assertj:assertj-core:3.23.1 (3 constraints: e42af49e) -org.checkerframework:checker-qual:3.24.0 (3 constraints: 08256088) -org.checkerframework:dataflow-errorprone:3.24.0 (4 constraints: 003ec05d) -org.checkerframework:dataflow-nullaway:3.24.0 (2 constraints: 671166f1) +org.checkerframework:checker-qual:3.25.0 (3 constraints: 08256088) +org.checkerframework:dataflow-errorprone:3.25.0 (4 constraints: 023e005f) +org.checkerframework:dataflow-nullaway:3.25.0 (2 constraints: 6911b8f1) org.codehaus.groovy:groovy:3.0.10 (3 constraints: e32879d6) org.codehaus.groovy:groovy-xml:3.0.10 (1 constraints: 791161da) org.codehaus.plexus:plexus-cipher:2.0 (1 constraints: 641174c7) diff --git a/versions.props b/versions.props index 0a2ffc103..923502ad5 100644 --- a/versions.props +++ b/versions.props @@ -5,14 +5,17 @@ com.palantir.safe-logging:* = 3.1.0 commons-lang:commons-lang = 2.6 org.apache.maven.shared:maven-dependency-analyzer = 1.13.0 org.apache.maven:maven-core = 3.8.5 +org.checkerframework:* = 3.25.0 org.inferred:freebuilder = 1.14.6 org.jooq:jooq = 3.17.4 org.slf4j:* = 1.7.36 org.immutables:* = 2.8.8 org.ow2.asm:asm = 9.4 +com.google.errorprone:error_prone_* = 2.16 com.googlecode.java-diff-utils:diffutils = 1.3.0 com.puppycrawl.tools:checkstyle = 10.3.4 com.palantir.gradle.utils:* = 0.1.0 +com.uber.nullaway:nullaway = 0.10.2 # test deps com.fasterxml.jackson.*:* = 2.13.3 @@ -42,11 +45,6 @@ org.mockito:* = 4.8.0 com.palantir.javaformat:gradle-palantir-java-format = 1.1.0 # Newer spotless versions have issues resolving dependencies at configuration time com.diffplug.spotless:spotless-plugin-gradle = 6.6.0 -# Newer releases of checkerframework do not support jdk15. These dependencies may -# be moved back to hte upgradable block once we've removed uses of jdk-15 MTS -com.google.errorprone:error_prone_* = 2.16 -com.uber.nullaway:nullaway = 0.9.9 -org.checkerframework:* = 3.23.0 # Groovy versions must be compatible with gradle org.codehaus.groovy:* = 3.0.10 # dependency-upgrader:ON From 33ebaa4c9744c6ea99e09ad06e5d7a61b51c118b Mon Sep 17 00:00:00 2001 From: svc-changelog Date: Thu, 13 Oct 2022 19:47:26 +0000 Subject: [PATCH 5/5] Add generated changelog entries --- changelog/@unreleased/pr-2432.v2.yml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelog/@unreleased/pr-2432.v2.yml diff --git a/changelog/@unreleased/pr-2432.v2.yml b/changelog/@unreleased/pr-2432.v2.yml new file mode 100644 index 000000000..48fcee737 --- /dev/null +++ b/changelog/@unreleased/pr-2432.v2.yml @@ -0,0 +1,6 @@ +type: improvement +improvement: + description: Upgrade error-prone to 2.16, removing support for compilation with + a jdk-15 target + links: + - https://github.com/palantir/gradle-baseline/pull/2432