From 5b224c2dec407dc87d283ba00494e894912dea18 Mon Sep 17 00:00:00 2001 From: Ivan Kopeykin Date: Mon, 29 Jul 2019 23:18:24 +0300 Subject: [PATCH 1/9] add reportInvalidScopeDisables flag, tests --- docs/user-guide/node-api.md | 8 ++ flow-typed/stylelint.js | 7 +- .../.stylelintignore | 0 .../disabled-ranges-1.css | 0 .../disabled-ranges-2.css | 0 .../disabled-ranges-3.css | 0 .../ignored-file.css | 0 lib/__tests__/invalidScopeDisables.test.js | 71 ++++++++++++ lib/assignDisabledRanges.js | 103 ++++++++++++------ lib/cli.js | 38 ++++++- ...=> disableOptionsReportStringFormatter.js} | 6 +- ...=> disableOptionsReportStringFormatter.js} | 2 +- lib/invalidScopeDisables.js | 61 +++++++++++ lib/needlessDisables.js | 2 +- lib/standalone.js | 10 ++ 15 files changed, 264 insertions(+), 44 deletions(-) rename lib/__tests__/fixtures/{needlessDisables => disableOptions}/.stylelintignore (100%) rename lib/__tests__/fixtures/{needlessDisables => disableOptions}/disabled-ranges-1.css (100%) rename lib/__tests__/fixtures/{needlessDisables => disableOptions}/disabled-ranges-2.css (100%) rename lib/__tests__/fixtures/{needlessDisables => disableOptions}/disabled-ranges-3.css (100%) rename lib/__tests__/fixtures/{needlessDisables => disableOptions}/ignored-file.css (100%) create mode 100644 lib/__tests__/invalidScopeDisables.test.js rename lib/formatters/__tests__/{needlessDisablesStringFormatter.test.js => disableOptionsReportStringFormatter.js} (83%) rename lib/formatters/{needlessDisablesStringFormatter.js => disableOptionsReportStringFormatter.js} (94%) create mode 100644 lib/invalidScopeDisables.js diff --git a/docs/user-guide/node-api.md b/docs/user-guide/node-api.md index 0ddcfd78e7..94f208b4c7 100644 --- a/docs/user-guide/node-api.md +++ b/docs/user-guide/node-api.md @@ -118,6 +118,14 @@ Use this report to clean up your codebase, keeping only the stylelint-disable co *The recommended way to use this option is through the CLI.* It will output a clean report to the console. +### `reportInvalidScopeDisables` + +If `true`, the returned data will contain a `invalidScopeDisables` property, whose value is an array of objects, one for each source, with tells you which rule in `stylelint-disable ` comment don't exist within the configuration object. + +Use this report to clean up your codebase, keeping only the stylelint-disable comments that serve a purpose. + +*The recommended way to use this option is through the CLI.* It will output a clean report to the console. + ### `maxWarnings` Sets a limit to the number of warnings accepted. Will add a `maxWarningsExceeded` property to the returned data if the number of found warnings exceeds the given limit. diff --git a/flow-typed/stylelint.js b/flow-typed/stylelint.js index 046f3b9866..d8f117195d 100644 --- a/flow-typed/stylelint.js +++ b/flow-typed/stylelint.js @@ -36,6 +36,7 @@ export type stylelint$options = { configOverrides?: Object, ignoreDisables?: boolean, ignorePath?: string, + reportInvalidScopeDisables?: boolean, reportNeedlessDisables?: boolean, syntax?: stylelint$syntaxes, customSyntax?: string, @@ -107,7 +108,7 @@ export type stylelint$cssSyntaxError = { source: string }; -export type stylelint$needlessDisablesReport = Array<{ +export type stylelint$disableOptionsReport = Array<{ source: string, ranges: Array<{ unusedRule: string, @@ -124,7 +125,8 @@ export type stylelint$standaloneReturnValue = { maxWarnings: number, foundWarnings: number }, - needlessDisables?: stylelint$needlessDisablesReport + needlessDisables?: stylelint$disableOptionsReport, + invalidScopeDisables?: stylelint$disableOptionsReport }; export type stylelint$standaloneOptions = { @@ -143,6 +145,7 @@ export type stylelint$standaloneOptions = { ignorePath?: string, ignorePattern?: RegExp, reportNeedlessDisables?: boolean, + reportInvalidScopeDisables?: boolean, maxWarnings?: number, syntax?: stylelint$syntaxes, customSyntax?: string, diff --git a/lib/__tests__/fixtures/needlessDisables/.stylelintignore b/lib/__tests__/fixtures/disableOptions/.stylelintignore similarity index 100% rename from lib/__tests__/fixtures/needlessDisables/.stylelintignore rename to lib/__tests__/fixtures/disableOptions/.stylelintignore diff --git a/lib/__tests__/fixtures/needlessDisables/disabled-ranges-1.css b/lib/__tests__/fixtures/disableOptions/disabled-ranges-1.css similarity index 100% rename from lib/__tests__/fixtures/needlessDisables/disabled-ranges-1.css rename to lib/__tests__/fixtures/disableOptions/disabled-ranges-1.css diff --git a/lib/__tests__/fixtures/needlessDisables/disabled-ranges-2.css b/lib/__tests__/fixtures/disableOptions/disabled-ranges-2.css similarity index 100% rename from lib/__tests__/fixtures/needlessDisables/disabled-ranges-2.css rename to lib/__tests__/fixtures/disableOptions/disabled-ranges-2.css diff --git a/lib/__tests__/fixtures/needlessDisables/disabled-ranges-3.css b/lib/__tests__/fixtures/disableOptions/disabled-ranges-3.css similarity index 100% rename from lib/__tests__/fixtures/needlessDisables/disabled-ranges-3.css rename to lib/__tests__/fixtures/disableOptions/disabled-ranges-3.css diff --git a/lib/__tests__/fixtures/needlessDisables/ignored-file.css b/lib/__tests__/fixtures/disableOptions/ignored-file.css similarity index 100% rename from lib/__tests__/fixtures/needlessDisables/ignored-file.css rename to lib/__tests__/fixtures/disableOptions/ignored-file.css diff --git a/lib/__tests__/invalidScopeDisables.test.js b/lib/__tests__/invalidScopeDisables.test.js new file mode 100644 index 0000000000..73bd83ccde --- /dev/null +++ b/lib/__tests__/invalidScopeDisables.test.js @@ -0,0 +1,71 @@ +"use strict"; + +const invalidScopeDisables = require("../invalidScopeDisables"); +const path = require("path"); +const standalone = require("../standalone"); +const stripIndent = require("common-tags").stripIndent; + +function fixture(name) { + return path.join(__dirname, "./fixtures/disableOptions", name); +} + +it("invalidScopeDisables simple case", () => { + const config = { + 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 report = invalidScopeDisables(linted.results, config); + + expect(report).toHaveLength(1); + expect(report[0].ranges).toHaveLength(2); + expect(report[0].ranges).toEqual([ + { end: 3, start: 1, unusedRule: "block-no-empty" }, + { end: 5, start: 5, unusedRule: "block-no-empty" } + ]); + }); +}); + +it("invalidScopeDisables complex case", () => { + const config = { + rules: { + "block-no-empty": true + } + }; + + return standalone({ + config, + files: [ + fixture("disabled-ranges-1.css"), + fixture("disabled-ranges-2.css"), + // ignore files contain `CssSyntaxError` + fixture("disabled-ranges-3.css") + ] + }).then(linted => { + expect(invalidScopeDisables(linted.results, config)).toEqual([ + { + source: fixture("disabled-ranges-1.css"), + ranges: [{ start: 1, end: 3, unusedRule: "color-named" }] + }, + { + source: fixture("disabled-ranges-2.css"), + ranges: [{ start: 5, end: 5, unusedRule: "color-named" }] + } + ]); + }); +}); diff --git a/lib/assignDisabledRanges.js b/lib/assignDisabledRanges.js index 9e489f04e4..bc92a28f08 100644 --- a/lib/assignDisabledRanges.js +++ b/lib/assignDisabledRanges.js @@ -10,13 +10,30 @@ const disableLineCommand = COMMAND_PREFIX + "disable-line"; const disableNextLineCommand = COMMAND_PREFIX + "disable-next-line"; const ALL_RULES = "all"; -/*:: type disabledRangeObject = { - [ruleName: string]: Array<{ +/*:: type disableRange = { start: number, end?: number, - }> + strictStart: boolean, + strictEnd?: boolean + } + */ + +/*:: type disabledRangeObject = { + [ruleName: string]: Array<> }*/ +function createDisableRange( + start /*: number*/, + strictStart /*: boolean*/ +) /*: disableRange*/ { + return { + start, + end: undefined, + strictStart, + strictEnd: undefined + }; +} + // Run it like a plugin ... module.exports = function( root /*: Object*/, @@ -66,38 +83,43 @@ module.exports = function( if (ruleName === ALL_RULES) { Object.keys(disabledRanges).forEach(disabledRuleName => { - startDisabledRange(line, disabledRuleName); - endDisabledRange(line, disabledRuleName); + const strict = disabledRuleName !== ALL_RULES; + + startDisabledRange(line, disabledRuleName, strict); + endDisabledRange(line, disabledRuleName, strict); }); } else { - startDisabledRange(line, ruleName); - endDisabledRange(line, ruleName); + startDisabledRange(line, ruleName, true); + endDisabledRange(line, ruleName, true); } } function processDisableCommand(comment /*: postcss$comment*/) { getCommandRules(disableCommand, comment.text).forEach(ruleToDisable => { - if (ruleToDisable === ALL_RULES) { - if (ruleIsDisabled(ALL_RULES)) { - throw comment.error("All rules have already been disabled", { - plugin: "stylelint" - }); - } + const isAllRules = ruleToDisable === ALL_RULES; - Object.keys(disabledRanges).forEach(ruleName => { - startDisabledRange(comment.source.start.line, ruleName); - }); - - return; + if (ruleIsDisabled(ruleToDisable)) { + throw comment.error( + isAllRules + ? "All rules have already been disabled" + : `"${ruleToDisable}" has already been disabled`, + { + plugin: "stylelint" + } + ); } - if (ruleIsDisabled(ruleToDisable)) { - throw comment.error(`"${ruleToDisable}" has already been disabled`, { - plugin: "stylelint" + if (isAllRules) { + Object.keys(disabledRanges).forEach(ruleName => { + startDisabledRange( + comment.source.start.line, + ruleName, + ruleName !== ALL_RULES + ); }); + } else { + startDisabledRange(comment.source.start.line, ruleToDisable, true); } - - startDisabledRange(comment.source.start.line, ruleToDisable); }); } @@ -116,7 +138,11 @@ module.exports = function( Object.keys(disabledRanges).forEach(ruleName => { if (!_.get(_.last(disabledRanges[ruleName]), "end")) { - endDisabledRange(comment.source.end.line, ruleName); + endDisabledRange( + comment.source.end.line, + ruleName, + ruleName !== ALL_RULES + ); } }); @@ -129,20 +155,26 @@ module.exports = function( ) { // Get a starting point from the where all rules were disabled if (!disabledRanges[ruleToEnable]) { - disabledRanges[ruleToEnable] = _.cloneDeep(disabledRanges.all); + disabledRanges[ruleToEnable] = disabledRanges.all.map( + ({ start, end }) => ({ + start, + end, + strict: false + }) + ); } else { disabledRanges[ruleToEnable].push( _.clone(_.last(disabledRanges[ALL_RULES])) ); } - endDisabledRange(comment.source.end.line, ruleToEnable); + endDisabledRange(comment.source.end.line, ruleToEnable, true); return; } if (ruleIsDisabled(ruleToEnable)) { - endDisabledRange(comment.source.end.line, ruleToEnable); + endDisabledRange(comment.source.end.line, ruleToEnable, true); return; } @@ -176,7 +208,7 @@ module.exports = function( function getCommandRules( command /*: string*/, fullText /*: string*/ - ) /*: Array*/ { + ) /*: {rules: Array}*/ { const rules = _.compact(fullText.slice(command.length).split(",")).map(r => r.trim() ); @@ -188,14 +220,22 @@ module.exports = function( return rules; } - function startDisabledRange(line /*: number*/, ruleName /*: string*/) { - const rangeObj = { start: line }; + function startDisabledRange( + line /*: number*/, + ruleName /*: string*/, + strict /*: boolean*/ + ) { + const rangeObj = createDisableRange(line, strict); ensureRuleRanges(ruleName); disabledRanges[ruleName].push(rangeObj); } - function endDisabledRange(line /*: number*/, ruleName /*: string*/) { + function endDisabledRange( + line /*: number*/, + ruleName /*: string*/, + strict /*: boolean*/ + ) { const lastRangeForRule = _.last(disabledRanges[ruleName]); if (!lastRangeForRule) { @@ -204,6 +244,7 @@ module.exports = function( // Add an `end` prop to the last range of that rule lastRangeForRule.end = line; + lastRangeForRule.strictEnd = strict; } function ensureRuleRanges(ruleName /*: string*/) { diff --git a/lib/cli.js b/lib/cli.js index 4b859b5d3b..43a6c6a6a0 100644 --- a/lib/cli.js +++ b/lib/cli.js @@ -3,13 +3,13 @@ const chalk = require("chalk"); const checkInvalidCLIOptions = require("./utils/checkInvalidCLIOptions"); +const disableOptionsReportStringFormatter = require("./formatters/disableOptionsReportStringFormatter"); const dynamicRequire = require("./dynamicRequire"); const EOL = require("os").EOL; const getFormatterOptionsText = require("./utils/getFormatterOptionsText"); const getModulePath = require("./utils/getModulePath"); const getStdin = require("get-stdin"); const meow = require("meow"); -const needlessDisablesStringFormatter = require("./formatters/needlessDisablesStringFormatter"); const path = require("path"); const printConfig = require("./printConfig"); const resolveFrom = require("resolve-from"); @@ -126,7 +126,8 @@ const EXIT_CODE_ERROR = 2; maxWarnings: number, outputFile: string, quiet: any, - reportNeedlessDisables: any, + reportNeedlessDisables: boolean, + reportInvalidScopeDisables: boolean, stdinFilename: any, syntax: any, v: string, @@ -156,7 +157,8 @@ const EXIT_CODE_ERROR = 2; ignoreDisables?: any, ignorePath?: any, outputFile?: string, - reportNeedlessDisables?: any, + reportNeedlessDisables?: boolean, + reportInvalidScopeDisables?: boolean, maxWarnings?: any, syntax?: any, disableDefaultIgnores?: any, @@ -362,6 +364,10 @@ const meowOptions /*: meowOptionsType*/ = { alias: "rd", type: "boolean" }, + "report-invalid-scope-disables": { + alias: "risd", + type: "boolean" + }, "max-warnings": { alias: "mw" }, @@ -483,11 +489,16 @@ module.exports = (argv /*: string[]*/) /*: Promise|void*/ => { } const reportNeedlessDisables = cli.flags.reportNeedlessDisables; + const reportInvalidScopeDisables = cli.flags.reportInvalidScopeDisables; if (reportNeedlessDisables) { optionsBase.reportNeedlessDisables = reportNeedlessDisables; } + if (reportInvalidScopeDisables) { + optionsBase.reportInvalidScopeDisables = reportInvalidScopeDisables; + } + const maxWarnings = cli.flags.maxWarnings; if (maxWarnings !== undefined) { @@ -542,16 +553,31 @@ module.exports = (argv /*: string[]*/) /*: Promise|void*/ => { return standalone(options) .then(linted => { + const reports = []; + if (reportNeedlessDisables) { - const report = needlessDisablesStringFormatter( + const report = disableOptionsReportStringFormatter( linted.needlessDisables ); - process.stdout.write(report); + if (report) { + reports.push(report); + } + } + + if (reportInvalidScopeDisables) { + const report = disableOptionsReportStringFormatter( + linted.invalidScopeDisables + ); if (report) { - process.exitCode = EXIT_CODE_ERROR; + reports.push(report); } + } + + if (reports.length > 0) { + reports.forEach(process.stdout.write.bind(process.stdout)); + process.exitCode = EXIT_CODE_ERROR; return; } diff --git a/lib/formatters/__tests__/needlessDisablesStringFormatter.test.js b/lib/formatters/__tests__/disableOptionsReportStringFormatter.js similarity index 83% rename from lib/formatters/__tests__/needlessDisablesStringFormatter.test.js rename to lib/formatters/__tests__/disableOptionsReportStringFormatter.js index 5a53a88c7c..1ed3d5705e 100644 --- a/lib/formatters/__tests__/needlessDisablesStringFormatter.test.js +++ b/lib/formatters/__tests__/disableOptionsReportStringFormatter.js @@ -1,13 +1,13 @@ "use strict"; -const needlessDisablesStringFormatter = require("../needlessDisablesStringFormatter"); +const disableOptionsReportStringFormatter = require("../disableOptionsReportStringFormatter"); const stripAnsi = require("strip-ansi"); const stripIndent = require("common-tags").stripIndent; -describe("needlessDisablesStringFormatter", () => { +describe("disableOptionsReportStringFormatter", () => { it("formatter stringified", () => { const actual = stripAnsi( - needlessDisablesStringFormatter([ + disableOptionsReportStringFormatter([ { source: "foo", ranges: [ diff --git a/lib/formatters/needlessDisablesStringFormatter.js b/lib/formatters/disableOptionsReportStringFormatter.js similarity index 94% rename from lib/formatters/needlessDisablesStringFormatter.js rename to lib/formatters/disableOptionsReportStringFormatter.js index 3c4a13fabb..ccd6966398 100644 --- a/lib/formatters/needlessDisablesStringFormatter.js +++ b/lib/formatters/disableOptionsReportStringFormatter.js @@ -15,7 +15,7 @@ function logFrom(fromValue /*: string */) /*: string */ { } module.exports = function( - report /*:: ?: stylelint$needlessDisablesReport */ + report /*:: ?: stylelint$disableOptionsReport */ ) /*: string */ { if (!report) return ""; diff --git a/lib/invalidScopeDisables.js b/lib/invalidScopeDisables.js new file mode 100644 index 0000000000..679fc9d517 --- /dev/null +++ b/lib/invalidScopeDisables.js @@ -0,0 +1,61 @@ +/* @flow */ +"use strict"; + +/*:: type rangeDataType = { + all: Array, +} +*/ + +/*:: type rangeType = { + unusedRule: string, + end?: number, + start: number, + used?: boolean, +}*/ + +/*:: type unusedRangeT = { + start: number, + end?: number, +}*/ + +module.exports = function( + results /*: Array*/, + config /*: stylelint$config*/ +) /*: stylelint$disableOptionsReport*/ { + const report = []; + const usedRules = new Set(Object.keys(config.rules || {})); + + usedRules.add("all"); + + results.forEach(result => { + // File with `CssSyntaxError` have not `_postcssResult` + if (!result._postcssResult) { + return; + } + + const sourceReport = { source: result.source, ranges: [] }; + const rangeData /*: rangeDataType*/ = + result._postcssResult.stylelint.disabledRanges; + const disabledRules = Object.keys(rangeData); + + disabledRules.forEach(rule => { + if (usedRules.has(rule)) { + return; + } + + rangeData[rule].forEach((range /*: unusedRangeT */) => { + sourceReport.ranges.push({ + unusedRule: rule, + start: range.start, + end: range.end + }); + }); + }); + + if (sourceReport.ranges.length > 0) { + report.push(sourceReport); + } + }); + + return report; +}; diff --git a/lib/needlessDisables.js b/lib/needlessDisables.js index f6c206e268..97dd37ff1d 100644 --- a/lib/needlessDisables.js +++ b/lib/needlessDisables.js @@ -22,7 +22,7 @@ const _ = require("lodash"); module.exports = function( results /*: Array*/ -) /*: stylelint$needlessDisablesReport*/ { +) /*: stylelint$disableOptionsReport*/ { const report = []; results.forEach(result => { diff --git a/lib/standalone.js b/lib/standalone.js index 13b1605981..ba845758c0 100644 --- a/lib/standalone.js +++ b/lib/standalone.js @@ -13,6 +13,7 @@ const getFormatterOptionsText = require("./utils/getFormatterOptionsText"); const globby /*: Function*/ = require("globby"); const hash = require("./utils/hash"); const ignore = require("ignore"); +const invalidScopeDisables /*: Function*/ = require("./invalidScopeDisables"); const needlessDisables /*: Function*/ = require("./needlessDisables"); const NoFilesFoundError = require("./utils/noFilesFoundError"); const path = require("path"); @@ -41,6 +42,7 @@ module.exports = function( const formatter = options.formatter; const ignoreDisables = options.ignoreDisables; const reportNeedlessDisables = options.reportNeedlessDisables; + const reportInvalidScopeDisables = options.reportInvalidScopeDisables; const maxWarnings = options.maxWarnings; const syntax = options.syntax; const allowEmptyInput = options.allowEmptyInput || false; @@ -100,6 +102,7 @@ module.exports = function( configOverrides, ignoreDisables, reportNeedlessDisables, + reportInvalidScopeDisables, syntax, customSyntax, fix @@ -290,6 +293,13 @@ module.exports = function( returnValue.needlessDisables = needlessDisables(stylelintResults); } + if (reportInvalidScopeDisables) { + returnValue.invalidScopeDisables = invalidScopeDisables( + stylelintResults, + stylelint._options.config + ); + } + if (maxWarnings !== undefined) { const foundWarnings = stylelintResults.reduce((count, file) => { return count + file.warnings.length; From 928f57a940ad55da0515a1328907a7b4b133646b Mon Sep 17 00:00:00 2001 From: Ivan Kopeykin Date: Tue, 30 Jul 2019 12:55:21 +0300 Subject: [PATCH 2/9] add strictStart, strictEnd flags to disableRange --- lib/__tests__/disableRanges.test.js | 283 ++++++++++++++++++++++++---- lib/assignDisabledRanges.js | 28 +-- 2 files changed, 262 insertions(+), 49 deletions(-) diff --git a/lib/__tests__/disableRanges.test.js b/lib/__tests__/disableRanges.test.js index cac168117e..081566a857 100644 --- a/lib/__tests__/disableRanges.test.js +++ b/lib/__tests__/disableRanges.test.js @@ -16,7 +16,8 @@ it("disable without re-enabling", () => { expect(result.stylelint.disabledRanges).toEqual({ all: [ { - start: 1 + start: 1, + strictStart: true } ] }); @@ -30,7 +31,14 @@ it("disable and re-enable", () => { /* stylelint-enable */ .foo {}`).then(result => { expect(result.stylelint.disabledRanges).toEqual({ - all: [{ start: 2, end: 4 }] + all: [ + { + start: 2, + end: 4, + strictStart: true, + strictEnd: true + } + ] }); }); }); @@ -46,7 +54,20 @@ it("disable and re-enable twice", () => { /* stylelint-enable */ .foo {}`).then(result => { expect(result.stylelint.disabledRanges).toEqual({ - all: [{ start: 2, end: 4 }, { start: 6, end: 8 }] + all: [ + { + start: 2, + end: 4, + strictStart: true, + strictEnd: true + }, + { + start: 6, + end: 8, + strictStart: true, + strictEnd: true + } + ] }); }); }); @@ -56,7 +77,14 @@ it("disable rule without re-enabling", () => { .then(result => { expect(result.stylelint.disabledRanges).toEqual({ all: [], - "foo-bar": [{ start: 1 }] + "foo-bar": [ + { + start: 1, + strictStart: true, + end: undefined, + strictEnd: undefined + } + ] }); }) .then(() => { @@ -65,7 +93,14 @@ it("disable rule without re-enabling", () => { ).then(result => { expect(result.stylelint.disabledRanges).toEqual({ all: [], - "selector-combinator-space-before": [{ start: 1 }] + "selector-combinator-space-before": [ + { + start: 1, + strictStart: true, + end: undefined, + strictEnd: undefined + } + ] }); }); }); @@ -83,8 +118,28 @@ it("mixed disabling of specific and all rules, enabling of all", () => { .foo {}`).then(result => { expect(result.stylelint.disabledRanges).toEqual({ all: [], - "foo-bar": [{ start: 2, end: 4 }, { start: 6, end: 8 }], - "baz-maz": [{ start: 6, end: 8 }] + "foo-bar": [ + { + start: 2, + end: 4, + strictStart: true, + strictEnd: false + }, + { + start: 6, + end: 8, + strictStart: true, + strictEnd: false + } + ], + "baz-maz": [ + { + start: 6, + end: 8, + strictStart: true, + strictEnd: false + } + ] }); }); }); @@ -95,9 +150,24 @@ it("disable rules with newline in rule list", () => { ).then(result => { expect(result.stylelint.disabledRanges).toEqual({ all: [], - "foo-bar": [{ start: 1 }], - "hoo-hah": [{ start: 1 }], - slime: [{ start: 1 }] + "foo-bar": [ + { + start: 1, + strictStart: true + } + ], + "hoo-hah": [ + { + start: 1, + strictStart: true + } + ], + slime: [ + { + start: 1, + strictStart: true + } + ] }); }); }); @@ -109,7 +179,9 @@ it("disable single line all rules", () => { all: [ { start: 1, - end: 1 + end: 1, + strictStart: true, + strictEnd: true } ] }, @@ -128,7 +200,9 @@ it("disable single line one rule", () => { "block-no-empty": [ { start: 1, - end: 1 + end: 1, + strictStart: true, + strictEnd: true } ] }, @@ -147,13 +221,17 @@ it("disable single line multiple rules", () => { "block-no-empty": [ { start: 3, - end: 3 + end: 3, + strictStart: true, + strictEnd: true } ], blergh: [ { start: 3, - end: 3 + end: 3, + strictStart: true, + strictEnd: true } ] }, @@ -170,7 +248,9 @@ it("disable next line all rules", () => { all: [ { start: 2, - end: 2 + end: 2, + strictStart: true, + strictEnd: true } ] }, @@ -190,7 +270,9 @@ it("disable next line one rule", () => { "block-no-empty": [ { start: 2, - end: 2 + end: 2, + strictStart: true, + strictEnd: true } ] }, @@ -213,13 +295,17 @@ it("disable next line multiple rules", () => { "block-no-empty": [ { start: 5, - end: 5 + end: 5, + strictStart: true, + strictEnd: true } ], blergh: [ { start: 5, - end: 5 + end: 5, + strictStart: true, + strictEnd: true } ] }, @@ -243,7 +329,9 @@ it("SCSS // line-disabling comment", () => { "declaration-no-important": [ { start: 2, - end: 2 + end: 2, + strictStart: true, + strictEnd: true } ] }); @@ -264,7 +352,9 @@ it("Less // line-disabling comment", () => { "declaration-no-important": [ { start: 2, - end: 2 + end: 2, + strictStart: true, + strictEnd: true } ] }); @@ -280,10 +370,38 @@ it("nested ranges all rule-specific", () => { /* stylelint-enable baz */`).then(result => { expect(result.stylelint.disabledRanges).toEqual({ all: [], - foo: [{ start: 1, end: 5 }], - bar: [{ start: 2, end: 4 }], - baz: [{ start: 3, end: 6 }], - hop: [{ start: 3, end: 5 }] + foo: [ + { + start: 1, + end: 5, + strictStart: true, + strictEnd: true + } + ], + bar: [ + { + start: 2, + end: 4, + strictStart: true, + strictEnd: true + } + ], + baz: [ + { + start: 3, + end: 6, + strictStart: true, + strictEnd: true + } + ], + hop: [ + { + start: 3, + end: 5, + strictStart: true, + strictEnd: true + } + ] }); }); }); @@ -294,8 +412,28 @@ it("nested ranges all for all rules", () => { /* stylelint-disable bar */ /* stylelint-enable */`).then(result => { expect(result.stylelint.disabledRanges).toEqual({ - all: [{ start: 1, end: 4 }], - bar: [{ start: 1, end: 2 }, { start: 3, end: 4 }] + all: [ + { + start: 1, + end: 4, + strictStart: true, + strictEnd: true + } + ], + bar: [ + { + start: 1, + end: 2, + strictStart: false, + strictEnd: true + }, + { + start: 3, + end: 4, + strictStart: true, + strictEnd: false + } + ] }); }); }); @@ -306,9 +444,30 @@ it("nested ranges disable rules enable all", () => { /* stylelint-enable */`).then(result => { expect(result.stylelint.disabledRanges).toEqual({ all: [], - foo: [{ start: 1, end: 3 }], - bar: [{ start: 2, end: 3 }], - baz: [{ start: 2, end: 3 }] + foo: [ + { + start: 1, + end: 3, + strictStart: true, + strictEnd: false + } + ], + bar: [ + { + start: 2, + end: 3, + strictStart: true, + strictEnd: false + } + ], + baz: [ + { + start: 2, + end: 3, + strictStart: true, + strictEnd: false + } + ] }); }); }); @@ -319,9 +478,36 @@ it("nested ranges mix disabling enabling all rules and specific rules", () => { /* stylelint-enable */ /* stylelint-disable bar */`).then(result => { expect(result.stylelint.disabledRanges).toEqual({ - all: [{ start: 1, end: 3 }], - foo: [{ start: 1, end: 2 }], - bar: [{ start: 1, end: 3 }, { start: 4 }] + all: [ + { + start: 1, + end: 3, + strictStart: true, + strictEnd: true + } + ], + foo: [ + { + start: 1, + end: 2, + strictStart: false, + strictEnd: true + } + ], + bar: [ + { + start: 1, + end: 3, + strictStart: false, + strictEnd: false + }, + { + start: 4, + strictStart: true, + strictEnd: undefined, + end: undefined + } + ] }); }); }); @@ -333,9 +519,36 @@ it("nested ranges another mix", () => { /* stylelint-disable foo */ /* stylelint-enable */`).then(result => { expect(result.stylelint.disabledRanges).toEqual({ - all: [{ start: 1, end: 5 }], - foo: [{ start: 1, end: 3 }, { start: 4, end: 5 }], - bar: [{ start: 1, end: 2 }] + all: [ + { + start: 1, + end: 5, + strictStart: true, + strictEnd: true + } + ], + foo: [ + { + start: 1, + end: 3, + strictStart: false, + strictEnd: true + }, + { + start: 4, + end: 5, + strictStart: true, + strictEnd: false + } + ], + bar: [ + { + start: 1, + end: 2, + strictStart: false, + strictEnd: true + } + ] }); }); }); diff --git a/lib/assignDisabledRanges.js b/lib/assignDisabledRanges.js index bc92a28f08..4be27386f3 100644 --- a/lib/assignDisabledRanges.js +++ b/lib/assignDisabledRanges.js @@ -19,18 +19,20 @@ const ALL_RULES = "all"; */ /*:: type disabledRangeObject = { - [ruleName: string]: Array<> + [ruleName: string]: Array }*/ function createDisableRange( start /*: number*/, - strictStart /*: boolean*/ + strictStart /*: boolean*/, + end /*: ?number*/, + strictEnd /*: ?boolean*/ ) /*: disableRange*/ { return { start, - end: undefined, + end: (end && end) || undefined, strictStart, - strictEnd: undefined + strictEnd: typeof strictEnd === "boolean" ? strictEnd : undefined }; } @@ -83,7 +85,7 @@ module.exports = function( if (ruleName === ALL_RULES) { Object.keys(disabledRanges).forEach(disabledRuleName => { - const strict = disabledRuleName !== ALL_RULES; + const strict = disabledRuleName === ALL_RULES; startDisabledRange(line, disabledRuleName, strict); endDisabledRange(line, disabledRuleName, strict); @@ -114,7 +116,7 @@ module.exports = function( startDisabledRange( comment.source.start.line, ruleName, - ruleName !== ALL_RULES + ruleName === ALL_RULES ); }); } else { @@ -141,7 +143,7 @@ module.exports = function( endDisabledRange( comment.source.end.line, ruleName, - ruleName !== ALL_RULES + ruleName === ALL_RULES ); } }); @@ -156,11 +158,7 @@ module.exports = function( // Get a starting point from the where all rules were disabled if (!disabledRanges[ruleToEnable]) { disabledRanges[ruleToEnable] = disabledRanges.all.map( - ({ start, end }) => ({ - start, - end, - strict: false - }) + ({ start, end }) => createDisableRange(start, false, end, false) ); } else { disabledRanges[ruleToEnable].push( @@ -208,7 +206,7 @@ module.exports = function( function getCommandRules( command /*: string*/, fullText /*: string*/ - ) /*: {rules: Array}*/ { + ) /*: Array*/ { const rules = _.compact(fullText.slice(command.length).split(",")).map(r => r.trim() ); @@ -249,7 +247,9 @@ module.exports = function( function ensureRuleRanges(ruleName /*: string*/) { if (!disabledRanges[ruleName]) { - disabledRanges[ruleName] = _.cloneDeep(disabledRanges.all); + disabledRanges[ruleName] = disabledRanges.all.map(({ start, end }) => + createDisableRange(start, false, end, false) + ); } } From 96779a8342381b1ba69f38458d2ccc4fbfdba744 Mon Sep 17 00:00:00 2001 From: Ivan Kopeykin Date: Tue, 30 Jul 2019 13:06:43 +0300 Subject: [PATCH 3/9] fix needlessDisables regarding to new disableRanges --- lib/__tests__/needlessDisables.test.js | 2 +- lib/needlessDisables.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/__tests__/needlessDisables.test.js b/lib/__tests__/needlessDisables.test.js index 59eaead81d..1f86b9fdb0 100644 --- a/lib/__tests__/needlessDisables.test.js +++ b/lib/__tests__/needlessDisables.test.js @@ -6,7 +6,7 @@ const standalone = require("../standalone"); const stripIndent = require("common-tags").stripIndent; function fixture(name) { - return path.join(__dirname, "./fixtures/needlessDisables", name); + return path.join(__dirname, "./fixtures/disableOptions", name); } it("needlessDisables simple case", () => { diff --git a/lib/needlessDisables.js b/lib/needlessDisables.js index 97dd37ff1d..0dafa6c786 100644 --- a/lib/needlessDisables.js +++ b/lib/needlessDisables.js @@ -80,7 +80,7 @@ module.exports = function( // mark this range as unused if (!range.used && !alreadyMarkedUnused) { range.unusedRule = rule; - unused.ranges.push(range); + unused.ranges.push(_.omit(range, "strictStart", "strictEnd")); } // If this range is used but an equivalent has been marked as unused, From 0fa42579130e31317e6dc83abaf5942c03536986 Mon Sep 17 00:00:00 2001 From: Ivan Kopeykin Date: Tue, 30 Jul 2019 13:08:23 +0300 Subject: [PATCH 4/9] fix invalidScopeDisables regarding to new disableRanges --- lib/invalidScopeDisables.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/invalidScopeDisables.js b/lib/invalidScopeDisables.js index 679fc9d517..0b20e1a4bb 100644 --- a/lib/invalidScopeDisables.js +++ b/lib/invalidScopeDisables.js @@ -44,6 +44,10 @@ module.exports = function( } rangeData[rule].forEach((range /*: unusedRangeT */) => { + if (!range.strictStart && !range.strictEnd) { + return; + } + sourceReport.ranges.push({ unusedRule: rule, start: range.start, From 459259cc64a2fe9a00dd998ab153f904810c3a8e Mon Sep 17 00:00:00 2001 From: Ivan Kopeykin Date: Tue, 30 Jul 2019 13:20:58 +0300 Subject: [PATCH 5/9] add invalidScopeDisables tests, flow types fix --- .../fixtures/disableOptions/ignored-file.css | 3 +++ lib/__tests__/invalidScopeDisables.test.js | 22 +++++++++++++++++++ lib/assignDisabledRanges.js | 2 +- lib/invalidScopeDisables.js | 2 ++ 4 files changed, 28 insertions(+), 1 deletion(-) diff --git a/lib/__tests__/fixtures/disableOptions/ignored-file.css b/lib/__tests__/fixtures/disableOptions/ignored-file.css index 47647af08b..5e12653335 100644 --- a/lib/__tests__/fixtures/disableOptions/ignored-file.css +++ b/lib/__tests__/fixtures/disableOptions/ignored-file.css @@ -1,3 +1,6 @@ /* stylelint-disable color-named */ a {} /* stylelint-enable */ +/* stylelint-disable block-no-empty */ +a {} +/* stylelint-enable */ diff --git a/lib/__tests__/invalidScopeDisables.test.js b/lib/__tests__/invalidScopeDisables.test.js index 73bd83ccde..00fdba92ea 100644 --- a/lib/__tests__/invalidScopeDisables.test.js +++ b/lib/__tests__/invalidScopeDisables.test.js @@ -69,3 +69,25 @@ it("invalidScopeDisables complex case", () => { ]); }); }); + +it("invalidScopeDisables ignored case", () => { + const config = { + rules: { + "color-named": "never" + } + }; + + return standalone({ + config, + files: [fixture("disabled-ranges-1.css"), fixture("ignored-file.css")], + ignoreDisables: true, + ignorePath: fixture(".stylelintignore") + }).then(linted => { + expect(invalidScopeDisables(linted.results, config)).toEqual([ + { + source: fixture("disabled-ranges-1.css"), + ranges: [{ start: 5, end: 7, unusedRule: "block-no-empty" }] + } + ]); + }); +}); diff --git a/lib/assignDisabledRanges.js b/lib/assignDisabledRanges.js index 4be27386f3..9b5370780c 100644 --- a/lib/assignDisabledRanges.js +++ b/lib/assignDisabledRanges.js @@ -30,7 +30,7 @@ function createDisableRange( ) /*: disableRange*/ { return { start, - end: (end && end) || undefined, + end: end || undefined, strictStart, strictEnd: typeof strictEnd === "boolean" ? strictEnd : undefined }; diff --git a/lib/invalidScopeDisables.js b/lib/invalidScopeDisables.js index 0b20e1a4bb..9f19b923c6 100644 --- a/lib/invalidScopeDisables.js +++ b/lib/invalidScopeDisables.js @@ -16,6 +16,8 @@ /*:: type unusedRangeT = { start: number, end?: number, + strictStart: boolean, + strictEnd?: boolean }*/ module.exports = function( From 6c9b9c767d32df3e0280a64828cc160836a1441f Mon Sep 17 00:00:00 2001 From: Ivan Kopeykin Date: Tue, 30 Jul 2019 13:57:45 +0300 Subject: [PATCH 6/9] add cli description --- lib/cli.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/cli.js b/lib/cli.js index 43a6c6a6a0..471f203167 100644 --- a/lib/cli.js +++ b/lib/cli.js @@ -279,6 +279,11 @@ const meowOptions /*: meowOptionsType*/ = { Report stylelint-disable comments that are not blocking a lint warning. The process will exit with code ${EXIT_CODE_ERROR} if needless disables are found. + --report-invalid-scope-disables, --risd + + 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. + --max-warnings, --mw Number of warnings above which the process will exit with code ${EXIT_CODE_ERROR}. From e49c91df9038cc5eead622caea9c42fb75168698 Mon Sep 17 00:00:00 2001 From: Ivan Kopeykin Date: Tue, 30 Jul 2019 14:12:36 +0300 Subject: [PATCH 7/9] standalone-invalidScopeDisables test --- .../standalone-invalidScopeDisables.test.js | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 lib/__tests__/standalone-invalidScopeDisables.test.js diff --git a/lib/__tests__/standalone-invalidScopeDisables.test.js b/lib/__tests__/standalone-invalidScopeDisables.test.js new file mode 100644 index 0000000000..5a02ea3aba --- /dev/null +++ b/lib/__tests__/standalone-invalidScopeDisables.test.js @@ -0,0 +1,50 @@ +"use strict"; + +const path = require("path"); +const standalone = require("../standalone"); + +const fixturesPath = path.join(__dirname, "fixtures"); +const config = { + quiet: true, + rules: { + "block-no-empty": true + } +}; + +it("standalone with input css and `reportInvalidScopeDisables`", () => { + return standalone({ + code: "/* stylelint-disable color-named */\na {}", + config, + reportInvalidScopeDisables: true + }).then(linted => { + const invalidScopeDisables = linted.invalidScopeDisables; + + expect(typeof invalidScopeDisables).toBe("object"); + expect(invalidScopeDisables).toHaveLength(1); + expect(invalidScopeDisables[0].ranges).toHaveLength(1); + expect(invalidScopeDisables[0].ranges[0]).toEqual({ + start: 1, + unusedRule: "color-named" + }); + }); +}); + +it("standalone with input file(s) and `reportInvalidScopeDisables`", () => { + return standalone({ + files: path.join(fixturesPath, "empty-block-with-disables.css"), + config, + reportInvalidScopeDisables: true + }).then(linted => { + const invalidScopeDisables = linted.invalidScopeDisables; + + expect(typeof invalidScopeDisables).toBe("object"); + expect(invalidScopeDisables).toHaveLength(1); + expect(invalidScopeDisables[0].source).toBe( + path.join(fixturesPath, "empty-block-with-disables.css") + ); + expect(invalidScopeDisables[0].ranges[0]).toEqual({ + start: 1, + unusedRule: "color-named" + }); + }); +}); From cdde5ca60f444859358562d60114a8000462889c Mon Sep 17 00:00:00 2001 From: Ivan Kopeykin Date: Tue, 30 Jul 2019 14:29:21 +0300 Subject: [PATCH 8/9] fix tests --- ....js => disableOptionsReportStringFormatter.test.js} | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) rename lib/formatters/__tests__/{disableOptionsReportStringFormatter.js => disableOptionsReportStringFormatter.test.js} (78%) diff --git a/lib/formatters/__tests__/disableOptionsReportStringFormatter.js b/lib/formatters/__tests__/disableOptionsReportStringFormatter.test.js similarity index 78% rename from lib/formatters/__tests__/disableOptionsReportStringFormatter.js rename to lib/formatters/__tests__/disableOptionsReportStringFormatter.test.js index 1ed3d5705e..9a575fc5a1 100644 --- a/lib/formatters/__tests__/disableOptionsReportStringFormatter.js +++ b/lib/formatters/__tests__/disableOptionsReportStringFormatter.test.js @@ -1,13 +1,13 @@ "use strict"; -const disableOptionsReportStringFormatter = require("../disableOptionsReportStringFormatter"); +const disableOptionsReportStringFormatterTest = require("../disableOptionsReportStringFormatter"); const stripAnsi = require("strip-ansi"); const stripIndent = require("common-tags").stripIndent; describe("disableOptionsReportStringFormatter", () => { it("formatter stringified", () => { const actual = stripAnsi( - disableOptionsReportStringFormatter([ + disableOptionsReportStringFormatterTest([ { source: "foo", ranges: [ @@ -42,4 +42,10 @@ describe("disableOptionsReportStringFormatter", () => { expect(actual).toBe(expected); }); + + it("empty report", () => { + const actual = stripAnsi(disableOptionsReportStringFormatterTest()); + + expect(actual).toBe(""); + }); }); From 8d44adbe63c74dbe3641bdc9db00d2f1d15ed785 Mon Sep 17 00:00:00 2001 From: Ivan Kopeykin Date: Wed, 31 Jul 2019 14:40:11 +0300 Subject: [PATCH 9/9] fix tests --- lib/cli.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cli.js b/lib/cli.js index 78cbe7cf4e..20b7bd08b1 100644 --- a/lib/cli.js +++ b/lib/cli.js @@ -581,7 +581,7 @@ module.exports = (argv /*: string[]*/) /*: Promise|void*/ => { } if (reports.length > 0) { - reports.forEach(process.stdout.write.bind(process.stdout)); + reports.forEach(report => process.stdout.write(report)); process.exitCode = EXIT_CODE_ERROR; }