From 81b8589e4ac677fc45a3b6dbfc839b4ae3bbb857 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Sun, 30 Oct 2022 10:00:00 +0100 Subject: [PATCH] Improve error on invalid `-//foo` and `-@repo//foo` options `-//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. --- .../common/options/OptionsParserImpl.java | 10 ++++++++ .../common/options/OptionsParserTest.java | 24 +++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java index 79b3abbdcba889..daae9d8e887fac 100644 --- a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java +++ b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java @@ -476,6 +476,16 @@ 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 diff --git a/src/test/java/com/google/devtools/common/options/OptionsParserTest.java b/src/test/java/com/google/devtools/common/options/OptionsParserTest.java index 4bc56d1084881a..b18672d26141af 100644 --- a/src/test/java/com/google/devtools/common/options/OptionsParserTest.java +++ b/src/test/java/com/google/devtools/common/options/OptionsParserTest.java @@ -2369,6 +2369,30 @@ 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); }