From 7b76683d03518991218399a915870acccc94621a Mon Sep 17 00:00:00 2001 From: janakr Date: Wed, 30 Dec 2020 08:16:25 -0800 Subject: [PATCH] Expose TargetsBelowDirectory as a visible subclass of TargetPattern: it has too many dedicated methods for it to remain hidden. PiperOrigin-RevId: 349560782 --- .../build/lib/cmdline/TargetPattern.java | 185 ++++++++---------- .../GraphBackedRecursivePackageProvider.java | 20 +- .../lib/skyframe/TargetPatternValue.java | 67 ++++--- .../build/lib/cmdline/TargetPatternTest.java | 155 ++++----------- 4 files changed, 173 insertions(+), 254 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java b/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java index e6e112238a675b..c04f40955a38b6 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java @@ -62,7 +62,6 @@ public abstract class TargetPattern implements Serializable { private static final Parser DEFAULT_PARSER = new Parser(PathFragment.EMPTY_FRAGMENT); - private final Type type; private final String originalPattern; private final PathFragment offset; @@ -117,9 +116,8 @@ static String normalize(String path) { return SLASH_JOINER.join(pieces); } - private TargetPattern(Type type, String originalPattern, PathFragment offset) { + private TargetPattern(String originalPattern, PathFragment offset) { // Don't allow inheritance outside this class. - this.type = type; this.originalPattern = Preconditions.checkNotNull(originalPattern); this.offset = Preconditions.checkNotNull(offset); } @@ -128,9 +126,7 @@ private TargetPattern(Type type, String originalPattern, PathFragment offset) { * Return the type of the pattern. Examples include "below directory" like "foo/..." and "single * target" like "//x:y". */ - public Type getType() { - return type; - } + public abstract Type getType(); /** * Return the string that was parsed into this pattern. @@ -208,72 +204,6 @@ public ListenableFuture evalAsync( resolver, ignoredSubdirectories, excludedSubdirectories, callback, exceptionClass); } - /** - * Returns {@code true} iff this pattern has type {@code Type.TARGETS_BELOW_DIRECTORY} and - * {@code directory} is contained by or equals this pattern's directory. - * - *

For example, returns {@code true} for {@code this = TargetPattern ("//...")} and - * {@code directory = "foo")}. - */ - public abstract boolean containsAllTransitiveSubdirectoriesForTBD(PackageIdentifier directory); - - /** A tristate return value for {@link #containsTBDForTBD}. */ - public enum ContainsTBDForTBDResult { - /** - * Evaluating this TBD pattern with a directory exclusion of the other TBD pattern's directory - * would result in exactly the same set of targets as evaluating the subtraction of the other - * TBD pattern from this one. - */ - DIRECTORY_EXCLUSION_WOULD_BE_EXACT, - /** - * A directory exclusion of the other TBD pattern's directory would be too broad because this - * TBD pattern is not "rules only" and the other one is, meaning that this TBD pattern - * potentially matches more targets underneath the directory in question than the other one - * does. Thus, a directory exclusion would incorrectly exclude non-rule targets. - */ - DIRECTORY_EXCLUSION_WOULD_BE_TOO_BROAD, - /** - * None of the above. Perhaps the other pattern isn't a TBD pattern or perhaps it's not - * contained by this pattern. - */ - OTHER, - } - - /** - * Determines how, if it all, the evaluation of this TBD pattern with a directory exclusion of the - * given TBD {@code containedPattern}'s directory relates to the evaluation of the subtraction of - * the given {@code containedPattern} from this one. - */ - public ContainsTBDForTBDResult containsTBDForTBD(TargetPattern containedPattern) { - if (containedPattern.getType() != Type.TARGETS_BELOW_DIRECTORY) { - return ContainsTBDForTBDResult.OTHER; - } else if (containsAllTransitiveSubdirectoriesForTBD( - containedPattern.getDirectoryForTargetsUnderDirectory())) { - return !getRulesOnly() && containedPattern.getRulesOnly() - ? ContainsTBDForTBDResult.DIRECTORY_EXCLUSION_WOULD_BE_TOO_BROAD - : ContainsTBDForTBDResult.DIRECTORY_EXCLUSION_WOULD_BE_EXACT; - } else { - return ContainsTBDForTBDResult.OTHER; - } - - } - - /** - * For patterns of type {@link Type#TARGETS_BELOW_DIRECTORY}, returns a {@link PackageIdentifier} - * identifying the most specific containing directory of the patterns that could be matched by - * this pattern. - * - *

Note that we are using the {@link PackageIdentifier} type as a convenience; there may not - * actually be a package corresponding to this directory! - * - *

This returns a {@link PackageIdentifier} that identifies the referred-to directory. For - * example, for a {@link Type#TARGETS_BELOW_DIRECTORY} corresponding to "//foo/bar/...", this - * method returns a {@link PackageIdentifier} for "foo/bar". - */ - public PackageIdentifier getDirectoryForTargetsUnderDirectory() { - throw new IllegalStateException(); - } - /** * For patterns of type {@link Type#PATH_AS_TARGET}, returns the path in question. * @@ -318,7 +248,7 @@ private SingleTarget( PackageIdentifier directory, String originalPattern, PathFragment offset) { - super(Type.SINGLE_TARGET, originalPattern, offset); + super(originalPattern, offset); this.targetName = Preconditions.checkNotNull(targetName); this.directory = Preconditions.checkNotNull(directory); } @@ -334,11 +264,6 @@ public void eval( callback.process(resolver.getExplicitTarget(label(targetName)).getTargets()); } - @Override - public boolean containsAllTransitiveSubdirectoriesForTBD(PackageIdentifier directory) { - return false; - } - @Override public PackageIdentifier getDirectoryForTargetOrTargetsInPackage() { return directory; @@ -359,6 +284,11 @@ public String getSingleTargetPath() { return targetName; } + @Override + public Type getType() { + return Type.SINGLE_TARGET; + } + @Override public boolean equals(Object o) { if (this == o) { @@ -378,11 +308,10 @@ public int hashCode() { } private static final class InterpretPathAsTarget extends TargetPattern { - private final String path; private InterpretPathAsTarget(String path, String originalPattern, PathFragment offset) { - super(Type.PATH_AS_TARGET, originalPattern, offset); + super(originalPattern, offset); this.path = normalize(Preconditions.checkNotNull(path)); } @@ -421,11 +350,6 @@ public void eval( } } - @Override - public boolean containsAllTransitiveSubdirectoriesForTBD(PackageIdentifier directory) { - return false; - } - @Override public String getPathForPathAsTarget() { return path; @@ -443,6 +367,11 @@ public boolean getRulesOnly() { return false; } + @Override + public Type getType() { + return Type.PATH_AS_TARGET; + } + @Override public boolean equals(Object o) { if (this == o) { @@ -462,7 +391,6 @@ public int hashCode() { } private static final class TargetsInPackage extends TargetPattern { - private final PackageIdentifier packageIdentifier; private final String suffix; private final boolean wasOriginallyAbsolute; @@ -477,7 +405,7 @@ private TargetsInPackage( boolean wasOriginallyAbsolute, boolean rulesOnly, boolean checkWildcardConflict) { - super(Type.TARGETS_IN_PACKAGE, originalPattern, offset); + super(originalPattern, offset); Preconditions.checkArgument(!packageIdentifier.getRepository().isDefault()); this.packageIdentifier = packageIdentifier; this.suffix = Preconditions.checkNotNull(suffix); @@ -506,11 +434,6 @@ public void eval( resolver.getTargetsInPackage(getOriginalPattern(), packageIdentifier, rulesOnly)); } - @Override - public boolean containsAllTransitiveSubdirectoriesForTBD(PackageIdentifier directory) { - return false; - } - @Override public PackageIdentifier getDirectoryForTargetOrTargetsInPackage() { return packageIdentifier; @@ -526,6 +449,11 @@ public boolean getRulesOnly() { return rulesOnly; } + @Override + public Type getType() { + return Type.TARGETS_IN_PACKAGE; + } + @Override public boolean equals(Object o) { if (this == o) { @@ -586,8 +514,13 @@ private ResolvedTargets getWildcardConflict(TargetPatternResolver reso } } - private static final class TargetsBelowDirectory extends TargetPattern { - + /** + * Specialization of {@link TargetPattern} for {@link Type#TARGETS_BELOW_DIRECTORY}. Exposed + * because it has a considerable number of specific methods. If {@link TargetPattern#getType} + * returns {@link Type#TARGETS_BELOW_DIRECTORY} the instance can safely be cast to {@code + * TargetsBelowDirectory}. + */ + public static final class TargetsBelowDirectory extends TargetPattern { private final PackageIdentifier directory; private final boolean rulesOnly; @@ -596,7 +529,7 @@ private TargetsBelowDirectory( PathFragment offset, PackageIdentifier directory, boolean rulesOnly) { - super(Type.TARGETS_BELOW_DIRECTORY, originalPattern, offset); + super(originalPattern, offset); Preconditions.checkArgument(!directory.getRepository().isDefault()); this.directory = Preconditions.checkNotNull(directory); this.rulesOnly = rulesOnly; @@ -641,8 +574,14 @@ public ListenableFuture evalAsync( executor); } - @Override - public boolean containsAllTransitiveSubdirectoriesForTBD(PackageIdentifier containedDirectory) { + /** + * Returns true if {@code containedDirectory} is contained by or equals this pattern's + * directory. + * + *

For example, returns {@code true} for {@code this = TargetPattern ("//...")} and {@code + * directory = "foo")}. + */ + public boolean containsAllTransitiveSubdirectories(PackageIdentifier containedDirectory) { // Note that merely checking to see if the directory startsWith the TargetsBelowDirectory's // directory is insufficient. "food" begins with "foo", but "//foo/..." does not contain // "//food/...". @@ -650,8 +589,51 @@ public boolean containsAllTransitiveSubdirectoriesForTBD(PackageIdentifier conta && containedDirectory.getPackageFragment().startsWith(directory.getPackageFragment()); } - @Override - public PackageIdentifier getDirectoryForTargetsUnderDirectory() { + /** + * Determines how, if it all, the evaluation of this pattern with a directory exclusion of the + * given {@code containedPattern}'s directory relates to the evaluation of the subtraction of + * the given {@code containedPattern} from this one. + */ + public ContainsResult contains(TargetsBelowDirectory containedPattern) { + if (containsAllTransitiveSubdirectories(containedPattern.getDirectory())) { + return !getRulesOnly() && containedPattern.getRulesOnly() + ? ContainsResult.DIRECTORY_EXCLUSION_WOULD_BE_TOO_BROAD + : ContainsResult.DIRECTORY_EXCLUSION_WOULD_BE_EXACT; + } else { + return ContainsResult.NOT_CONTAINED; + } + } + + /** A tristate return value for {@link #contains}. */ + public enum ContainsResult { + /** + * Evaluating this pattern with a directory exclusion of the other pattern's directory would + * result in exactly the same set of targets as evaluating the subtraction of the other + * pattern from this one. + */ + DIRECTORY_EXCLUSION_WOULD_BE_EXACT, + /** + * A directory exclusion of the other pattern's directory would be too broad because this + * pattern is not "rules only" and the other one is, meaning that this pattern potentially + * matches more targets underneath the directory in question than the other one does. Thus, a + * directory exclusion would incorrectly exclude non-rule targets. + */ + DIRECTORY_EXCLUSION_WOULD_BE_TOO_BROAD, + /** None of the above. The other pattern isn't contained by this pattern. */ + NOT_CONTAINED, + } + + /** + * Returns a {@link PackageIdentifier} identifying the most specific containing directory of the + * patterns that could be matched by this pattern. + * + *

Note that we are using the {@link PackageIdentifier} type as a convenience; there may not + * actually be a package corresponding to this directory! + * + *

This returns a {@link PackageIdentifier} that identifies the referred-to directory. For + * example, for "//foo/bar/...", this method returns a {@link PackageIdentifier} for "foo/bar". + */ + public PackageIdentifier getDirectory() { return directory; } @@ -665,6 +647,11 @@ public boolean getRulesOnly() { return rulesOnly; } + @Override + public Type getType() { + return Type.TARGETS_BELOW_DIRECTORY; + } + @Override public boolean equals(Object o) { if (this == o) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/GraphBackedRecursivePackageProvider.java b/src/main/java/com/google/devtools/build/lib/skyframe/GraphBackedRecursivePackageProvider.java index 05fd3fd9dc43bf..1776a293abab83 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/GraphBackedRecursivePackageProvider.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/GraphBackedRecursivePackageProvider.java @@ -26,6 +26,7 @@ import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.cmdline.TargetPattern; +import com.google.devtools.build.lib.cmdline.TargetPattern.TargetsBelowDirectory; import com.google.devtools.build.lib.concurrent.BatchCallback; import com.google.devtools.build.lib.concurrent.ParallelVisitor.UnusedException; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; @@ -42,8 +43,6 @@ import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; import com.google.devtools.build.skyframe.WalkableGraph; -import java.util.ArrayList; -import java.util.List; import java.util.Map; import java.util.Set; @@ -158,7 +157,7 @@ public boolean isPackage(ExtendedEventHandler eventHandler, PackageIdentifier pa return packageLookupValue.packageExists(); } - private List checkValidDirectoryAndGetRoots( + private ImmutableList checkValidDirectoryAndGetRoots( RepositoryName repository, PathFragment directory, ImmutableSet ignoredSubdirectories, @@ -171,9 +170,12 @@ private List checkValidDirectoryAndGetRoots( // Check that this package is covered by at least one of our universe patterns. boolean inUniverse = false; for (TargetPattern pattern : universeTargetPatterns) { - boolean isTBD = pattern.getType().equals(TargetPattern.Type.TARGETS_BELOW_DIRECTORY); + if (!pattern.getType().equals(TargetPattern.Type.TARGETS_BELOW_DIRECTORY)) { + continue; + } PackageIdentifier packageIdentifier = PackageIdentifier.create(repository, directory); - if (isTBD && pattern.containsAllTransitiveSubdirectoriesForTBD(packageIdentifier)) { + if (((TargetsBelowDirectory) pattern) + .containsAllTransitiveSubdirectories(packageIdentifier)) { inUniverse = true; break; } @@ -183,9 +185,8 @@ private List checkValidDirectoryAndGetRoots( return ImmutableList.of(); } - List roots = new ArrayList<>(); if (repository.isMain()) { - roots.addAll(pkgRoots); + return pkgRoots; } else { RepositoryDirectoryValue repositoryValue = (RepositoryDirectoryValue) graph.getValue(RepositoryDirectoryValue.key(repository)); @@ -194,9 +195,8 @@ private List checkValidDirectoryAndGetRoots( // "nothing". return ImmutableList.of(); } - roots.add(Root.fromPath(repositoryValue.getPath())); + return ImmutableList.of(Root.fromPath(repositoryValue.getPath())); } - return roots; } @Override @@ -208,7 +208,7 @@ public void streamPackagesUnderDirectory( ImmutableSet ignoredSubdirectories, ImmutableSet excludedSubdirectories) throws InterruptedException { - List roots = + ImmutableList roots = checkValidDirectoryAndGetRoots( repository, directory, ignoredSubdirectories, excludedSubdirectories); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternValue.java index 829dca39162305..0cb194c9ccbb74 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternValue.java @@ -24,7 +24,8 @@ import com.google.devtools.build.lib.cmdline.ResolvedTargets; import com.google.devtools.build.lib.cmdline.TargetParsingException; import com.google.devtools.build.lib.cmdline.TargetPattern; -import com.google.devtools.build.lib.cmdline.TargetPattern.ContainsTBDForTBDResult; +import com.google.devtools.build.lib.cmdline.TargetPattern.TargetsBelowDirectory; +import com.google.devtools.build.lib.cmdline.TargetPattern.TargetsBelowDirectory.ContainsResult; import com.google.devtools.build.lib.cmdline.TargetPattern.Type; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; @@ -229,21 +230,25 @@ private static TargetPatternKeyWithExclusionsResult computeTargetPatternKeyWithE continue; } if (laterParsedPattern.getType() == Type.TARGETS_BELOW_DIRECTORY) { - if (laterParsedPattern.containsTBDForTBD(targetPattern) - == ContainsTBDForTBDResult.DIRECTORY_EXCLUSION_WOULD_BE_EXACT) { - return new TargetPatternKeyWithExclusionsResult(Optional.empty(), ImmutableList.of()); - } else { - switch (targetPattern.containsTBDForTBD(laterParsedPattern)) { - case DIRECTORY_EXCLUSION_WOULD_BE_EXACT: - excludedDirectoriesBuilder.add( - laterParsedPattern.getDirectoryForTargetsUnderDirectory().getPackageFragment()); - break; - case DIRECTORY_EXCLUSION_WOULD_BE_TOO_BROAD: - indicesOfNegativePatternsThatNeedToBeIncludedBuilder.add(j); - break; - case OTHER: - default: - // Nothing to do with this pattern. + TargetsBelowDirectory laterParsedTargetsBelowDirectory = + (TargetsBelowDirectory) laterParsedPattern; + if (targetPattern.getType() == Type.TARGETS_BELOW_DIRECTORY) { + TargetsBelowDirectory targetsBelowDirectory = (TargetsBelowDirectory) targetPattern; + if (laterParsedTargetsBelowDirectory.contains(targetsBelowDirectory) + == ContainsResult.DIRECTORY_EXCLUSION_WOULD_BE_EXACT) { + return new TargetPatternKeyWithExclusionsResult(Optional.empty(), ImmutableList.of()); + } else { + switch (targetsBelowDirectory.contains(laterParsedTargetsBelowDirectory)) { + case DIRECTORY_EXCLUSION_WOULD_BE_EXACT: + excludedDirectoriesBuilder.add( + laterParsedTargetsBelowDirectory.getDirectory().getPackageFragment()); + break; + case DIRECTORY_EXCLUSION_WOULD_BE_TOO_BROAD: + indicesOfNegativePatternsThatNeedToBeIncludedBuilder.add(j); + break; + case NOT_CONTAINED: + // Nothing to do with this pattern. + } } } } else if (excludeSingleTargets && laterParsedPattern.getType() == Type.SINGLE_TARGET) { @@ -330,7 +335,7 @@ public ImmutableSet getExcludedSubdirectories() { } ImmutableSet getAllSubdirectoriesToExclude( - Iterable ignoredPackagePrefixes) throws InterruptedException { + ImmutableSet ignoredPackagePrefixes) throws InterruptedException { ImmutableSet.Builder excludedPathsBuilder = ImmutableSet.builder(); excludedPathsBuilder.addAll(getExcludedSubdirectories()); excludedPathsBuilder.addAll( @@ -339,25 +344,29 @@ ImmutableSet getAllSubdirectoriesToExclude( } public ImmutableSet getAllIgnoredSubdirectoriesToExclude( - InterruptibleSupplier> ignoredPackagePrefixes) + InterruptibleSupplier> ignoredPackagePrefixes) throws InterruptedException { return getAllIgnoredSubdirectoriesToExclude(parsedPattern, ignoredPackagePrefixes); } public static ImmutableSet getAllIgnoredSubdirectoriesToExclude( TargetPattern pattern, - InterruptibleSupplier> ignoredPackagePrefixes) + InterruptibleSupplier> ignoredPackagePrefixes) throws InterruptedException { - ImmutableSet.Builder ignoredPathsBuilder = ImmutableSet.builder(); - if (pattern.getType() == Type.TARGETS_BELOW_DIRECTORY) { - for (PathFragment ignoredPackagePrefix : ignoredPackagePrefixes.get()) { - PackageIdentifier pkgIdForIgnoredDirectorPrefix = - PackageIdentifier.create( - pattern.getDirectoryForTargetsUnderDirectory().getRepository(), - ignoredPackagePrefix); - if (pattern.containsAllTransitiveSubdirectoriesForTBD(pkgIdForIgnoredDirectorPrefix)) { - ignoredPathsBuilder.add(ignoredPackagePrefix); - } + if (pattern.getType() != Type.TARGETS_BELOW_DIRECTORY) { + return ImmutableSet.of(); + } + TargetsBelowDirectory targetsBelowDirectory = (TargetsBelowDirectory) pattern; + ImmutableSet ignoredPaths = ignoredPackagePrefixes.get(); + ImmutableSet.Builder ignoredPathsBuilder = + ImmutableSet.builderWithExpectedSize(0); + for (PathFragment ignoredPackagePrefix : ignoredPaths) { + PackageIdentifier pkgIdForIgnoredDirectorPrefix = + PackageIdentifier.create( + targetsBelowDirectory.getDirectory().getRepository(), ignoredPackagePrefix); + if (targetsBelowDirectory.containsAllTransitiveSubdirectories( + pkgIdForIgnoredDirectorPrefix)) { + ignoredPathsBuilder.add(ignoredPackagePrefix); } } return ignoredPathsBuilder.build(); diff --git a/src/test/java/com/google/devtools/build/lib/cmdline/TargetPatternTest.java b/src/test/java/com/google/devtools/build/lib/cmdline/TargetPatternTest.java index 0ff70ea1ce59a9..4b07663ff1d96d 100644 --- a/src/test/java/com/google/devtools/build/lib/cmdline/TargetPatternTest.java +++ b/src/test/java/com/google/devtools/build/lib/cmdline/TargetPatternTest.java @@ -18,7 +18,8 @@ import static org.junit.Assert.assertThrows; import com.google.common.collect.ImmutableMap; -import com.google.devtools.build.lib.cmdline.TargetPattern.ContainsTBDForTBDResult; +import com.google.devtools.build.lib.cmdline.TargetPattern.TargetsBelowDirectory; +import com.google.devtools.build.lib.cmdline.TargetPattern.TargetsBelowDirectory.ContainsResult; import java.util.Map; import org.junit.Test; import org.junit.runner.RunWith; @@ -89,162 +90,79 @@ public void testNormalize() { @Test public void testTargetsBelowDirectoryContainsColonStar() throws Exception { // Given an outer pattern '//foo/...', that matches rules only, - TargetPattern outerPattern = - parseAsExpectedType("//foo/...", TargetPattern.Type.TARGETS_BELOW_DIRECTORY); + TargetsBelowDirectory outerPattern = parseAsTBD("//foo/..."); // And a nested inner pattern '//foo/bar/...:*', that matches all targets, - TargetPattern innerPattern = - parseAsExpectedType("//foo/bar/...:*", TargetPattern.Type.TARGETS_BELOW_DIRECTORY); + TargetsBelowDirectory innerPattern = parseAsTBD("//foo/bar/...:*"); // Then a directory exclusion would exactly describe the subtraction of the inner pattern from // the outer pattern, - assertThat(outerPattern.containsTBDForTBD(innerPattern)) - .isEqualTo(ContainsTBDForTBDResult.DIRECTORY_EXCLUSION_WOULD_BE_EXACT); + assertThat(outerPattern.contains(innerPattern)) + .isEqualTo(ContainsResult.DIRECTORY_EXCLUSION_WOULD_BE_EXACT); // And the inner pattern does not contain the outer pattern. - assertThat(innerPattern.containsTBDForTBD(outerPattern)) - .isEqualTo(ContainsTBDForTBDResult.OTHER); + assertThat(innerPattern.contains(outerPattern)).isEqualTo(ContainsResult.NOT_CONTAINED); } @Test public void testTargetsBelowDirectoryColonStarContains() throws Exception { // Given an outer pattern '//foo/...:*', that matches all targets, - TargetPattern outerPattern = - parseAsExpectedType("//foo/...:*", TargetPattern.Type.TARGETS_BELOW_DIRECTORY); + TargetsBelowDirectory outerPattern = parseAsTBD("//foo/...:*"); // And a nested inner pattern '//foo/bar/...', that matches rules only, - TargetPattern innerPattern = - parseAsExpectedType("//foo/bar/...", TargetPattern.Type.TARGETS_BELOW_DIRECTORY); + TargetsBelowDirectory innerPattern = parseAsTBD("//foo/bar/..."); // Then a directory exclusion would be too broad, - assertThat(outerPattern.containsTBDForTBD(innerPattern)) - .isEqualTo(ContainsTBDForTBDResult.DIRECTORY_EXCLUSION_WOULD_BE_TOO_BROAD); + assertThat(outerPattern.contains(innerPattern)) + .isEqualTo(ContainsResult.DIRECTORY_EXCLUSION_WOULD_BE_TOO_BROAD); // And the inner pattern does not contain the outer pattern. - assertThat(innerPattern.containsTBDForTBD(outerPattern)) - .isEqualTo(ContainsTBDForTBDResult.OTHER); + assertThat(innerPattern.contains(outerPattern)).isEqualTo(ContainsResult.NOT_CONTAINED); } @Test public void testTargetsBelowDirectoryContainsNestedPatterns() throws Exception { // Given an outer pattern '//foo/...', - TargetPattern outerPattern = - parseAsExpectedType("//foo/...", TargetPattern.Type.TARGETS_BELOW_DIRECTORY); + TargetsBelowDirectory outerPattern = parseAsTBD("//foo/..."); // And a nested inner pattern '//foo/bar/...', - TargetPattern innerPattern = - parseAsExpectedType("//foo/bar/...", TargetPattern.Type.TARGETS_BELOW_DIRECTORY); + TargetsBelowDirectory innerPattern = parseAsTBD("//foo/bar/..."); // Then the outer pattern contains the inner pattern, - assertThat(outerPattern.containsTBDForTBD(innerPattern)) - .isEqualTo(ContainsTBDForTBDResult.DIRECTORY_EXCLUSION_WOULD_BE_EXACT); + assertThat(outerPattern.contains(innerPattern)) + .isEqualTo(ContainsResult.DIRECTORY_EXCLUSION_WOULD_BE_EXACT); // And the inner pattern does not contain the outer pattern. - assertThat(innerPattern.containsTBDForTBD(outerPattern)) - .isEqualTo(ContainsTBDForTBDResult.OTHER); + assertThat(innerPattern.contains(outerPattern)).isEqualTo(ContainsResult.NOT_CONTAINED); } @Test public void testTargetsBelowDirectoryIsExcludableFromForIndependentPatterns() throws Exception { // Given a pattern '//foo/...', - TargetPattern patternFoo = - parseAsExpectedType("//foo/...", TargetPattern.Type.TARGETS_BELOW_DIRECTORY); + TargetsBelowDirectory patternFoo = parseAsTBD("//foo/..."); // And a pattern '//bar/...', - TargetPattern patternBar = - parseAsExpectedType("//bar/...", TargetPattern.Type.TARGETS_BELOW_DIRECTORY); + TargetsBelowDirectory patternBar = parseAsTBD("//bar/..."); // Then neither pattern contains the other. - assertThat(patternFoo.containsTBDForTBD(patternBar)).isEqualTo(ContainsTBDForTBDResult.OTHER); - assertThat(patternBar.containsTBDForTBD(patternFoo)).isEqualTo(ContainsTBDForTBDResult.OTHER); - } - - @Test - public void testTargetsBelowDirectoryContainsForOtherPatternTypes() throws Exception { - // Given a TargetsBelowDirectory pattern, tbdFoo of '//foo/...', - TargetPattern tbdFoo = - parseAsExpectedType("//foo/...", TargetPattern.Type.TARGETS_BELOW_DIRECTORY); - - // And target patterns of each type other than TargetsBelowDirectory, e.g. 'foo/bar', - // '//foo:bar', and 'foo:all', - TargetPattern pathAsTargetPattern = - parseAsExpectedType("foo/bar", TargetPattern.Type.PATH_AS_TARGET); - TargetPattern singleTargetPattern = - parseAsExpectedType("//foo:bar", TargetPattern.Type.SINGLE_TARGET); - TargetPattern targetsInPackagePattern = - parseAsExpectedType("foo:all", TargetPattern.Type.TARGETS_IN_PACKAGE); - - // Then the non-TargetsBelowDirectory patterns do not contain tbdFoo. - assertThat(pathAsTargetPattern.containsTBDForTBD(tbdFoo)) - .isEqualTo(ContainsTBDForTBDResult.OTHER); - // And are not considered to be a contained directory of the TargetsBelowDirectory pattern. - assertThat(tbdFoo.containsTBDForTBD(pathAsTargetPattern)) - .isEqualTo(ContainsTBDForTBDResult.OTHER); - - assertThat(singleTargetPattern.containsTBDForTBD(tbdFoo)) - .isEqualTo(ContainsTBDForTBDResult.OTHER); - assertThat(tbdFoo.containsTBDForTBD(singleTargetPattern)) - .isEqualTo(ContainsTBDForTBDResult.OTHER); - - assertThat(targetsInPackagePattern.containsTBDForTBD(tbdFoo)) - .isEqualTo(ContainsTBDForTBDResult.OTHER); - assertThat(tbdFoo.containsTBDForTBD(targetsInPackagePattern)) - .isEqualTo(ContainsTBDForTBDResult.OTHER); + assertThat(patternFoo.contains(patternBar)).isEqualTo(ContainsResult.NOT_CONTAINED); + assertThat(patternBar.contains(patternFoo)).isEqualTo(ContainsResult.NOT_CONTAINED); } @Test public void testTargetsBelowDirectoryDoesNotContainCoincidentPrefixPatterns() throws Exception { // Given a TargetsBelowDirectory pattern, tbdFoo of '//foo/...', - TargetPattern tbdFoo = - parseAsExpectedType("//foo/...", TargetPattern.Type.TARGETS_BELOW_DIRECTORY); + TargetsBelowDirectory tbdFoo = parseAsTBD("//foo/..."); - // And target patterns with prefixes equal to the directory of the TBD pattern, but not below - // it, - TargetPattern targetsBelowDirectoryPattern = - parseAsExpectedType("//food/...", TargetPattern.Type.TARGETS_BELOW_DIRECTORY); - TargetPattern pathAsTargetPattern = - parseAsExpectedType("food/bar", TargetPattern.Type.PATH_AS_TARGET); - TargetPattern singleTargetPattern = - parseAsExpectedType("//food:bar", TargetPattern.Type.SINGLE_TARGET); - TargetPattern targetsInPackagePattern = - parseAsExpectedType("food:all", TargetPattern.Type.TARGETS_IN_PACKAGE); + // And a target pattern with prefix equal to the directory of the TBD pattern, but not below it, + TargetsBelowDirectory targetsBelowDirectoryPattern = parseAsTBD("//food/..."); - // Then the non-TargetsBelowDirectory patterns are not contained by tbdFoo. - assertThat(tbdFoo.containsTBDForTBD(targetsBelowDirectoryPattern)) - .isEqualTo(ContainsTBDForTBDResult.OTHER); - assertThat(tbdFoo.containsTBDForTBD(pathAsTargetPattern)) - .isEqualTo(ContainsTBDForTBDResult.OTHER); - assertThat(tbdFoo.containsTBDForTBD(singleTargetPattern)) - .isEqualTo(ContainsTBDForTBDResult.OTHER); - assertThat(tbdFoo.containsTBDForTBD(targetsInPackagePattern)) - .isEqualTo(ContainsTBDForTBDResult.OTHER); + // Then it is not contained in the first pattern. + assertThat(tbdFoo.contains(targetsBelowDirectoryPattern)) + .isEqualTo(ContainsResult.NOT_CONTAINED); } @Test public void testDepotRootTargetsBelowDirectoryContainsPatterns() throws Exception { // Given a TargetsBelowDirectory pattern, tbdDepot of '//...', - TargetPattern tbdDepot = - parseAsExpectedType("//...", TargetPattern.Type.TARGETS_BELOW_DIRECTORY); - - // And target patterns of each type other than TargetsBelowDirectory, e.g. 'foo/bar', - // '//foo:bar', and 'foo:all', - TargetPattern tbdFoo = - parseAsExpectedType("//foo/...", TargetPattern.Type.TARGETS_BELOW_DIRECTORY); - TargetPattern pathAsTargetPattern = - parseAsExpectedType("foo/bar", TargetPattern.Type.PATH_AS_TARGET); - TargetPattern singleTargetPattern = - parseAsExpectedType("//foo:bar", TargetPattern.Type.SINGLE_TARGET); - TargetPattern targetsInPackagePattern = - parseAsExpectedType("foo:all", TargetPattern.Type.TARGETS_IN_PACKAGE); - - // Then the patterns are contained by tbdDepot, and do not contain tbdDepot. - assertThat(tbdDepot.containsTBDForTBD(tbdFoo)) - .isEqualTo(ContainsTBDForTBDResult.DIRECTORY_EXCLUSION_WOULD_BE_EXACT); - assertThat(tbdFoo.containsTBDForTBD(tbdDepot)).isEqualTo(ContainsTBDForTBDResult.OTHER); + TargetsBelowDirectory tbdDepot = parseAsTBD("//..."); - assertThat(tbdDepot.containsTBDForTBD(pathAsTargetPattern)) - .isEqualTo(ContainsTBDForTBDResult.OTHER); - assertThat(pathAsTargetPattern.containsTBDForTBD(tbdDepot)) - .isEqualTo(ContainsTBDForTBDResult.OTHER); + // And a target pattern for a directory, + TargetsBelowDirectory tbdFoo = parseAsTBD("//foo/..."); - assertThat(tbdDepot.containsTBDForTBD(singleTargetPattern)). - isEqualTo(ContainsTBDForTBDResult.OTHER); - assertThat(singleTargetPattern.containsTBDForTBD(tbdDepot)) - .isEqualTo(ContainsTBDForTBDResult.OTHER); - - assertThat(tbdDepot.containsTBDForTBD(targetsInPackagePattern)) - .isEqualTo(ContainsTBDForTBDResult.OTHER); - assertThat(targetsInPackagePattern.containsTBDForTBD(tbdDepot)) - .isEqualTo(ContainsTBDForTBDResult.OTHER); + // Then the pattern is contained by tbdDepot, and does not contain tbdDepot. + assertThat(tbdDepot.contains(tbdFoo)) + .isEqualTo(ContainsResult.DIRECTORY_EXCLUSION_WOULD_BE_EXACT); + assertThat(tbdFoo.contains(tbdDepot)).isEqualTo(ContainsResult.NOT_CONTAINED); } @Test @@ -285,4 +203,9 @@ private static TargetPattern parseAsExpectedType(String pattern, TargetPattern.T assertThat(parsedPattern.getOriginalPattern()).isEqualTo(pattern); return parsedPattern; } + + private static TargetsBelowDirectory parseAsTBD(String pattern) throws TargetParsingException { + return (TargetsBelowDirectory) + parseAsExpectedType(pattern, TargetPattern.Type.TARGETS_BELOW_DIRECTORY); + } }