You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by bm...@apache.org on 2020/12/08 09:17:58 UTC

[maven-checkstyle-plugin] 01/01: [MCHECKSTYLE-385] Violation should be a value class

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

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

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

    [MCHECKSTYLE-385] Violation should be a value class
---
 .../checkstyle/CheckstyleViolationCheckMojo.java   | 137 ++++++++----
 .../apache/maven/plugins/checkstyle/Violation.java | 229 +++++++++++++++++++++
 2 files changed, 323 insertions(+), 43 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 b9c47d7..42c738e 100644
--- a/src/main/java/org/apache/maven/plugins/checkstyle/CheckstyleViolationCheckMojo.java
+++ b/src/main/java/org/apache/maven/plugins/checkstyle/CheckstyleViolationCheckMojo.java
@@ -156,10 +156,10 @@ public class CheckstyleViolationCheckMojo
      */
     @Parameter( property = "checkstyle.console", defaultValue = "true" )
     private boolean logViolationsToConsole;
-    
+
     /**
      * Output the detected violation count to the console.
-     * 
+     *
      * @since 3.0.1
      */
     @Parameter( property = "checkstyle.logViolationCount", defaultValue = "true" )
@@ -577,11 +577,14 @@ public class CheckstyleViolationCheckMojo
             XmlPullParser xpp = new MXParser();
             xpp.setInput( reader );
 
-            int violations = countViolations( xpp );
+            final List<Violation> violationsList = getViolations( xpp );
+            long violationCount = countViolations( violationsList );
+            printViolations( violationsList );
+
+            String msg = "You have " + violationCount + " Checkstyle violation"
+                + ( ( violationCount > 1 || violationCount == 0 ) ? "s" : "" ) + ".";
 
-            String msg = "You have " + violations + " Checkstyle violation"
-                + ( ( violations > 1 || violations == 0 ) ? "s" : "" ) + ".";
-            if ( violations > maxAllowedViolations )
+            if ( violationCount > maxAllowedViolations )
             {
                 if ( failOnViolation )
                 {
@@ -591,7 +594,7 @@ public class CheckstyleViolationCheckMojo
                     }
                     throw new MojoFailureException( msg );
                 }
-                
+
                 getLog().warn( "checkstyle:check violations detected but failOnViolation set to false" );
             }
             if ( logViolationCountToConsole )
@@ -621,16 +624,14 @@ 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 )
@@ -640,49 +641,99 @@ public class CheckstyleViolationCheckMojo
             else 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() ) )
+            else 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,
+                line,
+                severity,
+                message,
+                rule,
+                category
+            );
+            if ( column != null )
+            {
+                violation.setColumn( column );
+            }
 
-                String source = xpp.getAttributeValue( "", "source" );
+            violations.add( violation );
+        }
 
-                if ( ignore( ignores, source ) )
-                {
-                    ignoreCount++;
-                }
-                else
-                {
-                    count++;
+        return violations;
+    }
 
