[clangd] Rename: merge index/AST refs path-insensitively where needed

[sammccall]

Summary

If you have c:\foo open, and C:\foo indexed (case difference) then these
need to be considered the same file. Otherwise we emit edits to both,
and editors do... something that isn't pretty.

Maybe more centralized normalization is called for, but it's not trivial
to do this while also being case-preserving. see
https://github.com/clangd/clangd/issues/108

Fixes https://github.com/clangd/clangd/issues/665

[sammccall]

remove debug printfs

[sammccall]

revert unintended formatting changes

[hokein]

Thanks for fixing this!

clang-tools-extra/clangd/GlobalCompilationDatabase.cpp

DirectoryBasedGlobalCompilationDatabase::getCompileCommand(PathRef File) const {
 
   auto Candidates = Res->CDB->getCompileCommands(File);
   if (!Candidates.empty())
     return std::move(Candidates.front());
 
   return None;
 }
 
-// For platforms where paths are case-insensitive (but case-preserving),
-// we need to do case-insensitive comparisons and use lowercase keys.
-// FIXME: Make Path a real class with desired semantics instead.
-//        This class is not the only place this problem exists.
-// FIXME: Mac filesystems default to case-insensitive, but may be sensitive.
-
-static std::string maybeCaseFoldPath(PathRef Path) {
-#if defined(_WIN32) || defined(__APPLE__)
-  return Path.lower();
-#else
-  return std::string(Path);
-#endif
-}
-
 std::vector<DirectoryBasedGlobalCompilationDatabase::DirectoryCache *>
 DirectoryBasedGlobalCompilationDatabase::getDirectoryCaches(
     llvm::ArrayRef<llvm::StringRef> Dirs) const {
   std::vector<std::string> FoldedDirs;
   FoldedDirs.reserve(Dirs.size());
   for (const auto &Dir : Dirs) {
 #ifndef NDEBUG
     if (!llvm::sys::path::is_absolute(Dir))

clang-tools-extra/clangd/refactor/Rename.cpp

llvm::Optional<std::string> getOtherRefFile(const Decl &D, StringRef MainFile,
   // is smaller than the limit.
   Req.Limit = 100;
   Req.IDs.insert(getSymbolID(&D));
   llvm::Optional<std::string> OtherFile;
   Index.refs(Req, [&](const Ref &R) {
     if (OtherFile)
       return;
     if (auto RefFilePath = filePath(R.Location, /*HintFilePath=*/MainFile)) {
-      if (*RefFilePath != MainFile)
+      if (!pathEqual(*RefFilePath, MainFile))
         OtherFile = *RefFilePath;
     }
   });
   return OtherFile;
 }
 
 // Canonical declarations help simplify the process of renaming. Examples:
 // - Template's canonical decl is the templated declaration (i.e.
findOccurrencesOutsideFile(const NamedDecl &RenameDecl,
   // Absolute file path => rename occurrences in that file.
   llvm::StringMap<std::vector<Range>> AffectedFiles;
   bool HasMore = Index.refs(RQuest, [&](const Ref &R) {
     if (AffectedFiles.size() >= MaxLimitFiles)
       return;
     if ((R.Kind & RefKind::Spelled) == RefKind::Unknown)
       return;
     if (auto RefFilePath = filePath(R.Location, /*HintFilePath=*/MainFile)) {
-      if (*RefFilePath != MainFile)
+      if (!pathEqual(*RefFilePath, MainFile))
         AffectedFiles[*RefFilePath].push_back(toRange(R.Location));
     }
   });
 
   if (AffectedFiles.size() >= MaxLimitFiles)
     return error("The number of affected files exceeds the max limit {0}",
                  MaxLimitFiles);
   if (HasMore)

clang-tools-extra/clangd/support/CMakeLists.txt

 include_directories(..)
 add_clang_library(clangdSupport
   Cancellation.cpp
   Context.cpp
   FileCache.cpp
   Logger.cpp
   Markup.cpp
   MemoryTree.cpp
+  Path.cpp
   Shutdown.cpp
   Threading.cpp
   ThreadsafeFS.cpp
   Trace.cpp
 
   LINK_LIBS
   ${LLVM_PTHREAD_LIB}
   ${CLANGD_ATOMIC_LIB}
   )

clang-tools-extra/clangd/support/Path.h

 
 /// A typedef to represent a file path. Used solely for more descriptive
 /// signatures.
 using Path = std::string;
 /// A typedef to represent a ref to file path. Used solely for more descriptive
 /// signatures.
 using PathRef = llvm::StringRef;
 
+// For platforms where paths are case-insensitive (but case-preserving),
+// we need to do case-insensitive comparisons and use lowercase keys.
+// FIXME: Make Path a real class with desired semantics instead.
+std::string maybeCaseFoldPath(PathRef Path);
+bool pathEqual(PathRef, PathRef);
+
 } // namespace clangd
 } // namespace clang
 
 #endif

clang-tools-extra/clangd/support/Path.cpp

+//===--- Path.cpp -------------------------------------------*- C++-*------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "support/Path.h"
+namespace clang {
+namespace clangd {
+
+std::string maybeCaseFoldPath(PathRef Path) {
+#if defined(_WIN32) || defined(__APPLE__)
+  return Path.lower();
+#else
+  return std::string(Path);
+#endif
+}
+
+bool pathEqual(PathRef A, PathRef B) {
+#if defined(_WIN32) || defined(__APPLE__)
+  return A.equals_lower(B);
+#else
+  return A == B;
+#endif
+}
+
+} // namespace clangd
+} // namespace clang

clang-tools-extra/clangd/unittests/RenameTests.cpp

if (!WantRename) {
                                  << llvm::toString(Results.takeError());
       ASSERT_EQ(1u, Results->GlobalChanges.size());
       EXPECT_EQ(applyEdits(std::move(Results->GlobalChanges)).front().second,
                 expectedResult(T, NewName));
     }
   }
 }
 
