Checkstyle integration to codeclimate.com platformt

from Michael Bernstein:

We opened up the platform and invited developers to make "engines" that run on it - basically small wrappers around existing static analysis tools

checkstyle was one of the first that someone contributed

It's a small groovy script, basically - you can see it here https://github.com/sivakumar-kailasam/codeclimate-checkstyle

You can see it running on CC here: https://codeclimate.com/github/mrb/fixture_code_base/issues/categories/style

We think that this is a really awesome thing for people who work on tools like you do

Because it's a new way for people to use it - we run analysis for free for all OSS repos

cool, let me see

Basically you add your repo to codeclimate

And add a .codeclimate.yml to your repo

where I can reed more about your platform that allow to run custom analysers

Overview of the platform is here: codeclimate.com/platform

Blog post about how to build engines is here: blog.codeclimate.com/blog/2015/07/07/build-your-own-codeclimate-engine/

please share examples of .codeclimate.yml

https://github.com/mrb/fixture_code_base/blob/master/.codeclimate.yml

Let me know if you get a chance to check it out

Example of Cyclomatic complexity analysis over real projects

Processed by:

1) launch of https://github.com/checkstyle/contribution/tree/master/checkstyle-tester generate HTML report

2) html2text target/site/index.html | grep "Cyclomatic Complexity is" | cut -d' ' -f7 | sort | uniq --count | sort -h -k 2

3) % is calculated as "=TRUNC(((B2)/B$47)*100)"

So amount of methods the complicated then 10 (from all method that complicated than 5) in the project is about 5%.  Below is tables for: Checkstyle , Spring Framework, Open JDK 7.

Checkstyle 6.7 Cynclomatic complexity analysis:

CC	Count	%
5	130	15
6	83	12
7	66	9
8	52	10
9	53	4
10	22	4
11	23	4
12	25	1
13	7	0
14	3	1
15	8	0
16	4	0
17	2	0
18	5	1
19	10	0
20	1	2
21	13	0
22	1	0
23	5	0
24	1	0
27	1	0
29	1	0
30	1	0
32	1	0
34	2	0
44	2	0
47	2	0
81	1	100
	Total: 525

Spring frameworks v4.1.6.RELEASE Cyclomatic complexity analysis:

CC	Count	%
5	746	30
6	450	18
7	312	12
8	215	8
9	157	6
10	109	4
11	80	3
12	55	2
13	52	2
14	34	1
15	33	1
16	25	1
17	18	0
18	19	0
19	14	0
20	17	0
21	11	0
22	8	0
23	9	0
24	6	0
25	7	0
26	11	0
27	1	0
28	2	0
29	1	0
31	3	0
32	2	0
33	5	0
34	3	0
35	2	0
36	3	0
37	1	0
38	1	0
40	2	0
41	1	0
44	1	0
47	1	0
48	1	0
50	2	0
55	1	0
60	1	0
61	1	0
81	2	0
127	1	0
184	1	0
	Total:2427

OpenJdk7 Cyclomatic complexity analysis: 

CC	Count	%
5	4650	25
6	2947	16
7	2234	12
8	1542	8
9	1161	6
10	932	5
11	706	3
12	574	3
13	505	2
14	383	2
15	332	1
16	261	1
17	230	1
18	200	1
19	183	1
20	128	0
21	119	0
22	121	0
23	102	0
24	78	0
25	58	0
26	60	0
27	57	0
28	47	0
29	30	0
30	29	0
31	27	0
32	30	0
33	22	0
34	30	0
35	11	0
36	19	0
37	27	0
38	12	0
39	17	0
40	10	0
41	16	0
42	12	0
43	11	0
44	14	0
45	6	0
46	11	0
47	14	0
48	9	0
49	7	0
50	12	0
51	11	0
52	6	0
53	6	0
54	8	0
55	6	0
56	5	0
57	2	0
58	4	0
59	1	0
60	3	0
62	6	0
63	3	0
64	4	0
66	4	0
67	6	0
68	2	0
69	1	0
70	2	0
71	1	0
72	2	0
73	3	0
74	7	0
75	4	0
76	3	0
77	3	0
78	2	0
79	1	0
80	4	0
81	1	0
82	5	0
83	1	0
85	1	0
86	2	0
87	1	0
88	3	0
93	2	0
95	3	0
96	1	0
97	1	0
100	1	0
101	1	0
103	1	0
109	1	0
110	1	0
111	1	0
121	1	0
124	1	0
125	1	0
130	2	0
140	1	0
145	1	0
149	1	0
164	1	0
186	1	0
219	1	0
256	1	0
281	1	0
	Total:18136

