Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 1 | //===--- SimplifyBooleanExpr.cpp clang-tidy ---------------------*- C++ -*-===// |
| 2 | // |
| 3 | // The LLVM Compiler Infrastructure |
| 4 | // |
| 5 | // This file is distributed under the University of Illinois Open Source |
| 6 | // License. See LICENSE.TXT for details. |
| 7 | // |
| 8 | //===----------------------------------------------------------------------===// |
| 9 | |
| 10 | #include "SimplifyBooleanExprCheck.h" |
| 11 | #include "clang/Lex/Lexer.h" |
| 12 | |
| 13 | #include <cassert> |
| 14 | #include <string> |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 15 | #include <utility> |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 16 | |
| 17 | using namespace clang::ast_matchers; |
| 18 | |
| 19 | namespace clang { |
| 20 | namespace tidy { |
| 21 | namespace readability { |
| 22 | |
| 23 | namespace { |
| 24 | |
| 25 | StringRef getText(const MatchFinder::MatchResult &Result, SourceRange Range) { |
| 26 | return Lexer::getSourceText(CharSourceRange::getTokenRange(Range), |
| 27 | *Result.SourceManager, |
| 28 | Result.Context->getLangOpts()); |
| 29 | } |
| 30 | |
| 31 | template <typename T> |
| 32 | StringRef getText(const MatchFinder::MatchResult &Result, T &Node) { |
| 33 | return getText(Result, Node.getSourceRange()); |
| 34 | } |
| 35 | |
| 36 | const char RightExpressionId[] = "bool-op-expr-yields-expr"; |
| 37 | const char LeftExpressionId[] = "expr-op-bool-yields-expr"; |
| 38 | const char NegatedRightExpressionId[] = "bool-op-expr-yields-not-expr"; |
| 39 | const char NegatedLeftExpressionId[] = "expr-op-bool-yields-not-expr"; |
| 40 | const char ConditionThenStmtId[] = "if-bool-yields-then"; |
| 41 | const char ConditionElseStmtId[] = "if-bool-yields-else"; |
| 42 | const char TernaryId[] = "ternary-bool-yields-condition"; |
| 43 | const char TernaryNegatedId[] = "ternary-bool-yields-not-condition"; |
| 44 | const char IfReturnsBoolId[] = "if-return"; |
| 45 | const char IfReturnsNotBoolId[] = "if-not-return"; |
| 46 | const char ThenLiteralId[] = "then-literal"; |
| 47 | const char IfAssignVariableId[] = "if-assign-lvalue"; |
| 48 | const char IfAssignLocId[] = "if-assign-loc"; |
| 49 | const char IfAssignBoolId[] = "if-assign"; |
| 50 | const char IfAssignNotBoolId[] = "if-assign-not"; |
| 51 | const char IfAssignObjId[] = "if-assign-obj"; |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 52 | const char CompoundReturnId[] = "compound-return"; |
| 53 | const char CompoundBoolId[] = "compound-bool"; |
| 54 | const char CompoundNotBoolId[] = "compound-bool-not"; |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 55 | |
| 56 | const char IfStmtId[] = "if"; |
| 57 | const char LHSId[] = "lhs-expr"; |
| 58 | const char RHSId[] = "rhs-expr"; |
| 59 | |
| 60 | const char SimplifyOperatorDiagnostic[] = |
| 61 | "redundant boolean literal supplied to boolean operator"; |
| 62 | const char SimplifyConditionDiagnostic[] = |
| 63 | "redundant boolean literal in if statement condition"; |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 64 | const char SimplifyConditionalReturnDiagnostic[] = |
| 65 | "redundant boolean literal in conditional return statement"; |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 66 | |
| 67 | const CXXBoolLiteralExpr *getBoolLiteral(const MatchFinder::MatchResult &Result, |
| 68 | StringRef Id) { |
| 69 | const auto *Literal = Result.Nodes.getNodeAs<CXXBoolLiteralExpr>(Id); |
| 70 | return (Literal && |
| 71 | Result.SourceManager->isMacroBodyExpansion(Literal->getLocStart())) |
| 72 | ? nullptr |
| 73 | : Literal; |
| 74 | } |
| 75 | |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 76 | internal::Matcher<Stmt> returnsBool(bool Value, StringRef Id = "ignored") { |
| 77 | auto SimpleReturnsBool = |
Aaron Ballman | b9ea09c | 2015-09-17 13:31:25 +0000 | [diff] [blame] | 78 | returnStmt(has(cxxBoolLiteral(equals(Value)).bind(Id))) |
| 79 | .bind("returns-bool"); |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 80 | return anyOf(SimpleReturnsBool, |
| 81 | compoundStmt(statementCountIs(1), has(SimpleReturnsBool))); |
| 82 | } |
| 83 | |
| 84 | bool needsParensAfterUnaryNegation(const Expr *E) { |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 85 | E = E->IgnoreImpCasts(); |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 86 | if (isa<BinaryOperator>(E) || isa<ConditionalOperator>(E)) |
| 87 | return true; |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 88 | |
| 89 | if (const auto *Op = dyn_cast<CXXOperatorCallExpr>(E)) |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 90 | return Op->getNumArgs() == 2 && Op->getOperator() != OO_Call && |
| 91 | Op->getOperator() != OO_Subscript; |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 92 | |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 93 | return false; |
| 94 | } |
| 95 | |
| 96 | std::pair<BinaryOperatorKind, BinaryOperatorKind> Opposites[] = { |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 97 | {BO_LT, BO_GE}, {BO_GT, BO_LE}, {BO_EQ, BO_NE}}; |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 98 | |
| 99 | StringRef negatedOperator(const BinaryOperator *BinOp) { |
| 100 | const BinaryOperatorKind Opcode = BinOp->getOpcode(); |
| 101 | for (auto NegatableOp : Opposites) { |
| 102 | if (Opcode == NegatableOp.first) |
| 103 | return BinOp->getOpcodeStr(NegatableOp.second); |
| 104 | if (Opcode == NegatableOp.second) |
| 105 | return BinOp->getOpcodeStr(NegatableOp.first); |
| 106 | } |
| 107 | return StringRef(); |
| 108 | } |
| 109 | |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 110 | std::pair<OverloadedOperatorKind, StringRef> OperatorNames[] = { |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 111 | {OO_EqualEqual, "=="}, {OO_ExclaimEqual, "!="}, {OO_Less, "<"}, |
| 112 | {OO_GreaterEqual, ">="}, {OO_Greater, ">"}, {OO_LessEqual, "<="}}; |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 113 | |
| 114 | StringRef getOperatorName(OverloadedOperatorKind OpKind) { |
| 115 | for (auto Name : OperatorNames) { |
| 116 | if (Name.first == OpKind) |
| 117 | return Name.second; |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 118 | } |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 119 | |
| 120 | return StringRef(); |
| 121 | } |
| 122 | |
| 123 | std::pair<OverloadedOperatorKind, OverloadedOperatorKind> OppositeOverloads[] = |
| 124 | {{OO_EqualEqual, OO_ExclaimEqual}, |
| 125 | {OO_Less, OO_GreaterEqual}, |
| 126 | {OO_Greater, OO_LessEqual}}; |
| 127 | |
| 128 | StringRef negatedOperator(const CXXOperatorCallExpr *OpCall) { |
| 129 | const OverloadedOperatorKind Opcode = OpCall->getOperator(); |
| 130 | for (auto NegatableOp : OppositeOverloads) { |
| 131 | if (Opcode == NegatableOp.first) |
| 132 | return getOperatorName(NegatableOp.second); |
| 133 | if (Opcode == NegatableOp.second) |
| 134 | return getOperatorName(NegatableOp.first); |
| 135 | } |
| 136 | return StringRef(); |
| 137 | } |
| 138 | |
| 139 | std::string asBool(StringRef text, bool NeedsStaticCast) { |
| 140 | if (NeedsStaticCast) |
| 141 | return ("static_cast<bool>(" + text + ")").str(); |
| 142 | |
| 143 | return text; |
| 144 | } |
| 145 | |
| 146 | bool needsNullPtrComparison(const Expr *E) { |
| 147 | if (const auto *ImpCast = dyn_cast<ImplicitCastExpr>(E)) |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 148 | return ImpCast->getCastKind() == CK_PointerToBoolean || |
| 149 | ImpCast->getCastKind() == CK_MemberPointerToBoolean; |
| 150 | |
| 151 | return false; |
| 152 | } |
| 153 | |
| 154 | bool needsZeroComparison(const Expr *E) { |
| 155 | if (const auto *ImpCast = dyn_cast<ImplicitCastExpr>(E)) |
| 156 | return ImpCast->getCastKind() == CK_IntegralToBoolean; |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 157 | |
| 158 | return false; |
| 159 | } |
| 160 | |
| 161 | bool needsStaticCast(const Expr *E) { |
| 162 | if (const auto *ImpCast = dyn_cast<ImplicitCastExpr>(E)) { |
| 163 | if (ImpCast->getCastKind() == CK_UserDefinedConversion && |
| 164 | ImpCast->getSubExpr()->getType()->isBooleanType()) { |
| 165 | if (const auto *MemCall = |
| 166 | dyn_cast<CXXMemberCallExpr>(ImpCast->getSubExpr())) { |
| 167 | if (const auto *MemDecl = |
| 168 | dyn_cast<CXXConversionDecl>(MemCall->getMethodDecl())) { |
| 169 | if (MemDecl->isExplicit()) |
| 170 | return true; |
| 171 | } |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 172 | } |
| 173 | } |
| 174 | } |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 175 | |
| 176 | E = E->IgnoreImpCasts(); |
| 177 | return !E->getType()->isBooleanType(); |
| 178 | } |
| 179 | |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 180 | std::string compareExpressionToConstant(const MatchFinder::MatchResult &Result, |
| 181 | const Expr *E, bool Negated, |
| 182 | const char *Constant) { |
| 183 | E = E->IgnoreImpCasts(); |
| 184 | const std::string ExprText = |
| 185 | (isa<BinaryOperator>(E) ? ("(" + getText(Result, *E) + ")") |
| 186 | : getText(Result, *E)) |
| 187 | .str(); |
| 188 | return ExprText + " " + (Negated ? "!=" : "==") + " " + Constant; |
| 189 | } |
| 190 | |
| 191 | std::string compareExpressionToNullPtr(const MatchFinder::MatchResult &Result, |
| 192 | const Expr *E, bool Negated) { |
| 193 | const char *NullPtr = |
| 194 | Result.Context->getLangOpts().CPlusPlus11 ? "nullptr" : "NULL"; |
| 195 | return compareExpressionToConstant(Result, E, Negated, NullPtr); |
| 196 | } |
| 197 | |
| 198 | std::string compareExpressionToZero(const MatchFinder::MatchResult &Result, |
| 199 | const Expr *E, bool Negated) { |
| 200 | return compareExpressionToConstant(Result, E, Negated, "0"); |
| 201 | } |
| 202 | |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 203 | std::string replacementExpression(const MatchFinder::MatchResult &Result, |
| 204 | bool Negated, const Expr *E) { |
| 205 | E = E->ignoreParenBaseCasts(); |
| 206 | const bool NeedsStaticCast = needsStaticCast(E); |
| 207 | if (Negated) { |
| 208 | if (const auto *UnOp = dyn_cast<UnaryOperator>(E)) { |
| 209 | if (UnOp->getOpcode() == UO_LNot) { |
| 210 | if (needsNullPtrComparison(UnOp->getSubExpr())) |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 211 | return compareExpressionToNullPtr(Result, UnOp->getSubExpr(), true); |
| 212 | |
| 213 | if (needsZeroComparison(UnOp->getSubExpr())) |
| 214 | return compareExpressionToZero(Result, UnOp->getSubExpr(), true); |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 215 | |
| 216 | return replacementExpression(Result, false, UnOp->getSubExpr()); |
| 217 | } |
| 218 | } |
| 219 | |
| 220 | if (needsNullPtrComparison(E)) |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 221 | return compareExpressionToNullPtr(Result, E, false); |
| 222 | |
| 223 | if (needsZeroComparison(E)) |
| 224 | return compareExpressionToZero(Result, E, false); |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 225 | |
| 226 | StringRef NegatedOperator; |
| 227 | const Expr *LHS = nullptr; |
| 228 | const Expr *RHS = nullptr; |
| 229 | if (const auto *BinOp = dyn_cast<BinaryOperator>(E)) { |
| 230 | NegatedOperator = negatedOperator(BinOp); |
| 231 | LHS = BinOp->getLHS(); |
| 232 | RHS = BinOp->getRHS(); |
| 233 | } else if (const auto *OpExpr = dyn_cast<CXXOperatorCallExpr>(E)) { |
| 234 | if (OpExpr->getNumArgs() == 2) { |
| 235 | NegatedOperator = negatedOperator(OpExpr); |
| 236 | LHS = OpExpr->getArg(0); |
| 237 | RHS = OpExpr->getArg(1); |
| 238 | } |
| 239 | } |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 240 | if (!NegatedOperator.empty() && LHS && RHS) |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 241 | return (asBool((getText(Result, *LHS) + " " + NegatedOperator + " " + |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 242 | getText(Result, *RHS)) |
| 243 | .str(), |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 244 | NeedsStaticCast)); |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 245 | |
| 246 | StringRef Text = getText(Result, *E); |
| 247 | if (!NeedsStaticCast && needsParensAfterUnaryNegation(E)) |
| 248 | return ("!(" + Text + ")").str(); |
| 249 | |
| 250 | if (needsNullPtrComparison(E)) |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 251 | return compareExpressionToNullPtr(Result, E, false); |
| 252 | |
| 253 | if (needsZeroComparison(E)) |
| 254 | return compareExpressionToZero(Result, E, false); |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 255 | |
| 256 | return ("!" + asBool(Text, NeedsStaticCast)); |
| 257 | } |
| 258 | |
| 259 | if (const auto *UnOp = dyn_cast<UnaryOperator>(E)) { |
| 260 | if (UnOp->getOpcode() == UO_LNot) { |
| 261 | if (needsNullPtrComparison(UnOp->getSubExpr())) |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 262 | return compareExpressionToNullPtr(Result, UnOp->getSubExpr(), false); |
| 263 | |
| 264 | if (needsZeroComparison(UnOp->getSubExpr())) |
| 265 | return compareExpressionToZero(Result, UnOp->getSubExpr(), false); |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 266 | } |
| 267 | } |
| 268 | |
| 269 | if (needsNullPtrComparison(E)) |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 270 | return compareExpressionToNullPtr(Result, E, true); |
| 271 | |
| 272 | if (needsZeroComparison(E)) |
| 273 | return compareExpressionToZero(Result, E, true); |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 274 | |
| 275 | return asBool(getText(Result, *E), NeedsStaticCast); |
| 276 | } |
| 277 | |
| 278 | const CXXBoolLiteralExpr *stmtReturnsBool(const ReturnStmt *Ret, bool Negated) { |
| 279 | if (const auto *Bool = dyn_cast<CXXBoolLiteralExpr>(Ret->getRetValue())) { |
| 280 | if (Bool->getValue() == !Negated) |
| 281 | return Bool; |
| 282 | } |
| 283 | |
| 284 | return nullptr; |
| 285 | } |
| 286 | |
| 287 | const CXXBoolLiteralExpr *stmtReturnsBool(const IfStmt *IfRet, bool Negated) { |
| 288 | if (IfRet->getElse() != nullptr) |
| 289 | return nullptr; |
| 290 | |
| 291 | if (const auto *Ret = dyn_cast<ReturnStmt>(IfRet->getThen())) |
| 292 | return stmtReturnsBool(Ret, Negated); |
| 293 | |
| 294 | if (const auto *Compound = dyn_cast<CompoundStmt>(IfRet->getThen())) { |
| 295 | if (Compound->size() == 1) { |
| 296 | if (const auto *CompoundRet = dyn_cast<ReturnStmt>(Compound->body_back())) |
| 297 | return stmtReturnsBool(CompoundRet, Negated); |
| 298 | } |
| 299 | } |
| 300 | |
| 301 | return nullptr; |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 302 | } |
| 303 | |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 304 | bool containsDiscardedTokens(const MatchFinder::MatchResult &Result, |
| 305 | CharSourceRange CharRange) { |
| 306 | std::string ReplacementText = |
| 307 | Lexer::getSourceText(CharRange, *Result.SourceManager, |
| 308 | Result.Context->getLangOpts()) |
| 309 | .str(); |
| 310 | Lexer Lex(CharRange.getBegin(), Result.Context->getLangOpts(), |
| 311 | ReplacementText.data(), ReplacementText.data(), |
| 312 | ReplacementText.data() + ReplacementText.size()); |
| 313 | Lex.SetCommentRetentionState(true); |
| 314 | |
| 315 | Token Tok; |
| 316 | while (!Lex.LexFromRawLexer(Tok)) { |
| 317 | if (Tok.is(tok::TokenKind::comment) || Tok.is(tok::TokenKind::hash)) |
| 318 | return true; |
| 319 | } |
| 320 | |
| 321 | return false; |
| 322 | } |
| 323 | |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 324 | } // namespace |
| 325 | |
Alexander Kornienko | fb3e2cd | 2015-05-17 12:31:12 +0000 | [diff] [blame] | 326 | SimplifyBooleanExprCheck::SimplifyBooleanExprCheck(StringRef Name, |
| 327 | ClangTidyContext *Context) |
| 328 | : ClangTidyCheck(Name, Context), |
| 329 | ChainedConditionalReturn(Options.get("ChainedConditionalReturn", 0U)), |
| 330 | ChainedConditionalAssignment( |
| 331 | Options.get("ChainedConditionalAssignment", 0U)) {} |
| 332 | |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 333 | void SimplifyBooleanExprCheck::matchBoolBinOpExpr(MatchFinder *Finder, |
| 334 | bool Value, |
| 335 | StringRef OperatorName, |
| 336 | StringRef BooleanId) { |
| 337 | Finder->addMatcher( |
Aaron Ballman | b9ea09c | 2015-09-17 13:31:25 +0000 | [diff] [blame] | 338 | binaryOperator( |
| 339 | isExpansionInMainFile(), hasOperatorName(OperatorName), |
| 340 | hasLHS(allOf(expr().bind(LHSId), |
| 341 | cxxBoolLiteral(equals(Value)).bind(BooleanId))), |
| 342 | hasRHS(expr().bind(RHSId)), |
| 343 | unless(hasRHS(hasDescendant(cxxBoolLiteral())))), |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 344 | this); |
| 345 | } |
| 346 | |
| 347 | void SimplifyBooleanExprCheck::matchExprBinOpBool(MatchFinder *Finder, |
| 348 | bool Value, |
| 349 | StringRef OperatorName, |
| 350 | StringRef BooleanId) { |
| 351 | Finder->addMatcher( |
| 352 | binaryOperator( |
| 353 | isExpansionInMainFile(), hasOperatorName(OperatorName), |
| 354 | hasLHS(expr().bind(LHSId)), |
Aaron Ballman | b9ea09c | 2015-09-17 13:31:25 +0000 | [diff] [blame] | 355 | unless( |
| 356 | hasLHS(anyOf(cxxBoolLiteral(), hasDescendant(cxxBoolLiteral())))), |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 357 | hasRHS(allOf(expr().bind(RHSId), |
Aaron Ballman | b9ea09c | 2015-09-17 13:31:25 +0000 | [diff] [blame] | 358 | cxxBoolLiteral(equals(Value)).bind(BooleanId)))), |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 359 | this); |
| 360 | } |
| 361 | |
| 362 | void SimplifyBooleanExprCheck::matchBoolCompOpExpr(MatchFinder *Finder, |
| 363 | bool Value, |
| 364 | StringRef OperatorName, |
| 365 | StringRef BooleanId) { |
| 366 | Finder->addMatcher( |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 367 | binaryOperator( |
| 368 | isExpansionInMainFile(), hasOperatorName(OperatorName), |
| 369 | hasLHS(allOf( |
| 370 | expr().bind(LHSId), |
| 371 | ignoringImpCasts(cxxBoolLiteral(equals(Value)).bind(BooleanId)))), |
| 372 | hasRHS(expr().bind(RHSId)), |
| 373 | unless(hasRHS(hasDescendant(cxxBoolLiteral())))), |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 374 | this); |
| 375 | } |
| 376 | |
| 377 | void SimplifyBooleanExprCheck::matchExprCompOpBool(MatchFinder *Finder, |
| 378 | bool Value, |
| 379 | StringRef OperatorName, |
| 380 | StringRef BooleanId) { |
| 381 | Finder->addMatcher( |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 382 | binaryOperator( |
| 383 | isExpansionInMainFile(), hasOperatorName(OperatorName), |
| 384 | unless(hasLHS(hasDescendant(cxxBoolLiteral()))), |
| 385 | hasLHS(expr().bind(LHSId)), |
| 386 | hasRHS(allOf(expr().bind(RHSId), |
| 387 | ignoringImpCasts( |
| 388 | cxxBoolLiteral(equals(Value)).bind(BooleanId))))), |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 389 | this); |
| 390 | } |
| 391 | |
| 392 | void SimplifyBooleanExprCheck::matchBoolCondition(MatchFinder *Finder, |
| 393 | bool Value, |
| 394 | StringRef BooleanId) { |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 395 | Finder->addMatcher( |
| 396 | ifStmt(isExpansionInMainFile(), |
| 397 | hasCondition(cxxBoolLiteral(equals(Value)).bind(BooleanId))) |
| 398 | .bind(IfStmtId), |
| 399 | this); |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 400 | } |
| 401 | |
| 402 | void SimplifyBooleanExprCheck::matchTernaryResult(MatchFinder *Finder, |
| 403 | bool Value, |
| 404 | StringRef TernaryId) { |
| 405 | Finder->addMatcher( |
| 406 | conditionalOperator(isExpansionInMainFile(), |
Aaron Ballman | b9ea09c | 2015-09-17 13:31:25 +0000 | [diff] [blame] | 407 | hasTrueExpression(cxxBoolLiteral(equals(Value))), |
| 408 | hasFalseExpression(cxxBoolLiteral(equals(!Value)))) |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 409 | .bind(TernaryId), |
| 410 | this); |
| 411 | } |
| 412 | |
| 413 | void SimplifyBooleanExprCheck::matchIfReturnsBool(MatchFinder *Finder, |
| 414 | bool Value, StringRef Id) { |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 415 | if (ChainedConditionalReturn) |
Alexander Kornienko | fb3e2cd | 2015-05-17 12:31:12 +0000 | [diff] [blame] | 416 | Finder->addMatcher(ifStmt(isExpansionInMainFile(), |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 417 | hasThen(returnsBool(Value, ThenLiteralId)), |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 418 | hasElse(returnsBool(!Value))) |
| 419 | .bind(Id), |
Alexander Kornienko | fb3e2cd | 2015-05-17 12:31:12 +0000 | [diff] [blame] | 420 | this); |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 421 | else |
Alexander Kornienko | fb3e2cd | 2015-05-17 12:31:12 +0000 | [diff] [blame] | 422 | Finder->addMatcher(ifStmt(isExpansionInMainFile(), |
| 423 | unless(hasParent(ifStmt())), |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 424 | hasThen(returnsBool(Value, ThenLiteralId)), |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 425 | hasElse(returnsBool(!Value))) |
| 426 | .bind(Id), |
Alexander Kornienko | fb3e2cd | 2015-05-17 12:31:12 +0000 | [diff] [blame] | 427 | this); |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 428 | } |
| 429 | |
| 430 | void SimplifyBooleanExprCheck::matchIfAssignsBool(MatchFinder *Finder, |
| 431 | bool Value, StringRef Id) { |
| 432 | auto SimpleThen = binaryOperator( |
| 433 | hasOperatorName("="), |
| 434 | hasLHS(declRefExpr(hasDeclaration(decl().bind(IfAssignObjId)))), |
| 435 | hasLHS(expr().bind(IfAssignVariableId)), |
Aaron Ballman | b9ea09c | 2015-09-17 13:31:25 +0000 | [diff] [blame] | 436 | hasRHS(cxxBoolLiteral(equals(Value)).bind(IfAssignLocId))); |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 437 | auto Then = anyOf(SimpleThen, compoundStmt(statementCountIs(1), |
| 438 | hasAnySubstatement(SimpleThen))); |
| 439 | auto SimpleElse = binaryOperator( |
| 440 | hasOperatorName("="), |
| 441 | hasLHS(declRefExpr(hasDeclaration(equalsBoundNode(IfAssignObjId)))), |
Aaron Ballman | b9ea09c | 2015-09-17 13:31:25 +0000 | [diff] [blame] | 442 | hasRHS(cxxBoolLiteral(equals(!Value)))); |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 443 | auto Else = anyOf(SimpleElse, compoundStmt(statementCountIs(1), |
| 444 | hasAnySubstatement(SimpleElse))); |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 445 | if (ChainedConditionalAssignment) |
Alexander Kornienko | fb3e2cd | 2015-05-17 12:31:12 +0000 | [diff] [blame] | 446 | Finder->addMatcher( |
| 447 | ifStmt(isExpansionInMainFile(), hasThen(Then), hasElse(Else)).bind(Id), |
| 448 | this); |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 449 | else |
Alexander Kornienko | fb3e2cd | 2015-05-17 12:31:12 +0000 | [diff] [blame] | 450 | Finder->addMatcher(ifStmt(isExpansionInMainFile(), |
| 451 | unless(hasParent(ifStmt())), hasThen(Then), |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 452 | hasElse(Else)) |
| 453 | .bind(Id), |
Alexander Kornienko | fb3e2cd | 2015-05-17 12:31:12 +0000 | [diff] [blame] | 454 | this); |
Alexander Kornienko | fb3e2cd | 2015-05-17 12:31:12 +0000 | [diff] [blame] | 455 | } |
| 456 | |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 457 | void SimplifyBooleanExprCheck::matchCompoundIfReturnsBool(MatchFinder *Finder, |
| 458 | bool Value, |
| 459 | StringRef Id) { |
| 460 | Finder->addMatcher( |
Aaron Ballman | b9ea09c | 2015-09-17 13:31:25 +0000 | [diff] [blame] | 461 | compoundStmt(allOf(hasAnySubstatement(ifStmt(hasThen(returnsBool(Value)), |
| 462 | unless(hasElse(stmt())))), |
| 463 | hasAnySubstatement( |
Piotr Padlewski | e93a73f | 2016-05-31 15:26:56 +0000 | [diff] [blame] | 464 | returnStmt(has(ignoringParenImpCasts( |
| 465 | cxxBoolLiteral(equals(!Value))))) |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 466 | .bind(CompoundReturnId)))) |
| 467 | .bind(Id), |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 468 | this); |
| 469 | } |
| 470 | |
Alexander Kornienko | fb3e2cd | 2015-05-17 12:31:12 +0000 | [diff] [blame] | 471 | void SimplifyBooleanExprCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { |
| 472 | Options.store(Opts, "ChainedConditionalReturn", ChainedConditionalReturn); |
| 473 | Options.store(Opts, "ChainedConditionalAssignment", |
| 474 | ChainedConditionalAssignment); |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 475 | } |
| 476 | |
| 477 | void SimplifyBooleanExprCheck::registerMatchers(MatchFinder *Finder) { |
| 478 | matchBoolBinOpExpr(Finder, true, "&&", RightExpressionId); |
| 479 | matchBoolBinOpExpr(Finder, false, "||", RightExpressionId); |
| 480 | matchExprBinOpBool(Finder, false, "&&", RightExpressionId); |
| 481 | matchExprBinOpBool(Finder, true, "||", RightExpressionId); |
| 482 | matchBoolCompOpExpr(Finder, true, "==", RightExpressionId); |
| 483 | matchBoolCompOpExpr(Finder, false, "!=", RightExpressionId); |
| 484 | |
| 485 | matchExprBinOpBool(Finder, true, "&&", LeftExpressionId); |
| 486 | matchExprBinOpBool(Finder, false, "||", LeftExpressionId); |
| 487 | matchBoolBinOpExpr(Finder, false, "&&", LeftExpressionId); |
| 488 | matchBoolBinOpExpr(Finder, true, "||", LeftExpressionId); |
| 489 | matchExprCompOpBool(Finder, true, "==", LeftExpressionId); |
| 490 | matchExprCompOpBool(Finder, false, "!=", LeftExpressionId); |
| 491 | |
| 492 | matchBoolCompOpExpr(Finder, false, "==", NegatedRightExpressionId); |
| 493 | matchBoolCompOpExpr(Finder, true, "!=", NegatedRightExpressionId); |
| 494 | |
| 495 | matchExprCompOpBool(Finder, false, "==", NegatedLeftExpressionId); |
| 496 | matchExprCompOpBool(Finder, true, "!=", NegatedLeftExpressionId); |
| 497 | |
| 498 | matchBoolCondition(Finder, true, ConditionThenStmtId); |
| 499 | matchBoolCondition(Finder, false, ConditionElseStmtId); |
| 500 | |
| 501 | matchTernaryResult(Finder, true, TernaryId); |
| 502 | matchTernaryResult(Finder, false, TernaryNegatedId); |
| 503 | |
| 504 | matchIfReturnsBool(Finder, true, IfReturnsBoolId); |
| 505 | matchIfReturnsBool(Finder, false, IfReturnsNotBoolId); |
| 506 | |
| 507 | matchIfAssignsBool(Finder, true, IfAssignBoolId); |
| 508 | matchIfAssignsBool(Finder, false, IfAssignNotBoolId); |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 509 | |
| 510 | matchCompoundIfReturnsBool(Finder, true, CompoundBoolId); |
| 511 | matchCompoundIfReturnsBool(Finder, false, CompoundNotBoolId); |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 512 | } |
| 513 | |
| 514 | void SimplifyBooleanExprCheck::check(const MatchFinder::MatchResult &Result) { |
| 515 | if (const CXXBoolLiteralExpr *LeftRemoved = |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 516 | getBoolLiteral(Result, RightExpressionId)) |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 517 | replaceWithExpression(Result, LeftRemoved, false); |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 518 | else if (const CXXBoolLiteralExpr *RightRemoved = |
| 519 | getBoolLiteral(Result, LeftExpressionId)) |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 520 | replaceWithExpression(Result, RightRemoved, true); |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 521 | else if (const CXXBoolLiteralExpr *NegatedLeftRemoved = |
| 522 | getBoolLiteral(Result, NegatedRightExpressionId)) |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 523 | replaceWithExpression(Result, NegatedLeftRemoved, false, true); |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 524 | else if (const CXXBoolLiteralExpr *NegatedRightRemoved = |
| 525 | getBoolLiteral(Result, NegatedLeftExpressionId)) |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 526 | replaceWithExpression(Result, NegatedRightRemoved, true, true); |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 527 | else if (const CXXBoolLiteralExpr *TrueConditionRemoved = |
| 528 | getBoolLiteral(Result, ConditionThenStmtId)) |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 529 | replaceWithThenStatement(Result, TrueConditionRemoved); |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 530 | else if (const CXXBoolLiteralExpr *FalseConditionRemoved = |
| 531 | getBoolLiteral(Result, ConditionElseStmtId)) |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 532 | replaceWithElseStatement(Result, FalseConditionRemoved); |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 533 | else if (const auto *Ternary = |
| 534 | Result.Nodes.getNodeAs<ConditionalOperator>(TernaryId)) |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 535 | replaceWithCondition(Result, Ternary); |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 536 | else if (const auto *TernaryNegated = |
| 537 | Result.Nodes.getNodeAs<ConditionalOperator>(TernaryNegatedId)) |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 538 | replaceWithCondition(Result, TernaryNegated, true); |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 539 | else if (const auto *If = Result.Nodes.getNodeAs<IfStmt>(IfReturnsBoolId)) |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 540 | replaceWithReturnCondition(Result, If); |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 541 | else if (const auto *IfNot = |
| 542 | Result.Nodes.getNodeAs<IfStmt>(IfReturnsNotBoolId)) |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 543 | replaceWithReturnCondition(Result, IfNot, true); |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 544 | else if (const auto *IfAssign = |
| 545 | Result.Nodes.getNodeAs<IfStmt>(IfAssignBoolId)) |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 546 | replaceWithAssignment(Result, IfAssign); |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 547 | else if (const auto *IfAssignNot = |
| 548 | Result.Nodes.getNodeAs<IfStmt>(IfAssignNotBoolId)) |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 549 | replaceWithAssignment(Result, IfAssignNot, true); |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 550 | else if (const auto *Compound = |
| 551 | Result.Nodes.getNodeAs<CompoundStmt>(CompoundBoolId)) |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 552 | replaceCompoundReturnWithCondition(Result, Compound); |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 553 | else if (const auto *Compound = |
| 554 | Result.Nodes.getNodeAs<CompoundStmt>(CompoundNotBoolId)) |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 555 | replaceCompoundReturnWithCondition(Result, Compound, true); |
Alexander Kornienko | 4f74ec0 | 2015-12-28 13:21:22 +0000 | [diff] [blame] | 556 | } |
| 557 | |
| 558 | void SimplifyBooleanExprCheck::issueDiag( |
| 559 | const ast_matchers::MatchFinder::MatchResult &Result, SourceLocation Loc, |
| 560 | StringRef Description, SourceRange ReplacementRange, |
| 561 | StringRef Replacement) { |
Gabor Horvath | afad84c | 2016-09-24 02:13:45 +0000 | [diff] [blame] | 562 | CharSourceRange CharRange = |
| 563 | Lexer::makeFileCharRange(CharSourceRange::getTokenRange(ReplacementRange), |
| 564 | *Result.SourceManager, getLangOpts()); |
Alexander Kornienko | 4f74ec0 | 2015-12-28 13:21:22 +0000 | [diff] [blame] | 565 | |
| 566 | DiagnosticBuilder Diag = diag(Loc, Description); |
| 567 | if (!containsDiscardedTokens(Result, CharRange)) |
| 568 | Diag << FixItHint::CreateReplacement(CharRange, Replacement); |
| 569 | } |
| 570 | |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 571 | void SimplifyBooleanExprCheck::replaceWithExpression( |
| 572 | const ast_matchers::MatchFinder::MatchResult &Result, |
| 573 | const CXXBoolLiteralExpr *BoolLiteral, bool UseLHS, bool Negated) { |
| 574 | const auto *LHS = Result.Nodes.getNodeAs<Expr>(LHSId); |
| 575 | const auto *RHS = Result.Nodes.getNodeAs<Expr>(RHSId); |
| 576 | std::string Replacement = |
| 577 | replacementExpression(Result, Negated, UseLHS ? LHS : RHS); |
Alexander Kornienko | 4f74ec0 | 2015-12-28 13:21:22 +0000 | [diff] [blame] | 578 | SourceRange Range(LHS->getLocStart(), RHS->getLocEnd()); |
| 579 | issueDiag(Result, BoolLiteral->getLocStart(), SimplifyOperatorDiagnostic, |
| 580 | Range, Replacement); |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 581 | } |
| 582 | |
| 583 | void SimplifyBooleanExprCheck::replaceWithThenStatement( |
| 584 | const MatchFinder::MatchResult &Result, |
| 585 | const CXXBoolLiteralExpr *TrueConditionRemoved) { |
| 586 | const auto *IfStatement = Result.Nodes.getNodeAs<IfStmt>(IfStmtId); |
Alexander Kornienko | 4f74ec0 | 2015-12-28 13:21:22 +0000 | [diff] [blame] | 587 | issueDiag(Result, TrueConditionRemoved->getLocStart(), |
| 588 | SimplifyConditionDiagnostic, IfStatement->getSourceRange(), |
| 589 | getText(Result, *IfStatement->getThen())); |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 590 | } |
| 591 | |
| 592 | void SimplifyBooleanExprCheck::replaceWithElseStatement( |
| 593 | const MatchFinder::MatchResult &Result, |
| 594 | const CXXBoolLiteralExpr *FalseConditionRemoved) { |
| 595 | const auto *IfStatement = Result.Nodes.getNodeAs<IfStmt>(IfStmtId); |
| 596 | const Stmt *ElseStatement = IfStatement->getElse(); |
Alexander Kornienko | 4f74ec0 | 2015-12-28 13:21:22 +0000 | [diff] [blame] | 597 | issueDiag(Result, FalseConditionRemoved->getLocStart(), |
| 598 | SimplifyConditionDiagnostic, IfStatement->getSourceRange(), |
| 599 | ElseStatement ? getText(Result, *ElseStatement) : ""); |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 600 | } |
| 601 | |
| 602 | void SimplifyBooleanExprCheck::replaceWithCondition( |
| 603 | const MatchFinder::MatchResult &Result, const ConditionalOperator *Ternary, |
| 604 | bool Negated) { |
| 605 | std::string Replacement = |
| 606 | replacementExpression(Result, Negated, Ternary->getCond()); |
Alexander Kornienko | 4f74ec0 | 2015-12-28 13:21:22 +0000 | [diff] [blame] | 607 | issueDiag(Result, Ternary->getTrueExpr()->getLocStart(), |
| 608 | "redundant boolean literal in ternary expression result", |
| 609 | Ternary->getSourceRange(), Replacement); |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 610 | } |
| 611 | |
| 612 | void SimplifyBooleanExprCheck::replaceWithReturnCondition( |
| 613 | const MatchFinder::MatchResult &Result, const IfStmt *If, bool Negated) { |
| 614 | StringRef Terminator = isa<CompoundStmt>(If->getElse()) ? ";" : ""; |
| 615 | std::string Condition = replacementExpression(Result, Negated, If->getCond()); |
| 616 | std::string Replacement = ("return " + Condition + Terminator).str(); |
| 617 | SourceLocation Start = |
| 618 | Result.Nodes.getNodeAs<CXXBoolLiteralExpr>(ThenLiteralId)->getLocStart(); |
Alexander Kornienko | 4f74ec0 | 2015-12-28 13:21:22 +0000 | [diff] [blame] | 619 | issueDiag(Result, Start, SimplifyConditionalReturnDiagnostic, |
| 620 | If->getSourceRange(), Replacement); |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 621 | } |
| 622 | |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 623 | void SimplifyBooleanExprCheck::replaceCompoundReturnWithCondition( |
| 624 | const MatchFinder::MatchResult &Result, const CompoundStmt *Compound, |
| 625 | bool Negated) { |
| 626 | const auto *Ret = Result.Nodes.getNodeAs<ReturnStmt>(CompoundReturnId); |
| 627 | |
| 628 | // The body shouldn't be empty because the matcher ensures that it must |
| 629 | // contain at least two statements: |
| 630 | // 1) A `return` statement returning a boolean literal `false` or `true` |
Aaron Ballman | f034a8c | 2016-02-12 15:09:05 +0000 | [diff] [blame] | 631 | // 2) An `if` statement with no `else` clause that consists of a single |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 632 | // `return` statement returning the opposite boolean literal `true` or |
| 633 | // `false`. |
| 634 | assert(Compound->size() >= 2); |
| 635 | const IfStmt *BeforeIf = nullptr; |
| 636 | CompoundStmt::const_body_iterator Current = Compound->body_begin(); |
| 637 | CompoundStmt::const_body_iterator After = Compound->body_begin(); |
| 638 | for (++After; After != Compound->body_end() && *Current != Ret; |
| 639 | ++Current, ++After) { |
| 640 | if (const auto *If = dyn_cast<IfStmt>(*Current)) { |
| 641 | if (const CXXBoolLiteralExpr *Lit = stmtReturnsBool(If, Negated)) { |
| 642 | if (*After == Ret) { |
| 643 | if (!ChainedConditionalReturn && BeforeIf) |
| 644 | continue; |
| 645 | |
| 646 | const Expr *Condition = If->getCond(); |
| 647 | std::string Replacement = |
| 648 | "return " + replacementExpression(Result, Negated, Condition); |
Alexander Kornienko | 4f74ec0 | 2015-12-28 13:21:22 +0000 | [diff] [blame] | 649 | issueDiag( |
| 650 | Result, Lit->getLocStart(), SimplifyConditionalReturnDiagnostic, |
| 651 | SourceRange(If->getLocStart(), Ret->getLocEnd()), Replacement); |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 652 | return; |
| 653 | } |
| 654 | |
| 655 | BeforeIf = If; |
| 656 | } |
| 657 | } else { |
| 658 | BeforeIf = nullptr; |
| 659 | } |
| 660 | } |
| 661 | } |
| 662 | |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 663 | void SimplifyBooleanExprCheck::replaceWithAssignment( |
| 664 | const MatchFinder::MatchResult &Result, const IfStmt *IfAssign, |
| 665 | bool Negated) { |
| 666 | SourceRange Range = IfAssign->getSourceRange(); |
| 667 | StringRef VariableName = |
| 668 | getText(Result, *Result.Nodes.getNodeAs<Expr>(IfAssignVariableId)); |
| 669 | StringRef Terminator = isa<CompoundStmt>(IfAssign->getElse()) ? ";" : ""; |
| 670 | std::string Condition = |
| 671 | replacementExpression(Result, Negated, IfAssign->getCond()); |
| 672 | std::string Replacement = |
| 673 | (VariableName + " = " + Condition + Terminator).str(); |
| 674 | SourceLocation Location = |
| 675 | Result.Nodes.getNodeAs<CXXBoolLiteralExpr>(IfAssignLocId)->getLocStart(); |
Alexander Kornienko | 4f74ec0 | 2015-12-28 13:21:22 +0000 | [diff] [blame] | 676 | issueDiag(Result, Location, |
| 677 | "redundant boolean literal in conditional assignment", Range, |
| 678 | Replacement); |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 679 | } |
| 680 | |
| 681 | } // namespace readability |
| 682 | } // namespace tidy |
| 683 | } // namespace clang |