[clang-tidy] Enhance clang-tidy readability-simplify-boolean-expr...

Enhance clang-tidy readability-simplify-boolean-expr to handle 'if (e) return
true; return false;' and improve replacement expressions.

This changeset extends the simplify boolean expression check in clang-tidy to
simplify if (e) return true; return false; to return e; (note the lack of an
else clause on the if statement.) By default, chained conditional assignment is
left unchanged, unless a configuration parameter is set to non-zero to override
this behavior.

It also improves the handling of replacement expressions to apply
static_cast<bool>(expr) when expr is not of type bool.

http://reviews.llvm.org/D9810

Patch by Richard Thomson!

llvm-svn: 241155

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

@@ -12,6 +12,7 @@
 
 #include <cassert>
 #include <string>
+#include <utility>
 
 using namespace clang::ast_matchers;
 
@@ -48,6 +49,9 @@ const char IfAssignLocId[] = "if-assign-loc";
 const char IfAssignBoolId[] = "if-assign";
 const char IfAssignNotBoolId[] = "if-assign-not";
 const char IfAssignObjId[] = "if-assign-obj";
+const char CompoundReturnId[] = "compound-return";
+const char CompoundBoolId[] = "compound-bool";
+const char CompoundNotBoolId[] = "compound-bool-not";
 
 const char IfStmtId[] = "if";
 const char LHSId[] = "lhs-expr";
@@ -57,6 +61,8 @@ const char SimplifyOperatorDiagnostic[] =
     "redundant boolean literal supplied to boolean operator";
 const char SimplifyConditionDiagnostic[] =
     "redundant boolean literal in if statement condition";
+const char SimplifyConditionalReturnDiagnostic[] =
+    "redundant boolean literal in conditional return statement";
 
 const CXXBoolLiteralExpr *getBoolLiteral(const MatchFinder::MatchResult &Result,
                                          StringRef Id) {
@@ -67,25 +73,26 @@ const CXXBoolLiteralExpr *getBoolLiteral(const MatchFinder::MatchResult &Result,
              : Literal;
 }
 
-internal::Matcher<Stmt> ReturnsBool(bool Value, StringRef Id = "") {
+internal::Matcher<Stmt> returnsBool(bool Value, StringRef Id = "ignored") {
-  auto SimpleReturnsBool = returnStmt(
+  auto SimpleReturnsBool =
-      has(boolLiteral(equals(Value)).bind(Id.empty() ? "ignored" : Id)));
+      returnStmt(has(boolLiteral(equals(Value)).bind(Id))).bind("returns-bool");
   return anyOf(SimpleReturnsBool,
                compoundStmt(statementCountIs(1), has(SimpleReturnsBool)));
 }
 
 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))
+  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[] = {
-    std::make_pair(BO_LT, BO_GE), std::make_pair(BO_GT, BO_LE),
+    {BO_LT, BO_GE}, {BO_GT, BO_LE}, {BO_EQ, BO_NE}};
-    std::make_pair(BO_EQ, BO_NE)};
 
 StringRef negatedOperator(const BinaryOperator *BinOp) {
   const BinaryOperatorKind Opcode = BinOp->getOpcode();
@@ -98,24 +105,153 @@ StringRef negatedOperator(const BinaryOperator *BinOp) {
   return StringRef();
 }
 
-std::string replacementExpression(const MatchFinder::MatchResult &Result,
+std::pair<OverloadedOperatorKind, StringRef> OperatorNames[] = {
-                                  bool Negated, const Expr *E) {
+    {OO_EqualEqual, "=="},   {OO_ExclaimEqual, "!="}, {OO_Less, "<"},
-  while (const auto *Parenthesized = dyn_cast<ParenExpr>(E)) {
+    {OO_GreaterEqual, ">="}, {OO_Greater, ">"},       {OO_LessEqual, "<="}};
-    E = Parenthesized->getSubExpr();
+
+StringRef getOperatorName(OverloadedOperatorKind OpKind) {
+  for (auto Name : OperatorNames) {
+    if (Name.first == OpKind)
+      return Name.second;
   }
-  if (Negated) {
+
-    if (const auto *BinOp = dyn_cast<BinaryOperator>(E)) {
+  return StringRef();
-      StringRef NegatedOperator = negatedOperator(BinOp);
+}
-      if (!NegatedOperator.empty()) {
+
-        return (getText(Result, *BinOp->getLHS()) + " " + NegatedOperator +
+std::pair<OverloadedOperatorKind, OverloadedOperatorKind> OppositeOverloads[] =
-                " " + getText(Result, *BinOp->getRHS())).str();
+    {{OO_EqualEqual, OO_ExclaimEqual},
+     {OO_Less, OO_GreaterEqual},
+     {OO_Greater, OO_LessEqual}};
+
+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();
+}
+
+std::string asBool(StringRef text, bool NeedsStaticCast) {
+  if (NeedsStaticCast)
+    return ("static_cast<bool>(" + text + ")").str();
+
+  return text;
+}
+
+bool needsNullPtrComparison(const Expr *E) {
+  if (const auto *ImpCast = dyn_cast<ImplicitCastExpr>(E))
+    return ImpCast->getCastKind() == CK_PointerToBoolean;
+
+  return false;
+}
+
+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;
+        }
       }
     }
   }
-  StringRef Text = getText(Result, *E);
+
-  return (Negated ? (needsParensAfterUnaryNegation(E) ? "!(" + Text + ")"
+  E = E->IgnoreImpCasts();
-                                                      : "!" + Text)
+  return !E->getType()->isBooleanType();
-                  : Text).str();
+}
+
+std::string replacementExpression(const MatchFinder::MatchResult &Result,
+                                  bool Negated, const Expr *E) {
+  E = E->ignoreParenBaseCasts();
+  const bool NeedsStaticCast = needsStaticCast(E);
+  if (Negated) {
+    if (const auto *UnOp = dyn_cast<UnaryOperator>(E)) {
+      if (UnOp->getOpcode() == UO_LNot) {
+        if (needsNullPtrComparison(UnOp->getSubExpr()))
+          return (getText(Result, *UnOp->getSubExpr()) + " != nullptr").str();
+
+        return replacementExpression(Result, false, UnOp->getSubExpr());
+      }
+    }
+
+    if (needsNullPtrComparison(E))
+      return (getText(Result, *E) + " == nullptr").str();
+
+    StringRef NegatedOperator;
+    const Expr *LHS = nullptr;
+    const Expr *RHS = nullptr;
+    if (const auto *BinOp = dyn_cast<BinaryOperator>(E)) {
+      NegatedOperator = negatedOperator(BinOp);
+      LHS = BinOp->getLHS();
+      RHS = BinOp->getRHS();
+    } else if (const auto *OpExpr = dyn_cast<CXXOperatorCallExpr>(E)) {
+      if (OpExpr->getNumArgs() == 2) {
+        NegatedOperator = negatedOperator(OpExpr);
+        LHS = OpExpr->getArg(0);
+        RHS = OpExpr->getArg(1);
+      }
+    }
+    if (!NegatedOperator.empty() && LHS && RHS) {
+      return (asBool((getText(Result, *LHS) + " " + NegatedOperator + " " +
+                      getText(Result, *RHS))
+                         .str(),
+                     NeedsStaticCast));
+    }
+
+    StringRef Text = getText(Result, *E);
+    if (!NeedsStaticCast && needsParensAfterUnaryNegation(E))
+      return ("!(" + Text + ")").str();
+
+    if (needsNullPtrComparison(E))
+      return (getText(Result, *E) + " == nullptr").str();
+
+    return ("!" + asBool(Text, NeedsStaticCast));
+  }
+
+  if (const auto *UnOp = dyn_cast<UnaryOperator>(E)) {
+    if (UnOp->getOpcode() == UO_LNot) {
+      if (needsNullPtrComparison(UnOp->getSubExpr()))
+        return (getText(Result, *UnOp->getSubExpr()) + " == nullptr").str();
+    }
+  }
+
+  if (needsNullPtrComparison(E))
+    return (getText(Result, *E) + " != nullptr").str();
+
+  return asBool(getText(Result, *E), NeedsStaticCast);
+}
+
+const CXXBoolLiteralExpr *stmtReturnsBool(const ReturnStmt *Ret, bool Negated) {
+  if (const auto *Bool = dyn_cast<CXXBoolLiteralExpr>(Ret->getRetValue())) {
+    if (Bool->getValue() == !Negated)
+      return Bool;
+  }
+
+  return nullptr;
+}
+
+const CXXBoolLiteralExpr *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;
 }
 
 } // namespace
