Rename some --incompatible_* flags to be more specific

This is possibly a nit, but we don't want to reuse flag names in the future, so it's a good idea to include what the flag *does* in the name as opposed to just the feature it affects, in case the same feature is changed multiple times. Updated javadoc for incompatible change system to say so.

This rename is ok because these flags have only been submitted over the past couple days.

RELNOTES: None
PiperOrigin-RevId: 155371363

5 files changed, 27 insertions, 20 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
index 3a32412f69..5140f572b1 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/AllIncompatibleChangesExpansion.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/AllIncompatibleChangesExpansion.java
@@ -66,11 +66,18 @@ import java.util.Map;
  * 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.
+ * <p>After the breaking change has been enabled by default, it is recommended (required?) that the
+ * flag stick around for a few releases, to provide users the flexibility to opt out. Even after
+ * enabling the behavior unconditionally, it can still be useful to keep the flag around as a valid
+ * no-op so that Bazel invocations are not immediately broken.
+ *
+ * <p>Generally speaking, we should never reuse names for multiple options. Therefore, when choosing
+ * a name for a new incompatible change, try to describe not just the affected feature, but what the
+ * change to that feature is. This avoids conflicts in case the feature changes multiple times. For
+ * example, {@code "--incompatible_depset_constructor"} is ambiguous because it only communicates
+ * that there is a change to how depsets are constructed, but {@code
+ * "--incompatible_disallow_set_constructor"} uniquely says that the {@code set} alias for the
+ * depset constructor is being disallowed.
  */
 // Javadoc can't resolve inner classes.
 @SuppressWarnings("javadoc")
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java b/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java
index 70f4b95e1f..88275c3e80 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java
@@ -126,13 +126,13 @@ public class BazelLibrary {
       new BuiltinFunction("set") {
         public SkylarkNestedSet invoke(Object items, String order, Location loc, Environment env)
             throws EvalException {
-          if (env.getSemantics().incompatibleDepsetConstructor) {
+          if (env.getSemantics().incompatibleDisallowSetConstructor) {
             throw new EvalException(
                 loc,
                 "The `set` constructor for depsets is deprecated and will be removed. Please use "
                     + "the `depset` constructor instead. You can temporarily enable the "
                     + "deprecated `set` constructor by passing the flag "
-                    + "--incompatible_depset_constructor=false");
+                    + "--incompatible_disallow_set_constructor=false");
           }
           try {
             return new SkylarkNestedSet(Order.parse(order), items, loc);
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java
index ef8b7f1c55..ea8280ac8a 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java
@@ -209,7 +209,7 @@ public final class BinaryOperatorExpression extends Expression {
     }
 
     if ((lval instanceof MutableList) && (rval instanceof MutableList)) {
-      if (isAugmented && env.getSemantics().incompatibleListPlusEquals) {
+      if (isAugmented && env.getSemantics().incompatibleListPlusEqualsInplace) {
         @SuppressWarnings("unchecked")
         MutableList<Object> list = (MutableList) lval;
         list.addAll((MutableList<?>) rval, location, env);
@@ -219,12 +219,12 @@ public final class BinaryOperatorExpression extends Expression {
     }
 
     if (lval instanceof SkylarkDict && rval instanceof SkylarkDict) {
-      if (env.getSemantics().incompatibleDictPlus) {
+      if (env.getSemantics().incompatibleDisallowDictPlus) {
         throw new EvalException(
             location,
             "The `+` operator for dicts is deprecated and no longer supported. Please use the "
                 + "`update` method instead. You can temporarily enable the `+` operator by passing "
-                + "the flag --incompatible_dict_plus=false");
+                + "the flag --incompatible_disallow_dict_plus=false");
       }
       return SkylarkDict.plus((SkylarkDict<?, ?>) lval, (SkylarkDict<?, ?>) rval, env);
     }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java
index cde876e58a..201d084f40 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java
@@ -51,12 +51,12 @@ public class FunctionDefStatement extends Statement {
     }
 
     FunctionSignature sig = signature.getSignature();
-    if (env.getSemantics().incompatibleKeywordOnlySyntax
+    if (env.getSemantics().incompatibleDisallowKeywordOnlyArgs
         && sig.getShape().getMandatoryNamedOnly() > 0) {
       throw new EvalException(
           getLocation(),
           "Keyword-only argument is forbidden. You can temporarily disable this "
-              + "error using the flag --incompatible_keyword_only_syntax=false");
+              + "error using the flag --incompatible_disallow_keyword_only_args=false");
     }
 
     env.update(
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemanticsOptions.java
index abd5611704..8653e83707 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemanticsOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemanticsOptions.java
@@ -44,36 +44,36 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
   public boolean skylarkFlagTestCanary;
 
   @Option(
-    name = "incompatible_depset_constructor",
+    name = "incompatible_disallow_set_constructor",
     defaultValue = "false",
     category = "incompatible changes",
     help = "If set to true, disables the deprecated `set` constructor for depsets."
   )
-  public boolean incompatibleDepsetConstructor;
+  public boolean incompatibleDisallowSetConstructor;
 
   @Option(
-    name = "incompatible_keyword_only_syntax",
+    name = "incompatible_disallow_keyword_only_args",
     defaultValue = "false",
     category = "incompatible changes",
     help = "If set to true, disables the keyword-only argument syntax in function definition."
   )
-  public boolean incompatibleKeywordOnlySyntax;
+  public boolean incompatibleDisallowKeywordOnlyArgs;
 
   @Option(
-    name = "incompatible_list_plus_equals",
+    name = "incompatible_list_plus_equals_inplace",
     defaultValue = "false",
     category = "incompatible changes",
     help =
         "If set to true, `+=` on lists works like the `extend` method mutating the original "
             + "list. Otherwise it copies the original list without mutating it."
   )
-  public boolean incompatibleListPlusEquals;
+  public boolean incompatibleListPlusEqualsInplace;
 
   @Option(
-    name = "incompatible_dict_plus",
+    name = "incompatible_disallow_dict_plus",
     defaultValue = "false",
     category = "incompatible changes",
     help = "If set to true, the `+` becomes disabled for dicts."
   )
-  public boolean incompatibleDictPlus;
+  public boolean incompatibleDisallowDictPlus;
 }