Add --all_incompatible_changes, the user's shorthand for turning on all --incompatible_* flags

Note that if a developer adds a poorly-formatted incompatible change @Option, constructing an OptionsParser will now fail with an unchecked exception. This can cause some unit tests to fail in unexpected ways, but the developer should see an appropriate error message when the server starts up.

To be added: A separate integration test that ensures the expansions of --all_incompatible_changes don't clobber each other.

RELNOTES: None

PiperOrigin-RevId: 151858287

4 files changed, 186 insertions, 7 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/runtime/AllIncompatibleChangesExpansion.java b/src/main/java/com/google/devtools/build/lib/runtime/AllIncompatibleChangesExpansion.java
new file mode 100644
index 0000000000..3a32412f69
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/runtime/AllIncompatibleChangesExpansion.java
@@ -0,0 +1,161 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.runtime;
+
+import com.google.common.collect.Iterables;
+import com.google.devtools.common.options.Converter;
+import com.google.devtools.common.options.ExpansionFunction;
+import com.google.devtools.common.options.IsolatedOptionsData;
+import com.google.devtools.common.options.Option;
+import com.google.devtools.common.options.OptionsBase;
+import com.google.devtools.common.options.OptionsParser;
+import java.lang.reflect.Field;
+import java.util.ArrayList;
+import java.util.Map;
+
+/**
+ * Expansion function for {@code --all_incompatible_changes}. Expands to all options of form {@code
+ * --incompatible_*} that are declared in the {@link OptionsBase} subclasses that are passed to the
+ * parser.
+ *
+ * <p>The incompatible changes system provides users with a uniform way of opting into backwards-
+ * incompatible changes, in order to test whether their builds will be broken by an upcoming
+ * release. When adding a new breaking change to Bazel, prefer to use this mechanism for guarding
+ * the behavior.
+ *
+ * <p>An {@link Option}-annotated field that is considered an incompatible change must satisfy the
+ * following requirements.
+ *
+ * <ul>
+ *   <li>the {@link Option#name} must be prefixed with "incompatible_"
+ *   <li>the {@link Option#category} must be "incompatible changes"
+ *   <li>the {@link Option#help} field must be set, and must refer the user to information about
+ *       what the change does and how to migrate their code
+ *   <li>the following fields may not be used: {@link Option#abbrev}, {@link Option#valueHelp},
+ *       {@link Option#converter}, {@link Option#allowMultiple}, {@link Option#oldName}, and {@link
+ *       Option#wrapperOption}
+ * </ul>
+ *
+ * Example:
+ *
+ * <pre>{@code
+ * @Option(
+ *   name = "incompatible_foo",
+ *   category = "incompatible changes",
+ *   defaultValue = "false",
+ *   help = "Deprecates bar and changes the semantics of baz. To migrate your code see [...].")
+ * public boolean incompatibleFoo;
+ * }</pre>
+ *
+ * All options that satisfy either the name or category requirement will be validated using the
+ * above criteria. Any failure will cause {@link IllegalArgumentException} to be thrown, which will
+ * cause the construction of the {@link OptionsParser} to fail with the <i>unchecked</i> exception
+ * {@link OptionsParser.ConstructionException}. Therefore, when adding a new incompatible change, be
+ * aware that an error in the specification of the {@code @Option} will exercise failure code paths
+ * in the early part of the Bazel server execution.
+ *
+ * <p>After the breaking change has been enabled unconditionally, it is recommended (required?) that
+ * its corresponding incompatible change option be left as a valid no-op option, rather than
+ * removed. This helps avoid breaking invocations of Bazel upon upgrading to a new release. Just as
+ * for other options, names of incompatible change options must never be reused for a different
+ * option.
+ */
+// Javadoc can't resolve inner classes.
+@SuppressWarnings("javadoc")
+public class AllIncompatibleChangesExpansion implements ExpansionFunction {
+
+  // The reserved prefix for all incompatible change option names.
+  public static final String INCOMPATIBLE_NAME_PREFIX = "incompatible_";
+  // The reserved category for all incompatible change options.
+  public static final String INCOMPATIBLE_CATEGORY = "incompatible changes";
+
+  /**
+   * Ensures that the given option satisfies all the requirements on incompatible change options
+   * enumerated above.
+   *
+   * <p>If any of these requirements are not satisfied, {@link IllegalArgumentException} is thrown,
+   * as this constitutes an internal error in the declaration of the option.
+   */
+  private static void validateIncompatibleChange(Field field, Option annotation) {
+    String prefix = "Incompatible change option '--" + annotation.name() + "' ";
+
+    // To avoid ambiguity, and the suggestion of using .isEmpty().
+    String defaultString = "";
+
+    // Validate that disallowed fields aren't used. These will need updating if the default values
+    // in Option ever change, and perhaps if new fields are added.
+    if (annotation.abbrev() != '\0') {
+      throw new IllegalArgumentException(prefix + "must not use the abbrev field");
+    }
+    if (!annotation.valueHelp().equals(defaultString)) {
+      throw new IllegalArgumentException(prefix + "must not use the valueHelp field");
+    }
+    if (annotation.converter() != Converter.class) {
+      throw new IllegalArgumentException(prefix + "must not use the converter field");
+    }
+    if (annotation.allowMultiple()) {
+      throw new IllegalArgumentException(prefix + "must not use the allowMultiple field");
+    }
+    if (annotation.implicitRequirements().length > 0) {
+      throw new IllegalArgumentException(prefix + "must not use the implicitRequirements field");
+    }
+    if (!annotation.oldName().equals(defaultString)) {
+      throw new IllegalArgumentException(prefix + "must not use the oldName field");
+    }
+    if (annotation.wrapperOption()) {
+      throw new IllegalArgumentException(prefix + "must not use the wrapperOption field");
+    }
+
+    // Validate the fields that are actually allowed.
+    if (!annotation.name().startsWith(INCOMPATIBLE_NAME_PREFIX)) {
+      throw new IllegalArgumentException(prefix + "must have name starting with \"incompatible_\"");
+    }
+    if (!annotation.category().equals(INCOMPATIBLE_CATEGORY)) {
+      throw new IllegalArgumentException(prefix + "must have category \"incompatible changes\"");
+    }
+    if (!IsolatedOptionsData.isExpansionOption(annotation)) {
+      if (!field.getType().equals(Boolean.TYPE)) {
+        throw new IllegalArgumentException(
+            prefix + "must have boolean type (unless it's an expansion option)");
+      }
+    }
+    if (annotation.help().equals(defaultString)) {
+      throw new IllegalArgumentException(
+          prefix
+              + "must have a \"help\" string that refers the user to "
+              + "information about this change and how to migrate their code");
+    }
+  }
+
+  @Override
+  public String[] getExpansion(IsolatedOptionsData optionsData) {
+    // Grab all registered options that are identified as incompatible changes by either name or
+    // by category. Ensure they satisfy our requirements.
+    ArrayList<String> incompatibleChanges = new ArrayList<>();
+    for (Map.Entry<String, Field> entry : optionsData.getAllNamedFields()) {
+      Field field = entry.getValue();
+      Option annotation = field.getAnnotation(Option.class);
+      if (annotation.name().startsWith(INCOMPATIBLE_NAME_PREFIX)
+          || annotation.category().equals(INCOMPATIBLE_CATEGORY)) {
+        validateIncompatibleChange(field, annotation);
+        incompatibleChanges.add("--" + annotation.name());
+      }
+    }
+    // Sort to get a deterministic canonical order. This probably isn't necessary because the
+    // options parser will do its own sorting when canonicalizing, but it seems like it can't hurt.
+    incompatibleChanges.sort(null);
+    return Iterables.toArray(incompatibleChanges, String.class);
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java b/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java
index 2afb774217..8e6cc14b31 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java
@@ -97,6 +97,18 @@ public class CommonCommandOptions extends OptionsBase {
   }
 
 
+  // To create a new incompatible change, see the javadoc for AllIncompatibleChangesExpansion.
+  @Option(
+    name = "all_incompatible_changes",
+    defaultValue = "null",
+    category = "misc",
+    expansionFunction = AllIncompatibleChangesExpansion.class,
+    help =
+        "Enables all options of the form --incompatible_*. Use this option to find places where "
+            + "your build may break in the future due to deprecations or other changes."
+  )
+  public Void allIncompatibleChanges;
+
   @Option(name = "config",
           defaultValue = "",
           category = "misc",
diff --git a/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java b/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java
index 60a32a907f..5074b99bf5 100644
--- a/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java
+++ b/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java
@@ -45,7 +45,7 @@ import javax.annotation.concurrent.Immutable;
 // accepting Object values, and the List in allOptionsField should be ImmutableList. Either fix
 // this or remove @Immutable.
 @Immutable
-class IsolatedOptionsData extends OpaqueOptionsData {
+public class IsolatedOptionsData extends OpaqueOptionsData {
 
   /**
    * These are the options-declaring classes which are annotated with {@link Option} annotations.
@@ -181,6 +181,11 @@ class IsolatedOptionsData extends OpaqueOptionsData {
     return field.getType().equals(Void.class);
   }
 
+  /** Returns whether the arg is an expansion option. */
+  public static boolean isExpansionOption(Option annotation) {
+    return (annotation.expansion().length > 0 || OptionsData.usesExpansionFunction(annotation));
+  }
+
   /**
    * Returns whether the arg is an expansion option defined by an expansion function (and not a
    * constant expansion value).
diff --git a/src/main/java/com/google/devtools/common/options/OptionsParser.java b/src/main/java/com/google/devtools/common/options/OptionsParser.java
index 1e475e572c..62f9ee778c 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParser.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParser.java
@@ -66,7 +66,10 @@ public class OptionsParser implements OptionsProvider {
    * An unchecked exception thrown when there is a problem constructing a parser, e.g. an error
    * while validating an {@link Option} field in one of its {@link OptionsBase} subclasses.
    *
-   * <p>Although unchecked, we explicitly mark some methods as throwing it as a reminder in the API.
+   * <p>This exception is unchecked because it generally indicates an internal error affecting all
+   * invocations of the program. I.e., any such error should be immediately obvious to the
+   * developer. Although unchecked, we explicitly mark some methods as throwing it as a reminder in
+   * the API.
    */
   public static class ConstructionException extends RuntimeException {
     public ConstructionException(String message) {
@@ -187,7 +190,7 @@ public class OptionsParser implements OptionsProvider {
   public void setAllowSingleDashLongOptions(boolean allowSingleDashLongOptions) {
     this.impl.setAllowSingleDashLongOptions(allowSingleDashLongOptions);
   }
-  
+
   /** Enables the Parser to handle params files loacted insinde the provided {@link FileSystem}. */
   public void enableParamsFileSupport(FileSystem fs) {
     this.impl.setArgsPreProcessor(new ParamsFilePreProcessor(fs));
@@ -273,7 +276,7 @@ public class OptionsParser implements OptionsProvider {
       return expansions;
     }
   }
-  
+
   /**
    * The name and value of an option with additional metadata describing its
    * priority, source, whether it was set via an implicit dependency, and if so,
@@ -450,9 +453,7 @@ public class OptionsParser implements OptionsProvider {
     }
 
     boolean isExpansion() {
-      Option option = field.getAnnotation(Option.class);
-      return (option.expansion().length > 0
-          || OptionsData.usesExpansionFunction(option));
+      return OptionsData.isExpansionOption(field.getAnnotation(Option.class));
     }
 
     boolean isImplicitRequirement() {