[MPMD-281] Display found violations grouped by priority

* New property pmd.renderViolationsByPriority

src/it/MPMD-128-xref-link/pom.xml

@@ -51,6 +51,7 @@ under the License.
         <rulesets>
           <ruleset>src/main/config/pmd/rules.xml</ruleset>
         </rulesets>
+        <renderViolationsByPriority>false</renderViolationsByPriority>
       </configuration>
     </plugin>
     <plugin>

src/it/MPMD-253-xref-link-multi-module/pom.xml

@@ -52,7 +52,8 @@ under the License.
         <rulesets>
           <ruleset>src/main/config/pmd/rules.xml</ruleset>
         </rulesets>
-		<aggregate>true</aggregate>
+        <aggregate>true</aggregate>
+        <renderViolationsByPriority>false</renderViolationsByPriority>
       </configuration>
     </plugin>
     <plugin>

src/main/java/org/apache/maven/plugins/pmd/PmdReport.java

@@ -236,6 +236,15 @@ public class PmdReport
     @Parameter( property = "pmd.renderRuleViolationPriority", defaultValue = "true" )
     private boolean renderRuleViolationPriority = true;
 
+    /**
+     * Add a section in the HTML report, that groups the found violations by rule priority
+     * in addition to grouping by file.
+     *
+     * @since 3.12.0
+     */
+    @Parameter( property = "pmd.renderViolationsByPriority", defaultValue = "true" )
+    private boolean renderViolationsByPriority = true;
+
     @Component
     private DependencyResolver dependencyResolver;
 
@@ -561,6 +570,7 @@ private Report generateReport( Locale locale )
         Sink sink = getSink();
         PmdReportGenerator doxiaRenderer = new PmdReportGenerator( getLog(), sink, getBundle( locale ), aggregate );
         doxiaRenderer.setRenderRuleViolationPriority( renderRuleViolationPriority );
+        doxiaRenderer.setRenderViolationsByPriority( renderViolationsByPriority );
         doxiaRenderer.setFiles( filesToProcess );
         doxiaRenderer.setViolations( renderer.getViolations() );
         if ( renderProcessingErrors )

src/main/java/org/apache/maven/plugins/pmd/PmdReportGenerator.java

@@ -25,6 +25,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -36,6 +37,7 @@
 import org.codehaus.plexus.util.StringUtils;
 
 import net.sourceforge.pmd.Report.ProcessingError;
