Add regex option to no-suspicious-comment rule (#631)

* Fix #440: Add regex option to no-suspicious-comment * Fix #440 Mention regex in failure message * #631 Mark 'exceptionRegex' as readonly
microsoft · Nov 5, 2018 · d4b50a0 · d4b50a0
1 parent 173c226
commit d4b50a0
Show file tree

Hide file tree

Showing 2 changed files with 91 additions and 5 deletions.
diff --git a/src/noSuspiciousCommentRule.ts b/src/noSuspiciousCommentRule.ts
@@ -1,20 +1,28 @@
 import * as ts from 'typescript';
 import * as Lint from 'tslint';
 
-import {forEachTokenWithTrivia} from 'tsutils';
-import {ExtendedMetadata} from './utils/ExtendedMetadata';
+import { forEachTokenWithTrivia } from 'tsutils';
+import { ExtendedMetadata } from './utils/ExtendedMetadata';
 
 const FAILURE_STRING: string = 'Suspicious comment found: ';
 const SUSPICIOUS_WORDS = ['BUG', 'HACK', 'FIXME', 'LATER', 'LATER2', 'TODO'];
+const FAILURE_STRING_OPTION: string = '\nTo disable this warning, the comment should include one of the following regex: ';
 
 export class Rule extends Lint.Rules.AbstractRule {
 
     public static metadata: ExtendedMetadata = {
         ruleName: 'no-suspicious-comment',
         type: 'maintainability',
         description: `Do not use suspicious comments, such as ${SUSPICIOUS_WORDS.join(', ')}`,
-        options: null, // tslint:disable-line:no-null-keyword
-        optionsDescription: '',
+        options: {
+            type: 'array',
+            items: {
+                type: 'string'
+            }
+        },
+        optionsDescription: `One argument may be optionally provided: \n\n' +
+            '* an array of regex that disable the warning if one or several of them
+            are found in the comment text. (Example: \`['https://github.com/my-org/my-project/*']\`)`,
         typescriptOnly: true,
         issueClass: 'Non-SDL',
         issueType: 'Warning',
@@ -31,6 +39,17 @@ export class Rule extends Lint.Rules.AbstractRule {
 
 class NoSuspiciousCommentRuleWalker extends Lint.RuleWalker {
 
+    private readonly exceptionRegex: RegExp[] = [];
+
+    constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) {
+        super(sourceFile, options);
+        if (options.ruleArguments !== undefined && options.ruleArguments.length > 0) {
+            options.ruleArguments.forEach((regexStr: string) => {
+                this.exceptionRegex.push(new RegExp(regexStr));
+            });
+        }
+    }
+
     public visitSourceFile(node: ts.SourceFile) {
         forEachTokenWithTrivia(node, (text, tokenSyntaxKind, range) => {
             if (tokenSyntaxKind === ts.SyntaxKind.SingleLineCommentTrivia ||
@@ -41,6 +60,9 @@ class NoSuspiciousCommentRuleWalker extends Lint.RuleWalker {
     }
 
     private scanCommentForSuspiciousWords(startPosition: number, commentText: string): void {
+        if (this.commentContainsExceptionRegex(this.exceptionRegex, commentText)) {
+            return;
+        }
         SUSPICIOUS_WORDS.forEach((suspiciousWord: string) => {
             this.scanCommentForSuspiciousWord(suspiciousWord, commentText, startPosition);
         });
@@ -55,7 +77,27 @@ class NoSuspiciousCommentRuleWalker extends Lint.RuleWalker {
     }
 
     private foundSuspiciousComment(startPosition: number, commentText: string, suspiciousWord: string) {
-        const errorMessage: string = FAILURE_STRING + suspiciousWord;
+        let errorMessage: string = FAILURE_STRING + suspiciousWord;
+        if (this.exceptionRegex.length > 0) {
+            errorMessage += '.' + this.getFailureMessageWithExceptionRegexOption();
+        }
         this.addFailureAt(startPosition, commentText.length, errorMessage);
     }
+
+    private commentContainsExceptionRegex(exceptionRegex: RegExp[], commentText: string): boolean {
+        for (const regex of exceptionRegex) {
+            if (regex.test(commentText)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    private getFailureMessageWithExceptionRegexOption(): string {
+        let message: string = FAILURE_STRING_OPTION;
+        this.exceptionRegex.forEach((regex: RegExp) => {
+            message += regex.toString();
+        });
+        return message;
+    }
 }
diff --git a/src/tests/NoSuspiciousCommentRuleTests.ts b/src/tests/NoSuspiciousCommentRuleTests.ts
@@ -7,13 +7,15 @@ import {TestHelper} from './TestHelper';
 describe('noSuspiciousCommentRule', (): void => {
 
     const ruleName: string = 'no-suspicious-comment';
+    const option: string[]  = ['https://example.com/*'];
 
     it('should pass on normal comments', (): void => {
         const script: string = `
             // this comment is not suspicious
         `;
 
         TestHelper.assertViolations(ruleName, script, []);
+        TestHelper.assertViolationsWithOptions(ruleName, option, script, []);
     });
 
     it('should pass on multi-line comments', (): void => {
@@ -53,6 +55,17 @@ describe('noSuspiciousCommentRule', (): void => {
         }]);
     });
 
+    it('should pass on multiline TODO comments with regex option', (): void => {
+        const script: string = `
+            /**
+            * TODO: add failing example and update assertions
+            * See https://example.com/article/123
+            */
+        `;
+
+        TestHelper.assertViolationsWithOptions(ruleName, option, script, []);
+    });
+
     it('should pass on lower case todo comments without colons', (): void => {
         const script: string = `
             // todo add failing example and update assertions
@@ -76,6 +89,21 @@ describe('noSuspiciousCommentRule', (): void => {
                 "ruleName": "no-suspicious-comment",
                 "startPosition": {"character": 17, "line": 2}
             }]);
+            TestHelper.assertViolationsWithOptions(ruleName, option, script, [{
+                "failure": `Suspicious comment found: ${suspiciousWord}.
+To disable this warning, the comment should include one of the following regex: /https:\\/\\/example.com\\/*/`,
+                "name": Utils.absolutePath("file.ts"),
+                "ruleName": "no-suspicious-comment",
+                "startPosition": {"character": 17, "line": 2}
+            }]);
+        });
+
+        it(`should pass on upper case ${suspiciousWord} comments without colons and with regex option`, (): void => {
+            const script: string = `
+                // ${suspiciousWord} you should fix this https://example.com/article/123
+            `;
+
+            TestHelper.assertViolationsWithOptions(ruleName, option, script, []);
         });
 
         it(`should fail on upper case ${suspiciousWord} comments with colons`, (): void => {
@@ -91,6 +119,14 @@ describe('noSuspiciousCommentRule', (): void => {
             }]);
         });
 
+        it(`should pass on upper case ${suspiciousWord} comments with colons and with regex option`, (): void => {
+            const script: string = `
+                // ${suspiciousWord}: you should fix this (see https://example.com/article/123)
+            `;
+
+            TestHelper.assertViolationsWithOptions(ruleName, option, script, []);
+        });
+
         it(`should fail on lower case ${suspiciousWord} comments with colons`, (): void => {
             const script: string = `
                 // ${suspiciousWord}: you should fix this
@@ -103,6 +139,14 @@ describe('noSuspiciousCommentRule', (): void => {
                 "startPosition": {"character": 17, "line": 2}
             }]);
         });
+
+        it(`should pass on lower case ${suspiciousWord} comments with colons and with regex option`, (): void => {
+            const script: string = `
+                // ${suspiciousWord}: you should fix this  (see https://example.com/article/123)
+            `;
+
+            TestHelper.assertViolationsWithOptions(ruleName, option, script, []);
+        });
     });
 });
 /* tslint:enable:quotemark */