[clang-format] Fix cleanup of `AnnotatedLine` to include children nodes.

[ymandel]

Summary

AnnotatedLine has a tree structure, and things like the body of a lambda will be
a child of the lambda expression. For example,

[&]() { foo(a); };

will have an AnnotatedLine with a child:

[&]() {};
 '- foo(a);

Currently, when the Cleaner class analyzes the affected lines, it does not
cleanup the lines' children nodes, which results in missed cleanup
opportunities, like the lambda body in the example above.

This revision extends the algorithm to visit children, thereby fixing the above problem.

Patch by Eric Li.

[krasimir]

Thank you!

clang/lib/Format/Format.cpp

1420 ↗

(On Diff #220498)

nit: (I realize this code was like this before, but,)
I find it more useful to have auto *Line instead of auto &Line (and similarly auto *Child instead of auto &Child at line 1028 below) as that makes it immediately clear that inside the body of the for loop we need to use -> to access members of the object.

[ymandel]

Fixed auto use.

[ymandel]

clang/lib/Format/Format.cpp

1420 ↗

(On Diff #220498)

Agreed!

cfe/trunk/lib/Format/Format.cpp

analyze(TokenAnnotator &Annotator,
     // intercept with ranges of replacements that result in the redundancy.
     // To determine if some redundant code is actually introduced by
     // replacements(e.g. deletions), we need to come up with a more
     // sophisticated way of computing affected ranges.
     AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
 
     checkEmptyNamespace(AnnotatedLines);
 
-    for (auto &Line : AnnotatedLines) {
-      if (Line->Affected) {
-        cleanupRight(Line->First, tok::comma, tok::comma);
-        cleanupRight(Line->First, TT_CtorInitializerColon, tok::comma);
-        cleanupRight(Line->First, tok::l_paren, tok::comma);
-        cleanupLeft(Line->First, tok::comma, tok::r_paren);
-        cleanupLeft(Line->First, TT_CtorInitializerComma, tok::l_brace);
-        cleanupLeft(Line->First, TT_CtorInitializerColon, tok::l_brace);
-        cleanupLeft(Line->First, TT_CtorInitializerColon, tok::equal);
-      }
-    }
+    for (auto *Line : AnnotatedLines)
+      cleanupLine(Line);
+
+    return {generateFixes(), 0};
+  }
+
+private:
+  void cleanupLine(AnnotatedLine *Line) {
+    for (auto *Child : Line->Children) {
+      cleanupLine(Child);
+    }
+
 
-    return {generateFixes(), 0};
-  }
-
-private:
   bool containsOnlyComments(const AnnotatedLine &Line) {
     for (FormatToken *Tok = Line.First; Tok != nullptr; Tok = Tok->Next) {
       if (Tok->isNot(tok::comment))
         return false;
     }
     return true;
   }
 

cfe/trunk/unittests/Format/CleanupTest.cpp

TEST_F(CleanupTest, NoCleanupsForJavaScript) {
 
   EXPECT_EQ(Expected, cleanupAroundOffsets({30}, Code, Style));
 }
 
 TEST_F(CleanupTest, TrailingCommaInParens) {
   std::string Code = "int main() { f(,1,,2,3,f(1,2,),4,,);}";
   std::string Expected = "int main() { f(1,2,3,f(1,2),4);}";
   EXPECT_EQ(Expected, cleanupAroundOffsets({15, 18, 29, 33}, Code));
+
+  // Lambda contents are also checked for trailing commas.
+  Code = "int main() { [](){f(,1,,2,3,f(1,2,),4,,);}();}";
+  Expected = "int main() { [](){f(1,2,3,f(1,2),4);}();}";
+  EXPECT_EQ(Expected, cleanupAroundOffsets({20, 23, 34, 38}, Code));
 }
 
 TEST_F(CleanupTest, TrailingCommaInBraces) {
-  // Trainling comma is allowed in brace list.
+  // Trailing comma is allowed in brace list.
   // If there was trailing comma in the original code, then trailing comma is
   // preserved. In this example, element between the last two commas is deleted
   // causing the second-last comma to be redundant.
   std::string Code = "void f() { std::vector<int> v = {1,2,3,,}; }";
   std::string Expected = "void f() { std::vector<int> v = {1,2,3,}; }";
   EXPECT_EQ(Expected, cleanupAroundOffsets({39}, Code));
 
-  // If there was no trailing comma in the original code, then trainling comma
+  // If there was no trailing comma in the original code, then trailing comma
   // introduced by replacements should be cleaned up. In this example, the
   // element after the last comma is deleted causing the last comma to be
   // redundant.
   Code = "void f() { std::vector<int> v = {1,2,3,}; }";
   // FIXME: redundant trailing comma should be removed.
   Expected = "void f() { std::vector<int> v = {1,2,3,}; }";
   EXPECT_EQ(Expected, cleanupAroundOffsets({39}, Code));