Skip to content

Commit

Permalink
When we print an error message due to a missing target //p:t, helpf…
Browse files Browse the repository at this point in the history
…ully remind the user they can use `query //p:*` to see all the targets that exist in that package.

PiperOrigin-RevId: 470324753
Change-Id: I59a21a3400b74e63fb53de534085fe8345870009
  • Loading branch information
haxorz authored and copybara-github committed Aug 26, 2022
1 parent a9d655f commit 3ccf9a9
Show file tree
Hide file tree
Showing 8 changed files with 39 additions and 28 deletions.
15 changes: 12 additions & 3 deletions src/main/java/com/google/devtools/build/lib/packages/Package.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import static com.google.common.base.MoreObjects.firstNonNull;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
import com.google.common.collect.BiMap;
import com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -690,8 +691,8 @@ public Target getTarget(String targetName) throws NoSuchTargetException {
throw new NoSuchTargetException(
label,
String.format(
"target '%s' not declared in package '%s'%s defined by %s",
targetName, getName(), alternateTargetSuggestion, filename.asPath().getPathString()));
"target '%s' not declared in package '%s' defined by %s%s",
targetName, getName(), filename.asPath().getPathString(), alternateTargetSuggestion));
}
}

Expand Down Expand Up @@ -725,7 +726,15 @@ private String getAlternateTargetSuggestion(String targetName) {
+ getName()
+ "/BUILD?)";
} else {
return SpellChecker.didYouMean(targetName, targets.keySet());
String suggestedTarget = SpellChecker.suggest(targetName, targets.keySet());
String targetSuggestion =
suggestedTarget == null ? null : String.format("did you mean '%s'?", suggestedTarget);
String blazeQuerySuggestion =
String.format(
"Tip: use `query %s:*` to see all the targets in that package",
packageIdentifier.getCanonicalForm());
return String.format(
" (%s)", Joiner.on(" ").skipNulls().join(targetSuggestion, blazeQuerySuggestion));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -697,9 +697,10 @@ public void testHelpfulErrorForWrongPackageLabels() throws Exception {

AnalysisResult result = update(defaultFlags().with(Flag.KEEP_GOING), "//y:y");
assertThat(result.hasError()).isTrue();
assertContainsEvent("no such target '//x:z': "
+ "target 'z' not declared in package 'x' "
+ "defined by /workspace/x/BUILD and referenced by '//y:y'");
assertContainsEvent(
"no such target '//x:z': target 'z' not declared in package 'x' defined by"
+ " /workspace/x/BUILD (Tip: use `query //x:*` to see all the targets in that package)"
+ " and referenced by '//y:y'");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,9 @@ public void testFileThatsNotRegisteredYieldsUnknownTargetException() throws Exce
assertThat(e)
.hasMessageThat()
.isEqualTo(
"no such target '//pkg:baz.txt':"
+ " target 'baz.txt' not declared in package 'pkg' (did you mean 'bar.txt'?)"
+ " defined by /workspace/pkg/BUILD");
"no such target '//pkg:baz.txt': target 'baz.txt' not declared in package 'pkg' "
+ "defined by /workspace/pkg/BUILD (did you mean 'bar.txt'? Tip: use `query "
+ "//pkg:*` to see all the targets in that package)");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -283,8 +283,9 @@ public void testCreationOfInputFiles() throws Exception {
assertThat(e)
.hasMessageThat()
.isEqualTo(
"no such target '//foo:A': "
+ "target 'A' not declared in package 'foo' defined by /workspace/foo/BUILD");
"no such target '//foo:A': target 'A' not declared in package 'foo' defined by"
+ " /workspace/foo/BUILD (Tip: use `query //foo:*` to see all the targets in that"
+ " package)");

// These are the only input files: BUILD, Z
Set<String> inputFiles = Sets.newTreeSet();
Expand Down Expand Up @@ -412,27 +413,26 @@ public void testHelpfulErrorForMissingExportsFiles() throws Exception {
.hasMessageThat()
.isEqualTo(
"no such target '//x:y.cc': "
+ "target 'y.cc' not declared in package 'x'; "
+ "target 'y.cc' not declared in package 'x' "
+ "defined by /workspace/x/BUILD; "
+ "however, a source file of this name exists. "
+ "(Perhaps add 'exports_files([\"y.cc\"])' to x/BUILD?) "
+ "defined by /workspace/x/BUILD");
+ "(Perhaps add 'exports_files([\"y.cc\"])' to x/BUILD?)");

e = assertThrows(NoSuchTargetException.class, () -> pkg.getTarget("z.cc"));
assertThat(e)
.hasMessageThat()
.isEqualTo(
"no such target '//x:z.cc': "
+ "target 'z.cc' not declared in package 'x' (did you mean 'x.cc'?) "
+ "defined by /workspace/x/BUILD");
"no such target '//x:z.cc': target 'z.cc' not declared in package 'x' defined by"
+ " /workspace/x/BUILD (did you mean 'x.cc'? Tip: use `query //x:*` to see all the"
+ " targets in that package)");

