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.

Sign up for GitHub

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

Jump to bottom

Fix false negatives for where, is, nth-child and nth-last-child in selector-max-* (except selector-max-type) #4842

Merged
merged 4 commits into from Jul 23, 2020
Merged

Fix false negatives for where, is, nth-child and nth-last-child in selector-max-* (except selector-max-type) #4842

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5a0f0c4
updates keywordSets
mattxwang Jun 23, 2020
305f430
replaces functionality of isLogicalCombination with isContextFunction…
mattxwang Jun 23, 2020
edb6cb8
adds a check for nonstandard syntax type selectors to selector-max-type
mattxwang Jun 27, 2020
ea93406
Apply suggestions from code review
mattxwang Jun 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 6 additions & 6 deletions lib/reference/keywordSets.js
View file
Open in desktop
Expand Up @@ -208,9 +208,7 @@ keywordSets.pseudoElements = uniteSets(
);

keywordSets.aNPlusBNotationPseudoClasses = new Set([
'nth-child',
'nth-column',
'nth-last-child',
'nth-last-column',
'nth-last-of-type',
'nth-of-type',
Expand All @@ -220,6 +218,10 @@ keywordSets.linguisticPseudoClasses = new Set(['dir', 'lang']);

keywordSets.atRulePagePseudoClasses = new Set(['first', 'right', 'left', 'blank']);

keywordSets.logicalCombinationsPseudoClasses = new Set(['has', 'is', 'matches', 'not', 'where']);

keywordSets.aNPlusBOfSNotationPseudoClasses = new Set(['nth-child', 'nth-last-child']);

keywordSets.otherPseudoClasses = new Set([
'active',
'any-link',
Expand All @@ -241,19 +243,15 @@ keywordSets.otherPseudoClasses = new Set([
'focus-visible',
'fullscreen',
'future',
'has',
'host',
'host-context',
'hover',
'indeterminate',
'in-range',
'invalid',
'is',
'last-child',
'last-of-type',
'link',
'matches',
'not',
'only-child',
'only-of-type',
'optional',
Expand Down Expand Up @@ -302,6 +300,8 @@ keywordSets.webkitProprietaryPseudoClasses = new Set([
keywordSets.pseudoClasses = uniteSets(
keywordSets.aNPlusBNotationPseudoClasses,
keywordSets.linguisticPseudoClasses,
keywordSets.logicalCombinationsPseudoClasses,
keywordSets.aNPlusBOfSNotationPseudoClasses,
keywordSets.otherPseudoClasses,
);

Expand Down
6 changes: 3 additions & 3 deletions lib/rules/selector-max-attribute/index.js
View file
Open in desktop
Expand Up @@ -3,7 +3,7 @@
'use strict';

const _ = require('lodash');
const isLogicalCombination = require('../../utils/isLogicalCombination');
const isContextFunctionalPseudoClass = require('../../utils/isContextFunctionalPseudoClass');
const isStandardSyntaxRule = require('../../utils/isStandardSyntaxRule');
const optionsMatches = require('../../utils/optionsMatches');
const parseSelector = require('../../utils/parseSelector');
Expand Down Expand Up @@ -49,8 +49,8 @@ function rule(max, options) {

function checkSelector(selectorNode, ruleNode) {
const count = selectorNode.reduce((total, childNode) => {
// Only traverse inside actual selectors and logical combinations
if (childNode.type === 'selector' || isLogicalCombination(childNode)) {
// Only traverse inside actual selectors and context functional pseudo classes
mattxwang marked this conversation as resolved.
Show resolved Hide resolved
if (childNode.type === 'selector' || isContextFunctionalPseudoClass(childNode)) {
checkSelector(childNode, ruleNode);
}

Expand Down
6 changes: 3 additions & 3 deletions lib/rules/selector-max-class/index.js
View file
Open in desktop
Expand Up @@ -2,7 +2,7 @@

'use strict';

const isLogicalCombination = require('../../utils/isLogicalCombination');
const isContextFunctionalPseudoClass = require('../../utils/isContextFunctionalPseudoClass');
const isStandardSyntaxRule = require('../../utils/isStandardSyntaxRule');
const parseSelector = require('../../utils/parseSelector');
const report = require('../../utils/report');
Expand Down Expand Up @@ -34,8 +34,8 @@ function rule(max) {

function checkSelector(selectorNode, ruleNode) {
const count = selectorNode.reduce((total, childNode) => {
// Only traverse inside actual selectors and logical combinations
if (childNode.type === 'selector' || isLogicalCombination(childNode)) {
// Only traverse inside actual selectors and context functional pseudo classes
mattxwang marked this conversation as resolved.
Show resolved Hide resolved
if (childNode.type === 'selector' || isContextFunctionalPseudoClass(childNode)) {
checkSelector(childNode, ruleNode);
}

Expand Down
6 changes: 3 additions & 3 deletions lib/rules/selector-max-compound-selectors/index.js
View file
Open in desktop
Expand Up @@ -2,7 +2,7 @@

'use strict';

const isLogicalCombination = require('../../utils/isLogicalCombination');
const isContextFunctionalPseudoClass = require('../../utils/isContextFunctionalPseudoClass');
const isStandardSyntaxRule = require('../../utils/isStandardSyntaxRule');
const parseSelector = require('../../utils/parseSelector');
const report = require('../../utils/report');
Expand Down Expand Up @@ -39,8 +39,8 @@ function rule(max) {
let compoundCount = 1;

selectorNode.each((childNode) => {
// Only traverse inside actual selectors and logical combinations
if (childNode.type === 'selector' || isLogicalCombination(childNode)) {
// Only traverse inside actual selectors and context functional pseudo classes
mattxwang marked this conversation as resolved.
Show resolved Hide resolved
if (childNode.type === 'selector' || isContextFunctionalPseudoClass(childNode)) {
checkSelector(childNode, rule);
}

Expand Down
6 changes: 3 additions & 3 deletions lib/rules/selector-max-id/index.js
View file
Open in desktop
Expand Up @@ -3,7 +3,7 @@
'use strict';

const _ = require('lodash');
const isLogicalCombination = require('../../utils/isLogicalCombination');
const isContextFunctionalPseudoClass = require('../../utils/isContextFunctionalPseudoClass');
const isStandardSyntaxRule = require('../../utils/isStandardSyntaxRule');
const optionsMatches = require('../../utils/optionsMatches');
const parseSelector = require('../../utils/parseSelector');
Expand Down Expand Up @@ -47,10 +47,10 @@ function rule(max, options) {

function checkSelector(selectorNode, ruleNode) {
const count = selectorNode.reduce((total, childNode) => {
// Only traverse inside actual selectors and logical combinations that are not part of ignored functional pseudo-classes
// Only traverse inside actual selectors and context functional pseudo classes that are not part of ignored functional pseudo-classes
mattxwang marked this conversation as resolved.
Show resolved Hide resolved
if (
childNode.type === 'selector' ||
(isLogicalCombination(childNode) &&
(isContextFunctionalPseudoClass(childNode) &&
!isIgnoredContextFunctionalPseudoClass(childNode, options))
) {
checkSelector(childNode, ruleNode);
Expand Down
6 changes: 3 additions & 3 deletions lib/rules/selector-max-pseudo-class/index.js
View file
Open in desktop
Expand Up @@ -2,7 +2,7 @@

'use strict';

const isLogicalCombination = require('../../utils/isLogicalCombination');
const isContextFunctionalPseudoClass = require('../../utils/isContextFunctionalPseudoClass');
const isStandardSyntaxRule = require('../../utils/isStandardSyntaxRule');
const keywordSets = require('../../reference/keywordSets');
const parseSelector = require('../../utils/parseSelector');
Expand Down Expand Up @@ -35,8 +35,8 @@ function rule(max) {

function checkSelector(selectorNode, ruleNode) {
const count = selectorNode.reduce((total, childNode) => {
// Only traverse inside actual selectors and logical combinations
if (childNode.type === 'selector' || isLogicalCombination(childNode)) {
// Only traverse inside actual selectors and context functional pseudo classes
mattxwang marked this conversation as resolved.
Show resolved Hide resolved
if (childNode.type === 'selector' || isContextFunctionalPseudoClass(childNode)) {
checkSelector(childNode, ruleNode);
}

Expand Down
11 changes: 8 additions & 3 deletions lib/rules/selector-max-type/index.js
View file
Open in desktop
Expand Up @@ -3,11 +3,12 @@
'use strict';

const _ = require('lodash');
const isContextFunctionalPseudoClass = require('../../utils/isContextFunctionalPseudoClass');
const isKeyframeSelector = require('../../utils/isKeyframeSelector');
const isLogicalCombination = require('../../utils/isLogicalCombination');
const isOnlyWhitespace = require('../../utils/isOnlyWhitespace');
const isStandardSyntaxRule = require('../../utils/isStandardSyntaxRule');
const isStandardSyntaxSelector = require('../../utils/isStandardSyntaxSelector');
const isStandardSyntaxTypeSelector = require('../../utils/isStandardSyntaxTypeSelector');
const optionsMatches = require('../../utils/optionsMatches');
const parseSelector = require('../../utils/parseSelector');
const report = require('../../utils/report');
Expand Down Expand Up @@ -56,8 +57,8 @@ function rule(max, options) {

function checkSelector(selectorNode, ruleNode) {
const count = selectorNode.reduce((total, childNode) => {
// Only traverse inside actual selectors and logical combinations
if (childNode.type === 'selector' || isLogicalCombination(childNode)) {
// Only traverse inside actual selectors and context functional pseudo classes
mattxwang marked this conversation as resolved.
Show resolved Hide resolved
if (childNode.type === 'selector' || isContextFunctionalPseudoClass(childNode)) {
checkSelector(childNode, ruleNode);
}

Expand All @@ -81,6 +82,10 @@ function rule(max, options) {
return total;
}

if (childNode.type === 'tag' && !isStandardSyntaxTypeSelector(childNode)) {
return total;
}

return total + (childNode.type === 'tag');
}, 0);

Expand Down
48 changes: 48 additions & 0 deletions lib/utils/__tests__/isContextFunctionalPseudoClass.test.js
View file
Open in desktop
@@ -0,0 +1,48 @@
'use strict';

const isContextFunctionalPseudoClass = require('../isContextFunctionalPseudoClass');
const parseSelector = require('postcss-selector-parser');
const postcss = require('postcss');

function selector(css, cb) {
postcss.parse(css).walkRules((rule) => {
parseSelector((selectorAST) => {
selectorAST.walkPseudos(cb);
}).processSync(rule.selector);
});
}

describe('isContextFunctionalPseudoClass', () => {
it('handles non-context-functional pseudo-classes, like hover', () => {
selector('a:hover {}', (selector) => {
expect(isContextFunctionalPseudoClass(selector)).toBe(false);
});
});

it('handles logical combinations, ', () => {
selector('a:has(.foo) {}', (selector) => {
expect(isContextFunctionalPseudoClass(selector)).toBe(true);
});
selector('a:is(.foo) {}', (selector) => {
expect(isContextFunctionalPseudoClass(selector)).toBe(true);
});
selector('a:matches(.foo) {}', (selector) => {
expect(isContextFunctionalPseudoClass(selector)).toBe(true);
});
selector('a:not(.foo) {}', (selector) => {
expect(isContextFunctionalPseudoClass(selector)).toBe(true);
});
selector('a:where(.foo) {}', (selector) => {
expect(isContextFunctionalPseudoClass(selector)).toBe(true);
});
});

it('handles tree structural/NPlusBOfSNotationPseudoClasses classes', () => {
selector('a:nth-child(n+7) {}', (selector) => {
expect(isContextFunctionalPseudoClass(selector)).toBe(true);
});
selector('a:nth-last-child(n+7) {}', (selector) => {
expect(isContextFunctionalPseudoClass(selector)).toBe(true);
});
});
});
39 changes: 0 additions & 39 deletions lib/utils/__tests__/isLogicalCombination.test.js
View file
Open in desktop

This file was deleted.

23 changes: 23 additions & 0 deletions lib/utils/isContextFunctionalPseudoClass.js
View file
Open in desktop
@@ -0,0 +1,23 @@
'use strict';

const keywordSets = require('../reference/keywordSets');

/**
* Check whether a node is a context-functional pseudo-class (i.e. either a logical combination
* or a 'aNPlusBOfSNotationPseudoClasses' / tree-structural pseudo class)
mattxwang marked this conversation as resolved.
Show resolved Hide resolved
*
* @param {import('postcss-selector-parser').Pseudo} node postcss-selector-parser node (of type pseudo)
* @return {boolean} If `true`, the node is a context-functional pseudo-class
*/
module.exports = function isContextFunctionalPseudoClass(node) {
if (node.type === 'pseudo') {
const normalisedParentName = node.value.toLowerCase().replace(/:+/, '');

return (
keywordSets.logicalCombinationsPseudoClasses.has(normalisedParentName) ||
keywordSets.aNPlusBOfSNotationPseudoClasses.has(normalisedParentName)
);
}

return false;
};
22 changes: 0 additions & 22 deletions lib/utils/isLogicalCombination.js
View file
Open in desktop

This file was deleted.

1 change: 1 addition & 0 deletions lib/utils/isStandardSyntaxTypeSelector.js
View file
Open in desktop
Expand Up @@ -27,6 +27,7 @@ module.exports = function (node) {
if (
parentType === 'pseudo' &&
(keywordSets.aNPlusBNotationPseudoClasses.has(normalisedParentName) ||
keywordSets.aNPlusBOfSNotationPseudoClasses.has(normalisedParentName) ||
keywordSets.linguisticPseudoClasses.has(normalisedParentName) ||
keywordSets.shadowTreePseudoElements.has(normalisedParentName))
) {
Expand Down