Skip to content

Commit

Permalink
Fix false positives and memory leak for `function-calc-no-unspaced-op…
Browse files Browse the repository at this point in the history 
…erator`
  • Loading branch information
@ybiquitous
ybiquitous committed Apr 29, 2022
1 parent 0364db1 commit 66b83a5
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 127 deletions.
7 changes: 7 additions & 0 deletions lib/rules/function-calc-no-unspaced-operator/README.md
View file
Expand Up @@ -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
Expand Down Expand Up @@ -41,6 +43,11 @@ a { top: calc(1px + 2px); }
a { top: calc(calc(1em * 2) / 3); }
```

<!-- prettier-ignore -->
```css
a { top: calc(calc(1em*2)/3); }
```

<!-- prettier-ignore -->
```css
a {
Expand Down
192 changes: 75 additions & 117 deletions lib/rules/function-calc-no-unspaced-operator/__tests__/index.js
View file
Expand Up @@ -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); }',
},
Expand Down Expand Up @@ -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: [
Expand Down Expand Up @@ -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); }',
Expand Down Expand Up @@ -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); }',
Expand Down Expand Up @@ -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}); }',
Expand All @@ -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}); }',
Expand Down
43 changes: 33 additions & 10 deletions lib/rules/function-calc-no-unspaced-operator/index.js
View file
Expand Up @@ -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) => {
Expand Down Expand Up @@ -173,8 +174,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;
Expand Down Expand Up @@ -239,10 +242,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;
Expand All @@ -252,9 +267,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,
Expand Down Expand Up @@ -308,7 +322,7 @@ const rule = (primary, _secondaryOptions, context) => {
let foundOperatorNode = false;

for (const [nodeIndex, currNode] of node.nodes.entries()) {
if (currNode.type !== 'word' || !OPERATORS.has(currNode.value)) continue;
if (!isOperator(currNode)) continue;

foundOperatorNode = true;

Expand Down Expand Up @@ -351,6 +365,15 @@ function isSingleSpace(node) {
return node != null && node.type === 'space' && node.value === ' ';
}

/**
* @param {import('postcss-value-parser').Node | undefined} node
* @param {Set<string>} [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;
Expand Down

0 comments on commit 66b83a5

Please sign in to comment.