Skip to content

Commit 7fdad44

Browse files
bm1549claude
bm1549
and
claude
committed
feat: add Wave 2 static checks — Javadoc linter, CPD, ArchUnit, AssertJ preference, boxed primitives
- Task 009: Ban boxed primitive valueOf() in forbidden APIs - Task 010: JavadocLinter Gradle plugin (empty @return/@param detection) - Task 011: CopyPasteDetectorPlugin (hash-based duplicate method detection) - Task 012: ArchUnit architecture tests (bootstrap/core/instrumentation boundaries) - Task 013: AssertJPreferenceLinter (flag JUnit assertions in new test files) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 2da2eb2 commit 7fdad44

7 files changed

Lines changed: 332 additions & 0 deletions

File tree

‎buildSrc/build.gradle.kts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,21 @@ gradlePlugin {
7474
id = "dd-trace-java.naming-convention-linter"
7575
implementationClass = "datadog.gradle.plugin.lint.NamingConventionLinter"
7676
}
77+
78+
create("javadoc-linter") {
79+
id = "dd-trace-java.javadoc-linter"
80+
implementationClass = "datadog.gradle.plugin.lint.JavadocLinter"
81+
}
82+
83+
create("copy-paste-detector") {
84+
id = "dd-trace-java.copy-paste-detector"
85+
implementationClass = "datadog.gradle.plugin.lint.CopyPasteDetectorPlugin"
86+
}
87+
88+
create("assertj-preference-linter") {
89+
id = "dd-trace-java.assertj-preference-linter"
90+
implementationClass = "datadog.gradle.plugin.lint.AssertJPreferenceLinter"
91+
}
7792
}
7893
}
7994

‎buildSrc/src/main/kotlin/datadog/gradle/plugin/lint/AssertJPreferenceLinter.kt‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
package datadog.gradle.plugin.lint
2+
3+
import org.gradle.api.Plugin
4+
import org.gradle.api.Project
5+
import java.io.ByteArrayOutputStream
6+
7+
class AssertJPreferenceLinter : Plugin<Project> {
8+
override fun apply(target: Project) {
9+
target.tasks.register("checkAssertJPreference") {
10+
group = "verification"
11+
description = "Flags JUnit assertion imports in new test files — prefer AssertJ"
12+
13+
doLast {
14+
val newTestFiles = getNewTestFiles(target)
15+
if (newTestFiles.isEmpty()) {
16+
target.logger.lifecycle("checkAssertJPreference: No new test files to check")
17+
return@doLast
18+
}
19+
20+
val warnings = mutableListOf<String>()
21+
val junitAssertImport = Regex("""import\s+(?:static\s+)?org\.junit\.jupiter\.api\.Assertions""")
22+
23+
newTestFiles.forEach { file ->
24+
if (!file.exists()) return@forEach
25+
val lines = file.readLines()
26+
val relPath = file.relativeTo(target.rootProject.projectDir).path
27+
28+
for (i in lines.indices) {
29+
if (junitAssertImport.containsMatchIn(lines[i])) {
30+
warnings.add("STYLE: $relPath:${i + 1} — Prefer AssertJ (org.assertj.core.api.Assertions) over JUnit assertions for richer API")
31+
}
32+
}
33+
}
34+
35+
if (warnings.isNotEmpty()) {
36+
warnings.forEach { target.logger.warn(it) }
37+
target.logger.warn("checkAssertJPreference: ${warnings.size} file(s) using JUnit assertions instead of AssertJ")
38+
} else {
39+
target.logger.lifecycle("checkAssertJPreference: All new test files use AssertJ")
40+
}
41+
}
42+
}
43+
}
44+
45+
private fun getNewTestFiles(project: Project): List<java.io.File> {
46+
return try {
47+
val stdout = ByteArrayOutputStream()
48+
project.exec {
49+
commandLine("git", "diff", "--name-only", "--diff-filter=A", "HEAD~1")
50+
standardOutput = stdout
51+
isIgnoreExitValue = true
52+
}
53+
stdout.toString().trim().lines()
54+
.filter { it.endsWith(".java") && it.contains("src/test/") }
55+
.map { project.rootProject.file(it) }
56+
} catch (e: Exception) {
57+
emptyList()
58+
}
59+
}
60+
}

