Add 12 static checks derived from PR review analysis

.github/workflows/typos.yaml

@@ -0,0 +1,15 @@
+name: Typos Check
+on:
+  pull_request:
+    branches:
+    - master
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+jobs:
+  typos:
+    name: Typos Check
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: crate-ci/typos@v1

.gitlab-ci.yml

@@ -468,6 +468,17 @@ check_build_src:
   variables:
     GRADLE_TARGET: ":buildSrc:build"
 
+check_pr_hygiene:
+  extends: .gradle_build
+  needs: []
+  stage: tests
+  script:
+    - bash scripts/check-ci-debug-flags.sh || true
+    - bash scripts/check-extraneous-files.sh || true
+  rules:
+    - if: $CI_PIPELINE_SOURCE == "merge_request_event"
+    - if: $CI_COMMIT_BRANCH && $CI_COMMIT_BRANCH != "master"
+
 check_base:
   extends: .check_job
   variables:

.typos.toml

@@ -0,0 +1,31 @@
+[files]
+extend-exclude = [
+  "build/",
+  "workspace/",
+  ".gradle/",
+  "gradle.lockfile",
+  "settings-gradle.lockfile",
+  "LICENSE-3rdparty.csv",
+  "*.jar",
+  "*.class",
+  "*.so",
+  "*.dylib",
+]
+extend-include = [
+  "*.java",
+  "*.kt",
+  "*.groovy",
+  "*.md",
+  "*.yaml",
+  "*.yml",
+  "*.gradle",
+  "*.gradle.kts",
+]
+
+[default.extend-words]
+ot = "ot"
+ba = "ba"
+nd = "nd"
+crate = "crate"
+hashi = "hashi"
+pullrequest = "pullrequest"

build.gradle.kts

@@ -9,6 +9,12 @@ plugins {
   id("dd-trace-java.tracer-version")
   id("dd-trace-java.dump-hanged-test")
   id("dd-trace-java.config-inversion-linter")
+  id("dd-trace-java.empty-instrumentation-linter")
+  id("dd-trace-java.unnecessary-else-linter")
+  id("dd-trace-java.naming-convention-linter")
+  id("dd-trace-java.javadoc-linter")
+  id("dd-trace-java.copy-paste-detector")
+  id("dd-trace-java.assertj-preference-linter")
   id("dd-trace-java.ci-jobs")
 
   id("com.diffplug.spotless") version "8.2.1"

buildSrc/build.gradle.kts

@@ -59,6 +59,36 @@ gradlePlugin {
       id = "dd-trace-java.instrumentation-naming"
       implementationClass = "datadog.gradle.plugin.naming.InstrumentationNamingPlugin"
     }
+
+    create("empty-instrumentation-linter") {
+      id = "dd-trace-java.empty-instrumentation-linter"
+      implementationClass = "datadog.gradle.plugin.lint.EmptyInstrumentationLinter"
+    }
+
+    create("unnecessary-else-linter") {
+      id = "dd-trace-java.unnecessary-else-linter"
+      implementationClass = "datadog.gradle.plugin.lint.UnnecessaryElseLinter"
+    }
+
+    create("naming-convention-linter") {
+      id = "dd-trace-java.naming-convention-linter"
+      implementationClass = "datadog.gradle.plugin.lint.NamingConventionLinter"
+    }
+
+    create("javadoc-linter") {
+      id = "dd-trace-java.javadoc-linter"
+      implementationClass = "datadog.gradle.plugin.lint.JavadocLinter"
+    }
+
+    create("copy-paste-detector") {
+      id = "dd-trace-java.copy-paste-detector"
+      implementationClass = "datadog.gradle.plugin.lint.CopyPasteDetectorPlugin"
+    }
+
+    create("assertj-preference-linter") {
+      id = "dd-trace-java.assertj-preference-linter"
+      implementationClass = "datadog.gradle.plugin.lint.AssertJPreferenceLinter"
+    }
   }
 }
 

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

