Move parsed configuration logic into proguard configuration builder
This also fixes a bug where the rules of a given keep rule file were not
always emitted between the header and the footer in the parsed
configuration output.
Change-Id: Ib9dd11ba9bd56071d49d2c29e7bced254e6fba8b
diff --git a/src/main/java/com/android/tools/r8/processkeeprules/FilteredKeepRulesBuilder.java b/src/main/java/com/android/tools/r8/processkeeprules/FilteredKeepRulesBuilder.java
index 4694ecd..00f8172 100644
--- a/src/main/java/com/android/tools/r8/processkeeprules/FilteredKeepRulesBuilder.java
+++ b/src/main/java/com/android/tools/r8/processkeeprules/FilteredKeepRulesBuilder.java
@@ -163,6 +163,11 @@
}
@Override
+ public void addParsedConfiguration(ProguardConfigurationSourceParser parser) {
+ // Intentionally empty.
+ }
+
+ @Override
public void addRule(ProguardConfigurationRule rule) {
ensureNewlineAfterComment();
write(rule.getSource());
@@ -339,9 +344,6 @@
}
@Override
- public void addParsedConfiguration(String s) {}
-
- @Override
public void joinMaxRemovedAndroidLogLevel(int maxRemovedAndroidLogLevel) {}
@Override
diff --git a/src/main/java/com/android/tools/r8/processkeeprules/ValidateLibraryConsumerRulesKeepRuleProcessor.java b/src/main/java/com/android/tools/r8/processkeeprules/ValidateLibraryConsumerRulesKeepRuleProcessor.java
index bf2b323..f1866b4 100644
--- a/src/main/java/com/android/tools/r8/processkeeprules/ValidateLibraryConsumerRulesKeepRuleProcessor.java
+++ b/src/main/java/com/android/tools/r8/processkeeprules/ValidateLibraryConsumerRulesKeepRuleProcessor.java
@@ -18,6 +18,7 @@
import java.util.List;
class ValidateLibraryConsumerRulesKeepRuleProcessor implements ProguardConfigurationParserConsumer {
+
private final Reporter reporter;
public ValidateLibraryConsumerRulesKeepRuleProcessor(Reporter reporter) {
@@ -85,7 +86,7 @@
}
@Override
- public void addParsedConfiguration(String s) {}
+ public void addParsedConfiguration(ProguardConfigurationSourceParser parser) {}
@Override
public void addRule(ProguardConfigurationRule rule) {}
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 4634bf4..7778a92 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfiguration.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfiguration.java
@@ -8,6 +8,7 @@
import com.android.tools.r8.naming.DictionaryReader;
import com.android.tools.r8.position.Position;
import com.android.tools.r8.position.TextPosition;
+import com.android.tools.r8.shaking.ProguardConfigurationParser.IncludeWorkItem;
import com.android.tools.r8.shaking.ProguardConfigurationParser.ProguardConfigurationSourceParser;
import com.android.tools.r8.utils.InternalOptions.PackageObfuscationMode;
import com.android.tools.r8.utils.Reporter;
@@ -25,7 +26,7 @@
public static class Builder implements ProguardConfigurationParserConsumer {
- private final List<String> parsedConfiguration = new ArrayList<>();
+ private final StringBuilder parsedConfiguration = new StringBuilder();
private final List<FilteredClassPath> injars = new ArrayList<>();
private final List<FilteredClassPath> libraryJars = new ArrayList<>();
@@ -82,8 +83,22 @@
}
@Override
- public void addParsedConfiguration(String source) {
- parsedConfiguration.add(source);
+ public void addParsedConfiguration(ProguardConfigurationSourceParser parser) {
+ parsedConfiguration.append(
+ "# The proguard configuration file for the following section is " + parser.getOrigin());
+ parsedConfiguration.append(System.lineSeparator());
+ int lastIncludePositionEnd = 0;
+ for (IncludeWorkItem pendingInclude : parser.getPendingIncludes()) {
+ int includePositionStart = pendingInclude.includePositionStart.getOffsetAsInt();
+ parsedConfiguration.append(
+ parser.getContentInRange(lastIncludePositionEnd, includePositionStart));
+ lastIncludePositionEnd = pendingInclude.includePositionEnd;
+ }
+ parsedConfiguration.append(parser.getContentAfter(lastIncludePositionEnd));
+ parsedConfiguration.append(System.lineSeparator());
+ parsedConfiguration.append("# End of content from ");
+ parsedConfiguration.append(parser.getOrigin());
+ parsedConfiguration.append(System.lineSeparator());
}
@Override
@@ -399,7 +414,7 @@
}
ProguardConfiguration configuration =
new ProguardConfiguration(
- String.join(System.lineSeparator(), parsedConfiguration),
+ parsedConfiguration.toString(),
dexItemFactory,
injars,
libraryJars,
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 c6aaf59..368dad32 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
@@ -39,8 +39,11 @@
import java.nio.CharBuffer;
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
+import java.util.ArrayDeque;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.Collections;
+import java.util.Deque;
import java.util.List;
import java.util.function.BiConsumer;
import java.util.function.Consumer;
@@ -224,12 +227,13 @@
private final String name;
private final String contents;
private int position = 0;
- private int positionAfterInclude = 0;
private int line = 1;
private int lineStartPosition = 0;
private Path baseDirectory;
private final Origin origin;
+ private final Deque<IncludeWorkItem> pendingIncludes = new ArrayDeque<>();
+
ProguardConfigurationSourceParser(ProguardConfigurationSource source) throws IOException {
// Strip any leading BOM here so it is not included in the text position.
contents = StringUtils.stripLeadingBOM(source.get());
@@ -238,14 +242,26 @@
this.origin = source.getOrigin();
}
+ public String getContentAfter(int start) {
+ return getContentInRange(start, contents.length());
+ }
+
+ public String getContentInRange(int start, int end) {
+ return contents.substring(start, end);
+ }
+
public String getContentSince(TextPosition start) {
- return contents.substring(start.getOffsetAsInt(), position);
+ return getContentInRange(start.getOffsetAsInt(), position);
}
public Origin getOrigin() {
return origin;
}
+ public Collection<IncludeWorkItem> getPendingIncludes() {
+ return pendingIncludes;
+ }
+
public void parse() throws ProguardRuleParserException {
do {
TextPosition whitespaceStart = getPosition();
@@ -253,14 +269,11 @@
configurationConsumer.addWhitespace(this, whitespaceStart);
}
} while (parseOption());
- // This may be unknown, but we want to always ensure that we don't attribute lines to the
- // wrong configuration.
- configurationConsumer.addParsedConfiguration(
- "# The proguard configuration file for the following section is " + origin.toString());
-
- // Collect the parsed configuration.
- configurationConsumer.addParsedConfiguration(contents.substring(positionAfterInclude));
- configurationConsumer.addParsedConfiguration("# End of content from " + origin);
+ configurationConsumer.addParsedConfiguration(this);
+ while (!pendingIncludes.isEmpty()) {
+ IncludeWorkItem includeWorkItem = pendingIncludes.removeFirst();
+ parseInclude(includeWorkItem.includePath, includeWorkItem.includePositionStart);
+ }
reporter.failIfPendingErrors();
}
@@ -268,10 +281,10 @@
if (eof()) {
return false;
}
- if (acceptArobaseInclude()) {
+ TextPosition optionStart = getPosition();
+ if (acceptArobaseInclude(optionStart)) {
return true;
}
- TextPosition optionStart = getPosition();
expectChar('-');
if (parseIgnoredOption(optionStart)
|| parseIgnoredOptionAndWarn(optionStart)
@@ -424,12 +437,8 @@
ProguardAssumeValuesRule rule = parseAssumeValuesRule(optionStart);
configurationConsumer.addRule(rule);
} else if (acceptString("include")) {
- // Collect the parsed configuration until the include.
- configurationConsumer.addParsedConfiguration(
- contents.substring(positionAfterInclude, position - ("include".length() + 1)));
skipWhitespace();
- parseInclude();
- positionAfterInclude = position;
+ enqueueInclude(optionStart);
} else if (acceptString("basedirectory")) {
skipWhitespace();
baseDirectory = parseFileName();
@@ -732,29 +741,33 @@
|| parseOptimizationOption(optionStart);
}
- private void parseInclude() throws ProguardRuleParserException {
- TextPosition start = getPosition();
- Path included = parseFileInputDependency(inputDependencyConsumer::acceptProguardInclude);
+ private void enqueueInclude(TextPosition optionStart) throws ProguardRuleParserException {
+ Path includePath = parseFileInputDependency(inputDependencyConsumer::acceptProguardInclude);
+ pendingIncludes.add(new IncludeWorkItem(includePath, optionStart, position));
+ }
+
+ private void parseInclude(Path includePath, TextPosition includePositionStart)
+ throws ProguardRuleParserException {
try {
- new ProguardConfigurationSourceParser(new ProguardConfigurationSourceFile(included))
+ new ProguardConfigurationSourceParser(new ProguardConfigurationSourceFile(includePath))
.parse();
} catch (FileNotFoundException | NoSuchFileException e) {
- throw parseError("Included file '" + included.toString() + "' not found",
- start, e);
+ throw parseError("Included file '" + includePath + "' not found", includePositionStart, e);
} catch (IOException e) {
- throw parseError("Failed to read included file '" + included.toString() + "'",
- start, e);
+ throw parseError(
+ "Failed to read included file '" + includePath + "'", includePositionStart, e);
}
}
- private boolean acceptArobaseInclude() throws ProguardRuleParserException {
+ private boolean acceptArobaseInclude(TextPosition optionStart)
+ throws ProguardRuleParserException {
if (remainingChars() < 2) {
return false;
}
if (!acceptChar('@')) {
return false;
}
- parseInclude();
+ enqueueInclude(optionStart);
return true;
}
@@ -2335,4 +2348,17 @@
this.negated = negated;
}
}
+
+ static class IncludeWorkItem {
+
+ final Path includePath;
+ final TextPosition includePositionStart;
+ final int includePositionEnd;
+
+ IncludeWorkItem(Path includePath, TextPosition includePositionStart, int includePositionEnd) {
+ this.includePath = includePath;
+ this.includePositionStart = includePositionStart;
+ this.includePositionEnd = includePositionEnd;
+ }
+ }
}
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParserConsumer.java b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParserConsumer.java
index e3771ec..ab2dc15 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParserConsumer.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParserConsumer.java
@@ -12,7 +12,7 @@
public interface ProguardConfigurationParserConsumer {
- void addParsedConfiguration(String s);
+ void addParsedConfiguration(ProguardConfigurationSourceParser parser);
void addRule(ProguardConfigurationRule rule);
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 cb788dc..e50d5e6 100644
--- a/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
@@ -3023,4 +3023,47 @@
assertEquals(MaximumRemovedAndroidLogLevelRule.VERBOSE, rule.getMaxRemovedAndroidLogLevel());
}
}
+
+ @Test
+ public void testParsedConfigurationWithInclude() throws Exception {
+ Path config = temp.newFile("config.txt").toPath().toAbsolutePath();
+ Path include1 = temp.newFile("include1.txt").toPath().toAbsolutePath();
+ Path include11 = temp.newFile("include1_1.txt").toPath().toAbsolutePath();
+ Path include2 = temp.newFile("include2.txt").toPath().toAbsolutePath();
+ FileUtils.writeTextFile(
+ config,
+ StringUtils.joinLines(
+ "# Before Include 1",
+ "-include " + include1,
+ "# After Include 1",
+ "-include " + include2,
+ "# After Include 2"));
+ FileUtils.writeTextFile(
+ include1, StringUtils.joinLines("# Include 1", "-include " + include11));
+ FileUtils.writeTextFile(include11, "# Include 1.1");
+ FileUtils.writeTextFile(include2, "# Include 2");
+ parser.parse(config);
+ verifyParserEndsCleanly();
+ String parsedConfiguration = builder.build().getParsedConfiguration();
+ assertEquals(
+ StringUtils.lines(
+ "# The proguard configuration file for the following section is config.txt",
+ "# Before Include 1",
+ "", // -include include1.txt
+ "# After Include 1",
+ "", // -include include2.txt
+ "# After Include 2",
+ "# End of content from config.txt",
+ "# The proguard configuration file for the following section is include1.txt",
+ "# Include 1",
+ "", // -include include1_1.txt.
+ "# End of content from include1.txt",
+ "# The proguard configuration file for the following section is include1_1.txt",
+ "# Include 1.1",
+ "# End of content from include1_1.txt",
+ "# The proguard configuration file for the following section is include2.txt",
+ "# Include 2",
+ "# End of content from include2.txt"),
+ StringUtils.replaceAll(parsedConfiguration, temp.getRoot().toString() + "/", ""));
+ }
}
\ No newline at end of file
