You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2024/02/27 02:10:10 UTC
(commons-cli) 03/05: Use final
This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-cli.git
commit d676360220381e5a0ce67dd6f36c10e4b3a7fc90
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Mon Feb 26 21:08:26 2024 -0500
Use final
- Init collection on creation
- Use a switch
---
.../org/apache/commons/cli/OptionValidator.java | 4 +--
src/main/java/org/apache/commons/cli/Options.java | 2 +-
.../apache/commons/cli/OptionValidatorTest.java | 36 +++++++++++-----------
.../java/org/apache/commons/cli/OptionsTest.java | 7 ++---
.../org/apache/commons/cli/bug/BugCLI312Test.java | 23 +++++++++-----
.../org/apache/commons/cli/bug/BugCLI325Test.java | 6 ++--
6 files changed, 42 insertions(+), 36 deletions(-)
diff --git a/src/main/java/org/apache/commons/cli/OptionValidator.java b/src/main/java/org/apache/commons/cli/OptionValidator.java
index b85f76b..7a7fc38 100644
--- a/src/main/java/org/apache/commons/cli/OptionValidator.java
+++ b/src/main/java/org/apache/commons/cli/OptionValidator.java
@@ -78,7 +78,7 @@ final class OptionValidator {
* @return {@code true} if {@code c} was in {@code ary}, {@code false} otherwise.
*/
private static boolean search(final char[] chars, final char c) {
- for (char a : chars) {
+ for (final char a : chars) {
if (a == c) {
return true;
}
@@ -119,7 +119,7 @@ final class OptionValidator {
return null;
}
- char[] chars = option.toCharArray();
+ final char[] chars = option.toCharArray();
if (!isValidOpt(chars[0])) {
throw new IllegalArgumentException("Illegal option name '" + chars[0] + "'");
diff --git a/src/main/java/org/apache/commons/cli/Options.java b/src/main/java/org/apache/commons/cli/Options.java
index f886e7b..585f543 100644
--- a/src/main/java/org/apache/commons/cli/Options.java
+++ b/src/main/java/org/apache/commons/cli/Options.java
@@ -159,7 +159,7 @@ public class Options implements Serializable {
* @since 1.7.0
*/
public Options addOptions(final Options options) {
- for (Option opt : options.getOptions()) {
+ for (final Option opt : options.getOptions()) {
if (hasOption(opt.getKey())) {
throw new IllegalArgumentException("Duplicate key: " + opt.getKey());
}
diff --git a/src/test/java/org/apache/commons/cli/OptionValidatorTest.java b/src/test/java/org/apache/commons/cli/OptionValidatorTest.java
index aa1e212..17bc24c 100644
--- a/src/test/java/org/apache/commons/cli/OptionValidatorTest.java
+++ b/src/test/java/org/apache/commons/cli/OptionValidatorTest.java
@@ -72,7 +72,7 @@ public class OptionValidatorTest {
private static Stream<Arguments> optionParameters() {
- List<Arguments> args = new ArrayList<>();
+ final List<Arguments> args = new ArrayList<>();
args.add(Arguments.of("CamelCase", true, "Camel case error"));
args.add(Arguments.of("Snake_case", true, "Snake case error"));
@@ -83,24 +83,24 @@ public class OptionValidatorTest {
args.add(Arguments.of("UPPERCASE", true, "Upper case error"));
// build passing test cases
- for (char c : firstChars.toCharArray()) {
- String s = String.format("%sMoreText", c);
+ for (final char c : firstChars.toCharArray()) {
+ final String s = String.format("%sMoreText", c);
args.add(Arguments.of(s, true, String.format("testing: First character '%s'", c)));
}
- for (char c : restChars.toCharArray()) {
- String s = String.format("Some%sText", c);
+ for (final char c : restChars.toCharArray()) {
+ final String s = String.format("Some%sText", c);
args.add(Arguments.of(s, true, String.format("testing: Middle character '%s'", c)));
}
// build failing test cases
- for (char c : notFirstChars.toCharArray()) {
- String s = String.format("%sMoreText", c);
+ for (final char c : notFirstChars.toCharArray()) {
+ final String s = String.format("%sMoreText", c);
args.add(Arguments.of(s, false, String.format("testing: Bad first character '%s'", c)));
}
- for (char c : notRestChars.toCharArray()) {
- String s = String.format("Some%sText", c);
+ for (final char c : notRestChars.toCharArray()) {
+ final String s = String.format("Some%sText", c);
args.add(Arguments.of(s, false, String.format("testing: Bad middle character '%s'", c)));
}
@@ -110,10 +110,10 @@ public class OptionValidatorTest {
@BeforeAll
public static void setup() {
StringBuilder sb = new StringBuilder();
- StringBuilder sb2 = new StringBuilder();
+ final StringBuilder sb2 = new StringBuilder();
int idx;
- for (char c : PUNCTUATION.toCharArray()) {
+ for (final char c : PUNCTUATION.toCharArray()) {
if (Character.isJavaIdentifierPart(c)) {
sb.append(c);
} else {
@@ -124,24 +124,24 @@ public class OptionValidatorTest {
notAcceptablePunctuation = sb2.toString();
sb = new StringBuilder();
- for (char c : OptionValidator.ADDITIONAL_LONG_CHARS) {
+ for (final char c : OptionValidator.ADDITIONAL_LONG_CHARS) {
sb.append(c);
}
additionalLongChars = sb.toString();
sb = new StringBuilder();
- for (char c : OptionValidator.ADDITIONAL_OPTION_CHARS) {
+ for (final char c : OptionValidator.ADDITIONAL_OPTION_CHARS) {
sb.append(c);
}
additionalOptonChars = sb.toString();
- String javaIdentifierPart = LETTERS + DIGITS + CURRENCY + acceptablePunctuation + COMBINING_MARK
+ final String javaIdentifierPart = LETTERS + DIGITS + CURRENCY + acceptablePunctuation + COMBINING_MARK
+ NON_SPACING_MARK + IDENTIFIER_IGNORABLE;
firstChars = additionalOptonChars + javaIdentifierPart;
sb = new StringBuilder(notAcceptablePunctuation).append(additionalLongChars);
- for (char c : OptionValidator.ADDITIONAL_OPTION_CHARS) {
+ for (final char c : OptionValidator.ADDITIONAL_OPTION_CHARS) {
while ((idx = sb.indexOf(Character.toString(c))) > -1) {
sb.deleteCharAt(idx);
}
@@ -150,7 +150,7 @@ public class OptionValidatorTest {
restChars = additionalLongChars + javaIdentifierPart;
sb = new StringBuilder(notAcceptablePunctuation).append(additionalOptonChars);
- for (char c : OptionValidator.ADDITIONAL_LONG_CHARS) {
+ for (final char c : OptionValidator.ADDITIONAL_LONG_CHARS) {
while ((idx = sb.indexOf(Character.toString(c))) > -1) {
sb.deleteCharAt(idx);
}
@@ -163,10 +163,10 @@ public class OptionValidatorTest {
public void testExclusivity() {
/* since we modify acceptable chars by add and removing ADDITIONAL* chars we must verify that they do not exist in the
* base javaIdentiferPart that is used in OptionValidator to validate basic characters */
- for (char c : OptionValidator.ADDITIONAL_LONG_CHARS) {
+ for (final char c : OptionValidator.ADDITIONAL_LONG_CHARS) {
assertFalse(Character.isJavaIdentifierPart(c), () -> String.format("'%s' should not be in 'ADDITIONAL_LONG_CHARS", c));
}
- for (char c : OptionValidator.ADDITIONAL_OPTION_CHARS) {
+ for (final char c : OptionValidator.ADDITIONAL_OPTION_CHARS) {
assertFalse(Character.isJavaIdentifierPart(c), () -> String.format("'%s' should not be in 'ADDITIONAL_OPTION_CHARS", c));
}
}
diff --git a/src/test/java/org/apache/commons/cli/OptionsTest.java b/src/test/java/org/apache/commons/cli/OptionsTest.java
index 187ff46..63be98b 100644
--- a/src/test/java/org/apache/commons/cli/OptionsTest.java
+++ b/src/test/java/org/apache/commons/cli/OptionsTest.java
@@ -73,14 +73,13 @@ public class OptionsTest {
options1.addOption(Option.builder("e").build());
options1.addOption(Option.builder("f").build());
- Options underTest = new Options();
+ final Options underTest = new Options();
underTest.addOptions(options1);
underTest.addOptions(options2);
- List<OptionGroup> expected = Arrays.asList(group1, group2);
+ final List<OptionGroup> expected = Arrays.asList(group1, group2);
assertTrue(expected.size() == underTest.getOptionGroups().size() && expected.containsAll(underTest.getOptionGroups()));
- Set<Option> expectOpt = new HashSet<>();
- expectOpt.addAll(options1.getOptions());
+ final Set<Option> expectOpt = new HashSet<>(options1.getOptions());
expectOpt.addAll(options2.getOptions());
assertEquals(8, expectOpt.size());
assertTrue(expectOpt.size() == underTest.getOptions().size() && expectOpt.containsAll(underTest.getOptions()));
diff --git a/src/test/java/org/apache/commons/cli/bug/BugCLI312Test.java b/src/test/java/org/apache/commons/cli/bug/BugCLI312Test.java
index e110e5a..1ca7841 100644
--- a/src/test/java/org/apache/commons/cli/bug/BugCLI312Test.java
+++ b/src/test/java/org/apache/commons/cli/bug/BugCLI312Test.java
@@ -40,7 +40,7 @@ public class BugCLI312Test {
public void testNoOptionValues() {
final Option o1 = Option.builder("A").build();
final Option o2 = Option.builder().option("D").longOpt("define").numberOfArgs(2).valueSeparator('=').build();
- Options options = new Options().addOption(o1).addOption(o2);
+ final Options options = new Options().addOption(o1).addOption(o2);
final CommandLineParser parser = new DefaultParser();
@@ -59,7 +59,7 @@ public class BugCLI312Test {
final CommandLine cl = parser.parse(options, "-Dv -Dw=1 -D x=2 -D y -D z=3 other".split(" "));
assertArrayEquals(new String[] {"v", "w", "1", "x", "2", "y", "z", "3"}, cl.getOptionValues('D'));
- Properties properties = cl.getOptionProperties("D");
+ final Properties properties = cl.getOptionProperties("D");
assertEquals("true", properties.getProperty("v"));
assertEquals("1", properties.getProperty("w"));
assertEquals("2", properties.getProperty("x"));
@@ -86,18 +86,25 @@ public class BugCLI312Test {
if ("D".equals(o.getOpt())) {
defineOptionsFound++;
- if (defineOptionsFound == 1) {
+ switch (defineOptionsFound) {
+ case 1:
assertArrayEquals(new String[] {"v"}, o.getValues());
- } else if (defineOptionsFound == 2) {
+ break;
+ case 2:
assertArrayEquals(new String[] {"w", "1"}, o.getValues());
- } else if (defineOptionsFound == 3) {
+ break;
+ case 3:
assertArrayEquals(new String[] {"x", "2"}, o.getValues());
- } else if (defineOptionsFound == 4) {
+ break;
+ case 4:
assertArrayEquals(new String[] {"y"}, o.getValues());
- } else if (defineOptionsFound == 5) {
+ break;
+ case 5:
assertArrayEquals(new String[] {"z", "3"}, o.getValues());
- } else {
+ break;
+ default:
fail("Didn't expect " + defineOptionsFound + " occurrences of -D");
+ break;
}
}
}
diff --git a/src/test/java/org/apache/commons/cli/bug/BugCLI325Test.java b/src/test/java/org/apache/commons/cli/bug/BugCLI325Test.java
index e459873..735fffe 100644
--- a/src/test/java/org/apache/commons/cli/bug/BugCLI325Test.java
+++ b/src/test/java/org/apache/commons/cli/bug/BugCLI325Test.java
@@ -39,10 +39,10 @@ public class BugCLI325Test {
.desc("Multiple arg option with value separator.")
.build();
- String[] args = {"-x", "A=a", "B=b"};
+ final String[] args = {"-x", "A=a", "B=b"};
- CommandLine cmdLine = DefaultParser.builder().build().parse(new Options().addOption(option), args);
- Properties props = cmdLine.getOptionProperties(option);
+ final CommandLine cmdLine = DefaultParser.builder().build().parse(new Options().addOption(option), args);
+ final Properties props = cmdLine.getOptionProperties(option);
assertEquals(2, props.size());
assertEquals("a", props.get("A"));
assertEquals("b", props.get("B"));