Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

[new-rule-option] no-space-keywords for comment-format #4003

Closed
wants to merge 6 commits into from
Closed

[new-rule-option] no-space-keywords for comment-format #4003

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7b65f9d
Tests for new option in comment-format
Shinigami92 Jun 15, 2018
84694f6
implemented no-space-keywords for comment-format
Shinigami92 Jul 1, 2018
1f5cfd3
Merge branch 'master' into feature/comment-format-check-space-special…
Shinigami92 Jul 28, 2018
bf813e9
remove unused NO_SPACE_KEYWORDS_FACTORY
Shinigami92 Jul 28, 2018
e75444d
extend optionsDescription
Shinigami92 Nov 7, 2018
d1f4e96
Merge branch 'master' into feature/comment-format-check-space-special…
Shinigami92 Nov 10, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
152 changes: 118 additions & 34 deletions src/rules/commentFormatRule.ts
View file
Open in desktop
Expand Up @@ -22,13 +22,18 @@ 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;
}

interface Options {
space: boolean;
noSpaceExceptions?: RegExp;
case: Case;
exceptions?: RegExp;
failureSuffix: string;
Expand All @@ -37,7 +42,7 @@ interface Options {
const enum Case {
None,
Lower,
Upper,
Upper
}

const OPTION_SPACE = "check-space";
Expand Down Expand Up @@ -72,98 +77,159 @@ 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",
properties: {
"no-space-keywords": {
type: "array",
items: {
type: "string"
}
}
},
minProperties: 1,
maxProperties: 1
},
{
type: "object",
properties: {
"ignore-words": {
type: "array",
items: {
type: "string",
},
type: "string"
}
},
"ignore-pattern": {
type: "string",
},
type: "string"
}
},
minProperties: 1,
maxProperties: 1,
},
],
maxProperties: 1
}
]
},
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, "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 */
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved

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 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));
}
}

function parseOptions(options: Array<string | IExceptionsObject>): Options {
function parseOptions(options: Array<string | NoSpaceKeywordsObject | IExceptionsObject>): Options {
return {
case: options.indexOf(OPTION_LOWERCASE) !== -1
case:
options.indexOf(OPTION_LOWERCASE) !== -1
? Case.Lower
: options.indexOf(OPTION_UPPERCASE) !== -1
? Case.Upper
: Case.None,
failureSuffix: "",
noSpaceExceptions: parseNoSpaceKeywords(options[1]),
space: options.indexOf(OPTION_SPACE) !== -1,
...composeExceptions(options[options.length - 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"];
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<Options>) {
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 /// <reference path="...">
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
Expand All @@ -173,19 +239,37 @@ function walk(ctx: Lint.WalkContext<Options>) {
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);
Shinigami92 marked this conversation as resolved.
Show resolved Hide resolved
// Check optional colon after keyword
if (commentText[0] === ":") {
Shinigami92 marked this conversation as resolved.
Show resolved Hide resolved
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, " ") ]);
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;
}

Expand Down
51 changes: 51 additions & 0 deletions test/rules/comment-format/ignore-space-on-keyword-and-lower/test.ts.lint
View file
Open in desktop
@@ -0,0 +1,51 @@
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
}
/// <not a reference>
////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
~~~~~~~~~~~~~~~~~~~~~ [lower]
~~~~~~~~~~~~~~~~~~~~~ [space]

//HACK: Unknown keyword
~~~~~~~~~~~~~~~~~~~~~ [lower]
~~~~~~~~~~~~~~~~~~~~~ [space]

[lower]: comment must start with lowercase letter
[space]: comment must start with a space
5 changes: 5 additions & 0 deletions test/rules/comment-format/ignore-space-on-keyword-and-lower/tslint.json
View file
Open in desktop
@@ -0,0 +1,5 @@
{
"rules": {
"comment-format": [true, "check-space", {"no-space-keywords": ["TODO", "FIXME"]}, "check-lowercase"]
}
}
34 changes: 34 additions & 0 deletions test/rules/comment-format/ignore-space-on-keyword/test.ts.lint
View file
Open in desktop
@@ -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
}
/// <not a reference>
////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
5 changes: 5 additions & 0 deletions test/rules/comment-format/ignore-space-on-keyword/tslint.json
View file
Open in desktop
@@ -0,0 +1,5 @@
{
"rules": {
"comment-format": [true, "check-space", {"no-space-keywords": ["TODO", "FIXME"]}]
}
}