[PATCH] D25565: Deduplicate sets of replacements by file names.

[Eric Liu via cfe-commits]

ioeric updated this revision to Diff 74634.
ioeric added a comment.

- Merge branch 'master' of http://llvm.org/git/clang into arcpatch-D25565
- Forgot to update names in tests...


https://reviews.llvm.org/D25565

Files:
  include/clang/Tooling/Core/Replacement.h
  include/clang/Tooling/Refactoring.h
  lib/Tooling/Core/Replacement.cpp
  lib/Tooling/Refactoring.cpp
  unittests/Tooling/RefactoringTest.cpp


Index: unittests/Tooling/RefactoringTest.cpp
===================================================================
--- unittests/Tooling/RefactoringTest.cpp
+++ unittests/Tooling/RefactoringTest.cpp
@@ -972,5 +972,23 @@
       toReplacements({{"", 0, 3, "cc"}, {"", 3, 3, "dd"}}));
 }
 
+TEST(DeduplicateByFileTest, LeaveLeadingDotDot) {
+  std::map<std::string, Replacements> FileToReplaces;
+  FileToReplaces["../../a/b/.././c.h"] = Replacements();
+  FileToReplaces["../../a/c.h"] = Replacements();
+  FileToReplaces = groupReplacementsByFile(FileToReplaces);
+  EXPECT_EQ(1u, FileToReplaces.size());
+  EXPECT_EQ("../../a/c.h", FileToReplaces.begin()->first);
+}
+
+TEST(DeduplicateByFileTest, RemoveDotSlash) {
+  std::map<std::string, Replacements> FileToReplaces;
+  FileToReplaces["./a/b/.././c.h"] = Replacements();
+  FileToReplaces["a/c.h"] = Replacements();
+  FileToReplaces = groupReplacementsByFile(FileToReplaces);
+  EXPECT_EQ(1u, FileToReplaces.size());
+  EXPECT_EQ("a/c.h", FileToReplaces.begin()->first);
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: lib/Tooling/Refactoring.cpp
===================================================================
--- lib/Tooling/Refactoring.cpp
+++ lib/Tooling/Refactoring.cpp
@@ -57,7 +57,7 @@
 
 bool RefactoringTool::applyAllReplacements(Rewriter &Rewrite) {
   bool Result = true;
-  for (const auto &Entry : FileToReplaces)
+  for (const auto &Entry : groupReplacementsByFile(FileToReplaces))
     Result = tooling::applyAllReplacements(Entry.second, Rewrite) && Result;
   return Result;
 }
@@ -73,7 +73,7 @@
   FileManager &Files = SM.getFileManager();
 
   bool Result = true;
-  for (const auto &FileAndReplaces : FileToReplaces) {
+  for (const auto &FileAndReplaces : groupReplacementsByFile(FileToReplaces)) {
     const std::string &FilePath = FileAndReplaces.first;
     auto &CurReplaces = FileAndReplaces.second;
 
Index: lib/Tooling/Core/Replacement.cpp
===================================================================
--- lib/Tooling/Core/Replacement.cpp
+++ lib/Tooling/Core/Replacement.cpp
@@ -21,6 +21,7 @@
 #include "clang/Lex/Lexer.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/raw_os_ostream.h"
 
 namespace clang {
@@ -564,13 +565,15 @@
   return Result;
 }
 
-std::map<std::string, Replacements>
-groupReplacementsByFile(const Replacements &Replaces) {
-  std::map<std::string, Replacements> FileToReplaces;
-  for (const auto &Replace : Replaces)
-    // We can ignore the Error here since \p Replaces is already conflict-free.
-    FileToReplaces[Replace.getFilePath()].add(Replace);
-  return FileToReplaces;
+std::map<std::string, Replacements> groupReplacementsByFile(
+    const std::map<std::string, Replacements> &FileToReplaces) {
+  std::map<std::string, Replacements> Result;
+  for (const auto &Entry : FileToReplaces) {
+    llvm::SmallString<256> CleanPath(Entry.first.data());
+    llvm::sys::path::remove_dots(CleanPath, /*remove_dot_dot=*/true);
+    Result[CleanPath.str()] = std::move(Entry.second);
+  }
+  return Result;
 }
 
 } // end namespace tooling
Index: include/clang/Tooling/Refactoring.h
===================================================================
--- include/clang/Tooling/Refactoring.h
+++ include/clang/Tooling/Refactoring.h
@@ -55,6 +55,9 @@
 
   /// \brief Apply all stored replacements to the given Rewriter.
   ///
+  /// FileToReplaces will be deduplicated with `groupReplacementsByFile` before
+  /// application.
+  ///
   /// Replacement applications happen independently of the success of other
   /// applications.
   ///
@@ -75,6 +78,9 @@
 ///
 /// \pre Replacements must be conflict-free.
 ///
+/// FileToReplaces will be deduplicated with `groupReplacementsByFile` before
+/// application.
+///
 /// Replacement applications happen independently of the success of other
 /// applications.
 ///
Index: include/clang/Tooling/Core/Replacement.h
===================================================================
--- include/clang/Tooling/Core/Replacement.h
+++ include/clang/Tooling/Core/Replacement.h
@@ -230,8 +230,6 @@
   Replacements(const_iterator Begin, const_iterator End)
       : Replaces(Begin, End) {}
 
-  Replacements mergeReplacements(const ReplacementsImpl &Second) const;
-
   // Returns `R` with new range that refers to code after `Replaces` being
   // applied.
   Replacement getReplacementInChangedCode(const Replacement &R) const;
@@ -294,10 +292,11 @@
 calculateRangesAfterReplacements(const Replacements &Replaces,
                                  const std::vector<Range> &Ranges);
 
-/// \brief Groups a random set of replacements by file path. Replacements
-/// related to the same file entry are put into the same vector.
-std::map<std::string, Replacements>
-groupReplacementsByFile(const Replacements &Replaces);
+/// \brief If there are multiple <File, Replacements> pairs with the same file
+/// path after removing dots, we only keep one pair (with path after dots being
+/// removed) and discard the rest.
+std::map<std::string, Replacements> groupReplacementsByFile(
+    const std::map<std::string, Replacements> &FileToReplaces);
 
 template <typename Node>
 Replacement::Replacement(const SourceManager &Sources,

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits