[clang-tidy] Recognize labelled statements when simplifying boolean exprs

[LegalizeAdulthood]

Summary

[clang-tidy] Recognize labelled statements when simplifying boolean exprs

Inside a switch the caseStmt() and defaultStmt() have a nested statement
associated with them. Similarly, labelStmt() has a nested statement.
These statements were being missed when looking for a compound-if of the
form "if (x) return true; return false;" when the if is nested under one
of these labelling constructs.

Enhance the matchers to look for these nested statements using some
private matcher hasSubstatement() traversal matcher on case, default
and label statements. Add the private matcher hasSubstatementSequence()
to match the compound "if (x) return true; return false;" pattern.

• Add unit tests for private matchers and corresponding test infrastructure
• Add corresponding test file readability-simplify-bool-expr-case.cpp.
• Fix variable name copy/paste error in readability-simplify-bool-expr.cpp.
• Drop the asserts, which were used only for debugging matchers.
• Run clang-format on the whole check.
• Move local functions out of anonymous namespace and declare state, per LLVM style guide
• Declare labels constexpr
• Declare visitor arguments as pointer to const
• Drop braces around simple control statements per LLVM style guide
• Prefer explicit arguments over default arguments to methods

Fixes #27078

[LegalizeAdulthood]

clang-format

[JonasToth]

clang-tidy/readability/SimplifyBooleanExprCheck.cpp

386 ↗

