[PATCH] D54845: [clangd] Canonicalize file path in URIForFile.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Thanks!
Mostly just places that should be updated HintPath -> TUPath. Changing the whole codebase doesn't seem important, but in places that are touched...



================
Comment at: clangd/Protocol.cpp:43
+
+Expected<URIForFile> URIForFile::fromURI(const URI &U, StringRef HintPath) {
+  auto Resolved = URI::resolve(U, HintPath);
----------------
TUPath


================
Comment at: clangd/Protocol.cpp:62
     }
-    auto Path = URI::resolve(*U);
-    if (!Path) {
-      log("{0}", Path.takeError());
+    // "file" and "test" schemes do not require hint path.
+    auto U = URIForFile::fromURI(*Parsed, /*HintPath=*/"");
----------------
maybe more explicit: input should be "file" or "test" scheme, which do not...


================
Comment at: clangd/Protocol.h:87
+  static llvm::Expected<URIForFile> fromURI(const URI &U,
+                                            llvm::StringRef HintPath);
 
----------------
TUPath


================
Comment at: clangd/URI.cpp:221
 
 Expected<std::string> URI::resolve(const URI &Uri, StringRef HintPath) {
   auto S = findSchemeByName(Uri.Scheme);
----------------
TUPath


================
Comment at: clangd/URI.cpp:228
 
+Expected<std::string> URI::resolvePath(StringRef AbsPath, StringRef HintPath) {
+  if (!sys::path::is_absolute(AbsPath))
----------------
TUPath


================
Comment at: clangd/URI.h:65
   static llvm::Expected<std::string> resolve(const URI &U,
                                              llvm::StringRef HintPath = "");
 
----------------
TUPath (and in comment)


================
Comment at: clangd/URI.h:72
+  static llvm::Expected<std::string> resolvePath(llvm::StringRef AbsPath,
+                                                 llvm::StringRef HintPath = "");
+
----------------
TUPath


================
Comment at: unittests/clangd/TestFS.cpp:95
   Expected<std::string> getAbsolutePath(StringRef /*Authority*/, StringRef Body,
                                         StringRef HintPath) const override {
+    if (!HintPath.startswith(testRoot()))
----------------
TUPath


================
Comment at: unittests/clangd/URITests.cpp:140
 
+std::string resolvePathOrDie(StringRef AbsPath, StringRef HintPath = "") {
+  auto Path = URI::resolvePath(AbsPath, HintPath);
----------------
TUPath


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54845/new/

https://reviews.llvm.org/D54845