hiifong/pmd
1
0
Fork 0
forked from phoedos/pmd
Code Pull Requests Activity

Cleanup Rule methods

Browse Source
This commit is contained in:
Clément Fournier
2020-02-15 03:58:18 +01:00
parent c6965c62a0
commit f27d5fb5f6
13 changed files with 98 additions and 135 deletions
Download Patch File Download Diff File Expand all files Collapse all files

70
pmd-core/src/main/java/net/sourceforge/pmd/Rule.java
View File

@@ -4,8 +4,8 @@
package net.sourceforge.pmd;
import java.util.Collection;
import java.util.List;
import java.util.Set;
import net.sourceforge.pmd.annotation.Experimental;
import net.sourceforge.pmd.lang.Language;
@@ -14,6 +14,8 @@ import net.sourceforge.pmd.lang.ParserOptions;
import net.sourceforge.pmd.lang.ast.AstProcessingStage;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.rule.internal.TargetSelectionStrategy;
import net.sourceforge.pmd.lang.rule.internal.TargetSelectionStrategy.ClassRulechainVisits;
import net.sourceforge.pmd.lang.rule.internal.TargetSelectionStrategy.StringRulechainVisits;
import net.sourceforge.pmd.properties.PropertySource;
import net.sourceforge.pmd.properties.StringProperty;
@@ -289,57 +291,20 @@ public interface Rule extends PropertySource {
/**
* Gets whether this Rule uses the RuleChain.
*
* @return <code>true</code> if RuleChain is used.
* Returns the object that selects the nodes to which this rule applies
* in a tree.
*/
default boolean isRuleChain() {
return !getRuleChainVisits().isEmpty() || !getClassRuleChainVisits().isEmpty();
}
/**
* Gets the collection of AST node names visited by the Rule on the
* RuleChain.
*
* @return the list of AST node names
*/
Set<String> getRuleChainVisits();
Set<Class<?>> getClassRuleChainVisits();
/**
* Adds an AST node by class to be visited by the Rule on the RuleChain.
*
* @param nodeClass
* the AST node to add to the RuleChain visit list
*/
void addRuleChainVisit(Class<? extends Node> nodeClass);
/**
* Adds an AST node by XPath name to be visited by the Rule on the RuleChain.
*
* @param astNodeName
* the XPath name of the node to add to the RuleChain visit list
*
* @see Node#getXPathNodeName()
*/
void addRuleChainVisit(String astNodeName);
TargetSelectionStrategy getTargetingStrategy();
/**
* Start processing. Called once, before apply() is first called.
*
* @param ctx
* the rule context
* @param ctx the rule context
*/
void start(RuleContext ctx);
/**
* Apply this rule to the given collection of nodes, using the given
* context.
@@ -365,4 +330,25 @@ public interface Rule extends PropertySource {
* @return A new exact copy of this rule
*/
Rule deepCopy();
static TargetSelectionStrategy visitNodesNamed(Collection<String> names) {
if (names.isEmpty()) {
throw new IllegalArgumentException("Cannot visit zero nodes");
}
return new StringRulechainVisits(names);
}
static TargetSelectionStrategy visitNodesWithType(Collection<Class<?>> types) {
if (types.isEmpty()) {
throw new IllegalArgumentException("Cannot visit zero types");
}
return new ClassRulechainVisits(types);
}
static TargetSelectionStrategy visitRootOnly() {
return ClassRulechainVisits.ROOT_ONLY;
}
}

1
pmd-core/src/main/java/net/sourceforge/pmd/RuleSets.java
View File

@@ -13,7 +13,6 @@ import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import net.sourceforge.pmd.lang.Language;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.rule.internal.RuleApplicator;

25
pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/AbstractDelegateRule.java
View File