e = assertThrows(NoSuchTargetException.class, () -> pkg.getTarget("dir"));
assertThat(e)
.hasMessageThat()
.isEqualTo(
"no such target '//x:dir': target 'dir' not declared in package 'x'; "
+ "however, a source directory of this name exists. "
+ "(Perhaps add 'exports_files([\"dir\"])' to x/BUILD, "
+ "or define a filegroup?) defined by /workspace/x/BUILD");
"no such target '//x:dir': target 'dir' not declared in package 'x' defined by"
+ " /workspace/x/BUILD; however, a source directory of this name exists. (Perhaps"
+ " add 'exports_files([\"dir\"])' to x/BUILD, or define a filegroup?)");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,8 @@ public void testCompileOneDepOnMissingFile() throws Exception {
.hasMessageThat()
.isEqualTo(
"no such target '//foo:missing.cc': target 'missing.cc' not declared in package "
+ "'foo' defined by /workspace/foo/BUILD");
+ "'foo' defined by /workspace/foo/BUILD (Tip: use `query //foo:*` to see all the "
+ "targets in that package)");

// Also, try a valid input file which has no dependent rules in its package.
e = assertThrows(TargetParsingException.class, () -> parseCompileOneDep("//foo:baz/bang"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,8 @@ public void testGetNonexistentTarget() throws Exception {
.hasMessageThat()
.isEqualTo(
"no such target '//pkg1:not-there': target 'not-there' "
+ "not declared in package 'pkg1' defined by /workspace/pkg1/BUILD");
+ "not declared in package 'pkg1' defined by /workspace/pkg1/BUILD (Tip: use "
+ "`query //pkg1:*` to see all the targets in that package)");
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,10 +299,9 @@ public void testHelpfulMessageForDirectoryWhichIsASubdirectoryOfAPackage() throw
scratch.file("bar/BUILD");
scratch.file("bar/quux/somefile");
expectError(
"no such target '//bar:quux': target 'quux' not declared in package 'bar'; "
+ "however, a source directory of this name exists. (Perhaps add "
+ "'exports_files([\"quux\"])' to bar/BUILD, or define a filegroup?) defined by "
+ "/workspace/bar/BUILD",
"no such target '//bar:quux': target 'quux' not declared in package 'bar' defined by "
+ "/workspace/bar/BUILD; however, a source directory of this name exists. (Perhaps "
+ "add 'exports_files([\"quux\"])' to bar/BUILD, or define a filegroup?)",
"bar/quux");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ protected final void checkResultOfTargetLiteralWithMissingTargets(
"no such target '//a:b': target 'b' not declared in package 'a' "
+ "defined by "
+ helper.getRootDirectory().getPathString()
+ "/a/BUILD");
+ "/a/BUILD (Tip: use `query //a:*` to see all the targets in that package)");
assertThat(failureDetail.getPackageLoading().getCode())
.isEqualTo(FailureDetails.PackageLoading.Code.TARGET_MISSING);
}
Expand Down

0 comments on commit 3ccf9a9

Please sign in to comment.