[clang-tidy] Fix readability-braces-around-statements false positive

[curdeius]

Summary

This fixes a false-positive e.g. when string literals are returned from if statement.

This patch includes as well a small fix to includes and renames of the test suite that collided with the name of the check.

[hokein]

Thanks, LGTM with one nit.

clang-tidy/ReadabilityModuleTest.cpp

488

Extra blank line.

[curdeius]

Remove blank line.

[alexfh]

clang-tidy/modernize/UseUsingCheck.cpp

11–12 ↗

(On Diff #74624)

The change is fine, but it would be better to commit it separately (obviously, no need to do anything now, just for the future).

clang-tidy/readability/BracesAroundStatementsCheck.cpp

64 ↗

(On Diff #74624)

I'm thoroughly confused as to why the code in your test was not handled correctly and why this is the right fix. Can you explain?

clang-tidy/ReadabilityModuleTest.cpp

 using readability::BracesAroundStatementsCheck;
 using readability::NamespaceCommentCheck;
 
 TEST(NamespaceCommentCheckTest, Basic) {
   EXPECT_EQ("namespace i {\n} // namespace i",
             runCheckOnCode<NamespaceCommentCheck>("namespace i {\n}"));
   EXPECT_EQ("namespace {\n} // namespace",
             runCheckOnCode<NamespaceCommentCheck>("namespace {\n}"));
-  EXPECT_EQ(
-      "namespace i { namespace j {\n} // namespace j\n } // namespace i",
-      runCheckOnCode<NamespaceCommentCheck>("namespace i { namespace j {\n} }"));
+  EXPECT_EQ("namespace i { namespace j {\n} // namespace j\n } // namespace i",
+            runCheckOnCode<NamespaceCommentCheck>(
+                "namespace i { namespace j {\n} }"));
 }
 
 TEST(NamespaceCommentCheckTest, SingleLineNamespaces) {
   EXPECT_EQ(
       "namespace i { namespace j { } }",
       runCheckOnCode<NamespaceCommentCheck>("namespace i { namespace j { } }"));
 }
 
TEST(NamespaceCommentCheckTest, CheckExistingComments) {
   EXPECT_EQ("namespace {\n"
             "} // anonymous namespace",
             runCheckOnCode<NamespaceCommentCheck>("namespace {\n"
                                                   "} // anonymous namespace"));
   EXPECT_EQ("namespace {\n"
             "} // Anonymous namespace.",
             runCheckOnCode<NamespaceCommentCheck>("namespace {\n"
                                                   "} // Anonymous namespace."));
-  EXPECT_EQ("namespace q {\n"
-            "} // namespace q",
-            runCheckOnCode<NamespaceCommentCheck>("namespace q {\n"
-                                                  "} // anonymous namespace q"));
+  EXPECT_EQ(
+      "namespace q {\n"
   EXPECT_EQ(
       "namespace My_NameSpace123 {\n"
       "} // namespace My_NameSpace123",
       runCheckOnCode<NamespaceCommentCheck>("namespace My_NameSpace123 {\n"
                                             "} // namespace My_NameSpace123"));
   EXPECT_EQ(
       "namespace My_NameSpace123 {\n"
       "} //namespace My_NameSpace123",
TEST(NamespaceCommentCheckTest, FixWrongComments) {
   // Remove unknown line comments. These are likely to be an unrecognized form
   // of a namespace ending comment.
   EXPECT_EQ("namespace {\n"
             "} // namespace",
             runCheckOnCode<NamespaceCommentCheck>("namespace {\n"
                                                   "} // random text"));
 }
 
-TEST(BracesAroundStatementsCheck, IfWithComments) {
+TEST(BracesAroundStatementsCheckTest, IfWithComments) {
   EXPECT_EQ("int main() {\n"
             "  if (false /*dummy token*/) {\n"
             "    // comment\n"
             "    return -1; /**/\n"
             "}\n"
             "  if (false) {\n"
             "    return -1; // comment\n"
             "}\n"
EXPECT_EQ("int main() {\n"
             runCheckOnCode<BracesAroundStatementsCheck>(
                 "int main() {\n"
                 "  if (false /*dummy token*/)\n"
                 "    // comment\n"
                 "    return -1 /**/ ;\n"
                 "}"));
 }
 
-TEST(BracesAroundStatementsCheck, If) {
+TEST(BracesAroundStatementsCheckTest, If) {
   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"
EXPECT_EQ("int main() {\n"
                                                         "    return -1;\n"
                                                         "  else if (1 == 2) {\n"
                                                         "    return -2;\n"
                                                         "  } else\n"
                                                         "    return -3;\n"
                                                         "}"));
 }
 
-TEST(BracesAroundStatementsCheck, IfElseWithShortStatements) {
+TEST(BracesAroundStatementsCheckTest, IfElseWithShortStatements) {
   ClangTidyOptions Options;
   Options.CheckOptions["test-check-0.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"
EXPECT_EQ("int main() {\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) {
+TEST(BracesAroundStatementsCheckTest, For) {
   EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n"
                                                  "  for (;;) {\n"
                                                  "    ;\n"
                                                  "  }\n"
                                                  "}");
   EXPECT_EQ("int main() {\n"
             "  for (;;) {\n"
             "    ;\n"
EXPECT_EQ("int main() {\n"
             "}\n"
             "}",
             runCheckOnCode<BracesAroundStatementsCheck>("int main() {\n"
                                                         "  for (;;)\n"
                                                         "    return -1 /**/ ;\n"
                                                         "}"));
 }
 
-TEST(BracesAroundStatementsCheck, ForRange) {
+TEST(BracesAroundStatementsCheckTest, 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) {
+TEST(BracesAroundStatementsCheckTest, 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) {
+TEST(BracesAroundStatementsCheckTest, While) {
   EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n"
                                                  "  while (false) {\n"
                                                  "    ;\n"
                                                  "  }\n"
                                                  "}");
   EXPECT_EQ("int main() {\n"
             "  while (false) {\n"
             "    ;\n"
EXPECT_EQ("int main() {\n"
             "}\n"
             "}",
             runCheckOnCode<BracesAroundStatementsCheck>("int main() {\n"
                                                         "  while (false)\n"
                                                         "    /**/;\n"
                                                         "}"));
 }
 
-TEST(BracesAroundStatementsCheck, Nested) {
+TEST(BracesAroundStatementsCheckTest, 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"
EXPECT_EQ(
                                                   "    // comment\n"
                                                   "    if (false) {\n"
                                                   "      // comment\n"
                                                   "      /**/ ; // comment\n"
                                                   "    }\n"
                                                   "}"));
 }
 
-TEST(BracesAroundStatementsCheck, Macros) {
+TEST(BracesAroundStatementsCheckTest, 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"
EXPECT_EQ("#define DO_IT ++i\n"
                                                         "    DO_IT;\n"
                                                         "  else if (1 == 2)\n"
                                                         "    DO_IT;\n"
                                                         "  else\n"
                                                         "    DO_IT;\n"
                                                         "}"));
 }
 
+#define EXPECT_NO_CHANGES_WITH_OPTS(Check, Opts, Code)                         \
+  EXPECT_EQ(Code, runCheckOnCode<Check>(Code, nullptr, "input.cc", None, Opts))
+
+TEST(BracesAroundStatementsCheckTest, ImplicitCastInReturn) {
+
+  ClangTidyOptions Opts;
+  Opts.CheckOptions["test-check-0.ShortStatementLines"] = "1";
+
+  EXPECT_NO_CHANGES_WITH_OPTS(BracesAroundStatementsCheck, Opts,
+                              "const char *f() {\n"
+                              "  if (true) return \"\";\n"
+                              "  return \"abc\";\n"
+                              "}\n");
+}
+
 } // namespace test
 } // namespace tidy
 } // namespace clang

modernize/UseUsingCheck.cpp

 //===--- UseUsingCheck.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 "UseUsingCheck.h"
-#include "../utils/LexerUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/Lex/Lexer.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace modernize {
 
 void UseUsingCheck::registerMatchers(MatchFinder *Finder) {

readability/BracesAroundStatementsCheck.cpp

SourceLocation findEndLocation(SourceLocation LastTokenLoc,
                                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) {
+      TokKind == tok::r_brace || isStringLiteral(TokKind)) {
     // 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 ';'.