Issue #13553: False positive in FallThroughCheck on last case

config/checker-framework-suppressions/checker-lock-tainting-suppressions.xml

@@ -44,6 +44,36 @@
  </details>
  </checkerFrameworkError>
 
+ <checkerFrameworkError unstable="false">
+ <fileName>src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/FallThroughCheck.java</fileName>
+ <specifier>methodref.param</specifier>
+ <message>Incompatible parameter type for obj</message>
+ <lineContent>.filter(Objects::nonNull)</lineContent>
+ <details>
+ found : @GuardSatisfied Object
+ required: @GuardedBy DetailAST
+ Consequence: method in @GuardedBy Objects
+ @GuardedBy boolean nonNull(@GuardSatisfied Object p0)
+ is not a valid method reference for method in @GuardedBy Predicate&lt;@GuardedBy DetailAST&gt;
+ @GuardedBy boolean test(@GuardedBy Predicate&lt;@GuardedBy DetailAST&gt; this, @GuardedBy DetailAST p0)
+ </details>
+ </checkerFrameworkError>
+
+ <checkerFrameworkError unstable="false">
+ <fileName>src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/FallThroughCheck.java</fileName>
+ <specifier>methodref.param</specifier>
+ <message>Incompatible parameter type for obj</message>
+ <lineContent>Objects::nonNull,</lineContent>
+ <details>
+ found : @GuardSatisfied Object
+ required: @GuardedBy DetailAST
+ Consequence: method in @GuardedBy Objects
+ @GuardedBy boolean nonNull(@GuardSatisfied Object p0)
+ is not a valid method reference for method in @GuardedBy Predicate&lt;@GuardedBy DetailAST&gt;
+ @GuardedBy boolean test(@GuardedBy Predicate&lt;@GuardedBy DetailAST&gt; this, @GuardedBy DetailAST p0)
+ </details>
+ </checkerFrameworkError>
+
  <checkerFrameworkError unstable="false">
  <fileName>src/main/java/com/puppycrawl/tools/checkstyle/Main.java</fileName>
  <specifier>method.guarantee.violated</specifier>

config/checker-framework-suppressions/checker-nullness-optional-interning-suppressions.xml

@@ -1766,13 +1766,6 @@
  <lineContent>.isPresent();</lineContent>
  </checkerFrameworkError>
 
- <checkerFrameworkError unstable="false">
- <fileName>src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/FallThroughCheck.java</fileName>
- <specifier>introduce.eliminate</specifier>
- <message>It is bad style to create an Optional just to chain methods to get a non-optional value.</message>
- <lineContent>.orElse(Boolean.FALSE);</lineContent>
- </checkerFrameworkError>
-
  <checkerFrameworkError unstable="false">
  <fileName>src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/FinalLocalVariableCheck.java</fileName>
  <specifier>dereference.of.nullable</specifier>

src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/FallThroughCheck.java

@@ -20,9 +20,11 @@
 package com.puppycrawl.tools.checkstyle.checks.coding;
 
 import java.util.HashSet;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.regex.Pattern;
+import java.util.stream.Stream;
 
 import com.puppycrawl.tools.checkstyle.StatelessCheck;
 import com.puppycrawl.tools.checkstyle.api.AbstractCheck;
@@ -42,9 +44,9 @@
  * "fallthru", "fall thru", "fall-thru",
  * "fallthrough", "fall through", "fall-through"
  * "fallsthrough", "falls through", "falls-through" (case-sensitive).
- * The comment containing these words must be all on one line,
- * and must be on the last non-empty line before the {@code case} triggering
- * the warning or on the same line before the {@code case}(ugly, but possible).
+ * The comment containing these words must be on the last non-empty line
+ * before the {@code case} triggering the warning or on the same line before
+ * the {@code case}(ugly, but possible).
  * </p>
  * <p>
  * Note: The check assumes that there is no unreachable code in the {@code case}.
