Skip to content

[SymbolGraphGen] Add filename and module name to symbols' doc comments #58857

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 24, 2022
Merged

[SymbolGraphGen] Add filename and module name to symbols' doc comments #58857

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8521586
move symbol graph samples to the bottom of the file
QuietMisdreavus May 11, 2022
7182682
add information about a doc comment's file and module
QuietMisdreavus May 12, 2022
ef15476
refactor: group file URI collection/serialization together
QuietMisdreavus May 13, 2022
24b9620
test for docComment.module to identify externally-inherited docs
QuietMisdreavus May 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 55 additions & 18 deletions lib/SymbolGraphGen/Symbol.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,37 @@ const ValueDecl *Symbol::getDeclInheritingDocs() const {
}
}

namespace {

StringRef getFileNameForDecl(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 getFileNameForDecl(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)
Expand All @@ -221,6 +252,12 @@ void Symbol::serializeDocComment(llvm::json::OStream &OS) const {
splitIntoLines(Text, Lines);

OS.attributeObject("docComment", [&]() {
StringRef FileName = getFileNameForDecl(ClangD);
if (!FileName.empty())
serializeFileURI(OS, FileName);
if (const auto *ModuleD = VD->getModuleContext()) {
OS.attribute("module", ModuleD->getNameStr());
}
OS.attributeArray("lines", [&]() {
for (StringRef Line : Lines) {
OS.object([&](){
Expand All @@ -247,6 +284,12 @@ void Symbol::serializeDocComment(llvm::json::OStream &OS) const {
}

OS.attributeObject("docComment", [&](){
StringRef FileName = getFileNameForDecl(DocCommentProvidingDecl);
if (!FileName.empty())
serializeFileURI(OS, FileName);
if (const auto *ModuleD = DocCommentProvidingDecl->getModuleContext()) {
OS.attribute("module", ModuleD->getNameStr());
}
auto LL = Graph->Ctx.getLineList(RC);
StringRef FirstNonBlankLine;
for (const auto &Line : LL.getLines()) {
Expand Down Expand Up @@ -415,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 = getFileNameForDecl(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 = getFileNameForDecl(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);
});
}
Expand Down
16 changes: 12 additions & 4 deletions test/SourceKit/CursorInfo/cursor_info_multi_module.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ func test() {
// CHECK-NORMAL-NEXT: },
// CHECK-NORMAL-NEXT: "text": "Comment from A"
// CHECK-NORMAL-NEXT: }
// CHECK-NORMAL-NEXT: ]
// CHECK-NORMAL-NEXT: ],
// CHECK-NORMAL-NEXT: "module": "somemod",
// CHECK-NORMAL-NEXT: "uri": "file://{{.*}}cursor_info_multi_module.swift"
// CHECK-NORMAL-NEXT: },
// CHECK-NORMAL: "location": {
// CHECK-NORMAL-NEXT: "position": {
Expand Down Expand Up @@ -87,7 +89,9 @@ func test() {
// CHECK-BEFORE-NEXT: },
// CHECK-BEFORE-NEXT: "text": "Comment from B"
// CHECK-BEFORE-NEXT: }
// CHECK-BEFORE-NEXT: ]
// CHECK-BEFORE-NEXT: ],
// CHECK-BEFORE-NEXT: "module": "somemod",
// CHECK-BEFORE-NEXT: "uri": "file://{{.*}}cursor_info_multi_module.swift"
// CHECK-BEFORE-NEXT: },
// CHECK-BEFORE: "location": {
// CHECK-BEFORE-NEXT: "position": {
Expand Down Expand Up @@ -117,7 +121,9 @@ func test() {
// CHECK-IN-NEXT: },
// CHECK-IN-NEXT: "text": "Comment from #sourceLocation"
// CHECK-IN-NEXT: }
// CHECK-IN-NEXT: ]
// CHECK-IN-NEXT: ],
// CHECK-IN-NEXT: "module": "somemod",
// CHECK-IN-NEXT: "uri": "file://doesnotexist.swift"
// CHECK-IN-NEXT: },
// CHECK-IN: "location": {
// CHECK-IN-NEXT: "position": {
Expand Down Expand Up @@ -146,7 +152,9 @@ func test() {
// CHECK-AFTER-NEXT: },
// CHECK-AFTER-NEXT: "text": "Comment from B"
// CHECK-AFTER-NEXT: }
// CHECK-AFTER-NEXT: ]
// CHECK-AFTER-NEXT: ],
// CHECK-AFTER-NEXT: "module": "somemod",
// CHECK-AFTER-NEXT: "uri": "file://{{.*}}cursor_info_multi_module.swift"
// CHECK-AFTER-NEXT: },
// CHECK-AFTER: "location": {
// CHECK-AFTER-NEXT: "position": {
Expand Down
32 changes: 24 additions & 8 deletions test/SourceKit/CursorInfo/cursor_symbol_graph_objc.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,9 @@ func test(s: ObjCStruct) {
// CHECK-FUNC: {
// CHECK-FUNC: "text": "someFunc doc"
// CHECK-FUNC: }
// CHECK-FUNC: ]
// CHECK-FUNC: ],
// CHECK-FUNC: "module": "MyMod",
// CHECK-FUNC: "uri": "file://{{.*}}mod{{\\\\|/}}M.h"
// CHECK-FUNC: },
// CHECK-FUNC: "functionSignature": {
// CHECK-FUNC: "returns": [
Expand Down Expand Up @@ -164,7 +166,9 @@ struct ObjCStruct {
// CHECK-SINGLE1-NEXT: {
// CHECK-SINGLE1-NEXT: "text": "single line doc"
// CHECK-SINGLE1-NEXT: }
// CHECK-SINGLE1-NEXT: ]
// CHECK-SINGLE1-NEXT: ],
// CHECK-SINGLE1-NEXT: "module": "MyMod",
// CHECK-SINGLE1-NEXT: "uri": "file://{{.*}}mod{{\\\\|/}}M.h"
// CHECK-SINGLE1-NEXT: }

//! single line doc
Expand All @@ -174,7 +178,9 @@ struct ObjCStruct {
// CHECK-SINGLE2-NEXT: {
// CHECK-SINGLE2-NEXT: "text": "single line doc"
// CHECK-SINGLE2-NEXT: }
// CHECK-SINGLE2-NEXT: ]
// CHECK-SINGLE2-NEXT: ],
// CHECK-SINGLE2-NEXT: "module": "MyMod",
// CHECK-SINGLE2-NEXT: "uri": "file://{{.*}}mod{{\\\\|/}}M.h"
// CHECK-SINGLE2-NEXT: }

/** single line block doc */
Expand All @@ -184,7 +190,9 @@ struct ObjCStruct {
// CHECK-BLOCK1-NEXT: {
// CHECK-BLOCK1-NEXT: "text": "single line block doc "
// CHECK-BLOCK1-NEXT: }
// CHECK-BLOCK1-NEXT: ]
// CHECK-BLOCK1-NEXT: ],
// CHECK-BLOCK1-NEXT: "module": "MyMod",
// CHECK-BLOCK1-NEXT: "uri": "file://{{.*}}mod{{\\\\|/}}M.h"
// CHECK-BLOCK1-NEXT: }

/*! single line block doc */
Expand All @@ -194,7 +202,9 @@ struct ObjCStruct {
// CHECK-BLOCK2-NEXT: {
// CHECK-BLOCK2-NEXT: "text": "single line block doc "
// CHECK-BLOCK2-NEXT: }
// CHECK-BLOCK2-NEXT: ]
// CHECK-BLOCK2-NEXT: ],
// CHECK-BLOCK2-NEXT: "module": "MyMod",
// CHECK-BLOCK2-NEXT: "uri": "file://{{.*}}mod{{\\\\|/}}M.h"
// CHECK-BLOCK2-NEXT: }

/**
Expand All @@ -220,7 +230,9 @@ struct ObjCStruct {
// DISABLED-CHECK-ART-NEXT: {
// DISABLED-CHECK-ART-NEXT: "text": " "
// DISABLED-CHECK-ART-NEXT: }
// DISABLED-CHECK-ART-NEXT: ]
// DISABLED-CHECK-ART-NEXT: ],
// DISABLED-CHECK-ART-NEXT: "module": "MyMod",
// DISABLED-CHECK-ART-NEXT: "uri": "file://{{.*}}mod{{\\\\|/}}M.h"
// DISABLED-CHECK-ART-NEXT: }

/// doc1
Expand All @@ -237,7 +249,9 @@ struct ObjCStruct {
// CHECK-MIXED-TYPE-NEXT: {
// CHECK-MIXED-TYPE-NEXT: "text": "doc2 last"
// CHECK-MIXED-TYPE-NEXT: }
// CHECK-MIXED-TYPE-NEXT: ]
// CHECK-MIXED-TYPE-NEXT: ],
// CHECK-MIXED-TYPE-NEXT: "module": "MyMod",
// CHECK-MIXED-TYPE-NEXT: "uri": "file://{{.*}}mod{{\\\\|/}}M.h"
// CHECK-MIXED-TYPE-NEXT: }

/// doc1
Expand Down Expand Up @@ -274,7 +288,9 @@ struct ObjCStruct {
// DISABLED-CHECK-MIXED-DOC-NEXT: {
// DISABLED-CHECK-MIXED-DOC-NEXT: "text": "doc3"
// DISABLED-CHECK-MIXED-DOC-NEXT: }
// DISABLED-CHECK-MIXED-DOC-NEXT: ]
// DISABLED-CHECK-MIXED-DOC-NEXT: ],
// DISABLED-CHECK-MIXED-DOC-NEXT: "module": "MyMod",
// DISABLED-CHECK-MIXED-DOC-NEXT: "uri": "file://{{.*}}mod{{\\\\|/}}M.h"
// DISABLED-CHECK-MIXED-DOC-NEXT: }
};

Expand Down
Loading