Skip to content

Commit 273b32e

Browse files
bgriffithbgriffith
committedOct 7, 2015
Merge pull request #256 from DanPurdy/feature/mergeable-selectors
Feature/mergeable selectors
·
v1.13.11.3.0
2 parents bd5861f + 4d9bebb commit 273b32e

8 files changed

+901
-0
lines changed
 

‎docs/rules/no-mergeable-selectors.md

+68
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
# No Mergeable Selectors
2+
3+
Rule `no-mergeable-selectors` will enforce that selectors aren't repeated and that their properties are merged. You may also pass a whitelist of selectors you wish to exclude from merging.
4+
5+
## Options
6+
7+
* `whitelist`: `[array of selectors]` (defaults to empty array `[]`)
8+
9+
## Examples
10+
11+
When `enabled` with the default options, the following will generate a warning/error :
12+
13+
```scss
14+
.foo {
15+
content: 'bar';
16+
}
17+
18+
//duplicate selector
19+
.foo {
20+
color: red;
21+
}
22+
23+
h1,
24+
h2,
25+
h3 {
26+
content: '';
27+
}
28+
29+
// mergeable idents
30+
h1, h2, h3 {
31+
content: '';
32+
}
33+
34+
.test {
35+
.bar {
36+
color: blue;
37+
}
38+
}
39+
40+
// 2 mergeable selectors .test & .test .bar
41+
.test {
42+
.bar {
43+
color: red;
44+
}
45+
}
46+
```
47+
48+
When `whitelist: ['div p', 'div a']` the following will be allowed and no longer generate any mergeable warnings or errors:
49+
50+
```scss
51+
div p {
52+
color: red;
53+
}
54+
55+
// will not be warned as mergeable / duplicate
56+
div p {
57+
content: '';
58+
}
59+
60+
div a {
61+
color: blue;
62+
}
63+
64+
// will not be warned as mergeable / duplicate
65+
div a {
66+
content: '';
67+
}
68+
```

‎lib/config/sass-lint.yml

+1
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ rules:
2727
no-ids: 1
2828
no-important: 1
2929
no-invalid-hex: 1
30+
no-mergeable-selectors: 1
3031
no-misspelled-properties: 1
3132
no-qualifying-elements: 1
3233
no-trailing-zero: 1

‎lib/helpers.js

+36
Original file line numberDiff line numberDiff line change
@@ -168,4 +168,40 @@ helpers.stripPrefix = function (str) {
168168
return modProperty.slice(prefixLength + 1);
169169
};
170170

171+
/**
172+
* Removes the trailing space from a string
173+
* @param {string} curSelector - the current selector string
174+
* @returns {string} curSelector - the current selector minus any trailing space.
175+
*/
176+
177+
helpers.stripLastSpace = function (selector) {
178+
179+
if (selector.charAt(selector.length - 1) === ' ') {
180+
return selector.substr(0, selector.length - 1);
181+
182+
}
183+
184+
return selector;
185+
186+
};
187+
188+
/**
189+
* Scans through our selectors and keeps track of the order of selectors and delimiters
190+
* @param {object} selector - the current selector tree from our AST
191+
* @returns {array} mappedElements - an array / list representing the order of selectors and delimiters encountered
192+
*/
193+
194+
helpers.mapDelims = function (val) {
195+
196+
if (val.type === 'simpleSelector') {
197+
return 's';
198+
}
199+
200+
if (val.type === 'delimiter') {
201+
return 'd';
202+
}
203+
204+
return false;
205+
};
206+
171207
module.exports = helpers;

‎lib/rules/no-mergeable-selectors.js

