Fix and clean as suggested by alexfk.
- [clang-tidy] Fix BracesAroundStatementsCheck when processing 'if' and
'while' statements with comments inside condition.
http://reviews.llvm.org/D5395
Files:
clang-tidy/misc/BracesAroundStatementsCheck.cpp
clang-tidy/misc/BracesAroundStatementsCheck.h
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
test/clang-tidy/misc-braces-around-statements.cpp
unittests/clang-tidy/MiscModuleTest.cpp
Index: clang-tidy/misc/BracesAroundStatementsCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/misc/BracesAroundStatementsCheck.cpp
@@ -0,0 +1,111 @@
+//===--- 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 {
+
+SourceLocation
+backwardSkipWhitespacesAndComments(const SourceManager &SM,
+ const clang::ASTContext &Context,
+ SourceLocation Loc) {
+ for (;;) {
+ do {
+ Loc = Loc.getLocWithOffset(-1);
+ } while (isWhitespace(*FullSourceLoc(Loc, SM).getCharacterData()));
+
+ 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 || Tok.getKind() != tok::comment)
+ return Loc.getLocWithOffset(1);
+ }
+}
+
+} // end anonymous 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) {
+ // to put opening brace, get location after closing parenthesis or 'do'
+ if (auto FS = Result.Nodes.getNodeAs<ForStmt>("for")) {
+ checkStmt(Result, FS->getBody(), FS->getRParenLoc());
+ } else if (auto FRS = Result.Nodes.getNodeAs<CXXForRangeStmt>("for-range")) {
+ checkStmt(Result, FRS->getBody(), FRS->getRParenLoc());
+ } else if (auto DS = Result.Nodes.getNodeAs<DoStmt>("do")) {
+ checkStmt(Result, DS->getBody(), DS->getDoLoc());
+ } else if (auto WS = Result.Nodes.getNodeAs<WhileStmt>("while")) {
+ Token Tok;
+ // Find location of right parenthesis closing condition
+ SourceLocation StartLoc =
+ backwardSkipWhitespacesAndComments(
+ *Result.SourceManager, *Result.Context,
+ WS->getBody()->getLocStart()).getLocWithOffset(-1);
+ checkStmt(Result, WS->getBody(), StartLoc);
+ } else if (auto IS = Result.Nodes.getNodeAs<IfStmt>("if")) {
+ // Find location of right parenthesis closing condition
+ SourceLocation StartLoc =
+ backwardSkipWhitespacesAndComments(
+ *Result.SourceManager, *Result.Context,
+ IS->getThen()->getLocStart()).getLocWithOffset(-1);
+ checkStmt(Result, IS->getThen(), StartLoc);
+ const Stmt *Else = IS->getElse();
+ if (Else && !isa<IfStmt>(Else)) {
+ // omit 'else if' statements here, they will be handled directly
+ checkStmt(Result, Else, IS->getElseLoc());
+ }
+ }
+}
+
+void
+BracesAroundStatementsCheck::checkStmt(const MatchFinder::MatchResult &Result,
+ const Stmt *S,
+ SourceLocation InitialLoc) {
+ if (!S || isa<CompoundStmt>(S)) {
+ // already inside braces
+ return;
+ }
+ // InitialLoc points at the (beginning of the) last token before unexisting
+ // opening brace
+ SourceLocation StartLoc = Lexer::getLocForEndOfToken(
+ InitialLoc, 0, *Result.SourceManager, Result.Context->getLangOpts());
+ SourceLocation EndLoc = Lexer::getLocForEndOfToken(
+ S->getLocEnd(), 0, *Result.SourceManager, Result.Context->getLangOpts());
+ // EndLoc points past ";" only if S was a no-op ";" operation
+ if (S->getLocStart() != S->getLocEnd() ||
+ EndLoc != S->getLocEnd().getLocWithOffset(1)) {
+ // EndLoc points at ";"
+ EndLoc = EndLoc.getLocWithOffset(1);
+ }
+ auto Diag = diag(StartLoc, "statement should be inside braces");
+ // add braces
+ Diag << FixItHint::CreateInsertion(StartLoc, " {")
+ << FixItHint::CreateInsertion(EndLoc, "}");
+}
+
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/misc/BracesAroundStatementsCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/misc/BracesAroundStatementsCheck.h
@@ -0,0 +1,33 @@
+//===--- 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);
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_BRACES_AROUND_STATEMENTS_CHECK_H
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -3,6 +3,7 @@
add_clang_library(clangTidyMiscModule
ArgumentCommentCheck.cpp
BoolPointerImplicitConversion.cpp
+ BracesAroundStatementsCheck.cpp
FunctionSize.cpp
MiscTidyModule.cpp
RedundantSmartptrGet.cpp
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -12,6 +12,7 @@
#include "../ClangTidyModuleRegistry.h"
#include "ArgumentCommentCheck.h"
#include "BoolPointerImplicitConversion.h"
+#include "BracesAroundStatementsCheck.h"
#include "FunctionSize.h"
#include "RedundantSmartptrGet.h"
#include "SwappedArgumentsCheck.h"
@@ -28,6 +29,8 @@
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");
@@ -42,7 +45,7 @@
// Register the MiscTidyModule using this statically initialized variable.
static ClangTidyModuleRegistry::Add<MiscModule>
-X("misc-module", "Adds miscellaneous lint checks.");
+ 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.
Index: test/clang-tidy/misc-braces-around-statements.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-braces-around-statements.cpp
@@ -0,0 +1,137 @@
+// 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() {
+ // if
+ if (cond("if1") /*comment*/)
+ // some comment
+ do_something("if");
+ // CHECK-MESSAGES: :[[@LINE-3]]:31: warning: statement should be inside braces
+ // CHECK-FIXES: if (cond("if1") /*comment*/) {
+ // CHECK-FIXES: do_something("if");}
+ if (cond("if2")) {
+ do_something("if");
+ }
+ if (cond("if3"))
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:19: warning: statement should be inside braces
+ // CHECK-FIXES: if (cond("if3")) {
+ // CHECK-FIXES: ;}
+ if (cond("if4") EMPTY_MACRO)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:31: warning: statement should be inside braces
+ // CHECK-FIXES: if (cond("if4") EMPTY_MACRO) {
+ // CHECK-FIXES: ;}
+ if (cond("if5") EMPTY_MACRO_FUN())
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:37: warning: statement should be inside braces
+ // CHECK-FIXES: if (cond("if5") EMPTY_MACRO_FUN()) {
+ // CHECK-FIXES: ;}
+
+ // if-else
+ if (cond("if-else1"))
+ do_something("if");
+ // CHECK-MESSAGES: :[[@LINE-2]]:24: warning: statement should be inside braces
+ // CHECK-FIXES: if (cond("if-else1")) {
+ // CHECK-FIXES: do_something("if");}
+ else
+ do_something("else");
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: statement should be inside braces
+ // CHECK-FIXES: else {
+ // CHECK-FIXES: do_something("else");}
+ if (cond("if-else2")) {
+ do_something("if");
+ } else {
+ do_something("else");
+ }
+
+ // 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: if (cond("if-else if-else1")) {
+ else if (cond("else if1"))
+ do_something("else if");
+ // CHECK-MESSAGES: :[[@LINE-2]]:29: warning: statement should be inside braces
+ // CHECK-FIXES: else if (cond("else if1")) {
+ else
+ do_something("else");
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: statement should be inside braces
+ // CHECK-FIXES: do_something("else");}
+ if (cond("if-else if-else2")) {
+ do_something("if");
+ } else if (cond("else if2")) {
+ do_something("else if");
+ } else {
+ do_something("else");
+ }
+
+ for (;;)
+ do_something("for");
+ // CHECK-MESSAGES: :[[@LINE-2]]:11: warning: statement should be inside braces
+ // CHECK-FIXES: for (;;) {
+ // CHECK-FIXES: do_something("for");}
+ for (;;) {
+ do_something("for");
+ }
+ for (;;)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:11: warning: statement should be inside braces
+ // CHECK-FIXES: for (;;) {
+ // CHECK-FIXES: ;}
+
+ 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: do_something("for-range");}
+ 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: ;}
+
+ while (cond("while1"))
+ do_something("while");
+ // CHECK-MESSAGES: :[[@LINE-2]]:25: warning: statement should be inside braces
+ // CHECK-FIXES: while (cond("while1")) {
+ // CHECK-FIXES: do_something("while");}
+ 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: ;}
+
+ do
+ do_something("do");
+ while (cond("do1"));
+ // CHECK-MESSAGES: :[[@LINE-3]]:5: warning: statement should be inside braces
+ // CHECK-FIXES: do {
+ // CHECK-FIXES: do_something("do");}
+ do {
+ do_something("do");
+ } while (cond("do2"));
+
+ do
+ ;
+ while (false);
+ // CHECK-MESSAGES: :[[@LINE-3]]:5: warning: statement should be inside braces
+ // CHECK-FIXES: do {
+ // CHECK-FIXES: ;}
+}
Index: unittests/clang-tidy/MiscModuleTest.cpp
===================================================================
--- unittests/clang-tidy/MiscModuleTest.cpp
+++ unittests/clang-tidy/MiscModuleTest.cpp
@@ -1,5 +1,6 @@
#include "ClangTidyTest.h"
#include "misc/ArgumentCommentCheck.h"
+#include "misc/BracesAroundStatementsCheck.h"
#include "gtest/gtest.h"
namespace clang {
@@ -35,6 +36,201 @@
"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"
+ " return 0;\n"
+ "}",
+ runCheckOnCode<BracesAroundStatementsCheck>(
+ "int main() {\n"
+ " if (false /*dummy token*/)\n"
+ " // comment\n"
+ " return -1;\n"
+ " return 0;\n"
+ "}"));
+}
+
+TEST(BracesAroundStatementsCheck, If) {
+ // if only
+ EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n"
+ " if (false) {\n"
+ " return -1;\n"
+ " }\n"
+ " return 0;\n"
+ "}");
+ // if else
+ EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n"
+ " if (false) {\n"
+ " return -1;\n"
+ " } else {\n"
+ " return 0;\n"
+ " }\n"
+ "}");
+ // if only
+ EXPECT_EQ("int main() {\n"
+ " if (false) {\n"
+ " return -1;}\n"
+ " return 0;\n"
+ "}",
+ runCheckOnCode<BracesAroundStatementsCheck>("int main() {\n"
+ " if (false)\n"
+ " return -1;\n"
+ " return 0;\n"
+ "}"));
+ // if else
+ EXPECT_EQ("int main() {\n"
+ " if (false) {\n"
+ " return -1;}\n"
+ " else {\n"
+ " return 0;}\n"
+ "}",
+ runCheckOnCode<BracesAroundStatementsCheck>("int main() {\n"
+ " if (false)\n"
+ " return -1;\n"
+ " else\n"
+ " return 0;\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 0;}\n"
+ "}",
+ runCheckOnCode<BracesAroundStatementsCheck>("int main() {\n"
+ " if (false)\n"
+ " return -1;\n"
+ " else if (1 == 2)\n"
+ " return -2;\n"
+ " else\n"
+ " return 0;\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 0;}\n"
+ "}",
+ runCheckOnCode<BracesAroundStatementsCheck>("int main() {\n"
+ " if (false)\n"
+ " return -1;\n"
+ " else if (1 == 2) {\n"
+ " return -2;\n"
+ " } else\n"
+ " return 0;\n"
+ "}"));
+}
+
+TEST(BracesAroundStatementsCheck, For) {
+ EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n"
+ " for (;;) {\n"
+ " ;\n"
+ " }\n"
+ " return 0;\n"
+ "}");
+ EXPECT_EQ("int main() {\n"
+ " for (;;) {\n"
+ " ;}\n"
+ " return 0;\n"
+ "}",
+ runCheckOnCode<BracesAroundStatementsCheck>("int main() {\n"
+ " for (;;)\n"
+ " ;\n"
+ " return 0;\n"
+ "}"));
+}
+
+TEST(BracesAroundStatementsCheck, ForRange) {
+ EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n"
+ " int arr[4];\n"
+ " for (int i : arr) {\n"
+ " ;\n"
+ " }\n"
+ " return 0;\n"
+ "}");
+ EXPECT_EQ("int main() {\n"
+ " int arr[4];\n"
+ " for (int i : arr) {\n"
+ " ;}\n"
+ " return 0;\n"
+ "}",
+ runCheckOnCode<BracesAroundStatementsCheck>("int main() {\n"
+ " int arr[4];\n"
+ " for (int i : arr)\n"
+ " ;\n"
+ " return 0;\n"
+ "}"));
+}
+
+TEST(BracesAroundStatementsCheck, DoWhile) {
+ EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n"
+ " do {\n"
+ " ;\n"
+ " } while (false);\n"
+ " return 0;\n"
+ "}");
+ EXPECT_EQ("int main() {\n"
+ " do {\n"
+ " ;}\n"
+ " while (false);\n"
+ " return 0;\n"
+ "}",
+ runCheckOnCode<BracesAroundStatementsCheck>("int main() {\n"
+ " do\n"
+ " ;\n"
+ " while (false);\n"
+ " return 0;\n"
+ "}"));
+}
+
+TEST(BracesAroundStatementsCheck, While) {
+ EXPECT_NO_CHANGES(BracesAroundStatementsCheck, "int main() {\n"
+ " while (false) {\n"
+ " ;\n"
+ " }\n"
+ " return 0;\n"
+ "}");
+ EXPECT_EQ("int main() {\n"
+ " while (false) {\n"
+ " ;}\n"
+ " return 0;\n"
+ "}",
+ runCheckOnCode<BracesAroundStatementsCheck>("int main() {\n"
+ " while (false)\n"
+ " ;\n"
+ " return 0;\n"
+ "}"));
+ EXPECT_EQ("int main() {\n"
+ " while (false /*dummy token*/) {\n"
+ " ;}\n"
+ " return 0;\n"
+ "}",
+ runCheckOnCode<BracesAroundStatementsCheck>(
+ "int main() {\n"
+ " while (false /*dummy token*/)\n"
+ " ;\n"
+ " return 0;\n"
+ "}"));
+ EXPECT_EQ("int main() {\n"
+ " while (false) {\n"
+ " break;}\n"
+ " return 0;\n"
+ "}",
+ runCheckOnCode<BracesAroundStatementsCheck>("int main() {\n"
+ " while (false)\n"
+ " break;\n"
+ " return 0;\n"
+ "}"));
+}
+
} // namespace test
} // namespace tidy
} // namespace clang
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
