Skip to content

Commit 91f3acf

Browse files
committed
org.apache.commons.cli.Option.Builder.get() should throw
IllegalStateException instead of IllegalArgumentException Is the state that's wrong, there are no arguments to validate anyway.
1 parent cd3f2c4 commit 91f3acf

3 files changed

Lines changed: 9 additions & 9 deletions

File tree

src/changes/changes.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
<action type="fix" issue="CLI-349" dev="ggregory" due-to="Leo Fernandes, Gary Gregory">Fail faster with a more precise NullPointerException: Option.processValue() throws NullPointerException when passed null value with value separator configured.</action>
3939
<action type="fix" issue="CLI-344" dev="ggregory" due-to="Ruiqi Dong, Gary Gregory">Fail faster with a more precise NullPointerException: DefaultParser.parse() throws NullPointerException when options parameter is null.</action>
4040
<action type="fix" issue="CLI-347" dev="ggregory" due-to="Ruiqi Dong, Gary Gregory">Options.addOptionGroup(OptionGroup) does not remove required options from requiredOpts list.</action>
41+
<action type="fix" dev="ggregory" due-to="Gary Gregory">org.apache.commons.cli.Option.Builder.get() should throw IllegalStateException instead of IllegalArgumentException.</action>
4142
<!-- ADD -->
4243
<action type="add" issue="CLI-339" dev="ggregory" due-to="Claude Warren, Gary Gregory">Help formatter extension in the new package #314.</action>
4344
<action type="add" dev="ggregory" due-to="Gary Gregory">CommandLine.Builder implements Supplier&lt;CommandLine&gt;.</action>

src/main/java/org/apache/commons/cli/Option.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -188,11 +188,11 @@ public Builder desc(final String description) {
188188
* Constructs an Option with the values declared by this {@link Builder}.
189189
*
190190
* @return the new {@link Option}.
191-
* @throws IllegalArgumentException if neither {@code opt} or {@code longOpt} has been set.
191+
* @throws IllegalStateException if neither {@code opt} or {@code longOpt} has been set.
192192
*/
193193
public Option get() {
194194
if (option == null && longOption == null) {
195-
throw new IllegalArgumentException("Either opt or longOpt must be specified");
195+
throw new IllegalStateException("Either opt or longOpt must be specified");
196196
}
197197
return new Option(this);
198198
}
@@ -530,8 +530,7 @@ private void add(final String value) {
530530
*/
531531
@Deprecated
532532
public boolean addValue(final String value) {
533-
throw new UnsupportedOperationException(
534-
"The addValue method is not intended for client use. Subclasses should use the processValue method instead.");
533+
throw new UnsupportedOperationException("The addValue method is not intended for client use. Subclasses should use the processValue method instead.");
535534
}
536535

537536
/**

src/test/java/org/apache/commons/cli/OptionTest.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -109,27 +109,27 @@ void testAddValue() {
109109

110110
@Test
111111
void testBuilderDeprecatedBuildEmpty() {
112-
assertThrows(IllegalArgumentException.class, () -> Option.builder().build());
112+
assertThrows(IllegalStateException.class, () -> Option.builder().build());
113113
}
114114

115115
@Test
116116
void testBuilderEmpty() {
117-
assertThrows(IllegalArgumentException.class, () -> Option.builder().get());
117+
assertThrows(IllegalStateException.class, () -> Option.builder().get());
118118
}
119119

120120
@Test
121121
void testBuilderInsufficientParams1() {
122-
assertThrows(IllegalArgumentException.class, () -> Option.builder().desc("desc").get());
122+
assertThrows(IllegalStateException.class, () -> Option.builder().desc("desc").get());
123123
}
124124

125125
@Test
126126
void testBuilderInsufficientParams2() {
127-
assertThrows(IllegalArgumentException.class, () -> Option.builder(null).desc("desc").get());
127+
assertThrows(IllegalStateException.class, () -> Option.builder(null).desc("desc").get());
128128
}
129129

130130
@Test
131131
void testBuilderInvalidOptionName0() {
132-
assertThrows(IllegalArgumentException.class, () -> Option.builder().option(null).get());
132+
assertThrows(IllegalStateException.class, () -> Option.builder().option(null).get());
133133
assertThrows(IllegalArgumentException.class, () -> Option.builder().option(""));
134134
assertThrows(IllegalArgumentException.class, () -> Option.builder().option(" "));
135135
}

0 commit comments

Comments
 (0)