+import net.sourceforge.pmd.RulePriority;
 import net.sourceforge.pmd.RuleViolation;
 
 /**
@@ -62,6 +64,8 @@ public class PmdReportGenerator
 
     private boolean renderRuleViolationPriority;
 
+    private boolean renderViolationsByPriority;
+
     private Map<File, PmdFileInfo> files;
 
     // private List<Metric> metrics = new ArrayList<Metric>();
@@ -142,16 +146,16 @@ private PmdFileInfo determineFileInfo( String filename )
         return fileInfo;
     }
 
-    private void startFileSection( String currentFilename, PmdFileInfo fileInfo )
+    private void startFileSection( int level, String currentFilename, PmdFileInfo fileInfo )
     {
-        sink.section2();
-        sink.sectionTitle2();
+        sink.section( level, null );
+        sink.sectionTitle( level, null );
 
         // prepare the filename
         this.currentFilename = shortenFilename( currentFilename, fileInfo );
 
         sink.text( makeFileSectionName( this.currentFilename, fileInfo ) );
-        sink.sectionTitle2_();
+        sink.sectionTitle_( level );
 
         sink.table();
         sink.tableRow();
@@ -173,10 +177,10 @@ private void startFileSection( String currentFilename, PmdFileInfo fileInfo )
         sink.tableRow_();
     }
 
-    private void endFileSection()
+    private void endFileSection( int level )
     {
         sink.table_();
-        sink.section2_();
+        sink.section_( level );
     }
 
     private void processSingleRuleViolation( RuleViolation ruleViolation, PmdFileInfo fileInfo )
@@ -216,7 +220,7 @@ private void processSingleRuleViolation( RuleViolation ruleViolation, PmdFileInf
     // PMD might run the analysis multi-threaded, so the violations might be reported
     // out of order. We sort them here by filename and line number before writing them to
     // the report.
-    private void processViolations()
+    private void renderViolations()
         throws IOException
     {
         sink.section1();
@@ -227,7 +231,73 @@ private void processViolations()
         // TODO files summary
 
         List<RuleViolation> violations2 = new ArrayList<>( violations );
-        Collections.sort( violations2, new Comparator<RuleViolation>()
+        renderViolationsTable( 2, violations2 );
+
+        sink.section1_();
+    }
+
+    private void renderViolationsByPriority() throws IOException
+    {
+        if ( !renderViolationsByPriority )
+        {
+            return;
+        }
+
+        boolean oldPriorityColumn = this.renderRuleViolationPriority;
+        this.renderRuleViolationPriority = false;
+
+        sink.section1();
+        sink.sectionTitle1();
+        sink.text( bundle.getString( "report.pmd.violationsByPriority" ) );
+        sink.sectionTitle1_();
+
+        Map<RulePriority, List<RuleViolation>> violationsByPriority = new HashMap<>();
+        for ( RuleViolation violation : violations )
+        {
+            RulePriority priority = violation.getRule().getPriority();
+            List<RuleViolation> violationSegment = violationsByPriority.get( priority );
+            if ( violationSegment == null )
+            {
+                violationSegment = new ArrayList<>();
+                violationsByPriority.put( priority, violationSegment );
+            }
+            violationsByPriority.get( violation.getRule().getPriority() ).add( violation );
+        }
+
+        for ( RulePriority priority : RulePriority.values() )
+        {
+            List<RuleViolation> violationsWithPriority = violationsByPriority.get( priority );
+            if ( violationsWithPriority == null || violationsWithPriority.isEmpty() )
+            {
+                continue;
+            }
+
+            sink.section2();
+            sink.sectionTitle2();
+            sink.text( bundle.getString( "report.pmd.priority" ) + " " + priority.getPriority() );
+            sink.sectionTitle2_();
+
+            renderViolationsTable( 3, violationsWithPriority );
+
+            sink.section2_();
+        }
+
+        if ( violations.isEmpty() )
+        {
+            sink.paragraph();
+            sink.text( bundle.getString( "report.pmd.noProblems" ) );
+            sink.paragraph_();
+        }
+
+        sink.section1_();
+
+        this.renderRuleViolationPriority = oldPriorityColumn;
+    }
+
+    private void renderViolationsTable( int level, List<RuleViolation> violationSegment )
+    throws IOException
+    {
+        Collections.sort( violationSegment, new Comparator<RuleViolation>()
         {
             /** {@inheritDoc} */
             public int compare( RuleViolation o1, RuleViolation o2 )
@@ -246,19 +316,19 @@ public int compare( RuleViolation o1, RuleViolation o2 )
 
         boolean fileSectionStarted = false;
         String previousFilename = null;
-        for ( RuleViolation ruleViolation : violations2 )
+        for ( RuleViolation ruleViolation : violationSegment )
         {
             String currentFn = ruleViolation.getFilename();
             PmdFileInfo fileInfo = determineFileInfo( currentFn );
 
             if ( !currentFn.equalsIgnoreCase( previousFilename ) && fileSectionStarted )
             {
-                endFileSection();
+                endFileSection( level );
                 fileSectionStarted = false;
             }
             if ( !fileSectionStarted )
             {
-                startFileSection( currentFn, fileInfo );
+                startFileSection( level, currentFn, fileInfo );
                 fileSectionStarted = true;
             }
 
@@ -269,17 +339,8 @@ public int compare( RuleViolation o1, RuleViolation o2 )
 
         if ( fileSectionStarted )
         {
-            endFileSection();
+            endFileSection( level );
         }
-
-        if ( violations.isEmpty() )
-        {
-            sink.paragraph();
-            sink.text( bundle.getString( "report.pmd.noProblems" ) );
-            sink.paragraph_();
-        }
-
-        sink.section1_();
     }
 
     private void outputLineLink( int line, PmdFileInfo fileInfo )