(On Diff #180184)

const on values is uncommon in clang-tidy code. Please keep that consistent.

743 ↗

(On Diff #180184)

double space

745 ↗

(On Diff #180184)

redundant empty comment line

747 ↗

(On Diff #180184)

I think this assertion does not hold true from the matcher.
The matcher requires only hasDescendent(ifStmt()), but that does not ensure the first stmt is the ifStmt.

e.g.

case 10: {
  loggingCall();
  if(foo) ...

Is this excluded?
}

[aaron.ballman]

clang-tidy/readability/SimplifyBooleanExprCheck.cpp

511 ↗

(On Diff #180184)

binding -> Binding

529 ↗

(On Diff #180184)

binding -> Binding

Also, there is no "case statement" involved here. ;-)

533–540 ↗

(On Diff #180184)

The check duplication here is unfortunate -- can you factor out the hasDescendant() bits into a variable that is reused, and perhaps use anyOf(caseStmt(stuff()), defaultStmt(stuff())) rather than separate functions?

718 ↗

(On Diff #180184)

Don't use an identifier with the same name as a type, please.

719 ↗

(On Diff #180184)

Add a message to the assertion (same with the other ones).

[LegalizeAdulthood]

clang-tidy/readability/SimplifyBooleanExprCheck.cpp

386 ↗

(On Diff #180184)

I can change the code, but I don't understand the push back.

"That's the way it's done elsewhere" just doesn't seem like good reasoning.

I write const on values to signify that they are computed once and only once. What is gained by removing that statement of once-and-only-once?

533–540 ↗

(On Diff #180184)

I'm not a fan of duplication, either.

However, I have to know if it's a CaseStmt or DefaultStmt in the replacement code and that's tied to the id, so I'm not sure how to collapse it further.

719 ↗

(On Diff #180184)

I'm not sure what you're asking for. I based these asserts off the existing assert in the file.

743 ↗

(On Diff #180184)

What exactly is the problem?

745 ↗

(On Diff #180184)

Meh, it's not redundant. It's there to aid readability of a big block of text by visually separating it from the associated code.

Why is this a problem?

747 ↗

(On Diff #180184)

Look more carefully at the AST. CaseStmt has exactly one child. That child can either be a compound statement block (which was already correctly handled by the check) or it can be a single statement. This change enhances the check to handle the single statement child of the CaseStmt and DefaultStmt.

[riccibruno]

clang-tidy/readability/SimplifyBooleanExprCheck.cpp

719 ↗

(On Diff #180184)

Something like assert((SwitchCase != nullptr) && "Some appropriate message blablabla!");
Keep the parentheses around the operands of != even if they are not strictly needed
since otherwise some bots will complain.

[LegalizeAdulthood]

Update from review comments

[LegalizeAdulthood]

clang-tidy/readability/SimplifyBooleanExprCheck.cpp

719 ↗

(On Diff #180184)

OK, I can do that; I wasn't sure if it was being suggested that I use some custom assert macro that took the message as a parameter.

[LegalizeAdulthood]

Update from review comments

[LegalizeAdulthood]

No really, update from review comments :)

[aaron.ballman]

clang-tidy/readability/SimplifyBooleanExprCheck.cpp

386 ↗

(On Diff #180184)

"That's the way it's done elsewhere" just doesn't seem like good reasoning.

Keeping the code consistent with the vast majority of the remainder of the project is valid reasoning. I am echoing the request to drop the top-level const. We don't use this pattern for non-pointer/reference types and there's not a whole lot gained by using it inconsistently.

533–540 ↗

(On Diff #180184)

You can bind to the castStmt() and defaultStmt() matchers to see what you get back, no? e.g., anyOf(caseStmt(stuff()).bind("which"), defaultStmt(stuff()).bind("which")) and in the check, you can use isa<> on the node named "which" to see whether you matched the case or the default label.

[JonasToth]

clang-tidy/readability/SimplifyBooleanExprCheck.cpp

386 ↗

(On Diff #180184)

Plus we do have a clang-tidy check (in the pipeline) that could do that automatically if the LLVM projects decides to go that route. So we won't suffer in the future, if we add the const.

745 ↗

(On Diff #180184)

Its subjective, I wouldn't do it so I thought you might have overlooked it, if we prefer this its fine from my side.

[alexfh]

clang-tidy/readability/SimplifyBooleanExprCheck.cpp

517 ↗

(On Diff #180385)

Please put the more expensive matcher (hasDescendant may be *really* expensive) last. Same below.

[LegalizeAdulthood]

clang-tidy/readability/SimplifyBooleanExprCheck.cpp

386 ↗

(On Diff #180184)

You haven't addressed my point, which is that const is for values that don't change. Instead, you're just repeating "we haven't done it that way" instead of refuting the utility of const.

[LegalizeAdulthood]

Revive this patch on top-of-tree

[njames93]

A large portion of the changes, especially in the checks implementation file, appear to be NFC stylistic or formatting only fixes. While these changes are generally good, they shouldn't be a part of this patch. Instead they should be committed in their own NFC patch.
This makes it much easier to review the relevant changes needed to implement this new behaviour.

[LegalizeAdulthood]

In D56303#3266252, @njames93 wrote:

A large portion of the changes, especially in the checks implementation file, appear to be NFC stylistic or formatting only fixes. While these changes are generally good, they shouldn't be a part of this patch. Instead they should be committed in their own NFC patch.
This makes it much easier to review the relevant changes needed to implement this new behaviour.

It's going to be a significant amount of work to tease everything
apart and this patch has already been waiting literally for years.
Every time I put something up for review I get told "do this because
the style guide says so", so I anticipated those things and fixed them
pre-emptively.

Honestly, we need to make our review process more productive,
not less.

[LegalizeAdulthood]

clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp

72–74

FYI

These tabs here (and elsewhere) are phantoms somehow
introduced into the diff by Phabricator. My local diff file
that I uploaded has zero tabs in it anywhere.

[LegalizeAdulthood]

clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp

72–74

When I look more carefully, I see it's one column to the left of the
first column in the file.... so it's trying to somehow say that this line
had increased indent in the diff?

I thought it was a TAB indicator and another reviewer thought so as well.

But it isn't a TAB at all :)

[aaron.ballman]

In D56303#3270090, @LegalizeAdulthood wrote:

In D56303#3266252, @njames93 wrote:

A large portion of the changes, especially in the checks implementation file, appear to be NFC stylistic or formatting only fixes. While these changes are generally good, they shouldn't be a part of this patch. Instead they should be committed in their own NFC patch.
This makes it much easier to review the relevant changes needed to implement this new behaviour.

It's going to be a significant amount of work to tease everything
apart and this patch has already been waiting literally for years.

I don't think you need to tease it apart for this patch, but FWIW, I agree that this is much harder to review in this state.

Every time I put something up for review I get told "do this because
the style guide says so", so I anticipated those things and fixed them
pre-emptively.

The rule of thumb is: if you have significant NFC changes, do them first and push the changes (often doesn't even require a review because it's NFC, but that depends on what comfort level you have about the changes being NFC), and then do the functional changes on top of the improved code. Reviewers will ask you to update style for the lines of code you're touching when appropriate, but we (generally) shouldn't be asking you to fix style issues in code you're not touching. (Oddly, we touch on this in #2 at https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access which is not where I'd expect that information to live.)

FWIW, I looked through the changes and don't have major concerns. A second set of eyes approving this would be appreciated, but this LGTM.

clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp

72–74

Yeah, that's caught me a few times as well. The >> seems to mean "indentation changed" not "tab inserted".

423–448

I'd prefer the NOLINT comment be removed (we don't typically add those, same as clang-format comments or other special markings).

[LegalizeAdulthood]

In D56303#3272711, @aaron.ballman wrote:

The rule of thumb is: if you have significant NFC changes,
do them first and push the changes (often doesn't even
require a review because it's NFC, but that depends on
what comfort level you have about the changes being NFC),
and then do the functional changes on top of the improved
code.

This is reasonable, but is it written down anywhere so that people
don't have to through this trial by fire every time? :)

[LegalizeAdulthood]

Update from review comments

[aaron.ballman]

In D56303#3273694, @LegalizeAdulthood wrote:

In D56303#3272711, @aaron.ballman wrote:

The rule of thumb is: if you have significant NFC changes,
do them first and push the changes (often doesn't even
require a review because it's NFC, but that depends on
what comfort level you have about the changes being NFC),
and then do the functional changes on top of the improved
code.

This is reasonable, but is it written down anywhere so that people
don't have to through this trial by fire every time? :)

Yup, in about the most obtuse place you could image: https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access. We could probably stand to update https://llvm.org/docs/DeveloperPolicy.html to be more explicit about this and document it in a much more obvious place. (I agree with the complaint about us doing a poor job of writing these kinds of expectations down.)

Oops, forgot to accept the last time I said I thought this looked good. Please still give a day or two before landing in case @JonasToth or @alexfh have further comments or concerns.

[kwk]

clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp

1

@LegalizeAdulthood I'm doing standalone builds of the LLVM projects and we're seeing an error that this file cannot be found. I wonder why this even worked before because we're essentially checking out (or better extract) clang into /clang-15.0.0-src/ and clang-extra-tools into clang-extra-tools-15.0.0-src. So no matter how smart you design your -I include path flags, there`ll never be a ../../clang that contains the file to be included here. @tstellar does this ring a bell? I've looked at this with @serge-sans-paille today and be both thought this cannot work. Here's the logfile

builder-live.log.gz242 KBDownload

of this build (which might be unreachable in a few days): https://download.copr.fedorainfracloud.org/results/@fedora-llvm-team/llvm-snapshots/fedora-35-x86_64/04639555-clang/builder-live.log.gz

[njames93]

clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp

1

TBH I think this include, allTestClangConfigs below and the CMakeLists edits should have been removed in D125026. I'll put a patch up to address this.

clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h

private:
 
   void reportBinOp(const ast_matchers::MatchFinder::MatchResult &Result,
                    const BinaryOperator *Op);
 
   void matchBoolCondition(ast_matchers::MatchFinder *Finder, bool Value,
                           StringRef BooleanId);
 
   void matchTernaryResult(ast_matchers::MatchFinder *Finder, bool Value,
-                          StringRef TernaryId);
+                          StringRef Id);
 
   void matchIfReturnsBool(ast_matchers::MatchFinder *Finder, bool Value,
                           StringRef Id);
 
   void matchIfAssignsBool(ast_matchers::MatchFinder *Finder, bool Value,
                           StringRef Id);
 
   void matchCompoundIfReturnsBool(ast_matchers::MatchFinder *Finder, bool Value,
                                   StringRef Id);
 
+  void matchCaseIfReturnsBool(ast_matchers::MatchFinder *Finder, bool Value,
+                              StringRef Id);
+
+  void matchDefaultIfReturnsBool(ast_matchers::MatchFinder *Finder, bool Value,
+                                 StringRef Id);
+
+  void matchLabelIfReturnsBool(ast_matchers::MatchFinder *Finder, bool Value,
+                               StringRef Id);
+
   void
   replaceWithThenStatement(const ast_matchers::MatchFinder::MatchResult &Result,
                            const Expr *BoolLiteral);
 
   void
   replaceWithElseStatement(const ast_matchers::MatchFinder::MatchResult &Result,
-                           const Expr *FalseConditionRemoved);
+                           const Expr *BoolLiteral);
 
   void
   replaceWithCondition(const ast_matchers::MatchFinder::MatchResult &Result,
-                       const ConditionalOperator *Ternary,
-                       bool Negated = false);
+                       const ConditionalOperator *Ternary, bool Negated);
 
   void replaceWithReturnCondition(
       const ast_matchers::MatchFinder::MatchResult &Result, const IfStmt *If,
-      bool Negated = false);
+      bool Negated);
 
   void
   replaceWithAssignment(const ast_matchers::MatchFinder::MatchResult &Result,
-                        const IfStmt *If, bool Negated = false);
+                        const IfStmt *If, bool Negated);
 
   void replaceCompoundReturnWithCondition(
       const ast_matchers::MatchFinder::MatchResult &Result,
-      const CompoundStmt *Compound, bool Negated = false);
+      const CompoundStmt *Compound, bool Negated);
+
+  void replaceCompoundReturnWithCondition(
+      const ast_matchers::MatchFinder::MatchResult &Result, bool Negated,
+      const IfStmt *If);
+
+  void replaceCaseCompoundReturnWithCondition(
+      const ast_matchers::MatchFinder::MatchResult &Result, bool Negated);
+
+  void replaceDefaultCompoundReturnWithCondition(
+      const ast_matchers::MatchFinder::MatchResult &Result, bool Negated);
+
+  void replaceLabelCompoundReturnWithCondition(
+      const ast_matchers::MatchFinder::MatchResult &Result, bool Negated);
 
   void issueDiag(const ast_matchers::MatchFinder::MatchResult &Result,
                  SourceLocation Loc, StringRef Description,
                  SourceRange ReplacementRange, StringRef Replacement);
 
   const bool ChainedConditionalReturn;
   const bool ChainedConditionalAssignment;
 };
 
 } // namespace readability
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_SIMPLIFY_BOOLEAN_EXPR_H

clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp

 //===-- SimplifyBooleanExprCheck.cpp - clang-tidy -------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "SimplifyBooleanExprCheck.h"
+#include "SimplifyBooleanExprMatchers.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Lex/Lexer.h"
 
-#include <cassert>
 #include <string>
 #include <utility>
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace readability {
 
 namespace {
 
 StringRef getText(const MatchFinder::MatchResult &Result, SourceRange Range) {
   return Lexer::getSourceText(CharSourceRange::getTokenRange(Range),
                               *Result.SourceManager,
                               Result.Context->getLangOpts());
 }
 
 template <typename T>
 StringRef getText(const MatchFinder::MatchResult &Result, T &Node) {
   return getText(Result, Node.getSourceRange());
 }
 
-const char ConditionThenStmtId[] = "if-bool-yields-then";
-const char ConditionElseStmtId[] = "if-bool-yields-else";
-const char TernaryId[] = "ternary-bool-yields-condition";
-const char TernaryNegatedId[] = "ternary-bool-yields-not-condition";
-const char IfReturnsBoolId[] = "if-return";
-const char IfReturnsNotBoolId[] = "if-not-return";
-const char ThenLiteralId[] = "then-literal";
-const char IfAssignVariableId[] = "if-assign-lvalue";
-const char IfAssignLocId[] = "if-assign-loc";
-const char IfAssignBoolId[] = "if-assign";
-const char IfAssignNotBoolId[] = "if-assign-not";
-const char IfAssignVarId[] = "if-assign-var";
-const char CompoundReturnId[] = "compound-return";
-const char CompoundBoolId[] = "compound-bool";
-const char CompoundNotBoolId[] = "compound-bool-not";
+} // namespace
 
-const char IfStmtId[] = "if";
+static constexpr char ConditionThenStmtId[] = "if-bool-yields-then";
+static constexpr char ConditionElseStmtId[] = "if-bool-yields-else";
+static constexpr char TernaryId[] = "ternary-bool-yields-condition";
+static constexpr char TernaryNegatedId[] = "ternary-bool-yields-not-condition";
+static constexpr char IfReturnsBoolId[] = "if-return";
+static constexpr char IfReturnsNotBoolId[] = "if-not-return";
+static constexpr char ThenLiteralId[] = "then-literal";
+static constexpr char IfAssignVariableId[] = "if-assign-lvalue";
+static constexpr char IfAssignLocId[] = "if-assign-loc";
+static constexpr char IfAssignBoolId[] = "if-assign";
+static constexpr char IfAssignNotBoolId[] = "if-assign-not";
+static constexpr char IfAssignVarId[] = "if-assign-var";
+static constexpr char CompoundReturnId[] = "compound-return";
+static constexpr char CompoundIfId[] = "compound-if";
+static constexpr char CompoundBoolId[] = "compound-bool";
+static constexpr char CompoundNotBoolId[] = "compound-bool-not";
+static constexpr char CaseId[] = "case";
+static constexpr char CaseCompoundBoolId[] = "case-compound-bool";
+static constexpr char CaseCompoundNotBoolId[] = "case-compound-bool-not";
+static constexpr char DefaultId[] = "default";
+static constexpr char DefaultCompoundBoolId[] = "default-compound-bool";
+static constexpr char DefaultCompoundNotBoolId[] = "default-compound-bool-not";
+static constexpr char LabelId[] = "label";
+static constexpr char LabelCompoundBoolId[] = "label-compound-bool";
+static constexpr char LabelCompoundNotBoolId[] = "label-compound-bool-not";
+static constexpr char IfStmtId[] = "if";
 
-const char SimplifyOperatorDiagnostic[] =
+static constexpr char SimplifyOperatorDiagnostic[] =
     "redundant boolean literal supplied to boolean operator";
-const char SimplifyConditionDiagnostic[] =
+static constexpr char SimplifyConditionDiagnostic[] =
     "redundant boolean literal in if statement condition";
-const char SimplifyConditionalReturnDiagnostic[] =
+static constexpr char SimplifyConditionalReturnDiagnostic[] =
     "redundant boolean literal in conditional return statement";
 
-const Expr *getBoolLiteral(const MatchFinder::MatchResult &Result,
+static const Expr *getBoolLiteral(const MatchFinder::MatchResult &Result,
                                   StringRef Id) {
   if (const Expr *Literal = Result.Nodes.getNodeAs<CXXBoolLiteralExpr>(Id))
     return Literal->getBeginLoc().isMacroID() ? nullptr : Literal;
   if (const auto *Negated = Result.Nodes.getNodeAs<UnaryOperator>(Id)) {
     if (Negated->getOpcode() == UO_LNot &&
         isa<CXXBoolLiteralExpr>(Negated->getSubExpr()))
       return Negated->getBeginLoc().isMacroID() ? nullptr : Negated;
   }
   return nullptr;
 }
 
-internal::BindableMatcher<Stmt> literalOrNegatedBool(bool Value) {
+static internal::BindableMatcher<Stmt> literalOrNegatedBool(bool Value) {
   return expr(
       anyOf(cxxBoolLiteral(equals(Value)),
             unaryOperator(hasUnaryOperand(cxxBoolLiteral(equals(!Value))),
                           hasOperatorName("!"))));
 }
 
-internal::Matcher<Stmt> returnsBool(bool Value, StringRef Id = "ignored") {
+static internal::Matcher<Stmt> returnsBool(bool Value,
+                                           StringRef Id = "ignored") {
   auto SimpleReturnsBool = returnStmt(has(literalOrNegatedBool(Value).bind(Id)))
                                .bind("returns-bool");
   return anyOf(SimpleReturnsBool,
                compoundStmt(statementCountIs(1), has(SimpleReturnsBool)));
 }
 
-bool needsParensAfterUnaryNegation(const Expr *E) {
+static bool needsParensAfterUnaryNegation(const Expr *E) {
   E = E->IgnoreImpCasts();
   if (isa<BinaryOperator>(E) || isa<ConditionalOperator>(E))
     return true;
 
   if (const auto *Op = dyn_cast<CXXOperatorCallExpr>(E))
     return Op->getNumArgs() == 2 && Op->getOperator() != OO_Call &&
            Op->getOperator() != OO_Subscript;
 
   return false;
 }
 
-std::pair<BinaryOperatorKind, BinaryOperatorKind> Opposites[] = {
+static std::pair<BinaryOperatorKind, BinaryOperatorKind> Opposites[] = {
     {BO_LT, BO_GE}, {BO_GT, BO_LE}, {BO_EQ, BO_NE}};
 
-StringRef negatedOperator(const BinaryOperator *BinOp) {
+static StringRef negatedOperator(const BinaryOperator *BinOp) {
   const BinaryOperatorKind Opcode = BinOp->getOpcode();
   for (auto NegatableOp : Opposites) {
     if (Opcode == NegatableOp.first)
       return BinOp->getOpcodeStr(NegatableOp.second);
     if (Opcode == NegatableOp.second)
       return BinOp->getOpcodeStr(NegatableOp.first);
   }
-  return StringRef();
+  return {};
 }
 
-std::pair<OverloadedOperatorKind, StringRef> OperatorNames[] = {
+static std::pair<OverloadedOperatorKind, StringRef> OperatorNames[] = {
     {OO_EqualEqual, "=="},   {OO_ExclaimEqual, "!="}, {OO_Less, "<"},
     {OO_GreaterEqual, ">="}, {OO_Greater, ">"},       {OO_LessEqual, "<="}};
 
-StringRef getOperatorName(OverloadedOperatorKind OpKind) {
+static StringRef getOperatorName(OverloadedOperatorKind OpKind) {
   for (auto Name : OperatorNames) {
     if (Name.first == OpKind)
       return Name.second;
   }
 
-  return StringRef();
+  return {};
 }
 
-std::pair<OverloadedOperatorKind, OverloadedOperatorKind> OppositeOverloads[] =
-    {{OO_EqualEqual, OO_ExclaimEqual},
+static std::pair<OverloadedOperatorKind, OverloadedOperatorKind>
+    OppositeOverloads[] = {{OO_EqualEqual, OO_ExclaimEqual},
                            {OO_Less, OO_GreaterEqual},
                            {OO_Greater, OO_LessEqual}};
 
-StringRef negatedOperator(const CXXOperatorCallExpr *OpCall) {
+static StringRef negatedOperator(const CXXOperatorCallExpr *OpCall) {
   const OverloadedOperatorKind Opcode = OpCall->getOperator();
   for (auto NegatableOp : OppositeOverloads) {
     if (Opcode == NegatableOp.first)
       return getOperatorName(NegatableOp.second);
     if (Opcode == NegatableOp.second)
       return getOperatorName(NegatableOp.first);
   }
-  return StringRef();
+  return {};
 }
 
-std::string asBool(StringRef Text, bool NeedsStaticCast) {
+static std::string asBool(StringRef Text, bool NeedsStaticCast) {
   if (NeedsStaticCast)
     return ("static_cast<bool>(" + Text + ")").str();
 
   return std::string(Text);
 }
 
-bool needsNullPtrComparison(const Expr *E) {
+static bool needsNullPtrComparison(const Expr *E) {
   if (const auto *ImpCast = dyn_cast<ImplicitCastExpr>(E))
     return ImpCast->getCastKind() == CK_PointerToBoolean ||
            ImpCast->getCastKind() == CK_MemberPointerToBoolean;
 
   return false;
 }
 
-bool needsZeroComparison(const Expr *E) {
+static bool needsZeroComparison(const Expr *E) {
   if (const auto *ImpCast = dyn_cast<ImplicitCastExpr>(E))
     return ImpCast->getCastKind() == CK_IntegralToBoolean;
 
   return false;
 }
 
-bool needsStaticCast(const Expr *E) {
+static bool needsStaticCast(const Expr *E) {
   if (const auto *ImpCast = dyn_cast<ImplicitCastExpr>(E)) {
     if (ImpCast->getCastKind() == CK_UserDefinedConversion &&
         ImpCast->getSubExpr()->getType()->isBooleanType()) {
       if (const auto *MemCall =
               dyn_cast<CXXMemberCallExpr>(ImpCast->getSubExpr())) {
         if (const auto *MemDecl =
                 dyn_cast<CXXConversionDecl>(MemCall->getMethodDecl())) {
           if (MemDecl->isExplicit())
             return true;
         }
       }
     }
   }
 
   E = E->IgnoreImpCasts();
   return !E->getType()->isBooleanType();
 }
 
-std::string compareExpressionToConstant(const MatchFinder::MatchResult &Result,
-                                        const Expr *E, bool Negated,
-                                        const char *Constant) {
+static std::string
+compareExpressionToConstant(const MatchFinder::MatchResult &Result,
+                            const Expr *E, bool Negated, const char *Constant) {
   E = E->IgnoreImpCasts();
   const std::string ExprText =
       (isa<BinaryOperator>(E) ? ("(" + getText(Result, *E) + ")")
                               : getText(Result, *E))
           .str();
   return ExprText + " " + (Negated ? "!=" : "==") + " " + Constant;
 }
 
-std::string compareExpressionToNullPtr(const MatchFinder::MatchResult &Result,
-                                       const Expr *E, bool Negated) {
+static std::string
+compareExpressionToNullPtr(const MatchFinder::MatchResult &Result,
   const char *NullPtr =
       Result.Context->getLangOpts().CPlusPlus11 ? "nullptr" : "NULL";
   return compareExpressionToConstant(Result, E, Negated, NullPtr);
 }
 
-std::string compareExpressionToZero(const MatchFinder::MatchResult &Result,
-                                    const Expr *E, bool Negated) {
+static std::string
+compareExpressionToZero(const MatchFinder::MatchResult &Result, const Expr *E,
+                        bool Negated) {
   return compareExpressionToConstant(Result, E, Negated, "0");
 }
 
-std::string replacementExpression(const MatchFinder::MatchResult &Result,
+static std::string replacementExpression(const MatchFinder::MatchResult &Result,
                                          bool Negated, const Expr *E) {
   E = E->IgnoreParenBaseCasts();
   if (const auto *EC = dyn_cast<ExprWithCleanups>(E))
     E = EC->getSubExpr();
 
   const bool NeedsStaticCast = needsStaticCast(E);
   if (Negated) {
     if (const auto *UnOp = dyn_cast<UnaryOperator>(E)) {
       if (UnOp->getOpcode() == UO_LNot) {
if (needsNullPtrComparison(E))
     return compareExpressionToNullPtr(Result, E, true);
 
   if (needsZeroComparison(E))
     return compareExpressionToZero(Result, E, true);
 
   return asBool(getText(Result, *E), NeedsStaticCast);
 }
 
-const Expr *stmtReturnsBool(const ReturnStmt *Ret, bool Negated) {
+static const Expr *stmtReturnsBool(const ReturnStmt *Ret, bool Negated) {
   if (const auto *Bool = dyn_cast<CXXBoolLiteralExpr>(Ret->getRetValue())) {
     if (Bool->getValue() == !Negated)
       return Bool;
   }
   if (const auto *Unary = dyn_cast<UnaryOperator>(Ret->getRetValue())) {
     if (Unary->getOpcode() == UO_LNot) {
       if (const auto *Bool =
               dyn_cast<CXXBoolLiteralExpr>(Unary->getSubExpr())) {
         if (Bool->getValue() == Negated)
           return Bool;
       }
     }
   }
 
   return nullptr;
 }
 
-const Expr *stmtReturnsBool(const IfStmt *IfRet, bool Negated) {
+static const Expr *stmtReturnsBool(const IfStmt *IfRet, bool Negated) {
   if (IfRet->getElse() != nullptr)
     return nullptr;
 
   if (const auto *Ret = dyn_cast<ReturnStmt>(IfRet->getThen()))
     return stmtReturnsBool(Ret, Negated);
 
   if (const auto *Compound = dyn_cast<CompoundStmt>(IfRet->getThen())) {
     if (Compound->size() == 1) {
       if (const auto *CompoundRet = dyn_cast<ReturnStmt>(Compound->body_back()))
         return stmtReturnsBool(CompoundRet, Negated);
     }
   }
 
   return nullptr;
 }
 
-bool containsDiscardedTokens(const MatchFinder::MatchResult &Result,
+static bool containsDiscardedTokens(const MatchFinder::MatchResult &Result,
                                     CharSourceRange CharRange) {
   std::string ReplacementText =
       Lexer::getSourceText(CharRange, *Result.SourceManager,
                            Result.Context->getLangOpts())
           .str();
   Lexer Lex(CharRange.getBegin(), Result.Context->getLangOpts(),
             ReplacementText.data(), ReplacementText.data(),
             ReplacementText.data() + ReplacementText.size());
   Lex.SetCommentRetentionState(true);
 
   Token Tok;
   while (!Lex.LexFromRawLexer(Tok)) {
     if (Tok.is(tok::TokenKind::comment) || Tok.is(tok::TokenKind::hash))
       return true;
   }
 
   return false;
 }
 
-} // namespace
-
 class SimplifyBooleanExprCheck::Visitor : public RecursiveASTVisitor<Visitor> {
- public:
   Visitor(SimplifyBooleanExprCheck *Check,
           const MatchFinder::MatchResult &Result)
       : Check(Check), Result(Result) {}
 
-  bool VisitBinaryOperator(BinaryOperator *Op) {
+  bool VisitBinaryOperator(const BinaryOperator *Op) const {
     Check->reportBinOp(Result, Op);
     return true;
   }
 
- private:
   SimplifyBooleanExprCheck *Check;
   const MatchFinder::MatchResult &Result;
 };
 
 SimplifyBooleanExprCheck::SimplifyBooleanExprCheck(StringRef Name,
                                                    ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       ChainedConditionalReturn(Options.get("ChainedConditionalReturn", false)),
       ChainedConditionalAssignment(
           Options.get("ChainedConditionalAssignment", false)) {}
 
-bool containsBoolLiteral(const Expr *E) {
+static bool containsBoolLiteral(const Expr *E) {
   if (!E)
     return false;
   E = E->IgnoreParenImpCasts();
   if (isa<CXXBoolLiteralExpr>(E))
     return true;
   if (const auto *BinOp = dyn_cast<BinaryOperator>(E))
     return containsBoolLiteral(BinOp->getLHS()) ||
            containsBoolLiteral(BinOp->getRHS());
   if (const auto *UnaryOp = dyn_cast<UnaryOperator>(E))
     return containsBoolLiteral(UnaryOp->getSubExpr());
   return false;
 }
 
 void SimplifyBooleanExprCheck::reportBinOp(
     const MatchFinder::MatchResult &Result, const BinaryOperator *Op) {
   const auto *LHS = Op->getLHS()->IgnoreParenImpCasts();
   const auto *RHS = Op->getRHS()->IgnoreParenImpCasts();
 
   const CXXBoolLiteralExpr *Bool;
-  const Expr *Other = nullptr;
-  if ((Bool = dyn_cast<CXXBoolLiteralExpr>(LHS)))
+  const Expr *Other;
+  if ((Bool = dyn_cast<CXXBoolLiteralExpr>(LHS)) != nullptr)
     Other = RHS;
-  else if ((Bool = dyn_cast<CXXBoolLiteralExpr>(RHS)))
+  else if ((Bool = dyn_cast<CXXBoolLiteralExpr>(RHS)) != nullptr)
     Other = LHS;
   else
     return;
 
   if (Bool->getBeginLoc().isMacroID())
     return;
 
   // FIXME: why do we need this?
   if (!isa<CXXBoolLiteralExpr>(Other) && containsBoolLiteral(Other))
     return;
 
   bool BoolValue = Bool->getValue();
 
   auto ReplaceWithExpression = [this, &Result, LHS, RHS,
                                 Bool](const Expr *ReplaceWith, bool Negated) {
     std::string Replacement =
         replacementExpression(Result, Negated, ReplaceWith);
     SourceRange Range(LHS->getBeginLoc(), RHS->getEndLoc());
     issueDiag(Result, Bool->getBeginLoc(), SimplifyOperatorDiagnostic, Range,
               Replacement);
   };
 
   switch (Op->getOpcode()) {
-    case BO_LAnd:
-      if (BoolValue) {
-        // expr && true -> expr
-        ReplaceWithExpression(Other, /*Negated=*/false);
-      } else {
-        // expr && false -> false
-        ReplaceWithExpression(Bool, /*Negated=*/false);
-      }
-      break;
-    case BO_LOr:
-      if (BoolValue) {
-        // expr || true -> true
-        ReplaceWithExpression(Bool, /*Negated=*/false);
-      } else {
-        // expr || false -> expr
-        ReplaceWithExpression(Other, /*Negated=*/false);
-      }
-      break;
-    case BO_EQ:
-      // expr == true -> expr, expr == false -> !expr
-      ReplaceWithExpression(Other, /*Negated=*/!BoolValue);
-      break;
-    case BO_NE:
-      // expr != true -> !expr, expr != false -> expr
-      ReplaceWithExpression(Other, /*Negated=*/BoolValue);
-      break;
-    default:
-      break;
+    if (BoolValue)
+    else
+    if (BoolValue)
+    else
   }
 }
 
 void SimplifyBooleanExprCheck::matchBoolCondition(MatchFinder *Finder,
                                                   bool Value,
                                                   StringRef BooleanId) {
   Finder->addMatcher(
       ifStmt(hasCondition(literalOrNegatedBool(Value).bind(BooleanId)))
           .bind(IfStmtId),
       this);
 }
 
 void SimplifyBooleanExprCheck::matchTernaryResult(MatchFinder *Finder,
-                                                  bool Value,
-                                                  StringRef TernaryId) {
+                                                  bool Value, StringRef Id) {
   Finder->addMatcher(
       conditionalOperator(hasTrueExpression(literalOrNegatedBool(Value)),
                           hasFalseExpression(literalOrNegatedBool(!Value)))
-          .bind(TernaryId),
+          .bind(Id),
       this);
 }
 
 void SimplifyBooleanExprCheck::matchIfReturnsBool(MatchFinder *Finder,
                                                   bool Value, StringRef Id) {
   if (ChainedConditionalReturn)
     Finder->addMatcher(ifStmt(hasThen(returnsBool(Value, ThenLiteralId)),
                               hasElse(returnsBool(!Value)))
if (ChainedConditionalAssignment)
     Finder->addMatcher(ifStmt(hasThen(Then), hasElse(Else)).bind(Id), this);
   else
     Finder->addMatcher(
         ifStmt(unless(hasParent(ifStmt())), hasThen(Then), hasElse(Else))
             .bind(Id),
         this);
 }
 
+static internal::Matcher<Stmt> ifReturnValue(bool Value) {
+  return ifStmt(hasThen(returnsBool(Value)), unless(hasElse(stmt())))
+      .bind(CompoundIfId);
+}
+
+static internal::Matcher<Stmt> returnNotValue(bool Value) {
+  return returnStmt(has(literalOrNegatedBool(!Value))).bind(CompoundReturnId);
+}
+
 void SimplifyBooleanExprCheck::matchCompoundIfReturnsBool(MatchFinder *Finder,
                                                           bool Value,
                                                           StringRef Id) {
+  if (ChainedConditionalReturn)
+    Finder->addMatcher(
+        compoundStmt(hasSubstatementSequence(ifReturnValue(Value),
+                                             returnNotValue(Value)))
+            .bind(Id),
+        this);
+  else
     Finder->addMatcher(
-      compoundStmt(
-          hasAnySubstatement(
-              ifStmt(hasThen(returnsBool(Value)), unless(hasElse(stmt())))),
-          hasAnySubstatement(returnStmt(has(literalOrNegatedBool(!Value)))
-                                 .bind(CompoundReturnId)))
+        compoundStmt(hasSubstatementSequence(ifStmt(hasThen(returnsBool(Value)),
+                                                    unless(hasElse(stmt())),
+                                                    unless(hasParent(ifStmt())))
+                                                 .bind(CompoundIfId),
+                                             returnNotValue(Value)))
             .bind(Id),
         this);
 }
 
+void SimplifyBooleanExprCheck::matchCaseIfReturnsBool(MatchFinder *Finder,
+                                                      bool Value,
+                                                      StringRef Id) {
+  internal::Matcher<Stmt> CaseStmt =
+      caseStmt(hasSubstatement(ifReturnValue(Value))).bind(CaseId);
+  internal::Matcher<Stmt> CompoundStmt =
+      compoundStmt(hasSubstatementSequence(CaseStmt, returnNotValue(Value)))
+          .bind(Id);
+  Finder->addMatcher(switchStmt(has(CompoundStmt)), this);
+}
+
+void SimplifyBooleanExprCheck::matchDefaultIfReturnsBool(MatchFinder *Finder,
+                                                         bool Value,
+                                                         StringRef Id) {
+  internal::Matcher<Stmt> DefaultStmt =
+      defaultStmt(hasSubstatement(ifReturnValue(Value))).bind(DefaultId);
+  internal::Matcher<Stmt> CompoundStmt =
+      compoundStmt(hasSubstatementSequence(DefaultStmt, returnNotValue(Value)))
+          .bind(Id);
+  Finder->addMatcher(switchStmt(has(CompoundStmt)), this);
+}
+
+void SimplifyBooleanExprCheck::matchLabelIfReturnsBool(MatchFinder *Finder,
+                                                       bool Value,
+                                                       StringRef Id) {
+  internal::Matcher<Stmt> LabelStmt =
+      labelStmt(hasSubstatement(ifReturnValue(Value))).bind(LabelId);
+  internal::Matcher<Stmt> CompoundStmt =
+      compoundStmt(hasSubstatementSequence(LabelStmt, returnNotValue(Value)))
+          .bind(Id);
+  Finder->addMatcher(CompoundStmt, this);
+}
+
 void SimplifyBooleanExprCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "ChainedConditionalReturn", ChainedConditionalReturn);
   Options.store(Opts, "ChainedConditionalAssignment",
                 ChainedConditionalAssignment);
 }
 
 void SimplifyBooleanExprCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(translationUnitDecl().bind("top"), this);
 
   matchBoolCondition(Finder, true, ConditionThenStmtId);
   matchBoolCondition(Finder, false, ConditionElseStmtId);
 
   matchTernaryResult(Finder, true, TernaryId);
   matchTernaryResult(Finder, false, TernaryNegatedId);
 
   matchIfReturnsBool(Finder, true, IfReturnsBoolId);
   matchIfReturnsBool(Finder, false, IfReturnsNotBoolId);
 
   matchIfAssignsBool(Finder, true, IfAssignBoolId);
   matchIfAssignsBool(Finder, false, IfAssignNotBoolId);
 
   matchCompoundIfReturnsBool(Finder, true, CompoundBoolId);
   matchCompoundIfReturnsBool(Finder, false, CompoundNotBoolId);
+
+  matchCaseIfReturnsBool(Finder, true, CaseCompoundBoolId);
+  matchCaseIfReturnsBool(Finder, false, CaseCompoundNotBoolId);
+
+  matchDefaultIfReturnsBool(Finder, true, DefaultCompoundBoolId);
+  matchDefaultIfReturnsBool(Finder, false, DefaultCompoundNotBoolId);
+
+  matchLabelIfReturnsBool(Finder, true, LabelCompoundBoolId);
+  matchLabelIfReturnsBool(Finder, false, LabelCompoundNotBoolId);
 }
 
 void SimplifyBooleanExprCheck::check(const MatchFinder::MatchResult &Result) {
   if (Result.Nodes.getNodeAs<TranslationUnitDecl>("top"))
     Visitor(this, Result).TraverseAST(*Result.Context);
   else if (const Expr *TrueConditionRemoved =
                getBoolLiteral(Result, ConditionThenStmtId))
     replaceWithThenStatement(Result, TrueConditionRemoved);
   else if (const Expr *FalseConditionRemoved =
                getBoolLiteral(Result, ConditionElseStmtId))
     replaceWithElseStatement(Result, FalseConditionRemoved);
   else if (const auto *Ternary =
                Result.Nodes.getNodeAs<ConditionalOperator>(TernaryId))
-    replaceWithCondition(Result, Ternary);
+    replaceWithCondition(Result, Ternary, false);
   else if (const auto *TernaryNegated =
                Result.Nodes.getNodeAs<ConditionalOperator>(TernaryNegatedId))
     replaceWithCondition(Result, TernaryNegated, true);
   else if (const auto *If = Result.Nodes.getNodeAs<IfStmt>(IfReturnsBoolId))
-    replaceWithReturnCondition(Result, If);
+    replaceWithReturnCondition(Result, If, false);
   else if (const auto *IfNot =
                Result.Nodes.getNodeAs<IfStmt>(IfReturnsNotBoolId))
     replaceWithReturnCondition(Result, IfNot, true);
   else if (const auto *IfAssign =
                Result.Nodes.getNodeAs<IfStmt>(IfAssignBoolId))
-    replaceWithAssignment(Result, IfAssign);
+    replaceWithAssignment(Result, IfAssign, false);
   else if (const auto *IfAssignNot =
                Result.Nodes.getNodeAs<IfStmt>(IfAssignNotBoolId))
     replaceWithAssignment(Result, IfAssignNot, true);
   else if (const auto *Compound =
                Result.Nodes.getNodeAs<CompoundStmt>(CompoundBoolId))
-    replaceCompoundReturnWithCondition(Result, Compound);
-  else if (const auto *Compound =
+    replaceCompoundReturnWithCondition(Result, Compound, false);
+  else if (const auto *CompoundNot =
                Result.Nodes.getNodeAs<CompoundStmt>(CompoundNotBoolId))
-    replaceCompoundReturnWithCondition(Result, Compound, true);
-}
-
-void SimplifyBooleanExprCheck::issueDiag(
-    const ast_matchers::MatchFinder::MatchResult &Result, SourceLocation Loc,
-    StringRef Description, SourceRange ReplacementRange,
+    replaceCompoundReturnWithCondition(Result, CompoundNot, true);
+  else if (Result.Nodes.getNodeAs<CompoundStmt>(CaseCompoundBoolId))
+    replaceCaseCompoundReturnWithCondition(Result, false);
+  else if (Result.Nodes.getNodeAs<CompoundStmt>(CaseCompoundNotBoolId))
+    replaceCaseCompoundReturnWithCondition(Result, true);
+  else if (Result.Nodes.getNodeAs<CompoundStmt>(DefaultCompoundBoolId))
+    replaceDefaultCompoundReturnWithCondition(Result, false);
+  else if (Result.Nodes.getNodeAs<CompoundStmt>(DefaultCompoundNotBoolId))
+    replaceDefaultCompoundReturnWithCondition(Result, true);
+  else if (Result.Nodes.getNodeAs<CompoundStmt>(LabelCompoundBoolId))
+    replaceLabelCompoundReturnWithCondition(Result, false);
+  else if (Result.Nodes.getNodeAs<CompoundStmt>(LabelCompoundNotBoolId))
+    replaceLabelCompoundReturnWithCondition(Result, true);
+  else if (const auto TU = Result.Nodes.getNodeAs<Decl>("top"))
+    Visitor(this, Result).TraverseDecl(const_cast<Decl *>(TU));
+}
+
+void SimplifyBooleanExprCheck::issueDiag(const MatchFinder::MatchResult &Result,
+                                         SourceLocation Loc,
+                                         StringRef Description,
+                                         SourceRange ReplacementRange,
                                          StringRef Replacement) {
   CharSourceRange CharRange =
       Lexer::makeFileCharRange(CharSourceRange::getTokenRange(ReplacementRange),
                                *Result.SourceManager, getLangOpts());
 
   DiagnosticBuilder Diag = diag(Loc, Description);
   if (!containsDiscardedTokens(Result, CharRange))
     Diag << FixItHint::CreateReplacement(CharRange, Replacement);
 }
 
 void SimplifyBooleanExprCheck::replaceWithThenStatement(
-    const MatchFinder::MatchResult &Result, const Expr *TrueConditionRemoved) {
+    const MatchFinder::MatchResult &Result, const Expr *BoolLiteral) {
   const auto *IfStatement = Result.Nodes.getNodeAs<IfStmt>(IfStmtId);
-  issueDiag(Result, TrueConditionRemoved->getBeginLoc(),
-            SimplifyConditionDiagnostic, IfStatement->getSourceRange(),
+  issueDiag(Result, BoolLiteral->getBeginLoc(), SimplifyConditionDiagnostic,
+            IfStatement->getSourceRange(),
             getText(Result, *IfStatement->getThen()));
 }
 
 void SimplifyBooleanExprCheck::replaceWithElseStatement(
-    const MatchFinder::MatchResult &Result, const Expr *FalseConditionRemoved) {
+    const MatchFinder::MatchResult &Result, const Expr *BoolLiteral) {
   const auto *IfStatement = Result.Nodes.getNodeAs<IfStmt>(IfStmtId);
   const Stmt *ElseStatement = IfStatement->getElse();
-  issueDiag(Result, FalseConditionRemoved->getBeginLoc(),
-            SimplifyConditionDiagnostic, IfStatement->getSourceRange(),
+  issueDiag(Result, BoolLiteral->getBeginLoc(), SimplifyConditionDiagnostic,
+            IfStatement->getSourceRange(),
             ElseStatement ? getText(Result, *ElseStatement) : "");
 }
 
 void SimplifyBooleanExprCheck::replaceWithCondition(
     const MatchFinder::MatchResult &Result, const ConditionalOperator *Ternary,
     bool Negated) {
   std::string Replacement =
       replacementExpression(Result, Negated, Ternary->getCond());
issueDiag(Result, Start, SimplifyConditionalReturnDiagnostic,
             If->getSourceRange(), Replacement);
 }
 
 void SimplifyBooleanExprCheck::replaceCompoundReturnWithCondition(
     const MatchFinder::MatchResult &Result, const CompoundStmt *Compound,
     bool Negated) {
   const auto *Ret = Result.Nodes.getNodeAs<ReturnStmt>(CompoundReturnId);
 
-  // The body shouldn't be empty because the matcher ensures that it must
-  // contain at least two statements:
-  // 1) A `return` statement returning a boolean literal `false` or `true`
-  // 2) An `if` statement with no `else` clause that consists of a single
-  //    `return` statement returning the opposite boolean literal `true` or
-  //    `false`.
-  assert(Compound->size() >= 2);
+  // Scan through the CompoundStmt to look for a chained-if construct.
   const IfStmt *BeforeIf = nullptr;
   CompoundStmt::const_body_iterator Current = Compound->body_begin();
   CompoundStmt::const_body_iterator After = Compound->body_begin();
   for (++After; After != Compound->body_end() && *Current != Ret;
        ++Current, ++After) {
     if (const auto *If = dyn_cast<IfStmt>(*Current)) {
       if (const Expr *Lit = stmtReturnsBool(If, Negated)) {
         if (*After == Ret) {
           if (!ChainedConditionalReturn && BeforeIf)
             continue;
 
-          const Expr *Condition = If->getCond();
           std::string Replacement =
-              "return " + replacementExpression(Result, Negated, Condition);
+              "return " + replacementExpression(Result, Negated, If->getCond());
           issueDiag(
               Result, Lit->getBeginLoc(), SimplifyConditionalReturnDiagnostic,
               SourceRange(If->getBeginLoc(), Ret->getEndLoc()), Replacement);
           return;
         }
 
         BeforeIf = If;
       }
     } else {
       BeforeIf = nullptr;
     }
   }
 }
 
+void SimplifyBooleanExprCheck::replaceCompoundReturnWithCondition(
+    const MatchFinder::MatchResult &Result, bool Negated, const IfStmt *If) {
+  const auto *Lit = stmtReturnsBool(If, Negated);
+  const auto *Ret = Result.Nodes.getNodeAs<ReturnStmt>(CompoundReturnId);
+  const std::string Replacement =
+      "return " + replacementExpression(Result, Negated, If->getCond());
+  issueDiag(Result, Lit->getBeginLoc(), SimplifyConditionalReturnDiagnostic,
+            SourceRange(If->getBeginLoc(), Ret->getEndLoc()), Replacement);
+}
+
+void SimplifyBooleanExprCheck::replaceCaseCompoundReturnWithCondition(
+    const MatchFinder::MatchResult &Result, bool Negated) {
+  const auto *CaseDefault = Result.Nodes.getNodeAs<CaseStmt>(CaseId);
+  const auto *If = dyn_cast<IfStmt>(CaseDefault->getSubStmt());
+  replaceCompoundReturnWithCondition(Result, Negated, If);
+}
+
+void SimplifyBooleanExprCheck::replaceDefaultCompoundReturnWithCondition(
+    const MatchFinder::MatchResult &Result, bool Negated) {
+  const SwitchCase *CaseDefault =
+      Result.Nodes.getNodeAs<DefaultStmt>(DefaultId);
+  const auto *If = dyn_cast<IfStmt>(CaseDefault->getSubStmt());
+  replaceCompoundReturnWithCondition(Result, Negated, If);
+}
+
+void SimplifyBooleanExprCheck::replaceLabelCompoundReturnWithCondition(
+    const MatchFinder::MatchResult &Result, bool Negated) {
+  const auto *Label = Result.Nodes.getNodeAs<LabelStmt>(LabelId);
+  const auto *If = dyn_cast<IfStmt>(Label->getSubStmt());
+  replaceCompoundReturnWithCondition(Result, Negated, If);
+}
+
 void SimplifyBooleanExprCheck::replaceWithAssignment(
     const MatchFinder::MatchResult &Result, const IfStmt *IfAssign,
     bool Negated) {
   SourceRange Range = IfAssign->getSourceRange();
   StringRef VariableName =
       getText(Result, *Result.Nodes.getNodeAs<Expr>(IfAssignVariableId));
   StringRef Terminator = isa<CompoundStmt>(IfAssign->getElse()) ? ";" : "";
   std::string Condition =

clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprMatchers.h

+//===-- SimplifyBooleanExprMatchers.h - clang-tidy ------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/ASTMatchers/ASTMatchersMacros.h"
+
+namespace clang {
+namespace ast_matchers {
+
+/// Matches the substatement associated with a case, default or label statement.
+///
+/// Given
+/// \code
+///   switch (1) { case 1: break; case 2: return; break; default: return; break;
+///   }
+///   foo: return;
+///   bar: break;
+/// \endcode
+///
+/// caseStmt(hasSubstatement(returnStmt()))
+///   matches "case 2: return;"
+/// defaultStmt(hasSubstatement(returnStmt()))
+///   matches "default: return;"
+/// labelStmt(hasSubstatement(breakStmt()))
+///   matches "bar: break;"
+AST_POLYMORPHIC_MATCHER_P(hasSubstatement,
+                          AST_POLYMORPHIC_SUPPORTED_TYPES(CaseStmt, DefaultStmt,
+                                                          LabelStmt),
+                          internal::Matcher<Stmt>, InnerMatcher) {
+  return InnerMatcher.matches(*Node.getSubStmt(), Finder, Builder);
+}
+
+/// Matches two consecutive statements within a compound statement.
+///
+/// Given
+/// \code
+///   { if (x > 0) return true; return false; }
+/// \endcode
+/// compoundStmt(hasSubstatementSequence(ifStmt(), returnStmt()))
+///   matches '{ if (x > 0) return true; return false; }'
+AST_POLYMORPHIC_MATCHER_P2(hasSubstatementSequence,
+                           AST_POLYMORPHIC_SUPPORTED_TYPES(CompoundStmt,
+                                                           StmtExpr),
+                           internal::Matcher<Stmt>, InnerMatcher1,
+                           internal::Matcher<Stmt>, InnerMatcher2) {
+  if (const CompoundStmt *CS = CompoundStmtMatcher<NodeType>::get(Node)) {
+    auto It = matchesFirstInPointerRange(InnerMatcher1, CS->body_begin(),
+                                         CS->body_end(), Finder, Builder);
+    while (It != CS->body_end()) {
+      ++It;
+      if (It == CS->body_end())
+        return false;
+      if (InnerMatcher2.matches(**It, Finder, Builder))
+        return true;
+      It = matchesFirstInPointerRange(InnerMatcher1, It, CS->body_end(), Finder,
+                                      Builder);
+    }
+  }
+  return false;
+}
+
+} // namespace ast_matchers
+} // namespace clang

clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-case.cpp

+// RUN: %check_clang_tidy %s readability-simplify-boolean-expr %t
+
+bool switch_stmt(int i, int j, bool b) {
+  switch (i) {
+  case 0:
+    if (b == true)
+      j = 10;
+    break;
+    // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{if \(b\)}}
+    // CHECK-FIXES-NEXT: {{j = 10;}}
+
+  case 1:
+    if (b == false)
+      j = -20;
+    break;
+    // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{if \(!b\)}}
+    // CHECK-FIXES-NEXT: {{j = -20;}}
+
+  case 2:
+    if (b && true)
+      j = 10;
+    break;
+    // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{if \(b\)}}
+    // CHECK-FIXES-NEXT: {{j = 10;}}
+
+  case 3:
+    if (b && false)
+      j = -20;
+    break;
+    // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{if \(false\)}}
+    // CHECK-FIXES-NEXT: {{j = -20;}}
+
+  case 4:
+    if (b || true)
+      j = 10;
+    break;
+    // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{if \(true\)}}
+    // CHECK-FIXES-NEXT: {{j = 10;}}
+    // CHECK-FIXES-NEXT: {{break;}}
+
+  case 5:
+    if (b || false)
+      j = -20;
+    break;
+    // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{if \(b\)}}
+    // CHECK-FIXES-NEXT: {{j = -20;}}
+
+  case 6:
+    return i > 0 ? true : false;
+    // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: {{.*}} in ternary expression result
+    // CHECK-FIXES: {{return i > 0;}}
+
+  case 7:
+    return i > 0 ? false : true;
+    // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: {{.*}} in ternary expression result
+    // CHECK-FIXES: {{return i <= 0;}}
+
+  case 8:
+    if (true)
+      j = 10;
+    else
+      j = -20;
+    break;
+    // CHECK-MESSAGES: :[[@LINE-5]]:9: warning: {{.*}} in if statement condition
+    // CHECK-FIXES:      {{j = 10;$}}
+    // CHECK-FIXES-NEXT: {{break;$}}
+
+  case 9:
+    if (false)
+      j = -20;
+    else
+      j = 10;
+    break;
+    // CHECK-MESSAGES: :[[@LINE-5]]:9: warning: {{.*}} in if statement condition
+    // CHECK-FIXES: {{j = 10;}}
+    // CHECK-FIXES-NEXT: {{break;}}
+
+  case 10:
+    if (j > 10)
+      return true;
+    else
+      return false;
+    // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} in conditional return statement
+    // CHECK-FIXES: {{return j > 10;}}
+
+  case 11:
+    if (j > 10)
+      return false;
+    else
+      return true;
+    // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} in conditional return statement
+    // CHECK-FIXES: {{return j <= 10;}}
+
+  case 12:
+    if (j > 10)
+      b = true;
+    else
+      b = false;
+    return b;
+    // CHECK-MESSAGES: :[[@LINE-4]]:11: warning: {{.*}} in conditional assignment
+    // CHECK-FIXES: {{b = j > 10;}}
+    // CHECK-FIXES-NEXT: {{return b;}}
+
+  case 13:
+    if (j > 10)
+      b = false;
+    else
+      b = true;
+    return b;
+    // CHECK-MESSAGES: :[[@LINE-4]]:11: warning: {{.*}} in conditional assignment
+    // CHECK-FIXES: {{b = j <= 10;}}
+    // CHECK-FIXES-NEXT: {{return b;}}
+
+  case 14:
+    if (j > 10)
+      return true;
+    return false;
+    // CHECK-MESSAGES: :[[@LINE-2]]:14: warning: {{.*}} in conditional return
+    // FIXES: {{return j > 10;}}
+
+  case 15:
+    if (j > 10)
+      return false;
+    return true;
+    // CHECK-MESSAGES: :[[@LINE-2]]:14: warning: {{.*}} in conditional return
+    // FIXES: {{return j <= 10;}}
+
+  case 16:
+    if (j > 10)
+      return true;
+    return true;
+    return false;
+
+  case 17:
+    if (j > 10)
+      return false;
+    return false;
+    return true;
+
+  case 100: {
+    if (b == true)
+      j = 10;
+    break;
+    // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{if \(b\)}}
+    // CHECK-FIXES-NEXT: {{j = 10;}}
+  }
+
+  case 101: {
+    if (b == false)
+      j = -20;
+    break;
+    // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{if \(!b\)}}
+    // CHECK-FIXES-NEXT: {{j = -20;}}
+  }
+
+  case 102: {
+    if (b && true)
+      j = 10;
+    break;
+    // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{if \(b\)}}
+    // CHECK-FIXES-NEXT: {{j = 10;}}
+  }
+
+  case 103: {
+    if (b && false)
+      j = -20;
+    break;
+    // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{if \(false\)}}
+    // CHECK-FIXES-NEXT: {{j = -20;}}
+  }
+
+  case 104: {
+    if (b || true)
+      j = 10;
+    break;
+    // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{if \(true\)}}
+    // CHECK-FIXES-NEXT: {{j = 10;}}
+    // CHECK-FIXES-NEXT: {{break;}}
+  }
+
+  case 105: {
+    if (b || false)
+      j = -20;
+    break;
+    // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{if \(b\)}}
+    // CHECK-FIXES-NEXT: {{j = -20;}}
+  }
+
+  case 106: {
+    return i > 0 ? true : false;
+    // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: {{.*}} in ternary expression result
+    // CHECK-FIXES: {{return i > 0;}}
+  }
+
+  case 107: {
+    return i > 0 ? false : true;
+    // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: {{.*}} in ternary expression result
+    // CHECK-FIXES: {{return i <= 0;}}
+  }
+
+  case 108: {
+    if (true)
+      j = 10;
+    else
+      j = -20;
+    break;
+    // CHECK-MESSAGES: :[[@LINE-5]]:9: warning: {{.*}} in if statement condition
+    // CHECK-FIXES:      {{j = 10;$}}
+    // CHECK-FIXES-NEXT: {{break;$}}
+  }
+
+  case 109: {
+    if (false)
+      j = -20;
+    else
+      j = 10;
+    break;
+    // CHECK-MESSAGES: :[[@LINE-5]]:9: warning: {{.*}} in if statement condition
+    // CHECK-FIXES: {{j = 10;}}
+    // CHECK-FIXES-NEXT: {{break;}}
+  }
+
+  case 110: {
+    if (j > 10)
+      return true;
+    else
+      return false;
+    // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} in conditional return statement
+    // CHECK-FIXES: {{return j > 10;}}
+  }
+
+  case 111: {
+    if (j > 10)
+      return false;
+    else
+      return true;
+    // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} in conditional return statement
+    // CHECK-FIXES: {{return j <= 10;}}
+  }
+
+  case 112: {
+    if (j > 10)
+      b = true;
+    else
+      b = false;
+    return b;
+    // CHECK-MESSAGES: :[[@LINE-4]]:11: warning: {{.*}} in conditional assignment
+    // CHECK-FIXES: {{b = j > 10;}}
+    // CHECK-FIXES-NEXT: {{return b;}}
+  }
+
+  case 113: {
+    if (j > 10)
+      b = false;
+    else
+      b = true;
+    return b;
+    // CHECK-MESSAGES: :[[@LINE-4]]:11: warning: {{.*}} in conditional assignment
+    // CHECK-FIXES: {{b = j <= 10;}}
+    // CHECK-FIXES-NEXT: {{return b;}}
+  }
+
+  case 114: {
+    if (j > 10)
+      return true;
+    return false;
+    // CHECK-MESSAGES: :[[@LINE-2]]:14: warning: {{.*}} in conditional return
+    // CHECK-FIXES: {{return j > 10;}}
+  }
+
+  case 115: {
+    if (j > 10)
+      return false;
+    return true;
+    // CHECK-MESSAGES: :[[@LINE-2]]:14: warning: {{.*}} in conditional return
+    // CHECK-FIXES: {{return j <= 10;}}
+  }
+
+  case 116: {
+    return false;
+    if (j > 10)
+      return true;
+  }
+
+  case 117: {
+    return true;
+    if (j > 10)
+      return false;
+  }
+  }
+
+  return j > 0;
+}
+
+bool default_stmt0(int i, int j, bool b) {
+  switch (i) {
+  case 0:
+    return true;
+
+  default:
+    if (b == true)
+      j = 10;
+    break;
+    // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{if \(b\)}}
+    // CHECK-FIXES-NEXT: {{j = 10;}}
+  }
+  return false;
+}
+
+bool default_stmt1(int i, int j, bool b) {
+  switch (i) {
+  case 0:
+    return true;
+
+  default:
+    if (b == false)
+      j = -20;
+    break;
+    // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{if \(!b\)}}
+    // CHECK-FIXES-NEXT: {{j = -20;}}
+  }
+  return false;
+}
+
+bool default_stmt2(int i, int j, bool b) {
+  switch (i) {
+  case 0:
+    return true;
+
+  default:
+    if (b && true)
+      j = 10;
+    break;
+    // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{if \(b\)}}
+    // CHECK-FIXES-NEXT: {{j = 10;}}
+  }
+  return false;
+}
+
+bool default_stmt3(int i, int j, bool b) {
+  switch (i) {
+  case 0:
+    return true;
+
+  default:
+    if (b && false)
+      j = -20;
+    break;
+    // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{if \(false\)}}
+    // CHECK-FIXES-NEXT: {{j = -20;}}
+  }
+  return false;
+}
+
+bool default_stmt4(int i, int j, bool b) {
+  switch (i) {
+  case 0:
+    return true;
+
+  default:
+    if (b || true)
+      j = 10;
+    break;
+    // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{if \(true\)}}
+    // CHECK-FIXES-NEXT: {{j = 10;}}
+    // CHECK-FIXES-NEXT: {{break;}}
+  }
+  return false;
+}
+
+bool default_stmt5(int i, int j, bool b) {
+  switch (i) {
+  case 0:
+    return true;
+
+  default:
+    if (b || false)
+      j = -20;
+    break;
+    // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} to boolean operator
+    // CHECK-FIXES: {{if \(b\)}}
+    // CHECK-FIXES-NEXT: {{j = -20;}}
+  }
+  return false;
+}
+
+bool default_stmt6(int i, int j, bool b) {
+  switch (i) {
+  case 0:
+    return true;
+
+  default:
+    return i > 0 ? true : false;
+    // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: {{.*}} in ternary expression result
+    // CHECK-FIXES: {{return i > 0;}}
+  }
+  return false;
+}
+
+bool default_stmt7(int i, int j, bool b) {
+  switch (i) {
+  case 0:
+    return true;
+
+  default:
+    return i > 0 ? false : true;
+    // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: {{.*}} in ternary expression result
+    // CHECK-FIXES: {{return i <= 0;}}
+  }
+  return false;
+}
+
+bool default_stmt8(int i, int j, bool b) {
+  switch (i) {
+  case 0:
+    return true;
+
+  default:
+    if (true)
+      j = 10;
+    else
+      j = -20;
+    break;
+    // CHECK-MESSAGES: :[[@LINE-5]]:9: warning: {{.*}} in if statement condition
+    // CHECK-FIXES:      {{j = 10;$}}
+    // CHECK-FIXES-NEXT: {{break;$}}
+  }
+  return false;
+}
+
+bool default_stmt9(int i, int j, bool b) {
+  switch (i) {
+  case 0:
+    return true;
+
+  default:
+    if (false)
+      j = -20;
+    else
+      j = 10;
+    break;
+    // CHECK-MESSAGES: :[[@LINE-5]]:9: warning: {{.*}} in if statement condition
+    // CHECK-FIXES: {{j = 10;}}
+    // CHECK-FIXES-NEXT: {{break;}}
+  }
+  return false;
+}
+
+bool default_stmt10(int i, int j, bool b) {
+  switch (i) {
+  case 0:
+    return true;
+
+  default:
+    if (j > 10)
+      return true;
+    else
+      return false;
+    // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} in conditional return statement
+    // CHECK-FIXES: {{return j > 10;}}
+  }
+  return false;
+}
+
+bool default_stmt11(int i, int j, bool b) {
+  switch (i) {
+  case 0:
+    return true;
+
+  default:
+    if (j > 10)
+      return false;
+    else
+      return true;
+    // CHECK-MESSAGES: :[[@LINE-3]]:14: warning: {{.*}} in conditional return statement
+    // CHECK-FIXES: {{return j <= 10;}}
+  }
+  return false;
+}
+
+bool default_stmt12(int i, int j, bool b) {
+  switch (i) {
+  case 0:
+    return true;
+
+  default:
+    if (j > 10)
+      b = true;
+    else
+      b = false;
+    return b;
+    // CHECK-MESSAGES: :[[@LINE-4]]:11: warning: {{.*}} in conditional assignment
+    // CHECK-FIXES: {{b = j > 10;}}
+    // CHECK-FIXES-NEXT: {{return b;}}
+  }
+  return false;
+}
+
+bool default_stmt13(int i, int j, bool b) {
+  switch (i) {
+  case 0:
+    return true;
+
+  default:
+    if (j > 10)
+      b = false;
+    else
+      b = true;
+    return b;
+    // CHECK-MESSAGES: :[[@LINE-4]]:11: warning: {{.*}} in conditional assignment
+    // CHECK-FIXES: {{b = j <= 10;}}
+    // CHECK-FIXES-NEXT: {{return b;}}
+  }
+  return false;
+}
+
+bool default_stmt14(int i, int j, bool b) {
+  switch (i) {
+  case 0:
+    return true;
+
+  default:
+    if (j > 10)
+      return true;
+    return false;
+    // CHECK-MESSAGES: :[[@LINE-2]]:14: warning: {{.*}} in conditional return
+    // FIXES: {{return j > 10;}}
+  }
+  return false;
+}
+
+bool default_stmt15(int i, int j, bool b) {
+  switch (i) {
+  case 0:
+    return true;
+
+  default:
+    if (j > 10)
+      return false;
+    return true;
+    // CHECK-MESSAGES: :[[@LINE-2]]:14: warning: {{.*}} in conditional return
+    // FIXES: {{return j <= 10;}}
+  }
+  return false;
+}
+
+bool default_stmt16(int i, int j, bool b) {
+  switch (i) {
+  case 0:
+    return false;
+
+  default:
+    if (j > 10)
+      return true;
+  }
+  return false;
+}
+
+bool default_stmt17(int i, int j, bool b) {
+  switch (i) {
+  case 0:
+    return true;
+
+  default:
+    if (j > 10)
+      return false;
+  }
+  return false;
+}
+
+bool label_stmt0(int i, int j, bool b) {
+label:
+  if (b == true)
+    j = 10;
+  // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: {{.*}} to boolean operator
+  // CHECK-FIXES: {{if \(b\)}}
+  // CHECK-FIXES-NEXT: {{j = 10;}}
+  return false;
+}
+
+bool label_stmt1(int i, int j, bool b) {
+label:
+  if (b == false)
+    j = -20;
+  // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: {{.*}} to boolean operator
+  // CHECK-FIXES: {{if \(!b\)}}
+  // CHECK-FIXES-NEXT: {{j = -20;}}
+  return false;
+}
+
+bool label_stmt2(int i, int j, bool b) {
+label:
+  if (b && true)
+    j = 10;
+  // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: {{.*}} to boolean operator
+  // CHECK-FIXES: {{if \(b\)}}
+  // CHECK-FIXES-NEXT: {{j = 10;}}
+  return false;
+}
+
+bool label_stmt3(int i, int j, bool b) {
+label:
+  if (b && false)
+    j = -20;
+  // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: {{.*}} to boolean operator
+  // CHECK-FIXES: {{if \(false\)}}
+  // CHECK-FIXES-NEXT: {{j = -20;}}
+  return false;
+}
+
+bool label_stmt4(int i, int j, bool b) {
+label:
+  if (b || true)
+    j = 10;
+  // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: {{.*}} to boolean operator
+  // CHECK-FIXES: {{if \(true\)}}
+  // CHECK-FIXES-NEXT: {{j = 10;}}
+  return false;
+}
+
+bool label_stmt5(int i, int j, bool b) {
+label:
+  if (b || false)
+    j = -20;
+  // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: {{.*}} to boolean operator
+  // CHECK-FIXES: {{if \(b\)}}
+  // CHECK-FIXES-NEXT: {{j = -20;}}
+  return false;
+}
+
+bool label_stmt6(int i, int j, bool b) {
+label:
+  return i > 0 ? true : false;
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: {{.*}} in ternary expression result
+  // CHECK-FIXES: {{return i > 0;}}
+}
+
+bool label_stmt7(int i, int j, bool b) {
+label:
+  return i > 0 ? false : true;
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: {{.*}} in ternary expression result
+  // CHECK-FIXES: {{return i <= 0;}}
+}
+
+bool label_stmt8(int i, int j, bool b) {
+label:
+  if (true)
+    j = 10;
+  else
+    j = -20;
+  // CHECK-MESSAGES: :[[@LINE-4]]:7: warning: {{.*}} in if statement condition
+  // CHECK-FIXES:      {{j = 10;$}}
+  return false;
+}
+
+bool label_stmt9(int i, int j, bool b) {
+label:
+  if (false)
+    j = -20;
+  else
+    j = 10;
+  // CHECK-MESSAGES: :[[@LINE-4]]:7: warning: {{.*}} in if statement condition
+  // CHECK-FIXES: {{j = 10;}}
+  return false;
+}
+
+bool label_stmt10(int i, int j, bool b) {
+label:
+  if (j > 10)
+    return true;
+  else
+    return false;
+  // CHECK-MESSAGES: :[[@LINE-3]]:12: warning: {{.*}} in conditional return statement
+  // CHECK-FIXES: {{return j > 10;}}
+}
+
+bool label_stmt11(int i, int j, bool b) {
+label:
+  if (j > 10)
+    return false;
+  else
+    return true;
+  // CHECK-MESSAGES: :[[@LINE-3]]:12: warning: {{.*}} in conditional return statement
+  // CHECK-FIXES: {{return j <= 10;}}
+}
+
+bool label_stmt12(int i, int j, bool b) {
+label:
+  if (j > 10)
+    b = true;
+  else
+    b = false;
+  return b;
+  // CHECK-MESSAGES: :[[@LINE-4]]:9: warning: {{.*}} in conditional assignment
+  // CHECK-FIXES: {{b = j > 10;}}
+  // CHECK-FIXES-NEXT: {{return b;}}
+}
+
+bool label_stmt13(int i, int j, bool b) {
+label:
+  if (j > 10)
+    b = false;
+  else
+    b = true;
+  return b;
+  // CHECK-MESSAGES: :[[@LINE-4]]:9: warning: {{.*}} in conditional assignment
+  // CHECK-FIXES: {{b = j <= 10;}}
+  // CHECK-FIXES-NEXT: {{return b;}}
+}
+
+bool label_stmt14(int i, int j, bool b) {
+label:
+  if (j > 10)
+    return true;
+  return false;
+  // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: {{.*}} in conditional return
+  // FIXES: {{return j > 10;}}
+}
+
+bool label_stmt15(int i, int j, bool b) {
+label:
+  if (j > 10)
+    return false;
+  return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: {{.*}} in conditional return
+  // FIXES: {{return j <= 10;}}
+}

clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp

void complex_conditional_assignment_statements(int i) {
   } else {
     j = 20;
     f = false;
   }
 
   // Unchanged: multiple statements.
   bool g;
   if (j > 10)
-    f = true;
+    g = true;
   else {
     j = 20;
-    f = false;
+    g = false;
   }
 
   // Unchanged: multiple statements.
   bool h;
   if (j > 10) {
     j = 10;
-    f = true;
+    h = true;
   } else
-    f = false;
+    h = false;
 }
 
 // Unchanged: chained return statements, but ChainedConditionalReturn not set.
 bool chained_conditional_compound_return(int i) {
   if (i < 0) {
     return true;
   } else if (i < 10) {
     return false;

clang-tools-extra/unittests/clang-tidy/CMakeLists.txt

 clang_target_link_libraries(ClangTidyTests
   PRIVATE
   clangAST
   clangASTMatchers
   clangBasic
   clangFrontend
   clangLex
   clangSerialization
+  clangTesting
   clangTooling
   clangToolingCore
   clangTransformer
   )
 target_link_libraries(ClangTidyTests
   PRIVATE
   clangTidy
   clangTidyAndroidModule
   clangTidyGoogleModule
   clangTidyLLVMModule
   clangTidyObjCModule
   clangTidyReadabilityModule
   clangTidyUtils
   LLVMTestingSupport
   )

clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp

+#include "../../clang/unittests/ASTMatchers/ASTMatchersTest.h"
 #include "ClangTidyTest.h"
 #include "readability/BracesAroundStatementsCheck.h"
 #include "readability/NamespaceCommentCheck.h"
+#include "readability/SimplifyBooleanExprMatchers.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace tidy {
 namespace test {
 
 using readability::BracesAroundStatementsCheck;
 using readability::NamespaceCommentCheck;
+using namespace ast_matchers;
+
+TEST_P(ASTMatchersTest, HasCaseSubstatement) {
+  EXPECT_TRUE(matches(
+      "void f() { switch (1) { case 1: return; break; default: break; } }",
+      traverse(TK_AsIs, caseStmt(hasSubstatement(returnStmt())))));
+}
+
+TEST_P(ASTMatchersTest, HasDefaultSubstatement) {
+  EXPECT_TRUE(matches(
+      "void f() { switch (1) { case 1: return; break; default: break; } }",
+      traverse(TK_AsIs, defaultStmt(hasSubstatement(breakStmt())))));
+}
+
+TEST_P(ASTMatchersTest, HasLabelSubstatement) {
+  EXPECT_TRUE(
+      matches("void f() { while (1) { bar: break; foo: return; } }",
+              traverse(TK_AsIs, labelStmt(hasSubstatement(breakStmt())))));
+}
+
+TEST_P(ASTMatchersTest, HasSubstatementSequenceSimple) {
+  const char *Text = "int f() { int x = 5; if (x < 0) return 1; return 0; }";
+  EXPECT_TRUE(matches(
+      Text, compoundStmt(hasSubstatementSequence(ifStmt(), returnStmt()))));
+  EXPECT_FALSE(matches(
+      Text, compoundStmt(hasSubstatementSequence(ifStmt(), labelStmt()))));
+  EXPECT_FALSE(matches(
+      Text, compoundStmt(hasSubstatementSequence(returnStmt(), ifStmt()))));
+  EXPECT_FALSE(matches(
+      Text, compoundStmt(hasSubstatementSequence(switchStmt(), labelStmt()))));
+}
+
+TEST_P(ASTMatchersTest, HasSubstatementSequenceAlmost) {
+  const char *Text = R"code(
+int f() {
+  int x = 5;
+  if (x < 10)
+    ;
+  if (x < 0)
+    return 1;
+  return 0;
+}
+)code";
+  EXPECT_TRUE(matches(
+      Text, compoundStmt(hasSubstatementSequence(ifStmt(), returnStmt()))));
+  EXPECT_TRUE(
+      matches(Text, compoundStmt(hasSubstatementSequence(ifStmt(), ifStmt()))));
+}
+
+TEST_P(ASTMatchersTest, HasSubstatementSequenceComplex) {
+  const char *Text = R"code(
+int f() {
+  int x = 5;
+  if (x < 10)
+    x -= 10;
+  if (x < 0)
+    return 1;
+  return 0;
+}
+)code";
+  EXPECT_TRUE(matches(
+      Text, compoundStmt(hasSubstatementSequence(ifStmt(), returnStmt()))));
+  EXPECT_FALSE(
+      matches(Text, compoundStmt(hasSubstatementSequence(ifStmt(), expr()))));
+}
+
+TEST_P(ASTMatchersTest, HasSubstatementSequenceExpression) {
+  const char *Text = R"code(
+int f() {
+  return ({ int x = 5;
+      int result;
+      if (x < 10)
+        x -= 10;
+      if (x < 0)
+        result = 1;
+      else
+        result = 0;
+      result;
+    });
+  }
+)code";
+  EXPECT_TRUE(
+      matches(Text, stmtExpr(hasSubstatementSequence(ifStmt(), expr()))));
+  EXPECT_FALSE(
+      matches(Text, stmtExpr(hasSubstatementSequence(ifStmt(), returnStmt()))));
+}
+
+// Copied from ASTMatchersTests
+static std::vector<TestClangConfig> allTestClangConfigs() {
+  std::vector<TestClangConfig> all_configs;
+  for (TestLanguage lang : {Lang_C89, Lang_C99, Lang_CXX03, Lang_CXX11,
+                            Lang_CXX14, Lang_CXX17, Lang_CXX20}) {
+    TestClangConfig config;
+    config.Language = lang;
+
+    // Use an unknown-unknown triple so we don't instantiate the full system
+    // toolchain.  On Linux, instantiating the toolchain involves stat'ing
+    // large portions of /usr/lib, and this slows down not only this test, but
+    // all other tests, via contention in the kernel.
+    //
+    // FIXME: This is a hack to work around the fact that there's no way to do
+    // the equivalent of runToolOnCodeWithArgs without instantiating a full
+    // Driver.  We should consider having a function, at least for tests, that
+    // invokes cc1.
+    config.Target = "i386-unknown-unknown";
+    all_configs.push_back(config);
+
+    // Windows target is interesting to test because it enables
+    // `-fdelayed-template-parsing`.
+    config.Target = "x86_64-pc-win32-msvc";
+    all_configs.push_back(config);
+  }
+  return all_configs;
+}
+
+INSTANTIATE_TEST_SUITE_P(ASTMatchersTests, ASTMatchersTest,
+                         testing::ValuesIn(allTestClangConfigs()));
 
 TEST(NamespaceCommentCheckTest, Basic) {
   EXPECT_EQ("namespace i {\n} // namespace i",
             runCheckOnCode<NamespaceCommentCheck>("namespace i {\n}"));
   EXPECT_EQ("namespace {\n} // namespace",
             runCheckOnCode<NamespaceCommentCheck>("namespace {\n}"));
   EXPECT_EQ("namespace i { namespace j {\n} // namespace j\n } // namespace i",
             runCheckOnCode<NamespaceCommentCheck>(

 
 #define EXPECT_NO_CHANGES_WITH_OPTS(Check, Opts, Code)                         \
   EXPECT_EQ(Code, runCheckOnCode<Check>(Code, nullptr, "input.cc", None, Opts))
 TEST(BracesAroundStatementsCheckTest, ImplicitCastInReturn) {
   ClangTidyOptions Opts;
   Opts.CheckOptions["test-check-0.ShortStatementLines"] = "1";
 
   StringRef Input = "const char *f() {\n"
-                              "  if (true) return \"\";\n"
-                              "  return \"abc\";\n"
-                              "}\n";
   EXPECT_NO_CHANGES_WITH_OPTS(BracesAroundStatementsCheck, Opts, Input);
   EXPECT_EQ("const char *f() {\n"
             "  if (true) { return \"\";\n"
             "}\n"
             "  return \"abc\";\n"
             "}\n",
             runCheckOnCode<BracesAroundStatementsCheck>(Input));
 }
 
 } // namespace test
 } // namespace tidy
 } // namespace clang