[clangd] Use URIs in index symbols.

Reviewers: hokein, sammccall

Reviewed By: sammccall

Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits

Differential Revision: https://reviews.llvm.org/D42915

llvm-svn: 324358

8 files changed, 144 insertions, 81 deletions

diff --git a/clang-tools-extra/clangd/index/Index.cpp b/clang-tools-extra/clangd/index/Index.cpp
index f56b1bf5697..ddf0546ee76 100644
--- a/clang-tools-extra/clangd/index/Index.cpp
+++ b/clang-tools-extra/clangd/index/Index.cpp
@@ -54,7 +54,7 @@ static void own(Symbol &S, DenseSet<StringRef> &Strings,
   // 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);
diff --git a/clang-tools-extra/clangd/index/Index.h b/clang-tools-extra/clangd/index/Index.h
index 4ca5bcdee2b..fe4d5d67ff1 100644
--- a/clang-tools-extra/clangd/index/Index.h
+++ b/clang-tools-extra/clangd/index/Index.h
@@ -23,8 +23,8 @@ namespace clang {
 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;
diff --git a/clang-tools-extra/clangd/index/Merge.cpp b/clang-tools-extra/clangd/index/Merge.cpp
index 5c572c097f4..afa4a7b6094 100644
--- a/clang-tools-extra/clangd/index/Merge.cpp
+++ b/clang-tools-extra/clangd/index/Merge.cpp
@@ -63,7 +63,7 @@ mergeSymbol(const Symbol &L, const Symbol &R, Symbol::Details *Scratch) {
   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;
diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index 83927bdb615..b76c6b4b449 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -9,6 +9,8 @@
 
 #include "SymbolCollector.h"
 #include "../CodeCompletionStrings.h"
+#include "../Logger.h"
+#include "../URI.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/SourceManager.h"
@@ -22,14 +24,17 @@ namespace clang {
 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 URI of \p Path. Firstly, 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. Then, this tries creating an URI for the absolute
+// path with schemes specified in \p Opts. This returns an URI with the first
+// working scheme, if there is any; otherwise, this returns None.
 //
 // 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) {
+llvm::Optional<std::string> toURI(const SourceManager &SM, StringRef Path,
+                                  const SymbolCollector::Options &Opts) {
   llvm::SmallString<128> AbsolutePath(Path);
   if (std::error_code EC =
           SM.getFileManager().getVirtualFileSystem()->makeAbsolute(
@@ -56,11 +61,21 @@ std::string makeAbsolutePath(const SourceManager &SM, StringRef Path,
                               llvm::sys::path::filename(AbsolutePath.str()));
       AbsolutePath = AbsoluteFilename;
     }
-  } else if (!FallbackDir.empty()) {
-    llvm::sys::fs::make_absolute(FallbackDir, AbsolutePath);
+  } else if (!Opts.FallbackDir.empty()) {
+    llvm::sys::fs::make_absolute(Opts.FallbackDir, AbsolutePath);
     llvm::sys::path::remove_dots(AbsolutePath, /*remove_dot_dot=*/true);
   }
-  return AbsolutePath.str();
+
+  std::string ErrMsg;
+  for (const auto &Scheme : Opts.URISchemes) {
+    auto U = URI::create(AbsolutePath, Scheme);
+    if (U)
+      return U->toString();
+    ErrMsg += llvm::toString(U.takeError()) + "\n";
+  }
+  log(llvm::Twine("Failed to create an URI for file ") + AbsolutePath + ": " +
+      ErrMsg);
+  return llvm::None;
 }
 
 // "a::b::c", return {"a::b::", "c"}. Scope is empty if there's no qualifier.
@@ -117,35 +132,34 @@ bool shouldFilterDecl(const NamedDecl *ND, ASTContext *ASTCtx,
 // 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 Location;
-
-  SourceLocation Loc = SM.getSpellingLoc(D->getLocation());
-  if (D->getLocation().isMacroID()) {
-    // We use the expansion location for the following symbols, as spelling
-    // locations of these symbols are not interesting to us:
-    //   * symbols formed via macro concatenation, the spelling location will
-    //     be "<scratch space>"
-    //   * symbols controlled and defined by a compile command-line option
-    //     `-DName=foo`, the spelling location will be "<command line>".
-    std::string PrintLoc = Loc.printToString(SM);
+llvm::Optional<SymbolLocation>
+getSymbolLocation(const NamedDecl *D, SourceManager &SM,
+                  const SymbolCollector::Options &Opts,
+                  std::string &FileURIStorage) {
+  SourceLocation Loc = D->getLocation();
+  SourceLocation StartLoc = SM.getSpellingLoc(D->getLocStart());
+  SourceLocation EndLoc = SM.getSpellingLoc(D->getLocEnd());
+  if (Loc.isMacroID()) {
+    std::string PrintLoc = SM.getSpellingLoc(Loc).printToString(SM);
     if (llvm::StringRef(PrintLoc).startswith("<scratch") ||
         llvm::StringRef(PrintLoc).startswith("<command line>")) {
-      FilePathStorage = makeAbsolutePath(
-          SM, SM.getFilename(SM.getExpansionLoc(D->getLocation())),
-          FallbackDir);
-      return {FilePathStorage,
-              SM.getFileOffset(SM.getExpansionRange(D->getLocStart()).first),
-              SM.getFileOffset(SM.getExpansionRange(D->getLocEnd()).second)};
+      // We use the expansion location for the following symbols, as spelling
+      // locations of these symbols are not interesting to us:
+      //   * symbols formed via macro concatenation, the spelling location will
+      //     be "<scratch space>"
+      //   * symbols controlled and defined by a compile command-line option
+      //     `-DName=foo`, the spelling location will be "<command line>".
+      StartLoc = SM.getExpansionRange(D->getLocStart()).first;
+      EndLoc = SM.getExpansionRange(D->getLocEnd()).second;
     }
   }
 
-  FilePathStorage = makeAbsolutePath(SM, SM.getFilename(Loc), FallbackDir);
-  return {FilePathStorage,
-          SM.getFileOffset(SM.getSpellingLoc(D->getLocStart())),
-          SM.getFileOffset(SM.getSpellingLoc(D->getLocEnd()))};
+  auto U = toURI(SM, SM.getFilename(StartLoc), Opts);
+  if (!U)
+    return llvm::None;
+  FileURIStorage = std::move(*U);
+  return SymbolLocation{FileURIStorage, SM.getFileOffset(StartLoc),
+                        SM.getFileOffset(EndLoc)};
 }
 
 } // namespace
@@ -201,8 +215,9 @@ bool SymbolCollector::handleDeclOccurence(
     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 URIStorage;
+    if (auto DeclLoc = getSymbolLocation(ND, SM, Opts, URIStorage))
+      S.CanonicalDeclaration = *DeclLoc;
 
     // Add completion info.
     assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set.");
diff --git a/clang-tools-extra/clangd/index/SymbolCollector.h b/clang-tools-extra/clangd/index/SymbolCollector.h
index 08464e6e634..d07828a9b49 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.h
+++ b/clang-tools-extra/clangd/index/SymbolCollector.h
@@ -30,10 +30,15 @@ public:
     /// 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 URI schemes that can be used to generate URIs for file paths
+    /// in symbols. The list of schemes will be tried in order until a working
+    /// scheme is found. If no scheme works, symbol location will be dropped.
+    std::vector<std::string> URISchemes = {"file"};
   };
 
   SymbolCollector(Options Opts);
diff --git a/clang-tools-extra/clangd/index/SymbolYAML.cpp b/clang-tools-extra/clangd/index/SymbolYAML.cpp
index 83e8cc64f68..1553cee597e 100644
--- a/clang-tools-extra/clangd/index/SymbolYAML.cpp
+++ b/clang-tools-extra/clangd/index/SymbolYAML.cpp
@@ -48,7 +48,7 @@ template <> struct MappingTraits<SymbolLocation> {
   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);
   }
 };
 
diff --git a/clang-tools-extra/unittests/clangd/IndexTests.cpp b/clang-tools-extra/unittests/clangd/IndexTests.cpp
index 73406a48e4f..65904495f21 100644
--- a/clang-tools-extra/unittests/clangd/IndexTests.cpp
+++ b/clang-tools-extra/unittests/clangd/IndexTests.cpp
@@ -226,8 +226,8 @@ TEST(MergeTest, Merge) {
   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 @@ TEST(MergeTest, Merge) {
   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);
diff --git a/clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp b/clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
index 6340686cb5d..67e5654e849 100644
--- a/clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
@@ -46,7 +46,7 @@ MATCHER_P(Snippet, S, "") {
   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(DeclURI, 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 clang {
 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 @@ public:
 
 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,
                           const std::vector<std::string> &ExtraArgs = {}) {
     llvm::IntrusiveRefCntPtr<vfs::InMemoryFileSystem> InMemoryFileSystem(
@@ -104,7 +109,9 @@ public:
 
     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();
@@ -113,6 +120,10 @@ public:
   }
 
 protected:
+  std::string TestHeaderName;
+  std::string TestHeaderURI;
+  std::string TestFileName;
+  std::string TestFileURI;
   SymbolSlab Symbols;
   SymbolCollector::Options CollectorOpts;
 };
@@ -169,16 +180,49 @@ TEST_F(SymbolCollectorTest, SymbolRelativeNoFallback) {
   CollectorOpts.IndexMainFiles = false;
   runSymbolCollector("class Foo {};", /*Main=*/"");
   EXPECT_THAT(Symbols,
-              UnorderedElementsAre(AllOf(QName("Foo"), CPath("symbols.h"))));
+              UnorderedElementsAre(AllOf(QName("Foo"), DeclURI(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"), DeclURI(TestHeaderURI))));
+}
+
+#ifndef LLVM_ON_WIN32
+TEST_F(SymbolCollectorTest, CustomURIScheme) {
+  CollectorOpts.IndexMainFiles = false;
+  // Use test URI scheme from URITests.cpp
+  CollectorOpts.URISchemes.insert(CollectorOpts.URISchemes.begin(), "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"), DeclURI("unittest:x.h"))));
+}
+#endif
+
+TEST_F(SymbolCollectorTest, InvalidURIScheme) {
+  CollectorOpts.IndexMainFiles = false;
+  // Use test URI scheme from URITests.cpp
+  CollectorOpts.URISchemes = {"invalid"};
+  runSymbolCollector("class Foo {};", /*Main=*/"");
+  EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("Foo"), DeclURI(""))));
+}
+
+TEST_F(SymbolCollectorTest, FallbackToFileURI) {
+  CollectorOpts.IndexMainFiles = false;
+  // Use test URI scheme from URITests.cpp
+  CollectorOpts.URISchemes = {"invalid", "file"};
+  runSymbolCollector("class Foo {};", /*Main=*/"");
+  EXPECT_THAT(Symbols, UnorderedElementsAre(
+                           AllOf(QName("Foo"), DeclURI(TestHeaderURI))));
 }
 
 TEST_F(SymbolCollectorTest, IncludeEnums) {
@@ -233,14 +277,14 @@ TEST_F(SymbolCollectorTest, SymbolFormedFromMacro) {
   )");
 
   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")),
+                DeclURI(TestHeaderURI)),
+          AllOf(QName("Test"), LocationOffsets(Header.offsetRange("spelling")),
+                DeclURI(TestHeaderURI))));
 }
 
 TEST_F(SymbolCollectorTest, SymbolFormedFromMacroInMainFile) {
@@ -258,14 +302,13 @@ TEST_F(SymbolCollectorTest, SymbolFormedFromMacroInMainFile) {
     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")),
+                                 DeclURI(TestFileURI)),
+                           AllOf(QName("Test"),
+                                 LocationOffsets(Main.offsetRange("spelling")),
+                                 DeclURI(TestFileURI))));
 }
 
 TEST_F(SymbolCollectorTest, SymbolFormedByCLI) {
@@ -279,11 +322,10 @@ TEST_F(SymbolCollectorTest, SymbolFormedByCLI) {
 
   runSymbolCollector(Header.code(), /*Main=*/"",
                      /*ExtraArgs=*/{"-DNAME=name"});
-  EXPECT_THAT(Symbols,
-              UnorderedElementsAre(
-                  AllOf(QName("name"),
-                        LocationOffsets(Header.offsetRange("expansion")),
-                        CPath(TestHeaderName))));
+  EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(
+                           QName("name"),
+                           LocationOffsets(Header.offsetRange("expansion")),
+                           DeclURI(TestHeaderURI))));
 }
 
 TEST_F(SymbolCollectorTest, IgnoreSymbolsInMainFile) {
@@ -433,7 +475,7 @@ SymInfo:
 CanonicalDeclaration:
   StartOffset:     0
   EndOffset:       1
-  FilePath:        /path/foo.h
+  FileURI:        file:///path/foo.h
 CompletionLabel:    'Foo1-label'
 CompletionFilterText:    'filter'
 CompletionPlainInsertText:    'plain'
@@ -453,7 +495,7 @@ SymInfo:
 CanonicalDeclaration:
   StartOffset:     10
   EndOffset:       12
-  FilePath:        /path/foo.h
+  FileURI:        file:///path/bar.h
 CompletionLabel:    'Foo2-label'
 CompletionFilterText:    'filter'
 CompletionPlainInsertText:    'plain'
@@ -462,13 +504,14 @@ 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"), DeclURI("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()), DeclURI("file:///path/bar.h"))));
 
   std::string ConcatenatedYAML =
       SymbolsToYAML(Symbols1) + SymbolsToYAML(Symbols2);