[clang-tidy] Add check misc-braces-around-statements.

[curdeius]

Summary

This check looks for if statements and loops: for, range-for, while and do-while, and verifies that the inside statements are inside braces '{}'.
If not, proposes to add braces around them.

Example:

if (condition)
  action();

becomes

if (condition) {
  action();
}

This check ought to be used with the -format option, so that the braces be well-formatted.

[alexfh]

Thanks for working on this check!

Please try the suggestions in my comments and see whether it helps to resolve some of your issues.

clang-tidy/misc/BracesAroundStatementsCheck.cpp

72

Both options are wrong. You don't need to specify Offset explicitly. getLocEnd() returns the location of the first character of the last token, so you need to use Lexer::getLocForEndOfToken(S->getLocEnd()), which will get you a location right after the last token of the statement.

Also, if there's an else/while();, I'd better you put the closing brace right before them in case there are comments between the statement and else/while();.

Addressing this and the next comment will make it possible to maintain the LLVM-like style of arranging braces even without reformatting the changes.

75

If this check aims to work well for styles similar to the one used in LLVM, it needs to insert the opening brace right after the if()/else/for()/while()/do, in case there's a comment between if()/... and the statement, as clang-format doesn't rearrange tokens.

test/clang-tidy/misc-braces-around-statements.cpp

12

We don't usually check these notes, but I don't object, if you leave these checks.

27

Any reason for this #if? The check_clang_tidy_fix.sh script always runs clang-tidy with -std=c++11.

[alexfh]

clang-tidy/misc/BracesAroundStatementsCheck.cpp

72

And of course I meant:

SourceLocation EndLoc = Lexer::getLocForEndOfToken(
    S->getLocEnd(), 0, *Result.SourceManager,
    Result.Context->getLangOpts());

[curdeius]

• Fix: BracesAroundStatementsCheck.

[alexfh]

That's better, but still a few comments.

clang-tidy/misc/BracesAroundStatementsCheck.cpp

19

Please remove the empty line.

31

You can remove the "ast_matchers::" part as there's a "using namespace clang::ast_matchers;" above.

33

This is a perfect use case for "auto". The type is spelled on the RHS of the initialization, so you can safely skip it in the LHS.

34

I think, you can pass the starting location of the last token to checkStmt and call Lexer::getLocForEndOfToken inside the function. Then all these invocations will look more uniform and the total amount of code is going to be less.

56

Any reasons not to use isa<IfStmt>(Else)? This seems to be clearer to me.

83

If you insert " {", the result will be one step closer to correct formatting.

clang-tidy/misc/BracesAroundStatementsCheck.h

2

Please respect the 80 columns limit.

[curdeius]

Fix and clean as suggested by alexfk.

• [clang-tidy] Fix BracesAroundStatementsCheck when processing 'if' and 'while' statements with comments inside condition.

[curdeius]

Alex, thanks for the useful comments.
The code is much cleaner now.
Besides, trying to apply your suggestions, I stumbled upon one more case where previous implementation gave erroneous results, namely when there are comments or whitespace, or empty macro expansion inside the if/while condition.
I have therefore added a function backwardSkipWhitespacesAndComments (actually copied from clang-modernize/AddOverride/AddOverrideActions.cpp - maybe some refactoring is needed to pull up common utilities?).

clang-tidy/misc/BracesAroundStatementsCheck.h

34

This line does not hold the 80-column limit, but clang-format leaves it as it is.
What is the convention in this case?

[alexfh]

clang-tidy/misc/BracesAroundStatementsCheck.cpp

82

Please add:

else {
  llvm_unreachable("Invalid match"):
}

or something similar.

103

Unfortunately, this is wrong. Consider these examples:

if (x) f(); // here it will work
if (x) f() /* here it won't */ ;
do if(y) {}while (x); // and here too

I don't know if the AST provides a convenient way to find out if there's a semicolon behind the end of the statement and where it is located. I also don't know why the statements that end with a semicolon do not store its location or just include it in the source range. We need to ask someone more familiar with the AST.

clang-tidy/misc/BracesAroundStatementsCheck.h

34

It's fine here, as there's not much you can do. I was only talking about removing extra dashes from the file header comments.

[curdeius]

Fix BracesAroundStatementsCheck with nested statements. Apply comments from alexfh.

• Add llvm_unreachable.
• [clang-tidy] Fix BracesAroundStatementsCheck with nested statements.

[alexfh]

Thanks again for the hard work on this patch! It's now almost fine, but there's still one issue, which clang-format won't be able to fix. I'd slightly prefer that it be fixed before submitting the patch. See the comment inline.

clang-tidy/misc/BracesAroundStatementsCheck.cpp

58

s/Whitespaces/Whitespace/

75

nit: I'd prefer "// namespace" for consistency with how the llvm-namespace-comment check does it.

97

I think, RParenLoc should be stored inside WhileStmt and IfStmt, then this code will be much easier. This doesn't have to be in this patch, but please add a FIXME.

test/clang-tidy/misc-braces-around-statements.cpp

148

This looks slightly wrong to put a closing brace before a trailing comment, and clang-format won't fix this. I think, the check should try to keep the trailing comment next to the statement it belongs to. I'm not sure if there can be a 100% fool-proof solution, but I hope, we can come up with a decent heuristic.

Assuming the coding style is similar to the one used in LLVM, we could do the following:

1. if there's a corresponding "else" or "while", the check should insert "} " 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 should insert "} " right before that token.
3. otherwise the check should find the end of line (possibly after some block or line comments) and insert "}\n" right after that EOL. It could also insert some spaces to make the indentation nice, but this could be done by clang-format as well.

Do you see any problems with this plan?

[curdeius]

Thanks for your comments Alex.
Comments inline.

clang-tidy/misc/BracesAroundStatementsCheck.cpp

58

Ok.

75

Ok.

97

Ok for a FIXME.
I'd prefer creating a new patch for this, as this can have wider repercussions, not only on clang-tidy.

test/clang-tidy/misc-braces-around-statements.cpp

148

Looks like a plan!

1. is trivial (already done), 2. and 3. should be quite easy too.

In 3. I would leave indentation to clang-format, anyway one should run it after using clang-tidy.

Is there any easy way of checking if a block-comment spans over multiple lines?

[alexfh]

clang-tidy/misc/BracesAroundStatementsCheck.cpp

97

Sure. I've prepared D5528 to address this. Not sure though, whether we'll go this way or not.

test/clang-tidy/misc-braces-around-statements.cpp

148

In 3. I would leave indentation to clang-format, anyway one should run it after using clang-tidy.

Sounds good.

Is there any easy way of checking if a block-comment spans over multiple lines?

I'd try something like this:

Lexer::getSourceText(CharSourceRange::getTokenRange(Tok.getLocation(), Tok.getLocation()), Result.SourceManager, Result.Context->getLangOpts()).find('\n') != StringRef::npos

[curdeius]

Add heuristics for treating comments at end of statements.

• [clang-tidy] BracesAroundStatementsCheck: add EndLocHint (before 'else' in if-else stmt and 'while' in do-while loop).
• [clang-tidy] BracesAroundStatementsCheck: refactoring.
• [clang-tidy] BracesAroundStatementsCheck: heuristics for treating comments.

[curdeius]

Alex, I have applied a heuristics as you suggested.
I have done a minor change, for cases 2. (multi-line comments or non-comment token) and 3. (trailing comment), I put "\n}" before found end of line. I do not separate the two cases by adding or not a space after '}'. I think that in any case, a run of clang-format is needed here. Tell me if I'm wrong.

[alexfh]

This looks good for the initial version once you address the comments.

I'd also ask you to run the check over LLVM source code as a smoke test and take a look at the changes the script produces (you can use the run-clang-tidy.py script for this). This can be done after submitting the code.

Thanks again for the great job!

clang-tidy/misc/BracesAroundStatementsCheck.cpp

170

Sorry for missing this previously: here and in the next branch you could find the right parenthesis by skipping forward starting from Lexer::getLocForEndOfToken(WS->getCond()->getLocEnd(), 0, ...). This would probably be more efficient and use only one skip function.

test/clang-tidy/misc-braces-around-statements.cpp

180

What happens with the comment that was after the semicolon? Could you please add it to the appropriate check line?

[curdeius]

I wanted to apply your suggestion about skip functions, but stumbled upon a new problem: how should we handle macros when searching for brace location (in skip function)?
They are not expanded on the moment when clang-tidy runs over the file, so how can I expand them to check if they're empty?
I tried Token::hasLeadingEmptyMacro(), but it doesn't work as I expected.

test/clang-tidy/misc-braces-around-statements.cpp

180

I deleted it somehow. Restored.

[alexfh]

In D5395#30, @curdeius wrote:

I wanted to apply your suggestion about skip functions, but stumbled upon a new problem: how should we handle macros when searching for brace location (in skip function)?

I think, you could iterate over all tokens located from after the WS->getCond()->getLocEnd(), but before the WS->getBody()->getLocStart(). If the first non-comment and non-whitespace token you find is not a closing parenthesis, you can bail out, because something weird is going on. I don't think you'll need to handle macros here.

They are not expanded on the moment when clang-tidy runs over the file,

This is not exactly true, but it doesn't matter for our problem.

so how can I expand them to check if they're empty?
I tried Token::hasLeadingEmptyMacro(), but it doesn't work as I expected.

[curdeius]

Fixed treating macros and variable declarations in conditions.

• [clang-tidy] Skipping macros. Refactoring. Fixed treating variable declarations inside conditions.

[curdeius]

Hi Alex,
I've done some further modifications to correctly handle statements with a variable declaration in it (like while (auto x = f()) ;) and handling macro-expanded statements (ignoring them).
If you think that it's ok now, can you please land this patch (I don't have write access).

[curdeius]

Oh, and I've run this check over llvm source code, that's how I've added all these checks like SourceLocation::isValid() and skipped macros.

[alexfh]

In D5395#36, @curdeius wrote:

Hi Alex,
I've done some further modifications to correctly handle statements with a variable declaration in it (like while (auto x = f()) ;) and handling macro-expanded statements (ignoring them).
If you think that it's ok now, can you please land this patch (I don't have write access).

Hi Marek,

The check looks good now! I've made a few minor changes (spelling, comments, braces, etc.) and committing the patch now. I've added a few inline comments to document what I'm changing in the patch and why.

I've also added a FIXME to add some configurability to the check to make it useful for Google and LLVM. Both allow if/while/... bodies without braces with certain conditions, so it would be incorrect to just insert braces everywhere.

Thank you again for the great work!

clang-tidy/misc/BracesAroundStatementsCheck.cpp

30

Here and elsewhere: we normally don't use braces around single-line if/while/for/... bodies. It's not explicitly written down, but all examples in the Coding Standards document are written this way: http://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code

49

Here and elsewhere: the comments should be correct sentences, in particular, start with a capital character and end with a period.

54

These two methods do a very similar stuff and are only used once. I've inlined them.

62

nit: newline is a single word, hence s/hasNewLine/hasNewline/

