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

Commit

Permalink
[max-line-length] ignore strings and regex in max line length (#4798)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
vmk1vmk authored and Josh Goldberg committed Jul 16, 2019
1 parent 1e48ae7 commit b297249
Show file tree
Hide file tree
Showing 6 changed files with 171 additions and 35 deletions.
128 changes: 102 additions & 26 deletions src/rules/maxLineLengthRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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",
Expand All @@ -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,
},
Expand All @@ -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,
},
],
],
Expand All @@ -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;
}

Expand All @@ -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<MaxLineLengthRuleOptions>) {
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<MaxLineLengthRuleOptions>,
): 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;
}
33 changes: 33 additions & 0 deletions test/rules/max-line-length/check-strings-and-regex/test.ts.lint
Original file line number Diff line number Diff line change
@@ -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
10 changes: 10 additions & 0 deletions test/rules/max-line-length/check-strings-and-regex/tslint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"rules": {
"max-line-length": [true, {
"limit": "140",
"ignore-pattern": "^import ",
"check-strings": true,
"check-regex": true
}]
}
}
25 changes: 25 additions & 0 deletions test/rules/max-line-length/default/test.ts.lint
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
{
"rules": {
"max-line-length": [true, {
"limit": 140,
"ignore-pattern": "^import "
"limit": 140
}]
}
}
7 changes: 0 additions & 7 deletions test/rules/max-line-length/test.ts.lint

This file was deleted.

0 comments on commit b297249

Please sign in to comment.