Skip to content

Commit 4f53cba

Browse files
authored
Merge branch 'apache:master' into CLI-323_Supplier_for_getParsedOptionValue
2 parents a9e6110 + bcfcdde commit 4f53cba

8 files changed

Lines changed: 268 additions & 29 deletions

File tree

.github/workflows/codeql-analysis.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ jobs:
5757
5858
# Initializes the CodeQL tools for scanning.
5959
- name: Initialize CodeQL
60-
uses: github/codeql-action/init@b7bf0a3ed3ecfa44160715d7c442788f65f0f923 # 3.23.2
60+
uses: github/codeql-action/init@379614612a29c9e28f31f39a59013eb8012a51f0 # 3.24.3
6161
with:
6262
languages: ${{ matrix.language }}
6363
# If you wish to specify custom queries, you can do so here or in a config file.
@@ -68,7 +68,7 @@ jobs:
6868
# Autobuild attempts to build any compiled languages (C/C++, C#, or Java).
6969
# If this step fails, then you should remove it and run the build manually (see below)
7070
- name: Autobuild
71-
uses: github/codeql-action/autobuild@b7bf0a3ed3ecfa44160715d7c442788f65f0f923 # 3.23.2
71+
uses: github/codeql-action/autobuild@379614612a29c9e28f31f39a59013eb8012a51f0 # 3.24.3
7272

7373
# ℹ️ Command-line programs to run using the OS shell.
7474
# 📚 https://git.io/JvXDl
@@ -82,4 +82,4 @@ jobs:
8282
# make release
8383

8484
- name: Perform CodeQL Analysis
85-
uses: github/codeql-action/analyze@b7bf0a3ed3ecfa44160715d7c442788f65f0f923 # 3.23.2
85+
uses: github/codeql-action/analyze@379614612a29c9e28f31f39a59013eb8012a51f0 # 3.24.3

.github/workflows/scorecards-analysis.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,13 @@ jobs:
5757
publish_results: true
5858

5959
- name: "Upload artifact"
60-
uses: actions/upload-artifact@26f96dfa697d77e81fd5907df203aa23a56210a8 # v4.3.0
60+
uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3 # v4.3.1
6161
with:
6262
name: SARIF file
6363
path: results.sarif
6464
retention-days: 5
6565

6666
- name: "Upload to code-scanning"
67-
uses: github/codeql-action/upload-sarif@b7bf0a3ed3ecfa44160715d7c442788f65f0f923 # 3.23.2
67+
uses: github/codeql-action/upload-sarif@379614612a29c9e28f31f39a59013eb8012a51f0 # 3.24.3
6868
with:
6969
sarif_file: results.sarif

pom.xml

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,11 @@
187187
<artifactId>junit-jupiter-api</artifactId>
188188
<scope>test</scope>
189189
</dependency>
190+
<dependency>
191+
<groupId>org.junit.jupiter</groupId>
192+
<artifactId>junit-jupiter-params</artifactId>
193+
<scope>test</scope>
194+
</dependency>
190195
<dependency>
191196
<groupId>org.junit.vintage</groupId>
192197
<artifactId>junit-vintage-engine</artifactId>
@@ -252,7 +257,6 @@
252257
<configuration>
253258
<excludeFilterFile>${basedir}/src/conf/spotbugs-exclude-filter.xml</excludeFilterFile>
254259
</configuration>
255-
<version>4.8.3.0</version>
256260
</plugin>
257261
<plugin>
258262
<groupId>org.apache.maven.plugins</groupId>

src/changes/changes.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
<!-- ADD -->
2727
<action type="add" issue="CLI-321" dev="ggregory" due-to="Claude Warren, Gary Gregory">Add and use a Converter interface and implementations without using BeanUtils #216.</action>
2828
<action type="add" dev="ggregory" due-to="Gary Gregory">Add Maven property project.build.outputTimestamp for build reproducibility.</action>
29+
<action type="add" issue="CLI-322" dev="ggregory" due-to="Claude Warren, Gary Gregory">Add '-' as an option char and implemented extensive tests #217.</action>
2930
<!-- UPDATE -->
3031
<action type="update" dev="ggregory" due-to="Gary Gregory">Bump commons-parent from 64 to 66.</action>
3132
</release>

