Add except: ["inside-block"] to rule-empty-line-before

[octoxan]

This is a bug where the rule "rule-empty-line-before" doesn't work properly, telling me something needs a new line when it should not.

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

"rule-empty-line-before"

What CSS is needed to reproduce this issue?

e.g.

.foo-foo {
	font-size: .75em;
}

.foo {
	text-align: center;
	.bar {
		font-style: italic;
	}
}

What stylelint configuration is needed to reproduce this issue?

e.g.

{
  "rules": {
    "rule-empty-line-before": ["always", {
        except: ["inside-block-and-after-rule", "first-nested"]
      }]
  }
}

Which version of stylelint are you using?

8.1.1

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

With npm and inside of phpstorm.

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

Nesting, scss.

What did you expect to happen?

No warnings to be flagged. I want a warning on if there's no empty lines before non-nested rules, and if there's any empty lines on nested rules. .bar is nested, and should not throw an error.

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

.bar throws an error "Expected empty line before rule", but should fall under either first-nested or inside-block-and-after-rule

[jeddy3]

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

I believe the rule is behaving correctly here as .bar is inside a block, but it is after a declaration and not a rule.

The following is an unusual style that I've not seen before:

.foo {
	text-align: center;
	.bar {
		font-style: italic;
	}
}

Most requests are for:

.foo {
	text-align: center;

	.bar {
		font-style: italic;
	}
}

However, I see no harm in adding an except: ["inside-block"] secondary option to the rule to allow this code style. @octoxan Please consider contributing this option. There's a section in the Developer Guide on how to get started. There is also an existing ignore: ["inside-block"] option (code here) to work from, so adding this new option should be trival.

[octoxan]

@jeddy3
Oh yeah I'll totally do that tonight!

[gucong3000]

Meybe it can help you:

--- a/lib/rules/rule-empty-line-before/index.js
+++ b/lib/rules/rule-empty-line-before/index.js
@@ -34,6 +34,7 @@ const rule = function(expectation, options, context) {
           ignore: ["after-comment", "inside-block"],
           except: [
             "after-rule",
+            "inside-block",
             "after-single-line-comment",
             "first-nested",
             "inside-block-and-after-rule"
@@ -69,11 +70,10 @@ const rule = function(expectation, options, context) {
         return;
       }

+      const isNested = rule.parent.type !== "root";
+
       // Optionally ignore the expectation if inside a block
-      if (
-        optionsMatches(options, "ignore", "inside-block") &&
-        rule.parent !== root
-      ) {
+      if (optionsMatches(options, "ignore", "inside-block") && isNested) {
         return;
       }

@@ -114,10 +114,11 @@ const rule = function(expectation, options, context) {

       // Optionally reverse the expectation for single line comments
       if (
+        (optionsMatches(options, "except", "inside-block") && isNested) ||
         optionsMatches(options, "except", "after-single-line-comment") &&
-        rule.prev() &&
-        rule.prev().type === "comment" &&
-        isSingleLineString(rule.prev().toString())
+          rule.prev() &&
+          rule.prev().type === "comment" &&
+          isSingleLineString(rule.prev().toString())
       ) {
         expectEmptyLineBefore = !expectEmptyLineBefore;
       }

also resolves compatibility in HTML files rule.parent.type !== "root"

[jeddy3]

also resolves compatibility in HTML files rule.parent.type !== "root"

Interesting. SGTM.

[jeddy3]

also resolves compatibility in HTML files rule.parent.type !== "root"

@gucong3000 Let's pull that out into a separate issue and PR, and keep this one focused on just adding the new option. Feel free to create an issue about the HTML file incompatibility in the rule.

[gucong3000]

Let's pull that out into a separate issue and PR

#2891

[gucong3000]

@octoxan Are you still working on this?

[b13nxx]

@jeddy3 I believe this issue is no longer matter.

[jeddy3]

@GorwinJororis Thanks for spotting this!