Skip to content

Commit

Permalink
[WIP] Report disables in the same manner as lints
Browse files Browse the repository at this point in the history 
Closes #4896
  • Loading branch information
@nex3
nex3 committed Sep 25, 2020
1 parent 7449dc0 commit b7fc62b
Show file tree
Hide file tree
Showing 11 changed files with 270 additions and 175 deletions.
145 changes: 120 additions & 25 deletions lib/__tests__/needlessDisables.test.js
View file
Expand Up @@ -38,12 +38,58 @@ it('needlessDisables simple case', () => {
code: css,
reportNeedlessDisables: true,
}).then((linted) => {
const report = linted.needlessDisables;
const results = linted.results;

expect(report).toHaveLength(1);
expect(report[0].ranges).toEqual([
{ start: 7, end: 9, rule: 'all', unusedRule: 'all' },
{ start: 11, end: 11, rule: 'block-no-empty', unusedRule: 'block-no-empty' },
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 },
};

const css = stripIndent`
/* stylelint-disable-next-line block-no-empty, color-named */
a {}
`;

return standalone({
config,
code: css,
reportNeedlessDisables: true,
}).then((linted) => {
const results = linted.results;

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

expect(warnings).toEqual([
{
line: 1,
column: 1,
text: 'Needless disable for "color-named"',
rule: '--report-needless-disables',
severity: 'error',
},
]);
});
});
Expand All @@ -66,24 +112,51 @@ it('needlessDisables complex case', () => {
],
reportNeedlessDisables: true,
}).then((linted) => {
expect(linted.needlessDisables).toEqual([
const results = linted.results;

expect(results).toHaveLength(3);
expect(needlessDisables(results[0].warnings)).toEqual([
{
source: source('disabled-ranges-1.css'),
ranges: [
{ start: 1, end: 3, rule: 'color-named', unusedRule: 'color-named' },
{ start: 5, end: 7, rule: 'block-no-empty', unusedRule: 'block-no-empty' },
{ start: 10, end: 10, rule: 'block-no-empty', unusedRule: 'block-no-empty' },
],
line: 1,
column: 1,
text: 'Needless disable for "color-named"',
rule: '--report-needless-disables',
severity: 'error',
},
{
source: source('disabled-ranges-2.css'),
ranges: [
{ start: 5, end: 5, rule: 'color-named', unusedRule: 'color-named' },
{ start: 6, end: 6, rule: 'block-no-empty', unusedRule: 'block-no-empty' },
{ start: 8, end: 10, rule: 'block-no-empty', unusedRule: 'block-no-empty' },
],
line: 5,
column: 1,
text: 'Needless disable for "block-no-empty"',
rule: '--report-needless-disables',
severity: 'error',
},
]);

expect(needlessDisables(results[1].warnings)).toEqual([
{
line: 6,
column: 19,
text: 'Needless disable for "block-no-empty"',
rule: '--report-needless-disables',
severity: 'error',
},
{
line: 8,
column: 1,
text: 'Needless disable for "block-no-empty"',
rule: '--report-needless-disables',
severity: 'error',
},
{
line: 5,
column: 6,
text: 'Needless disable for "color-named"',
rule: '--report-needless-disables',
severity: 'error',
},
]);

expect(needlessDisables(results[2].warnings)).toHaveLength(0);
});
});

Expand All @@ -100,15 +173,37 @@ it('needlessDisables ignored case', () => {
reportNeedlessDisables: true,
ignorePath: fixture('.stylelintignore'),
}).then((linted) => {
expect(linted.needlessDisables).toEqual([
const results = linted.results;

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

expect(needlessDisables(warnings)).toEqual([
{
line: 10,
column: 19,
text: 'Needless disable for "all"',
rule: '--report-needless-disables',
severity: 'error',
},
{
line: 1,
column: 1,
text: 'Needless disable for "color-named"',
rule: '--report-needless-disables',
severity: 'error',
},
{
source: source('disabled-ranges-1.css'),
ranges: [
{ start: 1, end: 3, rule: 'color-named', unusedRule: 'color-named' },
{ start: 5, end: 7, rule: 'block-no-empty', unusedRule: 'block-no-empty' },
{ start: 10, end: 10, rule: 'all', unusedRule: 'all' },
],
line: 5,
column: 1,
text: 'Needless disable for "block-no-empty"',
rule: '--report-needless-disables',
severity: 'error',
},
]);
});
});

