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.
This commit is contained in:
Eric Anderson 2022-01-12 12:06:27 -08:00 committed by GitHub
parent 7c4fe69dfd
commit 58a7ace6ac
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
18 changed files with 41 additions and 45 deletions

View File

@ -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.
*
* <p>Note that we provide implementations of this interface that provide integrity only and

View File

@ -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;

View File

@ -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'),

View File

@ -46,11 +46,13 @@ import javax.annotation.concurrent.Immutable;
@Immutable
public final class Attributes {
private final Map<Key<?>, Object> data;
private final IdentityHashMap<Key<?>, Object> data;
public static final Attributes EMPTY = new Attributes(Collections.<Key<?>, Object>emptyMap());
private static final IdentityHashMap<Key<?>, Object> EMPTY_MAP =
new IdentityHashMap<Key<?>, Object>();
public static final Attributes EMPTY = new Attributes(EMPTY_MAP);
private Attributes(Map<Key<?>, Object> data) {
private Attributes(IdentityHashMap<Key<?>, 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<Key<?>, Object> newdata;
private IdentityHashMap<Key<?>, Object> newdata;
private Builder(Attributes base) {
assert base != null;
this.base = base;
}
private Map<Key<?>, Object> data(int size) {
private IdentityHashMap<Key<?>, 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 <T> Builder discard(Key<T> key) {
if (base.data.containsKey(key)) {
Map<Key<?>, Object> newBaseData = new IdentityHashMap<>(base.data);
IdentityHashMap<Key<?>, Object> newBaseData = new IdentityHashMap<>(base.data);
newBaseData.remove(key);
base = new Attributes(newBaseData);
}

View File

@ -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<EquivalentAddressGroup> servers, @ResolutionResultAttr Attributes attributes) {
// TODO(jihuncho) need to promote Listener2 if we want to use ConfigOrError

View File

@ -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,21 +239,17 @@ 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) {
tasks.withType(JavaCompile) {
options.errorprone.enabled = false
}
}
}
}
plugins.withId("java") {
sourceCompatibility = 1.7
@ -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") {

View File

@ -1042,7 +1042,7 @@ public class Context {
* Implements {@link io.grpc.Context#current}.
*
* <p>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}.
*
* <p>See also {@link #doAttach(Context)}.
*

View File

@ -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");
}

View File

@ -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(".")) {

View File

@ -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<Runnable>(),
new DefaultThreadFactory("flowcontrol-test-pool", true));
@AfterClass
public static void shutDownTests() {
executor.shutdown();
}
@Before
public void initTest() {
startServer(REGULAR_WINDOW);

View File

@ -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<Runnable>(),
new ThreadPoolExecutor(5, 5, 1, TimeUnit.SECONDS, new LinkedBlockingQueue<Runnable>(),
new DefaultThreadFactory("proxy-pool", true));
/**

View File

@ -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);
}

View File

@ -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);
}

View File

@ -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<NettySer
* future release.
*/
@Deprecated
@InlineMe(replacement = "this.maxInboundMessageSize(maxMessageSize)")
public NettyServerBuilder maxMessageSize(int maxMessageSize) {
return maxInboundMessageSize(maxMessageSize);
}
@ -458,6 +460,7 @@ public final class NettyServerBuilder extends AbstractServerImplBuilder<NettySer
* @deprecated Use {@link #maxInboundMetadataSize} instead
*/
@Deprecated
@InlineMe(replacement = "this.maxInboundMetadataSize(maxHeaderListSize)")
public NettyServerBuilder maxHeaderListSize(int maxHeaderListSize) {
return maxInboundMetadataSize(maxHeaderListSize);
}

View File

@ -42,6 +42,7 @@ class BinaryLogProviderImpl extends BinaryLogProvider {
* Deprecated and will be removed in a future version of gRPC.
*/
@Deprecated
@SuppressWarnings("InlineMeSuggester") // Only called internally; don't care
public BinaryLogProviderImpl(BinaryLogSink sink) throws IOException {
this(sink, System.getenv("GRPC_BINARY_LOG_CONFIG"));
}

View File

@ -391,7 +391,6 @@ public final class ClientCalls {
this.onReadyHandler = onReadyHandler;
}
@Deprecated
@Override
public void disableAutoInboundFlowControl() {
disableAutoRequestWithInitial(1);

View File

@ -422,7 +422,6 @@ public final class ServerCalls {
this.onCancelHandler = onCancelHandler;
}
@Deprecated
@Override
public void disableAutoInboundFlowControl() {
disableAutoRequest();

View File

@ -99,8 +99,6 @@ public class XdsServerWrapperTest {
@Mock
private Server mockServer;
@Mock
private static TlsContextManager tlsContextManager;
@Mock
private XdsServingStatusListener listener;
private FilterChainSelectorManager selectorManager = new FilterChainSelectorManager();
@ -441,7 +439,7 @@ public class XdsServerWrapperTest {
0L, Collections.singletonList(virtualHost), new ArrayList<NamedFilterConfig>());
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) {