Force braces on the else branch if they are being added to the if branch.

Summary:
Force braces on the else branch if they are being added to the if branch.
This ensures consistency in the transformed code.

Reviewers: alexfh

Subscribers: cfe-commits

Differential Revision: http://reviews.llvm.org/D8708

llvm-svn: 233697

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

@@ -151,11 +151,11 @@ BracesAroundStatementsCheck::check(const MatchFinder::MatchResult &Result) {
     SourceLocation StartLoc = findRParenLoc(S, SM, Context);
     if (StartLoc.isInvalid())
       return;
-    checkStmt(Result, S->getThen(), StartLoc, S->getElseLoc());
+    bool BracedIf = checkStmt(Result, S->getThen(), StartLoc, S->getElseLoc());
     const Stmt *Else = S->getElse();
     if (Else && !isa<IfStmt>(Else)) {
       // Omit 'else if' statements here, they will be handled directly.
-      checkStmt(Result, Else, S->getElseLoc());
+      checkStmt(Result, Else, S->getElseLoc(), SourceLocation(), BracedIf);
     }
   } else {
     llvm_unreachable("Invalid match");
@@ -199,10 +199,11 @@ BracesAroundStatementsCheck::findRParenLoc(const IfOrWhileStmt *S,
   return RParenLoc;
 }
 
-void
-BracesAroundStatementsCheck::checkStmt(const MatchFinder::MatchResult &Result,
-                                       const Stmt *S, SourceLocation InitialLoc,
-                                       SourceLocation EndLocHint) {
+/// Determine if the statement needs braces around it, and add them if it does.
+/// Returns true if braces where added.
+bool BracesAroundStatementsCheck::checkStmt(
+    const MatchFinder::MatchResult &Result, const Stmt *S,
+    SourceLocation InitialLoc, SourceLocation EndLocHint, bool ForceBraces) {
   // 1) If there's a corresponding "else" or "while", the check inserts "} "
   // right before that token.
   // 2) If there's a multi-line block comment starting on the same line after
@@ -212,11 +213,11 @@ BracesAroundStatementsCheck::checkStmt(const MatchFinder::MatchResult &Result,
   // line comments) and inserts "\n}" right before that EOL.
   if (!S || isa<CompoundStmt>(S)) {
     // Already inside braces.
-    return;
+    return false;
   }
   // Skip macros.
   if (S->getLocStart().isMacroID())
-    return;
+    return false;
 
   const SourceManager &SM = *Result.SourceManager;
   const ASTContext *Context = Result.Context;
@@ -240,16 +241,17 @@ BracesAroundStatementsCheck::checkStmt(const MatchFinder::MatchResult &Result,
   assert(EndLoc.isValid());
   // Don't require braces for statements spanning less than certain number of
   // lines.
-  if (ShortStatementLines) {
+  if (ShortStatementLines && !ForceBraces) {
     unsigned StartLine = SM.getSpellingLineNumber(StartLoc);
     unsigned EndLine = SM.getSpellingLineNumber(EndLoc);
     if (EndLine - StartLine < ShortStatementLines)
-      return;
+      return false;
   }
 
   auto Diag = diag(StartLoc, "statement should be inside braces");
   Diag << FixItHint::CreateInsertion(StartLoc, " {")
        << FixItHint::CreateInsertion(EndLoc, ClosingInsertion);
+  return true;
 }
 
 } // namespace readability

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

@@ -43,9 +43,10 @@ public:
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
 private:
-  void checkStmt(const ast_matchers::MatchFinder::MatchResult &Result,
+  bool checkStmt(const ast_matchers::MatchFinder::MatchResult &Result,
                  const Stmt *S, SourceLocation StartLoc,
-                 SourceLocation EndLocHint = SourceLocation());
+                 SourceLocation EndLocHint = SourceLocation(),
+                 bool ForceBraces = false);
   template <typename IfOrWhileStmt>
   SourceLocation findRParenLoc(const IfOrWhileStmt *S, const SourceManager &SM,
                                const ASTContext *Context);

clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h

@@ -42,11 +42,12 @@ private:
 };
 
 template <typename T>
-std::string runCheckOnCode(StringRef Code,
-                           std::vector<ClangTidyError> *Errors = nullptr,
+std::string
+runCheckOnCode(StringRef Code, std::vector<ClangTidyError> *Errors = nullptr,
                const Twine &Filename = "input.cc",
-                           ArrayRef<std::string> ExtraArgs = None) {
-  ClangTidyOptions Options;
+               ArrayRef<std::string> ExtraArgs = None,
+               const ClangTidyOptions &ExtraOptions = ClangTidyOptions()) {
+  ClangTidyOptions Options = ExtraOptions;
   Options.Checks = "*";
   ClangTidyContext Context(llvm::make_unique<DefaultOptionsProvider>(
       ClangTidyGlobalOptions(), Options));

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

@@ -235,6 +235,27 @@ TEST(BracesAroundStatementsCheck, If) {
                                                         "}"));
 }
 
+TEST(BracesAroundStatementsCheck, IfElseWithShortStatements) {
+  ClangTidyOptions Options;
+  Options.CheckOptions["test-check.ShortStatementLines"] = "1";
+
+  EXPECT_EQ("int main() {\n"
+            "  if (true) return 1;\n"
+            "  if (false) { return -1;\n"
+            "  } else if (1 == 2) { return -2;\n"
+            "  } else { return -3;\n"
+            "}\n"
+            "}",
+            runCheckOnCode<BracesAroundStatementsCheck>(
+                "int main() {\n"
+                "  if (true) return 1;\n"
+                "  if (false) return -1;\n"
+                "  else if (1 == 2) return -2;\n"
+                "  else return -3;\n"
+                "}",
+                nullptr, "input.cc", None, Options));
+}
+
 TEST(BracesAroundStatementsCheck, For) {
   EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n"
                                                  "  for (;;) {\n"