Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Add fixer and new option to commentFormatRule #3583

Merged
merged 6 commits into from Jun 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion .vscode/settings.json
Expand Up @@ -25,5 +25,6 @@

// Always use project's provided typescript compiler version
"typescript.tsdk": "node_modules/typescript/lib",
"files.eol": "\n"
"files.eol": "\n",
"git.ignoreLimitWarning": true
}
191 changes: 147 additions & 44 deletions src/rules/commentFormatRule.ts
Expand Up @@ -32,6 +32,17 @@ interface Options {
case: Case;
exceptions?: RegExp;
failureSuffix: string;
allowTrailingLowercase: boolean;
}

interface CommentStatus {
text: string;
start: number;
leadingSpaceError: boolean;
uppercaseError: boolean;
lowercaseError: boolean;
firstLetterPos: number;
validForTrailingLowercase: boolean;
}

const enum Case {
Expand All @@ -43,6 +54,7 @@ const enum Case {
const OPTION_SPACE = "check-space";
const OPTION_LOWERCASE = "check-lowercase";
const OPTION_UPPERCASE = "check-uppercase";
const OPTION_ALLOW_TRAILING_LOWERCASE = "allow-trailing-lowercase";

export class Rule extends Lint.Rules.AbstractRule {
/* tslint:disable:object-literal-sort-keys */
Expand All @@ -51,28 +63,37 @@ export class Rule extends Lint.Rules.AbstractRule {
description: "Enforces formatting rules for single-line comments.",
rationale: "Helps maintain a consistent, readable style in your codebase.",
optionsDescription: Lint.Utils.dedent`
Three arguments may be optionally provided:
Four arguments may be optionally provided:

* \`"check-space"\` requires that all single-line comments must begin with a space, as in \`// comment\`
* \`"${OPTION_SPACE}"\` requires that all single-line comments must begin with a space, as in \`// comment\`
* note that for comments starting with multiple slashes, e.g. \`///\`, leading slashes are ignored
* TypeScript reference comments are ignored completely
* \`"check-lowercase"\` requires that the first non-whitespace character of a comment must be lowercase, if applicable.
* \`"check-uppercase"\` requires that the first non-whitespace character of a comment must be uppercase, if applicable.
* \`"${OPTION_LOWERCASE}"\` requires that the first non-whitespace character of a comment must be lowercase, if applicable.
* \`"${OPTION_UPPERCASE}"\` requires that the first non-whitespace character of a comment must be uppercase, if applicable.
* \`"${OPTION_ALLOW_TRAILING_LOWERCASE}"\` allows that only the first comment of a series of comments needs to be uppercase.
* requires \`"${OPTION_UPPERCASE}"\`
* comments must start at the same position

Exceptions to \`"check-lowercase"\` or \`"check-uppercase"\` can be managed with object that may be passed as last argument.
Exceptions to \`"${OPTION_LOWERCASE}"\` or \`"${OPTION_UPPERCASE}"\` can be managed with object that may be passed as last
argument.

One of two options can be provided in this object:

* \`"ignore-words"\` - array of strings - words that will be ignored at the beginning of the comment.
* \`"ignore-pattern"\` - string - RegExp pattern that will be ignored at the beginning of the comment.
* \`"ignore-words"\` - array of strings - words that will be ignored at the beginning of the comment.
* \`"ignore-pattern"\` - string - RegExp pattern that will be ignored at the beginning of the comment.
`,
options: {
type: "array",
items: {
anyOf: [
{
type: "string",
enum: ["check-space", "check-lowercase", "check-uppercase"],
enum: [
OPTION_SPACE,
OPTION_LOWERCASE,
OPTION_UPPERCASE,
OPTION_ALLOW_TRAILING_LOWERCASE,
],
},
{
type: "object",
Expand All @@ -93,12 +114,12 @@ export class Rule extends Lint.Rules.AbstractRule {
],
},
minLength: 1,
maxLength: 4,
maxLength: 5,
},
optionExamples: [
[true, "check-space", "check-uppercase"],
[true, "check-lowercase", { "ignore-words": ["TODO", "HACK"] }],
[true, "check-lowercase", { "ignore-pattern": "STD\\w{2,3}\\b" }],
[true, OPTION_SPACE, OPTION_UPPERCASE, OPTION_ALLOW_TRAILING_LOWERCASE],
[true, OPTION_LOWERCASE, { "ignore-words": ["TODO", "HACK"] }],
[true, OPTION_LOWERCASE, { "ignore-pattern": "STD\\w{2,3}\\b" }],
],
type: "style",
typescriptOnly: false,
Expand All @@ -107,30 +128,42 @@ export class Rule extends Lint.Rules.AbstractRule {
/* tslint:enable:object-literal-sort-keys */

public static LOWERCASE_FAILURE = "comment must start with lowercase letter";
public static SPACE_LOWERCASE_FAILURE = "comment must start with a space and lowercase letter";
public static UPPERCASE_FAILURE = "comment must start with uppercase letter";
public static SPACE_UPPERCASE_FAILURE = "comment must start with a space and uppercase letter";
public static LEADING_SPACE_FAILURE = "comment must start with a space";
public static IGNORE_WORDS_FAILURE_FACTORY = (words: string[]): string =>
` or the word(s): ${words.join(", ")}`;
public static IGNORE_PATTERN_FAILURE_FACTORY = (pattern: string): string =>
` or its start must match the regex pattern "${pattern}"`;

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
return this.applyWithFunction(sourceFile, walk, parseOptions(this.ruleArguments));
const commentFormatWalker = new CommentFormatWalker(
sourceFile,
this.ruleName,
parseOptions(this.ruleArguments),
);
return this.applyWithWalker(commentFormatWalker);
}
}

function parseOptions(options: Array<string | IExceptionsObject>): Options {
return {
allowTrailingLowercase: has(OPTION_ALLOW_TRAILING_LOWERCASE),
case:
options.indexOf(OPTION_LOWERCASE) !== -1
? Case.Lower
: options.indexOf(OPTION_UPPERCASE) !== -1
? Case.Upper
: Case.None,
failureSuffix: "",
space: options.indexOf(OPTION_SPACE) !== -1,
space: has(OPTION_SPACE),
...composeExceptions(options[options.length - 1]),
};

function has(option: string): boolean {
return options.indexOf(option) !== -1;
}
}

function composeExceptions(
Expand Down Expand Up @@ -159,57 +192,127 @@ function composeExceptions(
return undefined;
}

function walk(ctx: Lint.WalkContext<Options>) {
utils.forEachComment(ctx.sourceFile, (fullText, { kind, pos, end }) => {
let start = pos + 2;
class CommentFormatWalker extends Lint.AbstractWalker<Options> {
private prevComment: ts.LineAndCharacter | undefined;
private prevCommentIsValid: boolean | undefined;

public walk(sourceFile: ts.SourceFile) {
utils.forEachComment(sourceFile, (fullText, comment) => {
const commentStatus = this.checkComment(fullText, comment);
this.handleFailure(commentStatus, comment.end);
// cache position of last comment
this.prevComment = ts.getLineAndCharacterOfPosition(sourceFile, comment.pos);
this.prevCommentIsValid = commentStatus.validForTrailingLowercase;
});
}

private checkComment(fullText: string, { kind, pos, end }: ts.CommentRange): CommentStatus {
const status: CommentStatus = {
firstLetterPos: -1,
leadingSpaceError: false,
lowercaseError: false,
start: pos + 2,
text: "",
uppercaseError: false,
validForTrailingLowercase: false,
};

if (
kind !== ts.SyntaxKind.SingleLineCommentTrivia ||
// exclude empty comments
start === end ||
status.start === end ||
// exclude /// <reference path="...">
(fullText[start] === "/" &&
ctx.sourceFile.referencedFiles.some(ref => ref.pos >= pos && ref.end <= end))
(fullText[status.start] === "/" &&
this.sourceFile.referencedFiles.some(ref => ref.pos >= pos && ref.end <= end))
) {
return;
return status;
}

// skip all leading slashes
while (fullText[start] === "/") {
++start;
while (fullText[status.start] === "/") {
++status.start;
}
if (start === end) {
return;
if (status.start === end) {
return status;
}
const commentText = fullText.slice(start, end);
status.text = fullText.slice(status.start, end);
// whitelist //#region and //#endregion and JetBrains IDEs' "//noinspection ...", "//region", "//endregion"
if (/^(?:#?(?:end)?region|noinspection\s)/.test(commentText)) {
return;
if (/^(?:#?(?:end)?region|noinspection\s)/.test(status.text)) {
return status;
}

if (ctx.options.space && commentText[0] !== " ") {
ctx.addFailure(start, end, Rule.LEADING_SPACE_FAILURE, [
Lint.Replacement.appendText(start, " "),
]);
if (this.options.space && status.text[0] !== " ") {
status.leadingSpaceError = true;
}

if (
ctx.options.case === Case.None ||
(ctx.options.exceptions !== undefined && ctx.options.exceptions.test(commentText)) ||
ENABLE_DISABLE_REGEX.test(commentText)
this.options.case === Case.None ||
(this.options.exceptions !== undefined && this.options.exceptions.test(status.text)) ||
ENABLE_DISABLE_REGEX.test(status.text)
) {
return;
return status;
}

// search for first non-space character to check if lower or upper
const charPos = commentText.search(/\S/);
const charPos = status.text.search(/\S/);
if (charPos === -1) {
return;
return status;
}
if (ctx.options.case === Case.Lower) {
if (!isLowerCase(commentText[charPos])) {
ctx.addFailure(start, end, Rule.LOWERCASE_FAILURE + ctx.options.failureSuffix);
// All non-empty and not whitelisted comments are valid for the trailing lowercase rule
status.validForTrailingLowercase = true;
status.firstLetterPos = charPos;
if (this.options.case === Case.Lower && !isLowerCase(status.text[charPos])) {
status.lowercaseError = true;
} else if (this.options.case === Case.Upper && !isUpperCase(status.text[charPos])) {
status.uppercaseError = true;
if (
this.options.allowTrailingLowercase &&
this.prevComment !== undefined &&
this.prevCommentIsValid
) {
const currentComment = ts.getLineAndCharacterOfPosition(this.sourceFile, pos);
if (
this.prevComment.line + 1 === currentComment.line &&
this.prevComment.character === currentComment.character
) {
status.uppercaseError = false;
}
}
} else if (!isUpperCase(commentText[charPos])) {
ctx.addFailure(start, end, Rule.UPPERCASE_FAILURE + ctx.options.failureSuffix);
}
});
return status;
}

private handleFailure(status: CommentStatus, end: number) {
// No failure detected
if (!status.leadingSpaceError && !status.lowercaseError && !status.uppercaseError) {
return;
}

// Only whitespace failure
if (status.leadingSpaceError && !status.lowercaseError && !status.uppercaseError) {
this.addFailure(
status.start,
end,
Rule.LEADING_SPACE_FAILURE,
Lint.Replacement.appendText(status.start, " "),
);
return;
}

let msg: string;
let firstLetterFix: string;

if (status.lowercaseError) {
msg = status.leadingSpaceError ? Rule.SPACE_LOWERCASE_FAILURE : Rule.LOWERCASE_FAILURE;
firstLetterFix = status.text[status.firstLetterPos].toLowerCase();
} else {
msg = status.leadingSpaceError ? Rule.SPACE_UPPERCASE_FAILURE : Rule.UPPERCASE_FAILURE;
firstLetterFix = status.text[status.firstLetterPos].toUpperCase();
}

const fix = status.leadingSpaceError
? new Lint.Replacement(status.start, 1, ` ${firstLetterFix}`)
: new Lint.Replacement(status.start + status.firstLetterPos, 1, firstLetterFix);
this.addFailure(status.start, end, msg + this.options.failureSuffix, fix);
}
}
15 changes: 15 additions & 0 deletions test/rules/comment-format/allow-trailing-lowercase/test.ts.fix
@@ -0,0 +1,15 @@
// This is a valid
// multiline comment

// This is an invalid
// multiline

let a = 1; // Some description
let b = "foo"; // Another description

// Another multiline
// comment ..
//
// With multiple
// paragraphs

20 changes: 20 additions & 0 deletions test/rules/comment-format/allow-trailing-lowercase/test.ts.lint
@@ -0,0 +1,20 @@
// This is a valid
// multiline comment

// This is an invalid
//multiline
~~~~~~~~~[space]

let a = 1; // Some description
let b = "foo"; // another description
~~~~~~~~~~~~~~~~~~~~[upper]

// Another multiline
// comment ..
//
// with multiple
~~~~~~~~~~~~~~[upper]
// paragraphs

[upper]: comment must start with uppercase letter
[space]: comment must start with a space
@@ -0,0 +1,5 @@
{
"rules": {
"comment-format": [true, "check-space", "check-uppercase", "allow-trailing-lowercase"]
}
}
8 changes: 4 additions & 4 deletions test/rules/comment-format/exceptions-pattern/test.ts.fix
Expand Up @@ -3,8 +3,8 @@ class Clazz { // this comment is correct
/* block comment
* adada
*/
public funcxion() { // This comment has a capital letter starting it
// This comment is on its own line, and starts with a capital _and_ no space
public funcxion() { // this comment has a capital letter starting it
// this comment is on its own line, and starts with a capital _and_ no space
console.log("test"); // this comment has no space
}
/// <not a reference/>
Expand All @@ -19,8 +19,8 @@ class Clazz { // this comment is correct
//noinspection JSUnusedGlobalSymbols
const unusedVar = 'unneeded value';

// TODO: Write more tests
// HACKING is not an exception
// tODO: Write more tests
// hACKING is not an exception

// STDIN for input
// STDOUT for output
Expand Down
4 changes: 2 additions & 2 deletions test/rules/comment-format/exceptions-pattern/test.ts.lint
Expand Up @@ -6,8 +6,7 @@ class Clazz { // this comment is correct
public funcxion() { // This comment has a capital letter starting it
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [lower]
//This comment is on its own line, and starts with a capital _and_ no space
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [lower]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [space]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [space_lower]
console.log("test"); //this comment has no space
~~~~~~~~~~~~~~~~~~~~~~~~~ [space]
}
Expand Down Expand Up @@ -37,3 +36,4 @@ const unusedVar = 'unneeded value';

[lower]: comment must start with lowercase letter or its start must match the regex pattern "STD\w{2,3}"
[space]: comment must start with a space
[space_lower]: comment must start with a space and lowercase letter or its start must match the regex pattern "STD\w{2,3}"