Skip to content

Commit

Permalink
Refactor to improve types for font-* rules (#5486)
Browse files Browse the repository at this point in the history
This change removes  `// @ts-nocheck` from the `font-*` rules.
  • Loading branch information
ybiquitous committed Aug 18, 2021
1 parent 52f2474 commit 544d857
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 45 deletions.
46 changes: 34 additions & 12 deletions lib/rules/font-family-name-quotes/index.js
@@ -1,5 +1,3 @@
// @ts-nocheck

'use strict';

const findFontFamily = require('../../utils/findFontFamily');
Expand All @@ -17,6 +15,10 @@ const messages = ruleMessages(ruleName, {
rejected: (family) => `Unexpected quotes around "${family}"`,
});

/**
* @param {string} font
* @returns {boolean}
*/
function isSystemFontKeyword(font) {
if (font.startsWith('-apple-')) {
return true;
Expand All @@ -29,25 +31,36 @@ function isSystemFontKeyword(font) {
return false;
}

// "To avoid mistakes in escaping, it is recommended to quote font family names
// that contain white space, digits, or punctuation characters other than hyphens"
// (https://www.w3.org/TR/CSS2/fonts.html#font-family-prop)
/**
* "To avoid mistakes in escaping, it is recommended to quote font family names
* that contain white space, digits, or punctuation characters other than hyphens"
* (https://www.w3.org/TR/CSS2/fonts.html#font-family-prop)
*
* @param {string} family
* @returns {boolean}
*/
function quotesRecommended(family) {
return !/^[-a-zA-Z]+$/.test(family);
}

// Quotes are required if the family is not a valid CSS identifier
// (regexes from https://mathiasbynens.be/notes/unquoted-font-family)
/**
* Quotes are required if the family is not a valid CSS identifier
* (regexes from https://mathiasbynens.be/notes/unquoted-font-family)
*
* @param {string} family
* @returns {boolean}
*/
function quotesRequired(family) {
return family.split(/\s+/).some((word) => {
return /^(-?\d|--)/.test(word) || !/^[-_a-zA-Z0-9\u{00A0}-\u{10FFFF}]+$/u.test(word);
});
}

function rule(expectation) {
/** @type {import('stylelint').StylelintRule} */
const rule = (primary) => {
return (root, result) => {
const validOptions = validateOptions(result, ruleName, {
actual: expectation,
actual: primary,
possible: ['always-where-required', 'always-where-recommended', 'always-unless-keyword'],
});

Expand All @@ -65,14 +78,18 @@ function rule(expectation) {
fontFamilies.forEach((fontFamilyNode) => {
let rawFamily = fontFamilyNode.value;

if (fontFamilyNode.quote) {
if ('quote' in fontFamilyNode) {
rawFamily = fontFamilyNode.quote + rawFamily + fontFamilyNode.quote;
}

checkFamilyName(rawFamily, decl);
});
});

/**
* @param {string} rawFamily
* @param {import('postcss').Declaration} decl
*/
function checkFamilyName(rawFamily, decl) {
if (!isStandardSyntaxValue(rawFamily)) {
return;
Expand Down Expand Up @@ -100,7 +117,7 @@ function rule(expectation) {
const required = quotesRequired(family);
const recommended = quotesRecommended(family);

switch (expectation) {
switch (primary) {
case 'always-unless-keyword':
if (!hasQuotes) {
return complain(messages.expected(family), family, decl);
Expand Down Expand Up @@ -130,6 +147,11 @@ function rule(expectation) {
}
}

/**
* @param {string} message
* @param {string} family
* @param {import('postcss').Declaration} decl
*/
function complain(message, family, decl) {
report({
result,
Expand All @@ -140,7 +162,7 @@ function rule(expectation) {
});
}
};
}
};

rule.ruleName = ruleName;
rule.messages = messages;
Expand Down
23 changes: 15 additions & 8 deletions lib/rules/font-family-no-duplicate-names/index.js
@@ -1,5 +1,3 @@
// @ts-nocheck

'use strict';

const declarationValueIndex = require('../../utils/declarationValueIndex');
Expand All @@ -17,17 +15,21 @@ const messages = ruleMessages(ruleName, {
rejected: (name) => `Unexpected duplicate name ${name}`,
});

/**
* @param {import('postcss-value-parser').Node} node
*/
const isFamilyNameKeyword = (node) =>
!node.quote && keywordSets.fontFamilyKeywords.has(node.value.toLowerCase());
!('quote' in node) && keywordSets.fontFamilyKeywords.has(node.value.toLowerCase());

function rule(actual, options) {
/** @type {import('stylelint').StylelintRule} */
const rule = (primary, secondaryOptions) => {
return (root, result) => {
const validOptions = validateOptions(
result,
ruleName,
{ actual },
{ actual: primary },
{
actual: options,
actual: secondaryOptions,
possible: {
ignoreFontFamilyNames: [isString, isRegExp],
},
Expand All @@ -52,7 +54,7 @@ function rule(actual, options) {
fontFamilies.forEach((fontFamilyNode) => {
const family = fontFamilyNode.value.trim();

if (optionsMatches(options, 'ignoreFontFamilyNames', fontFamilyNode.value.trim())) {
if (optionsMatches(secondaryOptions, 'ignoreFontFamilyNames', family)) {
return;
}

Expand Down Expand Up @@ -86,6 +88,11 @@ function rule(actual, options) {
});
});

/**
* @param {string} message
* @param {number} index
* @param {import('postcss').Declaration} decl
*/
function complain(message, index, decl) {
report({
result,
Expand All @@ -96,7 +103,7 @@ function rule(actual, options) {
});
}
};
}
};

rule.ruleName = ruleName;
rule.messages = messages;
Expand Down
38 changes: 24 additions & 14 deletions lib/rules/font-family-no-missing-generic-family-keyword/index.js
@@ -1,5 +1,3 @@
// @ts-nocheck

'use strict';

const declarationValueIndex = require('../../utils/declarationValueIndex');
Expand All @@ -12,6 +10,7 @@ const postcss = require('postcss');
const report = require('../../utils/report');
const ruleMessages = require('../../utils/ruleMessages');
const validateOptions = require('../../utils/validateOptions');
const { isAtRule } = require('../../utils/typeGuards');
const { isRegExp, isString } = require('../../utils/validateTypes');

const ruleName = 'font-family-no-missing-generic-family-keyword';
Expand All @@ -20,23 +19,32 @@ const messages = ruleMessages(ruleName, {
rejected: 'Unexpected missing generic font family',
});

/**
* @param {import('postcss-value-parser').Node} node
* @returns {boolean}
*/
const isFamilyNameKeyword = (node) =>
!node.quote && keywordSets.fontFamilyKeywords.has(node.value.toLowerCase());
!('quote' in node) && keywordSets.fontFamilyKeywords.has(node.value.toLowerCase());

/**
* @param {string} value
* @returns {boolean}
*/
const isLastFontFamilyVariable = (value) => {
const lastValue = postcss.list.comma(value).pop();

return isVariable(lastValue) || !isStandardSyntaxValue(lastValue);
return lastValue != null && (isVariable(lastValue) || !isStandardSyntaxValue(lastValue));
};

function rule(actual, options) {
/** @type {import('stylelint').StylelintRule} */
const rule = (primary, secondaryOptions) => {
return (root, result) => {
const validOptions = validateOptions(
result,
ruleName,
{ actual },
{ actual: primary },
{
actual: options,
actual: secondaryOptions,
possible: {
ignoreFontFamilies: [isString, isRegExp],
},
Expand All @@ -50,11 +58,9 @@ function rule(actual, options) {

root.walkDecls(/^font(-family)?$/i, (decl) => {
// Ignore @font-face
if (
decl.parent &&
decl.parent.type === 'atrule' &&
decl.parent.name.toLowerCase() === 'font-face'
) {
const parent = decl.parent;

if (parent && isAtRule(parent) && parent.name.toLowerCase() === 'font-face') {
return;
}

Expand All @@ -76,7 +82,11 @@ function rule(actual, options) {
return;
}

if (fontFamilies.some((node) => optionsMatches(options, 'ignoreFontFamilies', node.value))) {
if (
fontFamilies.some((node) =>
optionsMatches(secondaryOptions, 'ignoreFontFamilies', node.value),
)
) {
return;
}

Expand All @@ -89,7 +99,7 @@ function rule(actual, options) {
});
});
};
}
};

rule.ruleName = ruleName;
rule.messages = messages;
Expand Down
32 changes: 22 additions & 10 deletions lib/rules/font-weight-notation/index.js
@@ -1,5 +1,3 @@
// @ts-nocheck

'use strict';

const declarationValueIndex = require('../../utils/declarationValueIndex');
Expand All @@ -12,6 +10,7 @@ const postcss = require('postcss');
const report = require('../../utils/report');
const ruleMessages = require('../../utils/ruleMessages');
const validateOptions = require('../../utils/validateOptions');
const { isAtRule } = require('../../utils/typeGuards');

const ruleName = 'font-weight-notation';

Expand All @@ -25,17 +24,18 @@ const INITIAL_KEYWORD = 'initial';
const NORMAL_KEYWORD = 'normal';
const WEIGHTS_WITH_KEYWORD_EQUIVALENTS = new Set(['400', '700']);

function rule(expectation, options) {
/** @type {import('stylelint').StylelintRule} */
const rule = (primary, secondaryOptions) => {
return (root, result) => {
const validOptions = validateOptions(
result,
ruleName,
{
actual: expectation,
actual: primary,
possible: ['numeric', 'named-where-possible'],
},
{
actual: options,
actual: secondaryOptions,
possible: {
ignore: ['relative'],
},
Expand All @@ -57,6 +57,9 @@ function rule(expectation, options) {
}
});

/**
* @param {import('postcss').Declaration} decl
*/
function checkFont(decl) {
const valueList = postcss.list.space(decl.value);
// We do not need to more carefully distinguish font-weight
Expand All @@ -78,6 +81,10 @@ function rule(expectation, options) {
}
}

/**
* @param {string} weightValue
* @param {import('postcss').Declaration} decl
*/
function checkWeight(weightValue, decl) {
if (!isStandardSyntaxValue(weightValue)) {
return;
Expand All @@ -95,16 +102,18 @@ function rule(expectation, options) {
}

if (
optionsMatches(options, 'ignore', 'relative') &&
optionsMatches(secondaryOptions, 'ignore', 'relative') &&
keywordSets.fontWeightRelativeKeywords.has(weightValue.toLowerCase())
) {
return;
}

const weightValueOffset = decl.value.indexOf(weightValue);

if (expectation === 'numeric') {
if (decl.parent.type === 'atrule' && decl.parent.name.toLowerCase() === 'font-face') {
if (primary === 'numeric') {
const parent = decl.parent;

if (parent && isAtRule(parent) && parent.name.toLowerCase() === 'font-face') {
const weightValueNumbers = postcss.list.space(weightValue);

if (!weightValueNumbers.every(isNumbery)) {
Expand All @@ -119,7 +128,7 @@ function rule(expectation, options) {
}
}

if (expectation === 'named-where-possible') {
if (primary === 'named-where-possible') {
if (isNumbery(weightValue)) {
if (WEIGHTS_WITH_KEYWORD_EQUIVALENTS.has(weightValue)) {
complain(messages.expected('named'));
Expand All @@ -136,6 +145,9 @@ function rule(expectation, options) {
}
}

/**
* @param {string} message
*/
function complain(message) {
report({
ruleName,
Expand All @@ -147,7 +159,7 @@ function rule(expectation, options) {
}
}
};
}
};

rule.ruleName = ruleName;
rule.messages = messages;
Expand Down
2 changes: 1 addition & 1 deletion lib/utils/findFontFamily.js
Expand Up @@ -29,7 +29,7 @@ function joinValueNodes(firstNode, secondNode, charactersBetween) {
* Get the font-families within a `font` shorthand property value.
*
* @param {string} value
* @return {object} Collection font-family nodes
* @returns {Node[]} Collection font-family nodes
*/
module.exports = function findFontFamily(value) {
/** @type {Node[]} */
Expand Down

0 comments on commit 544d857

Please sign in to comment.