Skip to content

Commit a1f3b5b

Browse files
fix: ProjectAdvice is totally Comparable.
1 parent 8a5b381 commit a1f3b5b

File tree

3 files changed

+36
-5
lines changed

3 files changed

+36
-5
lines changed

src/main/kotlin/com/autonomousapps/model/ModuleAdvice.kt

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import com.squareup.moshi.JsonClass
99
import dev.zacsweers.moshix.sealed.annotations.TypeLabel
1010

1111
@JsonClass(generateAdapter = false, generator = "sealed:type")
12-
sealed class ModuleAdvice {
12+
sealed class ModuleAdvice : Comparable<ModuleAdvice> {
1313

1414
abstract val name: String
1515

@@ -19,11 +19,26 @@ sealed class ModuleAdvice {
1919

2020
internal abstract fun isActionable(): Boolean
2121

22+
override fun compareTo(other: ModuleAdvice): Int {
23+
if (this === other) return 0
24+
25+
if (this is AndroidScore && other is AndroidScore) {
26+
return compareBy(AndroidScore::hasAndroidAssets)
27+
.thenBy(AndroidScore::hasAndroidRes)
28+
.thenBy(AndroidScore::usesAndroidClasses)
29+
.thenBy(AndroidScore::hasBuildConfig)
30+
.thenBy(AndroidScore::hasAndroidDependencies)
31+
.compare(this, other)
32+
}
33+
34+
// Impossible until we had another kind of ModuleAdvice.
35+
error("Expected to be comparing AndroidScores, was this=${javaClass.simpleName}, other=${other.javaClass.simpleName}")
36+
}
37+
2238
internal companion object {
2339
/** Returns `true` if [moduleAdvice] is effectively empty or unactionable. */
2440
fun isEmpty(moduleAdvice: Set<ModuleAdvice>) = moduleAdvice.none { it.isActionable() }
2541

26-
2742
/** Returns `true` if [moduleAdvice] is in any way actionable. */
2843
fun isNotEmpty(moduleAdvice: Set<ModuleAdvice>) = !isEmpty(moduleAdvice)
2944
}

src/main/kotlin/com/autonomousapps/model/ProjectAdvice.kt

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// SPDX-License-Identifier: Apache-2.0
33
package com.autonomousapps.model
44

5+
import com.autonomousapps.internal.utils.LexicographicIterableComparator
56
import com.squareup.moshi.JsonClass
67

78
/** Collection of all advice for a single project, across all variants. */
@@ -19,6 +20,13 @@ data class ProjectAdvice(
1920
fun isEmpty(): Boolean = dependencyAdvice.isEmpty() && pluginAdvice.isEmpty() && warning.isEmpty()
2021
fun isNotEmpty(): Boolean = !isEmpty()
2122

22-
// TODO(tsr): this compareTo function violates the Comparable contract by only considering one field.
23-
override fun compareTo(other: ProjectAdvice): Int = projectPath.compareTo(other.projectPath)
23+
override fun compareTo(other: ProjectAdvice): Int {
24+
return compareBy(ProjectAdvice::projectPath)
25+
.thenBy(LexicographicIterableComparator()) { it.dependencyAdvice }
26+
.thenBy(LexicographicIterableComparator()) { it.pluginAdvice }
27+
.thenBy(LexicographicIterableComparator()) { it.moduleAdvice }
28+
.thenBy(ProjectAdvice::warning)
29+
.thenBy(ProjectAdvice::shouldFail)
30+
.compare(this, other)
31+
}
2432
}
Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,24 @@
11
package com.autonomousapps.model
22

3+
import com.autonomousapps.internal.utils.LexicographicIterableComparator
34
import com.squareup.moshi.JsonClass
45

56
/** Warnings about the project for which the plugin cannot yet make universally actionable advice. */
67
@JsonClass(generateAdapter = false)
7-
data class Warning(val duplicateClasses: Set<DuplicateClass>) {
8+
data class Warning(
9+
val duplicateClasses: Set<DuplicateClass>,
10+
) : Comparable<Warning> {
811

912
internal companion object {
1013
@JvmStatic
1114
fun empty() = Warning(emptySet())
1215
}
1316

17+
override fun compareTo(other: Warning): Int {
18+
return LexicographicIterableComparator<DuplicateClass>()
19+
.compare(duplicateClasses, other.duplicateClasses)
20+
}
21+
1422
fun isEmpty(): Boolean = duplicateClasses.isEmpty()
1523
fun isNotEmpty(): Boolean = !isEmpty()
1624
}

0 commit comments

Comments
 (0)