Feat: ignore pattern for max-line-length #3099

DanielKucal · 2017-08-04T23:59:11Z

PR checklist

Addresses an existing issue: max-line-length optionally disabled for import statements #1123, Exclude length from imports via max-line-length #1872
Enhancement
Includes tests
Documentation update

Overview of change:

We can pass a regexp (as a string) to max-line-length rule to ignore some lines, e.g. long imports. Example usage:

{
  "rules": {
    "max-line-length": [true, {"limit": 120, "ignore-pattern": "^import [^,]+ from |^export | implements "}],
  } 
}

will ignore the following lines:

import { SomeLongInterfaceName } from '../../../nested/directory/structure/target';
class MyClass implements SomeLongInterfaceName, OnInit, OnDestroy, OnViewChange {}
export { MyClass, SomeLongInterfaceName };

CHANGELOG.md entry:

[new-rule-option], [enhancement] allow to define ignore pattern for max-line-length rule

ajafff · 2017-08-05T06:36:35Z

Please help me get rid of the Additional parameters are not allowed message.

The schema used by vscode, webstorm etc. to validate your config comes from schemastore.org. It's updated by PR to their repository.

ajafff · 2017-08-05T06:54:31Z

src/rules/maxLineLengthRule.ts

@@ -49,15 +56,18 @@ export class Rule extends Lint.Rules.AbstractRule {
    }

    public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
-        return this.applyWithFunction(sourceFile, walk, this.ruleArguments[0]);
+        return this.applyWithFunction(sourceFile, walk, this.ruleArguments);


Parse the ruleArguments before passing it to applyWithFunction

Do you mean something like this?

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { const options = { ignorePattern: this.ruleArguments[1] && new RegExp(this.ruleArguments[1]), limit: parseInt(this.ruleArguments[0], 10), }; return this.applyWithFunction(sourceFile, walk, options); }

Yeah, that's what I had in mind.

ajafff · 2017-08-05T06:56:26Z

src/rules/maxLineLengthRule.ts

+        optionsDescription: Lint.Utils.dedent`It can take up to 2 arguments, but only first one is required:
+
+        * integer indicating the max length of lines.
+        * string defining ignore pattern, being parsed by \`new RegExp()\`.


Consider to make this an object. That makes it easier to add new options in the future without requiring strict ordering of options.

Do you mean object including the first number parameter this way?

"max-line-length": [true, { limit: 120, ignorePattern: "^import " }],

Can it accept number or object (like number|object) to avoid breaking change?

It should accept number|object. That's exactly what I meant.

DanielKucal · 2017-08-11T02:26:57Z

Hey @ajafff, can you ensure I understood you correctly?

osu5nc · 2017-09-05T19:29:40Z

@DanielKucal @ajafff Is this pull request still being looked at? I see there are some conflicts that need to be resolved before this can be merged.

…quirements

# Conflicts: # src/rules/maxLineLengthRule.ts # yarn.lock

navels · 2017-09-25T18:26:04Z

would love to see this resolved

DanielKucal · 2017-09-25T18:34:53Z

Unfortunately I'm currently unable to make the request changes and resolve conflicts because of issues described in #3244...

sgiradoa-psl · 2017-10-24T16:02:12Z

@DanielKucal Seems like #3244 was closed. How are things on your side?

DanielKucal · 2017-10-27T11:25:27Z

@sgiradoa-psl, am looking for some time to finish this

ajafff · 2017-10-29T20:53:55Z

src/rules/maxLineLengthRule.ts

+    const lineRanges = getLineRanges(ctx.sourceFile);
+    for (let i = 0; i < lines.length; i++) {
+        if (ignorePattern && ignorePattern.test(lines[i])) { continue; }
+        if (lineRanges[i].contentLength <= limit) { continue; }


consider moving this condition one line up to avoid regex matching if the line is not too long

ajafff · 2017-10-29T20:55:56Z

src/rules/maxLineLengthRule.ts

+function walk(ctx: Lint.WalkContext<{limit: number; ignorePattern: RegExp | undefined}>) {
+    const limit = ctx.options.limit;
+    const ignorePattern = ctx.options.ignorePattern;
+    const lines = (ctx.sourceFile && ctx.sourceFile.text.split("\n") || []);


please avoid this array by using ctx.sourceFile.text.substr(line.pos, line.contentLength) in the loop below

ajafff · 2017-10-29T20:57:19Z

src/rules/maxLineLengthRule.ts

@@ -29,12 +28,41 @@ export class Rule extends Lint.Rules.AbstractRule {
            Limiting the length of a line of code improves code readability.
            It also makes comparing code side-by-side easier and improves compatibility with
            various editors, IDEs, and diff viewers.`,
