From 4569842965fe510b6dccca49f6f497ba7509e345 Mon Sep 17 00:00:00 2001 From: Philip Graf Date: Sun, 24 Mar 2013 17:53:40 +0100 Subject: [PATCH] pmd: fix #1076 Report.treeIterator() does not return all violations The cause of the bug was the implementation of ViolationNode.equalsNode(otherNode) which returned true when two different violations start on the same line. Note: this change also adds a maven dependency on Mockito 1.9.5 (scope: test). Mockito helps creating mock objects and makes the test more readable. --- pmd/pom.xml | 6 ++ .../pmd/lang/dfa/report/ViolationNode.java | 3 + .../lang/dfa/report/ViolationNodeTest.java | 99 +++++++++++++++++++ 3 files changed, 108 insertions(+) create mode 100644 pmd/src/test/java/net/sourceforge/pmd/lang/dfa/report/ViolationNodeTest.java diff --git a/pmd/pom.xml b/pmd/pom.xml index e19d2d2ea9..c0f865b410 100644 --- a/pmd/pom.xml +++ b/pmd/pom.xml @@ -610,6 +610,12 @@ commons-io 2.2 + + org.mockito + mockito-all + 1.9.5 + test + diff --git a/pmd/src/main/java/net/sourceforge/pmd/lang/dfa/report/ViolationNode.java b/pmd/src/main/java/net/sourceforge/pmd/lang/dfa/report/ViolationNode.java index 36e8d396b9..36a85d1761 100644 --- a/pmd/src/main/java/net/sourceforge/pmd/lang/dfa/report/ViolationNode.java +++ b/pmd/src/main/java/net/sourceforge/pmd/lang/dfa/report/ViolationNode.java @@ -23,6 +23,9 @@ public class ViolationNode extends AbstractReportNode { return rv.getFilename().equals(getRuleViolation().getFilename()) && rv.getBeginLine() == getRuleViolation().getBeginLine() && + rv.getBeginColumn() == getRuleViolation().getBeginColumn() && + rv.getEndLine() == getRuleViolation().getEndLine() && + rv.getEndColumn()== getRuleViolation().getEndColumn() && rv.getVariableName().equals(getRuleViolation().getVariableName()); } diff --git a/pmd/src/test/java/net/sourceforge/pmd/lang/dfa/report/ViolationNodeTest.java b/pmd/src/test/java/net/sourceforge/pmd/lang/dfa/report/ViolationNodeTest.java new file mode 100644 index 0000000000..0653987396 --- /dev/null +++ b/pmd/src/test/java/net/sourceforge/pmd/lang/dfa/report/ViolationNodeTest.java @@ -0,0 +1,99 @@ +package net.sourceforge.pmd.lang.dfa.report; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import net.sourceforge.pmd.RuleViolation; + +import org.junit.Test; + +/** + * @author Philip Graf + */ +public final class ViolationNodeTest { + + /** + * Verifies that two violations nodes with equal {@code filename, beginLine, endLine, beginColumn, endColumn} and + * {@code variableName} are equal. + */ + @Test + public void testEqualsNodeWithTwoEqualViolations() { + final ViolationNode node1 = createViolationNode("Foo.java", 1, 1, 5, 15, ""); + final ViolationNode node2 = createViolationNode("Foo.java", 1, 1, 5, 15, ""); + assertTrue("Two equal violations should result in equal nodes", node1.equalsNode(node2)); + } + + /** + * Verifies that two violations nodes with different {@code filename} are not equal. + */ + @Test + public void testEqualsNodeWithTwoDifferentViolationsDifferentFilename() { + final ViolationNode node1 = createViolationNode("Foo.java", 1, 1, 5, 15, ""); + final ViolationNode node2 = createViolationNode("Bar.java", 1, 1, 5, 15, ""); + assertFalse("Two violations with different filename should result in not equal nodes", node1.equalsNode(node2)); + } + + /** + * Verifies that two violations nodes with different {@code beginLine} are not equal. + */ + @Test + public void testEqualsNodeWithTwoDifferentViolationsDifferentBeginLine() { + final ViolationNode node1 = createViolationNode("Foo.java", 1, 2, 5, 15, ""); + final ViolationNode node2 = createViolationNode("Foo.java", 2, 2, 5, 15, ""); + assertFalse("Two violations with different beginLine should result in not equal nodes", node1.equalsNode(node2)); + } + + /** + * Verifies that two violations nodes with different {@code endLine} are not equal. + */ + @Test + public void testEqualsNodeWithTwoDifferentViolationsDifferentEndLine() { + final ViolationNode node1 = createViolationNode("Foo.java", 1, 1, 5, 15, ""); + final ViolationNode node2 = createViolationNode("Foo.java", 1, 2, 5, 15, ""); + assertFalse("Two violations with different endLine should result in not equal nodes", node1.equalsNode(node2)); + } + + /** + * Verifies that two violations nodes with different {@code beginColumn} are not equal. + */ + @Test + public void testEqualsNodeWithTwoDifferentViolationsDifferentBeginColumn() { + final ViolationNode node1 = createViolationNode("Foo.java", 1, 1, 5, 15, ""); + final ViolationNode node2 = createViolationNode("Foo.java", 1, 1, 7, 15, ""); + assertFalse("Two violations with different beginColumn should result in not equal nodes", node1.equalsNode(node2)); + } + + /** + * Verifies that two violations nodes with different {@code endColumn} are not equal. + */ + @Test + public void testEqualsNodeWithTwoDifferentViolationsDifferentEndColumn() { + final ViolationNode node1 = createViolationNode("Foo.java", 1, 1, 5, 15, ""); + final ViolationNode node2 = createViolationNode("Foo.java", 1, 1, 5, 17, ""); + assertFalse("Two violations with different end column should result in not equal nodes", node1.equalsNode(node2)); + } + + /** + * Verifies that two violations with different {@code variableName} are not equal. + */ + @Test + public void testEqualsNodeWithTwoDifferentViolationsDifferentVariableName() { + final ViolationNode node1 = createViolationNode("Foo.java", 1, 1, 5, 15, "a"); + final ViolationNode node2 = createViolationNode("Foo.java", 1, 1, 5, 15, "b"); + assertFalse("Two violations with different variableName should result in not equal nodes", node1.equalsNode(node2)); + } + + private ViolationNode createViolationNode(final String filename, final int beginLine, final int endLine, + final int beginColumn, final int endColumn, final String variableName) { + final RuleViolation violation = mock(RuleViolation.class); + when(violation.getFilename()).thenReturn(filename); + when(violation.getBeginLine()).thenReturn(beginLine); + when(violation.getEndLine()).thenReturn(endLine); + when(violation.getBeginColumn()).thenReturn(beginColumn); + when(violation.getEndColumn()).thenReturn(endColumn); + when(violation.getVariableName()).thenReturn(variableName); + return new ViolationNode(violation); + } + +}