[PATCH] Add readability-simplify-boolean-expr check to clang-tidy

[Alexander Kornienko]

================
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:254
@@ +253,3 @@
+    const CXXBoolLiteralExpr *BoolLiteral, bool UseLHS, bool Negated) {
+  const auto LHS = Result.Nodes.getNodeAs<Expr>(LHSId);
+  const auto RHS = Result.Nodes.getNodeAs<Expr>(RHSId);
----------------
Please use "const auto *" for pointers.

================
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:315
@@ +314,3 @@
+  std::string Replacement =
+      (Negated ? (VariableName + " = !(" + Condition + ")" + Terminator)
+               : (VariableName + " = " + Condition + Terminator)).str();
----------------
1. Parentheses are not always needed. I'd suggest adding a couple of helper functions to format negation correctly. These may be missing something, but should be mostly correct:


    bool needsParensAfterUnaryNegation(const Expr &E) {
      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::string negateExpression(const MatchFinder::MatchResult &Result, const Expr &E) {
    StringRef Text = getText(Result, E);
    return (needsParensAfterUnaryNegation(E) ? "!(" + Text + ")" : "!" + Text).str();
  }

This applies to other replaceWith* methods as well.

2. How about moving out all non-conditional parts to shorten the expression?

  (VariableName + " = " + (Negated ? "!(" + Condition + ")" : Condition) + Terminator).str();

================
Comment at: test/clang-tidy/readability-simplify-bool-expr.cpp:51
@@ +50,3 @@
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: {{.*}} to boolean operator
+// X-CHECK-FIXES: {{^bool a8 = false;$}}
+bool a9 = a1 && true;
----------------
Why?

================
Comment at: test/clang-tidy/readability-simplify-bool-expr.cpp:62
@@ +61,3 @@
+
+void if_with_bool_literal_condition()
+{
----------------
Please make the test code closer to LLVM style, except for the places where special formatting is required for testing whitespace handling. In particular: 2 space indentation, braces on the same line as function headers or control flow statements, function names should be camelCaseStartingWithALowerCaseLetter, comments should use proper capitalization and punctuation.

================
Comment at: test/clang-tidy/readability-simplify-bool-expr.cpp:170
@@ +169,3 @@
+        // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: {{.*}} to boolean operator
+        // X-CHECK-FIXES: {{^    if \(true\) {$}}
+        i = 20;
----------------
Why?

http://reviews.llvm.org/D7648

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/