[cli] Move CLI progressbar rendering to pmd-cli

- Took the chance to improve how we render it
 - Added support for interactive terminals
 - Improved the overall performance / thread-safety of report rendering to STDOUT
This commit is contained in:
Juan Martín Sotuyo Dodero
2022-11-25 01:24:10 -03:00
parent e38326844a
commit b836a35c8c
15 changed files with 116 additions and 76 deletions

View File

@ -262,6 +262,10 @@
<groupId>info.picocli</groupId>
<artifactId>picocli</artifactId>
</dependency>
<dependency>
<groupId>me.tongfei</groupId>
<artifactId>progressbar</artifactId>
</dependency>
<dependency>
<groupId>org.checkerframework</groupId>
<artifactId>checker-qual</artifactId>

View File

@ -0,0 +1,43 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package me.tongfei.progressbar;
import static me.tongfei.progressbar.TerminalUtils.CARRIAGE_RETURN;
import java.io.PrintStream;
/**
* This is a friend class for me.tongfei.progressbar, as TerminalUtils is package-private.
*/
public final class PmdProgressBarFriend {
private PmdProgressBarFriend() {
throw new AssertionError("Can't instantiate utility classes");
}
public static ConsoleProgressBarConsumer createConsoleConsumer(PrintStream ps) {
return TerminalUtils.hasCursorMovementSupport()
? new InteractiveConsoleProgressBarConsumer(ps)
: new PostCarriageReturnConsoleProgressBarConsumer(ps);
}
private static class PostCarriageReturnConsoleProgressBarConsumer extends ConsoleProgressBarConsumer {
public PostCarriageReturnConsoleProgressBarConsumer(PrintStream out) {
super(out);
}
@Override
public void accept(String str) {
// Set the carriage return at the end instead of at the beginning
out.print(StringDisplayUtils.trimDisplayLength(str, getMaxRenderedLength()) + CARRIAGE_RETURN);
}
@Override
public void clear() {
// do nothing (prints an empty line otherwise)
}
}
}

View File

