Removed verifier management and rebased to master

Claudenw · Claudenw · commit a8d7c8936eef · 2024-01-11T13:26:22.000+01:00
diff --git a/src/main/java/org/apache/commons/cli/CommandLine.java b/src/main/java/org/apache/commons/cli/CommandLine.java
@@ -398,7 +398,7 @@ public <T> T getParsedOptionValue(final String opt) throws ParseException {
      * @since 1.7.0
      * @param <T> The return type for the method.
      */
-    public <T> T getParsedOptionValue(final char opt, T defaultValue) throws ParseException {
+    public <T> T getParsedOptionValue(final char opt, final T defaultValue) throws ParseException {
         return getParsedOptionValue(String.valueOf(opt), defaultValue);
     }
 
@@ -414,7 +414,7 @@ public <T> T getParsedOptionValue(final char opt, T defaultValue) throws ParseEx
      * @param <T> The return type for the method.
      */
     @SuppressWarnings("unchecked")
-    public <T> T getParsedOptionValue(final Option option, T defaultValue) throws ParseException {
+    public <T> T getParsedOptionValue(final Option option, final T defaultValue) throws ParseException {
         if (option == null) {
             return null;
         }
@@ -438,7 +438,7 @@ public <T> T getParsedOptionValue(final Option option, T defaultValue) throws Pa
      * @since 1.7.0
      * @param <T> The return type for the method.
      */
-    public <T> T getParsedOptionValue(final String opt, T defaultValue) throws ParseException {
+    public <T> T getParsedOptionValue(final String opt, final T defaultValue) throws ParseException {
         return getParsedOptionValue(resolveOption(opt), defaultValue);
     }
 
diff --git a/src/main/java/org/apache/commons/cli/Option.java b/src/main/java/org/apache/commons/cli/Option.java
@@ -285,7 +285,7 @@ public Builder valueSeparator(final char valueSeparator) {
          * @return this builder, to allow method chaining.
          * @since 1.7.0
          */
-        public Builder converter(Converter<?> converter) {
+        public Builder converter(final Converter<?> converter) {
             this.converter = converter;
             return this;
         }
@@ -297,7 +297,7 @@ public Builder converter(Converter<?> converter) {
          * @return this builder, to allow method chaining.
          * @since 1.7.0
          */
-        public Builder verifier(Predicate<String> verifier) {
+        public Builder verifier(final Predicate<String> verifier) {
             this.verifier = verifier;
             return this;
         }
@@ -925,7 +925,7 @@ public Converter<?> getConverter() {
      * @param converter The converter to convert the string value to the type.
      * @since 1.7.0
      */
-    public void setConverter(Converter<?> converter) {
+    public void setConverter(final Converter<?> converter) {
         this.converter = converter;
     }
 
@@ -935,15 +935,15 @@ public void setConverter(Converter<?> converter) {
      * @since 1.7.0
      */
     public Predicate<String> getVerifier() {
-        return verifier == null ? TypeHandler.getVerifier(type) : verifier;
+        return verifier == null ? Verifier.DEFAULT : verifier;
     }
 
     /**
      * Sets the value verifier to verify that a string value is the proper form to be converted to a string.
      * @param verifier the Verifier to use.
      * @since 1.7.0
      */
-    public void setVerifier(Predicate<String> verifier) {
+    public void setVerifier(final Predicate<String> verifier) {
         this.verifier = verifier;
     }
 
diff --git a/src/main/java/org/apache/commons/cli/OptionBuilder.java b/src/main/java/org/apache/commons/cli/OptionBuilder.java
@@ -110,7 +110,7 @@ public static Option create(final String opt) throws IllegalArgumentException {
             option.setArgs(argCount);
             option.setType(type);
             option.setConverter(TypeHandler.getConverter(type));
-            option.setVerifier(TypeHandler.getVerifier(type));
+            option.setVerifier(Verifier.DEFAULT);
             option.setValueSeparator(valueSeparator);
             option.setArgName(argName);
         } finally {
diff --git a/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java b/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java
@@ -110,7 +110,14 @@ public class PatternOptionBuilder {
     };
     
     static {
-        TypeHandler.register(PatternOptionBuilder.FILES_VALUE, NOT_IMPLEMENTED, null);
+        registerTypes();
+    }
+
+    /**
+     * Registers custom {@code Converter}s with the {@code TypeHandler}. 
+     */
+    public static void registerTypes() {
+        TypeHandler.register(PatternOptionBuilder.FILES_VALUE, NOT_IMPLEMENTED);
     }
 
     /**
@@ -204,7 +211,6 @@ public static Options parsePattern(final String pattern) {
             } else {
                 type = getValueType(ch);
                 converter = TypeHandler.getConverter(getValueType(ch));
-                verifier = TypeHandler.getVerifier(getValueType(ch));
             }
         }
 
diff --git a/src/main/java/org/apache/commons/cli/TypeHandler.java b/src/main/java/org/apache/commons/cli/TypeHandler.java
@@ -25,7 +25,6 @@ Licensed to the Apache Software Foundation (ASF) under one or more
 import java.util.Date;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.function.Predicate;
 
 /**
  * TypeHandler will handle the pluggable conversion and verification of 
@@ -46,13 +45,9 @@ public class TypeHandler {
 
     /** Map of classes to converters. */
     private static Map<Class<?>, Converter<?>> converterMap = new HashMap<>();
-    
-    /** Map of classes to verifiers. */
-    private static Map<Class<?>, Predicate<String>> verifierMap = new HashMap<>();
 
     static {
         resetConverters();
-        resetVerifiers();
     }
     
     /**
@@ -93,56 +88,19 @@ public static void noConverters() {
     }
 
     /**
-     * Resets the registered Verifiers to the default state.
-     * @since 1.7.0
-     */
-    public static void resetVerifiers() {
-        verifierMap.clear();
-        verifierMap.put(Object.class, Verifier.CLASS);
-        verifierMap.put(Class.class, Verifier.CLASS);
-        verifierMap.put(Number.class, Verifier.NUMBER);
-
-        verifierMap.put(Long.class, Verifier.INTEGER);
-        verifierMap.put(Integer.class, Verifier.INTEGER);
-        verifierMap.put(Short.class, Verifier.INTEGER);
-        verifierMap.put(Byte.class, Verifier.INTEGER);
-
-        verifierMap.put(Double.class, Verifier.NUMBER);
-        verifierMap.put(Float.class, Verifier.NUMBER);
-        verifierMap.put(BigInteger.class, Verifier.INTEGER);
-        verifierMap.put(BigDecimal.class, Verifier.NUMBER);
-    }
-
-    /**
-     * Unregisters all Verifiers.
-     * @since 1.7.0
-     */
-    public static void noVerifiers() {
-        verifierMap.clear();
-    }
-
-    /**
-     * Registers a Converter and Verifier for a Class.  If @code converter} or 
-     * {@code verifier} are null their respective registrations are cleared for {@code clazz}, and 
-     * defaults will be used in processing.
+     * 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 converter The Converter to associate with Class.  May be null.
-     * @param verifier The Verifier to associate with Class.  May be null.
      * @since 1.7.0
      */
-    public static void register(Class<?> clazz, Converter<?> converter, Predicate<String> verifier) {
+    public static void register(final Class<?> clazz, final Converter<?> converter) {
         if (converter == null) {
             converterMap.remove(clazz);
         } else {
             converterMap.put(clazz, converter);
         }
-
-        if (verifier == null) {
-            verifierMap.remove(clazz);
-        } else {
-            verifierMap.put(clazz, verifier);
-        }
     }
 
     /**
@@ -151,22 +109,11 @@ public static void register(Class<?> clazz, Converter<?> converter, Predicate<St
      * @return the registered converter if any, {@link Converter#DEFAULT} otherwise.
      * @since 1.7.0
      */
-    public static Converter<?> getConverter(Class<?> clazz) {
+    public static Converter<?> getConverter(final Class<?> clazz) {
         Converter<?> converter = converterMap.get(clazz);
         return converter == null ? Converter.DEFAULT : converter;
     }
 
-    /**
-     * Gets the verifier for the Class. Never null.
-     * @param clazz the Class to get the Verifier for.
-     * @return the registered verifier if any, {@link Verifier#DEFAULT} otherwise.
-     * @since 1.7.0
-     */
-    public static Predicate<String> getVerifier(Class<?> clazz) {
-        Predicate<String> verifier = verifierMap.get(clazz);
-        return verifier == null ? Verifier.DEFAULT : verifier;
-    }
-
     /**
      * Returns the class whose name is {@code className}.
      *
diff --git a/src/main/java/org/apache/commons/cli/Verifier.java b/src/main/java/org/apache/commons/cli/Verifier.java
@@ -78,13 +78,13 @@ private  Verifier() {
      * @param exemplar One of the values from the accepted Enum.
      * @return A {@code Predicate<String>} that matches the Enum names.
      */
-    public static Predicate<String> enumVerifier(Enum<?> exemplar) {
+    public static Predicate<String> enumVerifier(final Enum<?> exemplar) {
         return new Predicate<String>() {
             /** The list of valid names */
             private final List<String> names = Arrays.stream(exemplar.getDeclaringClass().getEnumConstants())
                     .map(t -> ((Enum<?>) t).name()).collect(Collectors.toList());
             @Override
-            public boolean test(String str) throws RuntimeException {
+            public boolean test(final String str) throws RuntimeException {
                 return names.contains(str);
             }
         };
diff --git a/src/test/java/org/apache/commons/cli/ConverterTests.java b/src/test/java/org/apache/commons/cli/ConverterTests.java
@@ -38,7 +38,7 @@ public class ConverterTests {
 
     @ParameterizedTest
     @MethodSource("numberTestParameters")
-    public void numberTests(String str, Number expected) throws Exception {
+    public void numberTests(final String str, final Number expected) throws Exception {
         if (expected != null) {
             assertEquals(expected, Converter.NUMBER.apply(str));
         } else {
@@ -72,7 +72,7 @@ public void classTests() throws Exception {
         assertNotNull(Converter.CLASS.apply(this.getClass().getTypeName()), this.getClass().getTypeName());
 
         assertThrows(ClassNotFoundException.class, () -> Converter.CLASS.apply("foo.bar"));
-        assertNotNull(Converter.CLASS.apply(TestClass.class.getName()));
+        assertNotNull(Converter.CLASS.apply(AClassWithoutADefaultConstructor.class.getName()));
     }
 
     @Test
@@ -84,7 +84,7 @@ public void objectTests() throws Exception {
         assertNotNull(Converter.OBJECT.apply(this.getClass().getTypeName()), this.getClass().getTypeName());
 
         assertThrows(ClassNotFoundException.class, () -> Converter.OBJECT.apply("foo.bar"));
-        assertThrows(NoSuchMethodException.class, () -> Converter.OBJECT.apply(TestClass.class.getName()));
+        assertThrows(NoSuchMethodException.class, () -> Converter.OBJECT.apply(AClassWithoutADefaultConstructor.class.getName()));
     }
 
     @Test
@@ -111,8 +111,8 @@ public void fileTests() throws Exception {
     }
 
     // A class without a default constructor.
-    public class TestClass {
-        public TestClass(int i) {
+    public class AClassWithoutADefaultConstructor {
+        public AClassWithoutADefaultConstructor(final int i) {
         }
     }
 }
diff --git a/src/test/java/org/apache/commons/cli/OptionTest.java b/src/test/java/org/apache/commons/cli/OptionTest.java
@@ -239,7 +239,7 @@ public void testSubclass() {
     }
 
 
-    private Option roundTrip(Option o) throws IOException, ClassNotFoundException {
+    private Option roundTrip(final Option o) throws IOException, ClassNotFoundException {
         ByteArrayOutputStream baos = new ByteArrayOutputStream();
         ObjectOutputStream oos = new ObjectOutputStream(baos);
         oos.writeObject(o);
@@ -267,16 +267,16 @@ public void testSerialization() throws IOException, ClassNotFoundException {
 
         // verify registered class converters and verifiers do not get reset to default.
         try {
-            TypeHandler.register(TypeHandlerTest.Instantiable.class, Converter.URL, Verifier.CLASS);
+            TypeHandler.register(TypeHandlerTest.Instantiable.class, Converter.URL);
             // verify earlier values still set.
             assertEquals(Converter.DATE, o.getConverter());
             assertEquals(Verifier.NUMBER, o.getVerifier());
             o2 = roundTrip(o);
-            // verify set to registered values
+            // verify set to registered value
             assertEquals(Converter.URL, o2.getConverter());
-            assertEquals(Verifier.CLASS, o2.getVerifier());
+            assertEquals(Verifier.DEFAULT, o2.getVerifier());
         } finally {
-            TypeHandler.register(TypeHandlerTest.Instantiable.class, null, null);
+            TypeHandler.register(TypeHandlerTest.Instantiable.class, null);
         }
     }
 }
diff --git a/src/test/java/org/apache/commons/cli/PatternOptionBuilderTest.java b/src/test/java/org/apache/commons/cli/PatternOptionBuilderTest.java
@@ -23,7 +23,6 @@ Licensed to the Apache Software Foundation (ASF) under one or more
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
-import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import java.io.File;
 import java.io.FileInputStream;
@@ -32,13 +31,19 @@ Licensed to the Apache Software Foundation (ASF) under one or more
 import java.util.Date;
 import java.util.Vector;
 
+import org.junit.BeforeClass;
 import org.junit.Test;
 
 /**
  * Test case for the PatternOptionBuilder class.
  */
 @SuppressWarnings("deprecation") // tests some deprecated classes
 public class PatternOptionBuilderTest {
+    @BeforeClass
+    public static void setup() {
+        PatternOptionBuilder.registerTypes();
+    }
+
     @Test
     public void testClassPattern() throws Exception {
         final Options options = PatternOptionBuilder.parsePattern("c+d+");
@@ -81,16 +86,16 @@ public void testNumberPattern() throws Exception {
         final Options options = PatternOptionBuilder.parsePattern("n%d%x%");
         final CommandLineParser parser = new PosixParser();
         // 3,5 fails validation.
-        assertThrows(ParseException.class, () -> parser.parse(options, new String[] {"-n", "1", "-d", "2.1", "-x", "3,5"}));
+        //assertThrows(ParseException.class, () -> parser.parse(options, new String[] {"-n", "1", "-d", "2.1", "-x", "3,5"}));
 
-        CommandLine line = parser.parse(options, new String[] {"-n", "1", "-d", "2.1", "-x", "3.5"});
+        CommandLine line = parser.parse(options, new String[] {"-n", "1", "-d", "2.1", "-x", "3,5"});
         assertEquals("n object class", Long.class, line.getOptionObject("n").getClass());
         assertEquals("n value", Long.valueOf(1), line.getOptionObject("n"));
 
         assertEquals("d object class", Double.class, line.getOptionObject("d").getClass());
         assertEquals("d value", Double.valueOf(2.1), line.getOptionObject("d"));
 
-        assertEquals("x object", Double.valueOf(3.5), line.getOptionObject("x"));
+        assertNull("x object", line.getOptionObject("x"));
     }
 
     @Test
diff --git a/src/test/java/org/apache/commons/cli/TypeHandlerTest.java b/src/test/java/org/apache/commons/cli/TypeHandlerTest.java
diff --git a/src/test/java/org/apache/commons/cli/VerifierTests.java b/src/test/java/org/apache/commons/cli/VerifierTests.java

Original file line number	Diff line number	Diff line change
`@@ -38,7 +38,7 @@ public class ConverterTests {`
`38`	`38`
`39`	`39`	`@ParameterizedTest`
`40`	`40`	`@MethodSource("numberTestParameters")`
`41`		`- public void numberTests(String str, Number expected) throws Exception {`
	`41`	`+ public void numberTests(final String str, final Number expected) throws Exception {`
`42`	`42`	`if (expected != null) {`
`43`	`43`	`assertEquals(expected, Converter.NUMBER.apply(str));`
`44`	`44`	`} else {`
`@@ -72,7 +72,7 @@ public void classTests() throws Exception {`
`72`	`72`	`assertNotNull(Converter.CLASS.apply(this.getClass().getTypeName()), this.getClass().getTypeName());`
`73`	`73`
`74`	`74`	`assertThrows(ClassNotFoundException.class, () -> Converter.CLASS.apply("foo.bar"));`
`75`		`- assertNotNull(Converter.CLASS.apply(TestClass.class.getName()));`
	`75`	`+ assertNotNull(Converter.CLASS.apply(AClassWithoutADefaultConstructor.class.getName()));`
`76`	`76`	`}`
`77`	`77`
`78`	`78`	`@Test`
`@@ -84,7 +84,7 @@ public void objectTests() throws Exception {`
`84`	`84`	`assertNotNull(Converter.OBJECT.apply(this.getClass().getTypeName()), this.getClass().getTypeName());`
`85`	`85`
`86`	`86`	`assertThrows(ClassNotFoundException.class, () -> Converter.OBJECT.apply("foo.bar"));`
`87`		`- assertThrows(NoSuchMethodException.class, () -> Converter.OBJECT.apply(TestClass.class.getName()));`
	`87`	`+ assertThrows(NoSuchMethodException.class, () -> Converter.OBJECT.apply(AClassWithoutADefaultConstructor.class.getName()));`
`88`	`88`	`}`
`89`	`89`
`90`	`90`	`@Test`
`@@ -111,8 +111,8 @@ public void fileTests() throws Exception {`
`111`	`111`	`}`
`112`	`112`
`113`	`113`	`// A class without a default constructor.`
`114`		`- public class TestClass {`
`115`		`- public TestClass(int i) {`
	`114`	`+ public class AClassWithoutADefaultConstructor {`
	`115`	`+ public AClassWithoutADefaultConstructor(final int i) {`
`116`	`116`	`}`
`117`	`117`	`}`
`118`	`118`	`}`