Index: clang-tools-extra/clangd/PathMapping.h =================================================================== --- clang-tools-extra/clangd/PathMapping.h +++ clang-tools-extra/clangd/PathMapping.h @@ -11,7 +11,7 @@ #include "llvm/Support/JSON.h" #include "llvm/Support/raw_ostream.h" #include -#include +#include #include namespace clang { @@ -22,43 +22,43 @@ /// PathMappings are a collection of paired client and server paths. /// These pairs are used to alter file:// URIs appearing in inbound and outbound /// LSP messages, as the client's environment may have source files or -/// dependencies at different locations than the server. +/// dependencies at different locations than the server. Therefore, both +/// paths are stored as they appear in file URI bodies, e.g. /usr/include or +/// /C:/config /// /// For example, if the mappings were {{"/home/user", "/workarea"}}, then -/// an inbound LSP message would have file:///home/user/foo.cpp remapped to -/// file:///workarea/foo.cpp, and the same would happen for replies (in the -/// opposite order). +/// a client-to-server LSP message would have file:///home/user/foo.cpp +/// remapped to file:///workarea/foo.cpp, and the same would happen for replies +/// (in the opposite order). struct PathMapping { - PathMapping() {} - PathMapping(llvm::StringRef ClientPath, llvm::StringRef ServerPath) - : ClientPath(ClientPath), ServerPath(ServerPath) {} std::string ClientPath; std::string ServerPath; + enum class Direction { ClientToServer, ServerToClient }; }; using PathMappings = std::vector; llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const PathMapping &M); -/// Parse the command line \pRawPathMappings (e.g. "/client=/server") into +/// Parse the command line \p RawPathMappings (e.g. "/client=/server") into /// pairs. Returns an error if the mappings are malformed, i.e. not absolute or /// not a proper pair. llvm::Expected parsePathMappings(llvm::StringRef RawPathMappings); -/// Returns a modified \pS with the first matching path in \pPathMappings +/// Returns a modified \p S with the first matching path in \p Mappings /// substituted, if applicable -llvm::Optional doPathMapping(llvm::StringRef S, bool IsIncoming, +llvm::Optional doPathMapping(llvm::StringRef S, + PathMapping::Direction Dir, const PathMappings &Mappings); -/// Applies the \pMappings to all the file:// URIs in \pParams. -/// \pIsIncoming affects which direction the mappings are applied. -/// NOTE: The first matching mapping will be applied, otherwise \pParams will be -/// untouched. -void applyPathMappings(llvm::json::Value &Params, bool IsIncoming, +/// Applies the \p Mappings to all the file:// URIs in \p Params. +/// NOTE: The first matching mapping will be applied, otherwise \p Params will +/// be untouched. +void applyPathMappings(llvm::json::Value &Params, PathMapping::Direction Dir, const PathMappings &Mappings); -/// Creates a wrapping transport over \pTransp that applies the \pMappings to +/// Creates a wrapping transport over \p Transp that applies the \p Mappings to /// all inbound and outbound LSP messages. All calls are then delegated to the -/// regular \pTransp (e.g. XPC, JSON). +/// regular transport (e.g. XPC, JSON). std::unique_ptr createPathMappingTransport(std::unique_ptr Transp, PathMappings Mappings); Index: clang-tools-extra/clangd/PathMapping.cpp =================================================================== --- clang-tools-extra/clangd/PathMapping.cpp +++ clang-tools-extra/clangd/PathMapping.cpp @@ -6,41 +6,33 @@ // //===----------------------------------------------------------------------===// #include "PathMapping.h" -#include "Logger.h" #include "Transport.h" #include "URI.h" #include "llvm/ADT/None.h" -#include "llvm/ADT/Optional.h" #include "llvm/ADT/STLExtras.h" -#include "llvm/ADT/SmallString.h" -#include "llvm/ADT/StringRef.h" #include "llvm/Support/Errno.h" -#include "llvm/Support/JSON.h" #include "llvm/Support/Path.h" -#include "llvm/Support/raw_ostream.h" #include -#include #include -#include namespace clang { namespace clangd { namespace { using MapperFunc = llvm::function_ref( - llvm::StringRef, bool, const PathMappings &)>; + llvm::StringRef, PathMapping::Direction, const PathMappings &)>; -// Recursively apply the \pMF to all string keys/values in \pV -void recursivelyMap(llvm::json::Value &V, bool IsIncoming, +// Recursively apply the MF to all string keys/values in V +void recursivelyMap(llvm::json::Value &V, PathMapping::Direction Dir, const PathMappings &Mappings, const MapperFunc &MF) { using Kind = llvm::json::Value::Kind; - const Kind &K = V.kind(); + Kind K = V.kind(); if (K == Kind::Object) { llvm::json::Object *Obj = V.getAsObject(); llvm::json::Object MappedObj; // 1. Map all the Keys for (auto &KV : *Obj) { if (llvm::Optional MappedKey = - MF(KV.first.str(), IsIncoming, Mappings)) { + MF(KV.first.str(), Dir, Mappings)) { MappedObj.try_emplace(std::move(*MappedKey), std::move(KV.second)); } else { MappedObj.try_emplace(std::move(KV.first), std::move(KV.second)); @@ -49,15 +41,15 @@ *Obj = std::move(MappedObj); // 2. Map all the values for (auto &KV : *Obj) { - recursivelyMap(KV.second, IsIncoming, Mappings, MF); + recursivelyMap(KV.second, Dir, Mappings, MF); } } else if (K == Kind::Array) { for (llvm::json::Value &Val : *V.getAsArray()) { - recursivelyMap(Val, IsIncoming, Mappings, MF); + recursivelyMap(Val, Dir, Mappings, MF); } } else if (K == Kind::String) { if (llvm::Optional Mapped = - MF(*V.getAsString(), IsIncoming, Mappings)) { + MF(*V.getAsString(), Dir, Mappings)) { V = std::move(*Mapped); } } @@ -70,20 +62,21 @@ : WrappedHandler(Handler), Mappings(Mappings) {} bool onNotify(llvm::StringRef Method, llvm::json::Value Params) override { - applyPathMappings(Params, /*IsIncoming=*/true, Mappings); + applyPathMappings(Params, PathMapping::Direction::ClientToServer, Mappings); return WrappedHandler.onNotify(Method, std::move(Params)); } bool onCall(llvm::StringRef Method, llvm::json::Value Params, llvm::json::Value ID) override { - applyPathMappings(Params, /*IsIncoming=*/true, Mappings); + applyPathMappings(Params, PathMapping::Direction::ClientToServer, Mappings); return WrappedHandler.onCall(Method, std::move(Params), std::move(ID)); } bool onReply(llvm::json::Value ID, llvm::Expected Result) override { if (Result) { - applyPathMappings(*Result, /*IsIncoming=*/true, Mappings); + applyPathMappings(*Result, PathMapping::Direction::ClientToServer, + Mappings); } return WrappedHandler.onReply(std::move(ID), std::move(Result)); } @@ -101,20 +94,21 @@ : WrappedTransport(std::move(Transp)), Mappings(std::move(Mappings)) {} void notify(llvm::StringRef Method, llvm::json::Value Params) override { - applyPathMappings(Params, /*IsIncoming=*/false, Mappings); + applyPathMappings(Params, PathMapping::Direction::ServerToClient, Mappings); WrappedTransport->notify(Method, std::move(Params)); } void call(llvm::StringRef Method, llvm::json::Value Params, llvm::json::Value ID) override { - applyPathMappings(Params, /*IsIncoming=*/false, Mappings); + applyPathMappings(Params, PathMapping::Direction::ServerToClient, Mappings); WrappedTransport->call(Method, std::move(Params), std::move(ID)); } void reply(llvm::json::Value ID, llvm::Expected Result) override { if (Result) { - applyPathMappings(*Result, /*IsIncoming=*/false, Mappings); + applyPathMappings(*Result, PathMapping::Direction::ServerToClient, + Mappings); } WrappedTransport->reply(std::move(ID), std::move(Result)); } @@ -134,8 +128,8 @@ llvm::inconvertibleErrorCode()); } -// Returns whether a path mapping should take place for \pOrigPath -// i.e. \pMappingPath is a proper sub-path of \p OrigPath +// Returns whether a path mapping should take place for OrigPath +// i.e. MappingPath is a proper sub-path of OrigPath bool mappingMatches(llvm::StringRef OrigPath, llvm::StringRef MappingPath) { namespace path = llvm::sys::path; auto OrigPathItr = path::begin(OrigPath, path::Style::posix); @@ -149,11 +143,10 @@ return std::equal(MappingPathItr, MappingPathEnd, OrigPathItr); } -// Converts \pPath to a posix-style absolute, i.e. if it's a windows path -// then the backward-slash notation will be converted to forward slash +// Converts a unix/windows path to the path portion of a file URI +// e.g. "C:\foo" -> "/C:/foo" llvm::Expected parsePath(llvm::StringRef Path) { namespace path = llvm::sys::path; - std::string ParsedPath; if (path::is_absolute(Path, path::Style::posix)) { return Path; } else if (path::is_absolute(Path, path::Style::windows)) { @@ -190,38 +183,43 @@ if (!ParsedServerPath) { return ParsedServerPath.takeError(); } - ParsedMappings.emplace_back(std::move(*ParsedClientPath), - std::move(*ParsedServerPath)); + ParsedMappings.push_back( + {std::move(*ParsedClientPath), std::move(*ParsedServerPath)}); } return ParsedMappings; } -llvm::Optional doPathMapping(llvm::StringRef S, bool IsIncoming, +llvm::Optional doPathMapping(llvm::StringRef S, + PathMapping::Direction Dir, const PathMappings &Mappings) { + // Retrun early to optimize for the common case, wherein S is not a file URI if (!S.startswith("file://")) return llvm::None; auto Uri = URI::parse(S); if (!Uri) { + llvm::consumeError(Uri.takeError()); return llvm::None; } for (const auto &Mapping : Mappings) { - const std::string &From = - IsIncoming ? Mapping.ClientPath : Mapping.ServerPath; - const std::string &To = - IsIncoming ? Mapping.ServerPath : Mapping.ClientPath; + const std::string &From = Dir == PathMapping::Direction::ClientToServer + ? Mapping.ClientPath + : Mapping.ServerPath; + const std::string &To = Dir == PathMapping::Direction::ClientToServer + ? Mapping.ServerPath + : Mapping.ClientPath; if (mappingMatches(Uri->body(), From)) { - llvm::SmallString<256> MappedPath(Uri->body()); - llvm::sys::path::replace_path_prefix(MappedPath, From, To); - auto MappedUri = URI(Uri->scheme(), Uri->authority(), MappedPath.c_str()); + std::string MappedBody = std::move(Uri->body()); + MappedBody.replace(MappedBody.find(From), From.length(), To); + auto MappedUri = URI(Uri->scheme(), Uri->authority(), MappedBody.c_str()); return MappedUri.toString(); } } return llvm::None; } -void applyPathMappings(llvm::json::Value &Params, bool IsIncoming, +void applyPathMappings(llvm::json::Value &Params, PathMapping::Direction Dir, const PathMappings &Mappings) { - recursivelyMap(Params, IsIncoming, Mappings, doPathMapping); + recursivelyMap(Params, Dir, Mappings, doPathMapping); } std::unique_ptr Index: clang-tools-extra/clangd/tool/ClangdMain.cpp =================================================================== --- clang-tools-extra/clangd/tool/ClangdMain.cpp +++ clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -346,7 +346,7 @@ "location where clangd is running," "e.g. " "/home/project=/workarea/project,/home/project/.includes=/opt/include" - "opt/include"), + ), llvm::cl::init("")); opt InputMirrorFile{ Index: clang-tools-extra/clangd/unittests/PathMappingTests.cpp =================================================================== --- clang-tools-extra/clangd/unittests/PathMappingTests.cpp +++ clang-tools-extra/clangd/unittests/PathMappingTests.cpp @@ -82,12 +82,11 @@ } bool mapsProperly(llvm::StringRef Orig, llvm::StringRef Expected, - llvm::StringRef RawMappings, bool IsIncoming) { + llvm::StringRef RawMappings, PathMapping::Direction Dir) { llvm::Expected Mappings = parsePathMappings(RawMappings); if (!Mappings) return false; - llvm::Optional MappedPath = - doPathMapping(Orig, IsIncoming, *Mappings); + llvm::Optional MappedPath = doPathMapping(Orig, Dir, *Mappings); std::string Actual = MappedPath ? *MappedPath : Orig.str(); EXPECT_STREQ(Expected.str().c_str(), Actual.c_str()); return Expected == Actual; @@ -95,48 +94,54 @@ TEST(DoPathMappingTests, PreservesOriginal) { // Preserves original path when no mapping - EXPECT_TRUE(mapsProperly("file:///home", "file:///home", "", true)); + EXPECT_TRUE(mapsProperly("file:///home", "file:///home", "", + PathMapping::Direction::ClientToServer)); } TEST(DoPathMappingTests, UsesFirstMatch) { EXPECT_TRUE(mapsProperly("file:///home/foo.cpp", "file:///workarea1/foo.cpp", - "/home=/workarea1,/home=/workarea2", true)); + "/home=/workarea1,/home=/workarea2", + PathMapping::Direction::ClientToServer)); } TEST(DoPathMappingTests, IgnoresSubstrings) { // Doesn't map substrings that aren't a proper path prefix EXPECT_TRUE(mapsProperly("file://home/foo-bar.cpp", "file://home/foo-bar.cpp", - "/home/foo=/home/bar", true)); + "/home/foo=/home/bar", + PathMapping::Direction::ClientToServer)); } TEST(DoPathMappingTests, MapsOutgoingPaths) { // When IsIncoming is false (i.e.a response), map the other way EXPECT_TRUE(mapsProperly("file:///workarea/foo.cpp", "file:///home/foo.cpp", - "/home=/workarea", false)); + "/home=/workarea", + PathMapping::Direction::ServerToClient)); } TEST(DoPathMappingTests, OnlyMapFileUris) { EXPECT_TRUE(mapsProperly("test:///home/foo.cpp", "test:///home/foo.cpp", - "/home=/workarea", true)); + "/home=/workarea", + PathMapping::Direction::ClientToServer)); } TEST(DoPathMappingTests, RespectsCaseSensitivity) { EXPECT_TRUE(mapsProperly("file:///HOME/foo.cpp", "file:///HOME/foo.cpp", - "/home=/workarea", true)); + "/home=/workarea", + PathMapping::Direction::ClientToServer)); } TEST(DoPathMappingTests, MapsWindowsPaths) { // Maps windows properly EXPECT_TRUE(mapsProperly("file:///C:/home/foo.cpp", - "file:///C:/workarea/foo.cpp", "C:/home=C:/workarea", - true)); + "file:///C:/workarea/foo.cpp", R"(C:\home=C:\workarea)", + PathMapping::Direction::ClientToServer)); } TEST(DoPathMappingTests, MapsWindowsUnixInterop) { // Path mappings with a windows-style client path and unix-style server path - EXPECT_TRUE(mapsProperly("file:///C:/home/foo.cpp", - "file:///C:/workarea/foo.cpp", - "C:/home=/C:/workarea", true)); + EXPECT_TRUE(mapsProperly( + "file:///C:/home/foo.cpp", "file:///workarea/foo.cpp", + R"(C:\home=/workarea)", PathMapping::Direction::ClientToServer)); } TEST(ApplyPathMappingTests, PreservesOriginalParams) { @@ -147,7 +152,7 @@ ASSERT_TRUE(bool(Params)); llvm::json::Value ExpectedParams = *Params; PathMappings Mappings; - applyPathMappings(*Params, /*IsIncoming=*/true, Mappings); + applyPathMappings(*Params, PathMapping::Direction::ClientToServer, Mappings); EXPECT_EQ(*Params, ExpectedParams); } @@ -163,7 +168,7 @@ })"); auto Mappings = parsePathMappings("/home=/workarea"); ASSERT_TRUE(bool(Params) && bool(ExpectedParams) && bool(Mappings)); - applyPathMappings(*Params, /*IsIncoming=*/true, *Mappings); + applyPathMappings(*Params, PathMapping::Direction::ClientToServer, *Mappings); EXPECT_EQ(*Params, *ExpectedParams); } @@ -183,7 +188,7 @@ auto Mappings = parsePathMappings("/home=/workarea,/home/.includes=/opt/include"); ASSERT_TRUE(bool(Params) && bool(ExpectedParams) && bool(Mappings)); - applyPathMappings(*Params, /*IsIncoming=*/false, *Mappings); + applyPathMappings(*Params, PathMapping::Direction::ServerToClient, *Mappings); EXPECT_EQ(*Params, *ExpectedParams); } @@ -202,7 +207,7 @@ })"); auto Mappings = parsePathMappings("/home=/workarea"); ASSERT_TRUE(bool(Params) && bool(ExpectedParams) && bool(Mappings)); - applyPathMappings(*Params, /*IsIncoming=*/true, *Mappings); + applyPathMappings(*Params, PathMapping::Direction::ClientToServer, *Mappings); EXPECT_EQ(*Params, *ExpectedParams); }