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

Enhance clang-tidy readability-simplify-boolean-expr check to handle chained
conditional assignment and chained conditional return.

Based on feedback from applying this tool to the clang/LLVM codebase, this
changeset improves the readability-simplify-boolean-expr check so that
conditional assignment or return statements at the end of a chain of if/else if
statements are left unchanged unless a configuration option is supplied.

http://reviews.llvm.org/D8996

Patch by Richard Thomson!

llvm-svn: 237541

1 files changed, 37 insertions, 11 deletions

diff --git a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
index c9a1182..f7754e4 100644
--- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -108,20 +108,25 @@ std::string replacementExpression(const MatchFinder::MatchResult &Result,
       StringRef NegatedOperator = negatedOperator(BinOp);
       if (!NegatedOperator.empty()) {
         return (getText(Result, *BinOp->getLHS()) + " " + NegatedOperator +
-                " " + getText(Result, *BinOp->getRHS()))
-            .str();
+                " " + getText(Result, *BinOp->getRHS())).str();
       }
     }
   }
   StringRef Text = getText(Result, *E);
   return (Negated ? (needsParensAfterUnaryNegation(E) ? "!(" + Text + ")"
                                                       : "!" + Text)
-                  : Text)
-      .str();
+                  : Text).str();
 }
 
 } // namespace
 
+SimplifyBooleanExprCheck::SimplifyBooleanExprCheck(StringRef Name,
+                                                   ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      ChainedConditionalReturn(Options.get("ChainedConditionalReturn", 0U)),
+      ChainedConditionalAssignment(
+          Options.get("ChainedConditionalAssignment", 0U)) {}
+
 void SimplifyBooleanExprCheck::matchBoolBinOpExpr(MatchFinder *Finder,
                                                   bool Value,
                                                   StringRef OperatorName,
@@ -199,10 +204,18 @@ void SimplifyBooleanExprCheck::matchTernaryResult(MatchFinder *Finder,
 
 void SimplifyBooleanExprCheck::matchIfReturnsBool(MatchFinder *Finder,
                                                   bool Value, StringRef Id) {
-  Finder->addMatcher(ifStmt(isExpansionInMainFile(),
-                            hasThen(ReturnsBool(Value, ThenLiteralId)),
-                            hasElse(ReturnsBool(!Value))).bind(Id),
-                     this);
+  if (ChainedConditionalReturn) {
+    Finder->addMatcher(ifStmt(isExpansionInMainFile(),
+                              hasThen(ReturnsBool(Value, ThenLiteralId)),
+                              hasElse(ReturnsBool(!Value))).bind(Id),
+                       this);
+  } else {
+    Finder->addMatcher(ifStmt(isExpansionInMainFile(),
+                              unless(hasParent(ifStmt())),
+                              hasThen(ReturnsBool(Value, ThenLiteralId)),
+                              hasElse(ReturnsBool(!Value))).bind(Id),
+                       this);
+  }
 }
 
 void SimplifyBooleanExprCheck::matchIfAssignsBool(MatchFinder *Finder,
@@ -220,9 +233,22 @@ void SimplifyBooleanExprCheck::matchIfAssignsBool(MatchFinder *Finder,
       hasRHS(boolLiteral(equals(!Value))));
   auto Else = anyOf(SimpleElse, compoundStmt(statementCountIs(1),
                                              hasAnySubstatement(SimpleElse)));
-  Finder->addMatcher(
-      ifStmt(isExpansionInMainFile(), hasThen(Then), hasElse(Else)).bind(Id),
-      this);
+  if (ChainedConditionalAssignment) {
+    Finder->addMatcher(
+        ifStmt(isExpansionInMainFile(), hasThen(Then), hasElse(Else)).bind(Id),
+        this);
+  } else {
+    Finder->addMatcher(ifStmt(isExpansionInMainFile(),
+                              unless(hasParent(ifStmt())), hasThen(Then),
+                              hasElse(Else)).bind(Id),
+                       this);
+  }
+}
+
+void SimplifyBooleanExprCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "ChainedConditionalReturn", ChainedConditionalReturn);
+  Options.store(Opts, "ChainedConditionalAssignment",
+                ChainedConditionalAssignment);
 }
 
 void SimplifyBooleanExprCheck::registerMatchers(MatchFinder *Finder) {