[clang-tools-extra] d99b2a9 - [clangd][remote] Add Windows paths support

[Aleksandr Platonov via cfe-commits]

Author: Aleksandr Platonov
Date: 2020-10-20T13:04:20+03:00
New Revision: d99b2a976a37f5a63117086d464df40c124f5777

URL: https://github.com/llvm/llvm-project/commit/d99b2a976a37f5a63117086d464df40c124f5777
DIFF: https://github.com/llvm/llvm-project/commit/d99b2a976a37f5a63117086d464df40c124f5777.diff

LOG: [clangd][remote] Add Windows paths support

Without this patch 6 marshalling tests fail on Windows.
This patch contains the following changes:
- Allow paths with Windows slashes (convert to the POSIX style instead of assertion)
- Add support for URI with Windows path.
- Change the value of the second parameter of several `llvm::sys::path::convert_to_slash()` calls: we should use `windows` instead of `posix` to ensure UNIX slashes in the path.
- Port `RemoteMarshallingTest::IncludeHeaderURI` test to Windows.

Reviewed By: kbobyrev

Differential Revision: https://reviews.llvm.org/D89529

Added: 
    

Modified: 
    clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
    clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
index d61848f295a3..6285022fc0a8 100644
--- a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -50,20 +50,23 @@ llvm::Expected<llvm::DenseSet<SymbolID>> getIDs(IDRange IDs) {
 Marshaller::Marshaller(llvm::StringRef RemoteIndexRoot,
                        llvm::StringRef LocalIndexRoot)
     : Strings(Arena) {
+  llvm::StringRef PosixSeparator =
+      llvm::sys::path::get_separator(llvm::sys::path::Style::posix);
   if (!RemoteIndexRoot.empty()) {
     assert(llvm::sys::path::is_absolute(RemoteIndexRoot));
-    assert(RemoteIndexRoot ==
-           llvm::sys::path::convert_to_slash(RemoteIndexRoot));
-    this->RemoteIndexRoot = RemoteIndexRoot.str();
-    if (!RemoteIndexRoot.endswith(llvm::sys::path::get_separator()))
-      *this->RemoteIndexRoot += llvm::sys::path::get_separator();
+    this->RemoteIndexRoot = llvm::sys::path::convert_to_slash(
+        RemoteIndexRoot, llvm::sys::path::Style::windows);
+    llvm::StringRef Path(*this->RemoteIndexRoot);
+    if (!Path.endswith(PosixSeparator))
+      *this->RemoteIndexRoot += PosixSeparator;
   }
   if (!LocalIndexRoot.empty()) {
     assert(llvm::sys::path::is_absolute(LocalIndexRoot));
-    assert(LocalIndexRoot == llvm::sys::path::convert_to_slash(LocalIndexRoot));
-    this->LocalIndexRoot = LocalIndexRoot.str();
-    if (!LocalIndexRoot.endswith(llvm::sys::path::get_separator()))
-      *this->LocalIndexRoot += llvm::sys::path::get_separator();
+    this->LocalIndexRoot = llvm::sys::path::convert_to_slash(
+        LocalIndexRoot, llvm::sys::path::Style::windows);
+    llvm::StringRef Path(*this->LocalIndexRoot);
+    if (!Path.endswith(PosixSeparator))
+      *this->LocalIndexRoot += PosixSeparator;
   }
   assert(!RemoteIndexRoot.empty() || !LocalIndexRoot.empty());
 }
@@ -92,6 +95,9 @@ Marshaller::fromProtobuf(const FuzzyFindRequest *Message) {
   for (const auto &Path : Message->proximity_paths()) {
     llvm::SmallString<256> LocalPath = llvm::StringRef(*RemoteIndexRoot);
     llvm::sys::path::append(LocalPath, Path);
+    // FuzzyFindRequest requires proximity paths to have platform-native format
+    // in order for SymbolIndex to process the query correctly.
+    llvm::sys::path::native(LocalPath);
     Result.ProximityPaths.push_back(std::string(LocalPath));
   }
   for (const auto &Type : Message->preferred_types())
@@ -209,7 +215,7 @@ FuzzyFindRequest Marshaller::toProtobuf(const clangd::FuzzyFindRequest &From) {
     llvm::SmallString<256> RelativePath = llvm::StringRef(Path);
     if (llvm::sys::path::replace_path_prefix(RelativePath, *LocalIndexRoot, ""))
       RPCRequest.add_proximity_paths(llvm::sys::path::convert_to_slash(
-          RelativePath, llvm::sys::path::Style::posix));
+          RelativePath, llvm::sys::path::Style::windows));
   }
   for (const auto &Type : From.PreferredTypes)
     RPCRequest.add_preferred_types(Type);
@@ -315,12 +321,17 @@ llvm::Expected<std::string> Marshaller::uriToRelativePath(llvm::StringRef URI) {
   if (ParsedURI->scheme() != "file")
     return error("Can not use URI schemes other than file, given: '{0}'.", URI);
   llvm::SmallString<256> Result = ParsedURI->body();
+  llvm::StringRef Path(Result);
+  // Check for Windows paths (URI=file:///X:/path => Body=/X:/path)
+  if (llvm::sys::path::is_absolute(Path.substr(1),
+                                   llvm::sys::path::Style::windows))
+    Result = Path.drop_front();
   if (!llvm::sys::path::replace_path_prefix(Result, *RemoteIndexRoot, ""))
     return error("File path '{0}' doesn't start with '{1}'.", Result.str(),
                  *RemoteIndexRoot);
-  // Make sure the result has UNIX slashes.
-  return llvm::sys::path::convert_to_slash(Result,
-                                           llvm::sys::path::Style::posix);
+  assert(Result == llvm::sys::path::convert_to_slash(
+                       Result, llvm::sys::path::Style::windows));
+  return std::string(Result);
 }
 
 clangd::SymbolLocation::Position

diff  --git a/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp b/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
index ff819146621a..6ef8da59861f 100644
--- a/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
+++ b/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
@@ -239,8 +239,8 @@ TEST(RemoteMarshallingTest, IncludeHeaderURIs) {
 
   clangd::Symbol::IncludeHeaderWithReferences Header;
   // Add only valid headers.
-  Header.IncludeHeader = Strings.save(
-      URI::createFile("/usr/local/user/home/project/Header.h").toString());
+  Header.IncludeHeader =
+      Strings.save(URI::createFile(testPath("project/Header.h")).toString());
   Header.References = 21;
   Sym.IncludeHeaders.push_back(Header);
   Header.IncludeHeader = Strings.save("<iostream>");
@@ -250,7 +250,7 @@ TEST(RemoteMarshallingTest, IncludeHeaderURIs) {
   Header.References = 200;
   Sym.IncludeHeaders.push_back(Header);
 
-  Marshaller ProtobufMarshaller(convert_to_slash("/"), convert_to_slash("/"));
+  Marshaller ProtobufMarshaller(testPath(""), testPath(""));
 
   auto Serialized = ProtobufMarshaller.toProtobuf(Sym);
   ASSERT_TRUE(bool(Serialized));