src/conf/checkstyle.xml

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,7 @@
7777
<module name="ParenPad" />
7878

7979
<module name="WhitespaceAfter" />
80-
<module name="WhitespaceAround">
81-
<property name="tokens"
82-
value="ASSIGN, BAND, BAND_ASSIGN, BOR, BOR_ASSIGN, BSR, BSR_ASSIGN, BXOR, BXOR_ASSIGN, COLON, DIV, DIV_ASSIGN, EQUAL, GE, GT, LAND, LCURLY, LE, LITERAL_ASSERT, LITERAL_CATCH, LITERAL_DO, LITERAL_ELSE, LITERAL_FINALLY, LITERAL_FOR, LITERAL_IF, LITERAL_RETURN, LITERAL_SYNCHRONIZED, LITERAL_TRY, LITERAL_WHILE, LOR, LT, MINUS, MINUS_ASSIGN, MOD, MOD_ASSIGN, NOT_EQUAL, PLUS, PLUS_ASSIGN, QUESTION, RCURLY, SL, SLIST, SL_ASSIGN, SR, SR_ASSIGN, STAR, STAR_ASSIGN, TYPE_EXTENSION_AND" />
83-
</module>
80+
<module name="WhitespaceAround" />
8481

8582
<!-- Modifier Checks -->
8683
<!-- See http://checkstyle.sf.net/config_modifiers.html -->

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

