Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make --report-needless-disables CLI flag respect stylelint-disable commands #4714

Merged
merged 1 commit into from May 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 6 additions & 7 deletions lib/__tests__/needlessDisables.test.js
@@ -1,6 +1,5 @@
'use strict';

const needlessDisables = require('../needlessDisables');
const path = require('path');
const replaceBackslashes = require('./replaceBackslashes');
const standalone = require('../standalone');
Expand Down Expand Up @@ -37,9 +36,9 @@ it('needlessDisables simple case', () => {
return standalone({
config,
code: css,
ignoreDisables: true,
reportNeedlessDisables: true,
}).then((linted) => {
const report = needlessDisables(linted.results);
const report = linted.needlessDisables;

expect(report).toHaveLength(1);
expect(report[0].ranges).toEqual([
Expand All @@ -65,9 +64,9 @@ it('needlessDisables complex case', () => {
// ignore files contain `CssSyntaxError`
fixture('disabled-ranges-3.css'),
],
ignoreDisables: true,
reportNeedlessDisables: true,
}).then((linted) => {
expect(needlessDisables(linted.results)).toEqual([
expect(linted.needlessDisables).toEqual([
{
source: source('disabled-ranges-1.css'),
ranges: [
Expand Down Expand Up @@ -98,10 +97,10 @@ it('needlessDisables ignored case', () => {
return standalone({
config,
files: [fixture('disabled-ranges-1.css'), fixture('ignored-file.css')],
ignoreDisables: true,
reportNeedlessDisables: true,
ignorePath: fixture('.stylelintignore'),
}).then((linted) => {
expect(needlessDisables(linted.results)).toEqual([
expect(linted.needlessDisables).toEqual([
{
source: source('disabled-ranges-1.css'),
ranges: [
Expand Down
75 changes: 75 additions & 0 deletions lib/__tests__/standalone-needlessDisables.test.js
Expand Up @@ -29,6 +29,81 @@ it('standalone with input css and `reportNeedlessDisables`', () => {
start: 1,
unusedRule: 'color-named',
});

expect(linted.results).toHaveLength(1);
const warnings = linted.results[0].warnings;

expect(typeof warnings).toBe('object');
expect(warnings).toHaveLength(1);
expect(warnings[0]).toEqual({
line: 2,
column: 3,
rule: 'block-no-empty',
severity: 'error',
text: 'Unexpected empty block (block-no-empty)',
});
});
});

it('standalone with `reportNeedlessDisables` and correctly `stylelint-disable`', () => {
const config = {
quiet: true,
rules: {
'color-named': 'never',
},
};

return standalone({
code: '/* stylelint-disable color-named */\na { color: black; }',
config,
reportNeedlessDisables: true,
}).then((linted) => {
const needlessDisables = linted.needlessDisables;

expect(typeof needlessDisables).toBe('object');
expect(needlessDisables).toHaveLength(1);
expect(needlessDisables[0].ranges).toHaveLength(0);

expect(linted.results).toHaveLength(1);
const warnings = linted.results[0].warnings;

expect(typeof warnings).toBe('object');
expect(warnings).toHaveLength(0);
});
});

it('standalone with `reportNeedlessDisables` and `ignoreDisables`', () => {
const config = {
quiet: true,
rules: {
'color-named': 'never',
},
};

return standalone({
code: '/* stylelint-disable color-named */\na { color: black; }',
config,
reportNeedlessDisables: true,
ignoreDisables: true,
}).then((linted) => {
const needlessDisables = linted.needlessDisables;

expect(typeof needlessDisables).toBe('object');
expect(needlessDisables).toHaveLength(1);
expect(needlessDisables[0].ranges).toHaveLength(0);

expect(linted.results).toHaveLength(1);
const warnings = linted.results[0].warnings;

expect(typeof warnings).toBe('object');
expect(warnings).toHaveLength(1);
expect(warnings[0]).toEqual({
line: 2,
column: 12,
rule: 'color-named',
severity: 'error',
text: 'Unexpected named color "black" (color-named)',
});
});
});

Expand Down
6 changes: 5 additions & 1 deletion lib/lintSource.js
Expand Up @@ -142,10 +142,14 @@ function lintPostcssResult(stylelint, postcssResult, config) {
assignDisabledRanges(postcssDoc, postcssResult);
}

if (stylelint._options.reportNeedlessDisables || stylelint._options.ignoreDisables) {
if (stylelint._options.ignoreDisables) {
postcssResult.stylelint.ignoreDisables = true;
}

if (stylelint._options.reportNeedlessDisables) {
postcssResult.stylelint.reportNeedlessDisables = true;
}

const isFileFixCompatible = isFixCompatible(postcssResult);

if (!isFileFixCompatible) {
Expand Down
6 changes: 4 additions & 2 deletions lib/needlessDisables.js
Expand Up @@ -30,7 +30,9 @@ module.exports = function (results) {
return;
}

result.warnings.forEach((warning) => {
const disabledWarnings = result._postcssResult.stylelint.disabledWarnings || [];

disabledWarnings.forEach((warning) => {
const rule = warning.rule;

const ruleRanges = rangeData[rule];
Expand Down Expand Up @@ -90,7 +92,7 @@ module.exports = function (results) {
};

/**
* @param {import('stylelint').StylelintWarning} warning
* @param {import('stylelint').DisabledWarning} warning
* @param {RangeType} range
* @return {boolean}
*/
Expand Down
25 changes: 23 additions & 2 deletions lib/utils/report.js
Expand Up @@ -50,7 +50,10 @@ 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) {
if (
result.stylelint.disabledRanges &&
(!result.stylelint.ignoreDisables || result.stylelint.reportNeedlessDisables)
) {
const ranges = result.stylelint.disabledRanges[ruleName] || result.stylelint.disabledRanges.all;

for (const range of ranges) {
Expand All @@ -62,7 +65,25 @@ module.exports = function (violation) {
(range.end === undefined || range.end >= startLine) &&
(!range.rules || range.rules.includes(ruleName))
) {
return;
if (result.stylelint.reportNeedlessDisables) {
// Collect disabled warnings
// Used to report `needlessDisables` in subsequent processing.
const disabledWarnings =
result.stylelint.disabledWarnings || (result.stylelint.disabledWarnings = []);

disabledWarnings.push({
rule: ruleName,
line: startLine,
});

if (!result.stylelint.ignoreDisables) {
return;
}

break;
} else {
return;
}
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions types/stylelint/index.d.ts
Expand Up @@ -42,13 +42,17 @@ declare module 'stylelint' {
[ruleName: string]: Array<DisabledRange>;
};

export type DisabledWarning = { line: number; rule: string };

export type StylelintPostcssResult = {
ruleSeverities: { [k: string]: any };
customMessages: { [k: string]: any };
quiet?: boolean;
disabledRanges: DisabledRangeObject;
disabledWarnings?: DisabledWarning[];
ignored?: boolean;
ignoreDisables?: boolean;
reportNeedlessDisables?: boolean;
stylelintError?: boolean;
disableWritingFix?: boolean;
};
Expand Down