function needlessDisables(warnings) {
return warnings.filter((warning) => warning.rule === '--report-needless-disables');
}
69 changes: 46 additions & 23 deletions lib/__tests__/reportDisables.test.js
View file
Expand Up @@ -19,17 +19,27 @@ describe('reportDisables', () => {
`;

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

expect(report).toHaveLength(1);
expect(report[0].ranges).toEqual([
{ start: 1, end: 3, rule: 'block-no-empty', unusedRule: 'block-no-empty' },
{ start: 5, end: 5, rule: 'block-no-empty', unusedRule: 'block-no-empty' },
const results = linted.results;

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

expect(warnings).toEqual([
{
line: 1,
column: 1,
text: 'Rule "block-no-empty" may not be disabled',
rule: 'reportDisables',
severity: 'error',
},
{
line: 5,
column: 8,
text: 'Rule "block-no-empty" may not be disabled',
rule: 'reportDisables',
severity: 'error',
},
]);

// Although these disables are reported as issues, they're still in effect
// so the underlying lint issues are not reported.
expect(linted.results[0].warnings).toHaveLength(0);
});
});

Expand All @@ -52,15 +62,27 @@ describe('reportDisables', () => {
code: css,
ignoreDisables: true,
}).then((linted) => {
const report = linted.reportedDisables;

expect(report).toHaveLength(1);
expect(report[0].ranges).toEqual([
{ start: 1, end: 3, rule: 'block-no-empty', unusedRule: 'block-no-empty' },
{ start: 5, end: 5, rule: 'block-no-empty', unusedRule: 'block-no-empty' },
const results = linted.results;

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

expect(warnings.filter((warning) => warning.rule === 'reportDisables')).toEqual([
{
line: 1,
column: 1,
text: 'Rule "block-no-empty" may not be disabled',
rule: 'reportDisables',
severity: 'error',
},
{
line: 5,
column: 8,
text: 'Rule "block-no-empty" may not be disabled',
rule: 'reportDisables',
severity: 'error',
},
]);

expect(linted.results[0].warnings).toHaveLength(2);
});
});

Expand All @@ -79,9 +101,10 @@ describe('reportDisables', () => {
`;

return standalone({ config, code: css }).then((linted) => {
const report = linted.reportedDisables;
const results = linted.results;

expect(report).toHaveLength(0);
expect(results).toHaveLength(1);
expect(results[0].warnings).toHaveLength(0);
});
});

Expand All @@ -96,10 +119,10 @@ describe('reportDisables', () => {
`;

return standalone({ config, code: css }).then((linted) => {
const report = linted.reportedDisables;
const results = linted.results;

expect(report).toHaveLength(1);
expect(report[0].ranges).toHaveLength(0);
expect(results).toHaveLength(1);
expect(results[0].warnings).toHaveLength(0);
});
});
});
31 changes: 17 additions & 14 deletions lib/assignDisabledRanges.js
View file
Expand Up @@ -16,15 +16,17 @@ const ALL_RULES = 'all';
/** @typedef {import('stylelint').DisabledRange} DisabledRange */

/**
* @param {PostcssComment} comment
* @param {number} start
* @param {boolean} strictStart
* @param {string|undefined} description
* @param {number} [end]
* @param {boolean} [strictEnd]
* @returns {DisabledRange}
*/
function createDisableRange(start, strictStart, description, end, strictEnd) {
function createDisableRange(comment, start, strictStart, description, end, strictEnd) {
return {
comment,
start,
end: end || undefined,
strictStart,
Expand Down Expand Up @@ -117,7 +119,7 @@ module.exports = function (root, result) {
const description = getDescription(comment.text);

getCommandRules(disableLineCommand, comment.text).forEach((ruleName) => {
disableLine(line, ruleName, comment, description);
disableLine(comment, line, ruleName, description);
});
}
}
Expand All @@ -131,18 +133,18 @@ module.exports = function (root, result) {
const description = getDescription(comment.text);

getCommandRules(disableNextLineCommand, comment.text).forEach((ruleName) => {
disableLine(line + 1, ruleName, comment, description);
disableLine(comment, line + 1, ruleName, description);
});
}
}

/**
* @param {PostcssComment} comment
* @param {number} line
* @param {string} ruleName
* @param {PostcssComment} comment
* @param {string|undefined} description
*/
function disableLine(line, ruleName, comment, description) {
function disableLine(comment, line, ruleName, description) {
if (ruleIsDisabled(ALL_RULES)) {
throw comment.error('All rules have already been disabled', {
plugin: 'stylelint',
Expand All @@ -155,7 +157,7 @@ module.exports = function (root, result) {

const strict = disabledRuleName === ALL_RULES;

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

startDisabledRange(line, ruleName, true, description);
startDisabledRange(comment, line, ruleName, true, description);
endDisabledRange(line, ruleName, true);
}
}
Expand Down Expand Up @@ -195,10 +197,10 @@ module.exports = function (root, result) {

if (isAllRules) {
Object.keys(disabledRanges).forEach((ruleName) => {
startDisabledRange(line, ruleName, ruleName === ALL_RULES, description);
startDisabledRange(comment, line, ruleName, ruleName === ALL_RULES, description);
});
} else {
startDisabledRange(line, ruleToDisable, true, description);
startDisabledRange(comment, line, ruleToDisable, true, description);
}
}
});
Expand Down Expand Up @@ -239,7 +241,7 @@ module.exports = function (root, result) {
// Get a starting point from the where all rules were disabled
if (!disabledRanges[ruleToEnable]) {
disabledRanges[ruleToEnable] = disabledRanges.all.map(({ start, end, description }) =>
createDisableRange(start, false, description, end, false),
createDisableRange(comment, start, false, description, end, false),
);
} else {
const range = _.last(disabledRanges[ALL_RULES]);
Expand Down Expand Up @@ -323,13 +325,14 @@ module.exports = function (root, result) {
}

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

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

0 comments on commit b7fc62b

Please sign in to comment.