‎buildSrc/src/main/kotlin/datadog/gradle/plugin/lint/CopyPasteDetectorPlugin.kt‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
package datadog.gradle.plugin.lint
2+
3+
import org.gradle.api.Plugin
4+
import org.gradle.api.Project
5+
import java.io.File
6+
7+
class CopyPasteDetectorPlugin : Plugin<Project> {
8+
override fun apply(target: Project) {
9+
target.tasks.register("checkCodeDuplication") {
10+
group = "verification"
11+
description = "Detect copy-pasted code using hash-based method body comparison"
12+
13+
doLast {
14+
val dirs = listOf("dd-java-agent/instrumentation", "dd-trace-core", "internal-api")
15+
.map { target.rootProject.file(it) }
16+
.filter { it.exists() }
17+
18+
val methodHashes = mutableMapOf<Int, MutableList<String>>()
19+
var totalFiles = 0
20+
21+
dirs.forEach { dir ->
22+
dir.walkTopDown()
23+
.filter { it.isFile && it.extension == "java" && !it.path.contains("/build/") && !it.path.contains("/generated/") }
24+
.forEach { file ->
25+
totalFiles++
26+
extractMethodBodies(file).forEach { (name, body) ->
27+
val normalized = normalizeCode(body)
28+
if (normalized.length > 200) {
29+
val hash = normalized.hashCode()
30+
val location = "${file.relativeTo(target.rootProject.projectDir).path}:$name"
31+
methodHashes.getOrPut(hash) { mutableListOf() }.add(location)
32+
}
33+
}
34+
}
35+
}
36+
37+
val duplicates = methodHashes.filter { it.value.size > 1 }
38+
if (duplicates.isNotEmpty()) {
39+
target.logger.warn("CPD: Found ${duplicates.size} group(s) of duplicate methods across $totalFiles files:")
40+
duplicates.entries.take(20).forEach { (_, locations) ->
41+
target.logger.warn(" DUPLICATE GROUP (${locations.size} copies):")
42+
locations.forEach { loc -> target.logger.warn(" - $loc") }
43+
}
44+
if (duplicates.size > 20) {
45+
target.logger.warn(" ... and ${duplicates.size - 20} more groups")
46+
}
47+
} else {
48+
target.logger.lifecycle("CPD: No significant duplicates found across $totalFiles files")
49+
}
50+
}
51+
}
52+
}
53+
54+
private fun extractMethodBodies(file: File): List<Pair<String, String>> {
55+
val content = file.readText()
56+
val results = mutableListOf<Pair<String, String>>()
57+
val methodPattern = Regex("""(?:public|private|protected|static|final|synchronized|\s)+[\w<>\[\],\s]+\s+(\w+)\s*\([^)]*\)[^{]*\{""")
58+
59+
methodPattern.findAll(content).forEach { match ->
60+
val methodName = match.groupValues[1]
61+
val startIdx = match.range.last + 1
62+
var braceCount = 1
63+
var idx = startIdx
64+
while (idx < content.length && braceCount > 0) {
65+
when (content[idx]) {
66+
'{' -> braceCount++
67+
'}' -> braceCount--
68+
}
69+
idx++
70+
}
71+
if (braceCount == 0) {
72+
val body = content.substring(startIdx, idx - 1)
73+
if (body.lines().size >= 5) {
74+
results.add(methodName to body)
75+
}
76+
}
77+
}
78+
return results
79+
}
80+
81+
private fun normalizeCode(code: String): String {
82+
return code.lines()
83+
.map { it.trim() }
84+
.filter { it.isNotEmpty() && !it.startsWith("//") && !it.startsWith("*") }
85+
.joinToString("\n")
86+
.replace(Regex("""\s+"""), " ")
87+
}
88+
}

