forked from OSchip/llvm-project
[clang-tidy] readability-misleading-indentation: fix chained if
Summary: Fixed erroneously flagging of chained if statements when styled like this: ``` if (cond) { } else if (cond) { } else { } ``` Reviewers: xazax.hun, alexfh Reviewed By: xazax.hun, alexfh Subscribers: JDevlieghere, cfe-commits Patch by Florian Gross! Differential Revision: https://reviews.llvm.org/D30841 llvm-svn: 298059
This commit is contained in:
parent
225b79524d
commit
bc8d14669d
|
@ -17,7 +17,22 @@ namespace clang {
|
|||
namespace tidy {
|
||||
namespace readability {
|
||||
|
||||
static const IfStmt *getPrecedingIf(const SourceManager &SM,
|
||||
ASTContext *Context, const IfStmt *If) {
|
||||
auto parents = Context->getParents(*If);
|
||||
if (parents.size() != 1)
|
||||
return nullptr;
|
||||
if (const auto *PrecedingIf = parents[0].get<IfStmt>()) {
|
||||
SourceLocation PreviousElseLoc = PrecedingIf->getElseLoc();
|
||||
if (SM.getExpansionLineNumber(PreviousElseLoc) ==
|
||||
SM.getExpansionLineNumber(If->getIfLoc()))
|
||||
return PrecedingIf;
|
||||
}
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
void MisleadingIndentationCheck::danglingElseCheck(const SourceManager &SM,
|
||||
ASTContext *Context,
|
||||
const IfStmt *If) {
|
||||
SourceLocation IfLoc = If->getIfLoc();
|
||||
SourceLocation ElseLoc = If->getElseLoc();
|
||||
|
@ -29,6 +44,11 @@ void MisleadingIndentationCheck::danglingElseCheck(const SourceManager &SM,
|
|||
SM.getExpansionLineNumber(ElseLoc))
|
||||
return;
|
||||
|
||||
// Find location of first 'if' in a 'if else if' chain.
|
||||
for (auto PrecedingIf = getPrecedingIf(SM, Context, If); PrecedingIf;
|
||||
PrecedingIf = getPrecedingIf(SM, Context, PrecedingIf))
|
||||
IfLoc = PrecedingIf->getIfLoc();
|
||||
|
||||
if (SM.getExpansionColumnNumber(IfLoc) !=
|
||||
SM.getExpansionColumnNumber(ElseLoc))
|
||||
diag(ElseLoc, "different indentation for 'if' and corresponding 'else'");
|
||||
|
@ -92,7 +112,7 @@ void MisleadingIndentationCheck::registerMatchers(MatchFinder *Finder) {
|
|||
|
||||
void MisleadingIndentationCheck::check(const MatchFinder::MatchResult &Result) {
|
||||
if (const auto *If = Result.Nodes.getNodeAs<IfStmt>("if"))
|
||||
danglingElseCheck(*Result.SourceManager, If);
|
||||
danglingElseCheck(*Result.SourceManager, Result.Context, If);
|
||||
|
||||
if (const auto *CStmt = Result.Nodes.getNodeAs<CompoundStmt>("compound"))
|
||||
missingBracesCheck(*Result.SourceManager, CStmt);
|
||||
|
|
|
@ -30,7 +30,8 @@ public:
|
|||
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
|
||||
|
||||
private:
|
||||
void danglingElseCheck(const SourceManager &SM, const IfStmt *If);
|
||||
void danglingElseCheck(const SourceManager &SM, ASTContext *Context,
|
||||
const IfStmt *If);
|
||||
void missingBracesCheck(const SourceManager &SM, const CompoundStmt *CStmt);
|
||||
};
|
||||
|
||||
|
|
|
@ -76,5 +76,31 @@ int main()
|
|||
{
|
||||
}
|
||||
|
||||
if(cond1) {
|
||||
}
|
||||
else if (cond2) {
|
||||
}
|
||||
else {
|
||||
}
|
||||
|
||||
if(cond1) {
|
||||
}
|
||||
else if (cond2) {
|
||||
}
|
||||
else {
|
||||
}
|
||||
// CHECK-MESSAGES: :[[@LINE-2]]:8: warning: different indentation for 'if' and corresponding 'else' [readability-misleading-indentation]
|
||||
|
||||
if (cond1) {
|
||||
if (cond1) {
|
||||
}
|
||||
else if (cond2) {
|
||||
}
|
||||
else {
|
||||
}
|
||||
}
|
||||
else if (cond2) {
|
||||
}
|
||||
|
||||
BLOCK
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue