Allow enabling nullaway and fix some nullness issues (#3218)

* Add support for enabling nullaway and fix a couple of projects.

* Finish

* Finish

* Cleanup
This commit is contained in:
Anuraag Agrawal 2021-05-12 08:43:32 +09:00 committed by GitHub
parent e6c1f65b09
commit 77f0b0adc4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 75 additions and 50 deletions

View File

@ -40,7 +40,7 @@ public final class W3CBaggagePropagator implements TextMapPropagator {
}
@Override
public <C> void inject(Context context, C carrier, TextMapSetter<C> setter) {
public <C> void inject(Context context, @Nullable C carrier, TextMapSetter<C> setter) {
if (context == null || setter == null) {
return;
}

View File

@ -8,6 +8,7 @@ package io.opentelemetry.api.common;
import io.opentelemetry.api.internal.ImmutableKeyValuePairs;
import java.util.ArrayList;
import java.util.Comparator;
import javax.annotation.Nullable;
import javax.annotation.concurrent.Immutable;
@Immutable
@ -32,6 +33,7 @@ final class ArrayBackedAttributes extends ImmutableKeyValuePairs<AttributeKey<?>
@SuppressWarnings("unchecked")
@Override
@Nullable
public <T> T get(AttributeKey<T> key) {
return (T) super.get(key);
}

View File

@ -8,7 +8,6 @@ package io.opentelemetry.api.common;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import javax.annotation.Nullable;
class ArrayBackedAttributesBuilder implements AttributesBuilder {
private final List<Object> data;
@ -53,11 +52,7 @@ class ArrayBackedAttributesBuilder implements AttributesBuilder {
return this;
}
@Nullable
static List<Double> toList(@Nullable double... values) {
if (values == null) {
return null;
}
static List<Double> toList(double... values) {
Double[] boxed = new Double[values.length];
for (int i = 0; i < values.length; i++) {
boxed[i] = values[i];
@ -65,11 +60,7 @@ class ArrayBackedAttributesBuilder implements AttributesBuilder {
return Arrays.asList(boxed);
}
@Nullable
static List<Long> toList(@Nullable long... values) {
if (values == null) {
return null;
}
static List<Long> toList(long... values) {
Long[] boxed = new Long[values.length];
for (int i = 0; i < values.length; i++) {
boxed[i] = values[i];
@ -77,11 +68,7 @@ class ArrayBackedAttributesBuilder implements AttributesBuilder {
return Arrays.asList(boxed);
}
@Nullable
static List<Boolean> toList(@Nullable boolean... values) {
if (values == null) {
return null;
}
static List<Boolean> toList(boolean... values) {
Boolean[] boxed = new Boolean[values.length];
for (int i = 0; i < values.length; i++) {
boxed[i] = values[i];

View File

@ -29,12 +29,7 @@ abstract class AttributeKeyImpl<T> implements AttributeKey<T> {
}
@Override
public String getKey() {
return key();
}
@Nullable
abstract String key();
public abstract String getKey();
@Override
public final String toString() {

View File

@ -9,6 +9,7 @@ import static io.opentelemetry.api.common.ArrayBackedAttributes.sortAndFilterToA
import java.util.Map;
import java.util.function.BiConsumer;
import javax.annotation.Nullable;
import javax.annotation.concurrent.Immutable;
/**
@ -33,6 +34,7 @@ import javax.annotation.concurrent.Immutable;
public interface Attributes {
/** Returns the value for the given {@link AttributeKey}, or {@code null} if not found. */
@Nullable
<T> T get(AttributeKey<T> key);
/** Iterates over all the key-value pairs of attributes contained by this instance. */

View File

@ -85,7 +85,10 @@ public interface AttributesBuilder {
* @return this Builder
*/
default AttributesBuilder put(String key, String... value) {
return put(stringArrayKey(key), value == null ? null : Arrays.asList(value));
if (value == null) {
return this;
}
return put(stringArrayKey(key), Arrays.asList(value));
}
/**
@ -97,6 +100,9 @@ public interface AttributesBuilder {
* @return this Builder
*/
default AttributesBuilder put(String key, long... value) {
if (value == null) {
return this;
}
return put(longArrayKey(key), toList(value));
}
@ -109,6 +115,9 @@ public interface AttributesBuilder {
* @return this Builder
*/
default AttributesBuilder put(String key, double... value) {
if (value == null) {
return this;
}
return put(doubleArrayKey(key), toList(value));
}
@ -121,6 +130,9 @@ public interface AttributesBuilder {
* @return this Builder
*/
default AttributesBuilder put(String key, boolean... value) {
if (value == null) {
return this;
}
return put(booleanArrayKey(key), toList(value));
}

View File

@ -29,6 +29,7 @@ import java.util.List;
import java.util.Map;
import java.util.NoSuchElementException;
import java.util.Set;
import javax.annotation.Nullable;
/** A read-only view of an array of key-value pairs. */
@SuppressWarnings("unchecked")
@ -74,6 +75,7 @@ public final class ReadOnlyArrayMap<K, V> extends AbstractMap<K, V> {
}
@Override
@Nullable
public V get(Object o) {
if (o == null) {
return null; // null keys are not allowed

View File

@ -25,19 +25,18 @@ final class ArrayBasedTraceStateBuilder implements TraceStateBuilder {
private static final int VALUE_MAX_SIZE = 256;
private static final int MAX_TENANT_ID_SIZE = 240;
private final ArrayBasedTraceState parent;
@Nullable private List<String> entries;
private final List<String> entries;
static TraceState empty() {
return EMPTY;
}
ArrayBasedTraceStateBuilder() {
parent = EMPTY;
entries = new ArrayList<>();
}
ArrayBasedTraceStateBuilder(ArrayBasedTraceState parent) {
this.parent = parent;
entries = new ArrayList<>(parent.getEntries());
}
/**
@ -57,10 +56,6 @@ final class ArrayBasedTraceStateBuilder implements TraceStateBuilder {
|| (entries != null && entries.size() >= MAX_KEY_VALUE_PAIRS)) {
return this;
}
if (entries == null) {
// Copy entries from the parent.
entries = new ArrayList<>(parent.getEntries());
}
removeEntry(key);
// Inserts the element at the front of this list. (note: probably pretty inefficient with an
// ArrayList as the underlying implementation!)
@ -74,10 +69,6 @@ final class ArrayBasedTraceStateBuilder implements TraceStateBuilder {
if (key == null) {
return this;
}
if (entries == null) {
// Copy entries from the parent.
entries = new ArrayList<>(parent.getEntries());
}
removeEntry(key);
return this;
}
@ -97,9 +88,6 @@ final class ArrayBasedTraceStateBuilder implements TraceStateBuilder {
@Override
public TraceState build() {
if (entries == null) {
return parent;
}
return ArrayBasedTraceState.create(entries);
}

View File

@ -18,7 +18,7 @@ class DefaultTracerProvider implements TracerProvider {
@Override
public Tracer get(String instrumentationName) {
return get(instrumentationName, null);
return DefaultTracer.getInstance();
}
@Override

View File

@ -5,8 +5,10 @@ import io.morethan.jmhreport.gradle.JmhReportExtension
import me.champeau.gradle.JMHPluginExtension
import me.champeau.gradle.japicmp.JapicmpTask
import nebula.plugin.release.git.opinion.Strategies
import net.ltgt.gradle.errorprone.CheckSeverity
import net.ltgt.gradle.errorprone.ErrorProneOptions
import net.ltgt.gradle.errorprone.ErrorPronePlugin
import net.ltgt.gradle.nullaway.NullAwayOptions
import org.gradle.api.plugins.JavaPlugin.*
import org.gradle.api.tasks.testing.logging.TestExceptionFormat
import ru.vyarus.gradle.plugin.animalsniffer.AnimalSnifferExtension
@ -24,6 +26,7 @@ plugins {
id("io.morethan.jmhreport") apply false
id("me.champeau.gradle.jmh") apply false
id("net.ltgt.errorprone") apply false
id("net.ltgt.nullaway") apply false
id("ru.vyarus.animalsniffer") apply false
id("me.champeau.gradle.japicmp") apply false
}
@ -103,6 +106,8 @@ nexusStaging {
delayBetweenRetriesInMillis = 10000
}
val enableNullaway: String? by project
subprojects {
group = "io.opentelemetry"
@ -114,6 +119,7 @@ subprojects {
plugins.apply("com.diffplug.spotless")
plugins.apply("net.ltgt.errorprone")
plugins.apply("net.ltgt.nullaway")
configure<BasePluginConvention> {
archivesBaseName = "opentelemetry-${name}"
@ -190,6 +196,17 @@ subprojects {
disableWarningsInGeneratedCode.set(true)
allDisabledChecksAsWarnings.set(true)
(this as ExtensionAware).extensions.configure<NullAwayOptions> {
// Enable nullaway on main sources.
// TODO(anuraaga): Remove enableNullaway flag when all errors fixed
if (!name.contains("Test") && !name.contains("Jmh") && enableNullaway == "true") {
severity.set(CheckSeverity.ERROR)
} else {
severity.set(CheckSeverity.OFF)
}
annotatedPackages.add("io.opentelemetry")
}
// Doesn't currently use Var annotations.
disable("Var") // "-Xep:Var:OFF"
@ -208,7 +225,7 @@ subprojects {
disable("UnnecessarilyFullyQualified")
// Ignore warnings for protobuf and jmh generated files.
excludedPaths.set(".*generated.*")
excludedPaths.set(".*generated.*|.*internal.shaded.*")
// "-XepExcludedPaths:.*/build/generated/source/proto/.*"
disable("Java7ApiChecker")
@ -375,6 +392,7 @@ subprojects {
add(TEST_RUNTIME_ONLY_CONFIGURATION_NAME, "org.junit.vintage:junit-vintage-engine")
add(ErrorPronePlugin.CONFIGURATION_NAME, "com.google.errorprone:error_prone_core")
add(ErrorPronePlugin.CONFIGURATION_NAME, "com.uber.nullaway:nullaway")
add(ANNOTATION_PROCESSOR_CONFIGURATION_NAME, "com.google.guava:guava-beta-checker")

View File

@ -30,6 +30,7 @@ import java.util.concurrent.Executor;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.stream.Collectors;
import javax.annotation.Nullable;
/**
* A {@link ContextStorage} which keeps track of opened and closed {@link Scope}s, reporting caller
@ -112,6 +113,7 @@ final class StrictContextStorage implements ContextStorage, AutoCloseable {
}
@Override
@Nullable
public Context current() {
return delegate.current();
}
@ -209,7 +211,8 @@ final class StrictContextStorage implements ContextStorage, AutoCloseable {
@Override
public String toString() {
return caller.getMessage();
String message = caller.getMessage();
return message != null ? message : super.toString();
}
}

View File

@ -80,6 +80,7 @@ val DEPENDENCIES = listOf(
"com.squareup.okhttp3:okhttp:4.9.1",
"com.sun.net.httpserver:http:20070405",
"com.tngtech.archunit:archunit-junit4:0.17.0",
"com.uber.nullaway:nullaway:0.9.1",
"edu.berkeley.cs.jqf:jqf-fuzz:1.6",
"eu.rekawek.toxiproxy:toxiproxy-java:2.1.4",
"io.github.netmikey.logunit:logunit-jul:1.1.0",

View File

@ -1,2 +1,5 @@
Comparing source compatibility of against
No changes.
=== UNCHANGED INTERFACE: PUBLIC ABSTRACT io.opentelemetry.api.common.Attributes (not serializable)
=== CLASS FILE FORMAT VERSION: 52.0 <- 52.0
=== UNCHANGED METHOD: PUBLIC ABSTRACT java.lang.Object get(io.opentelemetry.api.common.AttributeKey)
+++ NEW ANNOTATION: javax.annotation.Nullable

View File

@ -215,10 +215,13 @@ public final class JaegerPropagator implements TextMapPropagator {
return buildSpanContext(traceId, spanId, flags);
}
private static <C> Baggage getBaggageFromHeader(C carrier, TextMapGetter<C> getter) {
@Nullable
private static <C> Baggage getBaggageFromHeader(@Nullable C carrier, TextMapGetter<C> getter) {
BaggageBuilder builder = null;
for (String key : getter.keys(carrier)) {
Iterable<String> keys = carrier != null ? getter.keys(carrier) : Collections.emptyList();
for (String key : keys) {
if (key.startsWith(BAGGAGE_PREFIX)) {
if (key.length() == BAGGAGE_PREFIX.length()) {
continue;
@ -227,9 +230,19 @@ public final class JaegerPropagator implements TextMapPropagator {
if (builder == null) {
builder = Baggage.builder();
}
builder.put(key.substring(BAGGAGE_PREFIX.length()), getter.get(carrier, key));
String value = getter.get(carrier, key);
if (value != null) {
builder.put(key.substring(BAGGAGE_PREFIX.length()), value);
}
} else if (key.equals(BAGGAGE_HEADER)) {
builder = parseBaggageHeader(getter.get(carrier, key), builder);
String value = getter.get(carrier, key);
if (value != null) {
if (builder == null) {
builder = Baggage.builder();
}
builder = parseBaggageHeader(value, builder);
}
}
}
return builder == null ? null : builder.build();
@ -239,9 +252,6 @@ public final class JaegerPropagator implements TextMapPropagator {
for (String part : header.split("\\s*,\\s*")) {
String[] kv = part.split("\\s*=\\s*");
if (kv.length == 2) {
if (builder == null) {
builder = Baggage.builder();
}
builder.put(kv[0], kv[1]);
} else {
logger.fine("malformed token in " + BAGGAGE_HEADER + " header: " + part);

View File

@ -12,6 +12,8 @@ pluginManagement {
id("me.champeau.gradle.jmh") version "0.5.3"
id("nebula.release") version "15.3.1"
id("net.ltgt.errorprone") version "2.0.1"
id("net.ltgt.nullaway") version "1.1.0"
id("org.checkerframework") version "0.5.20"
id("org.jetbrains.kotlin.jvm") version "1.4.21"
id("org.unbroken-dome.test-sets") version "3.0.1"
id("ru.vyarus.animalsniffer") version "1.5.3"