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

[sbenza]

Summary

Force braces around leaf 'else if' for consistency.
This complements r233697.

[alexfh]

Thanks for the fix! LG

BTW, it seems wrong that we don't allow if-else chains without braces, when every body fits on one line.

if (...) ...;
else if (....) ...;
...

or

if (...)
  ...;
else if (....)
  ...;
...

depending on the style and ShortStatementLines setting.

[sbenza]

I think it does that because we are looking at the range between the start of the "then" until the "else" keyword, and that has a newline in it.
But that is a different bug to fix.

clang-tidy/readability/BracesAroundStatementsCheck.h

 /// Default value 0 means that braces will be added to all statements (not
 /// having them already).
 class BracesAroundStatementsCheck : public ClangTidyCheck {
 public:
   BracesAroundStatementsCheck(StringRef Name, ClangTidyContext *Context);
   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;
 };
 
 } // namespace readability
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_BRACESAROUNDSTATEMENTSCHECK_H

clang-tidy/readability/BracesAroundStatementsCheck.cpp

if (auto S = Result.Nodes.getNodeAs<ForStmt>("for")) {
     SourceLocation StartLoc = findRParenLoc(S, SM, Context);
     if (StartLoc.isInvalid())
       return;
     checkStmt(Result, S->getBody(), StartLoc);
   } else if (auto S = Result.Nodes.getNodeAs<IfStmt>("if")) {
     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");
   }
 }
 
 /// Find location of right parenthesis closing condition
 template <typename IfOrWhileStmt>
BracesAroundStatementsCheck::findRParenLoc(const IfOrWhileStmt *S,
   }
   return RParenLoc;
 }
 
 /// 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) {
+    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
   // the location we're inserting the closing brace at, or there's a non-comment
   // token, the check inserts "\n}" right before that token.
   // 3) Otherwise the check finds the end of line (possibly after some block or
   // line comments) and inserts "\n}" right before that EOL.
   if (!S || isa<CompoundStmt>(S)) {
if (EndLocHint.isValid()) {
     EndLoc = findEndLocation(S->getLocEnd(), SM, Context);
     ClosingInsertion = "\n}";
   }
 
   assert(StartLoc.isValid());
   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)
       return false;
   }
 
   auto Diag = diag(StartLoc, "statement should be inside braces");
   Diag << FixItHint::CreateInsertion(StartLoc, " {")
        << FixItHint::CreateInsertion(EndLoc, ClosingInsertion);
   return true;
 }
 
+void BracesAroundStatementsCheck::onEndOfTranslationUnit() {
+  ForceBracesStmts.clear();
+}
+
 } // namespace readability
 } // namespace tidy
 } // namespace clang

unittests/clang-tidy/ReadabilityModuleTest.cpp

EXPECT_EQ("int main() {\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));
+
+  // 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) {
   EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n"
                                                  "  for (;;) {\n"
                                                  "    ;\n"
                                                  "  }\n"
                                                  "}");