ioeric updated this revision to Diff 132958.
ioeric marked 2 inline comments as done.
ioeric added a comment.
- Make URIScheme customizable in SymbolCollector.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42915
Files:
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/Merge.cpp
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
clangd/index/SymbolYAML.cpp
unittests/clangd/IndexTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===================================================================
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -46,7 +46,7 @@
return arg.CompletionSnippetInsertText == S;
}
MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }
-MATCHER_P(CPath, P, "") { return arg.CanonicalDeclaration.FilePath == P; }
+MATCHER_P(Uri, P, "") { return arg.CanonicalDeclaration.FileUri == P; }
MATCHER_P(LocationOffsets, Offsets, "") {
// Offset range in SymbolLocation is [start, end] while in Clangd is [start,
// end).
@@ -58,8 +58,6 @@
namespace clangd {
namespace {
-const char TestHeaderName[] = "symbols.h";
-const char TestFileName[] = "symbol.cc";
class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
public:
SymbolIndexActionFactory(SymbolCollector::Options COpts)
@@ -82,6 +80,13 @@
class SymbolCollectorTest : public ::testing::Test {
public:
+ SymbolCollectorTest()
+ : TestHeaderName(getVirtualTestFilePath("symbol.h").str()),
+ TestFileName(getVirtualTestFilePath("symbol.cc").str()) {
+ TestHeaderUri = URI::createFile(TestHeaderName).toString();
+ TestFileUri = URI::createFile(TestFileName).toString();
+ }
+
bool runSymbolCollector(StringRef HeaderCode, StringRef MainCode) {
llvm::IntrusiveRefCntPtr<vfs::InMemoryFileSystem> InMemoryFileSystem(
new vfs::InMemoryFileSystem);
@@ -100,15 +105,21 @@
std::string Content = MainCode;
if (!HeaderCode.empty())
- Content = "#include\"" + std::string(TestHeaderName) + "\"\n" + Content;
+ Content = (llvm::Twine("#include\"") +
+ llvm::sys::path::filename(TestHeaderName) + "\"\n" + Content)
+ .str();
InMemoryFileSystem->addFile(TestFileName, 0,
llvm::MemoryBuffer::getMemBuffer(Content));
Invocation.run();
Symbols = Factory->Collector->takeSymbols();
return true;
}
protected:
+ std::string TestHeaderName;
+ std::string TestHeaderUri;
+ std::string TestFileName;
+ std::string TestFileUri;
SymbolSlab Symbols;
SymbolCollector::Options CollectorOpts;
};
@@ -165,17 +176,33 @@
CollectorOpts.IndexMainFiles = false;
runSymbolCollector("class Foo {};", /*Main=*/"");
EXPECT_THAT(Symbols,
- UnorderedElementsAre(AllOf(QName("Foo"), CPath("symbols.h"))));
+ UnorderedElementsAre(AllOf(QName("Foo"), Uri(TestHeaderUri))));
}
TEST_F(SymbolCollectorTest, SymbolRelativeWithFallback) {
CollectorOpts.IndexMainFiles = false;
+ TestHeaderName = "x.h";
+ TestFileName = "x.cpp";
+ TestHeaderUri =
+ URI::createFile(getVirtualTestFilePath(TestHeaderName)).toString();
CollectorOpts.FallbackDir = getVirtualTestRoot();
runSymbolCollector("class Foo {};", /*Main=*/"");
EXPECT_THAT(Symbols,
- UnorderedElementsAre(AllOf(
- QName("Foo"), CPath(getVirtualTestFilePath("symbols.h")))));
+ UnorderedElementsAre(AllOf(QName("Foo"), Uri(TestHeaderUri))));
+}
+
+#ifndef LLVM_ON_WIN32
+TEST_F(SymbolCollectorTest, CustomURIScheme) {
+ CollectorOpts.IndexMainFiles = false;
+ // Use test URI scheme from URITests.cpp
+ CollectorOpts.FileURIScheme = "unittest";
+ TestHeaderName = getVirtualTestFilePath("test-root/x.h").str();
+ TestFileName = getVirtualTestFilePath("test-root/x.cpp").str();
+ runSymbolCollector("class Foo {};", /*Main=*/"");
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(AllOf(QName("Foo"), Uri("unittest:x.h"))));
}
+#endif
TEST_F(SymbolCollectorTest, IncludeEnums) {
CollectorOpts.IndexMainFiles = false;
@@ -229,14 +256,14 @@
)");
runSymbolCollector(Header.code(), /*Main=*/"");
- EXPECT_THAT(Symbols,
- UnorderedElementsAre(
- AllOf(QName("abc_Test"),
- LocationOffsets(Header.offsetRange("expansion")),
- CPath(TestHeaderName)),
- AllOf(QName("Test"),
- LocationOffsets(Header.offsetRange("spelling")),
- CPath(TestHeaderName))));
+ EXPECT_THAT(
+ Symbols,
+ UnorderedElementsAre(
+ AllOf(QName("abc_Test"),
+ LocationOffsets(Header.offsetRange("expansion")),
+ Uri(TestHeaderUri)),
+ AllOf(QName("Test"), LocationOffsets(Header.offsetRange("spelling")),
+ Uri(TestHeaderUri))));
}
TEST_F(SymbolCollectorTest, SymbolFormedFromMacroInMainFile) {
@@ -254,14 +281,13 @@
FF2();
)");
runSymbolCollector(/*Header=*/"", Main.code());
- EXPECT_THAT(Symbols,
- UnorderedElementsAre(
- AllOf(QName("abc_Test"),
- LocationOffsets(Main.offsetRange("expansion")),
- CPath(TestFileName)),
- AllOf(QName("Test"),
- LocationOffsets(Main.offsetRange("spelling")),
- CPath(TestFileName))));
+ EXPECT_THAT(Symbols, UnorderedElementsAre(
+ AllOf(QName("abc_Test"),
+ LocationOffsets(Main.offsetRange("expansion")),
+ Uri(TestFileUri)),
+ AllOf(QName("Test"),
+ LocationOffsets(Main.offsetRange("spelling")),
+ Uri(TestFileUri))));
}
TEST_F(SymbolCollectorTest, IgnoreSymbolsInMainFile) {
@@ -411,7 +437,7 @@
CanonicalDeclaration:
StartOffset: 0
EndOffset: 1
- FilePath: /path/foo.h
+ FileUri: file:///path/foo.h
CompletionLabel: 'Foo1-label'
CompletionFilterText: 'filter'
CompletionPlainInsertText: 'plain'
@@ -431,22 +457,23 @@
CanonicalDeclaration:
StartOffset: 10
EndOffset: 12
- FilePath: /path/foo.h
+ FileUri: file:///path/bar.h
CompletionLabel: 'Foo2-label'
CompletionFilterText: 'filter'
CompletionPlainInsertText: 'plain'
CompletionSnippetInsertText: 'snippet'
...
)";
auto Symbols1 = SymbolFromYAML(YAML1);
- EXPECT_THAT(Symbols1, UnorderedElementsAre(
- AllOf(QName("clang::Foo1"), Labeled("Foo1-label"),
- Doc("Foo doc"), Detail("int"))));
+ EXPECT_THAT(Symbols1,
+ UnorderedElementsAre(AllOf(
+ QName("clang::Foo1"), Labeled("Foo1-label"), Doc("Foo doc"),
+ Detail("int"), Uri("file:///path/foo.h"))));
auto Symbols2 = SymbolFromYAML(YAML2);
- EXPECT_THAT(Symbols2, UnorderedElementsAre(AllOf(QName("clang::Foo2"),
- Labeled("Foo2-label"),
- Not(HasDetail()))));
+ EXPECT_THAT(Symbols2, UnorderedElementsAre(AllOf(
+ QName("clang::Foo2"), Labeled("Foo2-label"),
+ Not(HasDetail()), Uri("file:///path/bar.h"))));
std::string ConcatenatedYAML =
SymbolsToYAML(Symbols1) + SymbolsToYAML(Symbols2);
Index: unittests/clangd/IndexTests.cpp
===================================================================
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -226,8 +226,8 @@
Symbol L, R;
L.ID = R.ID = SymbolID("hello");
L.Name = R.Name = "Foo"; // same in both
- L.CanonicalDeclaration.FilePath = "left.h"; // differs
- R.CanonicalDeclaration.FilePath = "right.h";
+ L.CanonicalDeclaration.FileUri = "file:///left.h"; // differs
+ R.CanonicalDeclaration.FileUri = "file:///right.h";
L.CompletionPlainInsertText = "f00"; // present in left only
R.CompletionSnippetInsertText = "f0{$1:0}"; // present in right only
Symbol::Details DetL, DetR;
@@ -240,7 +240,7 @@
Symbol::Details Scratch;
Symbol M = mergeSymbol(L, R, &Scratch);
EXPECT_EQ(M.Name, "Foo");
- EXPECT_EQ(M.CanonicalDeclaration.FilePath, "left.h");
+ EXPECT_EQ(M.CanonicalDeclaration.FileUri, "file:///left.h");
EXPECT_EQ(M.CompletionPlainInsertText, "f00");
EXPECT_EQ(M.CompletionSnippetInsertText, "f0{$1:0}");
ASSERT_TRUE(M.Detail);
Index: clangd/index/SymbolYAML.cpp
===================================================================
--- clangd/index/SymbolYAML.cpp
+++ clangd/index/SymbolYAML.cpp
@@ -48,7 +48,7 @@
static void mapping(IO &IO, SymbolLocation &Value) {
IO.mapRequired("StartOffset", Value.StartOffset);
IO.mapRequired("EndOffset", Value.EndOffset);
- IO.mapRequired("FilePath", Value.FilePath);
+ IO.mapRequired("FileUri", Value.FileUri);
}
};
Index: clangd/index/SymbolCollector.h
===================================================================
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -30,10 +30,13 @@
/// Whether to collect symbols in main files (e.g. the source file
/// corresponding to a TU).
bool IndexMainFiles = false;
- // When symbol paths cannot be resolved to absolute paths (e.g. files in
- // VFS that does not have absolute path), combine the fallback directory
- // with symbols' paths to get absolute paths. This must be an absolute path.
+ /// When symbol paths cannot be resolved to absolute paths (e.g. files in
+ /// VFS that does not have absolute path), combine the fallback directory
+ /// with symbols' paths to get absolute paths. This must be an absolute
+ /// path.
std::string FallbackDir;
+ /// Specifies the URI scheme used to encode file paths in symbols.
+ std::string FileURIScheme = "file";
};
SymbolCollector(Options Opts);
Index: clangd/index/SymbolCollector.cpp
===================================================================
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -9,6 +9,7 @@
#include "SymbolCollector.h"
#include "../CodeCompletionStrings.h"
+#include "../URI.h"
#include "clang/AST/DeclCXX.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Basic/SourceManager.h"
@@ -22,14 +23,15 @@
namespace clangd {
namespace {
-// Make the Path absolute using the current working directory of the given
-// SourceManager if the Path is not an absolute path. If failed, this combine
-// relative paths with \p FallbackDir to get an absolute path.
+// Returns a file:// URI of \p Path. This makes the \p Path absolute using the
+// current working directory of the given SourceManager if the Path is not an
+// absolute path. If failed, this resolves relative paths against \p FallbackDir
+// to get an absolute path.
//
// The Path can be a path relative to the build directory, or retrieved from
// the SourceManager.
-std::string makeAbsolutePath(const SourceManager &SM, StringRef Path,
- StringRef FallbackDir) {
+std::string toUri(const SourceManager &SM, StringRef Path,
+ StringRef FallbackDir, StringRef UriScheme) {
llvm::SmallString<128> AbsolutePath(Path);
if (std::error_code EC =
SM.getFileManager().getVirtualFileSystem()->makeAbsolute(
@@ -60,7 +62,10 @@
llvm::sys::fs::make_absolute(FallbackDir, AbsolutePath);
llvm::sys::path::remove_dots(AbsolutePath, /*remove_dot_dot=*/true);
}
- return AbsolutePath.str();
+ auto U = URI::create(AbsolutePath, UriScheme);
+ if (!U)
+ llvm_unreachable(llvm::toString(U.takeError()).c_str());
+ return U->toString();
}
// "a::b::c", return {"a::b::", "c"}. Scope is empty if there's no qualifier.
@@ -117,29 +122,28 @@
// For symbols defined inside macros:
// * use expansion location, if the symbol is formed via macro concatenation.
// * use spelling location, otherwise.
-SymbolLocation GetSymbolLocation(const NamedDecl *D, SourceManager &SM,
- StringRef FallbackDir,
- std::string &FilePathStorage) {
+SymbolLocation getSymbolLocation(const NamedDecl *D, SourceManager &SM,
+ StringRef FallbackDir, StringRef UriScheme,
+ std::string &FileUriStorage) {
SymbolLocation Location;
SourceLocation Loc = SM.getSpellingLoc(D->getLocation());
if (D->getLocation().isMacroID()) {
// The symbol is formed via macro concatenation, the spelling location will
// be "<scratch space>", which is not interesting to us, use the expansion
// location instead.
if (llvm::StringRef(Loc.printToString(SM)).startswith("<scratch")) {
- FilePathStorage = makeAbsolutePath(
- SM, SM.getFilename(SM.getExpansionLoc(D->getLocation())),
- FallbackDir);
- return {FilePathStorage,
+ FileUriStorage =
+ toUri(SM, SM.getFilename(SM.getExpansionLoc(D->getLocation())),
+ FallbackDir, UriScheme);
+ return {FileUriStorage,
SM.getFileOffset(SM.getExpansionRange(D->getLocStart()).first),
SM.getFileOffset(SM.getExpansionRange(D->getLocEnd()).second)};
}
}
- FilePathStorage = makeAbsolutePath(SM, SM.getFilename(Loc), FallbackDir);
- return {FilePathStorage,
- SM.getFileOffset(SM.getSpellingLoc(D->getLocStart())),
+ FileUriStorage = toUri(SM, SM.getFilename(Loc), FallbackDir, UriScheme);
+ return {FileUriStorage, SM.getFileOffset(SM.getSpellingLoc(D->getLocStart())),
SM.getFileOffset(SM.getSpellingLoc(D->getLocEnd()))};
}
@@ -196,8 +200,9 @@
S.ID = std::move(ID);
std::tie(S.Scope, S.Name) = splitQualifiedName(QName);
S.SymInfo = index::getSymbolInfo(D);
- std::string FilePath;
- S.CanonicalDeclaration = GetSymbolLocation(ND, SM, Opts.FallbackDir, FilePath);
+ std::string Uri;
+ S.CanonicalDeclaration =
+ getSymbolLocation(ND, SM, Opts.FallbackDir, Opts.FileURIScheme, Uri);
// Add completion info.
assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set.");
Index: clangd/index/Merge.cpp
===================================================================
--- clangd/index/Merge.cpp
+++ clangd/index/Merge.cpp
@@ -63,7 +63,7 @@
Symbol S = L;
// For each optional field, fill it from R if missing in L.
// (It might be missing in R too, but that's a no-op).
- if (S.CanonicalDeclaration.FilePath == "")
+ if (S.CanonicalDeclaration.FileUri == "")
S.CanonicalDeclaration = R.CanonicalDeclaration;
if (S.CompletionLabel == "")
S.CompletionLabel = R.CompletionLabel;
Index: clangd/index/Index.h
===================================================================
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -23,8 +23,8 @@
namespace clangd {
struct SymbolLocation {
- // The absolute path of the source file where a symbol occurs.
- llvm::StringRef FilePath;
+ // The URI of the source file where a symbol occurs.
+ llvm::StringRef FileUri;
// The 0-based offset to the first character of the symbol from the beginning
// of the source file.
unsigned StartOffset;
Index: clangd/index/Index.cpp
===================================================================
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -54,7 +54,7 @@
// We need to copy every StringRef field onto the arena.
Intern(S.Name);
Intern(S.Scope);
- Intern(S.CanonicalDeclaration.FilePath);
+ Intern(S.CanonicalDeclaration.FileUri);
Intern(S.CompletionLabel);
Intern(S.CompletionFilterText);
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