‎buildSrc/src/main/kotlin/datadog/gradle/plugin/lint/JavadocLinter.kt‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
package datadog.gradle.plugin.lint
2+
3+
import org.gradle.api.Plugin
4+
import org.gradle.api.Project
5+
import java.io.ByteArrayOutputStream
6+
7+
class JavadocLinter : Plugin<Project> {
8+
override fun apply(target: Project) {
9+
target.tasks.register("checkJavadocQuality") {
10+
group = "verification"
11+
description = "Detects empty @return and @param tags in Javadoc comments"
12+
13+
doLast {
14+
val changedFiles = getChangedJavaFiles(target)
15+
if (changedFiles.isEmpty()) {
16+
target.logger.lifecycle("checkJavadocQuality: No changed Java files to check")
17+
return@doLast
18+
}
19+
20+
val warnings = mutableListOf<String>()
21+
val emptyReturnPattern = Regex("""@return\s*$""")
22+
val emptyParamPattern = Regex("""@param\s+\w+\s*$""")
23+
24+
changedFiles.forEach { file ->
25+
if (!file.exists()) return@forEach
26+
val lines = file.readLines()
27+
val relPath = file.relativeTo(target.rootProject.projectDir).path
28+
29+
for (i in lines.indices) {
30+
val trimmed = lines[i].trim().removePrefix("* ").removePrefix("*")
31+
if (emptyReturnPattern.containsMatchIn(trimmed)) {
32+
// Check next non-empty line isn't a continuation
33+
val nextContent = lines.getOrNull(i + 1)?.trim()?.removePrefix("* ")?.removePrefix("*")?.trim() ?: ""
34+
if (nextContent.isEmpty() || nextContent.startsWith("@") || nextContent.startsWith("*/")) {
35+
warnings.add("JAVADOC: $relPath:${i + 1} — empty @return tag")
36+
}
37+
}
38+
if (emptyParamPattern.containsMatchIn(trimmed)) {
39+
val nextContent = lines.getOrNull(i + 1)?.trim()?.removePrefix("* ")?.removePrefix("*")?.trim() ?: ""
40+
if (nextContent.isEmpty() || nextContent.startsWith("@") || nextContent.startsWith("*/")) {
41+
warnings.add("JAVADOC: $relPath:${i + 1} — empty @param tag")
42+
}
43+
}
44+
}
45+
}
46+
47+
if (warnings.isNotEmpty()) {
48+
warnings.forEach { target.logger.warn(it) }
49+
target.logger.warn("checkJavadocQuality: ${warnings.size} Javadoc warning(s) found")
50+
} else {
51+
target.logger.lifecycle("checkJavadocQuality: No empty Javadoc tags found")
52+
}
53+
}
54+
}
55+
}
56+
57+
private fun getChangedJavaFiles(project: Project): List<java.io.File> {
58+
return try {
59+
val stdout = ByteArrayOutputStream()
60+
project.exec {
61+
commandLine("git", "diff", "--name-only", "--diff-filter=ACMR", "HEAD~1")
62+
standardOutput = stdout
63+
isIgnoreExitValue = true
64+
}
65+
stdout.toString().trim().lines()
66+
.filter { it.endsWith(".java") }
67+
.map { project.rootProject.file(it) }
68+
} catch (e: Exception) {
69+
emptyList()
70+
}
71+
}
72+
}

