From 7b65f9de77a3e41abd6af4129e4611cbf9011fc5 Mon Sep 17 00:00:00 2001 From: Christopher Quadflieg Date: Fri, 15 Jun 2018 11:59:08 +0200 Subject: [PATCH 1/4] Tests for new option in comment-format --- .../test.ts.lint | 50 +++++++++++++++++++ .../tslint.json | 5 ++ .../ignore-space-on-keyword/test.ts.lint | 34 +++++++++++++ .../ignore-space-on-keyword/tslint.json | 5 ++ 4 files changed, 94 insertions(+) create mode 100644 test/rules/comment-format/ignore-space-on-keyword-and-lower/test.ts.lint create mode 100644 test/rules/comment-format/ignore-space-on-keyword-and-lower/tslint.json create mode 100644 test/rules/comment-format/ignore-space-on-keyword/test.ts.lint create mode 100644 test/rules/comment-format/ignore-space-on-keyword/tslint.json diff --git a/test/rules/comment-format/ignore-space-on-keyword-and-lower/test.ts.lint b/test/rules/comment-format/ignore-space-on-keyword-and-lower/test.ts.lint new file mode 100644 index 00000000000..e7f3b8c73aa --- /dev/null +++ b/test/rules/comment-format/ignore-space-on-keyword-and-lower/test.ts.lint @@ -0,0 +1,50 @@ +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] + console.log("test2"); //TODO: this comment has no space but TODO + } + /// + ////foo + ~~~ [space] +} + +//#region test +//#endregion + +`${location.protocol}//${location.hostname}` + +//noinspection JSUnusedGlobalSymbols +const unusedVar = 'unneeded value'; + +//normal comment + ~~~~~~~~~~~~~~ [space] + +// normal comment + +//TODO: todo description + +//TODO: Todo description + ~~~~~~~~~~~~~~~~ [lower] + +//FIXME: todo description + +//FIXME:todo description + ~~~~~~~~~~~~~~~~ [space] + +//HACK: unknown keyword + ~~~~~~~~~~~~~~~~~~~~~ [space] + +//HACK: Unknown keyword + ~~~~~~~~~~~~~~~ [lower] + ~~~~~~~~~~~~~~~~~~~~~ [space] + +[lower]: comment must start with lowercase letter +[space]: comment must start with a space diff --git a/test/rules/comment-format/ignore-space-on-keyword-and-lower/tslint.json b/test/rules/comment-format/ignore-space-on-keyword-and-lower/tslint.json new file mode 100644 index 00000000000..26326504388 --- /dev/null +++ b/test/rules/comment-format/ignore-space-on-keyword-and-lower/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "comment-format": [true, "check-space", {"no-space-keywords": ["TODO", "FIXME"]}, "check-lowercase"] + } +} diff --git a/test/rules/comment-format/ignore-space-on-keyword/test.ts.lint b/test/rules/comment-format/ignore-space-on-keyword/test.ts.lint new file mode 100644 index 00000000000..19bfd92ad2f --- /dev/null +++ b/test/rules/comment-format/ignore-space-on-keyword/test.ts.lint @@ -0,0 +1,34 @@ +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 + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [space] + console.log("test"); //this comment has no space + ~~~~~~~~~~~~~~~~~~~~~~~~~ [space] + console.log("test2"); //TODO: this comment has no space but TODO + } + /// + ////foo + ~~~ [space] +} + +//#region test +//#endregion + +`${location.protocol}//${location.hostname}` + +//normal comment + ~~~~~~~~~~~~~~ [space] + +// normal comment + +//TODO: todo description + +//FIXME: todo description + +//HACK: unknown keyword + ~~~~~~~~~~~~~~~~~~~~~ [space] + +[space]: comment must start with a space diff --git a/test/rules/comment-format/ignore-space-on-keyword/tslint.json b/test/rules/comment-format/ignore-space-on-keyword/tslint.json new file mode 100644 index 00000000000..2e8edbd9817 --- /dev/null +++ b/test/rules/comment-format/ignore-space-on-keyword/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "comment-format": [true, "check-space", {"no-space-keywords": ["TODO", "FIXME"]}] + } +} From 84694f6ea84d904f9d935f09be632eb68104d2ea Mon Sep 17 00:00:00 2001 From: Christopher Quadflieg Date: Sun, 1 Jul 2018 13:51:23 +0200 Subject: [PATCH 2/4] implemented no-space-keywords for comment-format --- src/rules/commentFormatRule.ts | 78 ++++++++++++++++++- .../test.ts.lint | 5 +- 2 files changed, 77 insertions(+), 6 deletions(-) diff --git a/src/rules/commentFormatRule.ts b/src/rules/commentFormatRule.ts index c9d5487bc5c..f71072172d6 100644 --- a/src/rules/commentFormatRule.ts +++ b/src/rules/commentFormatRule.ts @@ -22,6 +22,10 @@ import { ENABLE_DISABLE_REGEX } from "../enableDisableRules"; import * as Lint from "../index"; import { escapeRegExp, isLowerCase, isUpperCase } from "../utils"; +interface NoSpaceKeywordsObject { + "no-space-keywords": string[]; +} + interface IExceptionsObject { "ignore-words"?: string[]; "ignore-pattern"?: string; @@ -29,6 +33,7 @@ interface IExceptionsObject { interface Options { space: boolean; + noSpaceExceptions?: RegExp; case: Case; exceptions?: RegExp; failureSuffix: string; @@ -78,6 +83,19 @@ export class Rule extends Lint.Rules.AbstractRule { "check-uppercase", ], }, + { + type: "object", + properties: { + "no-space-keywords": { + type: "array", + items: { + type: "string", + }, + }, + }, + minProperties: 1, + maxProperties: 1, + }, { type: "object", properties: { @@ -97,10 +115,11 @@ export class Rule extends Lint.Rules.AbstractRule { ], }, minLength: 1, - maxLength: 4, + maxLength: 5, }, optionExamples: [ [true, "check-space", "check-uppercase"], + [true, "check-space", {"no-space-keywords": ["TODO", "FIXME"]}], [true, "check-lowercase", {"ignore-words": ["TODO", "HACK"]}], [true, "check-lowercase", {"ignore-pattern": "STD\\w{2,3}\\b"}], ], @@ -113,6 +132,7 @@ export class Rule extends Lint.Rules.AbstractRule { 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 NO_SPACE_KEYWORDS_FACTORY = (keywords: string[]): string => ` or the keyword(s): ${keywords.join(", ")}`; 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}"`; @@ -121,7 +141,7 @@ export class Rule extends Lint.Rules.AbstractRule { } } -function parseOptions(options: Array): Options { +function parseOptions(options: Array): Options { return { case: options.indexOf(OPTION_LOWERCASE) !== -1 ? Case.Lower @@ -129,13 +149,49 @@ function parseOptions(options: Array): Options { ? Case.Upper : Case.None, failureSuffix: "", + noSpaceExceptions: parseNoSpaceKeywords(options[1]), space: options.indexOf(OPTION_SPACE) !== -1, ...composeExceptions(options[options.length - 1]), }; } -function composeExceptions(option?: string | IExceptionsObject): undefined | {exceptions: RegExp; failureSuffix: string} { +function isNoSpaceKeywordsObject(option?: string | NoSpaceKeywordsObject | IExceptionsObject): option is NoSpaceKeywordsObject { + if (typeof option !== "object") { + return false; + } + if (!("no-space-keywords" in option)) { + return false; + } + if (!Array.isArray(option["no-space-keywords"])) { + return false; + } + return true; +} + +function parseNoSpaceKeywords(option?: string | NoSpaceKeywordsObject | IExceptionsObject): RegExp | undefined { + if (!isNoSpaceKeywordsObject(option)) { + return undefined; + } + const keywords = option["no-space-keywords"]; + return new RegExp(`^(${keywords.map((keyword) => escapeRegExp(keyword.trim())).join("|")})\\b`); +} + +function isIExceptionsObject(option?: string | NoSpaceKeywordsObject | IExceptionsObject): option is IExceptionsObject { if (typeof option !== "object") { + return false; + } + if (!("ignore-pattern" in option) && !("ignore-words" in option)) { + return false; + } + if (!(typeof option["ignore-pattern"] === "string") && !Array.isArray(option["ignore-words"])) { + return false; + } + return true; +} + +function composeExceptions( + option?: string | NoSpaceKeywordsObject | IExceptionsObject): undefined | {exceptions: RegExp; failureSuffix: string} { + if (!isIExceptionsObject(option)) { return undefined; } const ignorePattern = option["ignore-pattern"]; @@ -173,12 +229,26 @@ function walk(ctx: Lint.WalkContext) { if (start === end) { return; } - const commentText = fullText.slice(start, end); + let commentText = fullText.slice(start, end); // whitelist //#region and //#endregion and JetBrains IDEs' "//noinspection ..." if (/^(?:#(?:end)?region|noinspection\s)/.test(commentText)) { return; } + if (ctx.options.noSpaceExceptions !== undefined) { + const match = ctx.options.noSpaceExceptions.exec(commentText); + if (match !== null) { + const lengthOfMatch = match[0].length; + commentText = commentText.substring(lengthOfMatch); + // Check optional colon after keyword + if (commentText[0] === ":") { + commentText = commentText.substring(1); + ++start; + } + start += lengthOfMatch; + } + } + if (ctx.options.space && commentText[0] !== " ") { ctx.addFailure(start, end, Rule.LEADING_SPACE_FAILURE, [ Lint.Replacement.appendText(start, " ") ]); } diff --git a/test/rules/comment-format/ignore-space-on-keyword-and-lower/test.ts.lint b/test/rules/comment-format/ignore-space-on-keyword-and-lower/test.ts.lint index e7f3b8c73aa..14d3cd1c1b0 100644 --- a/test/rules/comment-format/ignore-space-on-keyword-and-lower/test.ts.lint +++ b/test/rules/comment-format/ignore-space-on-keyword-and-lower/test.ts.lint @@ -32,7 +32,7 @@ const unusedVar = 'unneeded value'; //TODO: todo description //TODO: Todo description - ~~~~~~~~~~~~~~~~ [lower] + ~~~~~~~~~~~~~~~~~ [lower] //FIXME: todo description @@ -40,10 +40,11 @@ const unusedVar = 'unneeded value'; ~~~~~~~~~~~~~~~~ [space] //HACK: unknown keyword + ~~~~~~~~~~~~~~~~~~~~~ [lower] ~~~~~~~~~~~~~~~~~~~~~ [space] //HACK: Unknown keyword - ~~~~~~~~~~~~~~~ [lower] + ~~~~~~~~~~~~~~~~~~~~~ [lower] ~~~~~~~~~~~~~~~~~~~~~ [space] [lower]: comment must start with lowercase letter From bf813e94c97e1442e7b042d7351de56e89492713 Mon Sep 17 00:00:00 2001 From: Christopher Quadflieg Date: Sat, 28 Jul 2018 10:33:46 +0200 Subject: [PATCH 3/4] remove unused NO_SPACE_KEYWORDS_FACTORY --- src/rules/commentFormatRule.ts | 98 +++++++++++++++++++--------------- 1 file changed, 56 insertions(+), 42 deletions(-) diff --git a/src/rules/commentFormatRule.ts b/src/rules/commentFormatRule.ts index f71072172d6..10b072eb923 100644 --- a/src/rules/commentFormatRule.ts +++ b/src/rules/commentFormatRule.ts @@ -42,7 +42,7 @@ interface Options { const enum Case { None, Lower, - Upper, + Upper } const OPTION_SPACE = "check-space"; @@ -77,11 +77,7 @@ export class Rule extends Lint.Rules.AbstractRule { anyOf: [ { type: "string", - enum: [ - "check-space", - "check-lowercase", - "check-uppercase", - ], + enum: ["check-space", "check-lowercase", "check-uppercase"] }, { type: "object", @@ -89,12 +85,12 @@ export class Rule extends Lint.Rules.AbstractRule { "no-space-keywords": { type: "array", items: { - type: "string", - }, - }, + type: "string" + } + } }, minProperties: 1, - maxProperties: 1, + maxProperties: 1 }, { type: "object", @@ -102,39 +98,40 @@ export class Rule extends Lint.Rules.AbstractRule { "ignore-words": { type: "array", items: { - type: "string", - }, + type: "string" + } }, "ignore-pattern": { - type: "string", - }, + type: "string" + } }, minProperties: 1, - maxProperties: 1, - }, - ], + maxProperties: 1 + } + ] }, minLength: 1, - maxLength: 5, + maxLength: 5 }, optionExamples: [ [true, "check-space", "check-uppercase"], - [true, "check-space", {"no-space-keywords": ["TODO", "FIXME"]}], - [true, "check-lowercase", {"ignore-words": ["TODO", "HACK"]}], - [true, "check-lowercase", {"ignore-pattern": "STD\\w{2,3}\\b"}], + [true, "check-space", { "no-space-keywords": ["TODO", "FIXME"] }], + [true, "check-lowercase", { "ignore-words": ["TODO", "HACK"] }], + [true, "check-lowercase", { "ignore-pattern": "STD\\w{2,3}\\b" }] ], type: "style", typescriptOnly: false, - hasFix: true, + hasFix: true }; /* 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 LEADING_SPACE_FAILURE = "comment must start with a space"; - public static NO_SPACE_KEYWORDS_FACTORY = (keywords: string[]): string => ` or the keyword(s): ${keywords.join(", ")}`; - 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 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)); @@ -143,7 +140,8 @@ export class Rule extends Lint.Rules.AbstractRule { function parseOptions(options: Array): Options { return { - case: options.indexOf(OPTION_LOWERCASE) !== -1 + case: + options.indexOf(OPTION_LOWERCASE) !== -1 ? Case.Lower : options.indexOf(OPTION_UPPERCASE) !== -1 ? Case.Upper @@ -151,11 +149,13 @@ function parseOptions(options: Array escapeRegExp(keyword.trim())).join("|")})\\b`); + return new RegExp(`^(${keywords.map(keyword => escapeRegExp(keyword.trim())).join("|")})\\b`); } -function isIExceptionsObject(option?: string | NoSpaceKeywordsObject | IExceptionsObject): option is IExceptionsObject { +function isIExceptionsObject( + option?: string | NoSpaceKeywordsObject | IExceptionsObject +): option is IExceptionsObject { if (typeof option !== "object") { return false; } @@ -190,7 +194,8 @@ function isIExceptionsObject(option?: string | NoSpaceKeywordsObject | IExceptio } function composeExceptions( - option?: string | NoSpaceKeywordsObject | IExceptionsObject): undefined | {exceptions: RegExp; failureSuffix: string} { + option?: string | NoSpaceKeywordsObject | IExceptionsObject +): undefined | { exceptions: RegExp; failureSuffix: string } { if (!isIExceptionsObject(option)) { return undefined; } @@ -198,28 +203,33 @@ function composeExceptions( if (ignorePattern !== undefined) { return { exceptions: new RegExp(`^\\s*(${ignorePattern})`), - failureSuffix: Rule.IGNORE_PATTERN_FAILURE_FACTORY(ignorePattern), + failureSuffix: Rule.IGNORE_PATTERN_FAILURE_FACTORY(ignorePattern) }; } const ignoreWords = option["ignore-words"]; if (ignoreWords !== undefined && ignoreWords.length !== 0) { return { - exceptions: new RegExp(`^\\s*(?:${ignoreWords.map((word) => escapeRegExp(word.trim())).join("|")})\\b`), - failureSuffix: Rule.IGNORE_WORDS_FAILURE_FACTORY(ignoreWords), + exceptions: new RegExp( + `^\\s*(?:${ignoreWords.map(word => escapeRegExp(word.trim())).join("|")})\\b` + ), + failureSuffix: Rule.IGNORE_WORDS_FAILURE_FACTORY(ignoreWords) }; } return undefined; } function walk(ctx: Lint.WalkContext) { - utils.forEachComment(ctx.sourceFile, (fullText, {kind, pos, end}) => { + utils.forEachComment(ctx.sourceFile, (fullText, { kind, pos, end }) => { let start = pos + 2; - if (kind !== ts.SyntaxKind.SingleLineCommentTrivia || + if ( + kind !== ts.SyntaxKind.SingleLineCommentTrivia || // exclude empty comments start === end || // exclude /// - fullText[start] === "/" && ctx.sourceFile.referencedFiles.some((ref) => ref.pos >= pos && ref.end <= end)) { + (fullText[start] === "/" && + ctx.sourceFile.referencedFiles.some(ref => ref.pos >= pos && ref.end <= end)) + ) { return; } // skip all leading slashes @@ -250,12 +260,16 @@ function walk(ctx: Lint.WalkContext) { } if (ctx.options.space && commentText[0] !== " ") { - ctx.addFailure(start, end, Rule.LEADING_SPACE_FAILURE, [ Lint.Replacement.appendText(start, " ") ]); + ctx.addFailure(start, end, Rule.LEADING_SPACE_FAILURE, [ + Lint.Replacement.appendText(start, " ") + ]); } - if (ctx.options.case === Case.None || - ctx.options.exceptions !== undefined && ctx.options.exceptions.test(commentText) || - ENABLE_DISABLE_REGEX.test(commentText)) { + if ( + ctx.options.case === Case.None || + (ctx.options.exceptions !== undefined && ctx.options.exceptions.test(commentText)) || + ENABLE_DISABLE_REGEX.test(commentText) + ) { return; } From e75444d0285cb4bba08c29d74b1392ce35607634 Mon Sep 17 00:00:00 2001 From: Christopher Quadflieg Date: Wed, 7 Nov 2018 09:17:39 +0100 Subject: [PATCH 4/4] extend optionsDescription --- src/rules/commentFormatRule.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/rules/commentFormatRule.ts b/src/rules/commentFormatRule.ts index 10b072eb923..8be72e4d288 100644 --- a/src/rules/commentFormatRule.ts +++ b/src/rules/commentFormatRule.ts @@ -70,6 +70,12 @@ export class Rule extends Lint.Rules.AbstractRule { * \`"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. + + Keywords can be managed with an object that can be passed as a second argument (right after \`"check-space"\`). + + One option can be provided in this object: + + * \`"no-space-keywords"\` - array of strings - keywords that can be used at the beginning of the comment. `, options: { type: "array",