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

Add a reportDisables secondary option #4897

Merged
merged 4 commits into from Aug 19, 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
4 changes: 2 additions & 2 deletions docs/developer-guide/formatters.md
Expand Up @@ -57,7 +57,7 @@ And the second argument (`returnValue`) is an object (type `StylelintStandaloneR
"ranges": [
{
"start": 10,
"unusedRule": "indentation"
"rule": "indentation"
}
]
}
Expand All @@ -69,7 +69,7 @@ And the second argument (`returnValue`) is an object (type `StylelintStandaloneR
"ranges": [
{
"start": 1,
"unusedRule": "color-named"
"rule": "color-named"
}
]
}
Expand Down
20 changes: 20 additions & 0 deletions docs/user-guide/configure.md
Expand Up @@ -135,6 +135,26 @@ For example:

Reporters may use these severity levels to display violations or exit the process differently.

### `reportDisables`

You can set the `reportDisables` secondary option to report any disable comments for this rule, effectively disallowing authors to opt out of it.

For example:

```json
{
"rules": {
"indentation": [
2,
{
"except": ["value"],
"reportDisables": true
}
]
}
}
```

## `defaultSeverity`

You can set the default severity level for all rules that do not have a severity specified in their secondary options. For example, you can set the default severity to `"warning"`:
Expand Down
18 changes: 17 additions & 1 deletion lib/__tests__/cli.test.js
Expand Up @@ -234,14 +234,30 @@ describe('CLI', () => {
expect(process.stdout.write).toHaveBeenCalledTimes(2);
expect(process.stdout.write).toHaveBeenNthCalledWith(
1,
expect.stringContaining('unused rule: color-named'),
expect.stringContaining('needless disable: color-named'),
);
expect(process.stdout.write).toHaveBeenNthCalledWith(
2,
expect.stringContaining('Unexpected empty block'),
);
});

it('reports disallowed disables', async () => {
await cli([
'--config',
replaceBackslashes(path.join(fixturesPath, 'config-block-no-empty-report-disables.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('forbidden disable: block-no-empty'),
);
});

it('--stdin', async () => {
await cli(['--stdin', '--config', `${fixturesPath}/config-no-empty-source.json`]);

Expand Down
@@ -0,0 +1,5 @@
{
"rules": {
"block-no-empty": [true, { "reportDisables": true }]
}
}
2 changes: 2 additions & 0 deletions lib/__tests__/fixtures/empty-block-with-relevant-disable.css
@@ -0,0 +1,2 @@
/* stylelint-disable block-no-empty */
a {}
11 changes: 6 additions & 5 deletions lib/__tests__/invalidScopeDisables.test.js
Expand Up @@ -40,8 +40,8 @@ it('invalidScopeDisables simple case', () => {
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' },
{ end: 3, start: 1, rule: 'block-no-empty', unusedRule: 'block-no-empty' },
{ end: 5, start: 5, rule: 'block-no-empty', unusedRule: 'block-no-empty' },
]);
});
});
Expand All @@ -65,11 +65,11 @@ it('invalidScopeDisables complex case', () => {
expect(invalidScopeDisables(linted.results, config)).toEqual([
{
source: source('disabled-ranges-1.css'),
ranges: [{ start: 1, end: 3, unusedRule: 'color-named' }],
ranges: [{ start: 1, end: 3, rule: 'color-named', unusedRule: 'color-named' }],
},
{
source: source('disabled-ranges-2.css'),
ranges: [{ start: 5, end: 5, unusedRule: 'color-named' }],
ranges: [{ start: 5, end: 5, rule: 'color-named', unusedRule: 'color-named' }],
},
]);
});
Expand All @@ -91,7 +91,7 @@ it('invalidScopeDisables ignored case', () => {
expect(invalidScopeDisables(linted.results, config)).toEqual([
{
source: source('disabled-ranges-1.css'),
ranges: [{ start: 5, end: 7, unusedRule: 'block-no-empty' }],
ranges: [{ start: 5, end: 7, rule: 'block-no-empty', unusedRule: 'block-no-empty' }],
},
]);
});
Expand Down Expand Up @@ -119,6 +119,7 @@ it('invalidScopeDisables for config file', () => {
{
end: undefined,
start: 4,
rule: 'foo',
unusedRule: 'foo',
},
]);
Expand Down
22 changes: 11 additions & 11 deletions lib/__tests__/needlessDisables.test.js
Expand Up @@ -42,8 +42,8 @@ it('needlessDisables simple case', () => {

expect(report).toHaveLength(1);
expect(report[0].ranges).toEqual([
{ start: 7, end: 9, unusedRule: 'all' },
{ start: 11, end: 11, unusedRule: 'block-no-empty' },
{ start: 7, end: 9, rule: 'all', unusedRule: 'all' },
{ start: 11, end: 11, rule: 'block-no-empty', unusedRule: 'block-no-empty' },
]);
});
});
Expand All @@ -70,17 +70,17 @@ it('needlessDisables complex case', () => {
{
source: source('disabled-ranges-1.css'),
ranges: [
{ start: 1, end: 3, unusedRule: 'color-named' },
{ start: 5, end: 7, unusedRule: 'block-no-empty' },
{ start: 10, end: 10, unusedRule: 'block-no-empty' },
{ 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' },
],
},
{
source: source('disabled-ranges-2.css'),
ranges: [
{ start: 5, end: 5, unusedRule: 'color-named' },
{ start: 6, end: 6, unusedRule: 'block-no-empty' },
{ start: 8, end: 10, unusedRule: 'block-no-empty' },
{ 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' },
],
},
]);
Expand All @@ -104,9 +104,9 @@ it('needlessDisables ignored case', () => {
{
source: source('disabled-ranges-1.css'),
ranges: [
{ start: 1, end: 3, unusedRule: 'color-named' },
{ start: 5, end: 7, unusedRule: 'block-no-empty' },
{ start: 10, end: 10, unusedRule: 'all' },
{ 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' },
],
},
]);
Expand Down
105 changes: 105 additions & 0 deletions lib/__tests__/reportDisables.test.js
@@ -0,0 +1,105 @@
'use strict';

const standalone = require('../standalone');
const stripIndent = require('common-tags').stripIndent;

describe('reportDisables', () => {
it('reports a disabled comment', () => {
const config = {
rules: { 'block-no-empty': [true, { reportDisables: true }] },
};

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 = 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' },
]);

// 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);
});
});

it('reports an ignored disabled comment', () => {
const config = {
rules: { 'block-no-empty': [true, { reportDisables: true }] },
};

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,
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' },
]);

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

it("doesn't report disables by default", () => {
const config = {
rules: { 'block-no-empty': [true] },
};

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 = linted.reportedDisables;

expect(report).toHaveLength(0);
});
});

