Skip to content

Commit

Permalink
🔍 Update variable for property options
Browse files Browse the repository at this point in the history
  • Loading branch information
DanPurdy committed Sep 2, 2017
1 parent 658f4c4 commit 31d8343
Show file tree
Hide file tree
Showing 5 changed files with 267 additions and 2 deletions.
95 changes: 95 additions & 0 deletions docs/rules/variable-for-property.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
45 changes: 43 additions & 2 deletions lib/rules/variable-for-property.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [];
Expand All @@ -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,
Expand Down
106 changes: 106 additions & 0 deletions tests/rules/variable-for-property.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
11 changes: 11 additions & 0 deletions tests/sass/variable-for-property.sass
Original file line number Diff line number Diff line change
Expand Up @@ -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)
12 changes: 12 additions & 0 deletions tests/sass/variable-for-property.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

0 comments on commit 31d8343

Please sign in to comment.