Skip to content

Commit e764ba6

Browse files
committed
build: ampersand selector lint rule not catching some cases
I noticed this while working on something else. The ampersand selector mixin looks for one `&` instance and then gives up, but there might be other ampersands inside the same selector which are invalid. These changes update the logic to be a bit more robust and remove the limitation where failures weren't reported on private mixins.
1 parent 3284496 commit e764ba6

File tree

1 file changed

+23
-30
lines changed

1 file changed

+23
-30
lines changed

tools/stylelint/no-ampersand-beyond-selector-start.ts

Lines changed: 23 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import {createPlugin, utils} from 'stylelint';
22
import {basename} from 'path';
3-
import {Node} from './stylelint-postcss-types';
43

54
const isStandardSyntaxRule = require('stylelint/lib/utils/isStandardSyntaxRule');
65
const isStandardSyntaxSelector = require('stylelint/lib/utils/isStandardSyntaxSelector');
@@ -37,43 +36,37 @@ const plugin = createPlugin(ruleName, (isEnabled: boolean, _options?) => {
3736
}
3837

3938
root.walkRules(rule => {
40-
if (
41-
rule.parent.type === 'rule' &&
42-
isStandardSyntaxRule(rule) &&
43-
isStandardSyntaxSelector(rule.selector) &&
44-
// Using the ampersand at the beginning is fine, anything else can cause issues in themes.
45-
rule.selector.indexOf('&') > 0) {
46-
47-
const mixinName = getClosestMixinName(rule);
48-
49-
// Skip rules inside private mixins.
50-
if (!mixinName || !mixinName.startsWith('_')) {
51-
utils.report({
52-
result,
53-
ruleName,
54-
message: messages.expected(),
55-
node: rule
56-
});
57-
}
39+
if (rule.parent.type === 'rule' &&
40+
isStandardSyntaxRule(rule) &&
41+
isStandardSyntaxSelector(rule.selector) &&
42+
hasInvalidAmpersandUsage(rule.selector)) {
43+
utils.report({
44+
result,
45+
ruleName,
46+
message: messages.expected(),
47+
node: rule
48+
});
5849
}
5950
});
6051
};
52+
});
6153

62-
/** Walks up the AST and finds the name of the closest mixin. */
63-
function getClosestMixinName(node: Node): string | undefined {
64-
let parent = node.parent;
65-
66-
while (parent) {
67-
if (parent.type === 'atrule' && parent.name === 'mixin') {
68-
return parent.params;
69-
}
54+
function hasInvalidAmpersandUsage(selector: string): boolean {
55+
let index = selector.indexOf('&');
7056

71-
parent = parent.parent;
57+
while (index > -1) {
58+
// The ampersand is invalid if it's used after the start of
59+
// the selector and not in a combination with another selector.
60+
if (index > 0 && selector[index - 1] === ' ' && (index === selector.length - 1 ||
61+
selector[index + 1] === ' ' || selector[index + 1] === ',')) {
62+
return true;
7263
}
7364

74-
return undefined;
65+
index = selector.indexOf('&', index + 1);
7566
}
76-
});
67+
68+
return false;
69+
}
7770

7871
plugin.ruleName = ruleName;
7972
plugin.messages = messages;

0 commit comments

Comments
 (0)