Skip to content

Commit

Permalink
Improve error on invalid -//foo and -@repo//foo options (#18516)
Browse files Browse the repository at this point in the history
`-//foo` and `-@repo//foo` are always invalid Bazel options, but are usually meant to be either negative target patterns (which have to come after the `--` separator) or Starlark options (which always start with `--`).

With this commit, the error shown to the user mentions these two situations and how to fix the issue in each case.

Closes #16563.

PiperOrigin-RevId: 486920951
Change-Id: I9390d3859aa62c2b67eac05cb96a06209a9b7c36

Co-authored-by: Fabian Meumertzheim <[email protected]>
  • Loading branch information
iancha1992 and fmeum authored Jun 1, 2023
1 parent 4afed73 commit 28ebcdc
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,18 @@ private OptionsParserImplResult parse(
continue; // not an option arg
}

if (arg.startsWith("-//") || arg.startsWith("-@")) {
// Fail with a helpful error when an invalid option looks like an absolute negative target
// pattern or a typoed Starlark option.
throw new OptionsParsingException(
String.format(
"Invalid options syntax: %s\n"
+ "Note: Negative target patterns can only appear after the end of options"
+ " marker ('--'). Flags corresponding to Starlark-defined build settings"
+ " always start with '--', not '-'.",
arg));
}

arg = swapShorthandAlias(arg);

if (arg.equals("--")) { // "--" means all remaining args aren't options
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2369,6 +2369,35 @@ public void setOptionValueAtSpecificPriorityWithoutExpansion_expandedFlag_setsVa
.containsExactly("--second=hello");
}

@Test
public void negativeTargetPatternsInOptions_failsDistinctively() {
OptionsParser parser = OptionsParser.builder().optionsClasses(ExampleFoo.class).build();
OptionsParsingException e =
assertThrows(OptionsParsingException.class, () -> parser.parse("//foo", "-//bar", "//baz"));
assertThat(e).hasMessageThat().contains("-//bar");
assertThat(e)
.hasMessageThat()
.contains("Negative target patterns can only appear after the end of options marker");
assertThat(e)
.hasMessageThat()
.contains("Flags corresponding to Starlark-defined build settings always start with '--'");
}

@Test
public void negativeExternalTargetPatternsInOptions_failsDistinctively() {
OptionsParser parser = OptionsParser.builder().optionsClasses(ExampleFoo.class).build();
OptionsParsingException e =
assertThrows(
OptionsParsingException.class, () -> parser.parse("//foo", "-@repo//bar", "//baz"));
assertThat(e).hasMessageThat().contains("-@repo//bar");
assertThat(e)
.hasMessageThat()
.contains("Negative target patterns can only appear after the end of options marker");
assertThat(e)
.hasMessageThat()
.contains("Flags corresponding to Starlark-defined build settings always start with '--'");
}

private static OptionInstanceOrigin createInvocationPolicyOrigin() {
return createInvocationPolicyOrigin(/*implicitDependent=*/ null, /*expandedFrom=*/ null);
}
Expand Down

0 comments on commit 28ebcdc

Please sign in to comment.