From 5ec208f646753096d8d1cd239004c9f73fa598e4 Mon Sep 17 00:00:00 2001 From: Lachlan Heywood Date: Tue, 19 Oct 2021 12:47:10 -0400 Subject: [PATCH 01/10] remove style-search to fix #5517 --- .../__tests__/index.js | 13 + .../index.js | 478 +++++++++++------- 2 files changed, 319 insertions(+), 172 deletions(-) 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 2bf9083171..7aa63ae226 100644 --- a/lib/rules/function-calc-no-unspaced-operator/__tests__/index.js +++ b/lib/rules/function-calc-no-unspaced-operator/__tests__/index.js @@ -87,6 +87,10 @@ testRule({ code: 'a { top: calc(\t+1px)}', description: 'tab before sign at start', }, + { + code: 'a { top: calc(100% - --my-custom-function(1rem)); }', + description: 'custom function with hyphens in name', + }, { code: 'a { top: calc(-$x - 2rem); }', description: 'postcss-simple-vars and SCSS variable syntax', @@ -154,6 +158,15 @@ testRule({ ], reject: [ + { + code: 'a { top: calc(2px+1px) }', + fixed: 'a { top: calc(2px + 1px) }', + description: 'no space before or after operator', + warnings: [ + { message: messages.expectedBefore('+'), line: 1, column: 18 }, + { message: messages.expectedAfter('+'), line: 1, column: 19 }, + ], + }, { code: 'a { top: calc(1px +\t-1px)}', fixed: 'a { top: calc(1px + -1px)}', diff --git a/lib/rules/function-calc-no-unspaced-operator/index.js b/lib/rules/function-calc-no-unspaced-operator/index.js index 5a7323db35..7409fb2e71 100644 --- a/lib/rules/function-calc-no-unspaced-operator/index.js +++ b/lib/rules/function-calc-no-unspaced-operator/index.js @@ -1,12 +1,13 @@ 'use strict'; -const balancedMatch = require('balanced-match'); -const isWhitespace = require('../../utils/isWhitespace'); +const valueParser = require('postcss-value-parser'); + +const declarationValueIndex = require('../../utils/declarationValueIndex'); +const getDeclarationValue = require('../../utils/getDeclarationValue'); const report = require('../../utils/report'); const ruleMessages = require('../../utils/ruleMessages'); -const styleSearch = require('style-search'); +const setDeclarationValue = require('../../utils/setDeclarationValue'); const validateOptions = require('../../utils/validateOptions'); -const valueParser = require('postcss-value-parser'); const ruleName = 'function-calc-no-unspaced-operator'; @@ -16,16 +17,12 @@ const messages = ruleMessages(ruleName, { expectedOperatorBeforeSign: (operator) => `Expected an operator before sign "${operator}"`, }); -/** @typedef {{ index: number, insert: boolean }} SymbolToFix */ - /** @type {import('stylelint').Rule} */ -const rule = (primary, _secondaryOptions, context) => { +const rule = (primary, secondaryOptions, context) => { return (root, result) => { const validOptions = validateOptions(result, ruleName, { actual: primary }); - if (!validOptions) { - return; - } + if (!validOptions) return; /** * @param {string} message @@ -37,227 +34,364 @@ const rule = (primary, _secondaryOptions, context) => { } root.walkDecls((decl) => { - /** @type {SymbolToFix[]} */ - const symbolsToFix = []; + let needsFix = false; + const expressionIndex = declarationValueIndex(decl); + const parsedValue = valueParser(getDeclarationValue(decl)); + + /** + * @param {import('postcss-value-parser').Node[]} nodes + * @param {number} operatorIndex + * @param {-1 | 1} direction + */ + function checkAroundOperator(nodes, operatorIndex, direction) { + const isBeforeOp = direction === -1; + const currentNode = nodes[operatorIndex + direction]; + const operator = nodes[operatorIndex].value; + const operatorSourceIndex = nodes[operatorIndex].sourceIndex; + + if (currentNode && !isSingleSpace(currentNode)) { + if (currentNode.type === 'word') { + if (isBeforeOp) { + const lastChar = currentNode.value.slice(-1); + + if (isOperarorChar(lastChar)) { + if (context.fix) { + currentNode.value = `${currentNode.value.slice(0, -1)} ${lastChar}`; + + return true; + } + + complain(messages.expectedOperatorBeforeSign(operator), decl, operatorSourceIndex); + + return true; + } + } else { + const firstChar = currentNode.value.slice(0, 1); + + if (isOperarorChar(firstChar)) { + if (context.fix) { + currentNode.value = `${firstChar} ${currentNode.value.slice(1)}`; + + return true; + } + + complain(messages.expectedAfter(operator), decl, operatorSourceIndex); + + return true; + } + } + + if (context.fix) { + needsFix = true; + currentNode.value = isBeforeOp ? `${currentNode.value} ` : ` ${currentNode.value}`; + + return true; + } + + complain( + isBeforeOp ? messages.expectedBefore(operator) : messages.expectedAfter(operator), + decl, + expressionIndex + operatorSourceIndex, + ); + + return true; + } + + if (currentNode.type === 'space') { + if (startsWithNewLine(currentNode) || startsWithCRLF(currentNode)) return; - valueParser(decl.value).walk((node) => { - if (node.type !== 'function' || node.value.toLowerCase() !== 'calc') { - return; + if (context.fix) { + needsFix = true; + + const indexOfFirstNewLine = currentNode.value.search(/(\n|\r\n)/); + + currentNode.value = + indexOfFirstNewLine === -1 ? ' ' : currentNode.value.slice(indexOfFirstNewLine); + + return true; + } + + const message = isBeforeOp + ? messages.expectedBefore(operator) + : messages.expectedAfter(operator); + + complain(message, decl, expressionIndex + operatorSourceIndex); + + return true; + } + + if (currentNode.type === 'function') { + if (context.fix) { + needsFix = true; + nodes.splice(operatorIndex, 0, { type: 'space', value: ' ', sourceIndex: 0 }); + + return true; + } + + const message = isBeforeOp + ? messages.expectedBefore(operator) + : messages.expectedAfter(operator); + + complain(message, decl, expressionIndex + operatorSourceIndex); + + return true; + } } - const nodeText = valueParser.stringify(node); - const parensMatch = balancedMatch('(', ')', nodeText); + return false; + } - if (!parensMatch) { - throw new Error(`No parens match: "${nodeText}"`); + /** + * @param {import('postcss-value-parser').Node[]} nodes + */ + function checkForOperatorInFirstNode(nodes) { + const firstNode = nodes[0]; + + const operatorIndex = + (firstNode.type === 'word' || -1) && firstNode.value.search(/[+\-*/]/); + const operator = firstNode.value.slice(operatorIndex, operatorIndex + 1); + + if (operatorIndex <= 0) return false; + + const charBefore = firstNode.value.charAt(operatorIndex - 1); + const charAfter = firstNode.value.charAt(operatorIndex + 1); + + if (charBefore && charBefore !== ' ' && charAfter && charAfter !== ' ') { + if (context.fix) { + needsFix = true; + firstNode.value = firstNode.value.replace(/[+\-*/]/, ' $& '); + } else { + complain( + messages.expectedBefore(operator), + decl, + expressionIndex + firstNode.sourceIndex + operatorIndex, + ); + complain( + messages.expectedAfter(operator), + decl, + expressionIndex + firstNode.sourceIndex + operatorIndex + 1, + ); + } + } else if (charBefore && charBefore !== ' ') { + if (context.fix) { + needsFix = true; + firstNode.value = firstNode.value.replace(/[+\-*/]/, ' $&'); + } else { + complain( + messages.expectedBefore(operator), + decl, + expressionIndex + firstNode.sourceIndex + operatorIndex, + ); + } + } else if (charAfter && charAfter !== ' ') { + if (context.fix) { + needsFix = true; + firstNode.value = firstNode.value.replace(/[+\-*/]/, '$& '); + } else { + complain( + messages.expectedAfter(operator), + decl, + expressionIndex + firstNode.sourceIndex + operatorIndex + 1, + ); + } } - if (decl.source == null || decl.source.start == null) { - throw new Error('Declaration source must be present'); + return true; + } + + /** + * @param {import('postcss-value-parser').Node[]} nodes + */ + function checkForOperatorInLastNode(nodes) { + if (nodes.length === 1) return false; + + const lastNode = nodes[nodes.length - 1]; + + const operatorIndex = (lastNode.type === 'word' || -1) && lastNode.value.search(/[*/\-+]/); + + if (lastNode.value[operatorIndex - 1] === ' ') return false; + + if (context.fix) { + needsFix = true; + lastNode.value = lastNode.value.replace(/[*/\-+]/, ' $& ').trim(); + + return true; } - const rawExpression = parensMatch.body; - const expressionIndex = - decl.source.start.column + - decl.prop.length + - (decl.raws.between || '').length + - node.sourceIndex; - const expression = blurVariables(rawExpression); - - const parensMatchStart = parensMatch.start; - - checkSymbol('+'); - checkSymbol('-'); - checkSymbol('*'); - checkSymbol('/'); - - /** - * @param {string} symbol - */ - function checkSymbol(symbol) { - /** @type {import('style-search').Options} */ - const styleSearchOptions = { - source: expression, - target: symbol, - functionArguments: 'skip', - }; - - styleSearch(styleSearchOptions, (match) => { - const index = match.startIndex; - const symbolIndex = node.sourceIndex + parensMatchStart + index + 1; - - // Deal with signs. - // (@ and $ are considered "digits" here to allow for variable syntaxes - // that permit signs in front of variables, e.g. `-$number`) - // As is "." to deal with fractional numbers without a leading zero - if ((symbol === '+' || symbol === '-') && /[\d@$.]/.test(expression[index + 1])) { - const expressionBeforeSign = expression.slice(0, index); - - // Ignore signs that directly follow a opening bracket - if (expressionBeforeSign[expressionBeforeSign.length - 1] === '(') { - return; - } + complain( + messages.expectedOperatorBeforeSign(lastNode.value[operatorIndex]), + decl, + expressionIndex + lastNode.sourceIndex + operatorIndex, + ); - // Ignore signs at the beginning of the expression - if (/^\s*$/.test(expressionBeforeSign)) { - return; - } + return true; + } + + /** + * @param {import('postcss-value-parser').Node[]} nodes + */ + function checkWords(nodes) { + if (checkForOperatorInFirstNode(nodes)) return; + + if (checkForOperatorInLastNode(nodes)) return; + + nodes.forEach((node, index) => { + const lastChar = node.value.slice(-1); + const firstChar = node.value.slice(0, 1); + + if (isWord(node)) { + if (index === 0 && ['*', '/', '+', '-'].includes(lastChar)) { + if (context.fix) { + node.value = `${node.value.slice(0, -1)} ${lastChar}`; - // Otherwise, ensure that there is a real operator preceding them - if (/[*/+-]\s*$/.test(expressionBeforeSign)) { return; } - if (!context.fix) { - // And if not, complain - complain( - messages.expectedOperatorBeforeSign(symbol), - decl, - expressionIndex + index, - ); + complain(messages.expectedBefore(lastChar), decl, node.sourceIndex); + } else if (index === nodes.length && ['*', '/', '+', '-'].includes(firstChar)) { + if (context.fix) { + node.value = `${firstChar} ${node.value.slice(1)}`; return; } + + complain(messages.expectedOperatorBeforeSign(firstChar), decl, node.sourceIndex); } + } + }); + } - const beforeOk = - (expression[index - 1] === ' ' && !isWhitespace(expression[index - 2])) || - newlineBefore(expression, index - 1); + parsedValue.walk((node) => { + if (!isCalcFunction(node)) return; - if (!beforeOk) { - if (context.fix) { - let step = 1; + node.nodes = fixLessVariablesWithSymbols(node.nodes); - // Remove all whitespace characters before the operator, e.g. `\t` - while (isWhitespace(expression[index - step])) { - symbolsToFix.push({ - index: symbolIndex - step, - insert: false, - }); + /** @type {number[]} */ + const operatorNodes = []; - step++; - } + node.nodes.forEach((opNode, index) => isOperator(opNode) && operatorNodes.push(index)); - // Add only one space character - symbolsToFix.push({ - index: symbolIndex, - insert: true, - }); - } else { - complain(messages.expectedBefore(symbol), decl, expressionIndex + index); - } - } + operatorNodes.forEach((operatorNodeIndex) => { + const nodeBefore = node.nodes[operatorNodeIndex - 1]; + const nodeAfter = node.nodes[operatorNodeIndex + 1]; - const afterOk = - (expression[index + 1] === ' ' && !isWhitespace(expression[index + 2])) || - isNewlineAtIndex(expression, index + 1); + if (isSingleSpace(nodeBefore) && isSingleSpace(nodeAfter)) return; - if (!afterOk) { - if (context.fix) { - let step = 1; - let spaceNeeded = true; - - // Remove all whitespace characters before the operator or \n, e.g. \t - while (isWhitespace(expression[index + step])) { - if (isNewlineAtIndex(expression, index + step)) { - spaceNeeded = false; - break; - } - - symbolsToFix.push({ - index: symbolIndex + step, - insert: false, - }); - step++; - } + if (checkAroundOperator(node.nodes, operatorNodeIndex, 1)) return; - // Insert one space character if there is no \n - if (spaceNeeded) { - symbolsToFix.push({ - index: symbolIndex + 1, - insert: true, - }); - } - } else { - complain(messages.expectedAfter(symbol), decl, expressionIndex + index); - } - } - }); + checkAroundOperator(node.nodes, operatorNodeIndex, -1); + }); + + if (operatorNodes.length === 0) { + checkWords(node.nodes); } }); - if (context.fix) { - decl.value = symbolsToFix.reduce((/** @type {string} */ fixedValue, { insert, index }) => { - shiftIndexes(symbolsToFix, index, insert); - - return insert - ? insertCharAtIndex(fixedValue, index, ' ') - : removeCharAtIndex(fixedValue, index); - }, decl.value); + if (needsFix) { + setDeclarationValue(decl, parsedValue.toString()); } }); }; }; /** - * @param {string} str - * @param {number} index + * + * @param {import('postcss-value-parser').Node[]} nodes */ -function isNewlineAtIndex(str, index) { - return str[index] === '\n' || str.slice(index, index + 2) === '\r\n'; +function fixLessVariablesWithSymbols(nodes) { + /** @type {import('postcss-value-parser').Node[]} */ + const fixedNodes = []; + let foundLessVariable = false; + + nodes.forEach((node) => { + if (isLessVariable(node)) { + foundLessVariable = true; + fixedNodes.push(node); + } else if (foundLessVariable) { + if (node.type === 'space') { + foundLessVariable = false; + fixedNodes.push(node); + } else { + fixedNodes[fixedNodes.length - 1].value += node.value; + } + } else { + fixedNodes.push(node); + } + }); + + return fixedNodes; } /** - * @param {SymbolToFix[]} symbolsToFix - * @param {number} index - * @param {boolean} insert + * @param {import('postcss-value-parser').Node} node + * @returns {node is import('postcss-value-parser').Node} */ -function shiftIndexes(symbolsToFix, index, insert) { - symbolsToFix.forEach((symbol) => { - if (symbol.index > index) { - symbol.index += insert ? 1 : -1; - } - }); +function isWord(node) { + return node && node.type === 'word'; } /** - * @param {string} str - * @param {number} index + * @param {import('postcss-value-parser').Node} node + * @returns {node is import('postcss-value-parser').WordNode & { value: '*' | '/' | '+' | '-' }} */ -function removeCharAtIndex(str, index) { - return str.slice(0, index) + str.slice(index + 1, str.length); +function isOperator(node) { + return node && node.type === 'word' && isOperarorChar(node.value); } /** - * @param {string} str - * @param {number} index * @param {string} char + * @returns {char is '*' | '/' | '+' | '-'} */ -function insertCharAtIndex(str, index, char) { - return str.slice(0, index) + char + str.slice(index, str.length); +function isOperarorChar(char) { + return ['*', '/', '+', '-'].includes(char); } /** - * @param {string} source + * @param {import('postcss-value-parser').Node} node + * @returns {node is import('postcss-value-parser').SpaceNode} */ -function blurVariables(source) { - return source.replace(/[$@][^)\s]+|#\{.+?\}/g, '0'); +function isSpace(node) { + return node && node.type === 'space'; } /** - * @param {string} str - * @param {number} startIndex + * @param {import('postcss-value-parser').Node} node + * @returns {boolean} `true` if node starts with new line */ -function newlineBefore(str, startIndex) { - let index = startIndex; +function isSingleSpace(node) { + return isSpace(node) && node.value === ' '; +} - while (index && isWhitespace(str[index])) { - if (str[index] === '\n') return true; +/** + * @param {import('postcss-value-parser').Node} node + * @returns {boolean} `true` if node starts with CRLF + */ +function startsWithCRLF(node) { + return isSpace(node) && node.value.startsWith('\r\n'); +} + +/** + * @param {import('postcss-value-parser').Node} node + * @returns {boolean} `true` if node starts with New Line + */ +function startsWithNewLine(node) { + return isSpace(node) && node.value.startsWith('\n'); +} - index--; - } +/** + * @param {import('postcss-value-parser').Node} node + * @returns {node is import('postcss-value-parser').FunctionNode} + */ +function isCalcFunction(node) { + return node && node.type === 'function' && node.value.toLowerCase() === 'calc'; +} - return false; +/** + * @param {import('postcss-value-parser').Node} node + * @returns {boolean} `true` if node looks like a less variable + */ +function isLessVariable(node) { + return node && node.type === 'word' && node.value.startsWith('@'); } rule.ruleName = ruleName; From 3ee06a9939db2f0c3916cf69966ee9da98dd2298 Mon Sep 17 00:00:00 2001 From: Lachlan Heywood Date: Tue, 19 Oct 2021 13:20:42 -0400 Subject: [PATCH 02/10] attempt to fix security issue with replace method --- .../index.js | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/lib/rules/function-calc-no-unspaced-operator/index.js b/lib/rules/function-calc-no-unspaced-operator/index.js index 7409fb2e71..fdec7c9bfa 100644 --- a/lib/rules/function-calc-no-unspaced-operator/index.js +++ b/lib/rules/function-calc-no-unspaced-operator/index.js @@ -148,7 +148,7 @@ const rule = (primary, secondaryOptions, context) => { const firstNode = nodes[0]; const operatorIndex = - (firstNode.type === 'word' || -1) && firstNode.value.search(/[+\-*/]/); + (firstNode.type === 'word' || -1) && findOperatorIndex(firstNode.value); const operator = firstNode.value.slice(operatorIndex, operatorIndex + 1); if (operatorIndex <= 0) return false; @@ -159,7 +159,8 @@ const rule = (primary, secondaryOptions, context) => { if (charBefore && charBefore !== ' ' && charAfter && charAfter !== ' ') { if (context.fix) { needsFix = true; - firstNode.value = firstNode.value.replace(/[+\-*/]/, ' $& '); + firstNode.value = insertCharAtIndex(firstNode.value, operatorIndex + 1, ' '); + firstNode.value = insertCharAtIndex(firstNode.value, operatorIndex, ' '); } else { complain( messages.expectedBefore(operator), @@ -175,7 +176,7 @@ const rule = (primary, secondaryOptions, context) => { } else if (charBefore && charBefore !== ' ') { if (context.fix) { needsFix = true; - firstNode.value = firstNode.value.replace(/[+\-*/]/, ' $&'); + firstNode.value = insertCharAtIndex(firstNode.value, operatorIndex, ' '); } else { complain( messages.expectedBefore(operator), @@ -186,7 +187,7 @@ const rule = (primary, secondaryOptions, context) => { } else if (charAfter && charAfter !== ' ') { if (context.fix) { needsFix = true; - firstNode.value = firstNode.value.replace(/[+\-*/]/, '$& '); + firstNode.value = insertCharAtIndex(firstNode.value, operatorIndex, ' '); } else { complain( messages.expectedAfter(operator), @@ -322,6 +323,22 @@ function fixLessVariablesWithSymbols(nodes) { return fixedNodes; } +/** + * @param {string} str + * @param {number} index + * @param {string} char + */ +function insertCharAtIndex(str, index, char) { + return str.slice(0, index) + char + str.slice(index, str.length); +} + +/** + * @param {string} string + */ +function findOperatorIndex(string) { + return string.search(/[*/\-+]/); +} + /** * @param {import('postcss-value-parser').Node} node * @returns {node is import('postcss-value-parser').Node} From 0146cf291c676a1b1fff8a83a4bcd122862256cd Mon Sep 17 00:00:00 2001 From: Lachlan Heywood Date: Tue, 19 Oct 2021 13:47:37 -0400 Subject: [PATCH 03/10] remove additional `.replace` method --- lib/rules/function-calc-no-unspaced-operator/index.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/rules/function-calc-no-unspaced-operator/index.js b/lib/rules/function-calc-no-unspaced-operator/index.js index fdec7c9bfa..df2dba3a6b 100644 --- a/lib/rules/function-calc-no-unspaced-operator/index.js +++ b/lib/rules/function-calc-no-unspaced-operator/index.js @@ -214,7 +214,8 @@ const rule = (primary, secondaryOptions, context) => { if (context.fix) { needsFix = true; - lastNode.value = lastNode.value.replace(/[*/\-+]/, ' $& ').trim(); + lastNode.value = insertCharAtIndex(lastNode.value, operatorIndex + 1, ' ').trim(); + lastNode.value = insertCharAtIndex(lastNode.value, operatorIndex, ' ').trim(); return true; } From 89ef9e0bfb6ac790537b69fd7d9dfbd912bb11d4 Mon Sep 17 00:00:00 2001 From: Lachlan Heywood Date: Fri, 22 Oct 2021 12:09:04 -0400 Subject: [PATCH 04/10] remove invalid less variable test and related code --- .../__tests__/index.js | 2 +- .../index.js | 38 ------------------- 2 files changed, 1 insertion(+), 39 deletions(-) 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 7aa63ae226..057b558db7 100644 --- a/lib/rules/function-calc-no-unspaced-operator/__tests__/index.js +++ b/lib/rules/function-calc-no-unspaced-operator/__tests__/index.js @@ -104,7 +104,7 @@ testRule({ description: 'postcss-simple-vars and SCSS variable with hyphens', }, { - code: 'a { top: calc(2rem + @fh+d*sf-as); }', + code: 'a { top: calc(2rem + @fh+dsf-as); }', description: 'Less variable with symbols', }, { diff --git a/lib/rules/function-calc-no-unspaced-operator/index.js b/lib/rules/function-calc-no-unspaced-operator/index.js index df2dba3a6b..5bea44300b 100644 --- a/lib/rules/function-calc-no-unspaced-operator/index.js +++ b/lib/rules/function-calc-no-unspaced-operator/index.js @@ -266,8 +266,6 @@ const rule = (primary, secondaryOptions, context) => { parsedValue.walk((node) => { if (!isCalcFunction(node)) return; - node.nodes = fixLessVariablesWithSymbols(node.nodes); - /** @type {number[]} */ const operatorNodes = []; @@ -296,34 +294,6 @@ const rule = (primary, secondaryOptions, context) => { }; }; -/** - * - * @param {import('postcss-value-parser').Node[]} nodes - */ -function fixLessVariablesWithSymbols(nodes) { - /** @type {import('postcss-value-parser').Node[]} */ - const fixedNodes = []; - let foundLessVariable = false; - - nodes.forEach((node) => { - if (isLessVariable(node)) { - foundLessVariable = true; - fixedNodes.push(node); - } else if (foundLessVariable) { - if (node.type === 'space') { - foundLessVariable = false; - fixedNodes.push(node); - } else { - fixedNodes[fixedNodes.length - 1].value += node.value; - } - } else { - fixedNodes.push(node); - } - }); - - return fixedNodes; -} - /** * @param {string} str * @param {number} index @@ -404,14 +374,6 @@ function isCalcFunction(node) { return node && node.type === 'function' && node.value.toLowerCase() === 'calc'; } -/** - * @param {import('postcss-value-parser').Node} node - * @returns {boolean} `true` if node looks like a less variable - */ -function isLessVariable(node) { - return node && node.type === 'word' && node.value.startsWith('@'); -} - rule.ruleName = ruleName; rule.messages = messages; module.exports = rule; From c05dfb85dd5984de875a431db683ba5d23455b66 Mon Sep 17 00:00:00 2001 From: Lachlan Heywood Date: Sun, 24 Oct 2021 15:32:43 -0400 Subject: [PATCH 05/10] Update lib/rules/function-calc-no-unspaced-operator/index.js Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com> --- lib/rules/function-calc-no-unspaced-operator/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/function-calc-no-unspaced-operator/index.js b/lib/rules/function-calc-no-unspaced-operator/index.js index 5bea44300b..d93da1f227 100644 --- a/lib/rules/function-calc-no-unspaced-operator/index.js +++ b/lib/rules/function-calc-no-unspaced-operator/index.js @@ -98,7 +98,7 @@ const rule = (primary, secondaryOptions, context) => { } if (currentNode.type === 'space') { - if (startsWithNewLine(currentNode) || startsWithCRLF(currentNode)) return; + if (/^(\n|\r\n)/.test(currentNode.value)) return; if (context.fix) { needsFix = true; From a7c2b05acf104b7408714228c0bfa79d140430be Mon Sep 17 00:00:00 2001 From: Lachlan Heywood Date: Sun, 24 Oct 2021 16:30:05 -0400 Subject: [PATCH 06/10] implement changes from review --- .../index.js | 63 +++++++------------ 1 file changed, 22 insertions(+), 41 deletions(-) diff --git a/lib/rules/function-calc-no-unspaced-operator/index.js b/lib/rules/function-calc-no-unspaced-operator/index.js index d93da1f227..dcbb38b46a 100644 --- a/lib/rules/function-calc-no-unspaced-operator/index.js +++ b/lib/rules/function-calc-no-unspaced-operator/index.js @@ -17,8 +17,11 @@ const messages = ruleMessages(ruleName, { expectedOperatorBeforeSign: (operator) => `Expected an operator before sign "${operator}"`, }); +const OPERATORS = new Set(['*', '/', '+', '-']); +const OPERATOR_REGEX = /[*/+-]/; + /** @type {import('stylelint').Rule} */ -const rule = (primary, secondaryOptions, context) => { +const rule = (primary, _secondaryOptions, context) => { return (root, result) => { const validOptions = validateOptions(result, ruleName, { actual: primary }); @@ -54,7 +57,7 @@ const rule = (primary, secondaryOptions, context) => { if (isBeforeOp) { const lastChar = currentNode.value.slice(-1); - if (isOperarorChar(lastChar)) { + if (OPERATORS.has(lastChar)) { if (context.fix) { currentNode.value = `${currentNode.value.slice(0, -1)} ${lastChar}`; @@ -68,7 +71,7 @@ const rule = (primary, secondaryOptions, context) => { } else { const firstChar = currentNode.value.slice(0, 1); - if (isOperarorChar(firstChar)) { + if (OPERATORS.has(firstChar)) { if (context.fix) { currentNode.value = `${firstChar} ${currentNode.value.slice(1)}`; @@ -98,13 +101,13 @@ const rule = (primary, secondaryOptions, context) => { } if (currentNode.type === 'space') { - if (/^(\n|\r\n)/.test(currentNode.value)) return; + const indexOfFirstNewLine = currentNode.value.search(/(\n|\r\n)/); + + if (indexOfFirstNewLine === 0) return; if (context.fix) { needsFix = true; - const indexOfFirstNewLine = currentNode.value.search(/(\n|\r\n)/); - currentNode.value = indexOfFirstNewLine === -1 ? ' ' : currentNode.value.slice(indexOfFirstNewLine); @@ -208,7 +211,8 @@ const rule = (primary, secondaryOptions, context) => { const lastNode = nodes[nodes.length - 1]; - const operatorIndex = (lastNode.type === 'word' || -1) && lastNode.value.search(/[*/\-+]/); + const operatorIndex = + (lastNode.type === 'word' || -1) && lastNode.value.search(OPERATOR_REGEX); if (lastNode.value[operatorIndex - 1] === ' ') return false; @@ -242,7 +246,7 @@ const rule = (primary, secondaryOptions, context) => { const firstChar = node.value.slice(0, 1); if (isWord(node)) { - if (index === 0 && ['*', '/', '+', '-'].includes(lastChar)) { + if (index === 0 && OPERATORS.has(lastChar)) { if (context.fix) { node.value = `${node.value.slice(0, -1)} ${lastChar}`; @@ -250,7 +254,7 @@ const rule = (primary, secondaryOptions, context) => { } complain(messages.expectedBefore(lastChar), decl, node.sourceIndex); - } else if (index === nodes.length && ['*', '/', '+', '-'].includes(firstChar)) { + } else if (index === nodes.length && OPERATORS.has(firstChar)) { if (context.fix) { node.value = `${firstChar} ${node.value.slice(1)}`; @@ -266,12 +270,13 @@ const rule = (primary, secondaryOptions, context) => { parsedValue.walk((node) => { if (!isCalcFunction(node)) return; - /** @type {number[]} */ - const operatorNodes = []; + let foundOperatorNode = false; + + for (const [operatorNodeIndex, operatorNode] of node.nodes.entries()) { + if (!isOperator(operatorNode)) continue; - node.nodes.forEach((opNode, index) => isOperator(opNode) && operatorNodes.push(index)); + foundOperatorNode = true; - operatorNodes.forEach((operatorNodeIndex) => { const nodeBefore = node.nodes[operatorNodeIndex - 1]; const nodeAfter = node.nodes[operatorNodeIndex + 1]; @@ -280,9 +285,9 @@ const rule = (primary, secondaryOptions, context) => { if (checkAroundOperator(node.nodes, operatorNodeIndex, 1)) return; checkAroundOperator(node.nodes, operatorNodeIndex, -1); - }); + } - if (operatorNodes.length === 0) { + if (!foundOperatorNode) { checkWords(node.nodes); } }); @@ -307,7 +312,7 @@ function insertCharAtIndex(str, index, char) { * @param {string} string */ function findOperatorIndex(string) { - return string.search(/[*/\-+]/); + return string.search(OPERATOR_REGEX); } /** @@ -323,15 +328,7 @@ function isWord(node) { * @returns {node is import('postcss-value-parser').WordNode & { value: '*' | '/' | '+' | '-' }} */ function isOperator(node) { - return node && node.type === 'word' && isOperarorChar(node.value); -} - -/** - * @param {string} char - * @returns {char is '*' | '/' | '+' | '-'} - */ -function isOperarorChar(char) { - return ['*', '/', '+', '-'].includes(char); + return node && node.type === 'word' && OPERATORS.has(node.value); } /** @@ -350,22 +347,6 @@ function isSingleSpace(node) { return isSpace(node) && node.value === ' '; } -/** - * @param {import('postcss-value-parser').Node} node - * @returns {boolean} `true` if node starts with CRLF - */ -function startsWithCRLF(node) { - return isSpace(node) && node.value.startsWith('\r\n'); -} - -/** - * @param {import('postcss-value-parser').Node} node - * @returns {boolean} `true` if node starts with New Line - */ -function startsWithNewLine(node) { - return isSpace(node) && node.value.startsWith('\n'); -} - /** * @param {import('postcss-value-parser').Node} node * @returns {node is import('postcss-value-parser').FunctionNode} From 60adbfba322dc545955ba98008b1916e6c263b97 Mon Sep 17 00:00:00 2001 From: Lachlan Heywood Date: Mon, 25 Oct 2021 11:36:10 -0400 Subject: [PATCH 07/10] inline functions and implement suggestions --- .../index.js | 51 ++++--------------- 1 file changed, 10 insertions(+), 41 deletions(-) diff --git a/lib/rules/function-calc-no-unspaced-operator/index.js b/lib/rules/function-calc-no-unspaced-operator/index.js index dcbb38b46a..6589a0f0af 100644 --- a/lib/rules/function-calc-no-unspaced-operator/index.js +++ b/lib/rules/function-calc-no-unspaced-operator/index.js @@ -151,7 +151,7 @@ const rule = (primary, _secondaryOptions, context) => { const firstNode = nodes[0]; const operatorIndex = - (firstNode.type === 'word' || -1) && findOperatorIndex(firstNode.value); + (firstNode.type === 'word' || -1) && firstNode.value.search(OPERATOR_REGEX); const operator = firstNode.value.slice(operatorIndex, operatorIndex + 1); if (operatorIndex <= 0) return false; @@ -245,7 +245,7 @@ const rule = (primary, _secondaryOptions, context) => { const lastChar = node.value.slice(-1); const firstChar = node.value.slice(0, 1); - if (isWord(node)) { + if (node.type === 'word') { if (index === 0 && OPERATORS.has(lastChar)) { if (context.fix) { node.value = `${node.value.slice(0, -1)} ${lastChar}`; @@ -272,19 +272,19 @@ const rule = (primary, _secondaryOptions, context) => { let foundOperatorNode = false; - for (const [operatorNodeIndex, operatorNode] of node.nodes.entries()) { - if (!isOperator(operatorNode)) continue; + for (const [nodeIndex, currNode] of node.nodes.entries()) { + if (currNode.type !== 'word' || !OPERATORS.has(currNode.value)) continue; foundOperatorNode = true; - const nodeBefore = node.nodes[operatorNodeIndex - 1]; - const nodeAfter = node.nodes[operatorNodeIndex + 1]; + const nodeBefore = node.nodes[nodeIndex - 1]; + const nodeAfter = node.nodes[nodeIndex + 1]; - if (isSingleSpace(nodeBefore) && isSingleSpace(nodeAfter)) return; + if (isSingleSpace(nodeBefore) && isSingleSpace(nodeAfter)) continue; - if (checkAroundOperator(node.nodes, operatorNodeIndex, 1)) return; + if (checkAroundOperator(node.nodes, nodeIndex, 1)) continue; - checkAroundOperator(node.nodes, operatorNodeIndex, -1); + checkAroundOperator(node.nodes, nodeIndex, -1); } if (!foundOperatorNode) { @@ -308,43 +308,12 @@ function insertCharAtIndex(str, index, char) { return str.slice(0, index) + char + str.slice(index, str.length); } -/** - * @param {string} string - */ -function findOperatorIndex(string) { - return string.search(OPERATOR_REGEX); -} - -/** - * @param {import('postcss-value-parser').Node} node - * @returns {node is import('postcss-value-parser').Node} - */ -function isWord(node) { - return node && node.type === 'word'; -} - -/** - * @param {import('postcss-value-parser').Node} node - * @returns {node is import('postcss-value-parser').WordNode & { value: '*' | '/' | '+' | '-' }} - */ -function isOperator(node) { - return node && node.type === 'word' && OPERATORS.has(node.value); -} - -/** - * @param {import('postcss-value-parser').Node} node - * @returns {node is import('postcss-value-parser').SpaceNode} - */ -function isSpace(node) { - return node && node.type === 'space'; -} - /** * @param {import('postcss-value-parser').Node} node * @returns {boolean} `true` if node starts with new line */ function isSingleSpace(node) { - return isSpace(node) && node.value === ' '; + return node && node.type === 'space' && node.value === ' '; } /** From 13740e605a1f93af336bd8da7cbbe26ceacaaf5e Mon Sep 17 00:00:00 2001 From: Lachlan Heywood Date: Mon, 25 Oct 2021 12:00:45 -0400 Subject: [PATCH 08/10] inline one more function --- lib/rules/function-calc-no-unspaced-operator/index.js | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/lib/rules/function-calc-no-unspaced-operator/index.js b/lib/rules/function-calc-no-unspaced-operator/index.js index 6589a0f0af..73b244ef56 100644 --- a/lib/rules/function-calc-no-unspaced-operator/index.js +++ b/lib/rules/function-calc-no-unspaced-operator/index.js @@ -268,7 +268,7 @@ const rule = (primary, _secondaryOptions, context) => { } parsedValue.walk((node) => { - if (!isCalcFunction(node)) return; + if (node.type !== 'function' || node.value.toLowerCase() !== 'calc') return; let foundOperatorNode = false; @@ -316,14 +316,6 @@ function isSingleSpace(node) { return node && node.type === 'space' && node.value === ' '; } -/** - * @param {import('postcss-value-parser').Node} node - * @returns {node is import('postcss-value-parser').FunctionNode} - */ -function isCalcFunction(node) { - return node && node.type === 'function' && node.value.toLowerCase() === 'calc'; -} - rule.ruleName = ruleName; rule.messages = messages; module.exports = rule; From 0ed074fb706421299c2664101d2f0a079df92a74 Mon Sep 17 00:00:00 2001 From: Lachlan Heywood Date: Mon, 25 Oct 2021 12:53:39 -0400 Subject: [PATCH 09/10] Update return value of `isSingleSpace` function --- lib/rules/function-calc-no-unspaced-operator/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/function-calc-no-unspaced-operator/index.js b/lib/rules/function-calc-no-unspaced-operator/index.js index 73b244ef56..efbdd1e184 100644 --- a/lib/rules/function-calc-no-unspaced-operator/index.js +++ b/lib/rules/function-calc-no-unspaced-operator/index.js @@ -310,7 +310,7 @@ function insertCharAtIndex(str, index, char) { /** * @param {import('postcss-value-parser').Node} node - * @returns {boolean} `true` if node starts with new line + * @returns {node is import('postcss-value-parser').SpaceNode & { value: ' ' } } */ function isSingleSpace(node) { return node && node.type === 'space' && node.value === ' '; From a5f1208a4f36f73760c70c116c06ce90d6dd1e17 Mon Sep 17 00:00:00 2001 From: Lachlan Heywood Date: Mon, 25 Oct 2021 15:41:49 -0400 Subject: [PATCH 10/10] update variable naming --- .../index.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/rules/function-calc-no-unspaced-operator/index.js b/lib/rules/function-calc-no-unspaced-operator/index.js index efbdd1e184..d902154c8a 100644 --- a/lib/rules/function-calc-no-unspaced-operator/index.js +++ b/lib/rules/function-calc-no-unspaced-operator/index.js @@ -38,7 +38,7 @@ const rule = (primary, _secondaryOptions, context) => { root.walkDecls((decl) => { let needsFix = false; - const expressionIndex = declarationValueIndex(decl); + const valueIndex = declarationValueIndex(decl); const parsedValue = valueParser(getDeclarationValue(decl)); /** @@ -94,7 +94,7 @@ const rule = (primary, _secondaryOptions, context) => { complain( isBeforeOp ? messages.expectedBefore(operator) : messages.expectedAfter(operator), decl, - expressionIndex + operatorSourceIndex, + valueIndex + operatorSourceIndex, ); return true; @@ -118,7 +118,7 @@ const rule = (primary, _secondaryOptions, context) => { ? messages.expectedBefore(operator) : messages.expectedAfter(operator); - complain(message, decl, expressionIndex + operatorSourceIndex); + complain(message, decl, valueIndex + operatorSourceIndex); return true; } @@ -135,7 +135,7 @@ const rule = (primary, _secondaryOptions, context) => { ? messages.expectedBefore(operator) : messages.expectedAfter(operator); - complain(message, decl, expressionIndex + operatorSourceIndex); + complain(message, decl, valueIndex + operatorSourceIndex); return true; } @@ -168,12 +168,12 @@ const rule = (primary, _secondaryOptions, context) => { complain( messages.expectedBefore(operator), decl, - expressionIndex + firstNode.sourceIndex + operatorIndex, + valueIndex + firstNode.sourceIndex + operatorIndex, ); complain( messages.expectedAfter(operator), decl, - expressionIndex + firstNode.sourceIndex + operatorIndex + 1, + valueIndex + firstNode.sourceIndex + operatorIndex + 1, ); } } else if (charBefore && charBefore !== ' ') { @@ -184,7 +184,7 @@ const rule = (primary, _secondaryOptions, context) => { complain( messages.expectedBefore(operator), decl, - expressionIndex + firstNode.sourceIndex + operatorIndex, + valueIndex + firstNode.sourceIndex + operatorIndex, ); } } else if (charAfter && charAfter !== ' ') { @@ -195,7 +195,7 @@ const rule = (primary, _secondaryOptions, context) => { complain( messages.expectedAfter(operator), decl, - expressionIndex + firstNode.sourceIndex + operatorIndex + 1, + valueIndex + firstNode.sourceIndex + operatorIndex + 1, ); } } @@ -227,7 +227,7 @@ const rule = (primary, _secondaryOptions, context) => { complain( messages.expectedOperatorBeforeSign(lastNode.value[operatorIndex]), decl, - expressionIndex + lastNode.sourceIndex + operatorIndex, + valueIndex + lastNode.sourceIndex + operatorIndex, ); return true;