@@ -454,11 +456,19 @@ private boolean hasFallThroughComment(DetailAST currentCase) {
  * @return true if relief comment found
  */
  private boolean hasReliefComment(DetailAST ast) {
- return Optional.ofNullable(getNextNonCommentAst(ast))
- .map(DetailAST::getPreviousSibling)
- .map(previous -> previous.getFirstChild().getText())
- .map(text -> reliefPattern.matcher(text).find())
- .orElse(Boolean.FALSE);
+ final DetailAST nonCommentAst = getNextNonCommentAst(ast);
+ boolean result = false;
+ if (nonCommentAst != null) {
+ final int prevLineNumber = nonCommentAst.getPreviousSibling().getLineNo();
+ result = Stream.iterate(nonCommentAst.getPreviousSibling(),
+ Objects::nonNull,
+ DetailAST::getPreviousSibling)
+ .takeWhile(sibling -> sibling.getLineNo() == prevLineNumber)
+ .map(DetailAST::getFirstChild)
+ .filter(Objects::nonNull)
+ .anyMatch(firstChild -> reliefPattern.matcher(firstChild.getText()).find());
+ }
+ return result;
  }
 
 }

src/main/resources/com/puppycrawl/tools/checkstyle/meta/checks/coding/FallThroughCheck.xml

@@ -15,9 +15,9 @@
  "fallthru", "fall thru", "fall-thru",
  "fallthrough", "fall through", "fall-through"
  "fallsthrough", "falls through", "falls-through" (case-sensitive).
- The comment containing these words must be all on one line,
- and must be on the last non-empty line before the {@code case} triggering
- the warning or on the same line before the {@code case}(ugly, but possible).
+ The comment containing these words must be on the last non-empty line
+ before the {@code case} triggering the warning or on the same line before
+ the {@code case}(ugly, but possible).
  </p>
  <p>
  Note: The check assumes that there is no unreachable code in the {@code case}.

src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/FallThroughCheckTest.java

