Fix LazyClassKey proguard file issues.

  * Fixes the name of the proguard file to contain an underscore between the package and simple names (e.g. `test.FooModule` was `testFooModule_LazyClassKeys.pro` and is now `test_FooModule_LazyClassKeys.pro`).
  * Fixes incremental processing by adding the originating element to the `writeResource` call (#4549).
  * Fixes bug where `StringBuilder` was declared outside module for-loop, which could lead to duplicate entries across proguard rules for different modules.
  * Fixed `ClassName#toString()` usage to `ClassName#canonicalName()`, since `toString()` is ambiguous and will silently break things when we migrate to `XClassName`.
  * Added test coverage for the proguard file name and contents.

Fixes #4549

RELNOTES=Fixes #4549: Fixes incremental processing for LazyClassKey proguard files by adding the originating element to the `writeResource` call.
PiperOrigin-RevId: 710126913

Author: bcorso

java/dagger/internal/codegen/processingstep/LazyClassKeyProcessingStep.java

@@ -18,6 +18,7 @@
 
 import static androidx.room.compiler.processing.XElementKt.isTypeElement;
 import static java.nio.charset.StandardCharsets.UTF_8;
+import static java.util.stream.Collectors.joining;
 
 import androidx.room.compiler.processing.XElement;
 import androidx.room.compiler.processing.XFiler;
@@ -38,15 +39,17 @@
 import java.io.OutputStream;
 import java.io.OutputStreamWriter;
 import java.nio.file.Path;
-import java.util.Collection;
 import java.util.Map;
 import java.util.Set;
 import javax.inject.Inject;
 
 /** Generate keep rules for LazyClassKey referenced classes to prevent class merging. */
 final class LazyClassKeyProcessingStep extends TypeCheckingProcessingStep<XElement> {
   private static final String PROGUARD_KEEP_RULE = "-keep,allowobfuscation,allowshrinking class ";
-  private final SetMultimap<ClassName, ClassName> processedElements = LinkedHashMultimap.create();
+
+  // Note: We aggregate @LazyClassKey usages across processing rounds, so we use ClassName instead
+  // of XElement as the map key to avoid storing XElement instances across processing rounds.
+  private final SetMultimap<ClassName, ClassName> lazyMapKeysByModule = LinkedHashMultimap.create();
   private final LazyMapKeyProxyGenerator lazyMapKeyProxyGenerator;
 
   @Inject
@@ -73,7 +76,7 @@ protected void process(XElement element, ImmutableSet<ClassName> annotations) {
       return;
     }
     XTypeElement moduleElement = XElements.asTypeElement(element.getEnclosingElement());
-    processedElements.put(moduleElement.getClassName(), lazyClassKey);
+    lazyMapKeysByModule.put(moduleElement.getClassName(), lazyClassKey);
     XMethodElement method = XElements.asMethod(element);
     lazyMapKeyProxyGenerator.generate(method);
   }
@@ -91,41 +94,57 @@ private static boolean isModuleOrProducerModule(XElement element) {
             || element.hasAnnotation(TypeNames.PRODUCER_MODULE));
   }
 
