skipEnhancedForLoopVariable property is added in ModifiedControlVaria…

[Bhavik3]

…bleCheck. solves #1015

[romani]

getDefaultTokens()

please do me favour, reuse getAcceptableTokens in all related methods.

private boolean shouldCheckEnhancedForLoopVariable(DetailAST ast) {

please move that method below Override's, and after first usage of it as close as possible. Please use that rule in future. Do not throw methods at any place.

private void leaveForEach(DetailAST forEach) {

Do one return point from method !!

private void leaveForDef(DetailAST ast) {

I do not like that IF in that method, please try to move it to "public void leaveToken(DetailAST ast) {" at we know where we are(EnchantedFor or just FOR) at that level already.

[Bhavik3]

Done

[romani]

0. please always reply me "done" to each point that I mentioned. Or explain a reason if you disagree.

 case TokenTypes.LITERAL_FOR:
if (!getCurrentVariables().isEmpty()) {
leaveForDef(ast);
.....
 private void leaveForEach(DetailAST forEach) {
final DetailAST paramDef =
forEach.findFirstToken(TokenTypes.VARIABLE_DEF);
if (shouldCheckEnhancedForLoopVariable(paramDef))
.....

What is a reason of calling method if you go out of by condition almost right away, if you move "shouldCheckEnhancedForLoopVariable" to SWITCH , it will be much better to read an algorithm of Check. Please try.

2. please generate report before and after, no difference is expected in default config.
   One Spring project (or any other big project that have a code that will be highlighted by new option) please activate your new option and review each violation to make sure it is valid, share report too.

[Bhavik3]

0 )
That was common 'done' for all.

It is good idea to take out shouldCheckEnhancedForLoopVariable.
In that case, I need to define this two times (in the method and in the SWITCH):
final DetailAST paramDef = ast.findFirstToken(TokenTypes.VARIABLE_DEF);

If that is ok, then fine. But my solution is to change the method leaveForEach(DetailAST forEach) to leaveForEach(DetailAST paramDef).

Before:
http://bhavik3.github.io/Before/guava/checkstyle.html

After:
With skipEnhancedForVariable: false (same as before)
http://bhavik3.github.io/After/guava/False/checkstyle.html

With skipEnhancedForVariable: true (two for each variable is skipped)
http://bhavik3.github.io/After/guava/True/checkstyle.html

[romani]

That was common 'done' for all.

Please keep typing "done" for each. I had so many cases when people skip my point and that the only way for contributors to recheck themself.

But my solution is to change the method leaveForEach(DetailAST forEach) to leaveForEach(DetailAST paramDef).

that is private method, do not hesitate to refactor it.

Files

in HTML report you have "Files" group , are you sure that you have the latest checkstyle-tester ? (I removed it form report a while ago)

reports are good, please confirm that there is no Exceptions base on launch over all projects in property file, see README for "./launch.sh -Dcheckstyle.config.location=all-checks-test-for-exceptions.xml"

[Bhavik3]

ok, I have made changes accordingly.

It is possible that I am using older version of checkstyle-tester.
https://github.com/Bhavik3/checkstyle-tester
give me the link to newer one.

This option is not available in older version I think.

[romani]

give me the link to newer one

https://github.com/checkstyle/contribution/tree/master/checkstyle-tester

[Bhavik3]

No Exception.
http://bhavik3.github.io/After/guava/Exceptions/checkstyle.html

[romani]

please recheck your properties file to be sure that all projects are activated , I expect about 41K java file to be tested, you have only 1691 files.

[Bhavik3]

@romani
Due to some connectivity problem or other technical problem, I can not clone findbugs project. so, I have skipped that one. others are added. report is updated now.
http://bhavik3.github.io/After/guava/Exceptions/checkstyle.html

I think point is clear from this report. it is checking about over 27K files. No Errors

[romani]

merged as FF