Understanding of Cyclomatic Complexity for measuring code quality

But does that mean a program with a high Cyclomatic Complexity (CC) has a bad quality ? For sure not !

It is just another metrics and some people think it is better to use Line numbers instead as more simple and more robust metric. Very good paper  - http://www.leshatton.org/Documents/TAIC2008-29-08-2008.pdf#17 (page 17) , see also Conclusion - page 45.

It is common when people say " ... reducing the cyclomatic complexity of the code can help reducing the number of test cases ... " - that is not always true!!
Blind following of Cyclomatic metric will lead you to code that have a lot of small methods that will be hard to maintain as encapsulation might be damaged. It is not a rare case there you should put ignore to some case where complexity is high as you defined for the rest of the project.
To keep all that logic(even a bit complicated) in one method to not let other methods use it and what is more problematic change them to their needs. Not everything should be decomposed to smaller parts.
Finally if your complicated method is "public" and you will decompose it internals for several "private" method - you will not reduce amount of tests !!! as in tests you will still check your public method with all possible values in arguments.

The main point of CC is to measure complexity to measure amount of tests, so it suppose to be used as (from wiki) :

 branch coverage cyclomatic complexity number of paths

from wiki:

McCabe showed that the cyclomatic complexity of any structured program with only one entrance point and one exit point is equal to the number of decision points (π) (i.e., "if" statements or conditional loops) contained in that program plus one. However, this is true only for decision points counted at the lowest, machine-level instructions. Decisions involving compound predicates like those found in high-level languages like IF cond1 AND cond2 THEN ... should be counted in terms of predicate variables involved, i.e. in this examples one should count two decision points, because at machine level it is equivalent to  IF cond1 THEN IF cond2 THEN ... 

Cyclomatic complexity may be extended to a program with multiple exit points; in this case it is equal to:

π − s + 2, 

where π is the number of decision points in the program, and s is the number of exit points

So amount of decision point is good measure of complexity ,  but as you can see in definition it could be reduced by adding more exit points. That is true for amount of test calculation  but that is not true to measure of code quality and readability.
So "multi exits/returns" could be a incentive for engineers to calm down Checkstyle or PMD . But that conflict with best practice to have one entry and one exit point in method.

Conclusion: Cyclomatic Complexity is for measuring of Tests amount and NOT about code quality. But CC could be used as quality measure with custom to project level/threshold selection. Do not be ashamed to have complexity level 15 or even more, but keep it below 20 to catch really bad designed code automatically.

I highly recommend to read https://www.cqse.eu/en/blog/mccabe-cyclomatic-complexity/ to see on code examples how weird complexity calculation could be.

Checkstyle's HTML documentation was updated for this metric: http://checkstyle.sourceforge.net/config_metrics.html#CyclomaticComplexity

I updated Chekstyle code to follow level less then 11, I can confirm that about 54 cases were real ugly and messy code and it  was good that that metrics is pointed to that code. But in non library code level 11 is too demanding but reachable, 15 is ok.

Example of Cyclomatic complexity calculation over code base of Checkstyle, Spring, OpenJDK.

Sources:
https://en.wikipedia.org/wiki/Cyclomatic_complexity (especially links to external and other ref)
http://damienlepage.com/cyclomatic-complexity/
http://www.sonarqube.org/discussing-cyclomatic-complexity/

Sevntu Checkstyle release 1.13.0

Braking backward compatibility:

VariableDeclarationUsageDistanceCheck was moved to Checkstyle library . Author: Roman Ivanov