112

It's a bit too defensive. The condition is checked in the previous statement.

121

nit: s/new line/newline/

test/clang-tidy/misc-braces-around-statements.cpp

177

There's no need to check this line.

unittests/clang-tidy/MiscModuleTest.cpp

78

These comments document facts that are obvious from a single glance at the code. I've removed the comments.

117

We don't commit commented-out code. I've added a FIXME and a test showing current behavior.

[alexfh]

Committed as r218898.

clang-tidy/misc/BracesAroundStatementsCheck.h

+//===--- BracesAroundStatementsCheck.h - clang-tidy -------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_BRACES_AROUND_STATEMENTS_CHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_BRACES_AROUND_STATEMENTS_CHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+
+class BracesAroundStatementsCheck : public ClangTidyCheck {
+public:
+  BracesAroundStatementsCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  void checkStmt(const ast_matchers::MatchFinder::MatchResult &Result,
+                 const Stmt *S, SourceLocation StartLoc,
+                 SourceLocation EndLocHint = SourceLocation());
+  template <typename IfOrWhileStmt>
+  SourceLocation findRParenLoc(const IfOrWhileStmt *S, const SourceManager &SM,
+                               const ASTContext *Context);
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_BRACES_AROUND_STATEMENTS_CHECK_H

clang-tidy/misc/BracesAroundStatementsCheck.cpp

+//===--- BracesAroundStatementsCheck.cpp - clang-tidy ---------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "BracesAroundStatementsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace {
+
+tok::TokenKind getTokenKind(SourceLocation Loc, const SourceManager &SM,
+                            const ASTContext *Context) {
+  Token Tok;
+  SourceLocation Beginning =
+      Lexer::GetBeginningOfToken(Loc, SM, Context->getLangOpts());
+  const bool Invalid =
+      Lexer::getRawToken(Beginning, Tok, SM, Context->getLangOpts());
+  assert(!Invalid && "Expected a valid token.");
+
+  if (Invalid) {
+    return tok::NUM_TOKENS;
+  }
+  return Tok.getKind();
+}
+
+SourceLocation forwardSkipWhitespaceAndComments(SourceLocation Loc,
+                                                const SourceManager &SM,
+                                                const ASTContext *Context) {
+  assert(Loc.isValid());
+  for (;;) {
+    while (isWhitespace(*FullSourceLoc(Loc, SM).getCharacterData())) {
+      Loc = Loc.getLocWithOffset(1);
+    }
+
+    tok::TokenKind TokKind = getTokenKind(Loc, SM, Context);
+    if (TokKind == tok::NUM_TOKENS || TokKind != tok::comment) {
+      return Loc;
+    }
+    // Fast-forward current token
+    Loc = Lexer::getLocForEndOfToken(Loc, 0, SM, Context->getLangOpts());
+  }
+}
+
+bool isBlockComment(SourceRange R, const SourceManager &SM,
+                    const ASTContext *Context) {
+  assert(R.isValid());
+  StringRef Comment = Lexer::getSourceText(CharSourceRange::getTokenRange(R),
+                                           SM, Context->getLangOpts());
+  return Comment.startswith("/*");
+}
+
+bool hasNewLine(SourceRange R, const SourceManager &SM,
+                const ASTContext *Context) {
+  assert(R.isValid());
+  StringRef Code = Lexer::getSourceText(CharSourceRange::getTokenRange(R), SM,
+                                        Context->getLangOpts());
+  return Code.find('\n') != StringRef::npos;
+}
+
+SourceLocation findEndLocation(SourceLocation LastTokenLoc,
+                               const SourceManager &SM,
+                               const ASTContext *Context) {
+  SourceLocation Loc = LastTokenLoc;
+  // Loc points to the beginning of the last (non-comment non-ws) token
+  // before end or ';'.
+  assert(Loc.isValid());
+  bool SkipEndWhitespaceAndComments = true;
+  tok::TokenKind TokKind = getTokenKind(Loc, SM, Context);
+  if (TokKind == tok::NUM_TOKENS || TokKind == tok::semi ||
+      TokKind == tok::r_brace) {
+    // If we are at ";" or "}", we found the last token. We could use as well
+    // `if (isa<NullStmt>(S))`, but it wouldn't work for nested statements.
+    SkipEndWhitespaceAndComments = false;
+  }
+
+  Loc = Lexer::getLocForEndOfToken(Loc, 0, SM, Context->getLangOpts());
+  // Loc points past the last token before end or after ';'
+
+  if (SkipEndWhitespaceAndComments) {
+    Loc = forwardSkipWhitespaceAndComments(Loc, SM, Context);
+    tok::TokenKind TokKind = getTokenKind(Loc, SM, Context);
+    if (TokKind == tok::semi) {
+      Loc = Lexer::getLocForEndOfToken(Loc, 0, SM, Context->getLangOpts());
+    }
+  }
+
+  for (;;) {
+    assert(Loc.isValid());
+    while (isHorizontalWhitespace(*FullSourceLoc(Loc, SM).getCharacterData())) {
+      Loc = Loc.getLocWithOffset(1);
+    }
+    if (isVerticalWhitespace(*FullSourceLoc(Loc, SM).getCharacterData())) {
+      // EOL, brace before
+      break;
+    }
+    tok::TokenKind TokKind = getTokenKind(Loc, SM, Context);
+    if (TokKind != tok::comment) {
+      // Non-comment token, brace before
+      break;
+    }
+
+    assert(TokKind == tok::comment);
+    SourceLocation TokEndLoc =
+        Lexer::getLocForEndOfToken(Loc, 0, SM, Context->getLangOpts());
+    SourceRange TokRange(Loc, TokEndLoc);
+    if (isBlockComment(TokRange, SM, Context)) {
+      if (hasNewLine(TokRange, SM, Context)) {
+        // Multi-line block comment, brace before
+        break;
+      }
+      // else: One-line block comment, brace after new line
+    }
+    // else: Trailing comment, brace after new line
+
+    // Fast-forward current token
+    Loc = TokEndLoc;
+  }
+  return Loc;
+}
+
+} // namespace
+
+void BracesAroundStatementsCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(ifStmt().bind("if"), this);
+  Finder->addMatcher(whileStmt().bind("while"), this);
+  Finder->addMatcher(doStmt().bind("do"), this);
+  Finder->addMatcher(forStmt().bind("for"), this);
+  Finder->addMatcher(forRangeStmt().bind("for-range"), this);
+}
+
+void
+BracesAroundStatementsCheck::check(const MatchFinder::MatchResult &Result) {
+  const SourceManager &SM = *Result.SourceManager;
+  const ASTContext *Context = Result.Context;
+
+  // Get location of closing parenthesis or 'do' to put opening brace
+  if (auto S = Result.Nodes.getNodeAs<ForStmt>("for")) {
+    checkStmt(Result, S->getBody(), S->getRParenLoc());
+  } else if (auto S = Result.Nodes.getNodeAs<CXXForRangeStmt>("for-range")) {
+    checkStmt(Result, S->getBody(), S->getRParenLoc());
+  } else if (auto S = Result.Nodes.getNodeAs<DoStmt>("do")) {
+    checkStmt(Result, S->getBody(), S->getDoLoc(), S->getWhileLoc());
+  } else if (auto S = Result.Nodes.getNodeAs<WhileStmt>("while")) {
+    // FIXME: WhileStmt should store RParenLoc
+    SourceLocation StartLoc = findRParenLoc(S, SM, Context);
+    if (StartLoc.isInvalid()) {
+      return;
+    }
+    checkStmt(Result, S->getBody(), StartLoc);
+  } else if (auto S = Result.Nodes.getNodeAs<IfStmt>("if")) {
+    // FIXME: IfStmt should store RParenLoc
+    SourceLocation StartLoc = findRParenLoc(S, SM, Context);
+    if (StartLoc.isInvalid()) {
+      return;
+    }
+    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());
+    }
+  } else {
+    llvm_unreachable("Invalid match");
+  }
+}
+
+/// Find location of right parenthesis closing condition
+template <typename IfOrWhileStmt>
+SourceLocation
+BracesAroundStatementsCheck::findRParenLoc(const IfOrWhileStmt *S,
+                                           const SourceManager &SM,
+                                           const ASTContext *Context) {
+  // Skip macros
+  if (S->getLocStart().isMacroID()) {
+    return SourceLocation();
+  }
+
+  static const char *const ErrorMessage =
+      "cannot find location of closing parenthesis ')'";
+  SourceLocation CondEndLoc = S->getCond()->getLocEnd();
+  if (const DeclStmt *CondVar = S->getConditionVariableDeclStmt()) {
+    CondEndLoc = CondVar->getLocEnd();
+  }
+  assert(CondEndLoc.isValid());
+  SourceLocation PastCondEndLoc =
+      Lexer::getLocForEndOfToken(CondEndLoc, 0, SM, Context->getLangOpts());
+  if (PastCondEndLoc.isInvalid()) {
+    diag(CondEndLoc, ErrorMessage);
+    return SourceLocation();
+  }
+  SourceLocation RParenLoc =
+      forwardSkipWhitespaceAndComments(PastCondEndLoc, SM, Context);
+  if (RParenLoc.isInvalid()) {
+    diag(PastCondEndLoc, ErrorMessage);
+    return SourceLocation();
+  }
+  tok::TokenKind TokKind = getTokenKind(RParenLoc, SM, Context);
+  if (TokKind != tok::r_paren) {
+    diag(RParenLoc, ErrorMessage);
+    return SourceLocation();
+  }
+  return RParenLoc;
+}
+
+void
+BracesAroundStatementsCheck::checkStmt(const MatchFinder::MatchResult &Result,
+                                       const Stmt *S, 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)) {
+    // Already inside braces
+    return;
+  }
+  // Skip macros
+  if (S->getLocStart().isMacroID()) {
+    return;
+  }
+
+  const SourceManager &SM = *Result.SourceManager;
+  const ASTContext *Context = Result.Context;
+
+  // InitialLoc points at the last token before opening brace to be inserted
+  assert(InitialLoc.isValid());
+  SourceLocation StartLoc =
+      Lexer::getLocForEndOfToken(InitialLoc, 0, SM, Context->getLangOpts());
+  // StartLoc points at the location of the opening brace to be inserted
+  SourceLocation EndLoc;
+  std::string ClosingInsertion = "\n}";
+  if (EndLocHint.isValid()) {
+    EndLoc = EndLocHint;
+    ClosingInsertion = "} ";
+  } else {
+    EndLoc = findEndLocation(S->getLocEnd(), SM, Context);
+  }
+
+  assert(StartLoc.isValid());
+  auto Diag = diag(StartLoc, "statement should be inside braces");
+  // Add braces
+  Diag << FixItHint::CreateInsertion(StartLoc, " {")
+       << FixItHint::CreateInsertion(EndLoc, ClosingInsertion);
+}
+
+} // namespace tidy
+} // namespace clang

