Skip to content

Commit 5e3eb8c

Browse files
fix: exceptions, main->test visibility, reason for android lint jars.
1. reason output said "implies implementation" when it should have been "implies runtimeOnly" in the android lint jar case. 2. The new "exceptions are special" handling was over-eager and flagging things that only referenced exceptions. We only want to flag things that provide the exception types. 3. MutableSet<Usage>.simplify(visibility, bucket) in StandardTransform wasn't properly handling the case when a usage was visible on both compile and runtime classpaths. In such a case, we can filter out all usages.
1 parent 99b10f8 commit 5e3eb8c

10 files changed

Lines changed: 198 additions & 36 deletions

File tree

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// Copyright (c) 2026. Tony Robalik.
2+
// SPDX-License-Identifier: Apache-2.0
3+
package com.autonomousapps.android
4+
5+
import com.autonomousapps.android.projects.ExceptionsAreSpecialProject
6+
import com.autonomousapps.utils.Colors
7+
8+
import static com.autonomousapps.advice.truth.BuildHealthSubject.buildHealth
9+
import static com.autonomousapps.kit.GradleBuilder.build
10+
import static com.google.common.truth.Truth.assertAbout
11+
import static com.google.common.truth.Truth.assertThat
12+
13+
class ExceptionsAreSpecialSpec extends AbstractAndroidSpec {
14+
15+
@SuppressWarnings('GroovyAssignabilityCheck')
16+
def "exception types are correctly mapped to the providing dependencies (#gradleVersion AGP #agpVersion)"() {
17+
given:
18+
def project = new ExceptionsAreSpecialProject(agpVersion)
19+
gradleProject = project.gradleProject
20+
21+
when:
22+
def result = build(gradleVersion, gradleProject.rootDir, ':buildHealth', ':consumer:reason', '--id', 'androidx.navigation:navigation-common:')
23+
24+
then:
25+
assertAbout(buildHealth())
26+
.that(project.actualProjectAdvice())
27+
.isEquivalentIgnoringModuleAdviceAndWarnings(project.expectedProjectAdvice())
28+
29+
and:
30+
assertThat(Colors.decolorize(result.output)).contains(project.expectedReason())
31+
32+
where:
33+
[gradleVersion, agpVersion] << gradleAgpMatrix()
34+
}
35+
}

