Misc dependency rake improvements (#444)

docs/dependency-rake.md

@@ -35,3 +35,21 @@ that we currently do not do.
 ## Implementation
 
 The core implementation of DR lives in `DependencyRake.kt`.
+
+## Usage
+
+To run dependency rake in a project, use the below command
+
+```bash
+$ ./gradlew rakeDependencies -Pslack.gradle.config.enableAnalysisPlugin=true --no-configuration-cache
+```
+
+This will run all `rakeDependencies` tasks in the project. This task exists on all subprojects as well, but it 
+works best if all are run together.
+
+Sometimes dependency rake will try to replace identifiers with ones that are not present in any available
+version catalogs. Sometimes this is acceptable, but often times it can result in "missing" dependencies from 
+the build after it runs. To help fix these, DR will write all missing identifiers out to a build output file.
+
+For convenience, you can also run `./gradlew aggregateMissingIdentifiers -Pslack.gradle.config.enableAnalysisPlugin=true --no-configuration-cache`
+to run all dependency rake tasks and aggregate these missing identifiers into a root project build output file.

slack-plugin/src/main/kotlin/slack/dependencyrake/DependencyRake.kt

@@ -25,6 +25,9 @@ import com.autonomousapps.model.ModuleCoordinates
 import com.autonomousapps.model.ProjectCoordinates
 import java.io.File
 import javax.inject.Inject
+import org.gradle.api.DefaultTask
+import org.gradle.api.Project
+import org.gradle.api.file.ConfigurableFileCollection
 import org.gradle.api.file.RegularFileProperty
 import org.gradle.api.model.ObjectFactory
 import org.gradle.api.provider.MapProperty
@@ -33,9 +36,13 @@ import org.gradle.api.provider.ProviderFactory
 import org.gradle.api.provider.SetProperty
 import org.gradle.api.tasks.Input
 import org.gradle.api.tasks.InputFile
+import org.gradle.api.tasks.InputFiles
+import org.gradle.api.tasks.OutputFile
 import org.gradle.api.tasks.PathSensitive
 import org.gradle.api.tasks.PathSensitivity
 import org.gradle.api.tasks.TaskAction
+import org.gradle.api.tasks.TaskProvider
+import org.gradle.api.tasks.UntrackedTask
 import slack.gradle.convertProjectPathToAccessor
 
 private const val IGNORE_COMMENT = "// dependency-rake=ignore"
@@ -72,8 +79,6 @@ constructor(objects: ObjectFactory, providers: ProviderFactory) : AbstractPostPr
   @get:InputFile
   abstract val buildFileProperty: RegularFileProperty
 
-  // We don't do ABI dependency cleanup yet, see
-  // https://github.com/tinyspeck/slack-android-ng/issues/20315
   @get:Input
   val modes: SetProperty<AnalysisMode> =
     objects
@@ -95,6 +100,8 @@ constructor(objects: ObjectFactory, providers: ProviderFactory) : AbstractPostPr
 
   @get:Input abstract val noApi: Property<Boolean>
 
+  @get:OutputFile abstract val missingIdentifiersFile: RegularFileProperty
+
   init {
     group = "rake"
   }
@@ -110,16 +117,23 @@ constructor(objects: ObjectFactory, providers: ProviderFactory) : AbstractPostPr
     val redundantPlugins = projectAdvice.pluginAdvice
     val advices: Set<Advice> = projectAdvice.dependencyAdvice
     val buildFile = buildFileProperty.asFile.get()
+    val missingIdentifiers = mutableSetOf<String>()
     logger.lifecycle("🌲 Raking $buildFile ")
-    rakeProject(buildFile, advices, redundantPlugins, noApi)
+    rakeProject(buildFile, advices, redundantPlugins, noApi, missingIdentifiers)
+    val identifiersFile = missingIdentifiersFile.asFile.get()
+    if (missingIdentifiers.isNotEmpty()) {
+      logger.lifecycle("⚠️ Missing identifiers found, written to $identifiersFile")
+    }
+    identifiersFile.writeText(missingIdentifiers.sorted().joinToString("\n"))
   }
 
   @Suppress("LongMethod", "ComplexMethod")
   private fun rakeProject(
     buildFile: File,
     advices: Set<Advice>,
     redundantPlugins: Set<PluginAdvice>,
-    noApi: Boolean
+    noApi: Boolean,
+    missingIdentifiers: MutableSet<String>,
   ) {
     val resolvedModes = modes.get()
     val abiModeEnabled = AnalysisMode.ABI in resolvedModes
@@ -129,7 +143,7 @@ constructor(objects: ObjectFactory, providers: ProviderFactory) : AbstractPostPr
         advices
           .filter { it.isRemove() }
           .filterNot { it.coordinates.identifier in MANAGED_DEPENDENCIES }
-          .associateBy { it.toDependencyString("UNUSED") }
+          .associateBy { it.toDependencyString("UNUSED", missingIdentifiers) }
       } else {
         emptyMap()
       }