-        optionsDescription: "An integer indicating the max length of lines.",
+        optionsDescription: Lint.Utils.dedent`
+        It can take up to 2 arguments, but only the first one is required:


the description needs an update to the new options format

ajafff · 2017-10-29T20:59:22Z

src/rules/maxLineLengthRule.ts

-        return this.applyWithFunction(sourceFile, walk, this.ruleArguments[0] as number);
+        const argument = this.ruleArguments[0];
+        const options = {
+            ignorePattern: (typeof argument === "object") ? new RegExp(argument["ignore-pattern"]) : undefined,


you also need to check if argument["ignore-pattern"] is present because new RegExp(undefined) matches everything

ajafff · 2017-10-29T21:00:03Z

src/rules/maxLineLengthRule.ts

+    const lines = (ctx.sourceFile && ctx.sourceFile.text.split("\n") || []);
+    const lineRanges = getLineRanges(ctx.sourceFile);
+    for (let i = 0; i < lines.length; i++) {
+        if (ignorePattern && ignorePattern.test(lines[i])) { continue; }


I guess the linter complains about ignorePattern &&
please amend to ignorePattern !== undefined &&

DanielKucal · 2017-10-31T18:47:20Z

Hey @ajafff, thanks for your comments, I've made the request changes. The Circle CI test seems to fail due to not found binary for npm-run-all. Let me know if something still needs improvement, thanks in advance.

ajafff

LGTM, I only found two style nits.

I just fixed the CI failures on master. This should be good to go after merging master into your branch.

ajafff · 2017-10-31T18:48:16Z

src/rules/maxLineLengthRule.ts

-        return super.isEnabled() && this.ruleArguments[0] as number > 0;
+        const argument: any = this.ruleArguments[0];
+        const numberGreaterThan0: boolean = (Number(argument) > 0);
+        const limitGreaterTHan0: boolean = (argument instanceof Object && argument.limit > 0);


s/THan/Than/

ajafff · 2017-10-31T18:50:31Z

src/rules/maxLineLengthRule.ts

@@ -45,19 +79,31 @@ export class Rule extends Lint.Rules.AbstractRule {
    }

    public isEnabled(): boolean {
-        return super.isEnabled() && this.ruleArguments[0] as number > 0;
+        const argument: any = this.ruleArguments[0];


you can omit the : any and : boolean type annotations.

Thought linter is complaining about infererred types

…pe in variable name

ajafff · 2017-10-31T19:17:40Z

@DanielKucal almost done. There are some lint errors that need to be fixed. Use yarn verify locally to make sure you fixed them.

/home/ubuntu/tslint/src/rules/maxLineLengthRule.ts
ERROR: 84:66  no-unsafe-any  Unsafe use of expression of type 'any'.
ERROR: 84:66  no-unsafe-any  Unsafe use of expression of type 'any'.
ERROR: 92:28  no-unsafe-any  Unsafe use of expression of type 'any'.
ERROR: 93:62  no-unsafe-any  Unsafe use of expression of type 'any'.
ERROR: 94:28  no-unsafe-any  Unsafe use of expression of type 'any'.
ERROR: 94:28  no-unsafe-any  Unsafe use of expression of type 'any'.

…on, resolve errors about unsafe types

ajafff

still LGTM

ajafff · 2017-10-31T23:20:46Z

Thanks @DanielKucal

sgiradoa-psl · 2017-11-30T15:32:29Z

Can someone tell me when or how I can use this? I have the latest version of tslint and is not there. 😞

ajafff · 2017-11-30T15:36:40Z

@sgiradoa-psl this change will be part of the next release

vinayakpatil · 2017-12-21T09:54:42Z

@ajafff Will this be available anytime soon? I don't see this in 5.8.0

MartinJohns · 2018-01-09T21:09:40Z

@ajafff It might be useful to add this issue to the milestone in which you plan to release this feature.

adidahiya · 2018-01-09T21:12:25Z

@MartinJohns it'll be in the next release, but we don't have strict release schedules right now. I know it's been a long time since the last release. I'm going through PRs now and will try to ship a minor release this week.

MartinJohns · 2018-01-09T21:13:51Z

The release schedule is not what I was pointing at, but just generally showing in what version the issue will ship. You could utilize the milestones feature and add the issues there, then it would quickly be visible what feature will ship with that version - just without a due date.

For example: https://github.com/palantir/tslint/milestone/43?closed=1 - and I can see all issues that will ship with version 5.9.

adidahiya · 2018-01-09T21:17:31Z

Sure, the milestone link would be nice to have, and we'll try to do it for some PRs / issues, but since it requires manual work it might not happen all the time. It should be easy enough for users to go to the project's root README and see the latest version on NPM and infer what the next release will be.

IMO the best thing is to click through to the commit where a PR was merged:

then check its tags to see where it was released:

ekilah · 2018-08-09T19:20:43Z

for people arriving here via google, two things:

the syntax is not as pictured in the original PR submission above. the correct syntax is:

{
  "rules": {
    "max-line-length": [true, {"limit": 120, "ignore-pattern": "your regex here"}],
  } 
}

(just to help others) Here's an example of a pattern I used to fix an issue where single-import import lines were fighting with prettier (see Prettier exceeds the maximum line length on single-import imports prettier/prettier#2564):

{
  "rules": {
    "max-line-length": [true, {"limit": 120, "ignore-pattern": "^import [^,]+ from"}],
  } 
}

DanielKucal · 2018-09-03T17:16:17Z

Thanks @ekilah, I've updated first post to avoid confusing.

DanielKucal added 2 commits August 5, 2017 01:27

feat(max-line-length): add configurable ignore pattern

fdbdac9

feat(tests/max-line-length): add test case for ignore pattern

6c7fa07

ajafff reviewed Aug 5, 2017

View reviewed changes

DanielKucal added 2 commits September 15, 2017 15:17

fix(max-line-length): update example and improve code according to re…

a1030dd

…quirements

Merge branch 'master' into max-line-length_ignore-pattern

05a5348

# Conflicts: # src/rules/maxLineLengthRule.ts # yarn.lock

ajafff mentioned this pull request Oct 9, 2017

[enhancement] max-line-length / config option to ignore lines w URLs #3303

Closed

4 tasks

DanielKucal added 2 commits October 29, 2017 19:27

Merge branch 'master' into max-line-length_ignore-pattern

d3c2f7b

feat(max-line-length): accept arguments as an object

c9fed89

ajafff suggested changes Oct 29, 2017

View reviewed changes

DanielKucal added 3 commits October 29, 2017 23:40

fix(max-line-length): fill types

43f7501

docs(max-line-length): update instructions

dfec9d7

fix(max-line-length): improve code according to requirements

d5adaea

ajafff approved these changes Oct 31, 2017

View reviewed changes

DanielKucal added 2 commits October 31, 2017 20:01

fix(max-line-length): remove unnecessary type assignments, correct ty…

aebf145

…pe in variable name

Merge branch 'master' into max-line-length_ignore-pattern

de66bac

refactor(max-line-length): create method for getting rule configurati…

f6f602e

…on, resolve errors about unsafe types

ajafff approved these changes Oct 31, 2017

View reviewed changes

ajafff merged commit d1caf11 into palantir:master Oct 31, 2017

ajafff mentioned this pull request Oct 31, 2017

max-line-length optionally disabled for import statements #1123

Closed

DanielKucal mentioned this pull request Nov 9, 2017

max-line-length: ignore long file paths #3403

Closed

PavelPikat mentioned this pull request Nov 11, 2017

Long-length import statements could wrap prettier/prettier#576

Closed

arthurescriou mentioned this pull request Dec 29, 2017

add prettier and tslint compatibility jhipster/generator-jhipster#6906

Merged

4 tasks

adidahiya added this to the TSLint v5.9 milestone Jan 9, 2018

HyphnKnight pushed a commit to HyphnKnight/tslint that referenced this pull request Apr 9, 2018

Feat: ignore pattern for max-line-length (palantir#3099)

0645ff4

Feat: ignore pattern for max-line-length #3099

Feat: ignore pattern for max-line-length #3099

Conversation

DanielKucal commented Aug 4, 2017 • edited Loading

PR checklist

Overview of change:

CHANGELOG.md entry:

ajafff commented Aug 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanielKucal Aug 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanielKucal commented Aug 11, 2017

osu5nc commented Sep 5, 2017

navels commented Sep 25, 2017

DanielKucal commented Sep 25, 2017

sgiradoa-psl commented Oct 24, 2017 • edited Loading

DanielKucal commented Oct 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanielKucal commented Oct 31, 2017 • edited Loading

ajafff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanielKucal Oct 31, 2017 • edited Loading

Choose a reason for hiding this comment

ajafff commented Oct 31, 2017

ajafff left a comment

Choose a reason for hiding this comment

ajafff commented Oct 31, 2017

sgiradoa-psl commented Nov 30, 2017

ajafff commented Nov 30, 2017

vinayakpatil commented Dec 21, 2017

MartinJohns commented Jan 9, 2018

adidahiya commented Jan 9, 2018

MartinJohns commented Jan 9, 2018 • edited Loading

adidahiya commented Jan 9, 2018

ekilah commented Aug 9, 2018 • edited Loading

DanielKucal commented Sep 3, 2018

DanielKucal commented Aug 4, 2017 •

edited

Loading

DanielKucal Aug 6, 2017 •

edited

Loading

sgiradoa-psl commented Oct 24, 2017 •

edited

Loading

DanielKucal commented Oct 31, 2017 •

edited

Loading

DanielKucal Oct 31, 2017 •

edited

Loading

MartinJohns commented Jan 9, 2018 •

edited

Loading

ekilah commented Aug 9, 2018 •

edited

Loading