You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by eo...@apache.org on 2020/01/03 09:55:30 UTC

[maven-checkstyle-plugin] 01/06: [MCHECKSTYLE-385] rework code to use a Violation.java class. This enables further optimizations.

This is an automated email from the ASF dual-hosted git repository.

eolivelli pushed a commit to branch MCHECKSTYLE-385
in repository https://gitbox.apache.org/repos/asf/maven-checkstyle-plugin.git

commit 98b612248511ef3956fd7575c17a8487dcb98b08
Author: Benjamin Marwell <bm...@gmail.com>
AuthorDate: Wed Dec 11 22:51:48 2019 +0100

    [MCHECKSTYLE-385] rework code to use a Violation.java class. This enables further optimizations.
---
 .../checkstyle/CheckstyleViolationCheckMojo.java   | 125 ++++++++++++-----
 .../apache/maven/plugins/checkstyle/Violation.java | 153 +++++++++++++++++++++
 2 files changed, 242 insertions(+), 36 deletions(-)

diff --git a/src/main/java/org/apache/maven/plugins/checkstyle/CheckstyleViolationCheckMojo.java b/src/main/java/org/apache/maven/plugins/checkstyle/CheckstyleViolationCheckMojo.java
index 404cce7..1f3decc 100644
--- a/src/main/java/org/apache/maven/plugins/checkstyle/CheckstyleViolationCheckMojo.java
+++ b/src/main/java/org/apache/maven/plugins/checkstyle/CheckstyleViolationCheckMojo.java
@@ -31,6 +31,8 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.atomic.LongAdder;
+import java.util.stream.Collectors;
 
 import org.apache.maven.artifact.Artifact;
 import org.apache.maven.model.Dependency;
@@ -50,6 +52,7 @@ import org.apache.maven.plugins.checkstyle.exec.CheckstyleExecutor;
 import org.apache.maven.plugins.checkstyle.exec.CheckstyleExecutorException;
 import org.apache.maven.plugins.checkstyle.exec.CheckstyleExecutorRequest;
 import org.apache.maven.project.MavenProject;
+import org.checkerframework.checker.nullness.qual.Nullable;
 import org.codehaus.plexus.configuration.PlexusConfiguration;
 import org.codehaus.plexus.util.FileUtils;
 import org.codehaus.plexus.util.PathTool;
@@ -569,7 +572,9 @@ public class CheckstyleViolationCheckMojo
             XmlPullParser xpp = new MXParser();
             xpp.setInput( reader );
 
-            int violations = countViolations( xpp );
+            final List<Violation> violationsList = getViolations( xpp );
+            long violations = countViolations( violationsList );
+            printViolations( violationsList );
 
             if ( violations > maxAllowedViolations )
             {
@@ -605,70 +610,118 @@ public class CheckstyleViolationCheckMojo
         }
     }
 
-    private int countViolations( XmlPullParser xpp )
+    private List<Violation> getViolations( XmlPullParser xpp )
         throws XmlPullParserException, IOException
     {
-        int count = 0;
-        int ignoreCount = 0;
-        List<RuleUtil.Matcher> ignores = violationIgnore == null ? Collections.<RuleUtil.Matcher>emptyList()
-                        : RuleUtil.parseMatchers( violationIgnore.split( "," ) );
+        List<Violation> violations = new ArrayList<>();
 
         String basedir = project.getBasedir().getAbsolutePath();
         String file = "";
+
         for ( int eventType = xpp.getEventType(); eventType != XmlPullParser.END_DOCUMENT; eventType = xpp.next() )
         {
             if ( eventType != XmlPullParser.START_TAG )
             {
                 continue;
             }
-            else if ( "file".equals( xpp.getName() ) )
+
+            if ( "file".equals( xpp.getName() ) )
             {
                 file = PathTool.getRelativeFilePath( basedir, xpp.getAttributeValue( "", "name" ) );
-                //file = file.substring( file.lastIndexOf( File.separatorChar ) + 1 );
+                continue;
             }
-            else if ( "error".equals( xpp.getName() ) )
+
+            if ( !"error".equals( xpp.getName() ) )
             {
-                String severity = xpp.getAttributeValue( "", "severity" );
+                continue;
+            }
 
-                if ( !isViolation( severity ) )
-                {
-                    continue;
-                }
+            String severity = xpp.getAttributeValue( "", "severity" );
+            String source = xpp.getAttributeValue( "", "source" );
+            String line = xpp.getAttributeValue( "", "line" );
+            @Nullable String column = xpp.getAttributeValue( "", "column" );
+            String message = xpp.getAttributeValue( "", "message" );
+            String rule = RuleUtil.getName( source );
+            String category = RuleUtil.getCategory( source );
+
+            Violation violation = new Violation(
+                source,
+                file,
+                Integer.parseInt( line, 10 ),
+                severity,
+                message,
+                rule,
+                category
+            );
+            if ( column != null )
+            {
+                violation.setColumn( Integer.parseInt( column, 10 ) );
+            }
+
+            violations.add( violation );
+        }
+
+        return Collections.unmodifiableList( violations );
+    }
+
+    private long countViolations( List<Violation> violations )
+    {
+        List<RuleUtil.Matcher> ignores = violationIgnore == null ? Collections.<RuleUtil.Matcher>emptyList()
+            : RuleUtil.parseMatchers( violationIgnore.split( "," ) );
 
-                String source = xpp.getAttributeValue( "", "source" );
+        LongAdder ignored = new LongAdder();
 
-                if ( ignore( ignores, source ) )
+        final List<Violation> violationStream = violations.stream()
+            .filter( violation -> isViolation( violation.getSeverity() ) )
+            .filter( violation ->
+            {
+                final boolean isIgnore = !ignore( ignores, violation.getSource() );
+                if ( isIgnore )
                 {
-                    ignoreCount++;
+                    ignored.increment();
                 }
-                else
-                {
-                    count++;
+                return isIgnore;
+            } )
+            .collect( Collectors.toList() );
 
-                    if ( logViolationsToConsole )
-                    {
-                        String line = xpp.getAttributeValue( "", "line" );
-                        String column = xpp.getAttributeValue( "", "column" );
-                        String message = xpp.getAttributeValue( "", "message" );
-                        String rule = RuleUtil.getName( source );
-                        String category = RuleUtil.getCategory( source );
-
-                        log( severity, file + ":[" + line + ( ( column == null ) ? "" : ( ',' + column ) ) + "] ("
-                            + category + ") " + rule + ": " + message );
-                    }
-                }
-            }
-        }
+        final int count = violationStream.size();
+        final long ignoreCount = ignored.sum();
 
         if ( ignoreCount > 0 )
         {
-            getLog().info( "Ignored " + ignoreCount + " error" + ( ( ignoreCount > 1 ) ? "s" : "" ) + ", " + count
-                               + " violation" + ( ( count > 1 ) ? "s" : "" ) + " remaining." );
+            getLog().info( "Ignored " + ignoreCount + " error" + ( ( ignoreCount > 1L ) ? "s" : "" ) + ", " + count
+                + " violation" + ( ( count > 1 ) ? "s" : "" ) + " remaining." );
         }
 
         return count;
     }
 
