Skip to content

Commit

Permalink
Report disables in the same manner as lints (#4973)
Browse files Browse the repository at this point in the history
Rather than returning disables that are considered "invalid" for
various reasons (such as --report-needless-disables) as a separate
list of objects that formatters must handle individually, stylelint
now converts them into standard lint warnings. This allows formatters
to automatically format them the same as normal warnings without any
extra disable-specific code.

Closes #4896
  • Loading branch information
nex3 committed Oct 14, 2020
1 parent a922acf commit a0eab08
Show file tree
Hide file tree
Showing 23 changed files with 549 additions and 663 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Expand Up @@ -2,6 +2,11 @@

All notable changes to this project are documented in this file.

## Head

- Added: disable comments that are reported as errors for various reasons are now reported as standard lint errors rather than a separate class of errors that must be handled specially.
- Deprecated: `StylelintStandaloneReturnValue.reportedDisables`, `.descriptionlessDisables`, `.needlessDisables`, and `.invalidScopeDisables`. `.reportedDisables` will always be empty and the other properties will always be undefined, since these errors now show up in `.results` instead.

## 13.7.2

- Fixed: regression for disable commands and adjacent double-slash comments ([#4950](https://github.com/stylelint/stylelint/pull/4950)).
Expand Down
24 changes: 0 additions & 24 deletions docs/developer-guide/formatters.md
Expand Up @@ -50,30 +50,6 @@ And the second argument (`returnValue`) is an object (type `StylelintStandaloneR
```js
{
"errored": false, // `true` if there were any warnings with "error" severity
"needlessDisables": [
// Present if stylelint was configured with `reportNeedlessDisables: true`
{
"source": "path/to/file.css",
"ranges": [
{
"start": 10,
"rule": "indentation"
}
]
}
],
"invalidScopeDisables": [
// Present if stylelint was configured with `reportInvalidScopeDisables: true`
{
"source": "path/to/file.css",
"ranges": [
{
"start": 1,
"rule": "color-named"
}
]
}
],
"maxWarningsExceeded": {
// Present if stylelint was configured with a `maxWarnings` count
"maxWarnings": 10,
Expand Down
2 changes: 1 addition & 1 deletion docs/user-guide/configure.md
Expand Up @@ -155,7 +155,7 @@ For example:
}
```

The report is included in the [`reportedDisables`](usage/node-api.md#reporteddisables) property to the returned data of Node.js API.
The report is considered to be a lint error.

## `defaultSeverity`

Expand Down
16 changes: 0 additions & 16 deletions docs/user-guide/usage/node-api.md
Expand Up @@ -66,22 +66,6 @@ An array containing all the stylelint result objects (the objects that formatter

An object containing the maximum number of warnings and the amount found, e.g. `{ maxWarnings: 0, foundWarnings: 12 }`.

### `reportedDisables`

An array of objects, one for each source, with tells you which `stylelint-disable` comments for rules enabled [`reportDisables`](../configure.md#reportdisables).

### `needlessDisables`

An array of objects, one for each source, with tells you which `stylelint-disable` comments are not blocking a lint violation.

### `invalidScopeDisables`

An array of objects, one for each source, with tells you which rule in a `stylelint-disable <rule>` comment doesn't exist within the configuration object.

### `descriptionlessDisables`

An array of objects, one for each source, with tells you which `stylelint-disable` comments without a description.

## Syntax errors

`stylelint.lint()` does not reject the Promise when your CSS contains syntax errors.
Expand Down
6 changes: 3 additions & 3 deletions docs/user-guide/usage/options.md
Expand Up @@ -146,7 +146,7 @@ Produce a report to clean up your codebase, keeping only the `stylelint-disable`
If needless disables are found, the:

- CLI process exits with code `2`
- Node.js API adds a [`needlessDisables`](node-api.md#needlessdisables) property to the returned data
- Node.js API adds errors to the returned data

## `reportInvalidScopeDisables`

Expand All @@ -157,7 +157,7 @@ Produce a report of the `stylelint-disable` comments that used for rules that do
If invalid scope disables are found, the:

- CLI process exits with code `2`
- Node.js API adds a [`invalidScopeDisables`](node-api.md#invalidscopedisables) property to the returned data
- Node.js API adds errors to the returned data

## `reportDescriptionlessDisables`

Expand Down Expand Up @@ -196,7 +196,7 @@ a {}
If descriptionless disables are found, the:

- CLI process exits with code `2`
- Node.js API adds a [`descriptionlessDisables`](node-api.md#descriptionlessdisables) property to the returned data
- Node.js API adds errors to the returned data

## `codeFilename`

Expand Down
21 changes: 7 additions & 14 deletions lib/__tests__/cli.test.js
Expand Up @@ -232,14 +232,9 @@ describe('CLI', () => {

expect(process.exitCode).toBe(2);

expect(process.stdout.write).toHaveBeenCalledTimes(2);
expect(process.stdout.write).toHaveBeenNthCalledWith(
1,
expect.stringContaining('needless disable: color-named'),
);
expect(process.stdout.write).toHaveBeenNthCalledWith(
2,
expect.stringContaining('Unexpected empty block'),
expect(process.stdout.write).toHaveBeenCalledTimes(1);
expect(process.stdout.write).toHaveBeenCalledWith(
expect.stringMatching(/Needless disable for "color-named".*Unexpected empty block/s),
);
});

Expand All @@ -253,9 +248,8 @@ describe('CLI', () => {
expect(process.exitCode).toBe(2);

expect(process.stdout.write).toHaveBeenCalledTimes(1);
expect(process.stdout.write).toHaveBeenNthCalledWith(
1,
expect.stringContaining('forbidden disable: block-no-empty'),
expect(process.stdout.write).toHaveBeenCalledWith(
expect.stringContaining('Rule "block-no-empty" may not be disabled'),
);
});

Expand All @@ -270,9 +264,8 @@ describe('CLI', () => {
expect(process.exitCode).toBe(2);

expect(process.stdout.write).toHaveBeenCalledTimes(1);
expect(process.stdout.write).toHaveBeenNthCalledWith(
1,
expect.stringContaining('descriptionless disable: block-no-empty'),
expect(process.stdout.write).toHaveBeenCalledWith(
expect.stringContaining('Disable for "block-no-empty" is missing a description'),
);
});

Expand Down
39 changes: 23 additions & 16 deletions lib/__tests__/descriptionlessDisables.test.js
Expand Up @@ -14,7 +14,7 @@ it('descriptionlessDisables', () => {
/* stylelint-enable */
a {
b {} /* stylelint-disable-line block-no-empty -- Description */
}
}
/* stylelint-disable-next-line block-no-empty
* --
* Description */
Expand All @@ -35,27 +35,34 @@ it('descriptionlessDisables', () => {
code: css,
reportDescriptionlessDisables: true,
}).then((linted) => {
const report = linted.descriptionlessDisables;
const results = linted.results;

expect(results).toHaveLength(1);
const warnings = results[0].warnings.filter(
(warning) => warning.rule === '--report-descriptionless-disables',
);

expect(report).toHaveLength(1);
expect(report[0].ranges).toEqual([
expect(warnings).toEqual([
{
start: 12,
end: 14,
rule: 'all',
unusedRule: 'all',
line: 12,
column: 1,
rule: '--report-descriptionless-disables',
severity: 'error',
text: 'Disable for "all" is missing a description',
},
{
start: 16,
end: 16,
rule: 'block-no-empty',
unusedRule: 'block-no-empty',
line: 16,
column: 8,
rule: '--report-descriptionless-disables',
severity: 'error',
text: 'Disable for "block-no-empty" is missing a description',
},
{
start: 19,
end: 19,
rule: 'block-no-empty',
unusedRule: 'block-no-empty',
line: 18,
column: 1,
rule: '--report-descriptionless-disables',
severity: 'error',
text: 'Disable for "block-no-empty" is missing a description',
},
]);
});
Expand Down

0 comments on commit a0eab08

Please sign in to comment.