[clang-tools-extra] r290966 - [change-namespace] get newlines around moved namespace right.
Eric Liu via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 4 06:49:08 PST 2017
Author: ioeric
Date: Wed Jan 4 08:49:08 2017
New Revision: 290966
URL: http://llvm.org/viewvc/llvm-project?rev=290966&view=rev
Log:
[change-namespace] get newlines around moved namespace right.
Summary: Previously, a `\n` might be left in the old namespace and thus not copied to the new namespace, which is bad.
Reviewers: hokein
Subscribers: alexshap, cfe-commits
Differential Revision: https://reviews.llvm.org/D28282
Modified:
clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp
Modified: clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp?rev=290966&r1=290965&r2=290966&view=diff
==============================================================================
--- clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp (original)
+++ clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp Wed Jan 4 08:49:08 2017
@@ -555,17 +555,16 @@ void ChangeNamespaceTool::moveOldNamespa
if (Decl::castToDeclContext(NsDecl)->decls_empty())
return;
+ const SourceManager &SM = *Result.SourceManager;
// Get the range of the code in the old namespace.
- SourceLocation Start = getLocAfterNamespaceLBrace(
- NsDecl, *Result.SourceManager, Result.Context->getLangOpts());
+ SourceLocation Start =
+ getLocAfterNamespaceLBrace(NsDecl, SM, Result.Context->getLangOpts());
assert(Start.isValid() && "Can't find l_brace for namespace.");
- SourceLocation End = NsDecl->getRBraceLoc().getLocWithOffset(-1);
- // Create a replacement that deletes the code in the old namespace merely for
- // retrieving offset and length from it.
- const auto R = createReplacement(Start, End, "", *Result.SourceManager);
MoveNamespace MoveNs;
- MoveNs.Offset = R.getOffset();
- MoveNs.Length = R.getLength();
+ MoveNs.Offset = SM.getFileOffset(Start);
+ // The range of the moved namespace is from the location just past the left
+ // brace to the location right before the right brace.
+ MoveNs.Length = SM.getFileOffset(NsDecl->getRBraceLoc()) - MoveNs.Offset;
// Insert the new namespace after `DiffOldNamespace`. For example, if
// `OldNamespace` is "a::b::c" and `NewNamespace` is `a::x::y`, then
@@ -577,18 +576,16 @@ void ChangeNamespaceTool::moveOldNamespa
const NamespaceDecl *OuterNs = getOuterNamespace(NsDecl, DiffOldNamespace);
SourceLocation InsertionLoc = Start;
if (OuterNs) {
- SourceLocation LocAfterNs =
- getStartOfNextLine(OuterNs->getRBraceLoc(), *Result.SourceManager,
- Result.Context->getLangOpts());
+ SourceLocation LocAfterNs = getStartOfNextLine(
+ OuterNs->getRBraceLoc(), SM, Result.Context->getLangOpts());
assert(LocAfterNs.isValid() &&
"Failed to get location after DiffOldNamespace");
InsertionLoc = LocAfterNs;
}
- MoveNs.InsertionOffset = Result.SourceManager->getFileOffset(
- Result.SourceManager->getSpellingLoc(InsertionLoc));
- MoveNs.FID = Result.SourceManager->getFileID(Start);
+ MoveNs.InsertionOffset = SM.getFileOffset(SM.getSpellingLoc(InsertionLoc));
+ MoveNs.FID = SM.getFileID(Start);
MoveNs.SourceMgr = Result.SourceManager;
- MoveNamespaces[R.getFilePath()].push_back(MoveNs);
+ MoveNamespaces[SM.getFilename(Start)].push_back(MoveNs);
}
// Removes a class forward declaration from the code in the moved namespace and
Modified: clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp?rev=290966&r1=290965&r2=290966&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp Wed Jan 4 08:49:08 2017
@@ -110,12 +110,52 @@ TEST_F(ChangeNamespaceTest, NewNsNestedI
"namespace nc {\n"
"class A {};\n"
"} // namespace nc\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, NewNsNestedInOldNsWithSurroundingNewLines) {
+ NewNamespace = "na::nb::nc";
+ std::string Code = "namespace na {\n"
+ "namespace nb {\n"
+ "\n"
+ "class A {};\n"
+ "\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+ std::string Expected = "namespace na {\n"
+ "namespace nb {\n"
+ "namespace nc {\n"
+ "\n"
+ "class A {};\n"
"\n"
+ "} // namespace nc\n"
"} // namespace nb\n"
"} // namespace na\n";
EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
}
+TEST_F(ChangeNamespaceTest, MoveOldNsWithSurroundingNewLines) {
+ NewNamespace = "nx::ny";
+ std::string Code = "namespace na {\n"
+ "namespace nb {\n"
+ "\n"
+ "class A {};\n"
+ "\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+ std::string Expected = "\n\n"
+ "namespace nx {\n"
+ "namespace ny {\n"
+ "\n"
+ "class A {};\n"
+ "\n"
+ "} // namespace ny\n"
+ "} // namespace nx\n";
+ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
TEST_F(ChangeNamespaceTest, NewNsNestedInOldNsWithRefs) {
NewNamespace = "na::nb::nc";
std::string Code = "namespace na {\n"
@@ -134,7 +174,6 @@ TEST_F(ChangeNamespaceTest, NewNsNestedI
"class C {};\n"
"void f() { A a; B b; }\n"
"} // namespace nc\n"
- "\n"
"} // namespace nb\n"
"} // namespace na\n";
EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
@@ -1497,7 +1536,7 @@ TEST_F(ChangeNamespaceTest, UsingAliasIn
"void f() {\n"
" GG<float> g;\n"
"}\n"
- "} // namespace nc\n\n"
+ "} // namespace nc\n"
"} // namespace nb\n"
"} // namespace na\n";
EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
@@ -1554,7 +1593,7 @@ TEST_F(ChangeNamespaceTest, TemplateUsin
" struct Derived::Nested nested;\n"
" const struct Derived::Nested *nested_ptr;\n"
"}\n"
- "} // namespace nc\n\n"
+ "} // namespace nc\n"
"} // namespace nb\n"
"} // namespace na\n";
EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
More information about the cfe-commits
mailing list