+    private void printViolations( List<Violation> violations )
+    {
+        if ( ! logViolationsToConsole )
+        {
+            return;
+        }
+
+        List<RuleUtil.Matcher> ignores = violationIgnore == null ? Collections.<RuleUtil.Matcher>emptyList()
+            : RuleUtil.parseMatchers( violationIgnore.split( "," ) );
+
+        violations.stream()
+            .filter( violation -> isViolation( violation.getSeverity() ) )
+            .filter( violation -> !ignore( ignores, violation.getSource() ) )
+            .forEach( violation ->
+            {
+                final String message = String.format( "%s:[%d%s] (%s) %s: %s",
+                    violation.getFile(),
+                    violation.getLine(),
+                    ( violation.getColumn() == null ) ? "" : ( ',' + violation.getColumn() ),
+                    violation.getCategory(),
+                    violation.getRuleName(),
+                    violation.getMessage() );
+                log( violation.getSeverity(), message );
+            } );
+    }
+
     private void log( String severity, String message )
     {
         if ( "info".equals( severity ) )
diff --git a/src/main/java/org/apache/maven/plugins/checkstyle/Violation.java b/src/main/java/org/apache/maven/plugins/checkstyle/Violation.java
new file mode 100644
index 0000000..5d0cb84
--- /dev/null
+++ b/src/main/java/org/apache/maven/plugins/checkstyle/Violation.java
@@ -0,0 +1,153 @@
+package org.apache.maven.plugins.checkstyle;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.Objects;
+import java.util.StringJoiner;
+
+/**
+ * Holds data about a single violation and represents the violation itself.
+ */
+public class Violation
+{
+  private final String source;
+
+  private final String file;
+
+  private final int line;
+
+  private @Nullable Integer column;
+
+  private final String severity;
+
+  private final String message;
+
+  private final String ruleName;
+
+  private final String category;
+
+  // Leaving out column, because there is no CHECKSTYLE:OFF support.
+  public Violation( String source,
+                    String file,
+                    int line,
+                    String severity,
+                    String message,
+                    String ruleName,
+                    String category )
+  {
+    this.source = Objects.requireNonNull( source );
+    this.file = file;
+    this.line = line;
+    this.severity = Objects.requireNonNull( severity );
+    this.message = Objects.requireNonNull( message );
+    this.ruleName = Objects.requireNonNull( ruleName );
+    this.category = Objects.requireNonNull( category );
+  }
+
+  public String getSource()
+  {
+    return source;
+  }
+
+  public String getFile()
+  {
+    return file;
+  }
+
+  public long getLine()
+  {
+    return line;
+  }
+
+  public @Nullable Integer getColumn()
+  {
+    return column;
+  }
+
+  public void setColumn( @Nullable Integer column )
+  {
+    this.column = column;
+  }
+
+  public String getSeverity()
+  {
+    return severity;
+  }
+
+  public String getMessage()
+  {
+    return message;
+  }
+
+  public String getRuleName()
+  {
+    return ruleName;
+  }
+
+  public String getCategory()
+  {
+    return category;
+  }
+
+  @Override
+  public boolean equals( Object other )
+  {
+    if ( this == other )
+    {
+      return true;
+    }
+    if ( !( other instanceof Violation ) )
+    {
+      return false;
+    }
+    Violation violation = ( Violation ) other;
+    return line == violation.line
+        && Objects.equals( column, violation.column )
+        && source.equals( violation.source )
+        && file.equals( violation.file )
+        && severity.equals( violation.severity )
+        && message.equals( violation.message )
+        && ruleName.equals( violation.ruleName )
+        && category.equals( violation.category );
+  }
+
+  @Override
+  public int hashCode()
+  {
+    return Objects.hash( source, file, line, column, severity, message, ruleName, category );
+  }
+
+  @Override
+  public String toString()
+  {
+    return new StringJoiner( ", ", Violation.class.getSimpleName() + "[", "]" )
+        .add( "source='" + source + "'" )
+        .add( "file='" + file + "'" )
+        .add( "line=" + line )
+        .add( "column=" + column )
+        .add( "severity='" + severity + "'" )
+        .add( "message='" + message + "'" )
+        .add( "ruleName='" + ruleName + "'" )
+        .add( "category='" + category + "'" )
+        .toString();
+  }
+}