-                    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 );
-                    }
-                }
+    private int countViolations( List<Violation> violations )
+    {
+        List<RuleUtil.Matcher> ignores = violationIgnore == null ? Collections.<RuleUtil.Matcher>emptyList()
+            : RuleUtil.parseMatchers( violationIgnore.split( "," ) );
+
+        int ignored = 0;
+        int countedViolations = 0;
+
+        for ( Violation violation : violations )
+        {
+            if ( ! isViolation( violation.getSeverity() ) )
+            {
+                continue;
+            }
+
+            if ( ignore( ignores, violation.getSource() ) )
+            {
+                ignored++;
+                continue;
             }
+
+            countedViolations++;
         }
 
-        if ( ignoreCount > 0 )
+        if ( ignored > 0 )
         {
-            getLog().info( "Ignored " + ignoreCount + " error" + ( ( ignoreCount > 1 ) ? "s" : "" ) + ", " + count
-                               + " violation" + ( ( count > 1 ) ? "s" : "" ) + " remaining." );
+            getLog().info( "Ignored " + ignored + " error" + ( ( ignored > 1L ) ? "s" : "" ) + ", " + countedViolations
+                + " violation" + ( ( countedViolations > 1 ) ? "s" : "" ) + " remaining." );
         }
 
-        return count;
+        return countedViolations;
+    }
+
+    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:[%s%s] (%s) %s: %s",
+                    violation.getFile(),
+                    violation.getLine(),
+                    ( Violation.NO_COLUMN.equals( violation.getColumn() ) ) ? "" : ( ',' + violation.getColumn() ),
+                    violation.getCategory(),
+                    violation.getRuleName(),
+                    violation.getMessage() );
+                log( violation.getSeverity(), message );
+            } );
     }
 
     private void log( String severity, String message )
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..803e948
--- /dev/null
+++ b/src/main/java/org/apache/maven/plugins/checkstyle/Violation.java
@@ -0,0 +1,229 @@
+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 java.util.Objects;
+import java.util.StringJoiner;
+
+/**
+ * Holds data about a single violation and represents the violation itself.
+ */
+class Violation
+{
+
+  /**
+   * Indicates that a column is not set.
+   */
+  protected static final String NO_COLUMN = "-1";
+
+  /** The full qualified class name of the checkstyle rule */
+  private final String source;
+
+  /** The absolute path of the file containing the violation */
+  private final String file;
+
+  private final String line;
+
+  private String column = NO_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.
+
+  /**
+   * Creates a violation instance without a column set.
+   *
+   * @param source
+   *     the fully qualified class name of the checkstyle rule
+   * @param file
+   *     the absolute file path in which the violation occurred
+   * @param line
+   *     the line in the file on which the violation occurred
+   * @param severity
+   *     the severity of the violation
+   * @param message
+   *     the message from checkstyle for this violation
+   * @param ruleName
+   *     the rule name from which this violation was created
+   * @param category
+   *     the category of the checkstyle violation
+   */
+  Violation( String source,
+                    String file,
+                    String 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 );
+  }
+
+  /**
+   * Returns the fully qualified class name of the checker rule.
+   *
+   * @return the fully qualified class name of the checker rule
+   */
+  protected String getSource( )
+  {
+    return source;
+  }
+
+  /**
+   * Returns the absolute file path to the checked file.
+   *
+   * @return the absolute file path to the checked file
+   */
+  protected String getFile( )
+  {
+    return file;
+  }
+
+  /**
+   * Returns the line in the checked file on which the violation occurred.
+   *
+   * @return the line in the checked file on which the violation occurred
+   */
+  protected String getLine( )
+  {
+    return line;
+  }
+
+  /**
+   * Returns the column in which the violation occurred, if available.
+   *
+   * @return the column in which the violation occurred, if available. Otherwise returns {@link #NO_COLUMN}.
+   */
+  protected String getColumn( )
+  {
+    return column;
+  }
+
+  /**
+   * Sets the column value for this violation to the given string value.
+   * @param column the column value to set. May be {@code null}, which will set it to the {@link #NO_COLUMN} value.
+   */
+  protected void setColumn( /* Nullable */ String column )
+  {
+    if ( column == null || column.length() < 1 )
+    {
+      this.column = NO_COLUMN;
+    }
+    else
+    {
+      this.column = column;
+    }
+  }
+
+  /**
+   * Returns the severity of the current violation.
+   *
+   * @return the severity of the current violation
+   */
+  protected String getSeverity( )
+  {
+    return severity;
+  }
+
+  /**
+   * Returns the message produced by checkstyle for the current violation.
+   *
+   * @return the message produced by checkstyle for the current violation
+   */
+  protected String getMessage( )
+  {
+    return message;
+  }
+
+  /**
+   * Returns the name of the rule which led to the current violation.
+   *
+   * @return the name of the rule which led to the current violation
+   */
+  protected String getRuleName( )
+  {
+    return ruleName;
+  }
+
+  /**
+   * Returns the category of the current violation.
+   *
+   * @return the category of the current violation
+   */
+  protected 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.equals( 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();
+  }
+}