wip

Author: autonomousapps

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

@@ -2,13 +2,17 @@
 // SPDX-License-Identifier: Apache-2.0
 package com.autonomousapps.internal
 
+import com.autonomousapps.ProjectType
 import com.autonomousapps.extension.DependenciesHandler.SerializableBundles
 import com.autonomousapps.graph.Graphs.children
 import com.autonomousapps.graph.Graphs.reachableNodes
+import com.autonomousapps.internal.utils.filterToOrderedSet
 import com.autonomousapps.model.*
 import com.autonomousapps.model.Coordinates.Companion.copy
 import com.autonomousapps.model.internal.DependencyGraphView
 import com.autonomousapps.model.internal.declaration.Bucket
+import com.autonomousapps.model.internal.declaration.ConfigurationNames
+import com.autonomousapps.model.internal.declaration.Declaration
 import com.autonomousapps.model.internal.intermediates.Usage
 
 /**
@@ -19,7 +23,11 @@ import com.autonomousapps.model.internal.intermediates.Usage
  * C -> used as API, part of bundle with B. Should not be declared!
  */
 @Suppress("UnstableApiUsage")
-internal class Bundles private constructor(private val dependencyUsages: Map<Coordinates, Set<Usage>>) {
+internal class Bundles private constructor(
+  private val dependencyUsages: Map<Coordinates, Set<Usage>>,
+  private val declarations: Set<Declaration>,
+  private val configurationNames: ConfigurationNames,
+) {
 
   // a sort of adjacency-list structure
   private val parentKeyedBundle = mutableMapOf<Coordinates, MutableSet<Coordinates>>()
@@ -44,8 +52,103 @@ internal class Bundles private constructor(private val dependencyUsages: Map<Coo
   }
 
   fun hasParentInBundle(coordinates: Coordinates): Boolean = parentPointers[coordinates] != null
+
   fun findParentInBundle(coordinates: Coordinates): Coordinates? = parentPointers[coordinates]
 
+  /**
+   * Requirements for calling this method:
+   * 1. [hasParentInBundle] has already been called and it returns true. Otherwise this method will throw.
+   * 2. [addAdvice] is add-advice.
+   *
+   * Can return either [addAdvice] exactly, or a change-advice. Will do the latter when the following is true:
+   * 1. [originalCoordinates] is in a bundle with a declared parent.
+   * 2. That declared parent is declared on another source set (currently either `commonMain` or `commonTest`) in a
+   *    Kotlin Multiplatform (KMP) project.
+   * 3. The parent declaration is implementation-scoped and [addAdvice] is api-scoped (that is, we'd like to upgrade).
+   *
+   * In this case, we suggest changing the parent declaration from `commonMainImplementation` or
+   * `commonTestImplementation` to `commonMainApi` or `commonTestApi`, respectively.
+   *
+   * We do this because KMP is a special case where we know a priori that the commonX source sets are upstream from
+   * target-specific source sets. We're being very conservative by only targeting the `implementation` -> `api` upgrade;
+   * that is, the goal is to provide _safe_ advice even more than maximally-correct advice. Part of the problem here is
+   * we're in an intermediate state where we only support JVM targets: we can't know how the other targets use the
+   * common dependencies.
+   *
+   * @see [maybePrimary]
+   */
+  fun maybeParent(addAdvice: Advice, originalCoordinates: Coordinates): Advice {
+    check(addAdvice.isAdd()) { "Must be add-advice" }
+
+    val parent = findParentInBundle(originalCoordinates)
+      ?: error("No parent for $originalCoordinates. Check 'hasParentInBundle()' before calling this method.")
+    val preferredCoordinatesNotation = preferredCoordinates(parent, addAdvice)
+
+    val preferredBucket = Bucket.of(addAdvice.toConfiguration!!, configurationNames)
+
+    // Get the source set name for the addAdvice. E.g., "jvmMain".
+    val adviceSourceSetName = DependencyScope.sourceSetName(addAdvice.toConfiguration)
+
+    // Find all declarations for this dependency. E.g., ["commonMainImplementation", "jvmMainImplementation"].
+    val parentDeclarations = declarations
+      .filterToOrderedSet { decl -> decl.identifier == preferredCoordinatesNotation.identifier }
+
+    // Find all declarations for the advice source set. E.g., ["jvmMainImplementation", "jvmMainApi"]. It's been known
+    // to happen.
+    var parentDeclaration = parentDeclarations
+      .filter { declaration -> DependencyScope.sourceSetName(declaration.configurationName) == adviceSourceSetName }
+      // Pick the "highest" one (api > implementation > everything else)
+      .maxByOrNull { declaration ->
+        when (declaration.bucket(configurationNames)) {
+          Bucket.API -> 10
+          Bucket.IMPL -> 1
+          else -> -1
+        }
+      }
+
+    // If there are no declarations within the same source set
+    if (parentDeclaration == null) {
+      parentDeclaration = parentDeclarations
+        .filter { declaration -> DependencyScope.sourceSetName(declaration.configurationName) != adviceSourceSetName }
+        // Pick the "highest" one (api > implementation > everything else)
+        .maxBy { declaration ->
+          when (declaration.bucket(configurationNames)) {
+            Bucket.API -> 10
+            Bucket.IMPL -> 1
+            else -> -1
+          }
+        }
+    }
+
+    val parentBucket = parentDeclaration.bucket(configurationNames)
+
+    // Only change the advice if it's from implementation -> api. We don't change compileOnly or runtimeOnly advice.
+    return if (preferredBucket == Bucket.API && parentBucket == Bucket.IMPL) {
+      // TODO(tsr): there's a bug that probably impacts all project types, but I've only just noticed while working on KMP.
+      val toConfiguration = if (configurationNames.projectType == ProjectType.KMP) {
+        when (parentDeclaration.configurationName) {
+          // Handle the `commonX` cases
+          "commonMainImplementation" -> "commonMainApi"
+          "commonTestImplementation" -> "commonTestApi"
+
+          // This means the parent is probably in the same source set?
+          else -> addAdvice.toConfiguration
+        }
+      } else {
+        // This means the parent is probably in the same source set?
+        addAdvice.toConfiguration
+      }
+
+      Advice.ofChange(
+        coordinates = preferredCoordinatesNotation.withoutDefaultCapability(),
+        fromConfiguration = parentDeclaration.configurationName,
+        toConfiguration = toConfiguration,
+      )
+    } else {
+      addAdvice
+    }
+  }
+
   fun hasUsedChild(coordinates: Coordinates): Boolean {
     val children = parentKeyedBundle[coordinates] ?: return false
 
@@ -65,25 +168,30 @@ internal class Bundles private constructor(private val dependencyUsages: Map<Coo
   fun maybePrimary(addAdvice: Advice, originalCoordinates: Coordinates): Advice {
     check(addAdvice.isAdd()) { "Must be add-advice" }
     return primaryPointers[originalCoordinates]?.let { primary ->
-      val preferredCoordinatesNotation =
-        if (primary is IncludedBuildCoordinates && addAdvice.coordinates is ProjectCoordinates) {
-          primary.resolvedProject
-        } else {
-          primary
-        }
+      val preferredCoordinatesNotation = preferredCoordinates(primary, addAdvice)
       addAdvice.copy(coordinates = preferredCoordinatesNotation.withoutDefaultCapability())
     } ?: addAdvice
   }
 
+  private fun preferredCoordinates(coordinates: Coordinates, advice: Advice): Coordinates {
+    return if (coordinates is IncludedBuildCoordinates && advice.coordinates is ProjectCoordinates) {
+      coordinates.resolvedProject
+    } else {
+      coordinates
+    }
+  }
+
   companion object {
     fun of(
       projectPath: String,
       dependencyGraph: Map<String, DependencyGraphView>,
       bundleRules: SerializableBundles,
       dependencyUsages: Map<Coordinates, Set<Usage>>,
+      declarations: Set<Declaration>,
+      configurationNames: ConfigurationNames,
       ignoreKtx: Boolean,
     ): Bundles {
-      val bundles = Bundles(dependencyUsages)
+      val bundles = Bundles(dependencyUsages, declarations, configurationNames)
 
       // Handle bundles with primary entry points
       bundleRules.primaries.forEach { (name, primaryId) ->

src/main/kotlin/com/autonomousapps/internal/transform/StandardTransform.kt

@@ -30,15 +30,13 @@ internal class StandardTransform(
   private val coordinates: Coordinates,
   private val declarations: Set<Declaration>,
   private val dependencyGraph: Map<String, DependencyGraphView>,
-  private val supportedSourceSets: Set<String>,
   private val buildPath: String,
   private val explicitSourceSets: Set<String> = emptySet(),
   private val projectType: ProjectType,
+  private val configurationNames: ConfigurationNames,
   private val isKaptApplied: Boolean = false,
 ) : Usage.Transform {
 
-  private val configurationNames = ConfigurationNames(projectType, supportedSourceSets)
-
   private val mapper = UsageToConfigurationMapper(
     isKaptApplied = isKaptApplied,
     projectType = projectType,

src/main/kotlin/com/autonomousapps/model/internal/declaration/ConfigurationNames.kt

@@ -12,7 +12,7 @@ import com.autonomousapps.model.source.SourceKind
 
 /** Utility for mapping configuration names to related types. */
 internal class ConfigurationNames(
-  private val projectType: ProjectType,
+  val projectType: ProjectType,
   private val supportedSourceSetNames: Set<String>,
 ) {
 

src/main/kotlin/com/autonomousapps/model/source/SourceKind.kt

@@ -334,27 +334,8 @@ public data class KmpSourceKind(
 
   // TODO(tsr): double-check this is correct.
   internal companion object {
-    // kotlin source set: commonMain
-    // - api            : commonMainApi
-    // - implementation : commonMainImplementation
-    // - compile-only   : commonMainCompileOnly
-    // - runtime-only   : commonMainRuntimeOnly
-    // kotlin source set: commonTest
-    // - (and so on)
-    // kotlin source set: iosArm64Main
-    // kotlin source set: iosArm64Test
-    // kotlin source set: iosSimulatorArm64Main
-    // kotlin source set: iosSimulatorArm64Test
-    // kotlin source set: jvmMain
-    // kotlin source set: jvmTest
-    //
-    // classpaths:
-    // - jvmRuntimeClasspath
-    // - jvmMainRuntimeClasspath
-    // - jvmTestRuntimeClasspath
-    // - jvmCompileClasspath
-    // - jvmMainCompileClasspath
-    // - jvmTestCompileClasspath
+    const val COMMON_MAIN_NAME = "commonMain"
+    const val COMMON_TEST_NAME = "commonTest"
     const val JVM_MAIN_NAME = "jvmMain"
     const val JVM_TEST_NAME = "jvmTest"
 

src/main/kotlin/com/autonomousapps/tasks/ComputeAdviceTask.kt

@@ -161,7 +161,7 @@ public abstract class ComputeAdviceTask @Inject constructor(
 
       val projectPath = parameters.projectPath.get()
       val buildPath = parameters.buildPath.get()
-      val declarations = parameters.declarations.fromJsonSet<Declaration>()
+      val declarations = parameters.declarations.fromJsonSet<Declaration>().toSortedSet()
       val dependencyGraph = DependencyGraphView.asMap(parameters.dependencyGraphViews)
       val androidScore = parameters.androidScoreReports.get()
         .map { it.fromJson<AndroidScoreVariant>() }
@@ -181,14 +181,16 @@ public abstract class ComputeAdviceTask @Inject constructor(
       val explicitSourceSets = parameters.explicitSourceSets.get()
       val projectType = parameters.projectType.get()
       val isKaptApplied = parameters.kapt.get()
-
       val ignoreKtx = parameters.ignoreKtx.get()
+      val configurationNames = ConfigurationNames(projectType, supportedSourceSets)
 
       val bundles = Bundles.of(
         projectPath = projectPath,
         dependencyGraph = dependencyGraph,
         bundleRules = bundleRules,
         dependencyUsages = dependencyUsages,
+        declarations = declarations,
+        configurationNames = configurationNames,
         ignoreKtx = ignoreKtx,
       )
 
@@ -203,6 +205,7 @@ public abstract class ComputeAdviceTask @Inject constructor(
         supportedSourceSets = supportedSourceSets,
         explicitSourceSets = explicitSourceSets,
         projectType = projectType,
+        configurationNames = configurationNames,
         isKaptApplied = isKaptApplied,
       )
 
@@ -275,11 +278,10 @@ internal class DependencyAdviceBuilder(
   private val supportedSourceSets: Set<String>,
   private val explicitSourceSets: Set<String>,
   private val projectType: ProjectType,
+  private val configurationNames: ConfigurationNames,
   private val isKaptApplied: Boolean,
 ) {
 
-  private val configurationNames = ConfigurationNames(projectType, supportedSourceSets)
-
   /** The unfiltered advice. */
   val advice: Set<Advice>
 
@@ -293,7 +295,8 @@ internal class DependencyAdviceBuilder(
   }
 
   private fun computeDependencyAdvice(projectPath: String): Sequence<Advice> {
-    val declarations = declarations.filterToSet { configurationNames.isForRegularDependency(it.configurationName) }
+    val declarations =
+      declarations.filterToOrderedSet { configurationNames.isForRegularDependency(it.configurationName) }
 
     fun Advice.isRemoveTestDependencyOnSelf(): Boolean {
       return coordinates.identifier == projectPath
@@ -314,7 +317,7 @@ internal class DependencyAdviceBuilder(
           coordinates = coordinates,
           declarations = declarations,
           dependencyGraph = dependencyGraph,
-          supportedSourceSets = supportedSourceSets,
+          configurationNames = configurationNames,
           buildPath = buildPath,
           explicitSourceSets = explicitSourceSets,
           projectType = projectType,
@@ -336,7 +339,14 @@ internal class DependencyAdviceBuilder(
           advice.isAdd() && bundles.hasParentInBundle(originalCoordinates) -> {
             val parent = bundles.findParentInBundle(originalCoordinates)!!
             bundledTraces.add(BundleTrace.DeclaredParent(parent = parent, child = originalCoordinates))
-            null
+
+            // TODO(tsr): need to update bundledTraces and Reason
+            val parentAdvice = bundles.maybeParent(advice, originalCoordinates)
+            if (parentAdvice != advice) {
+              parentAdvice
+            } else {
+              null
+            }
           }
 
           // Optionally map given advice to "primary" advice, if bundle has a primary
@@ -368,17 +378,18 @@ internal class DependencyAdviceBuilder(
 
   // nb: no bundle support for annotation processors
   private fun computeAnnotationProcessorAdvice(): Sequence<Advice> {
-    val declarations = declarations.filterToSet { configurationNames.isForAnnotationProcessor(it.configurationName) }
+    val declarations = declarations
+      .filterToOrderedSet { configurationNames.isForAnnotationProcessor(it.configurationName) }
     return annotationProcessorUsages.asSequence()
       .flatMap { (coordinates, usages) ->
         StandardTransform(
           coordinates = coordinates,
           declarations = declarations,
           dependencyGraph = emptyMap(),
-          supportedSourceSets = supportedSourceSets,
           buildPath = buildPath,
           explicitSourceSets = explicitSourceSets,
           projectType = projectType,
+          configurationNames = configurationNames,
           isKaptApplied = isKaptApplied,
         ).reduce(usages)
       }