From d32a785708c4b43bb2babf3ff76e8d23e3f530ae Mon Sep 17 00:00:00 2001 From: ryym Date: Sun, 24 May 2020 10:04:51 +0900 Subject: [PATCH 1/4] Add failing tests for assignment to namespaced variables --- lib/rules/property-no-unknown/__tests__/index.js | 4 ++++ .../__tests__/isStandardSyntaxDeclaration.test.js | 12 ++++++++++++ 2 files changed, 16 insertions(+) diff --git a/lib/rules/property-no-unknown/__tests__/index.js b/lib/rules/property-no-unknown/__tests__/index.js index d91505ae91..2861598661 100644 --- a/lib/rules/property-no-unknown/__tests__/index.js +++ b/lib/rules/property-no-unknown/__tests__/index.js @@ -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', diff --git a/lib/utils/__tests__/isStandardSyntaxDeclaration.test.js b/lib/utils/__tests__/isStandardSyntaxDeclaration.test.js index 3df0d982b5..c0a3fc5978 100644 --- a/lib/utils/__tests__/isStandardSyntaxDeclaration.test.js +++ b/lib/utils/__tests__/isStandardSyntaxDeclaration.test.js @@ -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(); From 4d0b2da246b98e4023d52730f85dd2fb04a8fc44 Mon Sep 17 00:00:00 2001 From: ryym Date: Sun, 24 May 2020 10:05:58 +0900 Subject: [PATCH 2/4] Ignore namespaced variables on standard declaration check --- lib/utils/isStandardSyntaxDeclaration.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/utils/isStandardSyntaxDeclaration.js b/lib/utils/isStandardSyntaxDeclaration.js index 9f36604450..ac19425af7 100644 --- a/lib/utils/isStandardSyntaxDeclaration.js +++ b/lib/utils/isStandardSyntaxDeclaration.js @@ -57,5 +57,10 @@ module.exports = function (decl) { return false; } + // Sass variables within a namespace (e.g. namespace.$var: x) + if (prop.includes('.$') && prop[0] !== '.' && prop[prop.length - 1] !== '$') { + return false; + } + return true; }; From 51d30c45e1d25580cb480910d42379a11d019d89 Mon Sep 17 00:00:00 2001 From: ryym Date: Sat, 6 Jun 2020 22:12:48 +0900 Subject: [PATCH 3/4] Add isScssVariable util function --- lib/utils/__tests__/isScssVariable.test.js | 36 ++++++++++++++++++++++ lib/utils/isScssVariable.js | 21 +++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 lib/utils/__tests__/isScssVariable.test.js create mode 100644 lib/utils/isScssVariable.js diff --git a/lib/utils/__tests__/isScssVariable.test.js b/lib/utils/__tests__/isScssVariable.test.js new file mode 100644 index 0000000000..59ac31e4d4 --- /dev/null +++ b/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(); + }); +}); diff --git a/lib/utils/isScssVariable.js b/lib/utils/isScssVariable.js new file mode 100644 index 0000000000..fbc938a723 --- /dev/null +++ b/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; +}; From 0fb66e6b6ae1b773b4440ff66b3c40dfc088090e Mon Sep 17 00:00:00 2001 From: ryym Date: Sat, 6 Jun 2020 22:14:37 +0900 Subject: [PATCH 4/4] Replace duplicate SCSS var checks with isScssVariable util --- lib/utils/__tests__/isStandardSyntaxProperty.test.js | 3 +++ lib/utils/isStandardSyntaxDeclaration.js | 10 +++------- lib/utils/isStandardSyntaxProperty.js | 5 +++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/utils/__tests__/isStandardSyntaxProperty.test.js b/lib/utils/__tests__/isStandardSyntaxProperty.test.js index ec440a26e0..5afb1a2469 100644 --- a/lib/utils/__tests__/isStandardSyntaxProperty.test.js +++ b/lib/utils/__tests__/isStandardSyntaxProperty.test.js @@ -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(); }); diff --git a/lib/utils/isStandardSyntaxDeclaration.js b/lib/utils/isStandardSyntaxDeclaration.js index ac19425af7..7c6cc2e41e 100644 --- a/lib/utils/isStandardSyntaxDeclaration.js +++ b/lib/utils/isStandardSyntaxDeclaration.js @@ -1,5 +1,6 @@ 'use strict'; +const isScssVariable = require('./isScssVariable'); const { isRoot } = require('./typeGuards'); /** @@ -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; } @@ -57,10 +58,5 @@ module.exports = function (decl) { return false; } - // Sass variables within a namespace (e.g. namespace.$var: x) - if (prop.includes('.$') && prop[0] !== '.' && prop[prop.length - 1] !== '$') { - return false; - } - return true; }; diff --git a/lib/utils/isStandardSyntaxProperty.js b/lib/utils/isStandardSyntaxProperty.js index b85d8d23ed..a42bf8a06e 100644 --- a/lib/utils/isStandardSyntaxProperty.js +++ b/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 @@ -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; }