revert: feat: check binary compatibility when there are duplicate classes on the classpath.

src/functionalTest/groovy/com/autonomousapps/AbstractFunctionalSpec.groovy

@@ -39,15 +39,15 @@ abstract class AbstractFunctionalSpec extends Specification {
    */
   private static boolean isCi = System.getenv("CI") == "true"
 
-  def cleanup() {
-    // Delete fixtures on CI to prevent disk space growing out of bounds
-    if (gradleProject != null && isCi) {
-      try {
-        gradleProject.rootDir.deleteDir()
-      } catch (Throwable t) {
-      }
-    }
-  }
+//  def cleanup() {
+//    // Delete fixtures on CI to prevent disk space growing out of bounds
+//    if (gradleProject != null && isCi) {
+//      try {
+//        gradleProject.rootDir.deleteDir()
+//      } catch (Throwable t) {
+//      }
+//    }
+//  }
 
   protected static Boolean quick() {
     return System.getProperty('com.autonomousapps.quick').toBoolean()

src/functionalTest/groovy/com/autonomousapps/jvm/DuplicateClasspathSpec.groovy

@@ -3,6 +3,8 @@ package com.autonomousapps.jvm
 import com.autonomousapps.jvm.projects.BinaryIncompatibilityProject
 import com.autonomousapps.jvm.projects.DuplicateClasspathProject
 import com.autonomousapps.utils.Colors
+import org.gradle.testkit.runner.TaskOutcome
+import spock.lang.PendingFeature
 
 import static com.autonomousapps.advice.truth.BuildHealthSubject.buildHealth
 import static com.autonomousapps.utils.Runner.build
@@ -20,17 +22,17 @@ final class DuplicateClasspathSpec extends AbstractJvmSpec {
     when:
     // This first invocation fixes the dependency declarations
     build(gradleVersion, gradleProject.rootDir, ':consumer:fixDependencies')
-    // This used to fail because of classpath duplication and the wrong dep getting loaded first. Recent improvements
-    // have improved this case, however.
-    build(gradleVersion, gradleProject.rootDir, 'buildHealth')
+    // This fails because of classpath duplication and the wrong dep getting loaded first
+    def result = buildAndFail(gradleVersion, gradleProject.rootDir, 'buildHealth')
 
     then:
-    assertThat(gradleProject.rootDir.toPath().resolve('consumer/build.gradle').text).contains(
-      '''\
-        dependencies {
-          implementation project(':producer-2')
-        }'''.stripIndent()
-    )
+    assertThat(result.task(':consumer:compileJava').outcome).isEqualTo(TaskOutcome.FAILED)
+//    assertThat(gradleProject.rootDir.toPath().resolve('consumer/build.gradle').text).contains(
+//      '''\
+//        dependencies {
+//          implementation project(':producer-2')
+//        }'''.stripIndent()
+//    )
 
     where:
     gradleVersion << [GRADLE_LATEST]
@@ -69,6 +71,7 @@ final class DuplicateClasspathSpec extends AbstractJvmSpec {
     gradleVersion << [GRADLE_LATEST]
   }
 
+  @PendingFeature(reason = "This feature was reverted")
   def "can report on which of the duplicates is needed for binary compatibility (#gradleVersion)"() {
     given:
     def project = new BinaryIncompatibilityProject()
@@ -123,6 +126,7 @@ final class DuplicateClasspathSpec extends AbstractJvmSpec {
     gradleVersion << [GRADLE_LATEST]
   }
 
+  @PendingFeature(reason = "This feature was reverted")
   def "suggests removing a binary-incompatible duplicate (#gradleVersion)"() {
     given:
     def project = new BinaryIncompatibilityProject(true)

src/functionalTest/groovy/com/autonomousapps/jvm/projects/DuplicateClasspathProject.groovy

@@ -176,6 +176,9 @@ final class DuplicateClasspathProject extends AbstractProject {
 
   private final Set<Advice> consumerAdvice = [
     Advice.ofRemove(projectCoordinates(':unused'), 'implementation'),
+    // This is actually bad advice, but we can't detect it without re-reverting the change to detect binary
+    // incompatibilities.
+    Advice.ofAdd(projectCoordinates(':producer-1'), 'implementation')
   ]
 
   final Set<ProjectAdvice> expectedProjectAdvice = [

src/main/kotlin/com/autonomousapps/internal/asm.kt

@@ -9,7 +9,6 @@ import com.autonomousapps.internal.kotlin.AccessFlags
 import com.autonomousapps.internal.utils.METHOD_DESCRIPTOR_REGEX
 import com.autonomousapps.internal.utils.efficient
 import com.autonomousapps.internal.utils.genericTypes
-import com.autonomousapps.model.internal.intermediates.producer.Member
 import com.autonomousapps.model.internal.intermediates.consumer.MemberAccess
 import kotlinx.metadata.jvm.Metadata
 import org.gradle.api.logging.Logger
@@ -26,13 +25,14 @@ internal class ClassNameAndAnnotationsVisitor(private val logger: Logger) : Clas
   private lateinit var access: Access
   private var outerClassName: String? = null
   private var superClassName: String? = null
-  private var interfaces: Set<String>? = null
+
+  // private var interfaces: Set<String>? = null
   private val retentionPolicyHolder = AtomicReference("")
   private var isAnnotation = false
   private val methods = mutableSetOf<Method>()
   private val innerClasses = mutableSetOf<String>()
-  private val effectivelyPublicFields = mutableSetOf<Member.Field>()
-  private val effectivelyPublicMethods = mutableSetOf<Member.Method>()
+  // private val effectivelyPublicFields = mutableSetOf<Member.Field>()
+  // private val effectivelyPublicMethods = mutableSetOf<Member.Method>()
 
   private var methodCount = 0
   private var fieldCount = 0
@@ -49,16 +49,16 @@ internal class ClassNameAndAnnotationsVisitor(private val logger: Logger) : Clas
       className = className,
       outerClassName = outerClassName,
       superClassName = superClassName!!,
-      interfaces = interfaces.orEmpty(),
+      // interfaces = interfaces.orEmpty(),
       retentionPolicy = retentionPolicyHolder.get(),
       isAnnotation = isAnnotation,
       hasNoMembers = hasNoMembers,
       access = access,
       methods = methods.efficient(),
       innerClasses = innerClasses.efficient(),
       constantClasses = constantClasses.efficient(),
-      effectivelyPublicFields = effectivelyPublicFields,
-      effectivelyPublicMethods = effectivelyPublicMethods,
+      // effectivelyPublicFields = effectivelyPublicFields,
+      // effectivelyPublicMethods = effectivelyPublicMethods,
     )
   }
 
@@ -72,7 +72,7 @@ internal class ClassNameAndAnnotationsVisitor(private val logger: Logger) : Clas
   ) {
     // This _must_ not be canonicalized, unless we also change accesses to be dotty instead of slashy
     this.superClassName = superName
-    this.interfaces = interfaces?.toSortedSet().orEmpty()
+    // this.interfaces = interfaces?.toSortedSet().orEmpty()
 
     className = canonicalize(name)
     if (interfaces?.contains("java/lang/annotation/Annotation") == true) {
@@ -107,15 +107,15 @@ internal class ClassNameAndAnnotationsVisitor(private val logger: Logger) : Clas
       methods.add(Method(descriptor))
     }
 
-    if (isEffectivelyPublic(access)) {
-      effectivelyPublicMethods.add(
-        Member.Method(
-          access = access,
-          name = name,
-          descriptor = descriptor,
-        )
-      )
-    }
+    // if (isEffectivelyPublic(access)) {
+    //   effectivelyPublicMethods.add(
+    //     Member.Method(
+    //       access = access,
+    //       name = name,
+    //       descriptor = descriptor,
+    //     )
+    //   )
+    // }
 
     return null
   }
@@ -131,15 +131,15 @@ internal class ClassNameAndAnnotationsVisitor(private val logger: Logger) : Clas
       constantClasses.add(name)
     }
 
-    if (isEffectivelyPublic(access)) {
-      effectivelyPublicFields.add(
-        Member.Field(
-          access = access,
-          name = name,
-          descriptor = descriptor,
-        )
-      )
-    }
+    // if (isEffectivelyPublic(access)) {
+    //   effectivelyPublicFields.add(
+    //     Member.Field(
+    //       access = access,
+    //       name = name,
+    //       descriptor = descriptor,
+    //     )
+    //   )
+    // }
 
     return null
   }

src/main/kotlin/com/autonomousapps/internal/models.kt

@@ -6,8 +6,6 @@ import com.autonomousapps.internal.asm.Opcodes
 import com.autonomousapps.internal.utils.efficient
 import com.autonomousapps.internal.utils.filterNotToSet
 import com.autonomousapps.internal.utils.mapToSet
-import com.autonomousapps.model.internal.intermediates.producer.BinaryClass
-import com.autonomousapps.model.internal.intermediates.producer.Member
 import com.squareup.moshi.JsonClass
 import java.lang.annotation.RetentionPolicy
 import java.util.regex.Pattern
@@ -45,22 +43,22 @@ internal data class AnalyzedClass(
   val methods: Set<Method>,
   val innerClasses: Set<String>,
   val constantFields: Set<String>,
-  val binaryClass: BinaryClass,
+  // val binaryClass: BinaryClass,
 ) : Comparable<AnalyzedClass> {
   constructor(
     className: String,
     outerClassName: String?,
     superClassName: String,
-    interfaces: Set<String>,
+    // interfaces: Set<String>,
     retentionPolicy: String?,
     isAnnotation: Boolean,
     hasNoMembers: Boolean,
     access: Access,
     methods: Set<Method>,
     innerClasses: Set<String>,
     constantClasses: Set<String>,
-    effectivelyPublicFields: Set<Member.Field>,
-    effectivelyPublicMethods: Set<Member.Method>,
+    // effectivelyPublicFields: Set<Member.Field>,
+    // effectivelyPublicMethods: Set<Member.Method>,
   ) : this(
     className = className,
     outerClassName = outerClassName,
@@ -71,13 +69,13 @@ internal data class AnalyzedClass(
     methods = methods,
     innerClasses = innerClasses,
     constantFields = constantClasses,
-    binaryClass = BinaryClass(
-      className = className.replace('.', '/'),
-      superClassName = superClassName.replace('.', '/'),
-      interfaces = interfaces,
-      effectivelyPublicFields = effectivelyPublicFields,
-      effectivelyPublicMethods = effectivelyPublicMethods,
-    ),
+    // binaryClass = BinaryClass(
+    //   className = className.replace('.', '/'),
+    //   superClassName = superClassName.replace('.', '/'),
+    //   interfaces = interfaces,
+    //   effectivelyPublicFields = effectivelyPublicFields,
+    //   effectivelyPublicMethods = effectivelyPublicMethods,
+    // ),
   )
 
   companion object {