diff --git a/.vscode/settings.json b/.vscode/settings.json index 22803497de8..af3c3c3b3af 100644 --- a/.vscode/settings.json +++ b/.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 } diff --git a/src/rules/commentFormatRule.ts b/src/rules/commentFormatRule.ts index 47ade09cd9f..0f7b7f3a67c 100644 --- a/src/rules/commentFormatRule.ts +++ b/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,20 +63,24 @@ 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", @@ -72,7 +88,12 @@ export class Rule extends Lint.Rules.AbstractRule { 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,7 +128,9 @@ 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(", ")}`; @@ -115,12 +138,18 @@ export class Rule extends Lint.Rules.AbstractRule { ` 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): Options { return { + allowTrailingLowercase: has(OPTION_ALLOW_TRAILING_LOWERCASE), case: options.indexOf(OPTION_LOWERCASE) !== -1 ? Case.Lower @@ -128,9 +157,13 @@ function parseOptions(options: Array): Options { ? 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) { - utils.forEachComment(ctx.sourceFile, (fullText, { kind, pos, end }) => { - let start = pos + 2; +class CommentFormatWalker extends Lint.AbstractWalker { + 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 /// - (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); + } } diff --git a/test/rules/comment-format/allow-trailing-lowercase/test.ts.fix b/test/rules/comment-format/allow-trailing-lowercase/test.ts.fix new file mode 100644 index 00000000000..29c8535bbd0 --- /dev/null +++ b/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 + diff --git a/test/rules/comment-format/allow-trailing-lowercase/test.ts.lint b/test/rules/comment-format/allow-trailing-lowercase/test.ts.lint new file mode 100644 index 00000000000..f53db41854f --- /dev/null +++ b/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 diff --git a/test/rules/comment-format/allow-trailing-lowercase/tslint.json b/test/rules/comment-format/allow-trailing-lowercase/tslint.json new file mode 100644 index 00000000000..705d0c25fc8 --- /dev/null +++ b/test/rules/comment-format/allow-trailing-lowercase/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "comment-format": [true, "check-space", "check-uppercase", "allow-trailing-lowercase"] + } +} diff --git a/test/rules/comment-format/exceptions-pattern/test.ts.fix b/test/rules/comment-format/exceptions-pattern/test.ts.fix index 546088ec95b..c142d9cb849 100644 --- a/test/rules/comment-format/exceptions-pattern/test.ts.fix +++ b/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 } /// @@ -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 diff --git a/test/rules/comment-format/exceptions-pattern/test.ts.lint b/test/rules/comment-format/exceptions-pattern/test.ts.lint index 51540de85f2..58cb88eeabd 100644 --- a/test/rules/comment-format/exceptions-pattern/test.ts.lint +++ b/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}" diff --git a/test/rules/comment-format/exceptions-words/test.ts.fix b/test/rules/comment-format/exceptions-words/test.ts.fix new file mode 100644 index 00000000000..ff593a9c2fe --- /dev/null +++ b/test/rules/comment-format/exceptions-words/test.ts.fix @@ -0,0 +1,29 @@ +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 + console.log("test"); // this comment has no space + } + /// + //// foo +} + +//#region test +//#endregion + +`${location.protocol}//${location.hostname}` + +//noinspection JSUnusedGlobalSymbols +const unusedVar = 'unneeded value'; + +// TODO: Write more tests + +// hACKING is not an exception + +// sTDIN for input +// sTDOUT for output +// stderr for errors + + diff --git a/test/rules/comment-format/exceptions-words/test.ts.lint b/test/rules/comment-format/exceptions-words/test.ts.lint index 3ce531fdf82..4a19e036682 100644 --- a/test/rules/comment-format/exceptions-words/test.ts.lint +++ b/test/rules/comment-format/exceptions-words/test.ts.lint @@ -5,8 +5,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 the word(s): TODO, HACK [space]: comment must start with a space +[space_lower]: comment must start with a space and lowercase letter or the word(s): TODO, HACK diff --git a/test/rules/comment-format/lower/test.ts.fix b/test/rules/comment-format/lower/test.ts.fix new file mode 100644 index 00000000000..d71c05768db --- /dev/null +++ b/test/rules/comment-format/lower/test.ts.fix @@ -0,0 +1,20 @@ +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 + console.log("test"); // this comment has no space + } + /// + //// foo +} + +//#region test +//#endregion + +`${location.protocol}//${location.hostname}` + +//noinspection JSUnusedGlobalSymbols +const unusedVar = 'unneeded value'; + diff --git a/test/rules/comment-format/lower/test.ts.lint b/test/rules/comment-format/lower/test.ts.lint index 5b3467f2ba6..07c3390abf6 100644 --- a/test/rules/comment-format/lower/test.ts.lint +++ b/test/rules/comment-format/lower/test.ts.lint @@ -5,8 +5,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] } @@ -25,3 +24,4 @@ const unusedVar = 'unneeded value'; [lower]: comment must start with lowercase letter [space]: comment must start with a space +[space_lower]: comment must start with a space and lowercase letter diff --git a/test/rules/comment-format/lower/test.tsx.fix b/test/rules/comment-format/lower/test.tsx.fix new file mode 100644 index 00000000000..7c09e671f03 --- /dev/null +++ b/test/rules/comment-format/lower/test.tsx.fix @@ -0,0 +1,9 @@ +const a = ( + + https://github.com/ { + // invalid comment + content + }, text + +); + diff --git a/test/rules/comment-format/upper/test.ts.fix b/test/rules/comment-format/upper/test.ts.fix new file mode 100644 index 00000000000..562af0396fa --- /dev/null +++ b/test/rules/comment-format/upper/test.ts.fix @@ -0,0 +1,27 @@ +class Clazz { // This comment is correct + /* block comment + * adada + */ + public funcxion() { // This comment has a lowercase letter starting it + // This comment is on its own line, and starts with a lowercase _and_ no space + console.log("test"); // This comment has no space + } + /// + //// Foo +} + +//#region test +//#endregion + +`${location.protocol}//${location.hostname}` + +// Tslint should show error here + +// Tslint: not a rule flag + +// tslint:disable-next-line:no-unused-expression +class Invalid {} + +// tslint:disable-next-line:no-unused-expression +class Valid {} + diff --git a/test/rules/comment-format/upper/test.ts.lint b/test/rules/comment-format/upper/test.ts.lint index 94a205758af..d9d21816446 100644 --- a/test/rules/comment-format/upper/test.ts.lint +++ b/test/rules/comment-format/upper/test.ts.lint @@ -5,15 +5,13 @@ class Clazz { // This comment is correct 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] - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [space] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [space_upper] console.log("test"); //This comment has no space ~~~~~~~~~~~~~~~~~~~~~~~~~ [space] } /// ////foo - ~~~ [space] - ~~~ [upper] + ~~~ [space_upper] } //#region test @@ -36,3 +34,4 @@ class Valid {} [upper]: comment must start with uppercase letter [space]: comment must start with a space +[space_upper]: comment must start with a space and uppercase letter