[ 
https://issues.apache.org/jira/browse/CLI-347?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gary D. Gregory resolved CLI-347.
---------------------------------
    Fix Version/s: 1.10.0
       Resolution: Fixed

[~rickydong] 

Thank you for your report.

This is now fixed in git master. 

Please check git master and/or snapshot builds in 
[https://repository.apache.org/content/repositories/snapshots/]

 

> Options.addOptionGroup() does not remove required options from requiredOpts 
> list
> --------------------------------------------------------------------------------
>
>                 Key: CLI-347
>                 URL: https://issues.apache.org/jira/browse/CLI-347
>             Project: Commons CLI
>          Issue Type: Bug
>    Affects Versions: 1.10.0
>            Reporter: Ruiqi Dong
>            Priority: Major
>             Fix For: 1.10.0
>
>   Original Estimate: 0.5h
>  Remaining Estimate: 0.5h
>
> When a required Option is added to an OptionGroup via 
> {{{}Options.addOptionGroup(){}}}, the option's {{required}} flag is correctly 
> set to false, but the option's key remains in the {{requiredOpts}} list. This 
> violates the documented design principle that "an Option cannot be required 
> if it is in an OptionGroup".
> *Impact:*
>  * {{getRequiredOptions()}} returns incorrect results
>  * Breaks the contract that options in OptionGroups cannot be required
>  * May cause incorrect validation behavior during command line parsing
>  * Inconsistent behavior depending on the order of operations
> *Test Case:*
>  
> {code:java}
> @Test
> public void testRequiredOptionInGroupShouldNotBeInRequiredList() {
>     // Create a required option
>     Option option = new Option("a", "along", false, "Option A");
>     option.setRequired(true);
>     
>     // Add required option to Options
>     Options options = new Options();
>     options.addOption(option);
>     
>     // Verify option is in required list
>     assertTrue(options.getRequiredOptions().contains("a"));
>     
>     // Create OptionGroup and add the same option
>     OptionGroup group = new OptionGroup();
>     group.addOption(option);
>     
>     // Add OptionGroup to Options
>     options.addOptionGroup(group);
>     
>     // Verify option's required flag is false
>     assertFalse(options.getOption("a").isRequired());
>     
>     // This assertion fails - demonstrating the bug
>     assertFalse(options.getRequiredOptions().contains("a"), 
>                "Option in group should NOT be in required options list");
> } {code}
> *Test Failure Scenario:*
> In the {{addOptionGroup()}} method, while the code correctly sets 
> {{{}option.setRequired(false){}}}, it does not remove the option's key from 
> the {{requiredOpts}} list. The {{addOption()}} method only adds to 
> {{requiredOpts}} when {{isRequired()}} is true, but never removes entries 
> when the required status changes.
>  
>  
> {code:java}
> [ERROR] 
> org.apache.commons.cli.OptionsTest.testRequiredOptionInGroupShouldNotBeInRequiredList
>  -- Time elapsed: 0.001 s <<< FAILURE!
> org.opentest4j.AssertionFailedError: Option in group should NOT be in 
> required options list ==> expected: <false> but was: <true>
>         at 
> org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
>         at 
> org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
>         at org.junit.jupiter.api.AssertFalse.failNotFalse(AssertFalse.java:63)
>         at org.junit.jupiter.api.AssertFalse.assertFalse(AssertFalse.java:36)
>         at org.junit.jupiter.api.Assertions.assertFalse(Assertions.java:239)
>         at 
> org.apache.commons.cli.OptionsTest.testRequiredOptionInGroupShouldNotBeInRequiredList(OptionsTest.java:55)
>         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) {code}
>  
> *Proposed Fix:*
> The {{addOptionGroup()}} method should remove the option from 
> {{requiredOpts}} after setting required to false:
> {code:java}
> public Options addOptionGroup(final OptionGroup group) {
>     if (group.isRequired()) {
>         requiredOpts.add(group);
>     }
>     for (final Option option : group.getOptions()) {
>         option.setRequired(false);
>         // Remove option from requiredOpts list
>         requiredOpts.remove(option.getKey());
>         addOption(option);
>         optionGroups.put(option.getKey(), group);
>     }
>     return this;
> } {code}



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to