diff --git a/lib/__tests__/__snapshots__/cli.test.js.snap b/lib/__tests__/__snapshots__/cli.test.js.snap index 8daa701743..2cda5d64de 100644 --- a/lib/__tests__/__snapshots__/cli.test.js.snap +++ b/lib/__tests__/__snapshots__/cli.test.js.snap @@ -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. diff --git a/lib/__tests__/cli.test.js b/lib/__tests__/cli.test.js index ba81d44264..9599f39d4e 100644 --- a/lib/__tests__/cli.test.js +++ b/lib/__tests__/cli.test.js @@ -26,6 +26,7 @@ describe('buildCLI', () => { ignoreDisables: false, printConfig: false, quiet: false, + reportDescriptionlessDisables: false, reportInvalidScopeDisables: false, reportNeedlessDisables: false, stdin: false, @@ -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`]); diff --git a/lib/__tests__/descriptionlessDisables.test.js b/lib/__tests__/descriptionlessDisables.test.js new file mode 100644 index 0000000000..d1983b83f3 --- /dev/null +++ b/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', + }, + ]); + }); +}); diff --git a/lib/__tests__/disableRanges.test.js b/lib/__tests__/disableRanges.test.js index d31bea78e8..d30a669b82 100644 --- a/lib/__tests__/disableRanges.test.js +++ b/lib/__tests__/disableRanges.test.js @@ -653,6 +653,7 @@ it('disable (with description) without re-enabling', () => { { start: 1, strictStart: true, + description: 'Description', }, ], }); @@ -672,6 +673,7 @@ it('disable and re-enable (with descriptions)', () => { end: 4, strictStart: true, strictEnd: true, + description: 'Description', }, ], }); @@ -689,6 +691,7 @@ it('disable rule (with description) without re-enabling', () => { strictStart: true, end: undefined, strictEnd: undefined, + description: 'Description', }, ], }); @@ -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', }, ], }); @@ -741,6 +747,7 @@ it('SCSS // line-disabling comment (with description)', () => { end: 2, strictStart: true, strictEnd: true, + description: 'Description', }, ], }); @@ -767,6 +774,7 @@ it('SCSS // disable next-line comment (with multi-line description)', () => { end: 5, strictStart: true, strictEnd: true, + description: 'Long-winded description', }, ], }); @@ -790,6 +798,7 @@ it('Less // line-disabling comment (with description)', () => { end: 2, strictStart: true, strictEnd: true, + description: 'Description', }, ], }); @@ -816,6 +825,7 @@ it('Less // disable next-line comment (with multi-line description)', () => { end: 5, strictStart: true, strictEnd: true, + description: 'Long-winded description', }, ], }); diff --git a/lib/assignDisabledRanges.js b/lib/assignDisabledRanges.js index 0f1b4352ba..9405207f00 100644 --- a/lib/assignDisabledRanges.js +++ b/lib/assignDisabledRanges.js @@ -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, }; } @@ -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); }); } } @@ -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); }); } } @@ -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', @@ -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 { @@ -154,7 +159,7 @@ module.exports = function (root, result) { }); } - startDisabledRange(line, ruleName, true); + startDisabledRange(line, ruleName, true, description); endDisabledRange(line, ruleName, true); } } @@ -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; @@ -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); } } }); @@ -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]); @@ -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); @@ -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), ); } } diff --git a/lib/cli.js b/lib/cli.js index 7c5a40608e..80189a7759 100644 --- a/lib/cli.js +++ b/lib/cli.js @@ -39,6 +39,7 @@ const EXIT_CODE_ERROR = 2; * @property {string} [stdinFilename] * @property {boolean} [reportNeedlessDisables] * @property {boolean} [reportInvalidScopeDisables] + * @property {boolean} [reportDescriptionlessDisables] * @property {number} [maxWarnings] * @property {string | boolean} quiet * @property {string} [syntax] @@ -73,6 +74,7 @@ const EXIT_CODE_ERROR = 2; * @property {string} [outputFile] * @property {boolean} [reportNeedlessDisables] * @property {boolean} [reportInvalidScopeDisables] + * @property {boolean} [reportDescriptionlessDisables] * @property {boolean} [disableDefaultIgnores] * @property {number} [maxWarnings] * @property {string} [syntax] @@ -204,6 +206,11 @@ const meowOptions = { Report stylelint-disable comments that used for rules that don't exist within the configuration object. The process will exit with code ${EXIT_CODE_ERROR} if invalid scope disables are found. + --report-descriptionless-disables, --rdd + + Report stylelint-disable comments without a description. + The process will exit with code ${EXIT_CODE_ERROR} if descriptionless disables are found. + --max-warnings, --mw Number of warnings above which the process will exit with code ${EXIT_CODE_ERROR}. @@ -292,6 +299,10 @@ const meowOptions = { alias: 'q', type: 'boolean', }, + reportDescriptionlessDisables: { + alias: 'rdd', + type: 'boolean', + }, reportInvalidScopeDisables: { alias: 'risd', type: 'boolean', @@ -417,6 +428,7 @@ module.exports = (argv) => { const reportNeedlessDisables = cli.flags.reportNeedlessDisables; const reportInvalidScopeDisables = cli.flags.reportInvalidScopeDisables; + const reportDescriptionlessDisables = cli.flags.reportDescriptionlessDisables; if (reportNeedlessDisables) { optionsBase.reportNeedlessDisables = reportNeedlessDisables; @@ -426,6 +438,10 @@ module.exports = (argv) => { optionsBase.reportInvalidScopeDisables = reportInvalidScopeDisables; } + if (reportDescriptionlessDisables) { + optionsBase.reportDescriptionlessDisables = reportDescriptionlessDisables; + } + const maxWarnings = cli.flags.maxWarnings; if (maxWarnings !== undefined) { @@ -510,6 +526,17 @@ module.exports = (argv) => { } } + if (reportDescriptionlessDisables) { + const report = disableOptionsReportStringFormatter( + linted.descriptionlessDisables || [], + 'descriptionless disable', + ); + + if (report) { + reports.push(report); + } + } + if (reports.length > 0) { reports.forEach((report) => process.stdout.write(report)); process.exitCode = EXIT_CODE_ERROR; diff --git a/lib/descriptionlessDisables.js b/lib/descriptionlessDisables.js new file mode 100644 index 0000000000..6bfb634cce --- /dev/null +++ b/lib/descriptionlessDisables.js @@ -0,0 +1,52 @@ +'use strict'; + +/** @typedef {import('stylelint').RangeType} RangeType */ +/** @typedef {import('stylelint').DisableReportRange} DisableReportRange */ +/** @typedef {import('stylelint').StylelintDisableOptionsReport} StylelintDisableOptionsReport */ + +/** + * @param {import('stylelint').StylelintResult[]} results + * @returns {StylelintDisableOptionsReport} + */ +module.exports = function (results) { + /** @type {StylelintDisableOptionsReport} */ + const report = []; + + results.forEach((result) => { + // File with `CssSyntaxError` have not `_postcssResult` + if (!result._postcssResult) { + return; + } + + const rangeData = result._postcssResult.stylelint.disabledRanges; + + /** @type {import('stylelint').StylelintDisableReportEntry} */ + const entry = { source: result.source, ranges: [] }; + + Object.keys(rangeData).forEach((rule) => { + rangeData[rule].forEach((range) => { + if (range.description) return; + + // Avoid duplicates from stylelint-disable comments with multiple rules. + const alreadyReported = entry.ranges.find((existing) => { + return existing.start === range.start && existing.end === range.end; + }); + + if (alreadyReported) return; + + entry.ranges.push({ + rule, + start: range.start, + end: range.end, + unusedRule: rule, + }); + }); + }); + + if (entry.ranges.length > 0) { + report.push(entry); + } + }); + + return report; +}; diff --git a/lib/prepareReturnValue.js b/lib/prepareReturnValue.js index 2153c4c1de..9319433cb2 100644 --- a/lib/prepareReturnValue.js +++ b/lib/prepareReturnValue.js @@ -1,5 +1,6 @@ 'use strict'; +const descriptionlessDisables = require('./descriptionlessDisables'); const invalidScopeDisables = require('./invalidScopeDisables'); const needlessDisables = require('./needlessDisables'); const reportDisables = require('./reportDisables'); @@ -17,7 +18,12 @@ const reportDisables = require('./reportDisables'); * @returns {StylelintStandaloneReturnValue} */ function prepareReturnValue(stylelintResults, options, formatter) { - const { reportNeedlessDisables, reportInvalidScopeDisables, maxWarnings } = options; + const { + reportNeedlessDisables, + reportInvalidScopeDisables, + reportDescriptionlessDisables, + maxWarnings, + } = options; const errored = stylelintResults.some( (result) => result.errored || result.parseErrors.length > 0, @@ -39,6 +45,10 @@ function prepareReturnValue(stylelintResults, options, formatter) { returnValue.invalidScopeDisables = invalidScopeDisables(stylelintResults); } + if (reportDescriptionlessDisables) { + returnValue.descriptionlessDisables = descriptionlessDisables(stylelintResults); + } + if (maxWarnings !== undefined) { const foundWarnings = stylelintResults.reduce((count, file) => { return count + file.warnings.length; diff --git a/lib/standalone.js b/lib/standalone.js index 34190d312b..16d84aa9bb 100644 --- a/lib/standalone.js +++ b/lib/standalone.js @@ -47,6 +47,7 @@ module.exports = function (options) { const ignoreDisables = options.ignoreDisables; const reportNeedlessDisables = options.reportNeedlessDisables; const reportInvalidScopeDisables = options.reportInvalidScopeDisables; + const reportDescriptionlessDisables = options.reportDescriptionlessDisables; const syntax = options.syntax; const allowEmptyInput = options.allowEmptyInput || false; const useCache = options.cache || false; @@ -95,6 +96,7 @@ module.exports = function (options) { ignorePath: ignoreFilePath, reportNeedlessDisables, reportInvalidScopeDisables, + reportDescriptionlessDisables, syntax, customSyntax, fix, diff --git a/types/stylelint/index.d.ts b/types/stylelint/index.d.ts index 7ee82c577a..800abaa67e 100644 --- a/types/stylelint/index.d.ts +++ b/types/stylelint/index.d.ts @@ -36,6 +36,7 @@ declare module 'stylelint' { end?: number; strictEnd?: boolean; rules?: string[]; + description?: string; }; export type DisabledRangeObject = { @@ -53,6 +54,7 @@ declare module 'stylelint' { ignored?: boolean; ignoreDisables?: boolean; reportNeedlessDisables?: boolean; + reportDescriptionlessDisables?: boolean; stylelintError?: boolean; disableWritingFix?: boolean; config?: StylelintConfig; @@ -101,6 +103,7 @@ declare module 'stylelint' { ignorePath?: string; reportInvalidScopeDisables?: boolean; reportNeedlessDisables?: boolean; + reportDescriptionlessDisables?: boolean; syntax?: string; customSyntax?: CustomSyntax; fix?: boolean; @@ -164,6 +167,7 @@ declare module 'stylelint' { ignoreDisables?: boolean; ignorePath?: string; ignorePattern?: string[]; + reportDescriptionlessDisables?: boolean; reportNeedlessDisables?: boolean; reportInvalidScopeDisables?: boolean; maxWarnings?: number; @@ -243,6 +247,7 @@ declare module 'stylelint' { foundWarnings: number; }; reportedDisables: StylelintDisableOptionsReport; + descriptionlessDisables?: StylelintDisableOptionsReport; needlessDisables?: StylelintDisableOptionsReport; invalidScopeDisables?: StylelintDisableOptionsReport; };