Regression :- https://kevin222004.github.io/Regression/

@romani please look at the config of this file and this part of code

can we update the documentation of https://checkstyle.org/config_coding.html#FallThrough:~:text=To%20configure%20the%20check%20to%20enable,have%20statement%20that%20transfer%20control%0A%20%20%20%20i%2B%2B%3B%0A%7D
It is been not getting clear that what exactly this property do

as per the doc the code example i provide must be violation

please look at the config of this file and this part of code

@Kevin222004 please reproduce by CLI to make it clear what you are asking; to me it looks like while we do have checkLastCaseGroup=true, we also have fall through comment to suppress violation. Am I missing something?

can you please explain the checkLastCaseGroup i have read the docs but i think still i don't gain proper knowledge of it
ok as per cli testing

public class Test {

   void methodFallThruCOtherWords(int i, int j, boolean cond) {
      while (true) {
          switch (i){
          case 0:
              i++; /* falls through */

          case 1:
              i++;
          /* falls through */
          case 2:
              i++;
          /* falls through */case 3:
                break;
          case 4:
              i++;
          /* falls through */
          }
      }
   }
}


<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
          "http://www.puppycrawl.com/dtds/configuration_1_3.dtd">

<module name="Checker">
    <module name="TreeWalker">
        <module name="FallThrough">
            <property name="checkLastCaseGroup" value="true"/>
            <property name="reliefPattern" value="(default)falls?[ -]?thr(u|ough)"/>
         </module>
    </module>
</module>

kevin@inspiron-15-5510:~/Desktop/check_style$ java -jar /home/kevin/Downloads/checkstyle-10.9.3-all.jar -c config.xml Test.java
Starting audit...
[ERROR] /home/kevin/Desktop/check_style/Test.java:9:11: Fall through from previous branch of the switch statement. [FallThrough]
[ERROR] /home/kevin/Desktop/check_style/Test.java:12:11: Fall through from previous branch of the switch statement. [FallThrough]
[ERROR] /home/kevin/Desktop/check_style/Test.java:14:30: Fall through from previous branch of the switch statement. [FallThrough]
[ERROR] /home/kevin/Desktop/check_style/Test.java:16:11: Fall through from the last branch of the switch statement. [FallThrough]
Audit done.
Checkstyle ends with 4 errors.

in cli output is perfect but in input file it is not showing violation for line 16 which i have pointed in #12966 (comment)

let me push the new changes so you can see the change properly

@Kevin222004 take a look at your config:

<property name="reliefPattern" value="(default)falls?[ -]?thr(u|ough)"/>

With our inline config parser, we use (default) notation, in real config, if you use (default), this is literally defining the comment regexp to create a matching group for default, which then none of your comments match. I would expect no violations in your example above if we did not have (default) in regexp.

#12966 (comment) noticed after making pr i have changed only 1 input file.
can you please explain what does checkLastCaseGroup do i am not getting clear idea about it from documenation

checkLastCaseGroup do i am not getting clear idea about it from documenation

It might be a good idea to work on #12974 and play around with check via CLI to understand check behavior, I find that writing documentation always helps me to get a clearer idea about a concept. If we can improve documentation about this property, please do.

@nrmancuso @romani i have got that what does checkLastCaseGroup do and i have removed the changes in input you can check and start review

@Kevin222004 please generate regression report by Github action with a few different configs now that we have pushed changes

pmd failure is not expected will solve it and then generate regression

direction is good.
please fix CI and generate regression report to make sure there is no bad changes in behavior.

@Kevin222004 please create test input mentioned at #12950 (comment), since we closed that PR in favor of this one.

#12966 (comment) done

https://kevin222004.github.io/Regression/

looks fine

please continue review

Very good.
Please copy java snippets from regression
https://kevin222004.github.io/Regression/Orekit/index.html#A1
And in infinispan as good examples of numerous comments in case blocks.

@romani #12966 (comment) done

Does BDD violation message conflict with this check?

yes, it is, we should update our test Input files to not mention keyword "fall through" in violation message
Message:

We might use Fall\ through from previous branch of the switch statement (escaping of space) to not let Check to consider it as violation as it is not matching.
@Kevin222004 , please open issue on this.

It does not say it has to be the last comment on that line.

I pretty sure we discussed this situation above and agreed to make it as regression but mark as "until #....".
@Kevin222004 , please create issue on this and mark case with "until".

@romani I have opened both the issue

#13552
and
#13553 i have marked the case with comment

Fix violation [ERROR] [checkstyle] [ERROR] /root/project/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/FallThroughCheckTest.java:359:14: Comment has incorrect indentation level 13, expected is 12, indentation should be the same level as line 360. [CommentsIndentation]

After working too much on this check I still feel that my concept is not clear related to this check.
This line still confuses me a lot https://checkstyle.org/checks/coding/fallthrough.html#FallThrough:~:text=The%20comment%20containing%20these%20words%20must%20be%20all%20on%20one%20line%2C%20and%20must%20be%20on%20the%20last%20non%2Dempty%20line%20before%20the%20case%20triggering%20the%20warning%20or%20on%20the%20same%20line%20before%20the%20case

All the reviewers please share thoughts on #13545 (comment)

The most confusing part of this whole PR is related to this issue #13545
either we can throw a violation or not. In this we have two behaviours, If throw a violation then also regression found diff. if skip violation then also their is diff.

I am not sure this is happening with me only or what but I think it needs discussion to decide some proper rule.

@Kevin222004 , please fix [ERROR] Medium: Method com.puppycrawl.tools.checkstyle.checks.coding.FallThroughCheck.hasReliefComment(DetailAST) needlessly boxes a boolean constant [com.puppycrawl.tools.checkstyle.checks.coding.FallThroughCheck] At FallThroughCheck.java:[line 418] NAB_NEEDLESS_BOOLEAN_CONSTANT_CONVERSION

https://ci.appveyor.com/project/Checkstyle/checkstyle/builds/47885530/job/juiya9s7rrnd2wg0#L1794

@nrmancuso , PR is ready for final review

@Kevin222004 please generate reports one last time before we merge

Github, generate report

final-report-2:
https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/525cd1e_2023113630/reports/diff/index.html

Github, generate report

final-report-2.1:
https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/525cd1e_2023125427/reports/diff/index.html

@nrmancuso @romani Reports are similar as previous one

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

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