Fail when resources use invalid java identifiers.

RELNOTES:None
PiperOrigin-RevId: 200766836

5 files changed, 135 insertions, 21 deletions

diff --git a/src/tools/android/java/com/google/devtools/build/android/DependencyAndroidData.java b/src/tools/android/java/com/google/devtools/build/android/DependencyAndroidData.java
index 6eb9aa80d8..0edba53761 100644
--- a/src/tools/android/java/com/google/devtools/build/android/DependencyAndroidData.java
+++ b/src/tools/android/java/com/google/devtools/build/android/DependencyAndroidData.java
@@ -54,6 +54,9 @@ class DependencyAndroidData extends SerializedAndroidData {
 
   @VisibleForTesting
   static DependencyAndroidData valueOf(String text, FileSystem fileSystem) {
+
+
+
     if (!VALID_REGEX.matcher(text).find()) {
       throw new IllegalArgumentException(text + " is not in the format '" + EXPECTED_FORMAT + "'");
     }
diff --git a/src/tools/android/java/com/google/devtools/build/android/ResourceProcessorBusyBox.java b/src/tools/android/java/com/google/devtools/build/android/ResourceProcessorBusyBox.java
index f22dec2740..0fe2d35a65 100644
--- a/src/tools/android/java/com/google/devtools/build/android/ResourceProcessorBusyBox.java
+++ b/src/tools/android/java/com/google/devtools/build/android/ResourceProcessorBusyBox.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.android;
 
 import com.google.devtools.build.android.AndroidResourceMerger.MergingException;
 import com.google.devtools.build.android.aapt2.Aapt2Exception;
+import com.google.devtools.build.android.resources.JavaIdentifierValidator.InvalidJavaIdentifier;
 import com.google.devtools.common.options.EnumConverter;
 import com.google.devtools.common.options.Option;
 import com.google.devtools.common.options.OptionDocumentationCategory;
@@ -167,18 +168,17 @@ public class ResourceProcessorBusyBox {
   /** Flag specifications for this action. */
   public static final class Options extends OptionsBase {
     @Option(
-      name = "tool",
-      defaultValue = "null",
-      converter = ToolConverter.class,
-      category = "input",
-      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
-      effectTags = {OptionEffectTag.UNKNOWN},
-      help =
-          "The processing tool to execute. "
-              + "Valid tools: PACKAGE, VALIDATE, GENERATE_BINARY_R, GENERATE_LIBRARY_R, PARSE, "
-              + "MERGE, GENERATE_AAR, SHRINK, MERGE_MANIFEST, COMPILE_LIBRARY_RESOURCES, "
-              + "LINK_STATIC_LIBRARY, AAPT2_PACKAGE, SHRINK_AAPT2, MERGE_COMPILED."
-    )
+        name = "tool",
+        defaultValue = "null",
+        converter = ToolConverter.class,
+        category = "input",
+        documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+        effectTags = {OptionEffectTag.UNKNOWN},
+        help =
+            "The processing tool to execute. "
+                + "Valid tools: PACKAGE, VALIDATE, GENERATE_BINARY_R, GENERATE_LIBRARY_R, PARSE, "
+                + "MERGE, GENERATE_AAR, SHRINK, MERGE_MANIFEST, COMPILE_LIBRARY_RESOURCES, "
+                + "LINK_STATIC_LIBRARY, AAPT2_PACKAGE, SHRINK_AAPT2, MERGE_COMPILED.")
     public Tool tool;
   }
 
@@ -191,7 +191,7 @@ public class ResourceProcessorBusyBox {
     Options options = optionsParser.getOptions(Options.class);
     try {
       options.tool.call(optionsParser.getResidue().toArray(new String[0]));
-    } catch (MergingException | IOException | Aapt2Exception e) {
+    } catch (MergingException | IOException | Aapt2Exception | InvalidJavaIdentifier e) {
       logger.severe(e.getMessage());
       logSuppressedAndExit(e);
     }
diff --git a/src/tools/android/java/com/google/devtools/build/android/resources/BUILD b/src/tools/android/java/com/google/devtools/build/android/resources/BUILD
index e911735cfa..a0cb57c4f4 100644
--- a/src/tools/android/java/com/google/devtools/build/android/resources/BUILD
+++ b/src/tools/android/java/com/google/devtools/build/android/resources/BUILD
@@ -2,7 +2,7 @@
 #   Tools for android resource processing
 
 package(default_visibility = [
-    "//src/test/java/com/google/devtools/build/android/resources:__pkg__",
+    "//src/test/java/com/google/devtools/build/android:__subpackages__",
     "//src/tools/android/java/com/google/devtools/build/android:__pkg__",
 ])
 
diff --git a/src/tools/android/java/com/google/devtools/build/android/resources/JavaIdentifierValidator.java b/src/tools/android/java/com/google/devtools/build/android/resources/JavaIdentifierValidator.java
new file mode 100644
index 0000000000..c37798168a
--- /dev/null
+++ b/src/tools/android/java/com/google/devtools/build/android/resources/JavaIdentifierValidator.java
@@ -0,0 +1,116 @@
+// Copyright 2018 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.android.resources;
+
+import static java.util.stream.Collectors.joining;
+
+import com.google.common.collect.ImmutableSet;
+import java.util.function.Predicate;
+import java.util.regex.Pattern;
+import java.util.stream.Stream;
+
+/** Validates resource identifiers and packages for java identifier validity. */
+public class JavaIdentifierValidator {
+
+  private JavaIdentifierValidator() {}
+
+  /** Thrown when a resource filed is not a valida java identifier. */
+  public static class InvalidJavaIdentifier extends RuntimeException {
+
+    /** Creates a new exception. */
+    public InvalidJavaIdentifier(String message) {
+      super(message);
+    }
+  }
+
+  /**
+   * Validates a resource identifier for java correctness.
+   *
+   * @param identifier an identifier derived from an android resource.
+   * @param additionalInformation optional information about the identifier.
+   * @return The identifier if valid.
+   * @throws InvalidJavaIdentifier if the identifier is invalid.
+   */
+  public static String validate(String identifier, Object... additionalInformation) {
+    if (VALID_JAVA_IDENTIFIER.test(identifier)) {
+      return identifier;
+    }
+    throw new InvalidJavaIdentifier(
+        String.format(
+            "%s is an invalid java identifier %s.",
+            identifier,
+            Stream.of(additionalInformation).map(Object::toString).collect(joining(" "))));
+  }
+
+  private static final ImmutableSet<String> JAVA_RESERVED =
+      ImmutableSet.of(
+          "abstract",
+          "assert",
+          "boolean",
+          "break",
+          "byte",
+          "case",
+          "catch",
+          "char",
+          "class",
+          "const",
+          "continue",
+          "default",
+          "double",
+          "do",
+          "else",
+          "enum",
+          "extends",
+          "false",
+          "final",
+          "finally",
+          "float",
+          "for",
+          "goto",
+          "if",
+          "implements",
+          "import",
+          "instanceof",
+          "int",
+          "interface",
+          "long",
+          "native",
+          "new",
+          "null",
+          "package",
+          "private",
+          "protected",
+          "public",
+          "return",
+          "short",
+          "static",
+          "strictfp",
+          "super",
+          "switch",
+          "synchronized",
+          "this",
+          "throw",
+          "throws",
+          "transient",
+          "true",
+          "try",
+          "void",
+          "volatile",
+          "while");
+
+  private static final Predicate<String> VALID_JAVA_IDENTIFIER =
+      ((Predicate<String>) JAVA_RESERVED::contains)
+          .negate()
+          .and(Pattern.compile("^([a-zA-Z_$][a-zA-Z\\d_$]*)$").asPredicate());
+}
diff --git a/src/tools/android/java/com/google/devtools/build/android/resources/RClassGenerator.java b/src/tools/android/java/com/google/devtools/build/android/resources/RClassGenerator.java
index 61bff97b79..2012a989d8 100644
--- a/src/tools/android/java/com/google/devtools/build/android/resources/RClassGenerator.java
+++ b/src/tools/android/java/com/google/devtools/build/android/resources/RClassGenerator.java
@@ -17,7 +17,6 @@ import static java.nio.file.StandardOpenOption.CREATE_NEW;
 
 import com.android.SdkConstants;
 import com.android.resources.ResourceType;
-import com.google.common.base.Preconditions;
 import com.google.common.base.Splitter;
 import com.google.common.collect.Iterables;
 import java.io.IOException;
@@ -142,12 +141,8 @@ public class RClassGenerator {
     }
     for (Map.Entry<String, FieldInitializer> entry : initializers.entrySet()) {
       FieldInitializer init = entry.getValue();
-      Preconditions.checkArgument(
-          !entry.getKey().contains(":"),
-          "%s in %s, %s is invalid java id",
-          entry.getKey(),
-          packageDir,
-          fullyQualifiedInnerClass);
+      JavaIdentifierValidator.validate(
+          entry.getKey(), "in class:", fullyQualifiedInnerClass, "and package:", packageDir);
       if (init.writeFieldDefinition(
           entry.getKey(), innerClassWriter, fieldAccessLevel, finalFields)) {
         deferredInitializers.put(entry.getKey(), init);