Skip to content

Commit d97e5ae

Browse files
authored
build: ampersand selector lint rule not catching some cases (#22974)
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 efabc01 commit d97e5ae

File tree

2 files changed

+17
-34
lines changed

2 files changed

+17
-34
lines changed

src/material/badge/_badge-theme.scss

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ $large-size: $default-size + 6;
1818

1919
// Mixin for building offset given different sizes
2020
@mixin _badge-size($size) {
21+
// This mixin isn't used in the context of a theme so we can disable the ampersand check.
22+
// stylelint-disable material/no-ampersand-beyond-selector-start
2123
.mat-badge-content {
2224
width: $size;
2325
height: $size;
@@ -89,6 +91,7 @@ $large-size: $default-size + 6;
8991
}
9092
}
9193
}
94+
// stylelint-enable
9295
}
9396

9497
@mixin color($config-or-theme) {
Lines changed: 14 additions & 34 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,44 +36,25 @@ 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
};
61-
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-
}
70-
71-
parent = parent.parent;
72-
}
73-
74-
return undefined;
75-
}
7652
});
7753

54+
function hasInvalidAmpersandUsage(selector: string): boolean {
55+
return selector.split(',').some(part => part.trim().indexOf('&', 1) > -1);
56+
}
57+
7858
plugin.ruleName = ruleName;
7959
plugin.messages = messages;
8060
export default plugin;

0 commit comments

Comments
 (0)