@@ -0,0 +1,60 @@
+package datadog.gradle.plugin.lint
+
+import org.gradle.api.Plugin
+import org.gradle.api.Project
+import java.io.ByteArrayOutputStream
+
+class AssertJPreferenceLinter : Plugin<Project> {
+  override fun apply(target: Project) {
+    target.tasks.register("checkAssertJPreference") {
+      group = "verification"
+      description = "Flags JUnit assertion imports in new test files — prefer AssertJ"
+
+      doLast {
+        val newTestFiles = getNewTestFiles(target)
+        if (newTestFiles.isEmpty()) {
+          target.logger.lifecycle("checkAssertJPreference: No new test files to check")
+          return@doLast
+        }
+
+        val warnings = mutableListOf<String>()
+        val junitAssertImport = Regex("""import\s+(?:static\s+)?org\.junit\.jupiter\.api\.Assertions""")
+
+        newTestFiles.forEach { file ->
+          if (!file.exists()) return@forEach
+          val lines = file.readLines()
+          val relPath = file.relativeTo(target.rootProject.projectDir).path
+
+          for (i in lines.indices) {
+            if (junitAssertImport.containsMatchIn(lines[i])) {
+              warnings.add("STYLE: $relPath:${i + 1} — Prefer AssertJ (org.assertj.core.api.Assertions) over JUnit assertions for richer API")
+            }
+          }
+        }
+
+        if (warnings.isNotEmpty()) {
+          warnings.forEach { target.logger.warn(it) }
+          target.logger.warn("checkAssertJPreference: ${warnings.size} file(s) using JUnit assertions instead of AssertJ")
+        } else {
+          target.logger.lifecycle("checkAssertJPreference: All new test files use AssertJ")
+        }
+      }
+    }
+  }
+
+  private fun getNewTestFiles(project: Project): List<java.io.File> {
+    return try {
+      val stdout = ByteArrayOutputStream()
+      project.exec {
+        commandLine("git", "diff", "--name-only", "--diff-filter=A", "origin/master...HEAD")
+        standardOutput = stdout
+        isIgnoreExitValue = true
+      }
+      stdout.toString().trim().lines()
+        .filter { it.endsWith(".java") && it.contains("src/test/") }
+        .map { project.rootProject.file(it) }
+    } catch (e: Exception) {
+      emptyList()
+    }
+  }
+}

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

@@ -0,0 +1,88 @@
+package datadog.gradle.plugin.lint
+
+import org.gradle.api.Plugin
+import org.gradle.api.Project
+import java.io.File
+
+class CopyPasteDetectorPlugin : Plugin<Project> {
+  override fun apply(target: Project) {
+    target.tasks.register("checkCodeDuplication") {
+      group = "verification"
+      description = "Detect copy-pasted code using hash-based method body comparison"
+
+      doLast {
+        val dirs = listOf("dd-java-agent/instrumentation", "dd-trace-core", "internal-api")
+          .map { target.rootProject.file(it) }
+          .filter { it.exists() }
+
+        val methodHashes = mutableMapOf<Int, MutableList<String>>()
+        var totalFiles = 0
+
+        dirs.forEach { dir ->
+          dir.walkTopDown()
+            .filter { it.isFile && it.extension == "java" && !it.path.contains("/build/") && !it.path.contains("/generated/") }
+            .forEach { file ->
+              totalFiles++
+              extractMethodBodies(file).forEach { (name, body) ->
+                val normalized = normalizeCode(body)
+                if (normalized.length > 200) {
+                  val hash = normalized.hashCode()
+                  val location = "${file.relativeTo(target.rootProject.projectDir).path}:$name"
+                  methodHashes.getOrPut(hash) { mutableListOf() }.add(location)
+                }
+              }
+            }
+        }
+
+        val duplicates = methodHashes.filter { it.value.size > 1 }
+        if (duplicates.isNotEmpty()) {
+          target.logger.warn("CPD: Found ${duplicates.size} group(s) of duplicate methods across $totalFiles files:")
+          duplicates.entries.take(20).forEach { (_, locations) ->
+            target.logger.warn("  DUPLICATE GROUP (${locations.size} copies):")
+            locations.forEach { loc -> target.logger.warn("    - $loc") }
+          }
+          if (duplicates.size > 20) {
+            target.logger.warn("  ... and ${duplicates.size - 20} more groups")
+          }
+        } else {
+          target.logger.lifecycle("CPD: No significant duplicates found across $totalFiles files")
+        }
+      }
+    }
+  }
+
+  private fun extractMethodBodies(file: File): List<Pair<String, String>> {
+    val content = file.readText()
+    val results = mutableListOf<Pair<String, String>>()
+    val methodPattern = Regex("""(?:public|private|protected|static|final|synchronized|\s)+[\w<>\[\],\s]+\s+(\w+)\s*\([^)]*\)[^{]*\{""")
+
+    methodPattern.findAll(content).forEach { match ->
+      val methodName = match.groupValues[1]
+      val startIdx = match.range.last + 1
+      var braceCount = 1
+      var idx = startIdx
+      while (idx < content.length && braceCount > 0) {
+        when (content[idx]) {
+          '{' -> braceCount++
+          '}' -> braceCount--
+        }
+        idx++
+      }
+      if (braceCount == 0) {
+        val body = content.substring(startIdx, idx - 1)
+        if (body.lines().size >= 5) {
+          results.add(methodName to body)
+        }
+      }
+    }
+    return results
+  }
+
+  private fun normalizeCode(code: String): String {
+    return code.lines()
+      .map { it.trim() }
+      .filter { it.isNotEmpty() && !it.startsWith("//") && !it.startsWith("*") }
+      .joinToString("\n")
+      .replace(Regex("""\s+"""), " ")
+  }
+}