@@ -185,10 +321,11 @@ void SimplifyBooleanExprCheck::matchExprCompOpBool(MatchFinder *Finder,
 void SimplifyBooleanExprCheck::matchBoolCondition(MatchFinder *Finder,
                                                   bool Value,
                                                   StringRef BooleanId) {
-  Finder->addMatcher(ifStmt(isExpansionInMainFile(),
+  Finder->addMatcher(
-                            hasCondition(boolLiteral(equals(Value))
+      ifStmt(isExpansionInMainFile(),
-                                             .bind(BooleanId))).bind(IfStmtId),
+             hasCondition(boolLiteral(equals(Value)).bind(BooleanId)))
-                     this);
+          .bind(IfStmtId),
+      this);
 }
 
 void SimplifyBooleanExprCheck::matchTernaryResult(MatchFinder *Finder,
@@ -206,14 +343,16 @@ void SimplifyBooleanExprCheck::matchIfReturnsBool(MatchFinder *Finder,
                                                   bool Value, StringRef Id) {
   if (ChainedConditionalReturn) {
     Finder->addMatcher(ifStmt(isExpansionInMainFile(),
-                              hasThen(ReturnsBool(Value, ThenLiteralId)),
+                              hasThen(returnsBool(Value, ThenLiteralId)),
-                              hasElse(ReturnsBool(!Value))).bind(Id),
+                              hasElse(returnsBool(!Value)))
+                           .bind(Id),
                        this);
   } else {
     Finder->addMatcher(ifStmt(isExpansionInMainFile(),
                               unless(hasParent(ifStmt())),
-                              hasThen(ReturnsBool(Value, ThenLiteralId)),
+                              hasThen(returnsBool(Value, ThenLiteralId)),
-                              hasElse(ReturnsBool(!Value))).bind(Id),
+                              hasElse(returnsBool(!Value)))
+                           .bind(Id),
                        this);
   }
 }
@@ -240,11 +379,25 @@ void SimplifyBooleanExprCheck::matchIfAssignsBool(MatchFinder *Finder,
   } else {
     Finder->addMatcher(ifStmt(isExpansionInMainFile(),
                               unless(hasParent(ifStmt())), hasThen(Then),
-                              hasElse(Else)).bind(Id),
+                              hasElse(Else))
+                           .bind(Id),
                        this);
   }
 }
 
+void SimplifyBooleanExprCheck::matchCompoundIfReturnsBool(MatchFinder *Finder,
+                                                          bool Value,
+                                                          StringRef Id) {
+  Finder->addMatcher(
+      compoundStmt(
+          allOf(hasAnySubstatement(ifStmt(hasThen(returnsBool(Value)),
+                                          unless(hasElse(stmt())))),
+                hasAnySubstatement(returnStmt(has(boolLiteral(equals(!Value))))
+                                       .bind(CompoundReturnId))))
+          .bind(Id),
+      this);
+}
+
 void SimplifyBooleanExprCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "ChainedConditionalReturn", ChainedConditionalReturn);
   Options.store(Opts, "ChainedConditionalAssignment",
@@ -283,6 +436,9 @@ void SimplifyBooleanExprCheck::registerMatchers(MatchFinder *Finder) {
 
   matchIfAssignsBool(Finder, true, IfAssignBoolId);
   matchIfAssignsBool(Finder, false, IfAssignNotBoolId);
+
+  matchCompoundIfReturnsBool(Finder, true, CompoundBoolId);
+  matchCompoundIfReturnsBool(Finder, false, CompoundNotBoolId);
 }
 
 void SimplifyBooleanExprCheck::check(const MatchFinder::MatchResult &Result) {
@@ -322,6 +478,12 @@ void SimplifyBooleanExprCheck::check(const MatchFinder::MatchResult &Result) {
   } 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 =
+                 Result.Nodes.getNodeAs<CompoundStmt>(CompoundNotBoolId)) {
+    replaceCompoundReturnWithCondition(Result, Compound, true);
   }
 }
 
