Make no -keepattributes flag keep all in Proguard compatibility mode
When running with --force-proguard-compatibility for the compat proguard
command treat no -keepattributes flag as keep all attributes when not
minifying.
This should follow the Proguard semantics where -keepattributes is
documented as "Only applicable when obfuscating".
Bug: 68246915
Change-Id: Ib0e71cd8b0819af6ca148365d9b4d155db06e5e0
diff --git a/src/main/java/com/android/tools/r8/R8Command.java b/src/main/java/com/android/tools/r8/R8Command.java
index 5b3cfb5..1df56db 100644
--- a/src/main/java/com/android/tools/r8/R8Command.java
+++ b/src/main/java/com/android/tools/r8/R8Command.java
@@ -224,6 +224,7 @@
throw new CompilationException(e.getMessage(), e.getCause());
}
configurationBuilder = parser.getConfigurationBuilder();
+ configurationBuilder.setForceProguardCompatibility(forceProguardCompatibility);
}
if (proguardConfigurationConsumer != null) {
diff --git a/src/main/java/com/android/tools/r8/compatproguard/CompatProguardCommandBuilder.java b/src/main/java/com/android/tools/r8/compatproguard/CompatProguardCommandBuilder.java
index 2c7c43b..71bc6eb 100644
--- a/src/main/java/com/android/tools/r8/compatproguard/CompatProguardCommandBuilder.java
+++ b/src/main/java/com/android/tools/r8/compatproguard/CompatProguardCommandBuilder.java
@@ -7,7 +7,7 @@
import com.android.tools.r8.R8Command;
public class CompatProguardCommandBuilder extends R8Command.Builder {
- CompatProguardCommandBuilder(boolean forceProguardCompatibility) {
+ public CompatProguardCommandBuilder(boolean forceProguardCompatibility) {
super(true, forceProguardCompatibility);
setEnableDesugaring(false);
}
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardConfiguration.java b/src/main/java/com/android/tools/r8/shaking/ProguardConfiguration.java
index 4a84321..b3cfcb2 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfiguration.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfiguration.java
@@ -44,6 +44,8 @@
private boolean useUniqueClassMemberNames;
private boolean keepParameterNames;
private ProguardClassFilter.Builder adaptClassStrings = ProguardClassFilter.builder();
+ private boolean forceProguardCompatibility = false;
+
private Builder(DexItemFactory dexItemFactory) {
this.dexItemFactory = dexItemFactory;
resetProguardDefaults();
@@ -205,7 +207,22 @@
adaptClassStrings.addPattern(pattern);
}
+ public void setForceProguardCompatibility(boolean forceProguardCompatibility) {
+ this.forceProguardCompatibility = forceProguardCompatibility;
+ }
+
public ProguardConfiguration build() throws CompilationException {
+ ProguardKeepAttributes keepAttributes;
+
+
+ if (forceProguardCompatibility
+ && !isObfuscating()
+ && keepAttributePatterns.size() == 0) {
+ keepAttributes = ProguardKeepAttributes.fromPatterns(ProguardKeepAttributes.KEEP_ALL);
+ } else {
+ keepAttributes = ProguardKeepAttributes.fromPatterns(keepAttributePatterns);
+ }
+
return new ProguardConfiguration(
dexItemFactory,
injars,
@@ -224,7 +241,7 @@
applyMappingFile,
verbose,
renameSourceFileAttribute,
- keepAttributePatterns,
+ keepAttributes,
dontWarnPatterns.build(),
rules,
printSeeds,
@@ -285,7 +302,7 @@
Path applyMappingFile,
boolean verbose,
String renameSourceFileAttribute,
- List<String> keepAttributesPatterns,
+ ProguardKeepAttributes keepAttributes,
ProguardClassFilter dontWarnPatterns,
List<ProguardConfigurationRule> rules,
boolean printSeeds,
@@ -313,7 +330,7 @@
this.applyMappingFile = applyMappingFile;
this.verbose = verbose;
this.renameSourceFileAttribute = renameSourceFileAttribute;
- this.keepAttributes = ProguardKeepAttributes.fromPatterns(keepAttributesPatterns);
+ this.keepAttributes = keepAttributes;
this.dontWarnPatterns = dontWarnPatterns;
this.rules = ImmutableList.copyOf(rules);
this.printSeeds = printSeeds;
diff --git a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/ForceProguardCompatibilityTest.java b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/ForceProguardCompatibilityTest.java
index 4d59f63..1519f6c 100644
--- a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/ForceProguardCompatibilityTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/ForceProguardCompatibilityTest.java
@@ -8,7 +8,10 @@
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
+import com.android.tools.r8.R8Command;
import com.android.tools.r8.TestBase;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.compatproguard.CompatProguardCommandBuilder;
import com.android.tools.r8.naming.MemberNaming.MethodSignature;
import com.android.tools.r8.utils.DexInspector;
import com.android.tools.r8.utils.DexInspector.ClassSubject;
@@ -49,4 +52,45 @@
test(TestMainArrayType.class, TestMainArrayType.MentionedClass.class, true, true);
test(TestMainArrayType.class, TestMainArrayType.MentionedClass.class, true, false);
}
+
+ private void runAnnotationsTest(boolean forceProguardCompatibility, boolean keepAnnotations) throws Exception {
+ R8Command.Builder builder =
+ new CompatProguardCommandBuilder(forceProguardCompatibility);
+ // Add application classes including the annotation class.
+ Class mainClass = TestMain.class;
+ Class mentionedClassWithAnnotations = TestMain.MentionedClassWithAnnotation.class;
+ Class annotationClass = TestAnnotation.class;
+ builder.addProgramFiles(ToolHelper.getClassFileForTestClass(mainClass));
+ builder.addProgramFiles(ToolHelper.getClassFileForTestClass(TestMain.MentionedClass.class));
+ builder.addProgramFiles(ToolHelper.getClassFileForTestClass(mentionedClassWithAnnotations));
+ builder.addProgramFiles(ToolHelper.getClassFileForTestClass(annotationClass));
+ // Keep main class and the annotation class.
+ builder.addProguardConfiguration(
+ ImmutableList.of(keepMainProguardConfiguration(mainClass, true, false)));
+ builder.addProguardConfiguration(
+ ImmutableList.of("-keep class " + annotationClass.getCanonicalName() + " { }"));
+ if (keepAnnotations) {
+ builder.addProguardConfiguration(ImmutableList.of("-keepattributes *Annotation*"));
+ }
+
+ DexInspector inspector = new DexInspector(ToolHelper.runR8(builder.build()));
+ assertTrue(inspector.clazz(mainClass.getCanonicalName()).isPresent());
+ ClassSubject clazz = inspector.clazz(getJavacGeneratedClassName(mentionedClassWithAnnotations));
+ assertTrue(clazz.isPresent());
+
+ assertEquals(!keepAnnotations && forceProguardCompatibility,
+ clazz.annotation("dalvik.annotation.EnclosingClass").isPresent());
+ assertEquals(!keepAnnotations && forceProguardCompatibility,
+ clazz.annotation("dalvik.annotation.InnerClass").isPresent());
+ assertEquals(forceProguardCompatibility || keepAnnotations,
+ clazz.annotation(annotationClass.getCanonicalName()).isPresent());
+ }
+
+ @Test
+ public void testAnnotations() throws Exception {
+ runAnnotationsTest(true, true);
+ runAnnotationsTest(true, false);
+ runAnnotationsTest(false, true);
+ runAnnotationsTest(false, false);
+ }
}
diff --git a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/TestMain.java b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/TestMain.java
index 94a9d0a..1a484b5 100644
--- a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/TestMain.java
+++ b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/TestMain.java
@@ -11,7 +11,14 @@
}
}
+ @TestAnnotation(0)
+ public static class MentionedClassWithAnnotation {
+ public MentionedClassWithAnnotation() {
+ }
+ }
+
public static void main(String[] args) {
System.out.println(MentionedClass.class.getCanonicalName());
+ System.out.println(MentionedClassWithAnnotation.class.getCanonicalName());
}
}
