Merge branch 'thread-safe-rsfactory' of https://github.com/Monits/pmd into pr-131

pmd-core/src/main/java/net/sourceforge/pmd/PMD.java

@@ -358,16 +358,18 @@ public class PMD {
 
         // Make sure the cache is listening for analysis results
         ctx.getReport().addListener(configuration.getAnalysisCache());
-        
+
+        final RuleSetFactory silentFactoy = new RuleSetFactory(ruleSetFactory, false);
+
         /*
          * Check if multithreaded support is available. ExecutorService can also
          * be disabled if threadCount is not positive, e.g. using the
          * "-threads 0" command line option.
          */
         if (configuration.getThreads() > 0) {
-            new MultiThreadProcessor(configuration).processFiles(ruleSetFactory, files, ctx, renderers);
+            new MultiThreadProcessor(configuration).processFiles(silentFactoy, files, ctx, renderers);
         } else {
-            new MonoThreadProcessor(configuration).processFiles(ruleSetFactory, files, ctx, renderers);
+            new MonoThreadProcessor(configuration).processFiles(silentFactoy, files, ctx, renderers);
         }
         
         // Persist the analysis cache

pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java

@@ -57,53 +57,43 @@ public class RuleSetFactory {
     private static final String MESSAGE = "message";
     private static final String EXTERNAL_INFO_URL = "externalInfoUrl";
 
-    private ClassLoader classLoader = RuleSetFactory.class.getClassLoader();
+    private final ClassLoader classLoader;
-    private RulePriority minimumPriority = RulePriority.LOW;
+    private final RulePriority minimumPriority;
-    private boolean warnDeprecated = false;
+    private final boolean warnDeprecated;
-    private RuleSetFactoryCompatibility compatibilityFilter = new RuleSetFactoryCompatibility();
+    private final RuleSetFactoryCompatibility compatibilityFilter;
 
-    /**
+    public RuleSetFactory() {
-     * Set the ClassLoader to use when loading Rules.
+        this(RuleSetFactory.class.getClassLoader(), RulePriority.LOW, false, true);
-     *
+    }
-     * @param classLoader The ClassLoader to use.
+
-     */
+    public RuleSetFactory(final ClassLoader classLoader, final RulePriority minimumPriority,
-    public void setClassLoader(ClassLoader classLoader) {
+            final boolean warnDeprecated, final boolean enableCompatibility) {
         this.classLoader = classLoader;
-    }
-
-    /**
-     * Set the minimum rule priority threshold for all Rules which are loaded
-     * from RuleSets via reference.
-     * 
-     * @param minimumPriority The minimum priority.
-     */
-    public void setMinimumPriority(RulePriority minimumPriority) {
         this.minimumPriority = minimumPriority;
-    }
-
-    /**
-     * Set whether warning messages should be logged for usage of deprecated
-     * Rules.
-     * 
-     * @param warnDeprecated <code>true</code> to log warning messages.
-     */
-    public void setWarnDeprecated(boolean warnDeprecated) {
         this.warnDeprecated = warnDeprecated;
+        
+        if (enableCompatibility) {
+            this.compatibilityFilter = new RuleSetFactoryCompatibility();
+        } else {
+            this.compatibilityFilter = null;
+        }
     }
-
+    
     /**
-     * Disable the ruleset compatibility filter. Disabling this filter will cause
+     * Constructor copying all configuration from another factory.
-     * exception when loading a ruleset, which uses references to old/not existing rules.
+     * @param factory The factory whose configuration to copy.
+     * @param warnDeprecated Whether deprecation warnings are to be produced by this factory.
      */
-    public void disableCompatibilityFilter() {
+    public RuleSetFactory(final RuleSetFactory factory, final boolean warnDeprecated) {
-        compatibilityFilter = null;
+        this(factory.classLoader, factory.minimumPriority,
+                warnDeprecated, factory.compatibilityFilter != null);
     }
 
     /**
      * Gets the compatibility filter in order to adjust it, e.g. add additional filters.
      * @return the {@link RuleSetFactoryCompatibility}
      */
-    public RuleSetFactoryCompatibility getCompatibilityFilter() {
+    /* package */ RuleSetFactoryCompatibility getCompatibilityFilter() {
         return compatibilityFilter;
     }
 
@@ -143,7 +133,7 @@ public class RuleSetFactory {
      * @return The new RuleSets.
      * @throws RuleSetNotFoundException if unable to find a resource.
      */
-    public synchronized RuleSets createRuleSets(String referenceString) throws RuleSetNotFoundException {
+    public RuleSets createRuleSets(String referenceString) throws RuleSetNotFoundException {
         return createRuleSets(RuleSetReferenceId.parse(referenceString));
     }
 
@@ -156,7 +146,7 @@ public class RuleSetFactory {
      * @return The new RuleSets.
      * @throws RuleSetNotFoundException if unable to find a resource.
      */
-    public synchronized RuleSets createRuleSets(List<RuleSetReferenceId> ruleSetReferenceIds)
+    public RuleSets createRuleSets(List<RuleSetReferenceId> ruleSetReferenceIds)
             throws RuleSetNotFoundException {
         RuleSets ruleSets = new RuleSets();
         for (RuleSetReferenceId ruleSetReferenceId : ruleSetReferenceIds) {
@@ -177,7 +167,7 @@ public class RuleSetFactory {
      * @return A new RuleSet.
      * @throws RuleSetNotFoundException if unable to find a resource.
      */
-    public synchronized RuleSet createRuleSet(String referenceString) throws RuleSetNotFoundException {
+    public RuleSet createRuleSet(String referenceString) throws RuleSetNotFoundException {
         List<RuleSetReferenceId> references = RuleSetReferenceId.parse(referenceString);
         if (references.isEmpty()) {
             throw new RuleSetNotFoundException("No RuleSetReferenceId can be parsed from the string: <"
@@ -195,11 +185,11 @@ public class RuleSetFactory {
      * @return A new RuleSet.
      * @throws RuleSetNotFoundException if unable to find a resource.
      */
-    public synchronized RuleSet createRuleSet(RuleSetReferenceId ruleSetReferenceId) throws RuleSetNotFoundException {
+    public RuleSet createRuleSet(RuleSetReferenceId ruleSetReferenceId) throws RuleSetNotFoundException {
         return createRuleSet(ruleSetReferenceId, false);
     }
 
-    private synchronized RuleSet createRuleSet(RuleSetReferenceId ruleSetReferenceId,
+    private RuleSet createRuleSet(RuleSetReferenceId ruleSetReferenceId,
             boolean withDeprecatedRuleReferences) throws RuleSetNotFoundException {
         return parseRuleSetNode(ruleSetReferenceId, withDeprecatedRuleReferences);
     }
@@ -359,13 +349,11 @@ public class RuleSetFactory {
             }
         }
 
-        RuleSetFactory ruleSetFactory = new RuleSetFactory();
+        RuleSetFactory ruleSetFactory = new RuleSetFactory(this, warnDeprecated);
-        ruleSetFactory.setClassLoader(classLoader);
         RuleSet otherRuleSet = ruleSetFactory.createRuleSet(RuleSetReferenceId.parse(ref).get(0));
         for (Rule rule : otherRuleSet.getRules()) {
             excludedRulesCheck.remove(rule.getName());
-            if (!ruleSetReference.getExcludes().contains(rule.getName())
+            if (!ruleSetReference.getExcludes().contains(rule.getName()) && !rule.isDeprecated()) {
-                    && rule.getPriority().compareTo(minimumPriority) <= 0 && !rule.isDeprecated()) {
                 RuleReference ruleReference = new RuleReference();
                 ruleReference.setRuleSetReference(ruleSetReference);
                 ruleReference.setRule(rule);
@@ -530,8 +518,7 @@ public class RuleSetFactory {
             return;
         }
 
-        RuleSetFactory ruleSetFactory = new RuleSetFactory();
+        RuleSetFactory ruleSetFactory = new RuleSetFactory(this, warnDeprecated);
-        ruleSetFactory.setClassLoader(classLoader);
 
         boolean isSameRuleSet = false;
         RuleSetReferenceId otherRuleSetReferenceId = RuleSetReferenceId.parse(ref).get(0);

pmd-core/src/main/java/net/sourceforge/pmd/RulesetsFactoryUtils.java

@@ -27,9 +27,7 @@ public final class RulesetsFactoryUtils {
     public static RuleSets getRuleSets(String rulesets, RuleSetFactory factory) {
         RuleSets ruleSets = null;
         try {
-            factory.setWarnDeprecated(true);
             ruleSets = factory.createRuleSets(rulesets);
-            factory.setWarnDeprecated(false);
             printRuleNamesInDebug(ruleSets);
             if (ruleSets.ruleCount() == 0) {
                 String msg = "No rules found. Maybe you mispelled a rule name? (" + rulesets + ")";
@@ -64,13 +62,12 @@ public final class RulesetsFactoryUtils {
         return ruleSets;
     }
 
-	public static RuleSetFactory getRulesetFactory(PMDConfiguration configuration) {
+	public static RuleSetFactory getRulesetFactory(final PMDConfiguration configuration) {
-		RuleSetFactory ruleSetFactory = new RuleSetFactory();
+	    return new RuleSetFactory(
-		ruleSetFactory.setMinimumPriority(configuration.getMinimumPriority());
+            configuration.getClassLoader(),
-		if (!configuration.isRuleSetFactoryCompatibilityEnabled()) {
+            configuration.getMinimumPriority(),
-		    ruleSetFactory.disableCompatibilityFilter();
+	        true,
-		}
+	        configuration.isRuleSetFactoryCompatibilityEnabled());
-		return ruleSetFactory;
 	}
 
 	/**

pmd-core/src/main/java/net/sourceforge/pmd/ant/internal/PMDTaskImpl.java

@@ -33,6 +33,7 @@ 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.ant.Formatter;
 import net.sourceforge.pmd.ant.PMDTask;
 import net.sourceforge.pmd.ant.SourceLanguage;
@@ -102,23 +103,15 @@ public class PMDTaskImpl {
         setupClassLoader();
 
         // Setup RuleSetFactory and validate RuleSets
-        RuleSetFactory ruleSetFactory = new RuleSetFactory();
+        RuleSetFactory ruleSetFactory = RulesetsFactoryUtils.getRulesetFactory(configuration);
-        ruleSetFactory.setClassLoader(configuration.getClassLoader());
-        if (!configuration.isRuleSetFactoryCompatibilityEnabled()) {
-            ruleSetFactory.disableCompatibilityFilter();
-        }
         try {
-            // This is just used to validate and display rules. Each thread will
+            // This is just used to validate and display rules. Each thread will create its own ruleset
-            // create its own ruleset
-            ruleSetFactory.setMinimumPriority(configuration.getMinimumPriority());
-            ruleSetFactory.setWarnDeprecated(true);
             String ruleSets = configuration.getRuleSets();
             if (StringUtil.isNotEmpty(ruleSets)) {
                 // Substitute env variables/properties
                 configuration.setRuleSets(project.replaceProperties(ruleSets));
             }
             RuleSets rules = ruleSetFactory.createRuleSets(configuration.getRuleSets());
-            ruleSetFactory.setWarnDeprecated(false);
             logRulesUsed(rules);
         } catch (RuleSetNotFoundException e) {
             throw new BuildException(e.getMessage(), e);

pmd-core/src/main/java/net/sourceforge/pmd/benchmark/Benchmarker.java

@@ -20,6 +20,7 @@ import net.sourceforge.pmd.PMDConfiguration;
 import net.sourceforge.pmd.PMDException;
 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;

pmd-core/src/main/java/net/sourceforge/pmd/processor/MultiThreadProcessor.java

@@ -4,7 +4,6 @@
 package net.sourceforge.pmd.processor;
 
 import java.util.ArrayList;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;

pmd-core/src/main/java/net/sourceforge/pmd/processor/PmdThreadFactory.java

@@ -3,9 +3,6 @@
  */
 package net.sourceforge.pmd.processor;
 
-import java.util.Collections;
-import java.util.LinkedList;
-import java.util.List;
 import java.util.concurrent.ThreadFactory;
 import java.util.concurrent.atomic.AtomicInteger;
 
@@ -17,7 +14,6 @@ public class PmdThreadFactory implements ThreadFactory {
     private final RuleSetFactory ruleSetFactory;
     private final RuleContext ctx;
     private final AtomicInteger counter = new AtomicInteger();
-    public List<Runnable> threadList = Collections.synchronizedList(new LinkedList<Runnable>());
 
     public PmdThreadFactory(RuleSetFactory ruleSetFactory, RuleContext ctx) {
         this.ruleSetFactory = ruleSetFactory;
@@ -27,7 +23,6 @@ public class PmdThreadFactory implements ThreadFactory {
     @Override
     public Thread newThread(Runnable r) {
         Thread t = PmdRunnable.createThread(counter.incrementAndGet(), r, ruleSetFactory, ctx);
-        threadList.add(t);
         return t;
     }
 

pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryTest.java

@@ -342,9 +342,8 @@ public class RuleSetFactoryTest {
 
 	@Test
 	public void testReferencePriority() throws RuleSetNotFoundException {
-		RuleSetFactory rsf = new RuleSetFactory();
+		RuleSetFactory rsf = new RuleSetFactory(getClass().getClassLoader(), RulePriority.LOW, false, true);
 
-		rsf.setMinimumPriority(RulePriority.LOW);
 		RuleSet ruleSet = rsf
 				.createRuleSet(createRuleSetReferenceId(REF_INTERNAL_TO_INTERNAL_CHAIN));
 		assertEquals("Number of Rules", 3, ruleSet.getRules().size());
@@ -352,20 +351,20 @@ public class RuleSetFactoryTest {
 		assertNotNull(ruleSet.getRuleByName("MockRuleNameRef"));
 		assertNotNull(ruleSet.getRuleByName("MockRuleNameRefRef"));
 
-		rsf.setMinimumPriority(RulePriority.MEDIUM_HIGH);
+		rsf = new RuleSetFactory(getClass().getClassLoader(), RulePriority.MEDIUM_HIGH, false, true);
 		ruleSet = rsf
 				.createRuleSet(createRuleSetReferenceId(REF_INTERNAL_TO_INTERNAL_CHAIN));
 		assertEquals("Number of Rules", 2, ruleSet.getRules().size());
 		assertNotNull(ruleSet.getRuleByName("MockRuleNameRef"));
 		assertNotNull(ruleSet.getRuleByName("MockRuleNameRefRef"));
 
-		rsf.setMinimumPriority(RulePriority.HIGH);
+		rsf = new RuleSetFactory(getClass().getClassLoader(), RulePriority.HIGH, false, true);
 		ruleSet = rsf
 				.createRuleSet(createRuleSetReferenceId(REF_INTERNAL_TO_INTERNAL_CHAIN));
 		assertEquals("Number of Rules", 1, ruleSet.getRules().size());
 		assertNotNull(ruleSet.getRuleByName("MockRuleNameRefRef"));
 
-		rsf.setMinimumPriority(RulePriority.LOW);
+		rsf = new RuleSetFactory(getClass().getClassLoader(), RulePriority.LOW, false, true);
 		ruleSet = rsf
 				.createRuleSet(createRuleSetReferenceId(REF_INTERNAL_TO_EXTERNAL_CHAIN));
 		assertEquals("Number of Rules", 3, ruleSet.getRules().size());
@@ -373,14 +372,14 @@ public class RuleSetFactoryTest {
 		assertNotNull(ruleSet.getRuleByName("ExternalRefRuleNameRef"));
 		assertNotNull(ruleSet.getRuleByName("ExternalRefRuleNameRefRef"));
 
-		rsf.setMinimumPriority(RulePriority.MEDIUM_HIGH);
+		rsf = new RuleSetFactory(getClass().getClassLoader(), RulePriority.MEDIUM_HIGH, false, true);
 		ruleSet = rsf
 				.createRuleSet(createRuleSetReferenceId(REF_INTERNAL_TO_EXTERNAL_CHAIN));
 		assertEquals("Number of Rules", 2, ruleSet.getRules().size());
 		assertNotNull(ruleSet.getRuleByName("ExternalRefRuleNameRef"));
 		assertNotNull(ruleSet.getRuleByName("ExternalRefRuleNameRefRef"));
 
-		rsf.setMinimumPriority(RulePriority.HIGH);
+		rsf = new RuleSetFactory(getClass().getClassLoader(), RulePriority.HIGH, false, true);
 		ruleSet = rsf
 				.createRuleSet(createRuleSetReferenceId(REF_INTERNAL_TO_EXTERNAL_CHAIN));
 		assertEquals("Number of Rules", 1, ruleSet.getRules().size());
@@ -407,11 +406,10 @@ public class RuleSetFactoryTest {
 
 	@Test
 	public void testSetPriority() throws RuleSetNotFoundException {
-		RuleSetFactory rsf = new RuleSetFactory();
+		RuleSetFactory rsf = new RuleSetFactory(getClass().getClassLoader(), RulePriority.MEDIUM_HIGH, false, true);
-		rsf.setMinimumPriority(RulePriority.MEDIUM_HIGH);
 		assertEquals(0, rsf
 				.createRuleSet(createRuleSetReferenceId(SINGLE_RULE)).size());
-		rsf.setMinimumPriority(RulePriority.MEDIUM_LOW);
+		rsf = new RuleSetFactory(getClass().getClassLoader(), RulePriority.MEDIUM_LOW, false, true);
 		assertEquals(1, rsf
 				.createRuleSet(createRuleSetReferenceId(SINGLE_RULE)).size());
 	}

src/site/markdown/overview/changelog.md

@@ -36,6 +36,7 @@
 *   [#128](https://github.com/pmd/pmd/pull/128): \[java] Minor optimizations to type resolution
 *   [#129](https://github.com/pmd/pmd/pull/129): \[plsql] Added correct parse of IS [NOT] NULL and multiline DML
 *   [#130](https://github.com/pmd/pmd/pull/130); \[core] Reduce thread contention
+*   [#131](https://github.com/pmd/pmd/pull/131): \[core] Make RuleSetFactory immutable
 *   [#135](https://github.com/pmd/pmd/pull/135): \[apex] New ruleset for Apex security
 
 **Bugfixes:**