Add fixer and new option to commentFormatRule (#3583)

* Change commentFormatRule to only emit one message per error

* Add fixer

* Add allow-trailing-lowercase option

* Changes after review: cache last comment, rework on structure

* Make allow-trailing-lowercase a bit stricter

.vscode/settings.json

@@ -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
 }

src/rules/commentFormatRule.ts

@@ -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 {
@@ -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 */
@@ -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",
@@ -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,
@@ -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(
@@ -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);
+    }
 }

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
+

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

test/rules/comment-format/allow-trailing-lowercase/tslint.json

@@ -0,0 +1,5 @@
+{
+  "rules": {
+    "comment-format": [true, "check-space", "check-uppercase", "allow-trailing-lowercase"]
+  }
+}

test/rules/comment-format/exceptions-pattern/test.ts.fix

@@ -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/>
@@ -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

test/rules/comment-format/exceptions-pattern/test.ts.lint

@@ -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]
     }
@@ -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}"