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

Fix tests

Browse Source
This commit is contained in:
Clément Fournier
2022-04-13 20:33:45 +02:00
parent a5f9ed8f7e
commit 271b8ab062
12 changed files with 124 additions and 104 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java
View File

@ -9,6 +9,7 @@ import static net.sourceforge.pmd.internal.util.xml.SchemaConstants.EXCLUDE;
import static net.sourceforge.pmd.internal.util.xml.SchemaConstants.EXCLUDE_PATTERN;
import static net.sourceforge.pmd.internal.util.xml.SchemaConstants.INCLUDE_PATTERN;
import static net.sourceforge.pmd.internal.util.xml.SchemaConstants.PRIORITY;
import static net.sourceforge.pmd.internal.util.xml.SchemaConstants.REF;
import static net.sourceforge.pmd.internal.util.xml.SchemaConstants.RULE;
import java.io.IOException;
@ -161,6 +162,7 @@ final class RuleSetFactory {
"Cannot parse a RuleSet from a non-external reference: <" + ruleSetReferenceId + ">.");
}
@SuppressWarnings("PMD.CloseResource")
AccumulatingMessageHandler handler = getXmlMessagePrinter();
DocumentBuilder builder = createDocumentBuilder();
InputSource inputSource = new InputSource(inputStream);
@ -329,7 +331,7 @@ final class RuleSetFactory {
return; // deleted rule
}
if (ref.endsWith("xml")) {
parseRuleSetReferenceNode(ruleSetBuilder, ruleNode, ref, rulesetReferences);
parseRuleSetReferenceNode(ruleSetBuilder, ruleNode, ref, rulesetReferences, err);
} else if (StringUtils.isBlank(ref)) {
parseSingleRuleNode(ruleSetReferenceId, ruleSetBuilder, ruleNode, err);
} else {
@ -351,11 +353,14 @@ final class RuleSetFactory {
* The RuleSet reference.
* @param rulesetReferences keeps track of already processed complete ruleset references in order to log a warning
*/
// todo error reporting
private void parseRuleSetReferenceNode(RuleSetBuilder ruleSetBuilder, Element ruleElement, String ref, Set<String> rulesetReferences) {
private void parseRuleSetReferenceNode(RuleSetBuilder ruleSetBuilder,
Element ruleElement,
String ref,
Set<String> rulesetReferences,
PmdXmlReporter err) {
String priority = null;
NodeList childNodes = ruleElement.getChildNodes();
Set<String> excludedRulesCheck = new HashSet<>();
Map<String, Element> excludedRulesCheck = new HashMap<>();
for (int i = 0; i < childNodes.getLength(); i++) {
Node child = childNodes.item(i);
if (EXCLUDE.isElementWithName(child)) {
@ -363,13 +368,13 @@ final class RuleSetFactory {
String excludedRuleName = excludeElement.getAttribute("name");
excludedRuleName = compatibilityFilter.applyExclude(ref, excludedRuleName, this.warnDeprecated);
if (excludedRuleName != null) {
excludedRulesCheck.add(excludedRuleName);
excludedRulesCheck.put(excludedRuleName, excludeElement);
}
} else if (PRIORITY.isElementWithName(child)) {
priority = XmlUtil.parseTextNode(child).trim();
}
}
final RuleSetReference ruleSetReference = new RuleSetReference(ref, true, excludedRulesCheck);
final RuleSetReference ruleSetReference = new RuleSetReference(ref, true, excludedRulesCheck.keySet());
// load the ruleset with minimum priority low, so that we get all rules, to be able to exclude any rule
// minimum priority will be applied again, before constructing the final ruleset
@ -397,8 +402,9 @@ final class RuleSetFactory {
if (!potentialRules.isEmpty() && potentialRules.size() == countDeprecated) {
// all rules in the ruleset have been deprecated - the ruleset itself is considered to be deprecated
rulesetDeprecated = true;
LOG.warn("The RuleSet {} has been deprecated and will be removed in PMD {}",
ref, PMDVersion.getNextMajorRelease());
err.at(REF.getAttributeNode(ruleElement))
.warn("The RuleSet {0} has been deprecated and will be removed in PMD {1}",
ref, PMDVersion.getNextMajorRelease());
}
for (RuleReference r : potentialRules) {
@ -411,14 +417,13 @@ final class RuleSetFactory {
}
if (!excludedRulesCheck.isEmpty()) {
LOG.warn(
"Unable to exclude rules {} from ruleset reference {}"
+ "; perhaps the rule name is misspelled or the rule doesn't exist anymore?",
excludedRulesCheck, ref);
excludedRulesCheck.forEach(
(name, elt) ->
err.at(elt).warn("Exclude pattern ''{0}'' did not match any rule in ruleset {1}", name, ref));
}
if (rulesetReferences.contains(ref)) {
LOG.warn("The ruleset {} is referenced multiple times in \"{}\".", ref, ruleSetBuilder.getName());
err.at(ruleElement).warn("The ruleset {0} is referenced multiple times in \"{1}\".", ref, ruleSetBuilder.getName());
}
rulesetReferences.add(ref);
}
@ -659,7 +664,7 @@ final class RuleSetFactory {
delayedExceptions.add(e);
}
public PmdXmlReporterImpl(MessageReporter pmdReporter, OoxmlFacade ooxml, XmlPositioner positioner) {
PmdXmlReporterImpl(MessageReporter pmdReporter, OoxmlFacade ooxml, XmlPositioner positioner) {
super(ooxml, positioner);
this.pmdReporter = pmdReporter;
}

4
pmd-core/src/main/java/net/sourceforge/pmd/RuleSetWriter.java
View File

@ -21,10 +21,10 @@ import javax.xml.transform.dom.DOMSource;
import javax.xml.transform.stream.StreamResult;
import org.apache.commons.io.IOUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.checkerframework.checker.nullness.qual.NonNull;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.w3c.dom.CDATASection;
import org.w3c.dom.DOMException;
import org.w3c.dom.Document;

2
pmd-core/src/main/java/net/sourceforge/pmd/internal/util/FileCollectionUtil.java
View File

@ -128,7 +128,7 @@ public final class FileCollectionUtil {
private static void addRoot(FileCollector collector, String rootLocation) throws IOException {
Path path = Paths.get(rootLocation);
if (!Files.exists(path)) {
collector.getReporter().error("No such file {}", path);
collector.getReporter().error("No such file {0}", path);
return;
}

1
pmd-core/src/main/java/net/sourceforge/pmd/internal/util/xml/SchemaConstants.java
View File

@ -25,6 +25,7 @@ public final class SchemaConstants {
public static final SchemaConstant EXCLUDE_PATTERN = new SchemaConstant("exclude-pattern");
public static final SchemaConstant INCLUDE_PATTERN = new SchemaConstant("include-pattern");
public static final SchemaConstant RULE = new SchemaConstant("rule");
public static final SchemaConstant REF = new SchemaConstant("ref");
public static final SchemaConstant EXCLUDE = new SchemaConstant("exclude");
public static final SchemaConstant PRIORITY = new SchemaConstant("priority");

2
pmd-core/src/main/java/net/sourceforge/pmd/lang/document/FileCollector.java
View File

@ -138,7 +138,7 @@ public final class FileCollector implements AutoCloseable {
*/
public boolean addFile(Path file) {
if (!Files.isRegularFile(file)) {
reporter.error("Not a regular file {}", file);
reporter.error("Not a regular file {0}", file);
return false;
}
LanguageVersion languageVersion = discoverLanguage(file.toString());

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

@ -35,7 +35,6 @@ import net.sourceforge.pmd.lang.Language;
import net.sourceforge.pmd.lang.LanguageRegistry;
import net.sourceforge.pmd.lang.LanguageVersion;
import net.sourceforge.pmd.lang.rule.RuleReference;
import net.sourceforge.pmd.lang.rule.internal.CommonPropertyDescriptors;
import net.sourceforge.pmd.properties.PropertyDescriptor;
import net.sourceforge.pmd.properties.PropertyTypeId;
import net.sourceforge.pmd.properties.PropertyTypeId.BuilderAndMapper;

6
pmd-core/src/main/java/net/sourceforge/pmd/util/log/internal/MessageReporterBase.java
View File

@ -5,6 +5,7 @@
package net.sourceforge.pmd.util.log.internal;
import java.text.MessageFormat;
import java.util.Objects;
import org.apache.commons.lang3.exception.ExceptionUtils;
import org.slf4j.event.Level;
@ -43,6 +44,11 @@ abstract class MessageReporterBase implements MessageReporter {
@Override
public void logEx(Level level, String message, Object[] formatArgs, Throwable error) {
if (isLoggable(level)) {
if (error == null) {
Objects.requireNonNull(message, "cannot call this method with null message and error");
log(level, message, formatArgs);
return;
}
message = MessageFormat.format(message, formatArgs);
String errorMessage = error.getMessage();
if (errorMessage == null) {

31
pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryDuplicatedRuleLoggingTest.java
View File

@ -6,19 +6,10 @@ package net.sourceforge.pmd;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import org.junit.Test;
import org.junit.contrib.java.lang.system.SystemErrRule;
import net.sourceforge.pmd.junit.LocaleRule;
public class RuleSetFactoryDuplicatedRuleLoggingTest {
@org.junit.Rule
public LocaleRule localeRule = LocaleRule.en();
@org.junit.Rule
public final SystemErrRule systemErrRule = new SystemErrRule().mute().enableLog();
public class RuleSetFactoryDuplicatedRuleLoggingTest extends RulesetFactoryTestBase {
@Test
public void duplicatedRuleReferenceShouldWarn() {
@ -28,8 +19,10 @@ public class RuleSetFactoryDuplicatedRuleLoggingTest {
Rule mockRule = ruleset.getRuleByName("DummyBasicMockRule");
assertNotNull(mockRule);
assertEquals(RulePriority.MEDIUM, mockRule.getPriority());
assertTrue(systemErrRule.getLog().contains("The rule DummyBasicMockRule is referenced multiple times in \"Custom Rules\". "
+ "Only the last rule configuration is used."));
verifyFoundAWarningWithMessage(containing(
"The rule DummyBasicMockRule is referenced multiple times in \"Custom Rules\". "
+ "Only the last rule configuration is used."
));
}
@Test
@ -41,7 +34,7 @@ public class RuleSetFactoryDuplicatedRuleLoggingTest {
assertNotNull(mockRule);
assertEquals(RulePriority.HIGH, mockRule.getPriority());
assertNotNull(ruleset.getRuleByName("SampleXPathRule"));
assertTrue(systemErrRule.getLog().isEmpty());
verifyNoWarnings();
}
@Test
@ -53,7 +46,7 @@ public class RuleSetFactoryDuplicatedRuleLoggingTest {
assertNotNull(mockRule);
assertEquals(RulePriority.HIGH, mockRule.getPriority());
assertNotNull(ruleset.getRuleByName("SampleXPathRule"));
assertTrue(systemErrRule.getLog().isEmpty());
verifyNoWarnings();
}
@Test
@ -65,12 +58,14 @@ public class RuleSetFactoryDuplicatedRuleLoggingTest {
assertNotNull(mockRule);
assertEquals(RulePriority.MEDIUM_HIGH, mockRule.getPriority());
assertNotNull(ruleset.getRuleByName("SampleXPathRule"));
assertTrue(systemErrRule.getLog().contains("The rule DummyBasicMockRule is referenced multiple times in \"Custom Rules\". "
verifyFoundAWarningWithMessage(containing(
"The rule DummyBasicMockRule is referenced multiple times in \"Custom Rules\". "
+ "Only the last rule configuration is used."));
assertTrue(systemErrRule.getLog().contains("The ruleset rulesets/dummy/basic.xml is referenced multiple times in \"Custom Rules\"."));
verifyFoundAWarningWithMessage(containing(
"The ruleset rulesets/dummy/basic.xml is referenced multiple times in \"Custom Rules\"."));
}
private RuleSet loadRuleSet(String ruleSetFilename) {
return new RuleSetLoader().loadFromResource("net/sourceforge/pmd/rulesets/duplicatedRuleLoggingTest/" + ruleSetFilename);
protected RuleSet loadRuleSet(String ruleSetFilename) {
return loadRuleSetInDir("net/sourceforge/pmd/rulesets/duplicatedRuleLoggingTest", ruleSetFilename);
}
}

75
pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryTest.java
View File

@ -16,45 +16,20 @@ import java.io.InputStream;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;
import java.util.function.Predicate;
import org.apache.commons.lang3.StringUtils;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.contrib.java.lang.system.SystemErrRule;
import org.mockito.Mockito;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.slf4j.event.Level;
import net.sourceforge.pmd.junit.LocaleRule;
import net.sourceforge.pmd.lang.DummyLanguageModule;
import net.sourceforge.pmd.lang.LanguageRegistry;
import net.sourceforge.pmd.lang.rule.MockRule;
import net.sourceforge.pmd.lang.rule.RuleReference;
import net.sourceforge.pmd.properties.PropertyDescriptor;
import net.sourceforge.pmd.util.ResourceLoader;
import net.sourceforge.pmd.util.log.MessageReporter;
public class RuleSetFactoryTest {
public class RuleSetFactoryTest extends RulesetFactoryTestBase {
private static final Logger LOG = LoggerFactory.getLogger(RuleSetFactoryTest.class);
@org.junit.Rule
public LocaleRule localeRule = LocaleRule.en();
@org.junit.Rule
public final SystemErrRule systemErrRule = new SystemErrRule().muteForSuccessfulTests().enableLog();
private MessageReporter mockReporter;
@Before
public void setup() {
// mockReporter = Mockito.spy(new SimpleMessageReporter(LOG));
mockReporter = Mockito.mock(MessageReporter.class);
}
@Test
public void testRuleSetFileName() {
@ -222,7 +197,7 @@ public class RuleSetFactoryTest {
assertNotNull(rule);
assertNull(rs.getRuleByName("OldName"));
assertTrue(systemErrRule.getLog().isEmpty());
verifyNoWarnings();
}
/**
@ -295,7 +270,7 @@ public class RuleSetFactoryTest {
assertNotNull(rs.getRuleByName("DummyBasicMockRule"));
assertNotNull(rs.getRuleByName("SampleXPathRule"));
assertTrue(systemErrRule.getLog().isEmpty());
verifyNoWarnings();
}
/**
@ -324,7 +299,7 @@ public class RuleSetFactoryTest {
assertNotNull(rs.getRuleByName("DummyBasicMockRule"));
assertNotNull(rs.getRuleByName("SampleXPathRule"));
assertTrue(systemErrRule.getLog().isEmpty());
verifyNoWarnings();
}
/**
@ -349,12 +324,13 @@ public class RuleSetFactoryTest {
assertNotNull(rs.getRuleByName("DummyBasicMockRule"));
assertNotNull(rs.getRuleByName("SampleXPathRule"));
assertEquals(0,
StringUtils.countMatches(systemErrRule.getLog(),
"WARN net.sourceforge.pmd.RuleSetFactory - Discontinue using Rule rulesets/dummy/basic.xml/DeprecatedRule as it is scheduled for removal from PMD."));
assertEquals(1,
StringUtils.countMatches(systemErrRule.getLog(),
"WARN net.sourceforge.pmd.RuleSetFactory - Unable to exclude rules [NonExistingRule] from ruleset reference rulesets/dummy/basic.xml; perhaps the rule name is misspelled or the rule doesn't exist anymore?"));
verifyFoundWarningWithMessage(
Mockito.never(),
containing("Discontinue using Rule rulesets/dummy/basic.xml/DeprecatedRule")
);
verifyFoundAWarningWithMessage(containing(
"Exclude pattern 'NonExistingRule' did not match any rule in ruleset"
));
}
/**
@ -371,9 +347,9 @@ public class RuleSetFactoryTest {
assertNotNull(rs.getRuleByName("DummyBasicMockRule"));
assertNotNull(rs.getRuleByName("SampleXPathRule"));
assertEquals(1,
StringUtils.countMatches(systemErrRule.getLog(),
"WARN net.sourceforge.pmd.RuleSetFactory - The RuleSet rulesets/dummy/deprecated.xml has been deprecated and will be removed in PMD"));
verifyFoundAWarningWithMessage(containing(
"The RuleSet rulesets/dummy/deprecated.xml has been deprecated and will be removed in PMD"
));
}
/**
@ -390,9 +366,10 @@ public class RuleSetFactoryTest {
assertEquals(1, rs.getRules().size());
assertNotNull(rs.getRuleByName("DummyBasic2MockRule"));
assertEquals(0,
StringUtils.countMatches(systemErrRule.getLog(),
"WARN net.sourceforge.pmd.RuleSetFactory - Use Rule name rulesets/dummy/basic.xml/DummyBasicMockRule instead of the deprecated Rule name rulesets/dummy/basic2.xml/DummyBasicMockRule. PMD"));
verifyFoundWarningWithMessage(
Mockito.never(),
containing("Use Rule name rulesets/dummy/basic.xml/DummyBasicMockRule instead of the deprecated Rule name rulesets/dummy/basic2.xml/DummyBasicMockRule")
);
}
@Test
@ -657,20 +634,6 @@ public class RuleSetFactoryTest {
verifyFoundAnErrorWithMessage(containing("version range"));
}
private void verifyFoundAnErrorWithMessage(Predicate<String> messageTest) {
Mockito.verify(mockReporter, Mockito.times(1))
.logEx(Mockito.eq(Level.ERROR), Mockito.argThat(messageTest::test), Mockito.any(), Mockito.any());
}
private void verifyFoundAWarningWithMessage(Predicate<String> messageTest) {
Mockito.verify(mockReporter, Mockito.times(1))
.logEx(Mockito.eq(Level.WARN), Mockito.argThat(messageTest::test), Mockito.any(), Mockito.any());
}
private static Predicate<String> containing(String part) {
return it -> it.contains(part);
}
@Test
public void testDirectDeprecatedRule() {
Rule r = loadFirstRule(DIRECT_DEPRECATED_RULE);
@ -776,7 +739,7 @@ public class RuleSetFactoryTest {
+ "</ruleset>\n");
assertEquals(0, ruleset.getRules().size());
assertTrue(systemErrRule.getLog().isEmpty());
verifyNoWarnings();
}
/**

58
pmd-core/src/test/java/net/sourceforge/pmd/RulesetFactoryTestBase.java Normal file
View File

@ -0,0 +1,58 @@
/*
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd;
import java.util.function.Predicate;
import org.junit.Before;
import org.mockito.Mockito;
import org.mockito.verification.VerificationMode;
import org.slf4j.event.Level;
import net.sourceforge.pmd.junit.LocaleRule;
import net.sourceforge.pmd.util.log.MessageReporter;
public class RulesetFactoryTestBase {
@org.junit.Rule
public LocaleRule localeRule = LocaleRule.en();
protected MessageReporter mockReporter;
@Before
public void setup() {
mockReporter = Mockito.mock(MessageReporter.class);
}
protected void verifyNoWarnings() {
Mockito.verifyZeroInteractions(mockReporter);
}
protected static Predicate<String> containing(String part) {
return it -> it.contains(part);
}
protected void verifyFoundAWarningWithMessage(Predicate<String> messageTest) {
verifyFoundWarningWithMessage(Mockito.times(1), messageTest);
}
protected void verifyFoundWarningWithMessage(VerificationMode mode, Predicate<String> messageTest) {
Mockito.verify(mockReporter, mode)
.logEx(Mockito.eq(Level.WARN), Mockito.argThat(messageTest::test), Mockito.any(), Mockito.any());
}
protected void verifyFoundAnErrorWithMessage(Predicate<String> messageTest) {
Mockito.verify(mockReporter, Mockito.times(1))
.logEx(Mockito.eq(Level.ERROR), Mockito.argThat(messageTest::test), Mockito.any(), Mockito.any());
}
protected RuleSet loadRuleSetInDir(String resourceDir, String ruleSetFilename) {
RuleSetLoader loader = new RuleSetLoader();
loader.setReporter(mockReporter);
return loader.loadFromResource(resourceDir + "/" + ruleSetFilename);
}
}

2
pmd-core/src/test/java/net/sourceforge/pmd/renderers/SummaryHTMLRendererTest.java
View File

@ -7,8 +7,8 @@ package net.sourceforge.pmd.renderers;
import static org.junit.Assert.assertEquals;
import java.util.Collections;
import java.util.function.Consumer;
import java.util.Optional;
import java.util.function.Consumer;
import org.junit.Test;

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

@ -5,8 +5,7 @@
package net.sourceforge.pmd.testframework;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@ -20,13 +19,14 @@ import net.sourceforge.pmd.RuleContext;
import net.sourceforge.pmd.lang.LanguageRegistry;
import net.sourceforge.pmd.lang.LanguageVersion;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.rule.AbstractRule;
import net.sourceforge.pmd.lang.rule.RuleTargetSelector;
import net.sourceforge.pmd.test.lang.DummyLanguageModule.DummyRootNode;
public class RuleTstTest {
private LanguageVersion dummyLanguage = LanguageRegistry.findLanguageByTerseName("dummy").getDefaultVersion();
private Rule rule = mock(Rule.class);
private Rule rule = spy(AbstractRule.class);
private RuleTst ruleTester = new RuleTst() {
};
@ -42,13 +42,6 @@ public class RuleTstTest {
verify(rule).start(any(RuleContext.class));
verify(rule).end(any(RuleContext.class));
verify(rule).getLanguage();
verify(rule, times(2)).getTargetSelector();
verify(rule).getMinimumLanguageVersion();
verify(rule).getMaximumLanguageVersion();
verify(rule).apply(any(Node.class), any(RuleContext.class));
verify(rule, times(4)).getName();
verify(rule).getPropertiesByPropertyDescriptor();
}
@Test