Lines changed: 72 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -23,24 +23,67 @@ Licensed to the Apache Software Foundation (ASF) under one or more
2323
* @since 1.1
2424
*/
2525
final class OptionValidator {
26+
/* package private for testing access */
27+
/** The array of additional characters allowed as the first character in the option but not in the rest of the option */
28+
static final char[] ADDITIONAL_OPTION_CHARS = {'?', '@'};
29+
/** The array of additional characters allowed in the rest of the option but not in the first position */
30+
static final char[] ADDITIONAL_LONG_CHARS = {'-'};
31+
32+
/**
33+
* Checks the char array for a matching char.
34+
* @param chars the char array to search
35+
* @param c the char to look for.
36+
* @return {@code true} if {@code c} was in {@code ary}, {@code false} otherwise.
37+
*/
38+
private static boolean search(final char[] chars, final char c) {
39+
for (char a : chars) {
40+
if (a == c) {
41+
return true;
42+
}
43+
}
44+
return false;
45+
}
46+
2647
/**
2748
* Returns whether the specified character is a valid character.
28-
*
49+
* A character is valid if any of the following conditions are true:
50+
* <ul>
51+
* <li>it is a letter</li>
52+
* <li>it is a currency symbol (such as '$')</li>
53+
* <li>it is a connecting punctuation character (such as '_')</li>
54+
* <li>it is a digit</li>
55+
* <li>it is a numeric letter (such as a Roman numeral character)</li>
56+
* <li>it is a combining mark</li>
57+
* <li>it is a non-spacing mark</li>
58+
* <li>isIdentifierIgnorable returns true for the character</li>
59+
* <li>it is a hyphen/dash ('-')</li>
60+
* </ul>
2961
* @param c the character to validate
30-
* @return true if {@code c} is a letter.
62+
* @return true if {@code c} is a valid character letter.
3163
*/
3264
private static boolean isValidChar(final char c) {
33-
return Character.isJavaIdentifierPart(c);
65+
return Character.isJavaIdentifierPart(c) || search(ADDITIONAL_LONG_CHARS, c);
3466
}
3567

3668
/**
3769
* Returns whether the specified character is a valid Option.
38-
*
70+
* A character is valid if any of the following conditions are true:
71+
* <ul>
72+
* <li>it is a letter</li>
73+
* <li>it is a currency symbol (such as '$')</li>
74+
* <li>it is a connecting punctuation character (such as '_')</li>
75+
* <li>it is a digit</li>
76+
* <li>it is a numeric letter (such as a Roman numeral character)</li>
77+
* <li>it is a combining mark</li>
78+
* <li>it is a non-spacing mark</li>
79+
* <li>isIdentifierIgnorable returns true for the character</li>
80+
* <li>it is a question mark or 'at' sign ('?' or '@')</li>
81+
* </ul>
3982
* @param c the option to validate
4083
* @return true if {@code c} is a letter, '?' or '@', otherwise false.
4184
*/
4285
private static boolean isValidOpt(final char c) {
43-
return isValidChar(c) || c == '?' || c == '@';
86+
return Character.isJavaIdentifierPart(c) || search(ADDITIONAL_OPTION_CHARS, c);
4487
}
4588

4689
/**
@@ -49,9 +92,22 @@ private static boolean isValidOpt(final char c) {
4992
*
5093
* <ul>
5194
* <li>a single character {@code opt} that is either ' '(special case), '?', '@' or a letter</li>
52-
* <li>a multi character {@code opt} that only contains letters.</li>
95+
* <li>a multi character {@code opt} that only contains valid characters</li>
5396
* </ul>
54-
* <p>
97+
* </p><p>
98+
* A character is valid if any of the following conditions are true:
99+
* <ul>
100+
* <li>it is a letter</li>
101+
* <li>it is a currency symbol (such as '$')</li>
102+
* <li>it is a connecting punctuation character (such as '_')</li>
103+
* <li>it is a digit</li>
104+
* <li>it is a numeric letter (such as a Roman numeral character)</li>
105+
* <li>it is a combining mark</li>
106+
* <li>it is a non-spacing mark</li>
107+
* <li>isIdentifierIgnorable returns true for the character</li>
108+
* <li>it is a hyphen/dash ('-')</li>
109+
* </ul>
110+
* </p><p>
55111
* In case {@code opt} is {@code null} no further validation is performed.
56112
*
57113
* @param option The option string to validate, may be null
@@ -63,18 +119,16 @@ static String validate(final String option) throws IllegalArgumentException {
63119
return null;
64120
}
65121

66-
// handle the single character opt
67-
if (option.length() == 1) {
68-
final char ch = option.charAt(0);
122+
char[] chars = option.toCharArray();
69123

70-
if (!isValidOpt(ch)) {
71-
throw new IllegalArgumentException("Illegal option name '" + ch + "'");
72-
}
73-
} else {
74-
// handle the multi character opt
75-
for (final char ch : option.toCharArray()) {
76-
if (!isValidChar(ch)) {
77-
throw new IllegalArgumentException("The option '" + option + "' contains an illegal " + "character : '" + ch + "'");
124+
if (!isValidOpt(chars[0])) {
125+
throw new IllegalArgumentException("Illegal option name '" + chars[0] + "'");
126+
}
127+
// handle the multi-character opt
128+
if (option.length() > 1) {
129+
for (int i = 1; i < chars.length; i++) {
130+
if (!isValidChar(chars[i])) {
131+
throw new IllegalArgumentException("The option '" + option + "' contains an illegal " + "character : '" + chars[i] + "'");
78132
}
79133
}
80134
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ public static File createFile(final String str) {
104104
*
105105
* <p>
106106
* This method is not yet implemented and always throws an {@link UnsupportedOperationException}.
107-
* <p>
107+
* </p>
108108
*
109109
* @param str the paths to the files
110110
* @return The File[] represented by {@code str}.
Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
/*
2+
Licensed to the Apache Software Foundation (ASF) under one or more
3+
contributor license agreements. See the NOTICE file distributed with
4+
this work for additional information regarding copyright ownership.
5+
The ASF licenses this file to You under the Apache License, Version 2.0
6+
(the "License"); you may not use this file except in compliance with
7+
the License. You may obtain a copy of the License at
8+
9+
http://www.apache.org/licenses/LICENSE-2.0
10+
11+
Unless required by applicable law or agreed to in writing, software
12+
distributed under the License is distributed on an "AS IS" BASIS,
13+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
See the License for the specific language governing permissions and
15+
limitations under the License.
16+
*/
17+
18+
package org.apache.commons.cli;
19+
20+
import static org.junit.jupiter.api.Assertions.assertEquals;
21+
import static org.junit.jupiter.api.Assertions.assertFalse;
22+
import static org.junit.jupiter.api.Assertions.assertThrows;
23+
24+
import java.util.ArrayList;
25+
import java.util.List;
26+
import java.util.stream.Stream;
27+
28+
import org.junit.jupiter.api.BeforeAll;
29+
import org.junit.jupiter.api.Test;
30+
import org.junit.jupiter.params.ParameterizedTest;
31+
import org.junit.jupiter.params.provider.Arguments;
32+
import org.junit.jupiter.params.provider.MethodSource;
33+
34+
public class OptionValidatorTest {
35+
36+
/*
37+
* Exemplars of various types of characters
38+
*/
39+
40+
private static final String LETTERS = "a\u00D1"; // a and Ñ
41+
42+
// '\u0660' through '\u0669', Arabic-Indic digits, '\u06F0' through '\u06F9',
43+
// Extended Arabic-Indic digits
44+
// '\u0966' through '\u096F', Devanagari digits, '\uFF10' through '\uFF19',
45+
// Fullwidth digits
46+
private static final String DIGITS = "1\u0661\u06f2\u0968\uFF14";
47+
48+
private static final String CURRENCY = "€$";
49+
50+
// this is the complete puncutation set do not modify it as Character.isJavaIdentifierPart filters
51+
// the good and bad ones out in the setup.
52+
private static final String PUNCTUATION = "!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~";
53+
54+
private static final String COMBINING_MARK = "\u0303";
55+
56+
private static final String NON_SPACING_MARK = "\u0CBF";
57+
58+
private static final String IDENTIFIER_IGNORABLE = "\u0000\u0001\u0002\u0003\u0004\u0005\u0006\u0007\u0008";
59+
60+
private static String acceptablePunctuation;
61+
62+
private static String notAcceptablePunctuation;
63+
64+
private static String additionalOptonChars;
65+
private static String additionalLongChars;
66+
67+
private static String firstChars;
68+
private static String notFirstChars;
69+
70+
private static String restChars;
71+
private static String notRestChars;
72+
73+
@BeforeAll
74+
public static void setup() {
75+
StringBuilder sb = new StringBuilder();
76+
StringBuilder sb2 = new StringBuilder();
77+
int idx;
78+
79+
for (char c : PUNCTUATION.toCharArray()) {
80+
if (Character.isJavaIdentifierPart(c)) {
81+
sb.append(c);
82+
} else {
83+
sb2.append(c);
84+
}
85+
}
86+
acceptablePunctuation = sb.toString();
87+
notAcceptablePunctuation = sb2.toString();
88+
89+
sb = new StringBuilder();
90+
for (char c : OptionValidator.ADDITIONAL_LONG_CHARS) {
91+
sb.append(c);
92+
}
93+
additionalLongChars = sb.toString();
94+
95+
sb = new StringBuilder();
96+
for (char c : OptionValidator.ADDITIONAL_OPTION_CHARS) {
97+
sb.append(c);
98+
}
99+
additionalOptonChars = sb.toString();
100+
101+
String javaIdentifierPart = LETTERS + DIGITS + CURRENCY + acceptablePunctuation + COMBINING_MARK
102+
+ NON_SPACING_MARK + IDENTIFIER_IGNORABLE;
103+
104+
firstChars = additionalOptonChars + javaIdentifierPart;
105+
106+
sb = new StringBuilder(notAcceptablePunctuation).append(additionalLongChars);
107+
for (char c : OptionValidator.ADDITIONAL_OPTION_CHARS) {
108+
while ((idx = sb.indexOf(Character.toString(c))) > -1) {
109+
sb.deleteCharAt(idx);
110+
}
111+
}
112+
notFirstChars = sb.toString();
113+
114+
restChars = additionalLongChars + javaIdentifierPart;
115+
sb = new StringBuilder(notAcceptablePunctuation).append(additionalOptonChars);
116+
for (char c : OptionValidator.ADDITIONAL_LONG_CHARS) {
117+
while ((idx = sb.indexOf(Character.toString(c))) > -1) {
118+
sb.deleteCharAt(idx);
119+
}
120+
}
121+
notRestChars = sb.toString();
122+
123+
}
124+
125+
@Test
126+
public void testExclusivity() {
127+
/* since we modify acceptable chars by add and removing ADDITIONAL* chars we must verify that they do not exist in the
128+
* base javaIdentiferPart that is used in OptionValidator to validate basic characters */
129+
for (char c : OptionValidator.ADDITIONAL_LONG_CHARS) {
130+
assertFalse(Character.isJavaIdentifierPart(c), () -> String.format("'%s' should not be in 'ADDITIONAL_LONG_CHARS", c));
131+
}
132+
for (char c : OptionValidator.ADDITIONAL_OPTION_CHARS) {
133+
assertFalse(Character.isJavaIdentifierPart(c), () -> String.format("'%s' should not be in 'ADDITIONAL_OPTION_CHARS", c));
134+
}
135+
}
136+
137+
@ParameterizedTest(name = "{2}")
138+
@MethodSource("optionParameters")
139+
public void validateTest(final String str, final boolean expected, final String name) {
140+
if (expected) {
141+
assertEquals(str, OptionValidator.validate(str));
142+
} else {
143+
assertThrows(IllegalArgumentException.class, () -> OptionValidator.validate(str));
144+
}
145+
}
146+
147+
private static Stream<Arguments> optionParameters() {
148+
149+
List<Arguments> args = new ArrayList<>();
150+
151+
args.add(Arguments.of("CamelCase", true, "Camel case error"));
152+
args.add(Arguments.of("Snake_case", true, "Snake case error"));
153+
args.add(Arguments.of("_leadingUnderscore", true, "Leading underscore error"));
154+
args.add(Arguments.of("kabob-case", true, "Kabob case error"));
155+
args.add(Arguments.of("-leadingDash", false, "Leading dash error"));
156+
args.add(Arguments.of("lowercase", true, "Lower case error"));
157+
args.add(Arguments.of("UPPERCASE", true, "Upper case error"));
158+
159+
// build passing test cases
160+
for (char c : firstChars.toCharArray()) {
161+
String s = String.format("%sMoreText", c);
162+
args.add(Arguments.of(s, true, String.format("testing: First character '%s'", c)));
163+
}
164+
165+
for (char c : restChars.toCharArray()) {
166+
String s = String.format("Some%sText", c);
167+
args.add(Arguments.of(s, true, String.format("testing: Middle character '%s'", c)));
168+
}
169+
170+
// build failing test cases
171+
for (char c : notFirstChars.toCharArray()) {
172+
String s = String.format("%sMoreText", c);
173+
args.add(Arguments.of(s, false, String.format("testing: Bad first character '%s'", c)));
174+
}
175+
176+
for (char c : notRestChars.toCharArray()) {
177+
String s = String.format("Some%sText", c);
178+
args.add(Arguments.of(s, false, String.format("testing: Bad middle character '%s'", c)));
179+
}
180+
181+
return args.stream();
182+
}
183+
}

0 commit comments

Comments
 (0)