@@ -139,7 +153,7 @@ constructor(objects: ObjectFactory, providers: ProviderFactory) : AbstractPostPr
         advices
           .filter { it.isRemove() }
           .filterNot { it.coordinates.identifier in MANAGED_DEPENDENCIES }
-          .associateBy { it.toDependencyString("MISUSED") }
+          .associateBy { it.toDependencyString("MISUSED", missingIdentifiers) }
       } else {
         emptyMap()
       }
@@ -151,7 +165,7 @@ constructor(objects: ObjectFactory, providers: ProviderFactory) : AbstractPostPr
         advices
           .filter { it.isChange() }
           .filterNot { it.coordinates.identifier in MANAGED_DEPENDENCIES }
-          .associateBy { it.toDependencyString("CHANGE") }
+          .associateBy { it.toDependencyString("CHANGE", missingIdentifiers) }
       } else {
         emptyMap()
       }
@@ -171,7 +185,7 @@ constructor(objects: ObjectFactory, providers: ProviderFactory) : AbstractPostPr
       if (AnalysisMode.COMPILE_ONLY in resolvedModes) {
         advices
           .filter { it.isCompileOnly() }
-          .associateBy { it.toDependencyString("ADD-COMPILE-ONLY") }
+          .associateBy { it.toDependencyString("ADD-COMPILE-ONLY", missingIdentifiers) }
       } else {
         emptyMap()
       }
