Clean up Label validation, and introduce a factory method for constructing a Label without validation.

RELNOTES: None
PiperOrigin-RevId: 164479651

1 files changed, 25 insertions, 33 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java
index f8e5643491..9e54fc10a6 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java
@@ -123,7 +123,7 @@ public final class Label implements Comparable<Label>, Serializable, SkylarkValu
     try {
       LabelValidator.PackageAndTarget labelParts = LabelValidator.parseAbsoluteLabel(absName);
       PackageIdentifier pkgIdWithoutRepo =
-          validate(labelParts.getPackageName(), labelParts.getTargetName());
+          validatePackageName(labelParts.getPackageName(), labelParts.getTargetName());
       PathFragment packageFragment = pkgIdWithoutRepo.getPackageFragment();
       if (repo.isEmpty() && ABSOLUTE_PACKAGE_NAMES.contains(packageFragment)) {
         repo = "@";
@@ -164,7 +164,7 @@ public final class Label implements Comparable<Label>, Serializable, SkylarkValu
    * @throws LabelSyntaxException if either of the arguments was invalid.
    */
   public static Label create(String packageName, String targetName) throws LabelSyntaxException {
-    return LABEL_INTERNER.intern(new Label(packageName, targetName));
+    return create(validatePackageName(packageName, targetName), targetName);
   }
 
   /**
@@ -173,7 +173,18 @@ public final class Label implements Comparable<Label>, Serializable, SkylarkValu
    */
   public static Label create(PackageIdentifier packageId, String targetName)
       throws LabelSyntaxException {
-    return LABEL_INTERNER.intern(new Label(packageId, targetName));
+    return createUnvalidated(packageId, validateTargetName(packageId, targetName));
+  }
+
+  /**
+   * Similar factory to above, but does not perform target name validation.
+   *
+   * <p>Only call this method if you know what you're doing; in particular, don't call it on
+   * arbitrary {@code targetName} inputs
+   */
+
+  public static Label createUnvalidated(PackageIdentifier packageId, String targetName) {
+    return LABEL_INTERNER.intern(new Label(packageId, StringCanonicalizer.intern(targetName)));
   }
 
   /**
@@ -213,13 +224,17 @@ public final class Label implements Comparable<Label>, Serializable, SkylarkValu
   }
 
   /**
-   * Validates the given target name and returns a canonical String instance if it is valid.
-   * Otherwise it throws a SyntaxException.
+   * Validates the given target name and returns a normalized name if it is valid. Otherwise it
+   * throws a SyntaxException.
    */
-  private static String canonicalizeTargetName(String name) throws LabelSyntaxException {
+  private static String validateTargetName(PackageIdentifier packageIdentifier, String name)
+      throws LabelSyntaxException {
     String error = LabelValidator.validateTargetName(name);
     if (error != null) {
       error = "invalid target name '" + StringUtilities.sanitizeControlChars(name) + "': " + error;
+      if (packageIdentifier.getPackageFragment().getPathString().endsWith("/" + name)) {
+        error += " (perhaps you meant \":" + name + "\"?)";
+      }
       throw new LabelSyntaxException(error);
     }
 
@@ -227,15 +242,14 @@ public final class Label implements Comparable<Label>, Serializable, SkylarkValu
     if (name.endsWith("/.")) {
       name = name.substring(0, name.length() - 2);
     }
-
-    return StringCanonicalizer.intern(name);
+    return name;
   }
 
   /**
    * Validates the given package name and returns a canonical {@link PackageIdentifier} instance
    * if it is valid. Otherwise it throws a SyntaxException.
    */
-  private static PackageIdentifier validate(String packageIdentifier, String name)
+  private static PackageIdentifier validatePackageName(String packageIdentifier, String name)
       throws LabelSyntaxException {
     String error = null;
     try {
@@ -262,34 +276,12 @@ public final class Label implements Comparable<Label>, Serializable, SkylarkValu
   /** Precomputed hash code. */
   private final int hashCode;
 
-  /**
-   * Constructor from a package name, target name. Both are checked for validity
-   * and a SyntaxException is thrown if either is invalid.
-   * TODO(bazel-team): move the validation to {@link PackageIdentifier}. Unfortunately, there are a
-   * bazillion tests that use invalid package names (taking advantage of the fact that calling
-   * Label(PathFragment, String) doesn't validate the package name).
-   */
-  private Label(String packageIdentifier, String name) throws LabelSyntaxException {
-    this(validate(packageIdentifier, name), name);
-  }
-
-  private Label(PackageIdentifier packageIdentifier, String name)
-      throws LabelSyntaxException {
+  private Label(PackageIdentifier packageIdentifier, String name) {
     Preconditions.checkNotNull(packageIdentifier);
     Preconditions.checkNotNull(name);
 
     this.packageIdentifier = packageIdentifier;
-    try {
-      this.name = canonicalizeTargetName(name);
-    } catch (LabelSyntaxException e) {
-      // This check is just for a more helpful error message
-      // i.e. valid target name, invalid package name, colon-free label form
-      // used => probably they meant "//foo:bar.c" not "//foo/bar.c".
-      if (packageIdentifier.getPackageFragment().getPathString().endsWith("/" + name)) {
-        throw new LabelSyntaxException(e.getMessage() + " (perhaps you meant \":" + name + "\"?)");
-      }
-      throw e;
-    }
+    this.name = name;
     this.hashCode = hashCode(this.name, this.packageIdentifier);
   }