Skip to content

Commit

Permalink
feat: support reasoning about multiple pieces of advice.
Browse files Browse the repository at this point in the history
  • Loading branch information
autonomousapps committed Jan 29, 2025
1 parent 1795581 commit 62207d6
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,16 @@ final class RuntimeOnlySpec extends AbstractJvmSpec {
then: 'advice is correct'
assertThat(project.actualBuildHealth()).containsExactlyElementsIn(project.expectedBuildHealth)
and: 'reason makes sense'
and: 'reason makes sense and supports multiple pieces of advice for the same dependency'
def output = Colors.decolorize(result.output)
assertThat(output).contains('You have been advised to change this dependency to \'runtimeOnly\' from \'implementation\'.')
assertThat(output).contains('There is no advice regarding this dependency.')
assertThat(output).contains(
'''\
------------------------------------------------------------
You asked about the dependency 'org.apache.spark:spark-sql_2.12:3.5.0'.
You have been advised to add this dependency to 'testImplementation'.
You have been advised to change this dependency to 'runtimeOnly' from 'implementation'.
------------------------------------------------------------'''.stripIndent()
)
where:
gradleVersion << gradleVersions()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,16 @@ internal class DependencyAdviceExplainer(
private val target: Coordinates,
private val requestedCapability: String,
private val usages: Set<Usage>,
private val advice: Advice?,
private val advice: Set<Advice>,
private val dependencyGraph: Map<String, DependencyGraphView>,
private val bundleTraces: Set<BundleTrace>,
private val wasFiltered: Boolean,
private val dependencyMap: ((String) -> String?)? = null
) : ReasonTask.Explainer {

override fun computeReason() = buildString {
val ruleLength = 60
private val ruleLength = 60

override fun computeReason() = buildString {
// Header
appendReproducibleNewLine()
append(Colors.BOLD)
Expand All @@ -60,9 +60,9 @@ internal class DependencyAdviceExplainer(

private val bundle = "bundle".colorize(Colors.BOLD)

private fun adviceText(): String = when {
advice == null -> {
if (bundleTraces.isNotEmpty()) {
private fun adviceText(): String {
if (advice.isEmpty()) {
return if (bundleTraces.isNotEmpty()) {
when (val trace = findTrace() ?: error("There must be a match. Available traces: $bundleTraces")) {
is BundleTrace.DeclaredParent -> {
"There is no advice regarding this dependency.\nIt was removed because it matched a $bundle rule for " +
Expand All @@ -87,29 +87,43 @@ internal class DependencyAdviceExplainer(
}
}

advice.isAdd() -> {
val trace = findTrace()
if (trace != null) {
check(trace is BundleTrace.PrimaryMap) { "Expected a ${BundleTrace.PrimaryMap::class.java.simpleName}" }
val builder = StringBuilder()

"You have been advised to add this dependency to '${advice.toConfiguration!!.colorize(Colors.GREEN)}'.\n" +
"It matched a $bundle rule: ${printableIdentifier(trace.primary).colorize(Colors.BOLD)} was substituted for " +
"${printableIdentifier(trace.subordinate).colorize(Colors.BOLD)}."
} else {
"You have been advised to add this dependency to '${advice.toConfiguration!!.colorize(Colors.GREEN)}'."
}
}
advice.forEach { a ->
val text = when {
a.isAdd() -> {
val trace = findTrace()
if (trace != null) {
check(trace is BundleTrace.PrimaryMap) { "Expected a ${BundleTrace.PrimaryMap::class.java.simpleName}" }

advice.isRemove() || advice.isProcessor() -> {
"You have been advised to remove this dependency from '${advice.fromConfiguration!!.colorize(Colors.RED)}'."
}
"You have been advised to add this dependency to '${a.toConfiguration!!.colorize(Colors.GREEN)}'.\n" +
"It matched a $bundle rule: ${printableIdentifier(trace.primary).colorize(Colors.BOLD)} was substituted for " +
"${printableIdentifier(trace.subordinate).colorize(Colors.BOLD)}."
} else {
"You have been advised to add this dependency to '${a.toConfiguration!!.colorize(Colors.GREEN)}'."
}
}

advice.isChange() || advice.isRuntimeOnly() || advice.isCompileOnly() -> {
"You have been advised to change this dependency to '${advice.toConfiguration!!.colorize(Colors.GREEN)}' " +
"from '${advice.fromConfiguration!!.colorize(Colors.YELLOW)}'."
a.isRemove() || a.isProcessor() -> {
"You have been advised to remove this dependency from '${a.fromConfiguration!!.colorize(Colors.RED)}'."
}

a.isChange() || a.isRuntimeOnly() || a.isCompileOnly() -> {
"You have been advised to change this dependency to '${a.toConfiguration!!.colorize(Colors.GREEN)}' " +
"from '${a.fromConfiguration!!.colorize(Colors.YELLOW)}'."
}

else -> error("Unknown advice type: $advice")
}

if (builder.isNotEmpty()) {
// If we've already added some text, we need to insert a newline before adding more.
builder.appendLine()
}
builder.append(text)
}

else -> error("Unknown advice type: $advice")
return builder.toString()
}

// TODO(tsr): what are the valid scenarios? How many traces could there be for a single target?
Expand Down
10 changes: 7 additions & 3 deletions src/main/kotlin/com/autonomousapps/tasks/ReasonTask.kt
Original file line number Diff line number Diff line change
Expand Up @@ -341,8 +341,8 @@ abstract class ReasonTask @Inject constructor(
}

/** Returns null if there is no advice for the given id. */
private fun findAdviceIn(projectAdvice: ProjectAdvice): Advice? {
return projectAdvice.dependencyAdvice.find { advice ->
private fun findAdviceIn(projectAdvice: ProjectAdvice): Set<Advice> {
return projectAdvice.dependencyAdvice.filterToSet { advice ->
val adviceGav = advice.coordinates.gav()
adviceGav == targetCoord.gav() || adviceGav == requestedCoord.gav()
}
Expand All @@ -356,7 +356,11 @@ abstract class ReasonTask @Inject constructor(
}
}

private fun wasFiltered(): Boolean = finalAdvice == null && unfilteredAdvice != null
private fun wasFiltered(): Boolean {
return unfilteredAdvice.any { unfiltered ->
unfiltered !in finalAdvice
}
}

internal companion object {
internal fun findFilteredDependencyKey(dependencies: Set<Map.Entry<String, Any>>, requestedId: String): String? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ class DependencyAdviceExplainerTest {
requestedCapability = "",
target = target,
usages = usages,
advice = advice,
advice = advice?.let { setOf(it) }.orEmpty(),
dependencyGraph = mapOf("main" to graphView),
bundleTraces = bundleTraces,
wasFiltered = wasFiltered,
Expand Down

0 comments on commit 62207d6

Please sign in to comment.