+  // TODO(b/386393062): Avoid generating proguard files in processOver.
   @Override
   public void processOver(
       XProcessingEnv env, Map<String, ? extends Set<? extends XElement>> elementsByAnnotation) {
     super.processOver(env, elementsByAnnotation);
-    StringBuilder proguardRules = new StringBuilder();
-    for (Map.Entry<ClassName, Collection<ClassName>> moduleToLazyClassKeys :
-        processedElements.asMap().entrySet()) {
-      String bindingGraphProguardName =
-          getFullyQualifiedEnclosedClassName(moduleToLazyClassKeys.getKey()) + "_LazyClassKeys.pro";
-      for (ClassName lazyClassKey : moduleToLazyClassKeys.getValue()) {
-        proguardRules.append(PROGUARD_KEEP_RULE).append(lazyClassKey).append("\n");
-      }
-      writeProguardFile(bindingGraphProguardName, proguardRules.toString(), env.getFiler());
-    }
+    lazyMapKeysByModule
+        .asMap()
+        .forEach(
+            (moduleClassName, lazyClassKeys) -> {
+              // Note: we could probably get better incremental performance by using the method
+              // element instead of the module element as the originating element. However, that
+              // would require appending the method name to each proguard file, which would probably
+              // cause issues with the filename length limit (256 characters) given it already must
+              // include the module's fully qualified name.
+              XTypeElement originatingElement =
+                  env.requireTypeElement(moduleClassName.canonicalName());
+
+              Path proguardFile =
+                  Path.of(
+                      "META-INF/proguard",
+                      getFullyQualifiedEnclosedClassName(moduleClassName) + "_LazyClassKeys.pro");
+
+              String proguardFileContents =
+                  lazyClassKeys.stream()
+                      .map(lazyClassKey -> PROGUARD_KEEP_RULE + lazyClassKey.canonicalName())
+                      .collect(joining("\n"));
+
+              writeResource(env.getFiler(), originatingElement, proguardFile, proguardFileContents);
+            });
+    // Processing is over so this shouldn't matter, but clear the map just incase.
+    lazyMapKeysByModule.clear();
   }
 
-  private void writeProguardFile(String proguardFileName, String proguardRules, XFiler filer) {
+  private void writeResource(
+      XFiler filer, XElement originatingElement, Path path, String contents) {
     try (OutputStream outputStream =
-            filer.writeResource(
-                Path.of("META-INF/proguard/" + proguardFileName),
-                ImmutableList.<XElement>of(),
-                XFiler.Mode.Isolating);
+            filer.writeResource(path, ImmutableList.of(originatingElement), XFiler.Mode.Isolating);
         BufferedWriter writer = new BufferedWriter(new OutputStreamWriter(outputStream, UTF_8))) {
-      writer.write(proguardRules);
+      writer.write(contents);
     } catch (IOException e) {
       throw new IllegalStateException(e);
     }
   }
 
   /** Returns the fully qualified class name, with _ instead of . */
   private static String getFullyQualifiedEnclosedClassName(ClassName className) {
-    return className.packageName().replace('.', '_') + getEnclosedName(className);
-  }
-
-  public static String getEnclosedName(ClassName name) {
-    return Joiner.on('_').join(name.simpleNames());
+    return Joiner.on('_')
+        .join(
+            ImmutableList.<String>builder()
+                .add(className.packageName().replace('.', '_'))
+                .addAll(className.simpleNames())
+                .build());
   }
 }

javatests/dagger/internal/codegen/LazyClassKeyMapBindingComponentProcessorTest.java

@@ -16,13 +16,19 @@
 
 package dagger.internal.codegen;
 
+import static com.google.common.truth.Truth.assertThat;
 import static com.google.testing.compile.CompilationSubject.assertThat;
+import static java.nio.charset.StandardCharsets.UTF_8;
 
 import androidx.room.compiler.processing.util.Source;
+import com.google.common.truth.PrimitiveByteArraySubject;
+import com.google.common.truth.StringSubject;
+import com.google.common.truth.Subject;
 import com.google.testing.compile.Compilation;
 import com.google.testing.compile.JavaFileObjects;
 import dagger.testing.compile.CompilerTests;
 import dagger.testing.golden.GoldenFileRule;
+import java.lang.reflect.Method;
 import java.util.Collection;
 import javax.tools.JavaFileObject;
 import org.junit.Rule;
@@ -276,4 +282,213 @@ public void scopedLazyClassKeyProvider_compilesSuccessfully() throws Exception {
         .withProcessingOptions(compilerMode.processorOptions())
         .compile(subject -> subject.hasErrorCount(0));
   }
