Merge branch 'pr-2741' into master

[core] Deprecate ruleviolation comparator #2741

docs/pages/release_notes.md

@@ -26,6 +26,12 @@ This is a {{ site.pmd.release_type }} release.
 
 ### API Changes
 
+#### Deprecated API
+
+##### For removal
+
+*   {% jdoc !!core::RuleViolationComparator %}. Use {% jdoc !!core::RuleViolation#DEFAULT_COMPARATOR %} instead.
+
 ### External Contributions
 
 * [#2747](https://github.com/pmd/pmd/pull/2747): \[java] Don't trigger FinalFieldCouldBeStatic when field is annotated with lombok @Builder.Default - [Ollie Abbey](https://github.com/ollieabbey)

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

@@ -345,7 +345,7 @@ public class Report implements Iterable<RuleViolation> {
             return;
         }
 
-        int index = Collections.binarySearch(violations, violation, RuleViolationComparator.INSTANCE);
+        int index = Collections.binarySearch(violations, violation, RuleViolation.DEFAULT_COMPARATOR);
         violations.add(index < 0 ? -index - 1 : index, violation);
         violationTree.addRuleViolation(violation);
         for (ThreadSafeReportListener listener : listeners) {
@@ -405,7 +405,7 @@ public class Report implements Iterable<RuleViolation> {
         suppressedRuleViolations.addAll(r.suppressedRuleViolations);
 
         for (RuleViolation violation : r.getViolations()) {
-            int index = Collections.binarySearch(violations, violation, RuleViolationComparator.INSTANCE);
+            int index = Collections.binarySearch(violations, violation, RuleViolation.DEFAULT_COMPARATOR);
             violations.add(index < 0 ? -index - 1 : index, violation);
             violationTree.addRuleViolation(violation);
         }
@@ -519,7 +519,7 @@ public class Report implements Iterable<RuleViolation> {
      * Returns an unmodifiable list of violations that have been
      * recorded until now. None of those violations were suppressed.
      *
-     * <p>The violations list is sorted with {@link RuleViolationComparator#INSTANCE}.
+     * <p>The violations list is sorted with {@link RuleViolation#DEFAULT_COMPARATOR}.
      */
     public final List<RuleViolation> getViolations() {
         return Collections.unmodifiableList(violations);

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

@@ -4,6 +4,8 @@
 
 package net.sourceforge.pmd;
 
+import java.util.Comparator;
+
 /**
  * A RuleViolation is created by a Rule when it identifies a violation of the
  * Rule constraints. RuleViolations are simple data holders that are collected
@@ -16,6 +18,14 @@ package net.sourceforge.pmd;
  */
 public interface RuleViolation {
 
+    /**
+     * A comparator for rule violations. This compares all exposed attributes
+     * of a violation, filename first. The remaining parameters are compared
+     * in an unspecified order.
+     */
+    // TODO in java 8 this can be a chained Comparator.comparing call
+    Comparator<RuleViolation> DEFAULT_COMPARATOR = RuleViolationComparator.INSTANCE;
+
     /**
      * Get the Rule which identified this violation.
      *

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

@@ -17,7 +17,19 @@ import java.util.Comparator;
  * <li>End column</li>
  * <li>Rule name</li>
  * </ol>
+ *
+ * TODO why is begin line/begin column split?? would make more sense to use
+ *  - filename
+ *  - begin line
+ *  - begin column
+ *  - description
+ *  - rule name
+ *  - end line
+ *  - end column
+ *
+ * @deprecated Use {@link RuleViolation#DEFAULT_COMPARATOR}
  */
+@Deprecated
 public final class RuleViolationComparator implements Comparator<RuleViolation> {
 
     public static final RuleViolationComparator INSTANCE = new RuleViolationComparator();

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

@@ -58,7 +58,7 @@ public class RuleViolationComparatorTest {
         Collections.shuffle(ruleViolations, random);
 
         // Sort
-        Collections.sort(ruleViolations, RuleViolationComparator.INSTANCE);
+        Collections.sort(ruleViolations, RuleViolation.DEFAULT_COMPARATOR);
 
         // Check
         int count = 0;

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

@@ -8,6 +8,7 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
 import java.io.File;
+import java.util.Comparator;
 
 import org.junit.Ignore;
 import org.junit.Test;
@@ -53,7 +54,7 @@ public class RuleViolationTest {
     @Test
     public void testComparatorWithDifferentFilenames() {
         Rule rule = new MockRule("name", "desc", "msg", "rulesetname");
-        RuleViolationComparator comp = RuleViolationComparator.INSTANCE;
+        Comparator<RuleViolation> comp = RuleViolation.DEFAULT_COMPARATOR;
         RuleContext ctx = new RuleContext();
         ctx.setSourceCodeFile(new File("filename1"));
         DummyNode s = new DummyNode(1);
@@ -72,7 +73,7 @@ public class RuleViolationTest {
     @Test
     public void testComparatorWithSameFileDifferentLines() {
         Rule rule = new MockRule("name", "desc", "msg", "rulesetname");
-        RuleViolationComparator comp = RuleViolationComparator.INSTANCE;
+        Comparator<RuleViolation> comp = RuleViolation.DEFAULT_COMPARATOR;
         RuleContext ctx = new RuleContext();
         ctx.setSourceCodeFile(new File("filename"));
         DummyNode s = new DummyNode(1);
@@ -91,7 +92,7 @@ public class RuleViolationTest {
     @Test
     public void testComparatorWithSameFileSameLines() {
         Rule rule = new MockRule("name", "desc", "msg", "rulesetname");
-        RuleViolationComparator comp = RuleViolationComparator.INSTANCE;
+        Comparator<RuleViolation> comp = RuleViolation.DEFAULT_COMPARATOR;
         RuleContext ctx = new RuleContext();
         ctx.setSourceCodeFile(new File("filename"));
         DummyNode s = new DummyNode(1);