From 387555b4a1e3f440a04d878d9fe1fc5a5bd64c70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Wed, 29 Jul 2020 20:18:59 +0200 Subject: [PATCH] Wrap renderer inside listener Fix tests --- .../main/java/net/sourceforge/pmd/PMD.java | 90 ++++------- .../main/java/net/sourceforge/pmd/Report.java | 62 ++++++-- .../java/net/sourceforge/pmd/RuleContext.java | 10 +- .../pmd/ThreadSafeAnalysisListener.java | 144 +++++++++++++++--- .../pmd/ant/internal/PMDTaskImpl.java | 80 +++++----- .../pmd/cache/AbstractAnalysisCache.java | 14 +- .../sourceforge/pmd/cache/AnalysisCache.java | 6 +- .../pmd/cache/FileAnalysisCache.java | 4 + .../pmd/cache/NoopAnalysisCache.java | 17 ++- .../lang/rule/internal/RuleApplicator.java | 2 +- .../pmd/processor/AbstractPMDProcessor.java | 39 ++--- .../pmd/processor/MonoThreadProcessor.java | 12 +- .../pmd/processor/MultiThreadProcessor.java | 35 ++--- .../pmd/processor/PmdRunnable.java | 98 +++++------- .../java/net/sourceforge/pmd/RuleSetTest.java | 103 +++++++------ .../pmd/cache/FileAnalysisCacheTest.java | 3 +- .../processor/MultiThreadProcessorTest.java | 125 ++++++--------- .../net/sourceforge/pmd/ExcludeLinesTest.java | 2 - .../pmd/lang/java/rule/XPathRuleTest.java | 2 - .../internal/XPathMetricFunctionTest.java | 2 - .../pmd/lang/jsp/ast/XPathJspRuleTest.java | 3 - .../lang/scala/ast/ScalaParsingHelper.java | 3 - .../pmd/testframework/RuleTst.java | 1 - 23 files changed, 457 insertions(+), 400 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/PMD.java b/pmd-core/src/main/java/net/sourceforge/pmd/PMD.java index dc0a292cce..0b13bcbec9 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/PMD.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/PMD.java @@ -4,6 +4,8 @@ package net.sourceforge.pmd; +import static net.sourceforge.pmd.util.CollectionUtil.listOf; + import java.io.File; import java.io.IOException; import java.io.OutputStreamWriter; @@ -16,11 +18,12 @@ import java.util.Comparator; import java.util.HashSet; import java.util.List; import java.util.Set; -import java.util.concurrent.atomic.AtomicInteger; import java.util.logging.ConsoleHandler; import java.util.logging.Level; import java.util.logging.Logger; +import net.sourceforge.pmd.Report.ReportBuilderListener; +import net.sourceforge.pmd.ThreadSafeAnalysisListener.GlobalAnalysisListener; import net.sourceforge.pmd.benchmark.TextTimingReportRenderer; import net.sourceforge.pmd.benchmark.TimeTracker; import net.sourceforge.pmd.benchmark.TimedOperation; @@ -176,34 +179,21 @@ public class PMD { try { Renderer renderer; - final List renderers; try (TimedOperation to = TimeTracker.startOperation(TimedOperationCategory.REPORTING)) { renderer = configuration.createRenderer(); - renderers = Collections.singletonList(renderer); renderer.setReportFile(configuration.getReportFile()); renderer.start(); } - final RuleContext ctx = new RuleContext(); - final AtomicInteger violations = new AtomicInteger(0); - ctx.getReport().addListener(new ThreadSafeReportListener() { - @Override - public void ruleViolationAdded(final RuleViolation ruleViolation) { - violations.getAndIncrement(); + ReportBuilderListener reportBuilder = new ReportBuilderListener(); + + try (GlobalAnalysisListener listener = GlobalAnalysisListener.forReporter(renderer)) { + try (TimedOperation to = TimeTracker.startOperation(TimedOperationCategory.FILE_PROCESSING)) { + encourageToUseIncrementalAnalysis(configuration); + processFiles(configuration, ruleSetFactory, files, listener); } - - }); - - try (TimedOperation to = TimeTracker.startOperation(TimedOperationCategory.FILE_PROCESSING)) { - encourageToUseIncrementalAnalysis(configuration); - processFiles(configuration, ruleSetFactory, files, ctx, renderers); - } - - try (TimedOperation rto = TimeTracker.startOperation(TimedOperationCategory.REPORTING)) { - renderer.end(); - renderer.flush(); - return violations.get(); } + return reportBuilder.getReport().getViolations().size(); } catch (final Exception e) { String message = e.getMessage(); if (message == null) { @@ -226,61 +216,43 @@ public class PMD { } } - /** - * Creates a new rule context, initialized with a new, empty report. - * - * @param sourceCodeFilename - * the source code filename - * @param sourceCodeFile - * the source code file - * @return the rule context - */ - public static RuleContext newRuleContext(String sourceCodeFilename, File sourceCodeFile) { - - RuleContext context = new RuleContext(); - context.setSourceCodeFile(sourceCodeFile); - context.setReport(new Report()); - return context; - } - /** * Run PMD on a list of files using multiple threads - if more than one is * available * - * @param configuration - * Configuration - * @param ruleSetFactory - * RuleSetFactory - * @param files - * List of {@link DataSource}s - * @param ctx - * RuleContext - * @param renderers - * List of {@link Renderer}s + * @param configuration Configuration + * @param ruleSetFactory RuleSetFactory + * @param files List of {@link DataSource}s + * @param listener RuleContext */ - public static void processFiles(final PMDConfiguration configuration, - final RuleSetFactory ruleSetFactory, - final List files, - final RuleContext ctx, - final List renderers) { + public static void processFiles(PMDConfiguration configuration, + RuleSetFactory ruleSetFactory, + List files, + GlobalAnalysisListener listener) { final RuleSetFactory silentFactory = new RuleSetFactory(ruleSetFactory, false); final RuleSets rs = RulesetsFactoryUtils.getRuleSets(configuration.getRuleSets(), silentFactory); - final Set brokenRules = removeBrokenRules(rs); - for (final Rule rule : brokenRules) { - ctx.getReport().addConfigError(new Report.ConfigurationError(rule, rule.dysfunctionReason())); - } + // Just like we throw for invalid properties, "broken rules" + // shouldn't be a "config error". This is the only instance of + // config errors... + + // final Set brokenRules = removeBrokenRules(rs); + // for (final Rule rule : brokenRules) { + // ctx.getReport().addConfigError(new Report.ConfigurationError(rule, rule.dysfunctionReason())); + // } encourageToUseIncrementalAnalysis(configuration); sortFiles(configuration, files); // Make sure the cache is listening for analysis results - ctx.getReport().addListener(configuration.getAnalysisCache()); + listener = GlobalAnalysisListener.tee(listOf(listener, configuration.getAnalysisCache())); configuration.getAnalysisCache().checkValidity(rs, configuration.getClassLoader()); - AbstractPMDProcessor.newFileProcessor(configuration).processFiles(rs, files, ctx, renderers); + try (AbstractPMDProcessor processor = AbstractPMDProcessor.newFileProcessor(configuration)) { + processor.processFiles(rs, files, listener); + } configuration.getAnalysisCache().persist(); } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/Report.java b/pmd-core/src/main/java/net/sourceforge/pmd/Report.java index a84975595a..3540c87e1d 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/Report.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/Report.java @@ -14,8 +14,9 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; -import net.sourceforge.pmd.lang.ast.FileAnalysisException; +import net.sourceforge.pmd.ThreadSafeAnalysisListener.GlobalAnalysisListener; import net.sourceforge.pmd.renderers.AbstractAccumulatingRenderer; +import net.sourceforge.pmd.util.datasource.DataSource; /** * A {@link Report} collects all informations during a PMD execution. This @@ -186,10 +187,12 @@ public class Report { * @param violation the violation to add */ public void addRuleViolation(RuleViolation violation) { - int index = Collections.binarySearch(violations, violation, RuleViolationComparator.INSTANCE); - violations.add(index < 0 ? -index - 1 : index, violation); - for (ThreadSafeReportListener listener : listeners) { - listener.ruleViolationAdded(violation); + synchronized (violations) { + int index = Collections.binarySearch(violations, violation, RuleViolationComparator.INSTANCE); + violations.add(index < 0 ? -index - 1 : index, violation); + // for (ThreadSafeReportListener listener : listeners) { + // listener.ruleViolationAdded(violation); + // } } } @@ -290,15 +293,23 @@ public class Report { } - public static class ReportBuilderListener implements ThreadSafeAnalysisListener { + public static final class ReportBuilderListener implements ThreadSafeAnalysisListener { - private final Report report = new Report(); + private final Report report; private boolean done; + public ReportBuilderListener() { + this(new Report()); + } + + ReportBuilderListener(Report report) { + this.report = report; + } + /** * Returns the final report. * - * @throws IllegalStateException If {@link #finish()} has not been called yet + * @throws IllegalStateException If {@link #close()} has not been called yet */ public Report getReport() { if (!done) { @@ -318,13 +329,42 @@ public class Report { } @Override - public void onError(FileAnalysisException exception) { - report.addError(new ProcessingError(exception, exception.getFileName())); + public void onError(ProcessingError error) { + report.addError(error); } @Override - public void finish() { + public void close() { done = true; } } + + + public final static class GlobalReportBuilder implements GlobalAnalysisListener { + + private final Report report = new Report(); + private boolean done; + + @Override + public ThreadSafeAnalysisListener startFileAnalysis(DataSource file) { + return new ReportBuilderListener(this.report); + } + + @Override + public void close() throws Exception { + done = true; + } + + /** + * Returns the final report. + * + * @throws IllegalStateException If {@link #close()} has not been called yet + */ + public Report getReport() { + if (!done) { + throw new IllegalStateException("Reporting not done"); + } + return report; + } + } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RuleContext.java b/pmd-core/src/main/java/net/sourceforge/pmd/RuleContext.java index 65ab774426..5054c4a36b 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleContext.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleContext.java @@ -73,11 +73,11 @@ public class RuleContext implements AutoCloseable { @Override public void close() throws Exception { - listener.finish(); + listener.close(); } - public void addError(ProcessingError error) { - + public void reportError(ProcessingError error) { + listener.onError(error); } @@ -97,6 +97,10 @@ public class RuleContext implements AutoCloseable { addViolationWithPosition(rule, location, -1, -1, message, formatArgs); } + public void addViolationNoSuppress(RuleViolation rv) { + listener.onRuleViolation(rv); + } + public void addViolationWithPosition(Rule rule, Node location, int beginLine, int endLine, String message, Object... formatArgs) { Objects.requireNonNull(rule); Objects.requireNonNull(location); diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/ThreadSafeAnalysisListener.java b/pmd-core/src/main/java/net/sourceforge/pmd/ThreadSafeAnalysisListener.java index 39e427ea3a..68501dd2bb 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/ThreadSafeAnalysisListener.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/ThreadSafeAnalysisListener.java @@ -4,15 +4,105 @@ package net.sourceforge.pmd; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; + +import net.sourceforge.pmd.Report.ProcessingError; import net.sourceforge.pmd.Report.ReportBuilderListener; import net.sourceforge.pmd.Report.SuppressedViolation; -import net.sourceforge.pmd.lang.ast.FileAnalysisException; +import net.sourceforge.pmd.benchmark.TimeTracker; +import net.sourceforge.pmd.benchmark.TimedOperation; +import net.sourceforge.pmd.benchmark.TimedOperationCategory; import net.sourceforge.pmd.renderers.Renderer; +import net.sourceforge.pmd.util.CollectionUtil; +import net.sourceforge.pmd.util.datasource.DataSource; /** * A handler for analysis events. This must be thread safe. */ -public interface ThreadSafeAnalysisListener { +public interface ThreadSafeAnalysisListener extends AutoCloseable { + + + interface GlobalAnalysisListener extends AutoCloseable { + + ThreadSafeAnalysisListener startFileAnalysis(DataSource file); + + + static GlobalAnalysisListener tee(List list) { + List listeners = Collections.unmodifiableList(new ArrayList<>(list)); + return new GlobalAnalysisListener() { + @Override + public ThreadSafeAnalysisListener startFileAnalysis(DataSource file) { + return ThreadSafeAnalysisListener.tee(CollectionUtil.map(listeners, it -> it.startFileAnalysis(file))); + } + + @Override + public void close() throws Exception { + Exception composed = null; + for (GlobalAnalysisListener it : list) { + try { + it.close(); + } catch (Exception e) { + if (composed == null) { + composed = e; + } else { + composed.addSuppressed(e); + } + } + } + if (composed != null) { + throw composed; + } + } + }; + } + + + static GlobalAnalysisListener forReporter(Renderer renderer) { + + return new GlobalAnalysisListener() { + @Override + public ThreadSafeAnalysisListener startFileAnalysis(DataSource file) { + renderer.startFileAnalysis(file); + return new ThreadSafeAnalysisListener() { + final ReportBuilderListener reportBuilder = new ReportBuilderListener(); + + @Override + public void onRuleViolation(RuleViolation violation) { + reportBuilder.onRuleViolation(violation); + } + + @Override + public void onSuppressedRuleViolation(SuppressedViolation violation) { + reportBuilder.onSuppressedRuleViolation(violation); + } + + @Override + public void onError(ProcessingError error) { + reportBuilder.onError(error); + } + + @Override + public void close() throws Exception { + reportBuilder.close(); + renderer.renderFileReport(reportBuilder.getReport()); + } + }; + } + + @Override + public void close() throws Exception { + try (TimedOperation to = TimeTracker.startOperation(TimedOperationCategory.REPORTING)) { + renderer.end(); + renderer.flush(); + } + } + }; + } + } + /** * Handle a new violation (not suppressed). @@ -21,6 +111,7 @@ public interface ThreadSafeAnalysisListener { } + /** * Handle a new suppressed violation. */ @@ -32,15 +123,13 @@ public interface ThreadSafeAnalysisListener { /** * Handle an error that occurred while processing a file. */ - default void onError(FileAnalysisException exception) { + default void onError(ProcessingError error) { } - /** - * All files have been processed. - */ - default void finish() throws Exception { + @Override + default void close() throws Exception { } @@ -50,33 +139,50 @@ public interface ThreadSafeAnalysisListener { } - static ThreadSafeAnalysisListener forReporter(Renderer renderer) { - ReportBuilderListener reportBuilder = new ReportBuilderListener(); + static ThreadSafeAnalysisListener tee(Collection listeners) { + List list = Collections.unmodifiableList(new ArrayList<>(listeners)); return new ThreadSafeAnalysisListener() { - @Override public void onRuleViolation(RuleViolation violation) { - reportBuilder.onRuleViolation(violation); + for (ThreadSafeAnalysisListener it : list) { + it.onRuleViolation(violation); + } } @Override public void onSuppressedRuleViolation(SuppressedViolation violation) { - reportBuilder.onSuppressedRuleViolation(violation); + for (ThreadSafeAnalysisListener it : list) { + it.onSuppressedRuleViolation(violation); + } } @Override - public void onError(FileAnalysisException exception) { - reportBuilder.onError(exception); + public void onError(ProcessingError error) { + for (ThreadSafeAnalysisListener it : list) { + it.onError(error); + } } @Override - public void finish() throws Exception { - reportBuilder.finish(); - renderer.renderFileReport(reportBuilder.getReport()); - renderer.end(); - renderer.flush(); + public void close() throws Exception { + Exception composed = null; + for (ThreadSafeAnalysisListener it : list) { + try { + it.close(); + } catch (Exception e) { + if (composed == null) { + composed = e; + } else { + composed.addSuppressed(e); + } + } + } + if (composed != null) { + throw composed; + } } }; + } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/ant/internal/PMDTaskImpl.java b/pmd-core/src/main/java/net/sourceforge/pmd/ant/internal/PMDTaskImpl.java index f60d1093bf..032eda2b4f 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/ant/internal/PMDTaskImpl.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/ant/internal/PMDTaskImpl.java @@ -12,6 +12,7 @@ import java.util.ArrayList; import java.util.LinkedList; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Collectors; import org.apache.commons.lang3.StringUtils; import org.apache.tools.ant.AntClassLoader; @@ -20,18 +21,19 @@ import org.apache.tools.ant.DirectoryScanner; import org.apache.tools.ant.Project; import org.apache.tools.ant.types.FileSet; import org.apache.tools.ant.types.Path; +import org.checkerframework.checker.nullness.qual.NonNull; import net.sourceforge.pmd.PMD; import net.sourceforge.pmd.PMDConfiguration; import net.sourceforge.pmd.Report; import net.sourceforge.pmd.Rule; -import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.RulePriority; import net.sourceforge.pmd.RuleSet; import net.sourceforge.pmd.RuleSetFactory; import net.sourceforge.pmd.RuleSetNotFoundException; import net.sourceforge.pmd.RuleSets; import net.sourceforge.pmd.RulesetsFactoryUtils; +import net.sourceforge.pmd.ThreadSafeAnalysisListener.GlobalAnalysisListener; import net.sourceforge.pmd.ant.Formatter; import net.sourceforge.pmd.ant.PMDTask; import net.sourceforge.pmd.ant.SourceLanguage; @@ -135,7 +137,6 @@ public class PMDTaskImpl { // TODO Do we really need all this in a loop over each FileSet? Seems // like a lot of redundancy - RuleContext ctx = new RuleContext(); Report errorReport = new Report(); final AtomicInteger reportSize = new AtomicInteger(); final String separator = System.getProperty("file.separator"); @@ -156,36 +157,7 @@ public class PMDTaskImpl { reportShortNamesPaths.add(commonInputPath); } - Renderer logRenderer = new AbstractRenderer("log", "Logging renderer") { - @Override - public void start() { - // Nothing to do - } - - @Override - public void startFileAnalysis(DataSource dataSource) { - project.log("Processing file " + dataSource.getNiceFileName(false, commonInputPath), - Project.MSG_VERBOSE); - } - - @Override - public void renderFileReport(Report r) { - int size = r.getViolations().size(); - if (size > 0) { - reportSize.addAndGet(size); - } - } - - @Override - public void end() { - // Nothing to do - } - - @Override - public String defaultFileExtension() { - return null; - } // not relevant - }; + Renderer logRenderer = makeRenderer(reportSize, commonInputPath); List renderers = new ArrayList<>(formatters.size() + 1); renderers.add(logRenderer); for (Formatter formatter : formatters) { @@ -193,10 +165,12 @@ public class PMDTaskImpl { renderer.setUseShortNames(reportShortNamesPaths); renderers.add(renderer); } + + GlobalAnalysisListener listener = GlobalAnalysisListener.tee(renderers.stream().map(GlobalAnalysisListener::forReporter).collect(Collectors.toList())); try { - PMD.processFiles(configuration, ruleSetFactory, files, ctx, renderers); + PMD.processFiles(configuration, ruleSetFactory, files, listener); } catch (RuntimeException pmde) { - handleError(ctx, errorReport, pmde); + handleError(errorReport, pmde); } } @@ -217,6 +191,40 @@ public class PMDTaskImpl { } } + @NonNull + private AbstractRenderer makeRenderer(AtomicInteger reportSize, String commonInputPath) { + return new AbstractRenderer("log", "Logging renderer") { + @Override + public void start() { + // Nothing to do + } + + @Override + public void startFileAnalysis(DataSource dataSource) { + project.log("Processing file " + dataSource.getNiceFileName(false, commonInputPath), + Project.MSG_VERBOSE); + } + + @Override + public void renderFileReport(Report r) { + int size = r.getViolations().size(); + if (size > 0) { + reportSize.addAndGet(size); + } + } + + @Override + public void end() { + // Nothing to do + } + + @Override + public String defaultFileExtension() { + return null; + } // not relevant + }; + } + private ResourceLoader setupResourceLoader() { if (classpath == null) { classpath = new Path(project); @@ -238,7 +246,7 @@ public class PMDTaskImpl { project, classpath, parentFirst)); } - private void handleError(RuleContext ctx, Report errorReport, RuntimeException pmde) { + private void handleError(Report errorReport, RuntimeException pmde) { pmde.printStackTrace(); project.log(pmde.toString(), Project.MSG_VERBOSE); @@ -261,7 +269,7 @@ public class PMDTaskImpl { if (failOnError) { throw new BuildException(pmde); } - errorReport.addError(new Report.ProcessingError(pmde, String.valueOf(ctx.getSourceCodeFile()))); + errorReport.addError(new Report.ProcessingError(pmde, "(unknown file)")); } private void setupClassLoader() { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/cache/AbstractAnalysisCache.java b/pmd-core/src/main/java/net/sourceforge/pmd/cache/AbstractAnalysisCache.java index 56e8c734f8..73cdd8ab90 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/cache/AbstractAnalysisCache.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/cache/AbstractAnalysisCache.java @@ -32,7 +32,9 @@ import org.apache.commons.io.IOUtils; import net.sourceforge.pmd.PMDVersion; import net.sourceforge.pmd.RuleSets; import net.sourceforge.pmd.RuleViolation; +import net.sourceforge.pmd.ThreadSafeAnalysisListener; import net.sourceforge.pmd.annotation.InternalApi; +import net.sourceforge.pmd.util.datasource.DataSource; /** * Abstract implementation of the analysis cache. Handles all operations, except for persistence. @@ -226,10 +228,14 @@ public abstract class AbstractAnalysisCache implements AnalysisCache { } @Override - public void ruleViolationAdded(final RuleViolation ruleViolation) { - final AnalysisResult analysisResult = updatedResultsCache.get(ruleViolation.getFilename()); + public ThreadSafeAnalysisListener startFileAnalysis(DataSource filename) { + return new ThreadSafeAnalysisListener() { + @Override + public void onRuleViolation(RuleViolation violation) { + final AnalysisResult analysisResult = updatedResultsCache.get(violation.getFilename()); - analysisResult.addViolation(ruleViolation); + analysisResult.addViolation(violation); // fixme this does NOT look thread-safe + } + }; } - } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/cache/AnalysisCache.java b/pmd-core/src/main/java/net/sourceforge/pmd/cache/AnalysisCache.java index 34e4c5cf9c..4cd417c136 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/cache/AnalysisCache.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/cache/AnalysisCache.java @@ -9,7 +9,7 @@ import java.util.List; import net.sourceforge.pmd.RuleSets; import net.sourceforge.pmd.RuleViolation; -import net.sourceforge.pmd.ThreadSafeReportListener; +import net.sourceforge.pmd.ThreadSafeAnalysisListener.GlobalAnalysisListener; import net.sourceforge.pmd.annotation.InternalApi; /** @@ -21,7 +21,7 @@ import net.sourceforge.pmd.annotation.InternalApi; */ @Deprecated @InternalApi -public interface AnalysisCache extends ThreadSafeReportListener { +public interface AnalysisCache extends GlobalAnalysisListener { /** * Persists the updated analysis results on whatever medium is used by the cache. @@ -31,7 +31,7 @@ public interface AnalysisCache extends ThreadSafeReportListener { /** * Checks if a given file is up to date in the cache and can be skipped from analysis. * Regardless of the return value of this method, each call adds the parameter to the - * updated cache, which allows {@link #ruleViolationAdded(RuleViolation)} to add a rule + * updated cache, which allows {@link #onRuleViolation(RuleViolation)} to add a rule * violation to the file. TODO is this really best behaviour? This side-effects seems counter-intuitive. * * @param sourceFile The file to check in the cache diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/cache/FileAnalysisCache.java b/pmd-core/src/main/java/net/sourceforge/pmd/cache/FileAnalysisCache.java index 769410a318..f646aca90d 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/cache/FileAnalysisCache.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/cache/FileAnalysisCache.java @@ -139,6 +139,10 @@ public class FileAnalysisCache extends AbstractAnalysisCache { } } + @Override + public void close() throws Exception { + // todo + } @Override protected boolean cacheExists() { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/cache/NoopAnalysisCache.java b/pmd-core/src/main/java/net/sourceforge/pmd/cache/NoopAnalysisCache.java index 46a4d77a89..456a2c9f12 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/cache/NoopAnalysisCache.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/cache/NoopAnalysisCache.java @@ -10,7 +10,9 @@ import java.util.List; import net.sourceforge.pmd.RuleSets; import net.sourceforge.pmd.RuleViolation; +import net.sourceforge.pmd.ThreadSafeAnalysisListener; import net.sourceforge.pmd.annotation.InternalApi; +import net.sourceforge.pmd.util.datasource.DataSource; /** * A NOOP analysis cache. Easier / safer than null-checking. @@ -21,11 +23,6 @@ import net.sourceforge.pmd.annotation.InternalApi; @InternalApi public class NoopAnalysisCache implements AnalysisCache { - @Override - public void ruleViolationAdded(final RuleViolation ruleViolation) { - // noop - } - @Override public void persist() { // noop @@ -50,4 +47,14 @@ public class NoopAnalysisCache implements AnalysisCache { public List getCachedViolations(File sourceFile) { return Collections.emptyList(); } + + @Override + public ThreadSafeAnalysisListener startFileAnalysis(DataSource filename) { + return ThreadSafeAnalysisListener.noop(); + } + + @Override + public void close() throws Exception { + // noop + } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/internal/RuleApplicator.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/internal/RuleApplicator.java index 74c9d114eb..1de6c25b84 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/internal/RuleApplicator.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/internal/RuleApplicator.java @@ -62,7 +62,7 @@ public class RuleApplicator { rcto.close(1); } catch (RuntimeException e) { if (ctx.isIgnoreExceptions()) { - ctx.addError(new ProcessingError(e, ctx.getSourceCodeFilename())); + ctx.reportError(new ProcessingError(e, ctx.getSourceCodeFilename())); if (LOG.isLoggable(Level.WARNING)) { LOG.log(Level.WARNING, "Exception applying rule " + rule.getName() + " on file " diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/processor/AbstractPMDProcessor.java b/pmd-core/src/main/java/net/sourceforge/pmd/processor/AbstractPMDProcessor.java index 6bbcf8cb5f..ec46589f02 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/processor/AbstractPMDProcessor.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/processor/AbstractPMDProcessor.java @@ -4,26 +4,19 @@ package net.sourceforge.pmd.processor; -import java.io.IOException; import java.util.List; import org.apache.commons.io.IOUtils; import net.sourceforge.pmd.PMDConfiguration; -import net.sourceforge.pmd.Report; -import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.RuleSets; -import net.sourceforge.pmd.benchmark.TimeTracker; -import net.sourceforge.pmd.benchmark.TimedOperation; -import net.sourceforge.pmd.benchmark.TimedOperationCategory; -import net.sourceforge.pmd.renderers.Renderer; +import net.sourceforge.pmd.ThreadSafeAnalysisListener.GlobalAnalysisListener; import net.sourceforge.pmd.util.datasource.DataSource; /** * @author Romain Pelisse <belaran@gmail.com> - * */ -public abstract class AbstractPMDProcessor { +public abstract class AbstractPMDProcessor implements AutoCloseable { protected final PMDConfiguration configuration; @@ -31,31 +24,14 @@ public abstract class AbstractPMDProcessor { this.configuration = configuration; } - protected void renderReports(final List renderers, final Report report) { - - try (TimedOperation to = TimeTracker.startOperation(TimedOperationCategory.REPORTING)) { - for (Renderer r : renderers) { - r.renderFileReport(report); - } - } catch (IOException ioe) { - throw new RuntimeException(ioe); - } - } - @SuppressWarnings("PMD.CloseResource") - public void processFiles(RuleSets rulesets, List files, RuleContext ctx, List renderers) { + public void processFiles(RuleSets rulesets, List files, GlobalAnalysisListener ctx) { // the data sources must only be closed after the threads are finished // this is done manually without a try-with-resources try { for (final DataSource dataSource : files) { - runAnalysis(new PmdRunnable(dataSource, renderers, ctx, rulesets, configuration)); + runAnalysis(new PmdRunnable(dataSource, ctx, rulesets, configuration)); } - - // render base report first - general errors - renderReports(renderers, ctx.getReport()); - - // then add analysis results per file - collectReports(renderers); } finally { // in case we analyzed files within Zip Files/Jars, we need to close them after // the analysis is finished @@ -67,7 +43,12 @@ public abstract class AbstractPMDProcessor { protected abstract void runAnalysis(PmdRunnable runnable); - protected abstract void collectReports(List renderers); + /** + * Joins tasks and await completion of the analysis. + */ + public void close() { + // join tasks + } /** * Returns a new file processor. The strategy used for threading is diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/processor/MonoThreadProcessor.java b/pmd-core/src/main/java/net/sourceforge/pmd/processor/MonoThreadProcessor.java index c1d490135c..572c7307ed 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/processor/MonoThreadProcessor.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/processor/MonoThreadProcessor.java @@ -9,7 +9,6 @@ import java.util.List; import net.sourceforge.pmd.PMDConfiguration; import net.sourceforge.pmd.Report; -import net.sourceforge.pmd.renderers.Renderer; /** * @author Romain Pelisse <belaran@gmail.com> @@ -26,16 +25,7 @@ final class MonoThreadProcessor extends AbstractPMDProcessor { @Override protected void runAnalysis(PmdRunnable runnable) { // single thread execution, run analysis on same thread - reports.add(runnable.call()); + runnable.run(); } - @Override - protected void collectReports(List renderers) { - for (Report r : reports) { - super.renderReports(renderers, r); - } - - // Since this thread may run PMD again, clean up the runnable - PmdRunnable.reset(); - } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/processor/MultiThreadProcessor.java b/pmd-core/src/main/java/net/sourceforge/pmd/processor/MultiThreadProcessor.java index d3633ff9c0..9c6d8c077c 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/processor/MultiThreadProcessor.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/processor/MultiThreadProcessor.java @@ -4,16 +4,11 @@ package net.sourceforge.pmd.processor; -import java.util.List; -import java.util.concurrent.CompletionService; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.ExecutorCompletionService; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; import net.sourceforge.pmd.PMDConfiguration; -import net.sourceforge.pmd.Report; -import net.sourceforge.pmd.renderers.Renderer; /** @@ -21,41 +16,29 @@ import net.sourceforge.pmd.renderers.Renderer; */ final class MultiThreadProcessor extends AbstractPMDProcessor { private final ExecutorService executor; - private final CompletionService completionService; - - private long submittedTasks = 0L; public MultiThreadProcessor(final PMDConfiguration configuration) { super(configuration); executor = Executors.newFixedThreadPool(configuration.getThreads(), new PmdThreadFactory()); - completionService = new ExecutorCompletionService<>(executor); } @Override protected void runAnalysis(PmdRunnable runnable) { - completionService.submit(runnable); - submittedTasks++; + executor.submit(runnable); } @Override - protected void collectReports(List renderers) { + public void close() { + super.close(); try { - for (int i = 0; i < submittedTasks; i++) { - final Report report = completionService.take().get(); - super.renderReports(renderers, report); + executor.shutdown(); + while (!executor.awaitTermination(10, TimeUnit.HOURS)) { + // still waiting + Thread.yield(); } - } catch (final InterruptedException ie) { + } catch (InterruptedException e) { Thread.currentThread().interrupt(); - } catch (final ExecutionException ee) { - final Throwable t = ee.getCause(); - if (t instanceof RuntimeException) { - throw (RuntimeException) t; - } else if (t instanceof Error) { - throw (Error) t; - } else { - throw new IllegalStateException("PmdRunnable exception", t); - } } finally { executor.shutdownNow(); } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/processor/PmdRunnable.java b/pmd-core/src/main/java/net/sourceforge/pmd/processor/PmdRunnable.java index 63aae77f1b..993b1b2d96 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/processor/PmdRunnable.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/processor/PmdRunnable.java @@ -9,7 +9,7 @@ import java.io.IOException; import java.io.InputStream; import java.util.Collections; import java.util.List; -import java.util.concurrent.Callable; +import java.util.function.Supplier; import java.util.logging.Level; import java.util.logging.Logger; @@ -21,6 +21,7 @@ import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.RuleSet; import net.sourceforge.pmd.RuleSets; import net.sourceforge.pmd.RuleViolation; +import net.sourceforge.pmd.ThreadSafeAnalysisListener.GlobalAnalysisListener; import net.sourceforge.pmd.benchmark.TimeTracker; import net.sourceforge.pmd.benchmark.TimedOperation; import net.sourceforge.pmd.benchmark.TimedOperationCategory; @@ -31,10 +32,9 @@ import net.sourceforge.pmd.lang.Parser.ParserTask; import net.sourceforge.pmd.lang.ast.FileAnalysisException; import net.sourceforge.pmd.lang.ast.RootNode; import net.sourceforge.pmd.lang.ast.SemanticErrorReporter; -import net.sourceforge.pmd.renderers.Renderer; import net.sourceforge.pmd.util.datasource.DataSource; -public class PmdRunnable implements Callable { +public class PmdRunnable implements Runnable { private static final Logger LOG = Logger.getLogger(PmdRunnable.class.getName()); @@ -42,33 +42,31 @@ public class PmdRunnable implements Callable { private final DataSource dataSource; private final File file; - private final List renderers; - private final RuleContext ruleContext; - private final RuleSets ruleSets; + private final GlobalAnalysisListener ruleContext; + + // rename so that it's significantly different from tc.rulesets + private final Supplier ruleSetMaker; private final PMDConfiguration configuration; private final RulesetStageDependencyHelper dependencyHelper; public PmdRunnable(DataSource dataSource, - List renderers, - RuleContext ruleContext, + GlobalAnalysisListener ruleContext, List ruleSets, PMDConfiguration configuration) { - this(dataSource, renderers, ruleContext, new RuleSets(ruleSets), configuration); + this(dataSource, ruleContext, new RuleSets(ruleSets), configuration); } public PmdRunnable(DataSource dataSource, - List renderers, - RuleContext ruleContext, + GlobalAnalysisListener ruleContext, RuleSets ruleSets, PMDConfiguration configuration) { - this.ruleSets = ruleSets; + this.ruleSetMaker = () -> new RuleSets(ruleSets); this.dataSource = dataSource; // this is the real, canonical and absolute filename (not shortened) String realFileName = dataSource.getNiceFileName(false, null); this.file = new File(realFileName); - this.renderers = renderers; this.ruleContext = ruleContext; this.configuration = configuration; this.dependencyHelper = new RulesetStageDependencyHelper(configuration); @@ -79,64 +77,54 @@ public class PmdRunnable implements Callable { } @Override - public Report call() { + public void run() { TimeTracker.initThread(); ThreadContext tc = LOCAL_THREAD_CONTEXT.get(); if (tc == null) { - tc = new ThreadContext(new RuleSets(ruleSets), new RuleContext(ruleContext)); + tc = new ThreadContext(ruleSetMaker.get()); LOCAL_THREAD_CONTEXT.set(tc); } - RuleContext ruleCtx = tc.ruleContext; - LanguageVersion langVersion = - ruleContext.getLanguageVersion() != null ? ruleCtx.getLanguageVersion() - : configuration.getLanguageVersionOfFile(file.getPath()); + try (RuleContext ruleCtx = new RuleContext(ruleContext.startFileAnalysis(dataSource))) { - Report report = new Report(); - // overtake the listener - report.addListeners(ruleCtx.getReport().getListeners()); - ruleCtx.setReport(report); - ruleCtx.setSourceCodeFile(file); - ruleCtx.setLanguageVersion(langVersion); + LanguageVersion langVersion = configuration.getLanguageVersionOfFile(file.getPath()); + ruleCtx.setLanguageVersion(langVersion); + ruleCtx.setSourceCodeFile(file); - if (LOG.isLoggable(Level.FINE)) { - LOG.fine("Processing " + file); - } + if (LOG.isLoggable(Level.FINE)) { + LOG.fine("Processing " + file); + } - for (Renderer r : renderers) { - r.startFileAnalysis(dataSource); - } + // Coarse check to see if any RuleSet applies to file, will need to do a finer RuleSet specific check later + if (tc.ruleSets.applies(file)) { + if (configuration.getAnalysisCache().isUpToDate(file)) { + reportCachedRuleViolations(ruleCtx); + } else { + try { + processSource(ruleCtx, langVersion, tc.ruleSets); + } catch (Exception e) { + configuration.getAnalysisCache().analysisFailed(file); + ruleCtx.reportError(new Report.ProcessingError(e, file.getPath())); - - // Coarse check to see if any RuleSet applies to file, will need to do a finer RuleSet specific check later - if (ruleSets.applies(file)) { - if (configuration.getAnalysisCache().isUpToDate(file)) { - reportCachedRuleViolations(ruleCtx); - } else { - try { - processSource(ruleCtx, langVersion); - } catch (Exception e) { - configuration.getAnalysisCache().analysisFailed(file); - report.addError(new Report.ProcessingError(e, file.getPath())); - - if (ruleCtx.isIgnoreExceptions()) { - LOG.log(Level.FINE, "Exception while processing file: " + file, e); - } else { - if (e instanceof FileAnalysisException) { - throw (FileAnalysisException) e; + if (ruleCtx.isIgnoreExceptions()) { + LOG.log(Level.FINE, "Exception while processing file: " + file, e); + } else { + if (e instanceof FileAnalysisException) { + throw (FileAnalysisException) e; + } + throw new FileAnalysisException(e); } - throw new FileAnalysisException(e); } } } + } catch (Exception e) { + throw new FileAnalysisException("Exception while closing listener for file " + file, e); } TimeTracker.finishThread(); - - return report; } - public void processSource(RuleContext ruleCtx, LanguageVersion languageVersion) throws IOException, FileAnalysisException { + public void processSource(RuleContext ruleCtx, LanguageVersion languageVersion, RuleSets ruleSets) throws IOException, FileAnalysisException { String fullSource; try (InputStream stream = dataSource.getInputStream()) { fullSource = IOUtils.toString(stream, configuration.getSourceEncoding()); @@ -154,7 +142,7 @@ public class PmdRunnable implements Callable { private void reportCachedRuleViolations(final RuleContext ctx) { for (final RuleViolation rv : configuration.getAnalysisCache().getCachedViolations(ctx.getSourceCodeFile())) { - ctx.getReport().addRuleViolation(rv); + ctx.addViolationNoSuppress(rv); } } @@ -190,11 +178,9 @@ public class PmdRunnable implements Callable { private static class ThreadContext { /* default */ final RuleSets ruleSets; - /* default */ final RuleContext ruleContext; - ThreadContext(RuleSets ruleSets, RuleContext ruleContext) { + ThreadContext(RuleSets ruleSets) { this.ruleSets = ruleSets; - this.ruleContext = ruleContext; } } } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetTest.java index 900355273b..c0fa7ef9f4 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetTest.java @@ -28,6 +28,7 @@ import org.checkerframework.checker.nullness.qual.NonNull; import org.junit.Test; import net.sourceforge.pmd.Report.ProcessingError; +import net.sourceforge.pmd.Report.ReportBuilderListener; import net.sourceforge.pmd.RuleSet.RuleSetBuilder; import net.sourceforge.pmd.lang.Dummy2LanguageModule; import net.sourceforge.pmd.lang.DummyLanguageModule; @@ -399,42 +400,46 @@ public class RuleSetTest { } @Test - public void testIncludeExcludeMultipleRuleSetWithRuleChainApplies() throws PMDException { + public void testIncludeExcludeMultipleRuleSetWithRuleChainApplies() throws Exception { File file = new File("C:\\myworkspace\\project\\some\\random\\package\\RandomClass.java"); Rule rule = new FooRule(); rule.setName("FooRule1"); RuleSet ruleSet1 = createRuleSetBuilder("RuleSet1") - .addRule(rule) - .build(); + .addRule(rule) + .build(); RuleSet ruleSet2 = createRuleSetBuilder("RuleSet2") - .addRule(rule) - .build(); + .addRule(rule) + .build(); RuleSets ruleSets = new RuleSets(listOf(ruleSet1, ruleSet2)); // Two violations - RuleContext ctx = new RuleContext(); - Report r = new Report(); - ctx.setReport(r); - ctx.setSourceCodeFile(file); - ctx.setLanguageVersion(dummyLang.getDefaultVersion()); - ruleSets.apply(makeCompilationUnits(), ctx); - assertEquals("Violations", 2, r.getViolations().size()); + ReportBuilderListener reportBuilder = new ReportBuilderListener(); + try (RuleContext ctx = new RuleContext(reportBuilder)) { + ctx.setSourceCodeFile(file); + ctx.setLanguageVersion(dummyLang.getDefaultVersion()); + ruleSets.apply(makeCompilationUnits(), ctx); + } + assertEquals("Violations", 2, reportBuilder.getReport().getViolations().size()); // One violation ruleSet1 = createRuleSetBuilder("RuleSet1") - .withFileExclusions(Pattern.compile(".*/package/.*")) - .addRule(rule) - .build(); + .withFileExclusions(Pattern.compile(".*/package/.*")) + .addRule(rule) + .build(); ruleSets = new RuleSets(listOf(ruleSet1, ruleSet2)); - r = new Report(); - ctx.setReport(r); - ruleSets.apply(makeCompilationUnits(), ctx); - assertEquals("Violations", 1, r.getViolations().size()); + reportBuilder = new ReportBuilderListener(); + + try (RuleContext ctx = new RuleContext(reportBuilder)) { + ctx.setSourceCodeFile(file); + ctx.setLanguageVersion(LanguageRegistry.getLanguage(DummyLanguageModule.NAME).getDefaultVersion()); + ruleSets.apply(makeCompilationUnits(), ctx); + } + assertEquals("Violations", 1, reportBuilder.getReport().getViolations().size()); } @Test @@ -482,7 +487,7 @@ public class RuleSetTest { } @Test - public void ruleExceptionShouldBeReported() { + public void ruleExceptionShouldBeReported() throws Exception { RuleSet ruleset = createRuleSetBuilder("ruleExceptionShouldBeReported") .addRule(new MockRule() { @Override @@ -491,15 +496,16 @@ public class RuleSetTest { } }) .build(); - RuleContext context = new RuleContext(); - context.setReport(new Report()); - context.setLanguageVersion(dummyLang.getDefaultVersion()); - context.setSourceCodeFile(new File(RuleSetTest.class.getName() + ".ruleExceptionShouldBeReported")); - context.setIgnoreExceptions(true); // the default - ruleset.apply(makeCompilationUnits(), context); + ReportBuilderListener reportBuilder = new ReportBuilderListener(); + try (RuleContext context = new RuleContext(reportBuilder)) { + context.setLanguageVersion(dummyLang.getDefaultVersion()); + context.setSourceCodeFile(new File("foo.dummy")); + context.setIgnoreExceptions(true); // the default + ruleset.apply(makeCompilationUnits(), context); + } - List errors = context.getReport().getProcessingErrors(); - assertTrue("Report should have processing errors", !errors.isEmpty()); + List errors = reportBuilder.getReport().getProcessingErrors(); + assertFalse("Report should have processing errors", errors.isEmpty()); assertEquals("Errors expected", 1, errors.size()); assertEquals("Wrong error message", "RuntimeException: Test exception while applying rule", errors.get(0).getMsg()); assertTrue("Should be a RuntimeException", errors.get(0).getError() instanceof RuntimeException); @@ -524,7 +530,7 @@ public class RuleSetTest { } @Test - public void ruleExceptionShouldNotStopProcessingFile() { + public void ruleExceptionShouldNotStopProcessingFile() throws Exception { RuleSet ruleset = createRuleSetBuilder("ruleExceptionShouldBeReported").addRule(new MockRule() { @Override public void apply(Node target, RuleContext ctx) { @@ -536,24 +542,24 @@ public class RuleSetTest { addViolationWithMessage(ctx, target, "Test violation of the second rule in the ruleset"); } }).build(); - RuleContext context = new RuleContext(); - context.setReport(new Report()); - context.setLanguageVersion(dummyLang.getDefaultVersion()); - context.setSourceCodeFile(new File(RuleSetTest.class.getName() + ".ruleExceptionShouldBeReported")); - context.setIgnoreExceptions(true); // the default - ruleset.apply(makeCompilationUnits(), context); - - List errors = context.getReport().getProcessingErrors(); + ReportBuilderListener reportBuilder = new ReportBuilderListener(); + try (RuleContext context = new RuleContext(reportBuilder)) { + context.setLanguageVersion(dummyLang.getDefaultVersion()); + context.setSourceCodeFile(new File("foo.dummy")); + context.setIgnoreExceptions(true); // the default + ruleset.apply(makeCompilationUnits(), context); + } + List errors = reportBuilder.getReport().getProcessingErrors(); assertFalse("Report should have processing errors", errors.isEmpty()); assertEquals("Errors expected", 1, errors.size()); assertEquals("Wrong error message", "RuntimeException: Test exception while applying rule", errors.get(0).getMsg()); assertTrue("Should be a RuntimeException", errors.get(0).getError() instanceof RuntimeException); - assertEquals("There should be a violation", 1, context.getReport().getViolations().size()); + assertEquals("There should be a violation", 1, reportBuilder.getReport().getViolations().size()); } @Test - public void ruleExceptionShouldNotStopProcessingFileWithRuleChain() { + public void ruleExceptionShouldNotStopProcessingFileWithRuleChain() throws Exception { RuleSet ruleset = createRuleSetBuilder("ruleExceptionShouldBeReported").addRule(new MockRule() { @Override @@ -577,20 +583,23 @@ public class RuleSetTest { addViolationWithMessage(ctx, target, "Test violation of the second rule in the ruleset"); } }).build(); - RuleContext context = new RuleContext(); - context.setReport(new Report()); - context.setLanguageVersion(dummyLang.getDefaultVersion()); - context.setSourceCodeFile(new File(RuleSetTest.class.getName() + ".ruleExceptionShouldBeReported")); - context.setIgnoreExceptions(true); // the default RuleSets rulesets = new RuleSets(ruleset); - rulesets.apply(makeCompilationUnits(), context); - List errors = context.getReport().getProcessingErrors(); + ReportBuilderListener reportBuilder = new ReportBuilderListener(); + try (RuleContext context = new RuleContext(reportBuilder)) { + context.setLanguageVersion(dummyLang.getDefaultVersion()); + context.setSourceCodeFile(new File("foo.dummy")); + context.setIgnoreExceptions(true); // the default + rulesets.apply(makeCompilationUnits(), context); + } + + + List errors = reportBuilder.getReport().getProcessingErrors(); assertEquals("Errors expected", 1, errors.size()); assertEquals("Wrong error message", "RuntimeException: Test exception while applying rule", errors.get(0).getMsg()); assertTrue("Should be a RuntimeException", errors.get(0).getError() instanceof RuntimeException); - assertEquals("There should be a violation", 1, context.getReport().getViolations().size()); + assertEquals("There should be a violation", 1, reportBuilder.getReport().getViolations().size()); } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/cache/FileAnalysisCacheTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/cache/FileAnalysisCacheTest.java index 06d98208f1..c2a3e38fbb 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/cache/FileAnalysisCacheTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/cache/FileAnalysisCacheTest.java @@ -33,6 +33,7 @@ import org.mockito.Mockito; import net.sourceforge.pmd.RuleSets; import net.sourceforge.pmd.RuleViolation; import net.sourceforge.pmd.lang.Language; +import net.sourceforge.pmd.util.datasource.DataSource; public class FileAnalysisCacheTest { @@ -104,7 +105,7 @@ public class FileAnalysisCacheTest { when(rule.getLanguage()).thenReturn(mock(Language.class)); when(rv.getRule()).thenReturn(rule); - cache.ruleViolationAdded(rv); + cache.startFileAnalysis(mock(DataSource.class)).onRuleViolation(rv); cache.persist(); final FileAnalysisCache reloadedCache = new FileAnalysisCache(newCacheFile); diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/processor/MultiThreadProcessorTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/processor/MultiThreadProcessorTest.java index 6fff6bb7ae..072946d0e9 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/processor/MultiThreadProcessorTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/processor/MultiThreadProcessorTest.java @@ -4,13 +4,12 @@ package net.sourceforge.pmd.processor; +import static net.sourceforge.pmd.util.CollectionUtil.listOf; + import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; -import java.nio.charset.StandardCharsets; import java.util.ArrayList; -import java.util.Collections; -import java.util.Iterator; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; @@ -18,65 +17,69 @@ import org.junit.Assert; import org.junit.Test; import net.sourceforge.pmd.PMDConfiguration; -import net.sourceforge.pmd.Report; -import net.sourceforge.pmd.Report.ConfigurationError; +import net.sourceforge.pmd.Report.GlobalReportBuilder; import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.RuleSetNotFoundException; import net.sourceforge.pmd.RuleSets; import net.sourceforge.pmd.RuleViolation; import net.sourceforge.pmd.RulesetsFactoryUtils; -import net.sourceforge.pmd.ThreadSafeReportListener; +import net.sourceforge.pmd.ThreadSafeAnalysisListener; +import net.sourceforge.pmd.ThreadSafeAnalysisListener.GlobalAnalysisListener; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.rule.AbstractRule; -import net.sourceforge.pmd.renderers.AbstractAccumulatingRenderer; -import net.sourceforge.pmd.renderers.Renderer; import net.sourceforge.pmd.util.datasource.DataSource; -import net.sourceforge.pmd.util.datasource.internal.AbstractDataSource; public class MultiThreadProcessorTest { - private RuleContext ctx; - private MultiThreadProcessor processor; + private GlobalAnalysisListener listener; + private List files; private SimpleReportListener reportListener; + private PMDConfiguration configuration; public RuleSets setUpForTest(final String ruleset) throws RuleSetNotFoundException { - PMDConfiguration configuration = new PMDConfiguration(); + configuration = new PMDConfiguration(); configuration.setThreads(2); files = new ArrayList<>(); - files.add(new StringDataSource("file1-violation.dummy", "ABC")); - files.add(new StringDataSource("file2-foo.dummy", "DEF")); + files.add(DataSource.forString("abc", "file1-violation.dummy")); + files.add(DataSource.forString("DEF", "file2-foo.dummy")); reportListener = new SimpleReportListener(); - ctx = new RuleContext(); - ctx.getReport().addListener(reportListener); + listener = GlobalAnalysisListener.tee(listOf( + new GlobalReportBuilder(), + reportListener + )); - processor = new MultiThreadProcessor(configuration); return RulesetsFactoryUtils.defaultFactory().createRuleSets(ruleset); } - @Test - public void testRulesDysnfunctionalLog() throws Exception { - RuleSets ruleSets = setUpForTest("rulesets/MultiThreadProcessorTest/dysfunctional.xml"); - final SimpleRenderer renderer = new SimpleRenderer(null, null); - renderer.start(); - processor.processFiles(ruleSets, files, ctx, Collections.singletonList(renderer)); - renderer.end(); - - final Iterator configErrors = renderer.getReport().getConfigurationErrors().iterator(); - final ConfigurationError error = configErrors.next(); - - Assert.assertEquals("Dysfunctional rule message not present", - DysfunctionalRule.DYSFUNCTIONAL_RULE_REASON, error.issue()); - Assert.assertEquals("Dysfunctional rule is wrong", - DysfunctionalRule.class, error.rule().getClass()); - Assert.assertFalse("More configuration errors found than expected", configErrors.hasNext()); - } + // Dysfunctional rules are pruned upstream of the processor. + // + // @Test + // public void testRulesDysnfunctionalLog() throws Exception { + // RuleSets ruleSets = setUpForTest("rulesets/MultiThreadProcessorTest/dysfunctional.xml"); + // final SimpleRenderer renderer = new SimpleRenderer(null, null); + // renderer.start(); + // processor.processFiles(ruleSets, files, listener); + // renderer.end(); + // + // final Iterator configErrors = renderer.getReport().getConfigurationErrors().iterator(); + // final ConfigurationError error = configErrors.next(); + // + // Assert.assertEquals("Dysfunctional rule message not present", + // DysfunctionalRule.DYSFUNCTIONAL_RULE_REASON, error.issue()); + // Assert.assertEquals("Dysfunctional rule is wrong", + // DysfunctionalRule.class, error.rule().getClass()); + // Assert.assertFalse("More configuration errors found than expected", configErrors.hasNext()); + // } @Test - public void testRulesThreadSafety() throws RuleSetNotFoundException { + public void testRulesThreadSafety() throws Exception { RuleSets ruleSets = setUpForTest("rulesets/MultiThreadProcessorTest/basic.xml"); - processor.processFiles(ruleSets, files, ctx, Collections.emptyList()); + try (AbstractPMDProcessor processor = AbstractPMDProcessor.newFileProcessor(configuration)) { + processor.processFiles(ruleSets, files, listener); + } + listener.close(); // if the rule is not executed, then maybe a // ConcurrentModificationException happened @@ -86,26 +89,6 @@ public class MultiThreadProcessorTest { Assert.assertEquals("Missing violation", 1, reportListener.violations.get()); } - private static class StringDataSource extends AbstractDataSource { - private final String data; - private final String name; - - StringDataSource(String name, String data) { - this.name = name; - this.data = data; - } - - @Override - public InputStream getInputStream() throws IOException { - return new ByteArrayInputStream(data.getBytes(StandardCharsets.UTF_8)); - } - - @Override - public String getNiceFileName(boolean shortNames, String inputFileName) { - return name; - } - } - public static class NotThreadSafeRule extends AbstractRule { public static AtomicInteger count = new AtomicInteger(0); private boolean hasViolation; // this variable will be overridden @@ -153,33 +136,23 @@ public class MultiThreadProcessorTest { } } - private static class SimpleReportListener implements ThreadSafeReportListener { + private static class SimpleReportListener implements GlobalAnalysisListener { + public AtomicInteger violations = new AtomicInteger(0); @Override - public void ruleViolationAdded(RuleViolation ruleViolation) { - violations.incrementAndGet(); - } - - } - - private static class SimpleRenderer extends AbstractAccumulatingRenderer { - - /* default */ SimpleRenderer(String name, String description) { - super(name, description); + public ThreadSafeAnalysisListener startFileAnalysis(DataSource file) { + return new ThreadSafeAnalysisListener() { + @Override + public void onRuleViolation(RuleViolation violation) { + violations.incrementAndGet(); + } + }; } @Override - public String defaultFileExtension() { - return null; - } + public void close() throws Exception { - @Override - public void end() throws IOException { - } - - public Report getReport() { - return report; } } } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/ExcludeLinesTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/ExcludeLinesTest.java index 7150e95c5a..9075bd0663 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/ExcludeLinesTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/ExcludeLinesTest.java @@ -4,7 +4,6 @@ package net.sourceforge.pmd; -import static java.util.Collections.emptyList; import static net.sourceforge.pmd.util.CollectionUtil.listOf; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; @@ -40,7 +39,6 @@ public class ExcludeLinesTest extends RuleTst { Report r = new PmdRunnable( DataSource.forString(TEST3, "test.java"), - emptyList(), new RuleContext(), listOf(rules), config diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/XPathRuleTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/XPathRuleTest.java index cbbae51162..8fd4473358 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/XPathRuleTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/XPathRuleTest.java @@ -7,7 +7,6 @@ package net.sourceforge.pmd.lang.java.rule; import static net.sourceforge.pmd.util.CollectionUtil.listOf; import static org.junit.Assert.assertEquals; -import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -175,7 +174,6 @@ public class XPathRuleTest extends RuleTst { RuleSet rules = RulesetsFactoryUtils.defaultFactory().createSingleRuleRuleSet(r); return new PmdRunnable( DataSource.forString(test, "test.java"), - Collections.emptyList(), RuleContext.throwingExceptions(), listOf(rules), new PMDConfiguration() diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/xpath/internal/XPathMetricFunctionTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/xpath/internal/XPathMetricFunctionTest.java index e483fa31f2..bee25358a2 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/xpath/internal/XPathMetricFunctionTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/xpath/internal/XPathMetricFunctionTest.java @@ -7,7 +7,6 @@ package net.sourceforge.pmd.lang.java.rule.xpath.internal; import static net.sourceforge.pmd.util.CollectionUtil.listOf; import static org.junit.Assert.assertTrue; -import java.util.Collections; import java.util.Iterator; import org.hamcrest.CoreMatchers; @@ -55,7 +54,6 @@ public class XPathMetricFunctionTest { return new PmdRunnable( DataSource.forString(code, "test.java"), - Collections.emptyList(), RuleContext.throwingExceptions(), listOf(rules), new PMDConfiguration() diff --git a/pmd-jsp/src/test/java/net/sourceforge/pmd/lang/jsp/ast/XPathJspRuleTest.java b/pmd-jsp/src/test/java/net/sourceforge/pmd/lang/jsp/ast/XPathJspRuleTest.java index af52c00058..9e21527423 100644 --- a/pmd-jsp/src/test/java/net/sourceforge/pmd/lang/jsp/ast/XPathJspRuleTest.java +++ b/pmd-jsp/src/test/java/net/sourceforge/pmd/lang/jsp/ast/XPathJspRuleTest.java @@ -7,8 +7,6 @@ package net.sourceforge.pmd.lang.jsp.ast; import static net.sourceforge.pmd.util.CollectionUtil.listOf; import static org.junit.Assert.assertEquals; -import java.util.Collections; - import org.junit.Test; import net.sourceforge.pmd.PMDConfiguration; @@ -43,7 +41,6 @@ public class XPathJspRuleTest extends RuleTst { Report report = new PmdRunnable( DataSource.forString(MATCH, "test.jsp"), - Collections.emptyList(), RuleContext.throwingExceptions(), listOf(rules), new PMDConfiguration() diff --git a/pmd-scala-modules/pmd-scala-common/src/test/java/net/sourceforge/pmd/lang/scala/ast/ScalaParsingHelper.java b/pmd-scala-modules/pmd-scala-common/src/test/java/net/sourceforge/pmd/lang/scala/ast/ScalaParsingHelper.java index 697bdc98b9..251210f4ed 100644 --- a/pmd-scala-modules/pmd-scala-common/src/test/java/net/sourceforge/pmd/lang/scala/ast/ScalaParsingHelper.java +++ b/pmd-scala-modules/pmd-scala-common/src/test/java/net/sourceforge/pmd/lang/scala/ast/ScalaParsingHelper.java @@ -6,8 +6,6 @@ package net.sourceforge.pmd.lang.scala.ast; import static net.sourceforge.pmd.util.CollectionUtil.listOf; -import java.util.Collections; - import net.sourceforge.pmd.PMDConfiguration; import net.sourceforge.pmd.Report; import net.sourceforge.pmd.Rule; @@ -37,7 +35,6 @@ public final class ScalaParsingHelper extends BaseParsingHelper