ilya-golovenko created this revision.
Herald added subscribers: cfe-commits, kbobyrev, usaxena95, kadircet, arphaman, 
jkorous, MaskRay, ilya-biryukov.
Herald added a project: clang.

The fix improves handling of Windows UNC paths to align with Appendix E. 
Nonstandard Syntax Variations of RFC 8089. Before this fix it was difficult to 
use Windows UNC paths in compile_commands.json as such paths were converted to 
file URIs using 'file:////authority/share/file.cpp' notation instead of 
recommended 'file://authority/share/file.cpp'. As an example, VS.Code cannot 
understand such file URIsm thus such features as go-to-definition, 
jump-to-file, hover tooltip, etc. stop working.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84172

Files:
  clang-tools-extra/clangd/URI.cpp
  clang-tools-extra/clangd/unittests/URITests.cpp

Index: clang-tools-extra/clangd/unittests/URITests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/URITests.cpp
+++ clang-tools-extra/clangd/unittests/URITests.cpp
@@ -76,6 +76,16 @@
 #endif
 }
 
+TEST(URITest, CreateUNC) {
+#ifdef _WIN32
+  EXPECT_THAT(createOrDie("\\\\test.org\\x\\y\\z"), "file://test.org/x/y/z");
+  EXPECT_THAT(createOrDie("\\\\10.0.0.1\\x\\y\\z"), "file://10.0.0.1/x/y/z");
+#else
+  EXPECT_THAT(createOrDie("//test.org/x/y/z"), "file://test.org/x/y/z");
+  EXPECT_THAT(createOrDie("//10.0.0.1/x/y/z"), "file://10.0.0.1/x/y/z");
+#endif
+}
+
 TEST(URITest, FailedCreate) {
   EXPECT_ERROR(URI::create("/x/y/z", "no"));
   // Path has to be absolute.
@@ -127,15 +137,29 @@
   EXPECT_THAT(resolveOrDie(parseOrDie("file:///c:/x/y/z")), "c:\\x\\y\\z");
 #else
   EXPECT_EQ(resolveOrDie(parseOrDie("file:/a/b/c")), "/a/b/c");
-  EXPECT_EQ(resolveOrDie(parseOrDie("file://auth/a/b/c")), "/a/b/c");
+  EXPECT_EQ(resolveOrDie(parseOrDie("file://auth/a/b/c")), "//auth/a/b/c");
   EXPECT_THAT(resolveOrDie(parseOrDie("file://au%3dth/%28x%29/y/%20z")),
-              "/(x)/y/ z");
+              "//au=th/(x)/y/ z");
   EXPECT_THAT(resolveOrDie(parseOrDie("file:///c:/x/y/z")), "c:/x/y/z");
 #endif
   EXPECT_EQ(resolveOrDie(parseOrDie("unittest:///a"), testPath("x")),
             testPath("a"));
 }
 
+TEST(URITest, ResolveUNC) {
+#ifdef _WIN32
+  EXPECT_THAT(resolveOrDie(parseOrDie("file://example.com/x/y/z")),
+              "\\\\example.com\\x\\y\\z");
+  EXPECT_THAT(resolveOrDie(parseOrDie("file://127.0.0.1/x/y/z")),
+              "\\\\127.0.0.1\\x\\y\\z");
+#else
+  EXPECT_THAT(resolveOrDie(parseOrDie("file://example.com/x/y/z")),
+              "//example.com/x/y/z");
+  EXPECT_THAT(resolveOrDie(parseOrDie("file://127.0.0.1/x/y/z")),
+              "//127.0.0.1/x/y/z");
+#endif
+}
+
 std::string resolvePathOrDie(llvm::StringRef AbsPath,
                              llvm::StringRef HintPath = "") {
   auto Path = URI::resolvePath(AbsPath, HintPath);
Index: clang-tools-extra/clangd/URI.cpp
===================================================================
--- clang-tools-extra/clangd/URI.cpp
+++ clang-tools-extra/clangd/URI.cpp
@@ -26,6 +26,15 @@
                                              llvm::inconvertibleErrorCode());
 }
 
+bool isWindowsPath(llvm::StringRef Path) {
+  return Path.size() > 1 && llvm::isAlpha(Path[0]) && Path[1] == ':';
+}
+
+bool isNetworkPath(llvm::StringRef Path) {
+  llvm::StringRef Sep = llvm::sys::path::get_separator();
+  return Path.consume_front(Sep) && Path.consume_front(Sep) && !Path.empty();
+}
+
 /// This manages file paths in the file system. All paths in the scheme
 /// are absolute (with leading '/').
 /// Note that this scheme is hardcoded into the library and not registered in
@@ -33,28 +42,39 @@
 class FileSystemScheme : public URIScheme {
 public:
   llvm::Expected<std::string>
-  getAbsolutePath(llvm::StringRef /*Authority*/, llvm::StringRef Body,
+  getAbsolutePath(llvm::StringRef Authority, llvm::StringRef Body,
                   llvm::StringRef /*HintPath*/) const override {
     if (!Body.startswith("/"))
       return make_string_error("File scheme: expect body to be an absolute "
                                "path starting with '/': " +
                                Body);
+    llvm::SmallString<128> Path;
+    // For Windows UNC paths e.g. \\server\share
+    if (!Authority.empty())
+      ("//" + Authority).toVector(Path);
     // For Windows paths e.g. /X:
-    if (Body.size() > 2 && Body[0] == '/' && Body[2] == ':')
+    if (isWindowsPath(Body.substr(1)))
       Body.consume_front("/");
-    llvm::SmallVector<char, 16> Path(Body.begin(), Body.end());
+    Path.append(Body);
     llvm::sys::path::native(Path);
-    return std::string(Path.begin(), Path.end());
+    return std::string(Path);
   }
 
   llvm::Expected<URI>
   uriFromAbsolutePath(llvm::StringRef AbsolutePath) const override {
     std::string Body;
+    llvm::StringRef Authority;
+    llvm::StringRef Root = llvm::sys::path::root_name(AbsolutePath);
+    // For Windows UNC paths e.g. \\server\share
+    if (isNetworkPath(Root)) {
+      Authority = Root.drop_front(2);
+      AbsolutePath.consume_front(Root);
+    }
     // For Windows paths e.g. X:
-    if (AbsolutePath.size() > 1 && AbsolutePath[1] == ':')
+    if (isWindowsPath(Root))
       Body = "/";
     Body += llvm::sys::path::convert_to_slash(AbsolutePath);
-    return URI("file", /*Authority=*/"", Body);
+    return URI("file", Authority, Body);
   }
 };
 
@@ -96,13 +116,13 @@
 void percentEncode(llvm::StringRef Content, std::string &Out) {
   std::string Result;
   for (unsigned char C : Content)
-    if (shouldEscape(C))
-    {
+    if (shouldEscape(C)) {
       Out.push_back('%');
       Out.push_back(llvm::hexdigit(C / 16));
       Out.push_back(llvm::hexdigit(C % 16));
-    } else
-    { Out.push_back(C); }
+    } else {
+      Out.push_back(C);
+    }
 }
 
 /// Decodes a string according to percent-encoding.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to