refactor: move code from BinaryClassCapability to the checker.

Better encapsulation, less complexity in the Capability.kt file. If further changes are needed, they can all be in one place.

Author: autonomousapps

src/main/kotlin/com/autonomousapps/internal/binary/BinaryCompatibilityChecker.kt

@@ -2,7 +2,9 @@
 // SPDX-License-Identifier: Apache-2.0
 package com.autonomousapps.internal.binary
 
+import com.autonomousapps.internal.strings.dotty
 import com.autonomousapps.internal.unsafeLazy
+import com.autonomousapps.internal.utils.efficient
 import com.autonomousapps.internal.utils.filterToOrderedSet
 import com.autonomousapps.internal.utils.mapToOrderedSet
 import com.autonomousapps.internal.utils.mapToSet
@@ -13,6 +15,10 @@ import com.autonomousapps.model.internal.intermediates.consumer.MemberAccess
 import com.autonomousapps.model.internal.intermediates.producer.BinaryClass
 import com.autonomousapps.visitor.GraphViewVisitor
 
+/**
+ * TODO(tsr): there are [reports](https://github.com/autonomousapps/dependency-analysis-gradle-plugin/issues/1604) that
+ * this analysis is blowing up heap usage and leading to OOMs.
+ */
 internal class BinaryCompatibilityChecker(
   private val coordinates: Coordinates,
   private val binaryClassCapability: BinaryClassCapability,
@@ -25,6 +31,28 @@ internal class BinaryCompatibilityChecker(
     val isBinaryCompatible: Boolean,
   )
 
+  private data class PartitionResult(
+    val matchingClasses: Set<BinaryClass>,
+    val nonMatchingClasses: Set<BinaryClass>,
+  ) {
+
+    companion object {
+      fun empty(): PartitionResult = PartitionResult(emptySet(), emptySet())
+    }
+
+    class Builder {
+      val matchingClasses = sortedSetOf<BinaryClass>()
+      val nonMatchingClasses = sortedSetOf<BinaryClass>()
+
+      fun build(): PartitionResult {
+        return PartitionResult(
+          matchingClasses = matchingClasses.efficient(),
+          nonMatchingClasses = nonMatchingClasses.efficient(),
+        )
+      }
+    }
+  }
+
   val result: Result? by unsafeLazy { compute() }
 
   private fun compute(): Result? {
@@ -77,7 +105,7 @@ internal class BinaryCompatibilityChecker(
     )
   }
 
-  private fun Set<BinaryClassCapability.PartitionResult>.reduce(): BinaryClassCapability.PartitionResult {
+  private fun Set<PartitionResult>.reduce(): PartitionResult {
     val matches = sortedSetOf<BinaryClass>()
     val nonMatches = sortedSetOf<BinaryClass>()
 
@@ -86,7 +114,7 @@ internal class BinaryCompatibilityChecker(
       nonMatches.addAll(result.nonMatchingClasses)
     }
 
-    return BinaryClassCapability.PartitionResult(
+    return PartitionResult(
       matchingClasses = matches.reduce(),
       nonMatchingClasses = nonMatches.reduce(),
     )
@@ -115,4 +143,95 @@ internal class BinaryCompatibilityChecker(
 
     return builders.values.mapToOrderedSet { it.build() }
   }
+
+  private fun BinaryClassCapability.findMatchingClasses(memberAccess: MemberAccess): PartitionResult {
+    val relevant = findRelevantBinaryClasses(memberAccess)
+
+    // lenient
+    if (relevant.isEmpty()) return PartitionResult.empty()
+
+    return relevant
+      .map { bin -> bin.partition(memberAccess) }
+      .fold(PartitionResult.Builder()) { acc, (match, nonMatch) ->
+        acc.apply {
+          match?.let { matchingClasses.add(it) }
+          nonMatch?.let { nonMatchingClasses.add(it) }
+        }
+      }
+      .build()
+  }
+
+  /**
+   * Example:
+   * 1. [memberAccess] is for `groovy/lang/MetaClass#getProperty`.
+   * 2. That method is actually provided by `groovy/lang/MetaObjectProtocol`, which `groovy/lang/MetaClass` implements.
+   *
+   * All of the above ("this" class, its super class, and its interfaces) are relevant for search purposes. Note we
+   * don't inspect the member names for this check.
+   */
+  private fun BinaryClassCapability.findRelevantBinaryClasses(memberAccess: MemberAccess): Set<BinaryClass> {
+    // direct references
+    val relevant = binaryClasses.filterTo(mutableSetOf()) { bin ->
+      bin.className == memberAccess.owner.dotty()
+    }
+
+    // Walk up the class hierarchy
+    fun walkUp(): Int {
+      binaryClasses.filterTo(relevant) { bin ->
+        bin.className in relevant.map { it.superClassName }
+          || bin.className in relevant.flatMap { it.interfaces }
+      }
+      return relevant.size
+    }
+
+    // TODO(tsr): this could be more performant
+    do {
+      val size = relevant.size
+      val newSize = walkUp()
+    } while (newSize > size)
+
+    return relevant
+  }
+
+  /**
+   * Partitions and returns artificial pair of [BinaryClasses][BinaryClass]. Non-null elements indicate relevant (to
+   * [memberAccess]) matching and non-matching members of this `BinaryClass`. Matching members are binary-compatible;
+   * and non-matching members have the same [name][com.autonomousapps.model.internal.intermediates.producer.Member.name]
+   * but incompatible [descriptors][com.autonomousapps.model.internal.intermediates.producer.Member.descriptor], and are
+   * therefore binary-incompatible.
+   *
+   * nb: We don't want this as a method directly in BinaryClass because it can't safely assert the prerequisite that
+   * it's only called on "relevant" classes. THIS class, however, can, via [findRelevantBinaryClasses].
+   */
+  private fun BinaryClass.partition(memberAccess: MemberAccess): Pair<BinaryClass?, BinaryClass?> {
+    // There can be only one match
+    val matchingFields = effectivelyPublicFields.firstOrNull { it.matches(memberAccess) }
+    val matchingMethods = effectivelyPublicMethods.firstOrNull { it.matches(memberAccess) }
+
+    // There can be many non-matches
+    val nonMatchingFields = effectivelyPublicFields.filterToOrderedSet { it.doesNotMatch(memberAccess) }
+    val nonMatchingMethods = effectivelyPublicMethods.filterToOrderedSet { it.doesNotMatch(memberAccess) }
+
+    // Create a view of the binary class containing only the matching members.
+    val match = if (matchingFields != null || matchingMethods != null) {
+      copy(
+        effectivelyPublicFields = matchingFields?.let { setOf(it) }.orEmpty(),
+        effectivelyPublicMethods = matchingMethods?.let { setOf(it) }.orEmpty()
+      )
+    } else {
+      null
+    }
+
+    // Create a view of the binary class containing only the non-matching members.
+    val nonMatch = if (nonMatchingFields.isNotEmpty() || nonMatchingMethods.isNotEmpty()) {
+      copy(
+        effectivelyPublicFields = nonMatchingFields,
+        effectivelyPublicMethods = nonMatchingMethods,
+      )
+    } else {
+      null
+    }
+
+    return match to nonMatch
+  }
 }

src/main/kotlin/com/autonomousapps/model/internal/Capability.kt

@@ -2,13 +2,10 @@
 // SPDX-License-Identifier: Apache-2.0
 package com.autonomousapps.model.internal
 
-import com.autonomousapps.internal.strings.dotty
 import com.autonomousapps.internal.unsafeLazy
 import com.autonomousapps.internal.utils.LexicographicIterableComparator
 import com.autonomousapps.internal.utils.efficient
-import com.autonomousapps.internal.utils.filterToOrderedSet
 import com.autonomousapps.internal.utils.mapToOrderedSet
-import com.autonomousapps.model.internal.intermediates.consumer.MemberAccess
 import com.autonomousapps.model.internal.intermediates.producer.BinaryClass
 import com.autonomousapps.model.internal.intermediates.producer.Constant
 import com.autonomousapps.model.internal.intermediates.producer.ReflectingDependency
@@ -152,124 +149,11 @@ internal data class BinaryClassCapability(
 
   val classes by unsafeLazy { binaryClasses.mapToOrderedSet { it.className } }
 
-  internal data class PartitionResult(
-    val matchingClasses: Set<BinaryClass>,
-    val nonMatchingClasses: Set<BinaryClass>,
-  ) {
-
-    companion object {
-      fun empty(): PartitionResult = PartitionResult(emptySet(), emptySet())
-    }
-
-    class Builder {
-      val matchingClasses = sortedSetOf<BinaryClass>()
-      val nonMatchingClasses = sortedSetOf<BinaryClass>()
-
-      fun build(): PartitionResult {
-        return PartitionResult(
-          matchingClasses = matchingClasses.efficient(),
-          nonMatchingClasses = nonMatchingClasses.efficient(),
-        )
-      }
-    }
-  }
-
   override fun merge(other: Capability): Capability {
     return newInstance(
       binaryClasses = (binaryClasses + (other as BinaryClassCapability).binaryClasses).efficient(),
     )
   }
-
-  internal fun findMatchingClasses(memberAccess: MemberAccess): PartitionResult {
-    val relevant = findRelevantBinaryClasses(memberAccess)
-
-    // lenient
-    if (relevant.isEmpty()) return PartitionResult.empty()
-
-    return relevant
-      .map { bin -> bin.partition(memberAccess) }
-      .fold(PartitionResult.Builder()) { acc, (match, nonMatch) ->
-        acc.apply {
-          match?.let { matchingClasses.add(it) }
-          nonMatch?.let { nonMatchingClasses.add(it) }
-        }
-      }
-      .build()
-  }
-
-  /**
-   * Example:
-   * 1. [memberAccess] is for `groovy/lang/MetaClass#getProperty`.
-   * 2. That method is actually provided by `groovy/lang/MetaObjectProtocol`, which `groovy/lang/MetaClass` implements.
-   *
-   * All of the above ("this" class, its super class, and its interfaces) are relevant for search purposes. Note we
-   * don't inspect the member names for this check.
-   */
-  private fun findRelevantBinaryClasses(memberAccess: MemberAccess): Set<BinaryClass> {
-    // direct references
-    val relevant = binaryClasses.filterTo(mutableSetOf()) { bin ->
-      bin.className == memberAccess.owner.dotty()
-    }
-
-    // Walk up the class hierarchy
-    fun walkUp(): Int {
-      binaryClasses.filterTo(relevant) { bin ->
-        bin.className in relevant.map { it.superClassName }
-          || bin.className in relevant.flatMap { it.interfaces }
-      }
-      return relevant.size
-    }
-
-    // TODO(tsr): this could be more performant
-    do {
-      val size = relevant.size
-      val newSize = walkUp()
-    } while (newSize > size)
-
-    return relevant
-  }
-
-  /**
-   * Partitions and returns artificial pair of [BinaryClasses][BinaryClass]. Non-null elements indicate relevant (to
-   * [memberAccess]) matching and non-matching members of this `BinaryClass`. Matching members are binary-compatible;
-   * and non-matching members have the same [name][com.autonomousapps.model.internal.intermediates.producer.Member.name]
-   * but incompatible [descriptors][com.autonomousapps.model.internal.intermediates.producer.Member.descriptor], and are
-   * therefore binary-incompatible.
-   *
-   * nb: We don't want this as a method directly in BinaryClass because it can't safely assert the prerequisite that
-   * it's only called on "relevant" classes. THIS class, however, can, via [findRelevantBinaryClasses].
-   */
-  private fun BinaryClass.partition(memberAccess: MemberAccess): Pair<BinaryClass?, BinaryClass?> {
-    // There can be only one match
-    val matchingFields = effectivelyPublicFields.firstOrNull { it.matches(memberAccess) }
-    val matchingMethods = effectivelyPublicMethods.firstOrNull { it.matches(memberAccess) }
-
-    // There can be many non-matches
-    val nonMatchingFields = effectivelyPublicFields.filterToOrderedSet { it.doesNotMatch(memberAccess) }
-    val nonMatchingMethods = effectivelyPublicMethods.filterToOrderedSet { it.doesNotMatch(memberAccess) }
-
-    // Create a view of the binary class containing only the matching members.
-    val match = if (matchingFields != null || matchingMethods != null) {
-      copy(
-        effectivelyPublicFields = matchingFields?.let { setOf(it) }.orEmpty(),
-        effectivelyPublicMethods = matchingMethods?.let { setOf(it) }.orEmpty()
-      )
-    } else {
-      null
-    }
-
-    // Create a view of the binary class containing only the non-matching members.
-    val nonMatch = if (nonMatchingFields.isNotEmpty() || nonMatchingMethods.isNotEmpty()) {
-      copy(
-        effectivelyPublicFields = nonMatchingFields,
-        effectivelyPublicMethods = nonMatchingMethods,
-      )
-    } else {
-      null
-    }
-
-    return match to nonMatch
-  }
 }
 
 @TypeLabel("const")