[Compose] Extend map parsing and compose with support for preambles
Bug: b/241763080
Change-Id: I6c07c64a65a25141a8deb855cfb3575b553a6b1a
diff --git a/src/main/java/com/android/tools/r8/dex/ApplicationReader.java b/src/main/java/com/android/tools/r8/dex/ApplicationReader.java
index a64b883..7db4fbd 100644
--- a/src/main/java/com/android/tools/r8/dex/ApplicationReader.java
+++ b/src/main/java/com/android/tools/r8/dex/ApplicationReader.java
@@ -276,7 +276,8 @@
content,
options.reporter,
false,
- options.testing.enableExperimentalMapFileVersion));
+ options.testing.enableExperimentalMapFileVersion,
+ false));
} catch (IOException | ResourceException e) {
throw new CompilationError("Failure to read proguard map file", e, map.getOrigin());
}
diff --git a/src/main/java/com/android/tools/r8/naming/ClassNameMapper.java b/src/main/java/com/android/tools/r8/naming/ClassNameMapper.java
index 668f6de..922fc3d 100644
--- a/src/main/java/com/android/tools/r8/naming/ClassNameMapper.java
+++ b/src/main/java/com/android/tools/r8/naming/ClassNameMapper.java
@@ -28,10 +28,14 @@
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedHashSet;
+import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
@@ -45,8 +49,10 @@
public static class Builder extends ProguardMap.Builder {
+ private boolean buildPreamble = false;
+ private final List<String> preamble = new ArrayList<>();
private final Map<String, ClassNamingForNameMapper.Builder> mapping = new HashMap<>();
- private LinkedHashSet<MapVersionMappingInformation> mapVersions = new LinkedHashSet<>();
+ private final LinkedHashSet<MapVersionMappingInformation> mapVersions = new LinkedHashSet<>();
private final Map<String, String> originalSourceFiles = new HashMap<>();
@Override
@@ -58,9 +64,22 @@
return classNamingBuilder;
}
+ Builder setBuildPreamble(boolean buildPreamble) {
+ this.buildPreamble = buildPreamble;
+ return this;
+ }
+
+ @Override
+ void addPreambleLine(String line) {
+ if (buildPreamble) {
+ preamble.add(line);
+ }
+ }
+
@Override
public ClassNameMapper build() {
- return new ClassNameMapper(buildClassNameMappings(), mapVersions, originalSourceFiles);
+ return new ClassNameMapper(
+ buildClassNameMappings(), mapVersions, originalSourceFiles, preamble);
}
private ImmutableMap<String, ClassNamingForNameMapper> buildClassNameMappings() {
@@ -100,6 +119,11 @@
return mapperFromBufferedReader(CharSource.wrap(contents).openBufferedStream(), null);
}
+ public static ClassNameMapper mapperFromStringWithPreamble(String contents) throws IOException {
+ return mapperFromBufferedReader(
+ CharSource.wrap(contents).openBufferedStream(), null, false, false, true);
+ }
+
public static ClassNameMapper mapperFromString(
String contents, DiagnosticsHandler diagnosticsHandler) throws IOException {
return mapperFromBufferedReader(
@@ -110,38 +134,43 @@
String contents,
DiagnosticsHandler diagnosticsHandler,
boolean allowEmptyMappedRanges,
- boolean allowExperimentalMapping)
+ boolean allowExperimentalMapping,
+ boolean readPreamble)
throws IOException {
return mapperFromLineReader(
LineReader.fromBufferedReader(CharSource.wrap(contents).openBufferedStream()),
diagnosticsHandler,
allowEmptyMappedRanges,
- allowExperimentalMapping);
+ allowExperimentalMapping,
+ readPreamble);
}
private static ClassNameMapper mapperFromBufferedReader(
BufferedReader reader, DiagnosticsHandler diagnosticsHandler) throws IOException {
- return mapperFromBufferedReader(reader, diagnosticsHandler, false, false);
+ return mapperFromBufferedReader(reader, diagnosticsHandler, false, false, false);
}
public static ClassNameMapper mapperFromBufferedReader(
BufferedReader reader,
DiagnosticsHandler diagnosticsHandler,
boolean allowEmptyMappedRanges,
- boolean allowExperimentalMapping)
+ boolean allowExperimentalMapping,
+ boolean buildPreamble)
throws IOException {
return mapperFromLineReader(
LineReader.fromBufferedReader(reader),
diagnosticsHandler,
allowEmptyMappedRanges,
- allowExperimentalMapping);
+ allowExperimentalMapping,
+ buildPreamble);
}
public static ClassNameMapper mapperFromLineReader(
LineReader reader,
DiagnosticsHandler diagnosticsHandler,
boolean allowEmptyMappedRanges,
- boolean allowExperimentalMapping)
+ boolean allowExperimentalMapping,
+ boolean buildPreamble)
throws IOException {
try (ProguardMapReader proguardReader =
new ProguardMapReader(
@@ -149,7 +178,7 @@
diagnosticsHandler != null ? diagnosticsHandler : new Reporter(),
allowEmptyMappedRanges,
allowExperimentalMapping)) {
- ClassNameMapper.Builder builder = ClassNameMapper.builder();
+ ClassNameMapper.Builder builder = ClassNameMapper.builder().setBuildPreamble(buildPreamble);
proguardReader.parse(builder);
return builder.build();
}
@@ -180,20 +209,27 @@
private final Map<Signature, Signature> signatureMap = new HashMap<>();
private final LinkedHashSet<MapVersionMappingInformation> mapVersions;
private final Map<String, String> originalSourceFiles;
+ private final List<String> preamble;
private ClassNameMapper(
ImmutableMap<String, ClassNamingForNameMapper> classNameMappings,
LinkedHashSet<MapVersionMappingInformation> mapVersions,
- Map<String, String> originalSourceFiles) {
+ Map<String, String> originalSourceFiles,
+ List<String> preamble) {
this.classNameMappings = classNameMappings;
this.mapVersions = mapVersions;
this.originalSourceFiles = originalSourceFiles;
+ this.preamble = preamble;
}
public Map<String, ClassNamingForNameMapper> getClassNameMappings() {
return classNameMappings;
}
+ public Collection<String> getPreamble() {
+ return preamble;
+ }
+
private Signature canonicalizeSignature(Signature signature) {
Signature result = signatureMap.get(signature);
if (result != null) {
@@ -274,7 +310,14 @@
// This will overwrite existing source files but the chance of that happening should be very
// slim.
newSourcesFiles.putAll(other.originalSourceFiles);
- return new ClassNameMapper(builder.build(), newMapVersions, newSourcesFiles);
+
+ List<String> newPreamble = Collections.emptyList();
+ if (!this.preamble.isEmpty() || !other.preamble.isEmpty()) {
+ newPreamble = new ArrayList<>();
+ newPreamble.addAll(this.preamble);
+ newPreamble.addAll(other.preamble);
+ }
+ return new ClassNameMapper(builder.build(), newMapVersions, newSourcesFiles, newPreamble);
}
@Override
@@ -301,7 +344,7 @@
ImmutableMap.Builder<String, ClassNamingForNameMapper> builder = ImmutableMap.builder();
builder.orderEntriesByValue(Comparator.comparing(x -> x.originalName));
classNameMappings.forEach(builder::put);
- return new ClassNameMapper(builder.build(), mapVersions, originalSourceFiles);
+ return new ClassNameMapper(builder.build(), mapVersions, originalSourceFiles, preamble);
}
public boolean verifyIsSorted() {
diff --git a/src/main/java/com/android/tools/r8/naming/ComposingBuilder.java b/src/main/java/com/android/tools/r8/naming/ComposingBuilder.java
index d99e801..3919bbb 100644
--- a/src/main/java/com/android/tools/r8/naming/ComposingBuilder.java
+++ b/src/main/java/com/android/tools/r8/naming/ComposingBuilder.java
@@ -119,7 +119,7 @@
List<ComposingClassBuilder> classBuilders = new ArrayList<>(committed.classBuilders.values());
classBuilders.sort(Comparator.comparing(ComposingClassBuilder::getOriginalName));
StringBuilder sb = new StringBuilder();
- // TODO(b/241763080): Keep preamble of mapping files
+ committed.preamble.forEach(preambleLine -> sb.append(preambleLine).append("\n"));
if (currentMapVersion != null) {
sb.append("# ").append(currentMapVersion.serialize()).append("\n");
}
@@ -152,8 +152,11 @@
private final Map<ClassDescriptorAndMethodName, UpdateOutlineCallsiteInformation>
outlineSourcePositionsUpdated = new HashMap<>();
+ private final List<String> preamble = new ArrayList<>();
+
public void commit(ComposingData current, ClassNameMapper classNameMapper)
throws MappingComposeException {
+ preamble.addAll(classNameMapper.getPreamble());
commitClassBuilders(current);
commitRewriteFrameInformation(current, classNameMapper);
commitOutlineCallsiteInformation(current, classNameMapper);
diff --git a/src/main/java/com/android/tools/r8/naming/ProguardMap.java b/src/main/java/com/android/tools/r8/naming/ProguardMap.java
index 6b95444..38fe729 100644
--- a/src/main/java/com/android/tools/r8/naming/ProguardMap.java
+++ b/src/main/java/com/android/tools/r8/naming/ProguardMap.java
@@ -13,6 +13,8 @@
abstract ClassNaming.Builder classNamingBuilder(
String renamedName, String originalName, Position position);
+ abstract void addPreambleLine(String line);
+
abstract Builder setCurrentMapVersion(MapVersionMappingInformation mapVersion);
abstract ProguardMap build();
diff --git a/src/main/java/com/android/tools/r8/naming/ProguardMapReader.java b/src/main/java/com/android/tools/r8/naming/ProguardMapReader.java
index a3a3aa0..edb8793 100644
--- a/src/main/java/com/android/tools/r8/naming/ProguardMapReader.java
+++ b/src/main/java/com/android/tools/r8/naming/ProguardMapReader.java
@@ -69,6 +69,7 @@
private final DiagnosticsHandler diagnosticsHandler;
private final boolean allowEmptyMappedRanges;
private final boolean allowExperimentalMapping;
+ private boolean seenClassMapping = false;
private final CardinalPositionRangeAllocator cardinalRangeCache =
PositionRangeAllocator.createCardinalPositionRangeAllocator();
@@ -147,11 +148,11 @@
}
}
- private boolean nextLine() throws IOException {
+ private boolean nextLine(ProguardMap.Builder mapBuilder) throws IOException {
if (line.length() != lineOffset) {
throw new ParseException("Expected end of line");
}
- return skipLine();
+ return skipLine(mapBuilder);
}
private boolean isEmptyOrCommentLine(String line) {
@@ -184,10 +185,6 @@
return false;
}
- private boolean isClassMapping() {
- return !isEmptyOrCommentLine(line) && line.endsWith(":");
- }
-
private static boolean hasFirstCharJsonBrace(String line, int commentCharIndex) {
for (int i = commentCharIndex + 1; i < line.length(); i++) {
char c = line.charAt(i);
@@ -200,12 +197,17 @@
return false;
}
- private boolean skipLine() throws IOException {
+ private boolean skipLine(ProguardMap.Builder mapBuilder) throws IOException {
lineOffset = 0;
+ boolean isEmptyOrCommentLine;
do {
- lineNo++;
line = reader.readLine();
- } while (hasLine() && isEmptyOrCommentLine(line));
+ lineNo++;
+ isEmptyOrCommentLine = isEmptyOrCommentLine(line);
+ if (!seenClassMapping && isEmptyOrCommentLine) {
+ mapBuilder.addPreambleLine(line);
+ }
+ } while (hasLine() && isEmptyOrCommentLine);
return hasLine();
}
@@ -242,10 +244,7 @@
void parse(ProguardMap.Builder mapBuilder) throws IOException {
// Read the first line.
- do {
- line = reader.readLine();
- lineNo++;
- } while (hasLine() && isEmptyOrCommentLine(line));
+ skipLine(mapBuilder);
parseClassMappings(mapBuilder);
}
@@ -255,17 +254,23 @@
while (hasLine()) {
skipWhitespace();
if (isCommentLineWithJsonBrace()) {
- parseMappingInformation(
+ if (!parseMappingInformation(
info -> {
assert info.isMapVersionMappingInformation()
|| info.isUnknownJsonMappingInformation();
if (info.isMapVersionMappingInformation()) {
mapBuilder.setCurrentMapVersion(info.asMapVersionMappingInformation());
+ } else if (!seenClassMapping) {
+ mapBuilder.addPreambleLine(line);
}
- });
+ })) {
+ if (!seenClassMapping) {
+ mapBuilder.addPreambleLine(line);
+ }
+ }
// Skip reading the rest of the line.
lineOffset = line.length();
- nextLine();
+ nextLine(mapBuilder);
continue;
}
String before = parseType(false);
@@ -284,40 +289,47 @@
String after = parseType(false);
skipWhitespace();
expect(':');
+ seenClassMapping = true;
ClassNaming.Builder currentClassBuilder =
mapBuilder.classNamingBuilder(after, before, getPosition());
skipWhitespace();
- if (nextLine()) {
- parseMemberMappings(currentClassBuilder);
+ if (nextLine(mapBuilder)) {
+ parseMemberMappings(mapBuilder, currentClassBuilder);
}
}
}
- private void parseMappingInformation(Consumer<MappingInformation> onMappingInfo) {
- MappingInformation.fromJsonObject(
- version,
- parseJsonInComment(),
- diagnosticsHandler,
- lineNo,
- info -> {
- MapVersionMappingInformation generatorInfo = info.asMapVersionMappingInformation();
- if (generatorInfo != null) {
- if (generatorInfo.getMapVersion().equals(MapVersion.MAP_VERSION_EXPERIMENTAL)) {
- // A mapping file that is marked "experimental" will be treated as an unversioned
- // file if the compiler/tool is not explicitly running with experimental support.
- version =
- allowExperimentalMapping
- ? MapVersion.MAP_VERSION_EXPERIMENTAL
- : MapVersion.MAP_VERSION_NONE;
- } else {
- version = generatorInfo.getMapVersion();
+ private boolean parseMappingInformation(Consumer<MappingInformation> onMappingInfo) {
+ JsonObject object = parseJsonInComment();
+ if (object != null) {
+ MappingInformation.fromJsonObject(
+ version,
+ object,
+ diagnosticsHandler,
+ lineNo,
+ info -> {
+ MapVersionMappingInformation generatorInfo = info.asMapVersionMappingInformation();
+ if (generatorInfo != null) {
+ if (generatorInfo.getMapVersion().equals(MapVersion.MAP_VERSION_EXPERIMENTAL)) {
+ // A mapping file that is marked "experimental" will be treated as an unversioned
+ // file if the compiler/tool is not explicitly running with experimental support.
+ version =
+ allowExperimentalMapping
+ ? MapVersion.MAP_VERSION_EXPERIMENTAL
+ : MapVersion.MAP_VERSION_NONE;
+ } else {
+ version = generatorInfo.getMapVersion();
+ }
}
- }
- onMappingInfo.accept(info);
- });
+ onMappingInfo.accept(info);
+ });
+ return true;
+ }
+ return false;
}
- private void parseMemberMappings(ClassNaming.Builder classNamingBuilder) throws IOException {
+ private void parseMemberMappings(
+ ProguardMap.Builder mapBuilder, ClassNaming.Builder classNamingBuilder) throws IOException {
MemberNaming lastAddedNaming = null;
MemberNaming activeMemberNaming = null;
MappedRange activeMappedRange = null;
@@ -421,7 +433,7 @@
activeMemberNaming =
new MemberNaming(signature, signature.asRenamed(renamedName), getPosition());
previousMappedRange = mappedRange;
- } while (nextLine());
+ } while (nextLine(mapBuilder));
if (activeMemberNaming != null) {
boolean notAdded =
diff --git a/src/main/java/com/android/tools/r8/naming/SeedMapper.java b/src/main/java/com/android/tools/r8/naming/SeedMapper.java
index bf8044e..45f7a5b 100644
--- a/src/main/java/com/android/tools/r8/naming/SeedMapper.java
+++ b/src/main/java/com/android/tools/r8/naming/SeedMapper.java
@@ -63,6 +63,11 @@
}
@Override
+ void addPreambleLine(String line) {
+ // Do nothing.
+ }
+
+ @Override
ProguardMap.Builder setCurrentMapVersion(MapVersionMappingInformation mapVersion) {
// Do nothing
return this;
diff --git a/src/test/java/com/android/tools/r8/mappingcompose/ComposePreambleCommentTest.java b/src/test/java/com/android/tools/r8/mappingcompose/ComposePreambleCommentTest.java
new file mode 100644
index 0000000..98a070d
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/mappingcompose/ComposePreambleCommentTest.java
@@ -0,0 +1,71 @@
+// Copyright (c) 2022, 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.mappingcompose;
+
+import static com.android.tools.r8.mappingcompose.ComposeHelpers.doubleToSingleQuote;
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.naming.ClassNameMapper;
+import com.android.tools.r8.naming.MappingComposer;
+import com.android.tools.r8.utils.StringUtils;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class ComposePreambleCommentTest extends TestBase {
+
+ @Parameter() public TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withNoneRuntime().build();
+ }
+
+ private static final String mappingFoo =
+ StringUtils.unixLines(
+ "# This is a multi line ",
+ "# preamble, with custom information",
+ "# {'id':'com.android.tools.r8.mapping','version':'2.1'}",
+ "# foo bar",
+ "# {'id':'this is invalid json due to no comma' neededInfo:'foobar' }",
+ "com.A -> a:",
+ "# This is a comment that will be removed.",
+ "com.B -> c:");
+ private static final String mappingBar =
+ StringUtils.unixLines(
+ "# Additional multiline ",
+ "# second preamble, with custom information",
+ "# {'id':'bar',neededInfo:'barbaz'}",
+ "# {'id':'com.android.tools.r8.mapping','version':'2.1'}",
+ "a -> b:",
+ "# This is another comment that will be removed.",
+ "c -> d:");
+ private static final String mappingResult =
+ StringUtils.unixLines(
+ "# This is a multi line ",
+ "# preamble, with custom information",
+ "# foo bar",
+ "# {'id':'this is invalid json due to no comma' neededInfo:'foobar' }",
+ "# Additional multiline ",
+ "# second preamble, with custom information",
+ "# {'id':'bar',neededInfo:'barbaz'}",
+ "# {'id':'com.android.tools.r8.mapping','version':'2.1'}",
+ "com.A -> b:",
+ "com.B -> d:");
+
+ @Test
+ public void testCompose() throws Exception {
+ ClassNameMapper mappingForFoo = ClassNameMapper.mapperFromStringWithPreamble(mappingFoo);
+ ClassNameMapper mappingForBar = ClassNameMapper.mapperFromStringWithPreamble(mappingBar);
+ String composed = MappingComposer.compose(mappingForFoo, mappingForBar);
+ assertEquals(mappingResult, doubleToSingleQuote(composed));
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/naming/MapReaderVersionTest.java b/src/test/java/com/android/tools/r8/naming/MapReaderVersionTest.java
index 5a4db90..d1991df 100644
--- a/src/test/java/com/android/tools/r8/naming/MapReaderVersionTest.java
+++ b/src/test/java/com/android/tools/r8/naming/MapReaderVersionTest.java
@@ -38,7 +38,8 @@
CharSource.wrap(StringUtils.joinLines(lines)).openBufferedStream(),
diagnosticsHandler,
false,
- true);
+ true,
+ false);
}
private static ClassNameMapper read(String... lines) throws IOException {
