Issue #12275: Resolve Pitest suppression for EmptyForInitializerPad-2

[Kevin222004]

Issue #12275: Resolve Pitest suppression for EmptyForInitializerPad-2

Diff Regression config: https://gist.githubusercontent.com/Kevin222004/a85bd5c1897e158b6495850ba792bf4b/raw/37385b138ced4f80b43ccf865900a812500f4f42/config.xml

[Kevin222004]

1. pitest report

   https://kevin222004.github.io/general/com.puppycrawl.tools.checkstyle.checks.whitespace/EmptyForInitializerPadCheck.java.html
   replaced call to com/puppycrawl/tools/checkstyle/api/DetailAST::getNextSibling with receiver

2. Mutated code

   current code
   final DetailAST semi = ast.getNextSibling();
   is Mutated to
   final DetailAST semi = ast;

3. Analysis

   If we change final DetailAST semi = ast.getNextSibling(); to final DetailAST semi = ast;
   it want make any difference because in this check we just care about the parenPad-> ( and ;
.getNextSibling() will point the condition of for statment statment if this is for loop and in the
   source code it returns as null.

   So the usage of .getNextSibling() is currently nothing so I think it is not possible to create a Unit
   test file

4. How to kill this

   removed the .getNextSibling()

Report of killed Mutation
https://kevin222004.github.io/emptpad/com.puppycrawl.tools.checkstyle.checks.whitespace/EmptyForInitializerPadCheck.java.html

[romani]

Ok, such code most likely existed for some good reason we need to find such java code.
Please provide regression diff report.
https://github.com/checkstyle/contribution/tree/master/checkstyle-tester#report-generation

Hint: https://github.com/checkstyle/checkstyle/wiki/How-to-Generate-and-Analyze-Pitest-Reports#how-to-find-java-code-that-can-kill-mutation

[romani]

Items

[romani]

Items

[Kevin222004]

Github, generate report

[github-actions]

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/15d9abb_2022091436/reports/diff/index.html

[romani]

ok to merge

[rnveach]

Diff Regression config: https://gist.githubusercontent.com/Kevin222004/a85bd5c1897e158b6495850ba792bf4b/raw/b3d6fd5577d468d81339aa9f80b1581729b22bb5/config.xml

https://checkstyle.org/config_whitespace.html#EmptyForInitializerPad
This config is not enough.
Please read https://github.com/checkstyle/contribution/tree/master/checkstyle-tester#checkstyle-pitest-regression and expand config to all permutations which this check can be.

so I think it is not possible to create a Unit test file

If nothing changes on new regression, prove to me you stepped through the source and there is no way to create an input file to kill this mutation and changing production code is the only option.
See #7797 , 4b .

To me it seems odd that we are removing references to a ; when code examples talk about ;.

[Kevin222004]

Github, generate report

[Kevin222004]

Updated config https://gist.githubusercontent.com/Kevin222004/a85bd5c1897e158b6495850ba792bf4b/raw/37385b138ced4f80b43ccf865900a812500f4f42/config.xml

[github-actions]

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/15d9abb_2022064517/reports/diff/index.html

[Kevin222004]

@rnveach please look at the updated config

[rnveach]

Updated config is better. It shows there is still no regression.

[romani]

AST structure might be changed from initial implementation.
https://checkstyle.sourceforge.io/apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#FOR_INIT
SEMI is sibling of FOR_INIT (the only token this Check run on).

I think we are good to merge this, in most worse case, user will report issue and we will fix it with full knowledge.

[Kevin222004]

To me it seems odd that we are removing references to a ; when code examples talk about ;.

@rnveach @romani if it is not looking good then we can make changes like this #12277 (review)

[rnveach]

prove to me you stepped through the source and there is no way to create an input file to kill this mutation and changing production code is the only option

In short, there is no input file we can create here.

In this case AST is FOR_INIT and the next sibling is SEMI. When looking at the code after the mutation, there is checks done against Line and Column numbers. My first attempt to examine this was to try and get the 2 ASTs on different lines, as this would cause violations to be on different lines and prove we need the violation to be on only one of them.

This failed to be done because of this check and the way our ASTs are formed. EmptyForInitializer is only for for loops which have no initialization (IE: for (;). Even with the FOR_INIT and SEMI on different lines, our AST structure always makes them the same line/column.

for (
 ;;) {}

creates:

        |      |--LITERAL_FOR -> for [3:0]
        |      |  |--LPAREN -> ( [3:4]
        |      |  |--FOR_INIT -> FOR_INIT [4:1]
        |      |  |--SEMI -> ; [4:1]

We have both tokens appear to be on the same line/column. It doesn't matter if there is comments inside the FOR_INIT as it is still the same behavior.

Since we can't get these 2 tokens to have different line/column, there isn't a way to kill the mutation as it is now.

[rnveach]

1 minor change.