src/functionalTest/groovy/com/autonomousapps/android/projects/AndroidTestSourceProject.groovy

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import com.autonomousapps.kit.GradleProject
66
import com.autonomousapps.kit.Source
77
import com.autonomousapps.kit.SourceType
88
import com.autonomousapps.kit.android.AndroidColorRes
9-
import com.autonomousapps.kit.android.AndroidManifest
109
import com.autonomousapps.kit.android.AndroidStyleRes
1110
import com.autonomousapps.kit.gradle.Dependency
1211
import com.autonomousapps.kit.gradle.Plugin
@@ -86,16 +85,6 @@ final class AndroidTestSourceProject extends AbstractAndroidProject {
8685

8786
private List<Source> appSources() {
8887
def sources = [
89-
new Source(
90-
SourceType.KOTLIN, 'App', 'com/example',
91-
"""\
92-
package com.example
93-
94-
class App {
95-
fun magic() = 42
96-
}
97-
""".stripIndent()
98-
),
9988
new Source(
10089
SourceType.KOTLIN, 'Test', 'com/example',
10190
"""\
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
// Copyright (c) 2026. Tony Robalik.
2+
// SPDX-License-Identifier: Apache-2.0
3+
package com.autonomousapps.android.projects
4+
5+
import com.autonomousapps.kit.GradleProject
6+
import com.autonomousapps.kit.Source
7+
import com.autonomousapps.kit.gradle.Dependency
8+
import com.autonomousapps.model.Advice
9+
import com.autonomousapps.model.ProjectAdvice
10+
11+
import static com.autonomousapps.AdviceHelper.*
12+
import static com.autonomousapps.kit.gradle.Dependency.implementation
13+
import static com.autonomousapps.kit.gradle.Dependency.testImplementation
14+
15+
final class ExceptionsAreSpecialProject extends AbstractAndroidProject {
16+
17+
private static final Dependency HILT_IMPL = implementation('androidx.hilt:hilt-navigation-compose:1.2.0')
18+
private static final Dependency LAYOUTLIB_IMPL = testImplementation('com.android.tools.layoutlib:layoutlib:15.2.3')
19+
20+
final GradleProject gradleProject
21+
22+
ExceptionsAreSpecialProject(String agpVersion) {
23+
super(agpVersion)
24+
this.gradleProject = build()
25+
}
26+
27+
private GradleProject build() {
28+
return newAndroidGradleProjectBuilder(agpVersion)
29+
.withAndroidLibProject('consumer') { s ->
30+
s.sources = consumerSources()
31+
s.withBuildScript { bs ->
32+
bs.android = defaultAndroidLibBlock(false, 'com.example.consumer')
33+
bs.plugins(androidLib(false))
34+
bs.dependencies(
35+
HILT_IMPL,
36+
LAYOUTLIB_IMPL,
37+
)
38+
}
39+
}
40+
.write()
41+
}
42+
43+
private static final List<Source> consumerSources() {
44+
return [
45+
Source.java(
46+
'''\
47+
package mutual.aid.consumer;
48+
49+
// layoutlib bundles this class
50+
import android.content.Context;
51+
52+
public abstract class ConsumerTest {
53+
public abstract Context getContext();
54+
}
55+
'''.stripIndent()
56+
)
57+
.withSourceSet('test')
58+
.build()
59+
]
60+
}
61+
62+
Set<ProjectAdvice> actualProjectAdvice() {
63+
return actualProjectAdvice(gradleProject)
64+
}
65+
66+
private final Set<Advice> consumerAdvice() {
67+
[
68+
Advice.ofRemove(moduleCoordinates(HILT_IMPL), HILT_IMPL.configuration),
69+
70+
// These two both provide runtime capabilities and would be removed with the removal of hilt, so we advise they
71+
// be added directly to minimize changes to the runtime classpath.
72+
Advice.ofAdd(moduleCoordinates('com.google.dagger:dagger-lint-aar:2.49'), 'runtimeOnly'),
73+
Advice.ofAdd(moduleCoordinates('org.jetbrains.kotlinx:kotlinx-coroutines-android:1.6.4'), 'runtimeOnly')
74+
]
75+
}
76+
77+
Set<ProjectAdvice> expectedProjectAdvice() {
78+
return [projectAdviceForDependencies(':consumer', consumerAdvice())]
79+
}
80+
81+
String expectedReason() {
82+
return '''\
83+
Source: debug, test
84+
-------------------
85+
(no usages)'''.stripIndent()
86+
}
87+
}

src/functionalTest/groovy/com/autonomousapps/jvm/ExceptionsAreSpecialSpec.groovy

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import static com.google.common.truth.Truth.assertThat
1111
class ExceptionsAreSpecialSpec extends AbstractJvmSpec {
1212

1313
@SuppressWarnings('GroovyAssignabilityCheck')
14-
def "annotations on public classes are part of the abi (#gradleVersion isBroken=#isBroken)"() {
14+
def "exception types are required at runtime (#gradleVersion isBroken=#isBroken)"() {
1515
given:
1616
def project = new ExceptionsAreSpecialProject(isBroken)
1717
gradleProject = project.gradleProject

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

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -209,22 +209,29 @@ internal class StandardTransform(
209209
sourceSetName: String,
210210
): MutableSet<Usage> {
211211
fun MutableSet<Usage>.maybeReduceFurther(): MutableSet<Usage> {
212-
return if (projectType == ProjectType.ANDROID && sourceSetName == SourceKind.TEST_NAME) {
212+
return if (projectType == ProjectType.ANDROID && sourceSetName == SourceKind.TEST_NAME) { // TODO: `&& SourceKind.ANDROID_TEST_NAME`?
213213
reduceUsages(this)
214214
} else {
215215
this
216216
}
217217
}
218218

219-
// TODO(tsr): is there a case where both forCompile and forRuntime are true? What then?
220-
return if (visibility.forCompile && !explicitFor(sourceSetName)) {
221-
asSequence()
222-
.filterNot { usage -> usage.bucket != Bucket.RUNTIME_ONLY }
223-
.toMutableSet()
224-
.maybeReduceFurther()
225-
} else if (visibility.forRuntime && !explicitFor(sourceSetName)) {
226-
asSequence()
227-
.filterNot { usage -> usage.bucket == Bucket.RUNTIME_ONLY }
219+
return if (visibility.forEither && !explicitFor(sourceSetName)) {
220+
// TODO(tsr): add test for this case.
221+
// 1. a dep that should be moved from impl->api
222+
// 2. dep has runtime capabilities as well and is on test runtime classpath
223+
// 3. previously we'd see advice to both move to api and also add to testRuntimeOnly. The latter is undesirable.
224+
225+
// Yes, if something is visible forCompile and forRuntime, then we filter out all usages.
226+
var seq = asSequence()
227+
if (visibility.forCompile) {
228+
seq = seq.filterNot { usage -> usage.bucket != Bucket.RUNTIME_ONLY }
229+
}
230+
if (visibility.forRuntime) {
231+
seq = seq.filterNot { usage -> usage.bucket == Bucket.RUNTIME_ONLY }
232+
}
233+
234+
seq
228235
.toMutableSet()
229236
.maybeReduceFurther()
230237
} else {

src/main/kotlin/com/autonomousapps/internal/utils/collections.kt

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,9 @@ internal inline fun <T, R : Any> Iterable<T>.mapNotNullToOrderedSet(transform: (
146146
return mapNotNullTo(TreeSet(), transform)
147147
}
148148

149-
internal inline fun <T, R : Any, V : Any> Iterable<T>.mapSecondNotNull(transform: (T) -> Pair<R, V?>): List<Pair<R, V>> {
149+
internal inline fun <T, R : Any, V : Any> Iterable<T>.mapSecondNotNull(
150+
transform: (T) -> Pair<R, V?>
151+
): List<Pair<R, V>> {
150152
return mapNotNull {
151153
val pair = transform(it)
152154
if (pair.second != null) {
@@ -158,6 +160,26 @@ internal inline fun <T, R : Any, V : Any> Iterable<T>.mapSecondNotNull(transform
158160
}
159161
}
160162

163+
internal inline fun <T, R : Any, V : Any> Sequence<T>.mapSecondNotNull(
164+
crossinline transform: (T) -> Pair<R, V?>
165+
): Sequence<Pair<R, V>> {
166+
return mapNotNull {
167+
val pair = transform(it)
168+
if (pair.second != null) {
169+
@Suppress("UNCHECKED_CAST")
170+
pair as Pair<R, V>
171+
} else {
172+
null
173+
}
174+
}
175+
}
176+
177+
internal inline fun <T : Any, K : Any, V : Any> Sequence<T>.associateNotNull(
178+
crossinline transform: (T) -> Pair<K, V>?
179+
): Map<K, V> {
180+
return mapNotNull { transform(it) }.toMap()
181+
}
182+
161183
/**
162184
* Sort elements keeping Comparable-equal elements (stable sorting).
163185
* This method has different semantics with standard toSortedSet(Comparator).

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,5 +129,7 @@ internal enum class Bucket(val value: String) {
129129
}
130130
}
131131

132-
class Visibility(val forCompile: Boolean, val forRuntime: Boolean)
132+
class Visibility(val forCompile: Boolean, val forRuntime: Boolean) {
133+
val forEither: Boolean get() = forCompile || forRuntime
134+
}
133135
}

src/main/kotlin/com/autonomousapps/model/internal/intermediates/Reason.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ internal sealed class Reason(open val reason: String) {
268268
@TypeLabel("lint")
269269
@JsonClass(generateAdapter = false)
270270
data class LintJar(override val reason: String) : Reason(reason) {
271-
override val configurationName: String = "implementation"
271+
override val configurationName: String = "runtimeOnly"
272272

273273
internal companion object {
274274
fun of(lintRegistry: String) = LintJar(

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

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -698,21 +698,43 @@ private class GraphVisitor(
698698
capability: ExceptionCapability,
699699
context: GraphViewVisitor.Context,
700700
): Boolean {
701+
// TODO(tsr): extract this to a testable function
701702
val codeSource = context.project.codeSource
702703
if (codeSource.isEmpty()) {
703-
// nothing to do here
704+
// Nothing to do here. We don't have any code, so we can't be referring to any exception types, by definition.
705+
// This is an optimization.
704706
return false
705707
}
706708

707-
// These are all the exception types referenced by `coordinates`, which is a specific dependency of `this` project
708-
// (i.e., `context.project`).
709-
val exceptions = capability.exceptions.values.flatten().toSet()
709+
// These are all the exception types referenced by `this` dependency.
710+
val referencedExceptions = capability.exceptions.values.flatten().toSet()
711+
712+
// Ideally this would be a single item, but it could be multiple if we have duplicates on the classpath.
713+
val exceptionProviders: Map<Coordinates, Set<String>> = context.dependencies
714+
// We only care about dependencies that have classes
715+
.mapSecondNotNull { dependency -> dependency.coordinates to dependency.findCapability<BinaryClassCapability>() }
716+
// Find the dependencies that have a class that matches any exception referenced by `this` dependency.
717+
.mapNotNull { (coordinates, binaryClass) ->
718+
val relevantTypes = binaryClass.classes.filterToOrderedSet { it in referencedExceptions }
719+
if (relevantTypes.isEmpty()) {
720+
null
721+
} else {
722+
coordinates to relevantTypes
723+
}
724+
}
725+
.toMap()
726+
727+
if (exceptionProviders.isEmpty()) {
728+
// Nothing to do here.
729+
return false
730+
}
710731

711-
// TODO(tsr): extract this to testable function
712732
// These are all the dependencies of this project that use the exception types provided by `coordinates`.
713-
val users: Map<Coordinates, Map<String, Set<String>>> = context.dependencies
733+
val users: Map<Coordinates, Map<String, Set<String>>> = context.dependencies.asSequence()
714734
// The dependency can see all its own exceptions
715735
.filterNot { dependency -> dependency.coordinates == coordinates }
736+
// Don't scan the exception providers, that doesn't make sense and leads to incorrect results
737+
.filterNot { dependency -> dependency.coordinates in exceptionProviders.keys }
716738
// If the dependency's runtime graph can reach `this` dependency, then we can skip analysis
717739
.filterNot { dependency ->
718740
val graph = context.graphRuntime.graph
@@ -728,17 +750,15 @@ private class GraphVisitor(
728750
.associateNotNull { (coordinates, exceptionCapability) ->
729751
val classToExceptionType = exceptionCapability.exceptions
730752
.mapNotNull { (className, types) ->
731-
val relevantTypes = types.filterToOrderedSet { type -> type in exceptions }
753+
val relevantTypes = types.filterToOrderedSet { type -> type in referencedExceptions }
732754
if (relevantTypes.isEmpty()) {
733755
null
734756
} else {
735757
className to relevantTypes
736758
}
737759
}
738760
// It only matters if the code of `this` project refers to a class that itself references a relevant type
739-
.filter { (className, _) ->
740-
codeSource.any { source -> className in source.usedNonAnnotationClasses }
741-
}
761+
.filter { (className, _) -> codeSource.any { source -> className in source.usedNonAnnotationClasses } }
742762
.toMap()
743763

744764
if (classToExceptionType.isNotEmpty()) {

src/test/kotlin/com/autonomousapps/internal/reason/DependencyAdviceExplainerTest.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ class DependencyAdviceExplainerTest {
8585
* Uses 6 classes, 5 of which are shown: One, Two, Three, Four, Five (implies implementation).
8686
* Imports 6 classes, 5 of which are shown: One, Two, Three, Four, Five (implies implementation).
8787
* Imports 6 inline members, 5 of which are shown: One, Two, Three, Four, Five (implies implementation).
88-
* Provides 1 lint registry: LintRegistry (implies implementation).
88+
* Provides 1 lint registry: LintRegistry (implies runtimeOnly).
8989
* Provides 2 native binaries: foo, bar (implies runtimeOnly).
9090
* Imports 2 resources: drawable, string (implies implementation).
9191
* Uses 2 resources: StyleParentRef(styleParent=AppCompat), AttrRef(type=drawable, id=logo) (implies implementation).

0 commit comments

Comments
 (0)