Enable NullAway for :core (#5820)

Motivation:

It would be nice if we can fail our build if there is any potential
`null` dereference, so our users have much less chance of getting
`NullPointerException`.

Modifications:

- Updated the build so that NullAway plugin is enabled for `:core`
  - Note that the root `build.gradle` was modified so that we can
    enable NullAway for other projects in the future.
  - Other caveats:
    - NullAway is disabled for MR-JAR sources because it seems to make
      the build fail with unrelated errors (e.g. no symbol)
- Added `assert` sentences and `@Nullable` annotations wherever
  applicable
- Fixed a few minor potential `null` dereferences

Result:

- We're more confident about our `@Nullable` usages in our API.
- Theoretically, there should be no chance of `NullPointerException` at
  least in the `:core`.
  - However, note that some `NullPointerException`s might have become
    `AssertionError`s, since we added a bunch of assertions in this PR,
    so please review carefully :-)
- Partially resolves #1680 and #5184

benchmarks/jmh/src/jmh/java/com/linecorp/armeria/server/RoutersBenchmark.java

@@ -78,7 +78,7 @@ private static ServiceConfig newServiceConfig(Route route) {
         final Path multipartUploadsLocation = Flags.defaultMultipartUploadsLocation();
         final ServiceErrorHandler serviceErrorHandler = ServerErrorHandler.ofDefault().asServiceErrorHandler();
         return new ServiceConfig(route, route,
-                                 SERVICE, defaultLogName, defaultServiceName, defaultServiceNaming, 0, 0,
+                                 SERVICE, defaultServiceName, defaultServiceNaming, defaultLogName, 0, 0,
                                  false, AccessLogWriter.disabled(), CommonPools.blockingTaskExecutor(),
                                  SuccessFunction.always(), 0, multipartUploadsLocation,
                                  MultipartRemovalStrategy.ON_RESPONSE_COMPLETION,

build.gradle

@@ -23,6 +23,7 @@ plugins {
     alias libs.plugins.kotlin apply false
     alias libs.plugins.ktlint apply false
     alias libs.plugins.errorprone apply false
+    alias libs.plugins.nullaway apply false
 }
 
 allprojects {
@@ -171,13 +172,34 @@ configure(projectsWithFlags('java')) {
     // Error Prone compiler
     if (!rootProject.hasProperty('noLint')) {
         apply plugin: 'net.ltgt.errorprone'
+        apply plugin: 'net.ltgt.nullaway'
 
         dependencies {
             errorprone libs.errorprone.core
+            errorprone libs.nullaway
+        }
+
+        nullaway {
+            annotatedPackages.add("com.linecorp.armeria")
         }
 
         tasks.withType(JavaCompile) {
             options.errorprone.excludedPaths = '.*/gen-src/.*'
+            options.errorprone.nullaway {
+                if (name.toLowerCase().contains("test")) {
+                    // Disable NullAway for tests for now.
+                    disable()
+                } else if (name.matches(/compileJava[0-9]+.*/)) {
+                    // Disable MR-JAR classes which seem to confuse NullAway and break the build.
+                    disable()
+                } else if (project != project(':core')) {
+                    // TODO(trustin): Enable NullAway for all projects once we fix all violations.
+                    warn()
+                } else {
+                    error()
+                    assertsEnabled = true
+                }
+            }
         }
     }
 

core/src/main/java/com/linecorp/armeria/client/AbstractHttpRequestHandler.java

@@ -216,6 +216,7 @@ RequestHeaders mergedRequestHeaders(RequestHeaders headers) {
      * {@link Channel#flush()} when each write unit is done.
      */
     final void writeHeaders(RequestHeaders headers, boolean needs100Continue) {
+        assert session != null;
         final SessionProtocol protocol = session.protocol();
         assert protocol != null;
         if (needs100Continue) {

core/src/main/java/com/linecorp/armeria/client/Client.java

@@ -24,6 +24,7 @@
 import com.linecorp.armeria.common.Response;
 import com.linecorp.armeria.common.RpcRequest;
 import com.linecorp.armeria.common.RpcResponse;
+import com.linecorp.armeria.common.annotation.Nullable;
 import com.linecorp.armeria.common.util.Unwrappable;
 
 /**
@@ -71,6 +72,7 @@ public interface Client<I extends Request, O extends Response> extends Unwrappab
      * @see ClientFactory#unwrap(Object, Class)
      * @see Unwrappable
      */
+    @Nullable
     @Override
     default <T> T as(Class<T> type) {
         requireNonNull(type, "type");

core/src/main/java/com/linecorp/armeria/client/ClientRequestContextWrapper.java

@@ -52,26 +52,31 @@ public ClientRequestContext newDerivedContext(RequestId id, @Nullable HttpReques
         return unwrap().newDerivedContext(id, req, rpcReq, endpoint);
     }
 
+    @Nullable
     @Override
     public EndpointGroup endpointGroup() {
         return unwrap().endpointGroup();
     }
 
+    @Nullable
     @Override
     public Endpoint endpoint() {
         return unwrap().endpoint();
     }
 
+    @Nullable
     @Override
     public String fragment() {
         return unwrap().fragment();
     }
 
+    @Nullable
     @Override
     public String authority() {
         return unwrap().authority();
     }
 
+    @Nullable
     @Override
     public String host() {
         return unwrap().host();

core/src/main/java/com/linecorp/armeria/client/DecoratingClientFactory.java

@@ -108,11 +108,13 @@ public Object newClient(ClientBuilderParams params) {
         return unwrap().newClient(params);
     }
 
+    @Nullable
     @Override
     public <T> ClientBuilderParams clientBuilderParams(T client) {
         return unwrap().clientBuilderParams(client);
     }
 
+    @Nullable
     @Override
     public <T> T unwrap(Object client, Class<T> type) {
         return unwrap().unwrap(client, type);

core/src/main/java/com/linecorp/armeria/client/DefaultClientFactory.java

@@ -184,6 +184,7 @@ public Object newClient(ClientBuilderParams params) {
                 "No ClientFactory for scheme: " + scheme + " matched clientType: " + clientType);
     }
 
+    @Nullable
     @Override
     public <T> T unwrap(Object client, Class<T> type) {
         final T params = ClientFactory.super.unwrap(client, type);

core/src/main/java/com/linecorp/armeria/client/DefaultDnsCache.java

@@ -147,6 +147,7 @@ public void cache(DnsQuestion question, UnknownHostException cause) {
         }
     }
 
+    @Nullable
     @Override
     public List<DnsRecord> get(DnsQuestion question) throws UnknownHostException {
         requireNonNull(question, "question");

core/src/main/java/com/linecorp/armeria/client/DefaultRequestOptions.java

@@ -68,6 +68,7 @@ public long maxResponseLength() {
         return maxResponseLength;
     }
 
+    @Nullable
     @Override
     public Long requestAutoAbortDelayMillis() {
         return requestAutoAbortDelayMillis;
@@ -78,6 +79,7 @@ public Map<AttributeKey<?>, Object> attrs() {
         return attributeMap;
     }
 
+    @Nullable
     @Override
     public ExchangeType exchangeType() {
         return exchangeType;

core/src/main/java/com/linecorp/armeria/client/Endpoint.java

@@ -37,6 +37,8 @@
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.function.Consumer;
 
+import javax.annotation.Nonnull;
+
 import com.github.benmanes.caffeine.cache.Cache;
 import com.github.benmanes.caffeine.cache.Caffeine;
 import com.google.common.annotations.VisibleForTesting;
@@ -323,6 +325,7 @@ public EndpointSelectionStrategy selectionStrategy() {
         return EndpointSelectionStrategy.weightedRoundRobin();
     }
 
+    @Nonnull
     @Override
     public Endpoint selectNow(ClientRequestContext ctx) {
         return this;

core/src/main/java/com/linecorp/armeria/client/FutureTransformingRequestPreparation.java

@@ -88,6 +88,7 @@ public CompletableFuture<T> execute() {
 
         return response.exceptionally(cause -> {
             cause = Exceptions.peel(cause);
+            assert errorHandler != null;
             final Object maybeRecovered = errorHandler.apply(cause);
             if (maybeRecovered instanceof Throwable) {
                 // The cause was translated.

core/src/main/java/com/linecorp/armeria/client/HttpChannelPool.java

@@ -110,9 +110,12 @@ final class HttpChannelPool implements AsyncCloseable {
                                           SessionProtocol.H2, SessionProtocol.H2C));
         pendingAcquisitions = newEnumMap(httpAndHttpsValues());
         allChannels = new IdentityHashMap<>();
-        connectTimeoutMillis = (Integer) clientFactory.options()
-                .channelOptions()
-                .get(ChannelOption.CONNECT_TIMEOUT_MILLIS);
+        final Integer connectTimeoutMillisBoxed =
+                (Integer) clientFactory.options()
+                                       .channelOptions()
+                                       .get(ChannelOption.CONNECT_TIMEOUT_MILLIS);
+        assert connectTimeoutMillisBoxed != null;
+        connectTimeoutMillis = connectTimeoutMillisBoxed;
         bootstraps = new Bootstraps(clientFactory, eventLoop, sslCtxHttp1Or2, sslCtxHttp1Only);
     }
 
@@ -828,6 +831,7 @@ private void handlePiggyback(SessionProtocol desiredProtocol,
 
             switch (result) {
                 case SUCCESS:
+                    assert pch != null;
                     timingsBuilder.pendingAcquisitionEnd();
                     childPromise.complete(pch);
                     break;

core/src/main/java/com/linecorp/armeria/client/HttpClientPipelineConfigurator.java

@@ -132,6 +132,14 @@ final class HttpClientPipelineConfigurator extends ChannelDuplexHandler {
                           .responseTimeoutMillis(0)
                           .maxResponseLength(UPGRADE_RESPONSE_MAX_LENGTH).build();
 
+    private static final RequestTarget REQ_TARGET_ASTERISK;
+
+    static {
+        final RequestTarget asterisk = RequestTarget.forClient("*");
+        assert asterisk != null;
+        REQ_TARGET_ASTERISK = asterisk;
+    }
+
     private enum HttpPreference {
         HTTP1_REQUIRED,
         HTTP2_PREFERRED,
@@ -215,7 +223,7 @@ public void connect(ChannelHandlerContext ctx, SocketAddress remoteAddress, Sock
      * See <a href="https://http2.github.io/http2-spec/#discover-https">HTTP/2 specification</a>.
      */
     private void configureAsHttps(Channel ch, SocketAddress remoteAddr) {
-        assert isHttps();
+        assert sslCtx != null;
 
         final ChannelPipeline p = ch.pipeline();
         final SSLEngine sslEngine;
@@ -564,7 +572,7 @@ public void onComplete() {}
             final DefaultClientRequestContext reqCtx = new DefaultClientRequestContext(
                     ctx.channel().eventLoop(), Flags.meterRegistry(), H1C, RequestId.random(),
                     com.linecorp.armeria.common.HttpMethod.OPTIONS,
-                    RequestTarget.forClient("*"), ClientOptions.of(),
+                    REQ_TARGET_ASTERISK, ClientOptions.of(),
                     HttpRequest.of(com.linecorp.armeria.common.HttpMethod.OPTIONS, "*"),
                     null, REQUEST_OPTIONS_FOR_UPGRADE_REQUEST, CancellationScheduler.noop(),
                     System.nanoTime(), SystemInfo.currentTimeMicros());

core/src/main/java/com/linecorp/armeria/client/HttpResponseWrapper.java

@@ -284,7 +284,9 @@ private void cancelTimeoutOrLog(@Nullable Throwable cause, boolean cancel) {
         }
 
         final StringBuilder logMsg = new StringBuilder("Unexpected exception while closing a request");
-        final String authority = ctx.request().authority();
+        final HttpRequest request = ctx.request();
+        assert request != null;
+        final String authority = request.authority();
         if (authority != null) {
             logMsg.append(" to ").append(authority);
         }
@@ -311,7 +313,9 @@ public boolean canSchedule() {
             @Override
             public void run(Throwable cause) {
                 delegate.close(cause);
-                ctx.request().abort(cause);
+                final HttpRequest request = ctx.request();
+                assert request != null;
+                request.abort(cause);
                 ctx.logBuilder().endResponse(cause);
             }
         };

core/src/main/java/com/linecorp/armeria/client/HttpSessionHandler.java

@@ -166,6 +166,7 @@ public SerializationFormat serializationFormat() {
         return serializationFormat;
     }
 
+    @Nullable
     @Override
     public SessionProtocol protocol() {
         return protocol;
@@ -223,8 +224,8 @@ public void invoke(PooledChannel pooledChannel, ClientRequestContext ctx,
         final long writeTimeoutMillis = ctx.writeTimeoutMillis();
 
         assert protocol != null;
-        assert responseDecoder != null;
         assert requestEncoder != null;
+        assert responseDecoder != null;
         if (!protocol.isMultiplex() && !serializationFormat.requiresNewConnection(protocol)) {
             // When HTTP/1.1 is used and the serialization format does not require
             // a new connection (w.g. WebSocket):
@@ -236,6 +237,7 @@ public void invoke(PooledChannel pooledChannel, ClientRequestContext ctx,
                     useHttp1Pipelining ? req.whenComplete()
                                        : CompletableFuture.allOf(req.whenComplete(), res.whenComplete());
             completionFuture.handle((ret, cause) -> {
+                assert responseDecoder != null;
                 if (isAcquirable(responseDecoder.keepAliveHandler())) {
                     pooledChannel.release();
                 }

core/src/main/java/com/linecorp/armeria/client/NoopHostFileEntriesResolver.java

@@ -18,12 +18,15 @@
 
 import java.net.InetAddress;
 
+import com.linecorp.armeria.common.annotation.Nullable;
+
 import io.netty.resolver.HostsFileEntriesResolver;
 import io.netty.resolver.ResolvedAddressTypes;
 
 enum NoopHostFileEntriesResolver implements HostsFileEntriesResolver {
     INSTANCE;
 
+    @Nullable
     @Override
     public InetAddress address(String inetHost, ResolvedAddressTypes resolvedAddressTypes) {
         return null;

core/src/main/java/com/linecorp/armeria/client/RedirectingClient.java

@@ -561,15 +561,21 @@ void validateRedirects(RequestTarget nextReqTarget, HttpMethod nextMethod, int m
                 redirectSignatures = new LinkedHashSet<>();
 
                 final String originalProtocol = ctx.sessionProtocol().isTls() ? "https" : "http";
+                final String originalAuthority = ctx.authority();
+                assert originalAuthority != null;
                 final RedirectSignature originalSignature = new RedirectSignature(originalProtocol,
-                                                                                  ctx.authority(),
+                                                                                  originalAuthority,
                                                                                   request.headers().path(),
                                                                                   request.method());
                 redirectSignatures.add(originalSignature);
             }
 
-            final RedirectSignature signature = new RedirectSignature(nextReqTarget.scheme(),
-                                                                      nextReqTarget.authority(),
+            final String nextScheme = nextReqTarget.scheme();
+            final String nextAuthority = nextReqTarget.authority();
+            assert nextScheme != null;
+            assert nextAuthority != null;
+            final RedirectSignature signature = new RedirectSignature(nextScheme,
+                                                                      nextAuthority,
                                                                       nextReqTarget.pathAndQuery(),
                                                                       nextMethod);
             if (!redirectSignatures.add(signature)) {

core/src/main/java/com/linecorp/armeria/client/RefreshingAddressResolver.java

@@ -350,6 +350,7 @@ void refresh() {
             }
             refreshing = true;
 
+            assert address != null;
             final String hostname = address.getHostName();
             // 'sendQueries()' always successfully completes.
             sendQueries(questions, hostname, originalCreationTimeNanos).thenAccept(entry -> {
@@ -370,6 +371,7 @@ private Object maybeUpdate(String hostname, CacheEntry entry) {
 
             final Throwable cause = entry.cause();
             if (cause != null) {
+                assert autoRefreshBackoff != null;
                 final long nextDelayMillis = autoRefreshBackoff.nextDelayMillis(numAttemptsSoFar++);
 
                 if (nextDelayMillis < 0) {
@@ -401,6 +403,8 @@ boolean refreshable() {
                 return false;
             }
 
+            assert autoRefreshTimeoutFunction != null;
+
             if (autoRefreshTimeoutFunction == DEFAULT_AUTO_REFRESH_TIMEOUT_FUNCTION) {
                 return true;
             }

core/src/main/java/com/linecorp/armeria/client/UnprocessedRequestException.java

@@ -55,7 +55,9 @@ private UnprocessedRequestException(Throwable cause) {
     @Nonnull
     @Override
     public Throwable getCause() {
-        return super.getCause();
+        final Throwable cause = super.getCause();
+        assert cause != null;
+        return cause;
     }
 
     @Override

core/src/main/java/com/linecorp/armeria/client/WebSocketHttp1ClientChannelHandler.java

@@ -189,6 +189,7 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
                         return;
                     }
 
+                    assert res != null;
                     res.startResponse();
                     final ResponseHeaders responseHeaders = ArmeriaHttpUtil.toArmeria(nettyRes);
                     if (responseHeaders.status() == HttpStatus.SWITCHING_PROTOCOLS) {
@@ -215,6 +216,7 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
                     final ByteBuf data = (ByteBuf) msg;
                     final int dataLength = data.readableBytes();
                     if (dataLength > 0) {
+                        assert res != null;
                         final long maxContentLength = res.maxContentLength();
                         final long writtenBytes = res.writtenBytes();
                         if (maxContentLength > 0 && writtenBytes > maxContentLength - dataLength) {

core/src/main/java/com/linecorp/armeria/client/circuitbreaker/CircuitBreakerMetrics.java

@@ -51,10 +51,16 @@ final class CircuitBreakerMetrics {
         parent.gauge(idPrefix.name("state"), idPrefix.tags(), state, AtomicDouble::get);
 
         final String requests = idPrefix.name("requests");
-        parent.gauge(requests, idPrefix.tags("result", "success"),
-                     latestEventCount, lec -> lec.get().success());
-        parent.gauge(requests, idPrefix.tags("result", "failure"),
-                     latestEventCount, lec -> lec.get().failure());
+        parent.gauge(requests, idPrefix.tags("result", "success"), latestEventCount, lec -> {
+            final EventCount eventCount = lec.get();
+            assert eventCount != null;
+            return eventCount.success();
+        });
+        parent.gauge(requests, idPrefix.tags("result", "failure"), latestEventCount, lec -> {
+            final EventCount eventCount = lec.get();
+            assert eventCount != null;
+            return eventCount.failure();
+        });
 
         final String transitions = idPrefix.name("transitions");
         transitionsToClosed = parent.counter(transitions, idPrefix.tags("state", CLOSED.name()));