clang-tidy/misc/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyMiscModule
   ArgumentCommentCheck.cpp
   BoolPointerImplicitConversion.cpp
+  BracesAroundStatementsCheck.cpp
   FunctionSize.cpp
   MiscTidyModule.cpp
   RedundantSmartptrGet.cpp
   SwappedArgumentsCheck.cpp
   UndelegatedConstructor.cpp
   UnusedRAII.cpp
   UseOverride.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   )

clang-tidy/misc/MiscTidyModule.cpp

 //===--- MiscTidyModule.cpp - clang-tidy ----------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "ArgumentCommentCheck.h"
 #include "BoolPointerImplicitConversion.h"
+#include "BracesAroundStatementsCheck.h"
 #include "FunctionSize.h"
 #include "RedundantSmartptrGet.h"
 #include "SwappedArgumentsCheck.h"
 #include "UndelegatedConstructor.h"
 #include "UnusedRAII.h"
 #include "UseOverride.h"
 
 namespace clang {
 namespace tidy {
 
 class MiscModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<ArgumentCommentCheck>("misc-argument-comment");
     CheckFactories.registerCheck<BoolPointerImplicitConversion>(
         "misc-bool-pointer-implicit-conversion");
+    CheckFactories.registerCheck<BracesAroundStatementsCheck>(
+        "misc-braces-around-statements");
     CheckFactories.registerCheck<FunctionSizeCheck>("misc-function-size");
     CheckFactories.registerCheck<RedundantSmartptrGet>(
         "misc-redundant-smartptr-get");
     CheckFactories.registerCheck<SwappedArgumentsCheck>(
         "misc-swapped-arguments");
     CheckFactories.registerCheck<UndelegatedConstructorCheck>(
         "misc-undelegated-constructor");
     CheckFactories.registerCheck<UnusedRAIICheck>("misc-unused-raii");
     CheckFactories.registerCheck<UseOverride>("misc-use-override");
   }
 };
 
 // Register the MiscTidyModule using this statically initialized variable.
 static ClangTidyModuleRegistry::Add<MiscModule>
     X("misc-module", "Adds miscellaneous lint checks.");
 
 // This anchor is used to force the linker to link in the generated object file
 // and thus register the MiscModule.
 volatile int MiscModuleAnchorSource = 0;
 
 } // namespace tidy
 } // namespace clang

