diff --git a/build.gradle b/build.gradle index caf31db2..4cfcba26 100644 --- a/build.gradle +++ b/build.gradle @@ -84,7 +84,7 @@ dependencies { } // PMD dependencies - implementation('net.sourceforge.pmd:pmd-java:6.0.0') { + implementation('net.sourceforge.pmd:pmd-java:7.8.0') { exclude group: 'jaxen' exclude group: 'xerces' exclude group: 'junit' @@ -102,8 +102,9 @@ dependencies { } // SpotBugs dependencies - implementation('com.github.spotbugs:spotbugs:4.2.0') { + implementation('com.github.spotbugs:spotbugs:4.8.6') { exclude group: 'org.slf4j' + exclude group: 'ch.qos.logback' } // Scalastyle http://www.scalastyle.org/ diff --git a/src/main/java/pl/touk/sputnik/processor/pmd/CollectorRenderer.java b/src/main/java/pl/touk/sputnik/processor/pmd/CollectorRenderer.java deleted file mode 100644 index 97a1a660..00000000 --- a/src/main/java/pl/touk/sputnik/processor/pmd/CollectorRenderer.java +++ /dev/null @@ -1,102 +0,0 @@ -package pl.touk.sputnik.processor.pmd; - -import lombok.Getter; -import lombok.extern.slf4j.Slf4j; -import net.sourceforge.pmd.Report; -import net.sourceforge.pmd.RulePriority; -import net.sourceforge.pmd.RuleViolation; -import net.sourceforge.pmd.renderers.AbstractRenderer; -import net.sourceforge.pmd.util.datasource.DataSource; -import org.apache.commons.lang3.StringUtils; -import org.jetbrains.annotations.NotNull; -import pl.touk.sputnik.configuration.Configuration; -import pl.touk.sputnik.configuration.ConfigurationBuilder; -import pl.touk.sputnik.configuration.GeneralOption; -import pl.touk.sputnik.review.ReviewResult; -import pl.touk.sputnik.review.Severity; -import pl.touk.sputnik.review.Violation; - -import java.io.IOException; -import java.util.Properties; - -@Slf4j -public class CollectorRenderer extends AbstractRenderer { - private static final String SPUTNIK_PMD_COLLECT_RENDERER = "Sputnik PMD Collect Renderer"; - private static final char LINE_SEPARATOR = '\n'; - private final Configuration configuration; - - @Getter - private final ReviewResult reviewResult = new ReviewResult(); - - public CollectorRenderer(Properties properties) { - this(ConfigurationBuilder.initFromProperties(properties)); - } - - public CollectorRenderer(Configuration configuration) { - super(SPUTNIK_PMD_COLLECT_RENDERER, SPUTNIK_PMD_COLLECT_RENDERER); - this.configuration = configuration; - } - - @Override - public String defaultFileExtension() { - return null; - } - - @Override - public void startFileAnalysis(DataSource dataSource) { - log.debug("PMD audit started for {}", dataSource); - } - - @Override - public void renderFileReport(Report report) throws IOException { - boolean showDetails = Boolean.valueOf(configuration.getProperty(GeneralOption.PMD_SHOW_VIOLATION_DETAILS)); - - for (RuleViolation ruleViolation : report) { - String violationDescription = showDetails ? renderViolationDetails(ruleViolation) :ruleViolation.getDescription(); - reviewResult.add(new Violation(ruleViolation.getFilename(), ruleViolation.getBeginLine(), violationDescription, convert(ruleViolation.getRule().getPriority()))); - } - } - - private String renderViolationDetails(RuleViolation ruleViolation) { - StringBuilder fullDescription = new StringBuilder(ruleViolation.getDescription()); - - String reason = ruleViolation.getRule().getDescription(); - if (StringUtils.isNotEmpty(reason)) { - fullDescription.append(LINE_SEPARATOR).append(reason); - } - String url = ruleViolation.getRule().getExternalInfoUrl(); - if (StringUtils.isNotEmpty(url)) { - fullDescription.append(LINE_SEPARATOR).append(url); - } - - return fullDescription.toString(); - } - - @Override - public void start() throws IOException { - log.info("PMD audit started"); - } - - @Override - public void end() throws IOException { - log.info("PMD audit finished"); - } - - @NotNull - private Severity convert(@NotNull RulePriority rulePriority) { - switch (rulePriority) { - case HIGH: - return Severity.ERROR; - case MEDIUM_HIGH: - return Severity.WARNING; - case MEDIUM: - return Severity.INFO; - case MEDIUM_LOW: - return Severity.INFO; - case LOW: - return Severity.IGNORE; - default: - throw new IllegalArgumentException("RulePriority " + rulePriority + " is not supported"); - } - } -} diff --git a/src/main/java/pl/touk/sputnik/processor/pmd/PmdProcessor.java b/src/main/java/pl/touk/sputnik/processor/pmd/PmdProcessor.java index 3550a05e..f0b72a3f 100644 --- a/src/main/java/pl/touk/sputnik/processor/pmd/PmdProcessor.java +++ b/src/main/java/pl/touk/sputnik/processor/pmd/PmdProcessor.java @@ -1,45 +1,41 @@ package pl.touk.sputnik.processor.pmd; -import com.google.common.base.Joiner; import lombok.extern.slf4j.Slf4j; -import net.sourceforge.pmd.PMD; import net.sourceforge.pmd.PMDConfiguration; -import net.sourceforge.pmd.Rule; -import net.sourceforge.pmd.RuleContext; -import net.sourceforge.pmd.RuleSet; -import net.sourceforge.pmd.RuleSetFactory; -import net.sourceforge.pmd.RuleSets; -import net.sourceforge.pmd.RulesetsFactoryUtils; -import net.sourceforge.pmd.benchmark.Benchmark; -import net.sourceforge.pmd.benchmark.Benchmarker; -import net.sourceforge.pmd.lang.Language; -import net.sourceforge.pmd.lang.LanguageVersion; -import net.sourceforge.pmd.lang.LanguageVersionDiscoverer; -import net.sourceforge.pmd.renderers.Renderer; -import net.sourceforge.pmd.util.ResourceLoader; -import net.sourceforge.pmd.util.datasource.DataSource; + +import net.sourceforge.pmd.PmdAnalysis; +import net.sourceforge.pmd.lang.document.FileId; +import net.sourceforge.pmd.lang.rule.RulePriority; + +import org.apache.commons.lang3.StringUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; + +import net.sourceforge.pmd.reporting.Report; +import net.sourceforge.pmd.reporting.RuleViolation; import pl.touk.sputnik.configuration.Configuration; import pl.touk.sputnik.configuration.GeneralOption; import pl.touk.sputnik.review.Review; import pl.touk.sputnik.review.ReviewException; import pl.touk.sputnik.review.ReviewProcessor; import pl.touk.sputnik.review.ReviewResult; +import pl.touk.sputnik.review.Severity; +import pl.touk.sputnik.review.Violation; import pl.touk.sputnik.review.filter.PmdFilter; import pl.touk.sputnik.review.transformer.FileNameTransformer; -import java.io.IOException; -import java.util.HashSet; -import java.util.LinkedList; +import java.nio.file.FileSystems; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Arrays; import java.util.List; -import java.util.Set; +import java.util.stream.Collectors; @Slf4j public class PmdProcessor implements ReviewProcessor { + private static final char LINE_SEPARATOR = '\n'; private static final String SOURCE_NAME = "PMD"; - private static final char PMD_INPUT_PATH_SEPARATOR = ','; - private Renderer renderer; + private static final String PMD_INPUT_PATH_SEPARATOR = ","; @NotNull private final Configuration config; @@ -51,22 +47,41 @@ public PmdProcessor(Configuration configuration) { @Nullable @Override public ReviewResult process(@NotNull Review review) { - List filesToReview = review.getFiles(new PmdFilter(), new FileNameTransformer()); + List filesToReview = review.getFiles(new PmdFilter(), new FileNameTransformer()).stream() + .map(file -> { + Path path = FileSystems.getDefault().getPath(file); + if (!path.toFile().exists()) { + throw new ReviewException("File [" + file + "] does not exist"); + } + return path; + }) + .collect(Collectors.toList()); if (filesToReview.isEmpty()) { return null; } try { PMDConfiguration configuration = new PMDConfiguration(); - configuration.setReportFormat(CollectorRenderer.class.getCanonicalName()); configuration.setRuleSets(getRulesets()); - configuration.setInputPaths(Joiner.on(PMD_INPUT_PATH_SEPARATOR).join(filesToReview)); - doPMD(configuration); + configuration.setInputPathList(filesToReview); + Report report = doPMD(configuration); + return convertReportToReview(report); } catch (RuntimeException e) { log.error("PMD processing error. Something wrong with configuration or analyzed files are not in workspace.", e); throw new ReviewException("PMD processing error", e); } - return renderer != null ? ((CollectorRenderer)renderer).getReviewResult() : null; + } + + @NotNull + private ReviewResult convertReportToReview(Report report) { + ReviewResult reviewResult = new ReviewResult(); + boolean showDetails = Boolean.parseBoolean(config.getProperty(GeneralOption.PMD_SHOW_VIOLATION_DETAILS)); + for (RuleViolation ruleViolation : report.getViolations()) { + String violationDescription = showDetails ? renderViolationDetails(ruleViolation) :ruleViolation.getDescription(); + FileId myFileId = ruleViolation.getFileId(); + reviewResult.add(new Violation(myFileId.getOriginalPath(), ruleViolation.getBeginLine(), violationDescription, convert(ruleViolation.getRule().getPriority()))); + } + return reviewResult; } @NotNull @@ -75,74 +90,57 @@ public String getName() { return SOURCE_NAME; } - @Nullable - private String getRulesets() { + private List getRulesets() { String ruleSets = config.getProperty(GeneralOption.PMD_RULESETS); log.info("Using PMD rulesets {}", ruleSets); - return ruleSets; + if (ruleSets == null) { + return new ArrayList<>(); + } + return Arrays.asList(ruleSets.split(PMD_INPUT_PATH_SEPARATOR)); } /** - * PMD has terrible design of process configuration. You must use report file with it. I paste this method here and - * improve it. + * Run PMD analysis * - * @throws IllegalArgumentException - * if the configuration is not correct + * @return Report from PMD + * @throws IllegalArgumentException if the configuration is not correct */ - private void doPMD(@NotNull PMDConfiguration configuration) throws IllegalArgumentException { - // Load the RuleSets - RuleSetFactory ruleSetFactory = RulesetsFactoryUtils.getRulesetFactory(configuration, new ResourceLoader()); - - RuleSets ruleSets = RulesetsFactoryUtils.getRuleSets(configuration.getRuleSets(), ruleSetFactory); - // this is just double check - we don't get null here - // instead IllegalArgumentException/RuntimeException is thrown if configuration is wrong - if (ruleSets == null) { - return; + @NotNull + private Report doPMD(@NotNull PMDConfiguration configuration) throws IllegalArgumentException { + try (PmdAnalysis analysis = PmdAnalysis.create(configuration)) { + return analysis.performAnalysisAndCollectReport(); } + } - Set languages = getApplicableLanguages(configuration, ruleSets); - // this throws RuntimeException when modified file does not exist in workspace - List files = PMD.getApplicableFiles(configuration, languages); - - long reportStart = System.nanoTime(); - try { - renderer = configuration.createRenderer(); - List renderers = new LinkedList<>(); - renderers.add(renderer); - renderer.start(); - - Benchmarker.mark(Benchmark.Reporting, System.nanoTime() - reportStart, 0); - - RuleContext ctx = new RuleContext(); - - PMD.processFiles(configuration, ruleSetFactory, files, ctx, renderers); - - reportStart = System.nanoTime(); - renderer.end(); - } catch (IOException e) { - log.error("PMD analysis error", e); - } finally { - Benchmarker.mark(Benchmark.Reporting, System.nanoTime() - reportStart, 0); + @NotNull + private static Severity convert(@NotNull RulePriority rulePriority) { + switch (rulePriority) { + case HIGH: + return Severity.ERROR; + case MEDIUM_HIGH: + return Severity.WARNING; + case MEDIUM: + case MEDIUM_LOW: + return Severity.INFO; + case LOW: + return Severity.IGNORE; + default: + throw new IllegalArgumentException("RulePriority " + rulePriority + " is not supported"); } } - /** - * Paste from PMD - */ - private static Set getApplicableLanguages(PMDConfiguration configuration, RuleSets ruleSets) { - Set languages = new HashSet<>(); - LanguageVersionDiscoverer discoverer = configuration.getLanguageVersionDiscoverer(); - - for (Rule rule : ruleSets.getAllRules()) { - Language language = rule.getLanguage(); - if (languages.contains(language)) - continue; - LanguageVersion version = discoverer.getDefaultLanguageVersion(language); - if (RuleSet.applies(rule, version)) { - languages.add(language); - log.debug("Using {} version: {}", language.getShortName(), version.getShortName()); - } + private static String renderViolationDetails(RuleViolation ruleViolation) { + StringBuilder fullDescription = new StringBuilder(ruleViolation.getDescription()); + + String reason = ruleViolation.getRule().getDescription(); + if (StringUtils.isNotEmpty(reason)) { + fullDescription.append(LINE_SEPARATOR).append(reason); + } + String url = ruleViolation.getRule().getExternalInfoUrl(); + if (StringUtils.isNotEmpty(url)) { + fullDescription.append(LINE_SEPARATOR).append(url); } - return languages; + + return fullDescription.toString(); } } diff --git a/src/test/java/pl/touk/sputnik/processor/pmd/CollectorRendererTest.java b/src/test/java/pl/touk/sputnik/processor/pmd/CollectorRendererTest.java deleted file mode 100644 index cbbbd27e..00000000 --- a/src/test/java/pl/touk/sputnik/processor/pmd/CollectorRendererTest.java +++ /dev/null @@ -1,91 +0,0 @@ -package pl.touk.sputnik.processor.pmd; - -import com.google.common.base.Joiner; -import com.google.common.collect.ImmutableMap; -import net.sourceforge.pmd.Report; -import net.sourceforge.pmd.Rule; -import net.sourceforge.pmd.RulePriority; -import net.sourceforge.pmd.RuleViolation; -import org.jetbrains.annotations.NotNull; -import org.junit.jupiter.api.Test; -import pl.touk.sputnik.TestEnvironment; -import pl.touk.sputnik.configuration.Configuration; -import pl.touk.sputnik.configuration.ConfigurationSetup; -import pl.touk.sputnik.configuration.GeneralOption; -import pl.touk.sputnik.review.Severity; -import pl.touk.sputnik.review.Violation; - -import java.io.IOException; -import java.util.ArrayList; -import java.util.List; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -class CollectorRendererTest extends TestEnvironment { - - private CollectorRenderer renderer; - - private static final String VIOLATION_DESCRIPTION = "this is bug!"; - private static final String RULE_DESCRIPTION = "...and should be fixed"; - private static final String EXTERNAL_INFO_URL = "www.solution.tip"; - private static final String DESCRIPTION_WITH_DETAILS = Joiner.on('\n').join(VIOLATION_DESCRIPTION, RULE_DESCRIPTION, EXTERNAL_INFO_URL); - - @Test - void shouldReportViolationWithDetails() throws IOException { - Configuration config = new ConfigurationSetup().setUp(ImmutableMap.of(GeneralOption.PMD_SHOW_VIOLATION_DETAILS.getKey(), "true")); - Rule rule = createRule(RULE_DESCRIPTION, EXTERNAL_INFO_URL, RulePriority.HIGH, config); - Report report = createReportWithVolation(createRuleViolation(rule, VIOLATION_DESCRIPTION, config), config); - - renderer.renderFileReport(report); - - Violation violation = renderer.getReviewResult().getViolations().get(0); - assertThat(violation.getMessage()).isEqualTo(DESCRIPTION_WITH_DETAILS); - assertThat(violation.getSeverity()).isEqualTo(Severity.ERROR); - } - - @Test - void shouldReportViolationWithoutDetails() throws IOException { - Configuration config = new ConfigurationSetup().setUp(ImmutableMap.of(GeneralOption.PMD_SHOW_VIOLATION_DETAILS.getKey(), "false")); - Rule rule = createRule(RULE_DESCRIPTION, EXTERNAL_INFO_URL, RulePriority.MEDIUM, config); - Report report = createReportWithVolation(createRuleViolation(rule, VIOLATION_DESCRIPTION, config), config); - - renderer.renderFileReport(report); - - Violation violation = renderer.getReviewResult().getViolations().get(0); - assertThat(violation.getMessage()).isEqualTo(VIOLATION_DESCRIPTION); - } - - @NotNull - private Rule createRule(String ruleDescription, String externalInfoUrl, RulePriority priority, @NotNull Configuration config) { - renderer = new CollectorRenderer(config); - Rule rule = mock(Rule.class); - when(rule.getDescription()).thenReturn(ruleDescription); - when(rule.getExternalInfoUrl()).thenReturn(externalInfoUrl); - when(rule.getPriority()).thenReturn(priority); - - return rule; - } - - @NotNull - private RuleViolation createRuleViolation(@NotNull Rule rule, String violationDescription, @NotNull Configuration config) { - renderer = new CollectorRenderer(config); - RuleViolation violation = mock(RuleViolation.class); - when(violation.getRule()).thenReturn(rule); - when(violation.getDescription()).thenReturn(violationDescription); - - return violation; - } - - @NotNull - private Report createReportWithVolation(@NotNull RuleViolation violation, @NotNull Configuration config) { - renderer = new CollectorRenderer(config); - Report report = mock(Report.class); - List list = new ArrayList(); - list.add(violation); - when(report.iterator()).thenReturn(list.iterator()); - - return report; - } -} diff --git a/src/test/java/pl/touk/sputnik/processor/pmd/PmdProcessorTest.java b/src/test/java/pl/touk/sputnik/processor/pmd/PmdProcessorTest.java index cb1ae192..acddbe23 100644 --- a/src/test/java/pl/touk/sputnik/processor/pmd/PmdProcessorTest.java +++ b/src/test/java/pl/touk/sputnik/processor/pmd/PmdProcessorTest.java @@ -25,9 +25,9 @@ void shouldReturnPmdViolations() { assertThat(reviewResult.getViolations()) .isNotEmpty() - .hasSize(4) + .hasSize(5) .extracting("message") - .contains("All classes and interfaces must belong to a named package") + .contains("All classes, interfaces, enums and annotations must belong to a named package") .contains("Each class should declare at least one constructor"); } diff --git a/src/test/resources/test.properties b/src/test/resources/test.properties index c1458cea..b4d5c613 100644 --- a/src/test/resources/test.properties +++ b/src/test/resources/test.properties @@ -7,7 +7,7 @@ checkstyle.enabled=true checkstyle.configurationFile=sun_checks.xml checkstyle.propertiesFile= pmd.enabled=true -pmd.ruleSets=rulesets/java/android.xml,rulesets/java/basic.xml,rulesets/java/braces.xml,rulesets/java/clone.xml,rulesets/java/codesize.xml,rulesets/java/comments.xml,rulesets/java/controversial.xml,rulesets/java/coupling.xml,rulesets/java/design.xml,rulesets/java/empty.xml,rulesets/java/finalizers.xml,rulesets/java/imports.xml,rulesets/java/j2ee.xml,rulesets/java/javabeans.xml,rulesets/java/junit.xml,rulesets/java/logging-jakarta-commons.xml,rulesets/java/logging-java.xml,rulesets/java/migrating.xml,rulesets/java/naming.xml,rulesets/java/optimizations.xml,rulesets/java/strictexception.xml,rulesets/java/strings.xml,rulesets/java/sunsecure.xml,rulesets/java/unnecessary.xml,rulesets/java/unusedcode.xml +pmd.ruleSets=category/java/bestpractices.xml,category/java/codestyle.xml,category/java/design.xml,category/java/documentation.xml,category/java/errorprone.xml,category/java/multithreading.xml,category/java/performance.xml,category/java/security.xml spotbugs.enabled=true spotbugs.includeFilter= spotbugs.excludeFilter=