Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix selector-max-compound-selectors with ignoreSelectors for class selectors #7559

Merged
merged 3 commits into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions lib/rules/selector-max-compound-selectors/README.md
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
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
60 changes: 33 additions & 27 deletions lib/rules/selector-max-compound-selectors/index.cjs
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,39 @@ 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;
if (!isSelectorIgnored(childNode)) {
filteredChildNodes.push(childNode);
}
});

// 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 (isRoot(selectorNode) || isPseudo(selectorNode)) return;

if ((!previousNode || previousNode.type === 'combinator') && isSelectorIgnored(childNode)) {
compoundCount--;
}
});
// Normalize selector nodes and count combinators
const combinatorCount = filteredChildNodes.reduce((count, node, i, nodes) => {
// Not count a node except a combinator
if (!isCombinator(node)) return count;

// Not count a combinator at the edge
if (i === 0 || i === nodes.length - 1) return count;

// Not count a consecutive combinator
if (isCombinator(nodes[i + 1])) return count;

return count + 1;
}, 0);

const compoundCount = combinatorCount + 1;

if (
selectorNode.type !== 'root' &&
selectorNode.type !== 'pseudo' &&
compoundCount > primary
) {
if (compoundCount > primary) {
const selector = selectorNode.toString();

report({
Expand All @@ -115,7 +121,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
60 changes: 33 additions & 27 deletions lib/rules/selector-max-compound-selectors/index.mjs
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,39 @@ 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;
if (!isSelectorIgnored(childNode)) {
filteredChildNodes.push(childNode);
}
});

// 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 (isRoot(selectorNode) || isPseudo(selectorNode)) return;

if ((!previousNode || previousNode.type === 'combinator') && isSelectorIgnored(childNode)) {
compoundCount--;
}
});
// Normalize selector nodes and count combinators
const combinatorCount = filteredChildNodes.reduce((count, node, i, nodes) => {
// Not count a node except a combinator
if (!isCombinator(node)) return count;

// Not count a combinator at the edge
if (i === 0 || i === nodes.length - 1) return count;

// Not count a consecutive combinator
if (isCombinator(nodes[i + 1])) return count;

return count + 1;
}, 0);

const compoundCount = combinatorCount + 1;

if (
selectorNode.type !== 'root' &&
selectorNode.type !== 'pseudo' &&
compoundCount > primary
) {
if (compoundCount > primary) {
const selector = selectorNode.toString();

report({
Expand All @@ -112,7 +118,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
Loading