Skip to content

Commit

Permalink
Add constraints to stylelint-polaris/coverage disable comments (#7589)
Browse files Browse the repository at this point in the history
  • Loading branch information
aaronccasanova authored Nov 3, 2022
1 parent 1f2ec8b commit b7b0ef5
Show file tree
Hide file tree
Showing 3 changed files with 137 additions and 1 deletion.
5 changes: 5 additions & 0 deletions .changeset/tidy-cougars-talk.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/stylelint-polaris': patch
---

Add constraints to `stylelint-polaris/coverage` disable comments
76 changes: 75 additions & 1 deletion stylelint-polaris/plugins/coverage/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const stylelint = require('stylelint');

const {isObject} = require('../../utils');
const {isObject, isNumber} = require('../../utils');

const ruleName = 'stylelint-polaris/coverage';

Expand Down Expand Up @@ -55,10 +55,84 @@ module.exports = stylelint.createPlugin(
},
);
}

const disabledCoverageWarnings =
result.stylelint.disabledWarnings?.filter((disabledWarning) =>
disabledWarning.rule.startsWith(ruleName),
);

if (!disabledCoverageWarnings?.length) return;

const disabledCoverageLines = Array.from(
new Set(disabledCoverageWarnings.map(({line}) => line)),
);

// Ensure all stylelint-polaris/coverage disable comments
// have a description prefixed with "polaris:"
for (const disabledRange of result.stylelint.disabledRanges.all) {
if (
!isDisabledCoverageRule(disabledCoverageLines, disabledRange) ||
disabledRange.description?.startsWith('polaris:')
) {
continue;
}

const stylelintDisableText = disabledRange.comment.text
.split('--')[0]
.trim();

stylelint.utils.report({
message: `Expected /* ${stylelintDisableText} -- polaris: Reason for disabling */`,
ruleName,
result,
node: disabledRange.comment,
// Note: `stylelint-disable` comments (without next-line) appear to
// be special cased in that they do not trigger warnings when reported.
// Setting `line` to an invalid line number forces the warning to be
// reported and the above comment `node` is used to display the
// location information:
// https://github.com/stylelint/stylelint/blob/57cbcd4eb0ee809006a1e3d2ccfe73af48744ad5/lib/utils/report.js#L49-L52
line: -1,
});
}
};
},
);

/**
* @param {number[]} disabledCoverageLines
* @param {import('stylelint').DisabledRange} disabledRange
*/
function isDisabledCoverageRule(disabledCoverageLines, disabledRange) {
if (isUnclosedDisabledRange(disabledRange)) {
return disabledCoverageLines.some(
(disabledCoverageLine) => disabledCoverageLine >= disabledRange.start,
);
}

return disabledCoverageLines.some(
(coverageDisabledLine) =>
coverageDisabledLine >= disabledRange.start &&
coverageDisabledLine <= disabledRange.end,
);
}

/**
* Checks if the `disabledRange` is an unclosed `stylelint-disable` comment
* e.g. The `stylelint-disable` comment is NOT followed by `stylelint-enable`
* @param {import('stylelint').DisabledRange} disabledRange
*/
function isUnclosedDisabledRange(disabledRange) {
if (
!disabledRange.comment.text.startsWith('stylelint-disable-next-line') &&
!isNumber(disabledRange.end)
) {
return true;
}

return false;
}

function validatePrimaryOptions(primaryOptions) {
if (!isObject(primaryOptions)) return false;

Expand Down
57 changes: 57 additions & 0 deletions stylelint-polaris/plugins/coverage/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,31 @@ testRule({
code: '@media (min-width: 320px) {}',
description: 'Uses allowed at-rule',
},
{
code: `
/* stylelint-disable -- polaris: context */
@keyframes foo {}
/* stylelint-enable */
`,
description:
'Uses disallowed at-rule with disable/enable comment and context',
},
{
code: `
/* stylelint-disable -- polaris: context */
@keyframes foo {}
`,
description:
'Uses disallowed at-rule with disable comment and context and without enable comment',
},
{
code: `
/* stylelint-disable-next-line -- polaris: context */
@keyframes foo {}
`,
description:
'Uses disallowed at-rule with disable next line comment and context',
},
],

reject: [
Expand All @@ -25,5 +50,37 @@ testRule({
message:
'Unexpected at-rule "keyframes" (stylelint-polaris/coverage/motion)',
},
{
code: `
/* stylelint-disable */
@keyframes foo {}
/* stylelint-enable */
`,
description:
'Uses disallowed at-rule with disable/enable comment and without context',
message:
'Expected /* stylelint-disable -- polaris: Reason for disabling */',
},
{
code: `
/* stylelint-disable */
@keyframes fooz {}
`,
description:
'Uses disallowed at-rule with disable comment and without context and enable comment',
message:
'Expected /* stylelint-disable -- polaris: Reason for disabling */',
},
{
code: `
/* stylelint-disable-next-line */
@keyframes foo {}
`,
description:
'Uses disallowed at-rule with disable next line comment and without context',
message:
'Expected /* stylelint-disable-next-line -- polaris: Reason for disabling */',
},
],
});

0 comments on commit b7b0ef5

Please sign in to comment.