Merge pull request #715 from DanPurdy/feature/fix-variable-for-property

Incorrect variable for property value warning

docs/rules/variable-for-property.md

@@ -8,6 +8,8 @@ There are no properties by default, except for reserved words listed below which
 * none
 * currentColor
 
+The `!important` flag will also be excluded when used.
+
 ## Options
 
 * `properties`: `[array of property names]` (defaults to empty array `[]`)
@@ -62,3 +64,21 @@ When `properties` contains the values shown in the options section example the f
 }
 
 ```
+
+The `!important` flag will be excluded from any lint warnings.
+
+For example if `properties` contains the value `color` the following would be disallowed
+
+```scss
+.foo {
+  color: red !important;
+}
+```
+
+The following would be allowed
+
+```scss
+.foo {
+  color: $red-var !important;
+}
+```

lib/rules/variable-for-property.js

@@ -3,7 +3,8 @@
 var helpers = require('../helpers');
 
 // The whitelisted ident values
-var whitelistedValues = ['inherit', 'initial', 'transparent', 'none', 'currentColor'];
+var whitelistedValues = ['inherit', 'initial', 'transparent', 'none', 'currentColor'],
+    ignoredValueTypes = ['important', 'space'];
 
 /**
  * Checks If the property is of a valid type, either its a variable or it's a whitelisted ident value
@@ -23,6 +24,19 @@ var isValidProperty = function (propertyElem) {
   return false;
 };
 
+/**
+ * Checks If the property type is an ignored value type
+ *
+ * @param {Object} propertyElem - The property element
+ * @returns {boolean} Whether the property is an ignored type or not
+ */
+var isIgnoredType = function (propertyElem) {
+  if (propertyElem) {
+    return ignoredValueTypes.indexOf(propertyElem.type) !== -1;
+  }
+  return false;
+};
+
 module.exports = {
   'name': 'variable-for-property',
   'defaults': {
@@ -40,7 +54,7 @@ module.exports = {
         if (declarationType === 'ident') {
           if (parser.options.properties.indexOf(declarationIdent) !== -1) {
             node.forEach(function (valElem) {
-              if (!isValidProperty(valElem)) {
+              if (!isValidProperty(valElem) && !isIgnoredType(valElem)) {
                 result = helpers.addUnique(result, {
                   'ruleId': parser.rule.name,
                   'line': declaration.start.line,

tests/rules/variable-for-property.js

@@ -34,6 +34,22 @@ describe('variable for property - scss', function () {
       done();
     });
   });
+
+  it('[properties: color]', function (done) {
+    lint.test(file, {
+      'variable-for-property': [
+        1,
+        {
+          'properties': [
+            'color'
+          ]
+        }
+      ]
+    }, function (data) {
+      lint.assert.equal(1, data.warningCount);
+      done();
+    });
+  });
 });
 
 //////////////////////////////
@@ -68,4 +84,20 @@ describe('variable for property - sass', function () {
       done();
     });
   });
+
+  it('[properties: color]', function (done) {
+    lint.test(file, {
+      'variable-for-property': [
+        1,
+        {
+          'properties': [
+            'color'
+          ]
+        }
+      ]
+    }, function (data) {
+      lint.assert.equal(1, data.warningCount);
+      done();
+    });
+  });
 });

tests/sass/variable-for-property.sass

@@ -28,3 +28,10 @@
   background: initial
   background: transparent
   background: none
+
+// Issue #714 - !important and spaces were classed as erroneous value types
+.non-issue
+  color: $gray-chateau !important
+
+.issue
+  color: red !important

tests/sass/variable-for-property.scss

@@ -30,3 +30,12 @@
   background: transparent;
   background: none;
 }
+
+// Issue #714 - !important and spaces were classed as erroneous value types
+.t-neutral {
+  color: $gray-chateau !important;
+}
+
+.t-neutral {
+  color: red !important;
+}