From dc5ac081bccbe66a41dfb5afb0668665826bac39 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Tue, 9 Apr 2019 11:12:44 -0700 Subject: [PATCH 1/5] First draft Solves the initial problem but breaks commentCommentParsing. I also found a couple more interesting cases. --- src/compiler/parser.ts | 14 ++- .../quickInfoForJSDocUnknownTag.baseline | 112 ++++++++++++++++++ .../fourslash/quickInfoForJSDocUnknownTag.ts | 22 ++++ 3 files changed, 143 insertions(+), 5 deletions(-) create mode 100644 tests/baselines/reference/quickInfoForJSDocUnknownTag.baseline create mode 100644 tests/cases/fourslash/quickInfoForJSDocUnknownTag.ts diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index 2757c15c5241d..5ba3ad728af00 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -6536,16 +6536,19 @@ namespace ts { } } - function skipWhitespaceOrAsterisk(): void { + function skipWhitespaceOrAsterisk(): boolean { if (token() === SyntaxKind.WhitespaceTrivia || token() === SyntaxKind.NewLineTrivia) { if (lookAhead(isNextNonwhitespaceTokenEndOfFile)) { - return; // Don't skip whitespace prior to EoF (or end of comment) - that shouldn't be included in any node's range + return false; // Don't skip whitespace prior to EoF (or end of comment) - that shouldn't be included in any node's range } } let precedingLineBreak = scanner.hasPrecedingLineBreak(); + let resetIndent = false; while ((precedingLineBreak && token() === SyntaxKind.AsteriskToken) || token() === SyntaxKind.WhitespaceTrivia || token() === SyntaxKind.NewLineTrivia) { if (token() === SyntaxKind.NewLineTrivia) { + // TODO: Flip a bit here + resetIndent = true precedingLineBreak = true; } else if (token() === SyntaxKind.AsteriskToken) { @@ -6553,6 +6556,7 @@ namespace ts { } nextJSDocToken(); } + return resetIndent; } function parseTag(indent: number) { @@ -6561,7 +6565,7 @@ namespace ts { nextJSDocToken(); const tagName = parseJSDocIdentifierName(/*message*/ undefined); - skipWhitespaceOrAsterisk(); + const skippedNewline = skipWhitespaceOrAsterisk(); let tag: JSDocTag | undefined; switch (tagName.escapedText) { @@ -6606,7 +6610,7 @@ namespace ts { if (!tag.comment) { // some tags, like typedef and callback, have already parsed their comments earlier - tag.comment = parseTagComments(indent + tag.end - tag.pos); + tag.comment = parseTagComments(indent + (!!skippedNewline ? 0 : tag.end - tag.pos)); } return tag; } @@ -6646,7 +6650,7 @@ namespace ts { const whitespace = scanner.getTokenText(); // if the whitespace crosses the margin, take only the whitespace that passes the margin if (margin !== undefined && indent + whitespace.length > margin) { - comments.push(whitespace.slice(margin - indent - 1)); + comments.push(whitespace.slice(margin - indent)); } indent += whitespace.length; } diff --git a/tests/baselines/reference/quickInfoForJSDocUnknownTag.baseline b/tests/baselines/reference/quickInfoForJSDocUnknownTag.baseline new file mode 100644 index 0000000000000..a91d834593e2d --- /dev/null +++ b/tests/baselines/reference/quickInfoForJSDocUnknownTag.baseline @@ -0,0 +1,112 @@ +[ + { + "marker": { + "fileName": "/tests/cases/fourslash/quickInfoForJSDocUnknownTag.ts", + "position": 64 + }, + "quickInfo": { + "kind": "function", + "kindModifiers": "", + "textSpan": { + "start": 62, + "length": 3 + }, + "displayParts": [ + { + "text": "function", + "kind": "keyword" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "foo", + "kind": "functionName" + }, + { + "text": "(", + "kind": "punctuation" + }, + { + "text": ")", + "kind": "punctuation" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "string", + "kind": "keyword" + } + ], + "documentation": [], + "tags": [ + { + "name": "example", + "text": "if (true) {\n foo()\n}" + } + ] + } + }, + { + "marker": { + "fileName": "/tests/cases/fourslash/quickInfoForJSDocUnknownTag.ts", + "position": 134 + }, + "quickInfo": { + "kind": "function", + "kindModifiers": "", + "textSpan": { + "start": 132, + "length": 4 + }, + "displayParts": [ + { + "text": "function", + "kind": "keyword" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "foo2", + "kind": "functionName" + }, + { + "text": "(", + "kind": "punctuation" + }, + { + "text": ")", + "kind": "punctuation" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "string", + "kind": "keyword" + } + ], + "documentation": [], + "tags": [ + { + "name": "example", + "text": "{\n foo()\n}" + } + ] + } + } +] \ No newline at end of file diff --git a/tests/cases/fourslash/quickInfoForJSDocUnknownTag.ts b/tests/cases/fourslash/quickInfoForJSDocUnknownTag.ts new file mode 100644 index 0000000000000..68d52f8878ecf --- /dev/null +++ b/tests/cases/fourslash/quickInfoForJSDocUnknownTag.ts @@ -0,0 +1,22 @@ +/// + +/////** +//// * @example +//// * if (true) { +//// * foo() +//// * } +//// */ +////function fo/*1*/o() { +//// return '2'; +////} +/////** +//// @example +//// { +//// foo() +//// } +//// */ +////function fo/*2*/o2() { +//// return '2'; +////} + +verify.baselineQuickInfo(); From 80f1e426d3d2a13bf2c662ebc5dd8affc498951e Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Tue, 9 Apr 2019 14:48:52 -0700 Subject: [PATCH 2/5] Add more tests and fix their bugs --- src/compiler/parser.ts | 41 +++++-- .../quickInfoForJSDocUnknownTag.baseline | 113 ++++++++++++++++++ .../fourslash/quickInfoForJSDocUnknownTag.ts | 20 +++- 3 files changed, 163 insertions(+), 11 deletions(-) diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index 5ba3ad728af00..b98930e1964f5 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -6441,7 +6441,7 @@ namespace ts { // for malformed examples like `/** @param {string} x @returns {number} the length */` state = JSDocState.BeginningOfLine; margin = undefined; - indent++; + // indent++; } else { pushComment(scanner.getTokenText()); @@ -6536,27 +6536,37 @@ namespace ts { } } - function skipWhitespaceOrAsterisk(): boolean { + function skipWhitespaceOrAsterisk(stop?: boolean): number { if (token() === SyntaxKind.WhitespaceTrivia || token() === SyntaxKind.NewLineTrivia) { if (lookAhead(isNextNonwhitespaceTokenEndOfFile)) { - return false; // Don't skip whitespace prior to EoF (or end of comment) - that shouldn't be included in any node's range + return 0; // Don't skip whitespace prior to EoF (or end of comment) - that shouldn't be included in any node's range } } let precedingLineBreak = scanner.hasPrecedingLineBreak(); - let resetIndent = false; + let seenLineBreak = false; + let indent = 0; + // if you HAVE seen a newline (resetIndent is true), then don't read whitespace trivia after a newline-whitespace-asterisk sequence. Just stop. + // UNLESS (ugh) there is only whitespace-newline after it, in which case, fine. + // so this needs lookahead SOME HOW. while ((precedingLineBreak && token() === SyntaxKind.AsteriskToken) || token() === SyntaxKind.WhitespaceTrivia || token() === SyntaxKind.NewLineTrivia) { if (token() === SyntaxKind.NewLineTrivia) { - // TODO: Flip a bit here - resetIndent = true + scanner.getTokenText() + seenLineBreak = true; + indent = 0; precedingLineBreak = true; } else if (token() === SyntaxKind.AsteriskToken) { precedingLineBreak = false; + indent++; + } + else { + Debug.assert(token() === SyntaxKind.WhitespaceTrivia); + indent += scanner.getTokenText().length; } nextJSDocToken(); } - return resetIndent; + return stop && seenLineBreak ? indent : 0; } function parseTag(indent: number) { @@ -6565,7 +6575,7 @@ namespace ts { nextJSDocToken(); const tagName = parseJSDocIdentifierName(/*message*/ undefined); - const skippedNewline = skipWhitespaceOrAsterisk(); + const margin = skipWhitespaceOrAsterisk(/*stop*/ true); let tag: JSDocTag | undefined; switch (tagName.escapedText) { @@ -6610,12 +6620,12 @@ namespace ts { if (!tag.comment) { // some tags, like typedef and callback, have already parsed their comments earlier - tag.comment = parseTagComments(indent + (!!skippedNewline ? 0 : tag.end - tag.pos)); + tag.comment = parseTagComments(indent + (margin !== 0 ? 0 : tag.end - tag.pos), margin); } return tag; } - function parseTagComments(indent: number): string | undefined { + function parseTagComments(indent: number, initialMargin?: number): string | undefined { const comments: string[] = []; let state = JSDocState.BeginningOfLine; let margin: number | undefined; @@ -6626,6 +6636,17 @@ namespace ts { comments.push(text); indent += text.length; } + if (initialMargin) { + // change state and push a comment + let initialWhitespace = ""; + for (var i = 0; i < initialMargin - indent; i++) { + initialWhitespace += " "; + } + if (initialWhitespace) { + pushComment(initialWhitespace); + state = JSDocState.SavingComments; + } + } let tok = token() as JsDocSyntaxKind; loop: while (true) { switch (tok) { diff --git a/tests/baselines/reference/quickInfoForJSDocUnknownTag.baseline b/tests/baselines/reference/quickInfoForJSDocUnknownTag.baseline index a91d834593e2d..c12c0e8311abc 100644 --- a/tests/baselines/reference/quickInfoForJSDocUnknownTag.baseline +++ b/tests/baselines/reference/quickInfoForJSDocUnknownTag.baseline @@ -108,5 +108,118 @@ } ] } + }, + { + "marker": { + "fileName": "/tests/cases/fourslash/quickInfoForJSDocUnknownTag.ts", + "position": 219 + }, + "quickInfo": { + "kind": "function", + "kindModifiers": "", + "textSpan": { + "start": 218, + "length": 3 + }, + "displayParts": [ + { + "text": "function", + "kind": "keyword" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "moo", + "kind": "functionName" + }, + { + "text": "(", + "kind": "punctuation" + }, + { + "text": ")", + "kind": "punctuation" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "string", + "kind": "keyword" + } + ], + "documentation": [], + "tags": [ + { + "name": "example", + "text": " x y\n 12345\n b" + } + ] + } + }, + { + "marker": { + "fileName": "/tests/cases/fourslash/quickInfoForJSDocUnknownTag.ts", + "position": 313 + }, + "quickInfo": { + "kind": "function", + "kindModifiers": "", + "textSpan": { + "start": 312, + "length": 3 + }, + "displayParts": [ + { + "text": "function", + "kind": "keyword" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "boo", + "kind": "functionName" + }, + { + "text": "(", + "kind": "punctuation" + }, + { + "text": ")", + "kind": "punctuation" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "string", + "kind": "keyword" + } + ], + "documentation": [], + "tags": [ + { + "name": "func" + }, + { + "name": "example", + "text": " x y\n 12345\n b" + } + ] + } } ] \ No newline at end of file diff --git a/tests/cases/fourslash/quickInfoForJSDocUnknownTag.ts b/tests/cases/fourslash/quickInfoForJSDocUnknownTag.ts index 68d52f8878ecf..772a36ef3462b 100644 --- a/tests/cases/fourslash/quickInfoForJSDocUnknownTag.ts +++ b/tests/cases/fourslash/quickInfoForJSDocUnknownTag.ts @@ -18,5 +18,23 @@ ////function fo/*2*/o2() { //// return '2'; ////} - +/////** +//// * @example +//// * x y +//// * 12345 +//// * b +//// */ +////function m/*3*/oo() { +//// return '2'; +////} +/////** +//// * @func +//// * @example +//// * x y +//// * 12345 +//// * b +//// */ +////function b/*4*/oo() { +//// return '2'; +////} verify.baselineQuickInfo(); From 8ba79c18e3629b73b8fe615dd5779ad8971d31ba Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Tue, 9 Apr 2019 14:51:06 -0700 Subject: [PATCH 3/5] Another test case --- tests/cases/fourslash/quickInfoForJSDocUnknownTag.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/cases/fourslash/quickInfoForJSDocUnknownTag.ts b/tests/cases/fourslash/quickInfoForJSDocUnknownTag.ts index 772a36ef3462b..57597898373cd 100644 --- a/tests/cases/fourslash/quickInfoForJSDocUnknownTag.ts +++ b/tests/cases/fourslash/quickInfoForJSDocUnknownTag.ts @@ -37,4 +37,13 @@ ////function b/*4*/oo() { //// return '2'; ////} +/////** +//// * @func +//// * @example x y +//// * 12345 +//// * b +//// */ +////function go/*5*/o() { +//// return '2'; +////} verify.baselineQuickInfo(); From 59f82ebd7a9a9a96bd835c74cc786d4dac850233 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Tue, 9 Apr 2019 16:09:10 -0700 Subject: [PATCH 4/5] Some cleanup I may try do a little more; `margin += tag.end - tag.pos` bothers me a bit. --- src/compiler/parser.ts | 23 ++++---- .../quickInfoForJSDocUnknownTag.baseline | 58 +++++++++++++++++++ 2 files changed, 69 insertions(+), 12 deletions(-) diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index b98930e1964f5..29fdef08af1b9 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -6441,7 +6441,6 @@ namespace ts { // for malformed examples like `/** @param {string} x @returns {number} the length */` state = JSDocState.BeginningOfLine; margin = undefined; - // indent++; } else { pushComment(scanner.getTokenText()); @@ -6536,7 +6535,7 @@ namespace ts { } } - function skipWhitespaceOrAsterisk(stop?: boolean): number { + function skipWhitespaceOrAsterisk(): number { if (token() === SyntaxKind.WhitespaceTrivia || token() === SyntaxKind.NewLineTrivia) { if (lookAhead(isNextNonwhitespaceTokenEndOfFile)) { return 0; // Don't skip whitespace prior to EoF (or end of comment) - that shouldn't be included in any node's range @@ -6546,9 +6545,6 @@ namespace ts { let precedingLineBreak = scanner.hasPrecedingLineBreak(); let seenLineBreak = false; let indent = 0; - // if you HAVE seen a newline (resetIndent is true), then don't read whitespace trivia after a newline-whitespace-asterisk sequence. Just stop. - // UNLESS (ugh) there is only whitespace-newline after it, in which case, fine. - // so this needs lookahead SOME HOW. while ((precedingLineBreak && token() === SyntaxKind.AsteriskToken) || token() === SyntaxKind.WhitespaceTrivia || token() === SyntaxKind.NewLineTrivia) { if (token() === SyntaxKind.NewLineTrivia) { scanner.getTokenText() @@ -6566,16 +6562,16 @@ namespace ts { } nextJSDocToken(); } - return stop && seenLineBreak ? indent : 0; + return seenLineBreak ? indent : 0; } - function parseTag(indent: number) { + function parseTag(margin: number) { Debug.assert(token() === SyntaxKind.AtToken); const start = scanner.getTokenPos(); nextJSDocToken(); const tagName = parseJSDocIdentifierName(/*message*/ undefined); - const margin = skipWhitespaceOrAsterisk(/*stop*/ true); + const indent = skipWhitespaceOrAsterisk(); let tag: JSDocTag | undefined; switch (tagName.escapedText) { @@ -6596,7 +6592,7 @@ namespace ts { case "arg": case "argument": case "param": - return parseParameterOrPropertyTag(start, tagName, PropertyLikeParse.Parameter, indent); + return parseParameterOrPropertyTag(start, tagName, PropertyLikeParse.Parameter, margin); case "return": case "returns": tag = parseReturnTag(start, tagName); @@ -6608,10 +6604,10 @@ namespace ts { tag = parseTypeTag(start, tagName); break; case "typedef": - tag = parseTypedefTag(start, tagName, indent); + tag = parseTypedefTag(start, tagName, margin); break; case "callback": - tag = parseCallbackTag(start, tagName, indent); + tag = parseCallbackTag(start, tagName, margin); break; default: tag = parseUnknownTag(start, tagName); @@ -6620,7 +6616,10 @@ namespace ts { if (!tag.comment) { // some tags, like typedef and callback, have already parsed their comments earlier - tag.comment = parseTagComments(indent + (margin !== 0 ? 0 : tag.end - tag.pos), margin); + if (!indent) { + margin += tag.end - tag.pos; + } + tag.comment = parseTagComments(margin, indent); } return tag; } diff --git a/tests/baselines/reference/quickInfoForJSDocUnknownTag.baseline b/tests/baselines/reference/quickInfoForJSDocUnknownTag.baseline index c12c0e8311abc..25b2f8fe9c0fa 100644 --- a/tests/baselines/reference/quickInfoForJSDocUnknownTag.baseline +++ b/tests/baselines/reference/quickInfoForJSDocUnknownTag.baseline @@ -221,5 +221,63 @@ } ] } + }, + { + "marker": { + "fileName": "/tests/cases/fourslash/quickInfoForJSDocUnknownTag.ts", + "position": 426 + }, + "quickInfo": { + "kind": "function", + "kindModifiers": "", + "textSpan": { + "start": 424, + "length": 3 + }, + "displayParts": [ + { + "text": "function", + "kind": "keyword" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "goo", + "kind": "functionName" + }, + { + "text": "(", + "kind": "punctuation" + }, + { + "text": ")", + "kind": "punctuation" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "string", + "kind": "keyword" + } + ], + "documentation": [], + "tags": [ + { + "name": "func" + }, + { + "name": "example", + "text": "x y\n12345\n b" + } + ] + } } ] \ No newline at end of file From 45c54e54c686855f3699474edc83f2f385587b4c Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Tue, 9 Apr 2019 16:21:59 -0700 Subject: [PATCH 5/5] More cleanup --- src/compiler/parser.ts | 39 ++++++++++++++------------------------- 1 file changed, 14 insertions(+), 25 deletions(-) diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index 29fdef08af1b9..8eaa005b62e3d 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -6535,34 +6535,29 @@ namespace ts { } } - function skipWhitespaceOrAsterisk(): number { + function skipWhitespaceOrAsterisk(): string { if (token() === SyntaxKind.WhitespaceTrivia || token() === SyntaxKind.NewLineTrivia) { if (lookAhead(isNextNonwhitespaceTokenEndOfFile)) { - return 0; // Don't skip whitespace prior to EoF (or end of comment) - that shouldn't be included in any node's range + return ""; // Don't skip whitespace prior to EoF (or end of comment) - that shouldn't be included in any node's range } } let precedingLineBreak = scanner.hasPrecedingLineBreak(); let seenLineBreak = false; - let indent = 0; + let indentText = ""; while ((precedingLineBreak && token() === SyntaxKind.AsteriskToken) || token() === SyntaxKind.WhitespaceTrivia || token() === SyntaxKind.NewLineTrivia) { + indentText += scanner.getTokenText(); if (token() === SyntaxKind.NewLineTrivia) { - scanner.getTokenText() - seenLineBreak = true; - indent = 0; precedingLineBreak = true; + seenLineBreak = true; + indentText = ""; } else if (token() === SyntaxKind.AsteriskToken) { precedingLineBreak = false; - indent++; - } - else { - Debug.assert(token() === SyntaxKind.WhitespaceTrivia); - indent += scanner.getTokenText().length; } nextJSDocToken(); } - return seenLineBreak ? indent : 0; + return seenLineBreak ? indentText : ""; } function parseTag(margin: number) { @@ -6571,7 +6566,7 @@ namespace ts { nextJSDocToken(); const tagName = parseJSDocIdentifierName(/*message*/ undefined); - const indent = skipWhitespaceOrAsterisk(); + const indentText = skipWhitespaceOrAsterisk(); let tag: JSDocTag | undefined; switch (tagName.escapedText) { @@ -6616,15 +6611,15 @@ namespace ts { if (!tag.comment) { // some tags, like typedef and callback, have already parsed their comments earlier - if (!indent) { + if (!indentText) { margin += tag.end - tag.pos; } - tag.comment = parseTagComments(margin, indent); + tag.comment = parseTagComments(margin, indentText.slice(margin)); } return tag; } - function parseTagComments(indent: number, initialMargin?: number): string | undefined { + function parseTagComments(indent: number, initialMargin?: string): string | undefined { const comments: string[] = []; let state = JSDocState.BeginningOfLine; let margin: number | undefined; @@ -6636,15 +6631,9 @@ namespace ts { indent += text.length; } if (initialMargin) { - // change state and push a comment - let initialWhitespace = ""; - for (var i = 0; i < initialMargin - indent; i++) { - initialWhitespace += " "; - } - if (initialWhitespace) { - pushComment(initialWhitespace); - state = JSDocState.SavingComments; - } + // jump straight to saving comments if there is some initial indentation + pushComment(initialMargin); + state = JSDocState.SavingComments; } let tok = token() as JsDocSyntaxKind; loop: while (true) {