From 6cdbc2bf51f4fba689666054eb4da71cc78da225 Mon Sep 17 00:00:00 2001 From: vmk1vmk Date: Sat, 13 Jul 2019 22:22:52 +0200 Subject: [PATCH 01/11] Added option to ignore max line lenght errors when caused by strings or template strings. --- src/rules/maxLineLengthRule.ts | 94 ++++++++++++++++++++++++++++------ 1 file changed, 77 insertions(+), 17 deletions(-) diff --git a/src/rules/maxLineLengthRule.ts b/src/rules/maxLineLengthRule.ts index b84ee2b1f76..5ce1e78066f 100644 --- a/src/rules/maxLineLengthRule.ts +++ b/src/rules/maxLineLengthRule.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { getLineRanges } from "tsutils"; +import { getLineRanges, LineRange, getTokenAtPosition } from "tsutils"; import * as ts from "typescript"; import * as Lint from "../index"; @@ -23,6 +23,7 @@ import * as Lint from "../index"; interface MaxLineLengthRuleOptions { limit: number; ignorePattern?: RegExp; + checkStrings?: boolean; } export class Rule extends Lint.Rules.AbstractRule { @@ -46,6 +47,7 @@ export class Rule extends Lint.Rules.AbstractRule { * \`^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. + * \`check-strings\` - determines if strings should be checked, \`false\` by default. `, options: { type: "array", @@ -59,9 +61,9 @@ export class Rule extends Lint.Rules.AbstractRule { properties: { limit: { type: "number" }, "ignore-pattern": { type: "string" }, - }, - additionalProperties: false, - }, + "check-strings": { type: "boolean" } + } + } ], }, minLength: 1, @@ -76,6 +78,13 @@ export class Rule extends Lint.Rules.AbstractRule { "ignore-pattern": "^import |^export {(.*?)}", }, ], + [ + true, + { + limit: 120, + "check-strings": true + } + ] ], type: "formatting", typescriptOnly: false, @@ -98,30 +107,81 @@ export class Rule extends Lint.Rules.AbstractRule { private getRuleOptions(): MaxLineLengthRuleOptions { const argument = this.ruleArguments[0]; let options: MaxLineLengthRuleOptions = { limit: 0 }; + if (typeof argument === "number") { options.limit = argument; } else { - options = argument as MaxLineLengthRuleOptions; - const ignorePattern = (argument as { [key: string]: string })["ignore-pattern"]; + const { + "limit": limit, + "ignore-pattern": ignorePattern, + "check-strings": checkStrings + } = argument as { + "limit": number, + "ignore-pattern"?: string, + "check-strings"?: boolean + }; + + options.limit = Number(limit); + options.ignorePattern = typeof ignorePattern === "string" ? new RegExp(ignorePattern) : undefined; + + options.checkStrings = Boolean(checkStrings); } - 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 lineContent = ctx.sourceFile.text.substr(line.pos, line.contentLength); - if (ignorePattern !== undefined && ignorePattern.test(lineContent)) { - continue; + const { limit, ignorePattern, checkStrings } = ctx.options; + + getLineRanges(ctx.sourceFile) + .filter(({ contentLength }: LineRange): boolean => contentLength > limit) + .filter( + ({ pos, contentLength }: LineRange): boolean => { + let shouldIgnoreLine: boolean = false; + + if (ignorePattern !== undefined) { + shouldIgnoreLine = + shouldIgnoreLine || + ignorePattern.test(ctx.sourceFile.text.substr(pos, contentLength)); + } + + if (!checkStrings) { + const nodeAtLimit: ts.Node | undefined = getTokenAtPosition( + ctx.sourceFile, + pos + limit, + ); + + if (nodeAtLimit) { + shouldIgnoreLine = shouldIgnoreLine || isPartOfStringOrTemplate(nodeAtLimit, ctx.sourceFile); + } + } + + return !shouldIgnoreLine; + }, + ) + .forEach(({ pos, contentLength }: LineRange) => + ctx.addFailureAt(pos, contentLength, Rule.FAILURE_STRING_FACTORY(limit)), + ); + + return; +} + +function isPartOfStringOrTemplate(node: ts.Node, root: ts.Node): boolean { + let nodeReference: ts.Node = node; + + while (nodeReference !== root) { + if ( + ts.isStringLiteralLike(nodeReference) || + ts.isTemplateExpression(nodeReference) + ) { + return true; } - ctx.addFailureAt(line.pos, line.contentLength, Rule.FAILURE_STRING_FACTORY(limit)); + + nodeReference = nodeReference.parent; } + + return false; } From cd0a4751a7a2eba93f8e5413ecece362a07e6ea8 Mon Sep 17 00:00:00 2001 From: vmk1vmk Date: Sat, 13 Jul 2019 22:24:00 +0200 Subject: [PATCH 02/11] Added tests for default state, when strings are ignored, and for string checks enabled. --- .../check-strings/test.ts.lint | 21 +++++++++++++++++++ .../max-line-length/check-strings/tslint.json | 9 ++++++++ .../max-line-length/default/test.ts.lint | 17 +++++++++++++++ .../max-line-length/{ => default}/tslint.json | 3 +-- test/rules/max-line-length/test.ts.lint | 7 ------- 5 files changed, 48 insertions(+), 9 deletions(-) create mode 100644 test/rules/max-line-length/check-strings/test.ts.lint create mode 100644 test/rules/max-line-length/check-strings/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/test/rules/max-line-length/check-strings/test.ts.lint b/test/rules/max-line-length/check-strings/test.ts.lint new file mode 100644 index 00000000000..6f69842205c --- /dev/null +++ b/test/rules/max-line-length/check-strings/test.ts.lint @@ -0,0 +1,21 @@ +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] + +[0]: Exceeds maximum line length of 140 \ No newline at end of file diff --git a/test/rules/max-line-length/check-strings/tslint.json b/test/rules/max-line-length/check-strings/tslint.json new file mode 100644 index 00000000000..cf4f89eff0c --- /dev/null +++ b/test/rules/max-line-length/check-strings/tslint.json @@ -0,0 +1,9 @@ +{ + "rules": { + "max-line-length": [true, { + "limit": "140", + "ignore-pattern": "^import ", + "check-strings": 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..ce3f49f3348 --- /dev/null +++ b/test/rules/max-line-length/default/test.ts.lint @@ -0,0 +1,17 @@ +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 ${some-variable} ut labore et dolore magna aliquyam `; + +[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; From f83a8bf1ced13e0f4025febd6210c676df4353fd Mon Sep 17 00:00:00 2001 From: vmk1vmk Date: Sat, 13 Jul 2019 22:35:13 +0200 Subject: [PATCH 03/11] Fixed linter errors. --- src/rules/maxLineLengthRule.ts | 43 +++++++++++++++++----------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/src/rules/maxLineLengthRule.ts b/src/rules/maxLineLengthRule.ts index 5ce1e78066f..c0edbd2abd7 100644 --- a/src/rules/maxLineLengthRule.ts +++ b/src/rules/maxLineLengthRule.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { getLineRanges, LineRange, getTokenAtPosition } from "tsutils"; +import { getLineRanges, getTokenAtPosition, LineRange } from "tsutils"; import * as ts from "typescript"; import * as Lint from "../index"; @@ -61,9 +61,9 @@ export class Rule extends Lint.Rules.AbstractRule { properties: { limit: { type: "number" }, "ignore-pattern": { type: "string" }, - "check-strings": { type: "boolean" } - } - } + "check-strings": { type: "boolean" }, + }, + }, ], }, minLength: 1, @@ -82,9 +82,9 @@ export class Rule extends Lint.Rules.AbstractRule { true, { limit: 120, - "check-strings": true - } - ] + "check-strings": true, + }, + ], ], type: "formatting", typescriptOnly: false, @@ -106,20 +106,20 @@ export class Rule extends Lint.Rules.AbstractRule { private getRuleOptions(): MaxLineLengthRuleOptions { const argument = this.ruleArguments[0]; - let options: MaxLineLengthRuleOptions = { limit: 0 }; + const options: MaxLineLengthRuleOptions = { limit: 0 }; if (typeof argument === "number") { options.limit = argument; } else { const { - "limit": limit, + limit: limit, "ignore-pattern": ignorePattern, - "check-strings": checkStrings - } = argument as { - "limit": number, - "ignore-pattern"?: string, - "check-strings"?: boolean - }; + "check-strings": checkStrings, + } = argument as { + limit: number; + "ignore-pattern"?: string; + "check-strings"?: boolean; + }; options.limit = Number(limit); @@ -140,7 +140,7 @@ function walk(ctx: Lint.WalkContext) { .filter(({ contentLength }: LineRange): boolean => contentLength > limit) .filter( ({ pos, contentLength }: LineRange): boolean => { - let shouldIgnoreLine: boolean = false; + let shouldIgnoreLine = false; if (ignorePattern !== undefined) { shouldIgnoreLine = @@ -154,8 +154,10 @@ function walk(ctx: Lint.WalkContext) { pos + limit, ); - if (nodeAtLimit) { - shouldIgnoreLine = shouldIgnoreLine || isPartOfStringOrTemplate(nodeAtLimit, ctx.sourceFile); + if (nodeAtLimit !== undefined) { + shouldIgnoreLine = + shouldIgnoreLine || + isPartOfStringOrTemplate(nodeAtLimit, ctx.sourceFile); } } @@ -173,10 +175,7 @@ function isPartOfStringOrTemplate(node: ts.Node, root: ts.Node): boolean { let nodeReference: ts.Node = node; while (nodeReference !== root) { - if ( - ts.isStringLiteralLike(nodeReference) || - ts.isTemplateExpression(nodeReference) - ) { + if (ts.isStringLiteralLike(nodeReference) || ts.isTemplateExpression(nodeReference)) { return true; } From 0dce49fe04d88bba90e8c4e0de7624047d21449f Mon Sep 17 00:00:00 2001 From: vmk1vmk Date: Sat, 13 Jul 2019 22:49:05 +0200 Subject: [PATCH 04/11] Use different functions to detect if node is a string, because of tests with older versions. --- src/rules/maxLineLengthRule.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/rules/maxLineLengthRule.ts b/src/rules/maxLineLengthRule.ts index c0edbd2abd7..8a5b7697934 100644 --- a/src/rules/maxLineLengthRule.ts +++ b/src/rules/maxLineLengthRule.ts @@ -157,7 +157,9 @@ function walk(ctx: Lint.WalkContext) { if (nodeAtLimit !== undefined) { shouldIgnoreLine = shouldIgnoreLine || - isPartOfStringOrTemplate(nodeAtLimit, ctx.sourceFile); + ts.isStringLiteral(nodeAtLimit) || + ts.isNoSubstitutionTemplateLiteral(nodeAtLimit) || + isPartOfTemplate(nodeAtLimit, ctx.sourceFile); } } @@ -171,11 +173,11 @@ function walk(ctx: Lint.WalkContext) { return; } -function isPartOfStringOrTemplate(node: ts.Node, root: ts.Node): boolean { +function isPartOfTemplate(node: ts.Node, root: ts.Node): boolean { let nodeReference: ts.Node = node; while (nodeReference !== root) { - if (ts.isStringLiteralLike(nodeReference) || ts.isTemplateExpression(nodeReference)) { + if (ts.isTemplateExpression(nodeReference)) { return true; } From f1eeae3870468875d70a25c18a6d33be5673915d Mon Sep 17 00:00:00 2001 From: vmk1vmk Date: Sat, 13 Jul 2019 22:53:45 +0200 Subject: [PATCH 05/11] Using simple comparison to detect node kind. --- src/rules/maxLineLengthRule.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rules/maxLineLengthRule.ts b/src/rules/maxLineLengthRule.ts index 8a5b7697934..4677707b252 100644 --- a/src/rules/maxLineLengthRule.ts +++ b/src/rules/maxLineLengthRule.ts @@ -157,8 +157,8 @@ function walk(ctx: Lint.WalkContext) { if (nodeAtLimit !== undefined) { shouldIgnoreLine = shouldIgnoreLine || - ts.isStringLiteral(nodeAtLimit) || - ts.isNoSubstitutionTemplateLiteral(nodeAtLimit) || + nodeAtLimit.kind === ts.SyntaxKind.StringLiteral || + nodeAtLimit.kind === ts.SyntaxKind.NoSubstitutionTemplateLiteral || isPartOfTemplate(nodeAtLimit, ctx.sourceFile); } } From 0223f810e15ab9a8e33504ecdcee67856ba0ceba Mon Sep 17 00:00:00 2001 From: vmk1vmk Date: Sat, 13 Jul 2019 22:57:28 +0200 Subject: [PATCH 06/11] Fully switched to simple comparison instead of helper functions. --- src/rules/maxLineLengthRule.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/maxLineLengthRule.ts b/src/rules/maxLineLengthRule.ts index 4677707b252..de9075d17a0 100644 --- a/src/rules/maxLineLengthRule.ts +++ b/src/rules/maxLineLengthRule.ts @@ -177,7 +177,7 @@ function isPartOfTemplate(node: ts.Node, root: ts.Node): boolean { let nodeReference: ts.Node = node; while (nodeReference !== root) { - if (ts.isTemplateExpression(nodeReference)) { + if (nodeReference.kind === ts.SyntaxKind.TemplateExpression) { return true; } From 321d081c94de5c887753e5bd1e4b40adc8f7df52 Mon Sep 17 00:00:00 2001 From: vmk1vmk Date: Sun, 14 Jul 2019 04:05:58 +0200 Subject: [PATCH 07/11] Added option to check regular expressions if they exceed the maximum line length. --- src/rules/maxLineLengthRule.ts | 40 ++++++++++++++----- .../check-strings/test.ts.lint | 12 ++++++ .../max-line-length/check-strings/tslint.json | 3 +- .../max-line-length/default/test.ts.lint | 10 ++++- 4 files changed, 53 insertions(+), 12 deletions(-) diff --git a/src/rules/maxLineLengthRule.ts b/src/rules/maxLineLengthRule.ts index de9075d17a0..3936a349645 100644 --- a/src/rules/maxLineLengthRule.ts +++ b/src/rules/maxLineLengthRule.ts @@ -24,6 +24,7 @@ interface MaxLineLengthRuleOptions { limit: number; ignorePattern?: RegExp; checkStrings?: boolean; + checkRegex?: boolean; } export class Rule extends Lint.Rules.AbstractRule { @@ -48,6 +49,7 @@ export class Rule extends Lint.Rules.AbstractRule { * \`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. * \`check-strings\` - determines if strings should be checked, \`false\` by default. + * \`check-regex\` - determines if regular expressions should be checked, \`false\` by default. `, options: { type: "array", @@ -62,6 +64,7 @@ export class Rule extends Lint.Rules.AbstractRule { limit: { type: "number" }, "ignore-pattern": { type: "string" }, "check-strings": { type: "boolean" }, + "check-regex": { type: "boolean" }, }, }, ], @@ -76,13 +79,8 @@ export class Rule extends Lint.Rules.AbstractRule { { limit: 120, "ignore-pattern": "^import |^export {(.*?)}", - }, - ], - [ - true, - { - limit: 120, "check-strings": true, + "check-regex": true, }, ], ], @@ -115,10 +113,12 @@ export class Rule extends Lint.Rules.AbstractRule { limit: limit, "ignore-pattern": ignorePattern, "check-strings": checkStrings, + "check-regex": checkRegex, } = argument as { limit: number; "ignore-pattern"?: string; "check-strings"?: boolean; + "check-regex"?: boolean; }; options.limit = Number(limit); @@ -127,6 +127,7 @@ export class Rule extends Lint.Rules.AbstractRule { typeof ignorePattern === "string" ? new RegExp(ignorePattern) : undefined; options.checkStrings = Boolean(checkStrings); + options.checkRegex = Boolean(checkRegex); } return options; @@ -134,7 +135,7 @@ export class Rule extends Lint.Rules.AbstractRule { } function walk(ctx: Lint.WalkContext) { - const { limit, ignorePattern, checkStrings } = ctx.options; + const { limit, ignorePattern, checkStrings, checkRegex } = ctx.options; getLineRanges(ctx.sourceFile) .filter(({ contentLength }: LineRange): boolean => contentLength > limit) @@ -159,7 +160,22 @@ function walk(ctx: Lint.WalkContext) { shouldIgnoreLine || nodeAtLimit.kind === ts.SyntaxKind.StringLiteral || nodeAtLimit.kind === ts.SyntaxKind.NoSubstitutionTemplateLiteral || - isPartOfTemplate(nodeAtLimit, ctx.sourceFile); + hasParentMatchingTypes(nodeAtLimit, ctx.sourceFile, [ + ts.SyntaxKind.TemplateExpression, + ]); + } + } + + if (!checkRegex) { + const nodeAtLimit: ts.Node | undefined = getTokenAtPosition( + ctx.sourceFile, + pos + limit, + ); + + if (nodeAtLimit !== undefined) { + shouldIgnoreLine = + shouldIgnoreLine || + nodeAtLimit.kind === ts.SyntaxKind.RegularExpressionLiteral; } } @@ -173,11 +189,15 @@ function walk(ctx: Lint.WalkContext) { return; } -function isPartOfTemplate(node: ts.Node, root: ts.Node): boolean { +function hasParentMatchingTypes( + node: ts.Node, + root: ts.Node, + parentTypes: ts.SyntaxKind[], +): boolean { let nodeReference: ts.Node = node; while (nodeReference !== root) { - if (nodeReference.kind === ts.SyntaxKind.TemplateExpression) { + if (parentTypes.indexOf(nodeReference.kind) >= 0) { return true; } diff --git a/test/rules/max-line-length/check-strings/test.ts.lint b/test/rules/max-line-length/check-strings/test.ts.lint index 6f69842205c..438b13a6f8e 100644 --- a/test/rules/max-line-length/check-strings/test.ts.lint +++ b/test/rules/max-line-length/check-strings/test.ts.lint @@ -18,4 +18,16 @@ const extremelyLongTemplateString = `Lorem ipsum dolor sit amet, consetetur sadi 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/tslint.json b/test/rules/max-line-length/check-strings/tslint.json index cf4f89eff0c..fd6631ccaae 100644 --- a/test/rules/max-line-length/check-strings/tslint.json +++ b/test/rules/max-line-length/check-strings/tslint.json @@ -3,7 +3,8 @@ "max-line-length": [true, { "limit": "140", "ignore-pattern": "^import ", - "check-strings": true + "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 index ce3f49f3348..eb9b6dc1172 100644 --- a/test/rules/max-line-length/default/test.ts.lint +++ b/test/rules/max-line-length/default/test.ts.lint @@ -12,6 +12,14 @@ const extremelyLongDoubleQuoteString = "Lorem ipsum dolor sit amet, consetetur s 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 ${some-variable} ut labore et dolore magna aliquyam `; +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 From f8cb5b587fadca94bf3238fa8a29e7c26bff6e90 Mon Sep 17 00:00:00 2001 From: vmk1vmk Date: Sun, 14 Jul 2019 04:22:47 +0200 Subject: [PATCH 08/11] Moved ignore check logic into it's own function. --- src/rules/maxLineLengthRule.ts | 83 ++++++++++++++++------------------ 1 file changed, 39 insertions(+), 44 deletions(-) diff --git a/src/rules/maxLineLengthRule.ts b/src/rules/maxLineLengthRule.ts index 3936a349645..057c321c42f 100644 --- a/src/rules/maxLineLengthRule.ts +++ b/src/rules/maxLineLengthRule.ts @@ -135,53 +135,11 @@ export class Rule extends Lint.Rules.AbstractRule { } function walk(ctx: Lint.WalkContext) { - const { limit, ignorePattern, checkStrings, checkRegex } = ctx.options; + const { limit } = ctx.options; getLineRanges(ctx.sourceFile) .filter(({ contentLength }: LineRange): boolean => contentLength > limit) - .filter( - ({ pos, contentLength }: LineRange): boolean => { - let shouldIgnoreLine = false; - - if (ignorePattern !== undefined) { - shouldIgnoreLine = - shouldIgnoreLine || - ignorePattern.test(ctx.sourceFile.text.substr(pos, contentLength)); - } - - if (!checkStrings) { - const nodeAtLimit: ts.Node | undefined = getTokenAtPosition( - ctx.sourceFile, - pos + limit, - ); - - if (nodeAtLimit !== undefined) { - shouldIgnoreLine = - shouldIgnoreLine || - nodeAtLimit.kind === ts.SyntaxKind.StringLiteral || - nodeAtLimit.kind === ts.SyntaxKind.NoSubstitutionTemplateLiteral || - hasParentMatchingTypes(nodeAtLimit, ctx.sourceFile, [ - ts.SyntaxKind.TemplateExpression, - ]); - } - } - - if (!checkRegex) { - const nodeAtLimit: ts.Node | undefined = getTokenAtPosition( - ctx.sourceFile, - pos + limit, - ); - - if (nodeAtLimit !== undefined) { - shouldIgnoreLine = - shouldIgnoreLine || - nodeAtLimit.kind === ts.SyntaxKind.RegularExpressionLiteral; - } - } - - return !shouldIgnoreLine; - }, - ) + .filter((line: LineRange): boolean => !shouldIgnoreLine(line, ctx)) .forEach(({ pos, contentLength }: LineRange) => ctx.addFailureAt(pos, contentLength, Rule.FAILURE_STRING_FACTORY(limit)), ); @@ -189,6 +147,43 @@ function walk(ctx: Lint.WalkContext) { 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]); + } + } + + if (!checkRegex) { + const nodeAtLimit: ts.Node | undefined = getTokenAtPosition(sourceFile, pos + limit); + + if (nodeAtLimit !== undefined) { + shouldOmitLine = + shouldOmitLine || nodeAtLimit.kind === ts.SyntaxKind.RegularExpressionLiteral; + } + } + + return shouldOmitLine; +} + function hasParentMatchingTypes( node: ts.Node, root: ts.Node, From d13826b1f5e475c7811ccc7b1b3fc973ab74ec6c Mon Sep 17 00:00:00 2001 From: vmk1vmk Date: Sun, 14 Jul 2019 04:23:36 +0200 Subject: [PATCH 09/11] Renamed test folder name to express that strings and regex are checked. --- .../{check-strings => check-strings-and-regex}/test.ts.lint | 0 .../{check-strings => check-strings-and-regex}/tslint.json | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename test/rules/max-line-length/{check-strings => check-strings-and-regex}/test.ts.lint (100%) rename test/rules/max-line-length/{check-strings => check-strings-and-regex}/tslint.json (100%) diff --git a/test/rules/max-line-length/check-strings/test.ts.lint b/test/rules/max-line-length/check-strings-and-regex/test.ts.lint similarity index 100% rename from test/rules/max-line-length/check-strings/test.ts.lint rename to test/rules/max-line-length/check-strings-and-regex/test.ts.lint diff --git a/test/rules/max-line-length/check-strings/tslint.json b/test/rules/max-line-length/check-strings-and-regex/tslint.json similarity index 100% rename from test/rules/max-line-length/check-strings/tslint.json rename to test/rules/max-line-length/check-strings-and-regex/tslint.json From f0b4da57ddc682c9869edc2c1330f6f5f94af7d1 Mon Sep 17 00:00:00 2001 From: vmk1vmk Date: Tue, 16 Jul 2019 02:07:58 +0200 Subject: [PATCH 10/11] Refactored hardcoded option strings with constant variables that are used consistently. --- src/rules/maxLineLengthRule.ts | 84 +++++++++++++++++----------------- 1 file changed, 43 insertions(+), 41 deletions(-) diff --git a/src/rules/maxLineLengthRule.ts b/src/rules/maxLineLengthRule.ts index 057c321c42f..0827fe0f06e 100644 --- a/src/rules/maxLineLengthRule.ts +++ b/src/rules/maxLineLengthRule.ts @@ -20,11 +20,16 @@ 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; + [OPTION_LIMIT]: number; + [OPTION_IGNORE_PATTERN]?: RegExp; + [OPTION_CHECK_STRINGS]?: boolean; + [OPTION_CHECK_REGEX]?: boolean; } export class Rule extends Lint.Rules.AbstractRule { @@ -40,16 +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. - * \`check-strings\` - determines if strings should be checked, \`false\` by default. - * \`check-regex\` - determines if regular expressions should be checked, \`false\` by default. + * \`${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", @@ -61,10 +66,10 @@ export class Rule extends Lint.Rules.AbstractRule { { type: "object", properties: { - limit: { type: "number" }, - "ignore-pattern": { type: "string" }, - "check-strings": { type: "boolean" }, - "check-regex": { type: "boolean" }, + [OPTION_LIMIT]: { type: "number" }, + [OPTION_IGNORE_PATTERN]: { type: "string" }, + [OPTION_CHECK_STRINGS]: { type: "boolean" }, + [OPTION_CHECK_REGEX]: { type: "boolean" }, }, }, ], @@ -77,10 +82,10 @@ export class Rule extends Lint.Rules.AbstractRule { [ true, { - limit: 120, - "ignore-pattern": "^import |^export {(.*?)}", - "check-strings": true, - "check-regex": true, + [OPTION_LIMIT]: 120, + [OPTION_IGNORE_PATTERN]: "^import |^export {(.*?)}", + [OPTION_CHECK_STRINGS]: true, + [OPTION_CHECK_REGEX]: true, }, ], ], @@ -94,7 +99,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; } @@ -103,39 +108,31 @@ export class Rule extends Lint.Rules.AbstractRule { } private getRuleOptions(): MaxLineLengthRuleOptions { - const argument = this.ruleArguments[0]; - const options: MaxLineLengthRuleOptions = { limit: 0 }; + const argument = this.ruleArguments[0] as MaxLineLengthRuleOptions | number; if (typeof argument === "number") { - options.limit = argument; + return { [OPTION_LIMIT]: argument }; } else { const { - limit: limit, - "ignore-pattern": ignorePattern, - "check-strings": checkStrings, - "check-regex": checkRegex, - } = argument as { - limit: number; - "ignore-pattern"?: string; - "check-strings"?: boolean; - "check-regex"?: boolean; + [OPTION_CHECK_REGEX]: checkRegex, + [OPTION_CHECK_STRINGS]: checkStrings, + [OPTION_IGNORE_PATTERN]: ignorePattern, + [OPTION_LIMIT]: limit, + } = argument; + + return { + [OPTION_CHECK_REGEX]: Boolean(checkRegex), + [OPTION_CHECK_STRINGS]: Boolean(checkStrings), + [OPTION_IGNORE_PATTERN]: + typeof ignorePattern === "string" ? new RegExp(ignorePattern) : undefined, + [OPTION_LIMIT]: Number(limit), }; - - options.limit = Number(limit); - - options.ignorePattern = - typeof ignorePattern === "string" ? new RegExp(ignorePattern) : undefined; - - options.checkStrings = Boolean(checkStrings); - options.checkRegex = Boolean(checkRegex); } - - return options; } } function walk(ctx: Lint.WalkContext) { - const { limit } = ctx.options; + const { [OPTION_LIMIT]: limit } = ctx.options; getLineRanges(ctx.sourceFile) .filter(({ contentLength }: LineRange): boolean => contentLength > limit) @@ -151,7 +148,12 @@ function shouldIgnoreLine( { pos, contentLength }: LineRange, { options, sourceFile }: Lint.WalkContext, ): boolean { - const { checkRegex, checkStrings, ignorePattern, limit } = options; + const { + [OPTION_CHECK_REGEX]: checkRegex, + [OPTION_CHECK_STRINGS]: checkStrings, + [OPTION_IGNORE_PATTERN]: ignorePattern, + [OPTION_LIMIT]: limit, + } = options; let shouldOmitLine = false; From c3554d632d4743b1a817828ed4a6e8de49bd5922 Mon Sep 17 00:00:00 2001 From: vmk1vmk Date: Wed, 17 Jul 2019 00:04:48 +0200 Subject: [PATCH 11/11] Changed max line length rule options interfacee back to camelcase and combined the two consecutive filter calls into one. --- src/rules/maxLineLengthRule.ts | 36 ++++++++++++++++------------------ 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/src/rules/maxLineLengthRule.ts b/src/rules/maxLineLengthRule.ts index 0827fe0f06e..8cc36992c98 100644 --- a/src/rules/maxLineLengthRule.ts +++ b/src/rules/maxLineLengthRule.ts @@ -26,10 +26,10 @@ const OPTION_CHECK_STRINGS = "check-strings"; const OPTION_CHECK_REGEX = "check-regex"; interface MaxLineLengthRuleOptions { - [OPTION_LIMIT]: number; - [OPTION_IGNORE_PATTERN]?: RegExp; - [OPTION_CHECK_STRINGS]?: boolean; - [OPTION_CHECK_REGEX]?: boolean; + limit: number; + ignorePattern?: RegExp; + checkStrings?: boolean; + checkRegex?: boolean; } export class Rule extends Lint.Rules.AbstractRule { @@ -71,6 +71,7 @@ export class Rule extends Lint.Rules.AbstractRule { [OPTION_CHECK_STRINGS]: { type: "boolean" }, [OPTION_CHECK_REGEX]: { type: "boolean" }, }, + additionalProperties: false, }, ], }, @@ -108,10 +109,10 @@ export class Rule extends Lint.Rules.AbstractRule { } private getRuleOptions(): MaxLineLengthRuleOptions { - const argument = this.ruleArguments[0] as MaxLineLengthRuleOptions | number; + const argument = this.ruleArguments[0] as { [key: string]: any } | number; if (typeof argument === "number") { - return { [OPTION_LIMIT]: argument }; + return { limit: argument }; } else { const { [OPTION_CHECK_REGEX]: checkRegex, @@ -121,22 +122,24 @@ export class Rule extends Lint.Rules.AbstractRule { } = argument; return { - [OPTION_CHECK_REGEX]: Boolean(checkRegex), - [OPTION_CHECK_STRINGS]: Boolean(checkStrings), - [OPTION_IGNORE_PATTERN]: + checkRegex: Boolean(checkRegex), + checkStrings: Boolean(checkStrings), + ignorePattern: typeof ignorePattern === "string" ? new RegExp(ignorePattern) : undefined, - [OPTION_LIMIT]: Number(limit), + limit: Number(limit), }; } } } function walk(ctx: Lint.WalkContext) { - const { [OPTION_LIMIT]: limit } = ctx.options; + const { limit } = ctx.options; getLineRanges(ctx.sourceFile) - .filter(({ contentLength }: LineRange): boolean => contentLength > limit) - .filter((line: LineRange): boolean => !shouldIgnoreLine(line, ctx)) + .filter( + (line: LineRange): boolean => + line.contentLength > limit && !shouldIgnoreLine(line, ctx), + ) .forEach(({ pos, contentLength }: LineRange) => ctx.addFailureAt(pos, contentLength, Rule.FAILURE_STRING_FACTORY(limit)), ); @@ -148,12 +151,7 @@ function shouldIgnoreLine( { pos, contentLength }: LineRange, { options, sourceFile }: Lint.WalkContext, ): boolean { - const { - [OPTION_CHECK_REGEX]: checkRegex, - [OPTION_CHECK_STRINGS]: checkStrings, - [OPTION_IGNORE_PATTERN]: ignorePattern, - [OPTION_LIMIT]: limit, - } = options; + const { checkRegex, checkStrings, ignorePattern, limit } = options; let shouldOmitLine = false;