AbbreviationAsWordInNameCheck is transfered to Checkstyle library . Author: Roman Ivanov

 

 

New:

New Check RequiredParameterForAnnotationCheck. Fixes #321 . Author: Andrew Uljanenko

New Check UselessSuperCtorCallCheck implemeted. Fixes #186 . Author: Alexey

New Check EmptyPublicCtorInClassCheck implemented. fixes #291 . Author: Alexey

New Check NameConventionForJunit4TestClassesCheck implemented. Fixes #80 . Author: Alexey

New Check UselessSingleCatchCheck implemented. issue #267 fixed . Author: Alexey

New Check: AvoidConditionInversion . #112 . Author: alexkravin

New Check: NestedSwitchCheck - Checking if there is no switch block inside other switch block.  Issue #249. Author: Damian Szczepanik

 

Bugs:

fix for NPE in ConfusingConditionCheck . Author: melodicwing

Fixed false positive in check ForbidReturnInFinallyBlockChech. Fixes #269 . Author: Andrew Uljanenko

Diamond operator - fixed violations on anonymous classes . Issue #270 . Author: alexkravin

 

 

Miscellaneous:

Refactored prefixes, changed indentation of switch-case, issue #298 . Author: alexkravin

Moved test-input folder from src/test/java to src/test/resources . Author: alexkravin

no java6 support since 6.2 release . Author: Roman Ivanov

version 1.13.0 bump, java6, fixes for errors of m2e . Author: Roman Ivanov

Updated maximum line length to 100 symbols . Author: Daniil Yaroslavtsev

Small code changes and extended UT's. Fixes #317 . Author: Andrew Uljanenko

New test case added. Fixes #275 . Author: Andrew Uljanenko

New test case added. Fixes #274 . Author: Andrew Uljanenko

Extended javadoc, UTs. Fixed #264 . Author: Andrew Uljanenko

 

Code coverage improvement . Author: Damian Szczepanik

Short-circuit logic is used in boolean expressions, issue #169 . Author: Michal Kordas

Code coverage improvements for ReturnCountExtendedCheck . Author: Damian Szczepanik

Improved AvoidHidingCauseExceptionCheck code coverage . Author: Damian Szczepanik

Improved UselessSingleCatchCheck code coverage . Author: Damian Szczepanik

Improved AvoidDefaultSerializableInInnerClasses code coverage to 100% . Author: Damian Szczepanik

Fixes several Sonar violations . Author: Damian Szczepanik

Remove unused input for AvoidHidingCauseExceptionCheck . Author: Damian Szczepanik

Increase AvoidNotShortCircuitOperatorsForBooleanCheck code coverage Pull #324 . Author: Damian Szczepanik

Moved static duplicated methods into Utils class . Author: Damian Szczepanik

Sonar violation: Tabulation characters should not be used . Author: Damian Szczepanik

improved InterfaceTypeParameterNameCheck code coverage . Author: Damian Szczepanik

Improved UnnecessaryParenthesesExtendedCheck test coverage . Author: Damian Szczepanik

Fixed Sonar critical and major violations: - "static final" arrays should be "private" - "for" loop incrementers should modify the variable being tested in the loop's stop condition - Declarations should use Java collection interfaces such as "List" rather than specific implementation classes such as "LinkedList" . Author: Damian Szczepanik

Run cobentura with mvn check or mavn install and fail if code coverage criteria are not met. For new files we expect to have 100% code coverage, for older one that do not pass this rule we added some rules . Author: Damian Szczepanik

Added Utils class that has one method for reporting unsupported AST token . Author: Damian Szczepanik

Removed duplicate check for the empty list: first condition isEmpty() but then for-each which also skips iteration if list is empty . Author: Damian Szczepanik

Increase OverridableMethodInConstructorCheck code coverage: - Added more test cases (examples) - Replaced to Serializable/Clonable stirngs to Cloneable.class.getSimpleName() - Increase coverage by using true and false values of checkCloneMethod and checkReadObjectMethod . Author: Damian Szczepanik

