[clang-tools-extra] [clang-tidy] Fix readability-else-after-return for if statements appear in unbraced switch case labels (PR #181878)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-tidy

Author: Berkay Sahin (berkaysahiin)

<details>
<summary>Changes</summary>

- Updates matcher registration in ElseAfterReturnCheck to also handle switchCase(has(ifStmt(...))) patterns
- Adds a regression test for the unbraced switch-case scenario

Fixes #<!-- -->160033

---
Full diff: https://github.com/llvm/llvm-project/pull/181878.diff


3 Files Affected:

- (modified) clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp (+10-6) 
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4) 
- (modified) clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return.cpp (+16) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
index fccda912947eb..cb3818d93cc81 100644
--- a/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -169,14 +169,18 @@ void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) {
   const auto InterruptsControlFlow = stmt(anyOf(
       returnStmt().bind(InterruptingStr), continueStmt().bind(InterruptingStr),
       breakStmt().bind(InterruptingStr), cxxThrowExpr().bind(InterruptingStr)));
+
+  const auto IfWithInterruptingThenElse =
+      ifStmt(unless(isConstexpr()), unless(isConsteval()),
+             hasThen(stmt(anyOf(InterruptsControlFlow,
+                                compoundStmt(has(InterruptsControlFlow))))),
+             hasElse(stmt().bind("else")))
+          .bind("if");
+
   Finder->addMatcher(
       compoundStmt(
-          forEach(ifStmt(unless(isConstexpr()), unless(isConsteval()),
-                         hasThen(stmt(
-                             anyOf(InterruptsControlFlow,
-                                   compoundStmt(has(InterruptsControlFlow))))),
-                         hasElse(stmt().bind("else")))
-                      .bind("if")))
+          forEach(stmt(anyOf(IfWithInterruptingThenElse,
+                             switchCase(has(IfWithInterruptingThenElse))))))
           .bind("cs"),
       this);
 }
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 5c0060877a67f..ce3eee29949d6 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -218,6 +218,10 @@ Changes in existing checks
   <clang-tidy/checks/performance/move-const-arg>` check by avoiding false
   positives on trivially copyable types with a non-public copy constructor.
 
+- Improved :doc:`readability-else-after-return
+  <clang-tidy/checks/readability/else-after-return>` check by fixing missed
+  diagnostics when ``if`` statements appear in unbraced ``switch`` case labels
+
 - Improved :doc:`readability-enum-initial-value
   <clang-tidy/checks/readability/enum-initial-value>` check: the warning message
   now uses separate note diagnostics for each uninitialized enumerator, making
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return.cpp
index 220c7ba19fed0..63ece00dfde5f 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return.cpp
@@ -312,3 +312,19 @@ void testPPConditionals() {
   }
 #endif
 }
+
+void testSwitchCaseWithoutInnerCompound(int i, bool b) {
+  switch (i) {
+  case 0:
+    if (b) {
+      return;
+    } else {
+      // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'return'
+      // CHECK-FIXES: {{^}}    }
+      f(0);
+    }
+    break;
+  default:
+    break;
+  }
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/181878