Index: clang-tidy/readability/BracesAroundStatementsCheck.cpp =================================================================== --- clang-tidy/readability/BracesAroundStatementsCheck.cpp +++ clang-tidy/readability/BracesAroundStatementsCheck.cpp @@ -10,30 +10,16 @@ #include "BracesAroundStatementsCheck.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchers.h" -#include "clang/Lex/Lexer.h" +#include "../utils/LexerUtils.h" using namespace clang::ast_matchers; +using namespace clang::tidy::lexer_utils; namespace clang { namespace tidy { namespace readability { namespace { -tok::TokenKind getTokenKind(SourceLocation Loc, const SourceManager &SM, - const ASTContext *Context) { - 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) - return tok::NUM_TOKENS; - - return Tok.getKind(); -} - SourceLocation forwardSkipWhitespaceAndComments(SourceLocation Loc, const SourceManager &SM, const ASTContext *Context) { Index: clang-tidy/readability/CMakeLists.txt =================================================================== --- clang-tidy/readability/CMakeLists.txt +++ clang-tidy/readability/CMakeLists.txt @@ -15,6 +15,7 @@ RedundantStringCStrCheck.cpp RedundantSmartptrGetCheck.cpp SimplifyBooleanExprCheck.cpp + TernaryOperatorCheck.cpp UniqueptrDeleteReleaseCheck.cpp LINK_LIBS Index: clang-tidy/readability/ReadabilityTidyModule.cpp =================================================================== --- clang-tidy/readability/ReadabilityTidyModule.cpp +++ clang-tidy/readability/ReadabilityTidyModule.cpp @@ -22,6 +22,7 @@ #include "RedundantSmartptrGetCheck.h" #include "RedundantStringCStrCheck.h" #include "SimplifyBooleanExprCheck.h" +#include "TernaryOperatorCheck.h" #include "UniqueptrDeleteReleaseCheck.h" namespace clang { @@ -45,6 +46,8 @@ "readability-implicit-bool-cast"); CheckFactories.registerCheck( "readability-inconsistent-declaration-parameter-name"); + CheckFactories.registerCheck( + "readability-ternary-operator"); CheckFactories.registerCheck( "readability-named-parameter"); CheckFactories.registerCheck( Index: clang-tidy/readability/TernaryOperatorCheck.h =================================================================== --- /dev/null +++ clang-tidy/readability/TernaryOperatorCheck.h @@ -0,0 +1,64 @@ +//===--- TernaryOperatorCheck.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_READABILITY_TERNARY_OPERATOR_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_TERNARY_OPERATOR_H + +#include "../ClangTidy.h" +#include +#include + +namespace clang { +namespace tidy { +namespace readability { + +/// Check that suggests using ternary operator where it's possible to. +/// +/// Before: +/// +/// \code +/// if (condition) { +/// expression0; +/// } else { +/// expression1; +/// } +/// \endcode +/// +/// After: +/// +/// \code +/// (condition ? expression0 : expression1); +/// \endcode +/// +/// Note that if no autofix would be suggested if any comments may be lost. +/// +/// For the user-facing documentation and more examples see: +/// http://clang.llvm.org/extra/clang-tidy/checks/readability-ternary-operator.html +class TernaryOperatorCheck : public ClangTidyCheck { +public: + TernaryOperatorCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + bool matchIf(const IfStmt *IfNode); + std::vector getHints(const IfStmt *IfNode, const Expr *ThenNode, + const Expr *ElseNode); + + const SourceManager *SM; + const LangOptions *Opts; + ASTContext *Ctx; +}; + +} // namespace readability +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_TERNARY_OPERATOR_H Index: clang-tidy/readability/TernaryOperatorCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/readability/TernaryOperatorCheck.cpp @@ -0,0 +1,188 @@ +//===--- TernaryOperatorCheck.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 "TernaryOperatorCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "../utils/LexerUtils.h" + +#include + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace readability { +namespace { + +bool hasTokensInside(const std::vector TokenKinds, + const SourceRange SR, const SourceManager &SM, + const ASTContext *Context) { + for (SourceLocation it = SR.getBegin(); it != SR.getEnd(); + it = it.getLocWithOffset(1)) { + tok::TokenKind TokKind = lexer_utils::getTokenKind(it, SM, Context); + for (auto Kind : TokenKinds) { + if (Kind == TokKind) { + return true; + } + } + // Fast-forward current token. + // it = Lexer::getLocForEndOfToken(it, 0, SM, Context->getLangOpts()); + } + return false; +} + +SourceLocation getNextTokenLoc(tok::TokenKind TokKind, SourceLocation Loc, + const SourceManager &SM, + const ASTContext *Context, bool After = true) { + SourceLocation Result = + Lexer::getLocForEndOfToken(Loc, 0, SM, Context->getLangOpts()); + while (lexer_utils::getTokenKind(Result, SM, Context) != TokKind) { + Result = Result.getLocWithOffset(1); + } + return Result; +} + +SourceLocation getPrevTokenLoc(tok::TokenKind TokKind, SourceLocation Loc, + const SourceManager &SM, + const ASTContext *Context, bool After = true) { + SourceLocation Result = + Lexer::GetBeginningOfToken(Loc, SM, Context->getLangOpts()); + while (lexer_utils::getTokenKind(Result, SM, Context) != TokKind) { + Result = Result.getLocWithOffset(-1); + } + return Result; +} + +} // namespace + +void TernaryOperatorCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher(ifStmt().bind("IfNode"), this); +} + +bool TernaryOperatorCheck::matchIf(const IfStmt *IfNode) { + if (IfNode->getElse() == nullptr) { + return false; + } + if (const auto *ElseIfStmt = dyn_cast(IfNode->getElse())) { + if (ElseIfStmt->getElse() != nullptr) { + return false; + } + } + for (const auto Parent : Ctx->getParents(*IfNode)) { + if (const IfStmt *ParentIf = Parent.get()) { + if (ParentIf->getElse() == IfNode) { + return false; + } + } + } + if (const auto *ThenNode = dyn_cast(IfNode->getThen())) { + if (ThenNode->size() > 1) { + return false; + } + } + if (const auto *ElseNode = dyn_cast(IfNode->getElse())) { + if (ElseNode->size() > 1) { + return false; + } + } + return true; +} + +void TernaryOperatorCheck::check(const MatchFinder::MatchResult &Result) { + this->Opts = &Result.Context->getLangOpts(); + this->SM = Result.SourceManager; + this->Ctx = Result.Context; + + const auto *IfNode = Result.Nodes.getNodeAs("IfNode"); + if (!matchIf(IfNode)) { + return; + } + + const auto *ThenNode = + (dyn_cast(IfNode->getThen()) + ? dyn_cast(IfNode->getThen())->body_front() + : IfNode->getThen()); + const auto *ElseNode = + (dyn_cast(IfNode->getElse()) + ? dyn_cast(IfNode->getElse())->body_front() + : IfNode->getElse()); + + if (ThenNode->getStmtClass() != ElseNode->getStmtClass()) { + return; + } + if (IfNode->getLocStart().isMacroID()) { + return; + } + + std::vector Hints; + if (dyn_cast(ThenNode) && + !hasTokensInside({tok::comment, tok::hash}, IfNode->getSourceRange(), *SM, + Ctx)) { + Hints = + getHints(IfNode, dyn_cast(ThenNode), dyn_cast(ElseNode)); + } + + auto Diagnostic = diag(IfNode->getLocStart(), "use ternary operator"); + for (auto Hint : Hints) { + Diagnostic.AddFixItHint(Hint); + } +} + +std::vector TernaryOperatorCheck::getHints(const IfStmt *IfNode, + const Expr *ThenNode, + const Expr *ElseNode) { + // Having if (condition) { expression0; else { expression1; } + // ^ ^ ^ ^ ^ ^ ^ ^ + // 1 2 3 4 5 6 7 8 + // + // Transform into: (condition ? expression0 : expression1); + // 1. Replace 1-2 range with "(" + // 2. Replace 3-4 range with " ? " + // 3. Replace 5-6 range with " : " + // 4. Insert ")" before 7 + // 5. Remove 7-8 range + + SourceRange NextRange; + std::string NextReplacement; + std::vector Hints; + + NextRange = SourceRange( + IfNode->getLocStart(), + getPrevTokenLoc(tok::l_paren, IfNode->getCond()->getLocStart(), *SM, Ctx) + .getLocWithOffset(-1)); + Hints.push_back(FixItHint::CreateRemoval(NextRange)); + + NextRange = SourceRange( + getNextTokenLoc(tok::r_paren, IfNode->getCond()->getLocEnd(), *SM, Ctx), + ThenNode->getLocStart().getLocWithOffset(-1)); + NextReplacement = ") ?"; + + Hints.push_back(FixItHint::CreateReplacement(NextRange, NextReplacement)); + NextRange = + SourceRange(getNextTokenLoc(tok::semi, ThenNode->getLocEnd(), *SM, Ctx), + ElseNode->getLocStart().getLocWithOffset(-1)); + NextReplacement = " :"; + Hints.push_back(FixItHint::CreateReplacement(NextRange, NextReplacement)); + + NextRange = SourceRange( + getNextTokenLoc(tok::semi, ElseNode->getLocEnd(), *SM, Ctx) + .getLocWithOffset(1), + (dyn_cast(IfNode->getElse()) + ? IfNode->getElse()->getLocEnd() + : getNextTokenLoc(tok::semi, ElseNode->getLocEnd(), *SM, Ctx) + .getLocWithOffset(1))); + Hints.push_back(FixItHint::CreateRemoval(NextRange)); + + return Hints; +} + +} // namespace readability +} // namespace tidy +} // namespace clang Index: clang-tidy/utils/LexerUtils.h =================================================================== --- clang-tidy/utils/LexerUtils.h +++ clang-tidy/utils/LexerUtils.h @@ -22,6 +22,9 @@ Token getPreviousNonCommentToken(const ASTContext &Context, SourceLocation Location); +tok::TokenKind getTokenKind(SourceLocation Location, const SourceManager &SM, + const ASTContext *Context); + } // namespace lexer_utils } // namespace tidy } // namespace clang Index: clang-tidy/utils/LexerUtils.cpp =================================================================== --- clang-tidy/utils/LexerUtils.cpp +++ clang-tidy/utils/LexerUtils.cpp @@ -34,6 +34,18 @@ return Token; } + +tok::TokenKind getTokenKind(SourceLocation Location, const SourceManager &SM, + const ASTContext *Context) { + Token Tok; + SourceLocation Beginning = + Lexer::GetBeginningOfToken(Location, SM, Context->getLangOpts()); + const bool Invalid = + Lexer::getRawToken(Beginning, Tok, SM, Context->getLangOpts()); + + return (Invalid ? tok::NUM_TOKENS : Tok.getKind()); +} + } // namespace lexer_utils } // namespace tidy } // namespace clang Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -94,4 +94,5 @@ readability-redundant-smartptr-get readability-redundant-string-cstr readability-simplify-boolean-expr + readability-ternary-operator readability-uniqueptr-delete-release Index: docs/clang-tidy/checks/readability-ternary-operator.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/readability-ternary-operator.rst @@ -0,0 +1,26 @@ +.. title:: clang-tidy - readability-ternary-operator + +readability-ternary-operator +============================ + +This check suggests using ternary operator in where it's possible to do so. +A typical case where it might improve readability follows this pattern:: + if (condition) expression0; else expression1; +And it gets transformed into:: + (condition ? expression0 : expression1); + +Another situation when ternary operator would be suggested:: + if (condition) return literal0; else return literal1; + +Autofixes +---------------------------- + +The autofixes are only suggested when no comments and intermediate preprocessor +lines would be lost. + +Consider the following piece of code:: + if (condition) /* comment */ expression0; else /* and here */ expression1; +While trying to modify this into an equal piece of code with ternary operator +both comments will be lost, therefore there will be no autofix suggested for +such case. If you want the check to perform an autofix, either reposition or +remove the comments and intermediate preprocessor lines. Index: test/clang-tidy/readability-ternary-operator.cpp =================================================================== --- /dev/null +++ test/clang-tidy/readability-ternary-operator.cpp @@ -0,0 +1,134 @@ +// RUN: %check_clang_tidy %s readability-ternary-operator %t + +bool Condition = true; +int Variable = 0; + +void call_me(int value); +void or_me(int value); + +//===----------------------------------------------------------------------===// +// Section #0: warnings without autofixes +//===----------------------------------------------------------------------===// + +int foo() { + // regular expressions + if (Condition) { + Variable++; // comment + } else { + Variable--; + } + // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use ternary operator + + if (Condition) { + call_me(0); + } else { + /* something */or_me(1); + } + // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use ternary operator + + // autofixing is not currently supported for ReturnStmt's + if (Condition) { + return 0; + } else { + return 1; + } + // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use ternary operator + + if (true || /* whatever */ false) { + call_me(0); + } else { + or_me(/* whoops; */ 0); + } + // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use ternary operator + + if (Condition) { +#define BLAH 1 + Variable++; + } else { + Variable--; + } + // CHECK-MESSAGES: :[[@LINE-6]]:3: warning: use ternary operator +} + +//===----------------------------------------------------------------------===// +// Section #1: warnings with autofixes +//===----------------------------------------------------------------------===// + +void boo() { + // regular expressions + if (Condition) { + Variable++; + } else { + Variable--; + } + // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use ternary operator + // CHECK-FIXES: (Condition) ? Variable++ : Variable--; + + if (Condition) + Variable++; + else { + Variable--; + } + // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use ternary operator + // CHECK-FIXES: (Condition) ? Variable++ : Variable--; + + if (Condition) { + Variable++; + } else + Variable--; + // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use ternary operator + // CHECK-FIXES: (Condition) ? Variable++ : Variable--; + + if (Condition) + Variable++; + else + Variable--; + // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use ternary operator + // CHECK-FIXES: (Condition) ? Variable++ : Variable--; + + if (Condition) { + call_me(0); + } else { + or_me(0); + } + // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use ternary operator + // CHECK-FIXES: (Condition) ? call_me(0) : or_me(0); + + // p00rly f0rm4tt3d c0d3 + if ( true || false ) {true;} else false; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use ternary operator + // CHECK-FIXES: ( true || false ) ?true : false; + + if (true)true;else false; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use ternary operator + // CHECK-FIXES: (true) ?true : false; + + if (true){true;}else false; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use ternary operator + // CHECK-FIXES: (true) ?true : false; + + if (true)true;else{false;} + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use ternary operator + // CHECK-FIXES: (true) ?true :false; +} + +//===----------------------------------------------------------------------===// +// Section #2: no warnings and autofixes +//===----------------------------------------------------------------------===// + +void baz() { + if (Condition) { + Variable = 0; + } else if (!Condition) { + Variable = 1; + } else { + Variable = 2; + } + + if (Condition) { + Variable++; + Variable--; + } else { + Variable++; + } +}