Merge "Improve configuration parser error"
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 47d3743..1b74e3b 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
@@ -401,13 +401,22 @@
&& (unknownOption.equals("forceinline") || unknownOption.equals("neverinline"))) {
devMessage = ", this option needs to be turned on explicitly if used for tests.";
}
- reporter.error(new StringDiagnostic(
- "Unknown option \"-" + unknownOption + "\"" + devMessage,
- origin, getPosition(optionStart)));
+ unknownOption(unknownOption, optionStart, devMessage);
}
return true;
}
+ private void unknownOption(String unknownOption, TextPosition optionStart) {
+ unknownOption(unknownOption, optionStart, "");
+ }
+
+ private void unknownOption(
+ String unknownOption, TextPosition optionStart, String additionalMessage) {
+ throw reporter.fatalError((new StringDiagnostic(
+ "Unknown option \"-" + unknownOption + "\"" + additionalMessage,
+ origin, getPosition(optionStart))));
+ }
+
private boolean parseUnsupportedOptionAndErr(TextPosition optionStart) {
String option = Iterables.find(UNSUPPORTED_FLAG_OPTIONS, this::skipFlag, null);
if (option != null) {
@@ -768,14 +777,19 @@
TextPosition start = getPosition();
acceptString("-");
String unknownOption = acceptString();
- throw reporter.fatalError(new StringDiagnostic(
- "Unknown option \"-" + unknownOption + "\"",
- origin,
- start));
+ unknownOption(unknownOption, start);
}
} else {
builder.setType(ProguardKeepRuleType.KEEP);
}
+ if (!eof() && !Character.isWhitespace(peekChar()) && peekChar() != ',') {
+ // The only path to here is through "-keep" with an unsupported suffix.
+ unacceptString("-keep");
+ TextPosition start = getPosition();
+ acceptString("-");
+ String unknownOption = acceptString();
+ unknownOption(unknownOption, start);
+ }
parseRuleModifiers(builder);
}
diff --git a/src/test/java/com/android/tools/r8/TestBase.java b/src/test/java/com/android/tools/r8/TestBase.java
index e0baab1..c24ed3b 100644
--- a/src/test/java/com/android/tools/r8/TestBase.java
+++ b/src/test/java/com/android/tools/r8/TestBase.java
@@ -163,6 +163,8 @@
/**
* Write lines of text to a temporary file.
+ *
+ * The file will include a line separator after the last line.
*/
protected Path writeTextToTempFile(String... lines) throws IOException {
return writeTextToTempFile(System.lineSeparator(), Arrays.asList(lines));
@@ -170,11 +172,28 @@
/**
* Write lines of text to a temporary file, along with the specified line separator.
+ *
+ * The file will include a line separator after the last line.
*/
protected Path writeTextToTempFile(String lineSeparator, List<String> lines)
throws IOException {
+ return writeTextToTempFile(lineSeparator, lines, true);
+ }
+
+ /**
+ * Write lines of text to a temporary file, along with the specified line separator.
+ *
+ * The argument <code>includeTerminatingLineSeparator</code> control if the file will include
+ * a line separator after the last line.
+ */
+ protected Path writeTextToTempFile(
+ String lineSeparator, List<String> lines, boolean includeTerminatingLineSeparator)
+ throws IOException {
Path file = temp.newFile().toPath();
- String contents = String.join(lineSeparator, lines) + lineSeparator;
+ String contents = String.join(lineSeparator, lines);
+ if (includeTerminatingLineSeparator) {
+ contents += lineSeparator;
+ }
Files.write(file, contents.getBytes(StandardCharsets.UTF_8));
return file;
}
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 ff84486..db5f631 100644
--- a/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
@@ -1113,6 +1113,39 @@
}
@Test
+ public void parseInvalidKeepOption() throws Exception {
+ Path proguardConfig = writeTextToTempFile(
+ "-keepx public class * { ",
+ " native <methods>; ",
+ "} "
+ );
+ try {
+ ProguardConfigurationParser parser =
+ new ProguardConfigurationParser(new DexItemFactory(), reporter);
+ parser.parse(proguardConfig);
+ fail();
+ } catch (AbortException e) {
+ checkDiagnostics(handler.errors, proguardConfig, 1, 1,
+ "Unknown option", "-keepx");
+ }
+ }
+
+ @Test
+ public void parseKeepOptionEOF() throws Exception {
+ Path proguardConfig = writeTextToTempFile(
+ System.lineSeparator(), ImmutableList.of("-keep"), false);
+ try {
+ ProguardConfigurationParser parser =
+ new ProguardConfigurationParser(new DexItemFactory(), reporter);
+ parser.parse(proguardConfig);
+ fail();
+ } catch (AbortException e) {
+ checkDiagnostics(handler.errors, proguardConfig, 1, 6,
+ "Expected [!]interface|@interface|class|enum");
+ }
+ }
+
+ @Test
public void parseInvalidKeepClassOption() throws Exception {
Path proguardConfig = writeTextToTempFile(
"-keepclassx public class * { ",
@@ -1493,7 +1526,7 @@
parser.parse(createConfigurationForTesting(ImmutableList.of(option + " class A { *; }")));
fail("Expect to fail due to testing option being turned off.");
} catch (AbortException e) {
- assertEquals(2, handler.errors.size());
+ assertEquals(1, handler.errors.size());
checkDiagnostics(handler.errors, 0, null, 1, 1, "Unknown option \"" + option + "\"");
}
}
