Add error prone checks for internal javadoc and private constructors (#6844)

This commit is contained in:
jack-berg 2024-11-01 15:55:14 -05:00 committed by GitHub
parent d9fce84689
commit e4f39789bd
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
30 changed files with 340 additions and 12 deletions

View File

@ -10,6 +10,7 @@ plugins {
dependencies {
errorprone("com.google.errorprone:error_prone_core")
errorprone("com.uber.nullaway:nullaway")
errorprone(project(":custom-checks"))
}
val disableErrorProne = properties["disableErrorProne"]?.toString()?.toBoolean() ?: false
@ -86,9 +87,11 @@ tasks {
// cognitive load is dubious.
disable("YodaCondition")
if (name.contains("Jmh") || name.contains("Test")) {
if ((name.contains("Jmh") || name.contains("Test") || project.name.contains("testing-internal")) && !project.name.equals("custom-checks")) {
// Allow underscore in test-type method names
disable("MemberName")
// Internal javadoc not needed for test or jmh classes
disable("OtelInternalJavadoc")
}
option("NullAway:CustomContractAnnotations", "io.opentelemetry.api.internal.Contract")

View File

@ -0,0 +1,82 @@
plugins {
id("otel.java-conventions")
}
dependencies {
implementation("com.google.errorprone:error_prone_core")
testImplementation("com.google.errorprone:error_prone_test_helpers")
}
otelJava.moduleName.set("io.opentelemetry.javaagent.customchecks")
// We cannot use "--release" javac option here because that will forbid exporting com.sun.tools package.
// We also can't seem to use the toolchain without the "--release" option. So disable everything.
java {
sourceCompatibility = JavaVersion.VERSION_17
targetCompatibility = JavaVersion.VERSION_17
toolchain {
languageVersion.set(null as JavaLanguageVersion?)
}
}
tasks {
withType<JavaCompile>().configureEach {
with(options) {
release.set(null as Int?)
compilerArgs.addAll(
listOf(
"--add-exports",
"jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED",
"--add-exports",
"jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED",
"--add-exports",
"jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED",
"--add-exports",
"jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED",
"--add-exports",
"jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED",
),
)
}
}
// only test on java 17+
val testJavaVersion: String? by project
if (testJavaVersion != null && Integer.valueOf(testJavaVersion) < 17) {
test {
enabled = false
}
}
}
tasks.withType<Test>().configureEach {
// required on jdk17
jvmArgs("--add-opens=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED")
jvmArgs("--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED")
jvmArgs("--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED")
jvmArgs("--add-opens=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED")
jvmArgs("--add-opens=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED")
jvmArgs("--add-opens=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED")
jvmArgs("--add-opens=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED")
jvmArgs("--add-opens=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED")
jvmArgs("-XX:+IgnoreUnrecognizedVMOptions")
}
tasks.withType<Javadoc>().configureEach {
// using com.sun.tools.javac.api.JavacTrees breaks javadoc generation
enabled = false
}
// Our conventions apply this project as a dependency in the errorprone configuration, which would cause
// a circular dependency if trying to compile this project with that still there. So we filter this
// project out.
configurations {
named("errorprone") {
dependencies.removeIf {
it is ProjectDependency && it.dependencyProject == project
}
}
}

View File

@ -0,0 +1,72 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.gradle.customchecks;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.matchers.Description;
import com.sun.source.doctree.DocCommentTree;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.PackageTree;
import com.sun.tools.javac.api.JavacTrees;
import java.util.regex.Pattern;
import javax.annotation.Nullable;
import javax.lang.model.element.Modifier;
@BugPattern(
summary =
"This public internal class doesn't end with the javadoc disclaimer: \""
+ OtelInternalJavadoc.EXPECTED_INTERNAL_COMMENT
+ "\"",
severity = WARNING)
public class OtelInternalJavadoc extends BugChecker implements BugChecker.ClassTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Pattern INTERNAL_PACKAGE_PATTERN = Pattern.compile("\\binternal\\b");
static final String EXPECTED_INTERNAL_COMMENT =
"This class is internal and is hence not for public use."
+ " Its APIs are unstable and can change at any time.";
@Override
public Description matchClass(ClassTree tree, VisitorState state) {
if (!isPublic(tree) || !isInternal(state) || tree.getSimpleName().toString().endsWith("Test")) {
return Description.NO_MATCH;
}
String javadoc = getJavadoc(state);
if (javadoc != null && javadoc.contains(EXPECTED_INTERNAL_COMMENT)) {
return Description.NO_MATCH;
}
return describeMatch(tree);
}
private static boolean isPublic(ClassTree tree) {
return tree.getModifiers().getFlags().contains(Modifier.PUBLIC);
}
private static boolean isInternal(VisitorState state) {
PackageTree packageTree = state.getPath().getCompilationUnit().getPackage();
if (packageTree == null) {
return false;
}
String packageName = state.getSourceForNode(packageTree.getPackageName());
return packageName != null && INTERNAL_PACKAGE_PATTERN.matcher(packageName).find();
}
@Nullable
private static String getJavadoc(VisitorState state) {
DocCommentTree docCommentTree =
JavacTrees.instance(state.context).getDocCommentTree(state.getPath());
if (docCommentTree == null) {
return null;
}
return docCommentTree.toString().replace("\n", "");
}
}

View File

@ -0,0 +1,38 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.gradle.customchecks;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.matchers.Description.NO_MATCH;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.PrivateConstructorForUtilityClass;
import com.google.errorprone.matchers.Description;
import com.sun.source.tree.ClassTree;
@BugPattern(
summary =
"Classes which are not intended to be instantiated should be made non-instantiable with a private constructor. This includes utility classes (classes with only static members), and the main class.",
severity = WARNING)
public class OtelPrivateConstructorForUtilityClass extends BugChecker
implements BugChecker.ClassTreeMatcher {
private static final long serialVersionUID = 1L;
private final PrivateConstructorForUtilityClass delegate =
new PrivateConstructorForUtilityClass();
@Override
public Description matchClass(ClassTree tree, VisitorState state) {
Description description = delegate.matchClass(tree, state);
if (description == NO_MATCH) {
return description;
}
return describeMatch(tree);
}
}

View File

@ -0,0 +1,2 @@
io.opentelemetry.gradle.customchecks.OtelInternalJavadoc
io.opentelemetry.gradle.customchecks.OtelPrivateConstructorForUtilityClass

View File

@ -0,0 +1,24 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.gradle.customchecks;
import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.Test;
class OtelInternalJavadocTest {
@Test
void test() {
doTest("internal/InternalJavadocPositiveCases.java");
doTest("internal/InternalJavadocNegativeCases.java");
}
private static void doTest(String path) {
CompilationTestHelper.newInstance(OtelInternalJavadoc.class, OtelInternalJavadocTest.class)
.addSourceFile(path)
.doTest();
}
}

View File

@ -0,0 +1,21 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.gradle.customchecks.internal;
/**
* This class is internal and is hence not for public use. Its APIs are unstable and can change at
* any time.
*/
public class InternalJavadocNegativeCases {
/**
* This class is internal and is hence not for public use. Its APIs are unstable and can change at
* any time.
*/
public static class One {}
static class Two {}
}

View File

@ -0,0 +1,17 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.gradle.customchecks.internal;
// BUG: Diagnostic contains: doesn't end with the javadoc disclaimer
public class InternalJavadocPositiveCases {
// BUG: Diagnostic contains: doesn't end with the javadoc disclaimer
public static class One {}
/** Doesn't have the disclaimer. */
// BUG: Diagnostic contains: doesn't end with the javadoc disclaimer
public static class Two {}
}

View File

@ -38,6 +38,7 @@ val DEPENDENCIES = listOf(
"com.google.auto.value:auto-value-annotations:${autoValueVersion}",
"com.google.errorprone:error_prone_annotations:${errorProneVersion}",
"com.google.errorprone:error_prone_core:${errorProneVersion}",
"com.google.errorprone:error_prone_test_helpers:${errorProneVersion}",
"io.opencensus:opencensus-api:${opencensusVersion}",
"io.opencensus:opencensus-impl-core:${opencensusVersion}",
"io.opencensus:opencensus-impl:${opencensusVersion}",

View File

@ -46,7 +46,12 @@ public abstract class FailedExportException extends Exception {
/** Returns true if the export failed with a response from the server. */
public abstract boolean failedWithResponse();
/** Represents the failure of an HTTP exporter. */
/**
* Represents the failure of an HTTP exporter.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*/
public static final class HttpExportException extends FailedExportException {
private static final long serialVersionUID = -6787390183017184775L;
@ -85,7 +90,12 @@ public abstract class FailedExportException extends Exception {
}
}
/** Represents the failure of a gRPC exporter. */
/**
* Represents the failure of a gRPC exporter.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*/
public static final class GrpcExportException extends FailedExportException {
private static final long serialVersionUID = -9157548250286695364L;

View File

@ -9,7 +9,7 @@ import io.opentelemetry.context.Context;
/**
* This class is internal and is hence not for public use. Its APIs are unstable and can change at
* any time
* any time.
*
* @deprecated use {@link io.opentelemetry.api.internal.InstrumentationUtil} instead. This class
* should be removed once instrumentation does not refer to it anymore.

View File

@ -41,7 +41,12 @@ public interface HttpSender {
/** Shutdown the sender. */
CompletableResultCode shutdown();
/** The HTTP response. */
/**
* The HTTP response.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*/
interface Response {
/** The HTTP status code. */

View File

@ -21,6 +21,9 @@ import javax.annotation.Nullable;
* objects, that we call data. Both integers and objects can be read from the state in the order
* they were added (first in, first out). Additionally, this class provides various pools and caches
* for objects that can be reused between marshalling attempts.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*/
public final class MarshalerContext {
private final boolean marshalStringNoAllocation;
@ -203,6 +206,10 @@ public final class MarshalerContext {
private static final AtomicInteger KEY_INDEX = new AtomicInteger();
/**
* This class is internal and is hence not for public use. Its APIs are unstable and can change at
* any time.
*/
public static class Key {
final int index = KEY_INDEX.getAndIncrement();
}

View File

@ -12,7 +12,12 @@ import io.opentelemetry.exporter.internal.marshal.StatelessMarshaler;
import io.opentelemetry.exporter.internal.marshal.StatelessMarshalerUtil;
import java.io.IOException;
/** A Marshaler of key value pairs. See {@link AnyValueMarshaler}. */
/**
* A Marshaler of key value pairs. See {@link AnyValueMarshaler}.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*/
public final class KeyValueStatelessMarshaler implements StatelessMarshaler<KeyValue> {
public static final KeyValueStatelessMarshaler INSTANCE = new KeyValueStatelessMarshaler();

View File

@ -11,6 +11,9 @@ import io.opentelemetry.api.trace.TraceFlags;
* Represents the 32 bit span flags <a
* href="https://github.com/open-telemetry/opentelemetry-proto/blob/342e1d4c3a1fe43312823ffb53bd38327f263059/opentelemetry/proto/trace/v1/trace.proto#L133">as
* specified in the proto definition</a>.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*/
public final class SpanFlags {
// As defined at:

View File

@ -13,6 +13,9 @@ import io.opentelemetry.sdk.OpenTelemetrySdk;
*
* <p>This is not a standalone SPI. Instead, implementations of other SPIs can also implement this
* interface to receive a callback with the configured SDK.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*/
public interface AutoConfigureListener {

View File

@ -23,6 +23,9 @@ import io.opentelemetry.sdk.trace.samplers.Sampler;
* used, and {@link #create(StructuredConfigProperties)} is (currently) called with an empty {@link
* StructuredConfigProperties}.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*
* @param <T> the type of the SDK extension component. See {@link #getType()}. Supported values
* include: {@link SpanExporter}, {@link MetricExporter}, {@link LogRecordExporter}, {@link
* SpanProcessor}, {@link LogRecordProcessor}, {@link TextMapPropagator}, {@link Sampler},

View File

@ -21,6 +21,9 @@ import javax.annotation.Nullable;
* reading scalar properties, {@link #getStructured(String)} for reading children which are
* themselves mappings, and {@link #getStructuredList(String)} for reading children which are
* sequences of mappings.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*/
public interface StructuredConfigProperties {

View File

@ -13,6 +13,9 @@ import io.opentelemetry.sdk.resources.Resource;
/**
* {@link ResourceProvider} for automatically configuring {@link
* ResourceConfiguration#createEnvironmentResource(ConfigProperties)}.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*/
public final class EnvironmentResourceProvider implements ResourceProvider {
@Override

View File

@ -43,8 +43,8 @@ import java.util.AbstractList;
* elements to zero.
* </ul>
*
* <p>This class is an internal part of the OpenTelemetry SDK and is not intended for public use.
* Its API is unstable and subject to change.
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*
* <p>This class is not thread-safe.
*/

View File

@ -17,6 +17,9 @@ import javax.annotation.concurrent.Immutable;
/**
* A collection of configuration options which define the behavior of a {@link Logger}.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*
* @see SdkLoggerProviderUtil#setLoggerConfigurator(SdkLoggerProviderBuilder, ScopeConfigurator)
* @see SdkLoggerProviderUtil#addLoggerConfiguratorCondition(SdkLoggerProviderBuilder, Predicate,
* LoggerConfig)

View File

@ -21,6 +21,9 @@ import io.opentelemetry.sdk.common.Clock;
*
* <p>Delegates all calls to the configured {@link LoggerProvider}, and its {@link LoggerBuilder}s,
* {@link Logger}s.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*/
public final class SdkEventLoggerProvider implements EventLoggerProvider {

View File

@ -21,7 +21,12 @@ import org.openjdk.jmh.annotations.Setup;
import org.openjdk.jmh.annotations.State;
import org.openjdk.jmh.annotations.Warmup;
/** Measures runtime cost of computing bucket indexes for exponential histograms. */
/**
* Measures runtime cost of computing bucket indexes for exponential histograms.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*/
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@Measurement(iterations = 5, time = 1)

View File

@ -17,6 +17,9 @@ import javax.annotation.concurrent.Immutable;
/**
* A collection of configuration options which define the behavior of a {@link Meter}.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*
* @see SdkMeterProviderUtil#setMeterConfigurator(SdkMeterProviderBuilder, ScopeConfigurator)
* @see SdkMeterProviderUtil#addMeterConfiguratorCondition(SdkMeterProviderBuilder, Predicate,
* MeterConfig)

View File

@ -25,7 +25,7 @@ import javax.annotation.concurrent.Immutable;
* instruments.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time
* at any time.
*/
@Immutable
@AutoValue

View File

@ -17,7 +17,7 @@ import javax.annotation.concurrent.Immutable;
* A single data point that summarizes the values in a time series of numeric values.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time
* at any time.
*/
@Immutable
@AutoValue

View File

@ -13,7 +13,7 @@ import javax.annotation.concurrent.Immutable;
* A summary metric value.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time
* at any time.
*/
@Immutable
@AutoValue

View File

@ -13,6 +13,10 @@ import java.util.List;
import javax.annotation.Nullable;
import javax.annotation.concurrent.Immutable;
/**
* This class is internal and is hence not for public use. Its APIs are unstable and can change at
* any time.
*/
@AutoValue
@Immutable
public abstract class Advice {
@ -39,6 +43,10 @@ public abstract class Advice {
return getAttributes() != null;
}
/**
* This class is internal and is hence not for public use. Its APIs are unstable and can change at
* any time.
*/
@AutoValue.Builder
public abstract static class AdviceBuilder {

View File

@ -17,6 +17,9 @@ import javax.annotation.concurrent.Immutable;
/**
* A collection of configuration options which define the behavior of a {@link Tracer}.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*
* @see SdkTracerProviderUtil#setTracerConfigurator(SdkTracerProviderBuilder, ScopeConfigurator)
* @see SdkTracerProviderUtil#addTracerConfiguratorCondition(SdkTracerProviderBuilder, Predicate,
* TracerConfig)

View File

@ -29,6 +29,7 @@ include(":api:testing-internal")
include(":bom")
include(":bom-alpha")
include(":context")
include(":custom-checks")
include(":dependencyManagement")
include(":extensions:kotlin")
include(":extensions:trace-propagators")