From 505434bd28625fcfbacf32798381d75e4acbdc6c Mon Sep 17 00:00:00 2001 From: Marek Kurdej Date: Fri, 14 Oct 2016 08:10:08 +0000 Subject: [PATCH] [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 --- .../clang-tidy/modernize/UseUsingCheck.cpp | 3 +- .../BracesAroundStatementsCheck.cpp | 2 +- .../clang-tidy/ReadabilityModuleTest.cpp | 46 ++++++++++++------- 3 files changed, 33 insertions(+), 18 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp index 498a95c231b6..fdeda99a84ea 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp @@ -8,7 +8,8 @@ //===----------------------------------------------------------------------===// #include "UseUsingCheck.h" -#include "../utils/LexerUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/Lex/Lexer.h" using namespace clang::ast_matchers; diff --git a/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp b/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp index d65f60ba5bd0..5186c0b1f616 100644 --- a/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp @@ -61,7 +61,7 @@ SourceLocation findEndLocation(SourceLocation LastTokenLoc, 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(S))`, but it wouldn't work for nested statements. SkipEndWhitespaceAndComments = false; diff --git a/clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp b/clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp index ba504a733d5b..e79d02464dbf 100644 --- a/clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp +++ b/clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp @@ -15,9 +15,9 @@ TEST(NamespaceCommentCheckTest, Basic) { runCheckOnCode("namespace i {\n}")); EXPECT_EQ("namespace {\n} // namespace", runCheckOnCode("namespace {\n}")); - EXPECT_EQ( - "namespace i { namespace j {\n} // namespace j\n } // namespace i", - runCheckOnCode("namespace i { namespace j {\n} }")); + EXPECT_EQ("namespace i { namespace j {\n} // namespace j\n } // namespace i", + runCheckOnCode( + "namespace i { namespace j {\n} }")); } TEST(NamespaceCommentCheckTest, SingleLineNamespaces) { @@ -49,10 +49,11 @@ TEST(NamespaceCommentCheckTest, CheckExistingComments) { "} // Anonymous namespace.", runCheckOnCode("namespace {\n" "} // Anonymous namespace.")); - EXPECT_EQ("namespace q {\n" - "} // namespace q", - runCheckOnCode("namespace q {\n" - "} // anonymous namespace q")); + EXPECT_EQ( + "namespace q {\n" + "} // namespace q", + runCheckOnCode("namespace q {\n" + "} // anonymous namespace q")); EXPECT_EQ( "namespace My_NameSpace123 {\n" "} // namespace My_NameSpace123", @@ -97,7 +98,7 @@ TEST(NamespaceCommentCheckTest, FixWrongComments) { "} // random text")); } -TEST(BracesAroundStatementsCheck, IfWithComments) { +TEST(BracesAroundStatementsCheckTest, IfWithComments) { EXPECT_EQ("int main() {\n" " if (false /*dummy token*/) {\n" " // comment\n" @@ -134,7 +135,7 @@ TEST(BracesAroundStatementsCheck, IfWithComments) { "}")); } -TEST(BracesAroundStatementsCheck, If) { +TEST(BracesAroundStatementsCheckTest, If) { EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n" " if (false) {\n" " return -1;\n" @@ -235,7 +236,7 @@ TEST(BracesAroundStatementsCheck, If) { "}")); } -TEST(BracesAroundStatementsCheck, IfElseWithShortStatements) { +TEST(BracesAroundStatementsCheckTest, IfElseWithShortStatements) { ClangTidyOptions Options; Options.CheckOptions["test-check-0.ShortStatementLines"] = "1"; @@ -269,7 +270,7 @@ TEST(BracesAroundStatementsCheck, IfElseWithShortStatements) { nullptr, "input.cc", None, Options)); } -TEST(BracesAroundStatementsCheck, For) { +TEST(BracesAroundStatementsCheckTest, For) { EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n" " for (;;) {\n" " ;\n" @@ -304,7 +305,7 @@ TEST(BracesAroundStatementsCheck, For) { "}")); } -TEST(BracesAroundStatementsCheck, ForRange) { +TEST(BracesAroundStatementsCheckTest, ForRange) { EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n" " int arr[4];\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" " do {\n" " ;\n" @@ -347,7 +348,7 @@ TEST(BracesAroundStatementsCheck, DoWhile) { "}")); } -TEST(BracesAroundStatementsCheck, While) { +TEST(BracesAroundStatementsCheckTest, While) { EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n" " while (false) {\n" " ;\n" @@ -411,7 +412,7 @@ TEST(BracesAroundStatementsCheck, While) { "}")); } -TEST(BracesAroundStatementsCheck, Nested) { +TEST(BracesAroundStatementsCheckTest, Nested) { EXPECT_EQ("int main() {\n" " do { if (true) {}} while (false);\n" "}", @@ -446,7 +447,7 @@ TEST(BracesAroundStatementsCheck, Nested) { "}")); } -TEST(BracesAroundStatementsCheck, Macros) { +TEST(BracesAroundStatementsCheckTest, Macros) { EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "#define IF(COND) if (COND) return -1;\n" "int main() {\n" @@ -480,6 +481,19 @@ TEST(BracesAroundStatementsCheck, Macros) { "}")); } +#define EXPECT_NO_CHANGES_WITH_OPTS(Check, Opts, Code) \ + EXPECT_EQ(Code, runCheckOnCode(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