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

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.

Reviewers: alexfh, hokein

Subscribers: hokein

Tags: #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D25558

llvm-svn: 284212
This commit is contained in:
Marek Kurdej 2016-10-14 08:10:08 +00:00
parent 174d2e784b
commit 505434bd28
3 changed files with 33 additions and 18 deletions

View File

@ -8,7 +8,8 @@
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
#include "UseUsingCheck.h" #include "UseUsingCheck.h"
#include "../utils/LexerUtils.h" #include "clang/AST/ASTContext.h"
#include "clang/Lex/Lexer.h"
using namespace clang::ast_matchers; using namespace clang::ast_matchers;

View File

@ -61,7 +61,7 @@ SourceLocation findEndLocation(SourceLocation LastTokenLoc,
bool SkipEndWhitespaceAndComments = true; bool SkipEndWhitespaceAndComments = true;
tok::TokenKind TokKind = getTokenKind(Loc, SM, Context); tok::TokenKind TokKind = getTokenKind(Loc, SM, Context);
if (TokKind == tok::NUM_TOKENS || TokKind == tok::semi || 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 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. // `if (isa<NullStmt>(S))`, but it wouldn't work for nested statements.
SkipEndWhitespaceAndComments = false; SkipEndWhitespaceAndComments = false;

View File

@ -15,9 +15,9 @@ TEST(NamespaceCommentCheckTest, Basic) {
runCheckOnCode<NamespaceCommentCheck>("namespace i {\n}")); runCheckOnCode<NamespaceCommentCheck>("namespace i {\n}"));
EXPECT_EQ("namespace {\n} // namespace", EXPECT_EQ("namespace {\n} // namespace",
runCheckOnCode<NamespaceCommentCheck>("namespace {\n}")); runCheckOnCode<NamespaceCommentCheck>("namespace {\n}"));
EXPECT_EQ( EXPECT_EQ("namespace i { namespace j {\n} // namespace j\n } // namespace i",
"namespace i { namespace j {\n} // namespace j\n } // namespace i", runCheckOnCode<NamespaceCommentCheck>(
runCheckOnCode<NamespaceCommentCheck>("namespace i { namespace j {\n} }")); "namespace i { namespace j {\n} }"));
} }
TEST(NamespaceCommentCheckTest, SingleLineNamespaces) { TEST(NamespaceCommentCheckTest, SingleLineNamespaces) {
@ -49,7 +49,8 @@ TEST(NamespaceCommentCheckTest, CheckExistingComments) {
"} // Anonymous namespace.", "} // Anonymous namespace.",
runCheckOnCode<NamespaceCommentCheck>("namespace {\n" runCheckOnCode<NamespaceCommentCheck>("namespace {\n"
"} // Anonymous namespace.")); "} // Anonymous namespace."));
EXPECT_EQ("namespace q {\n" EXPECT_EQ(
"namespace q {\n"
"} // namespace q", "} // namespace q",
runCheckOnCode<NamespaceCommentCheck>("namespace q {\n" runCheckOnCode<NamespaceCommentCheck>("namespace q {\n"
"} // anonymous namespace q")); "} // anonymous namespace q"));
@ -97,7 +98,7 @@ TEST(NamespaceCommentCheckTest, FixWrongComments) {
"} // random text")); "} // random text"));
} }
TEST(BracesAroundStatementsCheck, IfWithComments) { TEST(BracesAroundStatementsCheckTest, IfWithComments) {
EXPECT_EQ("int main() {\n" EXPECT_EQ("int main() {\n"
" if (false /*dummy token*/) {\n" " if (false /*dummy token*/) {\n"
" // comment\n" " // comment\n"
@ -134,7 +135,7 @@ TEST(BracesAroundStatementsCheck, IfWithComments) {
"}")); "}"));
} }
TEST(BracesAroundStatementsCheck, If) { TEST(BracesAroundStatementsCheckTest, If) {
EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n" EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n"
" if (false) {\n" " if (false) {\n"
" return -1;\n" " return -1;\n"
@ -235,7 +236,7 @@ TEST(BracesAroundStatementsCheck, If) {
"}")); "}"));
} }
TEST(BracesAroundStatementsCheck, IfElseWithShortStatements) { TEST(BracesAroundStatementsCheckTest, IfElseWithShortStatements) {
ClangTidyOptions Options; ClangTidyOptions Options;
Options.CheckOptions["test-check-0.ShortStatementLines"] = "1"; Options.CheckOptions["test-check-0.ShortStatementLines"] = "1";
@ -269,7 +270,7 @@ TEST(BracesAroundStatementsCheck, IfElseWithShortStatements) {
nullptr, "input.cc", None, Options)); nullptr, "input.cc", None, Options));
} }
TEST(BracesAroundStatementsCheck, For) { TEST(BracesAroundStatementsCheckTest, For) {
EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n" EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n"
" for (;;) {\n" " for (;;) {\n"
" ;\n" " ;\n"
@ -304,7 +305,7 @@ TEST(BracesAroundStatementsCheck, For) {
"}")); "}"));
} }
TEST(BracesAroundStatementsCheck, ForRange) { TEST(BracesAroundStatementsCheckTest, ForRange) {
EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n" EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n"
" int arr[4];\n" " int arr[4];\n"
" for (int i : arr) {\n" " for (int i : arr) {\n"
@ -329,7 +330,7 @@ TEST(BracesAroundStatementsCheck, ForRange) {
"}")); "}"));
} }
TEST(BracesAroundStatementsCheck, DoWhile) { TEST(BracesAroundStatementsCheckTest, DoWhile) {
EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n" EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n"
" do {\n" " do {\n"
" ;\n" " ;\n"
@ -347,7 +348,7 @@ TEST(BracesAroundStatementsCheck, DoWhile) {
"}")); "}"));
} }
TEST(BracesAroundStatementsCheck, While) { TEST(BracesAroundStatementsCheckTest, While) {
EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n" EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n"
" while (false) {\n" " while (false) {\n"
" ;\n" " ;\n"
@ -411,7 +412,7 @@ TEST(BracesAroundStatementsCheck, While) {
"}")); "}"));
} }
TEST(BracesAroundStatementsCheck, Nested) { TEST(BracesAroundStatementsCheckTest, Nested) {
EXPECT_EQ("int main() {\n" EXPECT_EQ("int main() {\n"
" do { if (true) {}} while (false);\n" " do { if (true) {}} while (false);\n"
"}", "}",
@ -446,7 +447,7 @@ TEST(BracesAroundStatementsCheck, Nested) {
"}")); "}"));
} }
TEST(BracesAroundStatementsCheck, Macros) { TEST(BracesAroundStatementsCheckTest, Macros) {
EXPECT_NO_CHANGES(BracesAroundStatementsCheck, EXPECT_NO_CHANGES(BracesAroundStatementsCheck,
"#define IF(COND) if (COND) return -1;\n" "#define IF(COND) if (COND) return -1;\n"
"int main() {\n" "int main() {\n"
@ -480,6 +481,19 @@ TEST(BracesAroundStatementsCheck, Macros) {
"}")); "}"));
} }
#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 test
} // namespace tidy } // namespace tidy
} // namespace clang } // namespace clang