RELNOTES: Allow dots in package names.

--
MOS_MIGRATED_REVID=105512492

src/main/java/com/google/devtools/build/lib/cmdline/LabelValidator.java

@@ -14,6 +14,7 @@
 
 package com.google.devtools.build.lib.cmdline;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.CharMatcher;
 
 import java.util.Objects;
@@ -49,8 +50,13 @@ public final class LabelValidator {
           .or(PUNCTUATION_REQUIRING_QUOTING)
           .or(PUNCTUATION_NOT_REQUIRING_QUOTING);
 
-  private static final String PACKAGE_NAME_ERROR =
-      "package names may contain only A-Z, a-z, 0-9, '/', '-' and '_'";
+  @VisibleForTesting
+  static final String PACKAGE_NAME_ERROR =
+      "package names may contain only A-Z, a-z, 0-9, '/', '-', '.' and '_'";
+
+  @VisibleForTesting
+  static final String PACKAGE_NAME_DOT_ERROR =
+      "package name component contains only '.' characters";
 
   /**
    * Performs validity checking of the specified package name. Returns null on success or an error
@@ -72,28 +78,43 @@ public static String validatePackageName(String packageName) {
       return "package names may not start with '/'";
     }
 
-    // Fast path for packages with '.' in their name
-    if (packageName.lastIndexOf('.') != -1) {
-      return PACKAGE_NAME_ERROR;
-    }
-
-    // Check for any character outside of [/0-9A-Za-z_-]. Try to evaluate the
+    // Check for any character outside of [/0-9.A-Za-z_-]. Try to evaluate the
     // conditional quickly (by looking in decreasing order of character class
-    // likelihood).
-    for (int i = len - 1; i >= 0; --i) {
-      char c = packageName.charAt(i);
-      if ((c < 'a' || c > 'z') && c != '/' && c != '_' && c != '-' &&
-          (c < '0' || c > '9') && (c < 'A' || c > 'Z')) {
+    // likelihood). To deal with . and .. pretend that the name is surrounded by '/'
+    // on both sides.
+    boolean nonDot = false;
+    int lastSlash = len;
+    for (int i = len - 1; i >= -1; --i) {
+      char c = (i >= 0) ? packageName.charAt(i) : '/';
+      if ((c < 'a' || c > 'z')
+          && c != '/'
+          && c != '_'
+          && c != '-'
+          && c != '.'
+          && (c < '0' || c > '9')
+          && (c < 'A' || c > 'Z')) {
         return PACKAGE_NAME_ERROR;
       }
-    }
 
-    if (packageName.contains("//")) {
-      return "package names may not contain '//' path separators";
-    }
-    if (packageName.endsWith("/")) {
-      return "package names may not end with '/'";
+      if (c == '/') {
+        if (lastSlash == i + 1) {
+          return lastSlash == len
+              ? "package names may not end with '/'"
+              : "package names may not contain '//' path separators";
+        }
+
+        if (!nonDot) {
+          return PACKAGE_NAME_DOT_ERROR;
+        }
+        nonDot = false;
+        lastSlash = i;
+      } else {
+        if (c != '.') {
+          nonDot = true;
+        }
+      }
     }
+
     return null; // ok
   }
 

src/test/java/com/google/devtools/build/lib/cmdline/LabelValidatorTest.java

@@ -33,9 +33,6 @@
 @RunWith(JUnit4.class)
 public class LabelValidatorTest {
 
-  private static final String BAD_PACKAGE_CHARS =
-      "package names may contain only A-Z, a-z, 0-9, '/', '-' and '_'";
-
   private PackageAndTarget newFooTarget() {
     return new PackageAndTarget("foo", "foo");
   }
@@ -54,22 +51,38 @@ public void testValidatePackageName() throws Exception {
     assertNull(LabelValidator.validatePackageName("foo-bar"));
     assertNull(LabelValidator.validatePackageName("Foo-Bar"));
     assertNull(LabelValidator.validatePackageName("FOO-BAR"));
+    assertNull(LabelValidator.validatePackageName("bar.baz"));
+    assertNull(LabelValidator.validatePackageName("a/..b"));
+    assertNull(LabelValidator.validatePackageName("a/.b"));
+    assertNull(LabelValidator.validatePackageName("a/b."));
+    assertNull(LabelValidator.validatePackageName("a/b.."));
 
     // Bad:
-    assertEquals("package names may not start with '/'",
-        LabelValidator.validatePackageName("/foo"));
-    assertEquals("package names may not end with '/'",
-                 LabelValidator.validatePackageName("foo/"));
-    assertEquals(BAD_PACKAGE_CHARS,
-                 LabelValidator.validatePackageName("bar baz"));
-    assertEquals(BAD_PACKAGE_CHARS,
-                 LabelValidator.validatePackageName("foo:bar"));
-    assertEquals(BAD_PACKAGE_CHARS,
-                 LabelValidator.validatePackageName("baz@12345"));
-    assertEquals(BAD_PACKAGE_CHARS,
-                 LabelValidator.validatePackageName("baz(foo)"));
-    assertEquals(BAD_PACKAGE_CHARS,
-                 LabelValidator.validatePackageName("bazfoo)"));
+    assertEquals(
+        "package names may not start with '/'", LabelValidator.validatePackageName("/foo"));
+    assertEquals("package names may not end with '/'", LabelValidator.validatePackageName("foo/"));
+    assertEquals(LabelValidator.PACKAGE_NAME_ERROR, LabelValidator.validatePackageName("bar baz"));
+    assertEquals(LabelValidator.PACKAGE_NAME_ERROR, LabelValidator.validatePackageName("foo:bar"));
+    assertEquals(
+        LabelValidator.PACKAGE_NAME_ERROR, LabelValidator.validatePackageName("baz@12345"));
+    assertEquals(LabelValidator.PACKAGE_NAME_ERROR, LabelValidator.validatePackageName("baz(foo)"));
+    assertEquals(LabelValidator.PACKAGE_NAME_ERROR, LabelValidator.validatePackageName("bazfoo)"));
+
+    assertEquals(
+        LabelValidator.PACKAGE_NAME_DOT_ERROR, LabelValidator.validatePackageName("bar/../baz"));
+    assertEquals(
+        LabelValidator.PACKAGE_NAME_DOT_ERROR, LabelValidator.validatePackageName("bar/.."));
+    assertEquals(
+        LabelValidator.PACKAGE_NAME_DOT_ERROR, LabelValidator.validatePackageName("../bar"));
+    assertEquals(
+        LabelValidator.PACKAGE_NAME_DOT_ERROR, LabelValidator.validatePackageName("bar/..."));
+
+    assertEquals(
+        LabelValidator.PACKAGE_NAME_DOT_ERROR, LabelValidator.validatePackageName("bar/./baz"));
+    assertEquals(
+        LabelValidator.PACKAGE_NAME_DOT_ERROR, LabelValidator.validatePackageName("bar/."));
+    assertEquals(
+        LabelValidator.PACKAGE_NAME_DOT_ERROR, LabelValidator.validatePackageName("./bar"));
   }
 
   @Test

src/test/java/com/google/devtools/build/lib/cmdline/TargetPatternTest.java

@@ -57,7 +57,7 @@ public void testPassingValidations() throws TargetParsingException {
   @Test
   public void testInvalidPatterns() throws TargetParsingException {
     try {
-      parse("Bar.java");
+      parse("Bar&&&java");
       fail();
     } catch (TargetParsingException expected) {
     }