Skip to content
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

[SourceKit] Add documentation range in structure (SR-2487) #11264

Merged
merged 3 commits into from
Aug 2, 2017
Merged

[SourceKit] Add documentation range in structure (SR-2487) #11264

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8b4bc18
[SourceKit] Add documentation range in structure (SR-2487)
marcelofabri Jul 30, 2017
c61e0f3
Use Decl::getRawComment instead of RawDocCommentAttr
marcelofabri Aug 2, 2017
f3d49e1
PR feedback
marcelofabri Aug 2, 2017
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
1 change: 1 addition & 0 deletions include/swift/IDE/SyntaxModel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ struct SyntaxStructureNode {
CharSourceRange BodyRange;
CharSourceRange NameRange;
CharSourceRange TypeRange;
CharSourceRange DocRange;
std::vector<CharSourceRange> InheritedTypeRanges;
std::vector<SyntaxStructureElement> Elements;

Expand Down
37 changes: 23 additions & 14 deletions lib/IDE/SyntaxModel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,21 @@ CharSourceRange parameterNameRangeOfCallArg(const TupleExpr *TE,
return CharSourceRange();
}

static CharSourceRange findDocCommentRange(DeclAttributes Attrs) {
for (auto Attr : Attrs) {
if (auto DocAttr = dyn_cast_or_null<RawDocCommentAttr>(Attr)) {
return DocAttr->getCommentRange();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RawDocCommentAttr::getCommentRange returns raw comment range. For example:

// Copyright (c) FooBar Inc.

/// secret.
func doSomething() {

IIRC, getCommentRange() returns from // Copy on line 1 to secret.\n on line 3.
Is that desired behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that desired behavior?

Not really 😅

I've tested it and it indeed returns everything. Do you have any suggestions on how to improve this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, interface printing uses Decl::getRawComment for extracting doc comments, but I'm not sure we can use that here. It might be too costly for this purpose.
@nkcsgexi ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! @rintaro I cannot think of any reason not using Decl::getRawComment here, If we care only attached doc comment in the use case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how do I get a range from a RawComment? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a function getCharSourceRange() to RawComment that analyses the enclosed SingleRawComment where range is available?

Copy link
Member

@rintaro rintaro Aug 2, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like this I guess:

RawComment Raw = ...
if (!Raw.isEmpty()) {
  auto Start = Raw.Comments.front().Range.getStart();
  auto End = Raw.Comments.back().Range.getEnd();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated!

}
}
return CharSourceRange();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We really don't need a member function here. A static function in the file will be sufficient.


static void setDecl(SyntaxStructureNode &N, Decl *D) {
N.Dcl = D;
N.Attrs = D->getAttrs();
N.DocRange = findDocCommentRange(D->getAttrs());
}

} // anonymous namespace

bool SyntaxModelContext::walk(SyntaxModelWalker &Walker) {
Expand Down Expand Up @@ -753,7 +768,7 @@ bool ModelASTWalker::walkToDeclPre(Decl *D) {
} else {
// Pass Function / Method structure node.
SyntaxStructureNode SN;
SN.Dcl = D;
setDecl(SN, D);
const DeclContext *DC = AFD->getDeclContext();
if (DC->isTypeContext()) {
if (FD && FD->isStatic()) {
Expand All @@ -772,12 +787,11 @@ bool ModelASTWalker::walkToDeclPre(Decl *D) {
AFD->getBodySourceRange());
SN.NameRange = charSourceRangeFromSourceRange(SM,
AFD->getSignatureSourceRange());
SN.Attrs = AFD->getAttrs();
pushStructureNode(SN, AFD);
}
} else if (auto *NTD = dyn_cast<NominalTypeDecl>(D)) {
SyntaxStructureNode SN;
SN.Dcl = D;
setDecl(SN, D);
SN.Kind = syntaxStructureKindFromNominalTypeDecl(NTD);
SN.Range = charSourceRangeFromSourceRange(SM, NTD->getSourceRange());
SN.BodyRange = innerCharSourceRangeFromSourceRange(SM, NTD->getBraces());
Expand All @@ -792,12 +806,11 @@ bool ModelASTWalker::walkToDeclPre(Decl *D) {
SN.Elements.emplace_back(SyntaxStructureElementKind::TypeRef, TR);
}

SN.Attrs = NTD->getAttrs();
pushStructureNode(SN, NTD);

} else if (auto *ED = dyn_cast<ExtensionDecl>(D)) {
SyntaxStructureNode SN;
SN.Dcl = D;
setDecl(SN, D);
SN.Kind = SyntaxStructureKind::Extension;
SN.Range = charSourceRangeFromSourceRange(SM, ED->getSourceRange());
SN.BodyRange = innerCharSourceRangeFromSourceRange(SM, ED->getBraces());
Expand All @@ -811,7 +824,6 @@ bool ModelASTWalker::walkToDeclPre(Decl *D) {
SN.Elements.emplace_back(SyntaxStructureElementKind::TypeRef, TR);
}

SN.Attrs = ED->getAttrs();
pushStructureNode(SN, ED);

} else if (auto *PD = dyn_cast<ParamDecl>(D)) {
Expand All @@ -832,7 +844,7 @@ bool ModelASTWalker::walkToDeclPre(Decl *D) {
const DeclContext *DC = VD->getDeclContext();
if (DC->isTypeContext() || DC->isModuleScopeContext()) {
SyntaxStructureNode SN;
SN.Dcl = D;
setDecl(SN, D);
SourceRange SR;
if (auto *PBD = VD->getParentPatternBinding())
SR = PBD->getSourceRange();
Expand Down Expand Up @@ -863,7 +875,6 @@ bool ModelASTWalker::walkToDeclPre(Decl *D) {
} else {
SN.Kind = SyntaxStructureKind::GlobalVariable;
}
SN.Attrs = VD->getAttrs();
pushStructureNode(SN, VD);
}

Expand Down Expand Up @@ -915,7 +926,7 @@ bool ModelASTWalker::walkToDeclPre(Decl *D) {

} else if (auto *EnumCaseD = dyn_cast<EnumCaseDecl>(D)) {
SyntaxStructureNode SN;
SN.Dcl = D;
setDecl(SN, D);
SN.Kind = SyntaxStructureKind::EnumCase;
SN.Range = charSourceRangeFromSourceRange(SM, D->getSourceRange());

Expand All @@ -939,7 +950,7 @@ bool ModelASTWalker::walkToDeclPre(Decl *D) {
if (EnumElemD->getName().empty())
continue;
SyntaxStructureNode SN;
SN.Dcl = EnumElemD;
setDecl(SN, EnumElemD);
SN.Kind = SyntaxStructureKind::EnumElement;
SN.Range = charSourceRangeFromSourceRange(SM,
EnumElemD->getSourceRange());
Expand All @@ -956,23 +967,21 @@ bool ModelASTWalker::walkToDeclPre(Decl *D) {
}
} else if (auto *TypeAliasD = dyn_cast<TypeAliasDecl>(D)) {
SyntaxStructureNode SN;
SN.Dcl = TypeAliasD;
setDecl(SN, D);
SN.Kind = SyntaxStructureKind::TypeAlias;
SN.Range = charSourceRangeFromSourceRange(SM,
TypeAliasD->getSourceRange());
SN.NameRange = CharSourceRange(TypeAliasD->getNameLoc(),
TypeAliasD->getName().getLength());
SN.Attrs = TypeAliasD->getAttrs();
pushStructureNode(SN, TypeAliasD);
} else if (auto *SubscriptD = dyn_cast<SubscriptDecl>(D)) {
SyntaxStructureNode SN;
SN.Dcl = SubscriptD;
setDecl(SN, D);
SN.Kind = SyntaxStructureKind::Subscript;
SN.Range = charSourceRangeFromSourceRange(SM,
SubscriptD->getSourceRange());
SN.BodyRange = innerCharSourceRangeFromSourceRange(SM,
SubscriptD->getBracesRange());
SN.Attrs = SubscriptD->getAttrs();
pushStructureNode(SN, SubscriptD);
}

Expand Down
4 changes: 3 additions & 1 deletion test/SourceKit/DocumentStructure/structure.swift.placeholders.response
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,9 @@
key.offset: 225,
key.length: 36,
key.nameoffset: 229,
key.namelength: 7
key.namelength: 7,
key.docoffset: 91,
key.doclength: 133
},
{
key.kind: source.lang.swift.expr.array,
Expand Down
10 changes: 9 additions & 1 deletion test/SourceKit/DocumentStructure/structure.swift.response
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,8 @@
key.namelength: 11,
key.bodyoffset: 703,
key.bodylength: 186,
key.docoffset: 663,
key.doclength: 19,
key.substructure: [
{
key.kind: source.lang.swift.syntaxtype.comment.mark,
Expand All @@ -415,6 +417,8 @@
key.namelength: 16,
key.bodyoffset: 762,
key.bodylength: 125,
key.docoffset: 708,
key.doclength: 19,
key.substructure: [
{
key.kind: source.lang.swift.decl.var.parameter,
Expand Down Expand Up @@ -557,7 +561,9 @@
key.offset: 1079,
key.length: 15,
key.nameoffset: 1089,
key.namelength: 4
key.namelength: 4,
key.docoffset: 1059,
key.doclength: 18
},
{
key.kind: source.lang.swift.decl.function.free,
Expand Down Expand Up @@ -892,6 +898,8 @@
key.namelength: 13,
key.bodyoffset: 1444,
key.bodylength: 27,
key.docoffset: 1387,
key.doclength: 18,
key.inheritedtypes: [
{
key.name: "NSObject"
Expand Down
4 changes: 3 additions & 1 deletion test/SourceKit/DocumentStructure/structure_object_literals.swift.response
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@
key.length: 65,
key.typename: "S",
key.nameoffset: 148,
key.namelength: 5
key.namelength: 5,
key.docoffset: 0,
key.doclength: 142
},
{
key.kind: source.lang.swift.expr.object_literal,
Expand Down
Loading