[clang-tools-extra] r335035 - [clangd] Use workspace root path as hint path for resolving URIs in workspace/symbol
Eric Liu via cfe-commits
cfe-commits at lists.llvm.org
Tue Jun 19 02:33:54 PDT 2018
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"));
}
More information about the cfe-commits
mailing list