MapIterationInForEachLoopCheck - Converted strings to .class.getName() - moved initialization to beginTree method . Author: Damian Szczepanik

Improved test coverity of EitherLogOrThrowCheck by: - adding new test with inner class - invoking setters method by setting attributes in test scenario . Author: Damian Szczepanik

Improved test coverity in ConfusingConditionCheck by setting default parameters in test case . Author: Damian Szczepanik

Improved coverage of AvoidDefaultSerializableInInnerClasses by importing full qualified class name Serializable (use java.io.Serializable) . Author: Damian Szczepanik

Improved test coverity of class ChildBlockLengthCheckCheck by adding test for tested class . Author: Damian Szczepanik

Improved CustomDeclarationOrderCheck - Added tests to increase coverage - Fixed rule in test method - Fixed formatting (indents) . Author: Damian Szczepanik

Should logger be private static or not

Just read - http://wiki.apache.org/commons/Logging/StaticLog

decision is full of trade off but, that article is the best, so I am referencing it in my blog to let me quickly find it, as that question re-appear again and again.

My position:
Logger should never be public or protected and even share-able with or accessible by others classes, eventually will be enforced by Checkstyle - https://github.com/sevntu-checkstyle/sevntu.checkstyle/issues/286

Functional naming guides is not good for OOP

As you pass in parameter object - it have to be named as noun. Library method expect function. Function is a noun, so it is object.
Object Oriented languages have very good methodology for naming of all items, that let read code easily, mixing in different methodology that conflicts - it is not good approach.

-  names = FluentIterable.from(zipFiles).transformAndConcat(getTableNamesForZipFile).toSet();
+ names = FluentIterable.from(zipFiles).transformAndConcat(tableNamesForZipFileMaker).toSet();

  public <T> FluentIterable<T> transformAndConcat(
      Function<? super E, ? extends Iterable<? extends T>> function) {
    return from(Iterables.concat(transform(function)));
  }

Name should be:
tableNamesForZipFileGetter,
tableNamesForZipFileMaker,
tableNamesForZipFilePuller,
tableNamesForZipFileAlter,
tableNamesForZipFileResolver, ...

It is ok to have no standard prefix/suffix for name, as all functions are different by type (different functional libraries).

Economizing on 2-5 letter in comparison  to use "get" prefix does not cost that.

Interesting compromise for naming function objects : column2Name, file2name, zipFile2Name

Why JavaDoc is matter in checkstyle code ?

As a mentor I do a lot of code reviews ...

My moto: Good code with good naming does not need any comment and JavaDoc

but ..... in Checkstyle we demand javadoc for all elements of class

Why we need that ?


1)
to validate method logic I need to know author intentions

2) catch problems in naming like
    /**
     * Method makes recurse DetailAST and save types that it returns. Its an
.........
     */
    private void findAllMethodReturnTypes(DetailAST processingSiblingAST) {

    /**
     * Method checks if inner private defined type (class, interface or enumeration)
     * is private and does not implement(extend)
     * something, if so it adds type's name to the set of private types.......
     */
    private void getPrivateType(DetailAST innerTypeDefAST) {


3)
It is better for reading of newly created code.

What is better inversion in method names of inversion operator in usage ?

What is better inversion in method names of in usage ? What criteria we could use measure that ?

For private methods it might be not a problem, but mostly affects API and public methods

@Override

public void visitToken(DetailAST aAst) {

DetailAST conditionExpressionAst = aAst.findFirstToken(TokenTypes.EXPR);

switch (aAst.getType()) {

case TokenTypes.LITERAL_RETURN:

if (isNonEmptyReturn(aAst)) {

DetailAST inversionAst = getInversion(conditionExpressionAst);

if (isAvoidableInversion(inversionAst)) {

log(inversionAst);

}

}

break;

case TokenTypes.LITERAL_WHILE:

case TokenTypes.LITERAL_DO:

case TokenTypes.LITERAL_IF:

DetailAST inversionAst = getInversion(conditionExpressionAst);

if (isAvoidableInversion(inversionAst)) {

log(inversionAst);

}

break;

case TokenTypes.FOR_CONDITION:

if (isNonEmptyForCondition(aAst)) {

inversionAst = getInversion(conditionExpressionAst);

if (isAvoidableInversion(inversionAst)) {

log(inversionAst);

}

}

break;

default:

final String exceptionMsg = "Unexpected Token Type - "

+ TokenTypes.getTokenName(aAst.getType());

throw new IllegalStateException(exceptionMsg);

}

}

code:

/**

* Checks if return statement is not empty

* @param aReturnAst

*/

private static boolean isNonEmptyReturn(DetailAST aReturnAst) {

return aReturnAst.findFirstToken(TokenTypes.EXPR) != null;

}

/**

* Checks if condition in for-loop is not empty

* @param aForConditionAst

*/

private static boolean isNonEmptyForCondition(DetailAST aForConditionAst) {

return aForConditionAst.getFirstChild() != null;

}

Some code could not be written without "NOT"/"NON" , Example:

https://github.com/checkstyle/checkstyle/blob/master/src/main/java/com/puppycrawl/tools/checkstyle/checks/whitespace/EmptyLineSeparatorCheck.java method "hasNotAllowedTwoEmptyLinesBefore" .

Syntactically parallel elements in indentation

Why it is matter ?

https://google-styleguide.googlecode.com/svn-history/r130/trunk/javaguide.html#s4.5.1-line-wrapping-where-to-break
"The prime directive of line-wrapping is: prefer to break at a higher syntactic level."

https://google-styleguide.googlecode.com/svn-history/r130/trunk/javaguide.html#s4.5.2-line-wrapping-indent
"When there are multiple continuation lines, indentation may be varied beyond +4 as desired. In general, two continuation lines use the same indentation level if and only if they begin with syntactically parallel elements."

Cases from real code with bad indentation:
1)

 if ((rcurly != null) && !level.accept(rcurlyPos)
           && (rcurlyMustStart() || startsLine(rcurly))
      && !areOnSameLine(rcurly, lcurly))


2)
details.append("Focused : ").append(file.getPath())
.append(", start date :")
.append(", end date : ");

3)
if (toVisitAst != null
&& (toVisitAst.getParent().getParent().getNextSibling() == null
|| toVisitAst.getParent().getParent().getNextSibling().getType()
== TokenTypes.RCURLY)
&& toVisitAst.getType() == TokenTypes.LITERAL_RETURN
&& toVisitAst.getParent().getNextSibling()== null) { }


===========================


resulted in https://github.com/checkstyle/checkstyle/issues/270

Static Import Check for Checkstyle

Static import: http://docs.oracle.com/javase/1.5.0/docs/guide/language/static-import.html

Command to catch code:
 grep --include=*.java --exclude={*Test.java,Test*.java,*IT.java} -rw -m 1 . -e "import static"

we can not forbid static import usage at all, it is useful, we need to limit it, So we will forbid all static imports that are not applying to following options:

1) allow in target member is used more then X times.
see example in point "3)" and imagine there is a lot of such code blocks, that option could allow static import for "psvBuilder".

2) allow in Class of target member could brake Line-limit more then X times.
 that it is not required as will hide too much code that should be formatted appropriately

3) Allow in complicated statements with a lot of inner methods calls in arguments.

import static com.commons.domain.DsvFileDefinition.psvBuilder;
import static com.commons.domain.FeedDefinition.feedBuilder;
import static com.commons.domain.ZipFileDefinition.zipBuilder;

private static FeedDefinition getFeedDefinition() {
return feedBuilder("Premium", "/datafeeds/", "NAME", "NAME1").addFile(
zipBuilder("premium").addFiles(of(
psvBuilder("address").build(),
psvBuilder("entity").build(),public enum BetPosition {
    // ...
    STRAIGHT_32(96, ...),
    STRAIGHT_33(142, ...),
    STRAIGHT_34(52, ...),
    BET_1_2(53, ...),
    BET_4_5(55, ...),
    // ... long list of enum values

psvBuilder("entity_changes").build(),
psvBuilder("entity_identifiers").build(),
psvBuilder("entity_names").build(),
psvBuilder("entity_profiles").build(),
psvBuilder("entity_relationships").build(),
psvBuilder("entity_structure").build(),
psvBuilder("entity_map").build(),
psvBuilder("listing_map").build())).build()
).build();


4) Ignore list of classes/methods that team presume distinctive and whole team know.
"com.google.common.base.Strings,com.google.common.base.XXXXXXX"

import static com.google.common.base.Strings.isNullOrEmpty;
import static com.google.common.base.Strings.nullToEmpty;

this.schemaName = nullToEmpty(schemaName);

if (isNullOrEmpty(tableName)) {

throw new IllegalArgumentException("tableName could not be empty");

}

this.tableName = tableName;

5)
Invalid usage:
import static com.google.common.collect.Iterables.getFirst;
....

TreeSet<Name> names = Sets.newTreeSet(new ILifeCycleEntityComparator());
if (names.isEmpty()) {
// do smth 
} else {
Name firstName = getFirst(tradenames, null);public enum BetPosition {
    // ...
    STRAIGHT_32(96, ...),
    STRAIGHT_33(142, ...),
    STRAIGHT_34(52, ...),
    BET_1_2(53, ...),
    BET_4_5(55, ...),
    // ... long list of enum values

// do smth else
}

6)
Here looks like static import allow to keep lines short.

import static com.revere.det.core.domain.validation.ValidationUtils.formatPeriod;

details.append("Focused sector: ").append(focusSector.getPath())
.append(", start date of focus: ").append(formatPeriod(focus.getStartDate()))
.append(", end date of focus: ").append(formatPeriod(focus.getEndDate()))
.append(NEW_LINE);

7)
Inappropriate - used only one time in whole file.

import static org.joda.time.LocalDateTime.now;

public class Builder extends DataBuilder {

private static final java.sql.Date THREE_YEARS_AGO = new java.sql.Date(now().minusYears(3).toDate().getTime());

=======================

https://code.google.com/p/guava-libraries/wiki/FunctionalExplained

Even using static imports, even if the Function and the Predicate declarations are moved to a different file, the first implementation is less concise, less readable, and less efficient.   

=================

Here's a shortened version of a class where static import is suitable:

===============================
import static com.some.company.BetPosition.*;

public class NumberMapping {

    private final BetPosition[][] numberToPositions = new BetPosition[][] {
            /* 0 number */{ STRAIGHT_0, BET_0_1, BET_0_2, BET_0_3, BET_0_1_2, BET_0_2_3, BET_0_1_2_3 },                 
            // ... 1-35 ...
            /* 36 number */{ STRAIGHT_36, BET_35_36, BET_33_36, BET_34_35_36, BET_32_33_35_36, BET_31_32_33_34_35_36,
                    TOP_2_TO_1, DOZEN_3_RD_12, HIGH_19_TO_36, RED, EVEN } };

    public BetPosition[] getPositions(int number) {
        BetPosition[] positions = numberToPositions[number];
        // some logic
        return positions;
    }
}

Where BetPosition is an enum with such a content:

public enum BetPosition {
    // ...
    STRAIGHT_32(96, ...),
    STRAIGHT_33(142, ...),
    STRAIGHT_34(52, ...),
    BET_1_2(53, ...),
    BET_4_5(55, ...),

    // ... long list of enum values

====================================

TODO..... investigate Guava code to have be sure that their code could follow that Check.

Issue is registered:
https://github.com/sevntu-checkstyle/sevntu.checkstyle/issues/453 

How to use snapshot checkstyle version in maven plugin

Base on knowledge from http://maven.apache.org/plugins/maven-checkstyle-plugin/examples/custom-developed-checkstyle.html#Configure_the_Checkstyle_Plugin_to_use_your_custom_checks

you can easily force to maven checkstyle plugin to use custom/snapshot/different version of Checkstyle library.
It is very useful for testing of new Checks and generating HTML report on big sources on checkstyle library that you want and not a default old Checkstyle version that is attached to plugin.

<project>
...
  <build>
    <pluginManagement>
      <plugins>
....
   <plugin>
     <groupId>org.apache.maven.plugins</groupId>
     <artifactId>maven-checkstyle-plugin</artifactId>
     <version>2.15</version>
     <dependencies>
       <dependency>
   <groupId>com.puppycrawl.tools</groupId>
         <artifactId>checkstyle</artifactId>
          <version>6.5-SNAPSHOT</version>
       </dependency>
     </dependencies>