@@ -403,7 +464,18 @@ public void beginDocument()
     public void render()
         throws IOException
     {
-        processViolations();
+        if ( !violations.isEmpty() )
+        {
+            renderViolationsByPriority();
+
+            renderViolations();
+        }
+        else
+        {
+            sink.paragraph();
+            sink.text( bundle.getString( "report.pmd.noProblems" ) );
+            sink.paragraph_();
+        }
 
         if ( !processingErrors.isEmpty() )
         {
@@ -437,4 +509,9 @@ public void setRenderRuleViolationPriority( boolean renderRuleViolationPriority
     {
         this.renderRuleViolationPriority = renderRuleViolationPriority;
     }
+
+    public void setRenderViolationsByPriority( boolean renderViolationsByPriority )
+    {
+        this.renderViolationsByPriority = renderViolationsByPriority;
+    }
 }

src/main/resources/pmd-report.properties

@@ -24,6 +24,8 @@ report.pmd.column.priority=Priority
 report.pmd.column.line=Line
 report.pmd.pmdlink=The following document contains the results of
 report.pmd.files=Files
+report.pmd.violationsByPriority=Violations By Priority
+report.pmd.priority=Priority
 report.pmd.noProblems=PMD found no problems in your source code.
 report.pmd.processingErrors.title=Processing Errors
 report.pmd.processingErrors.column.filename=Filename

src/main/resources/pmd-report_de.properties

@@ -24,6 +24,8 @@ report.pmd.column.priority=Priorit\u00e4t
 report.pmd.column.line=Zeile
 report.pmd.pmdlink=Dieses Dokument enth\u00e4lt die Ergebnisse von
 report.pmd.files=Dateien
+report.pmd.violationsByPriority=Verst\u00f6\u00dfe nach Priorit\u00e4t
+report.pmd.priority=Priorit\u00e4t
 report.pmd.noProblems=PMD hat keine Probleme in dem Quellcode gefunden.
 report.pmd.processingErrors.title=Verarbeitungsprobleme
 report.pmd.processingErrors.column.filename=Datei

src/test/java/org/apache/maven/plugins/pmd/PmdReportTest.java

@@ -95,6 +95,13 @@ public void testDefaultConfiguration()
         assertTrue( str.contains( "<th>Rule</th>" ) );
         // along with a link to the rule
         assertTrue( str.contains( "pmd_rules_java_bestpractices.html#unusedprivatefield\">UnusedPrivateField</a>" ) );
+
+        // there should be the section Violations By Priority
+        assertTrue( str.contains( "Violations By Priority</h2>" ) );
+        assertTrue( str.contains( "Priority 3</h3>" ) );
+        assertTrue( str.contains( "Priority 4</h3>" ) );
+        // the file App.java is mentioned 3 times: in prio 3, in prio 4 and in the files section
+        assertEquals( 3, StringUtils.countMatches( str, "def/configuration/App.java" ) );
     }
 
     public void testDefaultConfigurationNotRenderRuleViolationPriority()
@@ -117,9 +124,37 @@ public void testDefaultConfigurationNotRenderRuleViolationPriority()
         String str = readFile( generatedFile );
 
         // check that there's no priority column
-        assertFalse( str.contains( "Priority" ) );
+        assertFalse( str.contains( "<th>Priority</th>" ) );
     }
 
+    public void testDefaultConfigurationNoRenderViolationsByPriority()
+            throws Exception
+        {
+            FileUtils.copyDirectoryStructure( new File( getBasedir(),
+                                                        "src/test/resources/unit/default-configuration/jxr-files" ),
+                                              new File( getBasedir(), "target/test/unit/default-configuration/target/site" ) );
+
+            File testPom =
+                new File( getBasedir(),
+                          "src/test/resources/unit/default-configuration/pmd-report-no-render-violations-by-priority.xml" );
+            PmdReport mojo = (PmdReport) lookupMojo( "pmd", testPom );
+            mojo.execute();
+
+            File generatedFile = new File( getBasedir(), "target/test/unit/default-configuration/target/site/pmd.html" );
+            renderer( mojo, generatedFile );
+            assertTrue( FileUtils.fileExists( generatedFile.getAbsolutePath() ) );
+
+            String str = readFile( generatedFile );
+
+            // there should be no section Violations By Priority
+            assertFalse( str.contains( "Violations By Priority</h2>" ) );
+            assertFalse( str.contains( "Priority 3</h3>" ) );
+            assertFalse( str.contains( "Priority 4</h3>" ) );
+            // the file App.java is mentioned once: in the files section
+            assertEquals( 1, StringUtils.countMatches( str, "def/configuration/App.java" ) );
+        }
+
+
     public void testDefaultConfigurationWithAnalysisCache()
             throws Exception
     {
@@ -344,7 +379,10 @@ public void testEmptyReportConfiguration()
         assertTrue( FileUtils.fileExists( generatedFile.getAbsolutePath() ) );
         String str = readFile( generatedFile );
         assertFalse( lowerCaseContains( str, "Hello.java" ) );
-        assertTrue( str.contains( "PMD found no problems in your source code." ) );
+        assertEquals( 1, StringUtils.countMatches( str, "PMD found no problems in your source code." ) );
+        // no sections files or violations by priority
+        assertFalse( str.contains( "Files</h2>" ) );
+        assertFalse( str.contains( "Violations By Priority</h2>" ) );
     }
 
     public void testInvalidFormat()