Index: clang-tidy/misc/BracesAroundStatementsCheck.h =================================================================== --- clang-tidy/misc/BracesAroundStatementsCheck.h +++ clang-tidy/misc/BracesAroundStatementsCheck.h @@ -15,10 +15,29 @@ namespace clang { namespace tidy { +/// \brief Checks that bodies of 'if' statements and loops ('for', 'range-for', +/// 'do-while', and 'while') are inside braces +/// +/// Before: +/// if (condition) +/// statement; +/// +/// After: +/// if (condition) { +/// statement; +/// } +/// +/// Additionally, one can define an option `ShortStatementLines` defining the +/// minimal number of lines that the statement should have in order to trigger +/// this check. +/// The number of lines is counted from the end of condition or initial keyword +/// (do/else) until the last line of the inner statement. +/// Default value 0 means that braces will be added to all statements (not +/// having them already). class BracesAroundStatementsCheck : public ClangTidyCheck { public: - BracesAroundStatementsCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + BracesAroundStatementsCheck(StringRef Name, ClangTidyContext *Context); + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; @@ -29,6 +48,9 @@ template SourceLocation findRParenLoc(const IfOrWhileStmt *S, const SourceManager &SM, const ASTContext *Context); + +private: + const unsigned ShortStatementLines; }; } // namespace tidy Index: clang-tidy/misc/BracesAroundStatementsCheck.cpp =================================================================== --- clang-tidy/misc/BracesAroundStatementsCheck.cpp +++ clang-tidy/misc/BracesAroundStatementsCheck.cpp @@ -110,6 +110,17 @@ } // namespace +BracesAroundStatementsCheck::BracesAroundStatementsCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + // Always add braces by default. + ShortStatementLines(Options.get("ShortStatementLines", 0U)) {} + +void +BracesAroundStatementsCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "ShortStatementLines", ShortStatementLines); +} + void BracesAroundStatementsCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher(ifStmt().bind("if"), this); Finder->addMatcher(whileStmt().bind("while"), this); @@ -206,11 +217,6 @@ if (S->getLocStart().isMacroID()) return; - // TODO: Add an option to insert braces if: - // * the body doesn't fit on the same line with the control statement - // * the body takes more than one line - // * always. - const SourceManager &SM = *Result.SourceManager; const ASTContext *Context = Result.Context; @@ -230,6 +236,16 @@ } assert(StartLoc.isValid()); + assert(EndLoc.isValid()); + // Don't require braces for statements spanning less than certain number of + // lines. + if (ShortStatementLines) { + unsigned StartLine = SM.getSpellingLineNumber(StartLoc); + unsigned EndLine = SM.getSpellingLineNumber(EndLoc); + if (EndLine - StartLine < ShortStatementLines) + return; + } + auto Diag = diag(StartLoc, "statement should be inside braces"); Diag << FixItHint::CreateInsertion(StartLoc, " {") << FixItHint::CreateInsertion(EndLoc, ClosingInsertion); Index: test/clang-tidy/misc-braces-around-statements-always.cpp =================================================================== --- /dev/null +++ test/clang-tidy/misc-braces-around-statements-always.cpp @@ -0,0 +1,42 @@ +// RUN: $(dirname %s)/check_clang_tidy_fix.sh %s misc-braces-around-statements %t -config="{CheckOptions: [{key: misc-braces-around-statements.ShortStatementLines, value: 0}]}" -- +// REQUIRES: shell + +void do_something(const char *) {} + +bool cond(const char *) { + return false; +} + +void test() { + if (cond("if1") /*comment*/) do_something("same-line"); + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: statement should be inside braces + // CHECK-FIXES: if (cond("if1") /*comment*/) { do_something("same-line"); + // CHECK-FIXES: } + + if (cond("if2")) + do_something("single-line"); + // CHECK-MESSAGES: :[[@LINE-2]]:19: warning: statement should be inside braces + // CHECK-FIXES: if (cond("if2")) { + // CHECK-FIXES: } + + if (cond("if3") /*comment*/) + // some comment + do_something("three" + "lines"); + // CHECK-MESSAGES: :[[@LINE-4]]:31: warning: statement should be inside braces + // CHECK-FIXES: if (cond("if3") /*comment*/) { + // CHECK-FIXES: } + + if (cond("if4") /*comment*/) + // some comment + do_something("many" + "many" + "many" + "many" + "lines"); + // CHECK-MESSAGES: :[[@LINE-7]]:31: warning: statement should be inside braces + // CHECK-FIXES: if (cond("if4") /*comment*/) { + // CHECK-FIXES: } + + // CHECK-NOT: warning +} Index: test/clang-tidy/misc-braces-around-statements-few-lines.cpp =================================================================== --- /dev/null +++ test/clang-tidy/misc-braces-around-statements-few-lines.cpp @@ -0,0 +1,36 @@ +// RUN: $(dirname %s)/check_clang_tidy_fix.sh %s misc-braces-around-statements %t -config="{CheckOptions: [{key: misc-braces-around-statements.ShortStatementLines, value: 4}]}" -- +// REQUIRES: shell + +void do_something(const char *) {} + +bool cond(const char *) { + return false; +} + +void test() { + if (cond("if1") /*comment*/) do_something("same-line"); + // CHECK-NOT: warning + + if (cond("if2")) + do_something("single-line"); + // CHECK-NOT: warning + + if (cond("if3") /*comment*/) + // some comment + do_something("three" + "lines"); + // CHECK-NOT: warning + + if (cond("if4") /*comment*/) + // some comment + do_something("many" + "many" + "many" + "many" + "lines"); + // CHECK-MESSAGES: :[[@LINE-7]]:31: warning: statement should be inside braces + // CHECK-FIXES: if (cond("if4") /*comment*/) { + // CHECK-FIXES: } + + // CHECK-NOT: warning +} Index: test/clang-tidy/misc-braces-around-statements-same-line.cpp =================================================================== --- /dev/null +++ test/clang-tidy/misc-braces-around-statements-same-line.cpp @@ -0,0 +1,40 @@ +// RUN: $(dirname %s)/check_clang_tidy_fix.sh %s misc-braces-around-statements %t -config="{CheckOptions: [{key: misc-braces-around-statements.ShortStatementLines, value: 1}]}" -- +// REQUIRES: shell + +void do_something(const char *) {} + +bool cond(const char *) { + return false; +} + +void test() { + if (cond("if1") /*comment*/) do_something("same-line"); + // CHECK-NOT: warning + + if (cond("if2")) + do_something("single-line"); + // CHECK-MESSAGES: :[[@LINE-2]]:19: warning: statement should be inside braces + // CHECK-FIXES: if (cond("if2")) { + // CHECK-FIXES: } + + if (cond("if3") /*comment*/) + // some comment + do_something("three" + "lines"); + // CHECK-MESSAGES: :[[@LINE-4]]:31: warning: statement should be inside braces + // CHECK-FIXES: if (cond("if3") /*comment*/) { + // CHECK-FIXES: } + + if (cond("if4") /*comment*/) + // some comment + do_something("many" + "many" + "many" + "many" + "lines"); + // CHECK-MESSAGES: :[[@LINE-7]]:31: warning: statement should be inside braces + // CHECK-FIXES: if (cond("if4") /*comment*/) { + // CHECK-FIXES: } + + // CHECK-NOT: warning +} Index: test/clang-tidy/misc-braces-around-statements-single-line.cpp =================================================================== --- /dev/null +++ test/clang-tidy/misc-braces-around-statements-single-line.cpp @@ -0,0 +1,38 @@ +// RUN: $(dirname %s)/check_clang_tidy_fix.sh %s misc-braces-around-statements %t -config="{CheckOptions: [{key: misc-braces-around-statements.ShortStatementLines, value: 2}]}" -- +// REQUIRES: shell + +void do_something(const char *) {} + +bool cond(const char *) { + return false; +} + +void test() { + if (cond("if1") /*comment*/) do_something("same-line"); + // CHECK-NOT: warning + + if (cond("if2")) + do_something("single-line"); + // CHECK-NOT: warning + + if (cond("if3") /*comment*/) + // some comment + do_something("three" + "lines"); + // CHECK-MESSAGES: :[[@LINE-4]]:31: warning: statement should be inside braces + // CHECK-FIXES: if (cond("if3") /*comment*/) { + // CHECK-FIXES: } + + if (cond("if4") /*comment*/) + // some comment + do_something("many" + "many" + "many" + "many" + "lines"); + // CHECK-MESSAGES: :[[@LINE-7]]:31: warning: statement should be inside braces + // CHECK-FIXES: if (cond("if4") /*comment*/) { + // CHECK-FIXES: } + + // CHECK-NOT: warning +}