Skip to content

Commit

Permalink
Add a reportDescriptionlessDisables flag (#4907)
Browse files Browse the repository at this point in the history
  • Loading branch information
jathak committed Aug 31, 2020
1 parent 5446be2 commit 0a17b64
Show file tree
Hide file tree
Showing 10 changed files with 226 additions and 15 deletions.
5 changes: 5 additions & 0 deletions lib/__tests__/__snapshots__/cli.test.js.snap
Expand Up @@ -122,6 +122,11 @@ exports[`CLI --help 1`] = `
Report stylelint-disable comments that used for rules that don't exist within the configuration object.
The process will exit with code 2 if invalid scope disables are found.
--report-descriptionless-disables, --rdd
Report stylelint-disable comments without a description.
The process will exit with code 2 if descriptionless disables are found.
--max-warnings, --mw
Number of warnings above which the process will exit with code 2.
Expand Down
18 changes: 18 additions & 0 deletions lib/__tests__/cli.test.js
Expand Up @@ -26,6 +26,7 @@ describe('buildCLI', () => {
ignoreDisables: false,
printConfig: false,
quiet: false,
reportDescriptionlessDisables: false,
reportInvalidScopeDisables: false,
reportNeedlessDisables: false,
stdin: false,
Expand Down Expand Up @@ -258,6 +259,23 @@ describe('CLI', () => {
);
});

it('reports descriptionless disables', async () => {
await cli([
'--report-descriptionless-disables',
'--config',
replaceBackslashes(path.join(fixturesPath, 'config-block-no-empty.json')),
replaceBackslashes(path.join(fixturesPath, 'empty-block-with-relevant-disable.css')),
]);

expect(process.exitCode).toBe(2);

expect(process.stdout.write).toHaveBeenCalledTimes(1);
expect(process.stdout.write).toHaveBeenNthCalledWith(
1,
expect.stringContaining('descriptionless disable: block-no-empty'),
);
});

it('--stdin', async () => {
await cli(['--stdin', '--config', `${fixturesPath}/config-no-empty-source.json`]);

Expand Down
62 changes: 62 additions & 0 deletions lib/__tests__/descriptionlessDisables.test.js
@@ -0,0 +1,62 @@
'use strict';

const standalone = require('../standalone');
const stripIndent = require('common-tags').stripIndent;

it('descriptionlessDisables', () => {
const config = {
rules: { 'block-no-empty': true },
};

const css = stripIndent`
/* stylelint-disable -- Description */
a {}
/* stylelint-enable */
a {
b {} /* stylelint-disable-line block-no-empty -- Description */
}
/* stylelint-disable-next-line block-no-empty
* --
* Description */
a {}
/* stylelint-disable */
a {}
/* stylelint-enable */
a {
b {} /* stylelint-disable-line block-no-empty */
}
/* stylelint-disable-next-line block-no-empty */
a {}
`;

return standalone({
config,
code: css,
reportDescriptionlessDisables: true,
}).then((linted) => {
const report = linted.descriptionlessDisables;

expect(report).toHaveLength(1);
expect(report[0].ranges).toEqual([
{
start: 12,
end: 14,
rule: 'all',
unusedRule: 'all',
},
{
start: 16,
end: 16,
rule: 'block-no-empty',
unusedRule: 'block-no-empty',
},
{
start: 19,
end: 19,
rule: 'block-no-empty',
unusedRule: 'block-no-empty',
},
]);
});
});
10 changes: 10 additions & 0 deletions lib/__tests__/disableRanges.test.js
Expand Up @@ -653,6 +653,7 @@ it('disable (with description) without re-enabling', () => {
{
start: 1,
strictStart: true,
description: 'Description',
},
],
});
Expand All @@ -672,6 +673,7 @@ it('disable and re-enable (with descriptions)', () => {
end: 4,
strictStart: true,
strictEnd: true,
description: 'Description',
},
],
});
Expand All @@ -689,6 +691,7 @@ it('disable rule (with description) without re-enabling', () => {
strictStart: true,
end: undefined,
strictEnd: undefined,
description: 'Description',
},
],
});
Expand All @@ -706,18 +709,21 @@ it('disable rules (with description) with newline in rule list', () => {
{
start: 1,
strictStart: true,
description: 'Description',
},
],
'hoo-hah': [
{
start: 1,
strictStart: true,
description: 'Description',
},
],
slime: [
{
start: 1,
strictStart: true,
description: 'Description',
},
],
});
Expand All @@ -741,6 +747,7 @@ it('SCSS // line-disabling comment (with description)', () => {
end: 2,
strictStart: true,
strictEnd: true,
description: 'Description',
},
],
});
Expand All @@ -767,6 +774,7 @@ it('SCSS // disable next-line comment (with multi-line description)', () => {
end: 5,
strictStart: true,
strictEnd: true,
description: 'Long-winded description',
},
],
});
Expand All @@ -790,6 +798,7 @@ it('Less // line-disabling comment (with description)', () => {
end: 2,
strictStart: true,
strictEnd: true,
description: 'Description',
},
],
});
Expand All @@ -816,6 +825,7 @@ it('Less // disable next-line comment (with multi-line description)', () => {
end: 5,
strictStart: true,
strictEnd: true,
description: 'Long-winded description',
},
],
});
Expand Down
48 changes: 34 additions & 14 deletions lib/assignDisabledRanges.js
Expand Up @@ -18,16 +18,18 @@ const ALL_RULES = 'all';
/**
* @param {number} start
* @param {boolean} strictStart
* @param {string|undefined} description
* @param {number} [end]
* @param {boolean} [strictEnd]
* @returns {DisabledRange}
*/
function createDisableRange(start, strictStart, end, strictEnd) {
function createDisableRange(start, strictStart, description, end, strictEnd) {
return {
start,
end: end || undefined,
strictStart,
strictEnd: typeof strictEnd === 'boolean' ? strictEnd : undefined,
description,
};
}

Expand Down Expand Up @@ -106,9 +108,10 @@ module.exports = function (root, result) {
function processDisableLineCommand(comment) {
if (comment.source && comment.source.start) {
const line = comment.source.start.line;
const description = getDescription(comment.text);

getCommandRules(disableLineCommand, comment.text).forEach((ruleName) => {
disableLine(line, ruleName, comment);
disableLine(line, ruleName, comment, description);
});
}
}
Expand All @@ -119,9 +122,10 @@ module.exports = function (root, result) {
function processDisableNextLineCommand(comment) {
if (comment.source && comment.source.end) {
const line = comment.source.end.line;
const description = getDescription(comment.text);

getCommandRules(disableNextLineCommand, comment.text).forEach((ruleName) => {
disableLine(line + 1, ruleName, comment);
disableLine(line + 1, ruleName, comment, description);
});
}
}
Expand All @@ -130,8 +134,9 @@ module.exports = function (root, result) {
* @param {number} line
* @param {string} ruleName
* @param {PostcssComment} comment
* @param {string|undefined} description
*/
function disableLine(line, ruleName, comment) {
function disableLine(line, ruleName, comment, description) {
if (ruleIsDisabled(ALL_RULES)) {
throw comment.error('All rules have already been disabled', {
plugin: 'stylelint',
Expand All @@ -144,7 +149,7 @@ module.exports = function (root, result) {

const strict = disabledRuleName === ALL_RULES;

startDisabledRange(line, disabledRuleName, strict);
startDisabledRange(line, disabledRuleName, strict, description);
endDisabledRange(line, disabledRuleName, strict);
});
} else {
Expand All @@ -154,7 +159,7 @@ module.exports = function (root, result) {
});
}

startDisabledRange(line, ruleName, true);
startDisabledRange(line, ruleName, true, description);
endDisabledRange(line, ruleName, true);
}
}
Expand All @@ -163,6 +168,8 @@ module.exports = function (root, result) {
* @param {PostcssComment} comment
*/
function processDisableCommand(comment) {
const description = getDescription(comment.text);

getCommandRules(disableCommand, comment.text).forEach((ruleToDisable) => {
const isAllRules = ruleToDisable === ALL_RULES;

Expand All @@ -182,10 +189,10 @@ module.exports = function (root, result) {

if (isAllRules) {
Object.keys(disabledRanges).forEach((ruleName) => {
startDisabledRange(line, ruleName, ruleName === ALL_RULES);
startDisabledRange(line, ruleName, ruleName === ALL_RULES, description);
});
} else {
startDisabledRange(line, ruleToDisable, true);
startDisabledRange(line, ruleToDisable, true, description);
}
}
});
Expand Down Expand Up @@ -225,8 +232,8 @@ module.exports = function (root, result) {
if (ruleIsDisabled(ALL_RULES) && disabledRanges[ruleToEnable] === undefined) {
// Get a starting point from the where all rules were disabled
if (!disabledRanges[ruleToEnable]) {
disabledRanges[ruleToEnable] = disabledRanges.all.map(({ start, end }) =>
createDisableRange(start, false, end, false),
disabledRanges[ruleToEnable] = disabledRanges.all.map(({ start, end, description }) =>
createDisableRange(start, false, description, end, false),
);
} else {
const range = _.last(disabledRanges[ALL_RULES]);
Expand Down Expand Up @@ -297,13 +304,26 @@ module.exports = function (root, result) {
return rules;
}

/**
* @param {string} fullText
* @returns {string|undefined}
*/
function getDescription(fullText) {
const descriptionStart = fullText.indexOf('--');

if (descriptionStart === -1) return;

return fullText.slice(descriptionStart + 2).trim();
}

/**
* @param {number} line
* @param {string} ruleName
* @param {boolean} strict
* @param {string|undefined} description
*/
function startDisabledRange(line, ruleName, strict) {
const rangeObj = createDisableRange(line, strict);
function startDisabledRange(line, ruleName, strict, description) {
const rangeObj = createDisableRange(line, strict, description);

ensureRuleRanges(ruleName);
disabledRanges[ruleName].push(rangeObj);
Expand Down Expand Up @@ -331,8 +351,8 @@ module.exports = function (root, result) {
*/
function ensureRuleRanges(ruleName) {
if (!disabledRanges[ruleName]) {
disabledRanges[ruleName] = disabledRanges.all.map(({ start, end }) =>
createDisableRange(start, false, end, false),
disabledRanges[ruleName] = disabledRanges.all.map(({ start, end, description }) =>
createDisableRange(start, false, description, end, false),
);
}
}
Expand Down

0 comments on commit 0a17b64

Please sign in to comment.