        <configuration>

          <configLocation>my_checkstyle.xml</configLocation>
          <enableFilesSummary>false</enableFilesSummary>

        </configuration>

   </plugin>        
.....
      </plugins>
    </pluginManagement>
  </build>
......

  <reporting>

    <plugins>
     <!-- that plugin is required to link violation with source code, without that plugin report is useless and hard to use -->
      <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-jxr-plugin</artifactId>
        <version>2.5</version>
      </plugin>

      <plugin>

        <groupId>org.apache.maven.plugins</groupId>

        <artifactId>maven-checkstyle-plugin</artifactId>

        <version>2.15</version>
<!-- Specifying configuration here will take effect on the execution of "mvn site",
          but will not take effect on the execution of "mvn checkstyle:checkstyle"  -->
<configuration>
           <configLocation>checkstyle.xml</configLocation>
            <failOnViolation>false</failOnViolation>
            <enableFilesSummary>false</enableFilesSummary>
</configuration>

      </plugin>

    </plugins>

  </reporting>

....

</project>

Build HTML report by "mvn site" command, or "mvn checkstyle:checkstyle".

to have different HTML reports (for each tested Check) over the same sources - just save index.html as index-MyCheck.html, and do report generation for each Check separately.

Sevntu Checkstyle release 1.12.0

new release of Checkstyle extension (Sevntu.Checkstyle) 1.12.0, with new checks:
DiamondOperatorForVariableDefinitionCheck,
PublicReferenceToPrivateTypeCheck,
TernaryPerExpressionCountCheck

Updates in:
RedundantReturnCheck
CustomDeclarationOrderCheck
LineLengthCheck
ForbidThrowAnonymousExceptionsCheck,

Details:
http://daniilyar.blogspot.com/2014/10/sevntu-checkstyle-release-112-checks.html

Google Java Style - mandatory braces rules

During development of Checkstyle configuration for Google Java Style,  we found two rules that are looks like conflicting . 

In 4.1.1 Google say:

Braces are used with if, else, for, do and while statements, even when the body is empty or contains only a single statement.

in 4.1.3 Google say:

An empty block or block-like construct may be closed immediately after it is opened, with no characters or line break in between ({}), unless it is part of a multi-block statement (one that directly contains multiple blocks: if/else-if/else or try/catch/finally).

So in 4.1.1 IF-ELSE could be {}, but in 4.1.3 there is unclear "unless" that either conflict with 4.1.1 or mean smth else that it not described.

After long discussion with friends and investigation :

Rule 4.1.1 tell us to always use braces for statements that use blocks

// BAD

if () {

  ...

} else

  i++;

It must be

// GOOD

if () {

  ...

} else {

  i++;

}

// it is OK, old fashioned but ok

int index = 0;

for (; index < s.length() && s[index] != 'x'; index++) {}

// OK, side effect in condition expression is bad design , but it is not matter of style/formatting 

while ((r = in.read()) != 0) {}

So ELSE is just another structure that could have braces so it have to have braces no matter what.

Rule 4.1.3 extend 4.1.1 and demand that braces should not be empty in multi-block statements, small wording conflict in ELSE case is not intentional and if apply rules in sequence 4.1.1 and then 4.1.3 all will be fine:

// BAD

try {

  doX();

} catch (Exception e) {}

// BETTER

try {

  doX();

} catch (Exception e) {

  // Here's why I'm swallowing this exception

}

// BAD

if (expr) {} else {

  doSomething();

}

// BETTER

if (expr) {

  // Here's why I'm not doing anything here....

} else {

  doSomething();

}

Sevntu Checkstyle release 1.11.0

official page: http://sevntu-checkstyle.github.io/sevntu.checkstyle/#1.11.0


