[clang-tools-extra] 3975c3b - [clangd] Fix conversion from Windows UNC paths to file URI format.
Kirill Bobyrev via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 22 03:13:42 PDT 2020
Author: Ilya Golovenko
Date: 2020-07-22T12:13:09+02:00
New Revision: 3975c3be80412bb6b1376bcef53ebce53984ddd7
URL: https://github.com/llvm/llvm-project/commit/3975c3be80412bb6b1376bcef53ebce53984ddd7
DIFF: https://github.com/llvm/llvm-project/commit/3975c3be80412bb6b1376bcef53ebce53984ddd7.diff
LOG: [clangd] Fix conversion from Windows UNC paths to file URI format.
Summary:
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 database as such paths were converted to file URIs using 'file:////auth/share/file.cpp' notation instead of recommended 'file://auth/share/file.cpp'.
As an example, VS.Code cannot understand file URIs with 4 starting slashes, thus such features as go-to-definition, jump-to-file, hover tooltip, etc. stop working. This also applicable to files which reside on Windows network-mapped drives because clangd internally resolves file paths to real paths in some cases and such paths get resolved to UNC paths.
Reviewers: sammccall, kadircet
Reviewed By: sammccall
Subscribers: ormris, ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, kbobyrev, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D84172
Added:
Modified:
clang-tools-extra/clangd/URI.cpp
clang-tools-extra/clangd/unittests/URITests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/URI.cpp b/clang-tools-extra/clangd/URI.cpp
index 2061a5601c58..fad93143a30d 100644
--- a/clang-tools-extra/clangd/URI.cpp
+++ b/clang-tools-extra/clangd/URI.cpp
@@ -26,6 +26,15 @@ inline llvm::Error make_string_error(const llvm::Twine &Message) {
llvm::inconvertibleErrorCode());
}
+bool isWindowsPath(llvm::StringRef Path) {
+ return Path.size() > 1 && llvm::isAlpha(Path[0]) && Path[1] == ':';
+}
+
+bool isNetworkPath(llvm::StringRef Path) {
+ return Path.size() > 2 && Path[0] == Path[1] &&
+ llvm::sys::path::is_separator(Path[0]);
+}
+
/// 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,40 @@ inline llvm::Error make_string_error(const llvm::Twine &Message) {
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);
- // For Windows paths e.g. /X:
- if (Body.size() > 2 && Body[0] == '/' && Body[2] == ':')
+ llvm::SmallString<128> Path;
+ if (!Authority.empty()) {
+ // Windows UNC paths e.g. file://server/share => \\server\share
+ ("//" + Authority).toVector(Path);
+ } else if (isWindowsPath(Body.substr(1))) {
+ // Windows paths e.g. file:///X:/path => X:\path
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;
- // For Windows paths e.g. X:
- if (AbsolutePath.size() > 1 && AbsolutePath[1] == ':')
+ llvm::StringRef Authority;
+ llvm::StringRef Root = llvm::sys::path::root_name(AbsolutePath);
+ if (isNetworkPath(Root)) {
+ // Windows UNC paths e.g. \\server\share => file://server/share
+ Authority = Root.drop_front(2);
+ AbsolutePath.consume_front(Root);
+ } else if (isWindowsPath(Root)) {
+ // Windows paths e.g. X:\path => file:///X:/path
Body = "/";
+ }
Body += llvm::sys::path::convert_to_slash(AbsolutePath);
- return URI("file", /*Authority=*/"", Body);
+ return URI("file", Authority, Body);
}
};
@@ -96,13 +117,13 @@ bool shouldEscape(unsigned char C) {
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.
diff --git a/clang-tools-extra/clangd/unittests/URITests.cpp b/clang-tools-extra/clangd/unittests/URITests.cpp
index 52ca7b4447cd..cd734cfe0837 100644
--- a/clang-tools-extra/clangd/unittests/URITests.cpp
+++ b/clang-tools-extra/clangd/unittests/URITests.cpp
@@ -76,6 +76,16 @@ TEST(URITest, Create) {
#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,32 @@ TEST(URITest, Resolve) {
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");
+ // Ensure non-traditional file URI still resolves to correct UNC path.
+ 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);
More information about the cfe-commits
mailing list