test/clang-tidy/misc-braces-around-statements.cpp

+// RUN: $(dirname %s)/check_clang_tidy_fix.sh %s misc-braces-around-statements %t
+// REQUIRES: shell
+
+void do_something(const char *) {}
+
+bool cond(const char *) {
+  return false;
+}
+
+#define EMPTY_MACRO
+#define EMPTY_MACRO_FUN()
+
+void test() {
+  // CHECK-NOT: warning
+  // if
+  if (cond("if1") /*comment*/)
+    // some comment
+    do_something("if1");
+  // CHECK-MESSAGES: :[[@LINE-3]]:31: warning: statement should be inside braces
+  // CHECK-FIXES: if (cond("if1") /*comment*/) {
+  // CHECK-FIXES: }
+  if (cond("if2")) {
+    do_something("if2");
+  }
+  if (cond("if3"))
+    ;
+  // CHECK-MESSAGES: :[[@LINE-2]]:19: warning: statement should be inside braces
+  // CHECK-FIXES: if (cond("if3")) {
+  // CHECK-FIXES: }
+
+  // if-else
+  if (cond("if-else1"))
+    do_something("if-else1");
+  // CHECK-MESSAGES: :[[@LINE-2]]:24: warning: statement should be inside braces
+  // CHECK-FIXES: if (cond("if-else1")) {
+  // CHECK-FIXES: } else {
+  else
+    do_something("if-else1 else");
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: statement should be inside braces
+  // CHECK-FIXES: }
+  if (cond("if-else2")) {
+    do_something("if-else2");
+  } else {
+    do_something("if-else2 else");
+  }
+
+  // CHECK-NOT: warning
+  // if-else if-else
+  if (cond("if-else if-else1"))
+    do_something("if");
+  // CHECK-MESSAGES: :[[@LINE-2]]:32: warning: statement should be inside braces
+  // CHECK-FIXES: } else if (cond("else if1")) {
+  else if (cond("else if1"))
+    do_something("else if");
+  // CHECK-MESSAGES: :[[@LINE-2]]:29: warning: statement should be inside braces
+  else
+    do_something("else");
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: statement should be inside braces
+  // CHECK-FIXES: }
+  if (cond("if-else if-else2")) {
+    do_something("if");
+  } else if (cond("else if2")) {
+    do_something("else if");
+  } else {
+    do_something("else");
+  }
+
+  // CHECK-NOT: warning
+  for (;;)
+    do_something("for");
+  // CHECK-MESSAGES: :[[@LINE-2]]:11: warning: statement should be inside braces
+  // CHECK-FIXES: for (;;) {
+  // CHECK-FIXES: }
+  for (;;) {
+    do_something("for");
+  }
+  for (;;)
+    ;
+  // CHECK-MESSAGES: :[[@LINE-2]]:11: warning: statement should be inside braces
+  // CHECK-FIXES: for (;;) {
+  // CHECK-FIXES: }
+
+  // CHECK-NOT: warning
+  int arr[4] = {1, 2, 3, 4};
+  for (int a : arr)
+    do_something("for-range");
+  // CHECK-MESSAGES: :[[@LINE-2]]:20: warning: statement should be inside braces
+  // CHECK-FIXES: for (int a : arr) {
+  // CHECK-FIXES: }
+  for (int a : arr) {
+    do_something("for-range");
+  }
+  for (int a : arr)
+    ;
+  // CHECK-MESSAGES: :[[@LINE-2]]:20: warning: statement should be inside braces
+  // CHECK-FIXES: for (int a : arr) {
+  // CHECK-FIXES: }
+
+  // CHECK-NOT: warning
+  while (cond("while1"))
+    do_something("while");
+  // CHECK-MESSAGES: :[[@LINE-2]]:25: warning: statement should be inside braces
+  // CHECK-FIXES: while (cond("while1")) {
+  // CHECK-FIXES: }
+  while (cond("while2")) {
+    do_something("while");
+  }
+  while (false)
+    ;
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: statement should be inside braces
+  // CHECK-FIXES: while (false) {
+  // CHECK-FIXES: }
+
+  // CHECK-NOT: warning
+  do
+    do_something("do1");
+  while (cond("do1"));
+  // CHECK-MESSAGES: :[[@LINE-3]]:5: warning: statement should be inside braces
+  // CHECK-FIXES: do {
+  // CHECK-FIXES: } while (cond("do1"));
+  do {
+    do_something("do2");
+  } while (cond("do2"));
+
+  do
+    ;
+  while (false);
+  // CHECK-MESSAGES: :[[@LINE-3]]:5: warning: statement should be inside braces
+  // CHECK-FIXES: do {
+  // CHECK-FIXES: } while (false);
+
+  // CHECK-NOT: warning
+  if (cond("ifif1"))
+    // comment
+    if (cond("ifif2"))
+      // comment
+      /*comment*/ ; // comment
+  // CHECK-MESSAGES: :[[@LINE-5]]:21: warning: statement should be inside braces
+  // CHECK-MESSAGES: :[[@LINE-4]]:23: warning: statement should be inside braces
+  // CHECK-FIXES: if (cond("ifif1")) {
+  // CHECK-FIXES: if (cond("ifif2")) {
+  // CHECK-FIXES: }
+  // CHECK-FIXES-NEXT: }
+
+  if (cond("ifif3"))
+    // comment
+    if (cond("ifif4")) {
+      // comment
+      /*comment*/; // comment
+    }
+  // CHECK-MESSAGES: :[[@LINE-6]]:21: warning: statement should be inside braces
+  // CHECK-FIXES: if (cond("ifif3")) {
+  // CHECK-FIXES: }
+
+  if (cond("ifif5"))
+    ; /* multi-line
+        comment */
+  // CHECK-MESSAGES: :[[@LINE-3]]:21: warning: statement should be inside braces
+  // CHECK-FIXES: if (cond("ifif5")) {
+  // CHECK-FIXES: }/* multi-line
+
+  // CHECK-NOT: warning
+  if (1) while (2) if (3) for (;;) do ; while(false) /**/;/**/
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: statement should be inside braces
+  // CHECK-MESSAGES: :[[@LINE-2]]:19: warning: statement should be inside braces
+  // CHECK-MESSAGES: :[[@LINE-3]]:26: warning: statement should be inside braces
+  // CHECK-MESSAGES: :[[@LINE-4]]:35: warning: statement should be inside braces
+  // CHECK-MESSAGES: :[[@LINE-5]]:38: warning: statement should be inside braces
+  // CHECK-FIXES: if (1) { while (2) { if (3) { for (;;) { do { ; } while(false) /**/;/**/
+  // CHECK-FIXES-NEXT: }
+  // CHECK-FIXES-NEXT: }
+  // CHECK-FIXES-NEXT: }
+  // CHECK-FIXES-NEXT: }
+
+  // CHECK-NOT: warning
+
+  // CHECK-MESSAGES: {{clang-tidy applied [0-9]+ of [0-9]+ suggested fixes.}}
+}

