[clang-tools-extra] r282247 - [clang-move] The new.cc file should include new_header.h instead of old_header.h

[Haojian Wu via cfe-commits]

Author: hokein
Date: Fri Sep 23 08:28:38 2016
New Revision: 282247

URL: http://llvm.org/viewvc/llvm-project?rev=282247&view=rev
Log:
[clang-move] The new.cc file should include new_header.h instead of old_header.h

Summary:
Previously, all #includes (includeing old_header.h) in old.cc will be copied to new.cc,
however, the new.cc should include new_header.h instead of the old_header.h

Before applying the patch, the new.cc looks like:

```
#include "old_header.h"
...
```

The new.cc looks like with this patch:

```
#include "new_header"
...
```

Reviewers: ioeric

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D24828

Modified:
    clang-tools-extra/trunk/clang-move/ClangMove.cpp
    clang-tools-extra/trunk/clang-move/ClangMove.h
    clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp

Modified: clang-tools-extra/trunk/clang-move/ClangMove.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-move/ClangMove.cpp?rev=282247&r1=282246&r2=282247&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-move/ClangMove.cpp (original)
+++ clang-tools-extra/trunk/clang-move/ClangMove.cpp Fri Sep 23 08:28:38 2016
@@ -41,15 +41,8 @@ public:
                           const clang::FileEntry * /*File*/,
                           StringRef /*SearchPath*/, StringRef /*RelativePath*/,
                           const clang::Module * /*Imported*/) override {
-    if (const auto *FileEntry = SM.getFileEntryForID(SM.getFileID(HashLoc))) {
-      if (IsAngled) {
-        MoveTool->addIncludes("#include <" + FileName.str() + ">\n",
-                              FileEntry->getName());
-      } else {
-        MoveTool->addIncludes("#include \"" + FileName.str() + "\"\n",
-                              FileEntry->getName());
-      }
-    }
+    if (const auto *FileEntry = SM.getFileEntryForID(SM.getFileID(HashLoc)))
+      MoveTool->addIncludes(FileName, IsAngled, FileEntry->getName());
   }
 
 private:
@@ -135,7 +128,7 @@ createInsertedReplacements(const std::ve
 
   // Add #Includes.
   std::string AllIncludesString;
-  // FIXME: Filter out the old_header.h and add header guard.
+  // FIXME: Add header guard.
   for (const auto &Include : Includes)
     AllIncludesString += Include;
   clang::tooling::Replacement InsertInclude(FileName, 0, 0, AllIncludesString);
@@ -205,6 +198,8 @@ ClangMoveTool::ClangMoveTool(
       std::map<std::string, tooling::Replacements> &FileToReplacements)
       : Spec(MoveSpec), FileToReplacements(FileToReplacements) {
   Spec.Name = llvm::StringRef(Spec.Name).ltrim(':');
+  if (!Spec.NewHeader.empty())
+    CCIncludes.push_back("#include \"" + Spec.NewHeader + "\"\n");
 }
 
 void ClangMoveTool::registerMatchers(ast_matchers::MatchFinder *Finder) {
@@ -290,12 +285,22 @@ void ClangMoveTool::run(const ast_matche
   }
 }
 
-void ClangMoveTool::addIncludes(llvm::StringRef IncludeLine,
+void ClangMoveTool::addIncludes(llvm::StringRef IncludeHeader, bool IsAngled,
                                 llvm::StringRef FileName) {
+  // FIXME: Add old.h to the new.cc/h when the new target has dependencies on
+  // old.h/c. For instance, when moved class uses another class defined in
+  // old.h, the old.h should be added in new.h.
+  if (!Spec.OldHeader.empty() &&
+      llvm::StringRef(Spec.OldHeader).endswith(IncludeHeader))
+    return;
+
+  std::string IncludeLine =
+      IsAngled ? ("#include <" + IncludeHeader + ">\n").str()
+               : ("#include \"" + IncludeHeader + "\"\n").str();
   if (!Spec.OldHeader.empty() && FileName.endswith(Spec.OldHeader))
-    HeaderIncludes.push_back(IncludeLine.str());
+    HeaderIncludes.push_back(IncludeLine);
   else if (!Spec.OldCC.empty() && FileName.endswith(Spec.OldCC))
-    CCIncludes.push_back(IncludeLine.str());
+    CCIncludes.push_back(IncludeLine);
 }
 
 void ClangMoveTool::removeClassDefinitionInOldFiles() {

Modified: clang-tools-extra/trunk/clang-move/ClangMove.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-move/ClangMove.h?rev=282247&r1=282246&r2=282247&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-move/ClangMove.h (original)
+++ clang-tools-extra/trunk/clang-move/ClangMove.h Fri Sep 23 08:28:38 2016
@@ -55,7 +55,8 @@ public:
 
   // Add #includes from old.h/cc files. The FileName is where the #include
   // comes from.
-  void addIncludes(llvm::StringRef IncludeLine, llvm::StringRef FileName);
+  void addIncludes(llvm::StringRef IncludeHeader, bool IsAngled,
+                   llvm::StringRef FileName);
 
 private:
   void removeClassDefinitionInOldFiles();

Modified: clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp?rev=282247&r1=282246&r2=282247&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp Fri Sep 23 08:28:38 2016
@@ -119,8 +119,7 @@ const char ExpectedNewHeader[] = "namesp
                                  "} // namespace b\n"
                                  "} // namespace a\n";
 
-const char ExpectedNewCC[] = "#include \"foo.h\"\n"
-                             "namespace a {\n"
+const char ExpectedNewCC[] = "namespace a {\n"
                              "namespace b {\n"
                              "namespace {\n"
                              "void f1() {}\n"
@@ -181,11 +180,12 @@ TEST(ClangMove, MoveHeaderAndCC) {
   Spec.OldCC = "foo.cc";
   Spec.NewHeader = "new_foo.h";
   Spec.NewCC = "new_foo.cc";
+  std::string ExpectedHeader = "#include \"" + Spec.NewHeader + "\"\n";
   auto Results = runClangMoveOnCode(Spec);
   EXPECT_EQ(ExpectedTestHeader, Results[Spec.OldHeader]);
   EXPECT_EQ(ExpectedTestCC, Results[Spec.OldCC]);
   EXPECT_EQ(ExpectedNewHeader, Results[Spec.NewHeader]);
-  EXPECT_EQ(ExpectedNewCC, Results[Spec.NewCC]);
+  EXPECT_EQ(ExpectedHeader + ExpectedNewCC, Results[Spec.NewCC]);
 }
 
 TEST(ClangMove, MoveHeaderOnly) {
@@ -204,10 +204,11 @@ TEST(ClangMove, MoveCCOnly) {
   Spec.Name = "a::b::Foo";
   Spec.OldCC = "foo.cc";
   Spec.NewCC = "new_foo.cc";
+  std::string ExpectedHeader = "#include \"foo.h\"\n";
   auto Results = runClangMoveOnCode(Spec);
   EXPECT_EQ(2u, Results.size());
   EXPECT_EQ(ExpectedTestCC, Results[Spec.OldCC]);
-  EXPECT_EQ(ExpectedNewCC, Results[Spec.NewCC]);
+  EXPECT_EQ(ExpectedHeader + ExpectedNewCC, Results[Spec.NewCC]);
 }
 
 TEST(ClangMove, MoveNonExistClass) {