[PATCH] D30841: [clang-tidy] readability-misleading-indentation: fix chained if

[Florian Gross via Phabricator via cfe-commits]

fgross updated this revision to Diff 92026.
fgross added a comment.

Now using `ASTContext::getParents` instead of `ChainedIfs` map.

For some reason I thought of `getParents` as an expensive function to call...


https://reviews.llvm.org/D30841

Files:
  clang-tidy/readability/MisleadingIndentationCheck.cpp
  clang-tidy/readability/MisleadingIndentationCheck.h
  test/clang-tidy/readability-misleading-indentation.cpp


Index: test/clang-tidy/readability-misleading-indentation.cpp
===================================================================
--- test/clang-tidy/readability-misleading-indentation.cpp
+++ test/clang-tidy/readability-misleading-indentation.cpp
@@ -76,5 +76,31 @@
     {
     }
 
+  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
 }
Index: clang-tidy/readability/MisleadingIndentationCheck.h
===================================================================
--- clang-tidy/readability/MisleadingIndentationCheck.h
+++ clang-tidy/readability/MisleadingIndentationCheck.h
@@ -30,7 +30,8 @@
   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);
 };
 
Index: clang-tidy/readability/MisleadingIndentationCheck.cpp
===================================================================
--- clang-tidy/readability/MisleadingIndentationCheck.cpp
+++ clang-tidy/readability/MisleadingIndentationCheck.cpp
@@ -17,7 +17,22 @@
 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 @@
       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::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);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D30841.92026.patch
Type: text/x-patch
Size: 3351 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170316/2c81040e/attachment.bin>