From 472d657c69d963e67c1ac8fb2746d5949ca2063d Mon Sep 17 00:00:00 2001 From: rrogowski Date: Thu, 28 Feb 2019 16:13:46 -0500 Subject: [PATCH 1/4] allow header to consist of consecutive single-line comments --- src/rules/fileHeaderRule.ts | 42 +++++++++++++++---- .../file-header/allow-multiline/test.ts.lint | 4 ++ .../file-header/allow-multiline/tslint.json | 5 +++ test/rules/file-header/bad/test2.ts.fix | 7 ++++ test/rules/file-header/bad/test2.ts.lint | 4 ++ 5 files changed, 55 insertions(+), 7 deletions(-) create mode 100644 test/rules/file-header/allow-multiline/test.ts.lint create mode 100644 test/rules/file-header/allow-multiline/tslint.json create mode 100644 test/rules/file-header/bad/test2.ts.fix create mode 100644 test/rules/file-header/bad/test2.ts.lint diff --git a/src/rules/fileHeaderRule.ts b/src/rules/fileHeaderRule.ts index 03d44d342ff..7420c442a93 100644 --- a/src/rules/fileHeaderRule.ts +++ b/src/rules/fileHeaderRule.ts @@ -20,6 +20,7 @@ import * as ts from "typescript"; import * as Lint from "../index"; const ENFORCE_TRAILING_NEWLINE = "enforce-trailing-newline"; +const ALLOW_MULTILINE = "allow-multiline"; export class Rule extends Lint.Rules.AbstractRule { /* tslint:disable:object-literal-sort-keys */ @@ -32,7 +33,9 @@ export class Rule extends Lint.Rules.AbstractRule { The second argument, which is optional, is a string that should be inserted as a header comment if fixing is enabled and no header that matches the first argument is found. The third argument, which is optional, is a string that denotes whether or not a newline should - exist on the header.`, + exist on the header. + The fourth argument, which is optional, is a string that denotes whether or not a header is + allowed to consist of consecutive single-line comments.`, options: { type: "array", items: [ @@ -45,12 +48,17 @@ export class Rule extends Lint.Rules.AbstractRule { { type: "string", }, + { + type: "string", + }, ], additionalItems: false, minLength: 1, - maxLength: 3, + maxLength: 4, }, - optionExamples: [[true, "Copyright \\d{4}", "Copyright 2018", ENFORCE_TRAILING_NEWLINE]], + optionExamples: [ + [true, "Copyright \\d{4}", "Copyright 2018", ENFORCE_TRAILING_NEWLINE, ALLOW_MULTILINE], + ], hasFix: true, type: "style", typescriptOnly: false, @@ -66,13 +74,11 @@ export class Rule extends Lint.Rules.AbstractRule { const textToInsert = this.ruleArguments[1] as string | undefined; const enforceExtraTrailingLine = this.ruleArguments.indexOf(ENFORCE_TRAILING_NEWLINE) !== -1; + const allowMultiline = this.ruleArguments.indexOf(ALLOW_MULTILINE) !== -1; // ignore shebang if it exists let offset = text.startsWith("#!") ? text.indexOf("\n") : 0; - // returns the text of the first comment or undefined - const commentText = ts.forEachLeadingCommentRange(text, offset, (pos, end, kind) => - text.substring(pos + 2, kind === ts.SyntaxKind.SingleLineCommentTrivia ? end : end - 2), - ); + const commentText = this.getFileHeaderText(text, offset, allowMultiline); if (commentText === undefined || !headerFormat.test(commentText)) { const isErrorAtStart = offset === 0; @@ -172,4 +178,26 @@ export class Rule extends Lint.Rules.AbstractRule { entireComment !== undefined && NEW_LINE_FOLLOWING_HEADER.test(entireComment) !== null ); } + + private getFileHeaderText( + text: string, + offset: number, + allowMultiline: boolean, + ): string | undefined { + const ranges = ts.getLeadingCommentRanges(text, offset); + if (ranges === undefined || ranges.length === 0) { + return undefined; + } + + const fileHeaderRanges = !allowMultiline ? ranges.slice(0, 1) : ranges; + return fileHeaderRanges + .map(range => { + const { pos, kind, end } = range; + return text.substring( + pos + 2, + kind === ts.SyntaxKind.SingleLineCommentTrivia ? end : end - 2, + ); + }) + .join("\n"); + } } diff --git a/test/rules/file-header/allow-multiline/test.ts.lint b/test/rules/file-header/allow-multiline/test.ts.lint new file mode 100644 index 00000000000..8aa13d57e2e --- /dev/null +++ b/test/rules/file-header/allow-multiline/test.ts.lint @@ -0,0 +1,4 @@ +// ********************************** +// Good header +// ********************************** + diff --git a/test/rules/file-header/allow-multiline/tslint.json b/test/rules/file-header/allow-multiline/tslint.json new file mode 100644 index 00000000000..be5475b8253 --- /dev/null +++ b/test/rules/file-header/allow-multiline/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "file-header": [true, "Good header", "", "allow-multiline"] + } + } diff --git a/test/rules/file-header/bad/test2.ts.fix b/test/rules/file-header/bad/test2.ts.fix new file mode 100644 index 00000000000..29f5b546f57 --- /dev/null +++ b/test/rules/file-header/bad/test2.ts.fix @@ -0,0 +1,7 @@ +/*! + * Good header 2 + */ + +// ********************************** +// Bad header +// ********************************** diff --git a/test/rules/file-header/bad/test2.ts.lint b/test/rules/file-header/bad/test2.ts.lint new file mode 100644 index 00000000000..ef259de4caf --- /dev/null +++ b/test/rules/file-header/bad/test2.ts.lint @@ -0,0 +1,4 @@ +// ********************************** +~nil [missing file header] +// Bad header +// ********************************** From 48b146baaf7a174a98f58172e6e4a2aa36fb391b Mon Sep 17 00:00:00 2001 From: rrogowski Date: Tue, 19 Mar 2019 16:19:45 -0400 Subject: [PATCH 2/4] new + legacy options --- src/rules/fileHeaderRule.ts | 117 ++++++++++++++---- .../file-header/allow-multiline/tslint.json | 5 - .../test.ts.lint | 0 .../tslint.json | 8 ++ 4 files changed, 98 insertions(+), 32 deletions(-) delete mode 100644 test/rules/file-header/allow-multiline/tslint.json rename test/rules/file-header/{allow-multiline => good-allow-single-line-comments}/test.ts.lint (100%) create mode 100644 test/rules/file-header/good-allow-single-line-comments/tslint.json diff --git a/src/rules/fileHeaderRule.ts b/src/rules/fileHeaderRule.ts index 7420c442a93..6b8bf8406e8 100644 --- a/src/rules/fileHeaderRule.ts +++ b/src/rules/fileHeaderRule.ts @@ -20,7 +20,13 @@ import * as ts from "typescript"; import * as Lint from "../index"; const ENFORCE_TRAILING_NEWLINE = "enforce-trailing-newline"; -const ALLOW_MULTILINE = "allow-multiline"; + +interface FileHeaderRuleOptions { + match: string; + allowSingleLineComments?: boolean; + default?: string; + enforceTrailingNewline?: boolean; +} export class Rule extends Lint.Rules.AbstractRule { /* tslint:disable:object-literal-sort-keys */ @@ -29,35 +35,75 @@ export class Rule extends Lint.Rules.AbstractRule { description: "Enforces a certain header comment for all files, matched by a regular expression.", optionsDescription: Lint.Utils.dedent` + A single object may be passed in for configuration that must contain: + + * \`match\`: a regular expression that all headers should match + + Any of the following optional fields may also be provided: + + * \`allowSingleLineComments\`: a boolean for whether \`//\` should be considered file headers in addition to \`/*\` comments + * \`default\`: text to add for file headers when running in \`--fix\` mode + * \`enforceTrailingNewline\`: a boolean for whether a newline must follow the header + + The rule will also accept array of strings as a legacy form of options, though the object form is recommended. The first option, which is mandatory, is a regular expression that all headers should match. The second argument, which is optional, is a string that should be inserted as a header comment if fixing is enabled and no header that matches the first argument is found. The third argument, which is optional, is a string that denotes whether or not a newline should - exist on the header. - The fourth argument, which is optional, is a string that denotes whether or not a header is - allowed to consist of consecutive single-line comments.`, + exist on the header.`, options: { - type: "array", - items: [ - { - type: "string", - }, - { - type: "string", - }, + oneOf: [ { - type: "string", + type: "array", + items: { + type: "object", + properties: { + match: { + type: "string", + }, + allowSingleLineComments: { + type: "boolean", + }, + default: { + type: "string", + }, + enforceTrailingNewline: { + type: "boolean", + }, + }, + additionalProperties: false, + }, }, { - type: "string", + type: "array", + items: [ + { + type: "string", + }, + { + type: "string", + }, + { + type: "string", + }, + ], + additionalItems: false, + minLength: 1, + maxLength: 3, }, ], - additionalItems: false, - minLength: 1, - maxLength: 4, }, optionExamples: [ - [true, "Copyright \\d{4}", "Copyright 2018", ENFORCE_TRAILING_NEWLINE, ALLOW_MULTILINE], + [ + true, + { + match: "Copyright \\d{4}", + allowSingleLineComments: true, + default: "Copyright 2018", + enforceTrailingNewline: true, + }, + ], + [true, "Copyright \\d{4}", "Copyright 2018", ENFORCE_TRAILING_NEWLINE], ], hasFix: true, type: "style", @@ -69,16 +115,15 @@ export class Rule extends Lint.Rules.AbstractRule { public static MISSING_NEW_LINE_FAILURE_STRING = "missing new line following the file header"; public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { + const options = this.getRuleOptions(); + const { text } = sourceFile; - const headerFormat = new RegExp(this.ruleArguments[0] as string); - const textToInsert = this.ruleArguments[1] as string | undefined; - const enforceExtraTrailingLine = - this.ruleArguments.indexOf(ENFORCE_TRAILING_NEWLINE) !== -1; - const allowMultiline = this.ruleArguments.indexOf(ALLOW_MULTILINE) !== -1; + const headerFormat = new RegExp(options.match); + const textToInsert = options.default; // ignore shebang if it exists let offset = text.startsWith("#!") ? text.indexOf("\n") : 0; - const commentText = this.getFileHeaderText(text, offset, allowMultiline); + const commentText = this.getFileHeaderText(text, offset, !!options.allowSingleLineComments); if (commentText === undefined || !headerFormat.test(commentText)) { const isErrorAtStart = offset === 0; @@ -113,7 +158,7 @@ export class Rule extends Lint.Rules.AbstractRule { } const trailingNewLineViolation = - enforceExtraTrailingLine && + options.enforceTrailingNewline && headerFormat.test(commentText) && this.doesNewLineEndingViolationExist(text, offset); @@ -141,6 +186,24 @@ export class Rule extends Lint.Rules.AbstractRule { return []; } + private getRuleOptions(): FileHeaderRuleOptions { + const options = this.ruleArguments; + if (options.length === 1 && typeof options[0] === "object") { + return options[0] as FileHeaderRuleOptions; + } + + // Legacy options + const args = this.ruleArguments as string[]; + return { + default: args[1], + enforceTrailingNewline: + args[2] !== undefined + ? args[2].indexOf(ENFORCE_TRAILING_NEWLINE) !== -1 + : undefined, + match: args[0], + }; + } + private createComment( sourceFile: ts.SourceFile, commentText: string, @@ -182,14 +245,14 @@ export class Rule extends Lint.Rules.AbstractRule { private getFileHeaderText( text: string, offset: number, - allowMultiline: boolean, + allowSingleLineComments: boolean, ): string | undefined { const ranges = ts.getLeadingCommentRanges(text, offset); if (ranges === undefined || ranges.length === 0) { return undefined; } - const fileHeaderRanges = !allowMultiline ? ranges.slice(0, 1) : ranges; + const fileHeaderRanges = !allowSingleLineComments ? ranges.slice(0, 1) : ranges; return fileHeaderRanges .map(range => { const { pos, kind, end } = range; diff --git a/test/rules/file-header/allow-multiline/tslint.json b/test/rules/file-header/allow-multiline/tslint.json deleted file mode 100644 index be5475b8253..00000000000 --- a/test/rules/file-header/allow-multiline/tslint.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "rules": { - "file-header": [true, "Good header", "", "allow-multiline"] - } - } diff --git a/test/rules/file-header/allow-multiline/test.ts.lint b/test/rules/file-header/good-allow-single-line-comments/test.ts.lint similarity index 100% rename from test/rules/file-header/allow-multiline/test.ts.lint rename to test/rules/file-header/good-allow-single-line-comments/test.ts.lint diff --git a/test/rules/file-header/good-allow-single-line-comments/tslint.json b/test/rules/file-header/good-allow-single-line-comments/tslint.json new file mode 100644 index 00000000000..9705da3bac6 --- /dev/null +++ b/test/rules/file-header/good-allow-single-line-comments/tslint.json @@ -0,0 +1,8 @@ +{ + "rules": { + "file-header": [true, { + "match": "Good header", + "allowSingleLineComments": true + }] + } + } From 4dbbda12c7bd2e6945c6a36f119ce5b67e409739 Mon Sep 17 00:00:00 2001 From: rrogowski Date: Thu, 21 Mar 2019 09:53:31 -0400 Subject: [PATCH 3/4] kebab case --- src/rules/fileHeaderRule.ts | 55 +++++++++++-------- .../tslint.json | 2 +- 2 files changed, 32 insertions(+), 25 deletions(-) diff --git a/src/rules/fileHeaderRule.ts b/src/rules/fileHeaderRule.ts index 6b8bf8406e8..00b11c57f98 100644 --- a/src/rules/fileHeaderRule.ts +++ b/src/rules/fileHeaderRule.ts @@ -19,13 +19,16 @@ import * as ts from "typescript"; import * as Lint from "../index"; -const ENFORCE_TRAILING_NEWLINE = "enforce-trailing-newline"; +const OPTION_MATCH = "match"; +const OPTION_ALLOW_SINGLE_LINE_COMMENTS = "allow-single-line-comments"; +const OPTION_DEFAULT = "default"; +const OPTION_ENFORCE_TRAILING_NEWLINE = "enforce-trailing-newline"; interface FileHeaderRuleOptions { - match: string; - allowSingleLineComments?: boolean; - default?: string; - enforceTrailingNewline?: boolean; + [OPTION_MATCH]: string; + [OPTION_ALLOW_SINGLE_LINE_COMMENTS]?: boolean; + [OPTION_DEFAULT]?: string; + [OPTION_ENFORCE_TRAILING_NEWLINE]?: boolean; } export class Rule extends Lint.Rules.AbstractRule { @@ -37,13 +40,13 @@ export class Rule extends Lint.Rules.AbstractRule { optionsDescription: Lint.Utils.dedent` A single object may be passed in for configuration that must contain: - * \`match\`: a regular expression that all headers should match + * \`${OPTION_MATCH}\`: a regular expression that all headers should match Any of the following optional fields may also be provided: - * \`allowSingleLineComments\`: a boolean for whether \`//\` should be considered file headers in addition to \`/*\` comments - * \`default\`: text to add for file headers when running in \`--fix\` mode - * \`enforceTrailingNewline\`: a boolean for whether a newline must follow the header + * \`${OPTION_ALLOW_SINGLE_LINE_COMMENTS}\`: a boolean for whether \`//\` should be considered file headers in addition to \`/*\` comments + * \`${OPTION_DEFAULT}\`: text to add for file headers when running in \`--fix\` mode + * \`${OPTION_ENFORCE_TRAILING_NEWLINE}\`: a boolean for whether a newline must follow the header The rule will also accept array of strings as a legacy form of options, though the object form is recommended. The first option, which is mandatory, is a regular expression that all headers should match. @@ -58,16 +61,16 @@ export class Rule extends Lint.Rules.AbstractRule { items: { type: "object", properties: { - match: { + [OPTION_MATCH]: { type: "string", }, - allowSingleLineComments: { + [OPTION_ALLOW_SINGLE_LINE_COMMENTS]: { type: "boolean", }, - default: { + [OPTION_DEFAULT]: { type: "string", }, - enforceTrailingNewline: { + [OPTION_ENFORCE_TRAILING_NEWLINE]: { type: "boolean", }, }, @@ -97,13 +100,13 @@ export class Rule extends Lint.Rules.AbstractRule { [ true, { - match: "Copyright \\d{4}", - allowSingleLineComments: true, - default: "Copyright 2018", - enforceTrailingNewline: true, + [OPTION_MATCH]: "Copyright \\d{4}", + [OPTION_ALLOW_SINGLE_LINE_COMMENTS]: true, + [OPTION_DEFAULT]: "Copyright 2018", + [OPTION_ENFORCE_TRAILING_NEWLINE]: true, }, ], - [true, "Copyright \\d{4}", "Copyright 2018", ENFORCE_TRAILING_NEWLINE], + [true, "Copyright \\d{4}", "Copyright 2018", OPTION_ENFORCE_TRAILING_NEWLINE], ], hasFix: true, type: "style", @@ -123,7 +126,11 @@ export class Rule extends Lint.Rules.AbstractRule { // ignore shebang if it exists let offset = text.startsWith("#!") ? text.indexOf("\n") : 0; - const commentText = this.getFileHeaderText(text, offset, !!options.allowSingleLineComments); + const commentText = this.getFileHeaderText( + text, + offset, + !!options[OPTION_ALLOW_SINGLE_LINE_COMMENTS], + ); if (commentText === undefined || !headerFormat.test(commentText)) { const isErrorAtStart = offset === 0; @@ -158,7 +165,7 @@ export class Rule extends Lint.Rules.AbstractRule { } const trailingNewLineViolation = - options.enforceTrailingNewline && + options[OPTION_ENFORCE_TRAILING_NEWLINE] && headerFormat.test(commentText) && this.doesNewLineEndingViolationExist(text, offset); @@ -195,12 +202,12 @@ export class Rule extends Lint.Rules.AbstractRule { // Legacy options const args = this.ruleArguments as string[]; return { - default: args[1], - enforceTrailingNewline: + [OPTION_DEFAULT]: args[1], + [OPTION_ENFORCE_TRAILING_NEWLINE]: args[2] !== undefined - ? args[2].indexOf(ENFORCE_TRAILING_NEWLINE) !== -1 + ? args[2].indexOf(OPTION_ENFORCE_TRAILING_NEWLINE) !== -1 : undefined, - match: args[0], + [OPTION_MATCH]: args[0], }; } diff --git a/test/rules/file-header/good-allow-single-line-comments/tslint.json b/test/rules/file-header/good-allow-single-line-comments/tslint.json index 9705da3bac6..5f1256bb402 100644 --- a/test/rules/file-header/good-allow-single-line-comments/tslint.json +++ b/test/rules/file-header/good-allow-single-line-comments/tslint.json @@ -2,7 +2,7 @@ "rules": { "file-header": [true, { "match": "Good header", - "allowSingleLineComments": true + "allow-single-line-comments": true }] } } From 3366259d4ff570abd9cd3c011f8ac2853f01dca3 Mon Sep 17 00:00:00 2001 From: rrogowski Date: Thu, 21 Mar 2019 09:56:06 -0400 Subject: [PATCH 4/4] uniform access of options --- src/rules/fileHeaderRule.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rules/fileHeaderRule.ts b/src/rules/fileHeaderRule.ts index 00b11c57f98..764404d3f44 100644 --- a/src/rules/fileHeaderRule.ts +++ b/src/rules/fileHeaderRule.ts @@ -121,8 +121,8 @@ export class Rule extends Lint.Rules.AbstractRule { const options = this.getRuleOptions(); const { text } = sourceFile; - const headerFormat = new RegExp(options.match); - const textToInsert = options.default; + const headerFormat = new RegExp(options[OPTION_MATCH]); + const textToInsert = options[OPTION_DEFAULT]; // ignore shebang if it exists let offset = text.startsWith("#!") ? text.indexOf("\n") : 0;