Flip and delete the flag --incompatible_string_replace_count

Fixes #11244

RELNOTES:
  --incompatible_string_replace_count is flipped and removed (#11244)
PiperOrigin-RevId: 339046399

src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java

@@ -471,20 +471,6 @@ public class BuildLanguageOptions extends OptionsBase implements Serializable {
       help = "If set to true, the command parameter of actions.run_shell will only accept string")
   public boolean incompatibleRunShellCommandString;
 
-  @Option(
-      name = "incompatible_string_replace_count",
-      defaultValue = "false",
-      documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
-      effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
-      metadataTags = {
-        OptionMetadataTag.INCOMPATIBLE_CHANGE,
-        OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
-      },
-      help =
-          "If set to true, the `count` parameter of string.replace() is changed to behave as in "
-              + "Python: a negative count is ignored, and a None count is an error")
-  public boolean incompatibleStringReplaceCount;
-
   /** Used in an integration test to confirm that flags are visible to the interpreter. */
   @Option(
       name = "internal_starlark_flag_test_canary",
@@ -650,8 +636,6 @@ public StarlarkSemantics toStarlarkSemantics() {
             .setBool(INCOMPATIBLE_NO_IMPLICIT_FILE_EXPORT, incompatibleNoImplicitFileExport)
             .setBool(INCOMPATIBLE_NO_RULE_OUTPUTS_PARAM, incompatibleNoRuleOutputsParam)
             .setBool(INCOMPATIBLE_RUN_SHELL_COMMAND_STRING, incompatibleRunShellCommandString)
-            .setBool(
-                StarlarkSemantics.INCOMPATIBLE_STRING_REPLACE_COUNT, incompatibleStringReplaceCount)
             .setBool(INCOMPATIBLE_STRUCT_HAS_NO_METHODS, incompatibleStructHasNoMethods)
             .setBool(
                 INCOMPATIBLE_VISIBILITY_PRIVATE_ATTRIBUTES_AT_DEFINITION,

src/main/java/net/starlark/java/eval/StarlarkSemantics.java

@@ -205,10 +205,6 @@ public String toString() {
 
   // -- semantics options affecting the Starlark interpreter itself --
 
-  /** Ignore negative n in string.replace(count=n), and treat n=None as an error. */
-  public static final String INCOMPATIBLE_STRING_REPLACE_COUNT =
-      "-incompatible_string_replace_count";
-
   /** Change the behavior of 'print' statements. Used in tests to verify flag propagation. */
   public static final String PRINT_TEST_MARKER = "-print_test_marker";
 }

src/main/java/net/starlark/java/eval/StringModule.java

@@ -275,45 +275,18 @@ public String strip(String self, Object charsOrNone) {
         @Param(name = "new", doc = "The string to replace with."),
         @Param(
             name = "count",
-            allowedTypes = {
-              @ParamType(type = StarlarkInt.class),
-              @ParamType(
-                  type = NoneType.class), // TODO(#11244): remove once incompatible flag is deleted.
-            },
-            defaultValue = "unbound",
+            defaultValue = "-1",
             doc =
-                "The maximum number of replacements. If omitted, there is no limit."
-                    + "<p>If <code>--incompatible_string_replace_count</code> is true, a negative "
-                    + "value is ignored (so there's no limit) and a <code>None</code> value is an "
-                    + "error. Otherwise, a negative value is treated as 0 and a <code>None</code> "
-                    + "value is ignored. (See also issue <a "
-                    + "href='https://github.com/bazelbuild/bazel/issues/11244'>#11244</a>.)")
+                "The maximum number of replacements. If omitted, or if the value is negative, "
+                    + "there is no limit.")
       },
       useStarlarkThread = true)
   public String replace(
-      String self, String oldString, String newString, Object countUnchecked, StarlarkThread thread)
+      String self, String oldString, String newString, StarlarkInt countI, StarlarkThread thread)
       throws EvalException {
-    int count = Integer.MAX_VALUE;
-
-    StarlarkSemantics semantics = thread.getSemantics();
-    if (semantics.getBool(StarlarkSemantics.INCOMPATIBLE_STRING_REPLACE_COUNT)) {
-      if (countUnchecked == Starlark.NONE) {
-        throw Starlark.errorf(
-            "Cannot pass a None count to string.replace(); omit the count argument instead. (You "
-                + "can temporarily opt out of this change by setting "
-                + "--incompatible_string_replace_count=false.)");
-      }
-      if (countUnchecked != Starlark.UNBOUND) {
-        int x = Starlark.toInt(countUnchecked, "count");
-        if (x >= 0) {
-          count = x;
-        }
-      }
-    } else {
-      if (countUnchecked != Starlark.UNBOUND && countUnchecked != Starlark.NONE) {
-        // Negative has same effect as 0 below.
-        count = Starlark.toInt(countUnchecked, "count");
-      }
+    int count = countI.toInt("count");
+    if (count < 0) {
+      count = Integer.MAX_VALUE;
     }
 
     StringBuilder sb = new StringBuilder();

src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java

@@ -150,7 +150,6 @@ private static BuildLanguageOptions buildRandomOptions(Random rand) throws Excep
         "--incompatible_no_rule_outputs_param=" + rand.nextBoolean(),
         "--incompatible_objc_provider_remove_compile_info=" + rand.nextBoolean(),
         "--incompatible_run_shell_command_string=" + rand.nextBoolean(),
-        "--incompatible_string_replace_count=" + rand.nextBoolean(),
         "--incompatible_struct_has_no_methods=" + rand.nextBoolean(),
         "--incompatible_visibility_private_attributes_at_definition=" + rand.nextBoolean(),
         "--incompatible_require_linker_input_cc_api=" + rand.nextBoolean(),
@@ -207,7 +206,6 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) {
         .setBool(
             BuildLanguageOptions.INCOMPATIBLE_OBJC_PROVIDER_REMOVE_COMPILE_INFO, rand.nextBoolean())
         .setBool(BuildLanguageOptions.INCOMPATIBLE_RUN_SHELL_COMMAND_STRING, rand.nextBoolean())
-        .setBool(StarlarkSemantics.INCOMPATIBLE_STRING_REPLACE_COUNT, rand.nextBoolean())
         .setBool(BuildLanguageOptions.INCOMPATIBLE_STRUCT_HAS_NO_METHODS, rand.nextBoolean())
         .setBool(
             BuildLanguageOptions.INCOMPATIBLE_VISIBILITY_PRIVATE_ATTRIBUTES_AT_DEFINITION,

src/test/java/net/starlark/java/eval/StringModuleTest.java

@@ -18,16 +18,15 @@
 import org.junit.runners.JUnit4;
 
 /** Tests for StringModule. */
-// TODO(bazel-team): Migrate these test cases back to string_misc.sky, once either
-// 1) --incompatible_string_replace_count has been flipped (#11244) and deleted, or 2) the
-// standalone Starlark interpreter and tests gain the ability to run with semantics flags.
+// TODO(bazel-team): Migrate these test cases back to string_misc.sky.
 @RunWith(JUnit4.class)
 public class StringModuleTest {
 
   private final EvaluationTestCase ev = new EvaluationTestCase();
 
-  private void runReplaceTest(StarlarkSemantics semantics) throws Exception {
-    ev.new Scenario(semantics)
+  @Test
+  public void testReplace() throws Exception {
+    ev.new Scenario()
         .testEval("'banana'.replace('a', 'o')", "'bonono'")
         .testEval("'banana'.replace('a', 'o', 2)", "'bonona'")
         .testEval("'banana'.replace('a', 'o', 0)", "'banana'")
@@ -43,42 +42,16 @@ ev.new Scenario(semantics)
         .testEval("'banana'.replace('', '')", "'banana'")
         .testEval("'banana'.replace('a', '')", "'bnn'")
         .testEval("'banana'.replace('a', '', 2)", "'bnna'");
-  }
-
-  @Test
-  public void testReplaceWithAndWithoutFlag() throws Exception {
-    runReplaceTest(replaceCount(false));
-    runReplaceTest(replaceCount(true));
-  }
 
-  @Test
-  public void testReplaceIncompatibleFlag() throws Exception {
-    // Test the scenario that changes with the incompatible flag
-    ev.new Scenario(replaceCount(false))
-        .testEval("'banana'.replace('a', 'o', -2)", "'banana'")
-        .testEval("'banana'.replace('a', 'e', -1)", "'banana'")
-        .testEval("'banana'.replace('a', 'e', -10)", "'banana'")
-        .testEval("'banana'.replace('', '-', -2)", "'banana'");
-
-    ev.new Scenario(replaceCount(true))
+    ev.new Scenario()
         .testEval("'banana'.replace('a', 'o', -2)", "'bonono'")
         .testEval("'banana'.replace('a', 'e', -1)", "'benene'")
         .testEval("'banana'.replace('a', 'e', -10)", "'benene'")
         .testEval("'banana'.replace('', '-', -2)", "'-b-a-n-a-n-a-'");
-  }
-
-  @Test
-  public void testReplaceNoneCount() throws Exception {
-    // Passing None as the max number of replacements is disallowed with the incompatible flag.
-    ev.new Scenario(replaceCount(false)).testEval("'banana'.replace('a', 'e', None)", "'benene'");
-    ev.new Scenario(replaceCount(true))
-        .testIfErrorContains("Cannot pass a None count", "'banana'.replace('a', 'e', None)");
-  }
 
-  // Returns semantics for --incompatible_string_replace_count=x.
-  private static final StarlarkSemantics replaceCount(boolean x) {
-    return StarlarkSemantics.builder()
-        .setBool(StarlarkSemantics.INCOMPATIBLE_STRING_REPLACE_COUNT, x)
-        .build();
+    ev.new Scenario()
+        .testIfErrorContains(
+            "parameter 'count' got value of type 'NoneType', want 'int'",
+            "'banana'.replace('a', 'e', None)");
   }
 }