Skip to content

Commit

Permalink
WIP: New: multiline-comment-style rule (fixes #8320)
Browse files Browse the repository at this point in the history 
This code is currently messy and incomplete. There are also some edge cases that the rule's design needs to address (e.g. allowing "banner" comments which are created from several consecutive line comments).
  • Loading branch information
@not-an-aardvark
not-an-aardvark committed May 5, 2017
1 parent 74ab344 commit a8d0931
Show file tree
Hide file tree
Showing 3 changed files with 568 additions and 0 deletions.
43 changes: 43 additions & 0 deletions docs/rules/multiline-comment-style.md
View file
@@ -0,0 +1,43 @@
# enforce a particular style for multiline comments (multiline-comment-style)

Many style guides require a particular style for comments that span multiple lines. For example, some style guides prefer the use of a single block comment for multiline comments, whereas other style guides prefer consecutive line comments.

## Rule Details

This rule aims to enforce a particular style for multiline comments.

### Options

This rule has a string option, which can have one of the following values:

* `"starred-block"` (default): Disallows consecutive line comments in favor of block comments. Additionally, requires block comments to have an aligned `*` character before each line.
* `"bare-block"`: Disallows consecutive line comments in favor of block comments, and disallows block comments from having a `"*"` character before each line.
* `"separate-lines"`: Disallows block comments in favor of consecutive line comments

The rule always ignores directive comments such as `/* eslint-disable */`. Additionally, unless the mode is `"starred-block"`, the rule ignores JSDoc comments.

TODO: Add examples

Examples of **incorrect** code for this rule:

```js

// fill me in

```

Examples of **correct** code for this rule:

```js

// fill me in

```

## When Not To Use It

Give a short description of when it would be appropriate to turn off this rule.

## Further Reading

If there are other links that describe the issue this rule addresses, please include them here in a bulleted list.
181 changes: 181 additions & 0 deletions lib/rules/multiline-comment-style.js
View file
@@ -0,0 +1,181 @@
/**
* @fileoverview enforce a particular style for multiline comments
* @author Teddy Katz
*/
"use strict";

const astUtils = require("../ast-utils");

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

module.exports = {
meta: {
docs: {
description: "enforce a particular style for multiline comments",
category: "Stylistic Issues",
recommended: false
},
fixable: "whitespace",
schema: [{ enum: ["starred-block", "separate-lines"] }]
},

create(context) {
const sourceCode = context.getSourceCode();
const option = context.options[0] || "starred-block";

//----------------------------------------------------------------------
// Helpers
//----------------------------------------------------------------------

/**
* 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
*/
function getCommentLines(commentGroup) {
if (commentGroup[0].type === "Line") {
return commentGroup.map(comment => comment.value);
}
return commentGroup[0].value
.split(astUtils.LINEBREAK_MATCHER)
.map(line => line.replace(/^\s*\*?/, ""));
}

/**
* Converts a comment into starred-block form
* @param {Token} firstComment The first comment of the group being converted
* @param {string[]} commentLinesList A list of lines to appear in the new starred-block comment
* @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}`);

return `\n${starredLines.join("\n")}\n${initialOffset} `;
}

/**
* Each method checks a group of comments to see if it's valid according to the given option.
* @param {Token[]} commentGroup A list of comments that appear together. This will either contain a single
* block comment or multiple line comments.
* @returns {void}
*/
const commentGroupCheckers = {
"starred-block"(commentGroup) {
const commentLines = getCommentLines(commentGroup);

if (commentLines.some(value => value.includes("*/"))) {
return;
}

if (commentGroup.length > 1) {
context.report({
loc: {
start: commentGroup[0].loc.start,
end: commentGroup[commentGroup.length - 1].loc.end
},
message: "Expected a block comment instead of consecutive line comments.",
fix(fixer) {
return fixer.replaceTextRange(
[commentGroup[0].range[0], commentGroup[commentGroup.length - 1].range[1]],
`/*${convertToStarredBlock(commentGroup[0], commentLines)}*/`
);
}
});
} 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])} *`;

if (!/^\*?\s*$/.test(lines[0])) {
const start = block.value.startsWith("*") ? block.range[0] + 1 : block.range[0];

context.report({
loc: {
start: block.loc.start,
end: { line: block.loc.start.line, column: block.loc.start.column + 2 }
},
message: "Expected a linebreak after '/*'.",
fix: fixer => fixer.insertTextAfterRange([start, start + 2], `\n${expectedLinePrefix}`)
});
}

if (!/^\s*$/.test(lines[lines.length - 1])) {
context.report({
loc: {
start: { line: block.loc.end.line, column: block.loc.end.column - 2 },
end: block.loc.end
},
message: "Expected a linebreak before '*/'.",
fix: fixer => fixer.replaceTextRange([block.range[1] - 2, block.range[1]], `\n${expectedLinePrefix}/`)
});
}

// TODO: clean this up
for (let lineNumber = block.loc.start.line + 1; lineNumber <= block.loc.end.line; lineNumber++) {
const lineText = sourceCode.lines[lineNumber - 1];

if (!lineText.startsWith(expectedLinePrefix)) {
context.report({
loc: {
start: { line: lineNumber, column: 0 },
end: { line: lineNumber, column: sourceCode.lines[lineNumber - 1].length }
},
message: /^\s*\*/.test(lineText)
? "Expected this line to be aligned with the start of the comment."
: "Expected a '*' at the start of this line.",
fix(fixer) {

// TODO: Make this more readable, possibly by splitting it into two separate cases.
const lineStartIndex = sourceCode.getIndexFromLoc({ line: lineNumber, column: 0 });
const commentStartIndex = lineStartIndex + lineText.match(/^\s*\*? ?/)[0].length;
const replacementText = lineNumber === block.loc.end.line ? expectedLinePrefix : `${expectedLinePrefix} `;

return fixer.replaceTextRange([lineStartIndex, commentStartIndex], replacementText);
}
});
}
}
}
},
"separate-lines"() {

// TODO
}
};

//----------------------------------------------------------------------
// Public
//----------------------------------------------------------------------

return {
Program() {
return sourceCode.getAllComments()
.filter(comment => comment.type !== "Shebang")
.filter(comment => !astUtils.COMMENTS_IGNORE_PATTERN.test(comment.value))
.filter(comment => {
const tokenBefore = sourceCode.getTokenBefore(comment, { includeComments: true });

return !tokenBefore || tokenBefore.loc.end.line < comment.loc.start.line;
})
.reduce((commentGroups, comment, index, commentList) => {
if (
comment.type === "Line" &&
index && commentList[index - 1].type === "Line" &&
sourceCode.getTokenBefore(comment, { includeComments: true }) === commentList[index - 1]
) {
commentGroups[commentGroups.length - 1].push(comment);
} else {
commentGroups.push([comment]);
}

return commentGroups;
}, [])
.filter(commentGroup => !(commentGroup.length === 1 && commentGroup[0].loc.start.line === commentGroup[0].loc.end.line))
.forEach(commentGroupCheckers[option]);
}
};
}
};

0 comments on commit a8d0931

Please sign in to comment.