Skip to content

Commit

Permalink
Fix selector-max-combinators end positions (stylelint#7596)
Browse files Browse the repository at this point in the history 
* Fix `selector-max-combinators` end positions

* Create violet-boxes-dream.md
  • Loading branch information
@romainmenke @emmacharp
romainmenke authored and emmacharp committed May 7, 2024
1 parent 94cc82d commit 9ad938e
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 57 deletions.
5 changes: 5 additions & 0 deletions .changeset/violet-boxes-dream.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"stylelint": patch
---

Fixed: `selector-max-combinators` end positions
30 changes: 10 additions & 20 deletions lib/rules/selector-max-combinators/__tests__/index.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,46 +184,38 @@ testRule({
{
code: 'foo { bar { baz { quux {} } } }',
description: 'nested selectors: greater than max combinators',
message: messages.expected('foo bar baz quux', 2),
message: messages.expected('quux', 2),
line: 1,
column: 19,
endLine: 1,
endColumn: 26,
// see #6234
// endColumn: 23,
endColumn: 23,
},
{
code: 'foo { bar > & baz ~ quux {} }',
description: 'nested selectors: parent selector',
message: messages.expected('bar > foo baz ~ quux', 2),
message: messages.expected('bar > & baz ~ quux', 2),
line: 1,
column: 7,
endLine: 1,
endColumn: 28,
// see #6234
// endColumn: 25,
endColumn: 25,
},
{
code: 'foo, bar { & > baz ~ quux + bat {} }',
description: 'nested selectors: superfluous parent selector',
warnings: [
{
message: messages.expected('foo > baz ~ quux + bat', 2),
message: messages.expected('& > baz ~ quux + bat', 2),
line: 1,
column: 12,
endLine: 1,
endColumn: 35,
// TODO: The following case is actually correct. It's hard to get the original selector before resolved.
// endColumn: 32,
endColumn: 32,
},
{
message: messages.expected('bar > baz ~ quux + bat', 2),
message: messages.expected('& > baz ~ quux + bat', 2),
line: 1,
column: 12,
endLine: 1,
endColumn: 35,
// TODO: The following case is actually correct. It's hard to get the original selector before resolved.
// endColumn: 32,
endColumn: 32,
},
],
},
Expand Down Expand Up @@ -285,13 +277,11 @@ testRule({
endColumn: 10,
},
{
message: messages.expected('foo + bar .aa', 0),
message: messages.expected('.aa', 0),
line: 1,
column: 29,
endLine: 1,
endColumn: 35,
// TODO: The following case is actually correct. It's hard to get the original selector before resolved.
// endColumn: 32,
endColumn: 32,
},
],
},
Expand Down
37 changes: 19 additions & 18 deletions lib/rules/selector-max-combinators/index.cjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
// please instead edit the ESM counterpart and rebuild with Rollup (npm run build).
'use strict';

const resolvedNestedSelector = require('postcss-resolve-nested-selector');
const flattenNestedSelectorsForRule = require('../../utils/flattenNestedSelectorsForRule.cjs');
const isNonNegativeInteger = require('../../utils/isNonNegativeInteger.cjs');
const isStandardSyntaxRule = require('../../utils/isStandardSyntaxRule.cjs');
const parseSelector = require('../../utils/parseSelector.cjs');
const isStandardSyntaxSelector = require('../../utils/isStandardSyntaxSelector.cjs');
const report = require('../../utils/report.cjs');
const ruleMessages = require('../../utils/ruleMessages.cjs');
const validateOptions = require('../../utils/validateOptions.cjs');
Expand Down Expand Up @@ -36,14 +36,15 @@ const rule = (primary) => {
}

/**
* @param {import('postcss-selector-parser').Container<string | undefined>} resolvedSelectorNode
* @param {import('postcss-selector-parser').Container<string | undefined>} selectorNode
* @param {import('postcss').Rule} ruleNode
*/
function checkSelector(selectorNode, ruleNode) {
const count = selectorNode.reduce((total, childNode) => {
function checkSelector(resolvedSelectorNode, selectorNode, ruleNode) {
const count = resolvedSelectorNode.reduce((total, childNode) => {
// Only traverse inside actual selectors
if (childNode.type === 'selector') {
checkSelector(childNode, ruleNode);
checkSelector(childNode, selectorNode, ruleNode);
}

if (childNode.type === 'combinator') total += 1;
Expand All @@ -52,31 +53,31 @@ const rule = (primary) => {
}, 0);

if (selectorNode.type !== 'root' && selectorNode.type !== 'pseudo' && count > primary) {
const selector = selectorNode.toString();
const index = selectorNode.first?.sourceIndex ?? 0;
const selectorStr = selectorNode.toString().trim();

report({
ruleName,
result,
node: ruleNode,
message: messages.expected,
messageArgs: [selector, primary],
word: selector,
messageArgs: [selectorStr, primary],
index,
endIndex: index + selectorStr.length,
});
}
}

root.walkRules((ruleNode) => {
if (!isStandardSyntaxRule(ruleNode)) {
return;
}
if (!isStandardSyntaxRule(ruleNode)) return;

for (const selector of ruleNode.selectors) {
for (const resolvedSelector of resolvedNestedSelector(selector, ruleNode)) {
parseSelector(resolvedSelector, result, ruleNode, (container) =>
checkSelector(container, ruleNode),
);
}
}
if (!isStandardSyntaxSelector(ruleNode.selector)) return;

flattenNestedSelectorsForRule(ruleNode, result).forEach(({ selector, resolvedSelectors }) => {
resolvedSelectors.forEach((resolvedSelector) => {
checkSelector(resolvedSelector, selector, ruleNode);
});
});
});
};
};
Expand Down
38 changes: 19 additions & 19 deletions lib/rules/selector-max-combinators/index.mjs
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import resolvedNestedSelector from 'postcss-resolve-nested-selector';

import flattenNestedSelectorsForRule from '../../utils/flattenNestedSelectorsForRule.mjs';
import isNonNegativeInteger from '../../utils/isNonNegativeInteger.mjs';
import isStandardSyntaxRule from '../../utils/isStandardSyntaxRule.mjs';
import parseSelector from '../../utils/parseSelector.mjs';
import isStandardSyntaxSelector from '../../utils/isStandardSyntaxSelector.mjs';
import report from '../../utils/report.mjs';
import ruleMessages from '../../utils/ruleMessages.mjs';
import validateOptions from '../../utils/validateOptions.mjs';
Expand Down Expand Up @@ -33,14 +32,15 @@ const rule = (primary) => {
}

/**
* @param {import('postcss-selector-parser').Container<string | undefined>} resolvedSelectorNode
* @param {import('postcss-selector-parser').Container<string | undefined>} selectorNode
* @param {import('postcss').Rule} ruleNode
*/
function checkSelector(selectorNode, ruleNode) {
const count = selectorNode.reduce((total, childNode) => {
function checkSelector(resolvedSelectorNode, selectorNode, ruleNode) {
const count = resolvedSelectorNode.reduce((total, childNode) => {
// Only traverse inside actual selectors
if (childNode.type === 'selector') {
checkSelector(childNode, ruleNode);
checkSelector(childNode, selectorNode, ruleNode);
}

if (childNode.type === 'combinator') total += 1;
Expand All @@ -49,31 +49,31 @@ const rule = (primary) => {
}, 0);

if (selectorNode.type !== 'root' && selectorNode.type !== 'pseudo' && count > primary) {
const selector = selectorNode.toString();
const index = selectorNode.first?.sourceIndex ?? 0;
const selectorStr = selectorNode.toString().trim();

report({
ruleName,
result,
node: ruleNode,
message: messages.expected,
messageArgs: [selector, primary],
word: selector,
messageArgs: [selectorStr, primary],
index,
endIndex: index + selectorStr.length,
});
}
}

root.walkRules((ruleNode) => {
if (!isStandardSyntaxRule(ruleNode)) {
return;
}
if (!isStandardSyntaxRule(ruleNode)) return;

for (const selector of ruleNode.selectors) {
for (const resolvedSelector of resolvedNestedSelector(selector, ruleNode)) {
parseSelector(resolvedSelector, result, ruleNode, (container) =>
checkSelector(container, ruleNode),
);
}
}
if (!isStandardSyntaxSelector(ruleNode.selector)) return;

flattenNestedSelectorsForRule(ruleNode, result).forEach(({ selector, resolvedSelectors }) => {
resolvedSelectors.forEach((resolvedSelector) => {
checkSelector(resolvedSelector, selector, ruleNode);
});
});
});
};
};
Expand Down

0 comments on commit 9ad938e

Please sign in to comment.