refactor: group file URI collection/serialization together

Author: QuietMisdreavus

lib/SymbolGraphGen/Symbol.cpp

@@ -197,14 +197,44 @@ const ValueDecl *Symbol::getDeclInheritingDocs() const {
   }
 }
 
+namespace {
+
+StringRef getFileURI(const ValueDecl *VD) {
+  if (!VD) return StringRef{};
+
+  SourceLoc Loc = VD->getLoc(/*SerializedOK=*/true);
+  if (Loc.isInvalid()) return StringRef{};
+
+  SourceManager &SourceM = VD->getASTContext().SourceMgr;
+  return SourceM.getDisplayNameForLoc(Loc);
+}
+
+StringRef getFileURI(const clang::Decl *ClangD) {
+  if (!ClangD) return StringRef{};
+
+  const clang::SourceManager &ClangSourceMgr = ClangD->getASTContext().getSourceManager();
+  clang::PresumedLoc Loc = ClangSourceMgr.getPresumedLoc(ClangD->getLocation());
+  if (Loc.isInvalid()) return StringRef{};
+
+  return StringRef(Loc.getFilename());
+}
+
+void serializeFileURI(llvm::json::OStream &OS, StringRef FileName) {
+  // FIXME: This can emit invalid URIs if the file name has a space in it (rdar://69242070)
+  SmallString<1024> FileURI("file://");
+  FileURI.append(FileName);
+  OS.attribute("uri", FileURI.str());
+}
+
+}
+
 void Symbol::serializeDocComment(llvm::json::OStream &OS) const {
   if (ClangNode ClangN = VD->getClangNode()) {
     if (!Graph->Walker.Options.IncludeClangDocs)
       return;
 
     if (auto *ClangD = ClangN.getAsDecl()) {
       const clang::ASTContext &ClangContext = ClangD->getASTContext();
-      const clang::SourceManager &ClangSourceMgr = ClangContext.getSourceManager();
       const clang::RawComment *RC =
           ClangContext.getRawCommentForAnyRedecl(ClangD);
       if (!RC || !RC->isDocumentation())
@@ -214,20 +244,17 @@ void Symbol::serializeDocComment(llvm::json::OStream &OS) const {
       // line and column ranges. Also consider handling cross-language
       // hierarchies, ie. if there's no comment on the ObjC decl we should
       // look up the hierarchy (and vice versa).
-      std::string Text = RC->getFormattedText(ClangSourceMgr,
+      std::string Text = RC->getFormattedText(ClangContext.getSourceManager(),
                                               ClangContext.getDiagnostics());
       Text = unicode::sanitizeUTF8(Text);
 
       SmallVector<StringRef, 8> Lines;
       splitIntoLines(Text, Lines);
 
       OS.attributeObject("docComment", [&]() {
-        clang::PresumedLoc Loc = ClangSourceMgr.getPresumedLoc(ClangD->getLocation());
-        if (Loc.isValid()) {
-          SmallString<1024> FileURI("file://");
-          FileURI.append(Loc.getFilename());
-          OS.attribute("uri", FileURI.str());
-        }
+        StringRef FileName = getFileURI(ClangD);
+        if (!FileName.empty())
+          serializeFileURI(OS, FileName);
         if (const auto *ModuleD = VD->getModuleContext()) {
           OS.attribute("module", ModuleD->getNameStr());
         }
@@ -257,15 +284,9 @@ void Symbol::serializeDocComment(llvm::json::OStream &OS) const {
   }
 
   OS.attributeObject("docComment", [&](){
-    auto Loc = DocCommentProvidingDecl->getLoc(/*SerializedOK=*/true);
-    if (Loc.isValid()) {
-      auto FileName = DocCommentProvidingDecl->getASTContext().SourceMgr.getDisplayNameForLoc(Loc);
-      if (!FileName.empty()) {
-        SmallString<1024> FileURI("file://");
-        FileURI.append(FileName);
-        OS.attribute("uri", FileURI.str());
-      }
-    }
+    StringRef FileName = getFileURI(DocCommentProvidingDecl);
+    if (!FileName.empty())
+      serializeFileURI(OS, FileName);
     if (const auto *ModuleD = DocCommentProvidingDecl->getModuleContext()) {
       OS.attribute("module", ModuleD->getNameStr());
     }
@@ -437,37 +458,31 @@ void Symbol::serializeLocationMixin(llvm::json::OStream &OS) const {
       return;
 
     if (auto *ClangD = ClangN.getAsDecl()) {
-      clang::SourceManager &ClangSM =
-          ClangD->getASTContext().getSourceManager();
-
-      clang::PresumedLoc Loc = ClangSM.getPresumedLoc(ClangD->getLocation());
-      if (Loc.isValid()) {
-        // TODO: We should use a common function to fill in the location
-        // information for both cursor info and symbol graph gen, then also
-        // include position here.
+      StringRef FileName = getFileURI(ClangD);
+      if (!FileName.empty()) {
         OS.attributeObject("location", [&](){
-          SmallString<1024> FileURI("file://");
-          FileURI.append(Loc.getFilename());
-          OS.attribute("uri", FileURI.str());
+          // TODO: We should use a common function to fill in the location
+          // information for both cursor info and symbol graph gen, then also
+          // include position here.
+          serializeFileURI(OS, FileName);
         });
       }
     }
 
     return;
   }
 
-  auto Loc = VD->getLoc(/*SerializedOK=*/true);
-  if (Loc.isInvalid()) {
+  auto FileName = getFileURI(VD);
+  if (FileName.empty()) {
     return;
   }
-  auto FileName = VD->getASTContext().SourceMgr.getDisplayNameForLoc(Loc);
-  if (FileName.empty()) {
+  // TODO: Fold serializePosition into serializeFileURI so we don't need to load Loc twice?
+  auto Loc = VD->getLoc(/*SerializedOK=*/true);
+  if (Loc.isInvalid()) {
     return;
   }
   OS.attributeObject("location", [&](){
-    SmallString<1024> FileURI("file://");
-    FileURI.append(FileName);
-    OS.attribute("uri", FileURI.str());
+    serializeFileURI(OS, FileName);
     serializePosition("position", Loc, Graph->M.getASTContext().SourceMgr, OS);
   });
 }