Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix false positives for selector lists and shared-line comments in comment-empty-line-before #2835

Closed
ntwb opened this issue Aug 27, 2017 · 4 comments · Fixed by #4360
Closed
Labels
status: wip is being worked on by someone type: bug a problem with a feature or rule

Comments

@ntwb
Copy link
Member

ntwb commented Aug 27, 2017

Describe the issue. Is it a bug or a feature request (new rule, new option, etc.)?

A bug where multiple shared-line comments using the comment-empty-line-before rule returns false results.

via https://github.com/stylelint/stylelint/tree/master/lib/rules/comment-empty-line-before

"If the comment is the very first node in a stylesheet then it is ignored. Shared-line comments are also ignored."

Which rule, if any, is this issue related to?

comment-empty-line-before

What CSS is needed to reproduce this issue?

/* comment */
.ok { /* comment */
  color: pink;
}

/* comment */
.foo, /* comment */
.bar { /* comment */
  color: pink;
}

What stylelint configuration is needed to reproduce this issue?

{
  "rules": {
    'comment-empty-line-before': [ 'always', {
      ignore: ['stylelint-commands'],
    } ],
  }
}

Which version of stylelint are you using?

8.0.0

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

n/a

Does your issue relate to non-standard syntax (e.g. SCSS, nesting, etc.)?

No

What did you expect to happen?

No warnings to be flagged.

What actually happened (e.g. what warnings or errors you are getting)?

The following warnings were flagged:

> test.css            
 8:8  ✖  Expected empty line before comment   comment-empty-line-before
@ntwb ntwb added the type: bug a problem with a feature or rule label Aug 27, 2017
@ntwb
Copy link
Member Author

ntwb commented Sep 1, 2017

Here's a couple of failing tests that should pass:

diff --git lib/rules/comment-empty-line-before/__tests__/index.js lib/rules/comment-empty-line-before/__tests__/index.js
index be684390..3b867c25 100644
--- lib/rules/comment-empty-line-before/__tests__/index.js
+++ lib/rules/comment-empty-line-before/__tests__/index.js
@@ -76,6 +76,10 @@ testRule(
       {
         code: "a {\n\n  /* comment */\n  color: pink;\n}",
         description: "first-nested with empty line before"
+      },
+      {
+        code: "a {\n /* comment */\n  color: pink;\n}",
+        description: "shared-line comment should be ignored"
       }
     ],
 
@@ -147,6 +151,10 @@ testRule(
         code:
           "a {\ncolor: pink;\n/* stylelint-disable something */\ntop: 0;\n}",
         description: "no newline before a stylelint command comment"
+      },
+      {
+        code: "a {\n /* comment */\n  color: pink;\n}",
+        description: "shared-line comment should be ignored"
       }
     ]
   })

@jeddy3 jeddy3 added status: needs discussion triage needs further discussion and removed type: bug a problem with a feature or rule labels Sep 2, 2017
@jeddy3 jeddy3 changed the title Multiple shared-line comments comment-empty-line-before issue False positives for multiple shared-line comments in comment-empty-line-before Sep 2, 2017
@jeddy3
Copy link
Member

jeddy3 commented Sep 2, 2017

Will this be fixed by #2827?

Neither of the additional tests above contain shared-line comments. So, I'm not sure those tests should pass.

@jeddy3
Copy link
Member

jeddy3 commented Sep 9, 2017

I can replicate this with 8.1.1. Very strange bug.

The following:

any selector,
.bar { /* comment */
  color: pink;
}

Will produce a violation:

2:8 error Expected empty line before comment (comment-empty-line-before)

Whereas the following two produce no violations:

.bar { /* comment */
  color: pink;
}
any rule { }
.bar { /* comment */
  color: pink;
}

@jeddy3 jeddy3 changed the title False positives for multiple shared-line comments in comment-empty-line-before Fix false positives for selector lists and shared-line comments in comment-empty-line-before Sep 9, 2017
@jeddy3 jeddy3 added status: ready to implement is ready to be worked on by someone type: bug a problem with a feature or rule and removed status: needs discussion triage needs further discussion labels Sep 9, 2017
@hudochenkov hudochenkov added status: wip is being worked on by someone and removed status: ready to implement is ready to be worked on by someone labels Oct 16, 2019
@hudochenkov
Copy link
Member

Demo with an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: wip is being worked on by someone type: bug a problem with a feature or rule
Development

Successfully merging a pull request may close this issue.

3 participants