[clang-move] Clever on handling header file which includes itself.

Summary:
Previously, we assume only old.cc includes "old.h", which would
introduce incorrect fixes for the cases where old.h also includes `#include "old.h"`

Although it should not be occurred in real projects, clang-move should handle this.

Old.h:

```
class Foo {};
```

after moving to a new old.h:

```
class Foo {};
```

Reviewers: ioeric

Reviewed By: ioeric

Subscribers: cfe-commits, klimek

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

llvm-svn: 323865

clang-tools-extra/clang-move/ClangMove.cpp

@@ -694,22 +694,28 @@ void ClangMoveTool::addIncludes(llvm::StringRef IncludeHeader, bool IsAngled,
                                 const SourceManager &SM) {
   SmallVector<char, 128> HeaderWithSearchPath;
   llvm::sys::path::append(HeaderWithSearchPath, SearchPath, IncludeHeader);
-  std::string AbsoluteOldHeader = makeAbsolutePath(Context->Spec.OldHeader);
+  std::string AbsoluteIncludeHeader =
-  if (AbsoluteOldHeader ==
       MakeAbsolutePath(SM, llvm::StringRef(HeaderWithSearchPath.data(),
-                                           HeaderWithSearchPath.size()))) {
+                                           HeaderWithSearchPath.size()));
-    OldHeaderIncludeRange = IncludeFilenameRange;
-    return;
-  }
-
   std::string IncludeLine =
       IsAngled ? ("#include <" + IncludeHeader + ">\n").str()
                : ("#include \"" + IncludeHeader + "\"\n").str();
 
+  std::string AbsoluteOldHeader = makeAbsolutePath(Context->Spec.OldHeader);
   std::string AbsoluteCurrentFile = MakeAbsolutePath(SM, FileName);
   if (AbsoluteOldHeader == AbsoluteCurrentFile) {
+    // Find old.h includes "old.h".
+    if (AbsoluteOldHeader == AbsoluteIncludeHeader) {
+      OldHeaderIncludeRangeInHeader = IncludeFilenameRange;
+      return;
+    }
     HeaderIncludes.push_back(IncludeLine);
   } else if (makeAbsolutePath(Context->Spec.OldCC) == AbsoluteCurrentFile) {
+    // Find old.cc includes "old.h".
+    if (AbsoluteOldHeader == AbsoluteIncludeHeader) {
+      OldHeaderIncludeRangeInCC = IncludeFilenameRange;
+      return;
+    }
     CCIncludes.push_back(IncludeLine);
   }
 }
@@ -857,13 +863,18 @@ void ClangMoveTool::moveAll(SourceManager &SM, StringRef OldFile,
   if (!NewFile.empty()) {
     auto AllCode = clang::tooling::Replacements(
         clang::tooling::Replacement(NewFile, 0, 0, Code));
-    // If we are moving from old.cc, an extra step is required: excluding
+    auto ReplaceOldInclude = [&](clang::CharSourceRange OldHeaderIncludeRange) {
-    // the #include of "old.h", instead, we replace it with #include of "new.h".
+      AllCode = AllCode.merge(clang::tooling::Replacements(
-    if (Context->Spec.NewCC == NewFile && OldHeaderIncludeRange.isValid()) {
+          clang::tooling::Replacement(SM, OldHeaderIncludeRange,
-      AllCode = AllCode.merge(
+                                      '"' + Context->Spec.NewHeader + '"')));
-          clang::tooling::Replacements(clang::tooling::Replacement(
+    };
-              SM, OldHeaderIncludeRange, '"' + Context->Spec.NewHeader + '"')));
+    // Fix the case where old.h/old.cc includes "old.h", we replace the
-    }
+    // `#include "old.h"` with `#include "new.h"`.
+    if (Context->Spec.NewCC == NewFile && OldHeaderIncludeRangeInCC.isValid())
+      ReplaceOldInclude(OldHeaderIncludeRangeInCC);
+    else if (Context->Spec.NewHeader == NewFile &&
+             OldHeaderIncludeRangeInHeader.isValid())
+      ReplaceOldInclude(OldHeaderIncludeRangeInHeader);
     Context->FileToReplacements[NewFile] = std::move(AllCode);
   }
 }

clang-tools-extra/clang-move/ClangMove.h

@@ -176,7 +176,11 @@ private:
   /// The source range for the written file name in #include (i.e. "old.h" for
   /// #include "old.h") in old.cc,  including the enclosing quotes or angle
   /// brackets.
-  clang::CharSourceRange OldHeaderIncludeRange;
+  clang::CharSourceRange OldHeaderIncludeRangeInCC;
+  /// The source range for the written file name in #include (i.e. "old.h" for
+  /// #include "old.h") in old.h,  including the enclosing quotes or angle
+  /// brackets.
+  clang::CharSourceRange OldHeaderIncludeRangeInHeader;
   /// Mapping from FilePath to FileID, which can be used in post processes like
   /// cleanup around replacements.
   llvm::StringMap<FileID> FilePathToFileID;

clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp

@@ -293,6 +293,32 @@ TEST(ClangMove, MoveNonExistClass) {
   EXPECT_EQ(0u, Results.size());
 }
 
+TEST(ClangMove, HeaderIncludeSelf) {
+  move::MoveDefinitionSpec Spec;
+  Spec.Names = {std::string("Foo")};
+  Spec.OldHeader = "foo.h";
+  Spec.OldCC = "foo.cc";
+  Spec.NewHeader = "new_foo.h";
+  Spec.NewCC = "new_foo.cc";
+
+  const char TestHeader[] = "#ifndef FOO_H\n"
+                            "#define FOO_H\n"
+                            "#include \"foo.h\"\n"
+                            "class Foo {};\n"
+                            "#endif\n";
+  const char TestCode[] = "#include \"foo.h\"";
+  const char ExpectedNewHeader[] = "#ifndef FOO_H\n"
+                                   "#define FOO_H\n"
+                                   "#include \"new_foo.h\"\n"
+                                   "class Foo {};\n"
+                                   "#endif\n";
+  const char ExpectedNewCC[] = "#include \"new_foo.h\"";
+  auto Results = runClangMoveOnCode(Spec, TestHeader, TestCode);
+  EXPECT_EQ("", Results[Spec.OldHeader]);
+  EXPECT_EQ(ExpectedNewHeader, Results[Spec.NewHeader]);
+  EXPECT_EQ(ExpectedNewCC, Results[Spec.NewCC]);
+}
+
 TEST(ClangMove, MoveAll) {
   std::vector<std::string> TestHeaders = {
     "class A {\npublic:\n  int f();\n};",