eslint: Support block-comment version of eslint-disable-line

Tell us about your environment

  • ESLint Version: master
  • Node Version: any
  • npm Version: any

What parser (default, Babel-ESLint, etc.) are you using? default

Please show your full configuration:

{ "rules": { "no-alert": [1] } }

What did you do? Please include the actual source code causing the issue.

/* eslint-disable-line */ alert(1);

What did you expect to happen?

It should work like in JSCS:

/* jscs:ignore */ alert(1)

What actually happened? Please include the actual, raw output from ESLint.

Error reported.

About this issue

  • Original URL
  • State: closed
  • Created 7 years ago
  • Reactions: 14
  • Comments: 47 (31 by maintainers)

Commits related to this issue

Most upvoted comments

  1. How should it do on block comments which have line break? I think it should ignore those or throw an error to avoid confusing.
    /* eslint-disable-line
     *
     */ var foo
    
    /* eslint-disable-next-line
     *
     */
    var foo
    

I think it would be best to report a problem at that location, and ignore the comment. However, it might be a breaking change to start reporting a problem for those cases, since we previously always ignored them.

We could ignore them for now, then start reporting a problem for them in the next major release.

  1. How should it handle block comments which are in middle of a line? I think the following code is confusing, so I’d like to ignore those or throw an error. However, it has to allow those to address the cases that the feature wants to support such as JSX case. It’s my concern.
    var /* eslint-disable-line */ foo
    

I think it’s fine to just ignore disable the entire line in that case. We already do that with // eslint-disable-line at the end of a line, so in my opinion it’s not too confusing to to also ignore the entire line if there is a block comment in the middle.

  1. How should it handle multiple block comments which are on a line? I think those are merged, but I’m OK if it ignores those or throw an error.
    /* eslint-disable-line no-shadow */ /* eslint-disable-line no-unused-vars */ var foo
    

I think this should disable both rules, by just processing the comments separately.

We already do the same thing in a similar case:

// eslint-disable-next-line no-shadow
var foo // eslint-disable-line no-unused-vars
  1. How should it handle a mix of eslint-enable and eslint-disable-line In the following case, I’m not sure how it should behave for errors at the foo. I guess it should be disabled.
    /* eslint-disable-line */ var /* eslint-enable */ foo
    
    If it’s /* eslint-disable */ var /* eslint-enable */ foo, the errors at the foo are reported because the foo is after /* eslint-enable */. Above sample is similar to this. If it’s var /* eslint-enable */ foo //eslint-disable-line, the errors at the foo are not reported because the foo is on the line which has //eslint-disable-line. I think that //eslint-disable-line and /* eslint-disable/enable */ are different type of comments to help avoiding confusion.

We already handle similar cases:

// eslint-disable-next-line
foo; /* eslint-enable */ bar
foo; /* eslint-enable */ bar // eslint-disable-line

With the current behavior, reporting is disabled for foo, but enabled for bar.


Right now, we preprocess all disable-line and disable-next-line directives into regular disable and enable directives:

// eslint-disable-next-line
foo;

// is processed the same way as ↓

/* eslint-disable */foo;
/* eslint-enable */

So I think we could just do the same thing here regardless of whether the disable-line comment is a block.

Would it make sense to also make eslint-disable-next-line also available in block comments to fix this issue?

I’d be happy to take this on if no one else wants it!

I don’t know what actual problem you’re trying to solve, but this feels like a clunky way to do it. I’m hoping we can come up with something better.

@platinumazure Mind explaining what it is that you don’t like about this solution? It’s clear that you’re not satisfied with this solution, but I don’t know what about it you’re against.

My 2c: The most compelling reason for me to support this is the JSX use case (which only allows block comments), and I would also argue that it would be even more confusing to only allow those style comments in JSX but not allow them elsewhere. So it seems to me like supporting this style comment everywhere makes the most sense.

The multiline comment is an interesting case. Seems like it should treat lines that contain the beginning or end line of the comment as “the line” in question.

I agree that a line comment would probably be clearer most of the time, but this feels like a case where we can decide on behavior, document it, and trust our users rather than limit them. And for what it’s worth, this seems like a pretty reasonable style for a eslint-disable-next-line comment:

/*
  eslint-disable-next-line
 */
var foo;

@jcrben FYI you can use /* eslint-disable */ and /* eslint-enable */ to do the same thing with ESLint, and you can even pass a rule or list of rules to enable/disable.

