Add warnings to Proguard parser for unusual types and field names
An unusual type or field name is one containing characters < and/or > which are
not parsed as a back reference.
Bug: 110021323
Change-Id: Ibbe59e32f6797f4f9af392d8397791dbba245cbd
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 c4276dc..1de3f17 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
@@ -873,6 +873,7 @@
ruleBuilder.setName(IdentifierPatternWithWildcards.withoutWildcards("<init>"));
ruleBuilder.setArguments(parseArgumentList());
} else {
+ TextPosition firstStart = getPosition();
IdentifierPatternWithWildcards first =
acceptIdentifierWithBackreference(IdentifierType.ANY);
if (first != null) {
@@ -885,6 +886,7 @@
ruleBuilder.setName(first);
ruleBuilder.setArguments(parseArgumentList());
} else {
+ TextPosition secondStart = getPosition();
IdentifierPatternWithWildcards second =
acceptIdentifierWithBackreference(IdentifierType.ANY);
if (second != null) {
@@ -897,6 +899,12 @@
ProguardTypeMatcher.create(first, ClassOrType.TYPE, dexItemFactory));
ruleBuilder.setArguments(parseArgumentList());
} else {
+ if (first.hasUnusualCharacters()) {
+ warnUnusualCharacters("type", first.pattern, "field", firstStart);
+ }
+ if (second.hasUnusualCharacters()) {
+ warnUnusualCharacters("field name", second.pattern, "field", secondStart);
+ }
ruleBuilder.setRuleType(ProguardMemberType.FIELD);
ruleBuilder.setName(second);
ruleBuilder
@@ -1490,6 +1498,16 @@
"Option -" + optionName + " overrides -" + victim, origin, getPosition(start)));
}
+ private void warnUnusualCharacters(
+ String kind, String pattern, String ruleType, TextPosition start) {
+ reporter.warning(new StringDiagnostic(
+ "The " + kind + " \"" + pattern + "\" is used in a " + ruleType + " rule. The "
+ + "characters in this " + kind + " are legal for the JVM, "
+ + "but unlikely to originate from a source language. "
+ + "Maybe this is not the rule you are looking for.",
+ origin, getPosition(start)));
+ }
+
private void failPartiallyImplementedOption(String optionName, TextPosition start) {
throw reporter.fatalError(new StringDiagnostic(
"Option " + optionName + " currently not supported", origin, getPosition(start)));
@@ -1528,5 +1546,25 @@
boolean isMatchAllNames() {
return pattern.equals("*");
}
+
+ boolean hasUnusualCharacters() {
+ if (pattern.contains("<") || pattern.contains(">")) {
+ int angleStartCount = 0;
+ int angleEndCount = 0;
+ for (int i = 0; i < pattern.length(); i++) {
+ char c = pattern.charAt(i);
+ if (c == '<') {
+ angleStartCount++;
+ }
+ if (c == '>') {
+ angleEndCount++;
+ }
+ }
+ // Check that start/end angles are matched, and *only* used for well-formed wildcard
+ // backreferences (e.g. '<1>', but not '<<1>>', '<<*>>' or '>1<').
+ return !(angleStartCount == angleEndCount && angleStartCount == wildcards.size());
+ }
+ return false;
+ }
}
}
diff --git a/src/test/java/com/android/tools/r8/DiagnosticsChecker.java b/src/test/java/com/android/tools/r8/DiagnosticsChecker.java
index 2f45ae2..d79aaf0 100644
--- a/src/test/java/com/android/tools/r8/DiagnosticsChecker.java
+++ b/src/test/java/com/android/tools/r8/DiagnosticsChecker.java
@@ -116,9 +116,14 @@
messageParts);
}
+ public static Diagnostic checkDiagnostics(List<Diagnostic> diagnostics, int index, Path path,
+ int lineStart, int columnStart, String... messageParts) {
+ return checkDiagnostic(diagnostics.get(index), path, lineStart, columnStart, messageParts);
+ }
+
public static Diagnostic checkDiagnostics(List<Diagnostic> diagnostics, Path path,
int lineStart, int columnStart, String... messageParts) {
assertEquals(1, diagnostics.size());
- return checkDiagnostic(diagnostics.get(0), path, lineStart, columnStart, messageParts);
+ return checkDiagnostics(diagnostics, 0, path, lineStart, columnStart, messageParts);
}
}
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 f96dc45..f484bc8 100644
--- a/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
@@ -1485,7 +1485,7 @@
ProguardConfigurationParser parser =
new ProguardConfigurationParser(new DexItemFactory(), reporter);
parser.parse(proguardConfig);
- verifyParserEndsCleanly();
+ checkDiagnostics(handler.warnings, proguardConfig, 3, 7, "The field name \"id<<*>>\" is");
verifyWithProguard6(proguardConfig);
}
@@ -1731,7 +1731,7 @@
ProguardConfigurationParser parser =
new ProguardConfigurationParser(new DexItemFactory(), reporter);
parser.parse(proguardConfig);
- verifyParserEndsCleanly();
+ checkDiagnostics(handler.warnings, proguardConfig, 2, 5, "The field name \"<fields>\" is");
verifyWithProguard(proguardConfig);
}
@@ -1753,6 +1753,26 @@
verifyWithProguard(proguardConfig);
}
+ @Test
+ public void parse_regress110021323() throws Exception {
+ Path proguardConfig = writeTextToTempFile(
+ "-keepclassmembernames class A {",
+ " <public methods>;",
+ " <public fields>;",
+ "}"
+ );
+ ProguardConfigurationParser parser =
+ new ProguardConfigurationParser(new DexItemFactory(), reporter);
+ parser.parse(proguardConfig);
+ assertEquals(4, handler.warnings.size());
+ checkDiagnostics(handler.warnings, 0, proguardConfig, 2, 3, "The type \"<public\" is");
+ checkDiagnostics(handler.warnings, 1, proguardConfig, 2, 11, "The field name \"methods>\" is");
+ checkDiagnostics(handler.warnings, 2, proguardConfig, 3, 3, "The type \"<public\" is");
+ checkDiagnostics(handler.warnings, 3, proguardConfig, 3, 11, "The field name \"fields>\" is");
+
+ verifyWithProguard(proguardConfig);
+ }
+
public void testNotSupported(String option) {
try {
reset();
