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

src/main/java/com/google/devtools/build/lib/runtime/AllIncompatibleChangesExpansion.java

@@ -66,11 +66,18 @@
  * 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")

src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java

@@ -126,13 +126,13 @@ public SkylarkNestedSet invoke(Object items, String order, Location loc)
       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);

src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java

@@ -209,7 +209,7 @@ private static Object plus(
     }
 
     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 @@ private static Object plus(
     }
 
     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);
     }

src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java

@@ -51,12 +51,12 @@ void doExec(Environment env) throws EvalException, InterruptedException {
     }
 
     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(

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;
 }

src/test/java/com/google/devtools/build/lib/syntax/FunctionTest.java

@@ -337,13 +337,13 @@ public void testKwargs() throws Exception {
 
   @Test
   public void testKeywordOnlyIsForbidden() throws Exception {
-    env = newEnvironmentWithSkylarkOptions("--incompatible_keyword_only_syntax=true");
+    env = newEnvironmentWithSkylarkOptions("--incompatible_disallow_keyword_only_args=true");
     checkEvalErrorContains("forbidden", "def foo(a, b, *, c): return a + b + c");
   }
 
   @Test
   public void testParamAfterStarArgs() throws Exception {
-    env = newEnvironmentWithSkylarkOptions("--incompatible_keyword_only_syntax=true");
+    env = newEnvironmentWithSkylarkOptions("--incompatible_disallow_keyword_only_args=true");
     checkEvalErrorContains("forbidden", "def foo(a, *b, c): return a");
   }
 
@@ -428,7 +428,7 @@ public void testStarParam() throws Exception {
 
   @Test
   public void testIncompatibleStarParam() throws Exception {
-    env = newEnvironmentWithSkylarkOptions("--incompatible_keyword_only_syntax=true");
+    env = newEnvironmentWithSkylarkOptions("--incompatible_disallow_keyword_only_args=true");
     eval("def f(name, value = '1', optional = '2', *rest):",
         "  r = name + value + optional + '|'",
         "  for x in rest: r += x",

src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java

@@ -988,7 +988,7 @@ public void testDotExpressionOnNonStructObject() throws Exception {
 
   @Test
   public void testPlusEqualsOnListCopying() throws Exception {
-    new SkylarkTest("--incompatible_list_plus_equals=false")
+    new SkylarkTest("--incompatible_list_plus_equals_inplace=false")
         .setUp(
             "def func():",
             "  l1 = [1, 2]",
@@ -1001,7 +1001,7 @@ public void testPlusEqualsOnListCopying() throws Exception {
 
   @Test
   public void testPlusEqualsOnListMutating() throws Exception {
-    new SkylarkTest("--incompatible_list_plus_equals=true")
+    new SkylarkTest("--incompatible_list_plus_equals_inplace=true")
         .setUp(
             "def func():",
             "  l1 = [1, 2]",
@@ -1012,7 +1012,7 @@ public void testPlusEqualsOnListMutating() throws Exception {
         .testLookup("lists", "([1, 2, 3, 4], [1, 2, 3, 4])");
 
     // The same but with += after an IndexExpression
-    new SkylarkTest("--incompatible_list_plus_equals=true")
+    new SkylarkTest("--incompatible_list_plus_equals_inplace=true")
         .setUp(
             "def func():",
             "  l = [1, 2]",
@@ -1025,7 +1025,7 @@ public void testPlusEqualsOnListMutating() throws Exception {
 
   @Test
   public void testPlusEqualsOnTuple() throws Exception {
-    new SkylarkTest("--incompatible_list_plus_equals=false")
+    new SkylarkTest("--incompatible_list_plus_equals_inplace=false")
         .setUp(
             "def func():",
             "  t1 = (1, 2)",
@@ -1038,8 +1038,9 @@ public void testPlusEqualsOnTuple() throws Exception {
             SkylarkList.Tuple.of(1, 2, 3, 4)
         ));
 
-    // This behavior should remain the same regardless of the incompatible_list_plus_equals flag
-    new SkylarkTest("--incompatible_list_plus_equals=true")
+    // This behavior should remain the same regardless of the
+    // --incompatible_list_plus_equals_inplace flag
+    new SkylarkTest("--incompatible_list_plus_equals_inplace=true")
         .setUp(
             "def func():",
             "  t1 = (1, 2)",
@@ -1065,10 +1066,10 @@ public void testPlusEqualsOnDict() throws Exception {
 
   @Test
   public void testPlusOnDictDeprecated() throws Exception {
-    new SkylarkTest("--incompatible_dict_plus=true")
+    new SkylarkTest("--incompatible_disallow_dict_plus=true")
         .testIfErrorContains(
             "The `+` operator for dicts is deprecated and no longer supported.", "{1: 2} + {3: 4}");
-    new SkylarkTest("--incompatible_dict_plus=true")
+    new SkylarkTest("--incompatible_disallow_dict_plus=true")
         .testIfErrorContains(
             "The `+` operator for dicts is deprecated and no longer supported.",
             "def func():",

src/test/java/com/google/devtools/build/lib/syntax/SkylarkNestedSetTest.java

@@ -45,7 +45,7 @@ public void testLegacyConstructor() throws Exception {
 
   @Test
   public void testLegacyConstructorDeprecation() throws Exception {
-    env = newEnvironmentWithSkylarkOptions("--incompatible_depset_constructor=true");
+    env = newEnvironmentWithSkylarkOptions("--incompatible_disallow_set_constructor=true");
     try {
       eval("s = set([1, 2, 3], order='postorder')");
       Assert.fail("`set` should have failed");