Merge pull request #139 from csscomb/tg/133-empty-rulesets

Remove empty nested rulesets (#133)

Author: mishanga

README.md

@@ -522,7 +522,7 @@ p[href^='https://']:before { content: 'secure' }
 
 Acceptable values: `{Boolean}` `true`
 
-Example: `{ "remove-empty-rulesets": true }` - remove rulesets that have no declarations or comments.
+Example: `{ "remove-empty-rulesets": true }` - remove rulesets that have nothing but spaces.
 
 `a { color: red; } p { /* hey */ } b { }` → `a { color: red; } p { /* hey */ } `
 

lib/options/remove-empty-rulesets.js

@@ -32,8 +32,26 @@ module.exports = {
 
     _removeEmptyRulesets: function(nodeContent) {
         var i = nodeContent.length;
+        // Loop through node and try to find a ruleset:
         while (i--) {
-            if (this._isRuleset(nodeContent[i]) && this._isEmptyRuleset(nodeContent[i])) {
+            var node = nodeContent[i];
+            if (!this._isRuleset(node)) continue;
+            // If a ruleset is found, try to find its nested rulesets and remove
+            // all empty ones:
+            var j = node.length;
+            while (j--) {
+                // Nested rulesets are located inside blocks, that's why look
+                // for blocks only:
+                var blockNode = node[j];
+                if (blockNode[0] !== 'block') continue;
+                blockNode.shift();
+                this._processStylesheetContent(blockNode);
+                blockNode.unshift('block');
+                node[j] = blockNode;
+            }
+            // If after removing all empty nested rulesets the parent has also
+            // become empty, remove it too:
+            if (this._isEmptyRuleset(node)) {
                 nodeContent.splice(i, 1);
             }
         }
@@ -60,10 +78,10 @@ module.exports = {
     },
 
     /**
-     * Block considered empty when it has no declarations or comments.
+     * Block is considered empty when it has nothing but spaces.
      */
     _isEmptyBlock: function(node) {
-        return !node.some(this._isDeclarationOrComment);
+        return node.length === 1 || !node.some(this._isNotWhitespace);
     },
 
     _isDeclarationOrComment: function(node) {
@@ -82,6 +100,10 @@ module.exports = {
         return node[0] === 's';
     },
 
+    _isNotWhitespace: function(node) {
+        return typeof node === 'object' && node[0] !== 's';
+    },
+
     /**
      * Detects the value of an option at the tree node.
      * This option is treated as `true` by default, but any trailing space would invalidate it.

test/remove-empty-rulesets-scss.js

@@ -0,0 +1,35 @@
+var Comb = require('../lib/csscomb');
+var assert = require('assert');
+var fs = require('fs');
+
+describe('options/remove-empty-rulesets (scss)', function() {
+    var comb = new Comb({ 'remove-empty-rulesets': true });
+    var input;
+    var expected;
+
+    function readFile(path) {
+        return fs.readFileSync('test/remove-empty-rulesets-scss/' + path, 'utf8');
+    }
+
+    it('Should not remove rulesets which contain only includes', function() {
+        input = readFile('include.scss', 'utf-8');
+        assert.equal(comb.processString(input, 'scss'), input);
+    });
+
+    it('Should remove rulesets with contain only empty nested rules', function() {
+        input = readFile('empty-nested-rule.scss', 'utf-8');
+        expected = readFile('empty-nested-rule.expected.scss', 'utf-8');
+        assert.equal(comb.processString(input, 'scss'), expected);
+    });
+
+    it('Should not remove rulesets with non-empty nested rules. Test 1', function() {
+        input = readFile('nested-rule-1.scss', 'utf-8');
+        assert.equal(comb.processString(input, 'scss'), input);
+    });
+
+    it('Should not remove rulesets with non-empty nested rules. Test 2', function() {
+        input = readFile('nested-rule-2.scss', 'utf-8');
+        expected = readFile('nested-rule-2.expected.scss', 'utf-8');
+        assert.equal(comb.processString(input, 'scss'), expected);
+    });
+});

test/remove-empty-rulesets-scss/empty-nested-rule.expected.scss

@@ -0,0 +1 @@
+

test/remove-empty-rulesets-scss/empty-nested-rule.scss

@@ -0,0 +1,5 @@
+.parent {
+    .child {
+        .grandchild { }
+    }
+}

test/remove-empty-rulesets-scss/include.scss

@@ -0,0 +1,4 @@
+.parent {
+    @include mix-all;
+    @include mix-top;
+}

test/remove-empty-rulesets-scss/nested-rule-1.scss

@@ -0,0 +1,6 @@
+.parent {
+    .child {
+        @include mix-all;
+        @include mix-top;
+    }
+}

test/remove-empty-rulesets-scss/nested-rule-2.expected.scss

@@ -0,0 +1,7 @@
+.parent {
+    .child1 {
+        @include mix-all;
+        @include mix-top;
+    }
+    
+}

test/remove-empty-rulesets-scss/nested-rule-2.scss

@@ -0,0 +1,9 @@
+.parent {
+    .child1 {
+        @include mix-all;
+        @include mix-top;
+    }
+    .child2 {
+        .grandchild { }
+    }
+}

test/remove-empty-rulesets.js

@@ -27,6 +27,10 @@ describe('options/remove-empty-rulesets', function() {
             assert.equal(comb.processString(' b {} '), '  ');
         });
 
+        it('should remove ruleset with spaces', function() {
+            assert.equal(comb.processString(' b {   } '), '  ');
+        });
+
         it('should leave ruleset with declarations', function() {
             assert.equal(comb.processString('a { width: 10px; }\nb {} '), 'a { width: 10px; }\n ');
         });