+226
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
1+
'use strict';
2+
3+
var helpers = require('../helpers');
4+
5+
/**
6+
* Constructs a syntax complete selector for our selector matching and warning output
7+
* @param {object} val - the current node / part of our selector
8+
* @returns {object} - content: The current node with correct syntax e.g. class my-class = '.my-class'
9+
*
10+
*/
11+
12+
var constructSelector = function (val) {
13+
14+
var content = val.content;
15+
16+
if (val.is('id')) {
17+
content = '#' + val.content;
18+
}
19+
20+
else if (val.is('class')) {
21+
content = '.' + val.content;
22+
}
23+
24+
else if (val.is('ident')) {
25+
content = val.content;
26+
}
27+
28+
else if (val.is('attribute')) {
29+
var selector = '[';
30+
31+
val.forEach( function (attrib) {
32+
selector += constructSelector(attrib);
33+
});
34+
35+
content = selector + ']';
36+
}
37+
38+
else if (val.is('pseudoClass')) {
39+
content = ':' + val.content;
40+
}
41+
42+
else if (val.is('pseudoElement')) {
43+
content = '::' + val.content;
44+
}
45+
46+
else if (val.is('nth')) {
47+
content = '(' + val.content + ')';
48+
}
49+
50+
else if (val.is('nthSelector')) {
51+
var nthSelector = ':';
52+
53+
val.forEach( function (attrib) {
54+
nthSelector += constructSelector(attrib);
55+
});
56+
57+
content = nthSelector;
58+
}
59+
60+
else if (val.is('space')) {
61+
content = ' ';
62+
}
63+
64+
else if (val.is('parentSelector')) {
65+
content = val.content;
66+
}
67+
68+
else if (val.is('combinator')) {
69+
content = val.content;
70+
}
71+
72+
return content;
73+
74+
};
75+
76+
module.exports = {
77+
'name': 'no-mergeable-selectors',
78+
'defaults': {
79+
'whitelist': []
80+
},
81+
'detect': function (ast, parser) {
82+
var result = [],
83+
selectorList = [],
84+
parentSelector = [],
85+
86+
// we use this array to keep track of the number of nested blocks and their levels
87+
// seen as we will always start with a single block at level 0 we just the first
88+
// level count to 1
89+
childBlocks = [1],
90+
level = 0;
91+
92+
ast.traverseByType('ruleset', function (ruleset) {
93+
var selectorBlock = {
94+
selector: parentSelector.join(' '),
95+
line: ''
96+
},
97+
curSelector = '',
98+
delimOrder = [];
99+
100+
ruleset.forEach('selector', function (selector) {
101+
// Keep track of the order of elements & delimeters
102+
selector.forEach( function (el) {
103+
var curNode = helpers.mapDelims(el);
104+
105+
if (curNode !== false) {
106+
delimOrder.push(curNode);
107+
}
108+
});
109+
110+
selector.forEach('simpleSelector', function (simpleSelector) {
111+
// check if the next selector is proceeded by a delimiter
112+
// if it is add it to the curSelector output
113+
var nextType = delimOrder[0];
114+
115+
if (nextType === 'd') {
116+
curSelector += ', ';
117+
// remove the next delim and selector from our order as we are only looping over selectors
118+
delimOrder.splice(0, 2);
119+
120+
}
121+
else {
122+
// if no delim then just remove the current selectors marker in the order array
123+
delimOrder.splice(0, 1);
124+
}
125+
126+
simpleSelector.forEach(function (val) {
127+
// construct our selector from its content parts
128+
curSelector += constructSelector(val);
129+
});
130+
});
131+
132+
// Gonzales keeps the spaces after the selector, we remove them here to keep the selectors recognisable
133+
// and consisten in the result output.
134+
curSelector = helpers.stripLastSpace(curSelector);
135+
136+
// check to see if we are on a level other than default (0)
137+
if (level) {
138+
// remove 1 from the block count on the current level
139+
childBlocks[level] -= 1;
140+
}
141+
142+
// check to see if the current ruleset contains any child rulesets
143+
if (ruleset.first('block').contains('ruleset')) {
144+
ruleset.first('block').forEach('ruleset', function () {
145+
146+
// Keep a record of the number of rulesets on the next level of nesting
147+
if (childBlocks[level + 1]) {
148+
childBlocks[level + 1] += 1;
149+
}
150+
else {
151+
childBlocks[level + 1] = 1;
152+
}
153+
});
154+
155+
// as we have another level we add a space to the end of the current selector reference
156+
if (parentSelector[level]) {
157+
parentSelector[level] += curSelector + ' ';
158+
}
159+
else {
160+
parentSelector[level] = curSelector + ' ';
161+
}
162+
163+
// increase our level counter before we start parsing the child rulesets
164+
level = level + 1;
165+
}
166+
// if no child rulesets/blocks then we need to find the level we will next be working on
167+
else {
168+
169+
// we scan backwards through our block counter array until we find a number greater than 0
170+
// We've been decrementing our block counts on the correct level as we've parsed them so a
171+
// level with a positive count means there are still rulesets here to parse.
172+
for (var i = childBlocks.length - 1; i >= 0; i-- ) {
173+
if (childBlocks[i] === 0) {
174+
175+
// remove the last element from the parent selector array as our level decreases effectively allowing
176+
// us to correctly concat our parent and child selectors
177+
parentSelector.splice(parentSelector.length - 1, 1);
178+
179+
// if we're not on level 0 then we want to decrement the level as there are no child rules of this block left to parse.
180+
// We then remove the 0 count element from the end of the childblocks array to make it ready for use again.
181+
if (level) {
182+
level = level - 1;
183+
childBlocks.splice(childBlocks.length - 1, 1);
184+
}
185+
}
186+
else {
187+
188+
// if the current level has rulesets to parse we break out of the for loop
189+
break;
190+
}
191+
}
192+
}
193+
194+
// set the line of the current selector blocks start for our error messages
195+
// keep a reference with this for our current selector
196+
selectorBlock.line = ruleset.start.line;
197+
selectorBlock.selector += curSelector;
198+
199+
});
200+
201+
// we check to see if our selector has already been encountered, if it has we generate a lint warning/error
202+
// detailing which selector is failing and on which line.
203+
var present = helpers.propertySearch(selectorList, selectorBlock.selector, 'selector');
204+
205+
if (present !== -1) {
206+
result = helpers.addUnique(result, {
207+
'ruleId': parser.rule.name,
208+
'line': ruleset.start.line,
209+
'column': ruleset.start.column,
210+
'message': 'Rule `' + curSelector + '` should be merged with the rule on line ' + selectorList[present].line,
211+
'severity': parser.severity
212+
});
213+
}
214+
else {
215+
216+
// if the current selector is whitelisted we don't add it to the selector list to be checked against.
217+
if (parser.options.whitelist.indexOf(selectorBlock.selector) === -1) {
218+
219+
// push the selector to our master list/array of selectors currently parsed without error.
220+
selectorList.push(selectorBlock);
221+
}
222+
}
223+
});
224+
return result;
225+
}
226+
};

