Use Jcommander 2.0 (#2552)

core/src/main/java/google/registry/tools/CreateOrUpdateBulkPricingPackageCommand.java

@@ -56,7 +56,6 @@ abstract class CreateOrUpdateBulkPricingPackageCommand extends MutatingCommand {
   @Nullable
   @Parameter(
       names = "--next_billing_date",
-      converter = DateTimeParameter.class,
       validateWith = DateTimeParameter.class,
       description =
           "The next date that the bulk pricing package should be billed for its annual fee")

core/src/main/java/google/registry/tools/CreateOrUpdateDomainCommand.java

@@ -43,7 +43,7 @@ abstract class CreateOrUpdateDomainCommand extends MutatingEppToolCommand {
   @Parameter(
       names = {"-n", "--nameservers"},
       description = "Comma-delimited list of nameservers, up to 13.",
-      converter = NameserversParameter.class,
+      listConverter = NameserversParameter.class,
       validateWith = NameserversParameter.class)
   Set<String> nameservers = new HashSet<>();
 

core/src/main/java/google/registry/tools/GenerateEscrowDepositCommand.java

@@ -32,7 +32,6 @@ import google.registry.batch.CloudTasksUtils;
 import google.registry.model.rde.RdeMode;
 import google.registry.rde.RdeStagingAction;
 import google.registry.request.Action.Service;
-import google.registry.tools.params.DateTimeParameter;
 import java.util.List;
 import java.util.stream.Collectors;
 import javax.inject.Inject;
@@ -54,8 +53,7 @@ final class GenerateEscrowDepositCommand implements Command {
   @Parameter(
       names = {"-w", "--watermark"},
       description = "Point-in-time timestamp(s) for which time the deposit should be generated",
-      required = true,
-      converter = DateTimeParameter.class)
+      required = true)
   private List<DateTime> watermarks;
 
   @Parameter(
@@ -85,7 +83,12 @@ final class GenerateEscrowDepositCommand implements Command {
 
   @Override
   public void run() {
+    if (watermarks.isEmpty()) {
+      throw new ParameterException("At least one watermark must be specified");
+    }
 
+    // We need to test for cases where "--watermark=" is passed in as a parameter, because it would
+    // first be converted to an empty list, and as such the DateTime converter would not be called.
     if (tlds.isEmpty()) {
       throw new ParameterException("At least one TLD must be specified");
     }

core/src/main/java/google/registry/tools/SetupOteCommand.java

@@ -30,7 +30,6 @@ import google.registry.util.Clock;
 import google.registry.util.RegistryEnvironment;
 import google.registry.util.StringGenerator;
 import java.nio.file.Path;
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
 import javax.inject.Inject;
@@ -52,7 +51,7 @@ final class SetupOteCommand extends ConfirmingCommand {
       names = {"-a", "--ip_allow_list"},
       description = "Comma-separated list of IP addresses or CIDR ranges.",
       required = true)
-  private List<String> ipAllowList = new ArrayList<>();
+  private List<String> ipAllowList;
 
   @Parameter(
       names = {"--email"},

core/src/main/java/google/registry/tools/UniformRapidSuspensionCommand.java

@@ -73,7 +73,7 @@ final class UniformRapidSuspensionCommand extends MutatingEppToolCommand {
       description =
           "Comma-delimited set of fully qualified host names to replace the current hosts"
               + " on the domain.",
-      converter = NameserversParameter.class,
+      listConverter = NameserversParameter.class,
       validateWith = NameserversParameter.class)
   private Set<String> newHosts = new HashSet<>();
 

core/src/main/java/google/registry/tools/UpdateDomainCommand.java

@@ -64,7 +64,7 @@ final class UpdateDomainCommand extends CreateOrUpdateDomainCommand {
       description =
           "Comma-delimited list of nameservers to add, up to 13. "
               + "Cannot be set if --nameservers is set.",
-      converter = NameserversParameter.class,
+      listConverter = NameserversParameter.class,
       validateWith = NameserversParameter.class)
   private Set<String> addNameservers = new HashSet<>();
 
@@ -95,7 +95,7 @@ final class UpdateDomainCommand extends CreateOrUpdateDomainCommand {
       description =
           "Comma-delimited list of nameservers to remove, up to 13. "
               + "Cannot be set if --nameservers is set.",
-      converter = NameserversParameter.class,
+      listConverter = NameserversParameter.class,
       validateWith = NameserversParameter.class)
   private Set<String> removeNameservers = new HashSet<>();
 

core/src/test/java/google/registry/tools/GenerateEscrowDepositCommandTest.java

@@ -53,9 +53,9 @@ public class GenerateEscrowDepositCommandTest
 
   @Test
   void testCommand_emptyTld() {
-    IllegalArgumentException thrown =
+    ParameterException thrown =
         assertThrows(
-            IllegalArgumentException.class,
+            ParameterException.class,
             () ->
                 runCommand(
                     "--tld=",
@@ -63,7 +63,7 @@ public class GenerateEscrowDepositCommandTest
                     "--mode=thin",
                     "-r 42",
                     "-o test"));
-    assertThat(thrown).hasMessageThat().contains("Null or empty TLD specified");
+    assertThat(thrown).hasMessageThat().contains("At least one TLD must be specified");
   }
 
   @Test
@@ -94,11 +94,20 @@ public class GenerateEscrowDepositCommandTest
 
   @Test
   void testCommand_emptyWatermark() {
+    ParameterException thrown =
+        assertThrows(
+            ParameterException.class,
+            () -> runCommand("--tld=tld", "--watermark=", "--mode=full", "-r 42", "-o test"));
+    assertThat(thrown).hasMessageThat().contains("At least one watermark must be specified");
+  }
+
+  @Test
+  void testCommand_malformedWatermark() {
     IllegalArgumentException thrown =
         assertThrows(
             IllegalArgumentException.class,
-            () -> runCommand("--tld=tld", "--watermark=", "--mode=full", "-r 42", "-o test"));
-    assertThat(thrown).hasMessageThat().contains("Invalid format: \"\"");
+            () -> runCommand("--tld=tld", "--watermark=blah", "--mode=full", "-r 42", "-o test"));
+    assertThat(thrown).hasMessageThat().contains("Invalid format: \"blah\"");
   }
 
   @Test