Skip to content

Commit

Permalink
Allow disable reporting to be controlled from the config file (#5126)
Browse files Browse the repository at this point in the history 
* Remove unused disable-reporting code

This was outmoded by #4973

* 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.

* Fix a typo

Co-authored-by: Jennifer Thakar <jathak@google.com>

* Code review changes

Co-authored-by: Jennifer Thakar <jathak@google.com>
  • Loading branch information
@nex3 @jathak
nex3 and jathak committed Feb 6, 2021
1 parent ef26ca8 commit 4fc7fa8
Show file tree
Hide file tree
Showing 16 changed files with 314 additions and 196 deletions.
76 changes: 76 additions & 0 deletions docs/user-guide/configure.md
View file
Expand Up @@ -168,6 +168,82 @@ You can set the default severity level for all rules that do not have a severity
}
```

## `ignoreDisables`

Ignore `stylelint-disable` (e.g. `/* stylelint-disable block-no-empty */`) comments.

For example:

```json
{
"ignoreDisables": true
}
```

## `reportNeedlessDisables`

Emit errors for `stylelint-disable` comments that don't 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:

<!-- prettier-ignore -->
```css
/* stylelint-disable */
a {}
```

<!-- prettier-ignore -->
```css
/* stylelint-disable-next-line block-no-empty */
a {}
```

But, the following patterns (`stylelint-disable -- <description>`) are _not_ reported:

<!-- prettier-ignore -->
```css
/* stylelint-disable -- This violation is ignorable. */
a {}
```

<!-- prettier-ignore -->
```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).
Expand Down
65 changes: 65 additions & 0 deletions lib/__tests__/descriptionlessDisables.test.js
View file
Expand Up @@ -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',
},
]);
});
});
23 changes: 23 additions & 0 deletions lib/__tests__/ignoreDisables.test.js
View file
Expand Up @@ -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);
});
});
45 changes: 45 additions & 0 deletions lib/__tests__/invalidScopeDisables.test.js
View file
Expand Up @@ -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: {
Expand Down
49 changes: 49 additions & 0 deletions lib/__tests__/needlessDisables.test.js
View file
Expand Up @@ -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 },
Expand Down
56 changes: 0 additions & 56 deletions lib/cli.js
View file
Expand Up @@ -2,7 +2,6 @@

const chalk = require('chalk');
const checkInvalidCLIOptions = require('./utils/checkInvalidCLIOptions');
const disableOptionsReportStringFormatter = require('./formatters/disableOptionsReportStringFormatter');
const EOL = require('os').EOL;
const getFormatterOptionsText = require('./utils/getFormatterOptionsText');
const getModulePath = require('./utils/getModulePath');
Expand Down Expand Up @@ -495,61 +494,6 @@ module.exports = (argv) => {

return standalone(options)
.then((linted) => {
const reports = [];

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

if (report) reports.push(report);

if (reportNeedlessDisables) {
// TODO: Issue #4985
// eslint-disable-next-line no-shadow
const report = disableOptionsReportStringFormatter(
linted.needlessDisables || [],
'needless disable',
);

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

if (reportInvalidScopeDisables) {
// TODO: Issue #4985
// eslint-disable-next-line no-shadow
const report = disableOptionsReportStringFormatter(
linted.invalidScopeDisables || [],
'disable with invalid scope',
);

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

if (reportDescriptionlessDisables) {
// TODO: Issue #4985
// eslint-disable-next-line no-shadow
const report = disableOptionsReportStringFormatter(
linted.descriptionlessDisables || [],
'descriptionless disable',
);

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

if (reports.length > 0) {
// TODO: Issue #4985
// eslint-disable-next-line no-shadow
reports.forEach((report) => process.stdout.write(report));
process.exitCode = EXIT_CODE_ERROR;
}

if (!linted.output) {
return;
}
Expand Down
18 changes: 18 additions & 0 deletions lib/createStylelint.js
View file
Expand Up @@ -26,6 +26,24 @@ module.exports = function (options = {}) {
/** @type {Partial<StylelintInternalApi>} */
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', {
Expand Down
8 changes: 7 additions & 1 deletion lib/descriptionlessDisables.js
View file
Expand Up @@ -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<PostcssComment>} */
const alreadyReported = new Set();
Expand Down

0 comments on commit 4fc7fa8

Please sign in to comment.