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; } }); @@ -468,13 +468,15 @@ // Absolute file path => rename occurrences in that file. llvm::StringMap> AffectedFiles; + llvm::errs() << "querying index\n"; bool HasMore = Index.refs(RQuest, [&](const Ref &R) { + llvm::errs() << "found...\n"; 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)); } }); @@ -675,6 +677,8 @@ if (!Opts.AllowCrossFile || RenameDecl.getParentFunctionOrMethod()) { Result.GlobalChanges = FileEdits( {std::make_pair(RInputs.MainFilePath, std::move(MainFileEdits))}); + llvm::errs() << "bail\n"; + llvm::errs() << Opts.AllowCrossFile << "\n"; return Result; } 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 = @@ -1576,43 +1622,43 @@ llvm::StringRef IndexedCode; llvm::StringRef DraftCode; } Tests[] = { - { - // both line and column are changed, not a near miss. - R"cpp( + { + // both line and column are changed, not a near miss. + R"cpp( int [[x]] = 0; )cpp", - R"cpp( + R"cpp( // insert a line. double x = 0; )cpp", - }, - { - // subset. - R"cpp( + }, + { + // subset. + R"cpp( int [[x]] = 0; )cpp", - R"cpp( + R"cpp( int [[x]] = 0; {int x = 0; } )cpp", - }, - { - // shift columns. - R"cpp(int [[x]] = 0; void foo(int x);)cpp", - R"cpp(double [[x]] = 0; void foo(double x);)cpp", - }, - { - // shift lines. - R"cpp( + }, + { + // shift columns. + R"cpp(int [[x]] = 0; void foo(int x);)cpp", + R"cpp(double [[x]] = 0; void foo(double x);)cpp", + }, + { + // shift lines. + R"cpp( int [[x]] = 0; void foo(int x); )cpp", - R"cpp( + R"cpp( // insert a line. int [[x]] = 0; void foo(int x); )cpp", - }, + }, }; LangOptions LangOpts; LangOpts.CPlusPlus = true; @@ -1635,69 +1681,62 @@ struct { llvm::StringRef IndexedCode; llvm::StringRef LexedCode; - } Tests[] = { - { - // no lexed ranges. - "[[]]", - "", - }, - { - // both line and column are changed, not a near miss. - R"([[]])", - R"( + } Tests[] = {{ + // no lexed ranges. + "[[]]", + "", + }, + { + // both line and column are changed, not a near miss. + R"([[]])", + R"( [[]] )", - }, - { - // subset. - "[[]]", - "^[[]] [[]]" - }, - { - // shift columns. - "[[]] [[]]", - " ^[[]] ^[[]] [[]]" - }, - { - R"( + }, + {// subset. + "[[]]", "^[[]] [[]]"}, + {// shift columns. + "[[]] [[]]", " ^[[]] ^[[]] [[]]"}, + { + R"( [[]] [[]] [[]] )", - R"( + R"( // insert a line ^[[]] ^[[]] ^[[]] )", - }, - { - R"( + }, + { + R"( [[]] [[]] [[]] )", - R"( + R"( // insert a line ^[[]] ^[[]] ^[[]] // column is shifted. )", - }, - { - R"( + }, + { + R"( [[]] [[]] [[]] )", - R"( + R"( // insert a line [[]] [[]] [[]] // not mapped (both line and column are changed). )", - }, - { - R"( + }, + { + R"( [[]] [[]] @@ -1706,7 +1745,7 @@ } )", - R"( + R"( // insert a new line ^[[]] ^[[]] @@ -1715,22 +1754,21 @@ ^[[]] [[]] // additional range )", - }, - { - // non-distinct result (two best results), not a near miss - R"( + }, + { + // non-distinct result (two best results), not a near miss + R"( [[]] [[]] [[]] )", - R"( + R"( [[]] [[]] [[]] [[]] )", - } - }; + }}; for (const auto &T : Tests) { SCOPED_TRACE(T.IndexedCode); auto Lexed = Annotations(T.LexedCode);