Skip to content

Commit 72f4cad

Browse files
DanPurdyDanPurdy
authored
Merge pull request #1128 from sasstools/feature_update-variable-for-property
Update variable for property options
2 parents 485a461 + f142793 commit 72f4cad

File tree

5 files changed

+282
-4
lines changed

5 files changed

+282
-4
lines changed

‎docs/rules/variable-for-property.md‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@ The `!important` flag will also be excluded when used.
1414

1515
* `properties`: `[array of property names]` (defaults to empty array `[]`)
1616

17+
* `allow-map-get`: `true`/`false` (defaults to `true`) You may allow/disallow the use of `map-get()` as property values
18+
19+
* `allowed-functions`: `[array of function names]` (defaults to empty array `[]`)
20+
1721
You may pass an array of properties you wish to enforce the use of variables for
1822

1923
```yaml
@@ -26,10 +30,54 @@ variable-for-property:
2630
- 'content'
2731
```
2832
33+
You may pass an array of function names you wish to allow as property values
34+
35+
```yaml
36+
37+
variable-for-property:
38+
- 1
39+
-
40+
'allowed-functions':
41+
- 'my-map-func'
42+
- 'palette'
43+
```
44+
45+
*** full config example ***
46+
47+
```yaml
48+
variable-for-property:
49+
- 1
50+
-
51+
allow-map-get: true
52+
allowed-functions:
53+
- my-map-func
54+
- palette
55+
properties:
56+
- margin
57+
- content
58+
```
59+
2960
## Examples
3061
3162
By default `properties` is an empty array and therefore no properties are forced to use variables as values.
3263

64+
```scss
65+
.bar {
66+
content: ' ';
67+
margin: 0;
68+
69+
&__element {
70+
margin: 0;
71+
}
72+
}
73+
74+
@mixin red() {
75+
margin: 0;
76+
}
77+
```
78+
79+
## [properties: []]
80+
3381
When `properties` contains the values shown in the options section example the following would be disallowed:
3482

3583
```scss
@@ -65,6 +113,53 @@ When `properties` contains the values shown in the options section example the f
65113
66114
```
67115

116+
## [allow-map-get: true]
117+
118+
When allow-map-get is set to `true` and properties contains the `color` property, the following would be allowed
119+
120+
```scss
121+
.foo {
122+
color: map-get(blueish, light);
123+
}
124+
```
125+
126+
When allow-map-get is set to `false` and properties contains the `color` property, the following would be disallowed
127+
128+
```scss
129+
.foo {
130+
color: map-get(blueish, light);
131+
}
132+
```
133+
134+
## [allowed-functions: []]
135+
136+
When `allowed-functions` contains the values shown in the options section and `properties` includes the `color` property the following would be disallowed:
137+
138+
```scss
139+
.foo {
140+
color: disallowed-function($test, $vars);
141+
142+
&__element {
143+
color: invalid-func-name($test, $vars);
144+
}
145+
}
146+
```
147+
148+
When `allowed-functions` contains the values shown in the options section and `properties` includes the `color` property the following would be disallowed:
149+
150+
```scss
151+
.foo {
152+
color: my-map-func($allowed);
153+
154+
&__element {
155+
color: palette(blue, light);
156+
}
157+
}
158+
159+
```
160+
161+
## Extra info
162+
68163
The `!important` flag will be excluded from any lint warnings.
69164

70165
For example if `properties` contains the value `color` the following would be disallowed

