Skip to content

Commit

Permalink
Switch TargetPattern.Parser to use LabelParser
Browse files Browse the repository at this point in the history 
- Target patterns and labels have very similar syntax, yet are parsed completely separately. Furthermore, the code for target pattern parsing wasn't written with repos in mind, causing some corner cases such as #4385.
- This CL augments LabelParser to deal with some syntax specific to target patterns (mostly the '...' thing), and switches TargetPattern.Parser to use LabelParser instead.
- This fixes some inconsistencies between the two and eliminates the bug linked above.
- Added LabelParserTest to make sure that the table in the javadoc of LabelParser.Parts#parse is actually accurate.
  - Switched LabelParser.Parts to use AutoValue instead to enable testing.
- Some more cleanup in TargetPattern.Parser; removing outdated fields/methods/etc.

Fixes #4385.

RELNOTES: `@foo` labels can now be used on the command line as the top-level target (that is, `bazel build @foo` now works). Double-dot syntax is now forbidden (`bazel build ../foo` will no longer work).
PiperOrigin-RevId: 520902549
Change-Id: I38d6400381a3c4ac4c370360578702d5dd870909
  • Loading branch information
@Wyverald @copybara-github
Wyverald authored and copybara-github committed Mar 31, 2023
1 parent dd82239 commit 24f6fe8
Show file tree
Hide file tree
Showing 11 changed files with 265 additions and 252 deletions.
32 changes: 19 additions & 13 deletions src/main/java/com/google/devtools/build/lib/cmdline/Label.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,12 @@ abstract static class PackageContextImpl implements PackageContext {}
*/
public static Label parseCanonical(String raw) throws LabelSyntaxException {
Parts parts = Parts.parse(raw);
parts.checkPkgDoesNotEndWithTripleDots();
parts.checkPkgIsAbsolute();
RepositoryName repoName =
parts.repo == null ? RepositoryName.MAIN : RepositoryName.createUnvalidated(parts.repo);
parts.repo() == null ? RepositoryName.MAIN : RepositoryName.createUnvalidated(parts.repo());
return createUnvalidated(
PackageIdentifier.create(repoName, PathFragment.create(parts.pkg)), parts.target);
PackageIdentifier.create(repoName, PathFragment.create(parts.pkg())), parts.target());
}

/** Like {@link #parseCanonical}, but throws an unchecked exception instead. */
Expand All @@ -139,18 +140,18 @@ public static Label parseCanonicalUnchecked(String raw) {
/** Computes the repo name for the label, within the context of a current repo. */
private static RepositoryName computeRepoNameWithRepoContext(
Parts parts, RepoContext repoContext) {
if (parts.repo == null) {
if (parts.repo() == null) {
// Certain package names when used without a "@" part are always absolutely in the main repo,
// disregarding the current repo and repo mappings.
return ABSOLUTE_PACKAGE_NAMES.contains(parts.pkg)
return ABSOLUTE_PACKAGE_NAMES.contains(parts.pkg())
? RepositoryName.MAIN
: repoContext.currentRepo();
}
if (parts.repoIsCanonical) {
if (parts.repoIsCanonical()) {
// This label uses the canonical label literal syntax starting with two @'s ("@@foo//bar").
return RepositoryName.createUnvalidated(parts.repo);
return RepositoryName.createUnvalidated(parts.repo());
}
return repoContext.repoMapping().get(parts.repo);
return repoContext.repoMapping().get(parts.repo());
}

/**
Expand All @@ -162,10 +163,11 @@ private static RepositoryName computeRepoNameWithRepoContext(
public static Label parseWithRepoContext(String raw, RepoContext repoContext)
throws LabelSyntaxException {
Parts parts = Parts.parse(raw);
parts.checkPkgDoesNotEndWithTripleDots();
parts.checkPkgIsAbsolute();
RepositoryName repoName = computeRepoNameWithRepoContext(parts, repoContext);
return createUnvalidated(
PackageIdentifier.create(repoName, PathFragment.create(parts.pkg)), parts.target);
PackageIdentifier.create(repoName, PathFragment.create(parts.pkg())), parts.target());
}

/**
Expand All @@ -178,14 +180,15 @@ public static Label parseWithRepoContext(String raw, RepoContext repoContext)
public static Label parseWithPackageContext(String raw, PackageContext packageContext)
throws LabelSyntaxException {
Parts parts = Parts.parse(raw);
parts.checkPkgDoesNotEndWithTripleDots();
// pkg is either absolute or empty
if (!parts.pkg.isEmpty()) {
if (!parts.pkg().isEmpty()) {
parts.checkPkgIsAbsolute();
}
RepositoryName repoName = computeRepoNameWithRepoContext(parts, packageContext);
PathFragment pkgFragment =
parts.pkgIsAbsolute ? PathFragment.create(parts.pkg) : packageContext.packageFragment();
return createUnvalidated(PackageIdentifier.create(repoName, pkgFragment), parts.target);
parts.pkgIsAbsolute() ? PathFragment.create(parts.pkg()) : packageContext.packageFragment();
return createUnvalidated(PackageIdentifier.create(repoName, pkgFragment), parts.target());
}

/**
Expand All @@ -200,7 +203,7 @@ public static Label parseWithPackageContext(String raw, PackageContext packageCo
public static Label create(String packageName, String targetName) throws LabelSyntaxException {
return createUnvalidated(
PackageIdentifier.parse(packageName),
validateAndProcessTargetName(packageName, targetName));
validateAndProcessTargetName(packageName, targetName, /* pkgEndsWithTripleDots= */ false));
}

