fix: address code review findings — hardcoded SHA, incremental checks, FreezingArchRule

- UnnecessaryElseLinter: replace hardcoded SHA with origin/master...HEAD
- JavadocLinter, AssertJPreferenceLinter: use origin/master...HEAD instead of HEAD~1
- EmptyInstrumentationLinter: make incremental (only check changed files)
- ArchitectureTest: wrap rules with FreezingArchRule to baseline existing violations
- EmptyInstrumentationLinter: fix duplicate-line bug in brace tracking

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: bm1549

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

@@ -46,7 +46,7 @@ class AssertJPreferenceLinter : Plugin<Project> {
     return try {
       val stdout = ByteArrayOutputStream()
       project.exec {
-        commandLine("git", "diff", "--name-only", "--diff-filter=A", "HEAD~1")
+        commandLine("git", "diff", "--name-only", "--diff-filter=A", "origin/master...HEAD")
         standardOutput = stdout
         isIgnoreExitValue = true
       }

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

@@ -3,6 +3,7 @@ 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) {
@@ -19,74 +20,99 @@ class EmptyInstrumentationLinter : Plugin<Project> {
           )
         }
 
+        // 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*\(""")
 
-        instrumentationsDir.walk()
-          .filter { it.isFile && it.name.endsWith(".java") }
-          .forEach { file ->
-            val lines = file.readLines()
-            val content = lines.joinToString("\n")
+        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()
+        }
 
-            if (!hasAdvicePattern.containsMatchIn(content)) return@forEach
+        filesToCheck.forEach { file ->
+          val lines = file.readLines()
+          val content = lines.joinToString("\n")
 
-            // Find methodAdvice( method and check its body for transform( calls
-            var methodAdviceLineIndex = -1
-            for (i in lines.indices) {
-              if (methodAdviceStartPattern.containsMatchIn(lines[i])) {
-                methodAdviceLineIndex = i
-                break
-              }
+          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
+          if (methodAdviceLineIndex < 0) return@forEach
 
-            // Extract method body by tracking brace depth
-            var braceDepth = 0
-            var methodBodyStart = -1
-            val methodBodyLines = mutableListOf<String>()
+          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) {
-                      methodBodyLines.add(line)
-                      break
-                    }
+          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 (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("Empty instrumentation stubs found! See errors above.")
+          throw GradleException("Found ${violations.size} new empty instrumentation stub(s)! See errors above.")
         } else {
-          target.logger.lifecycle("✓ No empty instrumentation stubs found")
+          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
+    }
+  }
 }

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

@@ -58,7 +58,7 @@ class JavadocLinter : Plugin<Project> {
     return try {
       val stdout = ByteArrayOutputStream()
       project.exec {
-        commandLine("git", "diff", "--name-only", "--diff-filter=ACMR", "HEAD~1")
+        commandLine("git", "diff", "--name-only", "--diff-filter=ACMR", "origin/master...HEAD")
         standardOutput = stdout
         isIgnoreExitValue = true
       }

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

@@ -10,8 +10,7 @@ class UnnecessaryElseLinter : Plugin<Project> {
       description = "Scan changed Java files for unnecessary else blocks after return/throw/continue/break"
 
       doLast {
-        val baseSha = "9b933669729ea8a7af00f5cf3c36b6720ec433bd"
-        val changedFiles = getChangedJavaFiles(target, baseSha)
+        val changedFiles = getChangedJavaFiles(target)
         val repoRoot = target.rootProject.projectDir.toPath()
         val warnings = mutableListOf<String>()
 
@@ -33,9 +32,9 @@ class UnnecessaryElseLinter : Plugin<Project> {
   }
 }
 
-private fun getChangedJavaFiles(project: Project, baseSha: String): List<java.io.File> {
+private fun getChangedJavaFiles(project: Project): List<java.io.File> {
   return try {
-    val process = ProcessBuilder("git", "diff", "--name-only", baseSha, "HEAD")
+    val process = ProcessBuilder("git", "diff", "--name-only", "--diff-filter=ACM", "origin/master...HEAD")
       .directory(project.rootProject.projectDir)
       .redirectErrorStream(true)
       .start()

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

@@ -6,13 +6,16 @@
 import com.tngtech.archunit.core.importer.ClassFileImporter;
 import com.tngtech.archunit.core.importer.ImportOption;
 import com.tngtech.archunit.lang.ArchRule;
+import com.tngtech.archunit.library.freeze.FreezingArchRule;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 
 /**
  * Architecture fitness tests enforcing module dependency rules. These rules prevent the most common
  * violations found during PR reviews: bootstrap depending on core, instrumentations reaching into
  * core internals, and forbidden JDK APIs in bootstrap code.
+ *
+ * <p>Uses {@link FreezingArchRule} to baseline existing violations so only new violations fail.
  */
 class ArchitectureTest {
 
@@ -37,7 +40,7 @@ void bootstrapShouldNotDependOnCore() {
             .resideInAPackage("datadog.trace.core..")
             .because("Bootstrap classes load before core and must not depend on core internals");
 
-    rule.check(allClasses);
+    FreezingArchRule.freeze(rule).check(allClasses);
   }
 
   @Test
@@ -51,7 +54,7 @@ void instrumentationShouldNotDependOnCoreInternals() {
             .resideInAPackage("datadog.trace.core.internal..")
             .because("Instrumentations should use internal-api, not core internals directly");
 
-    rule.check(allClasses);
+    FreezingArchRule.freeze(rule).check(allClasses);
   }
 
   @Test
@@ -67,7 +70,7 @@ void bootstrapShouldNotUseJavaUtilLogging() {
                 "java.util.logging locks in the log manager before the app configures it"
                     + " (see docs/bootstrap_design_guidelines.md)");
 
-    rule.check(allClasses);
+    FreezingArchRule.freeze(rule).check(allClasses);
   }
 
   @Test
@@ -83,6 +86,6 @@ void bootstrapShouldNotUseJavaxManagement() {
                 "javax.management causes class loading issues in premain"
                     + " (see docs/bootstrap_design_guidelines.md)");
 
-    rule.check(allClasses);
+    FreezingArchRule.freeze(rule).check(allClasses);
   }
 }