Remove redundant comma around parenthesis in parameter list.

[djasper]

unittests/Format/CleanupTest.cpp

147

Seems like it's worth to pull out a function getting (offset, input, expected output) and then doing all this. Of course doesn't *have* to be in this patch, but I guess now is as good a time as any.

Once that is done and we don't need to duplicate so much code, I think it might be worth duplicating this test to have it with and without trailing comma.

Also, I think there is actually a FIXME here. Lets assume I mark the offset that you put into ranges as X, then I think:

• {a, b, X} should lead to {a, b}
• {a, b, X,} should lead to {a, b,}

Does that make sense?

[ioeric]

• Refactored cleanup unittest; added more test cases for trailing commas.

unittests/Format/CleanupTest.cpp

147

Yep, make sense :)

[djasper]

Some remarks, but looks good.

unittests/Format/CleanupTest.cpp

38

I thin we normally just used "unsigned" instead of "unsigned int".

39

nit: alignment.

128

I think for such short test cases, at least, I'd just inline the string literals, e.g.:

EXPECT_EQ("class A {\nA()  {} };",
          cleanupAroundOffsets({17, 19}, "class A {\nA() : , {} };"));

But not sure whether it's better.

[ioeric]

• Minor cleanup.

unittests/Format/CleanupTest.cpp

128

I prefer defining variables since it is easier to read/compare and also consistent with other test cases.

lib/Format/Format.cpp

AffectedRangeMgr.computeAffectedLines(AnnotatedLines.begin(),
                                           AnnotatedLines.end());
 
     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);
       }
     }
 
     return generateFixes();
   }
 

unittests/Format/CleanupTest.cpp

std::string cleanup(llvm::StringRef Code,
     tooling::Replacements Replaces = format::cleanup(Style, Code, Ranges);
 
     auto Result = applyAllReplacements(Code, Replaces);
     EXPECT_TRUE(static_cast<bool>(Result));
     return *Result;
   }
 };
 
 TEST_F(CleanupTest, DeleteEmptyNamespaces) {
   std::string Code = "namespace A {\n"
                      "namespace B {\n"
                      "} // namespace B\n"
                      "} // namespace A\n\n"
                      "namespace C {\n"
                      "namespace D { int i; }\n"
                      "inline namespace E { namespace { } }\n"
                      "}";
   std::string Expected = "\n\n\n\n\nnamespace C {\n"

 
 TEST_F(CleanupTest, CtorInitializationSimpleRedundantComma) {
   std::string Code = "class A {\nA() : , {} };";
   std::string Expected = "class A {\nA()  {} };";
   std::vector<tooling::Range> Ranges;
   Ranges.push_back(tooling::Range(17, 0));
   Ranges.push_back(tooling::Range(19, 0));
   std::string Result = cleanup(Code, Ranges);
   EXPECT_EQ(Expected, Result);
 
   Code = "class A {\nA() : x(1), {} };";
   Expected = "class A {\nA() : x(1) {} };";
   Ranges.clear();
   Ranges.push_back(tooling::Range(23, 0));
   Result = cleanup(Code, Ranges);
   EXPECT_EQ(Expected, Result);
 
   Code = "class A {\nA() :,,,,{} };";
   Expected = "class A {\nA() {} };";
   Ranges.clear();
   Ranges.push_back(tooling::Range(15, 0));
   Result = cleanup(Code, Ranges);
   EXPECT_EQ(Expected, Result);
 }
 
-TEST_F(CleanupTest, ListSimpleRedundantComma) {
-  std::string Code = "void f() { std::vector<int> v = {1,2,,,3,{4,5}}; }";
-  std::string Expected = "void f() { std::vector<int> v = {1,2,3,{4,5}}; }";
+TEST_F(CleanupTest, ListRedundantComma) {
+  // Trailing comma in braces is not removed.
+  std::string Code = "void f() { std::vector<int> v = {1,2,,,3,{4,5,}}; }";
+  std::string Expected = "void f() { std::vector<int> v = {1,2,3,{4,5,}}; }";
   std::vector<tooling::Range> Ranges;
   Ranges.push_back(tooling::Range(40, 0));
   std::string Result = cleanup(Code, Ranges);
   EXPECT_EQ(Expected, Result);
 
-  Code = "int main() { f(1,,2,3,,4);}";
+  Code = "int main() { f(,1,,2,3,,4,);}";
   Expected = "int main() { f(1,2,3,4);}";
   Ranges.clear();
   Ranges.push_back(tooling::Range(17, 0));
   Ranges.push_back(tooling::Range(22, 0));
   Result = cleanup(Code, Ranges);
   EXPECT_EQ(Expected, Result);
 }