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

errorprone 2.3.3 -> 2.3.4 #1082

Merged
merged 5 commits into from
Dec 3, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -44,10 +43,12 @@ public final class GradleCacheableTaskAction extends BugChecker implements Lambd

private static final Matcher<ExpressionTree> TASK_ACTION = MethodMatchers.instanceMethod()
.onDescendantOf("org.gradle.api.Task")
.withNameMatching(Pattern.compile("doFirst|doLast"));
.namedAnyOf("doFirst", "doLast");

private static final Matcher<ExpressionTree> IS_TASK_ACTION =
Matchers.allOf(Matchers.parentNode(Matchers.methodInvocation(TASK_ACTION)), IS_ACTION);
Matchers.allOf(
Matchers.parentNode(Matchers.toType(ExpressionTree.class, Matchers.methodInvocation(TASK_ACTION))),
IS_ACTION);

@Override
public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState state) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ public Matcher<? super ExpressionTree> 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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,8 @@ private static ImmutableList<SuggestedFix> 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,15 @@
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;

@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;",
Expand Down Expand Up @@ -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;",
Expand Down Expand Up @@ -150,4 +142,7 @@ public void negative() throws Exception {
.doTest();
}

private CompilationTestHelper helper() {
return CompilationTestHelper.newInstance(Slf4jConstantLogMessage.class, getClass());
}
}
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ allprojects {
check("PreferSafeLoggableExceptions", CheckSeverity.OFF)
check("PreferSafeLoggingPreconditions", CheckSeverity.OFF)
check("PreconditionsConstantMessage", CheckSeverity.OFF)
check("GradleCacheableTaskAction", CheckSeverity.OFF)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can re-enable this as soon as this repo upgrades to a version of baseline that includes this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's really scary that a bump in error_prone_annotations alone (which looks like it should be a sort of API, not implementation) changes the implementation logic.

}
}
}
Expand Down
7 changes: 7 additions & 0 deletions changelog/@unreleased/pr-1082.v2.yml
Original file line number Diff line number Diff line change
@@ -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
8 changes: 4 additions & 4 deletions versions.props
Original file line number Diff line number Diff line change
@@ -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
Expand Down