‎tests/helpers.js

+103
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,44 @@ var classBlock =
5454
indexHasChanged: [ 0 ]
5555
};
5656

57+
var nodeSimpleSelector = {
58+
type: 'simpleSelector',
59+
content:
60+
[
61+
{
62+
type: 'ident',
63+
content: 'h1',
64+
syntax: 'scss',
65+
start: { line: 16, column: 1 },
66+
end: { line: 16, column: 2 },
67+
indexHasChanged: [ 0 ]
68+
}
69+
],
70+
syntax: 'scss',
71+
start: { line: 16, column: 1 },
72+
end: { line: 16, column: 2 },
73+
indexHasChanged: [ 0 ]
74+
},
75+
76+
nodeDelim = {
77+
type: 'delimiter',
78+
content: ',',
79+
syntax: 'scss',
80+
start: { line: 16, column: 3 },
81+
end: { line: 16, column: 3 },
82+
indexHasChanged: [ 0 ]
83+
},
84+
85+
nodeSpace = {
86+
type: 'space',
87+
content: ' ',
88+
syntax: 'scss',
89+
start: { line: 225, column: 5 },
90+
end: { line: 225, column: 5 },
91+
indexHasChanged: [ 0 ]
92+
};
93+
94+
5795
var detectTestA = {
5896
line: 1,
5997
column: 1
@@ -637,4 +675,69 @@ describe('helpers', function () {
637675
assert.equal('border-color', result);
638676
done();
639677
});
678+
679+
//////////////////////////////
680+
// stripLastSpace
681+
//////////////////////////////
682+
683+
it('stripLastSpace - [\'selector \']', function (done) {
684+
685+
var result = helpers.stripLastSpace('selector ');
686+
687+
assert.equal('selector', result);
688+
done();
689+
});
690+
691+
it('stripLastSpace - [\'selector test \']', function (done) {
692+
693+
var result = helpers.stripLastSpace('selector test');
694+
695+
assert.equal('selector test', result);
696+
done();
697+
});
698+
699+
it('stripLastSpace - [\'selector\']', function (done) {
700+
701+
var result = helpers.stripLastSpace('selector');
702+
703+
assert.equal('selector', result);
704+
done();
705+
});
706+
707+
it('stripLastSpace - [\'selector test\']', function (done) {
708+
709+
var result = helpers.stripLastSpace('selector test');
710+
711+
assert.equal('selector test', result);
712+
done();
713+
});
714+
715+
//////////////////////////////
716+
// mapDelims
717+
//////////////////////////////
718+
719+
it('mapDelims - selector', function (done) {
720+
721+
var result = helpers.mapDelims(nodeSimpleSelector);
722+
723+
assert.equal('s', result);
724+
done();
725+
});
726+
727+
it('mapDelims - delim', function (done) {
728+
729+
var result = helpers.mapDelims(nodeDelim);
730+
731+
assert.equal('d', result);
732+
done();
733+
});
734+
735+
it('mapDelims - space', function (done) {
736+
737+
var result = helpers.mapDelims(nodeSpace);
738+
739+
assert.equal(false, result);
740+
done();
741+
});
742+
640743
});

