dependency-analysis-gradle-plugin icon indicating copy to clipboard operation
dependency-analysis-gradle-plugin copied to clipboard

A type used as type parameter in a generic return type is not considered public API

Open gabrielittner opened this issue 3 years ago • 4 comments

Build scan link

Reproducer instead return-type-generic.zip

Plugin version

0.73.1-SNAPSHOT

Gradle version

7.0

Android Gradle Plugin (AGP) version

n/a

Describe the bug

I've got an interface like this:

interface CoachSettingsStateMachine {
    fun get(): Observable<ApiResult<CoachSettingsState>>
}

The ApiResult class is coming from a different project, which is currently declared as an api dependency (in the sample project from above it's coming from the rootProject).

I'm getting the advice to change the dependency from api to implementation:

Advice for project :settings
Existing dependencies which should be modified to be as indicated:
- implementation(project(":")) (was api)

I guess this generally makes sense because of type erasure, but consumers CoachSettingsStateMachine still need ApiResult to use it, so I'd consider it part of the public API.

To Reproduce Steps to reproduce the behavior:

  1. Run buildHealth on the sample project

Expected behavior

I'd expect no advice to be given in the sample project as said above, but I could also understand if this is working as intended

gabrielittner avatar Jun 03 '21 11:06 gabrielittner

Generics are a pain in my butt -__-

Thanks for the issue report. The reason is likely that generics don't end up in the bytecode; to workaround that, I do source analysis with ANTLR. To make that workaround simpler, I just look at import statements. So if a type is only used in a generic context, then the plugin won't know it's part of the ABI (since it only sees the import, not the actual usage). This is solvable in principle, but might take some time.

autonomousapps avatar Jun 04 '21 02:06 autonomousapps

After thinking about this for a while, I've come to the conclusion that it is more of a feature request than a bug. Type erasure really makes it challenging to accurately report how generic types are used in the JVM.

Following are notes for future-me, or anyone ambitious enough to take this on themselves. Here's how I think the algorithm for this would look, roughly:

  1. Use an enhanced source-parser that parsed the entirety of the source files, rather than just the imports. We'd need two parsers to start, one for Java and one for Kotlin. They should be created by a factory to make it easier to add support for additional languages in the future.
  2. The parser needs to look for (return) types for all members, making note of the member pseudo-signature and its visibility.
  3. The parser also needs to note all the imports.
  4. Use the combination of imports and type parameters to create type candidates and try to load them in a classloader. The classloader will need to have the relevant classpath available.
  5. Now we (might) know the actual return type!
  6. Later on, in the ABI analysis, match the pseudo-signature to the signature detected by the bytecode analysis and now we (might) know if the type is part of the ABI or just an implementation detail.

Pretty complex! And the best we can do is a heuristic rather than a certainty. For now, I'm going to re-triage this issue and then add a note in the wiki declaring this a known issue.

autonomousapps avatar Sep 06 '21 23:09 autonomousapps

The reason is likely that generics don't end up in the bytecode; 

That is not quite true. The generic signature is present in the bytecode, and it is even accessible via reflection.

See https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.7.9

vlsi avatar Jun 07 '22 22:06 vlsi

I actually think this issue is already resolved, but I'm not 100% sure. I recall writing some code that grabbed the generic types from the class files with asm.

So yes, I agree my original analysis was wrong. I've learned a lot since then!

autonomousapps avatar Jun 08 '22 02:06 autonomousapps