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.

Sign up for GitHub

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

Jump to bottom

Refactor to improve types for lib/assignDisabledRanges.js #6014

Merged
merged 5 commits into from
Apr 18, 2022
Merged

Refactor to improve types for lib/assignDisabledRanges.js #6014

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6db34af
Refactor to improve types for `lib/assignDisabledRanges.js`
ybiquitous Apr 15, 2022
7284b5f
Remove unreachable `else` block
ybiquitous Apr 15, 2022
4aad33c
Use `ALL_RULES` constant as possible
ybiquitous Apr 15, 2022
228c37e
Merge branch 'main' into refactor-assignDisabledRanges.js
ybiquitous Apr 15, 2022
ea33cde
Merge branch 'main' into refactor-assignDisabledRanges.js
ybiquitous Apr 18, 2022
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
49 changes: 26 additions & 23 deletions lib/assignDisabledRanges.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
'use strict';

const isStandardSyntaxComment = require('./utils/isStandardSyntaxComment');
const { assertNumber } = require('./utils/validateTypes');
const { assert, assertNumber, assertString } = require('./utils/validateTypes');

const COMMAND_PREFIX = 'stylelint-';
const disableCommand = `${COMMAND_PREFIX}disable`;
Expand Down Expand Up @@ -53,10 +53,10 @@ module.exports = function assignDisabledRanges(root, result) {

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

result.stylelint.disabledRanges = disabledRanges;
Expand Down Expand Up @@ -237,9 +237,13 @@ module.exports = function assignDisabledRanges(root, result) {

if (ruleToEnable === ALL_RULES) {
if (
Object.values(disabledRanges).every(
(ranges) => ranges.length === 0 || typeof ranges[ranges.length - 1].end === 'number',
)
Object.values(disabledRanges).every((ranges) => {
if (ranges.length === 0) return true;

const lastRange = ranges[ranges.length - 1];

return lastRange && typeof lastRange.end === 'number';
})
) {
throw comment.error('No rules have been disabled', {
plugin: 'stylelint',
Expand All @@ -259,18 +263,10 @@ module.exports = function assignDisabledRanges(root, result) {

if (ruleIsDisabled(ALL_RULES) && disabledRanges[ruleToEnable] === undefined) {
// Get a starting point from the where all rules were disabled
if (!disabledRanges[ruleToEnable]) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[note] If ruleIsDisabled(ALL_RULES) && disabledRanges[ruleToEnable] === undefined is true, !disabledRanges[ruleToEnable] should always be true. So, the else block should never be executed.

disabledRanges[ruleToEnable] = disabledRanges.all.map(({ start, end, description }) =>
disabledRanges[ruleToEnable] = disabledRanges[ALL_RULES].map(
({ start, end, description }) =>
createDisableRange(comment, start, false, description, end, false),
);
} else {
const ranges = disabledRanges[ALL_RULES];
const range = ranges ? ranges[ranges.length - 1] : null;

if (range) {
disabledRanges[ruleToEnable].push({ ...range });
}
}
);

endDisabledRange(endLine, ruleToEnable, true);

Expand Down Expand Up @@ -318,9 +314,11 @@ module.exports = function assignDisabledRanges(root, result) {
* @returns {string[]}
*/
function getCommandRules(command, fullText) {
const rules = fullText
.slice(command.length)
.split(/\s-{2,}\s/u)[0] // Allow for description (f.e. /* stylelint-disable a, b -- Description */).
// Allow for description (f.e. /* stylelint-disable a, b -- Description */).
const splitted = fullText.slice(command.length).split(/\s-{2,}\s/u)[0];

assertString(splitted);
const rules = splitted
.trim()
.split(',')
.filter(Boolean)
Expand Down Expand Up @@ -356,7 +354,11 @@ module.exports = function assignDisabledRanges(root, result) {
const rangeObj = createDisableRange(comment, line, strict, description);

ensureRuleRanges(ruleName);
disabledRanges[ruleName].push(rangeObj);

const range = disabledRanges[ruleName];

assert(range);
range.push(rangeObj);
}

/**
Expand All @@ -382,8 +384,9 @@ module.exports = function assignDisabledRanges(root, result) {
*/
function ensureRuleRanges(ruleName) {
if (!disabledRanges[ruleName]) {
disabledRanges[ruleName] = disabledRanges.all.map(({ comment, start, end, description }) =>
createDisableRange(comment, start, false, description, end, false),
disabledRanges[ruleName] = disabledRanges[ALL_RULES].map(
({ comment, start, end, description }) =>
createDisableRange(comment, start, false, description, end, false),
);
}
}
Expand Down