+MATCHER_P(newText, T, "") { return arg.newText == T; }
+
+TEST(RenameTest, IndexMergeMainFile) {
+  Annotations Code("int ^x();");
+  TestTU TU = TestTU::withCode(Code.code());
+  TU.Filename = "main.cc";
+  auto AST = TU.build();
+
+  auto Main = testPath("main.cc");
+
+  auto Rename = [&](const SymbolIndex *Idx) {
+    auto GetDirtyBuffer = [&](PathRef Path) -> llvm::Optional<std::string> {
+      return Code.code().str(); // Every file has the same content.
+    };
+    RenameOptions Opts;
+    Opts.AllowCrossFile = true;
+    RenameInputs Inputs{Code.point(), "xPrime", AST,           Main,
+                        Idx,          Opts,     GetDirtyBuffer};
+    auto Results = rename(Inputs);
+    EXPECT_TRUE(bool(Results)) << llvm::toString(Results.takeError());
+    return std::move(*Results);
+  };
+
+  // We do not expect to see duplicated edits from AST vs index.
+  auto Results = Rename(TU.index().get());
+  EXPECT_THAT(Results.GlobalChanges.keys(), ElementsAre(Main));
+  EXPECT_THAT(Results.GlobalChanges[Main].asTextEdits(),
+              ElementsAre(newText("xPrime")));
+
+  // Sanity check: we do expect to see index results!
+  TU.Filename = "other.cc";
+  Results = Rename(TU.index().get());
+  EXPECT_THAT(Results.GlobalChanges.keys(),
+              UnorderedElementsAre(Main, testPath("other.cc")));
+
+#if defined(_WIN32) || defined(__APPLE__)
+  // On case-insensitive systems, no duplicates if AST vs index case differs.
+  // https://github.com/clangd/clangd/issues/665
+  TU.Filename = "MAIN.CC";
+  Results = Rename(TU.index().get());
+  EXPECT_THAT(Results.GlobalChanges.keys(), ElementsAre(Main));
+  EXPECT_THAT(Results.GlobalChanges[Main].asTextEdits(),
+              ElementsAre(newText("xPrime")));
+#endif
+}
+
 TEST(RenameTest, MainFileReferencesOnly) {
   // filter out references not from main file.
   llvm::StringRef Test =
       R"cpp(
         void test() {
           int [[fo^o]] = 1;
           // rename references not from main file are not included.
           #include "foo.inc"