[PATCH] [clang-tidy] Force braces around leaf 'else if' for consistency.

[Samuel Benzaquen]

REPOSITORY
  rL LLVM

http://reviews.llvm.org/D10245

Files:
  clang-tools-extra/trunk/clang-tidy/readability/BracesAroundStatementsCheck.cpp
  clang-tools-extra/trunk/clang-tidy/readability/BracesAroundStatementsCheck.h
  clang-tools-extra/trunk/unittests/clang-tidy/ReadabilityModuleTest.cpp

Index: clang-tools-extra/trunk/clang-tidy/readability/BracesAroundStatementsCheck.cpp
===================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/BracesAroundStatementsCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/readability/BracesAroundStatementsCheck.cpp
@@ -151,11 +151,15 @@
     SourceLocation StartLoc = findRParenLoc(S, SM, Context);
     if (StartLoc.isInvalid())
       return;
+    if (ForceBracesStmts.erase(S))
+      ForceBracesStmts.insert(S->getThen());
     bool BracedIf = checkStmt(Result, S->getThen(), StartLoc, S->getElseLoc());
     const Stmt *Else = S->getElse();
+    if (Else && BracedIf)
+      ForceBracesStmts.insert(Else);
     if (Else && !isa<IfStmt>(Else)) {
       // Omit 'else if' statements here, they will be handled directly.
-      checkStmt(Result, Else, S->getElseLoc(), SourceLocation(), BracedIf);
+      checkStmt(Result, Else, S->getElseLoc(), SourceLocation());
     }
   } else {
     llvm_unreachable("Invalid match");
@@ -203,7 +207,7 @@
 /// Returns true if braces where added.
 bool BracesAroundStatementsCheck::checkStmt(
     const MatchFinder::MatchResult &Result, const Stmt *S,
-    SourceLocation InitialLoc, SourceLocation EndLocHint, bool ForceBraces) {
+    SourceLocation InitialLoc, SourceLocation EndLocHint) {
   // 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
@@ -241,7 +245,7 @@
   assert(EndLoc.isValid());
   // Don't require braces for statements spanning less than certain number of
   // lines.
-  if (ShortStatementLines && !ForceBraces) {
+  if (ShortStatementLines && !ForceBracesStmts.erase(S)) {
     unsigned StartLine = SM.getSpellingLineNumber(StartLoc);
     unsigned EndLine = SM.getSpellingLineNumber(EndLoc);
     if (EndLine - StartLine < ShortStatementLines)
@@ -254,6 +258,10 @@
   return true;
 }
 
+void BracesAroundStatementsCheck::onEndOfTranslationUnit() {
+  ForceBracesStmts.clear();
+}
+
 } // namespace readability
 } // namespace tidy
 } // namespace clang
Index: clang-tools-extra/trunk/clang-tidy/readability/BracesAroundStatementsCheck.h
===================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/BracesAroundStatementsCheck.h
+++ clang-tools-extra/trunk/clang-tidy/readability/BracesAroundStatementsCheck.h
@@ -41,17 +41,18 @@
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void onEndOfTranslationUnit() override;
 
 private:
   bool checkStmt(const ast_matchers::MatchFinder::MatchResult &Result,
                  const Stmt *S, SourceLocation StartLoc,
-                 SourceLocation EndLocHint = SourceLocation(),
-                 bool ForceBraces = false);
+                 SourceLocation EndLocHint = SourceLocation());
   template <typename IfOrWhileStmt>
   SourceLocation findRParenLoc(const IfOrWhileStmt *S, const SourceManager &SM,
                                const ASTContext *Context);
 
 private:
+  std::set<const Stmt*> ForceBracesStmts;
   const unsigned ShortStatementLines;
 };
 
Index: clang-tools-extra/trunk/unittests/clang-tidy/ReadabilityModuleTest.cpp
===================================================================
--- clang-tools-extra/trunk/unittests/clang-tidy/ReadabilityModuleTest.cpp
+++ clang-tools-extra/trunk/unittests/clang-tidy/ReadabilityModuleTest.cpp
@@ -254,6 +254,19 @@
                 "  else return -3;\n"
                 "}",
                 nullptr, "input.cc", None, Options));
+
+  // If the last else is an else-if, we also force it.
+  EXPECT_EQ("int main() {\n"
+            "  if (false) { return -1;\n"
+            "  } else if (1 == 2) { return -2;\n"
+            "}\n"
+            "}",
+            runCheckOnCode<BracesAroundStatementsCheck>(
+                "int main() {\n"
+                "  if (false) return -1;\n"
+                "  else if (1 == 2) return -2;\n"
+                "}",
+                nullptr, "input.cc", None, Options));
 }
 
 TEST(BracesAroundStatementsCheck, For) {

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D10245.27128.patch
Type: text/x-patch
Size: 4303 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150604/eb4aed9c/attachment.bin>