From 31d834371b9c92264168a49f33d461066e295b43 Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Sat, 2 Sep 2017 02:23:52 +0100 Subject: [PATCH 1/2] :mag: Update variable for property options --- docs/rules/variable-for-property.md | 95 +++++++++++++++++++++++ lib/rules/variable-for-property.js | 45 ++++++++++- tests/rules/variable-for-property.js | 106 ++++++++++++++++++++++++++ tests/sass/variable-for-property.sass | 11 +++ tests/sass/variable-for-property.scss | 12 +++ 5 files changed, 267 insertions(+), 2 deletions(-) diff --git a/docs/rules/variable-for-property.md b/docs/rules/variable-for-property.md index 07a4e9f7..c2809da9 100644 --- a/docs/rules/variable-for-property.md +++ b/docs/rules/variable-for-property.md @@ -14,6 +14,10 @@ The `!important` flag will also be excluded when used. * `properties`: `[array of property names]` (defaults to empty array `[]`) +* `allow-map-get`: `true`/`false` (defaults to `true`) You may allow/disallow the use of `map-get()` as property values + +* `allowed-functions`: `[array of function names]` (defaults to empty array `[]`) + You may pass an array of properties you wish to enforce the use of variables for ```yaml @@ -26,10 +30,54 @@ variable-for-property: - 'content' ``` +You may pass an array of function names you wish to allow as property values + +```yaml + +variable-for-property: + - 1 + - + 'allowed-functions': + - 'my-map-func' + - 'palette' +``` + +*** full config example *** + +```yaml +variable-for-property: + - 1 + - + allow-map-get: true + allowed-functions: + - my-map-func + - palette + properties: + - margin + - content +``` + ## Examples By default `properties` is an empty array and therefore no properties are forced to use variables as values. +```scss +.bar { + content: ' '; + margin: 0; + + &__element { + margin: 0; + } +} + +@mixin red() { + margin: 0; +} +``` + +## [properties: []] + When `properties` contains the values shown in the options section example the following would be disallowed: ```scss @@ -65,6 +113,53 @@ When `properties` contains the values shown in the options section example the f ``` +## [allow-map-get: true] + +When allow-map-get is set to `true` and properties contains the `color` property, the following would be allowed + +```scss +.foo { + color: map-get(blueish, light); +} +``` + +When allow-map-get is set to `false` and properties contains the `color` property, the following would be disallowed + +```scss +.foo { + color: map-get(blueish, light); +} +``` + +## [allowed-functions: []] + +When `allowed-functions` contains the values shown in the options section and `properties` includes the `color` property the following would be disallowed: + +```scss +.foo { + color: disallowed-function($test, $vars); + + &__element { + color: invalid-func-name($test, $vars); + } +} +``` + +When `allowed-functions` contains the values shown in the options section and `properties` includes the `color` property the following would be disallowed: + +```scss +.foo { + color: my-map-func($allowed); + + &__element { + color: palette(blue, light); + } +} + +``` + +## Extra info + The `!important` flag will be excluded from any lint warnings. For example if `properties` contains the value `color` the following would be disallowed diff --git a/lib/rules/variable-for-property.js b/lib/rules/variable-for-property.js index b006595b..a928ef32 100644 --- a/lib/rules/variable-for-property.js +++ b/lib/rules/variable-for-property.js @@ -37,10 +37,36 @@ var isIgnoredType = function (propertyElem) { return false; }; +/** + * Checks If the property type is a function and whether it is allowed + * + * @param {Object} propertyElem - The property element + * @param {boolean} allowMap - Whether the user has specified to allow Sass function map-get + * @param {Array} functionWhitelist - An array of string - function names we wish to allow + * @returns {boolean} Whether the property is an ignored type or not + */ +var isIgnoredFunction = function (propertyElem, allowMap, functionWhitelist) { + if (propertyElem && propertyElem.type === 'function') { + var funcIdent = propertyElem.first('ident'); + + if (allowMap && funcIdent.content === 'map-get') { + return true; + } + + if (functionWhitelist.length) { + return functionWhitelist.indexOf(funcIdent.content) !== -1; + } + } + return false; +}; + + module.exports = { 'name': 'variable-for-property', 'defaults': { - 'properties': [] + 'properties': [], + 'allow-map-get': true, + 'allowed-functions': [] }, 'detect': function (ast, parser) { var result = []; @@ -54,7 +80,22 @@ module.exports = { if (declarationType === 'ident') { if (parser.options.properties.indexOf(declarationIdent) !== -1) { node.forEach(function (valElem) { - if (!isValidProperty(valElem) && !isIgnoredType(valElem)) { + + if (valElem.is('function') && !isIgnoredFunction(valElem, parser.options['allow-map-get'], parser.options['allowed-functions'])) { + result = helpers.addUnique(result, { + 'ruleId': parser.rule.name, + 'line': declaration.start.line, + 'column': declaration.start.column, + 'message': 'The function passed to \'' + declarationIdent + '\' is not allowed', + 'severity': parser.severity + }); + } + else if ( + !valElem.is('function') && + !isValidProperty(valElem) && + !isIgnoredType(valElem) && + !valElem.is('interpolation') + ) { result = helpers.addUnique(result, { 'ruleId': parser.rule.name, 'line': declaration.start.line, diff --git a/tests/rules/variable-for-property.js b/tests/rules/variable-for-property.js index 8e803347..8b010599 100644 --- a/tests/rules/variable-for-property.js +++ b/tests/rules/variable-for-property.js @@ -45,6 +45,59 @@ describe('variable for property - scss', function () { ] } ] + }, function (data) { + lint.assert.equal(2, data.warningCount); + done(); + }); + }); + + it('[properties: [\'color\'], allow-map-get: true]', function (done) { + lint.test(file, { + 'variable-for-property': [ + 1, + { + 'properties': [ + 'color' + ], + 'allow-map-get': true + } + ] + }, function (data) { + lint.assert.equal(2, data.warningCount); + done(); + }); + }); + + it('[properties: [\'color\'], allow-map-get: false]', function (done) { + lint.test(file, { + 'variable-for-property': [ + 1, + { + 'properties': [ + 'color' + ], + 'allow-map-get': false + } + ] + }, function (data) { + lint.assert.equal(3, data.warningCount); + done(); + }); + }); + + it('[properties: [\'color\'], allowed-functions: [\'my-map-func\']]', function (done) { + lint.test(file, { + 'variable-for-property': [ + 1, + { + 'properties': [ + 'color' + ], + 'allowed-functions': [ + 'my-map-func' + ] + } + ] }, function (data) { lint.assert.equal(1, data.warningCount); done(); @@ -95,6 +148,59 @@ describe('variable for property - sass', function () { ] } ] + }, function (data) { + lint.assert.equal(2, data.warningCount); + done(); + }); + }); + + it('[properties: [\'color\'], allow-map-get: true]', function (done) { + lint.test(file, { + 'variable-for-property': [ + 1, + { + 'properties': [ + 'color' + ], + 'allow-map-get': true + } + ] + }, function (data) { + lint.assert.equal(2, data.warningCount); + done(); + }); + }); + + it('[properties: [\'color\'], allow-map-get: false]', function (done) { + lint.test(file, { + 'variable-for-property': [ + 1, + { + 'properties': [ + 'color' + ], + 'allow-map-get': false + } + ] + }, function (data) { + lint.assert.equal(3, data.warningCount); + done(); + }); + }); + + it('[properties: [\'color\'], allowed-functions: [\'my-map-func\']]', function (done) { + lint.test(file, { + 'variable-for-property': [ + 1, + { + 'properties': [ + 'color' + ], + 'allowed-functions': [ + 'my-map-func' + ] + } + ] }, function (data) { lint.assert.equal(1, data.warningCount); done(); diff --git a/tests/sass/variable-for-property.sass b/tests/sass/variable-for-property.sass index 78047682..45ea874d 100644 --- a/tests/sass/variable-for-property.sass +++ b/tests/sass/variable-for-property.sass @@ -35,3 +35,14 @@ .issue color: red !important + +.test + color: map-get($blue) + +.interp-test + color: #{var} + + // ensure interp is not flagged + +.func-name-test + color: my-map-func(blue, light) diff --git a/tests/sass/variable-for-property.scss b/tests/sass/variable-for-property.scss index b9621772..8725f95c 100644 --- a/tests/sass/variable-for-property.scss +++ b/tests/sass/variable-for-property.scss @@ -39,3 +39,15 @@ .t-neutral { color: red !important; } + +.test { + color: map-get($blue); +} + +.interp-test { + color: #{var}; // ensure interp is not flagged +} + +.func-name-test { + color: my-map-func(blue, light); +} From f1427932394e26b0ab7f6afdc88b94d34c6e3e64 Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Sat, 2 Sep 2017 02:39:34 +0100 Subject: [PATCH 2/2] :bug: Allow custom properties as variables --- lib/rules/variable-for-property.js | 11 ++++++++--- tests/sass/variable-for-property.sass | 4 ++++ tests/sass/variable-for-property.scss | 4 ++++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/lib/rules/variable-for-property.js b/lib/rules/variable-for-property.js index a928ef32..dd73361c 100644 --- a/lib/rules/variable-for-property.js +++ b/lib/rules/variable-for-property.js @@ -14,10 +14,10 @@ var whitelistedValues = ['inherit', 'initial', 'transparent', 'none', 'currentCo */ var isValidProperty = function (propertyElem) { if (propertyElem) { - if (propertyElem.type === 'variable') { + if (propertyElem.is('variable')) { return true; } - else if (propertyElem.type === 'ident' && whitelistedValues.indexOf(propertyElem.content) !== -1) { + else if (propertyElem.is('ident') && whitelistedValues.indexOf(propertyElem.content) !== -1) { return true; } } @@ -46,9 +46,14 @@ var isIgnoredType = function (propertyElem) { * @returns {boolean} Whether the property is an ignored type or not */ var isIgnoredFunction = function (propertyElem, allowMap, functionWhitelist) { - if (propertyElem && propertyElem.type === 'function') { + if (propertyElem && propertyElem.is('function')) { var funcIdent = propertyElem.first('ident'); + // allow custom properties as values + if (funcIdent.content === 'var') { + return true; + } + if (allowMap && funcIdent.content === 'map-get') { return true; } diff --git a/tests/sass/variable-for-property.sass b/tests/sass/variable-for-property.sass index 45ea874d..5f34c217 100644 --- a/tests/sass/variable-for-property.sass +++ b/tests/sass/variable-for-property.sass @@ -46,3 +46,7 @@ .func-name-test color: my-map-func(blue, light) + +.custom-prop + // ensure custom properties are valid + color: var(--my-prop) diff --git a/tests/sass/variable-for-property.scss b/tests/sass/variable-for-property.scss index 8725f95c..cba29b09 100644 --- a/tests/sass/variable-for-property.scss +++ b/tests/sass/variable-for-property.scss @@ -51,3 +51,7 @@ .func-name-test { color: my-map-func(blue, light); } + +.custom-prop { + color: var(--my-prop); // ensure custom properties are valid +}