diff --git a/src/functionalTest/groovy/com/autonomousapps/jvm/TestDependenciesSpec.groovy b/src/functionalTest/groovy/com/autonomousapps/jvm/TestDependenciesSpec.groovy index 755d77b9f..9c8eb30a2 100644 --- a/src/functionalTest/groovy/com/autonomousapps/jvm/TestDependenciesSpec.groovy +++ b/src/functionalTest/groovy/com/autonomousapps/jvm/TestDependenciesSpec.groovy @@ -1,5 +1,6 @@ package com.autonomousapps.jvm +import com.autonomousapps.jvm.projects.TestBundleProject import com.autonomousapps.jvm.projects.TestDependenciesProject import static com.autonomousapps.utils.Runner.build @@ -21,4 +22,19 @@ final class TestDependenciesSpec extends AbstractJvmSpec { where: gradleVersion << gradleVersions() } + + def "bundles work for test dependencies (#gradleVersion)"() { + given: + def project = new TestBundleProject() + gradleProject = project.gradleProject + + when: + build(gradleVersion, gradleProject.rootDir, ':buildHealth') + + then: + assertThat(actualAdvice()).containsExactlyElementsIn(project.expectedAdvice) + + where: + gradleVersion << gradleVersions() + } } diff --git a/src/functionalTest/groovy/com/autonomousapps/jvm/projects/TestBundleProject.groovy b/src/functionalTest/groovy/com/autonomousapps/jvm/projects/TestBundleProject.groovy new file mode 100644 index 000000000..d087aaa16 --- /dev/null +++ b/src/functionalTest/groovy/com/autonomousapps/jvm/projects/TestBundleProject.groovy @@ -0,0 +1,85 @@ +package com.autonomousapps.jvm.projects + +import com.autonomousapps.AbstractProject +import com.autonomousapps.advice.Advice +import com.autonomousapps.kit.GradleProject +import com.autonomousapps.kit.Plugin +import com.autonomousapps.kit.Source +import com.autonomousapps.kit.SourceType + +import static com.autonomousapps.kit.Dependency.junit +import static com.autonomousapps.kit.Dependency.kotestAssertions +import static java.util.Collections.emptyList + +final class TestBundleProject extends AbstractProject { + + private static final junit = junit('testImplementation') + private static final kotest = kotestAssertions('testImplementation') + + final GradleProject gradleProject + + TestBundleProject() { + this.gradleProject = build() + } + + private GradleProject build() { + def builder = newGradleProjectBuilder() + builder.withSubproject('proj') { s -> + s.sources = sources + s.withBuildScript { bs -> + bs.plugins = [Plugin.kotlinPluginNoVersion] + bs.dependencies = [kotest, junit] + } + } + builder.withRootProject { + it.withBuildScript { bs -> + bs.additions = """ + dependencyAnalysis { + dependencies { + bundle('kotest-assertions') { + includeDependency('io.kotest:kotest-assertions-core-jvm') + includeDependency('io.kotest:kotest-assertions-shared-jvm') + } + } + } + """.stripIndent() + } + } + + def project = builder.build() + project.writer().write() + return project + } + + private List sources = [ + new Source( + SourceType.KOTLIN, "Main", "com/example", + """\ + package com.example + + class Main { + fun magic() = 42 + } + """.stripIndent() + ), + new Source( + SourceType.KOTLIN, "MainTest", "com/example", + """\ + package com.example + + import io.kotest.matchers.shouldBe + import org.junit.Test + + class MainTest { + @Test + public fun test() { + "a" shouldBe "b" + } + } + """.stripIndent(), + 'test' + ) + ] + + final List expectedAdvice = emptyList() +} diff --git a/src/main/kotlin/com/autonomousapps/DependencyAnalysisPlugin.kt b/src/main/kotlin/com/autonomousapps/DependencyAnalysisPlugin.kt index 745b3660b..a424755e1 100644 --- a/src/main/kotlin/com/autonomousapps/DependencyAnalysisPlugin.kt +++ b/src/main/kotlin/com/autonomousapps/DependencyAnalysisPlugin.kt @@ -601,19 +601,21 @@ class DependencyAnalysisPlugin : Plugin { tasks.register("analyzeJar$variantTaskName") { val compileClasspath = configurations.getByName(dependencyAnalyzer.compileConfigurationName) this.compileClasspath = compileClasspath - artifactFiles.setFrom(compileClasspath - .incoming - .artifactViewFor(dependencyAnalyzer.attributeValueJar) - .artifacts - .artifactFiles + artifactFiles.setFrom( + compileClasspath + .incoming + .artifactViewFor(dependencyAnalyzer.attributeValueJar) + .artifacts + .artifactFiles ) val testCompileClasspath = configurations.findByName(dependencyAnalyzer.testCompileConfigurationName) this.testCompileClasspath = testCompileClasspath - testArtifactFiles.setFrom(testCompileClasspath - ?.incoming - ?.artifactViewFor(dependencyAnalyzer.attributeValueJar) - ?.artifacts - ?.artifactFiles + testArtifactFiles.setFrom( + testCompileClasspath + ?.incoming + ?.artifactViewFor(dependencyAnalyzer.attributeValueJar) + ?.artifacts + ?.artifactFiles ) allArtifacts.set(artifactsReportTask.flatMap { it.output }) @@ -730,13 +732,9 @@ class DependencyAnalysisPlugin : Plugin { // A report of all unused dependencies and used-transitive dependencies val misusedDependenciesTask = tasks.register("misusedDependencies$variantTaskName") { - val compileConfiguration = configurations.getByName(dependencyAnalyzer.compileConfigurationName) - artifactFiles = compileConfiguration - .incoming - .artifactViewFor(dependencyAnalyzer.attributeValueJar) - .artifacts - .artifactFiles - this@register.compileConfiguration = compileConfiguration + jarAttr.set(dependencyAnalyzer.attributeValueJar) + compileConfiguration = configurations.getByName(dependencyAnalyzer.compileConfigurationName) + testCompileConfiguration = configurations.findByName(dependencyAnalyzer.testCompileConfigurationName) declaredDependencies.set(analyzeJarTask.flatMap { it.allComponentsReport }) usedClasses.set(analyzeClassesTask.flatMap { it.output }) @@ -802,6 +800,23 @@ class DependencyAnalysisPlugin : Plugin { storeAbiDumpOutput(it, variantName) } + // Produces a json and graphviz file representing the dependency graph + val graphTask = tasks.register("graph$variantTaskName") { + jarAttr.set(dependencyAnalyzer.attributeValueJar) + compileClasspath = configurations.getByName(dependencyAnalyzer.compileConfigurationName) + testCompileClasspath = configurations.findByName(dependencyAnalyzer.testCompileConfigurationName) + + projectPath.set(this@analyzeDependencies.path) + + compileOutputJson.set(outputPaths.compileGraphPath) + compileOutputDot.set(outputPaths.compileGraphDotPath) + testCompileOutputJson.set(outputPaths.testCompileGraphPath) + testCompileOutputDot.set(outputPaths.testCompileGraphDotPath) + } + aggregateGraphTask.configure { + graphs.add(graphTask.flatMap { it.compileOutputJson }) + } + // Optionally transforms and prints advice to console val advicePrinterTask = tasks.register("advicePrinter$variantTaskName") @@ -820,6 +835,9 @@ class DependencyAnalysisPlugin : Plugin { serviceLoaders.set(serviceLoaderTask.flatMap { it.output }) usedVariantDependencies.set(misusedDependenciesTask.flatMap { it.outputUsedVariantDependencies }) + compileGraph.set(graphTask.flatMap { it.compileOutputJson }) + testCompileGraph.set(graphTask.flatMap { it.testCompileOutputJson }) + dependenciesHandler = getExtension().dependenciesHandler dataBindingEnabled.set(dependencyAnalyzer.isDataBindingEnabled) @@ -857,25 +875,6 @@ class DependencyAnalysisPlugin : Plugin { adviceConsoleReportTxt.set(outputPaths.adviceConsoleTxtPath) } - // Produces a json and graphviz file representing the dependency graph - val graphTask = tasks.register("graph$variantTaskName") { - val compileClasspath = configurations.getByName(dependencyAnalyzer.compileConfigurationName) - this.compileClasspath = compileClasspath - compileClasspathArtifacts.setFrom(compileClasspath - .incoming - .artifactViewFor(dependencyAnalyzer.attributeValueJar) - .artifacts - .artifactFiles - ) - projectPath.set(this@analyzeDependencies.path) - - outputJson.set(outputPaths.graphPath) - outputDot.set(outputPaths.graphDotPath) - } - aggregateGraphTask.configure { - graphs.add(graphTask.flatMap { it.outputJson }) - } - // Produces a report consolidating all information about dependencies in one place, for // ReasonTask to use as an input val reasonableDepsTask = tasks.register("reasonableDepsReport$variantTaskName") { @@ -909,7 +908,7 @@ class DependencyAnalysisPlugin : Plugin { // Emits to console the reason for a piece of advice tasks.register("reason$variantTaskName") { - graph.set(graphTask.flatMap { it.outputJson }) + graph.set(graphTask.flatMap { it.compileOutputJson }) advice.set(adviceTask.flatMap { it.adviceReport }) reasonableDependenciesReport.set(reasonableDepsTask.flatMap { it.output }) diff --git a/src/main/kotlin/com/autonomousapps/advice/ComponentWithTransitives.kt b/src/main/kotlin/com/autonomousapps/advice/ComponentWithTransitives.kt index 42276e6b7..33b7b537a 100644 --- a/src/main/kotlin/com/autonomousapps/advice/ComponentWithTransitives.kt +++ b/src/main/kotlin/com/autonomousapps/advice/ComponentWithTransitives.kt @@ -21,7 +21,7 @@ data class ComponentWithTransitives( val usedTransitiveDependencies: MutableSet? ) : HasDependency, Comparable { - override fun compareTo(other: ComponentWithTransitives): Int { - return dependency.compareTo(other.dependency) - } + val identifier: String = dependency.identifier + + override fun compareTo(other: ComponentWithTransitives) = dependency.compareTo(other.dependency) } diff --git a/src/main/kotlin/com/autonomousapps/advice/TransitiveDependency.kt b/src/main/kotlin/com/autonomousapps/advice/TransitiveDependency.kt index efeca5424..2d71bd93a 100644 --- a/src/main/kotlin/com/autonomousapps/advice/TransitiveDependency.kt +++ b/src/main/kotlin/com/autonomousapps/advice/TransitiveDependency.kt @@ -20,5 +20,7 @@ data class TransitiveDependency( val variants: Set = emptySet() ) : HasDependency, Comparable { + val identifier: String = dependency.identifier + override fun compareTo(other: TransitiveDependency): Int = dependency.compareTo(other.dependency) } \ No newline at end of file diff --git a/src/main/kotlin/com/autonomousapps/graph/ShortestPath.kt b/src/main/kotlin/com/autonomousapps/graph/ShortestPath.kt index 8d9186bb2..e28e69205 100644 --- a/src/main/kotlin/com/autonomousapps/graph/ShortestPath.kt +++ b/src/main/kotlin/com/autonomousapps/graph/ShortestPath.kt @@ -4,9 +4,11 @@ import java.util.* internal class ShortestPath( graph: DependencyGraph, - source: Node + source: String ) { + constructor(graph: DependencyGraph, source: Node) : this(graph, source.identifier) + private val distTo = linkedMapOf() private val edgeTo = linkedMapOf() @@ -18,7 +20,7 @@ internal class ShortestPath( for (node in graph.nodes()) { distTo[node.identifier] = Float.POSITIVE_INFINITY } - distTo[source.identifier] = 0f + distTo[source] = 0f // visit vertices in topological order val top = Topological(graph) diff --git a/src/main/kotlin/com/autonomousapps/internal/JarAnalyzer.kt b/src/main/kotlin/com/autonomousapps/internal/JarAnalyzer.kt index c82247af5..ac1ea5672 100644 --- a/src/main/kotlin/com/autonomousapps/internal/JarAnalyzer.kt +++ b/src/main/kotlin/com/autonomousapps/internal/JarAnalyzer.kt @@ -12,7 +12,6 @@ import org.gradle.api.artifacts.component.ProjectComponentIdentifier import org.gradle.api.artifacts.result.DependencyResult import org.gradle.api.artifacts.result.ResolvedDependencyResult import org.gradle.api.logging.Logger -import java.util.zip.ZipException import java.util.zip.ZipFile /** diff --git a/src/main/kotlin/com/autonomousapps/internal/OutputPaths.kt b/src/main/kotlin/com/autonomousapps/internal/OutputPaths.kt index 8a71d8113..3ddc69516 100644 --- a/src/main/kotlin/com/autonomousapps/internal/OutputPaths.kt +++ b/src/main/kotlin/com/autonomousapps/internal/OutputPaths.kt @@ -53,8 +53,10 @@ internal class OutputPaths(private val project: Project, variantName: String) { */ private val graphDir = "${variantDirectory}/graph" - val graphPath = layout("${graphDir}/graph.json") - val graphDotPath = layout("${graphDir}/graph.gv") + val compileGraphPath = layout("${graphDir}/graph-compile.json") + val testCompileGraphPath = layout("${graphDir}/graph-test-compile.json") + val compileGraphDotPath = layout("${graphDir}/graph-compile.gv") + val testCompileGraphDotPath = layout("${graphDir}/graph-test-compile.gv") val reasonableDependenciesPath = layout("${intermediatesDir}/reasonable-dependencies.json") val graphReasonPath = layout("${graphDir}/graph-reason.gv") @@ -148,7 +150,7 @@ fun getAbiAnalysisPath(variantName: String) = "${getVariantDirectory(variantName)}/intermediates/abi.json" fun getGraphPerVariantPath(variantName: String) = - "${getVariantDirectory(variantName)}/graph/graph.json" + "${getVariantDirectory(variantName)}/graph/graph-compile.json" fun getAdvicePath(variantName: String) = "${getVariantDirectory(variantName)}/advice.json" fun getAdviceConsolePath(variantName: String) = diff --git a/src/main/kotlin/com/autonomousapps/internal/advice/Advisor.kt b/src/main/kotlin/com/autonomousapps/internal/advice/Advisor.kt index 4967bcec8..5ca8251f3 100644 --- a/src/main/kotlin/com/autonomousapps/internal/advice/Advisor.kt +++ b/src/main/kotlin/com/autonomousapps/internal/advice/Advisor.kt @@ -64,7 +64,12 @@ internal class Advisor( val changeToImpl = computeImplDepsWronglyDeclared(unusedDependencies) if (dependencyBundles.isNotEmpty()) { - filterSpecBuilder.dependencyBundleFilter = DependencyBundleFilter(dependencyBundles) + filterSpecBuilder.dependencyBundleFilter = DependencyBundleFilter( + bundles = dependencyBundles, + compileGraph = filterSpecBuilder.compileGraph, + testCompileGraph = filterSpecBuilder.testCompileGraph, + transitives = filterSpecBuilder.usedTransitiveComponents + ) } // update filterSpecBuilder with ktxFilter diff --git a/src/main/kotlin/com/autonomousapps/internal/advice/filter/DependencyBundleFilter.kt b/src/main/kotlin/com/autonomousapps/internal/advice/filter/DependencyBundleFilter.kt index 69021ab0d..aebabee49 100644 --- a/src/main/kotlin/com/autonomousapps/internal/advice/filter/DependencyBundleFilter.kt +++ b/src/main/kotlin/com/autonomousapps/internal/advice/filter/DependencyBundleFilter.kt @@ -3,17 +3,34 @@ package com.autonomousapps.internal.advice.filter import com.autonomousapps.advice.ComponentWithTransitives import com.autonomousapps.advice.HasDependency import com.autonomousapps.advice.TransitiveDependency +import com.autonomousapps.graph.DependencyGraph +import com.autonomousapps.graph.ShortestPath +import com.autonomousapps.internal.TransitiveComponent +import com.autonomousapps.internal.utils.mapToSet internal class DependencyBundleFilter( - private val map: Map> + private val bundles: Map>, + private val compileGraph: DependencyGraph, + private val testCompileGraph: DependencyGraph?, + private val transitives: Set ) : DependencyFilter { companion object { - val EMPTY = DependencyBundleFilter(emptyMap()) + val EMPTY = DependencyBundleFilter(emptyMap(), DependencyGraph(), null, emptySet()) + } + + private val compileParents by lazy(mode = LazyThreadSafetyMode.NONE) { + compileGraph.adj(compileGraph.rootNode).mapToSet { it.to.identifier } + } + + private val testCompileParents by lazy(mode = LazyThreadSafetyMode.NONE) { + testCompileGraph?.let { graph -> + graph.adj(graph.rootNode).mapToSet { it.to.identifier } + }.orEmpty() } override val predicate: (HasDependency) -> Boolean = { - if (map.isEmpty()) { + if (bundles.isEmpty()) { // Exit early if we have no dependencies bundles true } else { @@ -28,44 +45,77 @@ internal class DependencyBundleFilter( } /** - * Return true if [dep] has a parent in the same dependency bundle. + * Return true if [trans] has a parent in the same dependency bundle. */ - private fun computeTransitive(dep: TransitiveDependency): Boolean { + private fun computeTransitive(trans: TransitiveDependency): Boolean { // Find groups that `dep` is a member of - val regexGroups = map.values + val regexGroups = bundles.values val groups = regexGroups.filter { regexes -> regexes.any { regex -> - regex.matches(dep.dependency.identifier) + regex.matches(trans.identifier) } } // Now look for parents in one of these groups, returning true if there is a match return groups.any { regexes -> regexes.any { regex -> - dep.parents.any { parent -> - regex.matches(parent.identifier) - } + isValidCompileBundle(regex, trans.identifier) || + isValidTestCompileBundle(regex, trans.identifier) } } } + private fun isValidCompileBundle(regex: Regex, trans: String): Boolean { + return compileParents.any { parent -> + regex.matches(parent) && isValidBundle(compileGraph, parent, trans) + } + } + + private fun isValidTestCompileBundle(regex: Regex, trans: String): Boolean { + return testCompileParents.any { parent -> + regex.matches(parent) && isValidBundle(testCompileGraph!!, parent, trans) + } + } + + private fun isValidBundle(graph: DependencyGraph, source: String, trans: String): Boolean { + return ShortestPath(graph, source).hasPathTo(trans) + } + /** - * Return true if [dep] has a used transitive in the same dependency bundle. + * Return true if [direct] has a used transitive in the same dependency bundle. */ - private fun computeDirect(dep: ComponentWithTransitives): Boolean { + private fun computeDirect(direct: ComponentWithTransitives): Boolean { // Find groups that `dep` is a member of - val regexGroups = map.values + val regexGroups = bundles.values val groups = regexGroups.filter { regexes -> regexes.any { regex -> - regex.matches(dep.dependency.identifier) + regex.matches(direct.dependency.identifier) } } // Now look for transitives in one of these groups, returning true if there is a match return groups.any { regexes -> regexes.any { regex -> - dep.usedTransitiveDependencies.orEmpty().any { usedTransitive -> - regex.matches(usedTransitive.identifier) + transitives.any { trans -> + isValidDirectBundle(regex, direct.identifier, trans.identifier) } } } } + + private fun isValidDirectBundle(regex: Regex, direct: String, trans: String): Boolean { + return regex.matches(trans) && hasRoute(direct, trans) + } + + private fun hasRoute(direct: String, trans: String): Boolean { + val hasCompileRoute = + if (compileGraph.hasNode(direct)) ShortestPath(compileGraph, direct).hasPathTo(trans) + else false + + if (hasCompileRoute) return true + + return if (testCompileGraph?.hasNode(direct) == true) { + ShortestPath(testCompileGraph, direct).hasPathTo(trans) + } else { + false + } + } } diff --git a/src/main/kotlin/com/autonomousapps/internal/advice/filter/FilterSpec.kt b/src/main/kotlin/com/autonomousapps/internal/advice/filter/FilterSpec.kt index 921c38d45..e6dbacac5 100644 --- a/src/main/kotlin/com/autonomousapps/internal/advice/filter/FilterSpec.kt +++ b/src/main/kotlin/com/autonomousapps/internal/advice/filter/FilterSpec.kt @@ -5,8 +5,15 @@ import com.autonomousapps.advice.HasDependency import com.autonomousapps.extension.Behavior import com.autonomousapps.extension.Ignore import com.autonomousapps.extension.Warn +import com.autonomousapps.graph.DependencyGraph +import com.autonomousapps.internal.TransitiveComponent internal class FilterSpecBuilder { + // Graphs + lateinit var compileGraph: DependencyGraph + var testCompileGraph: DependencyGraph? = null + lateinit var usedTransitiveComponents: Set + // Behaviors var anyBehavior: Behavior = Warn() var unusedDependenciesBehavior: Behavior = Warn() @@ -23,18 +30,16 @@ internal class FilterSpecBuilder { universalFilter = universalFilter.copy(filter) } - fun build(): FilterSpec { - return FilterSpec( - anyBehavior = anyBehavior, - unusedDependenciesBehavior = unusedDependenciesBehavior, - usedTransitivesBehavior = usedTransitivesBehavior, - incorrectConfigurationsBehavior = incorrectConfigurationsBehavior, - compileOnlyBehavior = compileOnlyBehavior, - unusedProcsBehavior = unusedProcsBehavior, - universalFilter = universalFilter, - dependencyBundleFilter = dependencyBundleFilter - ) - } + fun build() = FilterSpec( + anyBehavior = anyBehavior, + unusedDependenciesBehavior = unusedDependenciesBehavior, + usedTransitivesBehavior = usedTransitivesBehavior, + incorrectConfigurationsBehavior = incorrectConfigurationsBehavior, + compileOnlyBehavior = compileOnlyBehavior, + unusedProcsBehavior = unusedProcsBehavior, + universalFilter = universalFilter, + dependencyBundleFilter = dependencyBundleFilter + ) } /** diff --git a/src/main/kotlin/com/autonomousapps/internal/classReferenceParsers.kt b/src/main/kotlin/com/autonomousapps/internal/classReferenceParsers.kt index 034da37e1..9ecca5251 100644 --- a/src/main/kotlin/com/autonomousapps/internal/classReferenceParsers.kt +++ b/src/main/kotlin/com/autonomousapps/internal/classReferenceParsers.kt @@ -33,6 +33,7 @@ internal sealed class ProjectClassReferenceParser( val path = normalizedPath.value val fileExtension = path.substring(path.lastIndexOf(".")) + // TODO two unique files may have the same path (test/foo/Bar.kt, main/foo/Bar.kt) and will be mis-associated with both variants. Unclear if this matters. return variantFiles.filter { path.endsWith("${it.filePath}${fileExtension}") }.mapToOrderedSet { diff --git a/src/main/kotlin/com/autonomousapps/internal/models.kt b/src/main/kotlin/com/autonomousapps/internal/models.kt index 3ef9e6841..697906678 100644 --- a/src/main/kotlin/com/autonomousapps/internal/models.kt +++ b/src/main/kotlin/com/autonomousapps/internal/models.kt @@ -233,7 +233,9 @@ data class TransitiveComponent( * empty if we are unable to determine this. */ val variants: Set = emptySet() -) +) { + val identifier: String = dependency.identifier +} data class ComponentWithInlineMembers( /** diff --git a/src/main/kotlin/com/autonomousapps/tasks/AdvicePerVariantTask.kt b/src/main/kotlin/com/autonomousapps/tasks/AdvicePerVariantTask.kt index 646a4a0f4..2b6e5c84b 100644 --- a/src/main/kotlin/com/autonomousapps/tasks/AdvicePerVariantTask.kt +++ b/src/main/kotlin/com/autonomousapps/tasks/AdvicePerVariantTask.kt @@ -6,6 +6,7 @@ import com.autonomousapps.TASK_GROUP_DEP_INTERNAL import com.autonomousapps.advice.ComponentWithTransitives import com.autonomousapps.extension.Behavior import com.autonomousapps.extension.DependenciesHandler +import com.autonomousapps.graph.DependencyGraph import com.autonomousapps.internal.* import com.autonomousapps.internal.advice.Advisor import com.autonomousapps.internal.advice.filter.* @@ -74,6 +75,15 @@ abstract class AdvicePerVariantTask : DefaultTask() { @get:InputFile abstract val usedVariantDependencies: RegularFileProperty + @get:PathSensitive(PathSensitivity.NONE) + @get:InputFile + abstract val compileGraph: RegularFileProperty + + @get:Optional + @get:PathSensitive(PathSensitivity.NONE) + @get:InputFile + abstract val testCompileGraph: RegularFileProperty + @get:Internal lateinit var dependenciesHandler: DependenciesHandler @@ -134,6 +144,10 @@ abstract class AdvicePerVariantTask : DefaultTask() { @get:Internal abstract val inMemoryCacheProvider: Property + private val usedTransitiveComponents by lazy(mode = LazyThreadSafetyMode.NONE) { + usedTransitiveDependenciesReport.fromJsonSet() + } + @TaskAction fun action() { // Output @@ -147,7 +161,6 @@ abstract class AdvicePerVariantTask : DefaultTask() { val allComponents = allComponentsReport.fromJsonSet() val allComponentsWithTransitives = allComponentsWithTransitives.fromJsonSet() val unusedDirectComponents = unusedDependenciesReport.fromJsonSet() - val usedTransitiveComponents = usedTransitiveDependenciesReport.fromJsonSet() val abiDeps = abiDependenciesReport.orNull?.asFile?.readText()?.fromJsonSet() ?.mapToSet { it.dependency } ?: emptySet() @@ -195,6 +208,10 @@ abstract class AdvicePerVariantTask : DefaultTask() { } private fun filterSpecBuilder() = FilterSpecBuilder().apply { + compileGraph = this@AdvicePerVariantTask.compileGraph.fromJson() + testCompileGraph = this@AdvicePerVariantTask.testCompileGraph.orNull?.fromJson() + usedTransitiveComponents = this@AdvicePerVariantTask.usedTransitiveComponents + universalFilter = CompositeFilter(filters) anyBehavior = this@AdvicePerVariantTask.anyBehavior.get() unusedDependenciesBehavior = this@AdvicePerVariantTask.unusedDependenciesBehavior.get() diff --git a/src/main/kotlin/com/autonomousapps/tasks/DependencyGraphPerVariant.kt b/src/main/kotlin/com/autonomousapps/tasks/DependencyGraphPerVariant.kt index 3041f35d1..59d9cce6b 100644 --- a/src/main/kotlin/com/autonomousapps/tasks/DependencyGraphPerVariant.kt +++ b/src/main/kotlin/com/autonomousapps/tasks/DependencyGraphPerVariant.kt @@ -3,11 +3,8 @@ package com.autonomousapps.tasks import com.autonomousapps.TASK_GROUP_DEP_INTERNAL import com.autonomousapps.graph.DependencyGraph import com.autonomousapps.graph.GraphWriter +import com.autonomousapps.internal.artifactViewFor import com.autonomousapps.internal.utils.* -import com.autonomousapps.internal.utils.asString -import com.autonomousapps.internal.utils.getAndDelete -import com.autonomousapps.internal.utils.mapNotNullToSet -import com.autonomousapps.internal.utils.toIdentifier import org.gradle.api.DefaultTask import org.gradle.api.artifacts.Configuration import org.gradle.api.artifacts.FileCollectionDependency @@ -15,15 +12,10 @@ import org.gradle.api.artifacts.result.ResolvedComponentResult import org.gradle.api.artifacts.result.ResolvedDependencyResult import org.gradle.api.attributes.Attribute import org.gradle.api.attributes.Category -import org.gradle.api.file.ConfigurableFileCollection +import org.gradle.api.file.FileCollection import org.gradle.api.file.RegularFileProperty import org.gradle.api.provider.Property -import org.gradle.api.tasks.CacheableTask -import org.gradle.api.tasks.Classpath -import org.gradle.api.tasks.Input -import org.gradle.api.tasks.Internal -import org.gradle.api.tasks.OutputFile -import org.gradle.api.tasks.TaskAction +import org.gradle.api.tasks.* @CacheableTask abstract class DependencyGraphPerVariant : DefaultTask() { @@ -33,13 +25,28 @@ abstract class DependencyGraphPerVariant : DefaultTask() { description = "Produces the dependency graph, for a given variant, for the current project" } - @get:Classpath - abstract val compileClasspathArtifacts: ConfigurableFileCollection + @get:Internal + abstract val jarAttr: Property + + @Classpath + fun getCompileClasspathArtifacts(): FileCollection = compileClasspath + .incoming + .artifactViewFor(jarAttr.get()) + .artifacts + .artifactFiles + + @Optional + @Classpath + fun getTestCompileClasspathArtifacts(): FileCollection? = testCompileClasspath + ?.incoming + ?.artifactViewFor(jarAttr.get()) + ?.artifacts + ?.artifactFiles /** - * This is required as an input, in addition to [compileClasspathArtifacts], because the latter - * (which is just a classpath), can be the same for multiple projects, which leads to incorrect - * output, since we expect our output to have a node for _this_ project. + * This is required as an input, in addition to [getCompileClasspathArtifacts] and [getTestCompileClasspathArtifacts], + * because those (which are just classpaths), can be the same for multiple projects, which leads to incorrect output, + * since we expect our output to have a node for _this_ project. */ @get:Input abstract val projectPath: Property @@ -47,23 +54,43 @@ abstract class DependencyGraphPerVariant : DefaultTask() { @get:Internal lateinit var compileClasspath: Configuration + @get:Internal + var testCompileClasspath: Configuration? = null + + @get:OutputFile + abstract val compileOutputJson: RegularFileProperty + @get:OutputFile - abstract val outputJson: RegularFileProperty + abstract val testCompileOutputJson: RegularFileProperty @get:OutputFile - abstract val outputDot: RegularFileProperty + abstract val compileOutputDot: RegularFileProperty + + @get:OutputFile + abstract val testCompileOutputDot: RegularFileProperty @TaskAction fun action() { - val outputJsonFile = outputJson.getAndDelete() - val outputDotFile = outputDot.getAndDelete() + val compileOutputJsonFile = compileOutputJson.getAndDelete() + val testCompileOutputJsonFile = testCompileOutputJson.getAndDelete() + val compileOutputDotFile = compileOutputDot.getAndDelete() + val testCompileOutputDotFile = testCompileOutputDot.getAndDelete() + + val compileGraph = DependencyGraphWalker(compileClasspath).graph + val testCompileGraph = testCompileClasspath?.let { DependencyGraphWalker(it).graph } + + logger.log("Compile graph JSON at ${compileOutputJsonFile.path}") + compileOutputJsonFile.writeText(compileGraph.toJson()) - val graph = DependencyGraphWalker(compileClasspath).graph + logger.log("Graph DOT at ${compileOutputDotFile.path}") + compileOutputDotFile.writeText(GraphWriter.toDot(compileGraph)) - logger.log("Graph JSON at ${outputJsonFile.path}") - outputJsonFile.writeText(graph.toJson()) + if (testCompileGraph != null) { + logger.log("Test compile graph JSON at ${testCompileOutputJsonFile.path}") + testCompileOutputJsonFile.writeText(testCompileGraph.toJson()) - logger.log("Graph DOT at ${outputDotFile.path}") - outputDotFile.writeText(GraphWriter.toDot(graph)) + logger.log("Graph DOT at ${testCompileOutputDotFile.path}") + testCompileOutputDotFile.writeText(GraphWriter.toDot(testCompileGraph)) + } } } @@ -109,6 +136,8 @@ private class DependencyGraphWalker(conf: Configuration) { .filterNot { it.isConstraint } // For similar reasons as above .filterNot { it.isJavaPlatform() } + // Sometimes there is a self-dependency? + .filterNot { it.selected == root } .forEach { dependencyResult -> val depId = dependencyResult.selected.id.asString() diff --git a/src/main/kotlin/com/autonomousapps/tasks/DependencyMisuseTask.kt b/src/main/kotlin/com/autonomousapps/tasks/DependencyMisuseTask.kt index 5e8e68197..c0fab79c3 100644 --- a/src/main/kotlin/com/autonomousapps/tasks/DependencyMisuseTask.kt +++ b/src/main/kotlin/com/autonomousapps/tasks/DependencyMisuseTask.kt @@ -15,6 +15,7 @@ import org.gradle.api.attributes.Attribute import org.gradle.api.attributes.Category import org.gradle.api.file.FileCollection import org.gradle.api.file.RegularFileProperty +import org.gradle.api.provider.Property import org.gradle.api.tasks.* /** @@ -28,11 +29,18 @@ abstract class DependencyMisuseTask : DefaultTask() { description = "Produces a report of unused direct dependencies and used transitive dependencies" } + @get:Internal + abstract val jarAttr: Property + /** * This is the "official" input for wiring task dependencies correctly, but is otherwise unused. */ - @get:Classpath - lateinit var artifactFiles: FileCollection + @Classpath + fun getCompileArtifactFiles(): FileCollection = compileConfiguration + .incoming + .artifactViewFor(jarAttr.get()) + .artifacts + .artifactFiles /** * This is what the task actually uses as its input. @@ -40,6 +48,24 @@ abstract class DependencyMisuseTask : DefaultTask() { @get:Internal lateinit var compileConfiguration: Configuration + /** + * This is the "official" input for wiring task dependencies correctly, but is otherwise unused. + * May be absent if, e.g., Android unit tests are disabled for some variant. + */ + @Optional + @Classpath + fun getTestCompileArtifactFiles(): FileCollection? = testCompileConfiguration + ?.incoming + ?.artifactViewFor(jarAttr.get()) + ?.artifacts + ?.artifactFiles + + /** + * This is what the task actually uses as its input. + */ + @get:Internal + var testCompileConfiguration: Configuration? = null + @get:PathSensitive(PathSensitivity.NONE) @get:InputFile abstract val declaredDependencies: RegularFileProperty @@ -101,10 +127,14 @@ abstract class DependencyMisuseTask : DefaultTask() { val outputUsedVariantDependenciesFile = outputUsedVariantDependencies.getAndDelete() // Inputs - val resolvedComponentResult: ResolvedComponentResult = compileConfiguration + val resolvedCompileClasspath = compileConfiguration .incoming .resolutionResult .root + val resolvedTestCompileClasspath = testCompileConfiguration + ?.incoming + ?.resolutionResult + ?.root val dependencyReport = MisusedDependencyDetector( declaredComponents = declaredDependencies.fromJsonSet(), @@ -116,7 +146,8 @@ abstract class DependencyMisuseTask : DefaultTask() { usedAndroidResBySourceDependencies = usedAndroidResBySourceDependencies.fromNullableJsonSet(), usedAndroidResByResDependencies = usedAndroidResByResDependencies.fromNullableJsonSet(), nativeLibDependencies = nativeLibDependencies.fromNullableJsonSet(), - root = resolvedComponentResult + resolvedCompileClasspath = resolvedCompileClasspath, + resolvedTestCompileClasspath = resolvedTestCompileClasspath ).detect() // Reports @@ -137,7 +168,8 @@ internal class MisusedDependencyDetector( private val usedAndroidResBySourceDependencies: Set?, private val usedAndroidResByResDependencies: Set?, private val nativeLibDependencies: Set?, - private val root: ResolvedComponentResult + private val resolvedCompileClasspath: ResolvedComponentResult, + private val resolvedTestCompileClasspath: ResolvedComponentResult? ) { fun detect(): DependencyReport { val unusedDeps = mutableListOf() @@ -205,69 +237,40 @@ internal class MisusedDependencyDetector( if (variantClasses.isNotEmpty()) { val classes = variantClasses.mapToOrderedSet { it.theClass } val variants = variantClasses.flatMapToOrderedSet { it.variants } - usedTransitiveComponents.add(TransitiveComponent( - dependency = component.dependency, - usedTransitiveClasses = classes, - variants = variants - )) + usedTransitiveComponents.add( + TransitiveComponent( + dependency = component.dependency, + usedTransitiveClasses = classes, + variants = variants + ) + ) } } - // Connect used-transitives to direct dependencies - val withTransitives = LinkedHashMap>().apply { - // Seed with unused dependencies because final collection is expected to contain one entry per - // unused dep. - putAll(unusedDeps.map { it to mutableSetOf() }) - } - val visited = mutableSetOf() - - fun walk(root: ResolvedComponentResult) { - val rootId = root.id.asString() - // we map our current `root` to a known declared dependency (may be null if the root is not a - // declared dependency). - val rootComponent = declaredComponents.find { it.dependency.identifier == rootId } - - root.dependencies - .filterIsInstance() - // AGP adds all runtime dependencies as constraints to the compile classpath, and these show - // up in the resolution result. Filter them out. - .filterNot { it.isConstraint } - // For similar reasons as above - .filterNot { it.isJavaPlatform() } - .forEach { dependencyResult -> - val depId = dependencyResult.selected.id.asString() - if (!visited.contains(depId)) { - visited.add(depId) - // recursively walk the graph in a depth-first pattern - walk(dependencyResult.selected) - } - - if (rootComponent != null && usedTransitiveComponents.contains(depId)) { - val dep = Dependency( - identifier = depId, - resolvedVersion = dependencyResult.selected.id.resolvedVersion() - ) - withTransitives.merge(rootComponent.dependency, mutableSetOf(dep)) { acc, inc -> - acc.apply { addAll(inc) } - } - } - } - } - walk(root) - val declaredComponentsWithTransitives = withTransitives.map { (key, value) -> - val trans = value.ifEmpty { null } - ComponentWithTransitives(key, trans) - }.toSet() + val declaredCompileComponentsWithTransitives = ClasspathGraphWalker( + resolvedCompileClasspath, + declaredComponents, + usedTransitiveComponents, + unusedDeps + ).getComponents() + // TODO this is currently unused but that feels wrong... + val declaredTestCompileComponentsWithTransitives = resolvedTestCompileClasspath?.let { + ClasspathGraphWalker( + resolvedTestCompileClasspath, + declaredComponents, + usedTransitiveComponents, + unusedDeps + ).getComponents() + } ?: emptySet() // Filter above to only get those that are unused - val unusedDepsWithTransitives: Set = - declaredComponentsWithTransitives.filterToSet { comp -> - unusedDeps.any { it == comp.dependency } - } + val unusedComponentsWithTransitives = declaredCompileComponentsWithTransitives.filterToSet { comp -> + unusedDeps.any { it == comp.dependency } + } return DependencyReport( - allComponentsWithTransitives = declaredComponentsWithTransitives, - unusedComponentsWithTransitives = unusedDepsWithTransitives, + allComponentsWithTransitives = declaredCompileComponentsWithTransitives, + unusedComponentsWithTransitives = unusedComponentsWithTransitives, usedTransitives = usedTransitiveComponents, usedDependencies = usedDependencies.toVariantDependencies() ) @@ -314,10 +317,6 @@ internal class MisusedDependencyDetector( return manifest.componentMap.isEmpty() } - private fun Set.contains(identifier: String): Boolean { - return map { trans -> trans.dependency.identifier }.contains(identifier) - } - internal class DependencyReport( val allComponentsWithTransitives: Set, val unusedComponentsWithTransitives: Set, @@ -326,6 +325,76 @@ internal class MisusedDependencyDetector( ) } +private class ClasspathGraphWalker( + root: ResolvedComponentResult, + private val declaredComponents: Set, + private val usedTransitiveComponents: Set, + private val unusedDependencies: List +) { + + // Connect used-transitives to direct dependencies + private val withTransitives = LinkedHashMap>().apply { + // Seed with unused dependencies because final collection is expected to contain one entry per + // unused dep. + putAll(unusedDependencies.map { it to mutableSetOf() }) + } + private val visited = mutableSetOf() + + private var components: Set + + init { + walk(root) + + components = withTransitives.mapToOrderedSet { (key, value) -> + val trans = value.ifEmpty { null } + ComponentWithTransitives(key, trans) + } + } + + /** + * The returned components are all direct dependencies of a project. They may or may not be used, and are linked to + * zero or more transitive dependencies that _are_ used. + */ + fun getComponents(): Set = components + + private fun walk(root: ResolvedComponentResult) { + val rootId = root.id.asString() + // we map our current `root` to a known declared dependency (may be null if the root is not a + // declared dependency). + val rootComponent = declaredComponents.find { it.dependency.identifier == rootId } + + root.dependencies + .filterIsInstance() + // AGP adds all runtime dependencies as constraints to the compile classpath, and these show + // up in the resolution result. Filter them out. + .filterNot { it.isConstraint } + // For similar reasons as above + .filterNot { it.isJavaPlatform() } + .forEach { dependencyResult -> + val depId = dependencyResult.selected.id.asString() + if (!visited.contains(depId)) { + visited.add(depId) + // recursively walk the graph in a depth-first pattern + walk(dependencyResult.selected) + } + + if (rootComponent != null && usedTransitiveComponents.contains(depId)) { + val dep = Dependency( + identifier = depId, + resolvedVersion = dependencyResult.selected.id.resolvedVersion() + ) + withTransitives.merge(rootComponent.dependency, mutableSetOf(dep)) { acc, inc -> + acc.apply { addAll(inc) } + } + } + } + } + + private fun Set.contains(identifier: String): Boolean { + return map { trans -> trans.dependency.identifier }.contains(identifier) + } +} + /** * Returns true if any of the variants are a kind of platform. */ diff --git a/testkit/src/main/kotlin/com/autonomousapps/kit/Dependency.kt b/testkit/src/main/kotlin/com/autonomousapps/kit/Dependency.kt index 8ab21fdcd..92acd4e35 100644 --- a/testkit/src/main/kotlin/com/autonomousapps/kit/Dependency.kt +++ b/testkit/src/main/kotlin/com/autonomousapps/kit/Dependency.kt @@ -73,6 +73,11 @@ class Dependency @JvmOverloads constructor( return Dependency(configuration, "org.conscrypt:conscrypt-openjdk-uber:2.4.0") } + @JvmStatic + fun kotestAssertions(configuration: String): Dependency { + return Dependency(configuration, "io.kotest:kotest-assertions-core-jvm:4.6.0") + } + @JvmStatic fun moshi(configuration: String): Dependency { return Dependency(configuration, "com.squareup.moshi:moshi:1.11.0")