Skip to content

Commit 382961f

Browse files
ybiquitousybiquitous
authored
Fix report flags not reporting on subsequent runs when cache is used (#7483)
The bug's reason is that the `stylelintError`/`stylelintWarning` properties in a lint result object are not set when `styelint-disable` comment problems are reported. Also, this change refactors the code around `styelint-disable` comments to make it more readable and prevent possible bugs. The new code uses [`Result#warn()`](https://postcss.org/api/#result-warn) in PostCSS for a comment node, like reporting a normal rule problem.
1 parent f02d168 commit 382961f

18 files changed

+411
-400
lines changed

‎.changeset/afraid-cobras-heal.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"stylelint": patch
3+
---
4+
5+
Fixed: report flags not reporting on subsequent runs when cache is used

‎lib/__tests__/standalone-cache.test.mjs‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,28 @@ describe('standalone cache', () => {
158158
expect(cache.getKey(newFileDest)).toBeUndefined();
159159
});
160160

161+
it('files with a special comment problem are not cached', async () => {
162+
const inputFile = path.join(fixturesPath, 'empty-block-with-disables.css');
163+
164+
await copyFile(inputFile, newFileDest);
165+
166+
const { errored, results } = await standalone(
167+
getConfig({
168+
config: {
169+
rules: {},
170+
},
171+
reportNeedlessDisables: true,
172+
}),
173+
);
174+
175+
expect(errored).toBe(true);
176+
expect(results).toHaveProperty('[0].warnings[0].rule', '--report-needless-disables');
177+
178+
const { cache } = fCache.createFromFile(expectedCacheFilePath);
179+
180+
expect(cache.getKey(newFileDest)).toBeUndefined();
181+
});
182+
161183
it('cache file is removed when cache is disabled', async () => {
162184
await standalone(getConfig({ cache: false }));
163185
expect(existsSync(expectedCacheFilePath)).toBe(false);

‎lib/descriptionlessDisables.cjs‎

Lines changed: 32 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -3,61 +3,51 @@
33
'use strict';
44

55
const optionsMatches = require('./utils/optionsMatches.cjs');
6+
const reportCommentProblem = require('./utils/reportCommentProblem.cjs');
67
const validateDisableSettings = require('./validateDisableSettings.cjs');
78

8-
/** @typedef {import('postcss').Comment} PostcssComment */
9-
/** @typedef {import('stylelint').DisableReportRange} DisableReportRange */
10-
/** @typedef {import('stylelint').DisableOptionsReport} StylelintDisableOptionsReport */
11-
129
/**
13-
* @param {import('stylelint').LintResult[]} results
10+
* @param {import('stylelint').PostcssResult} postcssResult
11+
* @returns {void}
1412
*/
15-
function descriptionlessDisables(results) {
16-
for (const result of results) {
17-
const settings = validateDisableSettings(
18-
result._postcssResult,
19-
'reportDescriptionlessDisables',
20-
);
21-
22-
if (!settings) continue;
13+
function descriptionlessDisables(postcssResult) {
14+
const [enabled, options] = validateDisableSettings(
15+
postcssResult,
16+
'reportDescriptionlessDisables',
17+
);
2318

24-
const [enabled, options, stylelintResult] = settings;
19+
if (!options) return;
2520

26-
/** @type {Set<PostcssComment>} */
27-
const alreadyReported = new Set();
21+
/** @type {Set<import('postcss').Comment>} */
22+
const alreadyReported = new Set();
2823

29-
for (const [rule, ruleRanges] of Object.entries(stylelintResult.disabledRanges)) {
30-
for (const range of ruleRanges) {
31-
if (range.description) continue;
24+
for (const [rule, ruleRanges] of Object.entries(postcssResult.stylelint.disabledRanges)) {
25+
for (const range of ruleRanges) {
26+
if (range.description) continue;
3227

33-
if (alreadyReported.has(range.comment)) continue;
28+
const commentNode = range.comment;
3429

35-
if (enabled === optionsMatches(options, 'except', rule)) {
36-
// An 'all' rule will get copied for each individual rule. If the
37-
// configuration is `[false, {except: ['specific-rule']}]`, we
38-
// don't want to report the copies that match except, so we record
39-
// the comment as already reported.
40-
if (!enabled && rule === 'all') alreadyReported.add(range.comment);
30+
if (alreadyReported.has(commentNode)) continue;
4131

42-
continue;
43-
}
32+
if (enabled === optionsMatches(options, 'except', rule)) {
33+
// An 'all' rule will get copied for each individual rule. If the
34+
// configuration is `[false, {except: ['specific-rule']}]`, we
35+
// don't want to report the copies that match except, so we record
36+
// the comment as already reported.
37+
if (!enabled && rule === 'all') alreadyReported.add(commentNode);
4438

45-
alreadyReported.add(range.comment);
39+
continue;
40+
}
4641

47-
// If the comment doesn't have a location, we can't report a useful error.
48-
// In practice we expect all comments to have locations, though.
49-
if (!range.comment.source || !range.comment.source.start) continue;
42+
alreadyReported.add(commentNode);
5043

51-
result.warnings.push({
52-
text: `Disable for "${rule}" is missing a description`,
53-
rule: '--report-descriptionless-disables',
54-
line: range.comment.source.start.line,
55-
column: range.comment.source.start.column,
56-
endLine: range.comment.source.end && range.comment.source.end.line,
57-
endColumn: range.comment.source.end && range.comment.source.end.column,
58-
severity: options.severity,
59-
});
60-
}
44+
reportCommentProblem({
45+
rule: '--report-descriptionless-disables',
46+
message: `Disable for "${rule}" is missing a description`,
47+
severity: options.severity,
48+
commentNode,
49+
postcssResult,
50+
});
6151
}
6252
}
6353
}

‎lib/descriptionlessDisables.mjs‎

Lines changed: 40 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,59 +1,49 @@
11
import optionsMatches from './utils/optionsMatches.mjs';
2+
import reportCommentProblem from './utils/reportCommentProblem.mjs';
23
import validateDisableSettings from './validateDisableSettings.mjs';
34

4-
/** @typedef {import('postcss').Comment} PostcssComment */
5-
/** @typedef {import('stylelint').DisableReportRange} DisableReportRange */
6-
/** @typedef {import('stylelint').DisableOptionsReport} StylelintDisableOptionsReport */
7-
85
/**
9-
* @param {import('stylelint').LintResult[]} results
6+
* @param {import('stylelint').PostcssResult} postcssResult
7+
* @returns {void}
108
*/
11-
export default function descriptionlessDisables(results) {
12-
for (const result of results) {
13-
const settings = validateDisableSettings(
14-
result._postcssResult,
15-
'reportDescriptionlessDisables',
16-
);
17-
18-
if (!settings) continue;
19-
20-
const [enabled, options, stylelintResult] = settings;
21-
22-
/** @type {Set<PostcssComment>} */
23-
const alreadyReported = new Set();
24-
25-
for (const [rule, ruleRanges] of Object.entries(stylelintResult.disabledRanges)) {
26-
for (const range of ruleRanges) {
27-
if (range.description) continue;
28-
29-
if (alreadyReported.has(range.comment)) continue;
30-
31-
if (enabled === optionsMatches(options, 'except', rule)) {
32-
// An 'all' rule will get copied for each individual rule. If the
33-
// configuration is `[false, {except: ['specific-rule']}]`, we
34-
// don't want to report the copies that match except, so we record
35-
// the comment as already reported.
36-
if (!enabled && rule === 'all') alreadyReported.add(range.comment);
37-
38-
continue;
39-
}
40-
41-
alreadyReported.add(range.comment);
42-
43-
// If the comment doesn't have a location, we can't report a useful error.
44-
// In practice we expect all comments to have locations, though.
45-
if (!range.comment.source || !range.comment.source.start) continue;
46-
47-
result.warnings.push({
48-
text: `Disable for "${rule}" is missing a description`,
49-
rule: '--report-descriptionless-disables',
50-
line: range.comment.source.start.line,
51-
column: range.comment.source.start.column,
52-
endLine: range.comment.source.end && range.comment.source.end.line,
53-
endColumn: range.comment.source.end && range.comment.source.end.column,
54-
severity: options.severity,
55-
});
9+
export default function descriptionlessDisables(postcssResult) {
10+
const [enabled, options] = validateDisableSettings(
11+
postcssResult,
12+
'reportDescriptionlessDisables',
13+
);
14+
15+
if (!options) return;
16+
17+
/** @type {Set<import('postcss').Comment>} */
18+
const alreadyReported = new Set();
19+
20+
for (const [rule, ruleRanges] of Object.entries(postcssResult.stylelint.disabledRanges)) {
21+
for (const range of ruleRanges) {
22+
if (range.description) continue;
23+
24+
const commentNode = range.comment;
25+
26+
if (alreadyReported.has(commentNode)) continue;
27+
28+
if (enabled === optionsMatches(options, 'except', rule)) {
29+
// An 'all' rule will get copied for each individual rule. If the
30+
// configuration is `[false, {except: ['specific-rule']}]`, we
31+
// don't want to report the copies that match except, so we record
32+
// the comment as already reported.
33+
if (!enabled && rule === 'all') alreadyReported.add(commentNode);
34+
35+
continue;
5636
}
37+
38+
alreadyReported.add(commentNode);
39+
40+
reportCommentProblem({
41+
rule: '--report-descriptionless-disables',
42+
message: `Disable for "${rule}" is missing a description`,
43+
severity: options.severity,
44+
commentNode,
45+
postcssResult,
46+
});
5747
}
5848
}
5949
}

‎lib/invalidScopeDisables.cjs‎

Lines changed: 22 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3,47 +3,41 @@
33
'use strict';
44

55
const optionsMatches = require('./utils/optionsMatches.cjs');
6+
const reportCommentProblem = require('./utils/reportCommentProblem.cjs');
67
const validateDisableSettings = require('./validateDisableSettings.cjs');
78

89
/**
9-
* @param {import('stylelint').LintResult[]} results
10+
* @param {import('stylelint').PostcssResult} postcssResult
11+
* @returns {void}
1012
*/
11-
function invalidScopeDisables(results) {
12-
for (const result of results) {
13-
const settings = validateDisableSettings(result._postcssResult, 'reportInvalidScopeDisables');
13+
function invalidScopeDisables(postcssResult) {
14+
const [enabled, options] = validateDisableSettings(postcssResult, 'reportInvalidScopeDisables');
1415

15-
if (!settings) continue;
16+
if (!options) return;
1617

17-
const [enabled, options, stylelintResult] = settings;
18+
const configRules = postcssResult.stylelint.config?.rules;
1819

19-
const configRules = (stylelintResult.config || {}).rules || {};
20+
if (!configRules) return;
2021

21-
const usedRules = new Set(Object.keys(configRules));
22+
const usedRules = new Set(Object.keys(configRules));
2223

23-
usedRules.add('all');
24+
usedRules.add('all');
2425

25-
for (const [rule, ruleRanges] of Object.entries(stylelintResult.disabledRanges)) {
26-
if (usedRules.has(rule)) continue;
26+
for (const [rule, ruleRanges] of Object.entries(postcssResult.stylelint.disabledRanges)) {
27+
if (usedRules.has(rule)) continue;
2728

28-
if (enabled === optionsMatches(options, 'except', rule)) continue;
29+
if (enabled === optionsMatches(options, 'except', rule)) continue;
2930

30-
for (const range of ruleRanges) {
31-
if (!range.strictStart && !range.strictEnd) continue;
31+
for (const range of ruleRanges) {
32+
if (!range.strictStart && !range.strictEnd) continue;
3233

33-
// If the comment doesn't have a location, we can't report a useful error.
34-
// In practice we expect all comments to have locations, though.
35-
if (!range.comment.source || !range.comment.source.start) continue;
36-
37-
result.warnings.push({
38-
text: `Rule "${rule}" isn't enabled`,
39-
rule: '--report-invalid-scope-disables',
40-
line: range.comment.source.start.line,
41-
column: range.comment.source.start.column,
42-
endLine: range.comment.source.end && range.comment.source.end.line,
43-
endColumn: range.comment.source.end && range.comment.source.end.column,
44-
severity: options.severity,
45-
});
46-
}
34+
reportCommentProblem({
35+
rule: '--report-invalid-scope-disables',
36+
message: `Rule "${rule}" isn't enabled`,
37+
severity: options.severity,
38+
commentNode: range.comment,
39+
postcssResult,
40+
});
4741
}
4842
}
4943
}

‎lib/invalidScopeDisables.mjs‎

Lines changed: 22 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,39 @@
11
import optionsMatches from './utils/optionsMatches.mjs';
2+
import reportCommentProblem from './utils/reportCommentProblem.mjs';
23
import validateDisableSettings from './validateDisableSettings.mjs';
34

45
/**
5-
* @param {import('stylelint').LintResult[]} results
6+
* @param {import('stylelint').PostcssResult} postcssResult
7+
* @returns {void}
68
*/
7-
export default function invalidScopeDisables(results) {
8-
for (const result of results) {
9-
const settings = validateDisableSettings(result._postcssResult, 'reportInvalidScopeDisables');
9+
export default function invalidScopeDisables(postcssResult) {
10+
const [enabled, options] = validateDisableSettings(postcssResult, 'reportInvalidScopeDisables');
1011

11-
if (!settings) continue;
12+
if (!options) return;
1213

13-
const [enabled, options, stylelintResult] = settings;
14+
const configRules = postcssResult.stylelint.config?.rules;
1415

15-
const configRules = (stylelintResult.config || {}).rules || {};
16+
if (!configRules) return;
1617

17-
const usedRules = new Set(Object.keys(configRules));
18+
const usedRules = new Set(Object.keys(configRules));
1819

19-
usedRules.add('all');
20+
usedRules.add('all');
2021

21-
for (const [rule, ruleRanges] of Object.entries(stylelintResult.disabledRanges)) {
22-
if (usedRules.has(rule)) continue;
22+
for (const [rule, ruleRanges] of Object.entries(postcssResult.stylelint.disabledRanges)) {
23+
if (usedRules.has(rule)) continue;
2324

24-
if (enabled === optionsMatches(options, 'except', rule)) continue;
25+
if (enabled === optionsMatches(options, 'except', rule)) continue;
2526

26-
for (const range of ruleRanges) {
27-
if (!range.strictStart && !range.strictEnd) continue;
27+
for (const range of ruleRanges) {
28+
if (!range.strictStart && !range.strictEnd) continue;
2829

29-
// If the comment doesn't have a location, we can't report a useful error.
30-
// In practice we expect all comments to have locations, though.
31-
if (!range.comment.source || !range.comment.source.start) continue;
32-
33-
result.warnings.push({
34-
text: `Rule "${rule}" isn't enabled`,
35-
rule: '--report-invalid-scope-disables',
36-
line: range.comment.source.start.line,
37-
column: range.comment.source.start.column,
38-
endLine: range.comment.source.end && range.comment.source.end.line,
39-
endColumn: range.comment.source.end && range.comment.source.end.column,
40-
severity: options.severity,
41-
});
42-
}
30+
reportCommentProblem({
31+
rule: '--report-invalid-scope-disables',
32+
message: `Rule "${rule}" isn't enabled`,
33+
severity: options.severity,
34+
commentNode: range.comment,
35+
postcssResult,
36+
});
4337
}
4438
}
4539
}

‎lib/lintSource.cjs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,15 @@
33
'use strict';
44

55
const node_path = require('node:path');
6+
const descriptionlessDisables = require('./descriptionlessDisables.cjs');
67
const getConfigForFile = require('./getConfigForFile.cjs');
78
const getPostcssResult = require('./getPostcssResult.cjs');
9+
const invalidScopeDisables = require('./invalidScopeDisables.cjs');
810
const isPathIgnored = require('./isPathIgnored.cjs');
911
const isPathNotFoundError = require('./utils/isPathNotFoundError.cjs');
1012
const lintPostcssResult = require('./lintPostcssResult.cjs');
13+
const needlessDisables = require('./needlessDisables.cjs');
14+
const reportDisables = require('./reportDisables.cjs');
1115

1216
/** @typedef {import('stylelint').InternalApi} StylelintInternalApi */
1317
/** @typedef {import('stylelint').GetLintSourceOptions} Options */
@@ -110,6 +114,11 @@ async function lintSource(stylelint, options = {}) {
110114

111115
await lintPostcssResult(stylelint._options, stylelintPostcssResult, config);
112116

117+
reportDisables(stylelintPostcssResult);
118+
needlessDisables(stylelintPostcssResult);
119+
invalidScopeDisables(stylelintPostcssResult);
120+
descriptionlessDisables(stylelintPostcssResult);
121+
113122
return stylelintPostcssResult;
114123
}
115124

0 commit comments

Comments
 (0)