autonomousapps / dependency-analysis-gradle-plugin

Gradle plugin for JVM projects written in Java, Kotlin, Groovy, or Scala; and Android projects written in Java or Kotlin. Provides advice for managing dependencies and other applied plugins
Apache License 2.0
1.82k stars 120 forks source link

Synthetic bridge methods should be considered in ABI analysis. #1172

Open travisMiehm opened 7 months ago

travisMiehm commented 7 months ago

Plugin version 1.31.0

Gradle version 8.7

(Optional) Kotlin and Kotlin Gradle Plugin (KGP) version Kotlin 1.9.23

(Optional) reason output for bugs relating to incorrect advice

Source: main
------------
* Exposes 1 class: some.externa.dependency.ExternalEnum (implies api).

Describe the bug When declaring file-level private val variables which are typed with some type from some dependency, that dependency is required to be an api() dependency, even though that file-level variable is only used in non-public contexts, such as a constructor argument to an internal class

private val FOO_ENUMS = ExternalEnum.entries.filter { it.name.startsWith("Foo") }

internal class MyClass(private val foos: Set<ExternalEnum> = FOO_ENUMS)

To Reproduce Steps to reproduce the behavior: In on project, declare some type

enum class ExternalEnum {
  FOO_ONE
  FOO_TWO
  BAR
}

In a second project, declare a class and a file-level private val variable

private val FOO_ENUMS = ExternalEnum.entries.filter { it.name.startsWith("Foo") }

internal class MyClass(private val foos: Set<ExternalEnum> = FOO_ENUMS)

Now run ./gradlew :second-project:reason --id :first-project

Expected behavior The output of :reason should be that ExternalEnum is a used class, implying implementation

Additional context An existing work around is to move the file-level private val variables to the companion object of the class, eg

internal class MyClass(private val foos: Set<ExternalEnum> = FOO_ENUMS) {
  companion object {
    private val FOO_ENUMS = ExternalEnum.entries.filter { it.name.startsWith("Foo") }
  }
}

This class will result in the command giving the expected output.

autonomousapps commented 7 months ago

Thanks for the report.

autonomousapps commented 6 months ago

Interestingly enough, this only happens with the exact setup you have. If your internal class instead looks like internal class MyClass(private val foos: List<ExternalEnum>) then DAGP doesn't advise changing the dependency declaration. I think there's some kind of synthetic bridge class that is actually public in the bytecode. Investigating...

autonomousapps commented 6 months ago

I created another repro in my project and decompiled it to Java. Here's what it looks like. You can clearly see the public methods that "expose" the private property.

// DeleteMe.java
package com.autonomousapps;

import kotlin.Metadata;
import kotlin.jvm.internal.DefaultConstructorMarker;
import kotlin.jvm.internal.Intrinsics;
import okio.Buffer;
import org.jetbrains.annotations.NotNull;

@Metadata(
   mv = {1, 8, 0},
   k = 1,
   xi = 48,
   d1 = {"\u0000\u0012\n\u0002\u0018\u0002\n\u0002\u0010\u0000\n\u0000\n\u0002\u0018\u0002\n\u0002\b\u0002\b\u0000\u0018\u00002\u00020\u0001B\u000f\u0012\b\b\u0002\u0010\u0002\u001a\u00020\u0003¢\u0006\u0002\u0010\u0004R\u000e\u0010\u0002\u001a\u00020\u0003X\u0082\u0004¢\u0006\u0002\n\u0000¨\u0006\u0005"},
   d2 = {"Lcom/autonomousapps/DeleteMe;", "", "okio", "Lokio/Buffer;", "(Lokio/Buffer;)V", "dependency-analysis-gradle-plugin"}
)
public final class DeleteMe {
   @NotNull
   private final Buffer okio;

   public DeleteMe(@NotNull Buffer okio) {
      Intrinsics.checkNotNullParameter(okio, "okio");
      super();
      this.okio = okio;
   }

   // $FF: synthetic method
   public DeleteMe(Buffer var1, int var2, DefaultConstructorMarker var3) {
      if ((var2 & 1) != 0) {
         var1 = DeleteMeKt.access$getOKIO$p();
      }

      this(var1);
   }

   public DeleteMe() {
      this((Buffer)null, 1, (DefaultConstructorMarker)null);
   }
}
// DeleteMeKt.java
package com.autonomousapps;

import kotlin.Metadata;
import okio.Buffer;
import org.jetbrains.annotations.NotNull;

@Metadata(
   mv = {1, 8, 0},
   k = 2,
   xi = 48,
   d1 = {"\u0000\b\n\u0000\n\u0002\u0018\u0002\n\u0000\"\u000e\u0010\u0000\u001a\u00020\u0001X\u0082\u0004¢\u0006\u0002\n\u0000¨\u0006\u0002"},
   d2 = {"OKIO", "Lokio/Buffer;", "dependency-analysis-gradle-plugin"}
)
public final class DeleteMeKt {
   @NotNull
   private static final Buffer OKIO = new Buffer();

   // $FF: synthetic method
   public static final Buffer access$getOKIO$p() {
      return OKIO;
   }
}

It might be possible to detect this and provide more accurate advice. Might also be non-trivial.