From b202105640694a9d5d412e685afc756be26e59ab Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Tue, 3 Dec 2019 12:03:40 +0000 Subject: [PATCH 1/5] errorprone 2.3.3 -> 2.3.4 --- versions.props | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/versions.props b/versions.props index e0ac1f849..aab771283 100644 --- a/versions.props +++ b/versions.props @@ -1,9 +1,9 @@ com.diffplug.spotless:spotless-plugin-gradle = 3.26.0 com.google.auto.service:auto-service = 1.0-rc4 -com.google.errorprone:error_prone_annotations = 2.3.3 -com.google.errorprone:error_prone_core = 2.3.3 -com.google.errorprone:error_prone_refaster = 2.3.3 -com.google.errorprone:error_prone_test_helpers = 2.3.3 +com.google.errorprone:error_prone_annotations = 2.3.4 +com.google.errorprone:error_prone_core = 2.3.4 +com.google.errorprone:error_prone_refaster = 2.3.4 +com.google.errorprone:error_prone_test_helpers = 2.3.4 com.google.guava:guava = 27.1-jre com.netflix.nebula:nebula-dependency-recommender = 9.0.0 com.palantir.configurationresolver:gradle-configuration-resolver-plugin = 0.4.0 From 7d91096b5d835c832b8f07be6776ee966f6fe422 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Tue, 3 Dec 2019 12:03:54 +0000 Subject: [PATCH 2/5] GradleCacheableTaskAction seems kinda borked --- .../errorprone/GradleCacheableTaskAction.java | 19 ++++++++++--------- build.gradle | 1 + 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/GradleCacheableTaskAction.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/GradleCacheableTaskAction.java index fb96ca1d7..b7b2a3774 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/GradleCacheableTaskAction.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/GradleCacheableTaskAction.java @@ -46,16 +46,17 @@ public final class GradleCacheableTaskAction extends BugChecker implements Lambd .onDescendantOf("org.gradle.api.Task") .withNameMatching(Pattern.compile("doFirst|doLast")); - private static final Matcher IS_TASK_ACTION = - Matchers.allOf(Matchers.parentNode(Matchers.methodInvocation(TASK_ACTION)), IS_ACTION); + // private static final Matcher IS_TASK_ACTION = + // Matchers.allOf(Matchers.parentNode(Matchers.methodInvocation(TASK_ACTION)), IS_ACTION); @Override - public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState state) { - if (!IS_TASK_ACTION.matches(tree, state)) { - return Description.NO_MATCH; - } - return buildDescription(tree) - .setMessage("Gradle task actions are not cacheable when implemented by lambdas") - .build(); + public Description matchLambdaExpression(LambdaExpressionTree _tree, VisitorState _state) { + return Description.NO_MATCH; + // if (!IS_TASK_ACTION.matches(tree, state)) { + // return Description.NO_MATCH; + // } + // return buildDescription(tree) + // .setMessage("Gradle task actions are not cacheable when implemented by lambdas") + // .build(); } } diff --git a/build.gradle b/build.gradle index a4d93ed4c..fc90cfd57 100644 --- a/build.gradle +++ b/build.gradle @@ -54,6 +54,7 @@ allprojects { check("PreferSafeLoggableExceptions", CheckSeverity.OFF) check("PreferSafeLoggingPreconditions", CheckSeverity.OFF) check("PreconditionsConstantMessage", CheckSeverity.OFF) + check("GradleCacheableTaskAction", CheckSeverity.OFF) } } } From 85230aae3666aa428672da105175cd2932d7a63c Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Tue, 3 Dec 2019 12:04:06 +0000 Subject: [PATCH 3/5] Fix compilation --- .../palantir/baseline/errorprone/ReadReturnValueIgnored.java | 4 ++-- .../palantir/baseline/errorprone/StrictUnusedVariable.java | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ReadReturnValueIgnored.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ReadReturnValueIgnored.java index 4cbe2f196..b0792b605 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ReadReturnValueIgnored.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ReadReturnValueIgnored.java @@ -104,8 +104,8 @@ public Matcher specializedMatcher() { } @Override - public Description describe(MethodInvocationTree methodInvocationTree, VisitorState state) { - Description result = super.describe(methodInvocationTree, state); + public Description describeReturnValueIgnored(MethodInvocationTree methodInvocationTree, VisitorState state) { + Description result = super.describeReturnValueIgnored(methodInvocationTree, state); if (Description.NO_MATCH.equals(result)) { return result; } 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 38988e8ab..91885ce0b 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 @@ -434,7 +434,8 @@ private static ImmutableList buildUnusedVarFixes( ? null : state.getSourceForNode(variableTree.getModifiers())); String newContent = String.format( - "%s%s unused", modifiers.isEmpty() ? "" : (modifiers + " "), variableTree.getType()); + "%s%s unused", modifiers.isEmpty() ? "" : (modifiers + " "), + state.getSourceForNode(variableTree.getType())); // The new content for the second fix should be identical to the content for the first // fix in this case because we can't just remove the enhanced for loop variable. fix.replace(variableTree, newContent); From 0986c8d07ca701c65b3841be777a910a9babdd2b Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Tue, 3 Dec 2019 11:52:06 -0500 Subject: [PATCH 4/5] Fix GradleCacheableTaskAction on error-prone 2.3.4 (#1083) * Fix GradleCacheableTaskAction on error-prone 2.3.4 * Fix Slf4jConstantLogMessageTests: Don't reuse CompilationHelper instances --- .../errorprone/GradleCacheableTaskAction.java | 24 +++++++++---------- .../Slf4jConstantLogMessageTests.java | 19 ++++++--------- 2 files changed, 19 insertions(+), 24 deletions(-) diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/GradleCacheableTaskAction.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/GradleCacheableTaskAction.java index b7b2a3774..e179442a2 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/GradleCacheableTaskAction.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/GradleCacheableTaskAction.java @@ -28,7 +28,6 @@ import com.google.errorprone.matchers.method.MethodMatchers; import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.LambdaExpressionTree; -import java.util.regex.Pattern; @AutoService(BugChecker.class) @BugPattern( @@ -44,19 +43,20 @@ public final class GradleCacheableTaskAction extends BugChecker implements Lambd private static final Matcher TASK_ACTION = MethodMatchers.instanceMethod() .onDescendantOf("org.gradle.api.Task") - .withNameMatching(Pattern.compile("doFirst|doLast")); + .namedAnyOf("doFirst", "doLast"); - // private static final Matcher IS_TASK_ACTION = - // Matchers.allOf(Matchers.parentNode(Matchers.methodInvocation(TASK_ACTION)), IS_ACTION); + private static final Matcher IS_TASK_ACTION = + Matchers.allOf( + Matchers.parentNode(Matchers.toType(ExpressionTree.class, Matchers.methodInvocation(TASK_ACTION))), + IS_ACTION); @Override - public Description matchLambdaExpression(LambdaExpressionTree _tree, VisitorState _state) { - return Description.NO_MATCH; - // if (!IS_TASK_ACTION.matches(tree, state)) { - // return Description.NO_MATCH; - // } - // return buildDescription(tree) - // .setMessage("Gradle task actions are not cacheable when implemented by lambdas") - // .build(); + public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState state) { + if (!IS_TASK_ACTION.matches(tree, state)) { + return Description.NO_MATCH; + } + return buildDescription(tree) + .setMessage("Gradle task actions are not cacheable when implemented by lambdas") + .build(); } } diff --git a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/Slf4jConstantLogMessageTests.java b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/Slf4jConstantLogMessageTests.java index 44ba3936a..eaa85ab2a 100644 --- a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/Slf4jConstantLogMessageTests.java +++ b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/Slf4jConstantLogMessageTests.java @@ -17,7 +17,6 @@ package com.palantir.baseline.errorprone; import com.google.errorprone.CompilationTestHelper; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.parallel.Execution; import org.junit.jupiter.api.parallel.ExecutionMode; @@ -25,15 +24,8 @@ @Execution(ExecutionMode.CONCURRENT) public final class Slf4jConstantLogMessageTests { - private CompilationTestHelper compilationHelper; - - @BeforeEach - public void before() { - compilationHelper = CompilationTestHelper.newInstance(Slf4jConstantLogMessage.class, getClass()); - } - - private void test(String log) throws Exception { - compilationHelper + private void test(String log) { + helper() .addSourceLines( "Test.java", "import org.slf4j.Logger;", @@ -76,8 +68,8 @@ public void testNonCompileTimeConstantExpression() throws Exception { } @Test - public void negative() throws Exception { - compilationHelper + public void negative() { + helper() .addSourceLines( "Test.java", "import org.slf4j.Logger;", @@ -150,4 +142,7 @@ public void negative() throws Exception { .doTest(); } + private CompilationTestHelper helper() { + return CompilationTestHelper.newInstance(Slf4jConstantLogMessage.class, getClass()); + } } From 05f7a79bd19c56e7d02547d2eb24705d94110fe8 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Tue, 3 Dec 2019 16:52:06 +0000 Subject: [PATCH 5/5] Add generated changelog entries --- changelog/@unreleased/pr-1082.v2.yml | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 changelog/@unreleased/pr-1082.v2.yml diff --git a/changelog/@unreleased/pr-1082.v2.yml b/changelog/@unreleased/pr-1082.v2.yml new file mode 100644 index 000000000..7d5dde821 --- /dev/null +++ b/changelog/@unreleased/pr-1082.v2.yml @@ -0,0 +1,7 @@ +type: improvement +improvement: + description: "We now run error-prone 2.3.4, to benefit from smarter static analysis, + and hopefully pick up the claimed performance Improvements: \n\n> 40% speedup + when run against Google's codebase with errors enabled." + links: + - https://github.com/palantir/gradle-baseline/pull/1082