@@ -235,31 +235,6 @@ public abstract class AbstractDelegateRule implements Rule {
return rule.getPropertiesByPropertyDescriptor();
}
@Override
public boolean isRuleChain() {
return rule.isRuleChain();
}
@Override
public Set<String> getRuleChainVisits() {
return rule.getRuleChainVisits();
}
@Override
public Set<Class<?>> getClassRuleChainVisits() {
return rule.getClassRuleChainVisits();
}
@Override
public void addRuleChainVisit(Class<? extends Node> nodeClass) {
rule.addRuleChainVisit(nodeClass);
}
@Override
public void addRuleChainVisit(String astNodeName) {
rule.addRuleChainVisit(astNodeName);
}
@Override
public TargetSelectionStrategy getTargetingStrategy() {
return rule.getTargetingStrategy();

45
pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/AbstractRule.java
View File

@@ -22,7 +22,6 @@ import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.ast.RootNode;
import net.sourceforge.pmd.lang.rule.internal.TargetSelectionStrategy;
import net.sourceforge.pmd.lang.rule.internal.TargetSelectionStrategy.ClassRulechainVisits;
import net.sourceforge.pmd.lang.rule.internal.TargetSelectionStrategy.StringRulechainVisits;
import net.sourceforge.pmd.properties.AbstractPropertySource;
import net.sourceforge.pmd.properties.PropertyDescriptor;
@@ -235,53 +234,37 @@ public abstract class AbstractRule extends AbstractPropertySource implements Rul
return new ParserOptions();
}
@Override
public boolean isRuleChain() {
return !getRuleChainVisits().isEmpty();
}
@Override
public Set<String> getRuleChainVisits() {
return ruleChainVisits;
}
@Override
public Set<Class<?>> getClassRuleChainVisits() {
protected Set<Class<?>> getClassRuleChainVisits() {
if (classRuleChainVisits.isEmpty() && ruleChainVisits.isEmpty()) {
return Collections.singleton(RootNode.class);
}
return classRuleChainVisits;
}
@Override
public void addRuleChainVisit(Class<? extends Node> nodeClass) {
/**
* @deprecated Override {@link #buildTargetingStrategy()}, this is
* provided for legacy compatibility
*/
@Deprecated
protected void addRuleChainVisit(Class<? extends Node> nodeClass) {
classRuleChainVisits.add(nodeClass);
}
@Override
public void addRuleChainVisit(String astNodeName) {
ruleChainVisits.add(astNodeName);
}
@Override
public TargetSelectionStrategy getTargetingStrategy() {
public final TargetSelectionStrategy getTargetingStrategy() {
if (myStrategy == null) {
myStrategy = buildSelectionStrat();
myStrategy = buildTargetingStrategy();
}
return myStrategy;
}
@NonNull
private TargetSelectionStrategy buildSelectionStrat() {
Set<String> rvs = getRuleChainVisits();
protected TargetSelectionStrategy buildTargetingStrategy() {
Set<Class<?>> crvs = getClassRuleChainVisits();
if (rvs.isEmpty() && crvs.isEmpty()) {
return ClassRulechainVisits.ROOT_ONLY;
} else if (rvs.isEmpty()) {
return new ClassRulechainVisits(crvs);
} else {
return new StringRulechainVisits(rvs);
}
return crvs.isEmpty() ? ClassRulechainVisits.ROOT_ONLY
: Rule.visitNodesWithType(crvs);
}
@Override
@@ -426,4 +409,6 @@ public abstract class AbstractRule extends AbstractPropertySource implements Rul
}
return rule;
}
}

20
pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/XPathRule.java
View File

@@ -13,16 +13,17 @@ import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.logging.Level;
import java.util.logging.Logger;
import org.apache.commons.lang3.StringUtils;
import org.checkerframework.checker.nullness.qual.NonNull;
import net.sourceforge.pmd.Rule;
import net.sourceforge.pmd.RuleContext;
import net.sourceforge.pmd.lang.ast.AstProcessingStage;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.rule.internal.TargetSelectionStrategy;
import net.sourceforge.pmd.lang.ast.xpath.internal.DeprecatedAttrLogger;
import net.sourceforge.pmd.lang.rule.xpath.JaxenXPathRuleQuery;
import net.sourceforge.pmd.lang.rule.xpath.SaxonXPathRuleQuery;
@@ -72,7 +73,7 @@ public class XPathRule extends AbstractRule {
.build();
/**
* This is initialized only once when calling {@link #evaluate(Node, RuleContext)} or {@link #getRuleChainVisits()}.
* This is initialized only once when calling {@link #evaluate(Node, RuleContext)} {@link #getTargetingStrategy()}.
*/
private XPathRuleQuery xpathRuleQuery;
@@ -216,16 +217,17 @@ public class XPathRule extends AbstractRule {
}
@Override
public Set<String> getRuleChainVisits() {
protected @NonNull TargetSelectionStrategy buildTargetingStrategy() {
if (xPathRuleQueryNeedsInitialization()) {
initXPathRuleQuery();
for (String nodeName : xpathRuleQuery.getRuleChainVisits()) {
super.addRuleChainVisit(nodeName);
}
logXPathRuleChainUsage(!xpathRuleQuery.getRuleChainVisits().isEmpty());
}
return super.getRuleChainVisits();
List<String> visits = xpathRuleQuery.getRuleChainVisits();
logXPathRuleChainUsage(!visits.isEmpty());
return visits.isEmpty() ? Rule.visitRootOnly()
: Rule.visitNodesNamed(visits);
}

13
pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/internal/TargetSelectionStrategy.java
View File

@@ -5,7 +5,9 @@
package net.sourceforge.pmd.lang.rule.internal;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Set;
@@ -28,8 +30,8 @@ public abstract class TargetSelectionStrategy {
private final Set<String> visits;
public StringRulechainVisits(Set<String> visits) {
this.visits = visits;
public StringRulechainVisits(Collection<String> visits) {
this.visits = new HashSet<>(visits);
}
@@ -43,13 +45,12 @@ public abstract class TargetSelectionStrategy {
public static final TargetSelectionStrategy ROOT_ONLY = new ClassRulechainVisits(Collections.singleton(RootNode.class));
private final Set<? extends Class<?>> visits;
private final Set<Class<?>> visits;
public ClassRulechainVisits(Set<? extends Class<?>> visits) {
this.visits = visits;
public ClassRulechainVisits(Collection<Class<?>> visits) {
this.visits = new HashSet<>(visits);
}
@Override
Iterator<? extends Node> getVisitedNodes(NodeIdx index) {
return index.getByClass(visits).iterator();

9
pmd-core/src/main/java/net/sourceforge/pmd/util/CollectionUtil.java
View File

@@ -183,15 +183,6 @@ public final class CollectionUtil {
return map;
}
@SafeVarargs
public static <T> Set<T> setOf(T first, T... ts) {
LinkedHashSet<T> set = new LinkedHashSet<>(ts.length + 1);
set.add(first);
Collections.addAll(set, ts);
return set;
}
/**
* Consumes all the elements of the iterator and
* returns a list containing them. The iterator is

1
pmd-core/src/test/java/net/sourceforge/pmd/AbstractRuleTest.java
View File

@@ -221,7 +221,6 @@ public class AbstractRuleTest {
assertEquals(r1.getName(), r2.getName());
assertEquals(r1.getPriority(), r2.getPriority());
assertEquals(r1.getPropertyDescriptors(), r2.getPropertyDescriptors());
assertEquals(r1.getRuleChainVisits(), r2.getRuleChainVisits());
assertEquals(r1.getRuleClass(), r2.getRuleClass());
assertEquals(r1.getRuleSetName(), r2.getRuleSetName());
assertEquals(r1.getSince(), r2.getSince());

11
pmd-core/src/test/java/net/sourceforge/pmd/FooRule.java
View File

@@ -4,22 +4,33 @@
package net.sourceforge.pmd;
import static net.sourceforge.pmd.util.CollectionUtil.setOf;
import java.util.List;
import org.checkerframework.checker.nullness.qual.NonNull;
import net.sourceforge.pmd.lang.DummyLanguageModule;
import net.sourceforge.pmd.lang.LanguageRegistry;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.rule.AbstractRule;
import net.sourceforge.pmd.lang.rule.internal.TargetSelectionStrategy;
/**
* Sample rule that detect any node with an image of "Foo". Used for testing.
*/
public class FooRule extends AbstractRule {
public FooRule() {
setLanguage(LanguageRegistry.getLanguage(DummyLanguageModule.NAME));
setName("Foo");
}
@Override
protected @NonNull TargetSelectionStrategy buildTargetingStrategy() {
return Rule.visitNodesNamed(setOf("dummyNode"));
}
@Override
public String getMessage() {
return "blah";

18
pmd-core/src/test/java/net/sourceforge/pmd/RuleSetTest.java
View File

@@ -4,6 +4,7 @@
package net.sourceforge.pmd;
import static net.sourceforge.pmd.util.CollectionUtil.setOf;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
@@ -22,6 +23,7 @@ import java.util.Random;
import java.util.Set;
import java.util.regex.Pattern;
import org.checkerframework.checker.nullness.qual.NonNull;
import org.junit.Test;
import net.sourceforge.pmd.Report.ProcessingError;
@@ -35,6 +37,7 @@ import net.sourceforge.pmd.lang.ast.DummyRoot;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.rule.MockRule;
import net.sourceforge.pmd.lang.rule.RuleReference;
import net.sourceforge.pmd.lang.rule.internal.TargetSelectionStrategy;
public class RuleSetTest {
@@ -402,8 +405,6 @@ public class RuleSetTest {
Rule rule = new FooRule();
rule.setName("FooRule1");
rule.setLanguage(LanguageRegistry.getLanguage(DummyLanguageModule.NAME));
rule.addRuleChainVisit("dummyRootNode");
assertTrue("RuleChain rule", rule.isRuleChain());
RuleSet ruleSet1 = createRuleSetBuilder("RuleSet1")
.addRule(rule)
.build();
@@ -446,7 +447,6 @@ public class RuleSetTest {
Rule rule = new FooRule();
rule.setName("FooRule1");
rule.setLanguage(LanguageRegistry.getLanguage(DummyLanguageModule.NAME));
rule.addRuleChainVisit("dummyNode");
RuleSet ruleSet1 = createRuleSetBuilder("RuleSet1")
.addRule(rule)
.build();
@@ -568,8 +568,10 @@ public class RuleSetTest {
@Test
public void ruleExceptionShouldNotStopProcessingFileWithRuleChain() {
RuleSet ruleset = createRuleSetBuilder("ruleExceptionShouldBeReported").addRule(new MockRule() {
{
addRuleChainVisit("dummyRootNode");
@Override
protected @NonNull TargetSelectionStrategy buildTargetingStrategy() {
return Rule.visitNodesNamed(setOf("dummyRootNode"));
}
@Override
@@ -577,8 +579,10 @@ public class RuleSetTest {
throw new RuntimeException("Test exception while applying rule");
}
}).addRule(new MockRule() {
{
addRuleChainVisit("dummyRootNode");
@Override
protected @NonNull TargetSelectionStrategy buildTargetingStrategy() {
return Rule.visitNodesNamed(setOf("dummyRootNode"));
}
@Override

11
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/FormalParameterNamingConventionsRule.java
View File

@@ -5,9 +5,14 @@
package net.sourceforge.pmd.lang.java.rule.codestyle;
import static net.sourceforge.pmd.util.CollectionUtil.setOf;
import java.util.regex.Pattern;
import org.checkerframework.checker.nullness.qual.NonNull;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
import net.sourceforge.pmd.lang.rule.internal.TargetSelectionStrategy;
import net.sourceforge.pmd.properties.PropertyDescriptor;
@@ -34,10 +39,12 @@ public final class FormalParameterNamingConventionsRule extends AbstractNamingCo
definePropertyDescriptor(finalFormalParamRegex);
definePropertyDescriptor(lambdaParamRegex);
definePropertyDescriptor(explicitLambdaParamRegex);
addRuleChainVisit(ASTVariableDeclaratorId.class);
}
@Override
protected @NonNull TargetSelectionStrategy buildTargetingStrategy() {
return visitNodesWithType(setOf(ASTVariableDeclaratorId.class));
}
@Override
public Object visit(ASTVariableDeclaratorId node, Object data) {

2
pmd-test/src/main/java/net/sourceforge/pmd/AbstractLanguageVersionTest.java
View File

@@ -119,7 +119,7 @@ public class AbstractLanguageVersionTest {
@Test
public void testOldRegisteredRulesets() throws Exception {
// only check for languages, that support rules
if (expected == null || expected.getLanguage().getRuleChainVisitorClass() == null) {
if (expected == null) {
return;
}

7
pmd-test/src/test/java/net/sourceforge/pmd/testframework/RuleTstTest.java
View File

@@ -43,13 +43,14 @@ public class RuleTstTest {
Report report = new Report();
when(rule.getLanguage()).thenReturn(dummyLanguage.getLanguage());
when(rule.getName()).thenReturn("test rule");
when(rule.getTargetingStrategy()).thenReturn(Rule.visitRootOnly());
ruleTester.runTestFromString("the code", rule, report, dummyLanguage, false);
verify(rule).start(any(RuleContext.class));
verify(rule).end(any(RuleContext.class));
verify(rule, times(2)).getLanguage();
verify(rule, times(2)).isRuleChain();
verify(rule).getLanguage();
verify(rule).getTargetingStrategy();
verify(rule).getMinimumLanguageVersion();
verify(rule).getMaximumLanguageVersion();
verify(rule).apply(anyList(), any(RuleContext.class));
@@ -62,6 +63,8 @@ public class RuleTstTest {
public void shouldAssertLinenumbersSorted() {
when(rule.getLanguage()).thenReturn(dummyLanguage.getLanguage());
when(rule.getName()).thenReturn("test rule");
when(rule.getTargetingStrategy()).thenReturn(Rule.visitRootOnly());
Mockito.doAnswer(new Answer<Void>() {
private RuleViolation createViolation(RuleContext context, int beginLine, String message) {
DummyNode node = new DummyNode();