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

Allow disable reporting to be controlled from the config file #5126

Merged
merged 4 commits into from Feb 6, 2021
Merged
Show file tree
Hide file tree
Changes from 3 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
78 changes: 78 additions & 0 deletions docs/user-guide/configure.md
Expand Up @@ -168,6 +168,84 @@ You can set the default severity level for all rules that do not have a severity
}
```

## `ignoreDisables`

Ignore `styleline-disable` (e.g. `/* stylelint-disable block-no-empty */`) comments.
nex3 marked this conversation as resolved.
Show resolved Hide resolved

For example:

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

## `reportNeedlessDisables`

Emit errors for `stylelint-disable` comments that don't include actually match
nex3 marked this conversation as resolved.
Show resolved Hide resolved
any lints that need to be disabled.
nex3 marked this conversation as resolved.
Show resolved Hide resolved

For example:

```json
{
"reportNeedlessDisables": true
}
```

## `reportInvalidScopeDisables`

Emit errors for `stylelint-disable` comments that don't match rules that are
specified in the configuration object.
nex3 marked this conversation as resolved.
Show resolved Hide resolved

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