Add the new Google TODO format to todo_replace.

This has been silently leaking googlers' usernames for years. :(

GWSQ_IGNORE: [email protected]
PiperOrigin-RevId: 848303274
Change-Id: I1440803927eddf4218d9ec7137e131b4067f7d19

Author: ssbr

java/com/google/copybara/transform/TodoReplace.java

@@ -95,9 +95,11 @@ public TodoReplace(
   }
 
   private Pattern createPattern(ImmutableList<String> todoTags) {
-    return Pattern.compile("((?:"
-        + Joiner.on("|").join(todoTags.stream().map(Pattern::quote).collect(Collectors.toList()))
-        + ") ?)\\((.*?)\\)");
+    return Pattern.compile(
+        "((?:"
+            + Joiner.on("|")
+                .join(todoTags.stream().map(Pattern::quote).collect(Collectors.toList()))
+            + ") ?)(?:\\((.*?)\\):|: (.*?) -)");
   }
 
   @Override
@@ -125,16 +127,25 @@ private Set<FileState> run(Iterable<FileState> files, Console console)
       StringBuffer sb = new StringBuffer();
       boolean modified = false;
       while (matcher.find()) {
-        if (matcher.group(2).trim().isEmpty()) {
-          matcher.appendReplacement(sb, matcher.group(0));
+        boolean newTodoFormat = matcher.group(3) != null;
+        String matchedUsers = newTodoFormat ? matcher.group(3) : matcher.group(2);
+        if (matchedUsers.trim().isEmpty()) {
+          matcher.appendReplacement(sb, matcher.group(0) + ":");
           continue;
         }
-        List<String> users = Splitter.on(",").splitToList(matcher.group(2));
+        List<String> users = Splitter.on(",").splitToList(matchedUsers);
         List<String> mappedUsers = mapUsers(users, matcher.group(0), file.getPath(), console);
         modified |= !users.equals(mappedUsers);
         String result = matcher.group(1);
         if (!mappedUsers.isEmpty()) {
-          result += "(" + Joiner.on(",").join(mappedUsers) + ")";
+          String todoContent = Joiner.on(",").join(mappedUsers);
+          if (newTodoFormat) {
+            result += ": " + todoContent + " -";
+          } else {
+            result += "(" + todoContent + "):";
+          }
+        } else {
+          result += ":";
         }
         matcher.appendReplacement(sb, Matcher.quoteReplacement(result));
       }

javatests/com/google/copybara/transform/TodoReplaceTest.java

@@ -116,6 +116,26 @@ public void testMapping() throws Exception {
         .containsNoMoreFiles();
   }
 
+  @Test
+  public void testNewTodoMapping() throws Exception {
+    TodoReplace replace = todoReplace("mapping = { 'aaa': 'foo', 'bbb' : 'bar'}");
+    write("one", "" + "aaa\n" + "// TODO: aaa, bbb,other - Example\n");
+    write("two", "// TODO: aaa - Other Example\n");
+    run(replace);
+
+    assertThatPath(checkoutDir)
+        .containsFile("one", "" + "aaa\n" + "// TODO: foo, bar,other - Example\n")
+        .containsFile("two", "// TODO: foo - Other Example\n")
+        .containsNoMoreFiles();
+
+    run(replace.reverse());
+
+    assertThatPath(checkoutDir)
+        .containsFile("one", "" + "aaa\n" + "// TODO: aaa, bbb,other - Example\n")
+        .containsFile("two", "// TODO: aaa - Other Example\n")
+        .containsNoMoreFiles();
+  }
+
   @Test
   public void testMultipleNotFound() throws Exception {
     TodoReplace replace = todoReplace("mapping = { 'test': 'foo'}");
@@ -146,7 +166,7 @@ public void testMapOrFail() throws Exception {
     ValidationException notFound = assertThrows(ValidationException.class, () -> run(replace));
     assertThat(notFound)
         .hasMessageThat()
-        .contains("Cannot find a mapping 'bbb' in 'TODO(bbb)' (/one.txt)");
+        .contains("Cannot find a mapping 'bbb' in 'TODO(bbb):' (/one.txt)");
     // Does not conform the pattern for users
     write("one.txt", "// TODO(aaa foo/1234): Example\n");
 
@@ -240,6 +260,11 @@ public void testScrub() throws Exception {
         .containsFile("one.txt", "// TODO: Example\n")
         .containsNoMoreFiles();
 
+    // and the new format
+    write("one.txt", "// TODO: aaa, bbb - Example\n");
+    run(replace);
+    assertThatPath(checkoutDir).containsFile("one.txt", "// TODO: Example\n").containsNoMoreFiles();
+
     // Does not conform the pattern, will be scrubbed anyway
     write("one.txt", "// TODO(aaa, bbb foo/1234): Example\n");
     run(replace);