Merge "Add parsing of more Proguard options"
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 2a31604a..48ea17d 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfiguration.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfiguration.java
@@ -54,6 +54,10 @@
private Origin keepParameterNamesOptionOrigin;
private Position keepParameterNamesOptionPosition;
private final ProguardClassFilter.Builder adaptClassStrings = ProguardClassFilter.builder();
+ private final ProguardPathFilter.Builder adaptResourceFilenames = ProguardPathFilter.builder();
+ private final ProguardPathFilter.Builder adaptResourceFilecontents =
+ ProguardPathFilter.builder();
+ private final ProguardPathFilter.Builder keepDirectories = ProguardPathFilter.builder();
private boolean forceProguardCompatibility = false;
private boolean overloadAggressively;
@@ -222,6 +226,18 @@
adaptClassStrings.addPattern(pattern);
}
+ public void addAdaptResourceFilenames(ProguardPathList pattern) {
+ adaptResourceFilenames.addPattern(pattern);
+ }
+
+ public void addAdaptResourceFilecontents(ProguardPathList pattern) {
+ adaptResourceFilecontents.addPattern(pattern);
+ }
+
+ public void addKeepDirectories(ProguardPathList pattern) {
+ keepDirectories.addPattern(pattern);
+ }
+
public void setForceProguardCompatibility(boolean forceProguardCompatibility) {
this.forceProguardCompatibility = forceProguardCompatibility;
}
@@ -264,7 +280,10 @@
DictionaryReader.readAllNames(packageObfuscationDictionary, reporter),
useUniqueClassMemberNames,
keepParameterNames,
- adaptClassStrings.build());
+ adaptClassStrings.build(),
+ adaptResourceFilenames.build(),
+ adaptResourceFilecontents.build(),
+ keepDirectories.build());
reporter.failIfPendingErrors();
@@ -330,6 +349,9 @@
private final boolean useUniqueClassMemberNames;
private final boolean keepParameterNames;
private final ProguardClassFilter adaptClassStrings;
+ private final ProguardPathFilter adaptResourceFilenames;
+ private final ProguardPathFilter adaptResourceFilecontents;
+ private final ProguardPathFilter keepDirectories;
private ProguardConfiguration(
String parsedConfiguration,
@@ -363,7 +385,10 @@
ImmutableList<String> packageObfuscationDictionary,
boolean useUniqueClassMemberNames,
boolean keepParameterNames,
- ProguardClassFilter adaptClassStrings) {
+ ProguardClassFilter adaptClassStrings,
+ ProguardPathFilter adaptResourceFilenames,
+ ProguardPathFilter adaptResourceFilecontents,
+ ProguardPathFilter keepDirectories) {
this.parsedConfiguration = parsedConfiguration;
this.dexItemFactory = factory;
this.injars = ImmutableList.copyOf(injars);
@@ -396,6 +421,9 @@
this.useUniqueClassMemberNames = useUniqueClassMemberNames;
this.keepParameterNames = keepParameterNames;
this.adaptClassStrings = adaptClassStrings;
+ this.adaptResourceFilenames = adaptResourceFilenames;
+ this.adaptResourceFilecontents = adaptResourceFilecontents;
+ this.keepDirectories = keepDirectories;
}
/**
@@ -530,6 +558,18 @@
return adaptClassStrings;
}
+ public ProguardPathFilter getAdaptResourceFilenames() {
+ return adaptResourceFilenames;
+ }
+
+ public ProguardPathFilter getAdaptResourceFilecontents() {
+ return adaptResourceFilecontents;
+ }
+
+ public ProguardPathFilter getKeepDirectories() {
+ return keepDirectories;
+ }
+
public static ProguardConfiguration defaultConfiguration(DexItemFactory dexItemFactory,
Reporter reporter) {
return builder(dexItemFactory, reporter).build();
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
index 3d426dd..dd7bb60 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
@@ -50,7 +50,6 @@
"target");
private static final List<String> IGNORED_OPTIONAL_SINGLE_ARG_OPTIONS = ImmutableList.of(
- "keepdirectories",
"runtype",
"laststageoutput");
@@ -72,8 +71,7 @@
private static final List<String> WARNED_SINGLE_ARG_OPTIONS = ImmutableList.of(
// TODO -outjars (http://b/37137994) and -adaptresourcefilecontents (http://b/37139570)
// should be reported as errors, not just as warnings!
- "outjars",
- "adaptresourcefilecontents");
+ "outjars");
private static final List<String> WARNED_FLAG_OPTIONS = ImmutableList.of(
// TODO(b/73707846): add support -addconfigurationdebugging
@@ -217,6 +215,13 @@
} else if (acceptString("checkdiscard")) {
ProguardCheckDiscardRule rule = parseCheckDiscardRule();
configurationBuilder.addRule(rule);
+ } else if (acceptString("keepdirectories")) {
+ skipWhitespace();
+ if (isOptionalArgumentGiven()) {
+ configurationBuilder.addKeepDirectories(parsePathFilter());
+ } else {
+ configurationBuilder.addKeepDirectories(ProguardPathList.emptyList());
+ }
} else if (acceptString("keep")) {
ProguardKeepRule rule = parseKeepRule();
configurationBuilder.addRule(rule);
@@ -345,6 +350,20 @@
configurationBuilder.addAdaptClassStringsPattern(
ProguardClassNameList.singletonList(ProguardTypeMatcher.defaultAllMatcher()));
}
+ } else if (acceptString("adaptresourcefilenames")) {
+ skipWhitespace();
+ if (isOptionalArgumentGiven()) {
+ configurationBuilder.addAdaptResourceFilenames(parsePathFilter());
+ } else {
+ configurationBuilder.addAdaptResourceFilenames(ProguardPathList.emptyList());
+ }
+ } else if (acceptString("adaptresourcefilecontents")) {
+ skipWhitespace();
+ if (isOptionalArgumentGiven()) {
+ configurationBuilder.addAdaptResourceFilecontents(parsePathFilter());
+ } else {
+ configurationBuilder.addAdaptResourceFilecontents(ProguardPathList.emptyList());
+ }
} else if (acceptString("identifiernamestring")) {
configurationBuilder.addRule(parseIdentifierNameStringRule());
} else if (acceptString("if")) {
@@ -1292,6 +1311,34 @@
return name;
}
+ private boolean pathFilterMatcher(Integer character) {
+ return character != ',' && !Character.isWhitespace(character);
+ }
+
+ private ProguardPathList parsePathFilter() throws ProguardRuleParserException {
+ ProguardPathList.Builder builder = ProguardPathList.builder();
+ skipWhitespace();
+ boolean negated = acceptChar('!');
+ String fileFilter = acceptString(this::pathFilterMatcher);
+ if (fileFilter == null) {
+ throw parseError("Path filter expected");
+ }
+ builder.addFileName(negated, fileFilter);
+ skipWhitespace();
+ while (acceptChar(',')) {
+ skipWhitespace();
+ negated = acceptChar('!');
+ skipWhitespace();
+ fileFilter = acceptString(this::pathFilterMatcher);
+ if (fileFilter == null) {
+ throw parseError("Path filter expected");
+ }
+ builder.addFileName(negated, fileFilter);
+ skipWhitespace();
+ }
+ return builder.build();
+ }
+
private String snippetForPosition() {
// TODO(ager): really should deal with \r as well to get column right.
String[] lines = contents.split("\n", -1); // -1 to get trailing empty lines represented.
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardPathFilter.java b/src/main/java/com/android/tools/r8/shaking/ProguardPathFilter.java
new file mode 100644
index 0000000..cb2e117
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardPathFilter.java
@@ -0,0 +1,48 @@
+// Copyright (c) 2018, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.shaking;
+
+import com.google.common.collect.ImmutableList;
+
+public class ProguardPathFilter {
+ private final ImmutableList<ProguardPathList> patterns;
+
+ public static Builder builder() {
+ return new Builder();
+ }
+
+ public static class Builder {
+ private final ImmutableList.Builder<ProguardPathList> patterns = ImmutableList.builder();
+
+ private Builder() {
+ }
+
+ public Builder addPattern(ProguardPathList pattern) {
+ patterns.add(pattern);
+ return this;
+ }
+
+ ProguardPathFilter build() {
+ return new ProguardPathFilter(patterns.build());
+ }
+ }
+
+ private ProguardPathFilter(ImmutableList<ProguardPathList> patterns) {
+ if (patterns.isEmpty()) {
+ this.patterns = ImmutableList.of(ProguardPathList.emptyList());
+ } else {
+ this.patterns = patterns;
+ }
+ }
+
+ public boolean matches(String path) {
+ for (ProguardPathList pattern : patterns) {
+ if (pattern.matches(path)) {
+ return true;
+ }
+ }
+ return false;
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardPathList.java b/src/main/java/com/android/tools/r8/shaking/ProguardPathList.java
new file mode 100644
index 0000000..6766128
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardPathList.java
@@ -0,0 +1,128 @@
+// Copyright (c) 2018, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.shaking;
+
+import com.google.common.collect.ImmutableList;
+import java.util.List;
+
+public abstract class ProguardPathList {
+
+ public static Builder builder() {
+ return new Builder();
+ }
+
+ public static ProguardPathList emptyList() {
+ return new EmptyPathList();
+ }
+
+ abstract boolean matches(String path);
+
+ public static class Builder {
+
+ private final ImmutableList.Builder<FileNameMatcher> matchers = ImmutableList.builder();
+
+ private Builder() {
+ }
+
+ public Builder addFileName(boolean isNegated, String path) {
+ matchers.add(new FileNameMatcher(isNegated, path));
+ return this;
+ }
+
+ ProguardPathList build() {
+ List<FileNameMatcher> matchers = this.matchers.build();
+ if (matchers. size() > 0) {
+ return new PathList(matchers);
+ } else {
+ return emptyList();
+ }
+ }
+ }
+
+ private static class FileNameMatcher {
+ public final boolean negated;
+ public final String pattern;
+
+ FileNameMatcher(boolean negated, String pattern) {
+ this.negated = negated;
+ this.pattern = pattern;
+ }
+
+ private boolean match(String path) {
+ return matchImpl(pattern, 0, path, 0);
+ }
+
+ private boolean matchImpl(String pattern, int patternIndex, String path, int pathIndex) {
+ for (int i = patternIndex; i < pattern.length(); i++) {
+ char patternChar = pattern.charAt(i);
+ switch (patternChar) {
+ case '*':
+ boolean includeSeparators = pattern.length() > (i + 1) && pattern.charAt(i + 1) == '*';
+ int nextPatternIndex = i + (includeSeparators ? 2 : 1);
+ // Fast cases for the common case where a pattern ends with '**' or '*'.
+ if (nextPatternIndex == pattern.length()) {
+ return includeSeparators || !containsSeparatorsStartingAt(path, pathIndex);
+ }
+ // Match the rest of the pattern against the (non-empty) rest of the class name.
+ for (int nextPathIndex = pathIndex; nextPathIndex < path.length(); nextPathIndex++) {
+ if (!includeSeparators && path.charAt(nextPathIndex) == '/') {
+ return matchImpl(pattern, nextPatternIndex, path, nextPathIndex);
+ }
+ if (matchImpl(pattern, nextPatternIndex, path, nextPathIndex)) {
+ return true;
+ }
+ }
+ break;
+ case '?':
+ if (pathIndex == path.length() || path.charAt(pathIndex++) == '/') {
+ return false;
+ }
+ break;
+ default:
+ if (pathIndex == path.length() || patternChar != path.charAt(pathIndex++)) {
+ return false;
+ }
+ break;
+ }
+ }
+ return pathIndex == path.length();
+ }
+
+ private boolean containsSeparatorsStartingAt(String path, int pathIndex) {
+ return path.indexOf('/', pathIndex) != -1;
+ }
+
+ }
+
+ private static class PathList extends ProguardPathList {
+ private final List<FileNameMatcher> matchers;
+
+ private PathList(List<FileNameMatcher> matchers) {
+ this.matchers = matchers;
+ }
+
+ @Override
+ boolean matches(String path) {
+ for (FileNameMatcher matcher : matchers) {
+ if (matcher.match(path)) {
+ // If we match a negation, abort as non-match. If we match a positive, return true.
+ return !matcher.negated;
+ }
+ }
+ return false;
+ }
+ }
+
+ private static class EmptyPathList extends ProguardPathList {
+
+ private EmptyPathList() {
+ }
+
+ @Override
+ boolean matches(String path) {
+ return true;
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java b/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
index f26e48d..9e9f80e 100644
--- a/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
@@ -26,6 +26,7 @@
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.FieldAccessFlags;
import com.android.tools.r8.graph.MethodAccessFlags;
+import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.origin.PathOrigin;
import com.android.tools.r8.position.TextPosition;
import com.android.tools.r8.position.TextRange;
@@ -157,11 +158,13 @@
private Reporter reporter;
private KeepingDiagnosticHandler handler;
+ private ProguardConfigurationParser parser;
@Before
- public void setup() {
+ public void reset() {
handler = new KeepingDiagnosticHandler();
reporter = new Reporter(handler);
+ parser = new ProguardConfigurationParser(new DexItemFactory(), reporter);
}
@Test
@@ -1175,6 +1178,146 @@
}
}
+ private void checkFileFilterMatchAnything(ProguardPathFilter filter) {
+ assertTrue(filter.matches("x"));
+ assertTrue(filter.matches("x/y"));
+ assertTrue(filter.matches("x/y/x"));
+ }
+
+ @Test
+ public void parse_adaptresourcexxx_keepdirectories_noArguments1() {
+ ProguardConfiguration config = parseAndVerifyParserEndsCleanly(ImmutableList.of(
+ "-adaptresourcefilenames",
+ "-adaptresourcefilecontents",
+ "-keepdirectories"
+ ));
+ checkFileFilterMatchAnything(config.getAdaptResourceFilenames());
+ checkFileFilterMatchAnything(config.getAdaptResourceFilecontents());
+ checkFileFilterMatchAnything(config.getKeepDirectories());
+ }
+
+ @Test
+ public void parse_adaptresourcexxx_keepdirectories_noArguments2() {
+ ProguardConfiguration config = parseAndVerifyParserEndsCleanly(ImmutableList.of(
+ "-keepdirectories",
+ "-adaptresourcefilenames",
+ "-adaptresourcefilecontents"
+ ));
+ checkFileFilterMatchAnything(config.getAdaptResourceFilenames());
+ checkFileFilterMatchAnything(config.getAdaptResourceFilecontents());
+ checkFileFilterMatchAnything(config.getKeepDirectories());
+ }
+
+ @Test
+ public void parse_adaptresourcexxx_keepdirectories_noArguments3() {
+ ProguardConfiguration config = parseAndVerifyParserEndsCleanly(ImmutableList.of(
+ "-adaptresourcefilecontents",
+ "-keepdirectories",
+ "-adaptresourcefilenames"
+ ));
+ checkFileFilterMatchAnything(config.getAdaptResourceFilenames());
+ checkFileFilterMatchAnything(config.getAdaptResourceFilecontents());
+ checkFileFilterMatchAnything(config.getKeepDirectories());
+ }
+
+ private String FILE_FILTER_SINGLE = "xxx/*";
+
+ private void checkFileFilterSingle(ProguardPathFilter filter) {
+ assertTrue(filter.matches("xxx/x"));
+ assertTrue(filter.matches("xxx/"));
+ assertFalse(filter.matches("xxx/yyy/z"));
+ assertFalse(filter.matches("xxx"));
+ }
+
+ @Test
+ public void parse_adaptresourcexxx_keepdirectories_singleArgument() {
+ ProguardConfiguration config = parseAndVerifyParserEndsCleanly(ImmutableList.of(
+ "-adaptresourcefilenames " + FILE_FILTER_SINGLE,
+ "-adaptresourcefilecontents " + FILE_FILTER_SINGLE,
+ "-keepdirectories " + FILE_FILTER_SINGLE
+ ));
+ checkFileFilterSingle(config.getAdaptResourceFilenames());
+ checkFileFilterSingle(config.getAdaptResourceFilecontents());
+ checkFileFilterSingle(config.getKeepDirectories());
+ }
+
+ private String FILE_FILTER_MULTIPLE =
+ "xxx/*, !**.gif ,images/** , com/myapp/**/*.xml,com/mylib/*/*.xml";
+
+ private void checkFileFilterMultiple(ProguardPathFilter filter) {
+ assertTrue(filter.matches("xxx/x"));
+ assertTrue(filter.matches("xxx/x.gif"));
+ assertTrue(filter.matches("images/x.jpg"));
+ assertTrue(filter.matches("images/xxx/x.jpg"));
+ assertTrue(filter.matches("com/myapp/package1/x.xml"));
+ assertTrue(filter.matches("com/myapp/package1/package2/x.xml"));
+ assertTrue(filter.matches("com/mylib/package1/x.xml"));
+ assertFalse(filter.matches("x.gif"));
+ assertFalse(filter.matches("images/x.gif"));
+ assertFalse(filter.matches("images/xxx/y.gif"));
+ assertFalse(filter.matches("images/xxx/yyy/z.gif"));
+ assertFalse(filter.matches("com/myapp/package1/x.jpg"));
+ assertFalse(filter.matches("com/myapp/package1/package2/x.jpg"));
+ assertFalse(filter.matches("com/mylib/package1/package2/x.xml"));
+ }
+
+ @Test
+ public void parse_adaptresourcexxx_keepdirectories_multipleArgument() {
+ ProguardConfiguration config = parseAndVerifyParserEndsCleanly(ImmutableList.of(
+ "-adaptresourcefilenames " + FILE_FILTER_MULTIPLE,
+ "-adaptresourcefilecontents " + FILE_FILTER_MULTIPLE,
+ "-keepdirectories " + FILE_FILTER_MULTIPLE
+ ));
+ checkFileFilterMultiple(config.getAdaptResourceFilenames());
+ checkFileFilterMultiple(config.getAdaptResourceFilecontents());
+ checkFileFilterMultiple(config.getKeepDirectories());
+ }
+
+ @Test
+ public void parse_adaptresourcexxx_keepdirectories_leadingComma() {
+ List<String> options = ImmutableList.of(
+ "-adaptresourcefilenames", "-adaptresourcefilecontents", "-keepdirectories");
+ for (String option : options) {
+ try {
+ reset();
+ parser.parse(createConfigurationForTesting(ImmutableList.of(option + " ,")));
+ fail("Expect to fail due to the lack of path filter.");
+ } catch (AbortException e) {
+ checkDiagnostic(handler.errors, null, 1, option.length() + 2, "Path filter expected");
+ }
+ }
+ }
+
+ @Test
+ public void parse_adaptresourcexxx_keepdirectories_emptyListElement() {
+ List<String> options = ImmutableList.of(
+ "-adaptresourcefilenames", "-adaptresourcefilecontents", "-keepdirectories");
+ for (String option : options) {
+ try {
+ reset();
+ parser.parse(createConfigurationForTesting(ImmutableList.of(option + " xxx,,yyy")));
+ fail("Expect to fail due to the lack of path filter.");
+ } catch (AbortException e) {
+ checkDiagnostic(handler.errors, null, 1, option.length() + 6, "Path filter expected");
+ }
+ }
+ }
+
+ @Test
+ public void parse_adaptresourcexxx_keepdirectories_trailingComma() {
+ List<String> options = ImmutableList.of(
+ "-adaptresourcefilenames", "-adaptresourcefilecontents", "-keepdirectories");
+ for (String option : options) {
+ try {
+ reset();
+ parser.parse(createConfigurationForTesting(ImmutableList.of(option + " xxx,")));
+ fail("Expect to fail due to the lack of path filter.");
+ } catch (AbortException e) {
+ checkDiagnostic(handler.errors, null, 1, option.length() + 6, "Path filter expected");
+ }
+ }
+ }
+
@Test
public void parse_if() throws Exception {
Path proguardConfig = writeTextToTempFile(
@@ -1459,6 +1602,12 @@
verifyWithProguard(proguardConfig);
}
+ private ProguardConfiguration parseAndVerifyParserEndsCleanly(List<String> config) {
+ parser.parse(createConfigurationForTesting(config));
+ verifyParserEndsCleanly();
+ return parser.getConfig();
+ }
+
private void verifyParserEndsCleanly() {
assertEquals(0, handler.infos.size());
assertEquals(0, handler.warnings.size());
@@ -1469,7 +1618,11 @@
int columnStart, String... messageParts) {
assertEquals(1, diagnostics.size());
Diagnostic diagnostic = diagnostics.get(0);
- assertEquals(path, ((PathOrigin) diagnostic.getOrigin()).getPath());
+ if (path != null) {
+ assertEquals(path, ((PathOrigin) diagnostic.getOrigin()).getPath());
+ } else {
+ assertSame(Origin.unknown(), diagnostic.getOrigin());
+ }
TextPosition position;
if (diagnostic.getPosition() instanceof TextRange) {
position = ((TextRange) diagnostic.getPosition()).getStart();
