Fix false positives for two double slashed comments in *-empty-line-before

[Eugeno]

Clearly describe the bug

There are expected empty line before declaration after 2 or more lines of comments (with //). Neither in case of single line nor in case of version 13.6.1.

Which rule, if any, is the bug related to?

declaration-empty-line-before

What code is needed to reproduce the bug?

a {
  // Stylelint: Expected empty line before declaration
  // (declaration-empty-line-before)
  margin: 0;
}

b {
  // this is ok
  margin: 0;
}

i {
  /*
   * this is ok
   * too
   */
  margin: 0;
}

What stylelint configuration is needed to reproduce the bug?

stylelint-config-standard

"declaration-empty-line-before": [ "always", {
      except: [
        "after-declaration",
        "first-nested",
      ],
      ignore: [
        "after-comment",
        "inside-single-line-block",
      ],
    } ],

Which version of stylelint are you using?

13.7.0

How are you running stylelint: CLI, PostCSS plugin, Node.js API?

CLI

Does the bug relate to non-standard syntax (e.g. SCSS, Less etc.)?

Yep, comments at SCSS.

What did you expect to happen?

No requirement for an empty line after comments (as said in configuration, ignore: ["after-comment"]).

What actually happened (e.g. what warnings or errors did you get)?

test.css
31:5  ×  Expected empty line before declaration   declaration-empty-line-before

[sebastianhaeni]

I'm having a related issue.

The following SCSS code snippet will skip the suppress comment as well and yield 2 lint errors.

.icon {
  // TODO use variable instead
  // stylelint-disable-next-line
  font-size: 21px;
}

Result:

my.component.scss
 26:3  ✖  Expected variable, function or keyword for "21px" of "font-size"   scale-unlimited/declaration-strict-value
 26:3  ✖  Expected empty line before declaration                             declaration-empty-line-before

And if I change // stylelint-disable-next-line to /* stylelint-disable-next-line */ it works, but that's not what we want to write in SCSS.

[sebastianhaeni]

Ah probably related to #4911

[jeddy3]

@Eugeno Thanks for the report and for using the template.

I can reproduce this using the demo:

a {
  // 1
  // 2
  margin: 0;
}

I've labelled the issue as ready to implement. Please consider contributing if you have time.

There are steps on how to fix a bug in a rule in the Developer guide.

---

@sebastianhaeni Can you create a separate issue, please? Your issue is likely related to this one, but I think it's slightly different and will need a separate discussion. Thanks.

[sebastianhaeni]

@jeddy3 My issue has just been fixed by #4913. Thus all good and I'm not creating a separate issue.

[jeddy3]

My issue has just been fixed by #4913.

@sebastianhaeni Can you try master locally (npm install stylelint/stylelint), please? I'm not sure your issue will be fixed by #4913.

I think your issue is related to the change in #4895, which in hindsight may have been a breaking one.

If master doesn't work locally, you can rewrite your comments using the new description syntax:

// stylelint-disable-next-line -- TODO use variable instead

or:

// stylelint-disable-next-line
// -- TODO use variable instead

[sebastianhaeni]

@jeddy3 you are right. It's not fixed. Thanks for explaining the way how to write these comments.

[bartaz]

In our case it affects // stylelint-enable rule comments:

// stylelint-disable color-no-hex
a { color: #FFF }
// stylelint-enable color-no-hex

// some other unrelated comment

causing "color-no-hex some other unrelated comment" has not been disabled
demo

Adding double dash in the end works as a workaround // stylelint-enable color-no-hex --.

We have it in several places in the codebase. Any idea if this is going to be patched soon, so we can wait for updated version that fixes this regression, or should we update our codebase?

[stof]

@bartaz there is #4913 which is already merged with a fix related to that.

[agarzola]

Please see Draft PR #5050. I’ve added test cases which I believe accurately represent the issue observed with at-rule-empty-line-before for the ['always', { ignore: ['after-comment'] }] configuration. However, when I run tests, they pass.

More details in the PR. Any help or guidance will be appreciated!

[agarzola]

As far as I can tell, this issue has been fixed in v13.8.0. After fixing my test cases to use SCSS syntax and finding that tests still pass, I confirmed that the latest version indeed fixes the issue by following these steps:

1. Created a test.scss file with the sample SCSS in this issue’s description.
2. Created a .stylelintrc file with the configuration in this issue’s description (adding the requisite rule object around it and whatnot).
3. I ran npx stylelint@13.7.0 test.scss and confirmed that I got the error described in this issue’s description.
4. I ran npx stylelint test.scss and got no error whatsoever.

I followed these steps using the sample code and configuration in duplicate tickets #4924 and #4938 for good measure, with identical results. I’m closing #5050 and recommend closing this issue as well.

Special thanks to @jeddy3 for the assist on my draft PR!

[Eugeno]

Yep, I can confirm, in 13.8.0 it's ok.

[erwinlagu]

Good job