From 840231bf1aebceb301b4885b3167674f4515cb90 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Tue, 8 Nov 2022 05:14:57 -0800 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. Closes #16563. PiperOrigin-RevId: 486920951 Change-Id: I9390d3859aa62c2b67eac05cb96a06209a9b7c36 --- .../common/options/OptionsParserImpl.java | 12 ++++++++ .../common/options/OptionsParserTest.java | 29 +++++++++++++++++++ 2 files changed, 41 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..4bebaaf90eb35e 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,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 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..b6b63d9a6ece15 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,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); }