/**
Expand All @@ -211,7 +214,10 @@ public static Label create(PackageIdentifier packageId, String targetName)
throws LabelSyntaxException {
return createUnvalidated(
packageId,
validateAndProcessTargetName(packageId.getPackageFragment().getPathString(), targetName));
validateAndProcessTargetName(
packageId.getPackageFragment().getPathString(),
targetName,
/* pkgEndsWithTripleDots= */ false));
}

/**
Expand Down
149 changes: 91 additions & 58 deletions src/main/java/com/google/devtools/build/lib/cmdline/LabelParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

package com.google.devtools.build.lib.cmdline;

import com.google.auto.value.AutoValue;
import com.google.common.annotations.VisibleForTesting;
import com.google.devtools.build.lib.util.StringUtilities;
import com.google.errorprone.annotations.FormatMethod;
import javax.annotation.Nullable;
Expand All @@ -26,82 +28,83 @@ private LabelParser() {}
* Contains the parsed elements of a label string. The parts are validated (they don't contain
* invalid characters). See {@link #parse} for valid label patterns.
*/
static final class Parts {
@AutoValue
abstract static class Parts {
/**
* The {@code @repo} or {@code @@canonical_repo} part of the string (sans any leading
* {@literal @}s); can be null if it doesn't have such a part (i.e. if it doesn't start with a
* {@literal @}).
*/
@Nullable final String repo;
@Nullable
abstract String repo();
/**
* Whether the repo part is using the canonical repo syntax (two {@literal @}s) or not (one
* {@literal @}). If there is no repo part, this is false.
*/
final boolean repoIsCanonical;
abstract boolean repoIsCanonical();
/**
* Whether the package part of the string is prefixed by double-slash. This can only be false if
* the repo part is missing.
*/
final boolean pkgIsAbsolute;
/** The package part of the string (sans double-slash, if any). */
final String pkg;
abstract boolean pkgIsAbsolute();
/**
* The package part of the string (sans the leading double-slash, if present; also sans the
* final '...' segment, if present).
*/
abstract String pkg();
/** Whether the package part of the string ends with a '...' segment. */
abstract boolean pkgEndsWithTripleDots();
/** The target part of the string (sans colon). */
final String target;
abstract String target();
/** The original unparsed raw string. */
final String raw;
abstract String raw();

private Parts(
@Nullable String repo,
boolean repoIsCanonical,
boolean pkgIsAbsolute,
String pkg,
String target,
String raw) {
this.repo = repo;
this.repoIsCanonical = repoIsCanonical;
this.pkgIsAbsolute = pkgIsAbsolute;
this.pkg = pkg;
this.target = target;
this.raw = raw;
}

private static Parts validateAndCreate(
@VisibleForTesting
static Parts validateAndCreate(
@Nullable String repo,
boolean repoIsCanonical,
boolean pkgIsAbsolute,
String pkg,
boolean pkgEndsWithTripleDots,
String target,
String raw)
throws LabelSyntaxException {
validateRepoName(repo);
validatePackageName(pkg, target);
return new Parts(
return new AutoValue_LabelParser_Parts(
repo,
repoIsCanonical,
pkgIsAbsolute,
pkg,
validateAndProcessTargetName(pkg, target),
pkgEndsWithTripleDots,
validateAndProcessTargetName(pkg, target, pkgEndsWithTripleDots),
raw);
}

/**
* Parses a raw label string into parts. The logic can be summarized by the following table:
*
* {@code
* raw | repo | repoIsCanonical | pkgIsAbsolute | pkg | target
* ----------------------+--------+-----------------+---------------+-----------+-----------
* foo/bar | null | false | false | "" | "foo/bar"
* //foo/bar | null | false | true | "foo/bar" | "bar"
* @repo | "repo" | false | true | "" | "repo"
* @@repo | "repo" | true | true | "" | "repo"
* @repo//foo/bar | "repo" | false | true | "foo/bar" | "bar"
* @@repo//foo/bar | "repo" | true | true | "foo/bar" | "bar"
* :quux | null | false | false | "" | "quux"
* foo/bar:quux | null | false | false | "foo/bar" | "quux"
* //foo/bar:quux | null | false | true | "foo/bar" | "quux"
* @repo//foo/bar:quux | "repo" | false | true | "foo/bar" | "quux"
* @@repo//foo/bar:quux | "repo" | true | true | "foo/bar" | "quux"
* }
* <pre>{@code
* raw | repo | repoIs- | pkgIs- | pkg | pkgEndsWith- | target
* | | Canonical | Absolute | | TripleDots |
* ----------------------+--------+-----------+----------+-----------+--------------+-----------
* "foo/bar" | null | false | false | "" | false | "foo/bar"
* "..." | null | false | false | "" | true | ""
* "...:all" | null | false | false | "" | true | "all"
* "foo/..." | null | false | false | "foo" | true | ""
* "//foo/bar" | null | false | true | "foo/bar" | false | "bar"
* "//foo/..." | null | false | true | "foo" | true | ""
* "//foo/...:all" | null | false | true | "foo" | true | "all"
* "//foo/all" | null | false | true | "foo/all" | false | "all"
* "@repo" | "repo" | false | true | "" | false | "repo"
* "@@repo" | "repo" | true | true | "" | false | "repo"
* "@repo//foo/bar" | "repo" | false | true | "foo/bar" | false | "bar"
* "@@repo//foo/bar" | "repo" | true | true | "foo/bar" | false | "bar"
* ":quux" | null | false | false | "" | false | "quux"
* "foo/bar:quux" | null | false | false | "foo/bar" | false | "quux"
* "//foo/bar:quux" | null | false | true | "foo/bar" | false | "quux"
* "@repo//foo/bar:quux" | "repo" | false | true | "foo/bar" | false | "quux"
* }</pre>
*/
static Parts parse(String rawLabel) throws LabelSyntaxException {
@Nullable final String repo;
Expand All @@ -116,9 +119,10 @@ static Parts parse(String rawLabel) throws LabelSyntaxException {
return validateAndCreate(
repo,
repoIsCanonical,
/*pkgIsAbsolute=*/ true,
/*pkg=*/ "",
/*target=*/ repo,
/* pkgIsAbsolute= */ true,
/* pkg= */ "",
/* pkgEndsWithTripleDots= */ false,
/* target= */ repo,
rawLabel);
} else {
repo = rawLabel.substring(repoIsCanonical ? 2 : 1, doubleSlashIndex);
Expand All @@ -136,20 +140,39 @@ static Parts parse(String rawLabel) throws LabelSyntaxException {
final String pkg;
final String target;
final int colonIndex = rawLabel.indexOf(':', startOfPackage);
if (colonIndex >= 0) {
pkg = rawLabel.substring(startOfPackage, colonIndex);
target = rawLabel.substring(colonIndex + 1);
} else if (pkgIsAbsolute) {
// Special case: the label "[@repo]//foo/bar" is synonymous with "[@repo]//foo/bar:bar".
pkg = rawLabel.substring(startOfPackage);
// The target name is the last package segment (works even if `pkg` contains no slash)
target = pkg.substring(pkg.lastIndexOf('/') + 1);
} else {
final String rawPkg =
rawLabel.substring(startOfPackage, colonIndex >= 0 ? colonIndex : rawLabel.length());
final boolean pkgEndsWithTripleDots = rawPkg.endsWith("/...") || rawPkg.equals("...");
if (colonIndex < 0 && pkgEndsWithTripleDots) {
// Special case: if the entire label ends in '...', the target name is empty.
pkg = stripTrailingTripleDots(rawPkg);
target = "";
} else if (colonIndex < 0 && !pkgIsAbsolute) {
// Special case: the label "foo/bar" is synonymous with ":foo/bar".
pkg = "";
target = rawLabel.substring(startOfPackage);
} else {
pkg = stripTrailingTripleDots(rawPkg);
if (colonIndex >= 0) {
target = rawLabel.substring(colonIndex + 1);
} else {
// Special case: the label "[@repo]//foo/bar" is synonymous with "[@repo]//foo/bar:bar".
// The target name is the last package segment (works even if `pkg` contains no slash)
target = pkg.substring(pkg.lastIndexOf('/') + 1);
}
}
return validateAndCreate(
repo, repoIsCanonical, pkgIsAbsolute, pkg, pkgEndsWithTripleDots, target, rawLabel);
}

private static String stripTrailingTripleDots(String pkg) {
if (pkg.endsWith("/...")) {
return pkg.substring(0, pkg.length() - 4);
}
return validateAndCreate(repo, repoIsCanonical, pkgIsAbsolute, pkg, target, rawLabel);
if (pkg.equals("...")) {
return "";
}
return pkg;
}

private static void validateRepoName(@Nullable String repo) throws LabelSyntaxException {
Expand All @@ -167,8 +190,14 @@ private static void validatePackageName(String pkg, String target) throws LabelS
}

void checkPkgIsAbsolute() throws LabelSyntaxException {
if (!pkgIsAbsolute) {
throw syntaxErrorf("invalid label '%s': absolute label must begin with '@' or '//'", raw);
if (!pkgIsAbsolute()) {
throw syntaxErrorf("invalid label '%s': absolute label must begin with '@' or '//'", raw());
}
}

void checkPkgDoesNotEndWithTripleDots() throws LabelSyntaxException {
if (pkgEndsWithTripleDots()) {
throw syntaxErrorf("invalid label '%s': package name cannot contain '...'", raw());
}
}
}
Expand All @@ -183,8 +212,12 @@ private static String perhapsYouMeantMessage(String pkg, String target) {
return pkg.endsWith('/' + target) ? " (perhaps you meant \":" + target + "\"?)" : "";
}

static String validateAndProcessTargetName(String pkg, String target)
throws LabelSyntaxException {
static String validateAndProcessTargetName(
String pkg, String target, boolean pkgEndsWithTripleDots) throws LabelSyntaxException {
if (pkgEndsWithTripleDots && target.isEmpty()) {
// Allow empty target name if the package part ends in '...'.
return target;
}
String targetError = LabelValidator.validateTargetName(target);
if (targetError != null) {
throw syntaxErrorf(
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,8 @@ public static PackageIdentifier parse(String input) throws LabelSyntaxException
}
LabelParser.Parts parts = LabelParser.Parts.parse(input + ":dummy_target");
RepositoryName repoName =
parts.repo == null ? RepositoryName.MAIN : RepositoryName.createUnvalidated(parts.repo);
return create(repoName, PathFragment.create(parts.pkg));
parts.repo() == null ? RepositoryName.MAIN : RepositoryName.createUnvalidated(parts.repo());
return create(repoName, PathFragment.create(parts.pkg()));
}

public RepositoryName getRepository() {
Expand Down
Loading

0 comments on commit 24f6fe8

Please sign in to comment.