Better generics

- The compiler will no longer let you register a converter mismatch
- TODO No global map!

Author: garydgregory

src/main/java/org/apache/commons/cli/PatternOptionBuilder.java

@@ -104,7 +104,7 @@ public class PatternOptionBuilder {
     public static final Class<URL> URL_VALUE = URL.class;
 
     /** The converter to use for Unimplemented data types */
-    static final Converter<?, UnsupportedOperationException> NOT_IMPLEMENTED = s -> {
+    static final Converter<?, UnsupportedOperationException> UNSUPPORTED = s -> {
         throw new UnsupportedOperationException("Not yet implemented");
     };
 
@@ -221,6 +221,11 @@ public static Options parsePattern(final String pattern) {
      * @since 1.7.0
      */
     public static void registerTypes() {
-        TypeHandler.register(PatternOptionBuilder.FILES_VALUE, NOT_IMPLEMENTED);
+        TypeHandler.register(PatternOptionBuilder.FILES_VALUE, unsupported());
+    }
+
+    @SuppressWarnings("unchecked")
+    static <T> T unsupported() {
+        return (T) UNSUPPORTED;
     }
 }

src/main/java/org/apache/commons/cli/TypeHandler.java

@@ -235,13 +235,14 @@ public static FileInputStream openFile(final String string) throws ParseExceptio
     /**
      * Registers a Converter for a Class. If {@code converter} is null registration is cleared for {@code clazz}, and no converter will be used in processing.
      *
-     * @param clazz     the Class to register the Converter and Verifier to.
+     * @param <T>       The Class parameter type.
+     * @param clazz     the Class to register the Converter for.
      * @param converter The Converter to associate with Class. May be null.
      * @since 1.7.0
      */
-    public static void register(final Class<?> clazz, final Converter<?, ? extends Throwable> converter) {
+    public static <T> void register(final Class<T> clazz, final Converter<T, ? extends Throwable> converter) {
         if (converter == null) {
-            converterMap.remove(clazz);
+            unregister(clazz);
         } else {
             converterMap.put(clazz, converter);
         }
@@ -272,4 +273,14 @@ public static void resetConverters() {
         converterMap.put(BigInteger.class, BigInteger::new);
         converterMap.put(BigDecimal.class, BigDecimal::new);
     }
+
+    /**
+     * Unregisters a Converter for a Class. If {@code converter} is null registration is cleared for {@code clazz}, and no converter will be used in processing.
+     *
+     * @param clazz     the Class to unregister.
+     * @since 1.7.0
+     */
+    public static void unregister(final Class<?> clazz) {
+        converterMap.remove(clazz);
+    }
 }

src/test/java/org/apache/commons/cli/OptionTest.java

@@ -30,6 +30,9 @@
 import java.io.IOException;
 import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
+import java.nio.file.InvalidPathException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 
 import org.junit.jupiter.api.Test;
 
@@ -263,29 +266,33 @@ public void testHashCode() {
         assertNotEquals(Option.builder("test").build().hashCode(), Option.builder("test").longOpt("long test").build().hashCode());
     }
 
+    /** Always returns the same Path. */
+    private static final Converter<Path, InvalidPathException> PATH_CONVERTER = s -> Paths.get("foo");
 
     @Test
     public void testSerialization() throws IOException, ClassNotFoundException {
-        final Option o = Option.builder("o").type(TypeHandlerTest.Instantiable.class).build();
-        assertEquals(Converter.DEFAULT, o.getConverter());
-        Option o2 = roundTrip(o);
-        assertEquals(Converter.DEFAULT, o2.getConverter());
-
+        // TODO No global map!
+        TypeHandler.resetConverters();
+        final Option option = Option.builder("o").type(TypeHandlerTest.Instantiable.class).build();
+        assertEquals(Converter.DEFAULT, option.getConverter());
+        Option roundtrip = roundTrip(option);
+        assertEquals(Converter.DEFAULT, roundtrip.getConverter());
         // verify unregistered class converters and verifiers get reset to default.
-        o.setConverter(Converter.DATE);
-        o2 = roundTrip(o);
-        assertEquals(Converter.DEFAULT, o2.getConverter());
-
+        // converters are NOT Serializable, use a serialization proxy if you want that.
+        option.setConverter(Converter.DATE);
+        roundtrip = roundTrip(option);
+        assertEquals(Converter.DEFAULT, roundtrip.getConverter());
         // verify registered class converters and verifiers do not get reset to default.
+        // converters are NOT Serializable, use a serialization proxy if you want that.
         try {
-            TypeHandler.register(TypeHandlerTest.Instantiable.class, Converter.URL);
+            TypeHandler.register(Path.class, PATH_CONVERTER);
             // verify earlier values still set.
-            assertEquals(Converter.DATE, o.getConverter());
-            o2 = roundTrip(o);
-            // verify set to registered value
-            assertEquals(Converter.URL, o2.getConverter());
+            assertEquals(Converter.DATE, option.getConverter());
+            roundtrip = roundTrip(option);
+            assertEquals(Converter.DEFAULT, roundtrip.getConverter());
         } finally {
-            TypeHandler.register(TypeHandlerTest.Instantiable.class, null);
+            // TODO No global map!
+            TypeHandler.resetConverters();
         }
     }
 

src/test/java/org/apache/commons/cli/TypeHandlerTest.java

@@ -27,7 +27,9 @@ Licensed to the Apache Software Foundation (ASF) under one or more
 import java.math.BigInteger;
 import java.net.MalformedURLException;
 import java.net.URL;
+import java.nio.file.InvalidPathException;
 import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.text.DateFormat;
 import java.text.SimpleDateFormat;
 import java.util.ArrayList;
@@ -63,6 +65,9 @@ private NotInstantiable() {
 
     }
 
+    /** Always returns the same Path. */
+    private static final Converter<Path, InvalidPathException> PATH_CONVERTER = s -> Paths.get("foo");
+
     private static Stream<Arguments> createValueTestParameters() {
         // force the PatternOptionBuilder to load / modify the TypeHandler table.
         final Class<?> ignore = PatternOptionBuilder.FILES_VALUE;
@@ -200,27 +205,27 @@ public void testnstantiableEquals() {
 
     @Test
     public void testRegister() {
-        assertEquals(Converter.DEFAULT, TypeHandler.getConverter(NotInstantiable.class));
+        assertEquals(Converter.PATH, TypeHandler.getConverter(Path.class));
         try {
-            TypeHandler.register(NotInstantiable.class, Converter.DATE);
-            assertEquals(Converter.DATE, TypeHandler.getConverter(NotInstantiable.class));
+            TypeHandler.register(Path.class, PATH_CONVERTER);
+            assertEquals(PATH_CONVERTER, TypeHandler.getConverter(Path.class));
         } finally {
-            TypeHandler.register(NotInstantiable.class, null);
-            assertEquals(Converter.DEFAULT, TypeHandler.getConverter(NotInstantiable.class));
+            TypeHandler.unregister(Path.class);
+            assertEquals(Converter.DEFAULT, TypeHandler.getConverter(Path.class));
         }
     }
 
     @Test
     public void testResetConverters() {
-        assertEquals(Converter.DEFAULT, TypeHandler.getConverter(NotInstantiable.class));
+        assertEquals(Converter.PATH, TypeHandler.getConverter(Path.class));
         try {
-            TypeHandler.register(NotInstantiable.class, Converter.DATE);
-            assertEquals(Converter.DATE, TypeHandler.getConverter(NotInstantiable.class));
+            TypeHandler.register(Path.class, PATH_CONVERTER);
+            assertEquals(PATH_CONVERTER, TypeHandler.getConverter(Path.class));
             TypeHandler.resetConverters();
-            assertEquals(Converter.DEFAULT, TypeHandler.getConverter(NotInstantiable.class));
+            assertEquals(Converter.PATH, TypeHandler.getConverter(Path.class));
             assertEquals(Converter.DEFAULT, TypeHandler.getConverter(NotInstantiable.class));
         } finally {
-            TypeHandler.register(NotInstantiable.class, null);
+            TypeHandler.resetConverters();
         }
     }
 }