buildSrc/src/main/kotlin/datadog/gradle/plugin/lint/EmptyInstrumentationLinter.kt

@@ -0,0 +1,118 @@
+package datadog.gradle.plugin.lint
+
+import org.gradle.api.GradleException
+import org.gradle.api.Plugin
+import org.gradle.api.Project
+import java.io.ByteArrayOutputStream
+
+class EmptyInstrumentationLinter : Plugin<Project> {
+  override fun apply(target: Project) {
+    target.tasks.register("checkEmptyInstrumentations") {
+      group = "verification"
+      description = "Detects empty instrumentation stub classes with no transform() calls in methodAdvice()"
+
+      doLast {
+        val instrumentationsDir = target.rootProject.file("dd-java-agent/instrumentation")
+
+        if (!instrumentationsDir.exists() || !instrumentationsDir.isDirectory) {
+          throw GradleException(
+            "Instrumentations directory not found: ${instrumentationsDir.absolutePath}"
+          )
+        }
+
+        // Only check files changed on this branch to avoid flagging existing stubs
+        val changedFiles = getChangedInstrumentationFiles(target, instrumentationsDir)
+
+        val violations = mutableListOf<String>()
+        val hasAdvicePattern = Regex("""implements\s+[^{]*\b(?:HasMethodAdvice|HasAdvice)\b""")
+        val methodAdviceStartPattern = Regex("""(?:public\s+)?(?:\S+\s+)?methodAdvice\s*\(""")
+        val transformCallPattern = Regex("""\btransform\s*\(""")
+
+        val filesToCheck = if (changedFiles != null) {
+          changedFiles.filter { it.isFile && it.name.endsWith(".java") }
+        } else {
+          // Fallback: check all files if git diff fails
+          instrumentationsDir.walk()
+            .filter { it.isFile && it.name.endsWith(".java") }
+            .toList()
+        }
+
+        filesToCheck.forEach { file ->
+          val lines = file.readLines()
+          val content = lines.joinToString("\n")
+
+          if (!hasAdvicePattern.containsMatchIn(content)) return@forEach
+
+          var methodAdviceLineIndex = -1
+          for (i in lines.indices) {
+            if (methodAdviceStartPattern.containsMatchIn(lines[i])) {
+              methodAdviceLineIndex = i
+              break
+            }
+          }
+
+          if (methodAdviceLineIndex < 0) return@forEach
+
+          var braceDepth = 0
+          var methodBodyStart = -1
+          val methodBodyLines = mutableListOf<String>()
+
+          for (i in methodAdviceLineIndex until lines.size) {
+            val line = lines[i]
+            for (ch in line) {
+              when (ch) {
+                '{' -> {
+                  braceDepth++
+                  if (braceDepth == 1) methodBodyStart = i
+                }
+                '}' -> {
+                  braceDepth--
+                  if (braceDepth == 0 && methodBodyStart >= 0) {
+                    break
+                  }
+                }
+              }
+            }
+            if (methodBodyStart >= 0) {
+              methodBodyLines.add(line)
+            }
+            if (braceDepth == 0 && methodBodyStart >= 0) break
+          }
+
+          val methodBody = methodBodyLines.joinToString("\n")
+          if (!transformCallPattern.containsMatchIn(methodBody)) {
+            val classNamePattern = Regex("""class\s+(\w+)""")
+            val className = classNamePattern.find(content)?.groupValues?.get(1) ?: "<unknown>"
+            val relPath = file.relativeTo(target.rootProject.projectDir).path
+            violations.add("EMPTY STUB: $relPath:${methodAdviceLineIndex + 1} — $className.methodAdvice() contains no transform() calls")
+          }
+        }
+
+        if (violations.isNotEmpty()) {
+          violations.forEach { target.logger.error(it) }
+          throw GradleException("Found ${violations.size} new empty instrumentation stub(s)! See errors above.")
+        } else {
+          target.logger.lifecycle("checkEmptyInstrumentations: no new empty stubs found")
+        }
+      }
+    }
+  }
+
+  private fun getChangedInstrumentationFiles(project: Project, instrumentationsDir: java.io.File): List<java.io.File>? {
+    return try {
+      val stdout = ByteArrayOutputStream()
+      project.exec {
+        commandLine("git", "diff", "--name-only", "--diff-filter=ACM", "origin/master...HEAD")
+        standardOutput = stdout
+        isIgnoreExitValue = true
+      }
+      stdout.toString().trim().lines()
+        .filter { it.endsWith(".java") && it.startsWith("dd-java-agent/instrumentation/") }
+        .map { project.rootProject.file(it) }
+        .filter { it.exists() }
+    } catch (e: Exception) {
+      project.logger.warn("checkEmptyInstrumentations: could not get changed files, checking all — ${e.message}")
+      null
+    }
+  }
+}