diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp --- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp +++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp @@ -395,20 +395,6 @@ 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::getDirectoryCaches( llvm::ArrayRef Dirs) const { diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp --- a/clang-tools-extra/clangd/refactor/Rename.cpp +++ b/clang-tools-extra/clangd/refactor/Rename.cpp @@ -68,7 +68,7 @@ if (OtherFile) return; if (auto RefFilePath = filePath(R.Location, /*HintFilePath=*/MainFile)) { - if (*RefFilePath != MainFile) + if (!pathEqual(*RefFilePath, MainFile)) OtherFile = *RefFilePath; } }); @@ -474,7 +474,7 @@ 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 --- a/clang-tools-extra/clangd/support/CMakeLists.txt +++ b/clang-tools-extra/clangd/support/CMakeLists.txt @@ -23,6 +23,7 @@ Logger.cpp Markup.cpp MemoryTree.cpp + Path.cpp Shutdown.cpp Threading.cpp ThreadsafeFS.cpp diff --git a/clang-tools-extra/clangd/support/Path.h b/clang-tools-extra/clangd/support/Path.h --- a/clang-tools-extra/clangd/support/Path.h +++ b/clang-tools-extra/clangd/support/Path.h @@ -22,6 +22,12 @@ /// 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/support/Path.cpp b/clang-tools-extra/clangd/support/Path.cpp new file mode 100644 --- /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/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp --- a/clang-tools-extra/clangd/unittests/RenameTests.cpp +++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp @@ -1067,6 +1067,52 @@ } } +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 { + 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 =