From 182455023a9b2e3afce7497feeaa8d50a45ff10b Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Thu, 12 Aug 2021 14:50:39 +0900 Subject: [PATCH] Update nullaway and add a Contract for StringUtils. (#3432) * Update nullaway and add a Contract for StringUtils. * RUNTIME * SOURCE * Cleanup --- .../api/baggage/propagation/Element.java | 4 +- .../api/baggage/propagation/Parser.java | 23 +++- .../opentelemetry/api/internal/Contract.java | 100 ++++++++++++++++++ .../api/internal/StringUtils.java | 1 + .../otel.errorprone-conventions.gradle.kts | 2 + dependencyManagement/build.gradle.kts | 2 +- 6 files changed, 127 insertions(+), 5 deletions(-) create mode 100644 api/all/src/main/java/io/opentelemetry/api/internal/Contract.java diff --git a/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/Element.java b/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/Element.java index 7c8fc474cb..f6bdaec9eb 100644 --- a/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/Element.java +++ b/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/Element.java @@ -6,6 +6,7 @@ package io.opentelemetry.api.baggage.propagation; import java.util.BitSet; +import javax.annotation.Nullable; /** * Represents single element of a W3C baggage header (key or value). Allows tracking parsing of a @@ -36,7 +37,7 @@ class Element { private boolean trailingSpace; private int start; private int end; - private String value; + @Nullable private String value; static Element createKeyElement() { return new Element(EXCLUDED_KEY_CHARS); @@ -56,6 +57,7 @@ class Element { reset(0); } + @Nullable String getValue() { return value; } diff --git a/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/Parser.java b/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/Parser.java index 0eb6a2ef81..d03aa11bca 100644 --- a/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/Parser.java +++ b/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/Parser.java @@ -7,6 +7,7 @@ package io.opentelemetry.api.baggage.propagation; import io.opentelemetry.api.baggage.BaggageBuilder; import io.opentelemetry.api.baggage.BaggageEntryMetadata; +import javax.annotation.Nullable; /** * Implements single-pass Baggage parsing in accordance with https://w3c.github.io/baggage/ Key / @@ -82,7 +83,11 @@ class Parser { break; case KEY: // none } - baggageBuilder.put(key.getValue(), value.getValue(), BaggageEntryMetadata.create(meta)); + putBaggage( + baggageBuilder, + key.getValue(), + value.getValue(), + BaggageEntryMetadata.create(meta)); reset(i + 1); break; } @@ -107,20 +112,32 @@ class Parser { case META: { String rest = baggageHeader.substring(metaStart).trim(); - baggageBuilder.put(key.getValue(), value.getValue(), BaggageEntryMetadata.create(rest)); + putBaggage( + baggageBuilder, key.getValue(), value.getValue(), BaggageEntryMetadata.create(rest)); break; } case VALUE: { if (!skipToNext) { value.tryTerminating(baggageHeader.length(), baggageHeader); - baggageBuilder.put(key.getValue(), value.getValue()); + putBaggage( + baggageBuilder, key.getValue(), value.getValue(), BaggageEntryMetadata.empty()); break; } } } } + private static void putBaggage( + BaggageBuilder baggage, + @Nullable String key, + @Nullable String value, + BaggageEntryMetadata metadata) { + if (key != null && value != null) { + baggage.put(key, value, metadata); + } + } + /** * Resets parsing state, preparing to start a new list element (see spec). * diff --git a/api/all/src/main/java/io/opentelemetry/api/internal/Contract.java b/api/all/src/main/java/io/opentelemetry/api/internal/Contract.java new file mode 100644 index 0000000000..5fb30fd82a --- /dev/null +++ b/api/all/src/main/java/io/opentelemetry/api/internal/Contract.java @@ -0,0 +1,100 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +// Includes work from: + +/* + * Copyright 2000-2021 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.opentelemetry.api.internal; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Specifies some aspects of the method behavior depending on the arguments. Can be used by tools + * for advanced data flow analysis. Note that this annotation just describes how the code works and + * doesn't add any functionality by means of code generation. + * + *

Method contract has the following syntax:
+ * + * 

{@code
+ * contract ::= (clause ';')* clause
+ * clause ::= args '->' effect
+ * args ::= ((arg ',')* arg )?
+ * arg ::= value-constraint
+ * value-constraint ::= '_' | 'null' | '!null' | 'false' | 'true'
+ * effect ::= value-constraint | 'fail' | 'this' | 'new' | 'param'
+ * }
+ * + *

The constraints denote the following:
+ * + *

+ * + *

The additional return values denote the following:
+ * + *

+ * + *

Examples: + * + *

{@code @Contract("_, null -> null")} - the method returns null if its second argument is null + *
+ * {@code @Contract("_, null -> null; _, !null -> !null")} - the method returns null if its second + * argument is null and not-null otherwise
+ * {@code @Contract("true -> fail")} - a typical {@code assertFalse} method which throws an + * exception if {@code true} is passed to it
+ * {@code @Contract("_ -> this")} - the method always returns its qualifier (e.g. {@link + * StringBuilder#append(String)}). {@code @Contract("null -> fail; _ -> param1")} - the method + * throws an exception if the first argument is null, otherwise it returns the first argument (e.g. + * {@code Objects.requireNonNull}).
+ * {@code @Contract("!null, _ -> param1; null, !null -> param2; null, null -> fail")} - the method + * returns the first non-null argument, or throws an exception if both arguments are null (e.g. + * {@code Objects.requireNonNullElse} in Java 9).
+ * + *

This annotation is the same provided with Jetbrains annotations and used by Nullaway for + * verifying nullness. We copy the annotation to avoid an external dependency. + */ +@Documented +@Retention(RetentionPolicy.SOURCE) +@Target({ElementType.METHOD, ElementType.CONSTRUCTOR}) +public @interface Contract { + /** + * Contains the contract clauses describing causal relations between call arguments and the + * returned value. + */ + String value() default ""; +} diff --git a/api/all/src/main/java/io/opentelemetry/api/internal/StringUtils.java b/api/all/src/main/java/io/opentelemetry/api/internal/StringUtils.java index 0af3b5cdb1..d46a8b145c 100644 --- a/api/all/src/main/java/io/opentelemetry/api/internal/StringUtils.java +++ b/api/all/src/main/java/io/opentelemetry/api/internal/StringUtils.java @@ -27,6 +27,7 @@ public final class StringUtils { * @param string a string reference to check * @return {@code true} if the string is null or is the empty string */ + @Contract("null -> true") public static boolean isNullOrEmpty(@Nullable String string) { return string == null || string.isEmpty(); } diff --git a/buildSrc/src/main/kotlin/otel.errorprone-conventions.gradle.kts b/buildSrc/src/main/kotlin/otel.errorprone-conventions.gradle.kts index b883a91bc3..2d4b46b36d 100644 --- a/buildSrc/src/main/kotlin/otel.errorprone-conventions.gradle.kts +++ b/buildSrc/src/main/kotlin/otel.errorprone-conventions.gradle.kts @@ -64,6 +64,8 @@ tasks { // Allow underscore in test-type method names disable("MemberName") } + + option("NullAway:CustomContractAnnotations", "io.opentelemetry.api.internal.Contract") } errorprone.nullaway { diff --git a/dependencyManagement/build.gradle.kts b/dependencyManagement/build.gradle.kts index 7934dec4f4..89102cf194 100644 --- a/dependencyManagement/build.gradle.kts +++ b/dependencyManagement/build.gradle.kts @@ -80,7 +80,7 @@ val DEPENDENCIES = listOf( "com.squareup.okhttp3:okhttp:4.9.1", "com.sun.net.httpserver:http:20070405", "com.tngtech.archunit:archunit-junit4:0.19.0", - "com.uber.nullaway:nullaway:0.9.1", + "com.uber.nullaway:nullaway:0.9.2", "edu.berkeley.cs.jqf:jqf-fuzz:1.7", "eu.rekawek.toxiproxy:toxiproxy-java:2.1.4", "io.github.netmikey.logunit:logunit-jul:1.1.0",