Skip to content

Commit ff95909

Browse files
sandersnsandersn
authored
Fix some bad jsdoc comment indent (#30838)
* First draft Solves the initial problem but breaks commentCommentParsing. I also found a couple more interesting cases. * Add more tests and fix their bugs * Another test case * Some cleanup I may try do a little more; `margin += tag.end - tag.pos` bothers me a bit. * More cleanup
1 parent ef4acc8 commit ff95909

File tree

3 files changed

+356
-11
lines changed

3 files changed

+356
-11
lines changed

‎src/compiler/parser.ts

+24-11
Original file line numberDiff line numberDiff line change
@@ -6441,7 +6441,6 @@ namespace ts {
64416441
// for malformed examples like `/** @param {string} x @returns {number} the length */`
64426442
state = JSDocState.BeginningOfLine;
64436443
margin = undefined;
6444-
indent++;
64456444
}
64466445
else {
64476446
pushComment(scanner.getTokenText());
@@ -6536,32 +6535,38 @@ namespace ts {
65366535
}
65376536
}
65386537

6539-
function skipWhitespaceOrAsterisk(): void {
6538+
function skipWhitespaceOrAsterisk(): string {
65406539
if (token() === SyntaxKind.WhitespaceTrivia || token() === SyntaxKind.NewLineTrivia) {
65416540
if (lookAhead(isNextNonwhitespaceTokenEndOfFile)) {
6542-
return; // Don't skip whitespace prior to EoF (or end of comment) - that shouldn't be included in any node's range
6541+
return ""; // Don't skip whitespace prior to EoF (or end of comment) - that shouldn't be included in any node's range
65436542
}
65446543
}
65456544

65466545
let precedingLineBreak = scanner.hasPrecedingLineBreak();
6546+
let seenLineBreak = false;
6547+
let indentText = "";
65476548
while ((precedingLineBreak && token() === SyntaxKind.AsteriskToken) || token() === SyntaxKind.WhitespaceTrivia || token() === SyntaxKind.NewLineTrivia) {
6549+
indentText += scanner.getTokenText();
65486550
if (token() === SyntaxKind.NewLineTrivia) {
65496551
precedingLineBreak = true;
6552+
seenLineBreak = true;
6553+
indentText = "";
65506554
}
65516555
else if (token() === SyntaxKind.AsteriskToken) {
65526556
precedingLineBreak = false;
65536557
}
65546558
nextJSDocToken();
65556559
}
6560+
return seenLineBreak ? indentText : "";
65566561
}
65576562

6558-
function parseTag(indent: number) {
6563+
function parseTag(margin: number) {
65596564
Debug.assert(token() === SyntaxKind.AtToken);
65606565
const start = scanner.getTokenPos();
65616566
nextJSDocToken();
65626567

65636568
const tagName = parseJSDocIdentifierName(/*message*/ undefined);
6564-
skipWhitespaceOrAsterisk();
6569+
const indentText = skipWhitespaceOrAsterisk();
65656570

65666571
let tag: JSDocTag | undefined;
65676572
switch (tagName.escapedText) {
@@ -6582,7 +6587,7 @@ namespace ts {
65826587
case "arg":
65836588
case "argument":
65846589
case "param":
6585-
return parseParameterOrPropertyTag(start, tagName, PropertyLikeParse.Parameter, indent);
6590+
return parseParameterOrPropertyTag(start, tagName, PropertyLikeParse.Parameter, margin);
65866591
case "return":
65876592
case "returns":
65886593
tag = parseReturnTag(start, tagName);
@@ -6594,10 +6599,10 @@ namespace ts {
65946599
tag = parseTypeTag(start, tagName);
65956600
break;
65966601
case "typedef":
6597-
tag = parseTypedefTag(start, tagName, indent);
6602+
tag = parseTypedefTag(start, tagName, margin);
65986603
break;
65996604
case "callback":
6600-
tag = parseCallbackTag(start, tagName, indent);
6605+
tag = parseCallbackTag(start, tagName, margin);
66016606
break;
66026607
default:
66036608
tag = parseUnknownTag(start, tagName);
@@ -6606,12 +6611,15 @@ namespace ts {
66066611

66076612
if (!tag.comment) {
66086613
// some tags, like typedef and callback, have already parsed their comments earlier
6609-
tag.comment = parseTagComments(indent + tag.end - tag.pos);
6614+
if (!indentText) {
6615+
margin += tag.end - tag.pos;
6616+
}
6617+
tag.comment = parseTagComments(margin, indentText.slice(margin));
66106618
}
66116619
return tag;
66126620
}
66136621

6614-
function parseTagComments(indent: number): string | undefined {
6622+
function parseTagComments(indent: number, initialMargin?: string): string | undefined {
66156623
const comments: string[] = [];
66166624
let state = JSDocState.BeginningOfLine;
66176625
let margin: number | undefined;
@@ -6622,6 +6630,11 @@ namespace ts {
66226630
comments.push(text);
66236631
indent += text.length;
66246632
}
6633+
if (initialMargin) {
6634+
// jump straight to saving comments if there is some initial indentation
6635+
pushComment(initialMargin);
6636+
state = JSDocState.SavingComments;
6637+
}
66256638
let tok = token() as JsDocSyntaxKind;
66266639
loop: while (true) {
66276640
switch (tok) {
@@ -6646,7 +6659,7 @@ namespace ts {
66466659
const whitespace = scanner.getTokenText();
66476660
// if the whitespace crosses the margin, take only the whitespace that passes the margin
66486661
if (margin !== undefined && indent + whitespace.length > margin) {
6649-
comments.push(whitespace.slice(margin - indent - 1));
6662+
comments.push(whitespace.slice(margin - indent));
66506663
}
66516664
indent += whitespace.length;
66526665
}

‎tests/baselines/reference/quickInfoForJSDocUnknownTag.baseline

+283
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,283 @@
1+
[
2+
{
3+
"marker": {
4+
"fileName": "/tests/cases/fourslash/quickInfoForJSDocUnknownTag.ts",
5+
"position": 64
6+
},
7+
"quickInfo": {
8+
"kind": "function",
9+
"kindModifiers": "",
10+
"textSpan": {
11+
"start": 62,
12+
"length": 3
13+
},
14+
"displayParts": [
15+
{
16+
"text": "function",
17+
"kind": "keyword"
18+
},
19+
{
20+
"text": " ",
21+
"kind": "space"
22+
},
23+
{
24+
"text": "foo",
25+
"kind": "functionName"
26+
},
27+
{
28+
"text": "(",
29+
"kind": "punctuation"
30+
},
31+
{
32+
"text": ")",
33+
"kind": "punctuation"
34+
},
35+
{
36+
"text": ":",
37+
"kind": "punctuation"
38+
},
39+
{
40+
"text": " ",
41+
"kind": "space"
42+
},
43+
{
44+
"text": "string",
45+
"kind": "keyword"
46+
}
47+
],
48+
"documentation": [],
49+
"tags": [
50+
{
51+
"name": "example",
52+
"text": "if (true) {\n foo()\n}"
53+
}
54+
]
55+
}
56+
},
57+
{
58+
"marker": {
59+
"fileName": "/tests/cases/fourslash/quickInfoForJSDocUnknownTag.ts",
60+
"position": 134
61+
},
62+
"quickInfo": {
63+
"kind": "function",
64+
"kindModifiers": "",
65+
"textSpan": {
66+
"start": 132,
67+
"length": 4
68+
},
69+
"displayParts": [
70+
{
71+
"text": "function",
72+
"kind": "keyword"
73+
},
74+
{
75+
"text": " ",
76+
"kind": "space"
77+
},
78+
{
79+
"text": "foo2",
80+
"kind": "functionName"
81+
},
82+
{
83+
"text": "(",
84+
"kind": "punctuation"
85+
},
86+
{
87+
"text": ")",
88+
"kind": "punctuation"
89+
},
90+
{
91+
"text": ":",
92+
"kind": "punctuation"
93+
},
94+
{
95+
"text": " ",
96+
"kind": "space"
97+
},
98+
{
99+
"text": "string",
100+
"kind": "keyword"
101+
}
102+
],
103+
"documentation": [],
104+
"tags": [
105+
{
106+
"name": "example",
107+
"text": "{\n foo()\n}"
108+
}
109+
]
110+
}
111+
},
112+
{
113+
"marker": {
114+
"fileName": "/tests/cases/fourslash/quickInfoForJSDocUnknownTag.ts",
115+
"position": 219
116+
},
117+
"quickInfo": {
118+
"kind": "function",
119+
"kindModifiers": "",
120+
"textSpan": {
121+
"start": 218,
122+
"length": 3
123+
},
124+
"displayParts": [
125+
{
126+
"text": "function",
127+
"kind": "keyword"
128+
},
129+
{
130+
"text": " ",
131+
"kind": "space"
132+
},
133+
{
134+
"text": "moo",
135+
"kind": "functionName"
136+
},
137+
{
138+
"text": "(",
139+
"kind": "punctuation"
140+
},
141+
{
142+
"text": ")",
143+
"kind": "punctuation"
144+
},
145+
{
146+
"text": ":",
147+
"kind": "punctuation"
148+
},
149+
{
150+
"text": " ",
151+
"kind": "space"
152+
},
153+
{
154+
"text": "string",
155+
"kind": "keyword"
156+
}
157+
],
158+
"documentation": [],
159+
"tags": [
160+
{
161+
"name": "example",
162+
"text": " x y\n 12345\n b"
163+
}
164+
]
165+
}
166+
},
167+
{
168+
"marker": {
169+
"fileName": "/tests/cases/fourslash/quickInfoForJSDocUnknownTag.ts",
170+
"position": 313
171+
},
172+
"quickInfo": {
173+
"kind": "function",
174+
"kindModifiers": "",
175+
"textSpan": {
176+
"start": 312,
177+
"length": 3
178+
},
179+
"displayParts": [
180+
{
181+
"text": "function",
182+
"kind": "keyword"
183+
},
184+
{
185+
"text": " ",
186+
"kind": "space"
187+
},
188+
{
189+
"text": "boo",
190+
"kind": "functionName"
191+
},
192+
{
193+
"text": "(",
194+
"kind": "punctuation"
195+
},
196+
{
197+
"text": ")",
198+
"kind": "punctuation"
199+
},
200+
{
201+
"text": ":",
202+
"kind": "punctuation"
203+
},
204+
{
205+
"text": " ",
206+
"kind": "space"
207+
},
208+
{
209+
"text": "string",
210+
"kind": "keyword"
211+
}
212+
],
213+
"documentation": [],
214+
"tags": [
215+
{
216+
"name": "func"
217+
},
218+
{
219+
"name": "example",
220+
"text": " x y\n 12345\n b"
221+
}
222+
]
223+
}
224+
},
225+
{
226+
"marker": {
227+
"fileName": "/tests/cases/fourslash/quickInfoForJSDocUnknownTag.ts",
228+
"position": 426
229+
},
230+
"quickInfo": {
231+
"kind": "function",
232+
"kindModifiers": "",
233+
"textSpan": {
234+
"start": 424,
235+
"length": 3
236+
},
237+
"displayParts": [
238+
{
239+
"text": "function",
240+
"kind": "keyword"
241+
},
242+
{
243+
"text": " ",
244+
"kind": "space"
245+
},
246+
{
247+
"text": "goo",
248+
"kind": "functionName"
249+
},
250+
{
251+
"text": "(",
252+
"kind": "punctuation"
253+
},
254+
{
255+
"text": ")",
256+
"kind": "punctuation"
257+
},
258+
{
259+
"text": ":",
260+
"kind": "punctuation"
261+
},
262+
{
263+
"text": " ",
264+
"kind": "space"
265+
},
266+
{
267+
"text": "string",
268+
"kind": "keyword"
269+
}
270+
],
271+
"documentation": [],
272+
"tags": [
273+
{
274+
"name": "func"
275+
},
276+
{
277+
"name": "example",
278+
"text": "x y\n12345\n b"
279+
}
280+
]
281+
}
282+
}
283+
]

0 commit comments

Comments
 (0)