+
+  @Test
+  public void testProguardFile() throws Exception {
+    Source fooKey =
+        CompilerTests.javaSource(
+            "test.FooKey",
+            "package test;",
+            "",
+            "interface FooKey {}");
+    Source fooKeyModule =
+        CompilerTests.javaSource(
+            "test.FooKeyModule",
+            "package test;",
+            "",
+            "import dagger.Module;",
+            "import dagger.Provides;",
+            "import dagger.multibindings.LazyClassKey;",
+            "import dagger.multibindings.IntoMap;",
+            "",
+            "@Module",
+            "public interface FooKeyModule {",
+            "  @Provides",
+            "  @IntoMap",
+            "  @LazyClassKey(FooKey.class)",
+            "  static String provideString() { return \"\"; }",
+            "}");
+    CompilerTests.daggerCompiler(fooKey, fooKeyModule)
+        .withProcessingOptions(compilerMode.processorOptions())
+        .compile(
+            subject -> {
+              subject.hasErrorCount(0);
+              PrimitiveByteArraySubject proguardFile =
+                  subject.generatedResourceFileWithPath(
+                      "META-INF/proguard/test_FooKeyModule_LazyClassKeys.pro");
+              assertThatContentAsUtf8String(proguardFile)
+                .isEqualTo("-keep,allowobfuscation,allowshrinking class test.FooKey");
+            });
+  }
+
+  @Test
+  public void testProguardFile_nestedModule() throws Exception {
+    Source fooKey =
+        CompilerTests.javaSource(
+            "test.FooKey",
+            "package test;",
+            "",
+            "interface FooKey {}");
+    Source outerClass =
+        CompilerTests.javaSource(
+            "test.OuterClass",
+            "package test;",
+            "",
+            "import dagger.Module;",
+            "import dagger.Provides;",
+            "import dagger.multibindings.LazyClassKey;",
+            "import dagger.multibindings.IntoMap;",
+            "",
+            "public interface OuterClass {",
+            "  @Module",
+            "  public interface FooKeyModule {",
+            "    @Provides",
+            "    @IntoMap",
+            "    @LazyClassKey(FooKey.class)",
+            "    static String provideString() { return \"\"; }",
+            "  }",
+            "}");
+    CompilerTests.daggerCompiler(fooKey, outerClass)
+        .withProcessingOptions(compilerMode.processorOptions())
+        .compile(
+            subject -> {
+              subject.hasErrorCount(0);
+              PrimitiveByteArraySubject proguardFile =
+                  subject.generatedResourceFileWithPath(
+                      "META-INF/proguard/test_OuterClass_FooKeyModule_LazyClassKeys.pro");
+              assertThatContentAsUtf8String(proguardFile)
+                .isEqualTo("-keep,allowobfuscation,allowshrinking class test.FooKey");
+            });
+  }
+
+  @Test
+  public void testProguardFile_multipleModules() throws Exception {
+    Source fooKey =
+        CompilerTests.javaSource(
+            "test.FooKey",
+            "package test;",
+            "",
+            "interface FooKey {}");
+    Source barKey =
+        CompilerTests.javaSource(
+            "test.BarKey",
+            "package test;",
+            "",
+            "interface BarKey {}");
+    Source fooKeyModule =
+        CompilerTests.javaSource(
+            "test.FooKeyModule",
+            "package test;",
+            "",
+            "import dagger.Module;",
+            "import dagger.Provides;",
+            "import dagger.multibindings.LazyClassKey;",
+            "import dagger.multibindings.IntoMap;",
+            "",
+            "@Module",
+            "public interface FooKeyModule {",
+            "  @Provides",
+            "  @IntoMap",
+            "  @LazyClassKey(FooKey.class)",
+            "  static String provideString() { return \"\"; }",
+            "}");
+    Source barKeyModule =
+        CompilerTests.javaSource(
+            "test.BarKeyModule",
+            "package test;",
+            "",
+            "import dagger.Module;",
+            "import dagger.Provides;",
+            "import dagger.multibindings.LazyClassKey;",
+            "import dagger.multibindings.IntoMap;",
+            "",
+            "@Module",
+            "public interface BarKeyModule {",
+            "  @Provides",
+            "  @IntoMap",
+            "  @LazyClassKey(BarKey.class)",
+            "  static String provideString() { return \"\"; }",
+            "}");
+    CompilerTests.daggerCompiler(fooKey, fooKeyModule, barKey, barKeyModule)
+        .withProcessingOptions(compilerMode.processorOptions())
+        .compile(
+            subject -> {
+              subject.hasErrorCount(0);
+              PrimitiveByteArraySubject fooKeyModuleProguardFile =
+                  subject.generatedResourceFileWithPath(
+                      "META-INF/proguard/test_FooKeyModule_LazyClassKeys.pro");
+              assertThatContentAsUtf8String(fooKeyModuleProguardFile)
+                .isEqualTo("-keep,allowobfuscation,allowshrinking class test.FooKey");
+
+              PrimitiveByteArraySubject barKeyModuleProguardFile =
+                  subject.generatedResourceFileWithPath(
+                      "META-INF/proguard/test_BarKeyModule_LazyClassKeys.pro");
+              assertThatContentAsUtf8String(barKeyModuleProguardFile)
+                .isEqualTo("-keep,allowobfuscation,allowshrinking class test.BarKey");
+            });
+  }
+
+  @Test
+  public void testProguardFile_multipleKeys() throws Exception {
+    Source fooKey =
+        CompilerTests.javaSource(
+            "test.FooKey",
+            "package test;",
+            "",
+            "interface FooKey {}");
+    Source barKey =
+        CompilerTests.javaSource(
+            "test.BarKey",
+            "package test;",
+            "",
+            "interface BarKey {}");
+    Source fooKeyAndBarKeyModule =
+        CompilerTests.javaSource(
+            "test.FooKeyAndBarKeyModule",
+            "package test;",
+            "",
+            "import dagger.Module;",
+            "import dagger.Provides;",
+            "import dagger.multibindings.LazyClassKey;",
+            "import dagger.multibindings.IntoMap;",
+            "",
+            "@Module",
+            "public interface FooKeyAndBarKeyModule {",
+            "  @Provides",
+            "  @IntoMap",
+            "  @LazyClassKey(FooKey.class)",
+            "  static String provideFooKeyString() { return \"\"; }",
+            "",
+            "  @Provides",
+            "  @IntoMap",
+            "  @LazyClassKey(BarKey.class)",
+            "  static String provideBarKeyString() { return \"\"; }",
+            "}");
+    CompilerTests.daggerCompiler(fooKey, barKey, fooKeyAndBarKeyModule)
+        .withProcessingOptions(compilerMode.processorOptions())
+        .compile(
+            subject -> {
+              subject.hasErrorCount(0);
+              PrimitiveByteArraySubject proguardFile =
+                  subject.generatedResourceFileWithPath(
+                      "META-INF/proguard/test_FooKeyAndBarKeyModule_LazyClassKeys.pro");
+              assertThatContentAsUtf8String(proguardFile)
+                .isEqualTo(
+                    "-keep,allowobfuscation,allowshrinking class test.FooKey\n"
+                      + "-keep,allowobfuscation,allowshrinking class test.BarKey");
+            });
+  }
+
+  // TODO(b/386213524): Add support for getting a resource file as a StringSubject.
+  // Use reflection to get the subject's byte array and then convert it to a StringSubject.
+  private static StringSubject assertThatContentAsUtf8String(PrimitiveByteArraySubject subject) {
+    try {
+      Method protectedActualMethod = Subject.class.getDeclaredMethod("actual");
+      protectedActualMethod.setAccessible(true);
+      byte[] actualBytes = (byte[]) protectedActualMethod.invoke(subject);
+      return assertThat(new String(actualBytes, UTF_8));
+    } catch (Exception e) {
+      throw new RuntimeException(e);
+    }
+  }
 }