From f8c43dc600f7d544dfe209330fbfe80f15b7745d Mon Sep 17 00:00:00 2001 From: Andrii Dieiev Date: Sun, 20 Nov 2016 04:06:14 +0200 Subject: [PATCH 1/4] Add exceptions support for comment-format rule (fixes #562) --- src/rules/commentFormatRule.ts | 44 +++++++++++++++---- .../comment-format/exceptions/test.js.lint | 32 ++++++++++++++ .../comment-format/exceptions/test.ts.lint | 30 +++++++++++++ .../comment-format/exceptions/tslint.json | 8 ++++ test/rules/comment-format/lower/test.js.lint | 2 +- test/rules/comment-format/lower/test.ts.lint | 2 +- test/rules/comment-format/upper/test.ts.lint | 2 +- 7 files changed, 109 insertions(+), 11 deletions(-) create mode 100644 test/rules/comment-format/exceptions/test.js.lint create mode 100644 test/rules/comment-format/exceptions/test.ts.lint create mode 100644 test/rules/comment-format/exceptions/tslint.json diff --git a/src/rules/commentFormatRule.ts b/src/rules/commentFormatRule.ts index e65a16b3fc3..d3c1a42a14c 100644 --- a/src/rules/commentFormatRule.ts +++ b/src/rules/commentFormatRule.ts @@ -39,20 +39,25 @@ export class Rule extends Lint.Rules.AbstractRule { options: { type: "array", items: { - type: "string", - enum: ["check-space", "check-lowercase", "check-uppercase"], + oneOf: [{ + type: "string", + enum: ["check-space", "check-lowercase", "check-uppercase"], + }, { + type: "array", + items: { type: "string" }, + }], }, minLength: 1, - maxLength: 3, + maxLength: 4, }, - optionExamples: ['[true, "check-space", "check-lowercase"]'], + optionExamples: ['[true, "check-space", "check-uppercase"], [true, "check-lowercase", ["TODO", "HACK"]]'], type: "style", typescriptOnly: false, }; /* tslint:enable:object-literal-sort-keys */ - public static LOWERCASE_FAILURE = "comment must start with lowercase letter"; - public static UPPERCASE_FAILURE = "comment must start with uppercase letter"; + public static LOWERCASE_FAILURE = "comment must start with lowercase letter or word from exceptions list"; + public static UPPERCASE_FAILURE = "comment must start with uppercase letter or word from exceptions list"; public static LEADING_SPACE_FAILURE = "comment must start with a space"; public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { @@ -61,6 +66,22 @@ export class Rule extends Lint.Rules.AbstractRule { } class CommentWalker extends Lint.SkippableTokenAwareRuleWalker { + private exceptionsPattern: RegExp | null; + + constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) { + super(sourceFile, options); + + const optionsList = this.getOptions() as Array; + const possibleExceptions = optionsList[optionsList.length - 1]; + if (Array.isArray(possibleExceptions)) { + const wordsPattern = possibleExceptions.map(String).join("|"); + // regex is "start of string"//"any amount of whitespace"("any word from exception list") followed by non alphanumeric character + this.exceptionsPattern = new RegExp(`^\\/\\/\\s*(${wordsPattern})\\W`); + } else { + this.exceptionsPattern = null; + } + } + public visitSourceFile(node: ts.SourceFile) { super.visitSourceFile(node); Lint.scanAllTokens(ts.createScanner(ts.ScriptTarget.ES5, false, ts.LanguageVariant.Standard, node.text), (scanner: ts.Scanner) => { @@ -82,12 +103,12 @@ class CommentWalker extends Lint.SkippableTokenAwareRuleWalker { } } if (this.hasOption(OPTION_LOWERCASE)) { - if (!startsWithLowercase(commentText)) { + if (!startsWithLowercase(commentText) && !this.startsWithException(commentText)) { this.addFailureAt(startPosition, width, Rule.LOWERCASE_FAILURE); } } if (this.hasOption(OPTION_UPPERCASE)) { - if (!startsWithUppercase(commentText) && !isEnableDisableFlag(commentText)) { + if (!startsWithUppercase(commentText) && !isEnableDisableFlag(commentText) && !this.startsWithException(commentText)) { this.addFailureAt(startPosition, width, Rule.UPPERCASE_FAILURE); } } @@ -95,6 +116,13 @@ class CommentWalker extends Lint.SkippableTokenAwareRuleWalker { }); } + private startsWithException(commentText: string): boolean { + if (this.exceptionsPattern == null) { + return false; + } + + return this.exceptionsPattern.test(commentText); + } } function startsWith(commentText: string, changeCase: (str: string) => string) { diff --git a/test/rules/comment-format/exceptions/test.js.lint b/test/rules/comment-format/exceptions/test.js.lint new file mode 100644 index 00000000000..c2a6109917c --- /dev/null +++ b/test/rules/comment-format/exceptions/test.js.lint @@ -0,0 +1,32 @@ +class Clazz { // This comment is correct + /* block comment + * adada + */ + public funcxion() { // this comment has a lowercase letter starting it + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [upper] + //this comment is on its own line, and starts with a lowercase _and_ no space + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [upper] + console.log("test"); //This comment has no space + } + /// +} + +//#region test +//#endregion + +`${location.protocol}//${location.hostname}` + +// tslint should show error here + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [upper] + +// tslint: not a rule flag + ~~~~~~~~~~~~~~~~~~~~~~~~ [upper] + +class Invalid {} + +// tslint:disable-next-line:no-unused-expression +class Valid {} + +// todo write more tests + +[upper]: comment must start with uppercase letter or word from exceptions list diff --git a/test/rules/comment-format/exceptions/test.ts.lint b/test/rules/comment-format/exceptions/test.ts.lint new file mode 100644 index 00000000000..47c80bf2a0b --- /dev/null +++ b/test/rules/comment-format/exceptions/test.ts.lint @@ -0,0 +1,30 @@ +class Clazz { // this comment is correct + /* block comment + * adada + */ + 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] + console.log("test"); //this comment has no space + ~~~~~~~~~~~~~~~~~~~~~~~~~ [space] + } + /// +} + +//#region test +//#endregion + +`${location.protocol}//${location.hostname}` + +//noinspection JSUnusedGlobalSymbols +const unusedVar = 'unneeded value'; + +// TODO: Write more tests + +// HACKING is not an exception + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [lower] + +[lower]: comment must start with lowercase letter or word from exceptions list +[space]: comment must start with a space diff --git a/test/rules/comment-format/exceptions/tslint.json b/test/rules/comment-format/exceptions/tslint.json new file mode 100644 index 00000000000..e632889f489 --- /dev/null +++ b/test/rules/comment-format/exceptions/tslint.json @@ -0,0 +1,8 @@ +{ + "rules": { + "comment-format": [true, "check-space", "check-lowercase", ["TODO", "HACK"]] + }, + "jsRules": { + "comment-format": [true, "check-uppercase", ["todo"]] + } +} diff --git a/test/rules/comment-format/lower/test.js.lint b/test/rules/comment-format/lower/test.js.lint index 6b0f7e4cfb2..80c4efeb429 100644 --- a/test/rules/comment-format/lower/test.js.lint +++ b/test/rules/comment-format/lower/test.js.lint @@ -21,5 +21,5 @@ class Clazz { // this comment is correct //noinspection JSUnusedGlobalSymbols const unusedVar = 'unneeded value'; -[lower]: comment must start with lowercase letter +[lower]: comment must start with lowercase letter or word from exceptions list [space]: comment must start with a space diff --git a/test/rules/comment-format/lower/test.ts.lint b/test/rules/comment-format/lower/test.ts.lint index 6b0f7e4cfb2..80c4efeb429 100644 --- a/test/rules/comment-format/lower/test.ts.lint +++ b/test/rules/comment-format/lower/test.ts.lint @@ -21,5 +21,5 @@ class Clazz { // this comment is correct //noinspection JSUnusedGlobalSymbols const unusedVar = 'unneeded value'; -[lower]: comment must start with lowercase letter +[lower]: comment must start with lowercase letter or word from exceptions list [space]: comment must start with a space diff --git a/test/rules/comment-format/upper/test.ts.lint b/test/rules/comment-format/upper/test.ts.lint index c94e6226882..c922ca68d9e 100644 --- a/test/rules/comment-format/upper/test.ts.lint +++ b/test/rules/comment-format/upper/test.ts.lint @@ -31,5 +31,5 @@ class Invalid {} // tslint:disable-next-line:no-unused-expression class Valid {} -[upper]: comment must start with uppercase letter +[upper]: comment must start with uppercase letter or word from exceptions list [space]: comment must start with a space From c8a03f440d79491cf578bdd6b78b0db3be635899 Mon Sep 17 00:00:00 2001 From: Andrii Dieiev Date: Sun, 11 Dec 2016 02:43:13 +0200 Subject: [PATCH 2/4] Add support for RexExp pattern in comment-format rule exceptions (fixes #562) --- src/rules/commentFormatRule.ts | 106 ++++++++++++++---- src/utils.ts | 7 ++ .../exceptions-pattern/test.js.lint | 38 +++++++ .../test.ts.lint | 10 +- .../exceptions-pattern/tslint.json | 8 ++ .../test.js.lint | 7 +- .../exceptions-words/test.ts.lint | 37 ++++++ .../exceptions-words/tslint.json | 8 ++ .../comment-format/exceptions/tslint.json | 8 -- test/rules/comment-format/lower/test.js.lint | 2 +- test/rules/comment-format/lower/test.ts.lint | 2 +- test/rules/comment-format/upper/test.ts.lint | 2 +- 12 files changed, 198 insertions(+), 37 deletions(-) create mode 100644 test/rules/comment-format/exceptions-pattern/test.js.lint rename test/rules/comment-format/{exceptions => exceptions-pattern}/test.ts.lint (83%) create mode 100644 test/rules/comment-format/exceptions-pattern/tslint.json rename test/rules/comment-format/{exceptions => exceptions-words}/test.js.lint (81%) create mode 100644 test/rules/comment-format/exceptions-words/test.ts.lint create mode 100644 test/rules/comment-format/exceptions-words/tslint.json delete mode 100644 test/rules/comment-format/exceptions/tslint.json diff --git a/src/rules/commentFormatRule.ts b/src/rules/commentFormatRule.ts index d3c1a42a14c..fb64d8b9fe9 100644 --- a/src/rules/commentFormatRule.ts +++ b/src/rules/commentFormatRule.ts @@ -18,6 +18,14 @@ import * as ts from "typescript"; import * as Lint from "../index"; +import { escapeRegExp } from "../utils"; + +interface IExceptionsObject { + ignoreWords?: string[]; + ignorePattern?: string[]; +} + +type ExceptionsRegExp = RegExp | null; const OPTION_SPACE = "check-space"; const OPTION_LOWERCASE = "check-lowercase"; @@ -35,29 +43,60 @@ export class Rule extends Lint.Rules.AbstractRule { * \`"check-space"\` requires that all single-line comments must begin with a space, as in \`// comment\` * note that comments starting with \`///\` are also allowed, for things such as \`///\` * \`"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.`, + * \`"check-uppercase"\` requires that the first non-whitespace character of a comment must be uppercase, if applicable. + + Exceptions to \`"check-lowercase"\` or \`"check-uppercase"\` can be managed with object that may be passed as last argument. + + One of two options can be provided in this object: + + * \`"ignoreWords"\` - array of strings - words that will be ignored at the beginning of the comment. + * \`"ignorePattern"\` - string - RegExp pattern that will be ignored at the beginning of the comment. + `, options: { type: "array", items: { - oneOf: [{ - type: "string", - enum: ["check-space", "check-lowercase", "check-uppercase"], - }, { - type: "array", - items: { type: "string" }, - }], + anyOf: [ + { + type: "string", + enum: [ + "check-space", + "check-lowercase", + "check-uppercase", + ], + }, + { + type: "object", + properties: { + ignoreWords: { + type: "array", + items: { + type: "string", + }, + }, + ignorePattern: { + type: "string", + }, + }, + minProperties: 1, + maxProperties: 1, + }, + ], }, minLength: 1, maxLength: 4, }, - optionExamples: ['[true, "check-space", "check-uppercase"], [true, "check-lowercase", ["TODO", "HACK"]]'], + optionExamples: [ + '[true, "check-space", "check-uppercase"]', + '[true, "check-lowercase", {"ignoreWords": ["TODO", "HACK"]}]', + '[true, "check-lowercase", {"ignorePattern": "STD\\w{2,3}\\b"}]', + ], type: "style", typescriptOnly: false, }; /* tslint:enable:object-literal-sort-keys */ - public static LOWERCASE_FAILURE = "comment must start with lowercase letter or word from exceptions list"; - public static UPPERCASE_FAILURE = "comment must start with uppercase letter or word from exceptions list"; + public static LOWERCASE_FAILURE = "comment must start with lowercase letter, word from exceptions list or exceptions pattern"; + public static UPPERCASE_FAILURE = "comment must start with uppercase letter, word from exceptions list or exceptions pattern"; public static LEADING_SPACE_FAILURE = "comment must start with a space"; public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { @@ -66,20 +105,12 @@ export class Rule extends Lint.Rules.AbstractRule { } class CommentWalker extends Lint.SkippableTokenAwareRuleWalker { - private exceptionsPattern: RegExp | null; + private exceptionsRegExp: ExceptionsRegExp; constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) { super(sourceFile, options); - const optionsList = this.getOptions() as Array; - const possibleExceptions = optionsList[optionsList.length - 1]; - if (Array.isArray(possibleExceptions)) { - const wordsPattern = possibleExceptions.map(String).join("|"); - // regex is "start of string"//"any amount of whitespace"("any word from exception list") followed by non alphanumeric character - this.exceptionsPattern = new RegExp(`^\\/\\/\\s*(${wordsPattern})\\W`); - } else { - this.exceptionsPattern = null; - } + this.exceptionsRegExp = this.composeExceptionsRegExp(); } public visitSourceFile(node: ts.SourceFile) { @@ -117,11 +148,40 @@ class CommentWalker extends Lint.SkippableTokenAwareRuleWalker { } private startsWithException(commentText: string): boolean { - if (this.exceptionsPattern == null) { + if (this.exceptionsRegExp == null) { return false; } - return this.exceptionsPattern.test(commentText); + return this.exceptionsRegExp.test(commentText); + } + + private composeExceptionsRegExp(): ExceptionsRegExp { + const optionsList = this.getOptions() as Array; + const exceptionsObject = optionsList[optionsList.length - 1]; + + // early return if last element is string instead of exceptions object + if (typeof exceptionsObject === "string" || !exceptionsObject) { + return null; + } + + if (exceptionsObject.ignorePattern) { + // regex is "start of string"//"any amount of whitespace" followed by user provided ignore pattern + return new RegExp(`^//\\s*(${exceptionsObject.ignorePattern})`); + } + + if (exceptionsObject.ignoreWords) { + // Converts all exceptions values to strings, trim whitespace, escapes RegExp special characters and combines into alternation + const wordsPattern = exceptionsObject.ignoreWords + .map(String) + .map((str) => str.trim()) + .map(escapeRegExp) + .join("|"); + + // regex is "start of string"//"any amount of whitespace"("any word from ignore list") followed by non alphanumeric character + return new RegExp(`^//\\s*(${wordsPattern})\\b`); + } + + return null; } } diff --git a/src/utils.ts b/src/utils.ts index 6dbf524e026..71ced1e92b0 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -92,3 +92,10 @@ export function stripComments(content: string): string { }); return result; }; + +/** + * Escapes all special characters in RegExp pattern to avoid broken regular expressions and ensure proper matches + */ +export function escapeRegExp(re: string): string { + return re.replace(/[.+*?|^$\[]{}()\\]/g, "$&"); +} diff --git a/test/rules/comment-format/exceptions-pattern/test.js.lint b/test/rules/comment-format/exceptions-pattern/test.js.lint new file mode 100644 index 00000000000..7dc99aaa7b3 --- /dev/null +++ b/test/rules/comment-format/exceptions-pattern/test.js.lint @@ -0,0 +1,38 @@ +class Clazz { // This comment is correct + /* block comment + * adada + */ + public funcxion() { // this comment has a lowercase letter starting it + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [upper] + //this comment is on its own line, and starts with a lowercase _and_ no space + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [upper] + console.log("test"); //This comment has no space + } + /// +} + +//#region test +//#endregion + +`${location.protocol}//${location.hostname}` + +// tslint should show error here + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [upper] + +// tslint: not a rule flag + ~~~~~~~~~~~~~~~~~~~~~~~~ [upper] + +class Invalid {} + +// tslint:disable-next-line:no-unused-expression +class Valid {} + +// todo write more tests + ~~~~~~~~~~~~~~~~~~~~~~ [upper] + +// STDIN for input +// STDOUT for output +// stderr for errors + + +[upper]: comment must start with uppercase letter, word from exceptions list or exceptions pattern diff --git a/test/rules/comment-format/exceptions/test.ts.lint b/test/rules/comment-format/exceptions-pattern/test.ts.lint similarity index 83% rename from test/rules/comment-format/exceptions/test.ts.lint rename to test/rules/comment-format/exceptions-pattern/test.ts.lint index 47c80bf2a0b..818932591e9 100644 --- a/test/rules/comment-format/exceptions/test.ts.lint +++ b/test/rules/comment-format/exceptions-pattern/test.ts.lint @@ -22,9 +22,15 @@ class Clazz { // this comment is correct const unusedVar = 'unneeded value'; // TODO: Write more tests - + ~~~~~~~~~~~~~~~~~~~~~~~ [lower] // HACKING is not an exception ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [lower] -[lower]: comment must start with lowercase letter or word from exceptions list +// STDIN for input +// STDOUT for output +// stderr for errors + + + +[lower]: comment must start with lowercase letter, word from exceptions list or exceptions pattern [space]: comment must start with a space diff --git a/test/rules/comment-format/exceptions-pattern/tslint.json b/test/rules/comment-format/exceptions-pattern/tslint.json new file mode 100644 index 00000000000..012c339dec3 --- /dev/null +++ b/test/rules/comment-format/exceptions-pattern/tslint.json @@ -0,0 +1,8 @@ +{ + "rules": { + "comment-format": [true, "check-space", "check-lowercase", {"ignorePattern": "STD\\w{2,3}"}] + }, + "jsRules": { + "comment-format": [true, "check-uppercase", {"ignorePattern": "std(in|out|err)\\b"}] + } +} diff --git a/test/rules/comment-format/exceptions/test.js.lint b/test/rules/comment-format/exceptions-words/test.js.lint similarity index 81% rename from test/rules/comment-format/exceptions/test.js.lint rename to test/rules/comment-format/exceptions-words/test.js.lint index c2a6109917c..b004c9e72f7 100644 --- a/test/rules/comment-format/exceptions/test.js.lint +++ b/test/rules/comment-format/exceptions-words/test.js.lint @@ -29,4 +29,9 @@ class Valid {} // todo write more tests -[upper]: comment must start with uppercase letter or word from exceptions list +// STDIN for input +// STDOUT for output +// stderr for errors + ~~~~~~~~~~~~~~~~~~ [upper] + +[upper]: comment must start with uppercase letter, word from exceptions list or exceptions pattern diff --git a/test/rules/comment-format/exceptions-words/test.ts.lint b/test/rules/comment-format/exceptions-words/test.ts.lint new file mode 100644 index 00000000000..ddd039b0431 --- /dev/null +++ b/test/rules/comment-format/exceptions-words/test.ts.lint @@ -0,0 +1,37 @@ +class Clazz { // this comment is correct + /* block comment + * adada + */ + 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] + console.log("test"); //this comment has no space + ~~~~~~~~~~~~~~~~~~~~~~~~~ [space] + } + /// +} + +//#region test +//#endregion + +`${location.protocol}//${location.hostname}` + +//noinspection JSUnusedGlobalSymbols +const unusedVar = 'unneeded value'; + +// TODO: Write more tests + +// HACKING is not an exception + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [lower] + +// STDIN for input + ~~~~~~~~~~~~~~~~ [lower] +// STDOUT for output + ~~~~~~~~~~~~~~~~~~ [lower] +// stderr for errors + + +[lower]: comment must start with lowercase letter, word from exceptions list or exceptions pattern +[space]: comment must start with a space diff --git a/test/rules/comment-format/exceptions-words/tslint.json b/test/rules/comment-format/exceptions-words/tslint.json new file mode 100644 index 00000000000..91aa3c20aac --- /dev/null +++ b/test/rules/comment-format/exceptions-words/tslint.json @@ -0,0 +1,8 @@ +{ + "rules": { + "comment-format": [true, "check-space", "check-lowercase", {"ignoreWords": ["TODO", "HACK"]}] + }, + "jsRules": { + "comment-format": [true, "check-uppercase", {"ignoreWords": ["todo"]}] + } +} diff --git a/test/rules/comment-format/exceptions/tslint.json b/test/rules/comment-format/exceptions/tslint.json deleted file mode 100644 index e632889f489..00000000000 --- a/test/rules/comment-format/exceptions/tslint.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "rules": { - "comment-format": [true, "check-space", "check-lowercase", ["TODO", "HACK"]] - }, - "jsRules": { - "comment-format": [true, "check-uppercase", ["todo"]] - } -} diff --git a/test/rules/comment-format/lower/test.js.lint b/test/rules/comment-format/lower/test.js.lint index 80c4efeb429..55d8b06a647 100644 --- a/test/rules/comment-format/lower/test.js.lint +++ b/test/rules/comment-format/lower/test.js.lint @@ -21,5 +21,5 @@ class Clazz { // this comment is correct //noinspection JSUnusedGlobalSymbols const unusedVar = 'unneeded value'; -[lower]: comment must start with lowercase letter or word from exceptions list +[lower]: comment must start with lowercase letter, word from exceptions list or exceptions pattern [space]: comment must start with a space diff --git a/test/rules/comment-format/lower/test.ts.lint b/test/rules/comment-format/lower/test.ts.lint index 80c4efeb429..55d8b06a647 100644 --- a/test/rules/comment-format/lower/test.ts.lint +++ b/test/rules/comment-format/lower/test.ts.lint @@ -21,5 +21,5 @@ class Clazz { // this comment is correct //noinspection JSUnusedGlobalSymbols const unusedVar = 'unneeded value'; -[lower]: comment must start with lowercase letter or word from exceptions list +[lower]: comment must start with lowercase letter, word from exceptions list or exceptions pattern [space]: comment must start with a space diff --git a/test/rules/comment-format/upper/test.ts.lint b/test/rules/comment-format/upper/test.ts.lint index c922ca68d9e..553b2664664 100644 --- a/test/rules/comment-format/upper/test.ts.lint +++ b/test/rules/comment-format/upper/test.ts.lint @@ -31,5 +31,5 @@ class Invalid {} // tslint:disable-next-line:no-unused-expression class Valid {} -[upper]: comment must start with uppercase letter or word from exceptions list +[upper]: comment must start with uppercase letter, word from exceptions list or exceptions pattern [space]: comment must start with a space From 6fe20ce103dd9647dfc7712a44987c6e70ab2b2c Mon Sep 17 00:00:00 2001 From: Andrii Dieiev Date: Mon, 12 Dec 2016 22:46:04 +0200 Subject: [PATCH 3/4] Fix escapeRegExp and add test --- src/utils.ts | 2 +- test/utilsTests.ts | 13 ++++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index 71ced1e92b0..8820f3cdd43 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -97,5 +97,5 @@ export function stripComments(content: string): string { * Escapes all special characters in RegExp pattern to avoid broken regular expressions and ensure proper matches */ export function escapeRegExp(re: string): string { - return re.replace(/[.+*?|^$\[]{}()\\]/g, "$&"); + return re.replace(/[.+*?|^$[\]{}()\\]/g, "\\$&"); } diff --git a/test/utilsTests.ts b/test/utilsTests.ts index 52206e3d151..37cfbcee5e3 100644 --- a/test/utilsTests.ts +++ b/test/utilsTests.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import {arrayify, dedent, objectify} from "../src/utils"; +import {arrayify, dedent, escapeRegExp, objectify} from "../src/utils"; describe("Utils", () => { it("arrayify", () => { @@ -46,4 +46,15 @@ describe("Utils", () => { assert.equal(dedent` `, " "); assert.equal(dedent``, ""); }); + + it("escapeRegExp", () => { + const plus = escapeRegExp("(a+|d)?b[ci]{2,}"); + const plusRe = new RegExp(plus); + + // contains substring that matches regular expression pattern + assert.equal(plusRe.test("regexpaaaabcicmatch"), false); + + // properly matches exact string with special characters + assert.equal(plusRe.test("string(a+|d)?b[ci]{2,}match"), true); + }); }); From 26167bb042f0587ddfd2b39000f4394c5f775d64 Mon Sep 17 00:00:00 2001 From: Andrii Dieiev Date: Sun, 8 Jan 2017 05:15:47 +0200 Subject: [PATCH 4/4] Make failure messages more detailed --- src/rules/commentFormatRule.ts | 15 ++++++++++----- .../exceptions-pattern/test.js.lint | 2 +- .../exceptions-pattern/test.ts.lint | 2 +- .../comment-format/exceptions-words/test.js.lint | 2 +- .../comment-format/exceptions-words/test.ts.lint | 2 +- test/rules/comment-format/lower/test.js.lint | 2 +- test/rules/comment-format/lower/test.ts.lint | 2 +- test/rules/comment-format/upper/test.ts.lint | 2 +- 8 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/rules/commentFormatRule.ts b/src/rules/commentFormatRule.ts index fb64d8b9fe9..bcaab54a377 100644 --- a/src/rules/commentFormatRule.ts +++ b/src/rules/commentFormatRule.ts @@ -22,7 +22,7 @@ import { escapeRegExp } from "../utils"; interface IExceptionsObject { ignoreWords?: string[]; - ignorePattern?: string[]; + ignorePattern?: string; } type ExceptionsRegExp = RegExp | null; @@ -95,9 +95,11 @@ export class Rule extends Lint.Rules.AbstractRule { }; /* tslint:enable:object-literal-sort-keys */ - public static LOWERCASE_FAILURE = "comment must start with lowercase letter, word from exceptions list or exceptions pattern"; - public static UPPERCASE_FAILURE = "comment must start with uppercase letter, word from exceptions list or exceptions pattern"; + public static LOWERCASE_FAILURE = "comment must start with lowercase letter"; + public static UPPERCASE_FAILURE = "comment must start with 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.applyWithWalker(new CommentWalker(sourceFile, this.getOptions())); @@ -106,6 +108,7 @@ export class Rule extends Lint.Rules.AbstractRule { class CommentWalker extends Lint.SkippableTokenAwareRuleWalker { private exceptionsRegExp: ExceptionsRegExp; + private failureIgnorePart: string = ""; constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) { super(sourceFile, options); @@ -135,12 +138,12 @@ class CommentWalker extends Lint.SkippableTokenAwareRuleWalker { } if (this.hasOption(OPTION_LOWERCASE)) { if (!startsWithLowercase(commentText) && !this.startsWithException(commentText)) { - this.addFailureAt(startPosition, width, Rule.LOWERCASE_FAILURE); + this.addFailureAt(startPosition, width, Rule.LOWERCASE_FAILURE + this.failureIgnorePart); } } if (this.hasOption(OPTION_UPPERCASE)) { if (!startsWithUppercase(commentText) && !isEnableDisableFlag(commentText) && !this.startsWithException(commentText)) { - this.addFailureAt(startPosition, width, Rule.UPPERCASE_FAILURE); + this.addFailureAt(startPosition, width, Rule.UPPERCASE_FAILURE + this.failureIgnorePart); } } } @@ -165,11 +168,13 @@ class CommentWalker extends Lint.SkippableTokenAwareRuleWalker { } if (exceptionsObject.ignorePattern) { + this.failureIgnorePart = Rule.IGNORE_PATTERN_FAILURE_FACTORY(exceptionsObject.ignorePattern); // regex is "start of string"//"any amount of whitespace" followed by user provided ignore pattern return new RegExp(`^//\\s*(${exceptionsObject.ignorePattern})`); } if (exceptionsObject.ignoreWords) { + this.failureIgnorePart = Rule.IGNORE_WORDS_FAILURE_FACTORY(exceptionsObject.ignoreWords); // Converts all exceptions values to strings, trim whitespace, escapes RegExp special characters and combines into alternation const wordsPattern = exceptionsObject.ignoreWords .map(String) diff --git a/test/rules/comment-format/exceptions-pattern/test.js.lint b/test/rules/comment-format/exceptions-pattern/test.js.lint index 7dc99aaa7b3..34b4a0818fa 100644 --- a/test/rules/comment-format/exceptions-pattern/test.js.lint +++ b/test/rules/comment-format/exceptions-pattern/test.js.lint @@ -35,4 +35,4 @@ class Valid {} // stderr for errors -[upper]: comment must start with uppercase letter, word from exceptions list or exceptions pattern +[upper]: comment must start with uppercase letter or its start must match the regex pattern "std(in|out|err)\b" diff --git a/test/rules/comment-format/exceptions-pattern/test.ts.lint b/test/rules/comment-format/exceptions-pattern/test.ts.lint index 818932591e9..42adb252e09 100644 --- a/test/rules/comment-format/exceptions-pattern/test.ts.lint +++ b/test/rules/comment-format/exceptions-pattern/test.ts.lint @@ -32,5 +32,5 @@ const unusedVar = 'unneeded value'; -[lower]: comment must start with lowercase letter, word from exceptions list or exceptions pattern +[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 diff --git a/test/rules/comment-format/exceptions-words/test.js.lint b/test/rules/comment-format/exceptions-words/test.js.lint index b004c9e72f7..309e97d9e1d 100644 --- a/test/rules/comment-format/exceptions-words/test.js.lint +++ b/test/rules/comment-format/exceptions-words/test.js.lint @@ -34,4 +34,4 @@ class Valid {} // stderr for errors ~~~~~~~~~~~~~~~~~~ [upper] -[upper]: comment must start with uppercase letter, word from exceptions list or exceptions pattern +[upper]: comment must start with uppercase letter or the word(s): todo diff --git a/test/rules/comment-format/exceptions-words/test.ts.lint b/test/rules/comment-format/exceptions-words/test.ts.lint index ddd039b0431..aa7103ff2a4 100644 --- a/test/rules/comment-format/exceptions-words/test.ts.lint +++ b/test/rules/comment-format/exceptions-words/test.ts.lint @@ -33,5 +33,5 @@ const unusedVar = 'unneeded value'; // stderr for errors -[lower]: comment must start with lowercase letter, word from exceptions list or exceptions pattern +[lower]: comment must start with lowercase letter or the word(s): TODO, HACK [space]: comment must start with a space diff --git a/test/rules/comment-format/lower/test.js.lint b/test/rules/comment-format/lower/test.js.lint index 55d8b06a647..6b0f7e4cfb2 100644 --- a/test/rules/comment-format/lower/test.js.lint +++ b/test/rules/comment-format/lower/test.js.lint @@ -21,5 +21,5 @@ class Clazz { // this comment is correct //noinspection JSUnusedGlobalSymbols const unusedVar = 'unneeded value'; -[lower]: comment must start with lowercase letter, word from exceptions list or exceptions pattern +[lower]: comment must start with lowercase letter [space]: comment must start with a space diff --git a/test/rules/comment-format/lower/test.ts.lint b/test/rules/comment-format/lower/test.ts.lint index 55d8b06a647..6b0f7e4cfb2 100644 --- a/test/rules/comment-format/lower/test.ts.lint +++ b/test/rules/comment-format/lower/test.ts.lint @@ -21,5 +21,5 @@ class Clazz { // this comment is correct //noinspection JSUnusedGlobalSymbols const unusedVar = 'unneeded value'; -[lower]: comment must start with lowercase letter, word from exceptions list or exceptions pattern +[lower]: comment must start with lowercase letter [space]: comment must start with a space diff --git a/test/rules/comment-format/upper/test.ts.lint b/test/rules/comment-format/upper/test.ts.lint index 553b2664664..c94e6226882 100644 --- a/test/rules/comment-format/upper/test.ts.lint +++ b/test/rules/comment-format/upper/test.ts.lint @@ -31,5 +31,5 @@ class Invalid {} // tslint:disable-next-line:no-unused-expression class Valid {} -[upper]: comment must start with uppercase letter, word from exceptions list or exceptions pattern +[upper]: comment must start with uppercase letter [space]: comment must start with a space