@ -27,6 +27,7 @@ import net.sourceforge.pmd.benchmark.TimingReportRenderer;
import net.sourceforge.pmd.cli.commands.typesupport.internal.PmdLanguageTypeSupport;
import net.sourceforge.pmd.cli.commands.typesupport.internal.PmdLanguageVersionTypeSupport;
import net.sourceforge.pmd.cli.internal.ExecutionResult;
import net.sourceforge.pmd.cli.internal.ProgressBarListener;
import net.sourceforge.pmd.internal.LogMessages;
import net.sourceforge.pmd.lang.Language;
import net.sourceforge.pmd.lang.LanguageVersion;
@ -279,7 +280,6 @@ public class PmdCommand extends AbstractAnalysisPmdSubcommand {
configuration.setFailOnViolation(failOnViolation);
configuration.setAnalysisCacheLocation(cacheLocation != null ? cacheLocation.toString() : null);
configuration.setIgnoreIncrementalAnalysis(noCache);
configuration.setProgressBar(showProgressBar);
if (languageVersion != null) {
configuration.setDefaultLanguageVersions(languageVersion);
@ -323,6 +323,16 @@ public class PmdCommand extends AbstractAnalysisPmdSubcommand {
}
pmdReporter.log(Level.DEBUG, "Current classpath:\n{0}", System.getProperty("java.class.path"));
if (showProgressBar) {
if (reportFile == null) {
pmdReporter.log(Level.WARN, "Progressbar rendering conflicts with reporting to STDOUT. "
+ "No progressbar will be shown. Try running with '-r'");
} else {
pmd.addListener(new ProgressBarListener());
}
}
final ReportStats stats = pmd.runAndReturnStats();
if (pmdReporter.numErrors() > 0) {
// processing errors are ignored

View File

@ -5,7 +5,6 @@
package net.sourceforge.pmd.cli.internal;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Consumer;
import net.sourceforge.pmd.Report;
import net.sourceforge.pmd.RuleViolation;
@ -13,7 +12,7 @@ import net.sourceforge.pmd.lang.document.TextFile;
import net.sourceforge.pmd.reporting.FileAnalysisListener;
import net.sourceforge.pmd.reporting.GlobalAnalysisListener;
import me.tongfei.progressbar.DelegatingProgressBarConsumer;
import me.tongfei.progressbar.PmdProgressBarFriend;
import me.tongfei.progressbar.ProgressBar;
import me.tongfei.progressbar.ProgressBarBuilder;
import me.tongfei.progressbar.ProgressBarStyle;
@ -23,33 +22,30 @@ import me.tongfei.progressbar.ProgressBarStyle;
* Toggled off through --no-progress command line argument.
*/
public final class ProgressBarListener implements GlobalAnalysisListener {
private final ProgressBar progressBar;
private ProgressBar progressBar;
private final AtomicInteger numErrors = new AtomicInteger(0);
private final AtomicInteger numViolations = new AtomicInteger(0);
public ProgressBarListener(int totalFiles, Consumer<String> loggingFunction) {
@Override
public void startAnalysis(int totalFilesToAnalyze) {
// We need to delay initialization until we know how many files there are to avoid a first bogus render
progressBar = new ProgressBarBuilder()
.setTaskName("Processing files")
.setInitialMax(totalFiles)
.setStyle(ProgressBarStyle.ASCII)
.hideEta()
.continuousUpdate()
.setConsumer(new DelegatingProgressBarConsumer(loggingFunction))
.setInitialMax(totalFilesToAnalyze)
.setConsumer(PmdProgressBarFriend.createConsoleConsumer(System.out))
.clearDisplayOnFinish()
.build();
progressBar.setExtraMessage(extraMessage() + "\r");
progressBar.setExtraMessage(extraMessage());
}
/**
* Updates progress bar string and forces it to be output regardless of its update interval.
*/
private void refreshProgressBar() {
// Use trailing carriage return to interleave with other output
if (progressBar.getCurrent() != progressBar.getMax()) {
progressBar.setExtraMessage(extraMessage() + "\r");
} else {
// Don't include trailing carriage return on last draw
progressBar.setExtraMessage(extraMessage() + System.lineSeparator());
}
progressBar.setExtraMessage(extraMessage());
progressBar.refresh();
}
@ -59,9 +55,6 @@ public final class ProgressBarListener implements GlobalAnalysisListener {
@Override
public FileAnalysisListener startFileAnalysis(TextFile file) {
// Refresh progress on file analysis start
refreshProgressBar();
return new FileAnalysisListener() {
@Override
public void onRuleViolation(RuleViolation violation) {
@ -89,6 +82,6 @@ public final class ProgressBarListener implements GlobalAnalysisListener {
@Override
public void close() throws Exception {
/*ProgressBar auto-closed*/
progressBar.close();
}
}

View File

@ -153,10 +153,5 @@
<artifactId>system-lambda</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>me.tongfei</groupId>
<artifactId>progressbar</artifactId>
<version>0.9.3</version>
</dependency>
</dependencies>
</project>

View File

@ -134,7 +134,6 @@ public class PMDConfiguration extends AbstractConfiguration {
private AnalysisCache analysisCache = new NoopAnalysisCache();
private boolean ignoreIncrementalAnalysis;
private final LanguageRegistry langRegistry;
private boolean progressBar = false;
public PMDConfiguration() {
this(DEFAULT_REGISTRY);
@ -576,7 +575,6 @@ public class PMDConfiguration extends AbstractConfiguration {
* Get the input URI to process for source code objects.
*
* @return URI
* @deprecated Use {@link #getUri}
*/
public URI getUri() {
return inputUri;
@ -911,26 +909,4 @@ public class PMDConfiguration extends AbstractConfiguration {
public boolean isIgnoreIncrementalAnalysis() {
return ignoreIncrementalAnalysis;
}
/**
* Sets whether to indicate analysis progress in command line output.
*
* @param progressBar Whether to enable progress bar indicator in CLI
*/
public void setProgressBar(boolean progressBar) {
this.progressBar = progressBar;
}
/**
* Returns whether progress bar indicator should be used. The default
* is false.
*
* @return {@code true} if progress bar indicator is enabled
*/
public boolean isProgressBar() {
return progressBar;
}
}

View File

@ -24,7 +24,6 @@ import net.sourceforge.pmd.benchmark.TimedOperation;
import net.sourceforge.pmd.benchmark.TimedOperationCategory;
import net.sourceforge.pmd.cache.AnalysisCacheListener;
import net.sourceforge.pmd.cache.NoopAnalysisCache;
import net.sourceforge.pmd.cli.internal.ProgressBarListener;
import net.sourceforge.pmd.internal.LogMessages;
import net.sourceforge.pmd.internal.util.AssertionUtil;
import net.sourceforge.pmd.internal.util.FileCollectionUtil;
@ -280,11 +279,8 @@ public final class PmdAnalysis implements AutoCloseable {
GlobalAnalysisListener listener;
try {
@SuppressWarnings("PMD.CloseResource") AnalysisCacheListener cacheListener = new AnalysisCacheListener(configuration.getAnalysisCache(), rulesets, configuration.getClassLoader());
if (configuration.isProgressBar()) {
@SuppressWarnings("PMD.CloseResource") ProgressBarListener progressBarListener = new ProgressBarListener(textFiles.size(), System.out::print);
addListener(progressBarListener);
}
@SuppressWarnings("PMD.CloseResource")
AnalysisCacheListener cacheListener = new AnalysisCacheListener(configuration.getAnalysisCache(), rulesets, configuration.getClassLoader());
listener = GlobalAnalysisListener.tee(listOf(createComposedRendererListener(renderers),
GlobalAnalysisListener.tee(listeners),
GlobalAnalysisListener.tee(extraListeners),
@ -295,6 +291,8 @@ public final class PmdAnalysis implements AutoCloseable {
}
try (TimedOperation ignored = TimeTracker.startOperation(TimedOperationCategory.FILE_PROCESSING)) {
// Notify analysis is starting with so many files collected
listener.startAnalysis(textFiles.size());
for (final Rule rule : removeBrokenRules(rulesets)) {
// todo Just like we throw for invalid properties, "broken rules"

View File

@ -171,9 +171,6 @@ public class PMDParameters {
@Parameter(names = "--use-version", description = "The language version PMD should use when parsing source code in the language-version format, ie: 'java-1.8'")
private List<String> languageVersions = new ArrayList<>();
@Parameter(names = { "--no-progress", "-no-progress" }, description = "Disables progress bar indicator of live analysis progress.")
private boolean noProgressBar = false;
// this has to be a public static class, so that JCommander can use it!
public static class PropertyConverter implements IStringConverter<Properties> {
@ -272,7 +269,6 @@ public class PMDParameters {
configuration.setFailOnViolation(this.isFailOnViolation());
configuration.setAnalysisCacheLocation(this.cacheLocation);
configuration.setIgnoreIncrementalAnalysis(this.isIgnoreIncrementalAnalysis());
configuration.setProgressBar(this.isProgressBar());
LanguageVersion forceLangVersion = getForceLangVersion(registry);
if (forceLangVersion != null) {
@ -438,10 +434,6 @@ public class PMDParameters {
return failOnViolation;
}
public boolean isProgressBar() {
return !noProgressBar;
}
/**
* @return the uri alternative to source directory.

View File

@ -25,7 +25,7 @@ import net.sourceforge.pmd.util.IOUtil;
/**
* Listens to an analysis. This object produces new {@link FileAnalysisListener}
* for each analysed file, which themselves handle events like violations,
* for each analyzed file, which themselves handle events like violations,
* in their file. Thread-safety is required.
*
* <p>Listeners are the main API to obtain results of an analysis. The
@ -38,6 +38,16 @@ import net.sourceforge.pmd.util.IOUtil;
*/
public interface GlobalAnalysisListener extends AutoCloseable {
/**
* Notify the implementation that the analysis is about to start,
* and how many files are to be processed.
*
* @param totalFilesToAnalyze The total number of files to be analyzed.
*/
default void startAnalysis(int totalFilesToAnalyze) {
// Nothing to do
}
/**
* Returns a file listener that will handle events occurring during
* the analysis of the given file. The new listener may receive events
@ -115,6 +125,11 @@ public interface GlobalAnalysisListener extends AutoCloseable {
TeeListener(List<GlobalAnalysisListener> myList) {
this.myList = myList;
}
@Override
public void startAnalysis(int totalFilesToAnalyze) {
myList.forEach(it -> it.startAnalysis(totalFilesToAnalyze));
}
@Override
public FileAnalysisListener startFileAnalysis(TextFile file) {

View File

@ -120,7 +120,21 @@ public final class IOUtil {
return new OutputStreamWriter(new FilterOutputStream(System.out) {
@Override
public void close() {
// do nothing, avoid closing stdout
// avoid closing stdout, simply flush
try {
out.flush();
} catch (IOException ignored) {
// Nothing left to do
}
}
@Override
public void write(byte[] b, int off, int len) throws IOException {
/*
* FilterOutputStream iterates over each byte, asking subclasses to provide more efficient implementations
* It therefore negates any such optimizations that the underlying stream actually may implement.
*/
out.write(b, off, len);
}
}, charset);
}

View File

@ -138,7 +138,7 @@ class MultiThreadProcessorTest {
private static class SimpleReportListener implements GlobalAnalysisListener {
public AtomicInteger violations = new AtomicInteger(0);
@Override
public FileAnalysisListener startFileAnalysis(TextFile file) {
return new FileAnalysisListener() {

View File

@ -21,9 +21,6 @@ import net.sourceforge.pmd.internal.Slf4jSimpleConfiguration;
* @author Romain Pelisse &lt;belaran@gmail.com&gt;
*/
public class CLITest extends BaseCLITest {
// note that the progress bar sometimes messes up the log so it is
// disabled here in most tests.
// restoring system properties: -debug might change logging properties
// See Slf4jSimpleConfigurationForAnt and resetLogging
@Rule
@ -65,7 +62,7 @@ public class CLITest extends BaseCLITest {
@Test
public void changeJavaVersion() {
String[] args = { "-d", SOURCE_FOLDER, "-f", "text", "-R", RSET_NO_VIOLATION, "-version", "1.5", "-language", "java", "--debug", "--no-progress", };
String[] args = { "-d", SOURCE_FOLDER, "-f", "text", "-R", RSET_NO_VIOLATION, "-version", "1.5", "-language", "java", "--debug", };
String log = runTest(args);
assertThat(log, containsPattern("Adding file .*\\.java \\(lang: java 1\\.5\\)"));
}
@ -77,21 +74,21 @@ public class CLITest extends BaseCLITest {
@Test
public void exitStatusWithViolations() {
String[] args = { "-d", SOURCE_FOLDER, "-f", "text", "-R", RSET_WITH_VIOLATION, "--no-progress", };
String[] args = { "-d", SOURCE_FOLDER, "-f", "text", "-R", RSET_WITH_VIOLATION, };
String log = runTest(StatusCode.VIOLATIONS_FOUND, args);
assertThat(log, containsString("Violation from test-rset-1.xml"));
}
@Test
public void exitStatusWithViolationsAndWithoutFailOnViolations() {
String[] args = { "-d", SOURCE_FOLDER, "-f", "text", "-R", RSET_WITH_VIOLATION, "-failOnViolation", "false", "--no-progress", };
String[] args = { "-d", SOURCE_FOLDER, "-f", "text", "-R", RSET_WITH_VIOLATION, "-failOnViolation", "false", };
String log = runTest(StatusCode.OK, args);
assertThat(log, containsString("Violation from test-rset-1.xml"));
}
@Test
public void exitStatusWithViolationsAndWithoutFailOnViolationsLongOption() {
String[] args = { "-d", SOURCE_FOLDER, "-f", "text", "-R", RSET_WITH_VIOLATION, "--fail-on-violation", "false", "--no-progress", };
String[] args = { "-d", SOURCE_FOLDER, "-f", "text", "-R", RSET_WITH_VIOLATION, "--fail-on-violation", "false", };
String log = runTest(StatusCode.OK, args);
assertThat(log, containsString("Violation from test-rset-1.xml"));
}

View File

@ -24,7 +24,6 @@ public class CLITest extends BaseCLITest {
"xml",
"-R",
"rulesets/testing/js-rset1.xml",
"--no-progress",
"--debug");
assertThat(log, containsPattern("Adding file .*\\.js \\(lang: ecmascript ES6\\)"));
}

View File

@ -26,7 +26,6 @@ public class XmlCliTest extends BaseCLITest {
"-f",
"text",
"--no-cache",
"--no-progress",
"-R",
BASE_DIR + "/ruleset.xml",
"-d",

View File

@ -715,6 +715,11 @@
<artifactId>picocli</artifactId>
<version>4.7.0</version>
</dependency>
<dependency>
<groupId>me.tongfei</groupId>
<artifactId>progressbar</artifactId>
<version>0.9.5</version>
</dependency>
<dependency>
<groupId>org.ow2.asm</groupId>
<artifactId>asm</artifactId>