Skip to content

Commit

Permalink
apply suggestions from code review
Browse files Browse the repository at this point in the history
  • Loading branch information
romainmenke committed May 1, 2024
1 parent 3c5eb19 commit ae8b36e
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 42 deletions.
15 changes: 15 additions & 0 deletions lib/rules/function-calc-no-unspaced-operator/__tests__/index.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,11 @@ testRule({
},
{
code: 'a { padding: 10px calc([10px+5px]); }',
description: 'simple blocks with square brackets are ignored',
},
{
code: 'a { padding: 10px calc({10px+5px}); }',
description: 'simple blocks with curly braces are ignored',
},
{
code: 'a { top: calc(5--6px-7px); }',
Expand Down Expand Up @@ -621,6 +626,16 @@ testRule({
endLine: 1,
endColumn: 20,
},
{
code: 'a { top: calc(calc(1px+ 1px)); }',
fixed: 'a { top: calc(calc(1px + 1px)); }',
description: 'nested math expression',
message: messages.expectedBefore('+'),
line: 1,
column: 23,
endLine: 1,
endColumn: 24,
},
{
code: 'a { top: calc(((1px+ 1px))); }',
fixed: 'a { top: calc(((1px + 1px))); }',
Expand Down
51 changes: 30 additions & 21 deletions lib/rules/function-calc-no-unspaced-operator/index.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,17 @@ const meta = {

const OPERATORS = new Set(['+', '-']);
const OPERATOR_REGEX = /[+-]/;
// #7618
const alternatives = [...functions.mathFunctions].join('|');
const FUNC_NAMES_REGEX = new RegExp(`^(?:${alternatives})$`, 'i');
const FUNC_CALLS_REGEX = new RegExp(`(?:${alternatives})\\(`, 'i');

const NEWLINE_REGEX = /\n|\r\n/;

/**
* @typedef {import ('@csstools/css-parser-algorithms').ComponentValue} ComponentValue
* @typedef {import ('@csstools/css-parser-algorithms').ContainerNode} ContainerNode
*/

/** @type {import('stylelint').Rule} */
const rule = (primary, _secondaryOptions, context) => {
return (root, result) => {
Expand Down Expand Up @@ -76,7 +80,12 @@ const rule = (primary, _secondaryOptions, context) => {
// NOTE:
// This is a mess, I am fairly certain that it is correct, but too much is happening with strings.
// This will be brittle and might break when the css-tokenizer is updated.
// It will also produce invalid CSS for exotic units, nu such units exists today.

// Note:
// This will produce invalid CSS for exotic units (e.g. `1\\23`).
// No such units are specified today.
// Correctly serializing units would resolve any issues.

if (!token || token[0] !== cssTokenizer.TokenType.Dimension) continue;

if (token[4].unit.startsWith('--')) continue;
Expand Down Expand Up @@ -134,7 +143,7 @@ const rule = (primary, _secondaryOptions, context) => {
if (nodes.length === 0) return;

/**
* @param {import('@csstools/css-parser-algorithms').ContainerNode} node
* @param {ContainerNode} node
* @param {CompleteOperation} operation
*/
function checkCompleteOperation(node, operation) {
Expand Down Expand Up @@ -178,7 +187,7 @@ const rule = (primary, _secondaryOptions, context) => {
}

/**
* @param {import('@csstools/css-parser-algorithms').ContainerNode} node
* @param {ContainerNode} node
* @param {Operation} operation
*/
function checkOperationWithoutOperator(node, operation) {
Expand Down Expand Up @@ -374,7 +383,7 @@ const OPERAND_TOKEN_TYPES = new Set([
]);

/**
* @param {import('@csstools/css-parser-algorithms').ComponentValue | undefined} node
* @param {ComponentValue | undefined} node
* @returns {boolean}
*/
function isOperandNode(node) {
Expand All @@ -397,20 +406,20 @@ function isOperandNode(node) {
* NOTE: this is messy.
*
* @typedef {{
* firstOperand: import ('@csstools/css-parser-algorithms').ComponentValue,
* before: Array<import ('@csstools/css-parser-algorithms').ComponentValue>,
* secondOperand: import ('@csstools/css-parser-algorithms').ComponentValue,
* after: Array<import ('@csstools/css-parser-algorithms').ComponentValue>,
* operator: import ('@csstools/css-parser-algorithms').ComponentValue | undefined,
* firstOperand: ComponentValue,
* before: Array<ComponentValue>,
* secondOperand: ComponentValue,
* after: Array<ComponentValue>,
* operator: ComponentValue | undefined,
* operatorToken: import ('@csstools/css-tokenizer').TokenDelim | undefined,
* }} Operation
*
* @typedef {{
* firstOperand: import ('@csstools/css-parser-algorithms').ComponentValue,
* before: Array<import ('@csstools/css-parser-algorithms').ComponentValue>,
* secondOperand: import ('@csstools/css-parser-algorithms').ComponentValue,
* after: Array<import ('@csstools/css-parser-algorithms').ComponentValue>,
* operator: import ('@csstools/css-parser-algorithms').ComponentValue,
* firstOperand: ComponentValue,
* before: Array<ComponentValue>,
* secondOperand: ComponentValue,
* after: Array<ComponentValue>,
* operator: ComponentValue,
* operatorToken: import ('@csstools/css-tokenizer').TokenDelim,
* }} CompleteOperation
*/
Expand Down Expand Up @@ -442,20 +451,20 @@ function isOperationWithoutOperator(operation) {
}

/**
* @param {import('@csstools/css-parser-algorithms').ContainerNode} container
* @param {ContainerNode} container
* @param {number} cursor
* @returns {[Operation | undefined, number]}
*/
function parseOperation(container, cursor) {
/** @type {import ('@csstools/css-parser-algorithms').ComponentValue | undefined} */
/** @type {ComponentValue | undefined} */
let firstOperand = undefined;
/** @type {import ('@csstools/css-parser-algorithms').ComponentValue | undefined} */
/** @type {ComponentValue | undefined} */
let secondOperand = undefined;
/** @type {Array<import ('@csstools/css-parser-algorithms').ComponentValue>} */
/** @type {Array<ComponentValue>} */
const before = [];
/** @type {Array<import ('@csstools/css-parser-algorithms').ComponentValue>} */
/** @type {Array<ComponentValue>} */
const after = [];
/** @type {import ('@csstools/css-parser-algorithms').ComponentValue | undefined} */
/** @type {ComponentValue | undefined} */
let operator = undefined;
/** @type {import ('@csstools/css-tokenizer').TokenDelim | undefined} */
let operatorToken = undefined;
Expand Down
51 changes: 30 additions & 21 deletions lib/rules/function-calc-no-unspaced-operator/index.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,17 @@ const meta = {

const OPERATORS = new Set(['+', '-']);
const OPERATOR_REGEX = /[+-]/;
// #7618
const alternatives = [...mathFunctions].join('|');
const FUNC_NAMES_REGEX = new RegExp(`^(?:${alternatives})$`, 'i');
const FUNC_CALLS_REGEX = new RegExp(`(?:${alternatives})\\(`, 'i');

const NEWLINE_REGEX = /\n|\r\n/;

/**
* @typedef {import ('@csstools/css-parser-algorithms').ComponentValue} ComponentValue
* @typedef {import ('@csstools/css-parser-algorithms').ContainerNode} ContainerNode
*/

/** @type {import('stylelint').Rule} */
const rule = (primary, _secondaryOptions, context) => {
return (root, result) => {
Expand Down Expand Up @@ -84,7 +88,12 @@ const rule = (primary, _secondaryOptions, context) => {
// NOTE:
// This is a mess, I am fairly certain that it is correct, but too much is happening with strings.
// This will be brittle and might break when the css-tokenizer is updated.
// It will also produce invalid CSS for exotic units, nu such units exists today.

// Note:
// This will produce invalid CSS for exotic units (e.g. `1\\23`).
// No such units are specified today.
// Correctly serializing units would resolve any issues.

if (!token || token[0] !== TokenType.Dimension) continue;

if (token[4].unit.startsWith('--')) continue;
Expand Down Expand Up @@ -142,7 +151,7 @@ const rule = (primary, _secondaryOptions, context) => {
if (nodes.length === 0) return;

/**
* @param {import('@csstools/css-parser-algorithms').ContainerNode} node
* @param {ContainerNode} node
* @param {CompleteOperation} operation
*/
function checkCompleteOperation(node, operation) {
Expand Down Expand Up @@ -186,7 +195,7 @@ const rule = (primary, _secondaryOptions, context) => {
}

/**
* @param {import('@csstools/css-parser-algorithms').ContainerNode} node
* @param {ContainerNode} node
* @param {Operation} operation
*/
function checkOperationWithoutOperator(node, operation) {
Expand Down Expand Up @@ -382,7 +391,7 @@ const OPERAND_TOKEN_TYPES = new Set([
]);

/**
* @param {import('@csstools/css-parser-algorithms').ComponentValue | undefined} node
* @param {ComponentValue | undefined} node
* @returns {boolean}
*/
function isOperandNode(node) {
Expand All @@ -405,20 +414,20 @@ function isOperandNode(node) {
* NOTE: this is messy.
*
* @typedef {{
* firstOperand: import ('@csstools/css-parser-algorithms').ComponentValue,
* before: Array<import ('@csstools/css-parser-algorithms').ComponentValue>,
* secondOperand: import ('@csstools/css-parser-algorithms').ComponentValue,
* after: Array<import ('@csstools/css-parser-algorithms').ComponentValue>,
* operator: import ('@csstools/css-parser-algorithms').ComponentValue | undefined,
* firstOperand: ComponentValue,
* before: Array<ComponentValue>,
* secondOperand: ComponentValue,
* after: Array<ComponentValue>,
* operator: ComponentValue | undefined,
* operatorToken: import ('@csstools/css-tokenizer').TokenDelim | undefined,
* }} Operation
*
* @typedef {{
* firstOperand: import ('@csstools/css-parser-algorithms').ComponentValue,
* before: Array<import ('@csstools/css-parser-algorithms').ComponentValue>,
* secondOperand: import ('@csstools/css-parser-algorithms').ComponentValue,
* after: Array<import ('@csstools/css-parser-algorithms').ComponentValue>,
* operator: import ('@csstools/css-parser-algorithms').ComponentValue,
* firstOperand: ComponentValue,
* before: Array<ComponentValue>,
* secondOperand: ComponentValue,
* after: Array<ComponentValue>,
* operator: ComponentValue,
* operatorToken: import ('@csstools/css-tokenizer').TokenDelim,
* }} CompleteOperation
*/
Expand Down Expand Up @@ -450,20 +459,20 @@ function isOperationWithoutOperator(operation) {
}

/**
* @param {import('@csstools/css-parser-algorithms').ContainerNode} container
* @param {ContainerNode} container
* @param {number} cursor
* @returns {[Operation | undefined, number]}
*/
function parseOperation(container, cursor) {
/** @type {import ('@csstools/css-parser-algorithms').ComponentValue | undefined} */
/** @type {ComponentValue | undefined} */
let firstOperand = undefined;
/** @type {import ('@csstools/css-parser-algorithms').ComponentValue | undefined} */
/** @type {ComponentValue | undefined} */
let secondOperand = undefined;
/** @type {Array<import ('@csstools/css-parser-algorithms').ComponentValue>} */
/** @type {Array<ComponentValue>} */
const before = [];
/** @type {Array<import ('@csstools/css-parser-algorithms').ComponentValue>} */
/** @type {Array<ComponentValue>} */
const after = [];
/** @type {import ('@csstools/css-parser-algorithms').ComponentValue | undefined} */
/** @type {ComponentValue | undefined} */
let operator = undefined;
/** @type {import ('@csstools/css-tokenizer').TokenDelim | undefined} */
let operatorToken = undefined;
Expand Down

0 comments on commit ae8b36e

Please sign in to comment.