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:57 UTC

[maven-checkstyle-plugin] branch MCHECKSTYLE-385 updated (4fa6377 -> b259e84)

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

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


 discard 4fa6377  [MCHECKSTYLE-385] Violation should be a value class
     add 354fad1  update JUnit
     add 1ec4c24  Merge pull request #37 from apache/ver
     add d8522d5  remove unused dependency
     add 895b91f  Merge pull request #38 from apache/ver2
     add 4b4101c  use try with resources instead of deprecated method
     add 04be4e1  Merge pull request #39 from apache/close
     add bd635da  remove unused imports
     add 91b8b16  Merge pull request #40 from apache/ver3
     add 7688eb9  [MCHECKSTYLE-387] emit a warning when using an old version of checkstyle.
     new b259e84  [MCHECKSTYLE-385] Violation should be a value class

This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version.  This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:

 * -- * -- B -- O -- O -- O   (4fa6377)
            \
             N -- N -- N   refs/heads/MCHECKSTYLE-385 (b259e84)

You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.

Any revisions marked "omit" are not gone; other references still
refer to them.  Any revisions marked "discard" are gone forever.

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 pom.xml                                             |  7 +------
 .../checkstyle/exec/DefaultCheckstyleExecutor.java  | 21 +++++++++++----------
 .../plugins/checkstyle/CheckstyleReportTest.java    |  1 -
 .../CheckstyleViolationCheckMojoTest.java           |  1 -
 .../plugins/checkstyle/ReportResourceTest.java      |  1 -
 .../CheckstyleReportListenerMultiSourceTest.java    |  6 ------
 .../exec/CheckstyleReportListenerTest.java          |  3 ---
 .../checkstyle/exec/CheckstyleResultsTest.java      |  3 ---
 8 files changed, 12 insertions(+), 31 deletions(-)


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

Posted by bm...@apache.org.
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();
+  }
+}