Fix cli tests

This commit is contained in:
Clément Fournier
2023-02-20 14:29:05 +01:00
parent 11e2a97c5f
commit 60f28c5c35
8 changed files with 107 additions and 48 deletions

View File

@@ -9,24 +9,27 @@ import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import net.sourceforge.pmd.AbstractConfiguration;
import net.sourceforge.pmd.cli.commands.mixins.internal.EncodingMixin;
import net.sourceforge.pmd.cli.internal.CliExitCode;
import net.sourceforge.pmd.cli.internal.PmdRootLogger;
import picocli.CommandLine.Mixin;
import picocli.CommandLine.Option;
import picocli.CommandLine.ParameterException;
import picocli.CommandLine.Parameters;
public abstract class AbstractAnalysisPmdSubcommand extends AbstractPmdSubcommand {
public abstract class AbstractAnalysisPmdSubcommand<C extends AbstractConfiguration> extends AbstractPmdSubcommand {
@Mixin
protected EncodingMixin encoding;
@Option(names = { "--dir", "-d" },
description = "Path to a source file, or directory containing source files to analyze. "
+ "Zip and Jar files are also supported, if they are specified directly "
+ "(archive files found while exploring a directory are not recursively expanded). "
+ "This option can be repeated, and multiple arguments can be provided to a single occurrence of the option. "
+ "One of --dir, --file-list or --uri must be provided.",
+ "Zip and Jar files are also supported, if they are specified directly "
+ "(archive files found while exploring a directory are not recursively expanded). "
+ "This option can be repeated, and multiple arguments can be provided to a single occurrence of the option. "
+ "One of --dir, --file-list or --uri must be provided.",
arity = "1..*", split = ",")
protected List<Path> inputPaths;
@@ -63,8 +66,22 @@ public abstract class AbstractAnalysisPmdSubcommand extends AbstractPmdSubcomman
if ((inputPaths == null || inputPaths.isEmpty()) && uri == null && fileListPath == null) {
throw new ParameterException(spec.commandLine(),
"Please provide a parameter for source root directory (--dir or -d), "
+ "database URI (--uri or -u), or file list path (--file-list)");
"Please provide a parameter for source root directory (--dir or -d), "
+ "database URI (--uri or -u), or file list path (--file-list)");
}
}
protected abstract C toConfiguration();
protected abstract CliExitCode doExecute(C conf);
@Override
protected CliExitCode execute() {
final C configuration = toConfiguration();
return PmdRootLogger.executeInLoggingContext(configuration,
this::doExecute);
}
}

View File

