Skip to content

Commit

Permalink
Fix column position and ignore Sass function for `font-weight-notatio…
Browse files Browse the repository at this point in the history 
…n` (#6005)
  • Loading branch information
@ybiquitous
ybiquitous committed Apr 4, 2022
1 parent 379f3c3 commit 4b4e792
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 37 deletions.
20 changes: 17 additions & 3 deletions lib/rules/font-weight-notation/README.md
View file
Expand Up @@ -4,13 +4,17 @@ Require numeric or named (where possible) `font-weight` values. Also, when named

<!-- prettier-ignore -->
```css
a { font-weight: bold }
a { font-weight: bold; }
/** ↑
* This notation */
* This notation */

a { font: italic small-caps 600 16px/3 cursive; }
/** ↑
* And this notation, too */
* And this notation, too */

@font-face { font-weight: normal bold; }
/** ↑
* Multiple notations are available in @font-face */
```

Valid font-weight names are `normal`, `bold`, `bolder`, and `lighter`.
Expand All @@ -37,6 +41,11 @@ a { font-weight: bold; }
a { font: italic normal 20px sans-serif; }
```

<!-- prettier-ignore -->
```css
@font-face { font-weight: normal bold; }
```

The following patterns are _not_ considered problems:

<!-- prettier-ignore -->
Expand All @@ -49,6 +58,11 @@ a { font-weight: 700; }
a { font: italic 400 20px; }
```

<!-- prettier-ignore -->
```css
@font-face { font-weight: 400 700; }
```

### `"named-where-possible"`

`font-weight` values _must always_ be keywords when an appropriate keyword is available.
Expand Down
10 changes: 9 additions & 1 deletion lib/rules/font-weight-notation/__tests__/index.js
View file
Expand Up @@ -180,7 +180,7 @@ testRule({
code: '@font-face { font-weight: 400 bold; }',
message: messages.expected('numeric'),
line: 1,
column: 27,
column: 31,
},
{
code: '@font-face { font-weight: normal 700; }',
Expand Down Expand Up @@ -380,5 +380,13 @@ testRule({
code: 'a { font-weight: $foo }',
description: 'ignore sass variable',
},
{
code: 'a { font-weight: map-deep-get($theme, typography, weight, semibold); }',
description: 'ignore sass function',
},
{
code: 'a { font-weight: /*comment*/ map-deep-get($theme, typography, weight, semibold); }',
description: 'ignore sass function with comment',
},
],
});
106 changes: 73 additions & 33 deletions lib/rules/font-weight-notation/index.js
View file
@@ -1,12 +1,13 @@
'use strict';

const valueParser = require('postcss-value-parser');

const declarationValueIndex = require('../../utils/declarationValueIndex');
const isNumbery = require('../../utils/isNumbery');
const isStandardSyntaxValue = require('../../utils/isStandardSyntaxValue');
const isVariable = require('../../utils/isVariable');
const keywordSets = require('../../reference/keywordSets');
const optionsMatches = require('../../utils/optionsMatches');
const postcss = require('postcss');
const report = require('../../utils/report');
const ruleMessages = require('../../utils/ruleMessages');
const validateOptions = require('../../utils/validateOptions');
Expand Down Expand Up @@ -51,12 +52,12 @@ const rule = (primary, secondaryOptions) => {
return;
}

root.walkDecls((decl) => {
if (decl.prop.toLowerCase() === 'font-weight') {
checkWeight(decl.value, decl);
}
root.walkDecls(/^font(-weight)?$/i, (decl) => {
const prop = decl.prop.toLowerCase();

if (decl.prop.toLowerCase() === 'font') {
if (prop === 'font-weight') {
checkWeight(decl, decl.value);
} else if (prop === 'font') {
checkFont(decl);
}
});
Expand All @@ -65,31 +66,35 @@ const rule = (primary, secondaryOptions) => {
* @param {import('postcss').Declaration} decl
*/
function checkFont(decl) {
const valueList = postcss.list.space(decl.value);
const valueNodes = findFontWeights(decl.value);

// We do not need to more carefully distinguish font-weight
// numbers from unitless line-heights because line-heights in
// `font` values need to be part of a font-size/line-height pair
const hasNumericFontWeight = valueList.some((value) => isNumbery(value));
const hasNumericFontWeight = valueNodes.some(({ value }) => isNumbery(value));

for (const valueNode of valueNodes) {
const value = valueNode.value;
const lowerValue = value.toLowerCase();

for (const value of postcss.list.space(decl.value)) {
if (
(value.toLowerCase() === NORMAL_KEYWORD && !hasNumericFontWeight) ||
(lowerValue === NORMAL_KEYWORD && !hasNumericFontWeight) ||
isNumbery(value) ||
(value.toLowerCase() !== NORMAL_KEYWORD &&
keywordSets.fontWeightKeywords.has(value.toLowerCase()))
(lowerValue !== NORMAL_KEYWORD && keywordSets.fontWeightKeywords.has(lowerValue))
) {
checkWeight(value, decl);
checkWeight(decl, value, valueNode);

return;
}
}
}

/**
* @param {string} weightValue
* @param {import('postcss').Declaration} decl
* @param {string} weightValue
* @param {import('postcss-value-parser').Node} [weightValueNode]
*/
function checkWeight(weightValue, decl) {
function checkWeight(decl, weightValue, weightValueNode) {
if (!isStandardSyntaxValue(weightValue)) {
return;
}
Expand All @@ -98,73 +103,108 @@ const rule = (primary, secondaryOptions) => {
return;
}

if (
weightValue.toLowerCase() === INHERIT_KEYWORD ||
weightValue.toLowerCase() === INITIAL_KEYWORD
) {
if (includesOnlyFunction(weightValue)) {
return;
}

const lowerWeightValue = weightValue.toLowerCase();

if (lowerWeightValue === INHERIT_KEYWORD || lowerWeightValue === INITIAL_KEYWORD) {
return;
}

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

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

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((value) => isNumbery(value))) {
return complain(messages.expected('numeric'));
// @font-face allows multiple values.
for (const valueNode of findFontWeights(weightValue)) {
if (!isNumbery(valueNode.value)) {
return complain(messages.expected('numeric'), valueNode);
}
}

return;
}

if (!isNumbery(weightValue)) {
return complain(messages.expected('numeric'));
return complain(messages.expected('numeric'), weightValueNode);
}
}

if (primary === 'named-where-possible') {
if (isNumbery(weightValue)) {
if (WEIGHTS_WITH_KEYWORD_EQUIVALENTS.has(weightValue)) {
complain(messages.expected('named'));
complain(messages.expected('named'), weightValueNode);
}

return;
}

if (
!keywordSets.fontWeightKeywords.has(weightValue.toLowerCase()) &&
weightValue.toLowerCase() !== NORMAL_KEYWORD
!keywordSets.fontWeightKeywords.has(lowerWeightValue) &&
lowerWeightValue !== NORMAL_KEYWORD
) {
return complain(messages.invalidNamed(weightValue));
return complain(messages.invalidNamed(weightValue), weightValueNode);
}
}

/**
* @param {string} message
* @param {import('postcss-value-parser').Node} [valueNode]
*/
function complain(message) {
function complain(message, valueNode) {
const index = declarationValueIndex(decl) + (valueNode ? valueNode.sourceIndex : 0);

report({
ruleName,
result,
message,
node: decl,
index: declarationValueIndex(decl) + weightValueOffset,
index,
});
}
}
};
};

/**
* @param {string} value
* @returns {import('postcss-value-parser').Node[]}
*/
function findFontWeights(value) {
return valueParser(value).nodes.filter((node, index, nodes) => {
if (node.type !== 'word') return false;

// Exclude `<font-size>/<line-height>` format like `16px/3`.
const prevNode = nodes[index - 1];
const nextNode = nodes[index + 1];

if (prevNode && prevNode.type === 'div') return false;

if (nextNode && nextNode.type === 'div') return false;

return true;
});
}

/**
* @param {string} value
* @returns {boolean}
*/
function includesOnlyFunction(value) {
return valueParser(value).nodes.every(({ type }) => {
return type === 'function' || type === 'comment' || type === 'space';
});
}

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

0 comments on commit 4b4e792

Please sign in to comment.