Update ErrorProne to 2.1.3 and fix failures

The fixes could have subtle side-effects, but I did take care when making them.

benchmarks/src/main/java/io/grpc/benchmarks/qps/OpenLoopClient.java

@@ -137,7 +137,7 @@ public class OpenLoopClient {
       stub = BenchmarkServiceGrpc.newStub(checkNotNull(channel, "channel"));
       this.request = checkNotNull(request, "request");
       this.targetQps = targetQps;
-      numRpcs = targetQps * duration;
+      numRpcs = (long) targetQps * duration;
       rnd = new Random();
     }
 

build.gradle

@@ -35,7 +35,7 @@ subprojects {
       dependencies {
         // The ErrorProne plugin defaults to the latest, which would break our
         // build if error prone releases a new version with a new check
-        errorprone 'com.google.errorprone:error_prone_core:2.0.21'
+        errorprone 'com.google.errorprone:error_prone_core:2.1.3'
         apt 'com.google.guava:guava-beta-checker:1.0'
       }
     } else {
@@ -76,7 +76,8 @@ subprojects {
 
     compileTestJava {
       // serialVersionUID is basically guaranteed to be useless in our tests
-      options.compilerArgs += ["-Xlint:-serial"]
+      // LinkedList doesn't hurt much in tests and has lots of usages
+      options.compilerArgs += ["-Xlint:-serial", "-Xep:JdkObsolete:OFF"]
     }
 
     jar.manifest {

core/src/main/java/io/grpc/Metadata.java

@@ -24,13 +24,13 @@ import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.io.BaseEncoding;
 import java.nio.ByteBuffer;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.BitSet;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -347,7 +347,7 @@ public final class Metadata {
     List<T> ret = null;
     for (; readIdx < size; readIdx++) {
       if (bytesEqual(key.asciiName(), name(readIdx))) {
-        ret = ret != null ? ret : new LinkedList<T>();
+        ret = ret != null ? ret : new ArrayList<T>();
         ret.add(key.parseBytes(value(readIdx)));
         continue;
       }

core/src/main/java/io/grpc/internal/ApplicationThreadDeframer.java

@@ -20,7 +20,7 @@ import static com.google.common.base.Preconditions.checkNotNull;
 
 import io.grpc.Decompressor;
 import java.io.InputStream;
-import java.util.LinkedList;
+import java.util.ArrayDeque;
 import java.util.Queue;
 import javax.annotation.Nullable;
 
@@ -41,7 +41,7 @@ public class ApplicationThreadDeframer implements Deframer, MessageDeframer.List
   private final TransportExecutor transportExecutor;
 
   /** Queue for messages returned by the deframer when deframing in the application thread. */
-  private final Queue<InputStream> messageReadQueue = new LinkedList<InputStream>();
+  private final Queue<InputStream> messageReadQueue = new ArrayDeque<InputStream>();
 
   ApplicationThreadDeframer(
       MessageDeframer.Listener listener,

core/src/main/java/io/grpc/internal/ChannelExecutor.java

@@ -19,7 +19,8 @@ package io.grpc.internal;
 import static com.google.common.base.Preconditions.checkNotNull;
 
 import com.google.common.annotations.VisibleForTesting;
-import java.util.LinkedList;
+import java.util.ArrayDeque;
+import java.util.Queue;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import javax.annotation.concurrent.GuardedBy;
@@ -39,7 +40,7 @@ final class ChannelExecutor {
   private final Object lock = new Object();
 
   @GuardedBy("lock")
-  private final LinkedList<Runnable> queue = new LinkedList<Runnable>();
+  private final Queue<Runnable> queue = new ArrayDeque<Runnable>();
   @GuardedBy("lock")
   private boolean draining;
 

core/src/test/java/io/grpc/MethodDescriptorTest.java

@@ -21,7 +21,9 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 import io.grpc.MethodDescriptor.MethodType;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
@@ -30,6 +32,9 @@ import org.junit.runners.JUnit4;
  */
 @RunWith(JUnit4.class)
 public class MethodDescriptorTest {
+  @Rule
+  public final ExpectedException thrown = ExpectedException.none();
+
   @Test
   public void createMethodDescriptor() {
     @SuppressWarnings("deprecation") // MethodDescriptor.create
@@ -80,7 +85,7 @@ public class MethodDescriptorTest {
     assertEquals("package.service/method", newDescriptor.getFullMethodName());
   }
 
-  @Test(expected = IllegalArgumentException.class)
+  @Test
   public void safeAndNonUnary() {
     MethodDescriptor<String, String> descriptor = MethodDescriptor.<String, String>newBuilder()
         .setType(MethodType.SERVER_STREAMING)
@@ -89,10 +94,8 @@ public class MethodDescriptorTest {
         .setResponseMarshaller(new StringMarshaller())
         .build();
 
-
+    thrown.expect(IllegalArgumentException.class);
-    MethodDescriptor<String, String> discard = descriptor.toBuilder().setSafe(true).build();
+    MethodDescriptor<String, String> unused = descriptor.toBuilder().setSafe(true).build();
-    // Never reached
-    assert discard == null;
   }
 
   @Test

interop-testing/src/main/java/io/grpc/testing/integration/TestServiceImpl.java

@@ -39,9 +39,9 @@ import io.grpc.testing.integration.Messages.StreamingOutputCallRequest;
 import io.grpc.testing.integration.Messages.StreamingOutputCallResponse;
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.ArrayDeque;
 import java.util.Arrays;
 import java.util.HashSet;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Queue;
 import java.util.Random;
@@ -209,7 +209,7 @@ public class TestServiceImpl extends TestServiceGrpc.TestServiceImplBase {
   @Override
   public StreamObserver<Messages.StreamingOutputCallRequest> halfDuplexCall(
       final StreamObserver<Messages.StreamingOutputCallResponse> responseObserver) {
-    final Queue<Chunk> chunks = new LinkedList<Chunk>();
+    final Queue<Chunk> chunks = new ArrayDeque<Chunk>();
     return new StreamObserver<StreamingOutputCallRequest>() {
       @Override
       public void onNext(StreamingOutputCallRequest request) {
@@ -371,7 +371,7 @@ public class TestServiceImpl extends TestServiceGrpc.TestServiceImplBase {
    * Breaks down the request and creates a queue of response chunks for the given request.
    */
   public Queue<Chunk> toChunkQueue(StreamingOutputCallRequest request) {
-    Queue<Chunk> chunkQueue = new LinkedList<Chunk>();
+    Queue<Chunk> chunkQueue = new ArrayDeque<Chunk>();
     int offset = 0;
     boolean compressable = compressableResponse(request.getResponseType());
     for (ResponseParameters params : request.getResponseParametersList()) {

netty/src/test/java/io/grpc/netty/AbstractHttp2HeadersTest.java

@@ -50,8 +50,8 @@ public class AbstractHttp2HeadersTest {
       } catch (InvocationTargetException ex) {
         assertEquals("For method: " + method,
             UnsupportedOperationException.class, ex.getCause().getClass());
-      } catch (Throwable t) {
+      } catch (Exception ex) {
-        throw new RuntimeException("Failure with method: " + method, t);
+        throw new AssertionError("Failure with method: " + method, ex);
       }
     }
   }

okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java

@@ -165,6 +165,7 @@ class OkHttpClientTransport implements ConnectionClientTransport {
   private Socket socket;
   @GuardedBy("lock")
   private int maxConcurrentStreams = 0;
+  @SuppressWarnings("JdkObsolete") // Usage is bursty; want low memory usage when empty
   @GuardedBy("lock")
   private LinkedList<OkHttpClientStream> pendingStreams = new LinkedList<OkHttpClientStream>();
   private final ConnectionSpec connectionSpec;

services/src/main/java/io/grpc/protobuf/services/ProtoReflectionService.java

@@ -42,10 +42,10 @@ import io.grpc.reflection.v1alpha.ServerReflectionResponse;
 import io.grpc.reflection.v1alpha.ServiceResponse;
 import io.grpc.stub.ServerCallStreamObserver;
 import io.grpc.stub.StreamObserver;
+import java.util.ArrayDeque;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Queue;
@@ -300,7 +300,7 @@ public final class ProtoReflectionService extends ServerReflectionGrpc.ServerRef
       FileDescriptorResponse.Builder fdRBuilder = FileDescriptorResponse.newBuilder();
 
       Set<String> seenFiles = new HashSet<String>();
-      Queue<FileDescriptor> frontier = new LinkedList<FileDescriptor>();
+      Queue<FileDescriptor> frontier = new ArrayDeque<FileDescriptor>();
       seenFiles.add(fd.getName());
       frontier.add(fd);
       while (!frontier.isEmpty()) {
@@ -408,7 +408,7 @@ public final class ProtoReflectionService extends ServerReflectionGrpc.ServerRef
         new HashMap<String, Map<Integer, FileDescriptor>>();
 
     FileDescriptorIndex(List<ServerServiceDefinition> services) {
-      Queue<FileDescriptor> fileDescriptorsToProcess = new LinkedList<FileDescriptor>();
+      Queue<FileDescriptor> fileDescriptorsToProcess = new ArrayDeque<FileDescriptor>();
       Set<String> seenFiles = new HashSet<String>();
       for (ServerServiceDefinition service : services) {
         io.grpc.ServiceDescriptor serviceDescriptor = service.getServiceDescriptor();

stub/src/test/java/io/grpc/stub/ClientCallsTest.java

@@ -557,7 +557,7 @@ public class ClientCallsTest {
     try {
       iter.next();
       fail("Should fail");
-    } catch (Throwable e) {
+    } catch (Exception e) {
       Status status = Status.fromThrowable(e);
       assertEquals(Status.INTERNAL, status);
       Metadata metadata = Status.trailersFromThrowable(e);