Add tests, dedup collected files

This commit is contained in:
Clément Fournier committed 2022-11-13 14:36:36 +01:00
1 parent d3e68d795b
commit 99b24b702f
11 files changed
+214 -47

No files matched your search

@@ -22,6 +22,8 @@ import java.util.logging.ConsoleHandler;
import java.util.logging.Level;
import java.util.logging.Logger;
import org.apache.commons.lang3.exception.ExceptionUtils;
import net.sourceforge.pmd.annotation.InternalApi;
import net.sourceforge.pmd.benchmark.TextTimingReportRenderer;
import net.sourceforge.pmd.benchmark.TimeTracker;
@@ -472,7 +474,16 @@ public class PMD {
System.err.println(CliMessages.runWithHelpFlagMessage());
return StatusCode.ERROR;
}
return runPmd(parseResult.toConfiguration());
PMDConfiguration conf;
try {
conf = parseResult.toConfiguration();
return runPmd(conf);
} catch (IllegalArgumentException e) {
System.err.println("Cannot start analysis: " + e);
LOG.fine(ExceptionUtils.getStackTrace(e));
return StatusCode.ERROR;
}
}
private static void printErrorDetected(int errors) {
@@ -949,14 +949,13 @@ public class PMDConfiguration extends AbstractConfiguration {
* to a directory and not a file. See {@link #getRelativizeRoots()}
* for the interpretation.
*
* <p>Setting to null is not recommended as it is only used for
* compatibility with the older {@link #isReportShortNames()} functionality.
* It will possibly be disallowed with PMD 7. The default value is
* null.
* <p>If several paths are added, the shortest paths possible are
* built.
*
* @param path A path
*
* @throws IllegalArgumentException If the path points to a file
* @throws IllegalArgumentException If the path points to a file, and not a directory
* @throws NullPointerException If the path is null
*/
public void addRelativizeRoot(Path path) {
// TODO symlinks?
@@ -967,12 +966,28 @@ public class PMDConfiguration extends AbstractConfiguration {
}
}
/**
* Returns the path used to shorten paths output in the report.
* Add several paths to shorten paths that are output in the report.
* See {@link #addRelativizeRoot(Path)}.
*
* @param paths A list of non-null paths
*
* @throws IllegalArgumentException If any path points to a file, and not a directory
* @throws NullPointerException If the list, or any path in the list is null
*/
public void addRelativizeRoots(List<Path> paths) {
for (Path path : paths) {
addRelativizeRoot(path);
}
}
/**
* Returns the paths used to shorten paths output in the report.
* <ul>
* <li>If the path is {@code /} (root), then paths are rendered as absolute.
* <li>If the path is null, then paths are not touched (unless {@link #isReportShortNames()} is true)
* <li>Otherwise, the path is a directory.
* <li>If the list is empty, then paths are not touched (unless {@link #isReportShortNames()} is true)
* <li>If the list is non-empty, then source file paths are relativized with all the items in the list.
* The shortest of these relative paths is taken as the display name of the file.
* </ul>
*/
public List<Path> getRelativizeRoots() {
@@ -47,9 +47,7 @@ public class PMDTaskImpl {
public PMDTaskImpl(PMDTask task) {
configuration.setReportShortNames(task.isShortFilenames());
for (java.nio.file.Path path : task.getRelativizeRoots()) {
configuration.addRelativizeRoot(path);
}
configuration.addRelativizeRoots(task.getRelativizeRoots());
configuration.setSuppressMarker(task.getSuppressMarker());
this.failOnError = task.isFailOnError();
this.failOnRuleViolation = task.isFailOnRuleViolation();
@@ -4,6 +4,8 @@
package net.sourceforge.pmd.cli;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.List;
@@ -123,12 +125,17 @@ public class PMDParameters {
+ "If this option is not specified, the report is rendered to standard output.")
private String reportfile = null;
@Parameter(names = { RELATIVIZE_PATHS_WITH },
arity = 1,
@Parameter(names = { RELATIVIZE_PATHS_WITH, "-z" },
variableArity = true,
description = "Path relative to which directories are rendered in the report."
+ "This option can be used to render shorter paths. "
+ "This option replaces --short-names since PMD 6.52.0.")
private String relativizePathRoot = null;
+ "This option allows shortening directories in the report; "
+ "without it, paths are rendered as absolute paths. "
+ "The option can be repeated, in which case the shortest relative path."
+ "If / is mentioned (root path), then the paths will be rendered as absolute."
+ "This option replaces --short-names since PMD 6.52.0.",
validateValueWith = PathToRelativizeRootValidator.class,
converter = StringToPathConverter.class)
private List<Path> relativizePathRoot = new ArrayList<>();
@Parameter(names = { "-version", "-v" }, description = "Specify version of a language PMD should use.")
private String version = null;
@@ -227,6 +234,27 @@ public class PMDParameters {
}
}
public static class PathToRelativizeRootValidator implements IValueValidator<List<Path>> {
@Override
public void validate(String name, List<Path> value) throws ParameterException {
for (Path p : value) {
if (Files.isRegularFile(p)) {
throw new ParameterException("Expected a directory path for option " + name + ", found a file: " + p);
}
}
}
}
public static class StringToPathConverter implements IStringConverter<Path> {
@Override
public Path convert(String value) {
return Paths.get(value);
}
}
/**
* Converts these parameters into a configuration.
@@ -244,8 +272,8 @@ public class PMDParameters {
configuration.setReportFormat(this.getFormat());
configuration.setBenchmark(this.isBenchmark());
configuration.setDebug(this.isDebug());
if (relativizePathRoot != null) {
configuration.addRelativizeRoot(Paths.get(this.relativizePathRoot));
for (Path path: relativizePathRoot) {
configuration.addRelativizeRoot(path);
}
configuration.setMinimumPriority(this.getMinimumPriority());
configuration.setReportFile(this.getReportfile());
@@ -110,7 +110,6 @@ public final class FileCollectionUtil {
public static void collectFiles(FileCollector collector, List<String> fileLocations) {
for (String rootLocation : fileLocations) {
try {
// no relativizeWith call
addRoot(collector, Paths.get(rootLocation));
} catch (IOException e) {
collector.getReporter().errorEx("Error collecting " + rootLocation, e);
@@ -24,6 +24,7 @@ import java.util.Collections;
import java.util.Comparator;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Objects;
import java.util.Set;
@@ -46,7 +47,7 @@ import net.sourceforge.pmd.util.log.MessageReporter;
@SuppressWarnings("PMD.CloseResource")
public final class FileCollector implements AutoCloseable {
private final List<TextFile> allFilesToProcess = new ArrayList<>();
private final Set<TextFile> allFilesToProcess = new LinkedHashSet<>();
private final List<Closeable> resourcesToClose = new ArrayList<>();
private Charset charset = StandardCharsets.UTF_8;
private final LanguageVersionDiscoverer discoverer;
@@ -84,6 +85,7 @@ public final class FileCollector implements AutoCloseable {
if (closed) {
throw new IllegalStateException("Collector was closed!");
}
List<TextFile> allFilesToProcess = new ArrayList<>(this.allFilesToProcess);
Collections.sort(allFilesToProcess, new Comparator<TextFile>() {
@Override
public int compare(TextFile o1, TextFile o2) {
@@ -135,8 +137,7 @@ public final class FileCollector implements AutoCloseable {
}
LanguageVersion languageVersion = discoverLanguage(file.toString());
if (languageVersion != null) {
addFileImpl(new NioTextFile(file, charset, languageVersion, getDisplayName(file)));
return true;
return addFileImpl(new NioTextFile(file, charset, languageVersion, getDisplayName(file)));
}
return false;
}
@@ -158,8 +159,7 @@ public final class FileCollector implements AutoCloseable {
return false;
}
NioTextFile nioTextFile = new NioTextFile(file, charset, discoverer.getDefaultLanguageVersion(language), getDisplayName(file));
addFileImpl(nioTextFile);
return true;
return addFileImpl(nioTextFile);
}
/**
@@ -172,8 +172,7 @@ public final class FileCollector implements AutoCloseable {
public boolean addFile(TextFile textFile) {
AssertionUtil.requireParamNotNull("textFile", textFile);
if (checkContextualVersion(textFile)) {
addFileImpl(textFile);
return true;
return addFileImpl(textFile);
}
return false;
}
@@ -190,16 +189,19 @@ public final class FileCollector implements AutoCloseable {
LanguageVersion version = discoverLanguage(pathId);
if (version != null) {
addFileImpl(new StringTextFile(sourceContents, pathId, pathId, version));
return true;
return addFileImpl(new StringTextFile(sourceContents, pathId, pathId, version));
}
return false;
}
private void addFileImpl(TextFile textFile) {
private boolean addFileImpl(TextFile textFile) {
reporter.trace("Adding file {0} (lang: {1}) ", textFile.getPathId(), textFile.getLanguageVersion().getTerseName());
allFilesToProcess.add(textFile);
if (allFilesToProcess.add(textFile)) {
return true;
}
reporter.trace("File was already collected, skipping");
return false;
}
private LanguageVersion discoverLanguage(String file) {
@@ -411,7 +413,7 @@ public final class FileCollector implements AutoCloseable {
*/
public void exclude(FileCollector excludeCollector) {
HashSet<TextFile> toExclude = new HashSet<>(excludeCollector.allFilesToProcess);
for (Iterator<TextFile> iterator = allFilesToProcess.iterator(); iterator.hasNext(); ) {
for (Iterator<TextFile> iterator = allFilesToProcess.iterator(); iterator.hasNext();) {
TextFile file = iterator.next();
if (toExclude.contains(file)) {
reporter.trace("Excluding file {0}", file.getPathId());
@@ -22,6 +22,7 @@ import java.nio.charset.StandardCharsets;
import java.nio.file.FileSystems;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;
import java.util.logging.Logger;
import org.hamcrest.Matcher;
@@ -36,7 +37,10 @@ import org.junit.rules.TemporaryFolder;
import net.sourceforge.pmd.PMD;
import net.sourceforge.pmd.PMD.StatusCode;
import net.sourceforge.pmd.RuleContext;
import net.sourceforge.pmd.junit.JavaUtilLoggingRule;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.rule.MockRule;
import net.sourceforge.pmd.util.IOUtil;
/**
@@ -45,6 +49,7 @@ import net.sourceforge.pmd.util.IOUtil;
public class CoreCliTest {
private static final String DUMMY_RULESET = "net/sourceforge/pmd/cli/FakeRuleset.xml";
private static final String DUMMY_RULESET_WITH_VIOLATIONS = "net/sourceforge/pmd/cli/FakeRuleset2.xml";
private static final String STRING_TO_REPLACE = "__should_be_replaced__";
@Rule
@@ -127,6 +132,42 @@ public class CoreCliTest {
assertTrue("Report file should have been created", Files.exists(reportFile));
}
@Test
public void testNoRelativizeWith() {
startCapturingErrAndOut();
runPmd(StatusCode.VIOLATIONS_FOUND, "--no-cache", "--dir", srcDir, "--rulesets", DUMMY_RULESET_WITH_VIOLATIONS);
assertThat(outStreamCaptor.getLog(), containsString(srcDir.resolve("someSource.dummy").toString()));
}
@Test
public void testRelativizeWith() {
startCapturingErrAndOut();
runPmd(StatusCode.VIOLATIONS_FOUND, "--no-cache", "--dir", srcDir, "--rulesets", DUMMY_RULESET_WITH_VIOLATIONS, "-z", srcDir.getParent());
assertThat(outStreamCaptor.getLog(), not(containsString(srcDir.resolve("someSource.dummy").toString())));
assertThat(outStreamCaptor.getLog(), containsString("src/someSource.dummy"));
}
@Test
public void testRelativizeWithMultiple() {
startCapturingErrAndOut();
runPmd(StatusCode.VIOLATIONS_FOUND, "--no-cache", "--dir", srcDir, "--rulesets", DUMMY_RULESET_WITH_VIOLATIONS, "-z", srcDir.getParent(), srcDir);
assertThat(outStreamCaptor.getLog(), not(containsString(srcDir.resolve("someSource.dummy").toString())));
assertThat(outStreamCaptor.getLog(), containsString("someSource.dummy"));
}
@Test
public void testRelativizeWithFileIsError() {
startCapturingErrAndOut();
runPmd(StatusCode.ERROR, "--no-cache", "--dir", srcDir, "--rulesets", DUMMY_RULESET_WITH_VIOLATIONS, "-z", srcDir.resolve("someSource.dummy"));
assertThat(errStreamCaptor.getLog(), containsString(
"Expected a directory path for option --relativize-paths-with, found a file: "
+ srcDir.resolve("someSource.dummy")));
}
@Test
public void testFileCollectionWithUnknownFiles() throws IOException {
Path reportFile = tempRoot().resolve("out/reportFile.txt");
@@ -283,5 +324,15 @@ public class CoreCliTest {
assertEquals("Exit code", expectedExitCode, actualExitCode);
}
public static class FooRule extends MockRule {
@Override
public void apply(List<? extends Node> nodes, RuleContext ctx) {
for (Node node : nodes) {
ctx.addViolation(node);
}
}
}
}
@@ -6,7 +6,11 @@ package net.sourceforge.pmd.cli;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.endsWith;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasSize;
import java.nio.file.Paths;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashSet;
@@ -18,16 +22,18 @@ import org.junit.Test;
import net.sourceforge.pmd.PMD;
import net.sourceforge.pmd.PMDConfiguration;
import net.sourceforge.pmd.PmdAnalysis;
import net.sourceforge.pmd.lang.DummyLanguageModule;
import net.sourceforge.pmd.lang.Language;
import net.sourceforge.pmd.lang.document.TextFile;
import net.sourceforge.pmd.util.datasource.DataSource;
public class PMDFilelistTest {
private final Set<Language> languages = new HashSet<Language>(Arrays.asList(new DummyLanguageModule()));
@Test
public void testGetApplicableFiles() {
Set<Language> languages = new HashSet<>();
languages.add(new DummyLanguageModule());
PMDConfiguration configuration = new PMDConfiguration();
configuration.setInputFilePath("src/test/resources/net/sourceforge/pmd/cli/filelist.txt");
@@ -39,24 +45,59 @@ public class PMDFilelistTest {
@Test
public void testGetApplicableFilesMultipleLines() {
Set<Language> languages = new HashSet<>();
languages.add(new DummyLanguageModule());
PMDConfiguration configuration = new PMDConfiguration();
configuration.setInputFilePath("src/test/resources/net/sourceforge/pmd/cli/filelist2.txt");
List<DataSource> applicableFiles = PMD.getApplicableFiles(configuration, languages);
Assert.assertEquals(3, applicableFiles.size());
Assert.assertEquals(2, applicableFiles.size());
assertThat(applicableFiles.get(0).getNiceFileName(false, ""), endsWith("anotherfile.dummy"));
assertThat(applicableFiles.get(1).getNiceFileName(false, ""), endsWith("somefile.dummy"));
assertThat(applicableFiles.get(2).getNiceFileName(false, ""), endsWith("somefile.dummy"));
}
@Test
public void testRelativizeWith() {
PMDConfiguration conf = new PMDConfiguration();
conf.setInputFilePath(Paths.get("src/test/resources/net/sourceforge/pmd/cli/filelist2.txt"));
conf.addRelativizeRoot(Paths.get("src/test/resources"));
try (PmdAnalysis pmd = PmdAnalysis.create(conf)) {
List<TextFile> files = pmd.files().getCollectedFiles();
assertThat(files, hasSize(2));
assertThat(files.get(0).getDisplayName(), equalTo("net/sourceforge/pmd/cli/src/anotherfile.dummy"));
assertThat(files.get(1).getDisplayName(), equalTo("net/sourceforge/pmd/cli/src/somefile.dummy"));
}
}
@Test
public void testRelativizeWithOtherDir() {
PMDConfiguration conf = new PMDConfiguration();
conf.setInputFilePath(Paths.get("src/test/resources/net/sourceforge/pmd/cli/filelist4.txt"));
conf.addRelativizeRoot(Paths.get("src/test/resources/net/sourceforge/pmd/cli/src"));
try (PmdAnalysis pmd = PmdAnalysis.create(conf)) {
List<TextFile> files = pmd.files().getCollectedFiles();
assertThat(files, hasSize(3));
assertThat(files.get(0).getDisplayName(), equalTo("../otherSrc/somefile.dummy"));
assertThat(files.get(1).getDisplayName(), equalTo("anotherfile.dummy"));
assertThat(files.get(2).getDisplayName(), equalTo("somefile.dummy"));
}
}
@Test
public void testRelativizeWithSeveralDirs() {
PMDConfiguration conf = new PMDConfiguration();
conf.setInputFilePath(Paths.get("src/test/resources/net/sourceforge/pmd/cli/filelist4.txt"));
conf.addRelativizeRoot(Paths.get("src/test/resources/net/sourceforge/pmd/cli/src"));
conf.addRelativizeRoot(Paths.get("src/test/resources/net/sourceforge/pmd/cli/otherSrc"));
try (PmdAnalysis pmd = PmdAnalysis.create(conf)) {
List<TextFile> files = pmd.files().getCollectedFiles();
assertThat(files, hasSize(3));
assertThat(files.get(0).getDisplayName(), equalTo("somefile.dummy"));
assertThat(files.get(1).getDisplayName(), equalTo("anotherfile.dummy"));
assertThat(files.get(2).getDisplayName(), equalTo("somefile.dummy"));
}
}
@Test
public void testGetApplicatbleFilesWithIgnores() {
Set<Language> languages = new HashSet<>();
languages.add(new DummyLanguageModule());
PMDConfiguration configuration = new PMDConfiguration();
configuration.setInputFilePath("src/test/resources/net/sourceforge/pmd/cli/filelist3.txt");
configuration.setIgnoreFilePath("src/test/resources/net/sourceforge/pmd/cli/ignorelist.txt");
@@ -69,9 +110,6 @@ public class PMDFilelistTest {
@Test
public void testGetApplicatbleFilesWithDirAndIgnores() {
Set<Language> languages = new HashSet<>();
languages.add(new DummyLanguageModule());
PMDConfiguration configuration = new PMDConfiguration();
configuration.setInputPaths("src/test/resources/net/sourceforge/pmd/cli/src");
configuration.setIgnoreFilePath("src/test/resources/net/sourceforge/pmd/cli/ignorelist.txt");
@@ -0,0 +1,21 @@
<?xml version="1.0"?>
<ruleset name="Test Ruleset" xmlns="http://pmd.sourceforge.net/ruleset/2.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 https://pmd.sourceforge.io/ruleset_2_0_0.xsd">
<description>
Ruleset used by test RuleSetFactoryTest
</description>
<rule name="Ruleset3Rule1" language="dummy" since="1.0" message="Test Rule 1" class="net.sourceforge.pmd.cli.CoreCliTest$FooRule"
externalInfoUrl="${pmd.website.baseurl}/rules/test/TestRuleset3.xml#Ruleset3Rule1">
<description>
Just for test
</description>
<priority>3</priority>
<example>
<![CDATA[
]]>
</example>
</rule>
</ruleset>
@@ -0,0 +1,4 @@
src/test/resources/net/sourceforge/pmd/cli/src/somefile.dummy,
src/test/resources/net/sourceforge/pmd/cli/otherSrc/somefile.dummy,
src/test/resources/net/sourceforge/pmd/cli/src/anotherfile.dummy
src/test/resources/net/sourceforge/pmd/cli/src/somefile.dummy