[clang-tidy] Fix bug 22785 - readability-braces-around-statements check doesn't work well with macros.

[curdeius]

Summary

Fix http://llvm.org/PR22785. Bug 22785 - readability-braces-around-statements doesn't work well with macros.

[alexfh]

LG. Committed revision 247163.

clang-tidy/readability/BracesAroundStatementsCheck.cpp

 }
 
 /// Find location of right parenthesis closing condition
 template <typename IfOrWhileStmt>
 SourceLocation
 BracesAroundStatementsCheck::findRParenLoc(const IfOrWhileStmt *S,
                                            const SourceManager &SM,
                                            const ASTContext *Context) {
-  // Skip macros
+  // 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();
bool BracesAroundStatementsCheck::checkStmt(
   // 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 false;
   }
-  // Skip macros.
-  if (S->getLocStart().isMacroID())
-    return false;
 
   const SourceManager &SM = *Result.SourceManager;
   const ASTContext *Context = Result.Context;
 
+  // Treat macros.
+  CharSourceRange FileRange = Lexer::makeFileCharRange(
+      CharSourceRange::getTokenRange(S->getSourceRange()), SM,
+      Context->getLangOpts());
+  if (FileRange.isInvalid())
+    return false;
+
   // 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;
   if (EndLocHint.isValid()) {
     EndLoc = EndLocHint;
     ClosingInsertion = "} ";
   } else {
-    EndLoc = findEndLocation(S->getLocEnd(), SM, Context);
+    const auto FREnd = FileRange.getEnd().getLocWithOffset(-1);
+    EndLoc = findEndLocation(FREnd, 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 && !ForceBracesStmts.erase(S)) {

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

void test() {
   // 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: }
 }
+
+#define M(x) x
+
+int test_macros(bool b) {
+  if (b) {
+    return 1;
+  } else
+    M(return 2);
+  // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: statement should be inside braces
+  // CHECK-FIXES: } else {
+  // CHECK-FIXES-NEXT:   M(return 2);
+  // CHECK-FIXES-NEXT: }
+}

unittests/clang-tidy/ReadabilityModuleTest.cpp

EXPECT_NO_CHANGES(BracesAroundStatementsCheck,
                     "int main() {\n"
                     "  IF(false)\n"
                     "}");
   EXPECT_NO_CHANGES(BracesAroundStatementsCheck,
                     "#define FOR(COND) for (COND) return -1;\n"
                     "int main() {\n"
                     "  FOR(;;)\n"
                     "}");
+  EXPECT_EQ("#define DO_IT ++i\n"
+            "int i = 0;\n"
+            "int main() {\n"
+            "  if (false) {\n"
+            "    DO_IT;\n"
+            "  } else if (1 == 2) {\n"
+            "    DO_IT;\n"
+            "  } else {\n"
+            "    DO_IT;\n"
+            "}\n"
+            "}",
+            runCheckOnCode<BracesAroundStatementsCheck>("#define DO_IT ++i\n"
+                                                        "int i = 0;\n"
+                                                        "int main() {\n"
+                                                        "  if (false)\n"
+                                                        "    DO_IT;\n"
+                                                        "  else if (1 == 2)\n"
+                                                        "    DO_IT;\n"
+                                                        "  else\n"
+                                                        "    DO_IT;\n"
+                                                        "}"));
 }
 
 } // namespace test
 } // namespace tidy
 } // namespace clang