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

Add exceptions support for comment-format rule (fixes #562) #1757

Merged
merged 4 commits into from
Jan 8, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 102 additions & 9 deletions src/rules/commentFormatRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -35,17 +43,53 @@ 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 \`///<reference>\`
* \`"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: {
type: "string",
enum: ["check-space", "check-lowercase", "check-uppercase"],
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: 3,
maxLength: 4,
},
optionExamples: ['[true, "check-space", "check-lowercase"]'],
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,
};
Expand All @@ -54,13 +98,24 @@ 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 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()));
}
}

class CommentWalker extends Lint.SkippableTokenAwareRuleWalker {
private exceptionsRegExp: ExceptionsRegExp;
private failureIgnorePart: string = "";

constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) {
super(sourceFile, options);

this.exceptionsRegExp = this.composeExceptionsRegExp();
}

public visitSourceFile(node: ts.SourceFile) {
super.visitSourceFile(node);
Lint.scanAllTokens(ts.createScanner(ts.ScriptTarget.ES5, false, ts.LanguageVariant.Standard, node.text), (scanner: ts.Scanner) => {
Expand All @@ -82,19 +137,57 @@ class CommentWalker extends Lint.SkippableTokenAwareRuleWalker {
}
}
if (this.hasOption(OPTION_LOWERCASE)) {
if (!startsWithLowercase(commentText)) {
this.addFailureAt(startPosition, width, Rule.LOWERCASE_FAILURE);
if (!startsWithLowercase(commentText) && !this.startsWithException(commentText)) {
this.addFailureAt(startPosition, width, Rule.LOWERCASE_FAILURE + this.failureIgnorePart);
}
}
if (this.hasOption(OPTION_UPPERCASE)) {
if (!startsWithUppercase(commentText) && !isEnableDisableFlag(commentText)) {
this.addFailureAt(startPosition, width, Rule.UPPERCASE_FAILURE);
if (!startsWithUppercase(commentText) && !isEnableDisableFlag(commentText) && !this.startsWithException(commentText)) {
this.addFailureAt(startPosition, width, Rule.UPPERCASE_FAILURE + this.failureIgnorePart);
}
}
}
});
}

private startsWithException(commentText: string): boolean {
if (this.exceptionsRegExp == null) {
return false;
}

return this.exceptionsRegExp.test(commentText);
}

private composeExceptionsRegExp(): ExceptionsRegExp {
const optionsList = this.getOptions() as Array<string | IExceptionsObject>;
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) {
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)
.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;
}
}

function startsWith(commentText: string, changeCase: (str: string) => string) {
Expand Down
7 changes: 7 additions & 0 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, "\\$&");
}
38 changes: 38 additions & 0 deletions test/rules/comment-format/exceptions-pattern/test.js.lint
Original file line number Diff line number Diff line change
@@ -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
}
/// <reference or something>
}

//#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 or its start must match the regex pattern "std(in|out|err)\b"
36 changes: 36 additions & 0 deletions test/rules/comment-format/exceptions-pattern/test.ts.lint
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
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]
}
/// <reference or something>
}

//#region test
//#endregion

`${location.protocol}//${location.hostname}`

//noinspection JSUnusedGlobalSymbols
const unusedVar = 'unneeded value';

// TODO: Write more tests
~~~~~~~~~~~~~~~~~~~~~~~ [lower]
// HACKING is not an exception
~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [lower]

// STDIN for input
// STDOUT for output
// stderr for errors



[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
8 changes: 8 additions & 0 deletions test/rules/comment-format/exceptions-pattern/tslint.json
Original file line number Diff line number Diff line change
@@ -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"}]
}
}
37 changes: 37 additions & 0 deletions test/rules/comment-format/exceptions-words/test.js.lint
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
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
}
/// <reference or something>
}

//#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

// STDIN for input
// STDOUT for output
// stderr for errors
~~~~~~~~~~~~~~~~~~ [upper]

[upper]: comment must start with uppercase letter or the word(s): todo
37 changes: 37 additions & 0 deletions test/rules/comment-format/exceptions-words/test.ts.lint
Original file line number Diff line number Diff line change
@@ -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]
}
/// <reference or something>
}

//#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 or the word(s): TODO, HACK
[space]: comment must start with a space
8 changes: 8 additions & 0 deletions test/rules/comment-format/exceptions-words/tslint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"rules": {
"comment-format": [true, "check-space", "check-lowercase", {"ignoreWords": ["TODO", "HACK"]}]
},
"jsRules": {
"comment-format": [true, "check-uppercase", {"ignoreWords": ["todo"]}]
}
}
13 changes: 12 additions & 1 deletion test/utilsTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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);
});
});