Enable NullAway for all non-test sources (#5830)

Motivation:

This is a follow-up PR for #5820

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:

- Enabled NullAway for all projects
- Added `assert` sentences and `@Nullable` annotations wherever
  applicable
- Fixed a few minor potential `null` dereferences

Result:

- Theoretically, there should be no chance of `NullPointerException`
  anymore.
  - However, note that some NullPointerExceptions might have become
    `AssertionError`s, since we added a bunch of assertions in this PR,
    so please review carefully :-)
- Closes #1680
- Closes #5184

annotation-processor/src/main/java/com/linecorp/armeria/server/annotation/processor/DocumentationProcessor.java

@@ -38,6 +38,7 @@
 import javax.lang.model.SourceVersion;
 import javax.lang.model.element.ElementKind;
 import javax.lang.model.element.ExecutableElement;
+import javax.lang.model.element.QualifiedNameable;
 import javax.lang.model.element.TypeElement;
 import javax.lang.model.element.VariableElement;
 import javax.tools.Diagnostic.Kind;
@@ -148,7 +149,9 @@ private void processAnnotation(TypeElement annotationElement, RoundEnvironment r
     }
 
     private void processMethod(ExecutableElement method) throws IOException {
-        final String className = ((TypeElement) method.getEnclosingElement()).getQualifiedName().toString();
+        final QualifiedNameable enclosingElement = (QualifiedNameable) method.getEnclosingElement();
+        assert enclosingElement != null;
+        final String className = enclosingElement.getQualifiedName().toString();
         final Properties properties = readProperties(className);
         final String docComment = processingEnv.getElementUtils().getDocComment(method);
         if (docComment == null || !docComment.contains("@param")) {

brave/brave6/src/main/java/com/linecorp/armeria/common/brave/RequestContextCurrentTraceContext.java

@@ -174,7 +174,7 @@ public Scope newScope(@Nullable TraceContext currentSpan) {
 
     @UnstableApi
     @Override
-    public Scope decorateScope(TraceContext context, Scope scope) {
+    public Scope decorateScope(@Nullable TraceContext context, Scope scope) {
         // If a `Scope` is decorated, `ScopeDecorator`s populate some contexts as such as MDC, which are stored
         // to a thread-local. The activated contexts will be removed when `decoratedScope.close()` is called.
         // If `Scope.NOOP` is specified, CurrentTraceContext.decorateScope() performs nothing.

brave/brave6/src/main/java/com/linecorp/armeria/internal/common/brave/TraceContextUtil.java

@@ -39,7 +39,7 @@ public static TraceContext traceContext(RequestContext ctx) {
         return ctx.attr(TRACE_CONTEXT_KEY);
     }
 
-    public static void setTraceContext(RequestContext ctx, TraceContext traceContext) {
+    public static void setTraceContext(RequestContext ctx, @Nullable TraceContext traceContext) {
         ctx.setAttr(TRACE_CONTEXT_KEY, traceContext);
     }
 

build.gradle

@@ -186,15 +186,12 @@ configure(projectsWithFlags('java')) {
         tasks.withType(JavaCompile) {
             options.errorprone.excludedPaths = '.*/gen-src/.*'
             options.errorprone.nullaway {
-                if (name.toLowerCase().contains("test")) {
-                    // Disable NullAway for tests for now.
+                if (name.contains("Test") || name.contains("Jmh")) {
+                    // Disable NullAway for tests and benchmarks 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

consul/src/main/java/com/linecorp/armeria/internal/consul/HealthClient.java

@@ -110,6 +110,16 @@ CompletableFuture<List<Endpoint>> healthyEndpoints(String serviceName, @Nullable
 
     @Nullable
     private static Endpoint toEndpoint(HealthService healthService) {
+        if (healthService.service == null) {
+            return null;
+        }
+        if (healthService.node == null) {
+            return null;
+        }
+
+        assert healthService.service != null;
+        assert healthService.node != null;
+
         if (!Strings.isNullOrEmpty(healthService.service.address)) {
             return Endpoint.of(healthService.service.address, healthService.service.port);
         } else if (!Strings.isNullOrEmpty(healthService.node.address)) {
@@ -122,59 +132,74 @@ private static Endpoint toEndpoint(HealthService healthService) {
     @JsonIgnoreProperties(ignoreUnknown = true)
     @JsonInclude(Include.NON_NULL)
     private static final class HealthService {
+        @Nullable
         @JsonProperty("Node")
         Node node;
 
+        @Nullable
         @JsonProperty("Service")
         Service service;
     }
 
     @JsonIgnoreProperties(ignoreUnknown = true)
     @JsonInclude(Include.NON_NULL)
     private static final class Node {
+        @Nullable
         @JsonProperty("ID")
         String id;
 
+        @Nullable
         @JsonProperty("Node")
         String node;
 
+        @Nullable
         @JsonProperty("Address")
         String address;
 
+        @Nullable
         @JsonProperty("Datacenter")
         String datacenter;
 
+        @Nullable
         @JsonProperty("TaggedAddresses")
         Object taggedAddresses;
 
+        @Nullable
         @JsonProperty("Meta")
         Map<String, Object> meta;
     }
 
     @JsonIgnoreProperties(ignoreUnknown = true)
     @JsonInclude(Include.NON_NULL)
     public static final class Service {
+        @Nullable
         @JsonProperty("ID")
         String id;
 
+        @Nullable
         @JsonProperty("Service")
         String service;
 
+        @Nullable
         @JsonProperty("Tags")
         String[] tags;
 
+        @Nullable
         @JsonProperty("Address")
         String address;
 
+        @Nullable
         @JsonProperty("TaggedAddresses")
         Object taggedAddresses;
 
+        @Nullable
         @JsonProperty("Meta")
         Map<String, Object> meta;
 
         @JsonProperty("Port")
         int port;
 
+        @Nullable
         @JsonProperty("Weights")
         Map<String, Object> weights;
     }

dropwizard2/src/main/java/com/linecorp/armeria/dropwizard/ArmeriaConfigurationUtil.java

@@ -127,7 +127,10 @@ static void configureServer(ServerBuilder serverBuilder, ArmeriaSettings setting
         }
 
         if (settings.getPorts().isEmpty()) {
-            serverBuilder.port(new ServerPort(DEFAULT_PORT.getPort(), DEFAULT_PORT.getProtocols()));
+            final int port = DEFAULT_PORT.getPort();
+            final List<SessionProtocol> protocols = DEFAULT_PORT.getProtocols();
+            assert protocols != null;
+            serverBuilder.port(new ServerPort(port, protocols));
         } else {
             configurePorts(serverBuilder, settings.getPorts());
         }
@@ -445,8 +448,9 @@ private static void configureAccessLog(ServerBuilder serverBuilder, AccessLog ac
         } else if ("combined".equals(accessLog.getType())) {
             serverBuilder.accessLogWriter(AccessLogWriter.combined(), true);
         } else if ("custom".equals(accessLog.getType())) {
-            serverBuilder
-                    .accessLogWriter(AccessLogWriter.custom(accessLog.getFormat()), true);
+            final String format = accessLog.getFormat();
+            assert format != null;
+            serverBuilder.accessLogWriter(AccessLogWriter.custom(format), true);
         }
     }
 

dropwizard2/src/main/java/com/linecorp/armeria/dropwizard/ArmeriaServerFactory.java

@@ -64,6 +64,7 @@ class ArmeriaServerFactory extends AbstractServerFactory {
     public static final String TYPE = "armeria";
     private static final Logger logger = LoggerFactory.getLogger(ArmeriaServerFactory.class);
 
+    @Nullable
     @JsonUnwrapped
     private @Valid ArmeriaSettings armeriaSettings;
 
@@ -80,6 +81,7 @@ class ArmeriaServerFactory extends AbstractServerFactory {
     @JsonProperty
     private boolean jerseyEnabled = true;
 
+    @Nullable
     @JsonIgnore
     public ServerBuilder getServerBuilder() {
         return serverBuilder;
@@ -184,6 +186,7 @@ private ScheduledThreadPoolExecutor newBlockingTaskExecutor() {
         return blockingTaskExecutor;
     }
 
+    @Nullable
     ArmeriaSettings getArmeriaSettings() {
         return armeriaSettings;
     }

dropwizard2/src/main/java/com/linecorp/armeria/dropwizard/ManagedArmeriaServer.java

@@ -65,6 +65,7 @@ class ManagedArmeriaServer<T extends Configuration> implements Managed {
     public void start() throws Exception {
         logger.trace("Getting Armeria Server Builder");
         final ServerBuilder sb = ((ArmeriaServerFactory) serverFactory).getServerBuilder();
+        assert sb != null;
         logger.trace("Calling Server Configurator");
         serverConfigurator.configure(sb);
         server = sb.build();

eureka/src/main/java/com/linecorp/armeria/client/eureka/EurekaEndpointGroup.java

@@ -21,6 +21,7 @@
 import java.io.IOException;
 import java.net.URI;
 import java.util.List;
+import java.util.Objects;
 import java.util.Set;
 import java.util.StringJoiner;
 import java.util.concurrent.CompletableFuture;
@@ -284,7 +285,7 @@ private static Function<byte[], List<Endpoint>> responseConverter(
                 } else if (appName != null) {
                     filter = instanceInfo -> appName.equals(instanceInfo.getAppName());
                 } else {
-                    filter = instanceInfo -> instanceId.equals(instanceInfo.getInstanceId());
+                    filter = instanceInfo -> Objects.equals(instanceId, instanceInfo.getInstanceId());
                 }
             }
             final StringJoiner joiner = new StringJoiner(",");

eureka/src/main/java/com/linecorp/armeria/internal/common/eureka/InstanceInfo.java

@@ -43,6 +43,7 @@ public final class InstanceInfo {
 
     private static final Logger logger = LoggerFactory.getLogger(InstanceInfo.class);
 
+    @Nullable
     private final String instanceId;
 
     @Nullable

eureka/src/main/java/com/linecorp/armeria/server/eureka/EurekaUpdatingListener.java

@@ -133,6 +133,7 @@ public static EurekaUpdatingListenerBuilder builder(
 
     private final EurekaWebClient client;
     private final InstanceInfo initialInstanceInfo;
+    @Nullable
     private InstanceInfo instanceInfo;
     @Nullable
     private volatile ScheduledFuture<?> heartBeatFuture;
@@ -335,8 +336,9 @@ public void serverStopping(Server server) throws Exception {
         if (heartBeatFuture != null) {
             heartBeatFuture.cancel(false);
         }
+        final InstanceInfo instanceInfo = this.instanceInfo;
         final String appName = this.appName;
-        if (appName != null) {
+        if (instanceInfo != null && appName != null) {
             final String instanceId = instanceInfo.getInstanceId();
             assert instanceId != null;
             client.cancel(appName, instanceId).aggregate().handle((res, cause) -> {

graphql-protocol/src/main/java/com/linecorp/armeria/common/graphql/protocol/DefaultGraphqlRequest.java

@@ -44,6 +44,7 @@ public String query() {
         return query;
     }
 
+    @Nullable
     @Override
     public String operationName() {
         return operationName;

graphql/src/main/java/com/linecorp/armeria/server/graphql/DefaultGraphqlErrorHandler.java

@@ -22,6 +22,8 @@
 
 import javax.annotation.Nonnull;
 
+import com.google.common.collect.ImmutableMap;
+
 import com.linecorp.armeria.common.HttpResponse;
 import com.linecorp.armeria.common.HttpStatus;
 import com.linecorp.armeria.common.MediaType;
@@ -42,7 +44,7 @@ enum DefaultGraphqlErrorHandler implements GraphqlErrorHandler {
     public HttpResponse handle(
             ServiceRequestContext ctx,
             ExecutionInput input,
-            ExecutionResult result,
+            @Nullable ExecutionResult result,
             @Nullable Throwable cause) {
 
         final MediaType produceType = GraphqlUtil.produceType(ctx.request().headers());
@@ -60,11 +62,11 @@ public HttpResponse handle(
             return HttpResponse.ofJson(HttpStatus.INTERNAL_SERVER_ERROR, produceType, specification);
         }
 
-        if (result.getErrors().stream().anyMatch(ValidationError.class::isInstance)) {
+        if (result != null && result.getErrors().stream().anyMatch(ValidationError.class::isInstance)) {
             return HttpResponse.ofJson(HttpStatus.BAD_REQUEST, produceType, result.toSpecification());
         }
 
-        return HttpResponse.ofJson(produceType, result.toSpecification());
+        return HttpResponse.ofJson(produceType, result != null ? result.toSpecification() : ImmutableMap.of());
     }
 
     private static Map<String, Object> toSpecification(Throwable cause) {

graphql/src/main/java/com/linecorp/armeria/server/graphql/DefaultGraphqlService.java

@@ -139,7 +139,9 @@ private HttpResponse execute(
                     }));
         } catch (Throwable cause) {
             cause = Exceptions.peel(cause);
-            return errorHandler.handle(ctx, input, null, cause);
+            final HttpResponse res = errorHandler.handle(ctx, input, null, cause);
+            assert res != null : "DefaultGraphqlService.handle() returned null?";
+            return res;
         }
     }
 

graphql/src/main/java/com/linecorp/armeria/server/graphql/GraphqlWSSubProtocol.java

@@ -374,6 +374,7 @@ private static void writeError(WebSocketWriter out, String operationId, Throwabl
                 "id", operationId,
                 "payload", ImmutableList.of(
                         new GraphQLError() {
+                            @Nullable
                             @Override
                             public String getMessage() {
                                 return t.getMessage();
@@ -430,4 +431,3 @@ public Throwable fillInStackTrace() {
         }
     }
 }
-

grpc-protocol/src/main/java/com/linecorp/armeria/common/grpc/protocol/DeframedMessage.java

@@ -124,6 +124,7 @@ public void close() {
         if (buf != null) {
             buf.release();
         } else {
+            assert stream != null;
             try {
                 stream.close();
             } catch (IOException e) {

grpc-protocol/src/main/java/com/linecorp/armeria/server/grpc/protocol/AbstractUnsafeUnaryGrpcService.java

@@ -142,6 +142,7 @@ protected final HttpResponse doPost(ServiceRequestContext ctx, HttpRequest req)
                             final HttpData content = framer.writePayload(responseMessage);
                             final ResponseHeaders responseHeaders = RESPONSE_HEADERS_MAP.get(
                                     serializationFormat);
+                            assert responseHeaders != null;
                             if (UnaryGrpcSerializationFormats.isGrpcWeb(serializationFormat)) {
                                 // Send trailer as a part of the body for gRPC-web.
                                 final HttpData serializedTrailers = framer.writePayload(

grpc/src/main/java/com/linecorp/armeria/internal/client/grpc/ArmeriaChannel.java

@@ -239,6 +239,9 @@ private <I, O> DefaultClientRequestContext newContext(HttpMethod method, HttpReq
         assert reqTarget != null : path;
         RequestTargetCache.putForClient(path, reqTarget);
 
+        final RequestOptions requestOptions = REQUEST_OPTIONS_MAP.get(methodDescriptor.getType());
+        assert requestOptions != null;
+
         return new DefaultClientRequestContext(
                 meterRegistry,
                 sessionProtocol,
@@ -248,7 +251,7 @@ private <I, O> DefaultClientRequestContext newContext(HttpMethod method, HttpReq
                 options(),
                 req,
                 null,
-                REQUEST_OPTIONS_MAP.get(methodDescriptor.getType()),
+                requestOptions,
                 System.nanoTime(),
                 SystemInfo.currentTimeMicros());
     }

grpc/src/main/java/com/linecorp/armeria/internal/client/grpc/ArmeriaClientCall.java

@@ -474,7 +474,7 @@ public void onComplete() {
     }
 
     @Override
-    public void transportReportStatus(Status status, Metadata metadata) {
+    public void transportReportStatus(Status status, @Nullable Metadata metadata) {
         // This method is invoked in ctx.eventLoop and we need to bounce it through
         // CallOptions executor (in case it's configured) to serialize with closes
         // that were potentially triggered when Listener throws (closeWhenListenerThrows).
@@ -519,7 +519,7 @@ private void closeWhenListenerThrows(Throwable t) {
         closeWhenEos(statusAndMetadata.status(), statusAndMetadata.metadata());
     }
 
-    private void closeWhenEos(Status status, Metadata metadata) {
+    private void closeWhenEos(Status status, @Nullable Metadata metadata) {
         if (needsDirectInvocation()) {
             close(status, metadata);
         } else {
@@ -535,7 +535,7 @@ private void closeWhenEos(Status status, Metadata metadata) {
     // never do this concurrently: the abnormal call into close() from the caller thread happens in case
     // of early return, before event-loop is being assigned to this call. After event-loop is being
     // assigned, the driving call won't be able to trigger close() anymore.
-    private void close(Status status, Metadata metadata) {
+    private void close(Status status, @Nullable Metadata metadata) {
         if (closed) {
             // 'close()' could be called twice if a call is closed with non-OK status.
             // See: https://github.com/line/armeria/issues/3799
@@ -548,7 +548,10 @@ private void close(Status status, Metadata metadata) {
             status = Status.DEADLINE_EXCEEDED;
             // Replace trailers to prevent mixing sources of status and trailers.
             metadata = new Metadata();
+        } else if (metadata == null) {
+            metadata = new Metadata();
         }
+
         if (status.getCode() == Code.DEADLINE_EXCEEDED) {
             status = status.augmentDescription("deadline exceeded after " +
                                                MILLISECONDS.toNanos(ctx.responseTimeoutMillis()) + "ns.");