hokein updated this revision to Diff 77821. hokein added a comment. Remove a trailing blank line.
https://reviews.llvm.org/D26493 Files: clang-move/ClangMove.cpp unittests/clang-move/ClangMoveTests.cpp
Index: unittests/clang-move/ClangMoveTests.cpp =================================================================== --- unittests/clang-move/ClangMoveTests.cpp +++ unittests/clang-move/ClangMoveTests.cpp @@ -127,8 +127,10 @@ const char ExpectedNewHeader[] = "#ifndef NEW_FOO_H\n" "#define NEW_FOO_H\n" + "\n" "namespace a {\n" "class C1; // test\n" + "\n" "template <typename T> class C2;\n" "namespace b {\n" "// This is a Foo class\n" @@ -144,6 +146,7 @@ "}; // abc\n" "} // namespace b\n" "} // namespace a\n" + "\n" "#endif // NEW_FOO_H\n"; const char ExpectedNewCC[] = "namespace a {\n" @@ -154,17 +157,21 @@ "/// comment2.\n" "int kConstInt1 = 0;\n" "} // namespace\n" + "\n" "/* comment 3*/\n" "static int kConstInt2 = 1;\n" + "\n" "/** comment4\n" "*/\n" "static int help() {\n" " int a = 0;\n" " return a;\n" "}\n" + "\n" "// comment5\n" "// comment5\n" "void Foo::f() { f1(); }\n" + "\n" "/////////////\n" "// comment //\n" "/////////////\n" @@ -312,15 +319,17 @@ TEST(ClangMove, DontMoveAll) { const char ExpectedHeader[] = "#ifndef NEW_FOO_H\n" "#define NEW_FOO_H\n" + "\n" "class A {\npublic:\n int f();\n};\n" + "\n" "#endif // NEW_FOO_H\n"; const char Code[] = "#include \"foo.h\"\nint A::f() { return 0; }"; std::vector<std::string> TestHeaders = { - "typedef int Int;\nclass A {\npublic:\n int f();\n};", - "using Int=int;\nclass A {\npublic:\n int f();\n};", - "class B {};\nclass A {\npublic:\n int f();\n};", - "void f() {};\nclass A {\npublic:\n int f();\n};", - "enum Color { RED };\nclass A {\npublic:\n int f();\n};", + "typedef int Int;\nclass A {\npublic:\n int f();\n};\n", + "using Int=int;\nclass A {\npublic:\n int f();\n};\n", + "class B {};\nclass A {\npublic:\n int f();\n};\n", + "void f() {};\nclass A {\npublic:\n int f();\n};\n", + "enum Color { RED };\nclass A {\npublic:\n int f();\n};\n", }; move::ClangMoveTool::MoveDefinitionSpec Spec; Spec.Names.push_back("A"); @@ -332,7 +341,7 @@ auto Results = runClangMoveOnCode(Spec, Header.c_str(), Code); EXPECT_EQ(ExpectedHeader, Results[Spec.NewHeader]); // The expected old header should not contain class A definition. - std::string ExpectedOldHeader = Header.substr(0, Header.size() - 31); + std::string ExpectedOldHeader = Header.substr(0, Header.size() - 32); EXPECT_EQ(ExpectedOldHeader, Results[Spec.OldHeader]); } } @@ -355,6 +364,61 @@ EXPECT_EQ(ExpectedNewCode, Results[Spec.NewCC]); } +TEST(ClangMove, WellFormattedCode) { + const std::string CommonHeader = + "namespace a {\n" + "namespace b {\n" + "namespace c {\n" + "class C;\n" + "\n" + "class A {\npublic:\n void f();\n void f2();\n};\n" + "} // namespace c\n" + "} // namespace b\n" + "\n" + "namespace d {\n" + "namespace e {\n" + "class B {\npublic:\n void f();\n};\n" + "} // namespace e\n" + "} // namespace d\n" + "} // namespace a\n"; + const std::string CommonCode = "\n" + "namespace a {\n" + "namespace b {\n" + "namespace c {\n" + "void A::f() {}\n" + "\n" + "void A::f2() {}\n" + "} // namespace c\n" + "} // namespace b\n" + "\n" + "namespace d {\n" + "namespace e {\n" + "void B::f() {}\n" + "} // namespace e\n" + "} // namespace d\n" + "} // namespace a\n"; + // Add dummy class to prevent behavior of moving all declarations from header. + const std::string TestHeader = CommonHeader + "class D {};\n"; + const std::string TestCode = "#include \"foo.h\"\n" + CommonCode; + const std::string ExpectedNewHeader = "#ifndef NEW_FOO_H\n" + "#define NEW_FOO_H\n" + "\n" + + CommonHeader + + "\n" + "#endif // NEW_FOO_H\n"; + const std::string ExpectedNewCC = "#include \"new_foo.h\"\n" + CommonCode; + move::ClangMoveTool::MoveDefinitionSpec Spec; + Spec.Names.push_back("a::b::c::A"); + Spec.Names.push_back("a::d::e::B"); + Spec.OldHeader = "foo.h"; + Spec.OldCC = "foo.cc"; + Spec.NewHeader = "new_foo.h"; + Spec.NewCC = "new_foo.cc"; + auto Results = runClangMoveOnCode(Spec, TestHeader.c_str(), TestCode.c_str()); + EXPECT_EQ(ExpectedNewCC, Results[Spec.NewCC]); + EXPECT_EQ(ExpectedNewHeader, Results[Spec.NewHeader]); +} + } // namespace } // namespce move } // namespace clang Index: clang-move/ClangMove.cpp =================================================================== --- clang-move/ClangMove.cpp +++ clang-move/ClangMove.cpp @@ -294,7 +294,7 @@ } GuardName = StringRef(GuardName).upper(); NewCode += "#ifndef " + GuardName + "\n"; - NewCode += "#define " + GuardName + "\n"; + NewCode += "#define " + GuardName + "\n\n"; } // Add #Includes. @@ -306,40 +306,63 @@ // Add moved class definition and its related declarations. All declarations // in same namespace are grouped together. + // + // Record namespaces where the current position is in. std::vector<std::string> CurrentNamespaces; for (const auto &MovedDecl : Decls) { + // The namespaces of the declaration being moved. std::vector<std::string> DeclNamespaces = GetNamespaces(MovedDecl.Decl); auto CurrentIt = CurrentNamespaces.begin(); auto DeclIt = DeclNamespaces.begin(); + // Skip the common prefix. while (CurrentIt != CurrentNamespaces.end() && DeclIt != DeclNamespaces.end()) { if (*CurrentIt != *DeclIt) break; ++CurrentIt; ++DeclIt; } + // Calculate the new namespaces after adding MovedDecl in CurrentNamespace, + // which is used for next iteration of this loop. std::vector<std::string> NextNamespaces(CurrentNamespaces.begin(), CurrentIt); NextNamespaces.insert(NextNamespaces.end(), DeclIt, DeclNamespaces.end()); + + + // End with CurrentNamespace. + bool HasEndCurrentNamespace = false; auto RemainingSize = CurrentNamespaces.end() - CurrentIt; for (auto It = CurrentNamespaces.rbegin(); RemainingSize > 0; --RemainingSize, ++It) { assert(It < CurrentNamespaces.rend()); NewCode += "} // namespace " + *It + "\n"; + HasEndCurrentNamespace = true; } + // Add trailing '\n' after the nested namespace definition. + if (HasEndCurrentNamespace) + NewCode += "\n"; + + // If the moved declaration is not in CurrentNamespace, add extra namespace + // definitions. + bool IsInNewNamespace = false; while (DeclIt != DeclNamespaces.end()) { NewCode += "namespace " + *DeclIt + " {\n"; + IsInNewNamespace = true; ++DeclIt; } + // If the moved declaration is in same namespace CurrentNamespace, add + // a preceeding `\n' before the moved declaration. + if (!IsInNewNamespace) + NewCode += "\n"; NewCode += getDeclarationSourceText(MovedDecl.Decl, MovedDecl.SM); CurrentNamespaces = std::move(NextNamespaces); } std::reverse(CurrentNamespaces.begin(), CurrentNamespaces.end()); for (const auto &NS : CurrentNamespaces) NewCode += "} // namespace " + NS + "\n"; if (IsHeader) - NewCode += "#endif // " + GuardName + "\n"; + NewCode += "\n#endif // " + GuardName + "\n"; return clang::tooling::Replacements( clang::tooling::Replacement(FileName, 0, 0, NewCode)); }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits