Handle negative count arg in string.replace

Also update the name of the argument to be "count" instead of "maxsplit",
to match the Starlark spec.

This fixes bazelbuild#9181

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

@@ -276,23 +276,23 @@ public String strip(String self, Object charsOrNone) {
             type = String.class,
             doc = "The string to replace with."),
         @Param(
-            name = "maxsplit",
+            name = "count",
             type = Integer.class,
             noneable = true,
             defaultValue = "None",
             // TODO(cparsons): This parameter should be positional-only.
             legacyNamed = true,
             doc = "The maximum number of replacements.")
       })
-  public String replace(String self, String oldString, String newString, Object maxSplitO)
+  public String replace(String self, String oldString, String newString, Object count)
       throws EvalException {
-    int maxSplit = Integer.MAX_VALUE;
-    if (maxSplitO != Runtime.NONE) {
-      maxSplit = Math.max(0, (Integer) maxSplitO);
+    int maxReplaces = Integer.MAX_VALUE;
+    if (count != Runtime.NONE && (Integer) count > -1) {
+      maxReplaces = (Integer) count;
     }
     StringBuilder sb = new StringBuilder();
     int start = 0;
-    for (int i = 0; i < maxSplit; i++) {
+    for (int i = 0; i < maxReplaces; i++) {
       if (oldString.isEmpty()) {
         sb.append(newString);
         if (start < self.length()) {

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

@@ -675,7 +675,7 @@ public void testArgBothPosKey() throws Exception {
     newTest()
         .testIfErrorContains(
             "got multiple values for keyword argument 'old', for call to method "
-                + "replace(old, new, maxsplit = None) of 'string'",
+                + "replace(old, new, count = None) of 'string'",
             "'banana'.replace('a', 'o', 3, old='a')");
   }
 

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

@@ -0,0 +1,46 @@
+// Copyright 2014 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.syntax;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
+import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
+import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
+import com.google.devtools.build.lib.syntax.util.EvaluationTestCase;
+import java.util.ArrayList;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/**
+ * Tests for SkylarkStringModule.
+ */
+@RunWith(JUnit4.class)
+public class SkylarkStringModuleTest extends EvaluationTestCase {
+
+  @Test
+  public void testReplace() throws Exception {
+    // From spec
+    assertThat(eval("\"banana\".replace(\"a\", \"o\")")).isEqualTo("bonono");
+    assertThat(eval("\"banana\".replace(\"a\", \"o\", 2)")).isEqualTo("bonona");
+
+    assertThat(eval("\"banana\".replace(\"a\", \"o\", 0)")).isEqualTo("banana");
+
+    // Negative values of 'count' should be ignored
+    assertThat(eval("\"banana\".replace(\"a\", \"o\", -1)")).isEqualTo("bonono");
+  }
+}