From 58a7ace6ac2041d3c9989a33fd46188ed56fed6a Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Wed, 12 Jan 2022 12:06:27 -0800 Subject: [PATCH] Bump ErrorProne to 2.10.0 Previous versions of error prone were incompatible with Java 17 javac. In grpc-api, errorprone is now api dependency because it is on a public API. I was happy to see that Gradle failed the build without the dep change, although the error message wasn't super clear as to the cause. It seems that previously -PerrorProne=false did nothing. I'm guessing this is due to a behavior change of Gradle at some point. Swapping to using the project does build without errorProne, although the build fails with Javac complaining certain classes are unavailable. It's unclear why. It doesn't seem to be caused by the error-prone plugin. I've left it failing as a pre-existing issue. ClientCalls/ServerCalls had Deprecated removed from some methods because they were only deprecated in the internal class, not the API. And with Deprecated, InlineMeSuggester complained. I'm finding InlineMeSuggester to be overzealous, complaining about package-private methods. In time we may figure out how to use it better, or we may request changes to the checker in error-prone. --- .../alts/internal/ChannelCrypterNetty.java | 4 ++-- .../java/io/grpc/alts/internal/TsiTest.java | 4 ++-- api/build.gradle | 4 ++-- api/src/main/java/io/grpc/Attributes.java | 14 ++++++++------ api/src/main/java/io/grpc/NameResolver.java | 5 +++++ build.gradle | 18 ++++++++---------- context/src/main/java/io/grpc/Context.java | 2 +- .../internal/AbstractServerImplBuilder.java | 2 ++ .../internal/JndiResourceResolverFactory.java | 2 +- .../integration/NettyFlowControlTest.java | 14 -------------- .../integration/TrafficControlProxy.java | 2 +- .../grpc/netty/GrpcHttp2ConnectionHandler.java | 1 + .../io/grpc/netty/NettyChannelBuilder.java | 2 ++ .../java/io/grpc/netty/NettyServerBuilder.java | 3 +++ .../services/BinaryLogProviderImpl.java | 1 + .../main/java/io/grpc/stub/ClientCalls.java | 1 - .../main/java/io/grpc/stub/ServerCalls.java | 1 - .../java/io/grpc/xds/XdsServerWrapperTest.java | 6 ++---- 18 files changed, 41 insertions(+), 45 deletions(-) diff --git a/alts/src/main/java/io/grpc/alts/internal/ChannelCrypterNetty.java b/alts/src/main/java/io/grpc/alts/internal/ChannelCrypterNetty.java index 4164560e7a..e2e7d4046f 100644 --- a/alts/src/main/java/io/grpc/alts/internal/ChannelCrypterNetty.java +++ b/alts/src/main/java/io/grpc/alts/internal/ChannelCrypterNetty.java @@ -21,8 +21,8 @@ import java.security.GeneralSecurityException; import java.util.List; /** - * A @{code ChannelCrypterNetty} performs stateful encryption and decryption of independent input - * and output streams. Both decrypt and encrypt gather their input from a list of Netty @{link + * A {@code ChannelCrypterNetty} performs stateful encryption and decryption of independent input + * and output streams. Both decrypt and encrypt gather their input from a list of Netty {@link * ByteBuf} instances. * *

Note that we provide implementations of this interface that provide integrity only and diff --git a/alts/src/test/java/io/grpc/alts/internal/TsiTest.java b/alts/src/test/java/io/grpc/alts/internal/TsiTest.java index f677a44a75..f75fb7fcee 100644 --- a/alts/src/test/java/io/grpc/alts/internal/TsiTest.java +++ b/alts/src/test/java/io/grpc/alts/internal/TsiTest.java @@ -34,13 +34,13 @@ import java.util.Collections; import java.util.List; import javax.crypto.AEADBadTagException; -/** Utility class that provides tests for implementations of @{link TsiHandshaker}. */ +/** Utility class that provides tests for implementations of {@link TsiHandshaker}. */ public final class TsiTest { private static final String DECRYPTION_FAILURE_RE = "Tag mismatch!|BAD_DECRYPT"; private TsiTest() {} - /** A @{code TsiHandshaker} pair for running tests. */ + /** A {@code TsiHandshaker} pair for running tests. */ public static class Handshakers { private final TsiHandshaker client; private final TsiHandshaker server; diff --git a/api/build.gradle b/api/build.gradle index 1348e49ad6..3c7ff8221e 100644 --- a/api/build.gradle +++ b/api/build.gradle @@ -12,9 +12,9 @@ evaluationDependsOn(project(':grpc-context').path) dependencies { api project(':grpc-context'), - libraries.jsr305 - implementation libraries.guava, + libraries.jsr305, libraries.errorprone + implementation libraries.guava testImplementation project(':grpc-context').sourceSets.test.output, project(':grpc-testing'), diff --git a/api/src/main/java/io/grpc/Attributes.java b/api/src/main/java/io/grpc/Attributes.java index b70cc2bc8c..26c2a72f90 100644 --- a/api/src/main/java/io/grpc/Attributes.java +++ b/api/src/main/java/io/grpc/Attributes.java @@ -46,11 +46,13 @@ import javax.annotation.concurrent.Immutable; @Immutable public final class Attributes { - private final Map, Object> data; + private final IdentityHashMap, Object> data; - public static final Attributes EMPTY = new Attributes(Collections., Object>emptyMap()); + private static final IdentityHashMap, Object> EMPTY_MAP = + new IdentityHashMap, Object>(); + public static final Attributes EMPTY = new Attributes(EMPTY_MAP); - private Attributes(Map, Object> data) { + private Attributes(IdentityHashMap, Object> data) { assert data != null; this.data = data; } @@ -212,14 +214,14 @@ public final class Attributes { */ public static final class Builder { private Attributes base; - private Map, Object> newdata; + private IdentityHashMap, Object> newdata; private Builder(Attributes base) { assert base != null; this.base = base; } - private Map, Object> data(int size) { + private IdentityHashMap, Object> data(int size) { if (newdata == null) { newdata = new IdentityHashMap<>(size); } @@ -241,7 +243,7 @@ public final class Attributes { @ExperimentalApi("https://github.com/grpc/grpc-java/issues/5777") public Builder discard(Key key) { if (base.data.containsKey(key)) { - Map, Object> newBaseData = new IdentityHashMap<>(base.data); + IdentityHashMap, Object> newBaseData = new IdentityHashMap<>(base.data); newBaseData.remove(key); base = new Attributes(newBaseData); } diff --git a/api/src/main/java/io/grpc/NameResolver.java b/api/src/main/java/io/grpc/NameResolver.java index f4c05aa6a6..78b35a14e3 100644 --- a/api/src/main/java/io/grpc/NameResolver.java +++ b/api/src/main/java/io/grpc/NameResolver.java @@ -21,6 +21,7 @@ import static com.google.common.base.Preconditions.checkNotNull; import com.google.common.base.MoreObjects; import com.google.common.base.Objects; +import com.google.errorprone.annotations.InlineMe; import java.lang.annotation.Documented; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; @@ -204,6 +205,10 @@ public abstract class NameResolver { */ @Override @Deprecated + @InlineMe( + replacement = "this.onResult(ResolutionResult.newBuilder().setAddresses(servers)" + + ".setAttributes(attributes).build())", + imports = "io.grpc.NameResolver.ResolutionResult") public final void onAddresses( List servers, @ResolutionResultAttr Attributes attributes) { // TODO(jihuncho) need to promote Listener2 if we want to use ConfigOrError diff --git a/build.gradle b/build.gradle index 149ade4ae0..9a649b2886 100644 --- a/build.gradle +++ b/build.gradle @@ -151,7 +151,7 @@ subprojects { animalsniffer_annotations: "org.codehaus.mojo:animal-sniffer-annotations:1.19", autovalue: "com.google.auto.value:auto-value:${autovalueVersion}", autovalue_annotation: "com.google.auto.value:auto-value-annotations:${autovalueVersion}", - errorprone: "com.google.errorprone:error_prone_annotations:2.9.0", + errorprone: "com.google.errorprone:error_prone_annotations:2.10.0", cronet_api: 'org.chromium.net:cronet-api:92.4515.131', cronet_embedded: 'org.chromium.net:cronet-embedded:92.4515.131', gson: "com.google.code.gson:gson:2.8.9", @@ -239,19 +239,15 @@ subprojects { } } - if (rootProject.properties.get('errorProne', true)) { + if (!project.hasProperty('errorProne') || errorProne.toBoolean()) { dependencies { - errorprone 'com.google.errorprone:error_prone_core:2.4.0' + errorprone 'com.google.errorprone:error_prone_core:2.10.0' errorproneJavac 'com.google.errorprone:javac:9+181-r4173-1' } } else { // Disable Error Prone - allprojects { - afterEvaluate { project -> - project.tasks.withType(JavaCompile) { - options.errorprone.enabled = false - } - } + tasks.withType(JavaCompile) { + options.errorprone.enabled = false } } @@ -304,7 +300,7 @@ subprojects { maxHeapSize = '1500m' } - if (rootProject.properties.get('errorProne', true)) { + if (!project.hasProperty('errorProne') || errorProne.toBoolean()) { dependencies { annotationProcessor 'com.google.guava:guava-beta-checker:1.0' } @@ -315,6 +311,7 @@ subprojects { options.errorprone.check("UnnecessaryAnonymousClass", CheckSeverity.OFF) // This project targets Java 7 (no time.Duration class) options.errorprone.check("PreferJavaTimeOverload", CheckSeverity.OFF) + options.errorprone.check("JavaUtilDate", CheckSeverity.OFF) // The warning fails to provide a source location options.errorprone.check("MissingSummary", CheckSeverity.OFF) } @@ -323,6 +320,7 @@ subprojects { options.errorprone.check("JdkObsolete", CheckSeverity.OFF) options.errorprone.check("UnnecessaryAnonymousClass", CheckSeverity.OFF) options.errorprone.check("PreferJavaTimeOverload", CheckSeverity.OFF) + options.errorprone.check("JavaUtilDate", CheckSeverity.OFF) } plugins.withId("ru.vyarus.animalsniffer") { diff --git a/context/src/main/java/io/grpc/Context.java b/context/src/main/java/io/grpc/Context.java index ad47ca9cb4..41d3a5c94a 100644 --- a/context/src/main/java/io/grpc/Context.java +++ b/context/src/main/java/io/grpc/Context.java @@ -1042,7 +1042,7 @@ public class Context { * Implements {@link io.grpc.Context#current}. * *

Caution: {@link Context} interprets a return value of {@code null} to mean the same - * thing as {@code Context{@link #ROOT}}. + * thing as {@link Context#ROOT}. * *

See also {@link #doAttach(Context)}. * diff --git a/core/src/main/java/io/grpc/internal/AbstractServerImplBuilder.java b/core/src/main/java/io/grpc/internal/AbstractServerImplBuilder.java index d20c60cd44..c1268cfdb0 100644 --- a/core/src/main/java/io/grpc/internal/AbstractServerImplBuilder.java +++ b/core/src/main/java/io/grpc/internal/AbstractServerImplBuilder.java @@ -17,6 +17,7 @@ package io.grpc.internal; import com.google.common.base.MoreObjects; +import com.google.errorprone.annotations.DoNotCall; import io.grpc.BinaryLog; import io.grpc.BindableService; import io.grpc.CompressorRegistry; @@ -53,6 +54,7 @@ public abstract class AbstractServerImplBuilder /** * This method serves to force sub classes to "hide" this static factory. */ + @DoNotCall("Unsupported") public static ServerBuilder forPort(int port) { throw new UnsupportedOperationException("Subclass failed to hide static factory"); } diff --git a/core/src/main/java/io/grpc/internal/JndiResourceResolverFactory.java b/core/src/main/java/io/grpc/internal/JndiResourceResolverFactory.java index a13c9fe466..22e08de9cf 100644 --- a/core/src/main/java/io/grpc/internal/JndiResourceResolverFactory.java +++ b/core/src/main/java/io/grpc/internal/JndiResourceResolverFactory.java @@ -140,7 +140,7 @@ final class JndiResourceResolverFactory implements DnsNameResolver.ResourceResol Level level = Level.WARNING; for (String rawSrv : rawSrvRecords) { try { - String[] parts = whitespace.split(rawSrv); + String[] parts = whitespace.split(rawSrv, 5); Verify.verify(parts.length == 4, "Bad SRV Record: %s", rawSrv); // SRV requires the host name to be absolute if (!parts[3].endsWith(".")) { diff --git a/interop-testing/src/test/java/io/grpc/testing/integration/NettyFlowControlTest.java b/interop-testing/src/test/java/io/grpc/testing/integration/NettyFlowControlTest.java index 4d7c8bdbca..c86bd8070a 100644 --- a/interop-testing/src/test/java/io/grpc/testing/integration/NettyFlowControlTest.java +++ b/interop-testing/src/test/java/io/grpc/testing/integration/NettyFlowControlTest.java @@ -39,17 +39,13 @@ import io.grpc.testing.integration.Messages.StreamingOutputCallResponse; import io.netty.channel.ChannelHandler; import io.netty.handler.codec.http2.Http2Stream; import io.netty.util.AsciiString; -import io.netty.util.concurrent.DefaultThreadFactory; import java.io.IOException; import java.net.InetSocketAddress; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Executors; -import java.util.concurrent.LinkedBlockingQueue; -import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; import org.junit.After; -import org.junit.AfterClass; import org.junit.Before; import org.junit.Ignore; import org.junit.Test; @@ -80,16 +76,6 @@ public class NettyFlowControlTest { private int proxyPort; private int serverPort; - private static final ThreadPoolExecutor executor = - new ThreadPoolExecutor(1, 10, 1, TimeUnit.SECONDS, new LinkedBlockingQueue(), - new DefaultThreadFactory("flowcontrol-test-pool", true)); - - - @AfterClass - public static void shutDownTests() { - executor.shutdown(); - } - @Before public void initTest() { startServer(REGULAR_WINDOW); diff --git a/interop-testing/src/test/java/io/grpc/testing/integration/TrafficControlProxy.java b/interop-testing/src/test/java/io/grpc/testing/integration/TrafficControlProxy.java index 19f22cb03f..a93d51628e 100644 --- a/interop-testing/src/test/java/io/grpc/testing/integration/TrafficControlProxy.java +++ b/interop-testing/src/test/java/io/grpc/testing/integration/TrafficControlProxy.java @@ -51,7 +51,7 @@ public final class TrafficControlProxy { private Socket serverSock; private Socket clientSock; private final ThreadPoolExecutor executor = - new ThreadPoolExecutor(5, 10, 1, TimeUnit.SECONDS, new LinkedBlockingQueue(), + new ThreadPoolExecutor(5, 5, 1, TimeUnit.SECONDS, new LinkedBlockingQueue(), new DefaultThreadFactory("proxy-pool", true)); /** diff --git a/netty/src/main/java/io/grpc/netty/GrpcHttp2ConnectionHandler.java b/netty/src/main/java/io/grpc/netty/GrpcHttp2ConnectionHandler.java index 459c313ede..25f4f9232c 100644 --- a/netty/src/main/java/io/grpc/netty/GrpcHttp2ConnectionHandler.java +++ b/netty/src/main/java/io/grpc/netty/GrpcHttp2ConnectionHandler.java @@ -58,6 +58,7 @@ public abstract class GrpcHttp2ConnectionHandler extends Http2ConnectionHandler * @deprecated Use the two argument method instead. */ @Deprecated + @SuppressWarnings("InlineMeSuggester") // the caller should consider providing securityInfo public void handleProtocolNegotiationCompleted(Attributes attrs) { handleProtocolNegotiationCompleted(attrs, /*securityInfo=*/ null); } diff --git a/netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java b/netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java index 17801afa38..ca029fe7cf 100644 --- a/netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java +++ b/netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java @@ -24,6 +24,7 @@ import static io.grpc.internal.GrpcUtil.KEEPALIVE_TIME_NANOS_DISABLED; import com.google.common.annotations.VisibleForTesting; import com.google.errorprone.annotations.CanIgnoreReturnValue; +import com.google.errorprone.annotations.InlineMe; import io.grpc.Attributes; import io.grpc.CallCredentials; import io.grpc.ChannelCredentials; @@ -393,6 +394,7 @@ public final class NettyChannelBuilder extends * @deprecated Use {@link #maxInboundMetadataSize} instead */ @Deprecated + @InlineMe(replacement = "this.maxInboundMetadataSize(maxHeaderListSize)") public NettyChannelBuilder maxHeaderListSize(int maxHeaderListSize) { return maxInboundMetadataSize(maxHeaderListSize); } diff --git a/netty/src/main/java/io/grpc/netty/NettyServerBuilder.java b/netty/src/main/java/io/grpc/netty/NettyServerBuilder.java index 9b67cd21f1..ff5553eb11 100644 --- a/netty/src/main/java/io/grpc/netty/NettyServerBuilder.java +++ b/netty/src/main/java/io/grpc/netty/NettyServerBuilder.java @@ -26,6 +26,7 @@ import static io.grpc.internal.GrpcUtil.SERVER_KEEPALIVE_TIME_NANOS_DISABLED; import com.google.common.annotations.VisibleForTesting; import com.google.errorprone.annotations.CanIgnoreReturnValue; +import com.google.errorprone.annotations.InlineMe; import io.grpc.Attributes; import io.grpc.ExperimentalApi; import io.grpc.Internal; @@ -437,6 +438,7 @@ public final class NettyServerBuilder extends AbstractServerImplBuilder()); EnvoyServerProtoData.FilterChain filterChain = new EnvoyServerProtoData.FilterChain( "filter-chain-foo", createMatch(), httpConnectionManager, createTls(), - tlsContextManager); + mock(TlsContextManager.class)); xdsClient.deliverLdsUpdate(Collections.singletonList(filterChain), null); start.get(5000, TimeUnit.MILLISECONDS); assertThat(ldsWatched).isEqualTo("grpc/server?udpa.resource.listening_address=0.0.0.0:1"); @@ -1190,7 +1188,7 @@ public class XdsServerWrapperTest { private static FilterChain createFilterChain(String name, HttpConnectionManager hcm) { return new EnvoyServerProtoData.FilterChain(name, createMatch(), - hcm, createTls(), tlsContextManager); + hcm, createTls(), mock(TlsContextManager.class)); } private static VirtualHost createVirtualHost(String name) {