@@ -252,7 +252,6 @@ public void testLastCase() throws Exception {
  final String[] expected = {
  "48:11: " + getCheckMessage(MSG_FALL_THROUGH_LAST),
  "83:11: " + getCheckMessage(MSG_FALL_THROUGH_LAST),
- "112:11: " + getCheckMessage(MSG_FALL_THROUGH_LAST),
  };
  verifyWithInlineConfigParser(
  getPath("InputFallThrough4.java"),
@@ -331,11 +330,7 @@ public void testFallThrough7() throws Exception {
  public void testLastLine() throws Exception {
  final String[] expected = {
  "21:13: " + getCheckMessage(MSG_FALL_THROUGH),
- // until https://github.com/checkstyle/checkstyle/issues/13553
- "33:13: " + getCheckMessage(MSG_FALL_THROUGH),
  "99:39: " + getCheckMessage(MSG_FALL_THROUGH_LAST),
- // until https://github.com/checkstyle/checkstyle/issues/13553
- "107:11: " + getCheckMessage(MSG_FALL_THROUGH_LAST),
  };
  verifyWithInlineConfigParser(
  getPath("InputFallThroughLastLineCommentCheck.java"),
@@ -355,12 +350,7 @@ public void testLastLine2() throws Exception {
 
  @Test
  public void testReliefCommentBetweenMultipleComment() throws Exception {
- final String[] expected = {
- // until https://github.com/checkstyle/checkstyle/issues/13553
- "25:17: " + getCheckMessage(MSG_FALL_THROUGH),
- // until https://github.com/checkstyle/checkstyle/issues/13553
- "34:13: " + getCheckMessage(MSG_FALL_THROUGH_LAST),
- };
+ final String[] expected = {};
  verifyWithInlineConfigParser(
  getPath("InputFallThrough8.java"),
  expected);
@@ -383,12 +373,45 @@ public void testLabeledBreak() throws Exception {
 
  @Test
  public void testSwitchLabeledRules() throws Exception {
- final String[] expected = {
+ final String[] expected = {};
+
+ verifyWithInlineConfigParser(
+ getNonCompilablePath("InputFallThroughSwitchRules.java"),
+ expected);
+ }
 
+ @Test
+ public void testInlineSingleCase() throws Exception {
+ final String[] expected = {
+ "12:17: " + getCheckMessage(MSG_FALL_THROUGH_LAST),
  };
 
  verifyWithInlineConfigParser(
- getNonCompilablePath("InputFallThroughSwitchRules.java"),
+ getPath("InputFallThroughInlineSingleCase.java"),
+ expected);
+ }
+
+ @Test
+ public void testInlineMultipleComment() throws Exception {
+ final String[] expected = {};
+
+ verifyWithInlineConfigParser(
+ getPath("InputFallThroughMultipleReliefPatterns.java"),
+ expected);
+ }
+
+ @Test
+ public void testFallThroughWithoutReliefPattern() throws Exception {
+ final String[] expected = {
+ "21:9: " + getCheckMessage(MSG_FALL_THROUGH),
+ "45:9: " + getCheckMessage(MSG_FALL_THROUGH),
+ "54:9: " + getCheckMessage(MSG_FALL_THROUGH),
+ "60:9: " + getCheckMessage(MSG_FALL_THROUGH),
+ "77:9: " + getCheckMessage(MSG_FALL_THROUGH),
+ "94:9: " + getCheckMessage(MSG_FALL_THROUGH),
+ };
+ verifyWithInlineConfigParser(
+ getPath("InputFallThroughWithoutReliefPattern.java"),
  expected);
  }
 }

src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/fallthrough/InputFallThrough4.java

@@ -109,7 +109,7 @@ void method4(int i, int j, boolean cond) {
  void method5(int i, int j, boolean cond) {
  while (true) {
  switch (i){
- case 5: // violation 'Fall\ through from the last branch of the switch statement'
+ case 5:
  i++;
  /* block */ /* fallthru */ // comment
  }

src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/fallthrough/InputFallThrough8.java

@@ -22,7 +22,7 @@ void methodLastLine(int i) {
  case 2:
  i++;
  /* comment */ /* fall thru */ /* comment */
- case 3: // violation 'Fall\ through from previous branch of the switch statement'
+ case 3:
  i--;
  break;
  }
@@ -31,7 +31,7 @@ void methodLastLine(int i) {
 
  void testLastCase(int i) {
  switch (i) {
- case 0: // violation 'Fall\ through from the last branch of the switch statement'
+ case 0:
  i++;
  /* comment */ /* fall thru */ /* comment */
  }

src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/fallthrough/InputFallThroughInlineSingleCase.java

@@ -0,0 +1,15 @@
+/*
+FallThrough
+checkLastCaseGroup = true
+reliefPattern = (default)falls?[ -]?thr(u|ough)
+
+
+*/
+package com.puppycrawl.tools.checkstyle.checks.coding.fallthrough;
+
+public class InputFallThroughInlineSingleCase{
+ void method(int a) {
+ switch (a) {case 1:;}
+ // violation above 'Fall\ through from the last branch of the switch statement.'
+ }
+}

src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/fallthrough/InputFallThroughLastLineCommentCheck.java

@@ -30,7 +30,7 @@ public void method2(int i) {
  case 1:
  i++;
  /* block */ /* fallthru */ // comment
- case 2: // violation 'Fall\ through from previous branch of the switch statement'
+ case 2:
  // this is comment
  i++;
  // fall through
@@ -104,7 +104,7 @@ void method6(String str) {
  void method7(int i, int j, boolean cond) {
  while (true) {
  switch (i){
- case 5: // violation 'Fall\ through from the last branch of the switch statement'
+ case 5:
  i++;
  /* block */ i++; /* fallthru */ // comment
  }

src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/fallthrough/InputFallThroughMultipleReliefPatterns.java

@@ -0,0 +1,26 @@
+/*
+FallThrough
+checkLastCaseGroup = true
+reliefPattern = (default)falls?[ -]?thr(u|ough)
+
+
+*/
+package com.puppycrawl.tools.checkstyle.checks.coding.fallthrough;
+
+public class InputFallThroughMultipleReliefPatterns {
+
+ void method(int i) {
+ while (true) {
+ switch (i) {
+ case 5: {
+ i++;
+ }
+ /* block */ /* fallthru */ // comment
+ case 6:
+ i++;
+ /* block */ /* fallthru */ // comment
+
+ }
+ }
+ }
+}