@not-an-aardvark’s comment here I believe covers everything we discussed during the meeting.

To simplify, rules for eslint-disable-line and eslint-disable-next-line (both line and block) comments should be:

  • They must be single line comments
  • They apply to the entire line, regardless of position

@erindepew Please let us know if anything is unclear!

We should make a decision about this. I’m still 👍 for the JSX use case. It looks like we have some team members for it and some against. Is it maybe time to bring this to the TSC? @eslint/eslint-tsc

I think that we have to clarify the specification of this feature before the decision. I think this proposal still has much ambiguity.

  1. How should it do on block comments which have line break? I think it should ignore those or throw an error to avoid confusing.
    /* eslint-disable-line
     *
     */ var foo
    
    /* eslint-disable-next-line
     *
     */
    var foo
    
  2. How should it handle block comments which are in middle of a line? I think the following code is confusing, so I’d like to ignore those or throw an error. However, it has to allow those to address the cases that the feature wants to support such as JSX case. It’s my concern.
    var /* eslint-disable-line */ foo
    
  3. How should it handle multiple block comments which are on a line? I think those are merged, but I’m OK if it ignores those or throw an error.
    /* eslint-disable-line no-shadow */ /* eslint-disable-line no-unused-vars */ var foo
    
  4. How should it handle a mix of eslint-enable and eslint-disable-line In the following case, I’m not sure how it should behave for errors at the foo. I guess it should be disabled.
    /* eslint-disable-line */ var /* eslint-enable */ foo
    
    If it’s /* eslint-disable */ var /* eslint-enable */ foo, the errors at the foo are reported because the foo is after /* eslint-enable */. Above sample is similar to this. If it’s var /* eslint-enable */ foo //eslint-disable-line, the errors at the foo are not reported because the foo is on the line which has //eslint-disable-line. I think that //eslint-disable-line and /* eslint-disable/enable */ are different type of comments to help avoiding confusion.
  5. How should it handle eslint and eslint-env comments which are in line comments?
    // eslint no-undef: error
    //        no-unused-var: error
    
    Currently, we support those only on block comments. If we need to handle line comments and block comments consistently, we should consider this. I think that we should keep about this behavior as is.

In line comment case, we can write only one comment on a line, so it fits as the directive to control the state of a line. We can write block comments with many variations. It can bring ambiguity and complexity to users and developers.

We can disable reports with line comments in most cases already. We can disable reports with a /* eslint-disable/enable */ comment pair in remaining cases. So I’m not sure that this feature has more value to introduce the ambiguity and complexity.

Just found some relative:

  • #6335 - resolved as PR with additional “clearer” documentation
  • #7890 - one more request for single line comments
  • #6416 - abandoned and closed
  • #6903, #5914 - jshint-like behaviour of line comments?
  • #7030 - closed by starter, but has comment about reopening with 11 likes

And that is music for me:

because JSHint treats line and block comments differently. A line comment applies to its containing block while a block comment does not. As we continue to have people switching from JSHint to ESLint, we don’t want to provide something that is looks like it might behave the same way.

Same was for JSCS. Was.

I doubt anyone disagrees with the note about development time. However, just because we can develop it, doesn’t mean we should (i.e., maybe there isn’t a “good, safe” block-comment version because the costs might not be worth the benefits).

I’m not about writing code with business logic. Just about thinking how it should work.

After this little research I’m even more sure we should support both versions for ALL directives. People keeps creating issues because they think it’s more native. And there are no real reasons to not change anything — We should move forward.

Thank you for contribution and I don’t know what I can do here but please mention me if I needed. p.s. Please, feel free to close it if you decide that it shouldn’t be done.

@platinumazure Well

Not sure I follow. I wasn’t aware block comments were parsed as such in template literals while line comments are part of the string.

Yes, you can’t put it into the template literal, but you dont need it, because you can place block-comment BEFORE it.

/* any directive */ const mytmpl = `...
...`;

And you can’t do this with line comment:

// any broken syntax const mytmpl = `...
...`;

I’d like to just throw out there that I think that both line and block comments should be allowed for all configuration comments, in regards to @platinumazure’s comment. I remember being confused why I had to have specific types for certain configuration options, and in the case of JSX, requiring line comments are very limiting and make for messy code, which is why we have this issue. That’s my vote.