//eslint-disable-line's behavior is changed in 4.7.0

[mysticatea]

Tell us about your environment

• ESLint Version: 4.7.0
• Node Version: 8.4.0
• npm Version: 5.4.2

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

• default

Please show your full configuration:

• nothing

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

/* eslint-disable no-unused-vars */

var foo; // eslint-disable-line no-unused-vars
var bar;

/* eslint-enable no-unused-vars */

$ eslint test.js --rule no-unused-vars:error

What did you expect to happen?

No errors as same as 4.6.0.

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

~\dev\sandbox> eslint test.js --rule no-unused-vars:error

C:\Users\t-nagashima.MSS\dev\sandbox\test.js
  6:5  error  'bar' is defined but never used  no-unused-vars

✖ 1 problem (1 error, 0 warnings)

[emmettnicholas]

In case it helps, here's a different repro for seemingly the same underlying issue:

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

let a; // eslint-disable-line no-unused-vars
let b; // eslint-disable-line no-unused-vars
let c; // eslint-disable-line no-unused-vars
let d; // eslint-disable-line no-unused-vars
let e; // eslint-disable-line no-unused-vars
let f; // eslint-disable-line no-unused-vars

What did you expect to happen?

no errors

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

2:5 warning 'b' is defined but never used no-unused-vars

[not-an-aardvark]

/* eslint-disable no-unused-vars */

var foo; // eslint-disable-line no-unused-vars
var bar;

/* eslint-enable no-unused-vars */

I'm not sure about this -- the // eslint-disable-next-line disables the rule for that one line and enables it afterwards, so it makes sense to me that the rule would be enabled after it. The problem could be solved by removing the // eslint-disable-next-line comment, since it's redundant otherwise.

let a; // eslint-disable-line no-unused-vars
let b; // eslint-disable-line no-unused-vars
let c; // eslint-disable-line no-unused-vars
let d; // eslint-disable-line no-unused-vars
let f; // eslint-disable-line no-unused-vars
let g; // eslint-disable-line no-unused-vars

This looks like a bug to me.

[sqal]

@not-an-aardvark what about this case It worked fine in previous version but now i cannot disable no-use-before-define rule as you can see. However if you remove // eslint-disable-line in line 5 it works again.

[not-an-aardvark]

Hmm, that's a good point. I suppose we should treat eslint-disable-line comments differnetly then. They're currently processed like this:

// eslint-disable-next-line
foo;

bar;


// is processed the same as:


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

But it seems like that isn't accurate, because applying a global /* eslint-enable */ after the comment seems to be unintuitive since it also gets rid of any previous /* eslint-disable */ comments.

[mysticatea]

#9216 seemed to include a breaking change. eslint-disable/enable pair can be nested, but it removed the feature. I think that we should revert the change, until the next major release at least.

[gyandeeps]

#9216 seemed to include a breaking change. eslint-disable/enable pair can be nested, but it removed the feature. I think that we should revert the change, until the next major release at least.

Wow, did not know about this feature... lol
can u share an example..

[not-an-aardvark]

#9216 (comment)

[mysticatea]

@gyandeeps see the first post of this issue :D

As @not-an-aardvark mentioned, eslint-disable-line is converted to eslint-disable/enable pair, so the example is nested.

[not-an-aardvark]

Are people actually using the nesting aside from eslint-disable-line comments? I had thought that the nesting cases in #9216 (comment) were bugs, so an alternative fix would be to update eslint-disable-line processing to avoid using nesting.

edit: The nesting behavior also had bugs like #9215.

[gyandeeps]

i agree with @not-an-aardvark .. those test (ref #9216 (comment)) are actually incorrect. Examples explained in that comment makes sense to me.

[mysticatea]

I'm not using actually because eslint-plugin-eslint-comments catches it. I have noticed this because I needed to change the plugin's logic and tests to follow the change.

[gyandeeps]

just to be clear, the code mentioned in this issue above and also the 2nd comment code example are bugs.... but #9216 (comment) this is slightly different.

[mysticatea]

In my perspective, those look same similar. One more example:

/* eslint-disable no-unused-vars */

function aaa() {
    /* eslint-disable */
    var foo;
    /* eslint-enable */
}

var bar;

/* eslint-enable no-unused-vars */

[not-an-aardvark]

I think it makes the most sense for nesting to not be supported, and for eslint-disable and eslint-disable-line comments to be considered separately without relying on nesting. That way, the examples in #9318 (comment) and #9318 (comment) will be considered correct), and the examples in #9216 (comment) and #9215 will be considered incorrect.

Alternatively, we could update the implementation to support nesting again. (I don't think reverting would work well since we've made some additional changes to the code since the original refactor landed). However, this could end up being confusing, because the behavior in #9215 definitely seems incorrect to me.

[mysticatea]

I agree that #9215 is a bug.

Hmm, I suspect that it's a breaking change because the behavior exists in a long time. On the other hand, the behavior has been never documented, so semver doesn't consider that it's a breaking change.

OK, I don't oppose the way no nesting.

[not-an-aardvark]

Just as a heads-up: I probably won't have time to fix this until Sunday night, so if someone else is able to fix it earlier, that might allow for more time to review before we do a patch release on Monday.

[shellscape]

As @mysticatea commented, our block-disabling comments stopped working in 4.7.0 as well.

/* eslint-disable quotes */
console.log("foo");
/* eslint-enable quotes */

That now throws an error on console.log as it violates the config quotes rule, even though it's disabled for that block.