[
https://issues.apache.org/jira/browse/CLI-343?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gary D. Gregory updated CLI-343:
--------------------------------
Affects Version/s: Nightly Builds
(was: 1.10.0)
> OptionFormatter.getBothOpt() lacks validation for Options without opt or
> longOpt
> --------------------------------------------------------------------------------
>
> Key: CLI-343
> URL: https://issues.apache.org/jira/browse/CLI-343
> Project: Commons CLI
> Issue Type: Bug
> Affects Versions: Nightly Builds
> Reporter: Ruiqi Dong
> Priority: Minor
> Attachments: OptionFormatter.java, OptionFormatterTest.java
>
> Original Estimate: 0.5h
> Remaining Estimate: 0.5h
>
> *Description:*
> The {{OptionFormatter.getBothOpt()}} method contains a comment stating
> "Option requires at least one of opt or longOpt", but the method itself does
> not validate this constraint. Instead, it relies entirely on the {{Option}}
> class validation.
>
> *Current Behavior:* The {{getBothOpt()}} method would return an empty string
> if called with an Option that has neither opt nor longOpt. The method
> includes a comment acknowledging the constraint but doesn't enforce it.
> *Expected Behavior:* {{OptionFormatter}} should validate the Option
> constraint independently, either in its constructor or in the
> {{getBothOpt()}} method
>
> *Unit Test Cases:*
> public class OptionFormatterTest {
> @Test
> @DisplayName("getBothOpt() should throw exception when both opt and longOpt
> are null")
> publicvoidtestGetBothOptWithNullShortAndLongOptShouldThrowException(){
> // Create an Option with both opt and longOpt as null
> // This violates the constraint mentioned in the comment:
> // "Option requires at least one of opt or longOpt"
> Optionoption = Option.builder()
> .required(false)
> .build();
>
> // This should throw an exception but doesn't in the current implementation
> assertThrows(IllegalArgumentException.class, () -> {
> OptionFormatterformatter = OptionFormatter.builder().build(option);
> formatter.getBothOpt();
> }, "OptionFormatter should throw exception when Option has neither opt nor
> longOpt");
> }
>
> @Test
> @DisplayName("OptionFormatter construction should fail when Option has
> neither opt nor longOpt")
> publicvoidtestOptionFormatterConstructionWithInvalidOption(){
> // Create an Option without any opt or longOpt
> OptioninvalidOption = Option.builder()
> .desc("This option has no way to be specified")
> .build();
>
> // The builder.build() method should validate and throw exception
> OptionFormatter.Builderbuilder = OptionFormatter.builder();
>
> assertThrows(IllegalArgumentException.class, () -> {
> builder.build(invalidOption);
> }, "OptionFormatter.Builder.build() should reject Options without opt or
> longOpt");
> }
> }
>
> *Test Results:*
> [*INFO*] Running org.apache.commons.cli.help.{*}OptionFormatterTest{*}{*}{*}
> [*ERROR*] *Tests* {*}run: 2{*}, Failures: 0, *Errors: 2*, Skipped: 0, Time
> elapsed: 0.001 s *<<< FAILURE!* -- in
> org.apache.commons.cli.help.{*}OptionFormatterTest{*}{*}{*}
> [*ERROR*]
> org.apache.commons.cli.help.OptionFormatterTest.testGetBothOptWithNullShortAndLongOptShouldThrowException
> -- Time elapsed: 0 s <<< ERROR!
> java.lang.IllegalArgumentException: Either opt or longOpt must be specified
> at org.apache.commons.cli.Option$Builder.build(Option.java:134)
> at
> org.apache.commons.cli.help.OptionFormatterTest.testGetBothOptWithNullShortAndLongOptShouldThrowException(OptionFormatterTest.java:36)
> at java.lang.reflect.Method.invoke(Method.java:498)
> at java.util.ArrayList.forEach(ArrayList.java:1259)
> at java.util.ArrayList.forEach(ArrayList.java:1259)
>
> *Suggested Fix:*
> Add validation in the {{OptionFormatter}} constructor
> private OptionFormatter(final Option option, final Builder builder) {
> if (Util.isEmpty(option.getOpt()) && Util.isEmpty(option.getLongOpt())) {
> throw new IllegalArgumentException("Option must have at least one of
> opt or longOpt");
> }
> // ... rest of constructor
> }
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
