Add integration test for --all_incompatible_changes flag conflicts

This adds a new warning when the same flag is expanded to by multiple expansion flags. This extends an existing suite of warnings, e.g. for when an expansion flag conflicts with an explicit option.

The blaze canonicalize-flags command now takes a new flag --show_warnings. This flag causes any warnings encountered while parsing the given command line to be printed to stderr.

RELNOTES: blaze canonicalize-flags now takes a --show_warnings flag

PiperOrigin-RevId: 152186672

2 files changed, 67 insertions, 12 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/CanonicalizeCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/CanonicalizeCommand.java
index 77fb5e669a..ee43018010 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/CanonicalizeCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/CanonicalizeCommand.java
@@ -14,6 +14,7 @@
 package com.google.devtools.build.lib.runtime.commands;
 
 import com.google.common.base.Joiner;
+import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.flags.InvocationPolicyEnforcer;
 import com.google.devtools.build.lib.flags.InvocationPolicyParser;
@@ -28,7 +29,6 @@ import com.google.devtools.common.options.OptionsBase;
 import com.google.devtools.common.options.OptionsParser;
 import com.google.devtools.common.options.OptionsParsingException;
 import com.google.devtools.common.options.OptionsProvider;
-
 import java.util.Collection;
 import java.util.List;
 
@@ -46,16 +46,53 @@ import java.util.List;
 public final class CanonicalizeCommand implements BlazeCommand {
 
   public static class Options extends OptionsBase {
-    @Option(name = "for_command",
-            defaultValue = "build",
-            category = "misc",
-            help = "The command for which the options should be canonicalized.")
+    @Option(
+      name = "for_command",
+      defaultValue = "build",
+      category = "misc",
+      help = "The command for which the options should be canonicalized."
+    )
     public String forCommand;
 
-    @Option(name = "invocation_policy",
-        defaultValue = "",
-        help = "Applies an invocation policy to the options to be canonicalized.")
+    @Option(
+      name = "invocation_policy",
+      defaultValue = "",
+      help = "Applies an invocation policy to the options to be canonicalized."
+    )
     public String invocationPolicy;
+
+    @Option(
+      name = "show_warnings",
+      defaultValue = "false",
+      help = "Output parser warnings to standard error (e.g. for conflicting flag options)."
+    )
+    public boolean showWarnings;
+  }
+
+  /**
+   * These options are used by the incompatible_changes_conflict_test.sh integration test, which
+   * confirms that the warning for conflicting expansion options is working correctly. These flags
+   * are undocumented no-ops, and are not to be used by anything outside of that test.
+   */
+  public static class FlagClashCanaryOptions extends OptionsBase {
+    @Option(name = "flag_clash_canary", defaultValue = "false", category = "undocumented")
+    public boolean flagClashCanary;
+
+    @Option(
+      name = "flag_clash_canary_expander1",
+      defaultValue = "null",
+      category = "undocumented",
+      expansion = {"--flag_clash_canary=1"}
+    )
+    public Void flagClashCanaryExpander1;
+
+    @Option(
+      name = "flag_clash_canary_expander2",
+      defaultValue = "null",
+      category = "undocumented",
+      expansion = {"--flag_clash_canary=0"}
+    )
+    public Void flagClashCanaryExpander2;
   }
 
   @Override
@@ -70,10 +107,12 @@ public final class CanonicalizeCommand implements BlazeCommand {
       return ExitCode.COMMAND_LINE_ERROR;
     }
     Collection<Class<? extends OptionsBase>> optionsClasses =
-        BlazeCommandUtils.getOptions(
-            command.getClass(), runtime.getBlazeModules(), runtime.getRuleClassProvider());
+        ImmutableList.<Class<? extends OptionsBase>>builder()
+            .addAll(BlazeCommandUtils.getOptions(
+                command.getClass(), runtime.getBlazeModules(), runtime.getRuleClassProvider()))
+            .add(FlagClashCanaryOptions.class)
+            .build();
     try {
-      
       OptionsParser parser = OptionsParser.newOptionsParser(optionsClasses);
       parser.setAllowResidue(false);
       parser.parse(options.getResidue());
@@ -83,8 +122,13 @@ public final class CanonicalizeCommand implements BlazeCommand {
               InvocationPolicyParser.parsePolicy(canonicalizeOptions.invocationPolicy));
       invocationPolicyEnforcer.enforce(parser, commandName);
 
-      List<String> result = parser.canonicalize();
+      if (canonicalizeOptions.showWarnings) {
+        for (String warning : parser.getWarnings()) {
+          env.getReporter().handle(Event.warn(warning));
+        }
+      }
 
+      List<String> result = parser.canonicalize();
       for (String piece : result) {
         env.getReporter().getOutErr().printOutLn(piece);
       }
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 94702c93bb..fd3a8cea13 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
@@ -296,6 +296,15 @@ class OptionsParserImpl {
           // Create a warning if an expansion option overrides an explicit option:
           warnings.add("The option '" + expandedFrom + "' was expanded and now overrides a "
               + "previous explicitly specified option '" + name + "'");
+        } else if ((entry.getExpansionParent() != null) && (expandedFrom != null)) {
+          warnings.add(
+              "The option '"
+                  + name
+                  + "' was expanded to from both options '"
+                  + entry.getExpansionParent()
+                  + "' and '"
+                  + expandedFrom
+                  + "'");
         }
 
         // Record the new value:
@@ -633,6 +642,8 @@ class OptionsParserImpl {
       field = optionsData.getFieldFromName(name);
 
       // Look for a "no"-prefixed option name: "no<optionName>" or "no_<optionName>".
+      // Note: It is impossible to specify "--no_foo" for a flag named "--_foo", since that'll be
+      // interpreted as the "no_" negating prefix for "--foo".
       if (field == null && name.startsWith("no")) {
         name = name.substring(name.startsWith("no_") ? 3 : 2);
         field = optionsData.getFieldFromName(name);