[clang-tools-extra] r237541 - [clang-tidy] Enhance clang-tidy readability-simplify-boolean-expr check...
Alexander Kornienko
alexfh at google.com
Sun May 17 05:31:12 PDT 2015
Author: alexfh
Date: Sun May 17 07:31:12 2015
New Revision: 237541
URL: http://llvm.org/viewvc/llvm-project?rev=237541&view=rev
Log:
[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!
Added:
clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr-chained-conditional-assignment.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr-chained-conditional-return.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.h
clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr.cpp
Modified: clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp?rev=237541&r1=237540&r2=237541&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp Sun May 17 07:31:12 2015
@@ -108,20 +108,25 @@ std::string replacementExpression(const
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::matchTern
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 @@ void SimplifyBooleanExprCheck::matchIfAs
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) {
Modified: clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.h?rev=237541&r1=237540&r2=237541&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.h (original)
+++ clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.h Sun May 17 07:31:12 2015
@@ -30,15 +30,25 @@ namespace readability {
/// `e ? false : true` becomes `!e`
/// `if (true) t(); else f();` becomes `t();`
/// `if (false) t(); else f();` becomes `f();`
-/// `if (e) return true; else return false;` becomes `return (e);`
-/// `if (e) return false; else return true;` becomes `return !(e);`
+/// `if (e) return true; else return false;` becomes `return e;`
+/// `if (e) return false; else return true;` becomes `return !e;`
/// `if (e) b = true; else b = false;` becomes `b = e;`
-/// `if (e) b = false; else b = true;` becomes `b = !(e);`
+/// `if (e) b = false; else b = true;` becomes `b = !e;`
+///
+/// Parenthesis from the resulting expression `e` are removed whenever
+/// possible.
+///
+/// When a conditional boolean return or assignment appears at the end of a
+/// chain of `if`, `else if` statements, the conditional statement is left
+/// unchanged unless the option `ChainedConditionalReturn` or
+/// `ChainedConditionalAssignment`, respectively, is specified as non-zero.
+/// The default value for both options is zero.
///
class SimplifyBooleanExprCheck : public ClangTidyCheck {
public:
- SimplifyBooleanExprCheck(StringRef Name, ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context) {}
+ SimplifyBooleanExprCheck(StringRef Name, ClangTidyContext *Context);
+
+ void storeOptions(ClangTidyOptions::OptionMap &Options) override;
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
@@ -92,6 +102,9 @@ private:
void
replaceWithAssignment(const ast_matchers::MatchFinder::MatchResult &Result,
const IfStmt *If, bool Negated = false);
+
+ const bool ChainedConditionalReturn;
+ const bool ChainedConditionalAssignment;
};
} // namespace readability
Added: clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr-chained-conditional-assignment.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr-chained-conditional-assignment.cpp?rev=237541&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr-chained-conditional-assignment.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr-chained-conditional-assignment.cpp Sun May 17 07:31:12 2015
@@ -0,0 +1,35 @@
+// RUN: $(dirname %s)/check_clang_tidy.sh %s readability-simplify-boolean-expr %t -config="{CheckOptions: [{key: "readability-simplify-boolean-expr.ChainedConditionalAssignment", value: 1}]}" --
+// REQUIRES: shell
+
+void chained_conditional_compound_assignment(int i) {
+ bool b;
+ if (i < 0) {
+ b = true;
+ } else if (i < 10) {
+ b = false;
+ } else if (i > 20) {
+ b = true;
+ } else {
+ b = false;
+ }
+ // CHECK-MESSAGES: :[[@LINE-4]]:9: warning: redundant boolean literal in conditional assignment [readability-simplify-boolean-expr]
+ // CHECK-FIXES: {{^}} } else if (i < 10) {{{$}}
+ // CHECK-FIXES-NEXT: {{^}} b = false;{{$}}
+ // CHECK-FIXES-NEXT: {{^}} } else b = i > 20;{{$}}
+}
+
+void chained_conditional_assignment(int i) {
+ bool b;
+ if (i < 0)
+ b = true;
+ else if (i < 10)
+ b = false;
+ else if (i > 20)
+ b = true;
+ else
+ b = false;
+ // CHECK-MESSAGES: :[[@LINE-3]]:9: warning: {{.*}} in conditional assignment
+ // CHECK-FIXES: {{^}} else if (i < 10)
+ // CHECK-FIXES-NEXT: {{^}} b = false;
+ // CHECK-FIXES-NEXT: {{^}} else b = i > 20;
+}
Added: clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr-chained-conditional-return.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr-chained-conditional-return.cpp?rev=237541&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr-chained-conditional-return.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr-chained-conditional-return.cpp Sun May 17 07:31:12 2015
@@ -0,0 +1,33 @@
+// RUN: $(dirname %s)/check_clang_tidy.sh %s readability-simplify-boolean-expr %t -config="{CheckOptions: [{key: "readability-simplify-boolean-expr.ChainedConditionalReturn", value: 1}]}" --
+// REQUIRES: shell
+
+bool chained_conditional_compound_return(int i) {
+ if (i < 0) {
+ return true;
+ } else if (i < 10) {
+ return false;
+ } else if (i > 20) {
+ return true;
+ } else {
+ return false;
+ }
+ // CHECK-MESSAGES: :[[@LINE-4]]:12: warning: redundant boolean literal in conditional return statement [readability-simplify-boolean-expr]
+ // CHECK-FIXES: {{^}} } else if (i < 10) {{{$}}
+ // CHECK-FIXES-NEXT: {{^}} return false;{{$}}
+ // CHECK-FIXES-NEXT: {{^}} } else return i > 20;{{$}}
+}
+
+bool chained_conditional_return(int i) {
+ if (i < 0)
+ return true;
+ else if (i < 10)
+ return false;
+ else if (i > 20)
+ return true;
+ else
+ return false;
+ // CHECK-MESSAGES: :[[@LINE-3]]:12: warning: {{.*}} in conditional return statement
+ // CHECK-FIXES: {{^}} else if (i < 10)
+ // CHECK-FIXES-NEXT: {{^}} return false;
+ // CHECK-FIXES-NEXT: {{^}} else return i > 20;
+}
Modified: clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr.cpp?rev=237541&r1=237540&r2=237541&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr.cpp Sun May 17 07:31:12 2015
@@ -541,3 +541,55 @@ void complex_conditional_assignment_stat
} else
f = false;
}
+
+// unchanged: chained return statements, but ChainedConditionalReturn not set
+bool chained_conditional_compound_return(int i) {
+ if (i < 0) {
+ return true;
+ } else if (i < 10) {
+ return false;
+ } else if (i > 20) {
+ return true;
+ } else {
+ return false;
+ }
+}
+
+// unchanged: chained return statements, but ChainedConditionalReturn not set
+bool chained_conditional_return(int i) {
+ if (i < 0)
+ return true;
+ else if (i < 10)
+ return false;
+ else if (i > 20)
+ return true;
+ else
+ return false;
+}
+
+// unchanged: chained assignments, but ChainedConditionalAssignment not set
+void chained_conditional_compound_assignment(int i) {
+ bool b;
+ if (i < 0) {
+ b = true;
+ } else if (i < 10) {
+ b = false;
+ } else if (i > 20) {
+ b = true;
+ } else {
+ b = false;
+ }
+}
+
+// unchanged: chained return statements, but ChainedConditionalReturn not set
+void chained_conditional_assignment(int i) {
+ bool b;
+ if (i < 0)
+ b = true;
+ else if (i < 10)
+ b = false;
+ else if (i > 20)
+ b = true;
+ else
+ b = false;
+}
More information about the cfe-commits
mailing list