Skip to content

Commit

Permalink
Fix selector-max-compound-selectors with ignoreSelectors for clas…
Browse files Browse the repository at this point in the history 
…s selectors

Follow-up to PR #7544
See #7544 (comment)

Note that this change doesn't need any changelog item
since the `ignoreSelectors` option is unreleased.
  • Loading branch information
@ybiquitous
ybiquitous committed Mar 14, 2024
1 parent 5078666 commit d566381
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 56 deletions.
10 changes: 10 additions & 0 deletions lib/rules/selector-max-compound-selectors/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,16 @@ The following patterns are considered problems:
p a :not(.foo .bar .baz) {}
```

<!-- prettier-ignore -->
```css
.foo .bar > .baz.ignored {}
```

<!-- prettier-ignore -->
```css
.foo .bar > .ignored.baz {}
```

The following patterns are _not_ considered problems:

<!-- prettier-ignore -->
Expand Down
16 changes: 16 additions & 0 deletions lib/rules/selector-max-compound-selectors/__tests__/index.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,22 @@ testRule({
endLine: 1,
endColumn: 33,
},
{
code: '.foo .bar > .baz.ignored {}',
message: messages.expected('.foo .bar > .baz.ignored', 2),
line: 1,
column: 1,
endLine: 1,
endColumn: 25,
},
{
code: '.foo .bar > .ignored.baz {}',
message: messages.expected('.foo .bar > .ignored.baz', 2),
line: 1,
column: 1,
endLine: 1,
endColumn: 25,
},
],
});

Expand Down
58 changes: 30 additions & 28 deletions lib/rules/selector-max-compound-selectors/index.cjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,26 @@
// please instead edit the ESM counterpart and rebuild with Rollup (npm run build).
'use strict';

const resolvedNestedSelector = require('postcss-resolve-nested-selector');
const resolveNestedSelector = require('postcss-resolve-nested-selector');
const selectorParser = require('postcss-selector-parser');
const validateTypes = require('../../utils/validateTypes.cjs');
const isContextFunctionalPseudoClass = require('../../utils/isContextFunctionalPseudoClass.cjs');
const isNonNegativeInteger = require('../../utils/isNonNegativeInteger.cjs');
const isStandardSyntaxRule = require('../../utils/isStandardSyntaxRule.cjs');
const optionsMatches = require('../../utils/optionsMatches.cjs');
const parseSelector = require('../../utils/parseSelector.cjs');
const pluralize = require('../../utils/pluralize.cjs');
const report = require('../../utils/report.cjs');
const ruleMessages = require('../../utils/ruleMessages.cjs');
const validateOptions = require('../../utils/validateOptions.cjs');

const { isCombinator, isPseudo, isRoot, isSelector } = selectorParser;

const ruleName = 'selector-max-compound-selectors';

const messages = ruleMessages(ruleName, {
expected: (selector, max) =>
`Expected "${selector}" to have no more than ${max} compound ${
max === 1 ? 'selector' : 'selectors'
}`,
expected: (selector, /** @type {number} */ max) =>
`Expected "${selector}" to have no more than ${max} compound ${pluralize('selector', max)}`,
});

const meta = {
Expand Down Expand Up @@ -54,8 +56,7 @@ const rule = (primary, secondaryOptions) => {
* @returns {boolean}
*/
function isSelectorIgnored(selectorNode) {
const selector =
selectorNode.type === 'pseudo' ? selectorNode.value : selectorNode.toString();
const selector = isPseudo(selectorNode) ? selectorNode.value : selectorNode.toString();

return optionsMatches(secondaryOptions, 'ignoreSelectors', selector);
}
Expand All @@ -67,34 +68,35 @@ const rule = (primary, secondaryOptions) => {
* @param {import('postcss').Rule} ruleNode
*/
function checkSelector(selectorNode, ruleNode) {
let compoundCount = 1;
/** @type {import('postcss-selector-parser').Node[]} */
const filteredChildNodes = [];

selectorNode.each((childNode, index) => {
selectorNode.each((childNode) => {
// Only traverse inside actual selectors and context functional pseudo-classes
if (childNode.type === 'selector' || isContextFunctionalPseudoClass(childNode)) {
if (isSelector(childNode) || isContextFunctionalPseudoClass(childNode)) {
checkSelector(childNode, ruleNode);
}

// Compound selectors are separated by combinators, so increase count when meeting one
if (childNode.type === 'combinator') {
compoundCount++;

return;
}

// Try ignoring the selector if the current node is the first, or the previous node is a combinator
const previousNode = selectorNode.at(index - 1);

if ((!previousNode || previousNode.type === 'combinator') && isSelectorIgnored(childNode)) {
compoundCount--;
if (!isSelectorIgnored(childNode)) {
filteredChildNodes.push(childNode);
}
});

if (
selectorNode.type !== 'root' &&
selectorNode.type !== 'pseudo' &&
compoundCount > primary
) {
// Normalize selector nodes and count combinators
const combinatorCount = filteredChildNodes
.filter((node, i, nodes) => {
// Filter out a consecutive combinator
return !(isCombinator(node) && i > 0 && isCombinator(nodes[i - 1]));
})
.filter((node, i, nodes) => {
// Filter out a combinator at the edge
return !(isCombinator(node) && (i === 0 || i === nodes.length - 1));
})
.filter(isCombinator).length;

const compoundCount = combinatorCount + 1;

if (!isRoot(selectorNode) && !isPseudo(selectorNode) && compoundCount > primary) {
const selector = selectorNode.toString();

report({
Expand All @@ -115,7 +117,7 @@ const rule = (primary, secondaryOptions) => {

// Using `.selectors` gets us each selector if there is a comma separated set
for (const selector of ruleNode.selectors) {
for (const resolvedSelector of resolvedNestedSelector(selector, ruleNode)) {
for (const resolvedSelector of resolveNestedSelector(selector, ruleNode)) {
// Process each resolved selector with `checkSelector` via postcss-selector-parser
parseSelector(resolvedSelector, result, ruleNode, (s) => checkSelector(s, ruleNode));
}
Expand Down
58 changes: 30 additions & 28 deletions lib/rules/selector-max-compound-selectors/index.mjs
View file
Original file line number Diff line number Diff line change
@@ -1,22 +1,24 @@
import resolvedNestedSelector from 'postcss-resolve-nested-selector';
import resolveNestedSelector from 'postcss-resolve-nested-selector';

import selectorParser from 'postcss-selector-parser';
const { isCombinator, isPseudo, isRoot, isSelector } = selectorParser;

import { isRegExp, isString } from '../../utils/validateTypes.mjs';
import isContextFunctionalPseudoClass from '../../utils/isContextFunctionalPseudoClass.mjs';
import isNonNegativeInteger from '../../utils/isNonNegativeInteger.mjs';
import isStandardSyntaxRule from '../../utils/isStandardSyntaxRule.mjs';
import optionsMatches from '../../utils/optionsMatches.mjs';
import parseSelector from '../../utils/parseSelector.mjs';
import pluralize from '../../utils/pluralize.mjs';
import report from '../../utils/report.mjs';
import ruleMessages from '../../utils/ruleMessages.mjs';
import validateOptions from '../../utils/validateOptions.mjs';

const ruleName = 'selector-max-compound-selectors';

const messages = ruleMessages(ruleName, {
expected: (selector, max) =>
`Expected "${selector}" to have no more than ${max} compound ${
max === 1 ? 'selector' : 'selectors'
}`,
expected: (selector, /** @type {number} */ max) =>
`Expected "${selector}" to have no more than ${max} compound ${pluralize('selector', max)}`,
});

const meta = {
Expand Down Expand Up @@ -51,8 +53,7 @@ const rule = (primary, secondaryOptions) => {
* @returns {boolean}
*/
function isSelectorIgnored(selectorNode) {
const selector =
selectorNode.type === 'pseudo' ? selectorNode.value : selectorNode.toString();
const selector = isPseudo(selectorNode) ? selectorNode.value : selectorNode.toString();

return optionsMatches(secondaryOptions, 'ignoreSelectors', selector);
}
Expand All @@ -64,34 +65,35 @@ const rule = (primary, secondaryOptions) => {
* @param {import('postcss').Rule} ruleNode
*/
function checkSelector(selectorNode, ruleNode) {
let compoundCount = 1;
/** @type {import('postcss-selector-parser').Node[]} */
const filteredChildNodes = [];

selectorNode.each((childNode, index) => {
selectorNode.each((childNode) => {
// Only traverse inside actual selectors and context functional pseudo-classes
if (childNode.type === 'selector' || isContextFunctionalPseudoClass(childNode)) {
if (isSelector(childNode) || isContextFunctionalPseudoClass(childNode)) {
checkSelector(childNode, ruleNode);
}

// Compound selectors are separated by combinators, so increase count when meeting one
if (childNode.type === 'combinator') {
compoundCount++;

return;
}

// Try ignoring the selector if the current node is the first, or the previous node is a combinator
const previousNode = selectorNode.at(index - 1);

if ((!previousNode || previousNode.type === 'combinator') && isSelectorIgnored(childNode)) {
compoundCount--;
if (!isSelectorIgnored(childNode)) {
filteredChildNodes.push(childNode);
}
});

if (
selectorNode.type !== 'root' &&
selectorNode.type !== 'pseudo' &&
compoundCount > primary
) {
// Normalize selector nodes and count combinators
const combinatorCount = filteredChildNodes
.filter((node, i, nodes) => {
// Filter out a consecutive combinator
return !(isCombinator(node) && i > 0 && isCombinator(nodes[i - 1]));
})
.filter((node, i, nodes) => {
// Filter out a combinator at the edge
return !(isCombinator(node) && (i === 0 || i === nodes.length - 1));
})
.filter(isCombinator).length;

const compoundCount = combinatorCount + 1;

if (!isRoot(selectorNode) && !isPseudo(selectorNode) && compoundCount > primary) {
const selector = selectorNode.toString();

report({
Expand All @@ -112,7 +114,7 @@ const rule = (primary, secondaryOptions) => {

// Using `.selectors` gets us each selector if there is a comma separated set
for (const selector of ruleNode.selectors) {
for (const resolvedSelector of resolvedNestedSelector(selector, ruleNode)) {
for (const resolvedSelector of resolveNestedSelector(selector, ruleNode)) {
// Process each resolved selector with `checkSelector` via postcss-selector-parser
parseSelector(resolvedSelector, result, ruleNode, (s) => checkSelector(s, ruleNode));
}
Expand Down

0 comments on commit d566381

Please sign in to comment.