Tslint: rule(max-line-length) cannot be disabled within a block comment

[westy92]

Bug Report

• TSLint version: 3.14.0
• TypeScript version: 2.0.0
• Running TSLint via: gulp (via gulp-tslint)
  

  TypeScript code being linted

/*
  // tslint:disable-next-line:max-line-length
  const lengthy = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit. Phasellus et mauris turpis. Phasellus tincidunt...';
*/

with tslint.json configuration:

{
  "extends": "tslint:recommended"
}

Actual behavior

(max-line-length) path/to/file.ts[123, 1]: Exceeds maximum line length of 120

Expected behavior

Rule is disabled and there is no error.

[jkillian]

Is it acceptable to disable for the whole comment?

// tslint:disable:max-line-length
/*
 * My comment with crazy long lines...
 */
// tslint:enable:max-line-length

[westy92]

Yes, that would be a workaround.

Upon further investigation, it looks like other rules may not be checked when inside a comment block.

i.e. a rule only allows single quotes:

const derp = "hi"; // error
/*
const derp2 = "hi"; // no error
*/

Maybe the issue isn't the fact that we cannot disable a rule inside a comment block--maybe it's that it is being evaluated at all.

[jkillian]

Your right, but that's intended behavior. It doesn't make sense to enforce a quotemark rule inside a comment, because as someone famous once said, "It's necessary to have both kinds of quotes in my comments!" 😉

[westy92]

If that rule is being ignored in comments, shouldn't max-line-length be ignored as well?
I don't know if other rules are ignored or not.

[jmlopez-rod]

Is there any plans to add other options to this rule? eslint can ignore this rule in a comment or make the length different if we choose to: http://eslint.org/docs/rules/max-len.

Would a PR be welcomed to incorporate these options?

[jkillian]

Well, the line length rule is about making code editor / IDE friendly, so I'd argue that an exception for comments doesn't really make sense. @jmlopez-rod, why would you want to limit code to a certain width but not limit comments in the same way?

I'm not categorically opposed though, just want to understand the reasoning first

[jmlopez-rod]

@JKillian I wouldn't personally like to have different limits so I don't really have a saying in this. I just wanted to point out of the options that eslint has. Some of those options are useful, some I would argue are not but the conventions should be left to the people to choose.

I have personally ran into several issues in which I have to disable this rule:

• URLS:
  
  
  
  • in comments
  
  
  • in strings
  
  

For this the ignoreUrls options would be useful.

• imports

This happens in particular with relative module imports which have default exports. For instance:

import SomeLongObjectNameBecauseWhyNot from '../../someDir/somewhere/because/things/happen/and/we/cant/help/it`;

Yeah, that example is ridiculous but I have had to turn off this rule at the beginning of the import block and turn it on again. I think it would be good to have an option to simply disable it in these cases.

Anyway, I was just wondering if more work will be done to this rule. I'd be happy to try to add some of these options.

[jkillian]

👍 Sounds good to me - an ignoreUrls, ignoreImports, and ignoreComments rule seems reasonable

[jmlopez-rod]

@JKillian , I have implemented the ignore url and comments options, the ignoreImports is almost done but I'm not sure how to handle one special case:

import {
    obj1, obj2, obj3, obj4, just, trying, to, be, a, rebel, here
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Exceeds maximum line length of 50]
} from 'my-favorite-module/with/lots/of/deep/nested/modules';

I think in this case it should warn since that list can be split. Currently I'm doing it on visitImportDeclaration, but perhaps I'll want to examine its children. Anyway, I'll keep playing around with it, the actual question I wanted to ask is: What is the correct way to write the metaData? Currently it stands like this:

    public static metadata: Lint.IRuleMetadata = {
        ruleName: "max-line-length",
        description: "Requires lines to be under a certain max length.",
        rationale: Lint.Utils.dedent`
            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.",
        options: {
            type: "number",
            minimum: "1",
        },
        optionExamples: ["[true, 120]"],
        type: "maintainability",
    };

I'm not sure how to write the options so that I can write

   optionExamples: [`[true, 50, "ignoreImports", "ignoreComments", "ignoreUrls"]`],

I'll try to make the PR as soon as I resolve these issues.

[jkillian]

Ah sorry @jmlopez-rod, just saw your previous comment now! I could try to explain, but I think it'll be easier for both of us if you just duplicate the way other rules do it. I linked to a rule in the review of your PR which is a fairly similar setup to this rule metadata-wise

[jmlopez-rod]

@JKillian , I have closed the PR since the code already has conflicts. Either way, you pointed me in the right direction. I'm leaving a link to the rule I made in the tslint-eslint-rules project for people interested in the rule. Please feel to copy part of the source code to update the tslint rule or point people the following link:

https://github.com/buzinas/tslint-eslint-rules/blob/master/src/docs/rules/terMaxLenRule.md

[beenotung]

Also will be nice if there is an option to ignore function signature. For some functions using the generic type, it can reach the limit quite often.

Here is an example:

  async toReviewTutorPostDetail(reviewPost: ReviewTutorPost<PostPair>): Promise<APIResponse<PostAPI.GetPostResult | ProfileAPI.GetPublicProfileResult, ReviewTutorPostDetail<PostPair>>> {
    ...
  }

[jmlopez-rod]

For this type I would either try to create a new type or break it like this:

async toReviewTutorPostDetail(
  reviewPost: ReviewTutorPost<PostPair>,
): Promise<
  APIResponse<
    PostAPI.GetPostResult | ProfileAPI.GetPublicProfileResult,
    ReviewTutorPostDetail<PostPair>
  >
> {
    ...
}

or just disable the next line as long as your code is more readable.

[JoshuaKGoldberg]

☠️ TSLint's time has come! ☠️

TSLint is no longer accepting most feature requests per #4534. See typescript-eslint.io for the new, shiny way to lint your TypeScript code with ESLint. ✨

It was a pleasure open sourcing with you all!

[JoshuaKGoldberg]

🤖 Beep boop! 👉 TSLint is deprecated 👈 _(#4534)_ and you should switch to typescript-eslint! 🤖

🔒 This issue is being locked to prevent further unnecessary discussions. Thank you! 👋