‎tests/rules/no-mergeable-selectors.js

+63
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
'use strict';
2+
3+
var lint = require('./_lint');
4+
5+
describe('no mergeable selectors - scss', function () {
6+
var file = lint.file('no-mergeable-selectors.scss');
7+
8+
it('[default]', function (done) {
9+
lint.test(file, {
10+
'no-mergeable-selectors': 1
11+
}, function (data) {
12+
lint.assert.equal(19, data.warningCount);
13+
done();
14+
});
15+
});
16+
17+
it('[whitelist: div p]', function (done) {
18+
lint.test(file, {
19+
'no-mergeable-selectors': [
20+
1,
21+
{
22+
'whitelist': [
23+
'div p'
24+
]
25+
}
26+
]
27+
}, function (data) {
28+
lint.assert.equal(18, data.warningCount);
29+
done();
30+
});
31+
});
32+
33+
});
34+
35+
describe('no mergeable selectors - sass', function () {
36+
var file = lint.file('no-mergeable-selectors.sass');
37+
38+
it('[default]', function (done) {
39+
lint.test(file, {
40+
'no-mergeable-selectors': 1
41+
}, function (data) {
42+
lint.assert.equal(19, data.warningCount);
43+
done();
44+
});
45+
});
46+
47+
it('[whitelist: div p]', function (done) {
48+
lint.test(file, {
49+
'no-mergeable-selectors': [
50+
1,
51+
{
52+
'whitelist': [
53+
'div p'
54+
]
55+
}
56+
]
57+
}, function (data) {
58+
lint.assert.equal(18, data.warningCount);
59+
done();
60+
});
61+
});
62+
63+
});

‎tests/sass/no-mergeable-selectors.sass