unittests/clang-tidy/MiscModuleTest.cpp

 #include "ClangTidyTest.h"
 #include "misc/ArgumentCommentCheck.h"
+#include "misc/BracesAroundStatementsCheck.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace tidy {
 namespace test {
 
 #define EXPECT_NO_CHANGES(Check, Code)                                         \
   EXPECT_EQ(Code, runCheckOnCode<Check>(Code))
EXPECT_EQ("struct C { C(int xxx, int yyy); }; C c(/*xxx=*/0, 0);",
                 "struct C { C(int xxx, int yyy); }; C c(/*Xxx=*/0, 0);"));
 }
 
 TEST(ArgumentCommentCheckTest, OtherEditDistanceBelowThreshold) {
   EXPECT_NO_CHANGES(ArgumentCommentCheck,
                     "void f(int xxx, int yyy); void g() { f(/*xxy=*/0, 0); }");
 }
 
+TEST(BracesAroundStatementsCheck, IfWithComments) {
+  // if only
+  EXPECT_EQ("int main() {\n"
+            "  if (false /*dummy token*/) {\n"
+            "    // comment\n"
+            "    return -1; /**/\n"
+            "}\n"
+            "  if (false) {\n"
+            "    return -1; // comment\n"
+            "}\n"
+            "  if (false) {\n"
+            "    return -1; \n"
+            "}/* multi-line \n comment */\n"
+            "}",
+            runCheckOnCode<BracesAroundStatementsCheck>(
+                "int main() {\n"
+                "  if (false /*dummy token*/)\n"
+                "    // comment\n"
+                "    return -1; /**/\n"
+                "  if (false)\n"
+                "    return -1; // comment\n"
+                "  if (false)\n"
+                "    return -1; /* multi-line \n comment */\n"
+                "}"));
+  EXPECT_EQ("int main() {\n"
+            "  if (false /*dummy token*/) {\n"
+            "    // comment\n"
+            "    return -1 /**/ ;\n"
+            "}\n"
+            "}",
+            runCheckOnCode<BracesAroundStatementsCheck>(
+                "int main() {\n"
+                "  if (false /*dummy token*/)\n"
+                "    // comment\n"
+                "    return -1 /**/ ;\n"
+                "}"));
+}
+
+TEST(BracesAroundStatementsCheck, If) {
+  // if only
+  EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n"
+                                                 "  if (false) {\n"
+                                                 "    return -1;\n"
+                                                 "  }\n"
+                                                 "}");
+  EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n"
+                                                 "  if (auto Cond = false) {\n"
+                                                 "    return -1;\n"
+                                                 "  }\n"
+                                                 "}");
+  // if else
+  EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n"
+                                                 "  if (false) {\n"
+                                                 "    return -1;\n"
+                                                 "  } else {\n"
+                                                 "    return -2;\n"
+                                                 "  }\n"
+                                                 "}");
+  // if only
+  EXPECT_EQ("int main() {\n"
+            "  if (false) {\n"
+            "    return -1;\n"
+            "}\n"
+            "}",
+            runCheckOnCode<BracesAroundStatementsCheck>("int main() {\n"
+                                                        "  if (false)\n"
+                                                        "    return -1;\n"
+                                                        "}"));
+  EXPECT_EQ("int main() {\n"
+            "  if (auto Cond = false /**/ ) {\n"
+            "    return -1;\n"
+            "}\n"
+            "}",
+            runCheckOnCode<BracesAroundStatementsCheck>(
+                "int main() {\n"
+                "  if (auto Cond = false /**/ )\n"
+                "    return -1;\n"
+                "}"));
+//EXPECT_EQ("#define EMPTY_MACRO\n"
+//          "int main() {\n"
+//          "  if (auto Cond = false EMPTY_MACRO /**/ ) { EMPTY_MACRO\n"
+//          "    return -1;\n"
+//          "}\n"
+//          "}",
+//          runCheckOnCode<BracesAroundStatementsCheck>(
+//              "#define EMPTY_MACRO\n"
+//              "int main() {\n"
+//              "  if (auto Cond = false EMPTY_MACRO /**/ ) EMPTY_MACRO\n"
+//              "    return -1;\n"
+//              "}"));
+  EXPECT_EQ("int main() {\n"
+            "  if (true) { return -1/**/ ;\n"
+            "}\n"
+            "}",
+            runCheckOnCode<BracesAroundStatementsCheck>(
+                "int main() {\n"
+                "  if (true) return -1/**/ ;\n"
+                "}"));
+  // if else
+  EXPECT_EQ("int main() {\n"
+            "  if (false) {\n"
+            "    return -1;\n"
+            "  } else {\n"
+            "    return -2;\n"
+            "}\n"
+            "}",
+            runCheckOnCode<BracesAroundStatementsCheck>("int main() {\n"
+                                                        "  if (false)\n"
+                                                        "    return -1;\n"
+                                                        "  else\n"
+                                                        "    return -2;\n"
+                                                        "}"));
+  // if else if else
+  EXPECT_EQ("int main() {\n"
+            "  if (false) {\n"
+            "    return -1;\n"
+            "  } else if (1 == 2) {\n"
+            "    return -2;\n"
+            "  } else {\n"
+            "    return -3;\n"
+            "}\n"
+            "}",
+            runCheckOnCode<BracesAroundStatementsCheck>("int main() {\n"
+                                                        "  if (false)\n"
+                                                        "    return -1;\n"
+                                                        "  else if (1 == 2)\n"
+                                                        "    return -2;\n"
+                                                        "  else\n"
+                                                        "    return -3;\n"
+                                                        "}"));
+  // if else if {} else
+  EXPECT_EQ("int main() {\n"
+            "  if (false) {\n"
+            "    return -1;\n"
+            "  } else if (1 == 2) {\n"
+            "    return -2;\n"
+            "  } else {\n"
+            "    return -3;\n"
+            "}\n"
+            "}",
+            runCheckOnCode<BracesAroundStatementsCheck>("int main() {\n"
+                                                        "  if (false)\n"
+                                                        "    return -1;\n"
+                                                        "  else if (1 == 2) {\n"
+                                                        "    return -2;\n"
+                                                        "  } else\n"
+                                                        "    return -3;\n"
+                                                        "}"));
+}
+
+TEST(BracesAroundStatementsCheck, For) {
+  EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n"
+                                                 "  for (;;) {\n"
+                                                 "    ;\n"
+                                                 "  }\n"
+                                                 "}");
+  EXPECT_EQ("int main() {\n"
+            "  for (;;) {\n"
+            "    ;\n"
+            "}\n"
+            "}",
+            runCheckOnCode<BracesAroundStatementsCheck>("int main() {\n"
+                                                        "  for (;;)\n"
+                                                        "    ;\n"
+                                                        "}"));
+  EXPECT_EQ("int main() {\n"
+            "  for (;;) {\n"
+            "    /**/ ;\n"
+            "}\n"
+            "}",
+            runCheckOnCode<BracesAroundStatementsCheck>("int main() {\n"
+                                                        "  for (;;)\n"
+                                                        "    /**/ ;\n"
+                                                        "}"));
+  EXPECT_EQ("int main() {\n"
+            "  for (;;) {\n"
+            "    return -1 /**/ ;\n"
+            "}\n"
+            "}",
+            runCheckOnCode<BracesAroundStatementsCheck>("int main() {\n"
+                                                        "  for (;;)\n"
+                                                        "    return -1 /**/ ;\n"
+                                                        "}"));
+}
+
+TEST(BracesAroundStatementsCheck, ForRange) {
+  EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n"
+                                                 "  int arr[4];\n"
+                                                 "  for (int i : arr) {\n"
+                                                 "    ;\n"
+                                                 "  }\n"
+                                                 "}");
+  EXPECT_EQ("int main() {\n"
+            "  int arr[4];\n"
+            "  for (int i : arr) {\n"
+            "    ;\n"
+            "}\n"
+            "  for (int i : arr) {\n"
+            "    return -1 ;\n"
+            "}\n"
+            "}",
+            runCheckOnCode<BracesAroundStatementsCheck>("int main() {\n"
+                                                        "  int arr[4];\n"
+                                                        "  for (int i : arr)\n"
+                                                        "    ;\n"
+                                                        "  for (int i : arr)\n"
+                                                        "    return -1 ;\n"
+                                                        "}"));
+}
+
+TEST(BracesAroundStatementsCheck, DoWhile) {
+  EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n"
+                                                 "  do {\n"
+                                                 "    ;\n"
+                                                 "  } while (false);\n"
+                                                 "}");
+  EXPECT_EQ("int main() {\n"
+            "  do {\n"
+            "    ;\n"
+            "  } while (false);\n"
+            "}",
+            runCheckOnCode<BracesAroundStatementsCheck>("int main() {\n"
+                                                        "  do\n"
+                                                        "    ;\n"
+                                                        "  while (false);\n"
+                                                        "}"));
+}
+
+TEST(BracesAroundStatementsCheck, While) {
+  EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n"
+                                                 "  while (false) {\n"
+                                                 "    ;\n"
+                                                 "  }\n"
+                                                 "}");
+  EXPECT_EQ("int main() {\n"
+            "  while (false) {\n"
+            "    ;\n"
+            "}\n"
+            "}",
+            runCheckOnCode<BracesAroundStatementsCheck>("int main() {\n"
+                                                        "  while (false)\n"
+                                                        "    ;\n"
+                                                        "}"));
+  EXPECT_EQ("int main() {\n"
+            "  while (auto Cond = false) {\n"
+            "    ;\n"
+            "}\n"
+            "}",
+            runCheckOnCode<BracesAroundStatementsCheck>(
+                "int main() {\n"
+                "  while (auto Cond = false)\n"
+                "    ;\n"
+                "}"));
+  EXPECT_EQ("int main() {\n"
+            "  while (false /*dummy token*/) {\n"
+            "    ;\n"
+            "}\n"
+            "}",
+            runCheckOnCode<BracesAroundStatementsCheck>(
+                "int main() {\n"
+                "  while (false /*dummy token*/)\n"
+                "    ;\n"
+                "}"));
+  EXPECT_EQ("int main() {\n"
+            "  while (false) {\n"
+            "    break;\n"
+            "}\n"
+            "}",
+            runCheckOnCode<BracesAroundStatementsCheck>("int main() {\n"
+                                                        "  while (false)\n"
+                                                        "    break;\n"
+                                                        "}"));
+  EXPECT_EQ("int main() {\n"
+            "  while (false) {\n"
+            "    break /**/;\n"
+            "}\n"
+            "}",
+            runCheckOnCode<BracesAroundStatementsCheck>("int main() {\n"
+                                                        "  while (false)\n"
+                                                        "    break /**/;\n"
+                                                        "}"));
+  EXPECT_EQ("int main() {\n"
+            "  while (false) {\n"
+            "    /**/;\n"
+            "}\n"
+            "}",
+            runCheckOnCode<BracesAroundStatementsCheck>("int main() {\n"
+                                                        "  while (false)\n"
+                                                        "    /**/;\n"
+                                                        "}"));
+}
+
+TEST(BracesAroundStatementsCheck, Nested) {
+  EXPECT_EQ("int main() {\n"
+            "  do { if (true) {}} while (false);\n"
+            "}",
+            runCheckOnCode<BracesAroundStatementsCheck>(
+                "int main() {\n"
+                "  do if (true) {}while (false);\n"
+                "}"));
+  EXPECT_EQ("int main() {\n"
+            "  do { if (true) {}} while (false);\n"
+            "}",
+            runCheckOnCode<BracesAroundStatementsCheck>(
+                "int main() {\n"
+                "  do if (true) {}while (false);\n"
+                "}"));
+  EXPECT_EQ(
+      "int main() {\n"
+      "  if (true) {\n"
+      "    // comment\n"
+      "    if (false) {\n"
+      "      // comment\n"
+      "      /**/ ; // comment\n"
+      "    }\n"
+      "}\n"
+      "}",
+      runCheckOnCode<BracesAroundStatementsCheck>("int main() {\n"
+                                                  "  if (true)\n"
+                                                  "    // comment\n"
+                                                  "    if (false) {\n"
+                                                  "      // comment\n"
+                                                  "      /**/ ; // comment\n"
+                                                  "    }\n"
+                                                  "}"));
+}
+
+TEST(BracesAroundStatementsCheck, Macros) {
+  EXPECT_NO_CHANGES(BracesAroundStatementsCheck,
+                    "#define IF(COND) if (COND) return -1;\n"
+                    "int main() {\n"
+                    "  IF(false)\n"
+                    "}");
+  EXPECT_NO_CHANGES(BracesAroundStatementsCheck,
+                    "#define FOR(COND) for (COND) return -1;\n"
+                    "int main() {\n"
+                    "  FOR(;;)\n"
+                    "}");
+}
+
 } // namespace test
 } // namespace tidy
 } // namespace clang