@@ -20,7 +20,6 @@ import net.sourceforge.pmd.cpd.CPDConfiguration;
import net.sourceforge.pmd.cpd.CpdAnalysis;
import net.sourceforge.pmd.cpd.Tokenizer;
import net.sourceforge.pmd.internal.LogMessages;
import net.sourceforge.pmd.internal.PmdRootLogger;
import net.sourceforge.pmd.lang.Language;
import net.sourceforge.pmd.util.StringUtil;
@@ -30,7 +29,7 @@ import picocli.CommandLine.ParameterException;
@Command(name = "cpd", showDefaultValues = true,
description = "Copy/Paste Detector - find duplicate code")
public class CpdCommand extends AbstractAnalysisPmdSubcommand {
public class CpdCommand extends AbstractAnalysisPmdSubcommand<CPDConfiguration> {
@Option(names = { "--language", "-l" }, description = "The source code language.%nValid values: ${COMPLETION-CANDIDATES}",
defaultValue = CPDConfiguration.DEFAULT_LANGUAGE, converter = CpdLanguageTypeSupport.class, completionCandidates = CpdLanguageTypeSupport.class)
@@ -112,7 +111,8 @@ public class CpdCommand extends AbstractAnalysisPmdSubcommand {
*
* @throws ParameterException if the parameters are inconsistent or incomplete
*/
public CPDConfiguration toConfiguration() {
@Override
protected CPDConfiguration toConfiguration() {
final CPDConfiguration configuration = new CPDConfiguration();
configuration.setDebug(debug);
configuration.setExcludes(excludes);
@@ -142,13 +142,8 @@ public class CpdCommand extends AbstractAnalysisPmdSubcommand {
}
@Override
protected CliExitCode execute() {
final CPDConfiguration configuration = toConfiguration();
return PmdRootLogger.executeInLoggingContext(configuration, CpdCommand::doExecute);
}
private static @NonNull CliExitCode doExecute(CPDConfiguration configuration) {
@NonNull
protected CliExitCode doExecute(CPDConfiguration configuration) {
try (CpdAnalysis cpd = CpdAnalysis.create(configuration)) {
MutableBoolean hasViolations = new MutableBoolean();

View File

@@ -31,7 +31,6 @@ import net.sourceforge.pmd.cli.commands.typesupport.internal.PmdLanguageVersionT
import net.sourceforge.pmd.cli.internal.CliExitCode;
import net.sourceforge.pmd.cli.internal.ProgressBarListener;
import net.sourceforge.pmd.internal.LogMessages;
import net.sourceforge.pmd.internal.PmdRootLogger;
import net.sourceforge.pmd.lang.Language;
import net.sourceforge.pmd.lang.LanguageVersion;
import net.sourceforge.pmd.properties.PropertyDescriptor;
@@ -48,7 +47,7 @@ import picocli.CommandLine.ParameterException;
@Command(name = "check", showDefaultValues = true,
description = "The PMD standard source code analyzer")
public class PmdCommand extends AbstractAnalysisPmdSubcommand {
public class PmdCommand extends AbstractAnalysisPmdSubcommand<PMDConfiguration> {
private static final Logger LOG = LoggerFactory.getLogger(PmdCommand.class);
static {
@@ -275,7 +274,8 @@ public class PmdCommand extends AbstractAnalysisPmdSubcommand {
*
* @throws ParameterException if the parameters are inconsistent or incomplete
*/
public PMDConfiguration toConfiguration() {
@Override
protected PMDConfiguration toConfiguration() {
final PMDConfiguration configuration = new PMDConfiguration();
configuration.setInputPathList(inputPaths);
configuration.setInputFilePath(fileListPath);
@@ -322,12 +322,8 @@ public class PmdCommand extends AbstractAnalysisPmdSubcommand {
}
@Override
protected CliExitCode execute() {
final PMDConfiguration configuration = toConfiguration();
return PmdRootLogger.executeInLoggingContext(configuration, this::doExecute);
}
private @NonNull CliExitCode doExecute(PMDConfiguration configuration) {
@NonNull
protected CliExitCode doExecute(PMDConfiguration configuration) {
if (benchmark) {
TimeTracker.startGlobalTracking();
}

View File

@@ -2,7 +2,7 @@
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.internal;
package net.sourceforge.pmd.cli.internal;
import java.util.function.Function;
@@ -12,17 +12,22 @@ import org.slf4j.LoggerFactory;
import org.slf4j.event.Level;
import net.sourceforge.pmd.AbstractConfiguration;
import net.sourceforge.pmd.internal.Slf4jSimpleConfiguration;
import net.sourceforge.pmd.util.log.MessageReporter;
import net.sourceforge.pmd.util.log.internal.SimpleMessageReporter;
/**
* Interacts with slf4j-simple to reconfigure logging levels based on
* the debug flag.
*
* @author Clément Fournier
*/
public final class PmdRootLogger {
private static final String PMD_PACKAGE = "net.sourceforge.pmd";
private static final String PMD_CLI_LOGGER = "net.sourceforge.pmd.cli";
// not final, in order to re-initialize logging
public static Logger log = LoggerFactory.getLogger(PMD_PACKAGE);
// This logger is used as backend for the MessageReporter currently.
private static Logger log = LoggerFactory.getLogger(PMD_CLI_LOGGER);
private PmdRootLogger() {
// utility class
@@ -37,7 +42,7 @@ public final class PmdRootLogger {
if (conf.isDebug()) {
Slf4jSimpleConfiguration.reconfigureDefaultLogLevel(Level.TRACE);
// need to reload the logger with the new configuration
log = LoggerFactory.getLogger(PMD_PACKAGE);
log = LoggerFactory.getLogger(PMD_CLI_LOGGER);
resetLogLevel = true;
}
@@ -48,7 +53,7 @@ public final class PmdRootLogger {
if (resetLogLevel) {
// reset to the previous value
Slf4jSimpleConfiguration.reconfigureDefaultLogLevel(curLogLevel);
log = LoggerFactory.getLogger(PMD_PACKAGE);
log = LoggerFactory.getLogger(PMD_CLI_LOGGER);
}
}
}

View File

@@ -81,13 +81,13 @@ class CpdCliTest extends BaseCliTest {
@Test
void debugLogging() throws Exception {
CliExecutionResult result = runCliSuccessfully("--debug", "--minimum-tokens", "340", "--dir", SRC_DIR);
result.checkStdErr(containsString("[main] INFO net.sourceforge.pmd - Log level is at TRACE"));
result.checkStdErr(containsString("[main] INFO net.sourceforge.pmd.cli - Log level is at TRACE"));
}
@Test
void defaultLogging() throws Exception {
CliExecutionResult result = runCliSuccessfully("--minimum-tokens", "340", "--dir", SRC_DIR);
result.checkStdErr(containsString("[main] INFO net.sourceforge.pmd - Log level is at INFO"));
result.checkStdErr(containsString("[main] INFO net.sourceforge.pmd.cli - Log level is at INFO"));
}
@Test

View File

@@ -187,7 +187,7 @@ class PmdCliTest extends BaseCliTest {
SystemLambda.restoreSystemProperties(() -> {
// change working directory
System.setProperty("user.dir", srcDir.toString());
runCliSuccessfully("--dir", ".", "--rulesets", DUMMY_RULESET_WITH_VIOLATIONS)
runCli(VIOLATIONS_FOUND, "--dir", ".", "--rulesets", DUMMY_RULESET_WITH_VIOLATIONS)
.verify(res -> res.checkStdOut(containsString("./src/test/resources/net/sourceforge/pmd/cli/src/anotherfile.dummy")));
});
@@ -197,13 +197,13 @@ class PmdCliTest extends BaseCliTest {
@Test
void debugLogging() throws Exception {
CliExecutionResult result = runCliSuccessfully("--debug", "--dir", srcDir.toString(), "--rulesets", RULESET_NO_VIOLATIONS);
result.checkStdErr(containsString("[main] INFO net.sourceforge.pmd.cli.commands.internal.AbstractPmdSubcommand - Log level is at TRACE"));
result.checkStdErr(containsString("[main] INFO net.sourceforge.pmd.cli - Log level is at TRACE"));
}
@Test
void defaultLogging() throws Exception {
CliExecutionResult result = runCliSuccessfully("--dir", srcDir.toString(), "--rulesets", RULESET_NO_VIOLATIONS);
result.checkStdErr(containsString("[main] INFO net.sourceforge.pmd.cli.commands.internal.AbstractPmdSubcommand - Log level is at INFO"));
result.checkStdErr(containsString("[main] INFO net.sourceforge.pmd.cli - Log level is at INFO"));
result.checkStdErr(not(containsPattern("Adding file .*"))); // not in debug mode
}
@@ -382,16 +382,16 @@ class PmdCliTest extends BaseCliTest {
// therefore we use the current directory and make sure, we are at the correct place - in pmd-core
Path cwd = Paths.get(".").toRealPath();
assertThat(cwd.toString(), endsWith("pmd-cli"));
String relativeSrcDir = IOUtil.normalizePath("src/test/resources/net/sourceforge/pmd/cli/src");
String relativeSrcDir = "src/test/resources/net/sourceforge/pmd/cli/src";
assertTrue(Files.isDirectory(cwd.resolve(relativeSrcDir)));
// use the parent directory
String relativeSrcDirWithParent = relativeSrcDir + File.separator + "..";
Path relativeSrcDirWithParent = Paths.get(relativeSrcDir, "..");
runCli(CliExitCode.VIOLATIONS_FOUND, "--dir", relativeSrcDirWithParent, "--rulesets",
runCli(CliExitCode.VIOLATIONS_FOUND, "--dir", relativeSrcDirWithParent.toString(), "--rulesets",
DUMMY_RULESET_WITH_VIOLATIONS)
.verify(result -> result.checkStdOut(
containsString("\n" + relativeSrcDirWithParent + IOUtil.normalizePath("/src/somefile.dummy"))));
containsString("\n" + relativeSrcDirWithParent + "/src/somefile.dummy")));
}
@Test

View File

@@ -47,7 +47,7 @@ public abstract class AbstractConfiguration {
private Path inputFilePath;
private Path ignoreFilePath;
private List<Path> excludes = new ArrayList<>();
private boolean collectRecursive;
private boolean collectRecursive = true;
protected AbstractConfiguration(LanguageRegistry languageRegistry, MessageReporter messageReporter) {

View File

@@ -17,6 +17,10 @@ import java.util.Objects;
import java.util.stream.Collectors;
import org.apache.commons.lang3.exception.ExceptionUtils;
import org.checkerframework.checker.nullness.qual.NonNull;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.slf4j.event.Level;
import net.sourceforge.pmd.Report.GlobalReportBuilderListener;
import net.sourceforge.pmd.benchmark.TextTimingReportRenderer;
@@ -26,12 +30,13 @@ import net.sourceforge.pmd.benchmark.TimingReportRenderer;
import net.sourceforge.pmd.cli.PMDCommandLineInterface;
import net.sourceforge.pmd.cli.PmdParametersParseResult;
import net.sourceforge.pmd.internal.LogMessages;
import net.sourceforge.pmd.internal.PmdRootLogger;
import net.sourceforge.pmd.internal.Slf4jSimpleConfiguration;
import net.sourceforge.pmd.lang.document.TextFile;
import net.sourceforge.pmd.renderers.Renderer;
import net.sourceforge.pmd.reporting.ReportStats;
import net.sourceforge.pmd.util.datasource.DataSource;
import net.sourceforge.pmd.util.log.MessageReporter;
import net.sourceforge.pmd.util.log.internal.SimpleMessageReporter;
/**
* Entry point for PMD's CLI. Use {@link #runPmd(PMDConfiguration)}
@@ -47,6 +52,10 @@ import net.sourceforge.pmd.util.log.MessageReporter;
@Deprecated
public final class PMD {
private static final String PMD_PACKAGE = "net.sourceforge.pmd";
// not final, in order to re-initialize logging
private static Logger log = LoggerFactory.getLogger(PMD_PACKAGE);
/**
* The line delimiter used by PMD in outputs. Usually the platform specific
* line separator.
@@ -131,8 +140,8 @@ public final class PMD {
// todo these warnings/errors should be output on a PmdRenderer
if (!parseResult.getDeprecatedOptionsUsed().isEmpty()) {
Entry<String, String> first = parseResult.getDeprecatedOptionsUsed().entrySet().iterator().next();
PmdRootLogger.log.warn("Some deprecated options were used on the command-line, including {}", first.getKey());
PmdRootLogger.log.warn("Consider replacing it with {}", first.getValue());
log.warn("Some deprecated options were used on the command-line, including {}", first.getKey());
log.warn("Consider replacing it with {}", first.getValue());
}
if (parseResult.isVersion()) {
@@ -155,12 +164,34 @@ public final class PMD {
);
} catch (IllegalArgumentException e) {
System.err.println("Cannot start analysis: " + e);
PmdRootLogger.log.debug(ExceptionUtils.getStackTrace(e));
log.debug(ExceptionUtils.getStackTrace(e));
return StatusCode.ERROR;
}
return PmdRootLogger.executeInLoggingContext(configuration, PMD::runPmd);
Level curLogLevel = Slf4jSimpleConfiguration.getDefaultLogLevel();
boolean resetLogLevel = false;
try {
// only reconfigure logging, if debug flag was used on command line
// otherwise just use whatever is in conf/simplelogger.properties which happens automatically
if (configuration.isDebug()) {
Slf4jSimpleConfiguration.reconfigureDefaultLogLevel(Level.TRACE);
// need to reload the logger with the new configuration
log = LoggerFactory.getLogger(PMD_PACKAGE);
resetLogLevel = true;
}
MessageReporter pmdReporter = setupMessageReporter();
configuration.setReporter(pmdReporter);
return runPmd(configuration);
} finally {
if (resetLogLevel) {
// reset to the previous value
Slf4jSimpleConfiguration.reconfigureDefaultLogLevel(curLogLevel);
log = LoggerFactory.getLogger(PMD_PACKAGE);
}
}
}
/**
@@ -189,7 +220,7 @@ public final class PMD {
return StatusCode.ERROR;
}
try {
PmdRootLogger.log.debug("Current classpath:\n{}", System.getProperty("java.class.path"));
log.debug("Current classpath:\n{}", System.getProperty("java.class.path"));
ReportStats stats = pmd.runAndReturnStats();
if (pmdReporter.numErrors() > 0) {
// processing errors are ignored
@@ -212,6 +243,21 @@ public final class PMD {
}
}
private static @NonNull MessageReporter setupMessageReporter() {
// create a top-level reporter
// TODO CLI errors should also be reported through this
// TODO this should not use the logger as backend, otherwise without
// slf4j implementation binding, errors are entirely ignored.
MessageReporter pmdReporter = new SimpleMessageReporter(log);
// always install java.util.logging to slf4j bridge
Slf4jSimpleConfiguration.installJulBridge();
// logging, mostly for testing purposes
Level defaultLogLevel = Slf4jSimpleConfiguration.getDefaultLogLevel();
log.info("Log level is at {}", defaultLogLevel);
return pmdReporter;
}
private static void finishBenchmarker(PMDConfiguration configuration) {
if (configuration.isBenchmark()) {
final TimingReport timingReport = TimeTracker.stopGlobalTracking();