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 f76a9b4a05a6c8d5fdad6c01db2058b3d9eaf827 Author: Benjamin Marwell <bmarw...@gmail.com> AuthorDate: Wed Dec 11 22:51:48 2019 +0100 [MCHECKSTYLE-385] Implementing reviews and comments. Signed-off-by: Benjamin Marwell <bmarw...@gmail.com> --- .../checkstyle/CheckstyleViolationCheckMojo.java | 61 +++++++++++----------- .../apache/maven/plugins/checkstyle/Violation.java | 54 ++++++++++++++----- 2 files changed, 72 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 1f3decc..4afeea6 100644 --- a/src/main/java/org/apache/maven/plugins/checkstyle/CheckstyleViolationCheckMojo.java +++ b/src/main/java/org/apache/maven/plugins/checkstyle/CheckstyleViolationCheckMojo.java @@ -31,8 +31,6 @@ 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; @@ -573,15 +571,16 @@ public class CheckstyleViolationCheckMojo xpp.setInput( reader ); final List<Violation> violationsList = getViolations( xpp ); - long violations = countViolations( violationsList ); + long violationCount = countViolations( violationsList ); printViolations( violationsList ); - if ( violations > maxAllowedViolations ) + if ( violationCount > maxAllowedViolations ) { if ( failOnViolation ) { String msg = - "You have " + violations + " Checkstyle violation" + ( ( violations > 1 ) ? "s" : "" ) + "."; + "You have " + violationCount + + " Checkstyle violation" + ( ( violationCount > 1 ) ? "s" : "" ) + "."; if ( maxAllowedViolations > 0 ) { msg += " The maximum number of allowed violations is " + maxAllowedViolations + "."; @@ -624,14 +623,12 @@ public class CheckstyleViolationCheckMojo { continue; } - - if ( "file".equals( xpp.getName() ) ) + else if ( "file".equals( xpp.getName() ) ) { file = PathTool.getRelativeFilePath( basedir, xpp.getAttributeValue( "", "name" ) ); continue; } - - if ( !"error".equals( xpp.getName() ) ) + else if ( ! "error".equals( xpp.getName() ) ) { continue; } @@ -647,7 +644,7 @@ public class CheckstyleViolationCheckMojo Violation violation = new Violation( source, file, - Integer.parseInt( line, 10 ), + line, severity, message, rule, @@ -655,7 +652,7 @@ public class CheckstyleViolationCheckMojo ); if ( column != null ) { - violation.setColumn( Integer.parseInt( column, 10 ) ); + violation.setColumn( column ); } violations.add( violation ); @@ -664,32 +661,36 @@ public class CheckstyleViolationCheckMojo return Collections.unmodifiableList( violations ); } - private long countViolations( List<Violation> violations ) + private int countViolations( List<Violation> violations ) { List<RuleUtil.Matcher> ignores = violationIgnore == null ? Collections.<RuleUtil.Matcher>emptyList() : RuleUtil.parseMatchers( violationIgnore.split( "," ) ); - LongAdder ignored = new LongAdder(); + int ignored = 0; - final List<Violation> violationStream = violations.stream() - .filter( violation -> isViolation( violation.getSeverity() ) ) - .filter( violation -> + List<Violation> actualViolations = new ArrayList<>(); + + for ( Violation violation : violations ) + { + if ( ! isViolation( violation.getSeverity() ) ) { - final boolean isIgnore = !ignore( ignores, violation.getSource() ); - if ( isIgnore ) - { - ignored.increment(); - } - return isIgnore; - } ) - .collect( Collectors.toList() ); + continue; + } + + if ( ignore( ignores, violation.getSource() ) ) + { + ignored++; + continue; + } + + actualViolations.add( violation ); + } - final int count = violationStream.size(); - final long ignoreCount = ignored.sum(); + final int count = actualViolations.size(); - if ( ignoreCount > 0 ) + if ( ignored > 0 ) { - getLog().info( "Ignored " + ignoreCount + " error" + ( ( ignoreCount > 1L ) ? "s" : "" ) + ", " + count + getLog().info( "Ignored " + ignored + " error" + ( ( ignored > 1L ) ? "s" : "" ) + ", " + count + " violation" + ( ( count > 1 ) ? "s" : "" ) + " remaining." ); } @@ -711,10 +712,10 @@ public class CheckstyleViolationCheckMojo .filter( violation -> !ignore( ignores, violation.getSource() ) ) .forEach( violation -> { - final String message = String.format( "%s:[%d%s] (%s) %s: %s", + final String message = String.format( "%s:[%s%s] (%s) %s: %s", violation.getFile(), violation.getLine(), - ( violation.getColumn() == null ) ? "" : ( ',' + violation.getColumn() ), + ( Violation.NO_COLUMN.equals( violation.getColumn() ) ) ? "" : ( ',' + violation.getColumn() ), violation.getCategory(), violation.getRuleName(), violation.getMessage() ); diff --git a/src/main/java/org/apache/maven/plugins/checkstyle/Violation.java b/src/main/java/org/apache/maven/plugins/checkstyle/Violation.java index 5d0cb84..90d0ed9 100644 --- a/src/main/java/org/apache/maven/plugins/checkstyle/Violation.java +++ b/src/main/java/org/apache/maven/plugins/checkstyle/Violation.java @@ -29,13 +29,16 @@ import java.util.StringJoiner; */ public class Violation { + + public static final String NO_COLUMN = "-1"; + private final String source; private final String file; - private final int line; + private final String line; - private @Nullable Integer column; + private String column = NO_COLUMN; private final String severity; @@ -46,9 +49,28 @@ public class Violation 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 source, to be defined. + * @param file + * the file 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. + */ public Violation( String source, String file, - int line, + String line, String severity, String message, String ruleName, @@ -63,47 +85,53 @@ public class Violation this.category = Objects.requireNonNull( category ); } - public String getSource() + protected String getSource( ) { return source; } - public String getFile() + protected String getFile( ) { return file; } - public long getLine() + protected String getLine( ) { return line; } - public @Nullable Integer getColumn() + protected String getColumn( ) { return column; } - public void setColumn( @Nullable Integer column ) + protected void setColumn( @Nullable String column ) { + if ( null == column || column.length() < 1 ) + { + this.column = NO_COLUMN; + return; + } + this.column = column; } - public String getSeverity() + protected String getSeverity( ) { return severity; } - public String getMessage() + protected String getMessage( ) { return message; } - public String getRuleName() + protected String getRuleName( ) { return ruleName; } - public String getCategory() + protected String getCategory( ) { return category; } @@ -120,7 +148,7 @@ public class Violation return false; } Violation violation = ( Violation ) other; - return line == violation.line + return line.equals( violation.line ) && Objects.equals( column, violation.column ) && source.equals( violation.source ) && file.equals( violation.file )