This is an automated email from the ASF dual-hosted git repository.

ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-cli.git


The following commit(s) were added to refs/heads/master by this push:
     new 78b86ce  Better generics
78b86ce is described below

commit 78b86ce7f0a34b21b64c1858dae4a4e41967b740
Author: Gary Gregory <garydgreg...@gmail.com>
AuthorDate: Sun Apr 7 11:05:25 2024 -0400

    Better generics
    
    - The compiler will no longer let you register a converter mismatch
    - TODO No global map!
---
 .../apache/commons/cli/PatternOptionBuilder.java   |  9 ++++--
 .../java/org/apache/commons/cli/TypeHandler.java   | 17 ++++++++--
 .../java/org/apache/commons/cli/OptionTest.java    | 37 +++++++++++++---------
 .../org/apache/commons/cli/TypeHandlerTest.java    | 25 +++++++++------
 4 files changed, 58 insertions(+), 30 deletions(-)

diff --git a/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java 
b/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java
index 19abbd3..1c48dfc 100644
--- a/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java
+++ b/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 class PatternOptionBuilder {
      * @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;
     }
 }
diff --git a/src/main/java/org/apache/commons/cli/TypeHandler.java 
b/src/main/java/org/apache/commons/cli/TypeHandler.java
index a08c6ac..dfaed36 100644
--- a/src/main/java/org/apache/commons/cli/TypeHandler.java
+++ b/src/main/java/org/apache/commons/cli/TypeHandler.java
@@ -235,13 +235,14 @@ public class TypeHandler {
     /**
      * 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 class TypeHandler {
         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);
+    }
 }
diff --git a/src/test/java/org/apache/commons/cli/OptionTest.java 
b/src/test/java/org/apache/commons/cli/OptionTest.java
index 68ec905..eaaf3c6 100644
--- a/src/test/java/org/apache/commons/cli/OptionTest.java
+++ b/src/test/java/org/apache/commons/cli/OptionTest.java
@@ -30,6 +30,9 @@ import java.io.ByteArrayOutputStream;
 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 class OptionTest {
         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();
         }
     }
 
diff --git a/src/test/java/org/apache/commons/cli/TypeHandlerTest.java 
b/src/test/java/org/apache/commons/cli/TypeHandlerTest.java
index b82a5ca..487a8d2 100644
--- a/src/test/java/org/apache/commons/cli/TypeHandlerTest.java
+++ b/src/test/java/org/apache/commons/cli/TypeHandlerTest.java
@@ -27,7 +27,9 @@ import java.math.BigDecimal;
 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 @@ public class TypeHandlerTest {
 
     }
 
+    /** 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 class TypeHandlerTest {
 
     @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();
         }
     }
 }

Reply via email to