@@ -200,7 +214,8 @@ constructor(objects: ObjectFactory, providers: ProviderFactory) : AbstractPostPr
               // Emit any remaining new dependencies to add
               depsToAdd.entries
                 .mapNotNull { (_, advice) ->
-                  advice.coordinates.toDependencyNotation("ADD-NEW")?.let { newNotation ->
+                  advice.coordinates.toDependencyNotation("ADD-NEW", missingIdentifiers)?.let {
+                    newNotation ->
                     var newConfiguration = advice.toConfiguration!!
                     if (noApi && newConfiguration == "api") {
                       newConfiguration = "implementation"
@@ -242,7 +257,8 @@ constructor(objects: ObjectFactory, providers: ProviderFactory) : AbstractPostPr
                 .filter { it.isAdd() }
                 .mapNotNull { depsToAdd.remove(it.coordinates.identifier) }
                 .mapNotNull { advice ->
-                  advice.coordinates.toDependencyNotation("ADD")?.let { newNotation ->
+                  advice.coordinates.toDependencyNotation("ADD", missingIdentifiers)?.let {
+                    newNotation ->
                     val newConfiguration =
                       if (!abiModeEnabled) {
                         "implementation"
@@ -266,7 +282,8 @@ constructor(objects: ObjectFactory, providers: ProviderFactory) : AbstractPostPr
                 if (depsToChange.keys.any { it in line }) depsToChange else compileOnlyDeps
               val (_, abiDep) =
                 which.entries.first { (_, v) ->
-                  v.coordinates.toDependencyNotation("ABI")?.let { it in line } ?: false
+                  v.coordinates.toDependencyNotation("ABI", missingIdentifiers)?.let { it in line }
+                    ?: false
                 }
               val oldConfiguration = abiDep.fromConfiguration!!
               var newConfiguration = abiDep.toConfiguration!!
@@ -309,14 +326,18 @@ constructor(objects: ObjectFactory, providers: ProviderFactory) : AbstractPostPr
   }
 
   /** Remaps a given [Coordinates] to a known toml lib reference or error if [error] is true. */
-  private fun Coordinates.mapIdentifier(context: String): Coordinates? {
+  private fun Coordinates.mapIdentifier(
+    context: String,
+    missingIdentifiers: MutableSet<String>
+  ): Coordinates? {
     return when (this) {
       is ModuleCoordinates -> {
         val preferredIdentifier = PREFERRED_BUNDLE_IDENTIFIERS.getOrDefault(identifier, identifier)
         val newIdentifier =
           identifierMap.get()[preferredIdentifier]
             ?: run {
               logger.lifecycle("($context) Unknown identifier: $identifier")
+              missingIdentifiers += identifier
               return null
             }
         ModuleCoordinates(newIdentifier, resolvedVersion, gradleVariantIdentification)
@@ -327,14 +348,20 @@ constructor(objects: ObjectFactory, providers: ProviderFactory) : AbstractPostPr
     }
   }
 
-  private fun Advice.toDependencyString(context: String): String {
-    return "${fromConfiguration ?: error("Transitive dep $this")}(${coordinates.toDependencyNotation(context)})"
+  private fun Advice.toDependencyString(
+    context: String,
+    missingIdentifiers: MutableSet<String>
+  ): String {
+    return "${fromConfiguration ?: error("Transitive dep $this")}(${coordinates.toDependencyNotation(context, missingIdentifiers)})"
   }
 
-  private fun Coordinates.toDependencyNotation(context: String): String? {
+  private fun Coordinates.toDependencyNotation(
+    context: String,
+    missingIdentifiers: MutableSet<String>
+  ): String? {
     return when (this) {
       is ProjectCoordinates -> "projects.${convertProjectPathToAccessor(identifier)}"
-      is ModuleCoordinates -> mapIdentifier(context)?.identifier
+      is ModuleCoordinates -> mapIdentifier(context, missingIdentifiers)?.identifier
       is FlatCoordinates -> gav()
       is IncludedBuildCoordinates -> gav()
     }
@@ -382,3 +409,38 @@ private fun List<String>.padNewline(): List<String> {
   val noEmpties = dropLastWhile { it.isBlank() }
   return noEmpties + ""
 }
+
+@UntrackedTask(because = "Dependency Rake tasks modify build files")
+internal abstract class MissingIdentifiersAggregatorTask : DefaultTask() {
+  @get:InputFiles
+  @get:PathSensitive(PathSensitivity.RELATIVE)
+  abstract val inputFiles: ConfigurableFileCollection
+
+  @get:OutputFile abstract val outputFile: RegularFileProperty
+
+  init {
+    group = "rake"
+    description = "Aggregates missing identifiers from all upstream dependency rake tasks."
+  }
+
+  @TaskAction
+  fun aggregate() {
+    val aggregated = inputFiles.flatMap { it.readLines() }.toSortedSet()
+
+    val output = outputFile.asFile.get()
+    logger.lifecycle("Writing aggregated missing identifiers to $output")
+    output.writeText(aggregated.joinToString("\n"))
+  }
+
+  companion object {
+    const val NAME = "aggregateMissingIdentifiers"
+
+    fun register(rootProject: Project): TaskProvider<MissingIdentifiersAggregatorTask> {
+      return rootProject.tasks.register(NAME, MissingIdentifiersAggregatorTask::class.java) {
+        outputFile.set(
+          rootProject.layout.buildDirectory.file("rake/aggregated_missing_identifiers.txt")
+        )
+      }
+    }
+  }
+}

slack-plugin/src/main/kotlin/slack/gradle/SlackRootPlugin.kt

@@ -28,6 +28,7 @@ import org.gradle.api.provider.Provider
 import org.gradle.api.tasks.Delete
 import org.gradle.jvm.toolchain.JvmVendorSpec
 import slack.cli.AppleSiliconCompat
+import slack.dependencyrake.MissingIdentifiersAggregatorTask
 import slack.gradle.agp.VersionNumber
 import slack.gradle.avoidance.ComputeAffectedProjectsTask
 import slack.gradle.lint.DetektTasks
@@ -219,6 +220,10 @@ internal class SlackRootPlugin : Plugin<Project> {
     // Dependency analysis plugin for build health
     // Usage: ./gradlew clean buildHealth
     project.pluginManager.withPlugin("com.autonomousapps.dependency-analysis") {
+      // Register the missing identifiers aggregator
+      if (slackProperties.enableAnalysisPlugin) {
+        MissingIdentifiersAggregatorTask.register(project)
+      }
       project.configure<DependencyAnalysisExtension> {
         issues { all { onAny { ignoreKtx(true) } } }
         abi {

slack-plugin/src/main/kotlin/slack/gradle/StandardProjectConfigurations.kt

@@ -58,6 +58,8 @@ import org.jetbrains.kotlin.gradle.dsl.kotlinExtension
 import org.jetbrains.kotlin.gradle.internal.KaptGenerateStubsTask
 import org.jetbrains.kotlin.gradle.plugin.KaptExtension
 import org.jetbrains.kotlin.gradle.plugin.KotlinBasePlugin
+import org.jetbrains.kotlin.gradle.utils.named
+import slack.dependencyrake.MissingIdentifiersAggregatorTask
 import slack.dependencyrake.RakeDependencies
 import slack.gradle.AptOptionsConfig.AptOptionsConfigurer
 import slack.gradle.AptOptionsConfigs.invoke
@@ -161,7 +163,7 @@ internal class StandardProjectConfigurations(
       applyPlatforms(slackProperties.versions.boms, platformProjectPath)
     }
 
-    if (slackProperties.enableAnalysisPlugin) {
+    if (slackProperties.enableAnalysisPlugin && project.path != platformProjectPath) {
       val buildFile = project.buildFile
       // This can run on some intermediate middle directories, like `carbonite` in
       // `carbonite:carbonite`
@@ -171,6 +173,9 @@ internal class StandardProjectConfigurations(
           val isNoApi = slackProperties.rakeNoApi
           val catalogNames =
             extensions.findByType<VersionCatalogsExtension>()?.catalogNames ?: return@withId
+
+          val catalogs = catalogNames.map { catalogName -> project.getVersionsCatalog(catalogName) }
+
           val rakeDependencies =
             tasks.register<RakeDependencies>("rakeDependencies") {
               // TODO https://github.com/gradle/gradle/issues/25014
@@ -179,19 +184,24 @@ internal class StandardProjectConfigurations(
               identifierMap.setDisallowChanges(
                 project.provider {
                   buildMap {
-                    for (catalogName in catalogNames) {
-                      putAll(
-                        project.getVersionsCatalog(catalogName).identifierMap().mapValues { (_, v)
-                          ->
-                          "$catalogName.$v"
-                        }
-                      )
+                    for (catalog in catalogs) {
+                      putAll(catalog.identifierMap().mapValues { (_, v) -> "${catalog.name}.$v" })
                     }
                   }
                 }
               )
+              missingIdentifiersFile.set(
+                project.layout.buildDirectory.file("rake/missing_identifiers.txt")
+              )
             }
           configure<DependencyAnalysisSubExtension> { registerPostProcessingTask(rakeDependencies) }
+          val aggregator =
+            project.rootProject.tasks.named<MissingIdentifiersAggregatorTask>(
+              MissingIdentifiersAggregatorTask.NAME
+            )
+          aggregator.configure {
+            inputFiles.from(rakeDependencies.flatMap { it.missingIdentifiersFile })
+          }
         }
       }
     }