From b2972495e05710fa55600c233bf46a8a5c02e3cd Mon Sep 17 00:00:00 2001 From: Vitalij Krotov Date: Wed, 17 Jul 2019 00:21:00 +0200 Subject: [PATCH] [max-line-length] ignore strings and regex in max line length (#4798) * Added option to ignore max line lenght errors when caused by strings or template strings. * Added tests for default state, when strings are ignored, and for string checks enabled. * Fixed linter errors. * Use different functions to detect if node is a string, because of tests with older versions. * Using simple comparison to detect node kind. * Fully switched to simple comparison instead of helper functions. * Added option to check regular expressions if they exceed the maximum line length. * Moved ignore check logic into it's own function. * Renamed test folder name to express that strings and regex are checked. * Refactored hardcoded option strings with constant variables that are used consistently. * Changed max line length rule options interfacee back to camelcase and combined the two consecutive filter calls into one. --- src/rules/maxLineLengthRule.ts | 128 ++++++++++++++---- .../check-strings-and-regex/test.ts.lint | 33 +++++ .../check-strings-and-regex/tslint.json | 10 ++ .../max-line-length/default/test.ts.lint | 25 ++++ .../max-line-length/{ => default}/tslint.json | 3 +- test/rules/max-line-length/test.ts.lint | 7 - 6 files changed, 171 insertions(+), 35 deletions(-) create mode 100644 test/rules/max-line-length/check-strings-and-regex/test.ts.lint create mode 100644 test/rules/max-line-length/check-strings-and-regex/tslint.json create mode 100644 test/rules/max-line-length/default/test.ts.lint rename test/rules/max-line-length/{ => default}/tslint.json (52%) delete mode 100644 test/rules/max-line-length/test.ts.lint diff --git a/src/rules/maxLineLengthRule.ts b/src/rules/maxLineLengthRule.ts index b84ee2b1f76..8cc36992c98 100644 --- a/src/rules/maxLineLengthRule.ts +++ b/src/rules/maxLineLengthRule.ts @@ -15,14 +15,21 @@ * limitations under the License. */ -import { getLineRanges } from "tsutils"; +import { getLineRanges, getTokenAtPosition, LineRange } from "tsutils"; import * as ts from "typescript"; import * as Lint from "../index"; +const OPTION_LIMIT = "limit"; +const OPTION_IGNORE_PATTERN = "ignore-pattern"; +const OPTION_CHECK_STRINGS = "check-strings"; +const OPTION_CHECK_REGEX = "check-regex"; + interface MaxLineLengthRuleOptions { limit: number; ignorePattern?: RegExp; + checkStrings?: boolean; + checkRegex?: boolean; } export class Rule extends Lint.Rules.AbstractRule { @@ -38,14 +45,16 @@ export class Rule extends Lint.Rules.AbstractRule { It can take one argument, which can be any of the following: * integer indicating maximum length of lines. * object with keys: - * \`limit\` - number greater than 0 defining the max line length - * \`ignore-pattern\` - string defining ignore pattern for this rule, being parsed by \`new RegExp()\`. + * \`${OPTION_LIMIT}\` - number greater than 0 defining the max line length + * \`${OPTION_IGNORE_PATTERN}\` - string defining ignore pattern for this rule, being parsed by \`new RegExp()\`. For example: * \`\/\/ \` pattern will ignore all in-line comments. * \`^import \` pattern will ignore all import statements. * \`^export \{(.*?)\}\` pattern will ignore all multiple export statements. * \`class [a-zA-Z]+ implements \` pattern will ignore all class declarations implementing interfaces. * \`^import |^export \{(.*?)\}|class [a-zA-Z]+ implements |// \` pattern will ignore all the cases listed above. + * \`${OPTION_CHECK_STRINGS}\` - determines if strings should be checked, \`false\` by default. + * \`${OPTION_CHECK_REGEX}\` - determines if regular expressions should be checked, \`false\` by default. `, options: { type: "array", @@ -57,8 +66,10 @@ export class Rule extends Lint.Rules.AbstractRule { { type: "object", properties: { - limit: { type: "number" }, - "ignore-pattern": { type: "string" }, + [OPTION_LIMIT]: { type: "number" }, + [OPTION_IGNORE_PATTERN]: { type: "string" }, + [OPTION_CHECK_STRINGS]: { type: "boolean" }, + [OPTION_CHECK_REGEX]: { type: "boolean" }, }, additionalProperties: false, }, @@ -72,8 +83,10 @@ export class Rule extends Lint.Rules.AbstractRule { [ true, { - limit: 120, - "ignore-pattern": "^import |^export {(.*?)}", + [OPTION_LIMIT]: 120, + [OPTION_IGNORE_PATTERN]: "^import |^export {(.*?)}", + [OPTION_CHECK_STRINGS]: true, + [OPTION_CHECK_REGEX]: true, }, ], ], @@ -87,7 +100,7 @@ export class Rule extends Lint.Rules.AbstractRule { } public isEnabled(): boolean { - const limit = this.getRuleOptions().limit; + const limit = this.getRuleOptions()[OPTION_LIMIT]; return super.isEnabled() && limit > 0; } @@ -96,32 +109,95 @@ export class Rule extends Lint.Rules.AbstractRule { } private getRuleOptions(): MaxLineLengthRuleOptions { - const argument = this.ruleArguments[0]; - let options: MaxLineLengthRuleOptions = { limit: 0 }; + const argument = this.ruleArguments[0] as { [key: string]: any } | number; + if (typeof argument === "number") { - options.limit = argument; + return { limit: argument }; } else { - options = argument as MaxLineLengthRuleOptions; - const ignorePattern = (argument as { [key: string]: string })["ignore-pattern"]; - options.ignorePattern = - typeof ignorePattern === "string" ? new RegExp(ignorePattern) : undefined; + const { + [OPTION_CHECK_REGEX]: checkRegex, + [OPTION_CHECK_STRINGS]: checkStrings, + [OPTION_IGNORE_PATTERN]: ignorePattern, + [OPTION_LIMIT]: limit, + } = argument; + + return { + checkRegex: Boolean(checkRegex), + checkStrings: Boolean(checkStrings), + ignorePattern: + typeof ignorePattern === "string" ? new RegExp(ignorePattern) : undefined, + limit: Number(limit), + }; } - options.limit = Number(options.limit); // user can pass a string instead of number - return options; } } function walk(ctx: Lint.WalkContext) { - const limit = ctx.options.limit; - const ignorePattern = ctx.options.ignorePattern; - for (const line of getLineRanges(ctx.sourceFile)) { - if (line.contentLength <= limit) { - continue; + const { limit } = ctx.options; + + getLineRanges(ctx.sourceFile) + .filter( + (line: LineRange): boolean => + line.contentLength > limit && !shouldIgnoreLine(line, ctx), + ) + .forEach(({ pos, contentLength }: LineRange) => + ctx.addFailureAt(pos, contentLength, Rule.FAILURE_STRING_FACTORY(limit)), + ); + + return; +} + +function shouldIgnoreLine( + { pos, contentLength }: LineRange, + { options, sourceFile }: Lint.WalkContext, +): boolean { + const { checkRegex, checkStrings, ignorePattern, limit } = options; + + let shouldOmitLine = false; + + if (ignorePattern !== undefined) { + shouldOmitLine = + shouldOmitLine || ignorePattern.test(sourceFile.text.substr(pos, contentLength)); + } + + if (!checkStrings) { + const nodeAtLimit: ts.Node | undefined = getTokenAtPosition(sourceFile, pos + limit); + + if (nodeAtLimit !== undefined) { + shouldOmitLine = + shouldOmitLine || + nodeAtLimit.kind === ts.SyntaxKind.StringLiteral || + nodeAtLimit.kind === ts.SyntaxKind.NoSubstitutionTemplateLiteral || + hasParentMatchingTypes(nodeAtLimit, sourceFile, [ts.SyntaxKind.TemplateExpression]); } - const lineContent = ctx.sourceFile.text.substr(line.pos, line.contentLength); - if (ignorePattern !== undefined && ignorePattern.test(lineContent)) { - continue; + } + + if (!checkRegex) { + const nodeAtLimit: ts.Node | undefined = getTokenAtPosition(sourceFile, pos + limit); + + if (nodeAtLimit !== undefined) { + shouldOmitLine = + shouldOmitLine || nodeAtLimit.kind === ts.SyntaxKind.RegularExpressionLiteral; } - ctx.addFailureAt(line.pos, line.contentLength, Rule.FAILURE_STRING_FACTORY(limit)); } + + return shouldOmitLine; +} + +function hasParentMatchingTypes( + node: ts.Node, + root: ts.Node, + parentTypes: ts.SyntaxKind[], +): boolean { + let nodeReference: ts.Node = node; + + while (nodeReference !== root) { + if (parentTypes.indexOf(nodeReference.kind) >= 0) { + return true; + } + + nodeReference = nodeReference.parent; + } + + return false; } diff --git a/test/rules/max-line-length/check-strings-and-regex/test.ts.lint b/test/rules/max-line-length/check-strings-and-regex/test.ts.lint new file mode 100644 index 00000000000..438b13a6f8e --- /dev/null +++ b/test/rules/max-line-length/check-strings-and-regex/test.ts.lint @@ -0,0 +1,33 @@ +import { KindOfOneVerySpecificComponentOrSomethingLikeThis } from '../../../../very/very/very/very/very/long/and/complicated/and/nested/directory/structure/target'; + +var simpleName = 1; +var complicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedName; +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] + +var complicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicat; + +const extremelyLongSingleQuoteString = 'Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam volu'; +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] + +const extremelyLongDoubleQuoteString = "Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam volu"; +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] + +const extremelyLongTemplateString = `Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam volu`; +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] + +const extremelyLongTemplateStringWithVariableAtLimit = `Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod ${simpleName} ut labore et dolore magna aliquyam `; +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] + +const multiLineTemplateString = ` +Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam volu +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] +Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam volu +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] +Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam volu +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] +`; + +const veryLongRegularExpression = /\s*********************************************************************************************************************/; +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] + +[0]: Exceeds maximum line length of 140 \ No newline at end of file diff --git a/test/rules/max-line-length/check-strings-and-regex/tslint.json b/test/rules/max-line-length/check-strings-and-regex/tslint.json new file mode 100644 index 00000000000..fd6631ccaae --- /dev/null +++ b/test/rules/max-line-length/check-strings-and-regex/tslint.json @@ -0,0 +1,10 @@ +{ + "rules": { + "max-line-length": [true, { + "limit": "140", + "ignore-pattern": "^import ", + "check-strings": true, + "check-regex": true + }] + } +} diff --git a/test/rules/max-line-length/default/test.ts.lint b/test/rules/max-line-length/default/test.ts.lint new file mode 100644 index 00000000000..eb9b6dc1172 --- /dev/null +++ b/test/rules/max-line-length/default/test.ts.lint @@ -0,0 +1,25 @@ +import { KindOfOneVerySpecificComponentOrSomethingLikeThis } from '../../../../very/very/very/very/very/long/and/complicated/and/nested/directory/structure/target'; + +var simpleName = 1; +var complicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedName; +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] + +var complicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicat; + +const extremelyLongSingleQuoteString = 'Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam volu'; + +const extremelyLongDoubleQuoteString = "Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam volu"; + +const extremelyLongTemplateString = `Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam volu`; + +const extremelyLongTemplateStringWithVariableAtLimit = `Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod ${someVariable} ut labore et dolore magna aliquyam `; + +const multiLineTemplateString = ` +Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam volu +Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam volu +Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam volu +`; + +const veryLongRegularExpression = /\s*********************************************************************************************************************/; + +[0]: Exceeds maximum line length of 140 \ No newline at end of file diff --git a/test/rules/max-line-length/tslint.json b/test/rules/max-line-length/default/tslint.json similarity index 52% rename from test/rules/max-line-length/tslint.json rename to test/rules/max-line-length/default/tslint.json index 365785eae5f..5937f62d6a7 100644 --- a/test/rules/max-line-length/tslint.json +++ b/test/rules/max-line-length/default/tslint.json @@ -1,8 +1,7 @@ { "rules": { "max-line-length": [true, { - "limit": 140, - "ignore-pattern": "^import " + "limit": 140 }] } } diff --git a/test/rules/max-line-length/test.ts.lint b/test/rules/max-line-length/test.ts.lint deleted file mode 100644 index f9a5b9d6a4d..00000000000 --- a/test/rules/max-line-length/test.ts.lint +++ /dev/null @@ -1,7 +0,0 @@ -import { KindOfOneVerySpecificComponentOrSomethingLikeThis } from '../../../../very/very/very/very/very/long/and/complicated/and/nested/directory/structure/target'; - -var simpleName = 1; -var complicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedName; -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Exceeds maximum line length of 140] - -var complicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicatedNameIsAcomplicat;