Author: ioeric Date: Tue Jun 19 02:33:53 2018 New Revision: 335035 URL: http://llvm.org/viewvc/llvm-project?rev=335035&view=rev Log: [clangd] Use workspace root path as hint path for resolving URIs in workspace/symbol
Summary: Some URI schemes require a hint path to be provided, and workspace root path seems to be a good fit. Reviewers: sammccall, malaperle Reviewed By: sammccall Subscribers: ilya-biryukov, MaskRay, jkorous, cfe-commits Differential Revision: https://reviews.llvm.org/D48290 Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp clang-tools-extra/trunk/clangd/FindSymbols.cpp clang-tools-extra/trunk/clangd/FindSymbols.h clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp clang-tools-extra/trunk/unittests/clangd/TestFS.cpp clang-tools-extra/trunk/unittests/clangd/URITests.cpp Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=335035&r1=335034&r2=335035&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Jun 19 02:33:53 2018 @@ -115,10 +115,15 @@ ClangdServer::ClangdServer(GlobalCompila } void ClangdServer::setRootPath(PathRef RootPath) { - std::string NewRootPath = llvm::sys::path::convert_to_slash( - RootPath, llvm::sys::path::Style::posix); - if (llvm::sys::fs::is_directory(NewRootPath)) - this->RootPath = NewRootPath; + auto FS = FSProvider.getFileSystem(); + auto Status = FS->status(RootPath); + if (!Status) + log("Failed to get status for RootPath " + RootPath + ": " + + Status.getError().message()); + else if (Status->isDirectory()) + this->RootPath = RootPath; + else + log("The provided RootPath " + RootPath + " is not a directory."); } void ClangdServer::addDocument(PathRef File, StringRef Contents, @@ -446,7 +451,8 @@ void ClangdServer::onFileEvent(const Did void ClangdServer::workspaceSymbols( StringRef Query, int Limit, Callback<std::vector<SymbolInformation>> CB) { - CB(clangd::getWorkspaceSymbols(Query, Limit, Index)); + CB(clangd::getWorkspaceSymbols(Query, Limit, Index, + RootPath ? *RootPath : "")); } std::vector<std::pair<Path, std::size_t>> Modified: clang-tools-extra/trunk/clangd/FindSymbols.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/FindSymbols.cpp?rev=335035&r1=335034&r2=335035&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/FindSymbols.cpp (original) +++ clang-tools-extra/trunk/clangd/FindSymbols.cpp Tue Jun 19 02:33:53 2018 @@ -95,8 +95,8 @@ struct ScoredSymbolGreater { } // namespace llvm::Expected<std::vector<SymbolInformation>> -getWorkspaceSymbols(StringRef Query, int Limit, - const SymbolIndex *const Index) { +getWorkspaceSymbols(StringRef Query, int Limit, const SymbolIndex *const Index, + StringRef HintPath) { std::vector<SymbolInformation> Result; if (Query.empty() || !Index) return Result; @@ -116,7 +116,7 @@ getWorkspaceSymbols(StringRef Query, int Req.MaxCandidateCount = Limit; TopN<ScoredSymbolInfo, ScoredSymbolGreater> Top(Req.MaxCandidateCount); FuzzyMatcher Filter(Req.Query); - Index->fuzzyFind(Req, [&Top, &Filter](const Symbol &Sym) { + Index->fuzzyFind(Req, [HintPath, &Top, &Filter](const Symbol &Sym) { // Prefer the definition over e.g. a function declaration in a header auto &CD = Sym.Definition ? Sym.Definition : Sym.CanonicalDeclaration; auto Uri = URI::parse(CD.FileURI); @@ -126,9 +126,7 @@ getWorkspaceSymbols(StringRef Query, int CD.FileURI, Sym.Name)); return; } - // FIXME: Passing no HintPath here will work for "file" and "test" schemes - // because they don't use it but this might not work for other custom ones. - auto Path = URI::resolve(*Uri); + auto Path = URI::resolve(*Uri, HintPath); if (!Path) { log(llvm::formatv("Workspace symbol: Could not resolve path for URI " "'{0}' for symbol '{1}'.", Modified: clang-tools-extra/trunk/clangd/FindSymbols.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/FindSymbols.h?rev=335035&r1=335034&r2=335035&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/FindSymbols.h (original) +++ clang-tools-extra/trunk/clangd/FindSymbols.h Tue Jun 19 02:33:53 2018 @@ -27,9 +27,11 @@ class SymbolIndex; /// "::". For example, "std::" will list all children of the std namespace and /// "::" alone will list all children of the global namespace. /// \p Limit limits the number of results returned (0 means no limit). +/// \p HintPath This is used when resolving URIs. If empty, URI resolution can +/// fail if a hint path is required for the scheme of a specific URI. llvm::Expected<std::vector<SymbolInformation>> getWorkspaceSymbols(llvm::StringRef Query, int Limit, - const SymbolIndex *const Index); + const SymbolIndex *const Index, llvm::StringRef HintPath); } // namespace clangd } // namespace clang Modified: clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp?rev=335035&r1=335034&r2=335035&view=diff ============================================================================== --- clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp (original) +++ clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp Tue Jun 19 02:33:53 2018 @@ -40,13 +40,18 @@ MATCHER_P(WithKind, Kind, "") { return a ClangdServer::Options optsForTests() { auto ServerOpts = ClangdServer::optsForTest(); ServerOpts.BuildDynamicSymbolIndex = true; + ServerOpts.URISchemes = {"unittest", "file"}; return ServerOpts; } class WorkspaceSymbolsTest : public ::testing::Test { public: WorkspaceSymbolsTest() - : Server(CDB, FSProvider, DiagConsumer, optsForTests()) {} + : Server(CDB, FSProvider, DiagConsumer, optsForTests()) { + // Make sure the test root directory is created. + FSProvider.Files[testPath("unused")] = ""; + Server.setRootPath(testRoot()); + } protected: MockFSProvider FSProvider; Modified: clang-tools-extra/trunk/unittests/clangd/TestFS.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TestFS.cpp?rev=335035&r1=335034&r2=335035&view=diff ============================================================================== --- clang-tools-extra/trunk/unittests/clangd/TestFS.cpp (original) +++ clang-tools-extra/trunk/unittests/clangd/TestFS.cpp Tue Jun 19 02:33:53 2018 @@ -66,7 +66,9 @@ std::string testPath(PathRef File) { return Path.str(); } -/// unittest: is a scheme that refers to files relative to testRoot() +/// unittest: is a scheme that refers to files relative to testRoot(). +/// URI body is a path relative to testRoot() e.g. unittest:///x.h for +/// /clangd-test/x.h. class TestScheme : public URIScheme { public: static const char *Scheme; @@ -75,6 +77,10 @@ public: getAbsolutePath(llvm::StringRef /*Authority*/, llvm::StringRef Body, llvm::StringRef HintPath) const override { assert(HintPath.startswith(testRoot())); + if (!Body.consume_front("/")) + return llvm::make_error<llvm::StringError>( + "Body of an unittest: URI must start with '/'", + llvm::inconvertibleErrorCode()); llvm::SmallString<16> Path(Body.begin(), Body.end()); llvm::sys::path::native(Path); return testPath(Path); Modified: clang-tools-extra/trunk/unittests/clangd/URITests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/URITests.cpp?rev=335035&r1=335034&r2=335035&view=diff ============================================================================== --- clang-tools-extra/trunk/unittests/clangd/URITests.cpp (original) +++ clang-tools-extra/trunk/unittests/clangd/URITests.cpp Tue Jun 19 02:33:53 2018 @@ -144,7 +144,7 @@ TEST(URITest, Resolve) { "/(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")), + EXPECT_EQ(resolveOrDie(parseOrDie("unittest:///a"), testPath("x")), testPath("a")); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
