replica
/
hanchenye-llvm-project
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

[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
This commit is contained in:
Alexander Kornienko 2015-05-17 12:31:12 +00:00
parent 53958e187a
commit fb3e2cd8cc
5 changed files with 175 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
View File

@ -108,20 +108,25 @@ std::string replacementExpression(const MatchFinder::MatchResult &Result,
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::matchTernaryResult(MatchFinder *Finder,
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::matchIfAssignsBool(MatchFinder *Finder,
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) {

23
clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
View File

@ -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

35
clang-tools-extra/test/clang-tidy/readability-simplify-bool-expr-chained-conditional-assignment.cpp Normal file
View File

@ -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;
}

33
clang-tools-extra/test/clang-tidy/readability-simplify-bool-expr-chained-conditional-return.cpp Normal file
View File

@ -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;
}

52
clang-tools-extra/test/clang-tidy/readability-simplify-bool-expr.cpp
View File

@ -541,3 +541,55 @@ void complex_conditional_assignment_statements(int i) {
} 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;
}