[clang-tidy] Enhance clang-tidy readability-simplify-boolean-expr check...
Enhance clang-tidy readability-simplify-boolean-expr check to handle chained
conditional assignment and chained conditional return.
Based on feedback from applying this tool to the clang/LLVM codebase, this
changeset improves the readability-simplify-boolean-expr check so that
conditional assignment or return statements at the end of a chain of if/else if
statements are left unchanged unless a configuration option is supplied.
http://reviews.llvm.org/D8996
Patch by Richard Thomson!
llvm-svn: 237541
diff --git a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
index c9a1182..f7754e4 100644
--- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -108,20 +108,25 @@
StringRef NegatedOperator = negatedOperator(BinOp);
if (!NegatedOperator.empty()) {
return (getText(Result, *BinOp->getLHS()) + " " + NegatedOperator +
- " " + getText(Result, *BinOp->getRHS()))
- .str();
+ " " + getText(Result, *BinOp->getRHS())).str();
}
}
}
StringRef Text = getText(Result, *E);
return (Negated ? (needsParensAfterUnaryNegation(E) ? "!(" + Text + ")"
: "!" + Text)
- : Text)
- .str();
+ : Text).str();
}
} // namespace
+SimplifyBooleanExprCheck::SimplifyBooleanExprCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ ChainedConditionalReturn(Options.get("ChainedConditionalReturn", 0U)),
+ ChainedConditionalAssignment(
+ Options.get("ChainedConditionalAssignment", 0U)) {}
+
void SimplifyBooleanExprCheck::matchBoolBinOpExpr(MatchFinder *Finder,
bool Value,
StringRef OperatorName,
@@ -199,10 +204,18 @@
void SimplifyBooleanExprCheck::matchIfReturnsBool(MatchFinder *Finder,
bool Value, StringRef Id) {
- Finder->addMatcher(ifStmt(isExpansionInMainFile(),
- hasThen(ReturnsBool(Value, ThenLiteralId)),
- hasElse(ReturnsBool(!Value))).bind(Id),
- this);
+ if (ChainedConditionalReturn) {
+ Finder->addMatcher(ifStmt(isExpansionInMainFile(),
+ hasThen(ReturnsBool(Value, ThenLiteralId)),
+ hasElse(ReturnsBool(!Value))).bind(Id),
+ this);
+ } else {
+ Finder->addMatcher(ifStmt(isExpansionInMainFile(),
+ unless(hasParent(ifStmt())),
+ hasThen(ReturnsBool(Value, ThenLiteralId)),
+ hasElse(ReturnsBool(!Value))).bind(Id),
+ this);
+ }
}
void SimplifyBooleanExprCheck::matchIfAssignsBool(MatchFinder *Finder,
@@ -220,9 +233,22 @@
hasRHS(boolLiteral(equals(!Value))));
auto Else = anyOf(SimpleElse, compoundStmt(statementCountIs(1),
hasAnySubstatement(SimpleElse)));
- Finder->addMatcher(
- ifStmt(isExpansionInMainFile(), hasThen(Then), hasElse(Else)).bind(Id),
- this);
+ if (ChainedConditionalAssignment) {
+ Finder->addMatcher(
+ ifStmt(isExpansionInMainFile(), hasThen(Then), hasElse(Else)).bind(Id),
+ this);
+ } else {
+ Finder->addMatcher(ifStmt(isExpansionInMainFile(),
+ unless(hasParent(ifStmt())), hasThen(Then),
+ hasElse(Else)).bind(Id),
+ this);
+ }
+}
+
+void SimplifyBooleanExprCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "ChainedConditionalReturn", ChainedConditionalReturn);
+ Options.store(Opts, "ChainedConditionalAssignment",
+ ChainedConditionalAssignment);
}
void SimplifyBooleanExprCheck::registerMatchers(MatchFinder *Finder) {