From b4cf60d79b6479f6d77b0ef1767c0c33cb59a647 Mon Sep 17 00:00:00 2001 From: Jack Berg <34418638+jack-berg@users.noreply.github.com> Date: Thu, 23 Apr 2026 15:45:48 -0500 Subject: [PATCH 1/2] Add new *.impl.* package convention, internal code with japicmp compatibility --- .../gradle/customchecks/OtelImplJavadoc.java | 75 +++++++++++++++++++ ...m.google.errorprone.bugpatterns.BugChecker | 1 + .../customchecks/OtelImplJavadocTest.java | 75 +++++++++++++++++++ docs/knowledge/README.md | 16 ++-- docs/knowledge/general-patterns.md | 26 +++++++ 5 files changed, 185 insertions(+), 8 deletions(-) create mode 100644 custom-checks/src/main/java/io/opentelemetry/gradle/customchecks/OtelImplJavadoc.java create mode 100644 custom-checks/src/test/java/io/opentelemetry/gradle/customchecks/OtelImplJavadocTest.java diff --git a/custom-checks/src/main/java/io/opentelemetry/gradle/customchecks/OtelImplJavadoc.java b/custom-checks/src/main/java/io/opentelemetry/gradle/customchecks/OtelImplJavadoc.java new file mode 100644 index 00000000000..22a0178fefe --- /dev/null +++ b/custom-checks/src/main/java/io/opentelemetry/gradle/customchecks/OtelImplJavadoc.java @@ -0,0 +1,75 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.gradle.customchecks; + +import static com.google.errorprone.BugPattern.LinkType.NONE; +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 impl class is missing the required javadoc disclaimer: \"" + + OtelImplJavadoc.EXPECTED_IMPL_COMMENT + + "\"", + severity = WARNING, + linkType = NONE) +public class OtelImplJavadoc extends BugChecker implements BugChecker.ClassTreeMatcher { + + private static final long serialVersionUID = 1L; + + private static final Pattern IMPL_PACKAGE_PATTERN = Pattern.compile("\\bimpl\\b"); + + static final String EXPECTED_IMPL_COMMENT = + "This class is not intended for use by application developers." + + " Its API is stable and will not be changed or removed in a backwards-incompatible" + + " manner."; + + @Override + public Description matchClass(ClassTree tree, VisitorState state) { + if (!isPublic(tree) || !isImpl(state) || tree.getSimpleName().toString().endsWith("Test")) { + return Description.NO_MATCH; + } + String javadoc = getJavadoc(state); + if (javadoc != null && javadoc.contains(EXPECTED_IMPL_COMMENT)) { + return Description.NO_MATCH; + } + return describeMatch(tree); + } + + private static boolean isPublic(ClassTree tree) { + return tree.getModifiers().getFlags().contains(Modifier.PUBLIC); + } + + private static boolean isImpl(VisitorState state) { + PackageTree packageTree = state.getPath().getCompilationUnit().getPackage(); + if (packageTree == null) { + return false; + } + String packageName = state.getSourceForNode(packageTree.getPackageName()); + return packageName != null && IMPL_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", " ").replace(" * ", " ").replaceAll("\\s+", " "); + } +} diff --git a/custom-checks/src/main/resources/META-INF/services/com.google.errorprone.bugpatterns.BugChecker b/custom-checks/src/main/resources/META-INF/services/com.google.errorprone.bugpatterns.BugChecker index 3440c60a31c..7bb2c5507d7 100644 --- a/custom-checks/src/main/resources/META-INF/services/com.google.errorprone.bugpatterns.BugChecker +++ b/custom-checks/src/main/resources/META-INF/services/com.google.errorprone.bugpatterns.BugChecker @@ -1,3 +1,4 @@ io.opentelemetry.gradle.customchecks.OtelDeprecatedApiUsage +io.opentelemetry.gradle.customchecks.OtelImplJavadoc io.opentelemetry.gradle.customchecks.OtelInternalJavadoc io.opentelemetry.gradle.customchecks.OtelPrivateConstructorForUtilityClass diff --git a/custom-checks/src/test/java/io/opentelemetry/gradle/customchecks/OtelImplJavadocTest.java b/custom-checks/src/test/java/io/opentelemetry/gradle/customchecks/OtelImplJavadocTest.java new file mode 100644 index 00000000000..ad294d61b2c --- /dev/null +++ b/custom-checks/src/test/java/io/opentelemetry/gradle/customchecks/OtelImplJavadocTest.java @@ -0,0 +1,75 @@ +/* + * 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 OtelImplJavadocTest { + + @Test + void positiveCases() { + CompilationTestHelper.newInstance(OtelImplJavadoc.class, OtelImplJavadocTest.class) + .addSourceLines( + "impl/ImplJavadocPositiveCases.java", + "/*", + " * Copyright The OpenTelemetry Authors", + " * SPDX-License-Identifier: Apache-2.0", + " */", + "package io.opentelemetry.gradle.customchecks.impl;", + "// BUG: Diagnostic contains: missing the required javadoc disclaimer", + "public class ImplJavadocPositiveCases {", + " // BUG: Diagnostic contains: missing the required javadoc disclaimer", + " public static class One {}", + " /** Doesn't have the disclaimer. */", + " // BUG: Diagnostic contains: missing the required javadoc disclaimer", + " public static class Two {}", + "}") + .doTest(); + } + + @Test + void negativeCases() { + CompilationTestHelper.newInstance(OtelImplJavadoc.class, OtelImplJavadocTest.class) + .addSourceLines( + "impl/ImplJavadocNegativeCases.java", + "/*", + " * Copyright The OpenTelemetry Authors", + " * SPDX-License-Identifier: Apache-2.0", + " */", + "package io.opentelemetry.gradle.customchecks.impl;", + "/**", + " * This class is not intended for use by application developers. Its API is stable and", + " * will not be changed or removed in a backwards-incompatible manner.", + " */", + "public class ImplJavadocNegativeCases {", + " /**", + " * This class is not intended for use by application developers. Its API is stable", + " * and will not be changed or removed in a backwards-incompatible manner.", + " */", + " public static class One {}", + " // Non-public class without disclaimer is fine.", + " static class Two {}", + "}") + .doTest(); + } + + @Test + void nonImplPackageIgnored() { + CompilationTestHelper.newInstance(OtelImplJavadoc.class, OtelImplJavadocTest.class) + .addSourceLines( + "other/NonImplPackageCases.java", + "/*", + " * Copyright The OpenTelemetry Authors", + " * SPDX-License-Identifier: Apache-2.0", + " */", + "package io.opentelemetry.gradle.customchecks.other;", + "public class NonImplPackageCases {", + " public static class One {}", + "}") + .doTest(); + } +} diff --git a/docs/knowledge/README.md b/docs/knowledge/README.md index a05ba773a04..51110ad3251 100644 --- a/docs/knowledge/README.md +++ b/docs/knowledge/README.md @@ -11,14 +11,14 @@ is the signal. ## Topics -| File | Load when | -| --- | --- | -| [build.md](build.md) | Always — build requirements and common tasks | -| [general-patterns.md](general-patterns.md) | Always — style, nullability, visibility, AutoValue, locking, logging | -| [api-stability.md](api-stability.md) | Public API additions, removals, renames, or deprecations; stable vs alpha compatibility | -| [gradle-conventions.md](gradle-conventions.md) | `build.gradle.kts` or `settings.gradle.kts` changes; new modules | -| [testing-patterns.md](testing-patterns.md) | Test files in scope — assertions, test utilities, test suites | -| [other-tasks.md](other-tasks.md) | Dev environment setup, benchmarks, composite builds, native image tests, OTLP protobuf updates | +| File | Load when | +|------------------------------------------------|------------------------------------------------------------------------------------------------| +| [build.md](build.md) | Always — build requirements and common tasks | +| [general-patterns.md](general-patterns.md) | Always — style, nullability, visibility, AutoValue, locking, logging, internal & impl packages | +| [api-stability.md](api-stability.md) | Public API additions, removals, renames, or deprecations, stable vs alpha compatibility | +| [gradle-conventions.md](gradle-conventions.md) | `build.gradle.kts` or `settings.gradle.kts` changes; new modules | +| [testing-patterns.md](testing-patterns.md) | Test files in scope — assertions, test utilities, test suites | +| [other-tasks.md](other-tasks.md) | Dev environment setup, benchmarks, composite builds, native image tests, OTLP protobuf updates | ## Conventions diff --git a/docs/knowledge/general-patterns.md b/docs/knowledge/general-patterns.md index 92f6ff0688f..0f4a4dea1bc 100644 --- a/docs/knowledge/general-patterns.md +++ b/docs/knowledge/general-patterns.md @@ -64,6 +64,29 @@ most violations automatically. - **EditorConfig** (`.editorconfig`) — configures IntelliJ to match project style automatically. It doesn't cover all rules, so `spotlessApply` is still required. +## Impl (Implementation) code + +Use `*.impl.*` sub-packages for code that must be `public` to be shared across OpenTelemetry +modules, but is not intended for use by application developers. This is the right choice when +`*.internal.*` is too restrictive — for example, a utility used by both the API and SDK modules +that needs a stable contract across module boundaries. + +Unlike `*.internal.*` packages, `*.impl.*` packages carry full backwards-compatibility guarantees +and are included in `japicmp` compatibility checks. + +Public classes in `impl` packages must carry the following disclaimer (enforced by the +`OtelImplJavadoc` Error Prone check in `custom-checks/`): + +```java +/** + * This class is not intended for use by application developers. Its API is stable and will not + * be changed or removed in a backwards-incompatible manner. + */ +``` + +See also [Internal code](#internal-code) for code that is not for application developers and has +no stability guarantees. + ## Internal code Prefer package-private over putting code in an `internal` package. Use `internal` only when the @@ -93,6 +116,9 @@ Internal code must not be used across module boundaries — module `foo` must no code from module `bar`. Cross-module internal usage is a known issue being tracked and cleaned up in open-telemetry/opentelemetry-java#6970. +See also [Impl (Implementation) code](#impl-implementation-code) for cross-module code that is not for application developers but +requires a stable ABI. + ## Javadoc All public classes, and their public and protected methods, must have complete Javadoc including From dec019eb04e1db9d90524392836853a3f621e545 Mon Sep 17 00:00:00 2001 From: Jack Berg <34418638+jack-berg@users.noreply.github.com> Date: Fri, 24 Apr 2026 09:21:25 -0500 Subject: [PATCH 2/2] Update docs/knowledge/general-patterns.md Co-authored-by: Jay DeLuca --- docs/knowledge/general-patterns.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/knowledge/general-patterns.md b/docs/knowledge/general-patterns.md index 0f4a4dea1bc..d61b9cea07f 100644 --- a/docs/knowledge/general-patterns.md +++ b/docs/knowledge/general-patterns.md @@ -117,7 +117,7 @@ code from module `bar`. Cross-module internal usage is a known issue being track in open-telemetry/opentelemetry-java#6970. See also [Impl (Implementation) code](#impl-implementation-code) for cross-module code that is not for application developers but -requires a stable ABI. +requires a stable API. ## Javadoc