From a7426a45d568ada60dbe9294e093dfa26bc9c5ae Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Fri, 29 Dec 2023 14:02:23 +0100 Subject: [PATCH 01/24] Fixes for CLI-321 Adds BeanUtils dependency to POM and implements TypeHandler using the BeanUtils classes. Handles all methods that were in the original TypeHandler as well as other default classes provided by BeanUtils. Test updated to show proper parsing of types that previously were not implemented. Includes new cli.converters package that may be better handled by BeanUtils as it moves forward to support FunctionalInterfaces. --- pom.xml | 5 + .../apache/commons/cli/ParseException.java | 4 + .../org/apache/commons/cli/TypeHandler.java | 247 ++++++++++-------- .../apache/commons/cli/converters/Func.java | 28 ++ .../cli/converters/SimpleConverter.java | 51 ++++ .../apache/commons/cli/TypeHandlerTest.java | 17 +- 6 files changed, 246 insertions(+), 106 deletions(-) create mode 100644 src/main/java/org/apache/commons/cli/converters/Func.java create mode 100644 src/main/java/org/apache/commons/cli/converters/SimpleConverter.java diff --git a/pom.xml b/pom.xml index 411f7fce9..eee1c9507 100644 --- a/pom.xml +++ b/pom.xml @@ -192,6 +192,11 @@ junit-vintage-engine test + + commons-beanutils + commons-beanutils + 1.9.4 + diff --git a/src/main/java/org/apache/commons/cli/ParseException.java b/src/main/java/org/apache/commons/cli/ParseException.java index 37bab9811..9386a7664 100644 --- a/src/main/java/org/apache/commons/cli/ParseException.java +++ b/src/main/java/org/apache/commons/cli/ParseException.java @@ -34,4 +34,8 @@ public class ParseException extends Exception { public ParseException(final String message) { super(message); } + + public ParseException(final Exception e) { + super(e); + } } diff --git a/src/main/java/org/apache/commons/cli/TypeHandler.java b/src/main/java/org/apache/commons/cli/TypeHandler.java index 25648f447..6a7535d68 100644 --- a/src/main/java/org/apache/commons/cli/TypeHandler.java +++ b/src/main/java/org/apache/commons/cli/TypeHandler.java @@ -19,63 +19,120 @@ Licensed to the Apache Software Foundation (ASF) under one or more import java.io.File; import java.io.FileInputStream; -import java.io.FileNotFoundException; -import java.net.MalformedURLException; import java.net.URL; import java.util.Date; +import org.apache.commons.beanutils.ConversionException; +import org.apache.commons.beanutils.ConvertUtilsBean; +import org.apache.commons.beanutils.Converter; +import org.apache.commons.beanutils.converters.ConverterFacade; +import org.apache.commons.beanutils.converters.DateConverter; +import org.apache.commons.cli.converters.Func; +import org.apache.commons.cli.converters.SimpleConverter; + /** - * This is a temporary implementation. TypeHandler will handle the pluggableness of OptionTypes and it will direct all - * of these types of conversion functionalities to ConvertUtils component in Commons already. BeanUtils I think. + * TypeHandler will handles the configuration of the Option type processing using + * the Commons BeanUtils ConvertUtilsBean class. */ public class TypeHandler { + + private static ConvertUtilsBean convertUtils; + + static { + convertUtils = new ConvertUtilsBean(); + convertUtils.register(true, true, 0); + /* + * this can not be a ternary because the unboxing operations will result in it + * always being a Double. + * https://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.25-300-A + */ + Func fn = new Func() { + + @Override + public Number apply(String str) throws ConversionException { + if (str.indexOf('.') == -1) { + return Long.valueOf(str); + } + return Double.valueOf(str); + } + }; + convertUtils.register(new SimpleConverter<>(fn, Number.class), Number.class); + + Func fo = (str) -> { + final Class cl; + + try { + cl = Class.forName(str); + } catch (final ClassNotFoundException cnfe) { + throw new ConversionException("Unable to find the class: " + str); + } + + try { + return cl.getConstructor().newInstance(); + } catch (final Exception e) { + throw new ConversionException(e.getClass().getName() + "; Unable to create an instance of: " + str); + } + }; + convertUtils.register(new SimpleConverter<>(fo, Object.class), Object.class); + + convertUtils.register( + new SimpleConverter<>(str -> new FileInputStream(str), FileInputStream.class), + FileInputStream.class); + + // fixup date parsing + DateConverter dc = new DateConverter(); + dc.setUseLocaleFormat(true); + convertUtils.register(new ConverterFacade(dc), Date.class); + } + /** * Returns the class whose name is {@code className}. * - * @param className the class name - * @return The class if it is found - * @throws ParseException if the class could not be found + * @param className the class name + * @return The class if it is found + * @throws ParseException if the class could not be found + * @deprecated use createValue(className,Class.class) */ + @Deprecated // (since="1.7") public static Class createClass(final String className) throws ParseException { - try { - return Class.forName(className); - } catch (final ClassNotFoundException e) { - throw new ParseException("Unable to find the class: " + className); - } + return createValue(className, Class.class); } /** - * Returns the date represented by {@code str}. - *

- * This method is not yet implemented and always throws an {@link UnsupportedOperationException}. + * Returns the date represented by {@code str}.

This method is not yet + * implemented and always throws an {@link UnsupportedOperationException}. * - * @param str the date string - * @return The date if {@code str} is a valid date string, otherwise return null. - * @throws UnsupportedOperationException always + * @param str the date string + * @return The date if {@code str} is a valid + * date string, otherwise return null. + * @throws UnsupportedOperationException always + * @deprecated use createValue(str, Date.class); */ + @Deprecated // (since="1.7") public static Date createDate(final String str) { - throw new UnsupportedOperationException("Not yet implemented"); + return createValueNoException(str, Date.class); } /** * Returns the File represented by {@code str}. * - * @param str the File location - * @return The file represented by {@code str}. + * @param str the File location + * @return The file represented by {@code str}. + * @deprecated use createValue(str, File.class); */ + @Deprecated // (since="1.7") public static File createFile(final String str) { - return new File(str); + return createValueNoException(str, File.class); } /** - * Returns the File[] represented by {@code str}. - *

- * This method is not yet implemented and always throws an {@link UnsupportedOperationException}. + * Returns the File[] represented by {@code str}.

This method is not yet + * implemented and always throws an {@link UnsupportedOperationException}. * - * @param str the paths to the files - * @return The File[] represented by {@code str}. - * @throws UnsupportedOperationException always + * @param str the paths to the files + * @return The File[] represented by {@code str}. */ + @Deprecated // (since="1.7") public static File[] createFiles(final String str) { // to implement/port: // return FileW.findFiles(str); @@ -83,110 +140,94 @@ public static File[] createFiles(final String str) { } /** - * Create a number from a String. If a '.' is present, it creates a Double, otherwise a Long. + * Create a number from a String. If a '.' is present, it creates a Double, + * otherwise a Long. * - * @param str the value - * @return the number represented by {@code str} - * @throws ParseException if {@code str} is not a number + * @param str the value + * @return the number represented by {@code str} + * @throws ParseException if {@code str} is not a number + * @deprecated use createValue(str, Number.class); */ + @Deprecated // (since="1.7") public static Number createNumber(final String str) throws ParseException { - try { - if (str.indexOf('.') != -1) { - return Double.valueOf(str); - } - return Long.valueOf(str); - } catch (final NumberFormatException e) { - throw new ParseException(e.getMessage()); - } + return createValue(str, Number.class); } /** * Create an Object from the class name and empty constructor. * - * @param className the argument value - * @return the initialized object - * @throws ParseException if the class could not be found or the object could not be created + * @param className the argument value + * @return the initialized object + * @throws ParseException if the class could not be found or the object + * could not be created + * @deprecated use createValue(str, Object.class); */ + @Deprecated // (since="1.7") public static Object createObject(final String className) throws ParseException { - final Class cl; - - try { - cl = Class.forName(className); - } catch (final ClassNotFoundException cnfe) { - throw new ParseException("Unable to find the class: " + className); - } - - try { - return cl.getConstructor().newInstance(); - } catch (final Exception e) { - throw new ParseException(e.getClass().getName() + "; Unable to create an instance of: " + className); - } + return createValue(className, Object.class); } /** * Returns the URL represented by {@code str}. * - * @param str the URL string - * @return The URL in {@code str} is well-formed - * @throws ParseException if the URL in {@code str} is not well-formed + * @param str the URL string + * @return The URL in {@code str} is well-formed + * @throws ParseException if the URL in {@code str} is not well-formed + * @deprecated use createValue(str, URL.class); */ + @Deprecated // (since="1.7") public static URL createURL(final String str) throws ParseException { + return createValue(str, URL.class); + } + + private static T createValueNoException(final String str, Class clazz) { try { - return new URL(str); - } catch (final MalformedURLException e) { - throw new ParseException("Unable to parse the URL: " + str); + return createValue(str, clazz); + } catch (ParseException e) { + throw new RuntimeException(e); } } /** - * Returns the {@code Object} of type {@code clazz} with the value of {@code str}. + * Returns the {@code Object} of type {@code clazz} with the value of + * {@code str}. * - * @param str the command line value - * @param clazz the class representing the type of argument - * @param type of argument - * @return The instance of {@code clazz} initialized with the value of {@code str}. + * @param str the command line value + * @param clazz the class representing the type of argument + * @param type of argument + * @return The instance of {@code clazz} initialized with the + * value of {@code str}. * @throws ParseException if the value creation for the given class failed */ @SuppressWarnings("unchecked") // returned value will have type T because it is fixed by clazz - public static T createValue(final String str, final Class clazz) throws ParseException { - if (PatternOptionBuilder.STRING_VALUE == clazz) { - return (T) str; - } - if (PatternOptionBuilder.OBJECT_VALUE == clazz) { - return (T) createObject(str); - } - if (PatternOptionBuilder.NUMBER_VALUE == clazz) { - return (T) createNumber(str); - } - if (PatternOptionBuilder.DATE_VALUE == clazz) { - return (T) createDate(str); - } - if (PatternOptionBuilder.CLASS_VALUE == clazz) { - return (T) createClass(str); - } - if (PatternOptionBuilder.FILE_VALUE == clazz) { - return (T) createFile(str); - } - if (PatternOptionBuilder.EXISTING_FILE_VALUE == clazz) { - return (T) openFile(str); - } + public static T createValue(final String str, Class clazz) throws ParseException { + // BeanUtils convertUtils handles File[] but looks for the string to parse into file names. if (PatternOptionBuilder.FILES_VALUE == clazz) { return (T) createFiles(str); } - if (PatternOptionBuilder.URL_VALUE == clazz) { - return (T) createURL(str); + Converter converter = convertUtils.lookup(String.class, clazz); + if (converter == null) { + throw new ParseException(String.format("No registered converter for %s found", clazz)); + } + try { + return converter.convert(clazz, str); + } catch (ConversionException e) { + throw new ParseException(e); } - throw new ParseException("Unable to handle the class: " + clazz); } /** * Returns the {@code Object} of type {@code obj} with the value of {@code str}. * - * @param str the command line value - * @param obj the type of argument - * @return The instance of {@code obj} initialized with the value of {@code str}. - * @throws ParseException if the value creation for the given object type failed + * @param str the command line value + * @param obj the type of argument + * @return The instance of {@code obj} initialized with the + * value of {@code str}. + * @throws ParseException if the value creation for the given object type + * failed + * @deprecated use createValue(str, Class); */ + @Deprecated // (since="1.7") public static Object createValue(final String str, final Object obj) throws ParseException { return createValue(str, (Class) obj); } @@ -194,15 +235,13 @@ public static Object createValue(final String str, final Object obj) throws Pars /** * Returns the opened FileInputStream represented by {@code str}. * - * @param str the file location - * @return The file input stream represented by {@code str}. - * @throws ParseException if the file is not exist or not readable + * @param str the file location + * @return The file input stream represented by {@code str}. + * @throws ParseException if the file is not exist or not readable + * @deprecated use createValue(str, URL.class); */ + @Deprecated // (since="1.7") public static FileInputStream openFile(final String str) throws ParseException { - try { - return new FileInputStream(str); - } catch (final FileNotFoundException e) { - throw new ParseException("Unable to find file: " + str); - } + return createValue(str, FileInputStream.class); } } diff --git a/src/main/java/org/apache/commons/cli/converters/Func.java b/src/main/java/org/apache/commons/cli/converters/Func.java new file mode 100644 index 000000000..84928a7c2 --- /dev/null +++ b/src/main/java/org/apache/commons/cli/converters/Func.java @@ -0,0 +1,28 @@ +/* + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ +package org.apache.commons.cli.converters; + +/** + * The definition of the functional interface to call when doing a conversion. + * Like Function but can throw an Exception. + * + * @param The return type for the function. + */ +@FunctionalInterface +public interface Func { + T apply(String str) throws Exception; +} \ No newline at end of file diff --git a/src/main/java/org/apache/commons/cli/converters/SimpleConverter.java b/src/main/java/org/apache/commons/cli/converters/SimpleConverter.java new file mode 100644 index 000000000..fc30483d0 --- /dev/null +++ b/src/main/java/org/apache/commons/cli/converters/SimpleConverter.java @@ -0,0 +1,51 @@ +/* + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ +package org.apache.commons.cli.converters; + +import org.apache.commons.beanutils.converters.AbstractConverter; + +/** + * A simple converter that takes a Func and the datatype to create a Converter + * for the BeanUtils. + * + * @param The type of object the converter will return. + */ +public class SimpleConverter extends AbstractConverter { + + private final Func func; + private final Class type; + + public SimpleConverter(Func func, Class type) { + this.func = func; + this.type = type; + } + + @SuppressWarnings("unchecked") + @Override + protected T2 convertToType(Class type, Object value) throws Throwable { + if (this.type.equals(type)) { + return (T2) func.apply(value.toString()); + } + + throw conversionException(type, value); + } + + @Override + protected Class getDefaultType() { + return type; + } +} diff --git a/src/test/java/org/apache/commons/cli/TypeHandlerTest.java b/src/test/java/org/apache/commons/cli/TypeHandlerTest.java index c288622e8..dbf61ac50 100644 --- a/src/test/java/org/apache/commons/cli/TypeHandlerTest.java +++ b/src/test/java/org/apache/commons/cli/TypeHandlerTest.java @@ -25,6 +25,8 @@ Licensed to the Apache Software Foundation (ASF) under one or more import java.io.File; import java.io.FileInputStream; import java.net.URL; +import java.text.DateFormat; +import java.util.Date; import org.junit.Test; @@ -51,11 +53,22 @@ public void testCreateValueClass_notFound() { } @Test - public void testCreateValueDate() { - assertThrows(UnsupportedOperationException.class, () -> + public void testBadValueDate() { + assertThrows(ParseException.class, () -> TypeHandler.createValue("what ever", PatternOptionBuilder.DATE_VALUE)); } + @Test + public void testCreateValueDate() throws Exception { + Date d = new Date(); + DateFormat fmt = DateFormat.getDateInstance(DateFormat.SHORT); + String s = fmt.format(d); + d = fmt.parse(s); + Date d2 = TypeHandler.createValue(s, PatternOptionBuilder.DATE_VALUE); + assertEquals( d, d2 ); + } + + @Test public void testCreateValueExistingFile() throws Exception { try (FileInputStream result = TypeHandler.createValue("src/test/resources/org/apache/commons/cli/existing-readable.file", From 7ae9b1cc6f5eb560fa7ed6958018b2af8eae93f2 Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Fri, 29 Dec 2023 14:35:45 +0100 Subject: [PATCH 02/24] Fixed issues with date parsing --- .../java/org/apache/commons/cli/TypeHandler.java | 15 ++++++++++++++- .../commons/cli/PatternOptionBuilderTest.java | 8 ++------ .../org/apache/commons/cli/TypeHandlerTest.java | 7 ++----- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/apache/commons/cli/TypeHandler.java b/src/main/java/org/apache/commons/cli/TypeHandler.java index 6a7535d68..b6e49bb75 100644 --- a/src/main/java/org/apache/commons/cli/TypeHandler.java +++ b/src/main/java/org/apache/commons/cli/TypeHandler.java @@ -20,6 +20,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more import java.io.File; import java.io.FileInputStream; import java.net.URL; +import java.text.SimpleDateFormat; import java.util.Date; import org.apache.commons.beanutils.ConversionException; @@ -81,9 +82,21 @@ public Number apply(String str) throws ConversionException { // fixup date parsing DateConverter dc = new DateConverter(); - dc.setUseLocaleFormat(true); + // should match "Thu Jun 06 17:48:57 EDT 2002" + dc.setPattern("EEE MMM dd HH:mm:ss zzz yyyy"); convertUtils.register(new ConverterFacade(dc), Date.class); } + + /** + * Registers a Converter for a class so that the class can be returned as the value of the command line option. + * Note: the converter will override any existing converter for the class. + * + * @param converter The converter to associate with the class. + * @param clazz the class that the converter handles. + */ + public static void register(Converter converter, Class clazz) { + convertUtils.register(converter, clazz); + } /** * Returns the class whose name is {@code className}. diff --git a/src/test/java/org/apache/commons/cli/PatternOptionBuilderTest.java b/src/test/java/org/apache/commons/cli/PatternOptionBuilderTest.java index 27c442a20..499696d0f 100644 --- a/src/test/java/org/apache/commons/cli/PatternOptionBuilderTest.java +++ b/src/test/java/org/apache/commons/cli/PatternOptionBuilderTest.java @@ -154,12 +154,8 @@ public void testSimplePattern() throws Exception { } // DATES NOT SUPPORTED YET - try { - assertEquals("date flag z", new Date(1023400137276L), line.getOptionObject('z')); - fail("Date is not supported yet, should have failed"); - } catch (final UnsupportedOperationException uoe) { - // expected - } + assertEquals("date flag z", new Date(1023400137000L), line.getOptionObject('z')); + } @Test diff --git a/src/test/java/org/apache/commons/cli/TypeHandlerTest.java b/src/test/java/org/apache/commons/cli/TypeHandlerTest.java index dbf61ac50..4307c5ce1 100644 --- a/src/test/java/org/apache/commons/cli/TypeHandlerTest.java +++ b/src/test/java/org/apache/commons/cli/TypeHandlerTest.java @@ -60,11 +60,8 @@ public void testBadValueDate() { @Test public void testCreateValueDate() throws Exception { - Date d = new Date(); - DateFormat fmt = DateFormat.getDateInstance(DateFormat.SHORT); - String s = fmt.format(d); - d = fmt.parse(s); - Date d2 = TypeHandler.createValue(s, PatternOptionBuilder.DATE_VALUE); + Date d = new Date(1023400137000L); + Date d2 = TypeHandler.createValue("Thu Jun 06 17:48:57 EDT 2002", PatternOptionBuilder.DATE_VALUE); assertEquals( d, d2 ); } From ad8466c78fafdc9beb39fef1ec2702ddc4721a9f Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Fri, 29 Dec 2023 16:55:07 +0100 Subject: [PATCH 03/24] Switched to ConvertUtilsBean2 for conversions. Added getParsedOptionValues methods with default values. Fixed som javadoc, --- .../org/apache/commons/cli/CommandLine.java | 63 ++++++++++++++++--- .../org/apache/commons/cli/TypeHandler.java | 17 +++-- .../apache/commons/cli/converters/Func.java | 10 ++- .../cli/converters/SimpleConverter.java | 11 +++- .../commons/cli/converters/package-info.java | 21 +++++++ .../commons/cli/PatternOptionBuilderTest.java | 1 - .../apache/commons/cli/TypeHandlerTest.java | 3 +- 7 files changed, 107 insertions(+), 19 deletions(-) create mode 100644 src/main/java/org/apache/commons/cli/converters/package-info.java diff --git a/src/main/java/org/apache/commons/cli/CommandLine.java b/src/main/java/org/apache/commons/cli/CommandLine.java index cea5b9320..6c73649cb 100644 --- a/src/main/java/org/apache/commons/cli/CommandLine.java +++ b/src/main/java/org/apache/commons/cli/CommandLine.java @@ -351,8 +351,9 @@ public String[] getOptionValues(final String opt) { * @throws ParseException if there are problems turning the option value into the desired type * @see PatternOptionBuilder * @since 1.5.0 + * @param The return type for the method. */ - public Object getParsedOptionValue(final char opt) throws ParseException { + public T getParsedOptionValue(final char opt) throws ParseException { return getParsedOptionValue(String.valueOf(opt)); } @@ -364,29 +365,77 @@ public Object getParsedOptionValue(final char opt) throws ParseException { * @throws ParseException if there are problems turning the option value into the desired type * @see PatternOptionBuilder * @since 1.5.0 + * @param The return type for the method. */ - public Object getParsedOptionValue(final Option option) throws ParseException { + public T getParsedOptionValue(final Option option) throws ParseException { + return getParsedOptionValue(option, null); + } + + /** + * Gets a version of this {@code Option} converted to a particular type. + * + * @param opt the name of the option. + * @return the value parsed into a particular object. + * @throws ParseException if there are problems turning the option value into the desired type + * @see PatternOptionBuilder + * @since 1.2 + * @param The return type for the method. + */ + public T getParsedOptionValue(final String opt) throws ParseException { + return getParsedOptionValue(resolveOption(opt)); + } + + /** + * Gets a version of this {@code Option} converted to a particular type. + * + * @param opt the name of the option. + * @param defaultValue the default value to return if opt is not set. + * @return the value parsed into a particular object. + * @throws ParseException if there are problems turning the option value into the desired type + * @see PatternOptionBuilder + * @since 1.7 + * @param The return type for the method. + */ + public T getParsedOptionValue(final char opt, T defaultValue) throws ParseException { + return getParsedOptionValue(String.valueOf(opt), defaultValue); + } + + /** + * Gets a version of this {@code Option} converted to a particular type. + * + * @param option the name of the option. + * @param defaultValue the default value to return if opt is not set. + * @return the value parsed into a particular object. + * @throws ParseException if there are problems turning the option value into the desired type + * @see PatternOptionBuilder + * @since 1.7 + * @param The return type for the method. + */ + @SuppressWarnings("unchecked") + public T getParsedOptionValue(final Option option, T defaultValue) throws ParseException { if (option == null) { return null; } final String res = getOptionValue(option); if (res == null) { - return null; + return defaultValue; } - return TypeHandler.createValue(res, option.getType()); + return (T) TypeHandler.createValue(res, (Class) option.getType()); } /** * Gets a version of this {@code Option} converted to a particular type. * * @param opt the name of the option. + * @param defaultValue the default value to return if opt is not set. * @return the value parsed into a particular object. * @throws ParseException if there are problems turning the option value into the desired type * @see PatternOptionBuilder - * @since 1.2 + * @since 1.7 + * @param The return type for the method. */ - public Object getParsedOptionValue(final String opt) throws ParseException { - return getParsedOptionValue(resolveOption(opt)); + public T getParsedOptionValue(final String opt, T defaultValue) throws ParseException { + return getParsedOptionValue(resolveOption(opt), defaultValue); } /** diff --git a/src/main/java/org/apache/commons/cli/TypeHandler.java b/src/main/java/org/apache/commons/cli/TypeHandler.java index b6e49bb75..8d5ad767a 100644 --- a/src/main/java/org/apache/commons/cli/TypeHandler.java +++ b/src/main/java/org/apache/commons/cli/TypeHandler.java @@ -20,11 +20,10 @@ Licensed to the Apache Software Foundation (ASF) under one or more import java.io.File; import java.io.FileInputStream; import java.net.URL; -import java.text.SimpleDateFormat; import java.util.Date; import org.apache.commons.beanutils.ConversionException; -import org.apache.commons.beanutils.ConvertUtilsBean; +import org.apache.commons.beanutils.ConvertUtilsBean2; import org.apache.commons.beanutils.Converter; import org.apache.commons.beanutils.converters.ConverterFacade; import org.apache.commons.beanutils.converters.DateConverter; @@ -37,10 +36,16 @@ Licensed to the Apache Software Foundation (ASF) under one or more */ public class TypeHandler { - private static ConvertUtilsBean convertUtils; + /** + * The conversion utilities from BeanUtils. + */ + private static ConvertUtilsBean2 convertUtils; + /** + * Setup the convertUtils object + */ static { - convertUtils = new ConvertUtilsBean(); + convertUtils = new ConvertUtilsBean2(); convertUtils.register(true, true, 0); /* * this can not be a ternary because the unboxing operations will result in it @@ -59,7 +64,7 @@ public Number apply(String str) throws ConversionException { }; convertUtils.register(new SimpleConverter<>(fn, Number.class), Number.class); - Func fo = (str) -> { + Func fo = str -> { final Class cl; try { @@ -238,7 +243,7 @@ public static T createValue(final String str, Class clazz) throws ParseEx * value of {@code str}. * @throws ParseException if the value creation for the given object type * failed - * @deprecated use createValue(str, Class); + * @deprecated use {@link #createValue(String, Class)}; */ @Deprecated // (since="1.7") public static Object createValue(final String str, final Object obj) throws ParseException { diff --git a/src/main/java/org/apache/commons/cli/converters/Func.java b/src/main/java/org/apache/commons/cli/converters/Func.java index 84928a7c2..0b01547eb 100644 --- a/src/main/java/org/apache/commons/cli/converters/Func.java +++ b/src/main/java/org/apache/commons/cli/converters/Func.java @@ -18,11 +18,17 @@ Licensed to the Apache Software Foundation (ASF) under one or more /** * The definition of the functional interface to call when doing a conversion. - * Like Function but can throw an Exception. + * Like {@code Function} but can throw an Exception. * * @param The return type for the function. */ @FunctionalInterface public interface Func { + /** + * Applies the conversion function to the String argument. + * @param str the String to convert + * @return the Object from the conversion. + * @throws Exception on error. + */ T apply(String str) throws Exception; -} \ No newline at end of file +} diff --git a/src/main/java/org/apache/commons/cli/converters/SimpleConverter.java b/src/main/java/org/apache/commons/cli/converters/SimpleConverter.java index fc30483d0..ccb95e05c 100644 --- a/src/main/java/org/apache/commons/cli/converters/SimpleConverter.java +++ b/src/main/java/org/apache/commons/cli/converters/SimpleConverter.java @@ -25,10 +25,19 @@ Licensed to the Apache Software Foundation (ASF) under one or more * @param The type of object the converter will return. */ public class SimpleConverter extends AbstractConverter { - + /** + * The Func to convert string values. + */ private final Func func; + /** + * The class type of the object that is returned. + */ private final Class type; + /** + * @param func the Func instance to use to convert values. + * @param type the Type that this Converter returns. + */ public SimpleConverter(Func func, Class type) { this.func = func; this.type = type; diff --git a/src/main/java/org/apache/commons/cli/converters/package-info.java b/src/main/java/org/apache/commons/cli/converters/package-info.java new file mode 100644 index 000000000..1cbf50ac4 --- /dev/null +++ b/src/main/java/org/apache/commons/cli/converters/package-info.java @@ -0,0 +1,21 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Apache Commons CLI converters to convert from String to Object of various types. + */ +package org.apache.commons.cli.converters; diff --git a/src/test/java/org/apache/commons/cli/PatternOptionBuilderTest.java b/src/test/java/org/apache/commons/cli/PatternOptionBuilderTest.java index 499696d0f..dcea28e37 100644 --- a/src/test/java/org/apache/commons/cli/PatternOptionBuilderTest.java +++ b/src/test/java/org/apache/commons/cli/PatternOptionBuilderTest.java @@ -153,7 +153,6 @@ public void testSimplePattern() throws Exception { // expected } - // DATES NOT SUPPORTED YET assertEquals("date flag z", new Date(1023400137000L), line.getOptionObject('z')); } diff --git a/src/test/java/org/apache/commons/cli/TypeHandlerTest.java b/src/test/java/org/apache/commons/cli/TypeHandlerTest.java index 4307c5ce1..2ad866d67 100644 --- a/src/test/java/org/apache/commons/cli/TypeHandlerTest.java +++ b/src/test/java/org/apache/commons/cli/TypeHandlerTest.java @@ -25,7 +25,6 @@ Licensed to the Apache Software Foundation (ASF) under one or more import java.io.File; import java.io.FileInputStream; import java.net.URL; -import java.text.DateFormat; import java.util.Date; import org.junit.Test; @@ -62,7 +61,7 @@ public void testBadValueDate() { public void testCreateValueDate() throws Exception { Date d = new Date(1023400137000L); Date d2 = TypeHandler.createValue("Thu Jun 06 17:48:57 EDT 2002", PatternOptionBuilder.DATE_VALUE); - assertEquals( d, d2 ); + assertEquals(d, d2); } From e02c36caae48b5fe1725a8a53626432131d1cdd6 Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Wed, 3 Jan 2024 11:40:34 +0100 Subject: [PATCH 04/24] Implementation with tests --- pom.xml | 6 +- .../org/apache/commons/cli/CommandLine.java | 8 +- .../java/org/apache/commons/cli/Option.java | 79 ++++++- .../org/apache/commons/cli/OptionBuilder.java | 3 + .../apache/commons/cli/ParseException.java | 27 +++ .../commons/cli/PatternOptionBuilder.java | 35 ++- .../org/apache/commons/cli/TypeHandler.java | 218 +++++++++--------- .../commons/cli/converters/Converter.java | 98 ++++++++ .../apache/commons/cli/converters/Func.java | 34 --- .../cli/converters/SimpleConverter.java | 60 ----- .../commons/cli/converters/Verifier.java | 64 +++++ .../org/apache/commons/cli/OptionTest.java | 6 +- .../commons/cli/PatternOptionBuilderTest.java | 7 +- .../cli/converters/ConverterTests.java | 122 ++++++++++ .../commons/cli/converters/VerifierTests.java | 86 +++++++ 15 files changed, 635 insertions(+), 218 deletions(-) create mode 100644 src/main/java/org/apache/commons/cli/converters/Converter.java delete mode 100644 src/main/java/org/apache/commons/cli/converters/Func.java delete mode 100644 src/main/java/org/apache/commons/cli/converters/SimpleConverter.java create mode 100644 src/main/java/org/apache/commons/cli/converters/Verifier.java create mode 100644 src/test/java/org/apache/commons/cli/converters/ConverterTests.java create mode 100644 src/test/java/org/apache/commons/cli/converters/VerifierTests.java diff --git a/pom.xml b/pom.xml index eee1c9507..9baf6d414 100644 --- a/pom.xml +++ b/pom.xml @@ -193,9 +193,9 @@ test - commons-beanutils - commons-beanutils - 1.9.4 + org.junit.jupiter + junit-jupiter-params + test diff --git a/src/main/java/org/apache/commons/cli/CommandLine.java b/src/main/java/org/apache/commons/cli/CommandLine.java index 6c73649cb..e4e2e509f 100644 --- a/src/main/java/org/apache/commons/cli/CommandLine.java +++ b/src/main/java/org/apache/commons/cli/CommandLine.java @@ -417,10 +417,12 @@ public T getParsedOptionValue(final Option option, T defaultValue) throws Pa return null; } final String res = getOptionValue(option); - if (res == null) { - return defaultValue; + + try { + return res == null ? defaultValue : (T) option.getConverter().apply(res); + } catch (Exception e) { + throw ParseException.wrap(e); } - return (T) TypeHandler.createValue(res, (Class) option.getType()); } /** diff --git a/src/main/java/org/apache/commons/cli/Option.java b/src/main/java/org/apache/commons/cli/Option.java index 7834fc894..bec2eda5e 100644 --- a/src/main/java/org/apache/commons/cli/Option.java +++ b/src/main/java/org/apache/commons/cli/Option.java @@ -24,6 +24,9 @@ Licensed to the Apache Software Foundation (ASF) under one or more import java.util.List; import java.util.Objects; +import org.apache.commons.cli.converters.Converter; +import org.apache.commons.cli.converters.Verifier; + /** * Describes a single command-line option. It maintains information regarding the short-name of the option, the * long-name, if any exists, a flag indicating if an argument is required for this option, and a self-documenting @@ -82,6 +85,12 @@ public static final class Builder { /** The character that is the value separator */ private char valueSeparator; + + /** The converter to convert to type **/ + private Converter converter = null; + + /** The verifier to verify string is valid for option */ + private Verifier verifier = null; /** * Constructs a new {@code Builder} with the minimum required parameters for an {@code Option} instance. @@ -234,6 +243,12 @@ public Builder required(final boolean required) { */ public Builder type(final Class type) { this.type = type; + if (verifier == null) { + verifier = TypeHandler.getVerifier(type); + } + if (converter == null) { + converter = TypeHandler.getConverter(type); + } return this; } @@ -270,6 +285,16 @@ public Builder valueSeparator(final char valueSeparator) { this.valueSeparator = valueSeparator; return this; } + + public Builder converter(Converter converter) { + this.converter = converter; + return this; + } + + public Builder verifier(Verifier verifier) { + this.verifier = verifier; + return this; + } } /** Specifies the number of argument values has not been specified */ @@ -335,6 +360,10 @@ public static Builder builder(final String option) { /** The character that is the value separator. */ private char valuesep; + + private Converter converter = Converter.DEFAULT; + + private Verifier verifier = Verifier.DEFAULT; /** * Private constructor used by the nested Builder class. @@ -351,6 +380,8 @@ private Option(final Builder builder) { this.required = builder.required; this.type = builder.type; this.valuesep = builder.valueSeparator; + this.converter = builder.converter == null ? Converter.DEFAULT : builder.converter; + this.verifier = builder.verifier == null ? Verifier.DEFAULT : builder.verifier; } /** @@ -419,11 +450,15 @@ boolean acceptsArg() { * * @since 1.0.1 */ - private void add(final String value) { + private void add(final String value) throws ParseException { if (!acceptsArg()) { throw new IllegalArgumentException("Cannot add value, list full."); } + if (!verifier.test(value)) { + throw new ParseException(String.format("'%s' is not a valid input string for option '%s'", value, option)); + } + // store value values.add(value); } @@ -448,7 +483,7 @@ public boolean addValue(final String value) { * * @param value is a/the value of this Option */ - void addValueForProcessing(final String value) { + void addValueForProcessing(final String value) throws ParseException { if (argCount == UNINITIALIZED) { throw new IllegalArgumentException("NO_ARGS_ALLOWED"); } @@ -729,7 +764,7 @@ public boolean isRequired() { * * @since 1.0.1 */ - private void processValue(String value) { + private void processValue(String value) throws ParseException { // this Option has a separator character if (hasValueSeparator()) { // get the separator character @@ -839,6 +874,12 @@ public void setRequired(final boolean required) { */ public void setType(final Class type) { this.type = type; + if (verifier == null || verifier == Verifier.DEFAULT) { + verifier = TypeHandler.getVerifier(type); + } + if (converter == null || converter == Converter.DEFAULT) { + converter = TypeHandler.getConverter(type); + } } /** @@ -864,6 +905,38 @@ public void setType(final Object type) { public void setValueSeparator(final char sep) { this.valuesep = sep; } + + /** + * Gets the value to type converter. + * @return the value to type converter + */ + public Converter getConverter() { + return converter; + } + + /** + * Sets the value to type converter. + * @param converter The converter to convert the string value to the type. + */ + public void setConverter(Converter converter) { + this.converter = converter; + } + + /** + * Gets the value verifier. + * @return the value verifier. + */ + public Verifier getVerifier() { + return 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. + */ + public void setVerifier(Verifier verifier) { + this.verifier = verifier; + } /** * Dump state, suitable for debugging. diff --git a/src/main/java/org/apache/commons/cli/OptionBuilder.java b/src/main/java/org/apache/commons/cli/OptionBuilder.java index e06f74737..d3655f4e2 100644 --- a/src/main/java/org/apache/commons/cli/OptionBuilder.java +++ b/src/main/java/org/apache/commons/cli/OptionBuilder.java @@ -31,6 +31,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more @Deprecated public final class OptionBuilder { + /** Long option */ private static String longOption; @@ -108,6 +109,8 @@ public static Option create(final String opt) throws IllegalArgumentException { option.setOptionalArg(optionalArg); option.setArgs(argCount); option.setType(type); + option.setConverter( TypeHandler.getConverter(type)); + option.setVerifier( TypeHandler.getVerifier(type)); option.setValueSeparator(valueSeparator); option.setArgName(argName); } finally { diff --git a/src/main/java/org/apache/commons/cli/ParseException.java b/src/main/java/org/apache/commons/cli/ParseException.java index 9386a7664..331633cb1 100644 --- a/src/main/java/org/apache/commons/cli/ParseException.java +++ b/src/main/java/org/apache/commons/cli/ParseException.java @@ -21,11 +21,33 @@ Licensed to the Apache Software Foundation (ASF) under one or more * Base for Exceptions thrown during parsing of a command-line. */ public class ParseException extends Exception { + /** * This exception {@code serialVersionUID}. */ private static final long serialVersionUID = 9112808380089253192L; + /** + * Converts any exception except UnsupportedOperationException to a ParseException. + * if {@code e} is an instance of ParseException it is returned, otherwise a ParseException is + * created that wraps it. + *

+ * Note: UnsupportedOperationExceptions are not wrapped. This is to solve a legacy expected exception problem and will be + * removed in the future.

+ * @param e the exception to convert. + * @return the ParseException. + * @throws UnsupportedOperationException due to legacy expectations. Will be removed in the future. + */ + public static ParseException wrap(final Exception e) throws UnsupportedOperationException { + if (e instanceof UnsupportedOperationException) { + throw (UnsupportedOperationException)e; + } + + if (e instanceof ParseException) { + return (ParseException) e; + } + return new ParseException(e); + } /** * Constructs a new {@code ParseException} with the specified detail message. * @@ -38,4 +60,9 @@ public ParseException(final String message) { public ParseException(final Exception e) { super(e); } + + public ParseException(String message, Throwable e) { + super(message, e); + } + } diff --git a/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java b/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java index 4e6919f94..3fd39f7c6 100644 --- a/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java +++ b/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java @@ -19,9 +19,13 @@ Licensed to the Apache Software Foundation (ASF) under one or more import java.io.File; import java.io.FileInputStream; +import java.io.FileNotFoundException; import java.net.URL; import java.util.Date; +import org.apache.commons.cli.converters.Converter; +import org.apache.commons.cli.converters.Verifier; + /** * Allows Options to be created from a single String. The pattern contains various single character flags and via an * optional punctuation character, their expected type. @@ -102,14 +106,25 @@ public class PatternOptionBuilder { /** URL class */ public static final Class URL_VALUE = URL.class; + + static final Converter NOT_IMPLEMENTED = s -> {throw new UnsupportedOperationException("Not yet implemented");}; + + static { + TypeHandler.register(PatternOptionBuilder.FILES_VALUE, NOT_IMPLEMENTED, null); + } /** * Retrieve the class that {@code ch} represents. * * @param ch the specified character * @return The class that {@code ch} represents + * @deprecated use {@link #getValueType(char)} */ public static Object getValueClass(final char ch) { + return getValueType(ch); + } + + public static Class getValueType(final char ch) { switch (ch) { case '@': return PatternOptionBuilder.OBJECT_VALUE; @@ -134,6 +149,15 @@ public static Object getValueClass(final char ch) { return null; } + public static Converter getConverter(final char ch) { + return TypeHandler.getConverter(getValueType(ch)); + } + + public static Verifier getVerifier(final char ch) { + return TypeHandler.getVerifier(getValueType(ch)); + } + + /** * Returns whether {@code ch} is a value code, i.e. whether it represents a class in a pattern. * @@ -154,6 +178,8 @@ public static Options parsePattern(final String pattern) { char opt = ' '; boolean required = false; Class type = null; + Converter converter = Converter.DEFAULT; + Verifier verifier = Verifier.DEFAULT; final Options options = new Options(); @@ -164,19 +190,24 @@ public static Options parsePattern(final String pattern) { // details about it if (!isValueCode(ch)) { if (opt != ' ') { - final Option option = Option.builder(String.valueOf(opt)).hasArg(type != null).required(required).type(type).build(); + final Option option = Option.builder(String.valueOf(opt)).hasArg(type != null).required(required).type(type) + .converter(converter).verifier(verifier).build(); // we have a previous one to deal with options.addOption(option); required = false; type = null; + converter = Converter.DEFAULT; + verifier = Verifier.DEFAULT; } opt = ch; } else if (ch == '!') { required = true; } else { - type = (Class) getValueClass(ch); + type = getValueType(ch); + converter = getConverter(ch); + verifier = getVerifier(ch); } } diff --git a/src/main/java/org/apache/commons/cli/TypeHandler.java b/src/main/java/org/apache/commons/cli/TypeHandler.java index 8d5ad767a..48638df49 100644 --- a/src/main/java/org/apache/commons/cli/TypeHandler.java +++ b/src/main/java/org/apache/commons/cli/TypeHandler.java @@ -19,88 +19,93 @@ Licensed to the Apache Software Foundation (ASF) under one or more import java.io.File; import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.math.BigDecimal; +import java.math.BigInteger; import java.net.URL; import java.util.Date; +import java.util.HashMap; +import java.util.Map; -import org.apache.commons.beanutils.ConversionException; -import org.apache.commons.beanutils.ConvertUtilsBean2; -import org.apache.commons.beanutils.Converter; -import org.apache.commons.beanutils.converters.ConverterFacade; -import org.apache.commons.beanutils.converters.DateConverter; -import org.apache.commons.cli.converters.Func; -import org.apache.commons.cli.converters.SimpleConverter; +import org.apache.commons.cli.converters.Converter; +import org.apache.commons.cli.converters.Verifier; /** - * TypeHandler will handles the configuration of the Option type processing using - * the Commons BeanUtils ConvertUtilsBean class. + * This is a temporary implementation. TypeHandler will handle the pluggableness + * of OptionTypes and it will direct all of these types of conversion + * functionalities to ConvertUtils component in Commons already. BeanUtils I + * think. */ public class TypeHandler { - /** - * The conversion utilities from BeanUtils. - */ - private static ConvertUtilsBean2 convertUtils; + private static Map, Converter> converterMap = new HashMap<>(); + private static Map, Verifier> verifierMap = new HashMap<>(); - /** - * Setup the convertUtils object - */ static { - convertUtils = new ConvertUtilsBean2(); - convertUtils.register(true, true, 0); - /* - * this can not be a ternary because the unboxing operations will result in it - * always being a Double. - * https://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.25-300-A - */ - Func fn = new Func() { + resetConverters(); + resetVerifiers(); + } - @Override - public Number apply(String str) throws ConversionException { - if (str.indexOf('.') == -1) { - return Long.valueOf(str); - } - return Double.valueOf(str); - } - }; - convertUtils.register(new SimpleConverter<>(fn, Number.class), Number.class); + public static void resetConverters() { + converterMap.clear(); + converterMap.put(Object.class, Converter.OBJECT); + converterMap.put(Class.class, Converter.CLASS); + converterMap.put(Date.class, Converter.DATE); + converterMap.put(File.class, Converter.FILE); + converterMap.put(Number.class, Converter.NUMBER); + converterMap.put(URL.class, Converter.URL); + converterMap.put(FileInputStream.class, s -> new FileInputStream(s)); + converterMap.put(Long.class, Long::parseLong); + converterMap.put(Integer.class, Integer::parseInt); + converterMap.put(Short.class, Short::parseShort); + converterMap.put(Byte.class, Byte::parseByte); + converterMap.put(Character.class, s -> s.charAt(0)); + converterMap.put(Double.class, Double::parseDouble); + converterMap.put(Float.class, Float::parseFloat); + converterMap.put(BigInteger.class, s -> new BigInteger(s)); + converterMap.put(BigDecimal.class, s -> new BigDecimal(s)); + } + + public static void resetVerifiers() { + verifierMap.clear(); + verifierMap.put(Object.class, Verifier.CLASS); + verifierMap.put(Class.class, Verifier.CLASS); + verifierMap.put(Number.class, Verifier.NUMBER); - Func fo = str -> { - final Class cl; + Verifier intVerifier = s -> s.matches("-?\\d+"); + verifierMap.put(Long.class, intVerifier); + verifierMap.put(Integer.class, intVerifier); + verifierMap.put(Short.class, intVerifier); + verifierMap.put(Byte.class, intVerifier); - try { - cl = Class.forName(str); - } catch (final ClassNotFoundException cnfe) { - throw new ConversionException("Unable to find the class: " + str); - } + verifierMap.put(Double.class, Verifier.NUMBER); + verifierMap.put(Float.class, Verifier.NUMBER); + verifierMap.put(BigInteger.class, intVerifier); + verifierMap.put(BigDecimal.class, Verifier.NUMBER); + } - try { - return cl.getConstructor().newInstance(); - } catch (final Exception e) { - throw new ConversionException(e.getClass().getName() + "; Unable to create an instance of: " + str); - } - }; - convertUtils.register(new SimpleConverter<>(fo, Object.class), Object.class); + public static void register(Class clazz, Converter converter, Verifier verifier) { + if (converter == null) { + converterMap.remove(clazz); + } else { + converterMap.put(clazz, converter); + } - convertUtils.register( - new SimpleConverter<>(str -> new FileInputStream(str), FileInputStream.class), - FileInputStream.class); + if (verifier == null) { + verifierMap.remove(clazz); + } else { + verifierMap.put(clazz, verifier); + } + } - // fixup date parsing - DateConverter dc = new DateConverter(); - // should match "Thu Jun 06 17:48:57 EDT 2002" - dc.setPattern("EEE MMM dd HH:mm:ss zzz yyyy"); - convertUtils.register(new ConverterFacade(dc), Date.class); + public static Converter getConverter(Class clazz) { + Converter converter = converterMap.get(clazz); + return converter == null ? Converter.DEFAULT : converter; } - - /** - * Registers a Converter for a class so that the class can be returned as the value of the command line option. - * Note: the converter will override any existing converter for the class. - * - * @param converter The converter to associate with the class. - * @param clazz the class that the converter handles. - */ - public static void register(Converter converter, Class clazz) { - convertUtils.register(converter, clazz); + + public static Verifier getVerifier(Class clazz) { + Verifier verifier = verifierMap.get(clazz); + return verifier == null ? Verifier.DEFAULT : verifier; } /** @@ -109,9 +114,9 @@ public static void register(Converter converter, Class clazz) { * @param className the class name * @return The class if it is found * @throws ParseException if the class could not be found - * @deprecated use createValue(className,Class.class) + * @deprecated use {@link #createValue(String, Class)} */ - @Deprecated // (since="1.7") + @Deprecated // sinze 1.7 public static Class createClass(final String className) throws ParseException { return createValue(className, Class.class); } @@ -120,15 +125,18 @@ public static Class createClass(final String className) throws ParseException * Returns the date represented by {@code str}.

This method is not yet * implemented and always throws an {@link UnsupportedOperationException}. * - * @param str the date string - * @return The date if {@code str} is a valid - * date string, otherwise return null. - * @throws UnsupportedOperationException always - * @deprecated use createValue(str, Date.class); + * @param str the date string + * @return The date if {@code str} is a valid date string, otherwise + * return null. + * @deprecated use {@link #createValue(String, Class)} */ - @Deprecated // (since="1.7") + @Deprecated // sinze 1.7 public static Date createDate(final String str) { - return createValueNoException(str, Date.class); + try { + return createValue(str, Date.class); + } catch (ParseException e) { + throw new RuntimeException(e); + } } /** @@ -136,21 +144,28 @@ public static Date createDate(final String str) { * * @param str the File location * @return The file represented by {@code str}. - * @deprecated use createValue(str, File.class); + * @deprecated use {@link #createValue(String, Class)} */ - @Deprecated // (since="1.7") + @Deprecated // sinze 1.7 public static File createFile(final String str) { - return createValueNoException(str, File.class); + try { + return createValue(str, File.class); + } catch (ParseException e) { + throw new RuntimeException(e); + } } /** * Returns the File[] represented by {@code str}.

This method is not yet * implemented and always throws an {@link UnsupportedOperationException}. * - * @param str the paths to the files - * @return The File[] represented by {@code str}. + * @param str the paths to the files + * @return The File[] represented by + * {@code str}. + * @throws UnsupportedOperationException always + * @deprecated with no replacement */ - @Deprecated // (since="1.7") + @Deprecated // sinze 1.7 public static File[] createFiles(final String str) { // to implement/port: // return FileW.findFiles(str); @@ -164,9 +179,8 @@ public static File[] createFiles(final String str) { * @param str the value * @return the number represented by {@code str} * @throws ParseException if {@code str} is not a number - * @deprecated use createValue(str, Number.class); */ - @Deprecated // (since="1.7") + @Deprecated // sinze 1.7 public static Number createNumber(final String str) throws ParseException { return createValue(str, Number.class); } @@ -178,9 +192,9 @@ public static Number createNumber(final String str) throws ParseException { * @return the initialized object * @throws ParseException if the class could not be found or the object * could not be created - * @deprecated use createValue(str, Object.class); + * @deprecated use {@link #createValue(String, Class)} */ - @Deprecated // (since="1.7") + @Deprecated // sinze 1.7 public static Object createObject(final String className) throws ParseException { return createValue(className, Object.class); } @@ -191,21 +205,13 @@ public static Object createObject(final String className) throws ParseException * @param str the URL string * @return The URL in {@code str} is well-formed * @throws ParseException if the URL in {@code str} is not well-formed - * @deprecated use createValue(str, URL.class); + * @deprecated use {@link #createValue(String, Class)} */ - @Deprecated // (since="1.7") + @Deprecated // sinze 1.7 public static URL createURL(final String str) throws ParseException { return createValue(str, URL.class); } - private static T createValueNoException(final String str, Class clazz) { - try { - return createValue(str, clazz); - } catch (ParseException e) { - throw new RuntimeException(e); - } - } - /** * Returns the {@code Object} of type {@code clazz} with the value of * {@code str}. @@ -218,19 +224,11 @@ private static T createValueNoException(final String str, Class clazz) { * @throws ParseException if the value creation for the given class failed */ @SuppressWarnings("unchecked") // returned value will have type T because it is fixed by clazz - public static T createValue(final String str, Class clazz) throws ParseException { - // BeanUtils convertUtils handles File[] but looks for the string to parse into file names. - if (PatternOptionBuilder.FILES_VALUE == clazz) { - return (T) createFiles(str); - } - Converter converter = convertUtils.lookup(String.class, clazz); - if (converter == null) { - throw new ParseException(String.format("No registered converter for %s found", clazz)); - } + public static T createValue(final String str, final Class clazz) throws ParseException { try { - return converter.convert(clazz, str); - } catch (ConversionException e) { - throw new ParseException(e); + return (T) getConverter(clazz).apply(str); + } catch (Exception e) { + throw ParseException.wrap(e); } } @@ -243,9 +241,9 @@ public static T createValue(final String str, Class clazz) throws ParseEx * value of {@code str}. * @throws ParseException if the value creation for the given object type * failed - * @deprecated use {@link #createValue(String, Class)}; + * @deprecated use {@link #createValue(String, Class)} */ - @Deprecated // (since="1.7") + @Deprecated // since 1.7 public static Object createValue(final String str, final Object obj) throws ParseException { return createValue(str, (Class) obj); } @@ -256,9 +254,9 @@ public static Object createValue(final String str, final Object obj) throws Pars * @param str the file location * @return The file input stream represented by {@code str}. * @throws ParseException if the file is not exist or not readable - * @deprecated use createValue(str, URL.class); + * @deprecated use {@link #createValue(String, Class)} */ - @Deprecated // (since="1.7") + @Deprecated // since 1.7 public static FileInputStream openFile(final String str) throws ParseException { return createValue(str, FileInputStream.class); } diff --git a/src/main/java/org/apache/commons/cli/converters/Converter.java b/src/main/java/org/apache/commons/cli/converters/Converter.java new file mode 100644 index 000000000..ce858295e --- /dev/null +++ b/src/main/java/org/apache/commons/cli/converters/Converter.java @@ -0,0 +1,98 @@ +/* + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ +package org.apache.commons.cli.converters; + +import java.io.File; +import java.net.MalformedURLException; +import java.net.URL; +import java.time.ZonedDateTime; +import java.time.format.DateTimeFormatter; +import java.util.Date; +import java.util.function.Function; +import java.util.function.Supplier; +import java.text.SimpleDateFormat; + +import org.apache.commons.cli.ParseException; + +/** + * The definition of the functional interface to call when doing a conversion. + * Like {@code Function} but can throw an Exception. + * + * @param The return type for the function. + */ +@FunctionalInterface +public interface Converter { + + /** + * The default converter. Does nothing. + */ + static final Converter DEFAULT = s -> s; + + /** + * Class name converter. Calls {@code Class.forName}. + */ + static final Converter> CLASS = s -> Class.forName(s); + + /** + * File name converter. Calls @{code new File(s)} + */ + static final Converter FILE = s -> new File(s); + + /** + * Number converter. Converts to a Double if a decimal point ('.') is in the string or + * a Long otherwise. + */ + static final Converter NUMBER = s -> { + if (s.indexOf('.') != -1) { + return Double.valueOf(s); + } + return Long.valueOf(s); + }; + + /** + * Converts a class name to an instance of the class. Uses the Class converter to + * find the class and then call the default constructor. + * @see #CLASS + */ + static final Converter OBJECT = s -> CLASS.apply(s).getConstructor().newInstance(); + + /** + * Creates a URL. Calls {@code new URL(s)}. + */ + static final Converter URL = s -> new URL(s); + + /** + * The simple date format used for the Date conversion. + * Format is: "EEE MMM dd HH:mm:ss zzz yyyy" + */ + static final SimpleDateFormat DATE_FMT = new SimpleDateFormat("EEE MMM dd HH:mm:ss zzz yyyy"); + + /** + * Converts to a date using the {@code DATE_FMT}. + * @see #DATE_FMT + */ + static final Converter DATE = s -> DATE_FMT.parse(s); + + + /** + * Applies the conversion function to the String argument. + * @param str the String to convert + * @return the Object from the conversion. + * @throws Exception on error. + */ + T apply(String str) throws Exception; +} diff --git a/src/main/java/org/apache/commons/cli/converters/Func.java b/src/main/java/org/apache/commons/cli/converters/Func.java deleted file mode 100644 index 0b01547eb..000000000 --- a/src/main/java/org/apache/commons/cli/converters/Func.java +++ /dev/null @@ -1,34 +0,0 @@ -/* - Licensed to the Apache Software Foundation (ASF) under one or more - contributor license agreements. See the NOTICE file distributed with - this work for additional information regarding copyright ownership. - The ASF licenses this file to You under the Apache License, Version 2.0 - (the "License"); you may not use this file except in compliance with - the License. You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - */ -package org.apache.commons.cli.converters; - -/** - * The definition of the functional interface to call when doing a conversion. - * Like {@code Function} but can throw an Exception. - * - * @param The return type for the function. - */ -@FunctionalInterface -public interface Func { - /** - * Applies the conversion function to the String argument. - * @param str the String to convert - * @return the Object from the conversion. - * @throws Exception on error. - */ - T apply(String str) throws Exception; -} diff --git a/src/main/java/org/apache/commons/cli/converters/SimpleConverter.java b/src/main/java/org/apache/commons/cli/converters/SimpleConverter.java deleted file mode 100644 index ccb95e05c..000000000 --- a/src/main/java/org/apache/commons/cli/converters/SimpleConverter.java +++ /dev/null @@ -1,60 +0,0 @@ -/* - Licensed to the Apache Software Foundation (ASF) under one or more - contributor license agreements. See the NOTICE file distributed with - this work for additional information regarding copyright ownership. - The ASF licenses this file to You under the Apache License, Version 2.0 - (the "License"); you may not use this file except in compliance with - the License. You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - */ -package org.apache.commons.cli.converters; - -import org.apache.commons.beanutils.converters.AbstractConverter; - -/** - * A simple converter that takes a Func and the datatype to create a Converter - * for the BeanUtils. - * - * @param The type of object the converter will return. - */ -public class SimpleConverter extends AbstractConverter { - /** - * The Func to convert string values. - */ - private final Func func; - /** - * The class type of the object that is returned. - */ - private final Class type; - - /** - * @param func the Func instance to use to convert values. - * @param type the Type that this Converter returns. - */ - public SimpleConverter(Func func, Class type) { - this.func = func; - this.type = type; - } - - @SuppressWarnings("unchecked") - @Override - protected T2 convertToType(Class type, Object value) throws Throwable { - if (this.type.equals(type)) { - return (T2) func.apply(value.toString()); - } - - throw conversionException(type, value); - } - - @Override - protected Class getDefaultType() { - return type; - } -} diff --git a/src/main/java/org/apache/commons/cli/converters/Verifier.java b/src/main/java/org/apache/commons/cli/converters/Verifier.java new file mode 100644 index 000000000..0a46c1e53 --- /dev/null +++ b/src/main/java/org/apache/commons/cli/converters/Verifier.java @@ -0,0 +1,64 @@ +/* + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ +package org.apache.commons.cli.converters; + +import java.util.regex.Pattern; + +/** + * The definition of the functional interface to call when verifying a string + * for input Like {@code Predicate} but can throw a RuntimeException. + * + */ +@FunctionalInterface +public interface Verifier { + + /** + * Default verifier. Always returns {@code true}. + */ + static final Verifier DEFAULT = (s) -> true; + + /** + * The Regex Pattern for the number matching. + */ + static Pattern NUMBER_PATTERN = Pattern.compile("-?([0-9]*\\.)?([0-9]+)$"); + + /** + * Verifies that a number is either a valid real number (e.g. may have a decimal + * point). + */ + static final Verifier NUMBER = s -> NUMBER_PATTERN.matcher(s).matches(); + + /** + * The Regex Pattern that matches valid class names. + */ + static Pattern CLAZZ_PATTERN = Pattern.compile( + "\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*(\\.\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*)*"); + + /** + * Verifies that a class name is valid. + */ + static final Verifier CLASS = s -> CLAZZ_PATTERN.matcher(s).matches(); + + /** + * Applies the verification function to the String argument. + * @param str the String to convert + * @return {@code true} if the string is valid, {@code false} + * otherwise. + * @throws RuntimeException on error. + */ + boolean test(String str) throws RuntimeException; +} diff --git a/src/test/java/org/apache/commons/cli/OptionTest.java b/src/test/java/org/apache/commons/cli/OptionTest.java index 7cc84f82c..1f542eca3 100644 --- a/src/test/java/org/apache/commons/cli/OptionTest.java +++ b/src/test/java/org/apache/commons/cli/OptionTest.java @@ -54,8 +54,12 @@ private static final class TestOption extends Option { @Override public boolean addValue(final String value) { + try { addValueForProcessing(value); return true; + } catch (ParseException e) { + throw new RuntimeException( e ); + } } } @@ -169,7 +173,7 @@ public void testClone() { } @Test - public void testGetValue() { + public void testGetValue() throws ParseException { final Option option = new Option("f", null); option.setArgs(Option.UNLIMITED_VALUES); diff --git a/src/test/java/org/apache/commons/cli/PatternOptionBuilderTest.java b/src/test/java/org/apache/commons/cli/PatternOptionBuilderTest.java index dcea28e37..378f5749a 100644 --- a/src/test/java/org/apache/commons/cli/PatternOptionBuilderTest.java +++ b/src/test/java/org/apache/commons/cli/PatternOptionBuilderTest.java @@ -23,6 +23,7 @@ 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; @@ -79,15 +80,17 @@ public void testExistingFilePatternFileNotExist() throws Exception { public void testNumberPattern() throws Exception { final Options options = PatternOptionBuilder.parsePattern("n%d%x%"); final CommandLineParser parser = new PosixParser(); - final CommandLine line = parser.parse(options, new String[] {"-n", "1", "-d", "2.1", "-x", "3,5"}); + // 3,5 fails validation. + 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"}); 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")); - assertNull("x object", line.getOptionObject("x")); + assertEquals("x object", Double.valueOf(3.5), line.getOptionObject("x")); } @Test diff --git a/src/test/java/org/apache/commons/cli/converters/ConverterTests.java b/src/test/java/org/apache/commons/cli/converters/ConverterTests.java new file mode 100644 index 000000000..cc361531e --- /dev/null +++ b/src/test/java/org/apache/commons/cli/converters/ConverterTests.java @@ -0,0 +1,122 @@ +/* + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ +package org.apache.commons.cli.converters; + +import static org.junit.Assert.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.net.URL; +import java.util.ArrayList; +import java.util.Date; +import java.util.List; +import java.util.stream.Stream; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +/** + * Tests for standard Converters. + */ +public class ConverterTests { + + @ParameterizedTest + @MethodSource("numberTestParameters") + public void numberTests(String str, Number expected) throws Exception { + if (expected != null) { + assertEquals(expected, Converter.NUMBER.apply(str)); + } else { + assertThrows(NumberFormatException.class, () -> Converter.NUMBER.apply(str)); + } + } + + private static Stream numberTestParameters() { + List lst = new ArrayList<>(); + + lst.add(Arguments.of("123", Long.valueOf("123"))); + lst.add(Arguments.of("12.3", Double.valueOf("12.3"))); + lst.add(Arguments.of("-123", Long.valueOf("-123"))); + lst.add(Arguments.of("-12.3", Double.valueOf("-12.3"))); + lst.add(Arguments.of(".3", Double.valueOf("0.3"))); + lst.add(Arguments.of("-.3", Double.valueOf("-0.3"))); + lst.add(Arguments.of("0x5F", null)); + lst.add(Arguments.of("2,3", null)); + lst.add(Arguments.of("1.2.3", null)); + + return lst.stream(); + } + + @Test + public void classTests() throws Exception { + + assertNotNull(Converter.CLASS.apply(this.getClass().getName()), this.getClass().getName()); + assertNotNull(Converter.CLASS.apply(this.getClass().getCanonicalName()), this.getClass().getCanonicalName()); + assertThrows(ClassNotFoundException.class, () -> Converter.CLASS.apply(this.getClass().getSimpleName()), + this.getClass().getSimpleName()); + 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())); + } + + @Test + public void objectTests() throws Exception { + assertNotNull(Converter.OBJECT.apply(this.getClass().getName()), this.getClass().getName()); + assertNotNull(Converter.OBJECT.apply(this.getClass().getCanonicalName()), this.getClass().getCanonicalName()); + assertThrows(ClassNotFoundException.class, () -> Converter.OBJECT.apply(this.getClass().getSimpleName()), + this.getClass().getSimpleName()); + 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())); + } + + @Test + public void dateTests() throws Exception { + assertThrows(java.text.ParseException.class, () -> Converter.DATE.apply("whatever")); + + Date d = new Date(1023400137000L); + assertEquals(d, Converter.DATE.apply("Thu Jun 06 17:48:57 EDT 2002")); + + assertThrows(java.text.ParseException.class, () -> Converter.DATE.apply("Jun 06 17:48:57 EDT 2002")); + } + + @Test + public void urlTests() throws Exception { + URL url = this.getClass().getClassLoader().getResource("org/apache/commons/cli/existing-readable.file"); + assertEquals(url, Converter.URL.apply(url.toString())); + + assertEquals(new URL("http://apache.org"), Converter.URL.apply("http://apache.org")); + + assertThrows(java.net.MalformedURLException.class, () -> Converter.URL.apply("foo.bar")); + } + + @Test + public void fileTests() throws Exception { + URL url = this.getClass().getClassLoader().getResource("org/apache/commons/cli/existing-readable.file"); + String fileName = url.toString().substring("file:".length()); + assertNotNull(Converter.FILE.apply(fileName)); + } + + // A class without a default constructor. + public class TestClass { + public TestClass(int i) { + } + } +} diff --git a/src/test/java/org/apache/commons/cli/converters/VerifierTests.java b/src/test/java/org/apache/commons/cli/converters/VerifierTests.java new file mode 100644 index 000000000..f35b7e180 --- /dev/null +++ b/src/test/java/org/apache/commons/cli/converters/VerifierTests.java @@ -0,0 +1,86 @@ +/* + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ +package org.apache.commons.cli.converters; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; + +/** + * Tests for standard Verifiers. + */ +public class VerifierTests { + + /** + * Verifies number formats + */ + @Test + public void numberTests() { + // test good numbers + for (String s : new String[] { "123", "12.3", "-123", "-12.3", ".3", "-.3" }) { + assertTrue(Verifier.NUMBER.test(s), s); + } + + // test bad numbers + for (String s : new String[] { "0x5F", "2,3", "1.2.3"}) { + assertFalse(Verifier.NUMBER.test(s), s); + } + } + + /** + * Verifies class names + */ + @Test + public void classTests() { + String testName = this.getClass().getName(); + assertTrue(Verifier.CLASS.test(testName), testName); + assertTrue(Verifier.CLASS.test(this.getClass().getCanonicalName()), this.getClass().getCanonicalName()); + assertTrue(Verifier.CLASS.test(this.getClass().getSimpleName()), this.getClass().getSimpleName()); + assertTrue(Verifier.CLASS.test(this.getClass().getTypeName()), this.getClass().getTypeName()); + + // test bad prefixes + for (String s : new String[] { "1", "-", "." }) { + assertFalse(Verifier.CLASS.test(s + testName), s + testName); + } + + // test good prefixes + for (String s : new String[] { "$" }) { + assertTrue(Verifier.CLASS.test(s + testName), s + testName); + } + + // test bad suffixes + for (String s : new String[] { "-", "." }) { + assertFalse(Verifier.CLASS.test(testName + s), testName + s); + } + + // test good suffixes + for (String s : new String[] { "1", "$" }) { + assertTrue(Verifier.CLASS.test(testName + s), testName + s); + } + + // test bad infixes + for (String s : new String[] { "..", "-" }) { + assertFalse(Verifier.CLASS.test("foo" + s + "bar"), "foo" + s + "bar"); + } + + // test good infixes + for (String s : new String[] { "$", "_" }) { + assertTrue(Verifier.CLASS.test("foo" + s + "bar"), "foo" + s + "bar"); + } + } +} From 9ae4c9935c93dbacd9b0a2a007fd3316226514cf Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Wed, 3 Jan 2024 12:15:34 +0100 Subject: [PATCH 05/24] fixed checkstyle issues --- .../java/org/apache/commons/cli/Option.java | 10 ++-- .../org/apache/commons/cli/OptionBuilder.java | 4 +- .../apache/commons/cli/ParseException.java | 2 +- .../commons/cli/PatternOptionBuilder.java | 6 ++- .../org/apache/commons/cli/TypeHandler.java | 4 +- .../commons/cli/converters/Converter.java | 46 ++++++++----------- .../commons/cli/converters/Verifier.java | 10 ++-- .../org/apache/commons/cli/OptionTest.java | 2 +- .../commons/cli/converters/VerifierTests.java | 20 ++++---- 9 files changed, 51 insertions(+), 53 deletions(-) diff --git a/src/main/java/org/apache/commons/cli/Option.java b/src/main/java/org/apache/commons/cli/Option.java index bec2eda5e..88d16d6c5 100644 --- a/src/main/java/org/apache/commons/cli/Option.java +++ b/src/main/java/org/apache/commons/cli/Option.java @@ -87,10 +87,10 @@ public static final class Builder { private char valueSeparator; /** The converter to convert to type **/ - private Converter converter = null; + private Converter converter; /** The verifier to verify string is valid for option */ - private Verifier verifier = null; + private Verifier verifier; /** * Constructs a new {@code Builder} with the minimum required parameters for an {@code Option} instance. @@ -361,8 +361,10 @@ public static Builder builder(final String option) { /** The character that is the value separator. */ private char valuesep; + /** The converter for this option */ private Converter converter = Converter.DEFAULT; + /** The verifier for this option */ private Verifier verifier = Verifier.DEFAULT; /** @@ -910,7 +912,7 @@ public void setValueSeparator(final char sep) { * Gets the value to type converter. * @return the value to type converter */ - public Converter getConverter() { + public Converter getConverter() { return converter; } @@ -918,7 +920,7 @@ public Converter getConverter() { * Sets the value to type converter. * @param converter The converter to convert the string value to the type. */ - public void setConverter(Converter converter) { + public void setConverter(Converter converter) { this.converter = converter; } diff --git a/src/main/java/org/apache/commons/cli/OptionBuilder.java b/src/main/java/org/apache/commons/cli/OptionBuilder.java index d3655f4e2..2125fbb94 100644 --- a/src/main/java/org/apache/commons/cli/OptionBuilder.java +++ b/src/main/java/org/apache/commons/cli/OptionBuilder.java @@ -109,8 +109,8 @@ public static Option create(final String opt) throws IllegalArgumentException { option.setOptionalArg(optionalArg); option.setArgs(argCount); option.setType(type); - option.setConverter( TypeHandler.getConverter(type)); - option.setVerifier( TypeHandler.getVerifier(type)); + option.setConverter(TypeHandler.getConverter(type)); + option.setVerifier(TypeHandler.getVerifier(type)); option.setValueSeparator(valueSeparator); option.setArgName(argName); } finally { diff --git a/src/main/java/org/apache/commons/cli/ParseException.java b/src/main/java/org/apache/commons/cli/ParseException.java index 331633cb1..0afe6094a 100644 --- a/src/main/java/org/apache/commons/cli/ParseException.java +++ b/src/main/java/org/apache/commons/cli/ParseException.java @@ -40,7 +40,7 @@ public class ParseException extends Exception { */ public static ParseException wrap(final Exception e) throws UnsupportedOperationException { if (e instanceof UnsupportedOperationException) { - throw (UnsupportedOperationException)e; + throw (UnsupportedOperationException) e; } if (e instanceof ParseException) { diff --git a/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java b/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java index 3fd39f7c6..b63caddf1 100644 --- a/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java +++ b/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java @@ -19,7 +19,6 @@ Licensed to the Apache Software Foundation (ASF) under one or more import java.io.File; import java.io.FileInputStream; -import java.io.FileNotFoundException; import java.net.URL; import java.util.Date; @@ -107,7 +106,10 @@ public class PatternOptionBuilder { /** URL class */ public static final Class URL_VALUE = URL.class; - static final Converter NOT_IMPLEMENTED = s -> {throw new UnsupportedOperationException("Not yet implemented");}; + /** The converter to use for Unimplemented data types */ + static final Converter NOT_IMPLEMENTED = s -> { + throw new UnsupportedOperationException("Not yet implemented"); + }; static { TypeHandler.register(PatternOptionBuilder.FILES_VALUE, NOT_IMPLEMENTED, null); diff --git a/src/main/java/org/apache/commons/cli/TypeHandler.java b/src/main/java/org/apache/commons/cli/TypeHandler.java index 48638df49..9029421c2 100644 --- a/src/main/java/org/apache/commons/cli/TypeHandler.java +++ b/src/main/java/org/apache/commons/cli/TypeHandler.java @@ -19,7 +19,6 @@ Licensed to the Apache Software Foundation (ASF) under one or more import java.io.File; import java.io.FileInputStream; -import java.io.FileNotFoundException; import java.math.BigDecimal; import java.math.BigInteger; import java.net.URL; @@ -38,7 +37,10 @@ Licensed to the Apache Software Foundation (ASF) under one or more */ public class TypeHandler { + /** Map of classes to converters. */ private static Map, Converter> converterMap = new HashMap<>(); + + /** Map of classes to verifiers. */ private static Map, Verifier> verifierMap = new HashMap<>(); static { diff --git a/src/main/java/org/apache/commons/cli/converters/Converter.java b/src/main/java/org/apache/commons/cli/converters/Converter.java index ce858295e..d4f08efed 100644 --- a/src/main/java/org/apache/commons/cli/converters/Converter.java +++ b/src/main/java/org/apache/commons/cli/converters/Converter.java @@ -17,16 +17,9 @@ Licensed to the Apache Software Foundation (ASF) under one or more package org.apache.commons.cli.converters; import java.io.File; -import java.net.MalformedURLException; import java.net.URL; -import java.time.ZonedDateTime; -import java.time.format.DateTimeFormatter; -import java.util.Date; -import java.util.function.Function; -import java.util.function.Supplier; import java.text.SimpleDateFormat; - -import org.apache.commons.cli.ParseException; +import java.util.Date; /** * The definition of the functional interface to call when doing a conversion. @@ -38,25 +31,25 @@ Licensed to the Apache Software Foundation (ASF) under one or more public interface Converter { /** - * The default converter. Does nothing. + * The default converter. Does nothing. */ - static final Converter DEFAULT = s -> s; + Converter DEFAULT = s -> s; /** - * Class name converter. Calls {@code Class.forName}. + * Class name converter. Calls {@code Class.forName}. */ - static final Converter> CLASS = s -> Class.forName(s); + Converter> CLASS = s -> Class.forName(s); /** - * File name converter. Calls @{code new File(s)} + * File name converter. Calls @{code new File(s)} */ - static final Converter FILE = s -> new File(s); + Converter FILE = s -> new File(s); /** - * Number converter. Converts to a Double if a decimal point ('.') is in the string or - * a Long otherwise. + * Number converter. Converts to a Double if a decimal point ('.') is in the + * string or a Long otherwise. */ - static final Converter NUMBER = s -> { + Converter NUMBER = s -> { if (s.indexOf('.') != -1) { return Double.valueOf(s); } @@ -64,29 +57,28 @@ public interface Converter { }; /** - * Converts a class name to an instance of the class. Uses the Class converter to - * find the class and then call the default constructor. + * Converts a class name to an instance of the class. Uses the Class converter + * to find the class and then call the default constructor. * @see #CLASS */ - static final Converter OBJECT = s -> CLASS.apply(s).getConstructor().newInstance(); + Converter OBJECT = s -> CLASS.apply(s).getConstructor().newInstance(); /** - * Creates a URL. Calls {@code new URL(s)}. + * Creates a URL. Calls {@code new URL(s)}. */ - static final Converter URL = s -> new URL(s); + Converter URL = s -> new URL(s); /** - * The simple date format used for the Date conversion. - * Format is: "EEE MMM dd HH:mm:ss zzz yyyy" + * The simple date format used for the Date conversion. Format is: "EEE MMM dd + * HH:mm:ss zzz yyyy" */ - static final SimpleDateFormat DATE_FMT = new SimpleDateFormat("EEE MMM dd HH:mm:ss zzz yyyy"); + SimpleDateFormat DATE_FMT = new SimpleDateFormat("EEE MMM dd HH:mm:ss zzz yyyy"); /** * Converts to a date using the {@code DATE_FMT}. * @see #DATE_FMT */ - static final Converter DATE = s -> DATE_FMT.parse(s); - + Converter DATE = s -> DATE_FMT.parse(s); /** * Applies the conversion function to the String argument. diff --git a/src/main/java/org/apache/commons/cli/converters/Verifier.java b/src/main/java/org/apache/commons/cli/converters/Verifier.java index 0a46c1e53..550312370 100644 --- a/src/main/java/org/apache/commons/cli/converters/Verifier.java +++ b/src/main/java/org/apache/commons/cli/converters/Verifier.java @@ -29,29 +29,29 @@ public interface Verifier { /** * Default verifier. Always returns {@code true}. */ - static final Verifier DEFAULT = (s) -> true; + Verifier DEFAULT = s -> true; /** * The Regex Pattern for the number matching. */ - static Pattern NUMBER_PATTERN = Pattern.compile("-?([0-9]*\\.)?([0-9]+)$"); + Pattern NUMBER_PATTERN = Pattern.compile("-?([0-9]*\\.)?([0-9]+)$"); /** * Verifies that a number is either a valid real number (e.g. may have a decimal * point). */ - static final Verifier NUMBER = s -> NUMBER_PATTERN.matcher(s).matches(); + Verifier NUMBER = s -> NUMBER_PATTERN.matcher(s).matches(); /** * The Regex Pattern that matches valid class names. */ - static Pattern CLAZZ_PATTERN = Pattern.compile( + Pattern CLAZZ_PATTERN = Pattern.compile( "\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*(\\.\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*)*"); /** * Verifies that a class name is valid. */ - static final Verifier CLASS = s -> CLAZZ_PATTERN.matcher(s).matches(); + Verifier CLASS = s -> CLAZZ_PATTERN.matcher(s).matches(); /** * Applies the verification function to the String argument. diff --git a/src/test/java/org/apache/commons/cli/OptionTest.java b/src/test/java/org/apache/commons/cli/OptionTest.java index 1f542eca3..4d92253cb 100644 --- a/src/test/java/org/apache/commons/cli/OptionTest.java +++ b/src/test/java/org/apache/commons/cli/OptionTest.java @@ -58,7 +58,7 @@ public boolean addValue(final String value) { addValueForProcessing(value); return true; } catch (ParseException e) { - throw new RuntimeException( e ); + throw new RuntimeException(e); } } } diff --git a/src/test/java/org/apache/commons/cli/converters/VerifierTests.java b/src/test/java/org/apache/commons/cli/converters/VerifierTests.java index f35b7e180..a5d27237b 100644 --- a/src/test/java/org/apache/commons/cli/converters/VerifierTests.java +++ b/src/test/java/org/apache/commons/cli/converters/VerifierTests.java @@ -32,12 +32,12 @@ public class VerifierTests { @Test public void numberTests() { // test good numbers - for (String s : new String[] { "123", "12.3", "-123", "-12.3", ".3", "-.3" }) { + for (String s : new String[] {"123", "12.3", "-123", "-12.3", ".3", "-.3"}) { assertTrue(Verifier.NUMBER.test(s), s); } - - // test bad numbers - for (String s : new String[] { "0x5F", "2,3", "1.2.3"}) { + + // test bad numbers + for (String s : new String[] {"0x5F", "2,3", "1.2.3"}) { assertFalse(Verifier.NUMBER.test(s), s); } } @@ -54,32 +54,32 @@ public void classTests() { assertTrue(Verifier.CLASS.test(this.getClass().getTypeName()), this.getClass().getTypeName()); // test bad prefixes - for (String s : new String[] { "1", "-", "." }) { + for (String s : new String[] {"1", "-", "."}) { assertFalse(Verifier.CLASS.test(s + testName), s + testName); } // test good prefixes - for (String s : new String[] { "$" }) { + for (String s : new String[] {"$"}) { assertTrue(Verifier.CLASS.test(s + testName), s + testName); } // test bad suffixes - for (String s : new String[] { "-", "." }) { + for (String s : new String[] {"-", "."}) { assertFalse(Verifier.CLASS.test(testName + s), testName + s); } // test good suffixes - for (String s : new String[] { "1", "$" }) { + for (String s : new String[] {"1", "$"}) { assertTrue(Verifier.CLASS.test(testName + s), testName + s); } // test bad infixes - for (String s : new String[] { "..", "-" }) { + for (String s : new String[] {"..", "-"}) { assertFalse(Verifier.CLASS.test("foo" + s + "bar"), "foo" + s + "bar"); } // test good infixes - for (String s : new String[] { "$", "_" }) { + for (String s : new String[] {"$", "_"}) { assertTrue(Verifier.CLASS.test("foo" + s + "bar"), "foo" + s + "bar"); } } From b5d221205845017fe569c65473f1e9aa17f21d69 Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Wed, 3 Jan 2024 13:33:53 +0100 Subject: [PATCH 06/24] fixed spotbugs error & some javadoc --- .../java/org/apache/commons/cli/Option.java | 20 +++++-------- .../commons/cli/converters/Converter.java | 29 ++++--------------- .../cli/converters/ConverterTests.java | 4 +-- 3 files changed, 15 insertions(+), 38 deletions(-) diff --git a/src/main/java/org/apache/commons/cli/Option.java b/src/main/java/org/apache/commons/cli/Option.java index 88d16d6c5..a164ecc48 100644 --- a/src/main/java/org/apache/commons/cli/Option.java +++ b/src/main/java/org/apache/commons/cli/Option.java @@ -362,10 +362,10 @@ public static Builder builder(final String option) { private char valuesep; /** The converter for this option */ - private Converter converter = Converter.DEFAULT; + private transient Converter converter = Converter.DEFAULT; /** The verifier for this option */ - private Verifier verifier = Verifier.DEFAULT; + private transient Verifier verifier = Verifier.DEFAULT; /** * Private constructor used by the nested Builder class. @@ -382,8 +382,8 @@ private Option(final Builder builder) { this.required = builder.required; this.type = builder.type; this.valuesep = builder.valueSeparator; - this.converter = builder.converter == null ? Converter.DEFAULT : builder.converter; - this.verifier = builder.verifier == null ? Verifier.DEFAULT : builder.verifier; + this.converter = builder.converter; + this.verifier = builder.verifier; } /** @@ -457,7 +457,7 @@ private void add(final String value) throws ParseException { throw new IllegalArgumentException("Cannot add value, list full."); } - if (!verifier.test(value)) { + if (!getVerifier().test(value)) { throw new ParseException(String.format("'%s' is not a valid input string for option '%s'", value, option)); } @@ -876,12 +876,6 @@ public void setRequired(final boolean required) { */ public void setType(final Class type) { this.type = type; - if (verifier == null || verifier == Verifier.DEFAULT) { - verifier = TypeHandler.getVerifier(type); - } - if (converter == null || converter == Converter.DEFAULT) { - converter = TypeHandler.getConverter(type); - } } /** @@ -913,7 +907,7 @@ public void setValueSeparator(final char sep) { * @return the value to type converter */ public Converter getConverter() { - return converter; + return converter == null ? TypeHandler.getConverter(type) : converter; } /** @@ -929,7 +923,7 @@ public void setConverter(Converter converter) { * @return the value verifier. */ public Verifier getVerifier() { - return verifier; + return verifier == null ? TypeHandler.getVerifier(type) : verifier; } /** diff --git a/src/main/java/org/apache/commons/cli/converters/Converter.java b/src/main/java/org/apache/commons/cli/converters/Converter.java index d4f08efed..4f654973c 100644 --- a/src/main/java/org/apache/commons/cli/converters/Converter.java +++ b/src/main/java/org/apache/commons/cli/converters/Converter.java @@ -30,19 +30,13 @@ Licensed to the Apache Software Foundation (ASF) under one or more @FunctionalInterface public interface Converter { - /** - * The default converter. Does nothing. - */ + /** The default converter. Does nothing. */ Converter DEFAULT = s -> s; - /** - * Class name converter. Calls {@code Class.forName}. - */ + /** Class name converter. Calls {@code Class.forName}. */ Converter> CLASS = s -> Class.forName(s); - /** - * File name converter. Calls @{code new File(s)} - */ + /** File name converter. Calls @{code new File(s)} */ Converter FILE = s -> new File(s); /** @@ -63,22 +57,11 @@ public interface Converter { */ Converter OBJECT = s -> CLASS.apply(s).getConstructor().newInstance(); - /** - * Creates a URL. Calls {@code new URL(s)}. - */ + /** Creates a URL. Calls {@code new URL(s)}. */ Converter URL = s -> new URL(s); - /** - * The simple date format used for the Date conversion. Format is: "EEE MMM dd - * HH:mm:ss zzz yyyy" - */ - SimpleDateFormat DATE_FMT = new SimpleDateFormat("EEE MMM dd HH:mm:ss zzz yyyy"); - - /** - * Converts to a date using the {@code DATE_FMT}. - * @see #DATE_FMT - */ - Converter DATE = s -> DATE_FMT.parse(s); + /** Converts to a date using the format string Form "EEE MMM dd HH:mm:ss zzz yyyy". */ + Converter DATE = s -> new SimpleDateFormat("EEE MMM dd HH:mm:ss zzz yyyy").parse(s); /** * Applies the conversion function to the String argument. diff --git a/src/test/java/org/apache/commons/cli/converters/ConverterTests.java b/src/test/java/org/apache/commons/cli/converters/ConverterTests.java index cc361531e..0a1bf53be 100644 --- a/src/test/java/org/apache/commons/cli/converters/ConverterTests.java +++ b/src/test/java/org/apache/commons/cli/converters/ConverterTests.java @@ -99,7 +99,7 @@ public void dateTests() throws Exception { @Test public void urlTests() throws Exception { - URL url = this.getClass().getClassLoader().getResource("org/apache/commons/cli/existing-readable.file"); + URL url = this.getClass().getClassLoader().getResource("./org/apache/commons/cli/existing-readable.file"); assertEquals(url, Converter.URL.apply(url.toString())); assertEquals(new URL("http://apache.org"), Converter.URL.apply("http://apache.org")); @@ -109,7 +109,7 @@ public void urlTests() throws Exception { @Test public void fileTests() throws Exception { - URL url = this.getClass().getClassLoader().getResource("org/apache/commons/cli/existing-readable.file"); + URL url = this.getClass().getClassLoader().getResource("./org/apache/commons/cli/existing-readable.file"); String fileName = url.toString().substring("file:".length()); assertNotNull(Converter.FILE.apply(fileName)); } From e36f2828f93694118352fd3a7e55683686952f73 Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Wed, 3 Jan 2024 15:32:47 +0100 Subject: [PATCH 07/24] Updated spotbugs to 4.8.2.0 --- pom.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/pom.xml b/pom.xml index 9baf6d414..ce88df218 100644 --- a/pom.xml +++ b/pom.xml @@ -251,6 +251,7 @@ ${basedir}/src/conf/spotbugs-exclude-filter.xml + 4.8.2.0 org.apache.maven.plugins From 178785985591e1df92f3bfefc7dd22ecdb848b37 Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Thu, 4 Jan 2024 11:54:01 +0100 Subject: [PATCH 08/24] added javadocs and tests --- .../java/org/apache/commons/cli/Option.java | 28 +- .../apache/commons/cli/ParseException.java | 15 +- .../commons/cli/PatternOptionBuilder.java | 19 +- .../org/apache/commons/cli/TypeHandler.java | 78 +++++- .../commons/cli/converters/Verifier.java | 14 +- .../org/apache/commons/cli/OptionTest.java | 138 ++++++---- .../org/apache/commons/cli/OptionsTest.java | 27 +- .../apache/commons/cli/TypeHandlerTest.java | 246 ++++++++++++------ .../cli/converters/ConverterTests.java | 4 - .../commons/cli/converters/VerifierTests.java | 16 ++ 10 files changed, 402 insertions(+), 183 deletions(-) diff --git a/src/main/java/org/apache/commons/cli/Option.java b/src/main/java/org/apache/commons/cli/Option.java index a164ecc48..270b4d592 100644 --- a/src/main/java/org/apache/commons/cli/Option.java +++ b/src/main/java/org/apache/commons/cli/Option.java @@ -243,12 +243,6 @@ public Builder required(final boolean required) { */ public Builder type(final Class type) { this.type = type; - if (verifier == null) { - verifier = TypeHandler.getVerifier(type); - } - if (converter == null) { - converter = TypeHandler.getConverter(type); - } return this; } @@ -285,12 +279,24 @@ public Builder valueSeparator(final char valueSeparator) { this.valueSeparator = valueSeparator; return this; } - + + /** + * Sets the converter for the option. + *

Note: see {@link TypeHandler} for serialization discussion.

+ * @param converter the Converter to use. + * @return this builder, to allow method chaining. + */ public Builder converter(Converter converter) { this.converter = converter; return this; } + /** + * Sets the verifier for the option. + *

Note: see {@link TypeHandler} for serialization discussion.

+ * @param verifier the Verifier to use. + * @return this builder, to allow method chaining. + */ public Builder verifier(Verifier verifier) { this.verifier = verifier; return this; @@ -361,11 +367,11 @@ public static Builder builder(final String option) { /** The character that is the value separator. */ private char valuesep; - /** The converter for this option */ - private transient Converter converter = Converter.DEFAULT; + /** The explicit converter for this option. May be null */ + private transient Converter converter; - /** The verifier for this option */ - private transient Verifier verifier = Verifier.DEFAULT; + /** The explicit verifier for this option. May be null */ + private transient Verifier verifier; /** * Private constructor used by the nested Builder class. diff --git a/src/main/java/org/apache/commons/cli/ParseException.java b/src/main/java/org/apache/commons/cli/ParseException.java index 0afe6094a..fd8f936b3 100644 --- a/src/main/java/org/apache/commons/cli/ParseException.java +++ b/src/main/java/org/apache/commons/cli/ParseException.java @@ -28,11 +28,11 @@ public class ParseException extends Exception { private static final long serialVersionUID = 9112808380089253192L; /** - * Converts any exception except UnsupportedOperationException to a ParseException. - * if {@code e} is an instance of ParseException it is returned, otherwise a ParseException is + * Converts any exception except {@code UnsupportedOperationException} to a {@code ParseException}. + * if {@code e} is an instance of {@code ParseException} it is returned, otherwise a {@code ParseException} is * created that wraps it. *

- * Note: UnsupportedOperationExceptions are not wrapped. This is to solve a legacy expected exception problem and will be + * Note: {@code UnsupportedOperationException} are not wrapped. This is to solve a legacy expected exception problem and will be * removed in the future.

* @param e the exception to convert. * @return the ParseException. @@ -57,12 +57,11 @@ public ParseException(final String message) { super(message); } + /** + * Constructs a new {@code ParseException} wrapping the specified exception. + * @param e the Exception to wrap. + */ public ParseException(final Exception e) { super(e); } - - public ParseException(String message, Throwable e) { - super(message, e); - } - } diff --git a/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java b/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java index b63caddf1..a01c65607 100644 --- a/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java +++ b/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java @@ -126,6 +126,12 @@ public static Object getValueClass(final char ch) { return getValueType(ch); } + /** + * Retrieve the class that {@code ch} represents. + * + * @param ch the specified character + * @return The class that {@code ch} represents + */ public static Class getValueType(final char ch) { switch (ch) { case '@': @@ -151,15 +157,6 @@ public static Class getValueType(final char ch) { return null; } - public static Converter getConverter(final char ch) { - return TypeHandler.getConverter(getValueType(ch)); - } - - public static Verifier getVerifier(final char ch) { - return TypeHandler.getVerifier(getValueType(ch)); - } - - /** * Returns whether {@code ch} is a value code, i.e. whether it represents a class in a pattern. * @@ -208,8 +205,8 @@ public static Options parsePattern(final String pattern) { required = true; } else { type = getValueType(ch); - converter = getConverter(ch); - verifier = getVerifier(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 index 9029421c2..9667ca0f2 100644 --- a/src/main/java/org/apache/commons/cli/TypeHandler.java +++ b/src/main/java/org/apache/commons/cli/TypeHandler.java @@ -30,12 +30,21 @@ Licensed to the Apache Software Foundation (ASF) under one or more import org.apache.commons.cli.converters.Verifier; /** - * This is a temporary implementation. TypeHandler will handle the pluggableness - * of OptionTypes and it will direct all of these types of conversion - * functionalities to ConvertUtils component in Commons already. BeanUtils I - * think. + * TypeHandler will handle the pluggable conversion and verification of + * Option types. It handles the mapping of classes to bot converters and verifiers. + * It provides the default conversion and verification methods when converters and verifiers + * are not explicitly set. + *

+ * If Options are serialized and deserialized their converters and verifiers will revert to the + * defaults defined in this class. To correctly de-serialize Options with custom converters and/or + * verifiers, using the default serialization methods, this class should be properly configured with the custom + * converters and verifiers for the specific class. + *

*/ public class TypeHandler { + + /** Value of hex conversion of strings */ + private static final int HEX_RADIX = 16; /** Map of classes to converters. */ private static Map, Converter> converterMap = new HashMap<>(); @@ -47,7 +56,10 @@ public class TypeHandler { resetConverters(); resetVerifiers(); } - + + /** + * Resets the registered Converters to the default state. + */ public static void resetConverters() { converterMap.clear(); converterMap.put(Object.class, Converter.OBJECT); @@ -61,31 +73,61 @@ public static void resetConverters() { converterMap.put(Integer.class, Integer::parseInt); converterMap.put(Short.class, Short::parseShort); converterMap.put(Byte.class, Byte::parseByte); - converterMap.put(Character.class, s -> s.charAt(0)); + converterMap.put(Character.class, s -> { + if (s.startsWith("\\u")) { + return Character.toChars(Integer.parseInt(s.substring(2), HEX_RADIX))[0]; + } else { + return s.charAt(0); + } }); converterMap.put(Double.class, Double::parseDouble); converterMap.put(Float.class, Float::parseFloat); converterMap.put(BigInteger.class, s -> new BigInteger(s)); converterMap.put(BigDecimal.class, s -> new BigDecimal(s)); } + + /** + * Unregisters all Converters. + */ + public static void noConverters() { + converterMap.clear(); + } + /** + * Resets the registered Verifiers to the default state. + */ public static void resetVerifiers() { verifierMap.clear(); verifierMap.put(Object.class, Verifier.CLASS); verifierMap.put(Class.class, Verifier.CLASS); verifierMap.put(Number.class, Verifier.NUMBER); - Verifier intVerifier = s -> s.matches("-?\\d+"); - verifierMap.put(Long.class, intVerifier); - verifierMap.put(Integer.class, intVerifier); - verifierMap.put(Short.class, intVerifier); - verifierMap.put(Byte.class, intVerifier); + 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, intVerifier); + verifierMap.put(BigInteger.class, Verifier.INTEGER); verifierMap.put(BigDecimal.class, Verifier.NUMBER); } + /** + * Unregisters all Verifiers. + */ + 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. + * + * @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. + */ public static void register(Class clazz, Converter converter, Verifier verifier) { if (converter == null) { converterMap.remove(clazz); @@ -100,11 +142,21 @@ public static void register(Class clazz, Converter converter, Verifier ver } } + /** + * Gets the converter for the the Class. Never null. + * @param clazz The Class to get the Converter for. + * @return the registered converter if any, {@link Converter#DEFAULT} otherwise. + */ public static Converter getConverter(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. + */ public static Verifier getVerifier(Class clazz) { Verifier verifier = verifierMap.get(clazz); return verifier == null ? Verifier.DEFAULT : verifier; @@ -223,7 +275,7 @@ public static URL createURL(final String str) throws ParseException { * @param type of argument * @return The instance of {@code clazz} initialized with the * value of {@code str}. - * @throws ParseException if the value creation for the given class failed + * @throws ParseException if the value creation for the given class threw an exception. */ @SuppressWarnings("unchecked") // returned value will have type T because it is fixed by clazz public static T createValue(final String str, final Class clazz) throws ParseException { diff --git a/src/main/java/org/apache/commons/cli/converters/Verifier.java b/src/main/java/org/apache/commons/cli/converters/Verifier.java index 550312370..0618ec41b 100644 --- a/src/main/java/org/apache/commons/cli/converters/Verifier.java +++ b/src/main/java/org/apache/commons/cli/converters/Verifier.java @@ -37,10 +37,20 @@ public interface Verifier { Pattern NUMBER_PATTERN = Pattern.compile("-?([0-9]*\\.)?([0-9]+)$"); /** - * Verifies that a number is either a valid real number (e.g. may have a decimal - * point). + * Verifies that a number string is either a valid real number (e.g. may have a decimal + * point) or an integer. */ Verifier NUMBER = s -> NUMBER_PATTERN.matcher(s).matches(); + + /** + * The Regex Pattern for the integer matching. + */ + Pattern INTEGER_PATTERN = Pattern.compile("-?\\d+"); + + /** + * Verifies that a number string is an integer. + */ + Verifier INTEGER = s -> INTEGER_PATTERN.matcher(s).matches(); /** * The Regex Pattern that matches valid class names. diff --git a/src/test/java/org/apache/commons/cli/OptionTest.java b/src/test/java/org/apache/commons/cli/OptionTest.java index 4d92253cb..d049eba34 100644 --- a/src/test/java/org/apache/commons/cli/OptionTest.java +++ b/src/test/java/org/apache/commons/cli/OptionTest.java @@ -25,6 +25,14 @@ import static org.junit.Assert.assertTrue; import static org.junit.jupiter.api.Assertions.assertThrows; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; + +import org.apache.commons.cli.converters.Converter; +import org.apache.commons.cli.converters.Verifier; import org.junit.Test; public class OptionTest { @@ -34,7 +42,8 @@ private static final class DefaultOption extends Option { private final String defaultValue; - DefaultOption(final String opt, final String description, final String defaultValue) throws IllegalArgumentException { + DefaultOption(final String opt, final String description, final String defaultValue) + throws IllegalArgumentException { super(opt, true, description); this.defaultValue = defaultValue; } @@ -55,16 +64,17 @@ private static final class TestOption extends Option { @Override public boolean addValue(final String value) { try { - addValueForProcessing(value); - return true; + addValueForProcessing(value); + return true; } catch (ParseException e) { throw new RuntimeException(e); } } } - private static void checkOption(final Option option, final String opt, final String description, final String longOpt, final int numArgs, - final String argName, final boolean required, final boolean optionalArg, final char valueSeparator, final Class cls) { + private static void checkOption(final Option option, final String opt, final String description, + final String longOpt, final int numArgs, final String argName, final boolean required, + final boolean optionalArg, final char valueSeparator, final Class cls) { assertEquals(opt, option.getOpt()); assertEquals(description, option.getDescription()); assertEquals(longOpt, option.getLongOpt()); @@ -79,70 +89,69 @@ private static void checkOption(final Option option, final String opt, final Str @Test public void testBuilderInsufficientParams1() { - assertThrows(IllegalArgumentException.class, () -> - Option.builder().desc("desc").build()); + assertThrows(IllegalArgumentException.class, () -> Option.builder().desc("desc").build()); } @Test public void testBuilderInsufficientParams2() { - assertThrows(IllegalArgumentException.class, () -> - Option.builder(null).desc("desc").build()); + assertThrows(IllegalArgumentException.class, () -> Option.builder(null).desc("desc").build()); } @Test public void testBuilderInvalidOptionName1() { - assertThrows(IllegalArgumentException.class, () -> - Option.builder().option("invalid?")); + assertThrows(IllegalArgumentException.class, () -> Option.builder().option("invalid?")); } @Test public void testBuilderInvalidOptionName2() { - assertThrows(IllegalArgumentException.class, () -> - Option.builder().option("invalid@")); + assertThrows(IllegalArgumentException.class, () -> Option.builder().option("invalid@")); } @Test public void testBuilderInvalidOptionName3() { - assertThrows(IllegalArgumentException.class, () -> - Option.builder("invalid?")); + assertThrows(IllegalArgumentException.class, () -> Option.builder("invalid?")); } @Test public void testBuilderInvalidOptionName4() { - assertThrows(IllegalArgumentException.class, () -> - Option.builder("invalid@")); + assertThrows(IllegalArgumentException.class, () -> Option.builder("invalid@")); } @Test public void testBuilderMethods() { final char defaultSeparator = (char) 0; - checkOption(Option.builder("a").desc("desc").build(), "a", "desc", null, Option.UNINITIALIZED, null, false, false, defaultSeparator, String.class); - checkOption(Option.builder("a").desc("desc").build(), "a", "desc", null, Option.UNINITIALIZED, null, false, false, defaultSeparator, String.class); - checkOption(Option.builder("a").desc("desc").longOpt("aaa").build(), "a", "desc", "aaa", Option.UNINITIALIZED, null, false, false, defaultSeparator, - String.class); - checkOption(Option.builder("a").desc("desc").hasArg(true).build(), "a", "desc", null, 1, null, false, false, defaultSeparator, String.class); - checkOption(Option.builder("a").desc("desc").hasArg(false).build(), "a", "desc", null, Option.UNINITIALIZED, null, false, false, defaultSeparator, - String.class); - checkOption(Option.builder("a").desc("desc").hasArg(true).build(), "a", "desc", null, 1, null, false, false, defaultSeparator, String.class); - checkOption(Option.builder("a").desc("desc").numberOfArgs(3).build(), "a", "desc", null, 3, null, false, false, defaultSeparator, String.class); - checkOption(Option.builder("a").desc("desc").required(true).build(), "a", "desc", null, Option.UNINITIALIZED, null, true, false, defaultSeparator, - String.class); - checkOption(Option.builder("a").desc("desc").required(false).build(), "a", "desc", null, Option.UNINITIALIZED, null, false, false, defaultSeparator, - String.class); - - checkOption(Option.builder("a").desc("desc").argName("arg1").build(), "a", "desc", null, Option.UNINITIALIZED, "arg1", false, false, defaultSeparator, - String.class); - checkOption(Option.builder("a").desc("desc").optionalArg(false).build(), "a", "desc", null, Option.UNINITIALIZED, null, false, false, defaultSeparator, - String.class); - checkOption(Option.builder("a").desc("desc").optionalArg(true).build(), "a", "desc", null, 1, null, false, true, defaultSeparator, - String.class); - checkOption(Option.builder("a").desc("desc").valueSeparator(':').build(), "a", "desc", null, Option.UNINITIALIZED, null, false, false, ':', - String.class); - checkOption(Option.builder("a").desc("desc").type(Integer.class).build(), "a", "desc", null, Option.UNINITIALIZED, null, false, false, defaultSeparator, - Integer.class); - checkOption(Option.builder().option("a").desc("desc").type(Integer.class).build(), "a", "desc", null, Option.UNINITIALIZED, null, false, false, - defaultSeparator, Integer.class); + checkOption(Option.builder("a").desc("desc").build(), "a", "desc", null, Option.UNINITIALIZED, null, false, + false, defaultSeparator, String.class); + checkOption(Option.builder("a").desc("desc").build(), "a", "desc", null, Option.UNINITIALIZED, null, false, + false, defaultSeparator, String.class); + checkOption(Option.builder("a").desc("desc").longOpt("aaa").build(), "a", "desc", "aaa", Option.UNINITIALIZED, + null, false, false, defaultSeparator, String.class); + checkOption(Option.builder("a").desc("desc").hasArg(true).build(), "a", "desc", null, 1, null, false, false, + defaultSeparator, String.class); + checkOption(Option.builder("a").desc("desc").hasArg(false).build(), "a", "desc", null, Option.UNINITIALIZED, + null, false, false, defaultSeparator, String.class); + checkOption(Option.builder("a").desc("desc").hasArg(true).build(), "a", "desc", null, 1, null, false, false, + defaultSeparator, String.class); + checkOption(Option.builder("a").desc("desc").numberOfArgs(3).build(), "a", "desc", null, 3, null, false, false, + defaultSeparator, String.class); + checkOption(Option.builder("a").desc("desc").required(true).build(), "a", "desc", null, Option.UNINITIALIZED, + null, true, false, defaultSeparator, String.class); + checkOption(Option.builder("a").desc("desc").required(false).build(), "a", "desc", null, Option.UNINITIALIZED, + null, false, false, defaultSeparator, String.class); + + checkOption(Option.builder("a").desc("desc").argName("arg1").build(), "a", "desc", null, Option.UNINITIALIZED, + "arg1", false, false, defaultSeparator, String.class); + checkOption(Option.builder("a").desc("desc").optionalArg(false).build(), "a", "desc", null, + Option.UNINITIALIZED, null, false, false, defaultSeparator, String.class); + checkOption(Option.builder("a").desc("desc").optionalArg(true).build(), "a", "desc", null, 1, null, false, true, + defaultSeparator, String.class); + checkOption(Option.builder("a").desc("desc").valueSeparator(':').build(), "a", "desc", null, + Option.UNINITIALIZED, null, false, false, ':', String.class); + checkOption(Option.builder("a").desc("desc").type(Integer.class).build(), "a", "desc", null, + Option.UNINITIALIZED, null, false, false, defaultSeparator, Integer.class); + checkOption(Option.builder().option("a").desc("desc").type(Integer.class).build(), "a", "desc", null, + Option.UNINITIALIZED, null, false, false, defaultSeparator, Integer.class); } @Test @@ -225,7 +234,8 @@ public void testHasArgs() { public void testHashCode() { assertNotEquals(Option.builder("test").build().hashCode(), Option.builder("test2").build().hashCode()); assertNotEquals(Option.builder("test").build().hashCode(), Option.builder().longOpt("test").build().hashCode()); - assertNotEquals(Option.builder("test").build().hashCode(), Option.builder("test").longOpt("long test").build().hashCode()); + assertNotEquals(Option.builder("test").build().hashCode(), + Option.builder("test").longOpt("long test").build().hashCode()); } @Test @@ -236,4 +246,44 @@ public void testSubclass() { assertEquals(DefaultOption.class, clone.getClass()); } + private Option roundTrip(Option o) throws IOException, ClassNotFoundException { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + ObjectOutputStream oos = new ObjectOutputStream(baos); + oos.writeObject(o); + ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray()); + ObjectInputStream ois = new ObjectInputStream(bais); + return (Option) ois.readObject(); + } + + @Test + public void testSerialization() throws IOException, ClassNotFoundException { + + Option o = Option.builder("o").type(TypeHandlerTest.Instantiable.class).build(); + assertEquals(Converter.DEFAULT, o.getConverter()); + assertEquals(Verifier.DEFAULT, o.getVerifier()); + Option o2 = roundTrip(o); + assertEquals(Converter.DEFAULT, o2.getConverter()); + assertEquals(Verifier.DEFAULT, o2.getVerifier()); + + // verify unregistered class converters and verifiers get reset to default. + o.setConverter(Converter.DATE); + o.setVerifier(Verifier.NUMBER); + o2 = roundTrip(o); + assertEquals(Converter.DEFAULT, o2.getConverter()); + assertEquals(Verifier.DEFAULT, o2.getVerifier()); + + // verify registered class converters and verifiers do not get reset to default. + try { + TypeHandler.register(TypeHandlerTest.Instantiable.class, Converter.URL, Verifier.CLASS); + // verify earlier values still set. + assertEquals(Converter.DATE, o.getConverter()); + assertEquals(Verifier.NUMBER, o.getVerifier()); + o2 = roundTrip(o); + // verify set to registered values + assertEquals(Converter.URL, o2.getConverter()); + assertEquals(Verifier.CLASS, o2.getVerifier()); + } finally { + TypeHandler.register(TypeHandlerTest.Instantiable.class, null, null); + } + } } diff --git a/src/test/java/org/apache/commons/cli/OptionsTest.java b/src/test/java/org/apache/commons/cli/OptionsTest.java index 8452cba08..3d7004f9a 100644 --- a/src/test/java/org/apache/commons/cli/OptionsTest.java +++ b/src/test/java/org/apache/commons/cli/OptionsTest.java @@ -49,8 +49,10 @@ public void testDuplicateSimple() { @Test public void testGetMatchingOpts() { final Options options = new Options(); - options.addOption(OptionBuilder.withLongOpt("version").create()); - options.addOption(OptionBuilder.withLongOpt("verbose").create()); + OptionBuilder.withLongOpt("version"); + options.addOption(OptionBuilder.create()); + OptionBuilder.withLongOpt("verbose"); + options.addOption(OptionBuilder.create()); assertTrue(options.getMatchingOptions("foo").isEmpty()); assertEquals(1, options.getMatchingOptions("version").size()); @@ -78,12 +80,16 @@ public void testGetOptionsGroups() { @Test public void testHelpOptions() { - final Option longOnly1 = OptionBuilder.withLongOpt("long-only1").create(); - final Option longOnly2 = OptionBuilder.withLongOpt("long-only2").create(); + OptionBuilder.withLongOpt("long-only1"); + final Option longOnly1 = OptionBuilder.create(); + OptionBuilder.withLongOpt("long-only2"); + final Option longOnly2 = OptionBuilder.create(); final Option shortOnly1 = OptionBuilder.create("1"); final Option shortOnly2 = OptionBuilder.create("2"); - final Option bothA = OptionBuilder.withLongOpt("bothA").create("a"); - final Option bothB = OptionBuilder.withLongOpt("bothB").create("b"); + OptionBuilder.withLongOpt("bothA"); + final Option bothA = OptionBuilder.create("a"); + OptionBuilder.withLongOpt("bothB"); + final Option bothB = OptionBuilder.create("b"); final Options options = new Options(); options.addOption(longOnly1); @@ -121,7 +127,8 @@ public void testLong() { @Test public void testMissingOptionException() throws ParseException { final Options options = new Options(); - options.addOption(OptionBuilder.isRequired().create("f")); + OptionBuilder.isRequired(); + options.addOption(OptionBuilder.create("f")); try { new PosixParser().parse(options, new String[0]); fail("Expected MissingOptionException to be thrown"); @@ -133,8 +140,10 @@ public void testMissingOptionException() throws ParseException { @Test public void testMissingOptionsException() throws ParseException { final Options options = new Options(); - options.addOption(OptionBuilder.isRequired().create("f")); - options.addOption(OptionBuilder.isRequired().create("x")); + OptionBuilder.isRequired(); + options.addOption(OptionBuilder.create("f")); + OptionBuilder.isRequired(); + options.addOption(OptionBuilder.create("x")); try { new PosixParser().parse(options, new String[0]); fail("Expected MissingOptionException to be thrown"); diff --git a/src/test/java/org/apache/commons/cli/TypeHandlerTest.java b/src/test/java/org/apache/commons/cli/TypeHandlerTest.java index 2ad866d67..370dbd8da 100644 --- a/src/test/java/org/apache/commons/cli/TypeHandlerTest.java +++ b/src/test/java/org/apache/commons/cli/TypeHandlerTest.java @@ -19,134 +19,218 @@ Licensed to the Apache Software Foundation (ASF) under one or more import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; import static org.junit.jupiter.api.Assertions.assertThrows; import java.io.File; import java.io.FileInputStream; +import java.math.BigDecimal; +import java.math.BigInteger; +import java.net.MalformedURLException; import java.net.URL; +import java.util.ArrayList; import java.util.Date; +import java.util.List; +import java.util.stream.Stream; -import org.junit.Test; +import org.apache.commons.cli.converters.Converter; +import org.apache.commons.cli.converters.Verifier; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; public class TypeHandlerTest { + /** Used for Class and Object creation tests. */ public static class Instantiable { + + @Override + public boolean equals(Object arg0) { + return arg0 instanceof Instantiable; + } + + @Override + public int hashCode() { + return 1; + } + } + + /* proof of equality for later tests */ + @Test + public void testnstantiableEquals() { + assertEquals(new Instantiable(), new Instantiable()); } + /** Used for Class and Object negative creation tests */ public static final class NotInstantiable { private NotInstantiable() { } + } @Test - public void testCreateValueClass() throws Exception { - final Object clazz = TypeHandler.createValue(Instantiable.class.getName(), PatternOptionBuilder.CLASS_VALUE); - assertEquals(Instantiable.class, clazz); + public void testRegister() { + assertEquals(Converter.DEFAULT, TypeHandler.getConverter(NotInstantiable.class)); + assertEquals(Verifier.DEFAULT, TypeHandler.getVerifier(NotInstantiable.class)); + try { + TypeHandler.register(NotInstantiable.class, Converter.DATE, Verifier.NUMBER); + assertEquals(Converter.DATE, TypeHandler.getConverter(NotInstantiable.class)); + assertEquals(Verifier.NUMBER, TypeHandler.getVerifier(NotInstantiable.class)); + } finally { + TypeHandler.register(NotInstantiable.class, null, null); + assertEquals(Converter.DEFAULT, TypeHandler.getConverter(NotInstantiable.class)); + assertEquals(Verifier.DEFAULT, TypeHandler.getVerifier(NotInstantiable.class)); + } } @Test - public void testCreateValueClass_notFound() { - assertThrows(ParseException.class, () -> - TypeHandler.createValue("what ever", PatternOptionBuilder.CLASS_VALUE)); + public void testResetConvertersAndVerifiers() { + assertEquals(Converter.DEFAULT, TypeHandler.getConverter(NotInstantiable.class)); + assertEquals(Verifier.DEFAULT, TypeHandler.getVerifier(NotInstantiable.class)); + try { + TypeHandler.register(NotInstantiable.class, Converter.DATE, Verifier.NUMBER); + assertEquals(Converter.DATE, TypeHandler.getConverter(NotInstantiable.class)); + assertEquals(Verifier.NUMBER, TypeHandler.getVerifier(NotInstantiable.class)); + TypeHandler.resetConverters(); + assertEquals(Converter.DEFAULT, TypeHandler.getConverter(NotInstantiable.class)); + assertEquals(Verifier.NUMBER, TypeHandler.getVerifier(NotInstantiable.class)); + TypeHandler.resetVerifiers(); + assertEquals(Converter.DEFAULT, TypeHandler.getConverter(NotInstantiable.class)); + assertEquals(Verifier.DEFAULT, TypeHandler.getVerifier(NotInstantiable.class)); + } finally { + TypeHandler.register(NotInstantiable.class, null, null); + } } - + @Test - public void testBadValueDate() { - assertThrows(ParseException.class, () -> - TypeHandler.createValue("what ever", PatternOptionBuilder.DATE_VALUE)); + public void testNoConverters() { + assertEquals(Converter.NUMBER, TypeHandler.getConverter(Number.class)); + assertEquals(Verifier.NUMBER, TypeHandler.getVerifier(Number.class)); + try { + TypeHandler.noConverters(); + assertEquals(Converter.DEFAULT, TypeHandler.getConverter(Number.class)); + assertEquals(Verifier.NUMBER, TypeHandler.getVerifier(Number.class)); + } finally { + TypeHandler.resetConverters(); + assertEquals(Converter.NUMBER, TypeHandler.getConverter(Number.class)); + assertEquals(Verifier.NUMBER, TypeHandler.getVerifier(Number.class)); + } } @Test - public void testCreateValueDate() throws Exception { - Date d = new Date(1023400137000L); - Date d2 = TypeHandler.createValue("Thu Jun 06 17:48:57 EDT 2002", PatternOptionBuilder.DATE_VALUE); - assertEquals(d, d2); + public void testNoVerifiers() { + assertEquals(Converter.NUMBER, TypeHandler.getConverter(Number.class)); + assertEquals(Verifier.NUMBER, TypeHandler.getVerifier(Number.class)); + try { + TypeHandler.noVerifiers(); + assertEquals(Converter.NUMBER, TypeHandler.getConverter(Number.class)); + assertEquals(Verifier.DEFAULT, TypeHandler.getVerifier(Number.class)); + } finally { + TypeHandler.resetVerifiers(); + assertEquals(Converter.NUMBER, TypeHandler.getConverter(Number.class)); + assertEquals(Verifier.NUMBER, TypeHandler.getVerifier(Number.class)); + } } - @Test public void testCreateValueExistingFile() throws Exception { - try (FileInputStream result = TypeHandler.createValue("src/test/resources/org/apache/commons/cli/existing-readable.file", - PatternOptionBuilder.EXISTING_FILE_VALUE)) { + try (FileInputStream result = TypeHandler.createValue( + "src/test/resources/org/apache/commons/cli/existing-readable.file", + PatternOptionBuilder.EXISTING_FILE_VALUE)) { assertNotNull(result); } } - @Test - public void testCreateValueExistingFile_nonExistingFile() { - assertThrows(ParseException.class, () -> - TypeHandler.createValue("non-existing.file", PatternOptionBuilder.EXISTING_FILE_VALUE)); + @SuppressWarnings("unchecked") + @ParameterizedTest(name = "{0} as {1}") + @MethodSource("createValueTestParameters") + public void createValueTests(String str, Class type, Object expected) throws Exception { + if (expected instanceof Class && Throwable.class.isAssignableFrom((Class) expected)) { + assertThrows((Class) expected, () -> TypeHandler.createValue(str, type)); + } else { + assertEquals(expected, TypeHandler.createValue(str, type)); + } } - @Test - public void testCreateValueFile() throws Exception { - final File result = TypeHandler.createValue("some-file.txt", PatternOptionBuilder.FILE_VALUE); - assertEquals("some-file.txt", result.getName()); - } + private static Stream createValueTestParameters() { + List lst = new ArrayList<>(); - @Test - public void testCreateValueFiles() { - assertThrows(UnsupportedOperationException.class, () -> - TypeHandler.createValue("some.files", PatternOptionBuilder.FILES_VALUE)); - } + try { + lst.add(Arguments.of(Instantiable.class.getName(), PatternOptionBuilder.CLASS_VALUE, Instantiable.class)); + lst.add(Arguments.of("what ever", PatternOptionBuilder.CLASS_VALUE, ParseException.class)); - @Test - public void testCreateValueInteger_failure() { - assertThrows(ParseException.class, () -> - TypeHandler.createValue("just-a-string", Integer.class)); - } + lst.add(Arguments.of("what ever", PatternOptionBuilder.DATE_VALUE, ParseException.class)); + lst.add(Arguments.of("Thu Jun 06 17:48:57 EDT 2002", PatternOptionBuilder.DATE_VALUE, + new Date(1023400137000L))); + lst.add(Arguments.of("Jun 06 17:48:57 EDT 2002", PatternOptionBuilder.DATE_VALUE, ParseException.class)); - @Test - public void testCreateValueNumber_Double() throws Exception { - assertEquals(1.5d, TypeHandler.createValue("1.5", PatternOptionBuilder.NUMBER_VALUE)); - } + lst.add(Arguments.of("non-existing.file", PatternOptionBuilder.EXISTING_FILE_VALUE, ParseException.class)); - @Test - public void testCreateValueNumber_Long() throws Exception { - assertEquals(Long.valueOf(15), TypeHandler.createValue("15", PatternOptionBuilder.NUMBER_VALUE)); - } + lst.add(Arguments.of("some-file.txt", PatternOptionBuilder.FILE_VALUE, new File("some-file.txt"))); - @Test - public void testCreateValueNumber_noNumber() { - assertThrows(ParseException.class, () -> - TypeHandler.createValue("not a number", PatternOptionBuilder.NUMBER_VALUE)); - } + lst.add(Arguments.of("some.files", PatternOptionBuilder.FILES_VALUE, UnsupportedOperationException.class)); - @Test - public void testCreateValueObject_InstantiableClass() throws Exception { - final Object result = TypeHandler.createValue(Instantiable.class.getName(), PatternOptionBuilder.OBJECT_VALUE); - assertTrue(result instanceof Instantiable); - } + lst.add(Arguments.of("just-a-string", Integer.class, ParseException.class)); + lst.add(Arguments.of("5", Integer.class, 5)); + lst.add(Arguments.of("5.5", Integer.class, ParseException.class)); + lst.add(Arguments.of(Long.valueOf(Long.MAX_VALUE).toString(), Integer.class, ParseException.class)); - @Test - public void testCreateValueObject_notInstantiableClass() { - assertThrows(ParseException.class, () -> - TypeHandler.createValue(NotInstantiable.class.getName(), PatternOptionBuilder.OBJECT_VALUE)); - } + lst.add(Arguments.of("just-a-string", Long.class, ParseException.class)); + lst.add(Arguments.of("5", Long.class, 5L)); + lst.add(Arguments.of("5.5", Long.class, ParseException.class)); - @Test - public void testCreateValueObject_unknownClass() { - assertThrows(ParseException.class, () -> - TypeHandler.createValue("unknown", PatternOptionBuilder.OBJECT_VALUE)); - } + lst.add(Arguments.of("just-a-string", Short.class, ParseException.class)); + lst.add(Arguments.of("5", Short.class, (short) 5)); + lst.add(Arguments.of("5.5", Short.class, ParseException.class)); + lst.add(Arguments.of(Integer.valueOf(Integer.MAX_VALUE).toString(), Short.class, ParseException.class)); - @Test - public void testCreateValueString() throws Exception { - assertEquals("String", TypeHandler.createValue("String", PatternOptionBuilder.STRING_VALUE)); - } + lst.add(Arguments.of("just-a-string", Byte.class, ParseException.class)); + lst.add(Arguments.of("5", Byte.class, (byte) 5)); + lst.add(Arguments.of("5.5", Byte.class, ParseException.class)); + lst.add(Arguments.of(Short.valueOf(Short.MAX_VALUE).toString(), Byte.class, ParseException.class)); - @Test - public void testCreateValueURL() throws Exception { - final String urlString = "https://commons.apache.org"; - final URL result = TypeHandler.createValue(urlString, PatternOptionBuilder.URL_VALUE); - assertEquals(urlString, result.toString()); - } + lst.add(Arguments.of("just-a-string", Character.class, 'j')); + lst.add(Arguments.of("5", Character.class, '5')); + lst.add(Arguments.of("5.5", Character.class, '5')); + lst.add(Arguments.of("\\u0124", Character.class, Character.toChars(0x0124)[0])); - @Test - public void testCreateValueURL_malformed() { - assertThrows(ParseException.class, () -> - TypeHandler.createValue("malformed-url", PatternOptionBuilder.URL_VALUE)); - } + lst.add(Arguments.of("just-a-string", Double.class, ParseException.class)); + lst.add(Arguments.of("5", Double.class, 5d)); + lst.add(Arguments.of("5.5", Double.class, 5.5)); + + lst.add(Arguments.of("just-a-string", Float.class, ParseException.class)); + lst.add(Arguments.of("5", Float.class, 5f)); + lst.add(Arguments.of("5.5", Float.class, 5.5f)); + lst.add(Arguments.of(Double.valueOf(Double.MAX_VALUE).toString(), Float.class, Float.POSITIVE_INFINITY)); + + lst.add(Arguments.of("just-a-string", BigInteger.class, ParseException.class)); + lst.add(Arguments.of("5", BigInteger.class, new BigInteger("5"))); + lst.add(Arguments.of("5.5", BigInteger.class, ParseException.class)); + + lst.add(Arguments.of("just-a-string", BigDecimal.class, ParseException.class)); + lst.add(Arguments.of("5", BigDecimal.class, new BigDecimal("5"))); + lst.add(Arguments.of("5.5", BigDecimal.class, new BigDecimal(5.5))); + + lst.add(Arguments.of("1.5", PatternOptionBuilder.NUMBER_VALUE, Double.valueOf(1.5))); + lst.add(Arguments.of("15", PatternOptionBuilder.NUMBER_VALUE, Long.valueOf(15))); + lst.add(Arguments.of("not a number", PatternOptionBuilder.NUMBER_VALUE, ParseException.class)); + + lst.add(Arguments.of(Instantiable.class.getName(), PatternOptionBuilder.OBJECT_VALUE, new Instantiable())); + lst.add(Arguments.of(NotInstantiable.class.getName(), PatternOptionBuilder.OBJECT_VALUE, + ParseException.class)); + lst.add(Arguments.of("unknown", PatternOptionBuilder.OBJECT_VALUE, ParseException.class)); + lst.add(Arguments.of("String", PatternOptionBuilder.STRING_VALUE, "String")); + + final String urlString = "https://commons.apache.org"; + lst.add(Arguments.of(urlString, PatternOptionBuilder.URL_VALUE, new URL(urlString))); + lst.add(Arguments.of("Malformed-url", PatternOptionBuilder.URL_VALUE, ParseException.class)); + + return lst.stream(); + + } catch (MalformedURLException e) { + throw new RuntimeException(e); + } + + } } diff --git a/src/test/java/org/apache/commons/cli/converters/ConverterTests.java b/src/test/java/org/apache/commons/cli/converters/ConverterTests.java index 0a1bf53be..dd0f304bf 100644 --- a/src/test/java/org/apache/commons/cli/converters/ConverterTests.java +++ b/src/test/java/org/apache/commons/cli/converters/ConverterTests.java @@ -99,11 +99,7 @@ public void dateTests() throws Exception { @Test public void urlTests() throws Exception { - URL url = this.getClass().getClassLoader().getResource("./org/apache/commons/cli/existing-readable.file"); - assertEquals(url, Converter.URL.apply(url.toString())); - assertEquals(new URL("http://apache.org"), Converter.URL.apply("http://apache.org")); - assertThrows(java.net.MalformedURLException.class, () -> Converter.URL.apply("foo.bar")); } diff --git a/src/test/java/org/apache/commons/cli/converters/VerifierTests.java b/src/test/java/org/apache/commons/cli/converters/VerifierTests.java index a5d27237b..ff655a920 100644 --- a/src/test/java/org/apache/commons/cli/converters/VerifierTests.java +++ b/src/test/java/org/apache/commons/cli/converters/VerifierTests.java @@ -42,6 +42,22 @@ public void numberTests() { } } + /** + * Verifies integer formats + */ + @Test + public void integerTests() { + // test good numbers + for (String s : new String[] {"123", "-123"}) { + assertTrue(Verifier.INTEGER.test(s), s); + } + + // test bad numbers + for (String s : new String[] {"12.3", "-12.3", ".3", "-.3", "0x5F", "2,3", "1.2.3"}) { + assertFalse(Verifier.INTEGER.test(s), s); + } + } + /** * Verifies class names */ From 037550db59535fb6b991d23ba16bcd8cb5fd0777 Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Fri, 5 Jan 2024 10:21:04 +0100 Subject: [PATCH 09/24] fixed breaking changes and javadoc --- .../java/org/apache/commons/cli/Option.java | 16 ++++++++++---- .../org/apache/commons/cli/TypeHandler.java | 21 ++++++++++++------- .../org/apache/commons/cli/OptionTest.java | 8 ++----- 3 files changed, 28 insertions(+), 17 deletions(-) diff --git a/src/main/java/org/apache/commons/cli/Option.java b/src/main/java/org/apache/commons/cli/Option.java index 270b4d592..df72cd0f5 100644 --- a/src/main/java/org/apache/commons/cli/Option.java +++ b/src/main/java/org/apache/commons/cli/Option.java @@ -285,6 +285,7 @@ public Builder valueSeparator(final char valueSeparator) { *

Note: see {@link TypeHandler} for serialization discussion.

* @param converter the Converter to use. * @return this builder, to allow method chaining. + * @since 1.7 */ public Builder converter(Converter converter) { this.converter = converter; @@ -296,6 +297,7 @@ public Builder converter(Converter converter) { *

Note: see {@link TypeHandler} for serialization discussion.

* @param verifier the Verifier to use. * @return this builder, to allow method chaining. + * @since 1.7 */ public Builder verifier(Verifier verifier) { this.verifier = verifier; @@ -458,13 +460,13 @@ boolean acceptsArg() { * * @since 1.0.1 */ - private void add(final String value) throws ParseException { + private void add(final String value) { if (!acceptsArg()) { throw new IllegalArgumentException("Cannot add value, list full."); } if (!getVerifier().test(value)) { - throw new ParseException(String.format("'%s' is not a valid input string for option '%s'", value, option)); + throw new IllegalArgumentException(String.format("'%s' is not a valid input string for option '%s'", value, option)); } // store value @@ -491,7 +493,7 @@ public boolean addValue(final String value) { * * @param value is a/the value of this Option */ - void addValueForProcessing(final String value) throws ParseException { + void addValueForProcessing(final String value) { if (argCount == UNINITIALIZED) { throw new IllegalArgumentException("NO_ARGS_ALLOWED"); } @@ -738,6 +740,8 @@ private boolean hasNoValues() { } /** + * Returns whether this Option can have an optional argument. + * * @return whether this Option can have an optional argument */ public boolean hasOptionalArg() { @@ -772,7 +776,7 @@ public boolean isRequired() { * * @since 1.0.1 */ - private void processValue(String value) throws ParseException { + private void processValue(String value) { // this Option has a separator character if (hasValueSeparator()) { // get the separator character @@ -911,6 +915,7 @@ public void setValueSeparator(final char sep) { /** * Gets the value to type converter. * @return the value to type converter + * @since 1.7 */ public Converter getConverter() { return converter == null ? TypeHandler.getConverter(type) : converter; @@ -919,6 +924,7 @@ public Converter getConverter() { /** * Sets the value to type converter. * @param converter The converter to convert the string value to the type. + * @since 1.7 */ public void setConverter(Converter converter) { this.converter = converter; @@ -927,6 +933,7 @@ public void setConverter(Converter converter) { /** * Gets the value verifier. * @return the value verifier. + * @since 1.7 */ public Verifier getVerifier() { return verifier == null ? TypeHandler.getVerifier(type) : verifier; @@ -935,6 +942,7 @@ public Verifier getVerifier() { /** * 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 */ public void setVerifier(Verifier verifier) { this.verifier = verifier; diff --git a/src/main/java/org/apache/commons/cli/TypeHandler.java b/src/main/java/org/apache/commons/cli/TypeHandler.java index 9667ca0f2..a7811162e 100644 --- a/src/main/java/org/apache/commons/cli/TypeHandler.java +++ b/src/main/java/org/apache/commons/cli/TypeHandler.java @@ -59,6 +59,7 @@ public class TypeHandler { /** * Resets the registered Converters to the default state. + * @since 1.7 */ public static void resetConverters() { converterMap.clear(); @@ -87,6 +88,7 @@ public static void resetConverters() { /** * Unregisters all Converters. + * @since 1.7 */ public static void noConverters() { converterMap.clear(); @@ -94,6 +96,7 @@ public static void noConverters() { /** * Resets the registered Verifiers to the default state. + * @since 1.7 */ public static void resetVerifiers() { verifierMap.clear(); @@ -114,6 +117,7 @@ public static void resetVerifiers() { /** * Unregisters all Verifiers. + * @since 1.7 */ public static void noVerifiers() { verifierMap.clear(); @@ -127,6 +131,7 @@ public static void noVerifiers() { * @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 */ public static void register(Class clazz, Converter converter, Verifier verifier) { if (converter == null) { @@ -146,6 +151,7 @@ public static void register(Class clazz, Converter converter, Verifier ver * Gets the converter for the the Class. Never null. * @param clazz The Class to get the Converter for. * @return the registered converter if any, {@link Converter#DEFAULT} otherwise. + * @since 1.7 */ public static Converter getConverter(Class clazz) { Converter converter = converterMap.get(clazz); @@ -156,6 +162,7 @@ public static Converter getConverter(Class clazz) { * 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 */ public static Verifier getVerifier(Class clazz) { Verifier verifier = verifierMap.get(clazz); @@ -170,7 +177,7 @@ public static Verifier getVerifier(Class clazz) { * @throws ParseException if the class could not be found * @deprecated use {@link #createValue(String, Class)} */ - @Deprecated // sinze 1.7 + @Deprecated // since 1.7 public static Class createClass(final String className) throws ParseException { return createValue(className, Class.class); } @@ -184,7 +191,7 @@ public static Class createClass(final String className) throws ParseException * return null. * @deprecated use {@link #createValue(String, Class)} */ - @Deprecated // sinze 1.7 + @Deprecated // since 1.7 public static Date createDate(final String str) { try { return createValue(str, Date.class); @@ -200,7 +207,7 @@ public static Date createDate(final String str) { * @return The file represented by {@code str}. * @deprecated use {@link #createValue(String, Class)} */ - @Deprecated // sinze 1.7 + @Deprecated // since 1.7 public static File createFile(final String str) { try { return createValue(str, File.class); @@ -219,7 +226,7 @@ public static File createFile(final String str) { * @throws UnsupportedOperationException always * @deprecated with no replacement */ - @Deprecated // sinze 1.7 + @Deprecated // since 1.7 public static File[] createFiles(final String str) { // to implement/port: // return FileW.findFiles(str); @@ -234,7 +241,7 @@ public static File[] createFiles(final String str) { * @return the number represented by {@code str} * @throws ParseException if {@code str} is not a number */ - @Deprecated // sinze 1.7 + @Deprecated // since 1.7 public static Number createNumber(final String str) throws ParseException { return createValue(str, Number.class); } @@ -248,7 +255,7 @@ public static Number createNumber(final String str) throws ParseException { * could not be created * @deprecated use {@link #createValue(String, Class)} */ - @Deprecated // sinze 1.7 + @Deprecated // since 1.7 public static Object createObject(final String className) throws ParseException { return createValue(className, Object.class); } @@ -261,7 +268,7 @@ public static Object createObject(final String className) throws ParseException * @throws ParseException if the URL in {@code str} is not well-formed * @deprecated use {@link #createValue(String, Class)} */ - @Deprecated // sinze 1.7 + @Deprecated // since 1.7 public static URL createURL(final String str) throws ParseException { return createValue(str, URL.class); } diff --git a/src/test/java/org/apache/commons/cli/OptionTest.java b/src/test/java/org/apache/commons/cli/OptionTest.java index d049eba34..8f196892c 100644 --- a/src/test/java/org/apache/commons/cli/OptionTest.java +++ b/src/test/java/org/apache/commons/cli/OptionTest.java @@ -63,12 +63,8 @@ private static final class TestOption extends Option { @Override public boolean addValue(final String value) { - try { - addValueForProcessing(value); - return true; - } catch (ParseException e) { - throw new RuntimeException(e); - } + addValueForProcessing(value); + return true; } } From e081e5615bc383814400f9f3c9031696aeebe626 Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Fri, 5 Jan 2024 10:26:57 +0100 Subject: [PATCH 10/24] added since annotation to Converter and Verifier --- src/main/java/org/apache/commons/cli/converters/Converter.java | 1 + src/main/java/org/apache/commons/cli/converters/Verifier.java | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/apache/commons/cli/converters/Converter.java b/src/main/java/org/apache/commons/cli/converters/Converter.java index 4f654973c..5d9827a9a 100644 --- a/src/main/java/org/apache/commons/cli/converters/Converter.java +++ b/src/main/java/org/apache/commons/cli/converters/Converter.java @@ -26,6 +26,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more * Like {@code Function} but can throw an Exception. * * @param The return type for the function. + * @since 1.7 */ @FunctionalInterface public interface Converter { diff --git a/src/main/java/org/apache/commons/cli/converters/Verifier.java b/src/main/java/org/apache/commons/cli/converters/Verifier.java index 0618ec41b..0b8af81a8 100644 --- a/src/main/java/org/apache/commons/cli/converters/Verifier.java +++ b/src/main/java/org/apache/commons/cli/converters/Verifier.java @@ -21,7 +21,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more /** * The definition of the functional interface to call when verifying a string * for input Like {@code Predicate} but can throw a RuntimeException. - * + * @since 1.7 */ @FunctionalInterface public interface Verifier { From 942364a54d539e688156b00f191a981fab555a2b Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Fri, 5 Jan 2024 13:43:42 +0100 Subject: [PATCH 11/24] Added an Enum Validator implementation --- .../commons/cli/converters/EnumVerifier.java | 46 +++++++++++++++ .../cli/converters/EnumVerifierTest.java | 59 +++++++++++++++++++ 2 files changed, 105 insertions(+) create mode 100644 src/main/java/org/apache/commons/cli/converters/EnumVerifier.java create mode 100644 src/test/java/org/apache/commons/cli/converters/EnumVerifierTest.java diff --git a/src/main/java/org/apache/commons/cli/converters/EnumVerifier.java b/src/main/java/org/apache/commons/cli/converters/EnumVerifier.java new file mode 100644 index 000000000..0d496065c --- /dev/null +++ b/src/main/java/org/apache/commons/cli/converters/EnumVerifier.java @@ -0,0 +1,46 @@ +/* + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ +package org.apache.commons.cli.converters; + +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; + +/** + * A verifier driven by the values in an enum. + */ +public class EnumVerifier implements Verifier { + /** The list of valid names */ + private List names; + + /** + * Constructs the Verifier from an exemplar of the Enum. For example + * {@code new EnumVerifier(java.time.format.TextStyle.FULL)} would create an + * Verifier that would accept the names for any of the + * {@code java.time.format.TextStyle} values. + * @param exemplar One of the values from the accepted Enum. + */ + public EnumVerifier(Enum exemplar) { + names = Arrays.stream(exemplar.getDeclaringClass().getEnumConstants()).map(t -> ((Enum) t).name()) + .collect(Collectors.toList()); + } + + @Override + public boolean test(String str) throws RuntimeException { + return names.contains(str); + } +} diff --git a/src/test/java/org/apache/commons/cli/converters/EnumVerifierTest.java b/src/test/java/org/apache/commons/cli/converters/EnumVerifierTest.java new file mode 100644 index 000000000..7a6b6e6e6 --- /dev/null +++ b/src/test/java/org/apache/commons/cli/converters/EnumVerifierTest.java @@ -0,0 +1,59 @@ +/* + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ +package org.apache.commons.cli.converters; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Stream; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +/** + * Tests for the EnumVerifier class + * + */ +public class EnumVerifierTest { + + enum MyEnum { + ONE, Two, three, someothernumber + } + + private EnumVerifier underTest = new EnumVerifier(MyEnum.Two); + + @ParameterizedTest(name = "{0}") + @MethodSource("testData") + public void test(String str, boolean expected) { + assertEquals(expected, underTest.test(str)); + } + + private static Stream testData() { + List lst = new ArrayList<>(); + + lst.add(Arguments.of("ONE", true)); + lst.add(Arguments.of("one", false)); + lst.add(Arguments.of("Two", true)); + lst.add(Arguments.of("three", true)); + lst.add(Arguments.of("someothernumber", true)); + lst.add(Arguments.of("NonValue", false)); + + return lst.stream(); + } +} From f3658727719a5d2476a36d2d980b358b23de7452 Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Fri, 5 Jan 2024 14:12:09 +0100 Subject: [PATCH 12/24] fixed formatting issues --- .../org/apache/commons/cli/CommandLine.java | 6 +- .../java/org/apache/commons/cli/Option.java | 12 +- .../org/apache/commons/cli/TypeHandler.java | 126 +++++++++--------- .../commons/cli/converters/Converter.java | 3 +- .../commons/cli/converters/Verifier.java | 11 +- .../org/apache/commons/cli/OptionTest.java | 89 ++++++------- 6 files changed, 121 insertions(+), 126 deletions(-) diff --git a/src/main/java/org/apache/commons/cli/CommandLine.java b/src/main/java/org/apache/commons/cli/CommandLine.java index e4e2e509f..41aeec7b7 100644 --- a/src/main/java/org/apache/commons/cli/CommandLine.java +++ b/src/main/java/org/apache/commons/cli/CommandLine.java @@ -393,7 +393,7 @@ public T getParsedOptionValue(final String opt) throws ParseException { * @return the value parsed into a particular object. * @throws ParseException if there are problems turning the option value into the desired type * @see PatternOptionBuilder - * @since 1.7 + * @since 1.7.0 * @param The return type for the method. */ public T getParsedOptionValue(final char opt, T defaultValue) throws ParseException { @@ -408,7 +408,7 @@ public T getParsedOptionValue(final char opt, T defaultValue) throws ParseEx * @return the value parsed into a particular object. * @throws ParseException if there are problems turning the option value into the desired type * @see PatternOptionBuilder - * @since 1.7 + * @since 1.7.0 * @param The return type for the method. */ @SuppressWarnings("unchecked") @@ -433,7 +433,7 @@ public T getParsedOptionValue(final Option option, T defaultValue) throws Pa * @return the value parsed into a particular object. * @throws ParseException if there are problems turning the option value into the desired type * @see PatternOptionBuilder - * @since 1.7 + * @since 1.7.0 * @param The return type for the method. */ public T getParsedOptionValue(final String opt, T defaultValue) throws ParseException { diff --git a/src/main/java/org/apache/commons/cli/Option.java b/src/main/java/org/apache/commons/cli/Option.java index df72cd0f5..afe476a02 100644 --- 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) { *

Note: see {@link TypeHandler} for serialization discussion.

* @param converter the Converter to use. * @return this builder, to allow method chaining. - * @since 1.7 + * @since 1.7.0 */ public Builder converter(Converter converter) { this.converter = converter; @@ -297,7 +297,7 @@ public Builder converter(Converter converter) { *

Note: see {@link TypeHandler} for serialization discussion.

* @param verifier the Verifier to use. * @return this builder, to allow method chaining. - * @since 1.7 + * @since 1.7.0 */ public Builder verifier(Verifier verifier) { this.verifier = verifier; @@ -915,7 +915,7 @@ public void setValueSeparator(final char sep) { /** * Gets the value to type converter. * @return the value to type converter - * @since 1.7 + * @since 1.7.0 */ public Converter getConverter() { return converter == null ? TypeHandler.getConverter(type) : converter; @@ -924,7 +924,7 @@ public Converter getConverter() { /** * Sets the value to type converter. * @param converter The converter to convert the string value to the type. - * @since 1.7 + * @since 1.7.0 */ public void setConverter(Converter converter) { this.converter = converter; @@ -933,7 +933,7 @@ public void setConverter(Converter converter) { /** * Gets the value verifier. * @return the value verifier. - * @since 1.7 + * @since 1.7.0 */ public Verifier getVerifier() { return verifier == null ? TypeHandler.getVerifier(type) : verifier; @@ -942,7 +942,7 @@ public Verifier getVerifier() { /** * 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 + * @since 1.7.0 */ public void setVerifier(Verifier verifier) { this.verifier = verifier; diff --git a/src/main/java/org/apache/commons/cli/TypeHandler.java b/src/main/java/org/apache/commons/cli/TypeHandler.java index a7811162e..31ca9ad01 100644 --- a/src/main/java/org/apache/commons/cli/TypeHandler.java +++ b/src/main/java/org/apache/commons/cli/TypeHandler.java @@ -59,7 +59,7 @@ public class TypeHandler { /** * Resets the registered Converters to the default state. - * @since 1.7 + * @since 1.7.0 */ public static void resetConverters() { converterMap.clear(); @@ -88,7 +88,7 @@ public static void resetConverters() { /** * Unregisters all Converters. - * @since 1.7 + * @since 1.7.0 */ public static void noConverters() { converterMap.clear(); @@ -96,7 +96,7 @@ public static void noConverters() { /** * Resets the registered Verifiers to the default state. - * @since 1.7 + * @since 1.7.0 */ public static void resetVerifiers() { verifierMap.clear(); @@ -117,7 +117,7 @@ public static void resetVerifiers() { /** * Unregisters all Verifiers. - * @since 1.7 + * @since 1.7.0 */ public static void noVerifiers() { verifierMap.clear(); @@ -131,7 +131,7 @@ public static void noVerifiers() { * @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 + * @since 1.7.0 */ public static void register(Class clazz, Converter converter, Verifier verifier) { if (converter == null) { @@ -151,7 +151,7 @@ public static void register(Class clazz, Converter converter, Verifier ver * Gets the converter for the the Class. Never null. * @param clazz The Class to get the Converter for. * @return the registered converter if any, {@link Converter#DEFAULT} otherwise. - * @since 1.7 + * @since 1.7.0 */ public static Converter getConverter(Class clazz) { Converter converter = converterMap.get(clazz); @@ -162,7 +162,7 @@ public static Converter getConverter(Class clazz) { * 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 + * @since 1.7.0 */ public static Verifier getVerifier(Class clazz) { Verifier verifier = verifierMap.get(clazz); @@ -172,26 +172,26 @@ public static Verifier getVerifier(Class clazz) { /** * Returns the class whose name is {@code className}. * - * @param className the class name - * @return The class if it is found - * @throws ParseException if the class could not be found - * @deprecated use {@link #createValue(String, Class)} + * @param className the class name + * @return The class if it is found + * @throws ParseException if the class could not be found + * @deprecated use {@link #createValue(String, Class)} */ - @Deprecated // since 1.7 + @Deprecated // since 1.7.0 public static Class createClass(final String className) throws ParseException { return createValue(className, Class.class); } /** - * Returns the date represented by {@code str}.

This method is not yet - * implemented and always throws an {@link UnsupportedOperationException}. + * Returns the date represented by {@code str}. + *

+ * This method is not yet implemented and always throws an {@link UnsupportedOperationException}. * - * @param str the date string - * @return The date if {@code str} is a valid date string, otherwise - * return null. - * @deprecated use {@link #createValue(String, Class)} + * @param str the date string + * @return The date if {@code str} is a valid date string, otherwise return null. + * @deprecated use {@link #createValue(String, Class)} */ - @Deprecated // since 1.7 + @Deprecated // since 1.7.0 public static Date createDate(final String str) { try { return createValue(str, Date.class); @@ -203,11 +203,11 @@ public static Date createDate(final String str) { /** * Returns the File represented by {@code str}. * - * @param str the File location - * @return The file represented by {@code str}. - * @deprecated use {@link #createValue(String, Class)} + * @param str the File location + * @return The file represented by {@code str}. + * @deprecated use {@link #createValue(String, Class)} */ - @Deprecated // since 1.7 + @Deprecated // since 1.7.0 public static File createFile(final String str) { try { return createValue(str, File.class); @@ -217,16 +217,15 @@ public static File createFile(final String str) { } /** - * Returns the File[] represented by {@code str}.

This method is not yet - * implemented and always throws an {@link UnsupportedOperationException}. + * Returns the File[] represented by {@code str}. + *

This method is not yet implemented and always throws an {@link UnsupportedOperationException}. * - * @param str the paths to the files - * @return The File[] represented by - * {@code str}. + * @param str the paths to the files + * @return The File[] represented by {@code str}. * @throws UnsupportedOperationException always - * @deprecated with no replacement + * @deprecated with no replacement */ - @Deprecated // since 1.7 + @Deprecated // since 1.7.0 public static File[] createFiles(final String str) { // to implement/port: // return FileW.findFiles(str); @@ -234,14 +233,13 @@ public static File[] createFiles(final String str) { } /** - * Create a number from a String. If a '.' is present, it creates a Double, - * otherwise a Long. + * Create a number from a String. If a '.' is present, it creates a Double, otherwise a Long. * - * @param str the value - * @return the number represented by {@code str} - * @throws ParseException if {@code str} is not a number + * @param str the value + * @return the number represented by {@code str} + * @throws ParseException if {@code str} is not a number */ - @Deprecated // since 1.7 + @Deprecated // since 1.7.0 public static Number createNumber(final String str) throws ParseException { return createValue(str, Number.class); } @@ -249,13 +247,12 @@ public static Number createNumber(final String str) throws ParseException { /** * Create an Object from the class name and empty constructor. * - * @param className the argument value - * @return the initialized object - * @throws ParseException if the class could not be found or the object - * could not be created - * @deprecated use {@link #createValue(String, Class)} + * @param className the argument value + * @return the initialized object + * @throws ParseException if the class could not be found or the object could not be created + * @deprecated use {@link #createValue(String, Class)} */ - @Deprecated // since 1.7 + @Deprecated // since 1.7.0 public static Object createObject(final String className) throws ParseException { return createValue(className, Object.class); } @@ -263,25 +260,24 @@ public static Object createObject(final String className) throws ParseException /** * Returns the URL represented by {@code str}. * - * @param str the URL string - * @return The URL in {@code str} is well-formed - * @throws ParseException if the URL in {@code str} is not well-formed - * @deprecated use {@link #createValue(String, Class)} + * @param str the URL string + * @return The URL in {@code str} is well-formed + * @throws ParseException if the URL in {@code str} is not well-formed + * @deprecated use {@link #createValue(String, Class)} */ - @Deprecated // since 1.7 + @Deprecated // since 1.7.0 public static URL createURL(final String str) throws ParseException { return createValue(str, URL.class); } /** - * Returns the {@code Object} of type {@code clazz} with the value of + * Returns the @code Object} of type {@code clazz} with the value of * {@code str}. * - * @param str the command line value - * @param clazz the class representing the type of argument - * @param type of argument - * @return The instance of {@code clazz} initialized with the - * value of {@code str}. + * @param str the command line value + * @param clazz the class representing the type of argument + * @param type of argument + * @return The instance of {@code clazz} initialized with the value of {@code str}. * @throws ParseException if the value creation for the given class threw an exception. */ @SuppressWarnings("unchecked") // returned value will have type T because it is fixed by clazz @@ -296,15 +292,13 @@ public static T createValue(final String str, final Class clazz) throws P /** * Returns the {@code Object} of type {@code obj} with the value of {@code str}. * - * @param str the command line value - * @param obj the type of argument - * @return The instance of {@code obj} initialized with the - * value of {@code str}. - * @throws ParseException if the value creation for the given object type - * failed - * @deprecated use {@link #createValue(String, Class)} + * @param str the command line value + * @param obj the type of argument + * @return The instance of {@code obj} initialized with the value of {@code str}. + * @throws ParseException if the value creation for the given object type failed + * @deprecated use {@link #createValue(String, Class)} */ - @Deprecated // since 1.7 + @Deprecated // since 1.7.0 public static Object createValue(final String str, final Object obj) throws ParseException { return createValue(str, (Class) obj); } @@ -312,12 +306,12 @@ public static Object createValue(final String str, final Object obj) throws Pars /** * Returns the opened FileInputStream represented by {@code str}. * - * @param str the file location - * @return The file input stream represented by {@code str}. - * @throws ParseException if the file is not exist or not readable - * @deprecated use {@link #createValue(String, Class)} + * @param str the file location + * @return The file input stream represented by {@code str}. + * @throws ParseException if the file is not exist or not readable + * @deprecated use {@link #createValue(String, Class)} */ - @Deprecated // since 1.7 + @Deprecated // since 1.7.0 public static FileInputStream openFile(final String str) throws ParseException { return createValue(str, FileInputStream.class); } diff --git a/src/main/java/org/apache/commons/cli/converters/Converter.java b/src/main/java/org/apache/commons/cli/converters/Converter.java index 5d9827a9a..ff03fa744 100644 --- a/src/main/java/org/apache/commons/cli/converters/Converter.java +++ b/src/main/java/org/apache/commons/cli/converters/Converter.java @@ -20,13 +20,14 @@ Licensed to the Apache Software Foundation (ASF) under one or more import java.net.URL; import java.text.SimpleDateFormat; import java.util.Date; +import java.util.function.Function; /** * The definition of the functional interface to call when doing a conversion. * Like {@code Function} but can throw an Exception. * * @param The return type for the function. - * @since 1.7 + * @since 1.7.0 */ @FunctionalInterface public interface Converter { diff --git a/src/main/java/org/apache/commons/cli/converters/Verifier.java b/src/main/java/org/apache/commons/cli/converters/Verifier.java index 0b8af81a8..d610f0f74 100644 --- a/src/main/java/org/apache/commons/cli/converters/Verifier.java +++ b/src/main/java/org/apache/commons/cli/converters/Verifier.java @@ -16,15 +16,16 @@ Licensed to the Apache Software Foundation (ASF) under one or more */ package org.apache.commons.cli.converters; +import java.util.function.Predicate; import java.util.regex.Pattern; /** * The definition of the functional interface to call when verifying a string * for input Like {@code Predicate} but can throw a RuntimeException. - * @since 1.7 + * @since 1.7.0 */ @FunctionalInterface -public interface Verifier { +public interface Verifier extends Predicate { /** * Default verifier. Always returns {@code true}. @@ -37,11 +38,11 @@ public interface Verifier { Pattern NUMBER_PATTERN = Pattern.compile("-?([0-9]*\\.)?([0-9]+)$"); /** - * Verifies that a number string is either a valid real number (e.g. may have a decimal - * point) or an integer. + * Verifies that a number string is either a valid real number (e.g. may have a + * decimal point) or an integer. */ Verifier NUMBER = s -> NUMBER_PATTERN.matcher(s).matches(); - + /** * The Regex Pattern for the integer matching. */ diff --git a/src/test/java/org/apache/commons/cli/OptionTest.java b/src/test/java/org/apache/commons/cli/OptionTest.java index 8f196892c..352e9f217 100644 --- a/src/test/java/org/apache/commons/cli/OptionTest.java +++ b/src/test/java/org/apache/commons/cli/OptionTest.java @@ -42,8 +42,7 @@ private static final class DefaultOption extends Option { private final String defaultValue; - DefaultOption(final String opt, final String description, final String defaultValue) - throws IllegalArgumentException { + DefaultOption(final String opt, final String description, final String defaultValue) throws IllegalArgumentException { super(opt, true, description); this.defaultValue = defaultValue; } @@ -68,9 +67,8 @@ public boolean addValue(final String value) { } } - private static void checkOption(final Option option, final String opt, final String description, - final String longOpt, final int numArgs, final String argName, final boolean required, - final boolean optionalArg, final char valueSeparator, final Class cls) { + private static void checkOption(final Option option, final String opt, final String description, final String longOpt, final int numArgs, + final String argName, final boolean required, final boolean optionalArg, final char valueSeparator, final Class cls) { assertEquals(opt, option.getOpt()); assertEquals(description, option.getDescription()); assertEquals(longOpt, option.getLongOpt()); @@ -85,69 +83,70 @@ private static void checkOption(final Option option, final String opt, final Str @Test public void testBuilderInsufficientParams1() { - assertThrows(IllegalArgumentException.class, () -> Option.builder().desc("desc").build()); + assertThrows(IllegalArgumentException.class, () -> + Option.builder().desc("desc").build()); } @Test public void testBuilderInsufficientParams2() { - assertThrows(IllegalArgumentException.class, () -> Option.builder(null).desc("desc").build()); + assertThrows(IllegalArgumentException.class, () -> + Option.builder(null).desc("desc").build()); } @Test public void testBuilderInvalidOptionName1() { - assertThrows(IllegalArgumentException.class, () -> Option.builder().option("invalid?")); + assertThrows(IllegalArgumentException.class, () -> + Option.builder().option("invalid?")); } @Test public void testBuilderInvalidOptionName2() { - assertThrows(IllegalArgumentException.class, () -> Option.builder().option("invalid@")); + assertThrows(IllegalArgumentException.class, () -> + Option.builder().option("invalid@")); } @Test public void testBuilderInvalidOptionName3() { - assertThrows(IllegalArgumentException.class, () -> Option.builder("invalid?")); + assertThrows(IllegalArgumentException.class, () -> + Option.builder("invalid?")); } @Test public void testBuilderInvalidOptionName4() { - assertThrows(IllegalArgumentException.class, () -> Option.builder("invalid@")); + assertThrows(IllegalArgumentException.class, () -> + Option.builder("invalid@")); } @Test public void testBuilderMethods() { final char defaultSeparator = (char) 0; - checkOption(Option.builder("a").desc("desc").build(), "a", "desc", null, Option.UNINITIALIZED, null, false, - false, defaultSeparator, String.class); - checkOption(Option.builder("a").desc("desc").build(), "a", "desc", null, Option.UNINITIALIZED, null, false, - false, defaultSeparator, String.class); - checkOption(Option.builder("a").desc("desc").longOpt("aaa").build(), "a", "desc", "aaa", Option.UNINITIALIZED, - null, false, false, defaultSeparator, String.class); - checkOption(Option.builder("a").desc("desc").hasArg(true).build(), "a", "desc", null, 1, null, false, false, - defaultSeparator, String.class); - checkOption(Option.builder("a").desc("desc").hasArg(false).build(), "a", "desc", null, Option.UNINITIALIZED, - null, false, false, defaultSeparator, String.class); - checkOption(Option.builder("a").desc("desc").hasArg(true).build(), "a", "desc", null, 1, null, false, false, - defaultSeparator, String.class); - checkOption(Option.builder("a").desc("desc").numberOfArgs(3).build(), "a", "desc", null, 3, null, false, false, - defaultSeparator, String.class); - checkOption(Option.builder("a").desc("desc").required(true).build(), "a", "desc", null, Option.UNINITIALIZED, - null, true, false, defaultSeparator, String.class); - checkOption(Option.builder("a").desc("desc").required(false).build(), "a", "desc", null, Option.UNINITIALIZED, - null, false, false, defaultSeparator, String.class); - - checkOption(Option.builder("a").desc("desc").argName("arg1").build(), "a", "desc", null, Option.UNINITIALIZED, - "arg1", false, false, defaultSeparator, String.class); - checkOption(Option.builder("a").desc("desc").optionalArg(false).build(), "a", "desc", null, - Option.UNINITIALIZED, null, false, false, defaultSeparator, String.class); - checkOption(Option.builder("a").desc("desc").optionalArg(true).build(), "a", "desc", null, 1, null, false, true, - defaultSeparator, String.class); - checkOption(Option.builder("a").desc("desc").valueSeparator(':').build(), "a", "desc", null, - Option.UNINITIALIZED, null, false, false, ':', String.class); - checkOption(Option.builder("a").desc("desc").type(Integer.class).build(), "a", "desc", null, - Option.UNINITIALIZED, null, false, false, defaultSeparator, Integer.class); - checkOption(Option.builder().option("a").desc("desc").type(Integer.class).build(), "a", "desc", null, - Option.UNINITIALIZED, null, false, false, defaultSeparator, Integer.class); + checkOption(Option.builder("a").desc("desc").build(), "a", "desc", null, Option.UNINITIALIZED, null, false, false, defaultSeparator, String.class); + checkOption(Option.builder("a").desc("desc").build(), "a", "desc", null, Option.UNINITIALIZED, null, false, false, defaultSeparator, String.class); + checkOption(Option.builder("a").desc("desc").longOpt("aaa").build(), "a", "desc", "aaa", Option.UNINITIALIZED, null, false, false, defaultSeparator, + String.class); + checkOption(Option.builder("a").desc("desc").hasArg(true).build(), "a", "desc", null, 1, null, false, false, defaultSeparator, String.class); + checkOption(Option.builder("a").desc("desc").hasArg(false).build(), "a", "desc", null, Option.UNINITIALIZED, null, false, false, defaultSeparator, + String.class); + checkOption(Option.builder("a").desc("desc").hasArg(true).build(), "a", "desc", null, 1, null, false, false, defaultSeparator, String.class); + checkOption(Option.builder("a").desc("desc").numberOfArgs(3).build(), "a", "desc", null, 3, null, false, false, defaultSeparator, String.class); + checkOption(Option.builder("a").desc("desc").required(true).build(), "a", "desc", null, Option.UNINITIALIZED, null, true, false, defaultSeparator, + String.class); + checkOption(Option.builder("a").desc("desc").required(false).build(), "a", "desc", null, Option.UNINITIALIZED, null, false, false, defaultSeparator, + String.class); + + checkOption(Option.builder("a").desc("desc").argName("arg1").build(), "a", "desc", null, Option.UNINITIALIZED, "arg1", false, false, defaultSeparator, + String.class); + checkOption(Option.builder("a").desc("desc").optionalArg(false).build(), "a", "desc", null, Option.UNINITIALIZED, null, false, false, defaultSeparator, + String.class); + checkOption(Option.builder("a").desc("desc").optionalArg(true).build(), "a", "desc", null, 1, null, false, true, defaultSeparator, + String.class); + checkOption(Option.builder("a").desc("desc").valueSeparator(':').build(), "a", "desc", null, Option.UNINITIALIZED, null, false, false, ':', + String.class); + checkOption(Option.builder("a").desc("desc").type(Integer.class).build(), "a", "desc", null, Option.UNINITIALIZED, null, false, false, defaultSeparator, + Integer.class); + checkOption(Option.builder().option("a").desc("desc").type(Integer.class).build(), "a", "desc", null, Option.UNINITIALIZED, null, false, false, + defaultSeparator, Integer.class); } @Test @@ -178,7 +177,7 @@ public void testClone() { } @Test - public void testGetValue() throws ParseException { + public void testGetValue() { final Option option = new Option("f", null); option.setArgs(Option.UNLIMITED_VALUES); @@ -230,8 +229,7 @@ public void testHasArgs() { public void testHashCode() { assertNotEquals(Option.builder("test").build().hashCode(), Option.builder("test2").build().hashCode()); assertNotEquals(Option.builder("test").build().hashCode(), Option.builder().longOpt("test").build().hashCode()); - assertNotEquals(Option.builder("test").build().hashCode(), - Option.builder("test").longOpt("long test").build().hashCode()); + assertNotEquals(Option.builder("test").build().hashCode(), Option.builder("test").longOpt("long test").build().hashCode()); } @Test @@ -242,6 +240,7 @@ public void testSubclass() { assertEquals(DefaultOption.class, clone.getClass()); } + private Option roundTrip(Option o) throws IOException, ClassNotFoundException { ByteArrayOutputStream baos = new ByteArrayOutputStream(); ObjectOutputStream oos = new ObjectOutputStream(baos); From 3ec932db42c67bd0d4201ca05070a61e211d88ce Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Fri, 5 Jan 2024 14:20:59 +0100 Subject: [PATCH 13/24] fixed checkstyle error --- src/main/java/org/apache/commons/cli/converters/Converter.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/org/apache/commons/cli/converters/Converter.java b/src/main/java/org/apache/commons/cli/converters/Converter.java index ff03fa744..2d857dc27 100644 --- a/src/main/java/org/apache/commons/cli/converters/Converter.java +++ b/src/main/java/org/apache/commons/cli/converters/Converter.java @@ -20,7 +20,6 @@ Licensed to the Apache Software Foundation (ASF) under one or more import java.net.URL; import java.text.SimpleDateFormat; import java.util.Date; -import java.util.function.Function; /** * The definition of the functional interface to call when doing a conversion. From 6d5a916a41c4cbfbf8ce4df50f55bb945d030e3a Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Thu, 11 Jan 2024 11:45:54 +0100 Subject: [PATCH 14/24] Converted Verifier to Predicate Verifier interface became static class to hold common Predicate instances for verification. Moved EnumVerifier into Verifier as a static method. All references to Verifier as a data type chagned to Predicate. --- .../java/org/apache/commons/cli/Option.java | 12 ++-- .../commons/cli/PatternOptionBuilder.java | 3 +- .../org/apache/commons/cli/TypeHandler.java | 9 +-- .../commons/cli/converters/EnumVerifier.java | 46 --------------- .../commons/cli/converters/Verifier.java | 48 ++++++++++----- .../cli/converters/EnumVerifierTest.java | 59 ------------------- .../commons/cli/converters/VerifierTests.java | 34 +++++++++++ 7 files changed, 80 insertions(+), 131 deletions(-) delete mode 100644 src/main/java/org/apache/commons/cli/converters/EnumVerifier.java delete mode 100644 src/test/java/org/apache/commons/cli/converters/EnumVerifierTest.java diff --git a/src/main/java/org/apache/commons/cli/Option.java b/src/main/java/org/apache/commons/cli/Option.java index afe476a02..6a5fc8dc1 100644 --- a/src/main/java/org/apache/commons/cli/Option.java +++ b/src/main/java/org/apache/commons/cli/Option.java @@ -23,9 +23,9 @@ Licensed to the Apache Software Foundation (ASF) under one or more import java.util.ArrayList; import java.util.List; import java.util.Objects; +import java.util.function.Predicate; import org.apache.commons.cli.converters.Converter; -import org.apache.commons.cli.converters.Verifier; /** * Describes a single command-line option. It maintains information regarding the short-name of the option, the @@ -90,7 +90,7 @@ public static final class Builder { private Converter converter; /** The verifier to verify string is valid for option */ - private Verifier verifier; + private Predicate verifier; /** * Constructs a new {@code Builder} with the minimum required parameters for an {@code Option} instance. @@ -299,7 +299,7 @@ public Builder converter(Converter converter) { * @return this builder, to allow method chaining. * @since 1.7.0 */ - public Builder verifier(Verifier verifier) { + public Builder verifier(Predicate verifier) { this.verifier = verifier; return this; } @@ -373,7 +373,7 @@ public static Builder builder(final String option) { private transient Converter converter; /** The explicit verifier for this option. May be null */ - private transient Verifier verifier; + private transient Predicate verifier; /** * Private constructor used by the nested Builder class. @@ -935,7 +935,7 @@ public void setConverter(Converter converter) { * @return the value verifier. * @since 1.7.0 */ - public Verifier getVerifier() { + public Predicate getVerifier() { return verifier == null ? TypeHandler.getVerifier(type) : verifier; } @@ -944,7 +944,7 @@ public Verifier getVerifier() { * @param verifier the Verifier to use. * @since 1.7.0 */ - public void setVerifier(Verifier verifier) { + public void setVerifier(Predicate verifier) { this.verifier = verifier; } diff --git a/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java b/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java index a01c65607..efab57dd1 100644 --- a/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java +++ b/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java @@ -21,6 +21,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more import java.io.FileInputStream; import java.net.URL; import java.util.Date; +import java.util.function.Predicate; import org.apache.commons.cli.converters.Converter; import org.apache.commons.cli.converters.Verifier; @@ -178,7 +179,7 @@ public static Options parsePattern(final String pattern) { boolean required = false; Class type = null; Converter converter = Converter.DEFAULT; - Verifier verifier = Verifier.DEFAULT; + Predicate verifier = Verifier.DEFAULT; final Options options = new Options(); diff --git a/src/main/java/org/apache/commons/cli/TypeHandler.java b/src/main/java/org/apache/commons/cli/TypeHandler.java index 31ca9ad01..0dccd5498 100644 --- a/src/main/java/org/apache/commons/cli/TypeHandler.java +++ b/src/main/java/org/apache/commons/cli/TypeHandler.java @@ -25,6 +25,7 @@ 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; import org.apache.commons.cli.converters.Converter; import org.apache.commons.cli.converters.Verifier; @@ -50,7 +51,7 @@ public class TypeHandler { private static Map, Converter> converterMap = new HashMap<>(); /** Map of classes to verifiers. */ - private static Map, Verifier> verifierMap = new HashMap<>(); + private static Map, Predicate> verifierMap = new HashMap<>(); static { resetConverters(); @@ -133,7 +134,7 @@ public static void noVerifiers() { * @param verifier The Verifier to associate with Class. May be null. * @since 1.7.0 */ - public static void register(Class clazz, Converter converter, Verifier verifier) { + public static void register(Class clazz, Converter converter, Predicate verifier) { if (converter == null) { converterMap.remove(clazz); } else { @@ -164,8 +165,8 @@ public static Converter getConverter(Class clazz) { * @return the registered verifier if any, {@link Verifier#DEFAULT} otherwise. * @since 1.7.0 */ - public static Verifier getVerifier(Class clazz) { - Verifier verifier = verifierMap.get(clazz); + public static Predicate getVerifier(Class clazz) { + Predicate verifier = verifierMap.get(clazz); return verifier == null ? Verifier.DEFAULT : verifier; } diff --git a/src/main/java/org/apache/commons/cli/converters/EnumVerifier.java b/src/main/java/org/apache/commons/cli/converters/EnumVerifier.java deleted file mode 100644 index 0d496065c..000000000 --- a/src/main/java/org/apache/commons/cli/converters/EnumVerifier.java +++ /dev/null @@ -1,46 +0,0 @@ -/* - Licensed to the Apache Software Foundation (ASF) under one or more - contributor license agreements. See the NOTICE file distributed with - this work for additional information regarding copyright ownership. - The ASF licenses this file to You under the Apache License, Version 2.0 - (the "License"); you may not use this file except in compliance with - the License. You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - */ -package org.apache.commons.cli.converters; - -import java.util.Arrays; -import java.util.List; -import java.util.stream.Collectors; - -/** - * A verifier driven by the values in an enum. - */ -public class EnumVerifier implements Verifier { - /** The list of valid names */ - private List names; - - /** - * Constructs the Verifier from an exemplar of the Enum. For example - * {@code new EnumVerifier(java.time.format.TextStyle.FULL)} would create an - * Verifier that would accept the names for any of the - * {@code java.time.format.TextStyle} values. - * @param exemplar One of the values from the accepted Enum. - */ - public EnumVerifier(Enum exemplar) { - names = Arrays.stream(exemplar.getDeclaringClass().getEnumConstants()).map(t -> ((Enum) t).name()) - .collect(Collectors.toList()); - } - - @Override - public boolean test(String str) throws RuntimeException { - return names.contains(str); - } -} diff --git a/src/main/java/org/apache/commons/cli/converters/Verifier.java b/src/main/java/org/apache/commons/cli/converters/Verifier.java index d610f0f74..75def954e 100644 --- a/src/main/java/org/apache/commons/cli/converters/Verifier.java +++ b/src/main/java/org/apache/commons/cli/converters/Verifier.java @@ -16,60 +16,78 @@ Licensed to the Apache Software Foundation (ASF) under one or more */ package org.apache.commons.cli.converters; +import java.util.Arrays; +import java.util.List; import java.util.function.Predicate; import java.util.regex.Pattern; +import java.util.stream.Collectors; /** * The definition of the functional interface to call when verifying a string * for input Like {@code Predicate} but can throw a RuntimeException. * @since 1.7.0 */ -@FunctionalInterface -public interface Verifier extends Predicate { +public final class Verifier { + + private Verifier() { + // do not instantiate + } /** * Default verifier. Always returns {@code true}. */ - Verifier DEFAULT = s -> true; + public static final Predicate DEFAULT = s -> true; /** * The Regex Pattern for the number matching. */ - Pattern NUMBER_PATTERN = Pattern.compile("-?([0-9]*\\.)?([0-9]+)$"); + public static final Pattern NUMBER_PATTERN = Pattern.compile("-?([0-9]*\\.)?([0-9]+)$"); /** * Verifies that a number string is either a valid real number (e.g. may have a * decimal point) or an integer. */ - Verifier NUMBER = s -> NUMBER_PATTERN.matcher(s).matches(); + public static final Predicate NUMBER = s -> NUMBER_PATTERN.matcher(s).matches(); /** * The Regex Pattern for the integer matching. */ - Pattern INTEGER_PATTERN = Pattern.compile("-?\\d+"); + public static final Pattern INTEGER_PATTERN = Pattern.compile("-?\\d+"); /** * Verifies that a number string is an integer. */ - Verifier INTEGER = s -> INTEGER_PATTERN.matcher(s).matches(); + public static final Predicate INTEGER = s -> INTEGER_PATTERN.matcher(s).matches(); /** * The Regex Pattern that matches valid class names. */ - Pattern CLAZZ_PATTERN = Pattern.compile( + public static final Pattern CLAZZ_PATTERN = Pattern.compile( "\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*(\\.\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*)*"); /** * Verifies that a class name is valid. */ - Verifier CLASS = s -> CLAZZ_PATTERN.matcher(s).matches(); + public static final Predicate CLASS = s -> CLAZZ_PATTERN.matcher(s).matches(); /** - * Applies the verification function to the String argument. - * @param str the String to convert - * @return {@code true} if the string is valid, {@code false} - * otherwise. - * @throws RuntimeException on error. + * Constructs a {@code Predicate} from an exemplar of an Enum. For + * example {@code new EnumVerifier(java.time.format.TextStyle.FULL)} would + * create an {@code Predicate} that would accept the names for any of + * the {@code java.time.format.TextStyle} values. + * @param exemplar One of the values from the accepted Enum. + * @return A {@code Predicate} that matches the Enum names. */ - boolean test(String str) throws RuntimeException; + public static Predicate enumVerifier(Enum exemplar) { + return new Predicate() { + /** The list of valid names */ + private final List names = Arrays.stream(exemplar.getDeclaringClass().getEnumConstants()) + .map(t -> ((Enum) t).name()).collect(Collectors.toList()); + @Override + public boolean test(String str) throws RuntimeException { + return names.contains(str); + } + }; + } + } diff --git a/src/test/java/org/apache/commons/cli/converters/EnumVerifierTest.java b/src/test/java/org/apache/commons/cli/converters/EnumVerifierTest.java deleted file mode 100644 index 7a6b6e6e6..000000000 --- a/src/test/java/org/apache/commons/cli/converters/EnumVerifierTest.java +++ /dev/null @@ -1,59 +0,0 @@ -/* - Licensed to the Apache Software Foundation (ASF) under one or more - contributor license agreements. See the NOTICE file distributed with - this work for additional information regarding copyright ownership. - The ASF licenses this file to You under the Apache License, Version 2.0 - (the "License"); you may not use this file except in compliance with - the License. You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - */ -package org.apache.commons.cli.converters; - -import static org.junit.jupiter.api.Assertions.assertEquals; - -import java.util.ArrayList; -import java.util.List; -import java.util.stream.Stream; - -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.MethodSource; - -/** - * Tests for the EnumVerifier class - * - */ -public class EnumVerifierTest { - - enum MyEnum { - ONE, Two, three, someothernumber - } - - private EnumVerifier underTest = new EnumVerifier(MyEnum.Two); - - @ParameterizedTest(name = "{0}") - @MethodSource("testData") - public void test(String str, boolean expected) { - assertEquals(expected, underTest.test(str)); - } - - private static Stream testData() { - List lst = new ArrayList<>(); - - lst.add(Arguments.of("ONE", true)); - lst.add(Arguments.of("one", false)); - lst.add(Arguments.of("Two", true)); - lst.add(Arguments.of("three", true)); - lst.add(Arguments.of("someothernumber", true)); - lst.add(Arguments.of("NonValue", false)); - - return lst.stream(); - } -} diff --git a/src/test/java/org/apache/commons/cli/converters/VerifierTests.java b/src/test/java/org/apache/commons/cli/converters/VerifierTests.java index ff655a920..d73de6593 100644 --- a/src/test/java/org/apache/commons/cli/converters/VerifierTests.java +++ b/src/test/java/org/apache/commons/cli/converters/VerifierTests.java @@ -16,10 +16,19 @@ Licensed to the Apache Software Foundation (ASF) under one or more */ package org.apache.commons.cli.converters; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.util.ArrayList; +import java.util.List; +import java.util.function.Predicate; +import java.util.stream.Stream; + import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; /** * Tests for standard Verifiers. @@ -99,4 +108,29 @@ public void classTests() { assertTrue(Verifier.CLASS.test("foo" + s + "bar"), "foo" + s + "bar"); } } + + enum MyEnum { + ONE, Two, three, someothernumber + } + + private Predicate underTest = Verifier.enumVerifier(MyEnum.Two); + + @ParameterizedTest(name = "{0}") + @MethodSource("testData") + public void test(String str, boolean expected) { + assertEquals(expected, underTest.test(str)); + } + + private static Stream testData() { + List lst = new ArrayList<>(); + + lst.add(Arguments.of("ONE", true)); + lst.add(Arguments.of("one", false)); + lst.add(Arguments.of("Two", true)); + lst.add(Arguments.of("three", true)); + lst.add(Arguments.of("someothernumber", true)); + lst.add(Arguments.of("NonValue", false)); + + return lst.stream(); + } } From 9b939367cdc5e9635d98e04a84250eb4f06640b7 Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Thu, 11 Jan 2024 11:53:59 +0100 Subject: [PATCH 15/24] Moved Converter, Verifier and their respective tests to base cli package. --- .../cli/{converters => }/Converter.java | 2 +- .../java/org/apache/commons/cli/Option.java | 2 -- .../commons/cli/PatternOptionBuilder.java | 3 --- .../org/apache/commons/cli/TypeHandler.java | 3 --- .../cli/{converters => }/Verifier.java | 2 +- .../commons/cli/converters/package-info.java | 21 ------------------- .../cli/{converters => }/ConverterTests.java | 2 +- .../org/apache/commons/cli/OptionTest.java | 2 -- .../apache/commons/cli/TypeHandlerTest.java | 2 -- .../cli/{converters => }/VerifierTests.java | 2 +- 10 files changed, 4 insertions(+), 37 deletions(-) rename src/main/java/org/apache/commons/cli/{converters => }/Converter.java (98%) rename src/main/java/org/apache/commons/cli/{converters => }/Verifier.java (98%) delete mode 100644 src/main/java/org/apache/commons/cli/converters/package-info.java rename src/test/java/org/apache/commons/cli/{converters => }/ConverterTests.java (99%) rename src/test/java/org/apache/commons/cli/{converters => }/VerifierTests.java (99%) diff --git a/src/main/java/org/apache/commons/cli/converters/Converter.java b/src/main/java/org/apache/commons/cli/Converter.java similarity index 98% rename from src/main/java/org/apache/commons/cli/converters/Converter.java rename to src/main/java/org/apache/commons/cli/Converter.java index 2d857dc27..66598d16e 100644 --- a/src/main/java/org/apache/commons/cli/converters/Converter.java +++ b/src/main/java/org/apache/commons/cli/Converter.java @@ -14,7 +14,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more See the License for the specific language governing permissions and limitations under the License. */ -package org.apache.commons.cli.converters; +package org.apache.commons.cli; import java.io.File; import java.net.URL; diff --git a/src/main/java/org/apache/commons/cli/Option.java b/src/main/java/org/apache/commons/cli/Option.java index 6a5fc8dc1..e55b8f5cb 100644 --- a/src/main/java/org/apache/commons/cli/Option.java +++ b/src/main/java/org/apache/commons/cli/Option.java @@ -25,8 +25,6 @@ Licensed to the Apache Software Foundation (ASF) under one or more import java.util.Objects; import java.util.function.Predicate; -import org.apache.commons.cli.converters.Converter; - /** * Describes a single command-line option. It maintains information regarding the short-name of the option, the * long-name, if any exists, a flag indicating if an argument is required for this option, and a self-documenting diff --git a/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java b/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java index efab57dd1..ce9be8ed3 100644 --- a/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java +++ b/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java @@ -23,9 +23,6 @@ Licensed to the Apache Software Foundation (ASF) under one or more import java.util.Date; import java.util.function.Predicate; -import org.apache.commons.cli.converters.Converter; -import org.apache.commons.cli.converters.Verifier; - /** * Allows Options to be created from a single String. The pattern contains various single character flags and via an * optional punctuation character, their expected type. diff --git a/src/main/java/org/apache/commons/cli/TypeHandler.java b/src/main/java/org/apache/commons/cli/TypeHandler.java index 0dccd5498..acd3ebfd8 100644 --- a/src/main/java/org/apache/commons/cli/TypeHandler.java +++ b/src/main/java/org/apache/commons/cli/TypeHandler.java @@ -27,9 +27,6 @@ Licensed to the Apache Software Foundation (ASF) under one or more import java.util.Map; import java.util.function.Predicate; -import org.apache.commons.cli.converters.Converter; -import org.apache.commons.cli.converters.Verifier; - /** * TypeHandler will handle the pluggable conversion and verification of * Option types. It handles the mapping of classes to bot converters and verifiers. diff --git a/src/main/java/org/apache/commons/cli/converters/Verifier.java b/src/main/java/org/apache/commons/cli/Verifier.java similarity index 98% rename from src/main/java/org/apache/commons/cli/converters/Verifier.java rename to src/main/java/org/apache/commons/cli/Verifier.java index 75def954e..7b0b17a14 100644 --- a/src/main/java/org/apache/commons/cli/converters/Verifier.java +++ b/src/main/java/org/apache/commons/cli/Verifier.java @@ -14,7 +14,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more See the License for the specific language governing permissions and limitations under the License. */ -package org.apache.commons.cli.converters; +package org.apache.commons.cli; import java.util.Arrays; import java.util.List; diff --git a/src/main/java/org/apache/commons/cli/converters/package-info.java b/src/main/java/org/apache/commons/cli/converters/package-info.java deleted file mode 100644 index 1cbf50ac4..000000000 --- a/src/main/java/org/apache/commons/cli/converters/package-info.java +++ /dev/null @@ -1,21 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -/** - * Apache Commons CLI converters to convert from String to Object of various types. - */ -package org.apache.commons.cli.converters; diff --git a/src/test/java/org/apache/commons/cli/converters/ConverterTests.java b/src/test/java/org/apache/commons/cli/ConverterTests.java similarity index 99% rename from src/test/java/org/apache/commons/cli/converters/ConverterTests.java rename to src/test/java/org/apache/commons/cli/ConverterTests.java index dd0f304bf..841646407 100644 --- a/src/test/java/org/apache/commons/cli/converters/ConverterTests.java +++ b/src/test/java/org/apache/commons/cli/ConverterTests.java @@ -14,7 +14,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more See the License for the specific language governing permissions and limitations under the License. */ -package org.apache.commons.cli.converters; +package org.apache.commons.cli; import static org.junit.Assert.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; diff --git a/src/test/java/org/apache/commons/cli/OptionTest.java b/src/test/java/org/apache/commons/cli/OptionTest.java index 352e9f217..180b04926 100644 --- a/src/test/java/org/apache/commons/cli/OptionTest.java +++ b/src/test/java/org/apache/commons/cli/OptionTest.java @@ -31,8 +31,6 @@ import java.io.ObjectInputStream; import java.io.ObjectOutputStream; -import org.apache.commons.cli.converters.Converter; -import org.apache.commons.cli.converters.Verifier; import org.junit.Test; public class OptionTest { diff --git a/src/test/java/org/apache/commons/cli/TypeHandlerTest.java b/src/test/java/org/apache/commons/cli/TypeHandlerTest.java index 370dbd8da..44e08a2f2 100644 --- a/src/test/java/org/apache/commons/cli/TypeHandlerTest.java +++ b/src/test/java/org/apache/commons/cli/TypeHandlerTest.java @@ -32,8 +32,6 @@ Licensed to the Apache Software Foundation (ASF) under one or more import java.util.List; import java.util.stream.Stream; -import org.apache.commons.cli.converters.Converter; -import org.apache.commons.cli.converters.Verifier; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; diff --git a/src/test/java/org/apache/commons/cli/converters/VerifierTests.java b/src/test/java/org/apache/commons/cli/VerifierTests.java similarity index 99% rename from src/test/java/org/apache/commons/cli/converters/VerifierTests.java rename to src/test/java/org/apache/commons/cli/VerifierTests.java index d73de6593..1122cc5b8 100644 --- a/src/test/java/org/apache/commons/cli/converters/VerifierTests.java +++ b/src/test/java/org/apache/commons/cli/VerifierTests.java @@ -14,7 +14,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more See the License for the specific language governing permissions and limitations under the License. */ -package org.apache.commons.cli.converters; +package org.apache.commons.cli; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; From a8d7c8936eef7a4074d950ed6b87c63f1be44c3b Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Thu, 11 Jan 2024 13:26:22 +0100 Subject: [PATCH 16/24] Removed verifier management and rebased to master --- .../org/apache/commons/cli/CommandLine.java | 6 +- .../java/org/apache/commons/cli/Option.java | 10 +-- .../org/apache/commons/cli/OptionBuilder.java | 2 +- .../commons/cli/PatternOptionBuilder.java | 10 ++- .../org/apache/commons/cli/TypeHandler.java | 61 ++----------------- .../java/org/apache/commons/cli/Verifier.java | 4 +- .../apache/commons/cli/ConverterTests.java | 10 +-- .../org/apache/commons/cli/OptionTest.java | 10 +-- .../commons/cli/PatternOptionBuilderTest.java | 13 ++-- .../apache/commons/cli/TypeHandlerTest.java | 44 +++---------- .../org/apache/commons/cli/VerifierTests.java | 2 +- 11 files changed, 53 insertions(+), 119 deletions(-) diff --git a/src/main/java/org/apache/commons/cli/CommandLine.java b/src/main/java/org/apache/commons/cli/CommandLine.java index f23d805d1..b7e83f581 100644 --- 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 getParsedOptionValue(final String opt) throws ParseException { * @since 1.7.0 * @param The return type for the method. */ - public T getParsedOptionValue(final char opt, T defaultValue) throws ParseException { + public T getParsedOptionValue(final char opt, final T defaultValue) throws ParseException { return getParsedOptionValue(String.valueOf(opt), defaultValue); } @@ -414,7 +414,7 @@ public T getParsedOptionValue(final char opt, T defaultValue) throws ParseEx * @param The return type for the method. */ @SuppressWarnings("unchecked") - public T getParsedOptionValue(final Option option, T defaultValue) throws ParseException { + public T getParsedOptionValue(final Option option, final T defaultValue) throws ParseException { if (option == null) { return null; } @@ -438,7 +438,7 @@ public T getParsedOptionValue(final Option option, T defaultValue) throws Pa * @since 1.7.0 * @param The return type for the method. */ - public T getParsedOptionValue(final String opt, T defaultValue) throws ParseException { + public 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 index adb7410fa..db84d1765 100644 --- 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 verifier) { + public Builder verifier(final Predicate 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,7 +935,7 @@ public void setConverter(Converter converter) { * @since 1.7.0 */ public Predicate getVerifier() { - return verifier == null ? TypeHandler.getVerifier(type) : verifier; + return verifier == null ? Verifier.DEFAULT : verifier; } /** @@ -943,7 +943,7 @@ public Predicate getVerifier() { * @param verifier the Verifier to use. * @since 1.7.0 */ - public void setVerifier(Predicate verifier) { + public void setVerifier(final Predicate 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 index 2125fbb94..6ffca8030 100644 --- 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 index ce9be8ed3..b65c11def 100644 --- 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 index acd3ebfd8..8f0dddc4a 100644 --- 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, Converter> converterMap = new HashMap<>(); - - /** Map of classes to verifiers. */ - private static Map, Predicate> 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 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 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 getVerifier(Class clazz) { - Predicate 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 index 7b0b17a14..7eb3967bc 100644 --- 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} that matches the Enum names. */ - public static Predicate enumVerifier(Enum exemplar) { + public static Predicate enumVerifier(final Enum exemplar) { return new Predicate() { /** The list of valid names */ private final List 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 index 841646407..a647f705a 100644 --- 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 index 180b04926..b1bec742d 100644 --- 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 index 378f5749a..a0ceb528b 100644 --- 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,6 +31,7 @@ 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; /** @@ -39,6 +39,11 @@ Licensed to the Apache Software Foundation (ASF) under one or more */ @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 index 44e08a2f2..fbaac0a0c 100644 --- a/src/test/java/org/apache/commons/cli/TypeHandlerTest.java +++ b/src/test/java/org/apache/commons/cli/TypeHandlerTest.java @@ -43,7 +43,7 @@ public class TypeHandlerTest { public static class Instantiable { @Override - public boolean equals(Object arg0) { + public boolean equals(final Object arg0) { return arg0 instanceof Instantiable; } @@ -69,64 +69,38 @@ private NotInstantiable() { @Test public void testRegister() { assertEquals(Converter.DEFAULT, TypeHandler.getConverter(NotInstantiable.class)); - assertEquals(Verifier.DEFAULT, TypeHandler.getVerifier(NotInstantiable.class)); try { - TypeHandler.register(NotInstantiable.class, Converter.DATE, Verifier.NUMBER); + TypeHandler.register(NotInstantiable.class, Converter.DATE); assertEquals(Converter.DATE, TypeHandler.getConverter(NotInstantiable.class)); - assertEquals(Verifier.NUMBER, TypeHandler.getVerifier(NotInstantiable.class)); } finally { - TypeHandler.register(NotInstantiable.class, null, null); + TypeHandler.register(NotInstantiable.class, null); assertEquals(Converter.DEFAULT, TypeHandler.getConverter(NotInstantiable.class)); - assertEquals(Verifier.DEFAULT, TypeHandler.getVerifier(NotInstantiable.class)); } } @Test - public void testResetConvertersAndVerifiers() { + public void testResetConverters() { assertEquals(Converter.DEFAULT, TypeHandler.getConverter(NotInstantiable.class)); - assertEquals(Verifier.DEFAULT, TypeHandler.getVerifier(NotInstantiable.class)); try { - TypeHandler.register(NotInstantiable.class, Converter.DATE, Verifier.NUMBER); + TypeHandler.register(NotInstantiable.class, Converter.DATE); assertEquals(Converter.DATE, TypeHandler.getConverter(NotInstantiable.class)); - assertEquals(Verifier.NUMBER, TypeHandler.getVerifier(NotInstantiable.class)); TypeHandler.resetConverters(); assertEquals(Converter.DEFAULT, TypeHandler.getConverter(NotInstantiable.class)); - assertEquals(Verifier.NUMBER, TypeHandler.getVerifier(NotInstantiable.class)); - TypeHandler.resetVerifiers(); assertEquals(Converter.DEFAULT, TypeHandler.getConverter(NotInstantiable.class)); - assertEquals(Verifier.DEFAULT, TypeHandler.getVerifier(NotInstantiable.class)); } finally { - TypeHandler.register(NotInstantiable.class, null, null); + TypeHandler.register(NotInstantiable.class, null); } } @Test public void testNoConverters() { assertEquals(Converter.NUMBER, TypeHandler.getConverter(Number.class)); - assertEquals(Verifier.NUMBER, TypeHandler.getVerifier(Number.class)); try { TypeHandler.noConverters(); assertEquals(Converter.DEFAULT, TypeHandler.getConverter(Number.class)); - assertEquals(Verifier.NUMBER, TypeHandler.getVerifier(Number.class)); } finally { TypeHandler.resetConverters(); assertEquals(Converter.NUMBER, TypeHandler.getConverter(Number.class)); - assertEquals(Verifier.NUMBER, TypeHandler.getVerifier(Number.class)); - } - } - - @Test - public void testNoVerifiers() { - assertEquals(Converter.NUMBER, TypeHandler.getConverter(Number.class)); - assertEquals(Verifier.NUMBER, TypeHandler.getVerifier(Number.class)); - try { - TypeHandler.noVerifiers(); - assertEquals(Converter.NUMBER, TypeHandler.getConverter(Number.class)); - assertEquals(Verifier.DEFAULT, TypeHandler.getVerifier(Number.class)); - } finally { - TypeHandler.resetVerifiers(); - assertEquals(Converter.NUMBER, TypeHandler.getConverter(Number.class)); - assertEquals(Verifier.NUMBER, TypeHandler.getVerifier(Number.class)); } } @@ -142,7 +116,7 @@ public void testCreateValueExistingFile() throws Exception { @SuppressWarnings("unchecked") @ParameterizedTest(name = "{0} as {1}") @MethodSource("createValueTestParameters") - public void createValueTests(String str, Class type, Object expected) throws Exception { + public void createValueTests(final String str, final Class type, final Object expected) throws Exception { if (expected instanceof Class && Throwable.class.isAssignableFrom((Class) expected)) { assertThrows((Class) expected, () -> TypeHandler.createValue(str, type)); } else { @@ -151,6 +125,7 @@ public void createValueTests(String str, Class type, Object expected) throws } private static Stream createValueTestParameters() { + TypeHandler.resetConverters(); List lst = new ArrayList<>(); try { @@ -166,7 +141,8 @@ private static Stream createValueTestParameters() { lst.add(Arguments.of("some-file.txt", PatternOptionBuilder.FILE_VALUE, new File("some-file.txt"))); - lst.add(Arguments.of("some.files", PatternOptionBuilder.FILES_VALUE, UnsupportedOperationException.class)); + // the PatternOptionBUilder.FILES_VALUE is not registered so it should just return the string + lst.add(Arguments.of("some.files", PatternOptionBuilder.FILES_VALUE, "some.files")); lst.add(Arguments.of("just-a-string", Integer.class, ParseException.class)); lst.add(Arguments.of("5", Integer.class, 5)); diff --git a/src/test/java/org/apache/commons/cli/VerifierTests.java b/src/test/java/org/apache/commons/cli/VerifierTests.java index 1122cc5b8..80302b1ac 100644 --- a/src/test/java/org/apache/commons/cli/VerifierTests.java +++ b/src/test/java/org/apache/commons/cli/VerifierTests.java @@ -117,7 +117,7 @@ enum MyEnum { @ParameterizedTest(name = "{0}") @MethodSource("testData") - public void test(String str, boolean expected) { + public void test(final String str, final boolean expected) { assertEquals(expected, underTest.test(str)); } From d588a865d0c01d3192f0056ce446ef0497e7d223 Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Thu, 11 Jan 2024 13:32:51 +0100 Subject: [PATCH 17/24] moved param tags --- .../java/org/apache/commons/cli/CommandLine.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/apache/commons/cli/CommandLine.java b/src/main/java/org/apache/commons/cli/CommandLine.java index b7e83f581..13c94e8ec 100644 --- a/src/main/java/org/apache/commons/cli/CommandLine.java +++ b/src/main/java/org/apache/commons/cli/CommandLine.java @@ -349,11 +349,11 @@ public String[] getOptionValues(final String opt) { * Gets a version of this {@code Option} converted to a particular type. * * @param opt the name of the option. + * @param The return type for the method. * @return the value parsed into a particular object. * @throws ParseException if there are problems turning the option value into the desired type * @see PatternOptionBuilder * @since 1.5.0 - * @param The return type for the method. */ public T getParsedOptionValue(final char opt) throws ParseException { return getParsedOptionValue(String.valueOf(opt)); @@ -363,11 +363,11 @@ public T getParsedOptionValue(final char opt) throws ParseException { * Gets a version of this {@code Option} converted to a particular type. * * @param option the name of the option. + * @param The return type for the method. * @return the value parsed into a particular object. * @throws ParseException if there are problems turning the option value into the desired type * @see PatternOptionBuilder * @since 1.5.0 - * @param The return type for the method. */ public T getParsedOptionValue(final Option option) throws ParseException { return getParsedOptionValue(option, null); @@ -377,11 +377,11 @@ public T getParsedOptionValue(final Option option) throws ParseException { * Gets a version of this {@code Option} converted to a particular type. * * @param opt the name of the option. + * @param The return type for the method. * @return the value parsed into a particular object. * @throws ParseException if there are problems turning the option value into the desired type * @see PatternOptionBuilder * @since 1.2 - * @param The return type for the method. */ public T getParsedOptionValue(final String opt) throws ParseException { return getParsedOptionValue(resolveOption(opt)); @@ -392,11 +392,11 @@ public T getParsedOptionValue(final String opt) throws ParseException { * * @param opt the name of the option. * @param defaultValue the default value to return if opt is not set. + * @param The return type for the method. * @return the value parsed into a particular object. * @throws ParseException if there are problems turning the option value into the desired type * @see PatternOptionBuilder * @since 1.7.0 - * @param The return type for the method. */ public T getParsedOptionValue(final char opt, final T defaultValue) throws ParseException { return getParsedOptionValue(String.valueOf(opt), defaultValue); @@ -407,11 +407,11 @@ public T getParsedOptionValue(final char opt, final T defaultValue) throws P * * @param option the name of the option. * @param defaultValue the default value to return if opt is not set. + * @param The return type for the method. * @return the value parsed into a particular object. * @throws ParseException if there are problems turning the option value into the desired type * @see PatternOptionBuilder * @since 1.7.0 - * @param The return type for the method. */ @SuppressWarnings("unchecked") public T getParsedOptionValue(final Option option, final T defaultValue) throws ParseException { @@ -432,11 +432,11 @@ public T getParsedOptionValue(final Option option, final T defaultValue) thr * * @param opt the name of the option. * @param defaultValue the default value to return if opt is not set. + * @param The return type for the method. * @return the value parsed into a particular object. * @throws ParseException if there are problems turning the option value into the desired type * @see PatternOptionBuilder * @since 1.7.0 - * @param The return type for the method. */ public T getParsedOptionValue(final String opt, final T defaultValue) throws ParseException { return getParsedOptionValue(resolveOption(opt), defaultValue); From 0eee294d794c40124b820f1c35dcd338c9454bcd Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Mon, 15 Jan 2024 13:38:10 +0100 Subject: [PATCH 18/24] updated numberic tests --- src/main/java/org/apache/commons/cli/Verifier.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/commons/cli/Verifier.java b/src/main/java/org/apache/commons/cli/Verifier.java index 7eb3967bc..e8a6e5525 100644 --- a/src/main/java/org/apache/commons/cli/Verifier.java +++ b/src/main/java/org/apache/commons/cli/Verifier.java @@ -41,7 +41,7 @@ private Verifier() { /** * The Regex Pattern for the number matching. */ - public static final Pattern NUMBER_PATTERN = Pattern.compile("-?([0-9]*\\.)?([0-9]+)$"); + public static final Pattern NUMBER_PATTERN = Pattern.compile("[-+]?(\\d*\\.)?(\\d+)$"); /** * Verifies that a number string is either a valid real number (e.g. may have a @@ -52,7 +52,7 @@ private Verifier() { /** * The Regex Pattern for the integer matching. */ - public static final Pattern INTEGER_PATTERN = Pattern.compile("-?\\d+"); + public static final Pattern INTEGER_PATTERN = Pattern.compile("[-+]?\\d+"); /** * Verifies that a number string is an integer. From 658aa2b0aa63a6042107d88ab50a33b76f664c1b Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Mon, 22 Jan 2024 13:19:39 +0100 Subject: [PATCH 19/24] Updated documentation, added Supplier as a default provider for getOptionValue --- .../org/apache/commons/cli/CommandLine.java | 43 ++++++++- src/site/xdoc/usage.xml | 88 +++++++++++++++++++ .../org/apache/commons/cli/bug/BugsTest.java | 2 +- 3 files changed, 130 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/apache/commons/cli/CommandLine.java b/src/main/java/org/apache/commons/cli/CommandLine.java index 13c94e8ec..b57c01b76 100644 --- a/src/main/java/org/apache/commons/cli/CommandLine.java +++ b/src/main/java/org/apache/commons/cli/CommandLine.java @@ -24,7 +24,9 @@ Licensed to the Apache Software Foundation (ASF) under one or more import java.util.Iterator; import java.util.LinkedList; import java.util.List; +import java.util.Objects; import java.util.Properties; +import java.util.function.Supplier; /** * Represents list of arguments parsed against a {@link Options} descriptor. @@ -254,6 +256,18 @@ public String getOptionValue(final char opt) { * @return Value of the argument if option is set, and has an argument, otherwise {@code defaultValue}. */ public String getOptionValue(final char opt, final String defaultValue) { + return getOptionValue(String.valueOf(opt), ()->defaultValue); + } + + /** + * Gets the argument, if any, of an option. + * + * @param opt character name of the option + * @param defaultValue is a supplier for the default value to be returned if the option is not specified. + * @return Value of the argument if option is set, and has an argument, otherwise {@code defaultValue}. + * @since 1.7.0 + */ + public String getOptionValue(final char opt, final Supplier defaultValue) { return getOptionValue(String.valueOf(opt), defaultValue); } @@ -281,10 +295,22 @@ public String getOptionValue(final Option option) { * @since 1.5.0 */ public String getOptionValue(final Option option, final String defaultValue) { - final String answer = getOptionValue(option); - return answer != null ? answer : defaultValue; + return getOptionValue(option, ()->defaultValue); } + /** + * Gets the first argument, if any, of an option. + * + * @param option name of the option. + * @param defaultValue is a supplier for the default value to be returned if the option is not specified. + * @return Value of the argument if option is set, and has an argument, otherwise {@code defaultValue}. + * @since 1.7.0 + */ + public String getOptionValue(final Option option, final Supplier defaultValue) { + final String answer = getOptionValue(option); + return answer != null ? answer : defaultValue.get(); + } + /** * Gets the first argument, if any, of this option. * @@ -303,9 +329,22 @@ public String getOptionValue(final String opt) { * @return Value of the argument if option is set, and has an argument, otherwise {@code defaultValue}. */ public String getOptionValue(final String opt, final String defaultValue) { + return getOptionValue(resolveOption(opt), ()->defaultValue); + } + + /** + * Gets the first argument, if any, of an option. + * + * @param opt name of the option. + * @param defaultValue is a supplier for the default value to be returned if the option is not specified. + * @return Value of the argument if option is set, and has an argument, otherwise {@code defaultValue}. + * @since 1.7.0 + */ + public String getOptionValue(final String opt, final Supplier defaultValue) { return getOptionValue(resolveOption(opt), defaultValue); } + /** * Gets the array of values, if any, of an option. * diff --git a/src/site/xdoc/usage.xml b/src/site/xdoc/usage.xml index ee116c775..f00f52de5 100644 --- a/src/site/xdoc/usage.xml +++ b/src/site/xdoc/usage.xml @@ -384,5 +384,93 @@ catch (ParseException exp) { System.out.println("Unexpected exception:" + exp.getMessage()); } +

+

+ By in most cases the values on the command line are retrieved as Strings via the + commandLine.getOptionValue(key) command. However, it is possible for + the CLI library to convert the string into a different object. For example to specify + that the "count" option should reutrn an Integer the following code could be used: +

+ + public static void main(String[] args) { + Option count = Option.builder("count") + .hasArg() + .desc("the number of things") + .type(Integer.class) + .build(); + Options options = new Options().addOption(cound); + // create the parser + CommandLineParser parser = new DefaultParser(); + try { + // parse the command line arguments + CommandLine line = parser.parse(options, args); + } + catch (ParseException exp) { + // oops, something went wrong + System.err.println("Parsing failed. Reason: " + exp.getMessage()); + } + + try { + Integer value = line.getParsedOptionValue(count); + System.out.format("The value is %s\m", value ); + } catch (ParseException e) { + e.printStackTrace(); + } + } +

+ The value types natively supported by commons-cli are: +

    +
  • Object.class - The string value must be the name of a class with a no argument constructor
    • +
  • Class.class - The string value must be the name of a class
    • +
  • Date.class - The string value must be a date parsable by new SimpleDateFormat("EEE MMM dd HH:mm:ss zzz yyyy")
    • +
  • File.class - The string value is the name of the file.
    • +
  • Number.class - The string value is a number representation can can be converted into an Integer or a Double.
    • +
  • URL.class - The string value is the textual representation of a URL
    • +
  • FileInputStream.class - The string value is passed to new FileInputStream(s).
    • +
  • Long.class - The string value is a valid argument to Long.parseLong().
    • +
  • Integer.class - The string value is a valid argument to Integer.parseInt().
    • +
  • Short.class - The string value is a valid argument to Short.parseShort().
    • +
  • Byte.class - The string value is a valid argument to Byte.parseByte().
    • +
  • Character.class - The string value is either a UTF-8 encoding for a character (e.g. "\\u0124") or the first character from the String."
    • +
  • Double.class - The string value is a valid argument to Double.parseDouble().
    • +
  • Float.class - The string value is a valid argument to Float.parseFloat().
    • +
  • BigInteger.class - The string value is a valid argument to new BigInteger(s).
    • +
  • BigDecimal.class - The string value is a valid argument to new BigDecimal(s).
    • +
+ Additional types may be added to the automatic parsing system by calling TypeHandler.register(Class<T> clazz, Converter<T> converter). + The Class<T> can be any defined class. The converter is a function that takes a String argument and returns an instance of + the class. Any expection thrown by the constructor will be caught and reported as a ParseException +

+

+ Conversions can be specified without using the TypeHandler class by specifying the converter + directly during the option build. For example: + + Option fooOpt = Option.builder("foo") + .hasArg() + .desc("the foo arg") + .converter((s) -> new Foo(s)) + .build(); + + The above will create an option that passes the string value to the Foo constructor when commandLine.getParsedOptionValue(fooOpt) is called. +

+
+
+

+ Acceptable values for options may be specified by using Option.Builder.verifier() to specify a java.util.function.Predicate + to verify the input text. There are a few examples in the Verifier source file. Verifier also provides a method to construct a verifier + to test enum values. For example: + + public enum MyEnum {ONE, TWO, THREE}; + + Option enumOpt = Option.builder("enumValue") + .hasArg() + .desc("the enum arg") + .varifier(Verifier.enumVerifier(MyEnum.ONE)) + .build(); + + The above will create an option that verifies any argument passed to enumOpt matches a name from MyEnum. + The initial verification occures when the option string is set during the execution of CommandLine line = parser.parse(options, args). +

+
diff --git a/src/test/java/org/apache/commons/cli/bug/BugsTest.java b/src/test/java/org/apache/commons/cli/bug/BugsTest.java index eed3381a7..11b1d4d24 100644 --- a/src/test/java/org/apache/commons/cli/bug/BugsTest.java +++ b/src/test/java/org/apache/commons/cli/bug/BugsTest.java @@ -137,7 +137,7 @@ public void test11680() throws Exception { cmd.getOptionValue("f", "default f"); cmd.getOptionValue("m", "default m"); // - assertNull(cmd.getOptionValue((String) null, null)); + assertNull(cmd.getOptionValue((String) null, (String)null)); assertEquals("default", cmd.getOptionValue((String) null, "default")); } From b68633a1f66db0044f7603a8da072b4b94cafd14 Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Mon, 22 Jan 2024 13:26:08 +0100 Subject: [PATCH 20/24] fixed checkstyle issues --- src/main/java/org/apache/commons/cli/CommandLine.java | 7 +++---- src/test/java/org/apache/commons/cli/bug/BugsTest.java | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/apache/commons/cli/CommandLine.java b/src/main/java/org/apache/commons/cli/CommandLine.java index b57c01b76..6570b8245 100644 --- a/src/main/java/org/apache/commons/cli/CommandLine.java +++ b/src/main/java/org/apache/commons/cli/CommandLine.java @@ -24,7 +24,6 @@ Licensed to the Apache Software Foundation (ASF) under one or more import java.util.Iterator; import java.util.LinkedList; import java.util.List; -import java.util.Objects; import java.util.Properties; import java.util.function.Supplier; @@ -256,7 +255,7 @@ public String getOptionValue(final char opt) { * @return Value of the argument if option is set, and has an argument, otherwise {@code defaultValue}. */ public String getOptionValue(final char opt, final String defaultValue) { - return getOptionValue(String.valueOf(opt), ()->defaultValue); + return getOptionValue(String.valueOf(opt), () -> defaultValue); } /** @@ -295,7 +294,7 @@ public String getOptionValue(final Option option) { * @since 1.5.0 */ public String getOptionValue(final Option option, final String defaultValue) { - return getOptionValue(option, ()->defaultValue); + return getOptionValue(option, () -> defaultValue); } /** @@ -329,7 +328,7 @@ public String getOptionValue(final String opt) { * @return Value of the argument if option is set, and has an argument, otherwise {@code defaultValue}. */ public String getOptionValue(final String opt, final String defaultValue) { - return getOptionValue(resolveOption(opt), ()->defaultValue); + return getOptionValue(resolveOption(opt), () -> defaultValue); } /** diff --git a/src/test/java/org/apache/commons/cli/bug/BugsTest.java b/src/test/java/org/apache/commons/cli/bug/BugsTest.java index 11b1d4d24..124da0a19 100644 --- a/src/test/java/org/apache/commons/cli/bug/BugsTest.java +++ b/src/test/java/org/apache/commons/cli/bug/BugsTest.java @@ -137,7 +137,7 @@ public void test11680() throws Exception { cmd.getOptionValue("f", "default f"); cmd.getOptionValue("m", "default m"); // - assertNull(cmd.getOptionValue((String) null, (String)null)); + assertNull(cmd.getOptionValue((String) null, (String) null)); assertEquals("default", cmd.getOptionValue((String) null, "default")); } From 1ca90bb7951d9861e2acd9074c9d0f1a38408bd0 Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Mon, 22 Jan 2024 13:59:26 +0100 Subject: [PATCH 21/24] Added Path to TypeHandler and updated documentation --- src/main/java/org/apache/commons/cli/Converter.java | 4 ++++ src/main/java/org/apache/commons/cli/TypeHandler.java | 2 ++ src/site/xdoc/usage.xml | 1 + src/test/java/org/apache/commons/cli/TypeHandlerTest.java | 6 ++++++ 4 files changed, 13 insertions(+) diff --git a/src/main/java/org/apache/commons/cli/Converter.java b/src/main/java/org/apache/commons/cli/Converter.java index 66598d16e..3ac04a16a 100644 --- a/src/main/java/org/apache/commons/cli/Converter.java +++ b/src/main/java/org/apache/commons/cli/Converter.java @@ -18,6 +18,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more import java.io.File; import java.net.URL; +import java.nio.file.Path; import java.text.SimpleDateFormat; import java.util.Date; @@ -40,6 +41,9 @@ public interface Converter { /** File name converter. Calls @{code new File(s)} */ Converter FILE = s -> new File(s); + /** Path converter. Calls @{code new Path(s)} */ + Converter PATH = s -> new File(s).toPath(); + /** * Number converter. Converts to a Double if a decimal point ('.') is in the * string or a Long otherwise. diff --git a/src/main/java/org/apache/commons/cli/TypeHandler.java b/src/main/java/org/apache/commons/cli/TypeHandler.java index 8f0dddc4a..0972e5da6 100644 --- a/src/main/java/org/apache/commons/cli/TypeHandler.java +++ b/src/main/java/org/apache/commons/cli/TypeHandler.java @@ -22,6 +22,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more import java.math.BigDecimal; import java.math.BigInteger; import java.net.URL; +import java.nio.file.Path; import java.util.Date; import java.util.HashMap; import java.util.Map; @@ -60,6 +61,7 @@ public static void resetConverters() { converterMap.put(Class.class, Converter.CLASS); converterMap.put(Date.class, Converter.DATE); converterMap.put(File.class, Converter.FILE); + converterMap.put(Path.class, Converter.PATH); converterMap.put(Number.class, Converter.NUMBER); converterMap.put(URL.class, Converter.URL); converterMap.put(FileInputStream.class, s -> new FileInputStream(s)); diff --git a/src/site/xdoc/usage.xml b/src/site/xdoc/usage.xml index f00f52de5..4ab1cf9e4 100644 --- a/src/site/xdoc/usage.xml +++ b/src/site/xdoc/usage.xml @@ -424,6 +424,7 @@ catch (ParseException exp) {
  • Class.class - The string value must be the name of a class
  • Date.class - The string value must be a date parsable by new SimpleDateFormat("EEE MMM dd HH:mm:ss zzz yyyy")
  • File.class - The string value is the name of the file.
    • +
  • Path.class - The string value is the name of a Path.
  • Number.class - The string value is a number representation can can be converted into an Integer or a Double.
  • URL.class - The string value is the textual representation of a URL
  • FileInputStream.class - The string value is passed to new FileInputStream(s).
    • diff --git a/src/test/java/org/apache/commons/cli/TypeHandlerTest.java b/src/test/java/org/apache/commons/cli/TypeHandlerTest.java index fbaac0a0c..26c051dfb 100644 --- a/src/test/java/org/apache/commons/cli/TypeHandlerTest.java +++ b/src/test/java/org/apache/commons/cli/TypeHandlerTest.java @@ -27,6 +27,7 @@ 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.Path; import java.util.ArrayList; import java.util.Date; import java.util.List; @@ -125,6 +126,9 @@ public void createValueTests(final String str, final Class type, final Object } private static Stream createValueTestParameters() { + // forse the PatternOptionBuilder to load / modify the TypeHandler table. + Class ignore = PatternOptionBuilder.FILES_VALUE; + // reset the type handler table. TypeHandler.resetConverters(); List lst = new ArrayList<>(); @@ -140,6 +144,8 @@ private static Stream createValueTestParameters() { lst.add(Arguments.of("non-existing.file", PatternOptionBuilder.EXISTING_FILE_VALUE, ParseException.class)); lst.add(Arguments.of("some-file.txt", PatternOptionBuilder.FILE_VALUE, new File("some-file.txt"))); + + lst.add(Arguments.of("some-path.txt", Path.class, new File("some-path.txt").toPath())); // the PatternOptionBUilder.FILES_VALUE is not registered so it should just return the string lst.add(Arguments.of("some.files", PatternOptionBuilder.FILES_VALUE, "some.files")); From 4d4f52c857c1c66de1f0e3c9afb36838b86c4f89 Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Fri, 26 Jan 2024 14:39:50 +0100 Subject: [PATCH 22/24] removed verifier --- .../java/org/apache/commons/cli/Option.java | 41 ------ .../org/apache/commons/cli/OptionBuilder.java | 1 - .../commons/cli/PatternOptionBuilder.java | 4 +- .../java/org/apache/commons/cli/Verifier.java | 93 ------------ .../org/apache/commons/cli/OptionTest.java | 6 - .../org/apache/commons/cli/VerifierTests.java | 136 ------------------ 6 files changed, 1 insertion(+), 280 deletions(-) delete mode 100644 src/main/java/org/apache/commons/cli/Verifier.java delete mode 100644 src/test/java/org/apache/commons/cli/VerifierTests.java diff --git a/src/main/java/org/apache/commons/cli/Option.java b/src/main/java/org/apache/commons/cli/Option.java index db84d1765..743a06bd9 100644 --- a/src/main/java/org/apache/commons/cli/Option.java +++ b/src/main/java/org/apache/commons/cli/Option.java @@ -86,9 +86,6 @@ public static final class Builder { /** The converter to convert to type **/ private Converter converter; - - /** The verifier to verify string is valid for option */ - private Predicate verifier; /** * Constructs a new {@code Builder} with the minimum required parameters for an {@code Option} instance. @@ -290,17 +287,6 @@ public Builder converter(final Converter converter) { return this; } - /** - * Sets the verifier for the option. - *

    Note: see {@link TypeHandler} for serialization discussion.

    - * @param verifier the Verifier to use. - * @return this builder, to allow method chaining. - * @since 1.7.0 - */ - public Builder verifier(final Predicate verifier) { - this.verifier = verifier; - return this; - } } /** Specifies the number of argument values has not been specified */ @@ -369,9 +355,6 @@ public static Builder builder(final String option) { /** The explicit converter for this option. May be null */ private transient Converter converter; - - /** The explicit verifier for this option. May be null */ - private transient Predicate verifier; /** * Private constructor used by the nested Builder class. @@ -389,7 +372,6 @@ private Option(final Builder builder) { this.type = builder.type; this.valuesep = builder.valueSeparator; this.converter = builder.converter; - this.verifier = builder.verifier; } /** @@ -462,11 +444,6 @@ private void add(final String value) { if (!acceptsArg()) { throw new IllegalArgumentException("Cannot add value, list full."); } - - if (!getVerifier().test(value)) { - throw new IllegalArgumentException(String.format("'%s' is not a valid input string for option '%s'", value, option)); - } - // store value values.add(value); } @@ -929,24 +906,6 @@ public void setConverter(final Converter converter) { this.converter = converter; } - /** - * Gets the value verifier. - * @return the value verifier. - * @since 1.7.0 - */ - public Predicate getVerifier() { - 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(final Predicate verifier) { - this.verifier = verifier; - } - /** * Dump state, suitable for debugging. * diff --git a/src/main/java/org/apache/commons/cli/OptionBuilder.java b/src/main/java/org/apache/commons/cli/OptionBuilder.java index 6ffca8030..f9a5c3076 100644 --- a/src/main/java/org/apache/commons/cli/OptionBuilder.java +++ b/src/main/java/org/apache/commons/cli/OptionBuilder.java @@ -110,7 +110,6 @@ public static Option create(final String opt) throws IllegalArgumentException { option.setArgs(argCount); option.setType(type); option.setConverter(TypeHandler.getConverter(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 index b65c11def..b5f47ec2d 100644 --- a/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java +++ b/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java @@ -183,7 +183,6 @@ public static Options parsePattern(final String pattern) { boolean required = false; Class type = null; Converter converter = Converter.DEFAULT; - Predicate verifier = Verifier.DEFAULT; final Options options = new Options(); @@ -195,14 +194,13 @@ public static Options parsePattern(final String pattern) { if (!isValueCode(ch)) { if (opt != ' ') { final Option option = Option.builder(String.valueOf(opt)).hasArg(type != null).required(required).type(type) - .converter(converter).verifier(verifier).build(); + .converter(converter).build(); // we have a previous one to deal with options.addOption(option); required = false; type = null; converter = Converter.DEFAULT; - verifier = Verifier.DEFAULT; } opt = ch; diff --git a/src/main/java/org/apache/commons/cli/Verifier.java b/src/main/java/org/apache/commons/cli/Verifier.java deleted file mode 100644 index e8a6e5525..000000000 --- a/src/main/java/org/apache/commons/cli/Verifier.java +++ /dev/null @@ -1,93 +0,0 @@ -/* - Licensed to the Apache Software Foundation (ASF) under one or more - contributor license agreements. See the NOTICE file distributed with - this work for additional information regarding copyright ownership. - The ASF licenses this file to You under the Apache License, Version 2.0 - (the "License"); you may not use this file except in compliance with - the License. You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - */ -package org.apache.commons.cli; - -import java.util.Arrays; -import java.util.List; -import java.util.function.Predicate; -import java.util.regex.Pattern; -import java.util.stream.Collectors; - -/** - * The definition of the functional interface to call when verifying a string - * for input Like {@code Predicate} but can throw a RuntimeException. - * @since 1.7.0 - */ -public final class Verifier { - - private Verifier() { - // do not instantiate - } - - /** - * Default verifier. Always returns {@code true}. - */ - public static final Predicate DEFAULT = s -> true; - - /** - * The Regex Pattern for the number matching. - */ - public static final Pattern NUMBER_PATTERN = Pattern.compile("[-+]?(\\d*\\.)?(\\d+)$"); - - /** - * Verifies that a number string is either a valid real number (e.g. may have a - * decimal point) or an integer. - */ - public static final Predicate NUMBER = s -> NUMBER_PATTERN.matcher(s).matches(); - - /** - * The Regex Pattern for the integer matching. - */ - public static final Pattern INTEGER_PATTERN = Pattern.compile("[-+]?\\d+"); - - /** - * Verifies that a number string is an integer. - */ - public static final Predicate INTEGER = s -> INTEGER_PATTERN.matcher(s).matches(); - - /** - * The Regex Pattern that matches valid class names. - */ - public static final Pattern CLAZZ_PATTERN = Pattern.compile( - "\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*(\\.\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*)*"); - - /** - * Verifies that a class name is valid. - */ - public static final Predicate CLASS = s -> CLAZZ_PATTERN.matcher(s).matches(); - - /** - * Constructs a {@code Predicate} from an exemplar of an Enum. For - * example {@code new EnumVerifier(java.time.format.TextStyle.FULL)} would - * create an {@code Predicate} that would accept the names for any of - * the {@code java.time.format.TextStyle} values. - * @param exemplar One of the values from the accepted Enum. - * @return A {@code Predicate} that matches the Enum names. - */ - public static Predicate enumVerifier(final Enum exemplar) { - return new Predicate() { - /** The list of valid names */ - private final List names = Arrays.stream(exemplar.getDeclaringClass().getEnumConstants()) - .map(t -> ((Enum) t).name()).collect(Collectors.toList()); - @Override - public boolean test(final String str) throws RuntimeException { - return names.contains(str); - } - }; - } - -} diff --git a/src/test/java/org/apache/commons/cli/OptionTest.java b/src/test/java/org/apache/commons/cli/OptionTest.java index b1bec742d..a49dbaae0 100644 --- a/src/test/java/org/apache/commons/cli/OptionTest.java +++ b/src/test/java/org/apache/commons/cli/OptionTest.java @@ -253,28 +253,22 @@ public void testSerialization() throws IOException, ClassNotFoundException { Option o = Option.builder("o").type(TypeHandlerTest.Instantiable.class).build(); assertEquals(Converter.DEFAULT, o.getConverter()); - assertEquals(Verifier.DEFAULT, o.getVerifier()); Option o2 = roundTrip(o); assertEquals(Converter.DEFAULT, o2.getConverter()); - assertEquals(Verifier.DEFAULT, o2.getVerifier()); // verify unregistered class converters and verifiers get reset to default. o.setConverter(Converter.DATE); - o.setVerifier(Verifier.NUMBER); o2 = roundTrip(o); assertEquals(Converter.DEFAULT, o2.getConverter()); - assertEquals(Verifier.DEFAULT, o2.getVerifier()); // verify registered class converters and verifiers do not get reset to default. try { 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 value assertEquals(Converter.URL, o2.getConverter()); - assertEquals(Verifier.DEFAULT, o2.getVerifier()); } finally { TypeHandler.register(TypeHandlerTest.Instantiable.class, null); } diff --git a/src/test/java/org/apache/commons/cli/VerifierTests.java b/src/test/java/org/apache/commons/cli/VerifierTests.java deleted file mode 100644 index 80302b1ac..000000000 --- a/src/test/java/org/apache/commons/cli/VerifierTests.java +++ /dev/null @@ -1,136 +0,0 @@ -/* - Licensed to the Apache Software Foundation (ASF) under one or more - contributor license agreements. See the NOTICE file distributed with - this work for additional information regarding copyright ownership. - The ASF licenses this file to You under the Apache License, Version 2.0 - (the "License"); you may not use this file except in compliance with - the License. You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - */ -package org.apache.commons.cli; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; - -import java.util.ArrayList; -import java.util.List; -import java.util.function.Predicate; -import java.util.stream.Stream; - -import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.MethodSource; - -/** - * Tests for standard Verifiers. - */ -public class VerifierTests { - - /** - * Verifies number formats - */ - @Test - public void numberTests() { - // test good numbers - for (String s : new String[] {"123", "12.3", "-123", "-12.3", ".3", "-.3"}) { - assertTrue(Verifier.NUMBER.test(s), s); - } - - // test bad numbers - for (String s : new String[] {"0x5F", "2,3", "1.2.3"}) { - assertFalse(Verifier.NUMBER.test(s), s); - } - } - - /** - * Verifies integer formats - */ - @Test - public void integerTests() { - // test good numbers - for (String s : new String[] {"123", "-123"}) { - assertTrue(Verifier.INTEGER.test(s), s); - } - - // test bad numbers - for (String s : new String[] {"12.3", "-12.3", ".3", "-.3", "0x5F", "2,3", "1.2.3"}) { - assertFalse(Verifier.INTEGER.test(s), s); - } - } - - /** - * Verifies class names - */ - @Test - public void classTests() { - String testName = this.getClass().getName(); - assertTrue(Verifier.CLASS.test(testName), testName); - assertTrue(Verifier.CLASS.test(this.getClass().getCanonicalName()), this.getClass().getCanonicalName()); - assertTrue(Verifier.CLASS.test(this.getClass().getSimpleName()), this.getClass().getSimpleName()); - assertTrue(Verifier.CLASS.test(this.getClass().getTypeName()), this.getClass().getTypeName()); - - // test bad prefixes - for (String s : new String[] {"1", "-", "."}) { - assertFalse(Verifier.CLASS.test(s + testName), s + testName); - } - - // test good prefixes - for (String s : new String[] {"$"}) { - assertTrue(Verifier.CLASS.test(s + testName), s + testName); - } - - // test bad suffixes - for (String s : new String[] {"-", "."}) { - assertFalse(Verifier.CLASS.test(testName + s), testName + s); - } - - // test good suffixes - for (String s : new String[] {"1", "$"}) { - assertTrue(Verifier.CLASS.test(testName + s), testName + s); - } - - // test bad infixes - for (String s : new String[] {"..", "-"}) { - assertFalse(Verifier.CLASS.test("foo" + s + "bar"), "foo" + s + "bar"); - } - - // test good infixes - for (String s : new String[] {"$", "_"}) { - assertTrue(Verifier.CLASS.test("foo" + s + "bar"), "foo" + s + "bar"); - } - } - - enum MyEnum { - ONE, Two, three, someothernumber - } - - private Predicate underTest = Verifier.enumVerifier(MyEnum.Two); - - @ParameterizedTest(name = "{0}") - @MethodSource("testData") - public void test(final String str, final boolean expected) { - assertEquals(expected, underTest.test(str)); - } - - private static Stream testData() { - List lst = new ArrayList<>(); - - lst.add(Arguments.of("ONE", true)); - lst.add(Arguments.of("one", false)); - lst.add(Arguments.of("Two", true)); - lst.add(Arguments.of("three", true)); - lst.add(Arguments.of("someothernumber", true)); - lst.add(Arguments.of("NonValue", false)); - - return lst.stream(); - } -} From 30d26f5aa15f3ce9b28766dd7acc007f190e2dfb Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Fri, 26 Jan 2024 14:42:36 +0100 Subject: [PATCH 23/24] removed verifier --- src/main/java/org/apache/commons/cli/Option.java | 1 - src/main/java/org/apache/commons/cli/PatternOptionBuilder.java | 1 - 2 files changed, 2 deletions(-) diff --git a/src/main/java/org/apache/commons/cli/Option.java b/src/main/java/org/apache/commons/cli/Option.java index 743a06bd9..6ffc0c559 100644 --- a/src/main/java/org/apache/commons/cli/Option.java +++ b/src/main/java/org/apache/commons/cli/Option.java @@ -23,7 +23,6 @@ Licensed to the Apache Software Foundation (ASF) under one or more import java.util.ArrayList; import java.util.List; import java.util.Objects; -import java.util.function.Predicate; /** * Describes a single command-line option. It maintains information regarding the short-name of the option, the diff --git a/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java b/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java index b5f47ec2d..16fdc8640 100644 --- a/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java +++ b/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java @@ -21,7 +21,6 @@ Licensed to the Apache Software Foundation (ASF) under one or more import java.io.FileInputStream; import java.net.URL; import java.util.Date; -import java.util.function.Predicate; /** * Allows Options to be created from a single String. The pattern contains various single character flags and via an From 2d6ff50e4ce2ab396f7cda814122c3298dcdd873 Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Mon, 29 Jan 2024 10:52:24 +0100 Subject: [PATCH 24/24] updated javadoc and site documentation --- .../org/apache/commons/cli/ParseException.java | 3 ++- .../commons/cli/PatternOptionBuilder.java | 3 +++ .../org/apache/commons/cli/TypeHandler.java | 2 +- src/site/xdoc/usage.xml | 18 ++---------------- 4 files changed, 8 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/apache/commons/cli/ParseException.java b/src/main/java/org/apache/commons/cli/ParseException.java index fd8f936b3..f0237797d 100644 --- a/src/main/java/org/apache/commons/cli/ParseException.java +++ b/src/main/java/org/apache/commons/cli/ParseException.java @@ -37,12 +37,13 @@ public class ParseException extends Exception { * @param e the exception to convert. * @return the ParseException. * @throws UnsupportedOperationException due to legacy expectations. Will be removed in the future. + * @since 1.7.0 */ public static ParseException wrap(final Exception e) throws UnsupportedOperationException { if (e instanceof UnsupportedOperationException) { throw (UnsupportedOperationException) e; } - + if (e instanceof ParseException) { return (ParseException) e; } diff --git a/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java b/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java index 16fdc8640..de3168e96 100644 --- a/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java +++ b/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java @@ -114,6 +114,7 @@ public class PatternOptionBuilder { /** * Registers custom {@code Converter}s with the {@code TypeHandler}. + * @since 1.7.0 */ public static void registerTypes() { TypeHandler.register(PatternOptionBuilder.FILES_VALUE, NOT_IMPLEMENTED); @@ -126,6 +127,7 @@ public static void registerTypes() { * @return The class that {@code ch} represents * @deprecated use {@link #getValueType(char)} */ + @Deprecated // since="1.7.0" public static Object getValueClass(final char ch) { return getValueType(ch); } @@ -135,6 +137,7 @@ public static Object getValueClass(final char ch) { * * @param ch the specified character * @return The class that {@code ch} represents + * @since 1.7.0 */ public static Class getValueType(final char ch) { switch (ch) { diff --git a/src/main/java/org/apache/commons/cli/TypeHandler.java b/src/main/java/org/apache/commons/cli/TypeHandler.java index 0972e5da6..602942a40 100644 --- a/src/main/java/org/apache/commons/cli/TypeHandler.java +++ b/src/main/java/org/apache/commons/cli/TypeHandler.java @@ -90,7 +90,7 @@ public static void noConverters() { } /** - * Registers a Converter for a Class. If @code converter} is null registration is cleared for {@code clazz}, and + * 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. diff --git a/src/site/xdoc/usage.xml b/src/site/xdoc/usage.xml index 4ab1cf9e4..858f1908e 100644 --- a/src/site/xdoc/usage.xml +++ b/src/site/xdoc/usage.xml @@ -454,23 +454,9 @@ catch (ParseException exp) { The above will create an option that passes the string value to the Foo constructor when commandLine.getParsedOptionValue(fooOpt) is called.

    - -

    - Acceptable values for options may be specified by using Option.Builder.verifier() to specify a java.util.function.Predicate - to verify the input text. There are a few examples in the Verifier source file. Verifier also provides a method to construct a verifier - to test enum values. For example: - - public enum MyEnum {ONE, TWO, THREE}; - - Option enumOpt = Option.builder("enumValue") - .hasArg() - .desc("the enum arg") - .varifier(Verifier.enumVerifier(MyEnum.ONE)) - .build(); - - The above will create an option that verifies any argument passed to enumOpt matches a name from MyEnum. - The initial verification occures when the option string is set during the execution of CommandLine line = parser.parse(options, args). + Conversions that are added to the TypeHandler or that are specified directly will not deserialize if the option is serialized unless the type is registered with the TypeHandler + before deserialization begins.