Skip to content

Commit

Permalink
Fix LazyClassKey proguard file issues.
Browse files Browse the repository at this point in the history 
  * 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: 709861817
  • Loading branch information
@bcorso
bcorso authored and Dagger Team committed Dec 27, 2024
1 parent caa7e17 commit 40488e9
Show file tree
Hide file tree
Showing 2 changed files with 258 additions and 24 deletions.
67 changes: 43 additions & 24 deletions java/dagger/internal/codegen/processingstep/LazyClassKeyProcessingStep.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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);
}
Expand All @@ -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());
}
}
215 changes: 215 additions & 0 deletions javatests/dagger/internal/codegen/LazyClassKeyMapBindingComponentProcessorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
}

0 comments on commit 40488e9

Please sign in to comment.