From d6af9b7cbc48a21d2e52073cd38a6fedd8189904 Mon Sep 17 00:00:00 2001 From: Neonit Date: Fri, 10 Apr 2020 17:46:08 +0200 Subject: [PATCH] Fix indentation preservation in JSDoc (#37717) This fixes two bugs in the parseJSDocCommentWorker(). 1. The initial indent was calculated wrongly. It was set to the difference between the index of the last newline or beginning of file and the current start marker (position of /**). By calculating it this way, the newline character itself is counted as indentation character as well. The initial indent is used as margin for the whole comment. The margin contains the amount of characters to skip before the actual content or payload of a comment line. The algorithm does not skip non-whitespace characters at the beginning of the content, but it would strip away one whitespace character for indented content (which does matter, if there is e.g. a Markdown code block with indentation in the comment). 2. When reducing initial whitespace sequences of comment lines by the remaining margin the algorithm cut off one character too much. This might have been introduced to fix 1. It had a similar effect as 1. --- src/compiler/parser.ts | 5 +++-- .../fourslash/jsDocIndentationPreservation1.ts | 13 +++++++++++++ .../fourslash/jsDocIndentationPreservation2.ts | 13 +++++++++++++ .../fourslash/jsDocIndentationPreservation3.ts | 13 +++++++++++++ 4 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 tests/cases/fourslash/jsDocIndentationPreservation1.ts create mode 100644 tests/cases/fourslash/jsDocIndentationPreservation2.ts create mode 100644 tests/cases/fourslash/jsDocIndentationPreservation3.ts diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index ae13b11389..b68d14ed5e 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -6884,7 +6884,8 @@ namespace ts { let state = JSDocState.SawAsterisk; let margin: number | undefined; // + 4 for leading '/** ' - let indent = start - Math.max(content.lastIndexOf("\n", start), 0) + 4; + // + 1 because the last index of \n is always one index before the first character in the line and coincidentally, if there is no \n before start, it is -1, which is also one index before the first character + let indent = start - (content.lastIndexOf("\n", start) + 1) + 4; function pushComment(text: string) { if (!margin) { margin = indent; @@ -6940,7 +6941,7 @@ namespace ts { comments.push(whitespace); } else if (margin !== undefined && indent + whitespace.length > margin) { - comments.push(whitespace.slice(margin - indent - 1)); + comments.push(whitespace.slice(margin - indent)); } indent += whitespace.length; break; diff --git a/tests/cases/fourslash/jsDocIndentationPreservation1.ts b/tests/cases/fourslash/jsDocIndentationPreservation1.ts new file mode 100644 index 0000000000..1e454ac4be --- /dev/null +++ b/tests/cases/fourslash/jsDocIndentationPreservation1.ts @@ -0,0 +1,13 @@ +/// +// @allowJs: true +// @Filename: Foo.js + +/////** +//// * Does some stuff. +//// * Second line. +//// * Third line. +//// */ +////function foo/**/(){} + +goTo.marker(); +verify.quickInfoIs("function foo(): void", "Does some stuff.\n Second line.\n\tThird line."); diff --git a/tests/cases/fourslash/jsDocIndentationPreservation2.ts b/tests/cases/fourslash/jsDocIndentationPreservation2.ts new file mode 100644 index 0000000000..e832bc1270 --- /dev/null +++ b/tests/cases/fourslash/jsDocIndentationPreservation2.ts @@ -0,0 +1,13 @@ +/// +// @allowJs: true +// @Filename: Foo.js + +/////** +//// Does some stuff. +//// Second line. +//// Third line. +////*/ +////function foo/**/(){} + +goTo.marker(); +verify.quickInfoIs("function foo(): void", "Does some stuff.\n Second line.\n\tThird line."); diff --git a/tests/cases/fourslash/jsDocIndentationPreservation3.ts b/tests/cases/fourslash/jsDocIndentationPreservation3.ts new file mode 100644 index 0000000000..117ea0dee4 --- /dev/null +++ b/tests/cases/fourslash/jsDocIndentationPreservation3.ts @@ -0,0 +1,13 @@ +/// +// @allowJs: true +// @Filename: Foo.js + +/////** +//// Does some stuff. +//// Second line. +//// Third line. +////*/ +////function foo/**/(){} + +goTo.marker(); +verify.quickInfoIs("function foo(): void", "Does some stuff.\n Second line.\n\tThird line.");