‎dd-java-agent/build.gradle‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,7 @@ dependencies {
353353
testImplementation project(':utils:test-agent-utils:decoder')
354354

355355
testImplementation libs.bundles.test.logging
356+
testImplementation 'com.tngtech.archunit:archunit-junit5:1.3.0'
356357
testImplementation libs.guava
357358
testImplementation libs.okhttp
358359
testImplementation group: 'io.opentracing', name: 'opentracing-util', version: '0.31.0'

‎dd-java-agent/src/test/java/datadog/trace/agent/ArchitectureTest.java‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
package datadog.trace.agent;
2+
3+
import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noClasses;
4+
5+
import com.tngtech.archunit.core.domain.JavaClasses;
6+
import com.tngtech.archunit.core.importer.ClassFileImporter;
7+
import com.tngtech.archunit.core.importer.ImportOption;
8+
import com.tngtech.archunit.lang.ArchRule;
9+
import org.junit.jupiter.api.BeforeAll;
10+
import org.junit.jupiter.api.Test;
11+
12+
/**
13+
* Architecture fitness tests enforcing module dependency rules. These rules prevent the most common
14+
* violations found during PR reviews: bootstrap depending on core, instrumentations reaching into
15+
* core internals, and forbidden JDK APIs in bootstrap code.
16+
*/
17+
class ArchitectureTest {
18+
19+
private static JavaClasses allClasses;
20+
21+
@BeforeAll
22+
static void importClasses() {
23+
allClasses =
24+
new ClassFileImporter()
25+
.withImportOption(ImportOption.Predefined.DO_NOT_INCLUDE_TESTS)
26+
.importPackages("datadog.trace");
27+
}
28+
29+
@Test
30+
void bootstrapShouldNotDependOnCore() {
31+
ArchRule rule =
32+
noClasses()
33+
.that()
34+
.resideInAPackage("datadog.trace.bootstrap..")
35+
.should()
36+
.dependOnClassesThat()
37+
.resideInAPackage("datadog.trace.core..")
38+
.because("Bootstrap classes load before core and must not depend on core internals");
39+
40+
rule.check(allClasses);
41+
}
42+
43+
@Test
44+
void instrumentationShouldNotDependOnCoreInternals() {
45+
ArchRule rule =
46+
noClasses()
47+
.that()
48+
.resideInAPackage("datadog.trace.instrumentation..")
49+
.should()
50+
.dependOnClassesThat()
51+
.resideInAPackage("datadog.trace.core.internal..")
52+
.because("Instrumentations should use internal-api, not core internals directly");
53+
54+
rule.check(allClasses);
55+
}
56+
57+
@Test
58+
void bootstrapShouldNotUseJavaUtilLogging() {
59+
ArchRule rule =
60+
noClasses()
61+
.that()
62+
.resideInAPackage("datadog.trace.bootstrap..")
63+
.should()
64+
.dependOnClassesThat()
65+
.resideInAPackage("java.util.logging..")
66+
.because(
67+
"java.util.logging locks in the log manager before the app configures it"
68+
+ " (see docs/bootstrap_design_guidelines.md)");
69+
70+
rule.check(allClasses);
71+
}
72+
73+
@Test
74+
void bootstrapShouldNotUseJavaxManagement() {
75+
ArchRule rule =
76+
noClasses()
77+
.that()
78+
.resideInAPackage("datadog.trace.bootstrap..")
79+
.should()
80+
.dependOnClassesThat()
81+
.resideInAPackage("javax.management..")
82+
.because(
83+
"javax.management causes class loading issues in premain"
84+
+ " (see docs/bootstrap_design_guidelines.md)");
85+
86+
rule.check(allClasses);
87+
}
88+
}

‎gradle/forbiddenApiFilters/main.txt‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,3 +54,11 @@ java.lang.reflect.Field#setLong(java.lang.Object,long)
5454
java.lang.reflect.Field#setFloat(java.lang.Object,float)
5555
java.lang.reflect.Field#setDouble(java.lang.Object,double)
5656
java.lang.invoke.MethodHandles.Lookup#unreflectSetter(java.lang.reflect.Field)
57+
58+
# avoid unnecessary boxing of primitives — use primitive types directly
59+
@defaultMessage Avoid unnecessary boxing. Use primitive types (long, int, double) instead of boxed types when possible.
60+
java.lang.Long#valueOf(long)
61+
java.lang.Integer#valueOf(int)
62+
java.lang.Double#valueOf(double)
63+
java.lang.Float#valueOf(float)
64+
java.lang.Boolean#valueOf(boolean)

0 commit comments

Comments
 (0)