‎lib/rules/variable-for-property.js‎

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@ var whitelistedValues = ['inherit', 'initial', 'transparent', 'none', 'currentCo
1414
*/
1515
var isValidProperty = function (propertyElem) {
1616
if (propertyElem) {
17-
if (propertyElem.type === 'variable') {
17+
if (propertyElem.is('variable')) {
1818
return true;
1919
}
20-
else if (propertyElem.type === 'ident' && whitelistedValues.indexOf(propertyElem.content) !== -1) {
20+
else if (propertyElem.is('ident') && whitelistedValues.indexOf(propertyElem.content) !== -1) {
2121
return true;
2222
}
2323
}
@@ -37,10 +37,41 @@ var isIgnoredType = function (propertyElem) {
3737
return false;
3838
};
3939

40+
/**
41+
* Checks If the property type is a function and whether it is allowed
42+
*
43+
* @param {Object} propertyElem - The property element
44+
* @param {boolean} allowMap - Whether the user has specified to allow Sass function map-get
45+
* @param {Array} functionWhitelist - An array of string - function names we wish to allow
46+
* @returns {boolean} Whether the property is an ignored type or not
47+
*/
48+
var isIgnoredFunction = function (propertyElem, allowMap, functionWhitelist) {
49+
if (propertyElem && propertyElem.is('function')) {
50+
var funcIdent = propertyElem.first('ident');
51+
52+
// allow custom properties as values
53+
if (funcIdent.content === 'var') {
54+
return true;
55+
}
56+
57+
if (allowMap && funcIdent.content === 'map-get') {
58+
return true;
59+
}
60+
61+
if (functionWhitelist.length) {
62+
return functionWhitelist.indexOf(funcIdent.content) !== -1;
63+
}
64+
}
65+
return false;
66+
};
67+
68+
4069
module.exports = {
4170
'name': 'variable-for-property',
4271
'defaults': {
43-
'properties': []
72+
'properties': [],
73+
'allow-map-get': true,
74+
'allowed-functions': []
4475
},
4576
'detect': function (ast, parser) {
4677
var result = [];
@@ -54,7 +85,22 @@ module.exports = {
5485
if (declarationType === 'ident') {
5586
if (parser.options.properties.indexOf(declarationIdent) !== -1) {
5687
node.forEach(function (valElem) {
57-
if (!isValidProperty(valElem) && !isIgnoredType(valElem)) {
88+
89+
if (valElem.is('function') && !isIgnoredFunction(valElem, parser.options['allow-map-get'], parser.options['allowed-functions'])) {
90+
result = helpers.addUnique(result, {
91+
'ruleId': parser.rule.name,
92+
'line': declaration.start.line,
93+
'column': declaration.start.column,
94+
'message': 'The function passed to \'' + declarationIdent + '\' is not allowed',
95+
'severity': parser.severity
96+
});
97+
}
98+
else if (
99+
!valElem.is('function') &&
100+
!isValidProperty(valElem) &&
101+
!isIgnoredType(valElem) &&
102+
!valElem.is('interpolation')
103+
) {
58104
result = helpers.addUnique(result, {
59105
'ruleId': parser.rule.name,
60106
'line': declaration.start.line,

‎tests/rules/variable-for-property.js‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,59 @@ describe('variable for property - scss', function () {
4545
]
4646
}
4747
]
48+
}, function (data) {
49+
lint.assert.equal(2, data.warningCount);
50+
done();
51+
});
52+
});
53+
54+
it('[properties: [\'color\'], allow-map-get: true]', function (done) {
55+
lint.test(file, {
56+
'variable-for-property': [
57+
1,
58+
{
59+
'properties': [
60+
'color'
61+
],
62+
'allow-map-get': true
63+
}
64+
]
65+
}, function (data) {
66+
lint.assert.equal(2, data.warningCount);
67+
done();
68+
});
69+
});
70+
71+
it('[properties: [\'color\'], allow-map-get: false]', function (done) {
72+
lint.test(file, {
73+
'variable-for-property': [
74+
1,
75+
{
76+
'properties': [
77+
'color'
78+
],
79+
'allow-map-get': false
80+
}
81+
]
82+
}, function (data) {
83+
lint.assert.equal(3, data.warningCount);
84+
done();
85+
});
86+
});
87+
88+
it('[properties: [\'color\'], allowed-functions: [\'my-map-func\']]', function (done) {
89+
lint.test(file, {
90+
'variable-for-property': [
91+
1,
92+
{
93+
'properties': [
94+
'color'
95+
],
96+
'allowed-functions': [
97+
'my-map-func'
98+
]
99+
}
100+
]
48101
}, function (data) {
49102
lint.assert.equal(1, data.warningCount);
50103
done();
@@ -95,6 +148,59 @@ describe('variable for property - sass', function () {
95148
]
96149
}
97150
]
151+
}, function (data) {
152+
lint.assert.equal(2, data.warningCount);
153+
done();
154+
});
155+
});
156+
157+
it('[properties: [\'color\'], allow-map-get: true]', function (done) {
158+
lint.test(file, {
159+
'variable-for-property': [
160+
1,
161+
{
162+
'properties': [
163+
'color'
164+
],
165+
'allow-map-get': true
166+
}
167+
]
168+
}, function (data) {
169+
lint.assert.equal(2, data.warningCount);
170+
done();
171+
});
172+
});
173+
174+
it('[properties: [\'color\'], allow-map-get: false]', function (done) {
175+
lint.test(file, {
176+
'variable-for-property': [
177+
1,
178+
{
179+
'properties': [
180+
'color'
181+
],
182+
'allow-map-get': false
183+
}
184+
]
185+
}, function (data) {
186+
lint.assert.equal(3, data.warningCount);
187+
done();
188+
});
189+
});
190+
191+
it('[properties: [\'color\'], allowed-functions: [\'my-map-func\']]', function (done) {
192+
lint.test(file, {
193+
'variable-for-property': [
194+
1,
195+
{
196+
'properties': [
197+
'color'
198+
],
199+
'allowed-functions': [
200+
'my-map-func'
201+
]
202+
}
203+
]
98204
}, function (data) {
99205
lint.assert.equal(1, data.warningCount);
100206
done();

‎tests/sass/variable-for-property.sass‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,18 @@
3535

3636
.issue
3737
color: red !important
38+
39+
.test
40+
color: map-get($blue)
41+
42+
.interp-test
43+
color: #{var}
44+
45+
// ensure interp is not flagged
46+
47+
.func-name-test
48+
color: my-map-func(blue, light)
49+
50+
.custom-prop
51+
// ensure custom properties are valid
52+
color: var(--my-prop)

‎tests/sass/variable-for-property.scss‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,19 @@
3939
.t-neutral {
4040
color: red !important;
4141
}
42+
43+
.test {
44+
color: map-get($blue);
45+
}
46+
47+
.interp-test {
48+
color: #{var}; // ensure interp is not flagged
49+
}
50+
51+
.func-name-test {
52+
color: my-map-func(blue, light);
53+
}
54+
55+
.custom-prop {
56+
color: var(--my-prop); // ensure custom properties are valid
57+
}

0 commit comments

Comments
 (0)