Merge pull request #835 from DanPurdy/feature/fix-mergeable-selectors

Fix mergeable selectors

lib/selector-helpers.js

@@ -16,6 +16,15 @@ var simpleIdents = [
   'attributeMatch'
 ];
 
+var subSelectors = [
+  'parentSelectorExtension',
+  'attributeName',
+  'attributeValue',
+  'dimension',
+  'selector',
+  'function'
+];
+
 /**
  * Adds grammar around our content blocks to construct selectors with
  * more readable formats.
@@ -61,36 +70,44 @@ var constructSubSelector = function (val, prefix, suffix, constructSelector) {
 var constructSelector = function (val) {
   var content = null;
 
-  if (val.is('id')) {
-    content = addGrammar(val, '#', '');
+  if (val.is('arguments')) {
+    content = constructSubSelector(val, '(', ')', constructSelector);
+  }
+
+  else if (val.is('atkeyword')) {
+    content = constructSubSelector(val, '@', '', constructSelector);
+  }
+
+  else if (val.is('attributeSelector')) {
+    content = constructSubSelector(val, '[', ']', constructSelector);
   }
 
   else if (val.is('class')) {
     content = addGrammar(val, '.', '');
   }
 
-  else if (simpleIdents.indexOf(val.type) !== -1) {
-    content = val.content;
+  else if (val.is('id')) {
+    content = addGrammar(val, '#', '');
   }
 
-  else if (val.is('arguments')) {
-    content = constructSubSelector(val, '(', ')', constructSelector);
+  else if (val.is('interpolation')) {
+    content = constructSubSelector(val, '#{', '}', constructSelector);
   }
 
-  else if (val.is('attributeSelector')) {
-    content = constructSubSelector(val, '[', ']', constructSelector);
+  else if (val.is('nth')) {
+    content = addGrammar(val, '(', ')');
   }
 
-  else if (val.is('atkeyword')) {
-    content = constructSubSelector(val, '@', '', constructSelector);
+  else if (val.is('nthSelector')) {
+    content = constructSubSelector(val, ':', '', constructSelector);
   }
 
-  else if (val.is('placeholder')) {
-    content = constructSubSelector(val, '%', '', constructSelector);
+  else if (val.is('parentheses')) {
+    content = constructSubSelector(val, '(', ')', constructSelector);
   }
 
-  else if (val.is('variable')) {
-    content = constructSubSelector(val, '$', '', constructSelector);
+  else if (val.is('placeholder')) {
+    content = constructSubSelector(val, '%', '', constructSelector);
   }
 
   else if (val.is('pseudoClass')) {
@@ -101,29 +118,22 @@ var constructSelector = function (val) {
     content = addGrammar(val, '::', '');
   }
 
-  else if (val.is('nth')) {
-    content = addGrammar(val, '(', ')');
-  }
-
-  else if (val.is('nthSelector')) {
-    content = constructSubSelector(val, ':', '', constructSelector);
+  else if (val.is('space')) {
+    content = ' ';
   }
 
-  else if (val.is('parentheses')) {
-    content = constructSubSelector(val, '(', ')', constructSelector);
+  else if (val.is('variable')) {
+    content = constructSubSelector(val, '$', '', constructSelector);
   }
 
-  else if (val.is('space')) {
-    content = ' ';
+  else if (simpleIdents.indexOf(val.type) !== -1) {
+    content = val.content;
   }
 
-  else if (val.is('parentSelectorExtension') || val.is('attributeName') || val.is('attributeValue') || val.is('dimension')) {
+  else if (subSelectors.indexOf(val.type) !== -1) {
     content = constructSubSelector(val, '', '', constructSelector);
   }
 
-  else if (val.is('interpolation')) {
-    content = constructSubSelector(val, '#{', '}', constructSelector);
-  }
   return content;
 };
 

tests/rules/no-mergeable-selectors.js

@@ -9,7 +9,7 @@ describe('no mergeable selectors - scss', function () {
     lint.test(file, {
       'no-mergeable-selectors': 1
     }, function (data) {
-      lint.assert.equal(22, data.warningCount);
+      lint.assert.equal(24, data.warningCount);
       done();
     });
   });
@@ -25,7 +25,7 @@ describe('no mergeable selectors - scss', function () {
         }
       ]
     }, function (data) {
-      lint.assert.equal(21, data.warningCount);
+      lint.assert.equal(23, data.warningCount);
       done();
     });
   });
@@ -40,7 +40,7 @@ describe('no mergeable selectors - sass', function () {
     lint.test(file, {
       'no-mergeable-selectors': 1
     }, function (data) {
-      lint.assert.equal(20, data.warningCount);
+      lint.assert.equal(21, data.warningCount);
       done();
     });
   });
@@ -57,7 +57,7 @@ describe('no mergeable selectors - sass', function () {
         }
       ]
     }, function (data) {
-      lint.assert.equal(19, data.warningCount);
+      lint.assert.equal(20, data.warningCount);
       done();
     });
   });

tests/sass/no-mergeable-selectors.sass

@@ -179,6 +179,20 @@ ul ~ p
 .bar
   content: ''
 
+// Issue #834 - selectors/typeselectors not properly recognised
+.fake-field
+  tbody
+    tr:nth-child(even)
+      background: lighten($theme-color-primary, 50%)
+    tr:nth-child(odd)
+      background: #FFFFFF
+
+.not-test
+  &:not(:first-child)
+    border-left: none
+
+  &:not(:first-child)
+      border-left: 2px
 
 .bar
   @media (max-width: 40em) and (min-width: 20em) and (orientation: landscape)
@@ -237,15 +251,3 @@ ul ~ p
 //     opacity: 1
 
 // Issue #703 - Interpolation in selector - ignored in Sass syntax for now due to gonzales issue
-
-.navigation
-  @media #{$media-query-lg-up}
-    .nav-item
-      display: inline-block
-  @media #{$media-query-md-down}
-    // should not merge with the media query above
-    .nav-item
-      display: block
-    // should merge with the ruleset directly above
-    .nav-item
-      color: $blue

tests/sass/no-mergeable-selectors.scss

@@ -320,3 +320,43 @@ ul ~ p {
     }
   }
 }
+
+// issue 826 - media queries with functions
+@media(min-width: break('large')) {
+  .test {
+    float: left;
+  }
+}
+
+@media(min-width: break('small')) {
+  .test {
+    float: left;
+  }
+}
+
+@media(min-width: break('small')) {
+  .test {
+    float: none;
+  }
+}
+
+// Issue #834 - selectors/typeselectors not properly recognised
+.fake-field {
+  tbody {
+    tr:nth-child(even) {
+      background: lighten($theme-color-primary, 50%);
+    }
+    tr:nth-child(odd) {
+      background: #FFFFFF;
+    }
+  }
+}
+
+.pseudo-not {
+  &:not(:first-child) {
+    border-left: none;
+  }
+  &:not(:first-child) {
+    border-left: 2px;
+  }
+}

tests/sass/selector-helpers/selector-helpers.scss

@@ -59,3 +59,10 @@ p:nth-of-type(2) {
     color: red;
   }
 }
+
+tr:nth-child(even) {
+  background: lighten($theme-color-primary, 50%);
+}
+tr:nth-child(odd) {
+  background: #FFFFFF;
+}

tests/selector-helpers/selectorHelpers.js

@@ -25,7 +25,11 @@ describe('selectorHelpers - constructSelector', function () {
         '.wrong-element:selection',
         'p:nth-of-type(2)',
         '.test',
-        '&__test'
+        '&__test',
+        'tr:nth-child(even)',
+        'even',
+        'tr:nth-child(odd)',
+        'odd'
       ],
       selectorList = [];
 
@@ -46,67 +50,86 @@ describe('selectorHelpers - constructSelector', function () {
   // contructSelector
   //////////////////////////////
 
+  // .test
   it('should return the correct class name', function (done) {
     assert(equal(selectorList[0], expectedSelectors[0]));
     done();
   });
 
+  // #test
   it('should return the correct ID name', function (done) {
     assert(equal(selectorList[1], expectedSelectors[1]));
     done();
   });
 
+  // %test
   it('should return the correct placeholder name', function (done) {
     assert(equal(selectorList[2], expectedSelectors[2]));
     done();
   });
 
+  // .#{test}
   it('should return the correct interpolated selector name', function (done) {
     assert(equal(selectorList[3], expectedSelectors[3]));
     done();
   });
 
+  // input[type="text"]
   it('should return the correct type selector name', function (done) {
     assert(equal(selectorList[6], expectedSelectors[6]));
     done();
   });
 
+  // .test > li
   it('should return the correct combinator selector name', function (done) {
     assert(equal(selectorList[7], expectedSelectors[7]));
     done();
   });
 
+  // span[lang~=en-us]
   it('should return the correct attribute selector name', function (done) {
     assert(equal(selectorList[8], expectedSelectors[8]));
     done();
   });
 
+  // .block__element-one
   it('should return the correct BEM selector name', function (done) {
     assert(equal(selectorList[9], expectedSelectors[9]));
     done();
   });
 
+  // ##{$id}
   it('should return the correct interpolated ID selector name', function (done) {
     assert(equal(selectorList[10], expectedSelectors[10]));
     done();
   });
 
+  // .right-element::-ms-backdrop
   it('should return the correct pseudo element selector name', function (done) {
     assert(equal(selectorList[11], expectedSelectors[11]));
     done();
   });
 
+  // .wrong-element:selection
   it('should return the correct pseudo selector name', function (done) {
     assert(equal(selectorList[12], expectedSelectors[12]));
     done();
   });
 
+  // p:nth-of-type(2)
   it('should return the correct nth selector name', function (done) {
     assert(equal(selectorList[13], expectedSelectors[13]));
     done();
   });
 
+  // &__test
   it('should return the correct parent selector name', function (done) {
+    assert(equal(selectorList[15], expectedSelectors[15]));
+    done();
+  });
+
+  // tr:nth-child(even)
+  it('should return the correct nth selector and typeselector', function (done) {
     assert(equal(selectorList[16], expectedSelectors[16]));
     done();
   });