Skip to content

Commit 4098a16

Browse files
bnbarhambnbarham
authored
Merge pull request #39086 from apple/keep-comments
[AST] Fix comment merging
2 parents 842b909 + 22f5983 commit 4098a16

File tree

3 files changed

+109
-36
lines changed

3 files changed

+109
-36
lines changed

‎include/swift/AST/RawComment.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,10 @@ struct SingleRawComment {
5555
return getKind() == CommentKind::OrdinaryLine ||
5656
getKind() == CommentKind::LineDoc;
5757
}
58+
59+
bool isGyb() const {
60+
return RawText.startswith("// ###");
61+
}
5862
};
5963

6064
struct RawComment {

‎lib/AST/RawComment.cpp

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,10 @@ SingleRawComment::SingleRawComment(StringRef RawText, unsigned ColumnIndent)
6868
: RawText(RawText), Kind(static_cast<unsigned>(getCommentKind(RawText))),
6969
ColumnIndent(ColumnIndent) {}
7070

71+
/// Converts a range of comments (ordinary or doc) to a \c RawComment with
72+
/// only the last range of doc comments. Gyb comments, ie. "// ###" are skipped
73+
/// entirely as if they did not exist (so two doc comments would still be
74+
/// merged if there was a gyb comment inbetween).
7175
static RawComment toRawComment(ASTContext &Context, CharSourceRange Range) {
7276
if (Range.isInvalid())
7377
return RawComment();
@@ -91,25 +95,34 @@ static RawComment toRawComment(ASTContext &Context, CharSourceRange Range) {
9195
assert(Tok.is(tok::comment));
9296

9397
auto SRC = SingleRawComment(Tok.getRange(), SM);
98+
unsigned Start =
99+
SM.getLineAndColumnInBuffer(Tok.getRange().getStart()).first;
100+
unsigned End = SM.getLineAndColumnInBuffer(Tok.getRange().getEnd()).first;
101+
if (SRC.RawText.back() == '\n')
102+
End--;
103+
94104
if (SRC.isOrdinary()) {
95-
// Skip gyb comments that are line number markers.
96-
if (!SRC.RawText.startswith("// ###")) {
105+
// If there's a normal comment then reset the current group, unless it's
106+
// a gyb comment in which case we should just skip it.
107+
if (SRC.isGyb())
108+
LastEnd = End;
109+
else
97110
Comments.clear();
98-
LastEnd = 0;
99-
}
100111
continue;
101112
}
102113

103-
// Merge comments if they are on same or consecutive lines.
104-
unsigned Start =
105-
SM.getLineAndColumnInBuffer(Tok.getRange().getStart()).first;
106-
if (LastEnd > 0 && LastEnd + 1 < Start) {
114+
// Merge and keep the *last* group, ie. if there's:
115+
// <comment1>
116+
// <whitespace>
117+
// <comment2>
118+
// <comment3>
119+
// Only keep <comment2/3> and group into the one RawComment.
120+
121+
if (!Comments.empty() && Start > LastEnd + 1)
107122
Comments.clear();
108-
LastEnd = 0;
109-
} else {
110-
Comments.push_back(SRC);
111-
LastEnd = SM.getLineAndColumnInBuffer(Tok.getRange().getEnd()).first;
112-
}
123+
Comments.push_back(SRC);
124+
125+
LastEnd = End;
113126
}
114127

115128
RawComment Result;

‎test/IDE/comment_merge.swift

Lines changed: 79 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -67,30 +67,81 @@ func is_doc14() {}
6767
/// IS_DOC_END
6868
func is_doc15() {}
6969

70-
/// is_doc16 IS_DOC_START
71-
/** Aaa bbb ccc. */
72-
/// IS_DOC_END
73-
func is_doc16() {}
70+
/// IS_DOC_NOT_ATTACHED
71+
/// Should not return with the comments for the function below it or prevent
72+
/// its actual comment from being returned.
7473

75-
/** Aaa is_doc17 IS_DOC_START *//** bbb */
76-
/// IS_DOC_END
77-
func is_doc17() {}
74+
/// IS_DOC_START priorCommentOneLineGap Aaa.
75+
///
76+
/// Bbb. IS_DOC_END
77+
func priorCommentOneLineGap() {}
78+
79+
/// IS_DOC_NOT_ATTACHED
80+
/// Same as above but with a two line gap this time.
81+
82+
83+
/// IS_DOC_START priorCommentTwoLineGap Aaa.
84+
///
85+
/// Bbb. IS_DOC_END
86+
func priorCommentTwoLineGap() {}
7887

7988
/// IS_DOC_NOT_ATTACHED
80-
// NOT_DOC
81-
/// is_doc18 IS_DOC_START IS_DOC_END
82-
func is_doc18() {}
89+
/// Make sure a gyb comment doesn't cause the previous comment to stay
90+
/// attached.
91+
// ###line 9001
92+
93+
/// IS_DOC_START priorCommentGyb Aaa.
94+
///
95+
/// Bbb. IS_DOC_END
96+
func priorCommentGyb() {}
97+
98+
/**
99+
IS_DOC_START priorBlockMultiLineComment Aaa.
100+
*/
101+
/**
102+
Bbb. IS_DOC_END
103+
*/
104+
func priorBlockMultiLineComment() {}
83105

84-
// Aaa. NOT_DOC
85-
/// is_doc19 IS_DOC_START
86-
/** Aaa
106+
/** IS_DOC_START priorBlockSingleLineComment Aaa. */
107+
/**
108+
Bbb. IS_DOC_END
109+
*/
110+
func priorBlockSingleLineComment() {}
111+
112+
/** IS_DOC_START priorBlockMixedComment Aaa. */
113+
/// Bbb.
114+
/// Multiline. IS_DOC_END
115+
func priorBlockMixedComment() {}
116+
117+
/// IS_DOC_START priorLineMixedComment Aaa.
118+
/// Multiline.
119+
/** Bbb. IS_DOC_END */
120+
func priorLineMixedComment() {}
121+
122+
/// IS_DOC_START priorSingleLineMixedComment Aaa.
123+
/**
124+
Bbb. IS_DOC_END
125+
*/
126+
func priorSingleLineMixedComment() {}
127+
128+
/**
129+
IS_DOC_START priorBlockMultiLineMixedComment Aaa.
130+
*/
131+
/// Bbb. IS_DOC_END
132+
func priorBlockMultiLineMixedComment() {}
133+
134+
// Aaa. NOT_DOC
135+
/// allTheThings IS_DOC_NOT_ATTACHED
136+
/** Bbb IS_DOC_NOT_ATTACHED
87137
*
88-
* Bbb */
89-
/// Ccc
90-
/** Ddd
91-
* Eee IS_DOC_END
138+
* Ccc. */
139+
// Ddd. NOT_DOC
140+
/// IS_DOC_START Eee
141+
/**
142+
* Fff. IS_DOC_END
92143
*/
93-
func is_doc19() {}
144+
func allTheThings() {}
94145

95146
// RUN: %target-swift-ide-test -print-comments -source-filename %s > %t.txt
96147
// RUN: %FileCheck %s -check-prefix=WRONG < %t.txt
@@ -121,8 +172,13 @@ func is_doc19() {}
121172
// CHECK-NEXT: comment_merge.swift:59:6: Func/not_doc13 RawComment=none
122173
// CHECK-NEXT: comment_merge.swift:63:6: Func/is_doc14 RawComment=[/// is_doc14 IS_DOC_START\n/// IS_DOC_END\n]
123174
// CHECK-NEXT: comment_merge.swift:68:6: Func/is_doc15 RawComment=[/// is_doc15 IS_DOC_START\n/// Aaa bbb ccc.\n/// IS_DOC_END\n]
124-
// CHECK-NEXT: comment_merge.swift:73:6: Func/is_doc16 RawComment=[/// is_doc16 IS_DOC_START\n/** Aaa bbb ccc. *//// IS_DOC_END\n]
125-
// CHECK-NEXT: comment_merge.swift:77:6: Func/is_doc17 RawComment=[/** Aaa is_doc17 IS_DOC_START *//** bbb *//// IS_DOC_END\n]
126-
// CHECK-NEXT: comment_merge.swift:82:6: Func/is_doc18 RawComment=[/// is_doc18 IS_DOC_START IS_DOC_END\n]
127-
// CHECK-NEXT: comment_merge.swift:93:6: Func/is_doc19 RawComment=[/// is_doc19 IS_DOC_START\n/** Aaa\n *\n * Bbb *//// Ccc\n/** Ddd\n * Eee IS_DOC_END\n */]
128-
175+
// CHECK-NEXT: comment_merge.swift:77:6: Func/priorCommentOneLineGap RawComment=[/// IS_DOC_START priorCommentOneLineGap Aaa.\n///\n/// Bbb. IS_DOC_END\n]
176+
// CHECK-NEXT: comment_merge.swift:86:6: Func/priorCommentTwoLineGap RawComment=[/// IS_DOC_START priorCommentTwoLineGap Aaa.\n///\n/// Bbb. IS_DOC_END\n]
177+
// CHECK-NEXT: comment_merge.swift:96:6: Func/priorCommentGyb RawComment=[/// IS_DOC_START priorCommentGyb Aaa.\n///\n/// Bbb. IS_DOC_END\n]
178+
// CHECK-NEXT: comment_merge.swift:104:6: Func/priorBlockMultiLineComment RawComment=[/**\n IS_DOC_START priorBlockMultiLineComment Aaa.\n *//**\n Bbb. IS_DOC_END\n */]
179+
// CHECK-NEXT: comment_merge.swift:110:6: Func/priorBlockSingleLineComment RawComment=[/** IS_DOC_START priorBlockSingleLineComment Aaa. *//**\n Bbb. IS_DOC_END\n */]
180+
// CHECK-NEXT: comment_merge.swift:115:6: Func/priorBlockMixedComment RawComment=[/** IS_DOC_START priorBlockMixedComment Aaa. *//// Bbb.\n/// Multiline. IS_DOC_END\n]
181+
// CHECK-NEXT: comment_merge.swift:120:6: Func/priorLineMixedComment RawComment=[/// IS_DOC_START priorLineMixedComment Aaa.\n/// Multiline.\n/** Bbb. IS_DOC_END */]
182+
// CHECK-NEXT: comment_merge.swift:126:6: Func/priorSingleLineMixedComment RawComment=[/// IS_DOC_START priorSingleLineMixedComment Aaa.\n/**\n Bbb. IS_DOC_END\n */]
183+
// CHECK-NEXT: comment_merge.swift:132:6: Func/priorBlockMultiLineMixedComment RawComment=[/**\n IS_DOC_START priorBlockMultiLineMixedComment Aaa.\n *//// Bbb. IS_DOC_END\n]
184+
// CHECK-NEXT: comment_merge.swift:144:6: Func/allTheThings RawComment=[/// IS_DOC_START Eee\n/**\n * Fff. IS_DOC_END\n */]

0 commit comments

Comments
 (0)