+177
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
// comments denote default rule behaviour
2+
3+
h1
4+
color: #fff
5+
6+
// mergeable with the rule above
7+
h1
8+
margin: 0
9+
10+
h1.new > p
11+
color: #000
12+
13+
h1,
14+
h2,
15+
h3
16+
color: #fff
17+
18+
// mergeable with the rule above
19+
h1, h2, h3
20+
color: #fff
21+
22+
.mergeable .test
23+
color: #000
24+
25+
// mergeable with the rule above
26+
.mergeable .test
27+
border: 1px solid #fff
28+
29+
.mergeable, .test
30+
color: #000
31+
32+
// mergeable with the rule above
33+
.mergeable, .test
34+
border: 1px solid #fff
35+
36+
.mergeable #h1.new
37+
color: #000
38+
39+
input[type='text']
40+
border: 0
41+
42+
// mergeable with the rule above
43+
input[type='text']
44+
background: none
45+
46+
input[type='radio']
47+
color: red
48+
49+
ul > li
50+
padding: 0
51+
52+
// mergeable with the rule above
53+
ul > li
54+
margin: 0
55+
56+
div p
57+
padding: 0
58+
59+
// meregeable with the rule above
60+
div p
61+
padding: 0
62+
63+
div > p
64+
margin: 0
65+
66+
// mergeable with the rule above
67+
div > p
68+
padding: 0
69+
70+
*
71+
margin: 0
72+
73+
//mergeable with the rule above
74+
*
75+
margin: 0
76+
77+
[target]
78+
padding: 0
79+
80+
// mergeable with the rule above
81+
[target]
82+
padding: 0
83+
84+
p:last-child
85+
margin: 0
86+
87+
// rule is mergeable with the rule above
88+
p:last-child
89+
margin: 0
90+
91+
p:nth-of-type(2)
92+
margin: 0
93+
94+
// rule is mergeable with the rule above
95+
p:nth-of-type(2)
96+
margin: 0
97+
98+
input:read-only
99+
content: ''
100+
101+
// mergeable with the rule above
102+
input:read-only
103+
content: ''
104+
105+
.b-some
106+
107+
.image + .buttons
108+
margin-top: 14px
109+
110+
&__test
111+
content: ''
112+
113+
#h1
114+
content: ''
115+
116+
#h2
117+
content: ''
118+
119+
.image
120+
content: ''
121+
122+
//mergeable with the .image rule directly above
123+
.image
124+
margin: 2px
125+
126+
.image + .buttons
127+
margin-top: 14px
128+
129+
&__test
130+
content: ''
131+
132+
#h1
133+
content: ''
134+
135+
#h2
136+
content: ''
137+
138+
&__end
139+
content: ''
140+
141+
// mergeable with the previous block
142+
.b-some
143+
144+
&__image
145+
width: 100%
146+
147+
&__image + &__buttons
148+
margin-top: 14px
149+
150+
//mergeable with the end rule in the previous block
151+
&__end
152+
content: ''
153+
154+
p::first-line
155+
color: #ff0000
156+
font-variant: small-caps
157+
158+
// mergeable with the rule above
159+
p::first-line
160+
color: #ff0000
161+
font-variant: small-caps
162+
163+
ul ~ p
164+
color: #ff0000
165+
166+
.test
167+
.bar
168+
color: blue
169+
170+
// 2 rules mergeable with the rules above
171+
.test
172+
.bar
173+
color: red
174+
175+
//make sure a low level selector doesn't flag with a nested selector above
176+
.bar
177+
content: ''

‎tests/sass/no-mergeable-selectors.scss

