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 max-* rules #5508

Merged
merged 2 commits into from Sep 2, 2021
Merged

Refactor to improve types for max-* rules #5508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c6c2536
Refactor to improve types for `max-*` rules
ybiquitous Sep 2, 2021
3006c57
Merge branch 'v14' into refactor-types-for-max-rule
ybiquitous Sep 2, 2021
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
58 changes: 35 additions & 23 deletions lib/rules/max-empty-lines/index.js
View file
Open in desktop
@@ -1,5 +1,3 @@
// @ts-nocheck

'use strict';

const optionsMatches = require('../../utils/optionsMatches');
Expand All @@ -15,7 +13,8 @@ const messages = ruleMessages(ruleName, {
expected: (max) => `Expected no more than ${max} empty ${max === 1 ? 'line' : 'lines'}`,
});

function rule(max, options, context) {
/** @type {import('stylelint').StylelintRule} */
const rule = (primary, secondaryOptions, context) => {
let emptyLines = 0;
let lastIndex = -1;

Expand All @@ -24,11 +23,11 @@ function rule(max, options, context) {
result,
ruleName,
{
actual: max,
actual: primary,
possible: isNumber,
},
{
actual: options,
actual: secondaryOptions,
possible: {
ignore: ['comments'],
},
Expand All @@ -40,20 +39,18 @@ function rule(max, options, context) {
return;
}

const ignoreComments = optionsMatches(options, 'ignore', 'comments');
const getChars = replaceEmptyLines.bind(null, max);
const ignoreComments = optionsMatches(secondaryOptions, 'ignore', 'comments');
const getChars = replaceEmptyLines.bind(null, primary);

/**
* 1. walk nodes & replace enterchar
* 2. deal with special case.
*/
if (context.fix) {
root.walk((node) => {
if (node.type === 'comment') {
if (!ignoreComments) {
node.raws.left = getChars(node.raws.left);
node.raws.right = getChars(node.raws.right);
}
if (node.type === 'comment' && !ignoreComments) {
node.raws.left = getChars(node.raws.left);
node.raws.right = getChars(node.raws.right);
}

if (node.raws.before) {
Expand All @@ -67,18 +64,19 @@ function rule(max, options, context) {
const rootRawsAfter = root.raws.after;

// not document node
// @ts-expect-error -- TS2339: Property 'document' does not exist on type 'Root'.
Copy link
Member

Choose a reason for hiding this comment

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

We'll likely need to revisit this document stuff at some point in light of postcss/postcss#1582.

I think it's mainly the max whitespace rules that touch it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I also think so. 👍🏼
I didn't know how I should handle it, so I've left // @ts-expect-error comments.

Copy link
Member

Choose a reason for hiding this comment

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

Leaving // @ts-expect-error comments was the right thing to do.

Feel free to merge.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you!

if ((root.document && root.document.constructor.name) !== 'Document') {
if (firstNodeRawsBefore) {
root.first.raws.before = getChars(firstNodeRawsBefore, true);
}

if (rootRawsAfter) {
// when max setted 0, should be treated as 1 in this situation.
root.raws.after = replaceEmptyLines(max === 0 ? 1 : max, rootRawsAfter, true);
root.raws.after = replaceEmptyLines(primary === 0 ? 1 : primary, rootRawsAfter, true);
}
} else if (rootRawsAfter) {
// `css in js` or `html`
root.raws.after = replaceEmptyLines(max === 0 ? 1 : max, rootRawsAfter);
root.raws.after = replaceEmptyLines(primary === 0 ? 1 : primary, rootRawsAfter);
}

return;
Expand All @@ -99,6 +97,12 @@ function rule(max, options, context) {
},
);

/**
* @param {string} source
* @param {number} matchStartIndex
* @param {number} matchEndIndex
* @param {import('postcss').Root} node
*/
function checkMatch(source, matchStartIndex, matchEndIndex, node) {
const eof = matchEndIndex === source.length;
let violation = false;
Expand All @@ -112,13 +116,13 @@ function rule(max, options, context) {

lastIndex = matchEndIndex;

if (emptyLines > max) violation = true;
if (emptyLines > primary) violation = true;

if (!eof && !violation) return;

if (violation) {
report({
message: messages.expected(max),
message: messages.expected(primary),
node,
index: matchStartIndex,
result,
Expand All @@ -127,12 +131,12 @@ function rule(max, options, context) {
}

// Additional check for end of file
if (eof && max) {
if (eof && primary) {
emptyLines++;

if (emptyLines > max && isEofNode(result.root, node)) {
if (emptyLines > primary && isEofNode(result.root, node)) {
report({
message: messages.expected(max),
message: messages.expected(primary),
node,
index: matchEndIndex,
result,
Expand All @@ -142,6 +146,11 @@ function rule(max, options, context) {
}
}

/**
* @param {number} maxLines
* @param {unknown} str
* @param {boolean?} isSpecialCase
*/
function replaceEmptyLines(maxLines, str, isSpecialCase = false) {
const repeatTimes = isSpecialCase ? maxLines : maxLines + 1;

Expand Down Expand Up @@ -169,28 +178,31 @@ function rule(max, options, context) {
});
}
};
}
};

/**
* Checks whether the given node is the last node of file.
* @param {Document|null} document the document node with `postcss-html` and `postcss-jsx`.
* @param {Root} root the root node of css
* @param {import('stylelint').PostcssResult['root']} document - the document node with `postcss-html` and `postcss-jsx`.
* @param {import('postcss').Root} root - the root node of css
*/
function isEofNode(document, root) {
if (!document || document.constructor.name !== 'Document') {
if (!document || document.constructor.name !== 'Document' || !('type' in document)) {
return true;
}

// In the `postcss-html` and `postcss-jsx` syntax, checks that there is text after the given node.
let after;

// @ts-expect-error -- TS2367: This condition will always return 'false' since the types 'Root' and 'ChildNode | undefined' have no overlap.
if (root === document.last) {
after = document.raws && document.raws.afterEnd;
} else {
// @ts-expect-error -- TS2345: Argument of type 'Root' is not assignable to parameter of type 'number | ChildNode'.
const rootIndex = document.index(root);

const nextNode = document.nodes[rootIndex + 1];

// @ts-expect-error -- TS2339: Property 'beforeStart' does not exist on type 'AtRuleRaws | RuleRaws | DeclarationRaws | CommentRaws'.
after = nextNode && nextNode.raws && nextNode.raws.beforeStart;
}

Expand Down
40 changes: 27 additions & 13 deletions lib/rules/max-line-length/index.js
View file
Open in desktop
@@ -1,5 +1,3 @@
// @ts-nocheck

'use strict';

const execall = require('execall');
Expand All @@ -21,17 +19,18 @@ const messages = ruleMessages(ruleName, {
`Expected line length to be no more than ${max} ${max === 1 ? 'character' : 'characters'}`,
});

function rule(maxLength, options, context) {
/** @type {import('stylelint').StylelintRule} */
const rule = (primary, secondaryOptions, context) => {
return (root, result) => {
const validOptions = validateOptions(
result,
ruleName,
{
actual: maxLength,
actual: primary,
possible: isNumber,
},
{
actual: options,
actual: secondaryOptions,
possible: {
ignore: ['non-comments', 'comments'],
ignorePattern: [isString, isRegExp],
Expand All @@ -44,10 +43,15 @@ function rule(maxLength, options, context) {
return;
}

const ignoreNonComments = optionsMatches(options, 'ignore', 'non-comments');
const ignoreComments = optionsMatches(options, 'ignore', 'comments');
if (root.source == null) {
throw new Error('The root node must have a source');
}

const ignoreNonComments = optionsMatches(secondaryOptions, 'ignore', 'non-comments');
const ignoreComments = optionsMatches(secondaryOptions, 'ignore', 'comments');
const rootString = context.fix ? root.toString() : root.source.input.css;
// Array of skipped sub strings, i.e `url(...)`, `@import "..."`
/** @type {Array<[number, number]>} */
let skippedSubStrings = [];
let skippedSubStringsIndex = 0;

Expand All @@ -69,16 +73,23 @@ function rule(maxLength, options, context) {
checkNewline(match),
);

/**
* @param {number} index
*/
function complain(index) {
report({
index,
result,
ruleName,
message: messages.expected(maxLength),
message: messages.expected(primary),
node: root,
});
}

/**
* @param {number} start
* @param {number} end
*/
function tryToPopSubString(start, end) {
const [startSubString, endSubString] = skippedSubStrings[skippedSubStringsIndex];

Expand All @@ -98,6 +109,9 @@ function rule(maxLength, options, context) {
return excluded;
}

/**
* @param {import('style-search').StyleSearchMatch | { endIndex: number }} match
*/
function checkNewline(match) {
let nextNewlineIndex = rootString.indexOf('\n', match.endIndex);

Expand All @@ -117,22 +131,22 @@ function rule(maxLength, options, context) {
const lineText = rootString.slice(match.endIndex, nextNewlineIndex);

// Case sensitive ignorePattern match
if (optionsMatches(options, 'ignorePattern', lineText)) {
if (optionsMatches(secondaryOptions, 'ignorePattern', lineText)) {
return;
}

// If the line's length is less than or equal to the specified
// max, ignore it ... So anything below is liable to be complained about.
// **Note that the length of any url arguments or import urls
// are excluded from the calculation.**
if (rawLineLength - excludedLength <= maxLength) {
if (rawLineLength - excludedLength <= primary) {
return;
}

const complaintIndex = nextNewlineIndex - 1;

if (ignoreComments) {
if (match.insideComment) {
if ('insideComment' in match && match.insideComment) {
return;
}

Expand All @@ -147,7 +161,7 @@ function rule(maxLength, options, context) {
}

if (ignoreNonComments) {
if (match.insideComment) {
if ('insideComment' in match && match.insideComment) {
return complain(complaintIndex);
}

Expand All @@ -173,7 +187,7 @@ function rule(maxLength, options, context) {
return complain(complaintIndex);
}
};
}
};

rule.ruleName = ruleName;
rule.messages = messages;
Expand Down