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(); } } }