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 35cf8df921caf3a71b05e6e7e450ee81e514bead
Author: Gary Gregory <garydgreg...@gmail.com>
AuthorDate: Thu Aug 8 09:32:03 2024 -0400
Replace internal StringBuffer with StringBuilder
---
pom.xml | 6 +
.../java/org/apache/commons/cli/HelpFormatter.java | 261 ++++++++++++---------
.../org/apache/commons/cli/HelpFormatterTest.java | 18 +-
3 files changed, 172 insertions(+), 113 deletions(-)
diff --git a/pom.xml b/pom.xml
index 0abb6f2..1e05519 100644
--- a/pom.xml
+++ b/pom.xml
@@ -63,6 +63,12 @@
<version>2.16.1</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.mockito</groupId>
+ <artifactId>mockito-core</artifactId>
+ <version>4.11.0</version>
+ <scope>test</scope>
+ </dependency>
</dependencies>
<properties>
<project.build.outputTimestamp>2024-05-23T21:37:00Z</project.build.outputTimestamp>
diff --git a/src/main/java/org/apache/commons/cli/HelpFormatter.java
b/src/main/java/org/apache/commons/cli/HelpFormatter.java
index 30e94cf..6593057 100644
--- a/src/main/java/org/apache/commons/cli/HelpFormatter.java
+++ b/src/main/java/org/apache/commons/cli/HelpFormatter.java
@@ -22,6 +22,7 @@ import java.io.IOException;
import java.io.PrintWriter;
import java.io.Serializable;
import java.io.StringReader;
+import java.io.UncheckedIOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
@@ -374,7 +375,6 @@ public class HelpFormatter {
if (!group.isRequired()) {
buff.append("[");
}
-
final List<Option> optList = new ArrayList<>(group.getOptions());
if (getOptionComparator() != null) {
Collections.sort(optList, getOptionComparator());
@@ -388,12 +388,135 @@ public class HelpFormatter {
buff.append(" | ");
}
}
-
if (!group.isRequired()) {
buff.append("]");
}
}
+ /**
+ * Renders the specified Options and return the rendered Options in a
StringBuffer.
+ *
+ * @param sb The StringBuffer to place the rendered Options into.
+ * @param width The number of characters to display per line
+ * @param options The command line Options
+ * @param leftPad the number of characters of padding to be prefixed to
each line
+ * @param descPad the number of characters of padding to be prefixed to
each description line
+ * @return the StringBuffer with the rendered Options contents.
+ * @throws IOException if an I/O error occurs.
+ */
+ <A extends Appendable> A appendOptions(final A sb, final int width, final
Options options, final int leftPad, final int descPad) throws IOException {
+ final String lpad = createPadding(leftPad);
+ final String dpad = createPadding(descPad);
+ // first create list containing only <lpad>-a,--aaa where
+ // -a is opt and --aaa is long opt; in parallel look for
+ // the longest opt string this list will be then used to
+ // sort options ascending
+ int max = 0;
+ final int maxSince = showSince ? determineMaxSinceLength(options) +
leftPad : 0;
+ final List<StringBuilder> prefixList = new ArrayList<>();
+ final List<Option> optList = options.helpOptions();
+ if (getOptionComparator() != null) {
+ Collections.sort(optList, getOptionComparator());
+ }
+ for (final Option option : optList) {
+ final StringBuilder optBuf = new StringBuilder();
+ if (option.getOpt() == null) {
+ optBuf.append(lpad).append("
").append(getLongOptPrefix()).append(option.getLongOpt());
+ } else {
+
optBuf.append(lpad).append(getOptPrefix()).append(option.getOpt());
+ if (option.hasLongOpt()) {
+
optBuf.append(',').append(getLongOptPrefix()).append(option.getLongOpt());
+ }
+ }
+ if (option.hasArg()) {
+ final String argName = option.getArgName();
+ if (argName != null && argName.isEmpty()) {
+ // if the option has a blank argname
+ optBuf.append(' ');
+ } else {
+ optBuf.append(option.hasLongOpt() ? longOptSeparator : "
");
+ optBuf.append("<").append(argName != null ?
option.getArgName() : getArgName()).append(">");
+ }
+ }
+
+ prefixList.add(optBuf);
+ max = Math.max(optBuf.length() + maxSince, max);
+ }
+ final int nextLineTabStop = max + descPad;
+ if (showSince) {
+ final StringBuilder optHeader = new
StringBuilder(HEADER_OPTIONS).append(createPadding(max - maxSince -
HEADER_OPTIONS.length() + leftPad))
+ .append(HEADER_SINCE);
+ optHeader.append(createPadding(max -
optHeader.length())).append(lpad).append(HEADER_DESCRIPTION);
+ appendWrappedText(sb, width, nextLineTabStop,
optHeader.toString());
+ sb.append(getNewLine());
+ }
+
+ int x = 0;
+ for (final Iterator<Option> it = optList.iterator(); it.hasNext();) {
+ final Option option = it.next();
+ final StringBuilder optBuf = new
StringBuilder(prefixList.get(x++).toString());
+ if (optBuf.length() < max) {
+ optBuf.append(createPadding(max - maxSince - optBuf.length()));
+ if (showSince) {
+ optBuf.append(lpad).append(option.getSince() == null ? "-"
: option.getSince());
+ }
+ optBuf.append(createPadding(max - optBuf.length()));
+ }
+ optBuf.append(dpad);
+
+ if (deprecatedFormatFunc != null && option.isDeprecated()) {
+ optBuf.append(deprecatedFormatFunc.apply(option).trim());
+ } else if (option.getDescription() != null) {
+ optBuf.append(option.getDescription());
+ }
+ appendWrappedText(sb, width, nextLineTabStop, optBuf.toString());
+ if (it.hasNext()) {
+ sb.append(getNewLine());
+ }
+ }
+ return sb;
+ }
+
+ /**
+ * Renders the specified text and return the rendered Options in a
StringBuffer.
+ *
+ * @param <A> The Appendable implementation.
+ * @param appendable The StringBuffer to place the rendered text into.
+ * @param width The number of characters to display per line
+ * @param nextLineTabStop The position on the next line for the first tab.
+ * @param text The text to be rendered.
+ * @return the StringBuffer with the rendered Options contents.
+ * @throws IOException if an I/O error occurs.
+ */
+ <A extends Appendable> A appendWrappedText(final A appendable, final int
width, final int nextLineTabStop, final String text) throws IOException {
+ String render = text;
+ int nextLineTabStopPos = nextLineTabStop;
+ int pos = findWrapPos(render, width, 0);
+ if (pos == -1) {
+ appendable.append(rtrim(render));
+ return appendable;
+ }
+ appendable.append(rtrim(render.substring(0,
pos))).append(getNewLine());
+ if (nextLineTabStopPos >= width) {
+ // stops infinite loop happening
+ nextLineTabStopPos = 1;
+ }
+ // all following lines must be padded with nextLineTabStop space
characters
+ final String padding = createPadding(nextLineTabStopPos);
+ while (true) {
+ render = padding + render.substring(pos).trim();
+ pos = findWrapPos(render, width, 0);
+ if (pos == -1) {
+ appendable.append(render);
+ return appendable;
+ }
+ if (render.length() > width && pos == nextLineTabStopPos - 1) {
+ pos = width;
+ }
+ appendable.append(rtrim(render.substring(0,
pos))).append(getNewLine());
+ }
+ }
+
/**
* Creates a String of padding of length {@code len}.
*
@@ -407,6 +530,12 @@ public class HelpFormatter {
return new String(padding);
}
+ private int determineMaxSinceLength(final Options options) {
+ final int minLen = HEADER_SINCE.length();
+ final int len = options.getOptions().stream().map(o -> o.getSince() ==
null ? minLen : o.getSince().length()).max(Integer::compareTo).orElse(minLen);
+ return len < minLen ? minLen : len;
+ }
+
/**
* Finds the next text wrap position after {@code startPos} for the text
in {@code text} with the column width
* {@code width}. The wrap point is the last position before
startPos+width having a whitespace character (space,
@@ -443,7 +572,6 @@ public class HelpFormatter {
}
// if we didn't find one, simply chop at startPos+width
pos = startPos + width;
-
return pos == text.length() ? -1 : pos;
}
@@ -685,9 +813,12 @@ public class HelpFormatter {
* @param descPad the number of characters of padding to be prefixed to
each description line
*/
public void printOptions(final PrintWriter pw, final int width, final
Options options, final int leftPad, final int descPad) {
- final StringBuffer sb = new StringBuffer();
- renderOptions(sb, width, options, leftPad, descPad);
- pw.println(sb.toString());
+ try {
+ pw.println(appendOptions(new StringBuilder(), width, options,
leftPad, descPad));
+ } catch (final IOException e) {
+ // Cannot happen
+ throw new UncheckedIOException(e);
+ }
}
/**
@@ -759,9 +890,7 @@ public class HelpFormatter {
* @param text The text to be written to the PrintWriter
*/
public void printWrapped(final PrintWriter pw, final int width, final int
nextLineTabStop, final String text) {
- final StringBuffer sb = new StringBuffer(text.length());
- renderWrappedTextBlock(sb, width, nextLineTabStop, text);
- pw.println(sb.toString());
+ pw.println(renderWrappedTextBlock(new StringBuilder(text.length()),
width, nextLineTabStop, text));
}
/**
@@ -775,12 +904,6 @@ public class HelpFormatter {
printWrapped(pw, width, 0, text);
}
- private int determineMaxSinceLength(final Options options) {
- int minLen = HEADER_SINCE.length();
- int len = options.getOptions().stream().map(o -> o.getSince() == null
? minLen : o.getSince().length()).max(Integer::compareTo).orElse(minLen);
- return len < minLen ? minLen : len;
- }
-
/**
* Renders the specified Options and return the rendered Options in a
StringBuffer.
*
@@ -793,76 +916,12 @@ public class HelpFormatter {
* @return the StringBuffer with the rendered Options contents.
*/
protected StringBuffer renderOptions(final StringBuffer sb, final int
width, final Options options, final int leftPad, final int descPad) {
- final String lpad = createPadding(leftPad);
- final String dpad = createPadding(descPad);
- // first create list containing only <lpad>-a,--aaa where
- // -a is opt and --aaa is long opt; in parallel look for
- // the longest opt string this list will be then used to
- // sort options ascending
- int max = 0;
- int maxSince = showSince ? determineMaxSinceLength(options) + leftPad
: 0;
- final List<StringBuilder> prefixList = new ArrayList<>();
- final List<Option> optList = options.helpOptions();
- if (getOptionComparator() != null) {
- Collections.sort(optList, getOptionComparator());
- }
- for (final Option option : optList) {
- final StringBuilder optBuf = new StringBuilder();
- if (option.getOpt() == null) {
- optBuf.append(lpad).append("
").append(getLongOptPrefix()).append(option.getLongOpt());
- } else {
-
optBuf.append(lpad).append(getOptPrefix()).append(option.getOpt());
- if (option.hasLongOpt()) {
-
optBuf.append(',').append(getLongOptPrefix()).append(option.getLongOpt());
- }
- }
- if (option.hasArg()) {
- final String argName = option.getArgName();
- if (argName != null && argName.isEmpty()) {
- // if the option has a blank argname
- optBuf.append(' ');
- } else {
- optBuf.append(option.hasLongOpt() ? longOptSeparator : "
");
- optBuf.append("<").append(argName != null ?
option.getArgName() : getArgName()).append(">");
- }
- }
-
- prefixList.add(optBuf);
- max = Math.max(optBuf.length() + maxSince, max);
- }
- final int nextLineTabStop = max + descPad;
- if (showSince) {
- StringBuilder optHeader = new
StringBuilder(HEADER_OPTIONS).append(createPadding(max - maxSince -
HEADER_OPTIONS.length() + leftPad))
- .append(HEADER_SINCE);
- optHeader.append(createPadding(max -
optHeader.length())).append(lpad).append(HEADER_DESCRIPTION);
- renderWrappedText(sb, width, nextLineTabStop,
optHeader.toString());
- sb.append(getNewLine());
- }
-
- int x = 0;
- for (final Iterator<Option> it = optList.iterator(); it.hasNext();) {
- final Option option = it.next();
- final StringBuilder optBuf = new
StringBuilder(prefixList.get(x++).toString());
- if (optBuf.length() < max) {
- optBuf.append(createPadding(max - maxSince - optBuf.length()));
- if (showSince) {
- optBuf.append(lpad).append(option.getSince() == null ? "-"
: option.getSince());
- }
- optBuf.append(createPadding(max - optBuf.length()));
- }
- optBuf.append(dpad);
-
- if (deprecatedFormatFunc != null && option.isDeprecated()) {
- optBuf.append(deprecatedFormatFunc.apply(option).trim());
- } else if (option.getDescription() != null) {
- optBuf.append(option.getDescription());
- }
- renderWrappedText(sb, width, nextLineTabStop, optBuf.toString());
- if (it.hasNext()) {
- sb.append(getNewLine());
- }
+ try {
+ return appendOptions(sb, width, options, leftPad, descPad);
+ } catch (final IOException e) {
+ // Cannot happen
+ throw new UncheckedIOException(e);
}
- return sb;
}
/**
@@ -876,31 +935,11 @@ public class HelpFormatter {
* @return the StringBuffer with the rendered Options contents.
*/
protected StringBuffer renderWrappedText(final StringBuffer sb, final int
width, final int nextLineTabStop, final String text) {
- String render = text;
- int nextLineTabStopPos = nextLineTabStop;
- int pos = findWrapPos(render, width, 0);
- if (pos == -1) {
- sb.append(rtrim(render));
- return sb;
- }
- sb.append(rtrim(render.substring(0, pos))).append(getNewLine());
- if (nextLineTabStopPos >= width) {
- // stops infinite loop happening
- nextLineTabStopPos = 1;
- }
- // all following lines must be padded with nextLineTabStop space
characters
- final String padding = createPadding(nextLineTabStopPos);
- while (true) {
- render = padding + render.substring(pos).trim();
- pos = findWrapPos(render, width, 0);
- if (pos == -1) {
- sb.append(render);
- return sb;
- }
- if (render.length() > width && pos == nextLineTabStopPos - 1) {
- pos = width;
- }
- sb.append(rtrim(render.substring(0, pos))).append(getNewLine());
+ try {
+ return appendWrappedText(sb, width, nextLineTabStop, text);
+ } catch (final IOException e) {
+ // Cannot happen.
+ throw new UncheckedIOException(e);
}
}
@@ -913,7 +952,7 @@ public class HelpFormatter {
* @param nextLineTabStop The position on the next line for the first tab.
* @param text The text to be rendered.
*/
- private Appendable renderWrappedTextBlock(final StringBuffer sb, final int
width, final int nextLineTabStop, final String text) {
+ private <A extends Appendable> A renderWrappedTextBlock(final A sb, final
int width, final int nextLineTabStop, final String text) {
try {
final BufferedReader in = new BufferedReader(new
StringReader(text));
String line;
@@ -924,7 +963,7 @@ public class HelpFormatter {
} else {
firstLine = false;
}
- renderWrappedText(sb, width, nextLineTabStop, line);
+ appendWrappedText(sb, width, nextLineTabStop, line);
}
} catch (final IOException e) { // NOPMD
// cannot happen
diff --git a/src/test/java/org/apache/commons/cli/HelpFormatterTest.java
b/src/test/java/org/apache/commons/cli/HelpFormatterTest.java
index 09b449f..fc9904e 100644
--- a/src/test/java/org/apache/commons/cli/HelpFormatterTest.java
+++ b/src/test/java/org/apache/commons/cli/HelpFormatterTest.java
@@ -21,11 +21,15 @@ import static
org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.when;
import java.io.ByteArrayOutputStream;
+import java.io.IOException;
import java.io.OutputStreamWriter;
import java.io.PrintWriter;
import java.io.StringWriter;
+import java.io.UncheckedIOException;
import java.util.ArrayList;
import java.util.List;
import java.util.stream.Stream;
@@ -36,6 +40,7 @@ import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
+
/**
* Test case for the HelpFormatter class.
*/
@@ -657,7 +662,7 @@ public class HelpFormatterTest {
}
@Test
- public void testRenderSince() {
+ public void testRenderSince() throws IOException {
final String[] expected = {"Options Since Description", "
-n,--no-since - Description for n",
" -W,--with-since 1.19.0 Descripton for W"};
final Options options = new Options()
@@ -668,6 +673,10 @@ public class HelpFormatterTest {
final StringBuffer sb = new StringBuffer();
formatter.renderOptions(sb, 50, options, 2, 5);
assertArrayEquals(expected,
sb.toString().split(System.lineSeparator()));
+ // check internal exception handling for coverage
+ final HelpFormatter spy = spy(formatter);
+ when(spy.appendOptions(sb, 50, options, 2,
5)).thenThrow(IOException.class);
+ assertThrows(UncheckedIOException.class, () -> spy.renderOptions(sb,
50, options, 2, 5));
}
@Test
@@ -706,7 +715,7 @@ public class HelpFormatterTest {
}
@Test
- public void testRenderWrappedTextSingleLine() {
+ public void testRenderWrappedTextSingleLine() throws IOException {
// single line text
final int width = 12;
final int padding = 0;
@@ -716,6 +725,11 @@ public class HelpFormatterTest {
final StringBuffer sb = new StringBuffer();
new HelpFormatter().renderWrappedText(sb, width, padding, text);
assertEquals(expected, sb.toString(), "single line text");
+ // check internal exception handling for coverage
+ final HelpFormatter spy = spy(new HelpFormatter());
+ when(spy.appendWrappedText(sb, width, padding,
text)).thenThrow(IOException.class);
+ assertThrows(UncheckedIOException.class, () ->
spy.renderWrappedText(sb, width, padding, text));
+
}
@Test
