From e7a29c5e8cf93aa6e376c36f4392126a99f563b3 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Thu, 4 Feb 2021 15:06:20 -0800 Subject: [PATCH] Allow disable reporting to be controlled from the config file This will also make it easier to control disables on a file-by-file basis once #3128 is implemented. It will also make it possible to add finer-grained configuration to these rules in the future. --- docs/user-guide/configure.md | 78 +++++++++++++++++++ lib/__tests__/descriptionlessDisables.test.js | 65 ++++++++++++++++ lib/__tests__/ignoreDisables.test.js | 23 ++++++ lib/__tests__/invalidScopeDisables.test.js | 45 +++++++++++ lib/__tests__/needlessDisables.test.js | 49 ++++++++++++ lib/createStylelint.js | 18 +++++ lib/descriptionlessDisables.js | 8 +- lib/invalidScopeDisables.js | 13 ++-- lib/lintPostcssResult.js | 8 -- lib/needlessDisables.js | 12 ++- lib/prepareReturnValue.js | 16 +--- lib/utils/report.js | 11 ++- types/stylelint/index.d.ts | 11 +-- 13 files changed, 316 insertions(+), 41 deletions(-) diff --git a/docs/user-guide/configure.md b/docs/user-guide/configure.md index 4f87595863..00b47fd30e 100644 --- a/docs/user-guide/configure.md +++ b/docs/user-guide/configure.md @@ -168,6 +168,84 @@ You can set the default severity level for all rules that do not have a severity } ``` +## `ignoreDisables` + +Ignore `styleline-disable` (e.g. `/* stylelint-disable block-no-empty */`) comments. + +For example: + +```json +{ + "ignoreDisables": true +} +``` + +## `reportNeedlessDisables` + +Emit errors for `stylelint-disable` comments that don't include actually match +any lints that need to be disabled. + +For example: + +```json +{ + "reportNeedlessDisables": true +} +``` + +## `reportInvalidScopeDisables` + +Emit errors for `stylelint-disable` comments that don't match rules that are +specified in the configuration object. + +For example: + +```json +{ + "reportInvalidScopeDisables": true +} +``` + +## `reportDescriptionlessDisables` + +Emit errors for `stylelint-disable` comments without a description. + +For example, when the configuration `{ block-no-empty: true }` is given, the following patterns are reported: + + +```css +/* stylelint-disable */ +a {} +``` + + +```css +/* stylelint-disable-next-line block-no-empty */ +a {} +``` + +But, the following patterns (`stylelint-disable -- `) are _not_ reported: + + +```css +/* stylelint-disable -- This violation is ignorable. */ +a {} +``` + + +```css +/* stylelint-disable-next-line block-no-empty -- This violation is ignorable. */ +a {} +``` + +For example: + +```json +{ + "reportDescriptionlessDisables": true +} +``` + ## `extends` You can _extend_ an existing configuration (whether your own or a third-party one). diff --git a/lib/__tests__/descriptionlessDisables.test.js b/lib/__tests__/descriptionlessDisables.test.js index b515bc79d7..ce23250fea 100644 --- a/lib/__tests__/descriptionlessDisables.test.js +++ b/lib/__tests__/descriptionlessDisables.test.js @@ -67,3 +67,68 @@ it('descriptionlessDisables', () => { ]); }); }); + +it('descriptionlessDisables from config', () => { + const config = { + reportDescriptionlessDisables: true, + 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, + }).then((linted) => { + const results = linted.results; + + expect(results).toHaveLength(1); + const warnings = results[0].warnings.filter( + (warning) => warning.rule === '--report-descriptionless-disables', + ); + + expect(warnings).toEqual([ + { + line: 12, + column: 1, + rule: '--report-descriptionless-disables', + severity: 'error', + text: 'Disable for "all" is missing a description', + }, + { + line: 16, + column: 8, + rule: '--report-descriptionless-disables', + severity: 'error', + text: 'Disable for "block-no-empty" is missing a description', + }, + { + line: 18, + column: 1, + rule: '--report-descriptionless-disables', + severity: 'error', + text: 'Disable for "block-no-empty" is missing a description', + }, + ]); + }); +}); diff --git a/lib/__tests__/ignoreDisables.test.js b/lib/__tests__/ignoreDisables.test.js index 4af3d3f971..2ad8246dcc 100644 --- a/lib/__tests__/ignoreDisables.test.js +++ b/lib/__tests__/ignoreDisables.test.js @@ -68,3 +68,26 @@ describe('ignoreDisables with standalone', () => { expect(results[0].warnings[1].text.indexOf('block-no-empty')).not.toBe(1); }); }); + +describe('ignoreDisables with config', () => { + let results; + + beforeEach(() => { + return standalone({ + config: { ignoreDisables: true, ...config }, + code: css, + }).then((data) => (results = data.results)); + }); + + it('expected number of warnings', () => { + expect(results[0].warnings).toHaveLength(2); + }); + + it('expected rule 0', () => { + expect(results[0].warnings[0].text.indexOf('block-no-empty')).not.toBe(1); + }); + + it('expected rule 1', () => { + expect(results[0].warnings[1].text.indexOf('block-no-empty')).not.toBe(1); + }); +}); diff --git a/lib/__tests__/invalidScopeDisables.test.js b/lib/__tests__/invalidScopeDisables.test.js index ffd888b73b..4180e84841 100644 --- a/lib/__tests__/invalidScopeDisables.test.js +++ b/lib/__tests__/invalidScopeDisables.test.js @@ -54,6 +54,51 @@ it('invalidScopeDisables simple case', () => { }); }); +it('invalidScopeDisables from config', () => { + const config = { + reportInvalidScopeDisables: true, + rules: { + 'block-closing-brace-newline-after': ['always'], + 'block-closing-brace-newline-before': ['always'], + }, + }; + + const css = stripIndent` + /* stylelint-disable block-no-empty */ + a {} + /* stylelint-enable block-no-empty */ + a { + b {} /* stylelint-disable-line block-no-empty */ + } + `; + + return standalone({ + config, + code: css, + }).then((linted) => { + const results = linted.results; + + expect(results).toHaveLength(1); + + expect(invalidScopeDisables(results[0].warnings)).toEqual([ + { + line: 1, + column: 1, + rule: '--report-invalid-scope-disables', + text: 'Rule "block-no-empty" isn\'t enabled', + severity: 'error', + }, + { + line: 5, + column: 8, + rule: '--report-invalid-scope-disables', + text: 'Rule "block-no-empty" isn\'t enabled', + severity: 'error', + }, + ]); + }); +}); + it('invalidScopeDisables complex case', () => { const config = { rules: { diff --git a/lib/__tests__/needlessDisables.test.js b/lib/__tests__/needlessDisables.test.js index dc222bb624..cb6719aa4c 100644 --- a/lib/__tests__/needlessDisables.test.js +++ b/lib/__tests__/needlessDisables.test.js @@ -58,6 +58,55 @@ it('needlessDisables simple case', () => { }); }); +it('needlessDisables with config', () => { + const config = { + reportNeedlessDisables: true, + rules: { 'block-no-empty': true }, + }; + + const css = stripIndent` + /* stylelint-disable */ + a {} + /* stylelint-enable */ + a { + b {} /* stylelint-disable-line block-no-empty */ + } + /* stylelint-disable */ + a { color: pink; } + /* stylelint-enable */ + a { + b { color: pink; } /* stylelint-disable-line block-no-empty */ + } + `; + + return standalone({ + config, + code: css, + }).then((linted) => { + const results = linted.results; + + expect(results).toHaveLength(1); + const warnings = results[0].warnings; + + expect(warnings).toEqual([ + { + line: 7, + column: 1, + text: 'Needless disable for "all"', + rule: '--report-needless-disables', + severity: 'error', + }, + { + line: 11, + column: 22, + text: 'Needless disable for "block-no-empty"', + rule: '--report-needless-disables', + severity: 'error', + }, + ]); + }); +}); + it('needlessDisables with multiple rules', () => { const config = { rules: { 'block-no-empty': true, 'color-named': true }, diff --git a/lib/createStylelint.js b/lib/createStylelint.js index 532ac28e45..310c0526b3 100644 --- a/lib/createStylelint.js +++ b/lib/createStylelint.js @@ -26,6 +26,24 @@ module.exports = function (options = {}) { /** @type {Partial} */ const stylelint = { _options: options }; + options.configOverrides = options.configOverrides || {}; + + if (options.ignoreDisables) { + options.configOverrides.ignoreDisables = options.ignoreDisables; + } + + if (options.reportNeedlessDisables) { + options.configOverrides.reportNeedlessDisables = options.reportNeedlessDisables; + } + + if (options.reportInvalidScopeDisables) { + options.configOverrides.reportInvalidScopeDisables = options.reportInvalidScopeDisables; + } + + if (options.reportDescriptionlessDisables) { + options.configOverrides.reportDescriptionlessDisables = options.reportDescriptionlessDisables; + } + // Two separate explorers so they can each have their own transform // function whose results are cached by cosmiconfig stylelint._fullExplorer = cosmiconfig('stylelint', { diff --git a/lib/descriptionlessDisables.js b/lib/descriptionlessDisables.js index 545722d7b1..a60f8a24f2 100644 --- a/lib/descriptionlessDisables.js +++ b/lib/descriptionlessDisables.js @@ -15,7 +15,13 @@ module.exports = function (results) { return; } - const rangeData = result._postcssResult.stylelint.disabledRanges; + const stylelintResult = result._postcssResult.stylelint; + + if (!stylelintResult.config) return; // Linting error + + if (!stylelintResult.config.reportDescriptionlessDisables) return; + + const rangeData = stylelintResult.disabledRanges; /** @type {Set} */ const alreadyReported = new Set(); diff --git a/lib/invalidScopeDisables.js b/lib/invalidScopeDisables.js index 401e2fd42f..9aad38cf4c 100644 --- a/lib/invalidScopeDisables.js +++ b/lib/invalidScopeDisables.js @@ -12,18 +12,19 @@ module.exports = function (results) { return; } - if (!result._postcssResult.stylelint.config) { - // Linting error - return; - } + const stylelintResult = result._postcssResult.stylelint; + + if (!stylelintResult.config) return; // Linting error + + if (!stylelintResult.config.reportInvalidScopeDisables) return; - const configRules = result._postcssResult.stylelint.config.rules || {}; + const configRules = stylelintResult.config.rules || {}; const usedRules = new Set(Object.keys(configRules)); usedRules.add('all'); - const rangeData = result._postcssResult.stylelint.disabledRanges; + const rangeData = stylelintResult.disabledRanges; const disabledRules = Object.keys(rangeData); disabledRules.forEach((rule) => { diff --git a/lib/lintPostcssResult.js b/lib/lintPostcssResult.js index b3e8f8051b..14930322bf 100644 --- a/lib/lintPostcssResult.js +++ b/lib/lintPostcssResult.js @@ -40,14 +40,6 @@ function lintPostcssResult(stylelintOptions, postcssResult, config) { assignDisabledRanges(postcssDoc, postcssResult); } - if (stylelintOptions.ignoreDisables) { - postcssResult.stylelint.ignoreDisables = true; - } - - if (stylelintOptions.reportNeedlessDisables) { - postcssResult.stylelint.reportNeedlessDisables = true; - } - const isFileFixCompatible = isFixCompatible(postcssResult); if (!isFileFixCompatible) { diff --git a/lib/needlessDisables.js b/lib/needlessDisables.js index 62c7815d15..a798e1f803 100644 --- a/lib/needlessDisables.js +++ b/lib/needlessDisables.js @@ -18,14 +18,20 @@ module.exports = function (results) { return; } + const stylelintResult = result._postcssResult.stylelint; + + if (!stylelintResult.config) return; // Linting error + + if (!stylelintResult.config.reportNeedlessDisables) return; + /** @type {{[ruleName: string]: Array}} */ - const rangeData = _.cloneDeep(result._postcssResult.stylelint.disabledRanges); + const rangeData = _.cloneDeep(stylelintResult.disabledRanges); if (!rangeData) { return; } - const disabledWarnings = result._postcssResult.stylelint.disabledWarnings || []; + const disabledWarnings = stylelintResult.disabledWarnings || []; // A map from `stylelint-disable` comments to the set of rules that // are usefully disabled by each comment. We track this @@ -72,7 +78,7 @@ module.exports = function (results) { // Only emit a warning if this range's comment isn't useful for this rule. // For the special rule "all", only emit a warning if it's not useful for - // *any* // rules, becuase it covers all of them. + // *any* rules, becuase it covers all of them. if (rule === 'all' ? useful.size !== 0 : useful.has(rule)) continue; // If the comment doesn't have a location, we can't report a useful error. diff --git a/lib/prepareReturnValue.js b/lib/prepareReturnValue.js index 891fde871e..65273aec1a 100644 --- a/lib/prepareReturnValue.js +++ b/lib/prepareReturnValue.js @@ -18,20 +18,12 @@ const reportDisables = require('./reportDisables'); * @returns {StylelintStandaloneReturnValue} */ function prepareReturnValue(stylelintResults, options, formatter) { - const { - reportNeedlessDisables, - reportInvalidScopeDisables, - reportDescriptionlessDisables, - maxWarnings, - } = options; + const { maxWarnings } = options; reportDisables(stylelintResults); - - if (reportNeedlessDisables) needlessDisables(stylelintResults); - - if (reportInvalidScopeDisables) invalidScopeDisables(stylelintResults); - - if (reportDescriptionlessDisables) descriptionlessDisables(stylelintResults); + needlessDisables(stylelintResults); + invalidScopeDisables(stylelintResults); + descriptionlessDisables(stylelintResults); const errored = stylelintResults.some( (result) => diff --git a/lib/utils/report.js b/lib/utils/report.js index ad09ef932a..6492c5df1d 100644 --- a/lib/utils/report.js +++ b/lib/utils/report.js @@ -50,10 +50,9 @@ module.exports = function (violation) { // line number that the complaint pertains to const startLine = line || node.positionBy({ index }).line; - if ( - result.stylelint.disabledRanges && - (!result.stylelint.ignoreDisables || result.stylelint.reportNeedlessDisables) - ) { + const { ignoreDisables, reportNeedlessDisables } = result.stylelint.config || {}; + + if (result.stylelint.disabledRanges && (!ignoreDisables || reportNeedlessDisables)) { const ranges = result.stylelint.disabledRanges[ruleName] || result.stylelint.disabledRanges.all; for (const range of ranges) { @@ -65,7 +64,7 @@ module.exports = function (violation) { (range.end === undefined || range.end >= startLine) && (!range.rules || range.rules.includes(ruleName)) ) { - if (result.stylelint.reportNeedlessDisables) { + if (reportNeedlessDisables) { // Collect disabled warnings // Used to report `needlessDisables` in subsequent processing. const disabledWarnings = @@ -76,7 +75,7 @@ module.exports = function (violation) { line: startLine, }); - if (!result.stylelint.ignoreDisables) { + if (!ignoreDisables) { return; } diff --git a/types/stylelint/index.d.ts b/types/stylelint/index.d.ts index 64b8f9e3a5..581128435a 100644 --- a/types/stylelint/index.d.ts +++ b/types/stylelint/index.d.ts @@ -26,6 +26,10 @@ declare module 'stylelint' { resultProcessors?: Array; quiet?: boolean; defaultSeverity?: string; + ignoreDisables?: boolean; + reportNeedlessDisables?: boolean; + reportInvalidScopeDisables?: boolean; + reportDescriptionlessDisables?: boolean; }; export type CosmiconfigResult = { config: StylelintConfig; filepath: string }; @@ -53,9 +57,6 @@ declare module 'stylelint' { disabledRanges: DisabledRangeObject; disabledWarnings?: DisabledWarning[]; ignored?: boolean; - ignoreDisables?: boolean; - reportNeedlessDisables?: boolean; - reportDescriptionlessDisables?: boolean; stylelintError?: boolean; disableWritingFix?: boolean; config?: StylelintConfig; @@ -106,7 +107,7 @@ declare module 'stylelint' { config?: StylelintConfig; configFile?: string; configBasedir?: string; - configOverrides?: Object; + configOverrides?: StylelintConfig; ignoreDisables?: boolean; ignorePath?: string; reportInvalidScopeDisables?: boolean; @@ -170,7 +171,7 @@ declare module 'stylelint' { config?: StylelintConfig; configFile?: string; configBasedir?: string; - configOverrides?: Object; + configOverrides?: StylelintConfig; printConfig?: string; ignoreDisables?: boolean; ignorePath?: string;