Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix false positives for namespaced variables in property-no-unknown #4803

Merged
merged 4 commits into from Jun 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions lib/rules/property-no-unknown/__tests__/index.js
Expand Up @@ -86,6 +86,10 @@ testRule({
code: '.foo { $bgColor: white; }',
description: 'ignore SCSS variables',
},
{
code: '.foo { namespace.$bgColor: white; }',
description: 'ignore SCSS variables within namespace',
},
{
code: '.foo { #{$prop}: black; }',
description: 'ignore property interpolation',
Expand Down
36 changes: 36 additions & 0 deletions lib/utils/__tests__/isScssVariable.test.js
@@ -0,0 +1,36 @@
'use strict';

const isScssVariable = require('../isScssVariable');

describe('isScssVariable', () => {
it('sass variable', () => {
expect(isScssVariable('$sass-variable')).toBeTruthy();
});
it('sass variable within namespace', () => {
expect(isScssVariable('namespace.$sass-variable')).toBeTruthy();
});
it('sass interpolation', () => {
expect(isScssVariable('#{$Attr}-color')).toBeFalsy();
});
it('single word property', () => {
expect(isScssVariable('top')).toBeFalsy();
});
it('hyphenated property', () => {
expect(isScssVariable('border-top-left-radius')).toBeFalsy();
});
it('property with vendor prefix', () => {
expect(isScssVariable('-webkit-appearance')).toBeFalsy();
});
it('custom property', () => {
expect(isScssVariable('--custom-property')).toBeFalsy();
});
it('less variable', () => {
expect(isScssVariable('@var')).toBeFalsy();
});
it('less append property value with comma', () => {
expect(isScssVariable('transform+')).toBeFalsy();
});
it('less append property value with space', () => {
expect(isScssVariable('transform+_')).toBeFalsy();
});
});
12 changes: 12 additions & 0 deletions lib/utils/__tests__/isStandardSyntaxDeclaration.test.js
Expand Up @@ -121,6 +121,18 @@ describe('isStandardSyntaxDeclaration', () => {
});
});

it('scss var within namespace', () => {
scssDecls('namespace.$var: b', (decl) => {
expect(isStandardSyntaxDeclaration(decl)).toBeFalsy();
});
});

it('nested scss var within namespace', () => {
scssDecls('a { namespace.$var: b }', (decl) => {
expect(isStandardSyntaxDeclaration(decl)).toBeFalsy();
});
});

it('sass list', () => {
sassDecls('$list: (key: value, key2: value2)', (decl) => {
expect(isStandardSyntaxDeclaration(decl)).toBeFalsy();
Expand Down
3 changes: 3 additions & 0 deletions lib/utils/__tests__/isStandardSyntaxProperty.test.js
Expand Up @@ -18,6 +18,9 @@ describe('isStandardSyntaxProperty', () => {
it('sass variable', () => {
expect(isStandardSyntaxProperty('$sass-variable')).toBeFalsy();
});
it('sass variable within namespace', () => {
expect(isStandardSyntaxProperty('namespace.$sass-variable')).toBeFalsy();
});
it('sass interpolation', () => {
expect(isStandardSyntaxProperty('#{$Attr}-color')).toBeFalsy();
});
Expand Down
21 changes: 21 additions & 0 deletions lib/utils/isScssVariable.js
@@ -0,0 +1,21 @@
'use strict';

/**
* Check whether a property is SCSS variable
*
* @param {string} property
* @returns {boolean}
*/
module.exports = function (property) {
// SCSS var (e.g. $var: x), list (e.g. $list: (x)) or map (e.g. $map: (key:value))
if (property.startsWith('$')) {
return true;
}

// SCSS var within a namespace (e.g. namespace.$var: x)
if (property.includes('.$')) {
return true;
}

return false;
};
5 changes: 3 additions & 2 deletions lib/utils/isStandardSyntaxDeclaration.js
@@ -1,5 +1,6 @@
'use strict';

const isScssVariable = require('./isScssVariable');
const { isRoot } = require('./typeGuards');

/**
Expand Down Expand Up @@ -29,8 +30,8 @@ module.exports = function (decl) {
return false;
}

// Sass var (e.g. $var: x), nested list (e.g. $list: (x)) or nested map (e.g. $map: (key:value))
if (prop[0] === '$') {
// SCSS var
if (isScssVariable(prop)) {
return false;
}

Expand Down
5 changes: 3 additions & 2 deletions lib/utils/isStandardSyntaxProperty.js
@@ -1,6 +1,7 @@
'use strict';

const hasInterpolation = require('../utils/hasInterpolation');
const isScssVariable = require('./isScssVariable');

/**
* Check whether a property is standard
Expand All @@ -9,8 +10,8 @@ const hasInterpolation = require('../utils/hasInterpolation');
* @returns {boolean}
*/
module.exports = function (property) {
// SCSS var (e.g. $var: x), list (e.g. $list: (x)) or map (e.g. $map: (key:value))
if (property.startsWith('$')) {
// SCSS var
if (isScssVariable(property)) {
return false;
}

Expand Down