forked from phoedos/pmd
Merge branch 'bug-1461' into pmd/5.4.x
This commit is contained in:
@ -217,8 +217,7 @@ public class PMD {
|
||||
|
||||
// Load the RuleSets
|
||||
RuleSetFactory ruleSetFactory = RulesetsFactoryUtils.getRulesetFactory(configuration);
|
||||
RuleSets ruleSets = RulesetsFactoryUtils.getRuleSetsWithBenchmark(configuration.getRuleSets(), configuration.getPmdRuleSets(), ruleSetFactory);
|
||||
configuration.setPmdRuleSets(ruleSets);
|
||||
RuleSets ruleSets = RulesetsFactoryUtils.getRuleSetsWithBenchmark(configuration.getRuleSets(), ruleSetFactory);
|
||||
if (ruleSets == null) {
|
||||
return 0;
|
||||
}
|
||||
|
@ -91,7 +91,6 @@ public class PMDConfiguration extends AbstractConfiguration {
|
||||
|
||||
// Rule and source file options
|
||||
private String ruleSets;
|
||||
private RuleSets pmdRuleSets;
|
||||
private RulePriority minimumPriority = RulePriority.LOW;
|
||||
private String inputPaths;
|
||||
private String inputUri;
|
||||
@ -269,24 +268,6 @@ public class PMDConfiguration extends AbstractConfiguration {
|
||||
this.ruleSets = ruleSets;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the RuleSets.
|
||||
*
|
||||
* @return the pmdRuleSets
|
||||
*/
|
||||
public RuleSets getPmdRuleSets() {
|
||||
return pmdRuleSets;
|
||||
}
|
||||
|
||||
/**
|
||||
* Set the RuleSets
|
||||
*
|
||||
* @param pmdRuleSets the pmdRuleSets to set
|
||||
*/
|
||||
public void setPmdRuleSets(RuleSets pmdRuleSets) {
|
||||
this.pmdRuleSets = pmdRuleSets;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the minimum priority threshold when loading Rules from RuleSets.
|
||||
*
|
||||
|
@ -105,9 +105,9 @@ public class RuleSetFactory {
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a RuleSets from a comma separated list of RuleSet reference IDs
|
||||
* when the parameter ruleSets is null. This is a convenience method which
|
||||
* calls {@link RuleSetReferenceId#parse(String)}, and then calls
|
||||
* Create a RuleSets from a comma separated list of RuleSet reference IDs.
|
||||
* This is a convenience method which calls
|
||||
* {@link RuleSetReferenceId#parse(String)}, and then calls
|
||||
* {@link #createRuleSets(List)}. The currently configured ClassLoader is
|
||||
* used.
|
||||
*
|
||||
@ -116,25 +116,6 @@ public class RuleSetFactory {
|
||||
* @throws RuleSetNotFoundException if unable to find a resource.
|
||||
*/
|
||||
public synchronized RuleSets createRuleSets(String referenceString) throws RuleSetNotFoundException {
|
||||
return createRuleSets(referenceString, null);
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a RuleSets from a comma separated list of RuleSet reference IDs when the parameter ruleSets is null.
|
||||
* This is a convenience method which calls
|
||||
* {@link RuleSetReferenceId#parse(String)}, and then calls
|
||||
* {@link #createRuleSets(List)}. The currently configured ClassLoader is
|
||||
* used.
|
||||
*
|
||||
* @param referenceString A comma separated list of RuleSet reference IDs.
|
||||
* @param ruleSets RuleSets initialized in PMDConfiguration.
|
||||
* @return The new RuleSets or the rulesets from PMDConfiguration if not null
|
||||
* @throws RuleSetNotFoundException if unable to find a resource.
|
||||
*/
|
||||
public synchronized RuleSets createRuleSets(String referenceString, RuleSets ruleSets) throws RuleSetNotFoundException {
|
||||
if (ruleSets != null) {
|
||||
return ruleSets;
|
||||
}
|
||||
return createRuleSets(RuleSetReferenceId.parse(referenceString));
|
||||
}
|
||||
|
||||
|
@ -25,24 +25,10 @@ public final class RulesetsFactoryUtils {
|
||||
* ruleset couldn't be found.
|
||||
*/
|
||||
public static RuleSets getRuleSets(String rulesets, RuleSetFactory factory) {
|
||||
return getRuleSets(rulesets, null, factory);
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a new rulesets with the given string. The resulting rulesets will contain
|
||||
* all referenced rulesets.
|
||||
* @param rulesets the string with the rulesets to load
|
||||
* @param factory the ruleset factory
|
||||
* @param pmdRuleSets RuleSets initialized in PMDConfiguration
|
||||
* @return the rulesets
|
||||
* @throws IllegalArgumentException if rulesets is empty (means, no rules have been found) or if a
|
||||
* ruleset couldn't be found.
|
||||
*/
|
||||
public static RuleSets getRuleSets(String rulesets, RuleSets pmdRuleSets, RuleSetFactory factory) {
|
||||
RuleSets ruleSets = null;
|
||||
try {
|
||||
factory.setWarnDeprecated(true);
|
||||
ruleSets = factory.createRuleSets(rulesets, pmdRuleSets);
|
||||
ruleSets = factory.createRuleSets(rulesets);
|
||||
factory.setWarnDeprecated(false);
|
||||
printRuleNamesInDebug(ruleSets);
|
||||
if (ruleSets.ruleCount() == 0) {
|
||||
@ -67,24 +53,10 @@ public final class RulesetsFactoryUtils {
|
||||
* ruleset couldn't be found.
|
||||
*/
|
||||
public static RuleSets getRuleSetsWithBenchmark(String rulesets, RuleSetFactory factory) {
|
||||
return getRuleSetsWithBenchmark(rulesets, null, factory);
|
||||
}
|
||||
|
||||
/**
|
||||
* See {@link #getRuleSets(String, RuleSetFactory)}. In addition, the loading of the rules
|
||||
* is benchmarked.
|
||||
* @param rulesets the string with the rulesets to load
|
||||
* @param pmdRuleSets RuleSets initialized in PMDConfiguration
|
||||
* @param factory the ruleset factory
|
||||
* @return the rulesets
|
||||
* @throws IllegalArgumentException if rulesets is empty (means, no rules have been found) or if a
|
||||
* ruleset couldn't be found.
|
||||
*/
|
||||
public static RuleSets getRuleSetsWithBenchmark(String rulesets, RuleSets pmdRuleSets, RuleSetFactory factory) {
|
||||
long loadRuleStart = System.nanoTime();
|
||||
RuleSets ruleSets = null;
|
||||
try {
|
||||
ruleSets = getRuleSets(rulesets, pmdRuleSets, factory);
|
||||
ruleSets = getRuleSets(rulesets, factory);
|
||||
} finally {
|
||||
long endLoadRules = System.nanoTime();
|
||||
Benchmarker.mark(Benchmark.LoadRules, endLoadRules - loadRuleStart, 0);
|
||||
|
@ -110,7 +110,7 @@ public class PMDTaskImpl {
|
||||
// Substitute env variables/properties
|
||||
configuration.setRuleSets(project.replaceProperties(ruleSets));
|
||||
}
|
||||
RuleSets rules = ruleSetFactory.createRuleSets(configuration.getRuleSets(), configuration.getPmdRuleSets());
|
||||
RuleSets rules = ruleSetFactory.createRuleSets(configuration.getRuleSets());
|
||||
ruleSetFactory.setWarnDeprecated(false);
|
||||
logRulesUsed(rules);
|
||||
} catch (RuleSetNotFoundException e) {
|
||||
|
@ -51,7 +51,7 @@ public abstract class AbstractPMDProcessor {
|
||||
}
|
||||
|
||||
protected RuleSets createRuleSets(RuleSetFactory factory) {
|
||||
return RulesetsFactoryUtils.getRuleSets(configuration.getRuleSets(), configuration.getPmdRuleSets(), factory);
|
||||
return RulesetsFactoryUtils.getRuleSets(configuration.getRuleSets(), factory);
|
||||
}
|
||||
|
||||
}
|
||||
|
@ -39,7 +39,6 @@ public final class MonoThreadProcessor extends AbstractPMDProcessor {
|
||||
// single threaded execution
|
||||
|
||||
RuleSets rs = createRuleSets(ruleSetFactory);
|
||||
configuration.setPmdRuleSets(rs);
|
||||
SourceCodeProcessor processor = new SourceCodeProcessor(configuration);
|
||||
|
||||
for (DataSource dataSource : files) {
|
||||
|
@ -36,7 +36,6 @@ public class MultiThreadProcessor extends AbstractPMDProcessor {
|
||||
final RuleContext ctx, final List<Renderer> renderers) {
|
||||
|
||||
RuleSets rs = createRuleSets(ruleSetFactory);
|
||||
configuration.setPmdRuleSets(rs);
|
||||
rs.start(ctx);
|
||||
|
||||
PmdThreadFactory factory = new PmdThreadFactory(ruleSetFactory, ctx);
|
||||
|
@ -60,8 +60,7 @@ public class PmdRunnable extends PMD implements Callable<Report> {
|
||||
PmdThread thread = (PmdThread) Thread.currentThread();
|
||||
|
||||
RuleContext ctx = thread.getRuleContext();
|
||||
RuleSets rs = thread.getRuleSets(configuration.getRuleSets(), configuration.getPmdRuleSets());
|
||||
configuration.setPmdRuleSets(rs);
|
||||
RuleSets rs = thread.getRuleSets(configuration.getRuleSets());
|
||||
|
||||
Report report = setupReport(rs, ctx, fileName);
|
||||
|
||||
@ -110,10 +109,10 @@ public class PmdRunnable extends PMD implements Callable<Report> {
|
||||
return context;
|
||||
}
|
||||
|
||||
public RuleSets getRuleSets(String rsList, RuleSets ruleSets) {
|
||||
public RuleSets getRuleSets(String rsList) {
|
||||
if (rulesets == null) {
|
||||
try {
|
||||
rulesets = ruleSetFactory.createRuleSets(rsList, ruleSets);
|
||||
rulesets = ruleSetFactory.createRuleSets(rsList);
|
||||
} catch (Exception e) {
|
||||
e.printStackTrace();
|
||||
}
|
||||
|
@ -0,0 +1,109 @@
|
||||
/**
|
||||
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
|
||||
*/
|
||||
package net.sourceforge.pmd.processor;
|
||||
|
||||
import java.io.ByteArrayInputStream;
|
||||
import java.io.IOException;
|
||||
import java.io.InputStream;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
import java.util.concurrent.atomic.AtomicInteger;
|
||||
|
||||
import org.junit.Assert;
|
||||
import org.junit.Test;
|
||||
|
||||
import net.sourceforge.pmd.PMDConfiguration;
|
||||
import net.sourceforge.pmd.ReportListener;
|
||||
import net.sourceforge.pmd.RuleContext;
|
||||
import net.sourceforge.pmd.RuleSetFactory;
|
||||
import net.sourceforge.pmd.RuleViolation;
|
||||
import net.sourceforge.pmd.lang.ast.Node;
|
||||
import net.sourceforge.pmd.lang.rule.AbstractRule;
|
||||
import net.sourceforge.pmd.renderers.Renderer;
|
||||
import net.sourceforge.pmd.stat.Metric;
|
||||
import net.sourceforge.pmd.util.datasource.DataSource;
|
||||
|
||||
public class MultiThreadProcessorTest {
|
||||
|
||||
@Test
|
||||
public void testRulesThreadSafety() {
|
||||
PMDConfiguration configuration = new PMDConfiguration();
|
||||
configuration.setRuleSets("rulesets/MultiThreadProcessorTest/basic.xml");
|
||||
configuration.setThreads(2);
|
||||
List<DataSource> files = new ArrayList<>();
|
||||
files.add(new StringDataSource("file1-violation.dummy", "ABC"));
|
||||
files.add(new StringDataSource("file2-foo.dummy", "DEF"));
|
||||
|
||||
SimpleReportListener reportListener = new SimpleReportListener();
|
||||
RuleContext ctx = new RuleContext();
|
||||
ctx.getReport().addListener(reportListener);
|
||||
|
||||
MultiThreadProcessor processor = new MultiThreadProcessor(configuration);
|
||||
RuleSetFactory ruleSetFactory = new RuleSetFactory();
|
||||
processor.processFiles(ruleSetFactory, files, ctx, Collections.<Renderer>emptyList());
|
||||
|
||||
// if the rule is not executed, then maybe a ConcurrentModificationException happened
|
||||
Assert.assertEquals("Test rule has not been executed", 2, NotThreadSafeRule.count.get());
|
||||
// if the violation is not reported, then the rule instances have been shared between the threads
|
||||
Assert.assertEquals("Missing violation", 1, reportListener.violations.get());
|
||||
}
|
||||
|
||||
private static class StringDataSource implements DataSource {
|
||||
private final String data;
|
||||
private final String name;
|
||||
public StringDataSource(String name, String data) {
|
||||
this.name = name;
|
||||
this.data = data;
|
||||
}
|
||||
@Override
|
||||
public InputStream getInputStream() throws IOException {
|
||||
return new ByteArrayInputStream(data.getBytes("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 between the threads
|
||||
@Override
|
||||
public void apply(List<? extends Node> nodes, RuleContext ctx) {
|
||||
count.incrementAndGet();
|
||||
|
||||
if (ctx.getSourceCodeFilename().contains("violation")) {
|
||||
hasViolation = true;
|
||||
} else {
|
||||
letTheOtherThreadRun(10);
|
||||
hasViolation = false;
|
||||
}
|
||||
|
||||
letTheOtherThreadRun(100);
|
||||
if (hasViolation) {
|
||||
addViolation(ctx, nodes.get(0));
|
||||
}
|
||||
}
|
||||
private void letTheOtherThreadRun(int millis) {
|
||||
try {
|
||||
Thread.yield();
|
||||
Thread.sleep(millis);
|
||||
} catch (InterruptedException e) {
|
||||
// ignored
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private static class SimpleReportListener implements ReportListener {
|
||||
public AtomicInteger violations = new AtomicInteger(0);
|
||||
@Override
|
||||
public void ruleViolationAdded(RuleViolation ruleViolation) {
|
||||
violations.incrementAndGet();
|
||||
}
|
||||
@Override
|
||||
public void metricAdded(Metric metric) {
|
||||
}
|
||||
}
|
||||
}
|
@ -0,0 +1,15 @@
|
||||
<?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 http://pmd.sourceforge.net/ruleset_2_0_0.xsd">
|
||||
|
||||
<description>
|
||||
Ruleset used by test RuleSetReferenceIdTest
|
||||
</description>
|
||||
|
||||
<rule name="NotThreadSafeRule" language="dummy" since="1.0" message="Not thread safe" class="net.sourceforge.pmd.processor.MultiThreadProcessorTest$NotThreadSafeRule"
|
||||
externalInfoUrl="foo">
|
||||
<description>Foo</description>
|
||||
<priority>3</priority>
|
||||
<example></example>
|
||||
</rule>
|
||||
</ruleset>
|
@ -47,6 +47,7 @@
|
||||
* [#1456](https://sourceforge.net/p/pmd/bugs/1456/): UnusedFormalParameter should ignore overriding methods
|
||||
* General
|
||||
* [#1455](https://sourceforge.net/p/pmd/bugs/1455/): PMD doesn't handle Java 8 explicit receiver parameters
|
||||
* [#1461](https://sourceforge.net/p/pmd/bugs/1461/): Possible threading issue due to PR#75
|
||||
|
||||
**API Changes:**
|
||||
|
||||
|
Reference in New Issue
Block a user