@@ -375,10 +537,51 @@ void SimplifyBooleanExprCheck::replaceWithReturnCondition(
   std::string Replacement = ("return " + Condition + Terminator).str();
   SourceLocation Start =
       Result.Nodes.getNodeAs<CXXBoolLiteralExpr>(ThenLiteralId)->getLocStart();
-  diag(Start, "redundant boolean literal in conditional return statement")
+  diag(Start, SimplifyConditionalReturnDiagnostic)
       << FixItHint::CreateReplacement(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 fo a single
+  //    `return` statement returning the opposite boolean literal `true` or
+  //    `false`.
+  assert(Compound->size() >= 2);
+  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 CXXBoolLiteralExpr *Lit = stmtReturnsBool(If, Negated)) {
+        if (*After == Ret) {
+          if (!ChainedConditionalReturn && BeforeIf)
+            continue;
+
+          const Expr *Condition = If->getCond();
+          std::string Replacement =
+              "return " + replacementExpression(Result, Negated, Condition);
+          diag(Lit->getLocStart(), SimplifyConditionalReturnDiagnostic)
+              << FixItHint::CreateReplacement(
+                  SourceRange(If->getLocStart(), Ret->getLocEnd()),
+                  Replacement);
+          return;
+        }
+
+        BeforeIf = If;
+      }
+    } else {
+      BeforeIf = nullptr;
+    }
+  }
+}
+
 void SimplifyBooleanExprCheck::replaceWithAssignment(
     const MatchFinder::MatchResult &Result, const IfStmt *IfAssign,
     bool Negated) {

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

@@ -34,9 +34,46 @@ namespace readability {
 /// `if (e) return false; else return true;`   becomes `return !e;`
 /// `if (e) b = true; else b = false;`         becomes `b = e;`
 /// `if (e) b = false; else b = true;`         becomes `b = !e;`
+/// `if (e) return true; return false;`        becomes `return e;`
+/// `if (e) return false; return true;`        becomes `return !e;`
 ///
-/// Parenthesis from the resulting expression `e` are removed whenever
+/// The resulting expression `e` is modified as follows:
-/// possible.
+/// 1. Unnecessary parentheses around the expression are removed.
+/// 2. Negated applications of `!` are eliminated.
+/// 3. Negated applications of comparison operators are changed to use the
+///    opposite condition.
+/// 4. Implicit conversions of pointer to `bool` are replaced with explicit
+///    comparisons to `nullptr`.
+/// 5. Implicit casts to `bool` are replaced with explicit casts to `bool`.
+/// 6. Object expressions with `explicit operator bool` conversion operators
+///    are replaced with explicit casts to `bool`.
+///
+/// Examples:
+/// 1. The ternary assignment `bool b = (i < 0) ? true : false;` has redundant
+/// parentheses and becomes `bool b = i < 0;`.
+///
+/// 2. The conditional return `if (!b) return false; return true;` has an
+/// implied double negation and becomes `return b;`.
+///
+/// 3. The conditional return `if (i < 0) return false; return true;` becomes
+/// `return i >= 0;`.
+/// The conditional return `if (i != 0) return false; return true;` becomes
+/// `return i == 0;`.
+///
+/// 4. The conditional return `if (p) return true; return false;` has an
+/// implicit conversion of a pointer to `bool` and becomes
+/// `return p != nullptr;`.
+/// The ternary assignment `bool b = (i & 1) ? true : false;` has an implicit
+/// conversion of `i & 1` to `bool` and becomes
+/// `bool b = static_cast<bool>(i & 1);`.
+///
+/// 5. The conditional return `if (i & 1) return true; else return false;` has
+/// an implicit conversion of an integer quantity `i & 1` to `bool` and becomes
+/// `return static_cast<bool>(i & 1);`
+///
+/// 6. Given `struct X { explicit operator bool(); };`, and an instance `x` of
+/// `struct X`, the conditional return `if (x) return true; return false;`
+/// becomes `return static_cast<bool>(x);`
 ///
 /// When a conditional boolean return or assignment appears at the end of a
 /// chain of `if`, `else if` statements, the conditional statement is left
@@ -77,6 +114,9 @@ private:
   void matchIfAssignsBool(ast_matchers::MatchFinder *Finder, bool Value,
                           StringRef Id);
 
+  void matchCompoundIfReturnsBool(ast_matchers::MatchFinder *Finder, bool Value,
+                                  StringRef Id);
+
   void
   replaceWithExpression(const ast_matchers::MatchFinder::MatchResult &Result,
                         const CXXBoolLiteralExpr *BoolLiteral, bool UseLHS,
@@ -103,6 +143,10 @@ private:
   replaceWithAssignment(const ast_matchers::MatchFinder::MatchResult &Result,
                         const IfStmt *If, bool Negated = false);
 
+  void replaceCompoundReturnWithCondition(
+      const ast_matchers::MatchFinder::MatchResult &Result,
+      const CompoundStmt *Compound, bool Negated = false);
+
   const bool ChainedConditionalReturn;
   const bool ChainedConditionalAssignment;
 };

clang-tools-extra/test/clang-tidy/readability-simplify-bool-expr-chained-conditional-return.cpp

@@ -31,3 +31,65 @@ bool chained_conditional_return(int i) {
   // CHECK-FIXES-NEXT: {{^}}    return false;
   // CHECK-FIXES-NEXT: {{^}}  else return i > 20;
 }
+
+bool chained_simple_if_return(int i) {
+  if (i < 5)
+    return true;
+  if (i > 10)
+    return true;
+  return false;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: {{^}}bool chained_simple_if_return(int i) {{{$}}
+// CHECK-FIXES: {{^}}  if (i < 5){{$}}
+// CHECK-FIXES: {{^    return true;$}}
+// CHECK-FIXES: {{^  return i > 10;$}}
+// CHECK-FIXES: {{^}$}}
+
+bool chained_simple_if_return_negated(int i) {
+  if (i < 5)
+    return false;
+  if (i > 10)
+    return false;
+  return true;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: {{^}}bool chained_simple_if_return_negated(int i) {{{$}}
+// CHECK-FIXES: {{^}}  if (i < 5){{$}}
+// CHECK-FIXES: {{^    return false;$}}
+// CHECK-FIXES: {{^  return i <= 10;$}}
+// CHECK-FIXES: {{^}$}}
+
+bool complex_chained_if_return_return(int i) {
+  if (i < 5) {
+    return true;
+  }
+  if (i > 10) {
+    return true;
+  }
+  return false;
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: {{^}}bool complex_chained_if_return_return(int i) {{{$}}
+// CHECK-FIXES: {{^}}  if (i < 5) {{{$}}
+// CHECK-FIXES: {{^}}    return true;{{$}}
+// CHECK-FIXES: {{^}}  }{{$}}
+// CHECK-FIXES: {{^  return i > 10;$}}
+// CHECK-FIXES: {{^}$}}
+
+bool complex_chained_if_return_return_negated(int i) {
+  if (i < 5) {
+    return false;
+  }
+  if (i > 10) {
+    return false;
+  }
+  return true;
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: {{^}}bool complex_chained_if_return_return_negated(int i) {{{$}}
+// CHECK-FIXES: {{^}}  if (i < 5) {{{$}}
+// CHECK-FIXES: {{^}}    return false;{{$}}
+// CHECK-FIXES: {{^}}  }{{$}}
+// CHECK-FIXES: {{^  return i <= 10;$}}
+// CHECK-FIXES: {{^}$}}

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

@@ -321,6 +321,13 @@ bool negated_conditional_return_statements(int i) {
 // CHECK-FIXES:      {{^}}  return i != 0;{{$}}
 // CHECK-FIXES-NEXT: {{^}$}}
 
+bool negative_condition_conditional_return_statement(int i) {
+  if (!(i == 0)) return false; else return true;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:25: warning: {{.*}} in conditional return statement
+// CHECK-FIXES:      {{^}}  return i == 0;{{$}}
+// CHECK-FIXES-NEXT: {{^}$}}
+
 bool conditional_compound_return_statements(int i) {
   if (i == 1) {
     return true;
@@ -447,26 +454,26 @@ void simple_conditional_assignment_statements(int i) {
   // CHECK-FIXES: {{^  }}c = i <= 20;{{$}}
   // CHECK-FIXES: bool c2 = false;
 
-  // unchanged; different variables
+  // Unchanged: different variables.
   bool b2;
   if (i > 12)
     b = true;
   else
     b2 = false;
 
-  // unchanged; no else statement
+  // Unchanged: no else statement.
   bool b3;
   if (i > 15)
     b3 = true;
 
-  // unchanged; not boolean assignment
+  // Unchanged: not boolean assignment.
   int j;
   if (i > 17)
     j = 10;
   else
     j = 20;
 
-  // unchanged; different variables assigned
+  // Unchanged: different variables assigned.
   int k = 0;
   bool b4 = false;
   if (i > 10)
@@ -500,13 +507,13 @@ void complex_conditional_assignment_statements(int i) {
   // CHECK-FIXES: {{^  }}e = i <= 40;{{$}}
   // CHECK-FIXES: e = false;
 
-  // unchanged; no else statement
+  // Unchanged: no else statement.
   bool b3;
   if (i > 15) {
     b3 = true;
   }
 
-  // unchanged; not a boolean assignment
+  // Unchanged: not a boolean assignment.
   int j;
   if (i > 17) {
     j = 10;
@@ -514,7 +521,7 @@ void complex_conditional_assignment_statements(int i) {
     j = 20;
   }
 
-  // unchanged; multiple statements
+  // Unchanged: multiple statements.
   bool f;
   if (j > 10) {
     j = 10;
@@ -524,7 +531,7 @@ void complex_conditional_assignment_statements(int i) {
     f = false;
   }
 
-  // unchanged; multiple statements
+  // Unchanged: multiple statements.
   bool g;
   if (j > 10)
     f = true;
@@ -533,7 +540,7 @@ void complex_conditional_assignment_statements(int i) {
     f = false;
   }
 
-  // unchanged; multiple statements
+  // Unchanged: multiple statements.
   bool h;
   if (j > 10) {
     j = 10;
@@ -542,7 +549,7 @@ void complex_conditional_assignment_statements(int i) {
     f = false;
 }
 
-// unchanged: chained return statements, but ChainedConditionalReturn not set
+// Unchanged: chained return statements, but ChainedConditionalReturn not set.
 bool chained_conditional_compound_return(int i) {
   if (i < 0) {
     return true;
@@ -555,7 +562,7 @@ bool chained_conditional_compound_return(int i) {
   }
 }
 
-// unchanged: chained return statements, but ChainedConditionalReturn not set
+// Unchanged: chained return statements, but ChainedConditionalReturn not set.
 bool chained_conditional_return(int i) {
   if (i < 0)
     return true;
@@ -567,7 +574,7 @@ bool chained_conditional_return(int i) {
     return false;
 }
 
-// unchanged: chained assignments, but ChainedConditionalAssignment not set
+// Unchanged: chained assignments, but ChainedConditionalAssignment not set.
 void chained_conditional_compound_assignment(int i) {
   bool b;
   if (i < 0) {
@@ -581,7 +588,7 @@ void chained_conditional_compound_assignment(int i) {
   }
 }
 
-// unchanged: chained return statements, but ChainedConditionalReturn not set
+// Unchanged: chained return statements, but ChainedConditionalReturn not set.
 void chained_conditional_assignment(int i) {
   bool b;
   if (i < 0)
@@ -593,3 +600,275 @@ void chained_conditional_assignment(int i) {
   else
     b = false;
 }
+
+// Unchanged: chained return statements, but ChainedConditionalReturn not set.
+bool chained_simple_if_return_negated(int i) {
+  if (i < 5)
+    return false;
+  if (i > 10)
+    return false;
+  return true;
+}
+
+// Unchanged: chained return statements, but ChainedConditionalReturn not set.
+bool complex_chained_if_return_return(int i) {
+  if (i < 5) {
+    return true;
+  }
+  if (i > 10) {
+    return true;
+  }
+  return false;
+}
+
+// Unchanged: chained return statements, but ChainedConditionalReturn not set.
+bool complex_chained_if_return_return_negated(int i) {
+  if (i < 5) {
+    return false;
+  }
+  if (i > 10) {
+    return false;
+  }
+  return true;
+}
+
+// Unchanged: chained return statements, but ChainedConditionalReturn not set.
+bool chained_simple_if_return(int i) {
+  if (i < 5)
+    return true;
+  if (i > 10)
+    return true;
+  return false;
+}
+
+bool simple_if_return_return(int i) {
+  if (i > 10)
+    return true;
+  return false;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: {{^}}bool simple_if_return_return(int i) {{{$}}
+// CHECK-FIXES: {{^  return i > 10;$}}
+// CHECK-FIXES: {{^}$}}
+
+bool simple_if_return_return_negated(int i) {
+  if (i > 10)
+    return false;
+  return true;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: {{^}}bool simple_if_return_return_negated(int i) {{{$}}
+// CHECK-FIXES: {{^  return i <= 10;$}}
+// CHECK-FIXES: {{^}$}}
+
+bool complex_if_return_return(int i) {
+  if (i > 10) {
+    return true;
+  }
+  return false;
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: {{^}}bool complex_if_return_return(int i) {{{$}}
+// CHECK-FIXES: {{^  return i > 10;$}}
+// CHECK-FIXES: {{^}$}}
+
+bool complex_if_return_return_negated(int i) {
+  if (i > 10) {
+    return false;
+  }
+  return true;
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: {{^}}bool complex_if_return_return_negated(int i) {{{$}}
+// CHECK-FIXES: {{^  return i <= 10;$}}
+// CHECK-FIXES: {{^}$}}
+
+bool if_implicit_bool_expr(int i) {
+  if (i & 1) {
+    return true;
+  } else {
+    return false;
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: {{^}}  return static_cast<bool>(i & 1);{{$}}
+
+bool negated_if_implicit_bool_expr(int i) {
+  if (i - 1) {
+    return false;
+  } else {
+    return true;
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: {{^}}  return !static_cast<bool>(i - 1);{{$}}
+
+bool implicit_int(int i) {
+  if (i) {
+    return true;
+  } else {
+    return false;
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: {{^}}  return static_cast<bool>(i);{{$}}
+
+bool explicit_bool(bool b) {
+  if (b) {
+    return true;
+  } else {
+    return false;
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: {{^}}  return b;{{$}}
+
+class Implicit {
+public:
+  operator bool() {
+    return true;
+  }
+};
+
+bool object_bool_implicit_conversion(Implicit O) {
+  if (O) {
+    return true;
+  } else {
+    return false;
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: {{^}}  return O;{{$}}
+
+bool negated_explicit_bool(bool b) {
+  if (!b) {
+    return true;
+  } else {
+    return false;
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: {{^}}  return !b;{{$}}
+
+bool bitwise_complement_conversion(int i) {
+  if (~i) {
+    return true;
+  } else {
+    return false;
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: {{^}}  return static_cast<bool>(~i);{{$}}
+
+bool logical_or(bool a, bool b) {
+  if (a || b) {
+    return true;
+  } else {
+    return false;
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: {{^}}  return a || b;{{$}}
+
+bool logical_and(bool a, bool b) {
+  if (a && b) {
+    return true;
+  } else {
+    return false;
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: {{^}}  return a && b;{{$}}
+
+class Comparable
+{
+public:
+  bool operator==(Comparable const &rhs) { return true; }
+  bool operator!=(Comparable const &rhs) { return false; }
+};
+
+bool comparable_objects() {
+  Comparable c;
+  Comparable d;
+  if (c == d) {
+    return true;
+  } else {
+    return false;
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: {{^}}  return c == d;{{$}}
+
+bool negated_comparable_objects() {
+  Comparable c;
+  Comparable d;
+  if (c == d) {
+    return false;
+  } else {
+    return true;
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: {{^}}  return c != d;{{$}}
+
+struct X {
+  explicit operator bool();
+};
+
+void explicit_conversion_assignment(X x) {
+  bool y;
+  if (x) {
+    y = true;
+  } else {
+    y = false;
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:9: warning: {{.*}} in conditional assignment
+// CHECK-FIXES: {{^  bool y;$}}
+// CHECK-FIXES: {{^}}  y = static_cast<bool>(x);{{$}}
+
+void ternary_integer_condition(int i) {
+  bool b = i ? true : false;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:16: warning: {{.*}} in ternary expression result
+// CHECK-FIXES: bool b = static_cast<bool>(i);{{$}}
+
+bool non_null_pointer_condition(int *p1) {
+  if (p1) {
+    return true;
+  } else {
+    return false;
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: return p1 != nullptr;{{$}}
+
+bool null_pointer_condition(int *p2) {
+  if (!p2) {
+    return true;
+  } else {
+    return false;
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: return p2 == nullptr;{{$}}
+
+bool negated_non_null_pointer_condition(int *p3) {
+  if (p3) {
+    return false;
+  } else {
+    return true;
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: return p3 == nullptr;{{$}}
+
+bool negated_null_pointer_condition(int *p4) {
+  if (!p4) {
+    return false;
+  } else {
+    return true;
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: return p4 != nullptr;{{$}}