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
Upgrade error_prone_annotations to 2.3.4 #6574
Changes from 25 commits
0417f50
c534c13
f4e6c09
484531b
e7f9c6a
d6b4454
f7fa412
ef4264e
9e1c3a5
51175e4
f26fe6d
61fb9f2
af940dc
5e13810
ab08958
6c98862
5a1fa57
6c20c9d
211c2c2
7488e21
5bd6d01
6fb9bac
fb4d7a6
028bb6e
a2f4a83
0a2a76b
749b67f
ef42e8b
dbcdc4c
10ec0cf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -113,7 +113,7 @@ subprojects { | |
libraries = [ | ||
android_annotations: "com.google.android:annotations:4.1.1.4", | ||
animalsniffer_annotations: "org.codehaus.mojo:animal-sniffer-annotations:1.18", | ||
errorprone: "com.google.errorprone:error_prone_annotations:2.3.3", | ||
errorprone: "com.google.errorprone:error_prone_annotations:2.3.4", | ||
gson: "com.google.code.gson:gson:2.8.6", | ||
guava: "com.google.guava:guava:${guavaVersion}", | ||
hpack: 'com.twitter:hpack:0.10.1', | ||
|
@@ -263,7 +263,7 @@ subprojects { | |
|
||
if (rootProject.properties.get('errorProne', true)) { | ||
dependencies { | ||
errorprone 'com.google.errorprone:error_prone_core:2.3.3' | ||
errorprone 'com.google.errorprone:error_prone_core:2.3.4' | ||
errorproneJavac 'com.google.errorprone:javac:9+181-r4173-1' | ||
|
||
annotationProcessor 'com.google.guava:guava-beta-checker:1.0' | ||
|
@@ -279,9 +279,17 @@ subprojects { | |
} | ||
} | ||
|
||
compileJava { | ||
// This project targets Java 7 (no method references) | ||
options.errorprone.check("UnnecessaryAnonymousClass", CheckSeverity.OFF) | ||
// This project targets Java 7 (no time.Duration class) | ||
options.errorprone.check("PreferJavaTimeOverload", CheckSeverity.OFF) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Without PreferJavaTimeOverload suppression, the build for Java 11 fails:
Setting this suppression at project level (rather than annotation) because whole grpc-java project targets Java 7, which does not have java.time.Duration. |
||
} | ||
compileTestJava { | ||
// LinkedList doesn't hurt much in tests and has lots of usages | ||
options.errorprone.check("JdkObsolete", CheckSeverity.OFF) | ||
options.errorprone.check("UnnecessaryAnonymousClass", CheckSeverity.OFF) | ||
options.errorprone.check("PreferJavaTimeOverload", CheckSeverity.OFF) | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,7 +16,10 @@ dependencies { | |
compile project(':grpc-api'), | ||
libraries.gson, | ||
libraries.android_annotations, | ||
libraries.perfmark | ||
libraries.errorprone // to avoid using perfmark's dependency | ||
compile (libraries.perfmark) { | ||
exclude group: 'com.google.errorprone', module: 'error_prone_annotations' | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Gradle complained perfmark's errorprone annotation 2.3.3. https://travis-ci.org/grpc/grpc-java/jobs/630091143?utm_medium=notification&utm_source=github_status There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. You did the appropriate thing here, although we normally put a comment saying we are using version X instead of version Y (so that in the future we can tell if the exclude is old and should be removed). Our build enforces dependency convergence because I've not found a better approach to detecting requireUpperBoundDeps violations from within Gradle (because they aren't a problem in Gradle...). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated comment as "prefer our version to perfmark's 2.3.3". |
||
compile (libraries.opencensus_api) { | ||
// prefer our own versions instead of opencensus-api's dependency | ||
exclude group: 'com.google.code.findbugs', module: 'jsr305' | ||
|
@@ -56,3 +59,12 @@ animalsniffer { | |
sourceSets.test | ||
] | ||
} | ||
|
||
import net.ltgt.gradle.errorprone.CheckSeverity | ||
|
||
plugins.withId("java") { | ||
compileJmhJava { | ||
// This project targets Java 7 (no method references) | ||
options.errorprone.check("UnnecessaryAnonymousClass", CheckSeverity.OFF) | ||
} | ||
} | ||
Comment on lines
+65
to
+70
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. grpc-core has special task
|
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -127,6 +127,7 @@ public void closed( | |
} | ||
|
||
@Test | ||
@SuppressWarnings("GuardedBy") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Without this suppression, the build fails. #6578 will take care of the root cause. |
||
public void cancel_started() { | ||
stream.start(new BaseClientStreamListener()); | ||
stream.transportState().start(1234); | ||
|
@@ -145,6 +146,7 @@ public Void answer(InvocationOnMock invocation) throws Throwable { | |
} | ||
|
||
@Test | ||
@SuppressWarnings("GuardedBy") | ||
public void start_alreadyCancelled() { | ||
stream.start(new BaseClientStreamListener()); | ||
stream.cancel(Status.CANCELLED); | ||
|
@@ -155,6 +157,7 @@ public void start_alreadyCancelled() { | |
} | ||
|
||
@Test | ||
@SuppressWarnings("GuardedBy") | ||
public void start_userAgentRemoved() throws IOException { | ||
Metadata metaData = new Metadata(); | ||
metaData.put(GrpcUtil.USER_AGENT_KEY, "misbehaving-application"); | ||
|
@@ -171,6 +174,7 @@ public void start_userAgentRemoved() throws IOException { | |
} | ||
|
||
@Test | ||
@SuppressWarnings("GuardedBy") | ||
public void start_headerFieldOrder() throws IOException { | ||
Metadata metaData = new Metadata(); | ||
metaData.put(GrpcUtil.USER_AGENT_KEY, "misbehaving-application"); | ||
|
@@ -194,6 +198,7 @@ public void start_headerFieldOrder() throws IOException { | |
} | ||
|
||
@Test | ||
@SuppressWarnings("GuardedBy") | ||
public void start_headerPlaintext() throws IOException { | ||
Metadata metaData = new Metadata(); | ||
metaData.put(GrpcUtil.USER_AGENT_KEY, "misbehaving-application"); | ||
|
@@ -218,6 +223,7 @@ public void start_headerPlaintext() throws IOException { | |
} | ||
|
||
@Test | ||
@SuppressWarnings("GuardedBy") | ||
public void getUnaryRequest() throws IOException { | ||
MethodDescriptor<?, ?> getMethod = MethodDescriptor.<Void, Void>newBuilder() | ||
.setType(MethodDescriptor.MethodType.UNARY) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2127,7 +2127,7 @@ void assertClosed() { | |
// The wait is safe; nextFrame is called in a loop and can have spurious wakeups | ||
@SuppressWarnings("WaitNotInLoop") | ||
@Override | ||
public boolean nextFrame(Handler handler) throws IOException { | ||
public boolean nextFrame(FrameReader.Handler handler) throws IOException { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SameNameButDifferent. |
||
Result result; | ||
try { | ||
result = nextResults.take(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ | |
|
||
package io.grpc.okhttp.internal.framed; | ||
|
||
import com.google.errorprone.annotations.FormatMethod; | ||
import io.grpc.okhttp.internal.Protocol; | ||
import java.io.IOException; | ||
import java.util.List; | ||
|
@@ -361,7 +362,7 @@ private void readWindowUpdate( | |
throws IOException { | ||
if (length != 4) throw ioException("TYPE_WINDOW_UPDATE length !=4: %s", length); | ||
long increment = (source.readInt() & 0x7fffffffL); | ||
if (increment == 0) throw ioException("windowSizeIncrement was 0", increment); | ||
if (increment == 0) throw ioException("windowSizeIncrement was 0"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That
|
||
handler.windowUpdate(streamId, increment); | ||
} | ||
|
||
|
@@ -586,10 +587,12 @@ void frameHeader(int streamId, int length, byte type, byte flags) throws IOExcep | |
} | ||
} | ||
|
||
@FormatMethod | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The build failed by https://errorprone.info/bugpattern/AnnotateFormatMethod |
||
private static IllegalArgumentException illegalArgument(String message, Object... args) { | ||
throw new IllegalArgumentException(format(message, args)); | ||
} | ||
|
||
@FormatMethod | ||
private static IOException ioException(String message, Object... args) throws IOException { | ||
throw new IOException(format(message, args)); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,7 @@ IO_GRPC_GRPC_JAVA_ARTIFACTS = [ | |
"com.google.auth:google-auth-library-oauth2-http:0.19.0", | ||
"com.google.code.findbugs:jsr305:3.0.2", | ||
"com.google.code.gson:gson:jar:2.8.6", | ||
"com.google.errorprone:error_prone_annotations:2.3.3", | ||
"com.google.errorprone:error_prone_annotations:2.3.4", | ||
"com.google.guava:failureaccess:1.0.1", | ||
"com.google.guava:guava:28.1-android", | ||
"com.google.j2objc:j2objc-annotations:1.3", | ||
|
@@ -226,9 +226,9 @@ def com_google_code_gson_gson(): | |
def com_google_errorprone_error_prone_annotations(): | ||
jvm_maven_import_external( | ||
name = "com_google_errorprone_error_prone_annotations", | ||
artifact = "com.google.errorprone:error_prone_annotations:2.3.3", | ||
artifact = "com.google.errorprone:error_prone_annotations:2.3.4", | ||
server_urls = ["https://repo.maven.apache.org/maven2/"], | ||
artifact_sha256 = "ec59f1b702d9afc09e8c3929f5c42777dec623a6ea2731ac694332c7d7680f5a", | ||
artifact_sha256 = "baf7d6ea97ce606c53e11b6854ba5f2ce7ef5c24dddf0afa18d1260bd25b002c", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
licenses = ["notice"], # Apache 2.0 | ||
) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,7 +46,6 @@ | |
import io.grpc.MethodDescriptor; | ||
import io.grpc.MethodDescriptor.Marshaller; | ||
import io.grpc.ServerCall; | ||
import io.grpc.ServerCall.Listener; | ||
import io.grpc.ServerCallHandler; | ||
import io.grpc.ServerInterceptor; | ||
import io.grpc.Status; | ||
|
@@ -408,7 +407,7 @@ public <ReqT, RespT> ClientCall<ReqT, RespT> interceptCall( | |
|
||
return new SimpleForwardingClientCall<ReqT, RespT>(next.newCall(method, callOptions)) { | ||
@Override | ||
public void start(final Listener<RespT> responseListener, Metadata headers) { | ||
public void start(final ClientCall.Listener<RespT> responseListener, Metadata headers) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SameNameButDifferent. |
||
final Duration timeout = deadline == null ? null | ||
: Durations.fromNanos(deadline.timeRemaining(TimeUnit.NANOSECONDS)); | ||
writer.logClientHeader( | ||
|
@@ -500,7 +499,7 @@ public void cancel(String message, Throwable cause) { | |
public ServerInterceptor getServerInterceptor(final long callId) { | ||
return new ServerInterceptor() { | ||
@Override | ||
public <ReqT, RespT> Listener<ReqT> interceptCall( | ||
public <ReqT, RespT> ServerCall.Listener<ReqT> interceptCall( | ||
final ServerCall<ReqT, RespT> call, | ||
Metadata headers, | ||
ServerCallHandler<ReqT, RespT> next) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,7 +36,6 @@ | |
import io.grpc.MethodDescriptor.Marshaller; | ||
import io.grpc.MethodDescriptor.MethodType; | ||
import io.grpc.ServerCall; | ||
import io.grpc.ServerCall.Listener; | ||
import io.grpc.ServerCallHandler; | ||
import io.grpc.ServerInterceptor; | ||
import io.grpc.ServerMethodDefinition; | ||
|
@@ -127,7 +126,7 @@ public <RequestT, ResponseT> ClientCall<RequestT, ResponseT> newCall( | |
MethodDescriptor<RequestT, ResponseT> methodDescriptor, CallOptions callOptions) { | ||
return new NoopClientCall<RequestT, ResponseT>() { | ||
@Override | ||
public void start(Listener<ResponseT> responseListener, Metadata headers) { | ||
public void start(ClientCall.Listener<ResponseT> responseListener, Metadata headers) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SameNameButDifferent |
||
listener.set(responseListener); | ||
} | ||
|
||
|
@@ -211,7 +210,7 @@ public void wrapMethodDefinition_methodDescriptor() throws Exception { | |
method, | ||
new ServerCallHandler<String, Integer>() { | ||
@Override | ||
public Listener<String> startCall( | ||
public ServerCall.Listener<String> startCall( | ||
ServerCall<String, Integer> call, Metadata headers) { | ||
throw new UnsupportedOperationException(); | ||
} | ||
|
@@ -310,7 +309,7 @@ public <ReqT, RespT> ClientCall<ReqT, RespT> interceptCall( | |
assertSame(BinaryLogProvider.BYTEARRAY_MARSHALLER, method.getResponseMarshaller()); | ||
return new SimpleForwardingClientCall<ReqT, RespT>(next.newCall(method, callOptions)) { | ||
@Override | ||
public void start(Listener<RespT> responseListener, Metadata headers) { | ||
public void start(ClientCall.Listener<RespT> responseListener, Metadata headers) { | ||
super.start( | ||
new SimpleForwardingClientCallListener<RespT>(responseListener) { | ||
@Override | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suppressing UnnecessaryAnonymousClass as project-level suppression, because grpc-java targets Java 7 source.