From 73a702cefc5faa33d250f908c800b123b00497d1 Mon Sep 17 00:00:00 2001 From: Masafumi Koba <473530+ybiquitous@users.noreply.github.com> Date: Fri, 29 Apr 2022 15:31:18 +0900 Subject: [PATCH 1/2] Fix false positives and memory leak for `function-calc-no-unspaced-operator` --- .../README.md | 7 + .../__tests__/index.js | 192 +++++++----------- .../index.js | 81 ++++---- 3 files changed, 129 insertions(+), 151 deletions(-) diff --git a/lib/rules/function-calc-no-unspaced-operator/README.md b/lib/rules/function-calc-no-unspaced-operator/README.md index 4801bdf33d..9f5ce7d8b3 100644 --- a/lib/rules/function-calc-no-unspaced-operator/README.md +++ b/lib/rules/function-calc-no-unspaced-operator/README.md @@ -11,6 +11,8 @@ a { top: calc(1px + 2px); } Before the operator, there must be a single whitespace or a newline plus indentation. After the operator, there must be a single whitespace or a newline. +Note: The `*` and `/` operators do not require whitespace (but it is usually recommened for consistency). + The [`fix` option](../../../docs/user-guide/usage/options.md#fix) can automatically fix all of the problems reported by this rule. ## Options @@ -41,6 +43,11 @@ a { top: calc(1px + 2px); } a { top: calc(calc(1em * 2) / 3); } ``` + +```css +a { top: calc(calc(1em*2)/3); } +``` + ```css a { diff --git a/lib/rules/function-calc-no-unspaced-operator/__tests__/index.js b/lib/rules/function-calc-no-unspaced-operator/__tests__/index.js index 09b588ea1f..f4bc6f9ebf 100644 --- a/lib/rules/function-calc-no-unspaced-operator/__tests__/index.js +++ b/lib/rules/function-calc-no-unspaced-operator/__tests__/index.js @@ -32,9 +32,27 @@ testRule({ { code: 'a { top: calc(1px * 2); }', }, + { + code: 'a { top: calc(1px*2); }', + }, + { + code: 'a { top: calc(1px *2); }', + }, + { + code: 'a { top: calc(1px* 2); }', + }, { code: 'a { top: calc(1px / 2); }', }, + { + code: 'a { top: calc(1px/2); }', + }, + { + code: 'a { top: calc(1px /2); }', + }, + { + code: 'a { top: calc(1px/ 2); }', + }, { code: 'a { top: calc(1px * -0.2); }', }, @@ -153,6 +171,36 @@ testRule({ code: 'margin-top: calc(var(--some-variable)\r\n\t+ var(--some-other-variable));', description: 'CRLF newline and tab before operator', }, + { + code: 'a { padding: 10px calc(calc(1px + 2px)* 3px); }', + }, + { + code: 'a { padding: 10px calc(calc(1px* 2px) + 3px); }', + }, + { + code: 'a { padding: 10px calc(1px /2); }', + }, + { + code: 'a { padding: 10px calc(1px/ 2); }', + }, + { + code: 'a { padding: 10px calc(1px *2); }', + }, + { + code: 'a { padding: 10px calc(1px* 2); }', + }, + { + code: 'a { top: calc(calc(1px + 2px)* 3px); }', + }, + { + code: 'a { top: calc(calc(1px* 2px) + 3px); }', + }, + { + code: 'a { top: calc(10px*var(--foo)); }', + }, + { + code: 'a { top: calc(10px/var(--foo)); }', + }, ], reject: [ @@ -257,60 +305,6 @@ testRule({ endLine: 1, endColumn: 19, }, - { - code: 'a { top: calc(1px* 2); }', - fixed: 'a { top: calc(1px * 2); }', - message: messages.expectedBefore('*'), - line: 1, - column: 18, - endLine: 1, - endColumn: 19, - }, - { - code: 'a { top: calc(1px *2); }', - fixed: 'a { top: calc(1px * 2); }', - message: messages.expectedAfter('*'), - line: 1, - column: 19, - endLine: 1, - endColumn: 20, - }, - { - code: 'a { top: calc(1px/ 2); }', - fixed: 'a { top: calc(1px / 2); }', - message: messages.expectedBefore('/'), - line: 1, - column: 18, - endLine: 1, - endColumn: 19, - }, - { - code: 'a { top: calc(1px /2); }', - fixed: 'a { top: calc(1px / 2); }', - message: messages.expectedAfter('/'), - line: 1, - column: 19, - endLine: 1, - endColumn: 20, - }, - { - code: 'a { top: calc(calc(1px* 2px) + 3px); }', - fixed: 'a { top: calc(calc(1px * 2px) + 3px); }', - message: messages.expectedBefore('*'), - line: 1, - column: 23, - endLine: 1, - endColumn: 24, - }, - { - code: 'a { top: calc(calc(1px + 2px)* 3px); }', - fixed: 'a { top: calc(calc(1px + 2px) * 3px); }', - message: messages.expectedBefore('*'), - line: 1, - column: 30, - endLine: 1, - endColumn: 31, - }, { code: 'a { top: calc(1px +2px); }', fixed: 'a { top: calc(1px + 2px); }', @@ -403,60 +397,6 @@ testRule({ endLine: 1, endColumn: 28, }, - { - code: 'a { padding: 10px calc(1px* 2); }', - fixed: 'a { padding: 10px calc(1px * 2); }', - message: messages.expectedBefore('*'), - line: 1, - column: 27, - endLine: 1, - endColumn: 28, - }, - { - code: 'a { padding: 10px calc(1px *2); }', - fixed: 'a { padding: 10px calc(1px * 2); }', - message: messages.expectedAfter('*'), - line: 1, - column: 28, - endLine: 1, - endColumn: 29, - }, - { - code: 'a { padding: 10px calc(1px/ 2); }', - fixed: 'a { padding: 10px calc(1px / 2); }', - message: messages.expectedBefore('/'), - line: 1, - column: 27, - endLine: 1, - endColumn: 28, - }, - { - code: 'a { padding: 10px calc(1px /2); }', - fixed: 'a { padding: 10px calc(1px / 2); }', - message: messages.expectedAfter('/'), - line: 1, - column: 28, - endLine: 1, - endColumn: 29, - }, - { - code: 'a { padding: 10px calc(calc(1px* 2px) + 3px); }', - fixed: 'a { padding: 10px calc(calc(1px * 2px) + 3px); }', - message: messages.expectedBefore('*'), - line: 1, - column: 32, - endLine: 1, - endColumn: 33, - }, - { - code: 'a { padding: 10px calc(calc(1px + 2px)* 3px); }', - fixed: 'a { padding: 10px calc(calc(1px + 2px) * 3px); }', - message: messages.expectedBefore('*'), - line: 1, - column: 39, - endLine: 1, - endColumn: 40, - }, { code: 'a { padding: 10px calc(1px +2px); }', fixed: 'a { padding: 10px calc(1px + 2px); }', @@ -564,6 +504,33 @@ testRule({ customSyntax: 'postcss-scss', fix: true, + accept: [ + { + code: 'a { top: calc(100%*#{$foo}); }', + }, + { + code: 'a { top: calc(100% *#{$foo}); }', + }, + { + code: 'a { top: calc(100%* #{$foo}); }', + }, + { + code: 'a { top: calc(100% * #{$foo}); }', + }, + { + code: 'a { top: calc(100%/#{$foo}); }', + }, + { + code: 'a { top: calc(100% /#{$foo}); }', + }, + { + code: 'a { top: calc(100%/ #{$foo}); }', + }, + { + code: 'a { top: calc(100% / #{$foo}); }', + }, + ], + reject: [ { code: 'a { top: calc(100%- #{$foo}); }', @@ -574,15 +541,6 @@ testRule({ endLine: 1, endColumn: 20, }, - { - code: 'a { top: calc(100% *#{$foo}); }', - fixed: 'a { top: calc(100% * #{$foo}); }', - message: messages.expectedAfter('*'), - line: 1, - column: 20, - endLine: 1, - endColumn: 21, - }, { code: 'a { top: calc(100% -#{$foo}); }', fixed: 'a { top: calc(100% - #{$foo}); }', diff --git a/lib/rules/function-calc-no-unspaced-operator/index.js b/lib/rules/function-calc-no-unspaced-operator/index.js index 1d26096e50..0c4846bcc3 100644 --- a/lib/rules/function-calc-no-unspaced-operator/index.js +++ b/lib/rules/function-calc-no-unspaced-operator/index.js @@ -22,8 +22,9 @@ const meta = { url: 'https://stylelint.io/user-guide/rules/list/function-calc-no-unspaced-operator', }; -const OPERATORS = new Set(['*', '/', '+', '-']); -const OPERATOR_REGEX = /[*/+-]/; +const OPERATORS = new Set(['+', '-']); +const OPERATOR_REGEX = /[+-]/; +const ALL_OPERATORS = new Set([...OPERATORS, '*', '/']); /** @type {import('stylelint').Rule} */ const rule = (primary, _secondaryOptions, context) => { @@ -50,18 +51,13 @@ const rule = (primary, _secondaryOptions, context) => { const parsedValue = valueParser(getDeclarationValue(decl)); /** - * @param {import('postcss-value-parser').Node[]} nodes - * @param {number} operatorIndex - * @param {-1 | 1} direction + * @param {import('postcss-value-parser').Node} operatorNode + * @param {import('postcss-value-parser').Node} currentNode + * @param {boolean} isBeforeOp */ - function checkAroundOperator(nodes, operatorIndex, direction) { - const isBeforeOp = direction === -1; - const currentNode = nodes[operatorIndex + direction]; - const node = nodes[operatorIndex]; - - assert(node); - const operator = node.value; - const operatorSourceIndex = node.sourceIndex; + function checkAroundOperator(operatorNode, currentNode, isBeforeOp) { + const operator = operatorNode.value; + const operatorSourceIndex = operatorNode.sourceIndex; if (currentNode && !isSingleSpace(currentNode)) { if (currentNode.type === 'word') { @@ -124,7 +120,6 @@ const rule = (primary, _secondaryOptions, context) => { if (context.fix) { needsFix = true; - currentNode.value = indexOfFirstNewLine === -1 ? ' ' : currentNode.value.slice(indexOfFirstNewLine); @@ -143,12 +138,7 @@ const rule = (primary, _secondaryOptions, context) => { if (currentNode.type === 'function') { if (context.fix) { needsFix = true; - nodes.splice(operatorIndex, 0, { - type: 'space', - value: ' ', - sourceIndex: 0, - sourceEndIndex: 1, - }); + currentNode.value = isBeforeOp ? `${currentNode.value} ` : ` ${currentNode.value}`; return true; } @@ -173,8 +163,10 @@ const rule = (primary, _secondaryOptions, context) => { const firstNode = nodes[0]; assert(firstNode); - const operatorIndex = - (firstNode.type === 'word' || -1) && firstNode.value.search(OPERATOR_REGEX); + + if (firstNode.type !== 'word') return false; + + const operatorIndex = firstNode.value.search(OPERATOR_REGEX); const operator = firstNode.value.slice(operatorIndex, operatorIndex + 1); if (operatorIndex <= 0) return false; @@ -239,10 +231,22 @@ const rule = (primary, _secondaryOptions, context) => { const lastNode = nodes[nodes.length - 1]; assert(lastNode); - const operatorIndex = - (lastNode.type === 'word' || -1) && lastNode.value.search(OPERATOR_REGEX); - if (lastNode.value[operatorIndex - 1] === ' ') return false; + if (lastNode.type !== 'word') return false; + + const operatorIndex = lastNode.value.search(OPERATOR_REGEX); + + if (operatorIndex === -1) return false; + + if (lastNode.value.charAt(operatorIndex - 1) === ' ') return false; + + // E.g. "10px * -2" when the last node is "-2" + if ( + isOperator(nodes[nodes.length - 3], ALL_OPERATORS) && + isSingleSpace(nodes[nodes.length - 2]) + ) { + return false; + } if (context.fix) { needsFix = true; @@ -252,9 +256,8 @@ const rule = (primary, _secondaryOptions, context) => { return true; } - const operator = lastNode.value[operatorIndex]; + const operator = lastNode.value.charAt(operatorIndex); - assert(operator); complain( messages.expectedOperatorBeforeSign(operator), decl, @@ -305,25 +308,26 @@ const rule = (primary, _secondaryOptions, context) => { parsedValue.walk((node) => { if (node.type !== 'function' || node.value.toLowerCase() !== 'calc') return; + const { nodes } = node; let foundOperatorNode = false; - for (const [nodeIndex, currNode] of node.nodes.entries()) { - if (currNode.type !== 'word' || !OPERATORS.has(currNode.value)) continue; + for (const [nodeIndex, currNode] of nodes.entries()) { + if (!isOperator(currNode)) continue; foundOperatorNode = true; - const nodeBefore = node.nodes[nodeIndex - 1]; - const nodeAfter = node.nodes[nodeIndex + 1]; + const nodeBefore = nodes[nodeIndex - 1]; + const nodeAfter = nodes[nodeIndex + 1]; if (isSingleSpace(nodeBefore) && isSingleSpace(nodeAfter)) continue; - if (checkAroundOperator(node.nodes, nodeIndex, 1)) continue; + if (nodeAfter && checkAroundOperator(currNode, nodeAfter, false)) continue; - checkAroundOperator(node.nodes, nodeIndex, -1); + nodeBefore && checkAroundOperator(currNode, nodeBefore, true); } if (!foundOperatorNode) { - checkWords(node.nodes); + checkWords(nodes); } }); @@ -351,6 +355,15 @@ function isSingleSpace(node) { return node != null && node.type === 'space' && node.value === ' '; } +/** + * @param {import('postcss-value-parser').Node | undefined} node + * @param {Set} [operators] + * @returns {node is import('postcss-value-parser').WordNode} + */ +function isOperator(node, operators = OPERATORS) { + return node != null && node.type === 'word' && operators.has(node.value); +} + rule.ruleName = ruleName; rule.messages = messages; rule.meta = meta; From 5365d1faeef70f243deacde88f204bdc5c4c0f6c Mon Sep 17 00:00:00 2001 From: Richard Hallows Date: Fri, 29 Apr 2022 13:22:19 +0100 Subject: [PATCH 2/2] Update README.md --- lib/rules/function-calc-no-unspaced-operator/README.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/rules/function-calc-no-unspaced-operator/README.md b/lib/rules/function-calc-no-unspaced-operator/README.md index 9f5ce7d8b3..4ea5587f77 100644 --- a/lib/rules/function-calc-no-unspaced-operator/README.md +++ b/lib/rules/function-calc-no-unspaced-operator/README.md @@ -9,9 +9,7 @@ a { top: calc(1px + 2px); } * The space around this operator */ ``` -Before the operator, there must be a single whitespace or a newline plus indentation. After the operator, there must be a single whitespace or a newline. - -Note: The `*` and `/` operators do not require whitespace (but it is usually recommened for consistency). +This rule checks that there is a single whitespace or a newline plus indentation before the `+` or `-` operator, and a single whitespace or a newline after that operator. The [`fix` option](../../../docs/user-guide/usage/options.md#fix) can automatically fix all of the problems reported by this rule.