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

Refactor to improve types for some rules #5498

Merged
merged 1 commit into from
Aug 25, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
9 changes: 4 additions & 5 deletions lib/rules/keyframe-declaration-no-important/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// @ts-nocheck

'use strict';

const report = require('../../utils/report');
Expand All @@ -12,9 +10,10 @@ const messages = ruleMessages(ruleName, {
rejected: 'Unexpected !important',
});

function rule(actual) {
/** @type {import('stylelint').StylelintRule} */
const rule = (primary) => {
return (root, result) => {
const validOptions = validateOptions(result, ruleName, { actual });
const validOptions = validateOptions(result, ruleName, { actual: primary });

if (!validOptions) {
return;
Expand All @@ -36,7 +35,7 @@ function rule(actual) {
});
});
};
}
};

rule.ruleName = ruleName;
rule.messages = messages;
Expand Down
13 changes: 6 additions & 7 deletions lib/rules/keyframes-name-pattern/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// @ts-nocheck

'use strict';

const atRuleParamIndex = require('../../utils/atRuleParamIndex');
Expand All @@ -15,18 +13,19 @@ const messages = ruleMessages(ruleName, {
`Expected keyframe name "${keyframeName}" to match pattern "${pattern}"`,
});

function rule(pattern) {
/** @type {import('stylelint').StylelintRule} */
const rule = (primary) => {
return (root, result) => {
const validOptions = validateOptions(result, ruleName, {
actual: pattern,
actual: primary,
possible: [isRegExp, isString],
});

if (!validOptions) {
return;
}

const regex = isString(pattern) ? new RegExp(pattern) : pattern;
const regex = isString(primary) ? new RegExp(primary) : primary;

root.walkAtRules(/keyframes/i, (keyframesNode) => {
const value = keyframesNode.params;
Expand All @@ -37,14 +36,14 @@ function rule(pattern) {

report({
index: atRuleParamIndex(keyframesNode),
message: messages.expected(value, pattern),
message: messages.expected(value, primary),
node: keyframesNode,
ruleName,
result,
});
});
};
}
};

rule.ruleName = ruleName;
rule.messages = messages;
Expand Down
50 changes: 43 additions & 7 deletions lib/rules/length-zero-no-unit/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// @ts-nocheck

'use strict';

const valueParser = require('postcss-value-parser');
Expand All @@ -26,7 +24,8 @@ const messages = ruleMessages(ruleName, {
rejected: 'Unexpected unit',
});

function rule(primary, secondary, context) {
/** @type {import('stylelint').StylelintRule} */
const rule = (primary, secondaryOptions, context) => {
return (root, result) => {
const validOptions = validateOptions(
result,
Expand All @@ -35,7 +34,7 @@ function rule(primary, secondary, context) {
actual: primary,
},
{
actual: secondary,
actual: secondaryOptions,
possible: {
ignore: ['custom-properties'],
ignoreFunctions: [isString, isRegExp],
Expand All @@ -48,12 +47,17 @@ function rule(primary, secondary, context) {

let needsFix;

/**
* @param {import('postcss').Node} node
* @param {number} nodeIndex
* @param {import('postcss-value-parser').Node} valueNode
*/
function check(node, nodeIndex, valueNode) {
const { value, sourceIndex } = valueNode;

if (isMathFunction(valueNode)) return false;

if (isFunction(valueNode) && optionsMatches(secondary, 'ignoreFunctions', value))
if (isFunction(valueNode) && optionsMatches(secondaryOptions, 'ignoreFunctions', value))
return false;

if (!isWord(valueNode)) return;
Expand Down Expand Up @@ -88,6 +92,9 @@ function rule(primary, secondary, context) {
});
}

/**
* @param {import('postcss').AtRule} node
*/
function checkAtRule(node) {
if (!isStandardSyntaxAtRule(node)) return;

Expand All @@ -105,6 +112,9 @@ function rule(primary, secondary, context) {
}
}

/**
* @param {import('postcss').Declaration} node
*/
function checkDecl(node) {
needsFix = false;

Expand All @@ -114,7 +124,7 @@ function rule(primary, secondary, context) {

if (isFlex(prop)) return;

if (optionsMatches(secondary, 'ignore', 'custom-properties') && isCustomProperty(prop))
if (optionsMatches(secondaryOptions, 'ignore', 'custom-properties') && isCustomProperty(prop))
return;

const index = declarationValueIndex(node);
Expand All @@ -134,8 +144,13 @@ function rule(primary, secondary, context) {
root.walkAtRules(checkAtRule);
root.walkDecls(checkDecl);
};
}
};

/**
* @param {import('postcss').Declaration} decl
* @param {import('postcss-value-parser').Node[]} nodes
* @param {number} index
*/
function isLineHeightValue({ prop }, nodes, index) {
return (
prop.toLowerCase() === 'font' &&
Expand All @@ -145,30 +160,51 @@ function isLineHeightValue({ prop }, nodes, index) {
);
}

/**
* @param {string} prop
*/
function isLineHeight(prop) {
return prop.toLowerCase() === 'line-height';
}

/**
* @param {string} prop
*/
function isFlex(prop) {
return prop.toLowerCase() === 'flex';
}

/**
* @param {import('postcss-value-parser').Node} node
*/
function isWord({ type }) {
return type === 'word';
}

/**
* @param {string} unit
*/
function isLength(unit) {
return keywordSets.lengthUnits.has(unit.toLowerCase());
}

/**
* @param {import('postcss-value-parser').Node} node
*/
function isFunction({ type }) {
return type === 'function';
}

/**
* @param {string} unit
*/
function isFraction(unit) {
return unit.toLowerCase() === 'fr';
}

/**
* @param {string} number
*/
function isZero(number) {
return Number.parseFloat(number) === 0;
}
Expand Down
55 changes: 39 additions & 16 deletions lib/rules/linebreaks/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// @ts-nocheck

'use strict';

const postcss = require('postcss');
Expand All @@ -13,35 +11,49 @@ const messages = ruleMessages(ruleName, {
expected: (linebreak) => `Expected linebreak to be ${linebreak}`,
});

function rule(actual, secondary, context) {
/** @type {import('stylelint').StylelintRule} */
const rule = (primary, _secondaryOptions, context) => {
return (root, result) => {
const validOptions = validateOptions(result, ruleName, {
actual,
actual: primary,
possible: ['unix', 'windows'],
});

if (!validOptions) {
return;
}

const shouldHaveCR = actual === 'windows';
const shouldHaveCR = primary === 'windows';

if (context.fix) {
const propertiesToUpdate = ['selector', 'value', 'text'];
const rawsPropertiesToUpdate = ['before', 'after'];
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] This is a more type-safe way, so that the property sets are unnecessary.


root.walk((node) => {
for (const property of propertiesToUpdate) {
node[property] = fixData(node[property]);
if ('selector' in node) {
node.selector = fixData(node.selector);
}

if ('value' in node) {
node.value = fixData(node.value);
}

if ('text' in node) {
node.text = fixData(node.text);
}

if (node.raws.before) {
node.raws.before = fixData(node.raws.before);
}

for (const property of rawsPropertiesToUpdate) {
node.raws[property] = fixData(node.raws[property]);
if ('after' in node.raws && node.raws.after) {
node.raws.after = fixData(node.raws.after);
}
});

root.raws.after = fixData(root.raws.after);
if ('after' in root.raws && root.raws.after) {
root.raws.after = fixData(root.raws.after);
}
} else {
if (root.source == null) throw new Error('The root node must have a source');

const lines = root.source.input.css.split('\n');

for (let i = 0; i < lines.length; i++) {
Expand All @@ -60,13 +72,19 @@ function rule(actual, secondary, context) {
}
}

/**
* @param {string} dataToCheck
*/
function hasError(dataToCheck) {
const hasNewlineToVerify = /[\r\n]/.test(dataToCheck);
const hasCR = hasNewlineToVerify ? /\r/.test(dataToCheck) : false;

return hasNewlineToVerify && hasCR !== shouldHaveCR;
}

/**
* @param {string} data
*/
function fixData(data) {
if (data) {
let res = data.replace(/\r/g, '');
Expand All @@ -81,23 +99,28 @@ function rule(actual, secondary, context) {
return data;
}

/**
* @param {number} line
* @param {number} column
*/
function reportNewlineError(line, column) {
// Creating a node manually helps us to point to empty lines.
const node = postcss.rule({
source: {
start: { line, column },
start: { line, column, offset: 0 },
input: new postcss.Input(''),
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]

...but, I'm not sure if the added dummy values are appropriate. 🤔

Copy link
Member

Choose a reason for hiding this comment

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

I think it's acceptable. We can always revisit.

},
});

report({
message: messages.expected(actual),
message: messages.expected(primary),
node,
result,
ruleName,
});
}
};
}
};

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