// This should be handled by the global `reportUnscopedDisables` option (#2292).
it("doesn't report unscoped disables", () => {
const config = {
rules: { 'block-no-empty': [true, { reportDisables: true }] },
};

const css = stripIndent`
a {} /* stylelint-disable-line */
`;

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

expect(report).toHaveLength(1);
expect(report[0].ranges).toHaveLength(0);
});
});
});
2 changes: 2 additions & 0 deletions lib/__tests__/standalone-invalidScopeDisables.test.js
Expand Up @@ -25,6 +25,7 @@ it('standalone with input css and `reportInvalidScopeDisables`', () => {
expect(invalidScopeDisables[0].ranges).toHaveLength(1);
expect(invalidScopeDisables[0].ranges[0]).toEqual({
start: 1,
rule: 'color-named',
unusedRule: 'color-named',
});
});
Expand All @@ -45,6 +46,7 @@ it('standalone with input file(s) and `reportInvalidScopeDisables`', () => {
);
expect(invalidScopeDisables[0].ranges[0]).toEqual({
start: 1,
rule: 'color-named',
unusedRule: 'color-named',
});
});
Expand Down
2 changes: 2 additions & 0 deletions lib/__tests__/standalone-needlessDisables.test.js
Expand Up @@ -27,6 +27,7 @@ it('standalone with input css and `reportNeedlessDisables`', () => {
expect(needlessDisables[0].ranges).toHaveLength(1);
expect(needlessDisables[0].ranges[0]).toEqual({
start: 1,
rule: 'color-named',
unusedRule: 'color-named',
});

Expand Down Expand Up @@ -130,6 +131,7 @@ it('standalone with input file(s) and `reportNeedlessDisables`', () => {
);
expect(needlessDisables[0].ranges[0]).toEqual({
start: 1,
rule: 'color-named',
unusedRule: 'color-named',
});
});
Expand Down
3 changes: 3 additions & 0 deletions lib/__tests__/stylelintignore-test/stylelintignore.test.js
Expand Up @@ -64,6 +64,7 @@ describe('stylelintignore', () => {
expect(data).toEqual({
errored: false,
output: '[]',
reportedDisables: [],
results: [],
});
});
Expand Down Expand Up @@ -98,6 +99,7 @@ describe('stylelintignore', () => {
expect(data).toEqual({
errored: false,
output: '[]',
reportedDisables: [],
results: [],
});
});
Expand All @@ -112,6 +114,7 @@ describe('stylelintignore', () => {
expect(data).toEqual({
errored: false,
output: '[]',
reportedDisables: [],
results: [],
});
});
Expand Down
17 changes: 15 additions & 2 deletions lib/cli.js
Expand Up @@ -481,16 +481,29 @@ module.exports = (argv) => {
.then((linted) => {
const reports = [];

const report = disableOptionsReportStringFormatter(
linted.reportedDisables || [],
'forbidden disable',
);

if (report) reports.push(report);

if (reportNeedlessDisables) {
const report = disableOptionsReportStringFormatter(linted.needlessDisables || []);
const report = disableOptionsReportStringFormatter(
linted.needlessDisables || [],
'needless disable',
);

if (report) {
reports.push(report);
}
}

if (reportInvalidScopeDisables) {
const report = disableOptionsReportStringFormatter(linted.invalidScopeDisables || []);
const report = disableOptionsReportStringFormatter(
linted.invalidScopeDisables || [],
'disable with invalid scope',
);

if (report) {
reports.push(report);
Expand Down