+227
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,227 @@
1+
// comments denote default rule behaviour
2+
3+
h1 {
4+
color: #fff;
5+
}
6+
7+
// mergeable with the rule above
8+
h1 {
9+
margin: 0;
10+
}
11+
12+
h1.new > p{
13+
color: #000;
14+
}
15+
16+
h1,
17+
h2,
18+
h3 {
19+
color: #fff;
20+
}
21+
22+
// mergeable with the rule above
23+
h1, h2, h3 {
24+
color: #fff;
25+
}
26+
27+
.mergeable .test {
28+
color: #000;
29+
}
30+
31+
// mergeable with the rule above
32+
.mergeable .test {
33+
border: 1px solid #fff;
34+
}
35+
36+
.mergeable, .test {
37+
color: #000;
38+
}
39+
40+
// mergeable with the rule above
41+
.mergeable, .test {
42+
border: 1px solid #fff;
43+
}
44+
45+
.mergeable #h1.new {
46+
color: #000;
47+
}
48+
49+
input[type='text'] {
50+
border: 0;
51+
}
52+
53+
// mergeable with the rule above
54+
input[type='text'] {
55+
background: none;
56+
}
57+
58+
input[type='radio'] {
59+
color: red;
60+
}
61+
62+
ul > li {
63+
padding: 0;
64+
}
65+
66+
// mergeable with the rule above
67+
ul > li {
68+
margin: 0;
69+
}
70+
71+
div p {
72+
padding: 0;
73+
}
74+
75+
// meregeable with the rule above
76+
div p {
77+
padding: 0;
78+
}
79+
80+
div > p {
81+
margin: 0;
82+
}
83+
84+
// mergeable with the rule above
85+
div > p {
86+
padding: 0;
87+
}
88+
89+
* {
90+
margin: 0;
91+
}
92+
93+
//mergeable with the rule above
94+
* {
95+
margin: 0;
96+
}
97+
98+
[target] {
99+
padding: 0;
100+
}
101+
102+
// mergeable with the rule above
103+
[target] {
104+
padding: 0;
105+
}
106+
107+
p:last-child {
108+
margin: 0;
109+
}
110+
111+
// rule is mergeable with the rule above
112+
p:last-child {
113+
margin: 0;
114+
}
115+
116+
p:nth-of-type(2) {
117+
margin: 0;
118+
}
119+
120+
// rule is mergeable with the rule above
121+
p:nth-of-type(2) {
122+
margin: 0;
123+
}
124+
125+
input:read-only {
126+
content: '';
127+
}
128+
129+
// mergeable with the rule above
130+
input:read-only {
131+
content: '';
132+
}
133+
134+
.b-some {
135+
136+
.image + .buttons {
137+
margin-top: 14px;
138+
139+
&__test {
140+
content: '';
141+
142+
#h1 {
143+
content: '';
144+
}
145+
146+
#h2 {
147+
content: '';
148+
}
149+
}
150+
}
151+
.image {
152+
content: '';
153+
}
154+
155+
//mergeable with the .image rule directly above
156+
.image {
157+
margin: 2px;
158+
159+
.image + .buttons {
160+
margin-top: 14px;
161+
162+
&__test {
163+
content: '';
164+
165+
#h1 {
166+
content: '';
167+
}
168+
169+
#h2 {
170+
content: '';
171+
}
172+
}
173+
}
174+
}
175+
&__end {
176+
content: '';
177+
}
178+
}
179+
180+
// mergeable with the previous block
181+
.b-some {
182+
183+
&__image {
184+
width: 100%;
185+
}
186+
&__image + &__buttons {
187+
margin-top: 14px;
188+
}
189+
190+
//mergeable with the end rule in the previous block
191+
&__end {
192+
content: '';
193+
}
194+
}
195+
196+
p::first-line {
197+
color: #ff0000;
198+
font-variant: small-caps;
199+
}
200+
201+
// mergeable with the rule above
202+
p::first-line {
203+
color: #ff0000;
204+
font-variant: small-caps;
205+
}
206+
207+
ul ~ p {
208+
color: #ff0000;
209+
}
210+
211+
.test {
212+
.bar {
213+
color: blue;
214+
}
215+
}
216+
217+
// 2 rules mergeable with the rules above
218+
.test {
219+
.bar {
220+
color: red;
221+
}
222+
}
223+
224+
//make sure a low level selector doesn't flag with a nested selector above
225+
.bar {
226+
content: '';
227+
}

0 commit comments

Comments
 (0)
Please sign in to comment.