Fix false positives in nested property declarations unit-no-unknown (#…

…5500)
stylelint · Sep 4, 2021 · 04151e9 · 04151e9
1 parent 10d0bd8
commit 04151e9
Show file tree

Hide file tree

Showing 6 changed files with 25 additions and 197 deletions.
diff --git a/lib/rules/unit-no-unknown/__tests__/index.js b/lib/rules/unit-no-unknown/__tests__/index.js
@@ -441,18 +441,6 @@ testRule({
 			code: 'a { margin: calc(100% - #{$margin * 2}); }',
 			description: 'work with interpolation',
 		},
-		{
-			code: '$foo: ( 1prop: 10px, 2prop: 12px, 3prop: /* comment */ 14px )',
-			description: 'ignore map property name',
-		},
-		{
-			code: '$breakpoints: ( small: /* comment */ 767px, 1medium: 992px, large: 1200px );',
-			description: 'ignore map property name',
-		},
-		{
-			code: '$breakpoints: ( small: /* comment */ 767px, medium: ( 1prop: 992px, 2prop: ( 1prop: 1200px ) ) );',
-			description: 'ignore map property name in nested maps',
-		},
 	],
 
 	reject: [
@@ -462,54 +450,12 @@ testRule({
 			line: 1,
 			column: 13,
 		},
-		{
-			code: 'a { $margin: 10pix; }',
-			message: messages.rejected('pix'),
-			line: 1,
-			column: 14,
-		},
-		{
-			code: 'a { $margin: 10px + 10pix; }',
-			message: messages.rejected('pix'),
-			line: 1,
-			column: 21,
-		},
 		{
 			code: 'a { margin: $margin + 10pix; }',
 			message: messages.rejected('pix'),
 			line: 1,
 			column: 23,
 		},
-		{
-			code: '$breakpoints: ( small: 767px, medium: 992pix, large: 1200px );',
-			message: messages.rejected('pix'),
-			line: 1,
-			column: 39,
-		},
-		{
-			code: '$breakpoints: ( small: 767px, 1medium: 992pix, large: 1200px );',
-			message: messages.rejected('pix'),
-			line: 1,
-			column: 40,
-		},
-		{
-			code: '$breakpoints: ( small: /* comment */ 767px, medium: ( 1prop: 992pix, 2prop: ( 1prop: 1200px ) ) );',
-			message: messages.rejected('pix'),
-			line: 1,
-			column: 49,
-		},
-		{
-			code: 'a { font: (italic bold 10px/8pix) }',
-			message: messages.rejected('pix'),
-			line: 1,
-			column: 29,
-		},
-		{
-			code: 'font: 14pix/#{$line-height};',
-			message: messages.rejected('pix'),
-			line: 1,
-			column: 7,
-		},
 		{
 			code: 'a { margin: calc(100% - #{$margin * 2pix}); }',
 			message: messages.rejected('pix'),

diff --git a/lib/rules/unit-no-unknown/index.js b/lib/rules/unit-no-unknown/index.js
@@ -5,7 +5,7 @@
 const atRuleParamIndex = require('../../utils/atRuleParamIndex');
 const declarationValueIndex = require('../../utils/declarationValueIndex');
 const getUnitFromValueNode = require('../../utils/getUnitFromValueNode');
-const isMap = require('../../utils/isMap');
+const isStandardSyntaxDeclaration = require('../../utils/isStandardSyntaxDeclaration');
 const keywordSets = require('../../reference/keywordSets');
 const mediaParser = require('postcss-media-query-parser').default;
 const optionsMatches = require('../../utils/optionsMatches');
@@ -22,10 +22,6 @@ const messages = ruleMessages(ruleName, {
 	rejected: (unit) => `Unexpected unknown unit "${unit}"`,
 });
 
-// The map property name (in map cleared from comments and spaces) always
-// has index that being divided by 4 gives remainder equals 0
-const mapPropertyNameIndexOffset = 4;
-
 function rule(actual, options) {
 	return (root, result) => {
 		const validOptions = validateOptions(
@@ -51,7 +47,6 @@ function rule(actual, options) {
 			// by postcss-value-parser
 			value = value.replace(/\*/g, ',');
 			const parsedValue = valueParser(value);
-			const ignoredMapProperties = [];
 
 			parsedValue.walk((valueNode) => {
 				// Ignore wrong units within `url` function
@@ -64,18 +59,6 @@ function rule(actual, options) {
 					return false;
 				}
 
-				if (isMap(valueNode)) {
-					valueNode.nodes.forEach(({ sourceIndex }, index) => {
-						if (!(index % mapPropertyNameIndexOffset)) {
-							ignoredMapProperties.push(sourceIndex);
-						}
-					});
-				}
-
-				if (ignoredMapProperties.includes(valueNode.sourceIndex)) {
-					return;
-				}
-
 				const unit = getUnitFromValueNode(valueNode);
 
 				if (!unit) {
@@ -152,7 +135,13 @@ function rule(actual, options) {
 
 			check(atRule, atRule.params, atRuleParamIndex);
 		});
-		root.walkDecls((decl) => check(decl, decl.value, declarationValueIndex));
+		root.walkDecls((decl) => {
+			if (!isStandardSyntaxDeclaration(decl)) {
+				return;
+			}
+
+			check(decl, decl.value, declarationValueIndex);
+		});
 	};
 }
 

diff --git a/lib/utils/__tests__/isMap.test.js b/lib/utils/__tests__/isMap.test.js
diff --git a/lib/utils/__tests__/isStandardSyntaxDeclaration.test.js b/lib/utils/__tests__/isStandardSyntaxDeclaration.test.js
@@ -138,6 +138,17 @@ describe('isStandardSyntaxDeclaration', () => {
 	it('less map', () => {
 		expect(isStandardSyntaxDeclaration(lessDecl('@map: { key: value; }'))).toBe(false);
 	});
+	it('scss map declaration', () => {
+		expect(isStandardSyntaxDeclaration(scssDecl('$foo: (key: value, key2: value2)'))).toBe(false);
+	});
+	it('scss map declaration with quotes', () => {
+		expect(isStandardSyntaxDeclaration(scssDecl('$foo: ("key": value, "key2": value2)'))).toBe(
+			false,
+		);
+	});
+	it('scss list declaration', () => {
+		expect(isStandardSyntaxDeclaration(scssDecl('$foo: (value, value2)'))).toBe(false);
+	});
 });
 
 function decl(css, parser = postcss) {

diff --git a/lib/utils/isMap.js b/lib/utils/isMap.js
diff --git a/lib/utils/isStandardSyntaxDeclaration.js b/lib/utils/isStandardSyntaxDeclaration.js
@@ -18,6 +18,7 @@ function isStandardSyntaxLang(lang) {
 module.exports = function (decl) {
 	const prop = decl.prop;
 	const parent = decl.parent;
+	const value = decl.value;
 
 	// Declarations belong in a declaration block or standard CSS source
 	if (
@@ -36,6 +37,11 @@ module.exports = function (decl) {
 		return false;
 	}
 
+	// SCSS map and list declarations
+	if (value.startsWith('(') && value.endsWith(')')) {
+		return false;
+	}
+
 	// Less var (e.g. @var: x), but exclude variable interpolation (e.g. @{var})
 	if (prop[0] === '@' && prop[1] !== '{') {
 		return false;