Skip to content

Commit

Permalink
Refactor to use shared constant for disabledRanges object (#7722)
Browse files Browse the repository at this point in the history 
`'all'` is a special rule name for `stylelint-disable` comments.
This change makes this name a constant shared in multiple files.
  • Loading branch information
@ybiquitous
ybiquitous committed May 27, 2024
1 parent b85be3c commit 72ec18a
Show file tree
Hide file tree
Showing 14 changed files with 95 additions and 78 deletions.
42 changes: 20 additions & 22 deletions lib/assignDisabledRanges.cjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,11 @@
// please instead edit the ESM counterpart and rebuild with Rollup (npm run build).
'use strict';

const constants = require('./constants.cjs');
const configurationComment = require('./utils/configurationComment.cjs');
const validateTypes = require('./utils/validateTypes.cjs');
const isStandardSyntaxComment = require('./utils/isStandardSyntaxComment.cjs');

const ALL_RULES = 'all';

/** @typedef {import('postcss').Comment} PostcssComment */
/** @typedef {import('postcss').Root} PostcssRoot */
/** @typedef {import('postcss').Document} PostcssDocument */
Expand Down Expand Up @@ -51,13 +50,14 @@ function assignDisabledRanges(root, result) {

/**
* Most of the functions below work via side effects mutating this object
* @type {DisabledRangeObject & { all: DisabledRange[] }}
* @type {DisabledRangeObject}
*/
const disabledRanges = {
[ALL_RULES]: [],
};
const disabledRanges = result.stylelint.disabledRanges;

/** @type {DisabledRange[]} */
const disabledRangesAll = [];

result.stylelint.disabledRanges = disabledRanges;
disabledRanges[constants.RULE_NAME_ALL] = disabledRangesAll;

// Work around postcss/postcss-scss#109 by merging adjacent `//` comments
// into a single node before passing to `checkComment`.
Expand Down Expand Up @@ -160,17 +160,17 @@ function assignDisabledRanges(root, result) {
* @param {string|undefined} description
*/
function disableLine(comment, line, ruleName, description) {
if (ruleIsDisabled(ALL_RULES)) {
if (ruleIsDisabled(constants.RULE_NAME_ALL)) {
throw comment.error('All rules have already been disabled', {
plugin: 'stylelint',
});
}

if (ruleName === ALL_RULES) {
if (ruleName === constants.RULE_NAME_ALL) {
for (const disabledRuleName of Object.keys(disabledRanges)) {
if (ruleIsDisabled(disabledRuleName)) continue;

const strict = disabledRuleName === ALL_RULES;
const strict = disabledRuleName === constants.RULE_NAME_ALL;

startDisabledRange(comment, line, disabledRuleName, strict, description);
endDisabledRange(line, disabledRuleName, strict);
Expand All @@ -194,7 +194,7 @@ function assignDisabledRanges(root, result) {
const description = getDescription(comment.text);

for (const ruleToDisable of getCommandRules(configurationComment.DISABLE_COMMAND, comment.text)) {
const isAllRules = ruleToDisable === ALL_RULES;
const isAllRules = ruleToDisable === constants.RULE_NAME_ALL;

if (ruleIsDisabled(ruleToDisable)) {
throw comment.error(
Expand All @@ -212,7 +212,7 @@ function assignDisabledRanges(root, result) {

if (isAllRules) {
for (const ruleName of Object.keys(disabledRanges)) {
startDisabledRange(comment, line, ruleName, ruleName === ALL_RULES, description);
startDisabledRange(comment, line, ruleName, ruleName === constants.RULE_NAME_ALL, description);
}
} else {
startDisabledRange(comment, line, ruleToDisable, true, description);
Expand All @@ -231,7 +231,7 @@ function assignDisabledRanges(root, result) {

validateTypes.assertNumber(endLine);

if (ruleToEnable === ALL_RULES) {
if (ruleToEnable === constants.RULE_NAME_ALL) {
if (
Object.values(disabledRanges).every((ranges) => {
if (ranges.length === 0) return true;
Expand All @@ -250,18 +250,17 @@ function assignDisabledRanges(root, result) {
const lastRange = ranges[ranges.length - 1];

if (!lastRange || !lastRange.end) {
endDisabledRange(endLine, ruleName, ruleName === ALL_RULES);
endDisabledRange(endLine, ruleName, ruleName === constants.RULE_NAME_ALL);
}
}

continue;
}

if (ruleIsDisabled(ALL_RULES) && disabledRanges[ruleToEnable] === undefined) {
if (ruleIsDisabled(constants.RULE_NAME_ALL) && disabledRanges[ruleToEnable] === undefined) {
// Get a starting point from the where all rules were disabled
disabledRanges[ruleToEnable] = disabledRanges[ALL_RULES].map(
({ start, end, description }) =>
createDisableRange(comment, start, false, description, end, false),
disabledRanges[ruleToEnable] = disabledRangesAll.map(({ start, end, description }) =>
createDisableRange(comment, start, false, description, end, false),
);

endDisabledRange(endLine, ruleToEnable, true);
Expand Down Expand Up @@ -325,7 +324,7 @@ function assignDisabledRanges(root, result) {
.map((r) => r.trim());

if (rules.length === 0) {
return [ALL_RULES];
return [constants.RULE_NAME_ALL];
}

return rules;
Expand Down Expand Up @@ -384,9 +383,8 @@ function assignDisabledRanges(root, result) {
*/
function ensureRuleRanges(ruleName) {
if (!disabledRanges[ruleName]) {
disabledRanges[ruleName] = disabledRanges[ALL_RULES].map(
({ comment, start, end, description }) =>
createDisableRange(comment, start, false, description, end, false),
disabledRanges[ruleName] = disabledRangesAll.map(({ comment, start, end, description }) =>
createDisableRange(comment, start, false, description, end, false),
);
}
}
Expand Down
43 changes: 21 additions & 22 deletions lib/assignDisabledRanges.mjs
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { RULE_NAME_ALL } from './constants.mjs';

import {
DISABLE_COMMAND,
DISABLE_LINE_COMMAND,
Expand All @@ -10,8 +12,6 @@ import {
import { assert, assertNumber, assertString } from './utils/validateTypes.mjs';
import isStandardSyntaxComment from './utils/isStandardSyntaxComment.mjs';

const ALL_RULES = 'all';

/** @typedef {import('postcss').Comment} PostcssComment */
/** @typedef {import('postcss').Root} PostcssRoot */
/** @typedef {import('postcss').Document} PostcssDocument */
Expand Down Expand Up @@ -55,13 +55,14 @@ export default function assignDisabledRanges(root, result) {

/**
* Most of the functions below work via side effects mutating this object
* @type {DisabledRangeObject & { all: DisabledRange[] }}
* @type {DisabledRangeObject}
*/
const disabledRanges = {
[ALL_RULES]: [],
};
const disabledRanges = result.stylelint.disabledRanges;

result.stylelint.disabledRanges = disabledRanges;
/** @type {DisabledRange[]} */
const disabledRangesAll = [];

disabledRanges[RULE_NAME_ALL] = disabledRangesAll;

// Work around postcss/postcss-scss#109 by merging adjacent `//` comments
// into a single node before passing to `checkComment`.
Expand Down Expand Up @@ -164,17 +165,17 @@ export default function assignDisabledRanges(root, result) {
* @param {string|undefined} description
*/
function disableLine(comment, line, ruleName, description) {
if (ruleIsDisabled(ALL_RULES)) {
if (ruleIsDisabled(RULE_NAME_ALL)) {
throw comment.error('All rules have already been disabled', {
plugin: 'stylelint',
});
}

if (ruleName === ALL_RULES) {
if (ruleName === RULE_NAME_ALL) {
for (const disabledRuleName of Object.keys(disabledRanges)) {
if (ruleIsDisabled(disabledRuleName)) continue;

const strict = disabledRuleName === ALL_RULES;
const strict = disabledRuleName === RULE_NAME_ALL;

startDisabledRange(comment, line, disabledRuleName, strict, description);
endDisabledRange(line, disabledRuleName, strict);
Expand All @@ -198,7 +199,7 @@ export default function assignDisabledRanges(root, result) {
const description = getDescription(comment.text);

for (const ruleToDisable of getCommandRules(DISABLE_COMMAND, comment.text)) {
const isAllRules = ruleToDisable === ALL_RULES;
const isAllRules = ruleToDisable === RULE_NAME_ALL;

if (ruleIsDisabled(ruleToDisable)) {
throw comment.error(
Expand All @@ -216,7 +217,7 @@ export default function assignDisabledRanges(root, result) {

if (isAllRules) {
for (const ruleName of Object.keys(disabledRanges)) {
startDisabledRange(comment, line, ruleName, ruleName === ALL_RULES, description);
startDisabledRange(comment, line, ruleName, ruleName === RULE_NAME_ALL, description);
}
} else {
startDisabledRange(comment, line, ruleToDisable, true, description);
Expand All @@ -235,7 +236,7 @@ export default function assignDisabledRanges(root, result) {

assertNumber(endLine);

if (ruleToEnable === ALL_RULES) {
if (ruleToEnable === RULE_NAME_ALL) {
if (
Object.values(disabledRanges).every((ranges) => {
if (ranges.length === 0) return true;
Expand All @@ -254,18 +255,17 @@ export default function assignDisabledRanges(root, result) {
const lastRange = ranges[ranges.length - 1];

if (!lastRange || !lastRange.end) {
endDisabledRange(endLine, ruleName, ruleName === ALL_RULES);
endDisabledRange(endLine, ruleName, ruleName === RULE_NAME_ALL);
}
}

continue;
}

if (ruleIsDisabled(ALL_RULES) && disabledRanges[ruleToEnable] === undefined) {
if (ruleIsDisabled(RULE_NAME_ALL) && disabledRanges[ruleToEnable] === undefined) {
// Get a starting point from the where all rules were disabled
disabledRanges[ruleToEnable] = disabledRanges[ALL_RULES].map(
({ start, end, description }) =>
createDisableRange(comment, start, false, description, end, false),
disabledRanges[ruleToEnable] = disabledRangesAll.map(({ start, end, description }) =>
createDisableRange(comment, start, false, description, end, false),
);

endDisabledRange(endLine, ruleToEnable, true);
Expand Down Expand Up @@ -329,7 +329,7 @@ export default function assignDisabledRanges(root, result) {
.map((r) => r.trim());

if (rules.length === 0) {
return [ALL_RULES];
return [RULE_NAME_ALL];
}

return rules;
Expand Down Expand Up @@ -388,9 +388,8 @@ export default function assignDisabledRanges(root, result) {
*/
function ensureRuleRanges(ruleName) {
if (!disabledRanges[ruleName]) {
disabledRanges[ruleName] = disabledRanges[ALL_RULES].map(
({ comment, start, end, description }) =>
createDisableRange(comment, start, false, description, end, false),
disabledRanges[ruleName] = disabledRangesAll.map(({ comment, start, end, description }) =>
createDisableRange(comment, start, false, description, end, false),
);
}
}
Expand Down
3 changes: 3 additions & 0 deletions lib/constants.cjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ const EXIT_CODE_LINT_PROBLEM = 2;
const EXIT_CODE_INVALID_USAGE = 64;
const EXIT_CODE_INVALID_CONFIG = 78;

const RULE_NAME_ALL = 'all';

exports.CACHE_STRATEGY_CONTENT = CACHE_STRATEGY_CONTENT;
exports.CACHE_STRATEGY_METADATA = CACHE_STRATEGY_METADATA;
exports.DEFAULT_CACHE_LOCATION = DEFAULT_CACHE_LOCATION;
Expand All @@ -29,3 +31,4 @@ exports.EXIT_CODE_INVALID_CONFIG = EXIT_CODE_INVALID_CONFIG;
exports.EXIT_CODE_INVALID_USAGE = EXIT_CODE_INVALID_USAGE;
exports.EXIT_CODE_LINT_PROBLEM = EXIT_CODE_LINT_PROBLEM;
exports.EXIT_CODE_SUCCESS = EXIT_CODE_SUCCESS;
exports.RULE_NAME_ALL = RULE_NAME_ALL;
2 changes: 2 additions & 0 deletions lib/constants.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,5 @@ export const EXIT_CODE_FATAL_ERROR = 1;
export const EXIT_CODE_LINT_PROBLEM = 2;
export const EXIT_CODE_INVALID_USAGE = 64;
export const EXIT_CODE_INVALID_CONFIG = 78;

export const RULE_NAME_ALL = 'all';
3 changes: 2 additions & 1 deletion lib/descriptionlessDisables.cjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// please instead edit the ESM counterpart and rebuild with Rollup (npm run build).
'use strict';

const constants = require('./constants.cjs');
const optionsMatches = require('./utils/optionsMatches.cjs');
const reportCommentProblem = require('./utils/reportCommentProblem.cjs');
const validateDisableSettings = require('./validateDisableSettings.cjs');
Expand Down Expand Up @@ -34,7 +35,7 @@ function descriptionlessDisables(postcssResult) {
// configuration is `[false, {except: ['specific-rule']}]`, we
// don't want to report the copies that match except, so we record
// the comment as already reported.
if (!enabled && rule === 'all') alreadyReported.add(commentNode);
if (!enabled && rule === constants.RULE_NAME_ALL) alreadyReported.add(commentNode);

continue;
}
Expand Down
4 changes: 3 additions & 1 deletion lib/descriptionlessDisables.mjs
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { RULE_NAME_ALL } from './constants.mjs';

import optionsMatches from './utils/optionsMatches.mjs';
import reportCommentProblem from './utils/reportCommentProblem.mjs';
import validateDisableSettings from './validateDisableSettings.mjs';
Expand Down Expand Up @@ -30,7 +32,7 @@ export default function descriptionlessDisables(postcssResult) {
// configuration is `[false, {except: ['specific-rule']}]`, we
// don't want to report the copies that match except, so we record
// the comment as already reported.
if (!enabled && rule === 'all') alreadyReported.add(commentNode);
if (!enabled && rule === RULE_NAME_ALL) alreadyReported.add(commentNode);

continue;
}
Expand Down
3 changes: 2 additions & 1 deletion lib/invalidScopeDisables.cjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// please instead edit the ESM counterpart and rebuild with Rollup (npm run build).
'use strict';

const constants = require('./constants.cjs');
const optionsMatches = require('./utils/optionsMatches.cjs');
const reportCommentProblem = require('./utils/reportCommentProblem.cjs');
const validateDisableSettings = require('./validateDisableSettings.cjs');
Expand All @@ -21,7 +22,7 @@ function invalidScopeDisables(postcssResult) {

const usedRules = new Set(Object.keys(configRules));

usedRules.add('all');
usedRules.add(constants.RULE_NAME_ALL);

for (const [rule, ruleRanges] of Object.entries(postcssResult.stylelint.disabledRanges)) {
if (usedRules.has(rule)) continue;
Expand Down
4 changes: 3 additions & 1 deletion lib/invalidScopeDisables.mjs
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { RULE_NAME_ALL } from './constants.mjs';

import optionsMatches from './utils/optionsMatches.mjs';
import reportCommentProblem from './utils/reportCommentProblem.mjs';
import validateDisableSettings from './validateDisableSettings.mjs';
Expand All @@ -17,7 +19,7 @@ export default function invalidScopeDisables(postcssResult) {

const usedRules = new Set(Object.keys(configRules));

usedRules.add('all');
usedRules.add(RULE_NAME_ALL);

for (const [rule, ruleRanges] of Object.entries(postcssResult.stylelint.disabledRanges)) {
if (usedRules.has(rule)) continue;
Expand Down
6 changes: 2 additions & 4 deletions lib/lintPostcssResult.cjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

const node_os = require('node:os');
const configurationComment = require('./utils/configurationComment.cjs');
const constants = require('./constants.cjs');
const assignDisabledRanges = require('./assignDisabledRanges.cjs');
const getStylelintRule = require('./utils/getStylelintRule.cjs');
const reportUnknownRuleNames = require('./reportUnknownRuleNames.cjs');
Expand Down Expand Up @@ -137,10 +138,7 @@ async function lintPostcssResult(stylelintOptions, postcssResult, config) {
* @returns {boolean}
*/
function isFixCompatible({ stylelint }) {
// Check for issue #2643
if (stylelint.disabledRanges.all && stylelint.disabledRanges.all.length) return false;

return true;
return !stylelint.disabledRanges[constants.RULE_NAME_ALL]?.length;
}

/**
Expand Down
6 changes: 2 additions & 4 deletions lib/lintPostcssResult.mjs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { EOL } from 'node:os';

import { DEFAULT_CONFIGURATION_COMMENT } from './utils/configurationComment.mjs';
import { RULE_NAME_ALL } from './constants.mjs';
import assignDisabledRanges from './assignDisabledRanges.mjs';
import getStylelintRule from './utils/getStylelintRule.mjs';
import reportUnknownRuleNames from './reportUnknownRuleNames.mjs';
Expand Down Expand Up @@ -134,10 +135,7 @@ export default async function lintPostcssResult(stylelintOptions, postcssResult,
* @returns {boolean}
*/
function isFixCompatible({ stylelint }) {
// Check for issue #2643
if (stylelint.disabledRanges.all && stylelint.disabledRanges.all.length) return false;

return true;
return !stylelint.disabledRanges[RULE_NAME_ALL]?.length;
}

/**
Expand Down
Loading

0 comments on commit 72ec18a

Please sign in to comment.