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; |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 88 | if (const auto *Op = dyn_cast<CXXOperatorCallExpr>(E)) { |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 89 | return Op->getNumArgs() == 2 && Op->getOperator() != OO_Call && |
| 90 | Op->getOperator() != OO_Subscript; |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 91 | } |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 92 | return false; |
| 93 | } |
| 94 | |
| 95 | std::pair<BinaryOperatorKind, BinaryOperatorKind> Opposites[] = { |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 96 | {BO_LT, BO_GE}, {BO_GT, BO_LE}, {BO_EQ, BO_NE}}; |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 97 | |
| 98 | StringRef negatedOperator(const BinaryOperator *BinOp) { |
| 99 | const BinaryOperatorKind Opcode = BinOp->getOpcode(); |
| 100 | for (auto NegatableOp : Opposites) { |
| 101 | if (Opcode == NegatableOp.first) |
| 102 | return BinOp->getOpcodeStr(NegatableOp.second); |
| 103 | if (Opcode == NegatableOp.second) |
| 104 | return BinOp->getOpcodeStr(NegatableOp.first); |
| 105 | } |
| 106 | return StringRef(); |
| 107 | } |
| 108 | |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 109 | std::pair<OverloadedOperatorKind, StringRef> OperatorNames[] = { |
| 110 | {OO_EqualEqual, "=="}, {OO_ExclaimEqual, "!="}, {OO_Less, "<"}, |
| 111 | {OO_GreaterEqual, ">="}, {OO_Greater, ">"}, {OO_LessEqual, "<="}}; |
| 112 | |
| 113 | StringRef getOperatorName(OverloadedOperatorKind OpKind) { |
| 114 | for (auto Name : OperatorNames) { |
| 115 | if (Name.first == OpKind) |
| 116 | return Name.second; |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 117 | } |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 118 | |
| 119 | return StringRef(); |
| 120 | } |
| 121 | |
| 122 | std::pair<OverloadedOperatorKind, OverloadedOperatorKind> OppositeOverloads[] = |
| 123 | {{OO_EqualEqual, OO_ExclaimEqual}, |
| 124 | {OO_Less, OO_GreaterEqual}, |
| 125 | {OO_Greater, OO_LessEqual}}; |
| 126 | |
| 127 | StringRef negatedOperator(const CXXOperatorCallExpr *OpCall) { |
| 128 | const OverloadedOperatorKind Opcode = OpCall->getOperator(); |
| 129 | for (auto NegatableOp : OppositeOverloads) { |
| 130 | if (Opcode == NegatableOp.first) |
| 131 | return getOperatorName(NegatableOp.second); |
| 132 | if (Opcode == NegatableOp.second) |
| 133 | return getOperatorName(NegatableOp.first); |
| 134 | } |
| 135 | return StringRef(); |
| 136 | } |
| 137 | |
| 138 | std::string asBool(StringRef text, bool NeedsStaticCast) { |
| 139 | if (NeedsStaticCast) |
| 140 | return ("static_cast<bool>(" + text + ")").str(); |
| 141 | |
| 142 | return text; |
| 143 | } |
| 144 | |
| 145 | bool needsNullPtrComparison(const Expr *E) { |
| 146 | if (const auto *ImpCast = dyn_cast<ImplicitCastExpr>(E)) |
| 147 | return ImpCast->getCastKind() == CK_PointerToBoolean; |
| 148 | |
| 149 | return false; |
| 150 | } |
| 151 | |
| 152 | bool needsStaticCast(const Expr *E) { |
| 153 | if (const auto *ImpCast = dyn_cast<ImplicitCastExpr>(E)) { |
| 154 | if (ImpCast->getCastKind() == CK_UserDefinedConversion && |
| 155 | ImpCast->getSubExpr()->getType()->isBooleanType()) { |
| 156 | if (const auto *MemCall = |
| 157 | dyn_cast<CXXMemberCallExpr>(ImpCast->getSubExpr())) { |
| 158 | if (const auto *MemDecl = |
| 159 | dyn_cast<CXXConversionDecl>(MemCall->getMethodDecl())) { |
| 160 | if (MemDecl->isExplicit()) |
| 161 | return true; |
| 162 | } |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 163 | } |
| 164 | } |
| 165 | } |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 166 | |
| 167 | E = E->IgnoreImpCasts(); |
| 168 | return !E->getType()->isBooleanType(); |
| 169 | } |
| 170 | |
| 171 | std::string replacementExpression(const MatchFinder::MatchResult &Result, |
| 172 | bool Negated, const Expr *E) { |
| 173 | E = E->ignoreParenBaseCasts(); |
| 174 | const bool NeedsStaticCast = needsStaticCast(E); |
| 175 | if (Negated) { |
| 176 | if (const auto *UnOp = dyn_cast<UnaryOperator>(E)) { |
| 177 | if (UnOp->getOpcode() == UO_LNot) { |
| 178 | if (needsNullPtrComparison(UnOp->getSubExpr())) |
| 179 | return (getText(Result, *UnOp->getSubExpr()) + " != nullptr").str(); |
| 180 | |
| 181 | return replacementExpression(Result, false, UnOp->getSubExpr()); |
| 182 | } |
| 183 | } |
| 184 | |
| 185 | if (needsNullPtrComparison(E)) |
| 186 | return (getText(Result, *E) + " == nullptr").str(); |
| 187 | |
| 188 | StringRef NegatedOperator; |
| 189 | const Expr *LHS = nullptr; |
| 190 | const Expr *RHS = nullptr; |
| 191 | if (const auto *BinOp = dyn_cast<BinaryOperator>(E)) { |
| 192 | NegatedOperator = negatedOperator(BinOp); |
| 193 | LHS = BinOp->getLHS(); |
| 194 | RHS = BinOp->getRHS(); |
| 195 | } else if (const auto *OpExpr = dyn_cast<CXXOperatorCallExpr>(E)) { |
| 196 | if (OpExpr->getNumArgs() == 2) { |
| 197 | NegatedOperator = negatedOperator(OpExpr); |
| 198 | LHS = OpExpr->getArg(0); |
| 199 | RHS = OpExpr->getArg(1); |
| 200 | } |
| 201 | } |
| 202 | if (!NegatedOperator.empty() && LHS && RHS) { |
| 203 | return (asBool((getText(Result, *LHS) + " " + NegatedOperator + " " + |
| 204 | getText(Result, *RHS)) |
| 205 | .str(), |
| 206 | NeedsStaticCast)); |
| 207 | } |
| 208 | |
| 209 | StringRef Text = getText(Result, *E); |
| 210 | if (!NeedsStaticCast && needsParensAfterUnaryNegation(E)) |
| 211 | return ("!(" + Text + ")").str(); |
| 212 | |
| 213 | if (needsNullPtrComparison(E)) |
| 214 | return (getText(Result, *E) + " == nullptr").str(); |
| 215 | |
| 216 | return ("!" + asBool(Text, NeedsStaticCast)); |
| 217 | } |
| 218 | |
| 219 | if (const auto *UnOp = dyn_cast<UnaryOperator>(E)) { |
| 220 | if (UnOp->getOpcode() == UO_LNot) { |
| 221 | if (needsNullPtrComparison(UnOp->getSubExpr())) |
| 222 | return (getText(Result, *UnOp->getSubExpr()) + " == nullptr").str(); |
| 223 | } |
| 224 | } |
| 225 | |
| 226 | if (needsNullPtrComparison(E)) |
| 227 | return (getText(Result, *E) + " != nullptr").str(); |
| 228 | |
| 229 | return asBool(getText(Result, *E), NeedsStaticCast); |
| 230 | } |
| 231 | |
| 232 | const CXXBoolLiteralExpr *stmtReturnsBool(const ReturnStmt *Ret, bool Negated) { |
| 233 | if (const auto *Bool = dyn_cast<CXXBoolLiteralExpr>(Ret->getRetValue())) { |
| 234 | if (Bool->getValue() == !Negated) |
| 235 | return Bool; |
| 236 | } |
| 237 | |
| 238 | return nullptr; |
| 239 | } |
| 240 | |
| 241 | const CXXBoolLiteralExpr *stmtReturnsBool(const IfStmt *IfRet, bool Negated) { |
| 242 | if (IfRet->getElse() != nullptr) |
| 243 | return nullptr; |
| 244 | |
| 245 | if (const auto *Ret = dyn_cast<ReturnStmt>(IfRet->getThen())) |
| 246 | return stmtReturnsBool(Ret, Negated); |
| 247 | |
| 248 | if (const auto *Compound = dyn_cast<CompoundStmt>(IfRet->getThen())) { |
| 249 | if (Compound->size() == 1) { |
| 250 | if (const auto *CompoundRet = dyn_cast<ReturnStmt>(Compound->body_back())) |
| 251 | return stmtReturnsBool(CompoundRet, Negated); |
| 252 | } |
| 253 | } |
| 254 | |
| 255 | return nullptr; |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 256 | } |
| 257 | |
| 258 | } // namespace |
| 259 | |
Alexander Kornienko | fb3e2cd | 2015-05-17 12:31:12 +0000 | [diff] [blame] | 260 | SimplifyBooleanExprCheck::SimplifyBooleanExprCheck(StringRef Name, |
| 261 | ClangTidyContext *Context) |
| 262 | : ClangTidyCheck(Name, Context), |
| 263 | ChainedConditionalReturn(Options.get("ChainedConditionalReturn", 0U)), |
| 264 | ChainedConditionalAssignment( |
| 265 | Options.get("ChainedConditionalAssignment", 0U)) {} |
| 266 | |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 267 | void SimplifyBooleanExprCheck::matchBoolBinOpExpr(MatchFinder *Finder, |
| 268 | bool Value, |
| 269 | StringRef OperatorName, |
| 270 | StringRef BooleanId) { |
| 271 | Finder->addMatcher( |
Aaron Ballman | b9ea09c | 2015-09-17 13:31:25 +0000 | [diff] [blame] | 272 | binaryOperator( |
| 273 | isExpansionInMainFile(), hasOperatorName(OperatorName), |
| 274 | hasLHS(allOf(expr().bind(LHSId), |
| 275 | cxxBoolLiteral(equals(Value)).bind(BooleanId))), |
| 276 | hasRHS(expr().bind(RHSId)), |
| 277 | unless(hasRHS(hasDescendant(cxxBoolLiteral())))), |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 278 | this); |
| 279 | } |
| 280 | |
| 281 | void SimplifyBooleanExprCheck::matchExprBinOpBool(MatchFinder *Finder, |
| 282 | bool Value, |
| 283 | StringRef OperatorName, |
| 284 | StringRef BooleanId) { |
| 285 | Finder->addMatcher( |
| 286 | binaryOperator( |
| 287 | isExpansionInMainFile(), hasOperatorName(OperatorName), |
| 288 | hasLHS(expr().bind(LHSId)), |
Aaron Ballman | b9ea09c | 2015-09-17 13:31:25 +0000 | [diff] [blame] | 289 | unless( |
| 290 | hasLHS(anyOf(cxxBoolLiteral(), hasDescendant(cxxBoolLiteral())))), |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 291 | hasRHS(allOf(expr().bind(RHSId), |
Aaron Ballman | b9ea09c | 2015-09-17 13:31:25 +0000 | [diff] [blame] | 292 | cxxBoolLiteral(equals(Value)).bind(BooleanId)))), |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 293 | this); |
| 294 | } |
| 295 | |
| 296 | void SimplifyBooleanExprCheck::matchBoolCompOpExpr(MatchFinder *Finder, |
| 297 | bool Value, |
| 298 | StringRef OperatorName, |
| 299 | StringRef BooleanId) { |
| 300 | Finder->addMatcher( |
| 301 | binaryOperator(isExpansionInMainFile(), hasOperatorName(OperatorName), |
| 302 | hasLHS(allOf(expr().bind(LHSId), |
Aaron Ballman | b9ea09c | 2015-09-17 13:31:25 +0000 | [diff] [blame] | 303 | ignoringImpCasts(cxxBoolLiteral(equals(Value)) |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 304 | .bind(BooleanId)))), |
| 305 | hasRHS(expr().bind(RHSId)), |
Aaron Ballman | b9ea09c | 2015-09-17 13:31:25 +0000 | [diff] [blame] | 306 | unless(hasRHS(hasDescendant(cxxBoolLiteral())))), |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 307 | this); |
| 308 | } |
| 309 | |
| 310 | void SimplifyBooleanExprCheck::matchExprCompOpBool(MatchFinder *Finder, |
| 311 | bool Value, |
| 312 | StringRef OperatorName, |
| 313 | StringRef BooleanId) { |
| 314 | Finder->addMatcher( |
| 315 | binaryOperator(isExpansionInMainFile(), hasOperatorName(OperatorName), |
Aaron Ballman | b9ea09c | 2015-09-17 13:31:25 +0000 | [diff] [blame] | 316 | unless(hasLHS(hasDescendant(cxxBoolLiteral()))), |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 317 | hasLHS(expr().bind(LHSId)), |
| 318 | hasRHS(allOf(expr().bind(RHSId), |
Aaron Ballman | b9ea09c | 2015-09-17 13:31:25 +0000 | [diff] [blame] | 319 | ignoringImpCasts(cxxBoolLiteral(equals(Value)) |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 320 | .bind(BooleanId))))), |
| 321 | this); |
| 322 | } |
| 323 | |
| 324 | void SimplifyBooleanExprCheck::matchBoolCondition(MatchFinder *Finder, |
| 325 | bool Value, |
| 326 | StringRef BooleanId) { |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 327 | Finder->addMatcher( |
| 328 | ifStmt(isExpansionInMainFile(), |
Aaron Ballman | b9ea09c | 2015-09-17 13:31:25 +0000 | [diff] [blame] | 329 | hasCondition(cxxBoolLiteral(equals(Value)).bind(BooleanId))) |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 330 | .bind(IfStmtId), |
| 331 | this); |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 332 | } |
| 333 | |
| 334 | void SimplifyBooleanExprCheck::matchTernaryResult(MatchFinder *Finder, |
| 335 | bool Value, |
| 336 | StringRef TernaryId) { |
| 337 | Finder->addMatcher( |
| 338 | conditionalOperator(isExpansionInMainFile(), |
Aaron Ballman | b9ea09c | 2015-09-17 13:31:25 +0000 | [diff] [blame] | 339 | hasTrueExpression(cxxBoolLiteral(equals(Value))), |
| 340 | hasFalseExpression(cxxBoolLiteral(equals(!Value)))) |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 341 | .bind(TernaryId), |
| 342 | this); |
| 343 | } |
| 344 | |
| 345 | void SimplifyBooleanExprCheck::matchIfReturnsBool(MatchFinder *Finder, |
| 346 | bool Value, StringRef Id) { |
Alexander Kornienko | fb3e2cd | 2015-05-17 12:31:12 +0000 | [diff] [blame] | 347 | if (ChainedConditionalReturn) { |
| 348 | Finder->addMatcher(ifStmt(isExpansionInMainFile(), |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 349 | hasThen(returnsBool(Value, ThenLiteralId)), |
| 350 | hasElse(returnsBool(!Value))) |
| 351 | .bind(Id), |
Alexander Kornienko | fb3e2cd | 2015-05-17 12:31:12 +0000 | [diff] [blame] | 352 | this); |
| 353 | } else { |
| 354 | Finder->addMatcher(ifStmt(isExpansionInMainFile(), |
| 355 | unless(hasParent(ifStmt())), |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 356 | hasThen(returnsBool(Value, ThenLiteralId)), |
| 357 | hasElse(returnsBool(!Value))) |
| 358 | .bind(Id), |
Alexander Kornienko | fb3e2cd | 2015-05-17 12:31:12 +0000 | [diff] [blame] | 359 | this); |
| 360 | } |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 361 | } |
| 362 | |
| 363 | void SimplifyBooleanExprCheck::matchIfAssignsBool(MatchFinder *Finder, |
| 364 | bool Value, StringRef Id) { |
| 365 | auto SimpleThen = binaryOperator( |
| 366 | hasOperatorName("="), |
| 367 | hasLHS(declRefExpr(hasDeclaration(decl().bind(IfAssignObjId)))), |
| 368 | hasLHS(expr().bind(IfAssignVariableId)), |
Aaron Ballman | b9ea09c | 2015-09-17 13:31:25 +0000 | [diff] [blame] | 369 | hasRHS(cxxBoolLiteral(equals(Value)).bind(IfAssignLocId))); |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 370 | auto Then = anyOf(SimpleThen, compoundStmt(statementCountIs(1), |
| 371 | hasAnySubstatement(SimpleThen))); |
| 372 | auto SimpleElse = binaryOperator( |
| 373 | hasOperatorName("="), |
| 374 | hasLHS(declRefExpr(hasDeclaration(equalsBoundNode(IfAssignObjId)))), |
Aaron Ballman | b9ea09c | 2015-09-17 13:31:25 +0000 | [diff] [blame] | 375 | hasRHS(cxxBoolLiteral(equals(!Value)))); |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 376 | auto Else = anyOf(SimpleElse, compoundStmt(statementCountIs(1), |
| 377 | hasAnySubstatement(SimpleElse))); |
Alexander Kornienko | fb3e2cd | 2015-05-17 12:31:12 +0000 | [diff] [blame] | 378 | if (ChainedConditionalAssignment) { |
| 379 | Finder->addMatcher( |
| 380 | ifStmt(isExpansionInMainFile(), hasThen(Then), hasElse(Else)).bind(Id), |
| 381 | this); |
| 382 | } else { |
| 383 | Finder->addMatcher(ifStmt(isExpansionInMainFile(), |
| 384 | unless(hasParent(ifStmt())), hasThen(Then), |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 385 | hasElse(Else)) |
| 386 | .bind(Id), |
Alexander Kornienko | fb3e2cd | 2015-05-17 12:31:12 +0000 | [diff] [blame] | 387 | this); |
| 388 | } |
| 389 | } |
| 390 | |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 391 | void SimplifyBooleanExprCheck::matchCompoundIfReturnsBool(MatchFinder *Finder, |
| 392 | bool Value, |
| 393 | StringRef Id) { |
| 394 | Finder->addMatcher( |
Aaron Ballman | b9ea09c | 2015-09-17 13:31:25 +0000 | [diff] [blame] | 395 | compoundStmt(allOf(hasAnySubstatement(ifStmt(hasThen(returnsBool(Value)), |
| 396 | unless(hasElse(stmt())))), |
| 397 | hasAnySubstatement( |
| 398 | returnStmt(has(cxxBoolLiteral(equals(!Value)))) |
| 399 | .bind(CompoundReturnId)))) |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 400 | .bind(Id), |
| 401 | this); |
| 402 | } |
| 403 | |
Alexander Kornienko | fb3e2cd | 2015-05-17 12:31:12 +0000 | [diff] [blame] | 404 | void SimplifyBooleanExprCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { |
| 405 | Options.store(Opts, "ChainedConditionalReturn", ChainedConditionalReturn); |
| 406 | Options.store(Opts, "ChainedConditionalAssignment", |
| 407 | ChainedConditionalAssignment); |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 408 | } |
| 409 | |
| 410 | void SimplifyBooleanExprCheck::registerMatchers(MatchFinder *Finder) { |
| 411 | matchBoolBinOpExpr(Finder, true, "&&", RightExpressionId); |
| 412 | matchBoolBinOpExpr(Finder, false, "||", RightExpressionId); |
| 413 | matchExprBinOpBool(Finder, false, "&&", RightExpressionId); |
| 414 | matchExprBinOpBool(Finder, true, "||", RightExpressionId); |
| 415 | matchBoolCompOpExpr(Finder, true, "==", RightExpressionId); |
| 416 | matchBoolCompOpExpr(Finder, false, "!=", RightExpressionId); |
| 417 | |
| 418 | matchExprBinOpBool(Finder, true, "&&", LeftExpressionId); |
| 419 | matchExprBinOpBool(Finder, false, "||", LeftExpressionId); |
| 420 | matchBoolBinOpExpr(Finder, false, "&&", LeftExpressionId); |
| 421 | matchBoolBinOpExpr(Finder, true, "||", LeftExpressionId); |
| 422 | matchExprCompOpBool(Finder, true, "==", LeftExpressionId); |
| 423 | matchExprCompOpBool(Finder, false, "!=", LeftExpressionId); |
| 424 | |
| 425 | matchBoolCompOpExpr(Finder, false, "==", NegatedRightExpressionId); |
| 426 | matchBoolCompOpExpr(Finder, true, "!=", NegatedRightExpressionId); |
| 427 | |
| 428 | matchExprCompOpBool(Finder, false, "==", NegatedLeftExpressionId); |
| 429 | matchExprCompOpBool(Finder, true, "!=", NegatedLeftExpressionId); |
| 430 | |
| 431 | matchBoolCondition(Finder, true, ConditionThenStmtId); |
| 432 | matchBoolCondition(Finder, false, ConditionElseStmtId); |
| 433 | |
| 434 | matchTernaryResult(Finder, true, TernaryId); |
| 435 | matchTernaryResult(Finder, false, TernaryNegatedId); |
| 436 | |
| 437 | matchIfReturnsBool(Finder, true, IfReturnsBoolId); |
| 438 | matchIfReturnsBool(Finder, false, IfReturnsNotBoolId); |
| 439 | |
| 440 | matchIfAssignsBool(Finder, true, IfAssignBoolId); |
| 441 | matchIfAssignsBool(Finder, false, IfAssignNotBoolId); |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 442 | |
| 443 | matchCompoundIfReturnsBool(Finder, true, CompoundBoolId); |
| 444 | matchCompoundIfReturnsBool(Finder, false, CompoundNotBoolId); |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 445 | } |
| 446 | |
| 447 | void SimplifyBooleanExprCheck::check(const MatchFinder::MatchResult &Result) { |
| 448 | if (const CXXBoolLiteralExpr *LeftRemoved = |
| 449 | getBoolLiteral(Result, RightExpressionId)) { |
| 450 | replaceWithExpression(Result, LeftRemoved, false); |
| 451 | } else if (const CXXBoolLiteralExpr *RightRemoved = |
| 452 | getBoolLiteral(Result, LeftExpressionId)) { |
| 453 | replaceWithExpression(Result, RightRemoved, true); |
| 454 | } else if (const CXXBoolLiteralExpr *NegatedLeftRemoved = |
| 455 | getBoolLiteral(Result, NegatedRightExpressionId)) { |
| 456 | replaceWithExpression(Result, NegatedLeftRemoved, false, true); |
| 457 | } else if (const CXXBoolLiteralExpr *NegatedRightRemoved = |
| 458 | getBoolLiteral(Result, NegatedLeftExpressionId)) { |
| 459 | replaceWithExpression(Result, NegatedRightRemoved, true, true); |
| 460 | } else if (const CXXBoolLiteralExpr *TrueConditionRemoved = |
| 461 | getBoolLiteral(Result, ConditionThenStmtId)) { |
| 462 | replaceWithThenStatement(Result, TrueConditionRemoved); |
| 463 | } else if (const CXXBoolLiteralExpr *FalseConditionRemoved = |
| 464 | getBoolLiteral(Result, ConditionElseStmtId)) { |
| 465 | replaceWithElseStatement(Result, FalseConditionRemoved); |
| 466 | } else if (const auto *Ternary = |
| 467 | Result.Nodes.getNodeAs<ConditionalOperator>(TernaryId)) { |
| 468 | replaceWithCondition(Result, Ternary); |
| 469 | } else if (const auto *TernaryNegated = |
| 470 | Result.Nodes.getNodeAs<ConditionalOperator>( |
| 471 | TernaryNegatedId)) { |
| 472 | replaceWithCondition(Result, TernaryNegated, true); |
| 473 | } else if (const auto *If = Result.Nodes.getNodeAs<IfStmt>(IfReturnsBoolId)) { |
| 474 | replaceWithReturnCondition(Result, If); |
| 475 | } else if (const auto *IfNot = |
| 476 | Result.Nodes.getNodeAs<IfStmt>(IfReturnsNotBoolId)) { |
| 477 | replaceWithReturnCondition(Result, IfNot, true); |
| 478 | } else if (const auto *IfAssign = |
| 479 | Result.Nodes.getNodeAs<IfStmt>(IfAssignBoolId)) { |
| 480 | replaceWithAssignment(Result, IfAssign); |
| 481 | } else if (const auto *IfAssignNot = |
| 482 | Result.Nodes.getNodeAs<IfStmt>(IfAssignNotBoolId)) { |
| 483 | replaceWithAssignment(Result, IfAssignNot, true); |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 484 | } else if (const auto *Compound = |
| 485 | Result.Nodes.getNodeAs<CompoundStmt>(CompoundBoolId)) { |
| 486 | replaceCompoundReturnWithCondition(Result, Compound); |
| 487 | } else if (const auto *Compound = |
| 488 | Result.Nodes.getNodeAs<CompoundStmt>(CompoundNotBoolId)) { |
| 489 | replaceCompoundReturnWithCondition(Result, Compound, true); |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 490 | } |
| 491 | } |
| 492 | |
| 493 | void SimplifyBooleanExprCheck::replaceWithExpression( |
| 494 | const ast_matchers::MatchFinder::MatchResult &Result, |
| 495 | const CXXBoolLiteralExpr *BoolLiteral, bool UseLHS, bool Negated) { |
| 496 | const auto *LHS = Result.Nodes.getNodeAs<Expr>(LHSId); |
| 497 | const auto *RHS = Result.Nodes.getNodeAs<Expr>(RHSId); |
| 498 | std::string Replacement = |
| 499 | replacementExpression(Result, Negated, UseLHS ? LHS : RHS); |
| 500 | SourceLocation Start = LHS->getLocStart(); |
| 501 | SourceLocation End = RHS->getLocEnd(); |
| 502 | diag(BoolLiteral->getLocStart(), SimplifyOperatorDiagnostic) |
| 503 | << FixItHint::CreateReplacement(SourceRange(Start, End), Replacement); |
| 504 | } |
| 505 | |
| 506 | void SimplifyBooleanExprCheck::replaceWithThenStatement( |
| 507 | const MatchFinder::MatchResult &Result, |
| 508 | const CXXBoolLiteralExpr *TrueConditionRemoved) { |
| 509 | const auto *IfStatement = Result.Nodes.getNodeAs<IfStmt>(IfStmtId); |
| 510 | diag(TrueConditionRemoved->getLocStart(), SimplifyConditionDiagnostic) |
| 511 | << FixItHint::CreateReplacement(IfStatement->getSourceRange(), |
| 512 | getText(Result, *IfStatement->getThen())); |
| 513 | } |
| 514 | |
| 515 | void SimplifyBooleanExprCheck::replaceWithElseStatement( |
| 516 | const MatchFinder::MatchResult &Result, |
| 517 | const CXXBoolLiteralExpr *FalseConditionRemoved) { |
| 518 | const auto *IfStatement = Result.Nodes.getNodeAs<IfStmt>(IfStmtId); |
| 519 | const Stmt *ElseStatement = IfStatement->getElse(); |
| 520 | diag(FalseConditionRemoved->getLocStart(), SimplifyConditionDiagnostic) |
| 521 | << FixItHint::CreateReplacement( |
| 522 | IfStatement->getSourceRange(), |
| 523 | ElseStatement ? getText(Result, *ElseStatement) : ""); |
| 524 | } |
| 525 | |
| 526 | void SimplifyBooleanExprCheck::replaceWithCondition( |
| 527 | const MatchFinder::MatchResult &Result, const ConditionalOperator *Ternary, |
| 528 | bool Negated) { |
| 529 | std::string Replacement = |
| 530 | replacementExpression(Result, Negated, Ternary->getCond()); |
| 531 | diag(Ternary->getTrueExpr()->getLocStart(), |
| 532 | "redundant boolean literal in ternary expression result") |
| 533 | << FixItHint::CreateReplacement(Ternary->getSourceRange(), Replacement); |
| 534 | } |
| 535 | |
| 536 | void SimplifyBooleanExprCheck::replaceWithReturnCondition( |
| 537 | const MatchFinder::MatchResult &Result, const IfStmt *If, bool Negated) { |
| 538 | StringRef Terminator = isa<CompoundStmt>(If->getElse()) ? ";" : ""; |
| 539 | std::string Condition = replacementExpression(Result, Negated, If->getCond()); |
| 540 | std::string Replacement = ("return " + Condition + Terminator).str(); |
| 541 | SourceLocation Start = |
| 542 | Result.Nodes.getNodeAs<CXXBoolLiteralExpr>(ThenLiteralId)->getLocStart(); |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 543 | diag(Start, SimplifyConditionalReturnDiagnostic) |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 544 | << FixItHint::CreateReplacement(If->getSourceRange(), Replacement); |
| 545 | } |
| 546 | |
Alexander Kornienko | 6ae400d | 2015-07-01 12:39:40 +0000 | [diff] [blame] | 547 | void SimplifyBooleanExprCheck::replaceCompoundReturnWithCondition( |
| 548 | const MatchFinder::MatchResult &Result, const CompoundStmt *Compound, |
| 549 | bool Negated) { |
| 550 | const auto *Ret = Result.Nodes.getNodeAs<ReturnStmt>(CompoundReturnId); |
| 551 | |
| 552 | // The body shouldn't be empty because the matcher ensures that it must |
| 553 | // contain at least two statements: |
| 554 | // 1) A `return` statement returning a boolean literal `false` or `true` |
| 555 | // 2) An `if` statement with no `else` clause that consists fo a single |
| 556 | // `return` statement returning the opposite boolean literal `true` or |
| 557 | // `false`. |
| 558 | assert(Compound->size() >= 2); |
| 559 | const IfStmt *BeforeIf = nullptr; |
| 560 | CompoundStmt::const_body_iterator Current = Compound->body_begin(); |
| 561 | CompoundStmt::const_body_iterator After = Compound->body_begin(); |
| 562 | for (++After; After != Compound->body_end() && *Current != Ret; |
| 563 | ++Current, ++After) { |
| 564 | if (const auto *If = dyn_cast<IfStmt>(*Current)) { |
| 565 | if (const CXXBoolLiteralExpr *Lit = stmtReturnsBool(If, Negated)) { |
| 566 | if (*After == Ret) { |
| 567 | if (!ChainedConditionalReturn && BeforeIf) |
| 568 | continue; |
| 569 | |
| 570 | const Expr *Condition = If->getCond(); |
| 571 | std::string Replacement = |
| 572 | "return " + replacementExpression(Result, Negated, Condition); |
| 573 | diag(Lit->getLocStart(), SimplifyConditionalReturnDiagnostic) |
| 574 | << FixItHint::CreateReplacement( |
| 575 | SourceRange(If->getLocStart(), Ret->getLocEnd()), |
| 576 | Replacement); |
| 577 | return; |
| 578 | } |
| 579 | |
| 580 | BeforeIf = If; |
| 581 | } |
| 582 | } else { |
| 583 | BeforeIf = nullptr; |
| 584 | } |
| 585 | } |
| 586 | } |
| 587 | |
Alexander Kornienko | f5e72b0 | 2015-04-10 19:26:43 +0000 | [diff] [blame] | 588 | void SimplifyBooleanExprCheck::replaceWithAssignment( |
| 589 | const MatchFinder::MatchResult &Result, const IfStmt *IfAssign, |
| 590 | bool Negated) { |
| 591 | SourceRange Range = IfAssign->getSourceRange(); |
| 592 | StringRef VariableName = |
| 593 | getText(Result, *Result.Nodes.getNodeAs<Expr>(IfAssignVariableId)); |
| 594 | StringRef Terminator = isa<CompoundStmt>(IfAssign->getElse()) ? ";" : ""; |
| 595 | std::string Condition = |
| 596 | replacementExpression(Result, Negated, IfAssign->getCond()); |
| 597 | std::string Replacement = |
| 598 | (VariableName + " = " + Condition + Terminator).str(); |
| 599 | SourceLocation Location = |
| 600 | Result.Nodes.getNodeAs<CXXBoolLiteralExpr>(IfAssignLocId)->getLocStart(); |
| 601 | this->diag(Location, "redundant boolean literal in conditional assignment") |
| 602 | << FixItHint::CreateReplacement(Range, Replacement); |
| 603 | } |
| 604 | |
| 605 | } // namespace readability |
| 606 | } // namespace tidy |
| 607 | } // namespace clang |