 New and noteworthy:

1) new Check "Enum Values Check", issue, done by Pavel Baranchikov.

Check forces enum values to match the specific pattern. According to "Java Coding Style" by Achut Reddy p 3.3 constants include "all static final object reference types that are never followed by " ." (dot).", i.e. enums, which are followed by dot while used in the code are to be treated as static object references, while enums, that are not used with following dot, should be treated as constants.

Enums are defined to be used as class have some own methods. This condition is used to distinguish between Values Enumeration and Class Enumeration. Values Enumeration looks like the following:

enum SimpleErrorEnum
  {
      FIRST_SIMPLE, SECOND_SIMPLE, THIRD_SIMPLE;
  }

While Class Enumeration has some methods, for example:

 enum SimpleErrorEnum
   {
       FIRST_SIMPLE, SECOND_SIMPLE, THIRD_SIMPLE;
 
       public String toString() {
           return Integer.toString(ordinal() + 10);
       }
   }

Name format for Class Enumeration is specified with setObjFormat(String) , while format for enum constants - with setConstFormat(String)

To avoid assuming enum as static object reference, while using some specific methods, setExcludes(List) can be used. For example to make enum in the previous example a constant set Excludes property to a value toString

By default toString is used as an exclusion.

2) new Check "Map Iteration In For Loop", issue, done by Max Vetrenko

This check can help you to write the whole for-each map iteration more correctly:

1. If you iterate over a map using map.keySet() or map.entrySet(), but your code uses only map values, Check will propose you to use map.values() instead of map.keySet() or map.entrySet(). Replacing map.keySet() or map.entrySet() with map.values() for such cases can a bit improve an iteration performance.Bad:
   

   for (Map.Entry; entry : map.entrySet()){
      System.out.println(entry.getValue());
   }

   for (String key : map.keySet(){
      System.out.println(map.get(key));
   }

   Good:
   

   for (String value : map.values()){
      System.out.println(value);
   }

2. If you iterate over a map using map.entrySet(), but never call entry.getValue(), Check will propose you to use map.keySet() instead of map.entrySet(). to iterate over map keys only.Bad:
   

   for (Map.Entry entry : map.entrySet()){
      System.out.println(entry.getKey());
   }

   Good:
   

   for (String key : map.keySet()){
      System.out.println(key);
   }

3. If you iterate over a map with map.keySet() and use both keys and values, check will propose you to use map.entrySet() to improve an iteration performance by avoiding search operations inside a map. For this case, iteration can significantly grow up a performance.Bad:
   

   for (String key : map.keySet()){
      System.out.println(key + "  " + map.get(key));
   }

   Good:
   

   for (Map.Entry entry : map.entrySet()){
      System.out.println(entry.getValue() + "   " + entry.getKey());
   }

3) new check "Forbid Throw Anonymous Exception", issue, done by Max Vetrenko.

Forbid throwing anonymous exception. limitation: This Check does not validate cases then Exception object is created before it is thrown.
For example:

catch (Exception e) {
   throw new RuntimeException() { //anonymous exception
    //some code
   };

4) new check "Finalize Implementation", issue, done by Max Vetrenko.

This Check detects 3 most common cases of incorrect finalize() method implementation:

• negates effect of superclass finalize
  

  protected void finalize() { }
  protected void finalize() { doSomething(); }

• useless (or worse) finalize
  

  protected void finalize() { super.finalize(); }

• public finalize
  

  public void finalize() {
     try { doSomething(); } 
     finally { super.finalize(); } 
  }
  

5) Update for CustomDeclarationOrderCheck, issue, done by Baratali Izmailov.

details, In short:

1) macros for Fields with assigning to Anonymous classes
2) macros for Setter/Geter
3) macros for nested interfaces, nested enumarations
4) Inner Classes in methods are ignored
5) macros for "main()" method

Format with usage of all marcoses could be found there.

General information:
repository: https://github.com/sevntu-checkstyle/sevntu.checkstyle
mail-list: https://groups.google.com/forum/?hl=en#!forum/sevntu-checkstyle