aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSam McCall <sam.mccall@gmail.com>2021-01-31 13:53:22 +0100
committerTom Stellard <tstellar@redhat.com>2021-02-22 12:19:07 -0800
commit8eeb3d99933a3246f2d850b807cf54f11a3a8dce (patch)
treeef0a14beaaa6d91381781142f3b14698f13f49e1
parent76e4c93ea42b3d23907611d14e347bfeae8d4b0a (diff)
downloadllvm-8eeb3d99933a3246f2d850b807cf54f11a3a8dce.zip
llvm-8eeb3d99933a3246f2d850b807cf54f11a3a8dce.tar.gz
llvm-8eeb3d99933a3246f2d850b807cf54f11a3a8dce.tar.bz2
[clangd] Rename: merge index/AST refs path-insensitively where needed
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 Differential Revision: https://reviews.llvm.org/D95759 (cherry picked from commit b63cd4db915c08e0cb4cf668a18de24b67f2c44c)
-rw-r--r--clang-tools-extra/clangd/GlobalCompilationDatabase.cpp14
-rw-r--r--clang-tools-extra/clangd/refactor/Rename.cpp4
-rw-r--r--clang-tools-extra/clangd/support/CMakeLists.txt1
-rw-r--r--clang-tools-extra/clangd/support/Path.cpp30
-rw-r--r--clang-tools-extra/clangd/support/Path.h6
-rw-r--r--clang-tools-extra/clangd/unittests/RenameTests.cpp46
6 files changed, 85 insertions, 16 deletions
diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
index 542d0c3..a38c8a5 100644
--- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -396,20 +396,6 @@ DirectoryBasedGlobalCompilationDatabase::getCompileCommand(PathRef File) const {
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 {
diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp
index d3c7da9..a857b34 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -68,7 +68,7 @@ llvm::Optional<std::string> getOtherRefFile(const Decl &D, StringRef MainFile,
if (OtherFile)
return;
if (auto RefFilePath = filePath(R.Location, /*HintFilePath=*/MainFile)) {
- if (*RefFilePath != MainFile)
+ if (!pathEqual(*RefFilePath, MainFile))
OtherFile = *RefFilePath;
}
});
@@ -474,7 +474,7 @@ findOccurrencesOutsideFile(const NamedDecl &RenameDecl,
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));
}
});
diff --git a/clang-tools-extra/clangd/support/CMakeLists.txt b/clang-tools-extra/clangd/support/CMakeLists.txt
index f0fe073..fc7d7a2 100644
--- a/clang-tools-extra/clangd/support/CMakeLists.txt
+++ b/clang-tools-extra/clangd/support/CMakeLists.txt
@@ -23,6 +23,7 @@ add_clang_library(clangdSupport
Logger.cpp
Markup.cpp
MemoryTree.cpp
+ Path.cpp
Shutdown.cpp
Threading.cpp
ThreadsafeFS.cpp
diff --git a/clang-tools-extra/clangd/support/Path.cpp b/clang-tools-extra/clangd/support/Path.cpp
new file mode 100644
index 0000000..f72d000
--- /dev/null
+++ b/clang-tools-extra/clangd/support/Path.cpp
@@ -0,0 +1,30 @@
+//===--- 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
diff --git a/clang-tools-extra/clangd/support/Path.h b/clang-tools-extra/clangd/support/Path.h
index 4d4ad7f..4029031 100644
--- a/clang-tools-extra/clangd/support/Path.h
+++ b/clang-tools-extra/clangd/support/Path.h
@@ -22,6 +22,12 @@ using Path = std::string;
/// 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
diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp
index 4bc0379..e25850a 100644
--- a/clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -1067,6 +1067,52 @@ TEST(RenameTest, Renameable) {
}
}
+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 =