Skip to content

Commit 338374a

Browse files
QuietMisdreavusQuietMisdreavus
authored
[5.7] [SymbolGraphGen] Add filename and module name to symbols' doc comments (#58964)
* move symbol graph samples to the bottom of the file * add information about a doc comment's file and module rdar://81190369 * refactor: group file URI collection/serialization together * test for docComment.module to identify externally-inherited docs
1 parent 16974a4 commit 338374a

File tree

6 files changed

+441
-241
lines changed

6 files changed

+441
-241
lines changed

‎lib/SymbolGraphGen/Symbol.cpp

Lines changed: 55 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,37 @@ const ValueDecl *Symbol::getDeclInheritingDocs() const {
197197
}
198198
}
199199

200+
namespace {
201+
202+
StringRef getFileNameForDecl(const ValueDecl *VD) {
203+
if (!VD) return StringRef{};
204+
205+
SourceLoc Loc = VD->getLoc(/*SerializedOK=*/true);
206+
if (Loc.isInvalid()) return StringRef{};
207+
208+
SourceManager &SourceM = VD->getASTContext().SourceMgr;
209+
return SourceM.getDisplayNameForLoc(Loc);
210+
}
211+
212+
StringRef getFileNameForDecl(const clang::Decl *ClangD) {
213+
if (!ClangD) return StringRef{};
214+
215+
const clang::SourceManager &ClangSourceMgr = ClangD->getASTContext().getSourceManager();
216+
clang::PresumedLoc Loc = ClangSourceMgr.getPresumedLoc(ClangD->getLocation());
217+
if (Loc.isInvalid()) return StringRef{};
218+
219+
return StringRef(Loc.getFilename());
220+
}
221+
222+
void serializeFileURI(llvm::json::OStream &OS, StringRef FileName) {
223+
// FIXME: This can emit invalid URIs if the file name has a space in it (rdar://69242070)
224+
SmallString<1024> FileURI("file://");
225+
FileURI.append(FileName);
226+
OS.attribute("uri", FileURI.str());
227+
}
228+
229+
}
230+
200231
void Symbol::serializeDocComment(llvm::json::OStream &OS) const {
201232
if (ClangNode ClangN = VD->getClangNode()) {
202233
if (!Graph->Walker.Options.IncludeClangDocs)
@@ -221,6 +252,12 @@ void Symbol::serializeDocComment(llvm::json::OStream &OS) const {
221252
splitIntoLines(Text, Lines);
222253

223254
OS.attributeObject("docComment", [&]() {
255+
StringRef FileName = getFileNameForDecl(ClangD);
256+
if (!FileName.empty())
257+
serializeFileURI(OS, FileName);
258+
if (const auto *ModuleD = VD->getModuleContext()) {
259+
OS.attribute("module", ModuleD->getNameStr());
260+
}
224261
OS.attributeArray("lines", [&]() {
225262
for (StringRef Line : Lines) {
226263
OS.object([&](){
@@ -247,6 +284,12 @@ void Symbol::serializeDocComment(llvm::json::OStream &OS) const {
247284
}
248285

249286
OS.attributeObject("docComment", [&](){
287+
StringRef FileName = getFileNameForDecl(DocCommentProvidingDecl);
288+
if (!FileName.empty())
289+
serializeFileURI(OS, FileName);
290+
if (const auto *ModuleD = DocCommentProvidingDecl->getModuleContext()) {
291+
OS.attribute("module", ModuleD->getNameStr());
292+
}
250293
auto LL = Graph->Ctx.getLineList(RC);
251294
StringRef FirstNonBlankLine;
252295
for (const auto &Line : LL.getLines()) {
@@ -415,37 +458,31 @@ void Symbol::serializeLocationMixin(llvm::json::OStream &OS) const {
415458
return;
416459

417460
if (auto *ClangD = ClangN.getAsDecl()) {
418-
clang::SourceManager &ClangSM =
419-
ClangD->getASTContext().getSourceManager();
420-
421-
clang::PresumedLoc Loc = ClangSM.getPresumedLoc(ClangD->getLocation());
422-
if (Loc.isValid()) {
423-
// TODO: We should use a common function to fill in the location
424-
// information for both cursor info and symbol graph gen, then also
425-
// include position here.
461+
StringRef FileName = getFileNameForDecl(ClangD);
462+
if (!FileName.empty()) {
426463
OS.attributeObject("location", [&](){
427-
SmallString<1024> FileURI("file://");
428-
FileURI.append(Loc.getFilename());
429-
OS.attribute("uri", FileURI.str());
464+
// TODO: We should use a common function to fill in the location
465+
// information for both cursor info and symbol graph gen, then also
466+
// include position here.
467+
serializeFileURI(OS, FileName);
430468
});
431469
}
432470
}
433471

434472
return;
435473
}
436474

437-
auto Loc = VD->getLoc(/*SerializedOK=*/true);
438-
if (Loc.isInvalid()) {
475+
auto FileName = getFileNameForDecl(VD);
476+
if (FileName.empty()) {
439477
return;
440478
}
441-
auto FileName = VD->getASTContext().SourceMgr.getDisplayNameForLoc(Loc);
442-
if (FileName.empty()) {
479+
// TODO: Fold serializePosition into serializeFileURI so we don't need to load Loc twice?
480+
auto Loc = VD->getLoc(/*SerializedOK=*/true);
481+
if (Loc.isInvalid()) {
443482
return;
444483
}
445484
OS.attributeObject("location", [&](){
446-
SmallString<1024> FileURI("file://");
447-
FileURI.append(FileName);
448-
OS.attribute("uri", FileURI.str());
485+
serializeFileURI(OS, FileName);
449486
serializePosition("position", Loc, Graph->M.getASTContext().SourceMgr, OS);
450487
});
451488
}

‎test/SourceKit/CursorInfo/cursor_info_multi_module.swift

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,9 @@ func test() {
5858
// CHECK-NORMAL-NEXT: },
5959
// CHECK-NORMAL-NEXT: "text": "Comment from A"
6060
// CHECK-NORMAL-NEXT: }
61-
// CHECK-NORMAL-NEXT: ]
61+
// CHECK-NORMAL-NEXT: ],
62+
// CHECK-NORMAL-NEXT: "module": "somemod",
63+
// CHECK-NORMAL-NEXT: "uri": "file://{{.*}}cursor_info_multi_module.swift"
6264
// CHECK-NORMAL-NEXT: },
6365
// CHECK-NORMAL: "location": {
6466
// CHECK-NORMAL-NEXT: "position": {
@@ -87,7 +89,9 @@ func test() {
8789
// CHECK-BEFORE-NEXT: },
8890
// CHECK-BEFORE-NEXT: "text": "Comment from B"
8991
// CHECK-BEFORE-NEXT: }
90-
// CHECK-BEFORE-NEXT: ]
92+
// CHECK-BEFORE-NEXT: ],
93+
// CHECK-BEFORE-NEXT: "module": "somemod",
94+
// CHECK-BEFORE-NEXT: "uri": "file://{{.*}}cursor_info_multi_module.swift"
9195
// CHECK-BEFORE-NEXT: },
9296
// CHECK-BEFORE: "location": {
9397
// CHECK-BEFORE-NEXT: "position": {
@@ -117,7 +121,9 @@ func test() {
117121
// CHECK-IN-NEXT: },
118122
// CHECK-IN-NEXT: "text": "Comment from #sourceLocation"
119123
// CHECK-IN-NEXT: }
120-
// CHECK-IN-NEXT: ]
124+
// CHECK-IN-NEXT: ],
125+
// CHECK-IN-NEXT: "module": "somemod",
126+
// CHECK-IN-NEXT: "uri": "file://doesnotexist.swift"
121127
// CHECK-IN-NEXT: },
122128
// CHECK-IN: "location": {
123129
// CHECK-IN-NEXT: "position": {
@@ -146,7 +152,9 @@ func test() {
146152
// CHECK-AFTER-NEXT: },
147153
// CHECK-AFTER-NEXT: "text": "Comment from B"
148154
// CHECK-AFTER-NEXT: }
149-
// CHECK-AFTER-NEXT: ]
155+
// CHECK-AFTER-NEXT: ],
156+
// CHECK-AFTER-NEXT: "module": "somemod",
157+
// CHECK-AFTER-NEXT: "uri": "file://{{.*}}cursor_info_multi_module.swift"
150158
// CHECK-AFTER-NEXT: },
151159
// CHECK-AFTER: "location": {
152160
// CHECK-AFTER-NEXT: "position": {

‎test/SourceKit/CursorInfo/cursor_symbol_graph_objc.swift

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,9 @@ func test(s: ObjCStruct) {
8989
// CHECK-FUNC: {
9090
// CHECK-FUNC: "text": "someFunc doc"
9191
// CHECK-FUNC: }
92-
// CHECK-FUNC: ]
92+
// CHECK-FUNC: ],
93+
// CHECK-FUNC: "module": "MyMod",
94+
// CHECK-FUNC: "uri": "file://{{.*}}mod{{\\\\|/}}M.h"
9395
// CHECK-FUNC: },
9496
// CHECK-FUNC: "functionSignature": {
9597
// CHECK-FUNC: "returns": [
@@ -164,7 +166,9 @@ struct ObjCStruct {
164166
// CHECK-SINGLE1-NEXT: {
165167
// CHECK-SINGLE1-NEXT: "text": "single line doc"
166168
// CHECK-SINGLE1-NEXT: }
167-
// CHECK-SINGLE1-NEXT: ]
169+
// CHECK-SINGLE1-NEXT: ],
170+
// CHECK-SINGLE1-NEXT: "module": "MyMod",
171+
// CHECK-SINGLE1-NEXT: "uri": "file://{{.*}}mod{{\\\\|/}}M.h"
168172
// CHECK-SINGLE1-NEXT: }
169173

170174
//! single line doc
@@ -174,7 +178,9 @@ struct ObjCStruct {
174178
// CHECK-SINGLE2-NEXT: {
175179
// CHECK-SINGLE2-NEXT: "text": "single line doc"
176180
// CHECK-SINGLE2-NEXT: }
177-
// CHECK-SINGLE2-NEXT: ]
181+
// CHECK-SINGLE2-NEXT: ],
182+
// CHECK-SINGLE2-NEXT: "module": "MyMod",
183+
// CHECK-SINGLE2-NEXT: "uri": "file://{{.*}}mod{{\\\\|/}}M.h"
178184
// CHECK-SINGLE2-NEXT: }
179185

180186
/** single line block doc */
@@ -184,7 +190,9 @@ struct ObjCStruct {
184190
// CHECK-BLOCK1-NEXT: {
185191
// CHECK-BLOCK1-NEXT: "text": "single line block doc "
186192
// CHECK-BLOCK1-NEXT: }
187-
// CHECK-BLOCK1-NEXT: ]
193+
// CHECK-BLOCK1-NEXT: ],
194+
// CHECK-BLOCK1-NEXT: "module": "MyMod",
195+
// CHECK-BLOCK1-NEXT: "uri": "file://{{.*}}mod{{\\\\|/}}M.h"
188196
// CHECK-BLOCK1-NEXT: }
189197

190198
/*! single line block doc */
@@ -194,7 +202,9 @@ struct ObjCStruct {
194202
// CHECK-BLOCK2-NEXT: {
195203
// CHECK-BLOCK2-NEXT: "text": "single line block doc "
196204
// CHECK-BLOCK2-NEXT: }
197-
// CHECK-BLOCK2-NEXT: ]
205+
// CHECK-BLOCK2-NEXT: ],
206+
// CHECK-BLOCK2-NEXT: "module": "MyMod",
207+
// CHECK-BLOCK2-NEXT: "uri": "file://{{.*}}mod{{\\\\|/}}M.h"
198208
// CHECK-BLOCK2-NEXT: }
199209

200210
/**
@@ -220,7 +230,9 @@ struct ObjCStruct {
220230
// DISABLED-CHECK-ART-NEXT: {
221231
// DISABLED-CHECK-ART-NEXT: "text": " "
222232
// DISABLED-CHECK-ART-NEXT: }
223-
// DISABLED-CHECK-ART-NEXT: ]
233+
// DISABLED-CHECK-ART-NEXT: ],
234+
// DISABLED-CHECK-ART-NEXT: "module": "MyMod",
235+
// DISABLED-CHECK-ART-NEXT: "uri": "file://{{.*}}mod{{\\\\|/}}M.h"
224236
// DISABLED-CHECK-ART-NEXT: }
225237

226238
/// doc1
@@ -237,7 +249,9 @@ struct ObjCStruct {
237249
// CHECK-MIXED-TYPE-NEXT: {
238250
// CHECK-MIXED-TYPE-NEXT: "text": "doc2 last"
239251
// CHECK-MIXED-TYPE-NEXT: }
240-
// CHECK-MIXED-TYPE-NEXT: ]
252+
// CHECK-MIXED-TYPE-NEXT: ],
253+
// CHECK-MIXED-TYPE-NEXT: "module": "MyMod",
254+
// CHECK-MIXED-TYPE-NEXT: "uri": "file://{{.*}}mod{{\\\\|/}}M.h"
241255
// CHECK-MIXED-TYPE-NEXT: }
242256

243257
/// doc1
@@ -274,7 +288,9 @@ struct ObjCStruct {
274288
// DISABLED-CHECK-MIXED-DOC-NEXT: {
275289
// DISABLED-CHECK-MIXED-DOC-NEXT: "text": "doc3"
276290
// DISABLED-CHECK-MIXED-DOC-NEXT: }
277-
// DISABLED-CHECK-MIXED-DOC-NEXT: ]
291+
// DISABLED-CHECK-MIXED-DOC-NEXT: ],
292+
// DISABLED-CHECK-MIXED-DOC-NEXT: "module": "MyMod",
293+
// DISABLED-CHECK-MIXED-DOC-NEXT: "uri": "file://{{.*}}mod{{\\\\|/}}M.h"
278294
// DISABLED-CHECK-MIXED-DOC-NEXT: }
279295
};
280296

0 commit comments

Comments
 (0)