From 59c89d65853bfd71484c08936afced69ddc6684c Mon Sep 17 00:00:00 2001 From: Kai Cataldo Date: Thu, 26 Sep 2019 03:52:06 -0400 Subject: [PATCH 1/7] Fix: preserve whitespace in multiline-comment-style (fixes #12312) --- lib/rules/multiline-comment-style.js | 17 ++- tests/lib/rules/multiline-comment-style.js | 161 ++++++++++++++++++++- 2 files changed, 171 insertions(+), 7 deletions(-) diff --git a/lib/rules/multiline-comment-style.js b/lib/rules/multiline-comment-style.js index 6578a120126..dd582697621 100644 --- a/lib/rules/multiline-comment-style.js +++ b/lib/rules/multiline-comment-style.js @@ -142,7 +142,8 @@ module.exports = { } else { const block = commentGroup[0]; const lines = block.value.split(astUtils.LINEBREAK_MATCHER); - const expectedLinePrefix = `${sourceCode.text.slice(block.range[0] - block.loc.start.column, block.range[0])} *`; + const expectedLeadingWhitespace = sourceCode.text.slice(block.range[0] - block.loc.start.column, block.range[0]); + const expectedLinePrefix = `${expectedLeadingWhitespace} *`; if (!/^\*?\s*$/u.test(lines[0])) { const start = block.value.startsWith("*") ? block.range[0] + 1 : block.range[0]; @@ -182,12 +183,16 @@ module.exports = { : "missingStar", fix(fixer) { const lineStartIndex = sourceCode.getIndexFromLoc({ line: lineNumber, column: 0 }); - const linePrefixLength = lineText.match(/^\s*\*? ?/u)[0].length; - const commentStartIndex = lineStartIndex + linePrefixLength; - - const replacementText = lineNumber === block.loc.end.line || lineText.length === linePrefixLength + const [linePrefix, whitespaceBefore, whitespaceAfter] = lineText.match(/^(\s*)\*?(\s*)/u); + const commentStartIndex = lineStartIndex + linePrefix.length; + const leadingWhitespace = whitespaceAfter || ` ${ + whitespaceBefore.startsWith(expectedLeadingWhitespace) + ? whitespaceBefore.replace(expectedLeadingWhitespace, "") + : "" + }`; + const replacementText = lineNumber === block.loc.end.line || lineText.length === linePrefix.length ? expectedLinePrefix - : `${expectedLinePrefix} `; + : `${expectedLinePrefix}${leadingWhitespace}`; return fixer.replaceTextRange([lineStartIndex, commentStartIndex], replacementText); } diff --git a/tests/lib/rules/multiline-comment-style.js b/tests/lib/rules/multiline-comment-style.js index 58c13581d4d..5c7da0bc6f5 100644 --- a/tests/lib/rules/multiline-comment-style.js +++ b/tests/lib/rules/multiline-comment-style.js @@ -322,7 +322,7 @@ ruleTester.run("multiline-comment-style", rule, { code: ` /* * the following line - is missing a '*' at the start + is missing a '*' at the start */ `, output: ` @@ -333,6 +333,21 @@ ruleTester.run("multiline-comment-style", rule, { `, errors: [{ messageId: "missingStar", line: 4 }] }, + { + code: ` + /* + * the following line + is missing a '*' at the start + */ + `, + output: ` + /* + * the following line + * is missing a '*' at the start + */ + `, + errors: [{ messageId: "missingStar", line: 4 }] + }, { code: ` /* @@ -482,6 +497,150 @@ ruleTester.run("multiline-comment-style", rule, { `, options: ["bare-block"], errors: [{ messageId: "expectedBareBlock", line: 2 }] + }, + { + code: ` + /* + { + "foo": 1, + "bar": 2 + } + */ + `, + output: ` + /* + * { + * "foo": 1, + * "bar": 2 + * } + */ + `, + errors: [ + { messageId: "missingStar", line: 3 }, + { messageId: "missingStar", line: 4 }, + { messageId: "missingStar", line: 5 }, + { messageId: "missingStar", line: 6 }, + { messageId: "alignment", line: 7 } + ] + }, + { + code: ` + /* + { + \t"foo": 1, + \t"bar": 2 + } + */ + `, + output: ` + /* + * { + * \t"foo": 1, + * \t"bar": 2 + * } + */ + `, + errors: [ + { messageId: "missingStar", line: 3 }, + { messageId: "missingStar", line: 4 }, + { messageId: "missingStar", line: 5 }, + { messageId: "missingStar", line: 6 }, + { messageId: "alignment", line: 7 } + ] + }, + { + code: ` + /* + { + \t "foo": 1, + \t "bar": 2 + } + */ + `, + output: ` + /* + * { + * \t "foo": 1, + * \t "bar": 2 + * } + */ + `, + errors: [ + { messageId: "missingStar", line: 3 }, + { messageId: "missingStar", line: 4 }, + { messageId: "missingStar", line: 5 }, + { messageId: "missingStar", line: 6 }, + { messageId: "alignment", line: 7 } + ] + }, + { + code: ` + /* + { + \t"foo": 1, + \t"bar": 2 + } + */ + `, + output: ` + /* + * { + * "foo": 1, + * "bar": 2 + * } + */ + `, + errors: [ + { messageId: "missingStar", line: 3 }, + { messageId: "missingStar", line: 4 }, + { messageId: "missingStar", line: 5 }, + { messageId: "missingStar", line: 6 }, + { messageId: "alignment", line: 7 } + ] + }, + { + code: ` + \t /* + \t { + \t "foo": 1, + \t "bar": 2 + } + */ + `, + output: ` + \t /* + \t * { + \t * "foo": 1, + \t * "bar": 2 + \t * } + \t */ + `, + errors: [ + { messageId: "missingStar", line: 3 }, + { messageId: "missingStar", line: 4 }, + { messageId: "missingStar", line: 5 }, + { messageId: "missingStar", line: 6 }, + { messageId: "alignment", line: 7 } + ] + }, + { + code: ` + //{ + // "foo": 1, + // "bar": 2 + //} + `, + output: ` + /* + *{ + * "foo": 1, + * "bar": 2 + *} + */ + `, + errors: [ + { messageId: "expectedBlock", line: 2 } + ] } ] }); From c55291d92ae8d3bb3d1a27d6f4bbddec170e142c Mon Sep 17 00:00:00 2001 From: Kai Cataldo Date: Sat, 5 Oct 2019 16:54:25 -0400 Subject: [PATCH 2/7] Add support for preserving whitespace in other comment types --- lib/rules/multiline-comment-style.js | 295 ++++++++++----- tests/lib/rules/multiline-comment-style.js | 415 ++++++++++++++++++++- 2 files changed, 612 insertions(+), 98 deletions(-) diff --git a/lib/rules/multiline-comment-style.js b/lib/rules/multiline-comment-style.js index dd582697621..bfb4b7fb448 100644 --- a/lib/rules/multiline-comment-style.js +++ b/lib/rules/multiline-comment-style.js @@ -43,17 +43,139 @@ module.exports = { //---------------------------------------------------------------------- /** - * Gets a list of comment lines in a group - * @param {Token[]} commentGroup A group of comments, containing either multiple line comments or a single block comment - * @returns {string[]} A list of comment lines + * Checks if a comment group is in starred-block form. + * @param {Token[]} commentGroup A group of comments, containing either multiple line comments or a single block comment. + * @returns {boolean} Whether or not the comment group is in starred block form. + */ + function isStarredBlockComment([firstComment]) { + if (firstComment.type !== "Block") { + return false; + } + + const lines = firstComment.value.split(astUtils.LINEBREAK_MATCHER); + + // The first and last lines can only contain whitespace. + return lines.length > 0 && lines.every((line, i) => (i === 0 || i === lines.length - 1 ? /^\s*$/u : /^\s*\*/u).test(line)); + } + + /** + * Checks if a comment group is in JSDoc form. + * @param {Token[]} commentGroup A group of comments, containing either multiple line comments or a single block comment. + * @returns {boolean} Whether or not the comment group is in JSDoc form. + */ + function isJSDocComment([firstComment]) { + if (firstComment.type !== "Block") { + return false; + } + + const lines = firstComment.value.split(astUtils.LINEBREAK_MATCHER); + + return /^\*\s*$/u.test(lines[0]) && + lines.slice(1, -1).every(line => /^\s* /u.test(line)) && + /^\s*$/u.test(lines[lines.length - 1]); + } + + /** + * Processes a comment group that is currently in separate-line form, calculating the offset for each line. + * @param {Token[]} commentGroup A group of comments containing multiple line comments. + * @returns {string[]} An array of the processed lines. + */ + function processSeparateLineComments(commentGroup) { + const allLinesHaveLeadingSpace = commentGroup + .map(({ value }) => value) + .filter(line => line.trim().length) + .every(line => line.startsWith(" ")); + + return commentGroup.map(({ value }) => (allLinesHaveLeadingSpace ? value.replace(/^ /u, "") : value)); + } + + /** + * Processes a comment group that is currently in starred-block form, calculating the offset for each line. + * @param {Token} comment A single block comment token in starred-block form. + * @returns {string[]} An array of the processed lines. + */ + function processStarredBlockComment(comment) { + const lines = comment.value.split(astUtils.LINEBREAK_MATCHER).map(line => line.replace(/^\s*$/u, "")); + const allLinesHaveLeadingSpace = lines + .map(line => line.replace(/\s*\*/u, "")) + .filter(line => line.trim().length) + .every(line => line.startsWith(" ")); + + return lines.map(line => line.replace(allLinesHaveLeadingSpace ? /\s*\* /u : /\s*\*/u, "")); + } + + /** + * Processes a comment group that is currently in bare-block form, calculating the offset for each line. + * @param {Token} comment A single block comment token in bare-block form. + * @returns {string[]} An array of the processed lines. + */ + function processBareBlockComment(comment) { + const lines = comment.value.split(astUtils.LINEBREAK_MATCHER).map(line => line.replace(/^\s*$/u, "")); + const leadingWhitespace = `${sourceCode.text.slice(comment.range[0] - comment.loc.start.column, comment.range[0])} `; + let offset = ""; + + /* + * Calculate the offset of the least indented line and use that as the basis for offsetting all the lines. + * The first line should not be checked because it is inline with the opening block comment delimter. + */ + for (const [i, line] of lines.entries()) { + if (!line.trim().length || i === 0) { + continue; + } + + const [, lineOffset] = line.match(/^(\s*\*?\s*)/u); + + if (lineOffset.length < leadingWhitespace.length) { + const newOffset = leadingWhitespace.slice(lineOffset.length - leadingWhitespace.length); + + if (newOffset.length > offset.length) { + offset = newOffset; + } + } + } + + return lines.map(line => { + const match = line.match(/^(\s*\*?\s*)(.*)/u); + const [, lineOffset, lineContents] = match; + + if (lineOffset.length > leadingWhitespace.length) { + return `${lineOffset.slice(leadingWhitespace.length - (offset.length + lineOffset.length))}${lineContents}`; + } + + if (lineOffset.length < leadingWhitespace.length) { + return `${lineOffset.slice(leadingWhitespace.length)}${lineContents}`; + } + + return lineContents; + }); + } + + /** + * Gets a list of comment lines in a group, formatting leading whitespace as necessary. + * @param {Token[]} commentGroup A group of comments containing either multiple line comments or a single block comment. + * @returns {string[]} A list of comment lines. */ function getCommentLines(commentGroup) { - if (commentGroup[0].type === "Line") { - return commentGroup.map(comment => comment.value); + const [firstComment] = commentGroup; + + if (firstComment.type === "Line") { + return processSeparateLineComments(commentGroup); } - return commentGroup[0].value - .split(astUtils.LINEBREAK_MATCHER) - .map(line => line.replace(/^\s*\*?/u, "")); + + if (isStarredBlockComment(commentGroup)) { + return processStarredBlockComment(firstComment); + } + + return processBareBlockComment(firstComment); + } + + /** + * Get the initial offset (whitespace) from the beginning of a line to a given comment token. + * @param {Token} comment The token to check. + * @returns {string} The offset from the beginning of a line to the token. + */ + function getInitialOffset(comment) { + return sourceCode.text.slice(comment.range[0] - comment.loc.start.column, comment.range[0]); } /** @@ -63,10 +185,9 @@ module.exports = { * @returns {string} A representation of the comment value in starred-block form, excluding start and end markers */ function convertToStarredBlock(firstComment, commentLinesList) { - const initialOffset = sourceCode.text.slice(firstComment.range[0] - firstComment.loc.start.column, firstComment.range[0]); - const starredLines = commentLinesList.map(line => `${initialOffset} *${line}`); + const initialOffset = getInitialOffset(firstComment); - return `\n${starredLines.join("\n")}\n${initialOffset} `; + return `/*\n${commentLinesList.map(line => `${initialOffset} * ${line}`).join("\n")}\n${initialOffset} */`; } /** @@ -76,10 +197,7 @@ module.exports = { * @returns {string} A representation of the comment value in separate-line form */ function convertToSeparateLines(firstComment, commentLinesList) { - const initialOffset = sourceCode.text.slice(firstComment.range[0] - firstComment.loc.start.column, firstComment.range[0]); - const separateLines = commentLinesList.map(line => `// ${line.trim()}`); - - return separateLines.join(`\n${initialOffset}`); + return commentLinesList.map(line => `// ${line}`).join(`\n${getInitialOffset(firstComment)}`); } /** @@ -89,24 +207,7 @@ module.exports = { * @returns {string} A representation of the comment value in bare-block form */ function convertToBlock(firstComment, commentLinesList) { - const initialOffset = sourceCode.text.slice(firstComment.range[0] - firstComment.loc.start.column, firstComment.range[0]); - const blockLines = commentLinesList.map(line => line.trim()); - - return `/* ${blockLines.join(`\n${initialOffset} `)} */`; - } - - /** - * Check a comment is JSDoc form - * @param {Token[]} commentGroup A group of comments, containing either multiple line comments or a single block comment - * @returns {boolean} if commentGroup is JSDoc form, return true - */ - function isJSDoc(commentGroup) { - const lines = commentGroup[0].value.split(astUtils.LINEBREAK_MATCHER); - - return commentGroup[0].type === "Block" && - /^\*\s*$/u.test(lines[0]) && - lines.slice(1, -1).every(line => /^\s* /u.test(line)) && - /^\s*$/u.test(lines[lines.length - 1]); + return `/* ${commentLinesList.join(`\n${getInitialOffset(firstComment)} `)} */`; } /** @@ -117,6 +218,7 @@ module.exports = { */ const commentGroupCheckers = { "starred-block"(commentGroup) { + const [firstComment] = commentGroup; const commentLines = getCommentLines(commentGroup); if (commentLines.some(value => value.includes("*/"))) { @@ -126,32 +228,30 @@ module.exports = { if (commentGroup.length > 1) { context.report({ loc: { - start: commentGroup[0].loc.start, + start: firstComment.loc.start, end: commentGroup[commentGroup.length - 1].loc.end }, messageId: "expectedBlock", fix(fixer) { - const range = [commentGroup[0].range[0], commentGroup[commentGroup.length - 1].range[1]]; - const starredBlock = `/*${convertToStarredBlock(commentGroup[0], commentLines)}*/`; + const range = [firstComment.range[0], commentGroup[commentGroup.length - 1].range[1]]; return commentLines.some(value => value.startsWith("/")) ? null - : fixer.replaceTextRange(range, starredBlock); + : fixer.replaceTextRange(range, convertToStarredBlock(firstComment, commentLines)); } }); } else { - const block = commentGroup[0]; - const lines = block.value.split(astUtils.LINEBREAK_MATCHER); - const expectedLeadingWhitespace = sourceCode.text.slice(block.range[0] - block.loc.start.column, block.range[0]); + const lines = firstComment.value.split(astUtils.LINEBREAK_MATCHER); + const expectedLeadingWhitespace = getInitialOffset(firstComment); const expectedLinePrefix = `${expectedLeadingWhitespace} *`; if (!/^\*?\s*$/u.test(lines[0])) { - const start = block.value.startsWith("*") ? block.range[0] + 1 : block.range[0]; + const start = firstComment.value.startsWith("*") ? firstComment.range[0] + 1 : firstComment.range[0]; context.report({ loc: { - start: block.loc.start, - end: { line: block.loc.start.line, column: block.loc.start.column + 2 } + start: firstComment.loc.start, + end: { line: firstComment.loc.start.line, column: firstComment.loc.start.column + 2 } }, messageId: "startNewline", fix: fixer => fixer.insertTextAfterRange([start, start + 2], `\n${expectedLinePrefix}`) @@ -161,15 +261,15 @@ module.exports = { if (!/^\s*$/u.test(lines[lines.length - 1])) { context.report({ loc: { - start: { line: block.loc.end.line, column: block.loc.end.column - 2 }, - end: block.loc.end + start: { line: firstComment.loc.end.line, column: firstComment.loc.end.column - 2 }, + end: firstComment.loc.end }, messageId: "endNewline", - fix: fixer => fixer.replaceTextRange([block.range[1] - 2, block.range[1]], `\n${expectedLinePrefix}/`) + fix: fixer => fixer.replaceTextRange([firstComment.range[1] - 2, firstComment.range[1]], `\n${expectedLinePrefix}/`) }); } - for (let lineNumber = block.loc.start.line + 1; lineNumber <= block.loc.end.line; lineNumber++) { + for (let lineNumber = firstComment.loc.start.line + 1; lineNumber <= firstComment.loc.end.line; lineNumber++) { const lineText = sourceCode.lines[lineNumber - 1]; if (!lineText.startsWith(expectedLinePrefix)) { @@ -190,7 +290,7 @@ module.exports = { ? whitespaceBefore.replace(expectedLeadingWhitespace, "") : "" }`; - const replacementText = lineNumber === block.loc.end.line || lineText.length === linePrefix.length + const replacementText = lineNumber === firstComment.loc.end.line || lineText.length === linePrefix.length ? expectedLinePrefix : `${expectedLinePrefix}${leadingWhitespace}`; @@ -202,67 +302,68 @@ module.exports = { } }, "separate-lines"(commentGroup) { - if (!isJSDoc(commentGroup) && commentGroup[0].type === "Block") { - const commentLines = getCommentLines(commentGroup); - const block = commentGroup[0]; - const tokenAfter = sourceCode.getTokenAfter(block, { includeComments: true }); + const [firstComment] = commentGroup; + + if (firstComment.type !== "Block" || isJSDocComment(commentGroup)) { + return; + } + + const commentLines = getCommentLines(commentGroup); + const tokenAfter = sourceCode.getTokenAfter(firstComment, { includeComments: true }); + + if (tokenAfter && firstComment.loc.end.line === tokenAfter.loc.start.line) { + return; + } - if (tokenAfter && block.loc.end.line === tokenAfter.loc.start.line) { - return; + context.report({ + loc: { + start: firstComment.loc.start, + end: { line: firstComment.loc.start.line, column: firstComment.loc.start.column + 2 } + }, + messageId: "expectedLines", + fix(fixer) { + return fixer.replaceText(firstComment, convertToSeparateLines(firstComment, commentLines.filter(line => line.length))); } + }); + }, + "bare-block"(commentGroup) { + if (isJSDocComment(commentGroup)) { + return; + } + + const [firstComment] = commentGroup; + const commentLines = getCommentLines(commentGroup); + // disallows consecutive line comments in favor of using a block comment. + if (firstComment.type === "Line" && commentLines.length > 1 && + !commentLines.some(value => value.includes("*/"))) { context.report({ loc: { - start: block.loc.start, - end: { line: block.loc.start.line, column: block.loc.start.column + 2 } + start: firstComment.loc.start, + end: commentGroup[commentGroup.length - 1].loc.end }, - messageId: "expectedLines", + messageId: "expectedBlock", fix(fixer) { - return fixer.replaceText(block, convertToSeparateLines(block, commentLines.filter(line => line))); + return fixer.replaceTextRange( + [firstComment.range[0], commentGroup[commentGroup.length - 1].range[1]], + convertToBlock(firstComment, commentLines) + ); } }); } - }, - "bare-block"(commentGroup) { - if (!isJSDoc(commentGroup)) { - const commentLines = getCommentLines(commentGroup); - - // disallows consecutive line comments in favor of using a block comment. - if (commentGroup[0].type === "Line" && commentLines.length > 1 && - !commentLines.some(value => value.includes("*/"))) { - context.report({ - loc: { - start: commentGroup[0].loc.start, - end: commentGroup[commentGroup.length - 1].loc.end - }, - messageId: "expectedBlock", - fix(fixer) { - const range = [commentGroup[0].range[0], commentGroup[commentGroup.length - 1].range[1]]; - const block = convertToBlock(commentGroup[0], commentLines.filter(line => line)); - - return fixer.replaceTextRange(range, block); - } - }); - } - - // prohibits block comments from having a * at the beginning of each line. - if (commentGroup[0].type === "Block") { - const block = commentGroup[0]; - const lines = block.value.split(astUtils.LINEBREAK_MATCHER).filter(line => line.trim()); - if (lines.length > 0 && lines.every(line => /^\s*\*/u.test(line))) { - context.report({ - loc: { - start: block.loc.start, - end: { line: block.loc.start.line, column: block.loc.start.column + 2 } - }, - messageId: "expectedBareBlock", - fix(fixer) { - return fixer.replaceText(block, convertToBlock(block, commentLines.filter(line => line))); - } - }); + // prohibits block comments from having a * at the beginning of each line. + if (isStarredBlockComment(commentGroup)) { + context.report({ + loc: { + start: firstComment.loc.start, + end: { line: firstComment.loc.start.line, column: firstComment.loc.start.column + 2 } + }, + messageId: "expectedBareBlock", + fix(fixer) { + return fixer.replaceText(firstComment, convertToBlock(firstComment, commentLines.filter(line => line.length))); } - } + }); } } }; diff --git a/tests/lib/rules/multiline-comment-style.js b/tests/lib/rules/multiline-comment-style.js index 5c7da0bc6f5..9e9e71b10a4 100644 --- a/tests/lib/rules/multiline-comment-style.js +++ b/tests/lib/rules/multiline-comment-style.js @@ -130,6 +130,50 @@ ruleTester.run("multiline-comment-style", rule, { `, options: ["starred-block"] }, + { + code: ` + /* + * foo + */ + `, + options: ["starred-block"] + }, + { + code: ` + /* foo */ + `, + options: ["bare-block"] + }, + { + code: ` + /* + foo */ + `, + options: ["bare-block"] + }, + { + code: ` + /* + foo + */ + `, + options: ["bare-block"] + }, + { + code: ` + /* + foo */ + `, + options: ["bare-block"] + }, + { + code: ` + /* + foo + */ + `, + options: ["bare-block"] + }, { code: ` // this is @@ -230,6 +274,60 @@ ruleTester.run("multiline-comment-style", rule, { * 4 is 20. */ `, options: ["bare-block"] + }, + { + code: ` + /* + * foo + * bar + * baz + * qux + */ + `, + options: ["starred-block"] + }, + { + code: ` + /* foo + * bar + * baz + * qux + */ + `, + options: ["bare-block"] + }, + { + code: ` + /** + * JSDoc blocks + * are + * ignored + * ! + */ + `, + options: ["bare-block"] + }, + { + code: ` + /** + * JSDoc blocks + * are + * ignored + * ! + */ + `, + options: ["starred-block"] + }, + { + code: ` + /** + * JSDoc blocks + * are + * ignored + * ! + */ + `, + options: ["separate-lines"] } ], @@ -276,6 +374,57 @@ ruleTester.run("multiline-comment-style", rule, { `, errors: [{ messageId: "expectedBlock", line: 2 }, { messageId: "expectedBlock", line: 5 }] }, + { + code: ` + // foo + // bar + // baz + // qux + `, + output: ` + /* + * foo + * bar + * baz + * qux + */ + `, + errors: [{ messageId: "expectedBlock", line: 2 }] + }, + { + code: ` + // foo + // + // baz + // qux + `, + output: ` + /* + * foo + *${" "} + * baz + * qux + */ + `, + errors: [{ messageId: "expectedBlock", line: 2 }] + }, + { + code: ` + // foo + // bar + // baz + // qux + `, + output: ` + /* + * foo + * bar + * baz + * qux + */ + `, + errors: [{ messageId: "expectedBlock", line: 2 }] + }, { code: ` /* this block @@ -484,6 +633,72 @@ ruleTester.run("multiline-comment-style", rule, { options: ["bare-block"], errors: [{ messageId: "expectedBlock", line: 2 }] }, + { + code: ` + // foo + // + // bar + `, + output: ` + /* foo + ${" ".repeat(3)} + bar */ + `, + options: ["bare-block"], + errors: [{ messageId: "expectedBlock", line: 2 }] + }, + { + code: ` + //foo + //bar + `, + output: ` + /* foo + bar */ + `, + options: ["bare-block"], + errors: [{ messageId: "expectedBlock", line: 2 }] + }, + { + code: ` + // foo + // bar + `, + output: ` + /* foo + bar */ + `, + options: ["bare-block"], + errors: [{ messageId: "expectedBlock", line: 2 }] + }, + { + code: ` + // foo + // bar + `, + output: ` + /* foo + bar */ + `, + options: ["bare-block"], + errors: [{ messageId: "expectedBlock", line: 2 }] + }, + { + code: ` + // foo + // bar + // baz + // qux + `, + output: ` + /* foo + bar + baz + qux */ + `, + options: ["bare-block"], + errors: [{ messageId: "expectedBlock", line: 2 }] + }, { code: ` /* @@ -498,6 +713,66 @@ ruleTester.run("multiline-comment-style", rule, { options: ["bare-block"], errors: [{ messageId: "expectedBareBlock", line: 2 }] }, + { + code: ` + /* + *foo + *bar + */ + `, + output: ` + /* foo + bar */ + `, + options: ["bare-block"], + errors: [{ messageId: "expectedBareBlock", line: 2 }] + }, + { + code: ` + /* + * foo + * bar + */ + `, + output: ` + /* foo + bar */ + `, + options: ["bare-block"], + errors: [{ messageId: "expectedBareBlock", line: 2 }] + }, + { + code: ` + /* + * foo + * bar + */ + `, + output: ` + /* foo + bar */ + `, + options: ["bare-block"], + errors: [{ messageId: "expectedBareBlock", line: 2 }] + }, + { + code: ` + /* + * foo + * bar + * baz + * qux + */ + `, + output: ` + /* foo + bar + baz + qux */ + `, + options: ["bare-block"], + errors: [{ messageId: "expectedBareBlock", line: 2 }] + }, { code: ` /* @@ -631,6 +906,39 @@ ruleTester.run("multiline-comment-style", rule, { //} `, output: ` + /* + * { + * "foo": 1, + * "bar": 2 + * } + */ + `, + errors: [ + { messageId: "expectedBlock", line: 2 } + ] + }, + { + code: ` + /* + * { + * "foo": 1, + * "bar": 2 + * } + */ + `, + output: ` + // { + // "foo": 1, + // "bar": 2 + // } + `, + options: ["separate-lines"], + errors: [ + { messageId: "expectedLines", line: 2 } + ] + }, + { + code: ` /* *{ * "foo": 1, @@ -638,8 +946,113 @@ ruleTester.run("multiline-comment-style", rule, { *} */ `, + output: ` + // { + // "foo": 1, + // "bar": 2 + // } + `, + options: ["separate-lines"], errors: [ - { messageId: "expectedBlock", line: 2 } + { messageId: "expectedLines", line: 2 } + ] + }, + { + code: ` + /* + * { + * "foo": 1, + * "bar": 2 + * } + */ + `, + output: ` + // { + // "foo": 1, + // "bar": 2 + // } + `, + options: ["separate-lines"], + errors: [ + { messageId: "expectedLines", line: 2 } + ] + }, + { + code: ` + /* + *{ + * "foo": 1, + * "bar": 2 + *} + */ + `, + output: ` + // { + // "foo": 1, + // "bar": 2 + // } + `, + options: ["separate-lines"], + errors: [ + { messageId: "expectedLines", line: 2 } + ] + }, + { + code: ` + /* + * { + * "foo": 1, + * "bar": 2 + *} + */ + `, + output: ` + // { + // "foo": 1, + // "bar": 2 + // } + `, + options: ["separate-lines"], + errors: [ + { messageId: "expectedLines", line: 2 } + ] + }, + { + code: ` + /* + { + "foo": 1, + "bar": 2 + } + */ + `, + output: ` + // { + // "foo": 1, + // "bar": 2 + // } + `, + options: ["separate-lines"], + errors: [ + { messageId: "expectedLines", line: 2 } + ] + }, + { + code: ` + /* { + "foo": 1, + "bar": 2 + } */ + `, + output: ` + // { + // "foo": 1, + // "bar": 2 + // }${" "} + `, + options: ["separate-lines"], + errors: [ + { messageId: "expectedLines", line: 2 } ] } ] From 2ff0cae4b9738881d559b5b975978c85ce6ce4ac Mon Sep 17 00:00:00 2001 From: Kai Cataldo Date: Thu, 24 Oct 2019 20:57:03 -0400 Subject: [PATCH 3/7] Handle blank lines in starred-block comments correctly --- lib/rules/multiline-comment-style.js | 10 +- tests/lib/rules/multiline-comment-style.js | 106 +++++++++++++++++++++ 2 files changed, 112 insertions(+), 4 deletions(-) diff --git a/lib/rules/multiline-comment-style.js b/lib/rules/multiline-comment-style.js index bfb4b7fb448..5bea92ded7b 100644 --- a/lib/rules/multiline-comment-style.js +++ b/lib/rules/multiline-comment-style.js @@ -95,13 +95,15 @@ module.exports = { * @returns {string[]} An array of the processed lines. */ function processStarredBlockComment(comment) { - const lines = comment.value.split(astUtils.LINEBREAK_MATCHER).map(line => line.replace(/^\s*$/u, "")); + const lines = comment.value.split(astUtils.LINEBREAK_MATCHER) + .filter((line, i, linesArr) => !(i === 0 || i === linesArr.length - 1)) + .map(line => line.replace(/^\s*$/u, "")); const allLinesHaveLeadingSpace = lines .map(line => line.replace(/\s*\*/u, "")) .filter(line => line.trim().length) .every(line => line.startsWith(" ")); - return lines.map(line => line.replace(allLinesHaveLeadingSpace ? /\s*\* /u : /\s*\*/u, "")); + return lines.map(line => line.replace(allLinesHaveLeadingSpace ? /\s*\* ?/u : /\s*\*/u, "")); } /** @@ -322,7 +324,7 @@ module.exports = { }, messageId: "expectedLines", fix(fixer) { - return fixer.replaceText(firstComment, convertToSeparateLines(firstComment, commentLines.filter(line => line.length))); + return fixer.replaceText(firstComment, convertToSeparateLines(firstComment, commentLines)); } }); }, @@ -361,7 +363,7 @@ module.exports = { }, messageId: "expectedBareBlock", fix(fixer) { - return fixer.replaceText(firstComment, convertToBlock(firstComment, commentLines.filter(line => line.length))); + return fixer.replaceText(firstComment, convertToBlock(firstComment, commentLines)); } }); } diff --git a/tests/lib/rules/multiline-comment-style.js b/tests/lib/rules/multiline-comment-style.js index 9e9e71b10a4..b44ea0a05d5 100644 --- a/tests/lib/rules/multiline-comment-style.js +++ b/tests/lib/rules/multiline-comment-style.js @@ -937,6 +937,78 @@ ruleTester.run("multiline-comment-style", rule, { { messageId: "expectedLines", line: 2 } ] }, + { + code: ` + /* + * + * { + * "foo": 1, + * "bar": 2 + * } + * + */ + `, + output: ` + //${" "} + // { + // "foo": 1, + // "bar": 2 + // } + //${" "} + `, + options: ["separate-lines"], + errors: [ + { messageId: "expectedLines", line: 2 } + ] + }, + { + code: ` + /* + * + * { + * "foo": 1, + * "bar": 2 + * } + * + */ + `, + output: ` + /*${" "} + { + "foo": 1, + "bar": 2 + } + */ + `, + options: ["bare-block"], + errors: [ + { messageId: "expectedBareBlock", line: 2 } + ] + }, + { + code: ` + /* + * + *{ + * "foo": 1, + * "bar": 2 + *} + * + */ + `, + output: ` + /*${" "} + { + "foo": 1, + "bar": 2 + } + */ + `, + options: ["bare-block"], + errors: [ + { messageId: "expectedBareBlock", line: 2 } + ] + }, { code: ` /* @@ -1027,10 +1099,12 @@ ruleTester.run("multiline-comment-style", rule, { */ `, output: ` + //${" "} // { // "foo": 1, // "bar": 2 // } + //${" "} `, options: ["separate-lines"], errors: [ @@ -1054,6 +1128,38 @@ ruleTester.run("multiline-comment-style", rule, { errors: [ { messageId: "expectedLines", line: 2 } ] + }, + { + code: ` + /* + * foo + * + * bar + */ + `, + output: ` + // foo + //${" "} + // bar + `, + options: ["separate-lines"], + errors: [{ messageId: "expectedLines", line: 2 }] + }, + { + code: ` + /* + * foo + *${" "} + * bar + */ + `, + output: ` + // foo + //${" "} + // bar + `, + options: ["separate-lines"], + errors: [{ messageId: "expectedLines", line: 2 }] } ] }); From cab04fd7a9bc15bd08240b86e4a582953461c8cf Mon Sep 17 00:00:00 2001 From: Kai Cataldo Date: Thu, 24 Oct 2019 23:10:52 -0400 Subject: [PATCH 4/7] Add more tests --- lib/rules/multiline-comment-style.js | 4 +- tests/lib/rules/multiline-comment-style.js | 204 +++++++++++++++++++++ 2 files changed, 206 insertions(+), 2 deletions(-) diff --git a/lib/rules/multiline-comment-style.js b/lib/rules/multiline-comment-style.js index 5bea92ded7b..17c297fad7e 100644 --- a/lib/rules/multiline-comment-style.js +++ b/lib/rules/multiline-comment-style.js @@ -286,8 +286,7 @@ module.exports = { fix(fixer) { const lineStartIndex = sourceCode.getIndexFromLoc({ line: lineNumber, column: 0 }); const [linePrefix, whitespaceBefore, whitespaceAfter] = lineText.match(/^(\s*)\*?(\s*)/u); - const commentStartIndex = lineStartIndex + linePrefix.length; - const leadingWhitespace = whitespaceAfter || ` ${ + const leadingWhitespace = whitespaceAfter.length && whitespaceAfter || ` ${ whitespaceBefore.startsWith(expectedLeadingWhitespace) ? whitespaceBefore.replace(expectedLeadingWhitespace, "") : "" @@ -295,6 +294,7 @@ module.exports = { const replacementText = lineNumber === firstComment.loc.end.line || lineText.length === linePrefix.length ? expectedLinePrefix : `${expectedLinePrefix}${leadingWhitespace}`; + const commentStartIndex = lineStartIndex + linePrefix.length; return fixer.replaceTextRange([lineStartIndex, commentStartIndex], replacementText); } diff --git a/tests/lib/rules/multiline-comment-style.js b/tests/lib/rules/multiline-comment-style.js index b44ea0a05d5..047e4e4f0c7 100644 --- a/tests/lib/rules/multiline-comment-style.js +++ b/tests/lib/rules/multiline-comment-style.js @@ -1160,6 +1160,210 @@ ruleTester.run("multiline-comment-style", rule, { `, options: ["separate-lines"], errors: [{ messageId: "expectedLines", line: 2 }] + }, + { + code: ` + /* + * foo + * + * bar + */ + `, + output: ` + /* foo +${" "} + bar */ + `, + options: ["bare-block"], + errors: [{ messageId: "expectedBareBlock", line: 2 }] + }, + { + code: ` + /* + * foo + *${" "} + * bar + */ + `, + output: ` + /* foo +${" "} + bar */ + `, + options: ["bare-block"], + errors: [{ messageId: "expectedBareBlock", line: 2 }] + }, + { + code: ` + // foo + // + // bar + `, + output: ` + /* + * foo + *${" "} + * bar + */ + `, + options: ["starred-block"], + errors: [{ messageId: "expectedBlock", line: 2 }] + }, + { + code: ` + // foo + //${" "} + // bar + `, + output: ` + /* + * foo + *${" "} + * bar + */ + `, + options: ["starred-block"], + errors: [{ messageId: "expectedBlock", line: 2 }] + }, + { + code: ` + // foo + // + // bar + `, + output: ` + /* foo +${" "} + bar */ + `, + options: ["bare-block"], + errors: [{ messageId: "expectedBlock", line: 2 }] + }, + { + code: ` + // foo + //${" "} + // bar + `, + output: ` + /* foo +${" "} + bar */ + `, + options: ["bare-block"], + errors: [{ messageId: "expectedBlock", line: 2 }] + }, + { + code: ` + /* foo + + bar */ + `, + output: ` + // foo + //${" "} + // bar${" "} + `, + options: ["separate-lines"], + errors: [{ messageid: "expectedlines", line: 2 }] + }, + { + code: ` + /* foo +${" "} + bar */ + `, + output: ` + // foo + //${" "} + // bar${" "} + `, + options: ["separate-lines"], + errors: [{ messageid: "expectedlines", line: 2 }] + }, + { + code: ` + /* foo + + bar */ + `, + output: ` + /* + * foo + * + * bar + */ + `, + options: ["starred-block"], + errors: [ + { messageId: "startNewline", line: 2 }, + { messageId: "missingStar", line: 3 }, + { messageId: "missingStar", line: 4 }, + { messageId: "endNewline", line: 4 } + ] + }, + { + code: ` + /* foo +${" "} + bar */ + `, + output: ` + /* + * foo + * + * bar + */ + `, + options: ["starred-block"], + errors: [ + { messageId: "startNewline", line: 2 }, + { messageId: "missingStar", line: 3 }, + { messageId: "missingStar", line: 4 }, + { messageId: "endNewline", line: 4 } + ] + }, + { + code: ` + /*foo + + bar */ + `, + output: ` + /* + *foo + * + *bar${" "} + */ + `, + options: ["starred-block"], + errors: [ + { messageId: "startNewline", line: 2 }, + { messageId: "missingStar", line: 3 }, + { messageId: "missingStar", line: 4 }, + { messageId: "endNewline", line: 4 } + ] + }, + { + code: ` + /*foo +${" "} + bar */ + `, + output: ` + /* + *foo + * + *bar${" "} + */ + `, + options: ["starred-block"], + errors: [ + { messageId: "startNewline", line: 2 }, + { messageId: "missingStar", line: 3 }, + { messageId: "missingStar", line: 4 }, + { messageId: "endNewline", line: 4 } + ] } ] }); From f3f123295d408656d46654f0fffea6f698e9cc87 Mon Sep 17 00:00:00 2001 From: Kai Cataldo Date: Fri, 25 Oct 2019 17:30:11 -0400 Subject: [PATCH 5/7] Fix starred-block alignment/missing star fixer --- lib/rules/multiline-comment-style.js | 53 ++++++++++++----- tests/lib/rules/multiline-comment-style.js | 69 +++++++++++++--------- 2 files changed, 79 insertions(+), 43 deletions(-) diff --git a/lib/rules/multiline-comment-style.js b/lib/rules/multiline-comment-style.js index 17c297fad7e..ad0bcaa954b 100644 --- a/lib/rules/multiline-comment-style.js +++ b/lib/rules/multiline-comment-style.js @@ -42,6 +42,15 @@ module.exports = { // Helpers //---------------------------------------------------------------------- + /** + * Checks if a comment line is starred. + * @param {string} line A string representing a comment line. + * @returns {boolean} Whether or not the comment line is starred. + */ + function isStarredCommentLine(line) { + return /^\s*\*/u.test(line); + } + /** * Checks if a comment group is in starred-block form. * @param {Token[]} commentGroup A group of comments, containing either multiple line comments or a single block comment. @@ -273,30 +282,42 @@ module.exports = { for (let lineNumber = firstComment.loc.start.line + 1; lineNumber <= firstComment.loc.end.line; lineNumber++) { const lineText = sourceCode.lines[lineNumber - 1]; + const errorType = isStarredCommentLine(lineText) + ? "alignment" + : "missingStar"; if (!lineText.startsWith(expectedLinePrefix)) { context.report({ loc: { start: { line: lineNumber, column: 0 }, - end: { line: lineNumber, column: sourceCode.lines[lineNumber - 1].length } + end: { line: lineNumber, column: lineText.length } }, - messageId: /^\s*\*/u.test(lineText) - ? "alignment" - : "missingStar", + messageId: errorType, fix(fixer) { const lineStartIndex = sourceCode.getIndexFromLoc({ line: lineNumber, column: 0 }); - const [linePrefix, whitespaceBefore, whitespaceAfter] = lineText.match(/^(\s*)\*?(\s*)/u); - const leadingWhitespace = whitespaceAfter.length && whitespaceAfter || ` ${ - whitespaceBefore.startsWith(expectedLeadingWhitespace) - ? whitespaceBefore.replace(expectedLeadingWhitespace, "") - : "" - }`; - const replacementText = lineNumber === firstComment.loc.end.line || lineText.length === linePrefix.length - ? expectedLinePrefix - : `${expectedLinePrefix}${leadingWhitespace}`; - const commentStartIndex = lineStartIndex + linePrefix.length; - - return fixer.replaceTextRange([lineStartIndex, commentStartIndex], replacementText); + + if (errorType === "alignment") { + const [, commentTextPrefix = ""] = lineText.match(/^(\s*\*)/u) || []; + const commentTextStartIndex = lineStartIndex + commentTextPrefix.length; + + return fixer.replaceTextRange([lineStartIndex, commentTextStartIndex], expectedLinePrefix); + } + + const [, commentTextPrefix = ""] = lineText.match(/^(\s*)/u) || []; + const commentTextStartIndex = lineStartIndex + commentTextPrefix.length; + let offset; + + for (const [idx, line] of lines.entries()) { + if (line.trim().length > 0) { + const lineTextToAlignWith = sourceCode.lines[firstComment.loc.start.line - 1 + idx]; + const [, prefix = "", initialOffset = ""] = lineTextToAlignWith.match(/^(\s*(?:\/?\*)?(\s*))/u) || []; + + offset = `${commentTextPrefix.slice(prefix.length)}${initialOffset}`; + break; + } + } + + return fixer.replaceTextRange([lineStartIndex, commentTextStartIndex], `${expectedLinePrefix}${offset}`); } }); } diff --git a/tests/lib/rules/multiline-comment-style.js b/tests/lib/rules/multiline-comment-style.js index 047e4e4f0c7..c69265ba9e2 100644 --- a/tests/lib/rules/multiline-comment-style.js +++ b/tests/lib/rules/multiline-comment-style.js @@ -471,7 +471,7 @@ ruleTester.run("multiline-comment-style", rule, { code: ` /* * the following line - is missing a '*' at the start + is missing a '*' at the start */ `, output: ` @@ -492,7 +492,22 @@ ruleTester.run("multiline-comment-style", rule, { output: ` /* * the following line - * is missing a '*' at the start + * is missing a '*' at the start + */ + `, + errors: [{ messageId: "missingStar", line: 4 }] + }, + { + code: ` + /* + * the following line + is missing a '*' at the start + */ + `, + output: ` + /* + * the following line + * is missing a '*' at the start */ `, errors: [{ messageId: "missingStar", line: 4 }] @@ -784,10 +799,10 @@ ruleTester.run("multiline-comment-style", rule, { `, output: ` /* - * { - * "foo": 1, - * "bar": 2 - * } + *{ + * "foo": 1, + * "bar": 2 + *} */ `, errors: [ @@ -809,10 +824,10 @@ ruleTester.run("multiline-comment-style", rule, { `, output: ` /* - * { - * \t"foo": 1, - * \t"bar": 2 - * } + *{ + *\t"foo": 1, + *\t"bar": 2 + *} */ `, errors: [ @@ -834,10 +849,10 @@ ruleTester.run("multiline-comment-style", rule, { `, output: ` /* - * { - * \t "foo": 1, - * \t "bar": 2 - * } + *{ + *\t "foo": 1, + *\t "bar": 2 + *} */ `, errors: [ @@ -859,10 +874,10 @@ ruleTester.run("multiline-comment-style", rule, { `, output: ` /* - * { - * "foo": 1, - * "bar": 2 - * } + *{ + *"foo": 1, + *"bar": 2 + *} */ `, errors: [ @@ -884,10 +899,10 @@ ruleTester.run("multiline-comment-style", rule, { `, output: ` \t /* - \t * { - \t * "foo": 1, - \t * "bar": 2 - \t * } + \t *{ + \t *"foo": 1, + \t *"bar": 2 + \t *} \t */ `, errors: [ @@ -1290,8 +1305,8 @@ ${" "} output: ` /* * foo - * - * bar + *${" "} + * bar${" "} */ `, options: ["starred-block"], @@ -1311,8 +1326,8 @@ ${" "} output: ` /* * foo - * - * bar + *${" "} + * bar${" "} */ `, options: ["starred-block"], @@ -1353,7 +1368,7 @@ ${" "} output: ` /* *foo - * + *${" "} *bar${" "} */ `, From e402fba71eba46a2c1186a5a6507b69e0263b526 Mon Sep 17 00:00:00 2001 From: Kai Cataldo Date: Wed, 6 Nov 2019 23:21:18 -0500 Subject: [PATCH 6/7] Use RegExp to check for whitespace-only lines --- lib/rules/multiline-comment-style.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/rules/multiline-comment-style.js b/lib/rules/multiline-comment-style.js index ad0bcaa954b..385cb5c8e5f 100644 --- a/lib/rules/multiline-comment-style.js +++ b/lib/rules/multiline-comment-style.js @@ -308,13 +308,15 @@ module.exports = { let offset; for (const [idx, line] of lines.entries()) { - if (line.trim().length > 0) { - const lineTextToAlignWith = sourceCode.lines[firstComment.loc.start.line - 1 + idx]; - const [, prefix = "", initialOffset = ""] = lineTextToAlignWith.match(/^(\s*(?:\/?\*)?(\s*))/u) || []; - - offset = `${commentTextPrefix.slice(prefix.length)}${initialOffset}`; - break; + if (!/\S+/u.test(line)) { + continue; } + + const lineTextToAlignWith = sourceCode.lines[firstComment.loc.start.line - 1 + idx]; + const [, prefix = "", initialOffset = ""] = lineTextToAlignWith.match(/^(\s*(?:\/?\*)?(\s*))/u) || []; + + offset = `${commentTextPrefix.slice(prefix.length)}${initialOffset}`; + break; } return fixer.replaceTextRange([lineStartIndex, commentTextStartIndex], `${expectedLinePrefix}${offset}`); From 831af55ca279ecf4d5ea7955a8f45acba976cb73 Mon Sep 17 00:00:00 2001 From: Kai Cataldo Date: Mon, 18 Nov 2019 12:15:04 -0500 Subject: [PATCH 7/7] Fix up comments --- lib/rules/multiline-comment-style.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/rules/multiline-comment-style.js b/lib/rules/multiline-comment-style.js index 385cb5c8e5f..fb50e1522ea 100644 --- a/lib/rules/multiline-comment-style.js +++ b/lib/rules/multiline-comment-style.js @@ -127,7 +127,7 @@ module.exports = { /* * Calculate the offset of the least indented line and use that as the basis for offsetting all the lines. - * The first line should not be checked because it is inline with the opening block comment delimter. + * The first line should not be checked because it is inline with the opening block comment delimiter. */ for (const [i, line] of lines.entries()) { if (!line.trim().length || i === 0) { @@ -181,7 +181,7 @@ module.exports = { } /** - * Get the initial offset (whitespace) from the beginning of a line to a given comment token. + * Gets the initial offset (whitespace) from the beginning of a line to a given comment token. * @param {Token} comment The token to check. * @returns {string} The offset from the beginning of a line to the token. */ @@ -359,7 +359,7 @@ module.exports = { const [firstComment] = commentGroup; const commentLines = getCommentLines(commentGroup); - // disallows consecutive line comments in favor of using a block comment. + // Disallows consecutive line comments in favor of using a block comment. if (firstComment.type === "Line" && commentLines.length > 1 && !commentLines.some(value => value.includes("*/"))) { context.report({ @@ -377,7 +377,7 @@ module.exports = { }); } - // prohibits block comments from having a * at the beginning of each line. + // Prohibits block comments from having a * at the beginning of each line. if (isStarredBlockComment(commentGroup)) { context.report({ loc: {