Index: lib/Format/Format.cpp =================================================================== --- lib/Format/Format.cpp +++ lib/Format/Format.cpp @@ -1816,6 +1816,13 @@ checkEmptyNamespace(AnnotatedLines); + for (auto &Line : AnnotatedLines) { + if (Line->Affected) { + cleanupRight(Line->First, Line->Last, tok::comma, tok::comma); + checkConstructorInitList(*Line); + } + } + return generateFixes(); } @@ -1904,6 +1911,99 @@ return true; } + FormatToken *getNextTokenNotDeletedUntilEnd(const FormatToken *Tok, + const FormatToken *End, + bool SkipComment) { + assert(Tok); + auto *Res = Tok->Next; + while (Res && ((SkipComment && Res->is(tok::comment)) || + DeletedTokens.find(Res) != DeletedTokens.end())) { + if (Res == End) + return nullptr; + Res = Res->Next; + } + return Res; + } + + // Checks pairs {start, start->next},..., {end->previous, end} and deletes one + // of the token in the pair if the left token has \p LK token kind and the + // right token has \p RK token kind. If \p DeleteLeft is true, the left token + // is deleted on match; otherwise, the right token is deleted. + void cleanupTokenInMatchedPair(FormatToken *Start, FormatToken *End, + tok::TokenKind LK, tok::TokenKind RK, + bool DeleteLeft) { + for (auto *Left = Start; Left != nullptr && Left != End;) { + auto *Right = + getNextTokenNotDeletedUntilEnd(Left, End, /*SkipComment=*/true); + if (!Right) + break; + if (Left->is(LK) && Right->is(RK)) { + deleteToken(DeleteLeft ? Left : Right); + // If the right token is deleted, we should keep the left token + // unchanged and pair it with the new right token. + if (!DeleteLeft) + continue; + } + Left = getNextTokenNotDeletedUntilEnd(Left, End, /*SkipComment=*/true); + } + } + + void cleanupLeft(FormatToken *Start, FormatToken *End, tok::TokenKind LK, + tok::TokenKind RK) { + cleanupTokenInMatchedPair(Start, End, LK, RK, /*DeleteLeft=*/true); + } + + void cleanupRight(FormatToken *Start, FormatToken *End, tok::TokenKind LK, + tok::TokenKind RK) { + cleanupTokenInMatchedPair(Start, End, LK, RK, /*DeleteLeft=*/false); + } + + // Looks for and removes redundant colon/commas in the constructor initializer + // list. + void checkConstructorInitList(AnnotatedLine &Line) { + FormatToken *Tok = Line.First; + while (Tok && Tok->Type != TT_CtorInitializerColon) { + Tok = Tok->Next; + } + if (!Tok) + return; + + assert(Tok->is(tok::colon) && Tok->Type == TT_CtorInitializerColon); + + // Clean up redundant comma after the constructor initializer colon. + cleanupRight(Tok, getNextTokenNotDeletedUntilEnd(Tok, Line.Last, + /*SkipComment=*/true), + tok::colon, tok::comma); + + // Find the left brace at the end of the initializer list. + Tok = Line.Last; + while (Tok && Tok->is(tok::comment)) { + Tok = Tok->Previous; + } + if (!Tok || Tok->isNot(tok::l_brace)) + return; // FIXME: error handling. Expecting tok::l_brace at the end. + + // Clean up redundant commas/redundant colon right before the left brace. + Tok = Tok->Previous; + while (Tok && (Tok->is(tok::comment) || + DeletedTokens.find(Tok) != DeletedTokens.end())) { + Tok = Tok->Previous; + } + // We only need to check one token before the left brace. + // + // If the token before the left brace is a comma, then the token before the + // comma must not be the initializer colon; otherwise, the comma should have + // been deleted in the previous steps. It must not be a comma either since a + // comma right before another comma should be deleted in + // `cleanupLeft(tok::comma, tok::comma)`. + // + // If the token before the left brace is the intiailizer colon, then we can + // simply delete the redundant colon since the initializer list is empty. + if (Tok && (Tok->Type == TT_CtorInitializerColon || Tok->is(tok::comma))) { + deleteToken(Tok); + } + } + // Delete the given token. inline void deleteToken(FormatToken *Tok) { if (Tok) Index: unittests/Format/CleanupTest.cpp =================================================================== --- unittests/Format/CleanupTest.cpp +++ unittests/Format/CleanupTest.cpp @@ -113,6 +113,133 @@ EXPECT_EQ(Expected, Result); } +TEST_F(CleanupTest, CtorInitializationSimpleRedundantComma) { + std::string Code = "class A {\nA() : , {} };"; + std::string Expected = "class A {\nA() {} };"; + std::vector 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 v = {1,2,,,3}; }"; + std::string Expected = "void f() { std::vector v = {1,2,3}; }"; + std::vector 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); }"; + 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); +} + +TEST_F(CleanupTest, CtorInitializationBracesInParens) { + std::string Code = "class A {\nA() : x({1}),, {} };"; + std::string Expected = "class A {\nA() : x({1}) {} };"; + std::vector Ranges; + Ranges.push_back(tooling::Range(24, 0)); + Ranges.push_back(tooling::Range(26, 0)); + std::string Result = cleanup(Code, Ranges); + EXPECT_EQ(Expected, Result); +} + +TEST_F(CleanupTest, RedundantCommaNotInAffectedRanges) { + std::string Code = + "class A {\nA() : x({1}), /* comment */, { int x = 0; } };"; + std::string Expected = + "class A {\nA() : x({1}), /* comment */, { int x = 0; } };"; + // Set the affected range to be "int x = 0", which does not intercept the + // constructor initialization list. + std::vector Ranges(1, tooling::Range(42, 9)); + std::string Result = cleanup(Code, Ranges); + EXPECT_EQ(Expected, Result); + + Code = "class A {\nA() : x(1), {} };"; + Expected = "class A {\nA() : x(1), {} };"; + // No range. Fixer should do nothing. + Ranges.clear(); + Result = cleanup(Code, Ranges); + EXPECT_EQ(Expected, Result); +} + +// FIXME: delete comments too. +TEST_F(CleanupTest, CtorInitializationCommentAroundCommas) { + // Remove redundant commas around comment. + std::string Code = "class A {\nA() : x({1}), /* comment */, {} };"; + std::string Expected = "class A {\nA() : x({1}) /* comment */ {} };"; + std::vector Ranges; + Ranges.push_back(tooling::Range(25, 0)); + Ranges.push_back(tooling::Range(40, 0)); + std::string Result = cleanup(Code, Ranges); + EXPECT_EQ(Expected, Result); + + // Remove trailing comma and ignore comment. + Code = "class A {\nA() : x({1}), // comment\n{} };"; + Expected = "class A {\nA() : x({1}) // comment\n{} };"; + Ranges = std::vector(1, tooling::Range(25, 0)); + Result = cleanup(Code, Ranges); + EXPECT_EQ(Expected, Result); + + // Remove trailing comma and ignore comment. + Code = "class A {\nA() : x({1}), // comment\n , y(1),{} };"; + Expected = "class A {\nA() : x({1}), // comment\n y(1){} };"; + Ranges = std::vector(1, tooling::Range(38, 0)); + Result = cleanup(Code, Ranges); + EXPECT_EQ(Expected, Result); + + // Remove trailing comma and ignore comment. + Code = "class A {\nA() : x({1}), \n/* comment */, y(1),{} };"; + Expected = "class A {\nA() : x({1}), \n/* comment */ y(1){} };"; + Ranges = std::vector(1, tooling::Range(40, 0)); + Result = cleanup(Code, Ranges); + EXPECT_EQ(Expected, Result); + + // Remove trailing comma and ignore comment. + Code = "class A {\nA() : , // comment\n y(1),{} };"; + Expected = "class A {\nA() : // comment\n y(1){} };"; + Ranges = std::vector(1, tooling::Range(17, 0)); + Result = cleanup(Code, Ranges); + EXPECT_EQ(Expected, Result); +} + +TEST_F(CleanupTest, CtorInitializerInNamespace) { + std::string Code = "namespace A {\n" + "namespace B {\n" // missing r_brace + "} // namespace A\n\n" + "namespace C {\n" + "class A { A() : x(0),, {} };\n" + "inline namespace E { namespace { } }\n" + "}"; + std::string Expected = "namespace A {\n" + "\n\n\nnamespace C {\n" + "class A { A() : x(0) {} };\n \n" + "}"; + std::vector Ranges(1, tooling::Range(0, Code.size())); + std::string Result = cleanup(Code, Ranges); + EXPECT_EQ(Expected, Result); +} + } // end namespace } // end namespace format } // end namespace clang