Skip to content

Commit b7b0ef5

Browse files
Add constraints to stylelint-polaris/coverage disable comments (#7589)
1 parent 1f2ec8b commit b7b0ef5

File tree

3 files changed

+137
-1
lines changed

3 files changed

+137
-1
lines changed

.changeset/tidy-cougars-talk.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@shopify/stylelint-polaris': patch
3+
---
4+
5+
Add constraints to `stylelint-polaris/coverage` disable comments

stylelint-polaris/plugins/coverage/index.js

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
const stylelint = require('stylelint');
22

3-
const {isObject} = require('../../utils');
3+
const {isObject, isNumber} = require('../../utils');
44

55
const ruleName = 'stylelint-polaris/coverage';
66

@@ -55,10 +55,84 @@ module.exports = stylelint.createPlugin(
5555
},
5656
);
5757
}
58+
59+
const disabledCoverageWarnings =
60+
result.stylelint.disabledWarnings?.filter((disabledWarning) =>
61+
disabledWarning.rule.startsWith(ruleName),
62+
);
63+
64+
if (!disabledCoverageWarnings?.length) return;
65+
66+
const disabledCoverageLines = Array.from(
67+
new Set(disabledCoverageWarnings.map(({line}) => line)),
68+
);
69+
70+
// Ensure all stylelint-polaris/coverage disable comments
71+
// have a description prefixed with "polaris:"
72+
for (const disabledRange of result.stylelint.disabledRanges.all) {
73+
if (
74+
!isDisabledCoverageRule(disabledCoverageLines, disabledRange) ||
75+
disabledRange.description?.startsWith('polaris:')
76+
) {
77+
continue;
78+
}
79+
80+
const stylelintDisableText = disabledRange.comment.text
81+
.split('--')[0]
82+
.trim();
83+
84+
stylelint.utils.report({
85+
message: `Expected /* ${stylelintDisableText} -- polaris: Reason for disabling */`,
86+
ruleName,
87+
result,
88+
node: disabledRange.comment,
89+
// Note: `stylelint-disable` comments (without next-line) appear to
90+
// be special cased in that they do not trigger warnings when reported.
91+
// Setting `line` to an invalid line number forces the warning to be
92+
// reported and the above comment `node` is used to display the
93+
// location information:
94+
// https://github.com/stylelint/stylelint/blob/57cbcd4eb0ee809006a1e3d2ccfe73af48744ad5/lib/utils/report.js#L49-L52
95+
line: -1,
96+
});
97+
}
5898
};
5999
},
60100
);
61101

102+
/**
103+
* @param {number[]} disabledCoverageLines
104+
* @param {import('stylelint').DisabledRange} disabledRange
105+
*/
106+
function isDisabledCoverageRule(disabledCoverageLines, disabledRange) {
107+
if (isUnclosedDisabledRange(disabledRange)) {
108+
return disabledCoverageLines.some(
109+
(disabledCoverageLine) => disabledCoverageLine >= disabledRange.start,
110+
);
111+
}
112+
113+
return disabledCoverageLines.some(
114+
(coverageDisabledLine) =>
115+
coverageDisabledLine >= disabledRange.start &&
116+
coverageDisabledLine <= disabledRange.end,
117+
);
118+
}
119+
120+
/**
121+
* Checks if the `disabledRange` is an unclosed `stylelint-disable` comment
122+
* e.g. The `stylelint-disable` comment is NOT followed by `stylelint-enable`
123+
* @param {import('stylelint').DisabledRange} disabledRange
124+
*/
125+
function isUnclosedDisabledRange(disabledRange) {
126+
if (
127+
!disabledRange.comment.text.startsWith('stylelint-disable-next-line') &&
128+
!isNumber(disabledRange.end)
129+
) {
130+
return true;
131+
}
132+
133+
return false;
134+
}
135+
62136
function validatePrimaryOptions(primaryOptions) {
63137
if (!isObject(primaryOptions)) return false;
64138

stylelint-polaris/plugins/coverage/index.test.js

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,31 @@ testRule({
1616
code: '@media (min-width: 320px) {}',
1717
description: 'Uses allowed at-rule',
1818
},
19+
{
20+
code: `
21+
/* stylelint-disable -- polaris: context */
22+
@keyframes foo {}
23+
/* stylelint-enable */
24+
`,
25+
description:
26+
'Uses disallowed at-rule with disable/enable comment and context',
27+
},
28+
{
29+
code: `
30+
/* stylelint-disable -- polaris: context */
31+
@keyframes foo {}
32+
`,
33+
description:
34+
'Uses disallowed at-rule with disable comment and context and without enable comment',
35+
},
36+
{
37+
code: `
38+
/* stylelint-disable-next-line -- polaris: context */
39+
@keyframes foo {}
40+
`,
41+
description:
42+
'Uses disallowed at-rule with disable next line comment and context',
43+
},
1944
],
2045

2146
reject: [
@@ -25,5 +50,37 @@ testRule({
2550
message:
2651
'Unexpected at-rule "keyframes" (stylelint-polaris/coverage/motion)',
2752
},
53+
{
54+
code: `
55+
/* stylelint-disable */
56+
@keyframes foo {}
57+
/* stylelint-enable */
58+
`,
59+
description:
60+
'Uses disallowed at-rule with disable/enable comment and without context',
61+
message:
62+
'Expected /* stylelint-disable -- polaris: Reason for disabling */',
63+
},
64+
{
65+
code: `
66+
/* stylelint-disable */
67+
68+
@keyframes fooz {}
69+
`,
70+
description:
71+
'Uses disallowed at-rule with disable comment and without context and enable comment',
72+
message:
73+
'Expected /* stylelint-disable -- polaris: Reason for disabling */',
74+
},
75+
{
76+
code: `
77+
/* stylelint-disable-next-line */
78+
@keyframes foo {}
79+
`,
80+
description:
81+
'Uses disallowed at-rule with disable next line comment and without context',
82+
message:
83+
'Expected /* stylelint-disable-next-line -- polaris: Reason for disabling */',
84+
},
2885
],
2986
});

0 commit comments

Comments
 (0)