Skip to content

fix: correctly identify font-size in font shorthand #526

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

Open
Pixel998 wants to merge 1 commit into
base: main
Choose a base branch
from
+47 −71
Open

fix: correctly identify font-size in font shorthand #526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 32 additions & 71 deletions src/rules/relative-font-units.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

/**
* @import { CSSRuleDefinition } from "../types.js"
* @import { CssNode, CssNodePlain } from "@eslint/css-tree"
* @typedef {"allowedFontUnits"} RelativeFontUnitsMessageIds
* @typedef {[{allowUnits?: string[]}]} RelativeFontUnitsOptions
* @typedef {CSSRuleDefinition<{ RuleOptions: RelativeFontUnitsOptions, MessageIds: RelativeFontUnitsMessageIds}>} RelativeFontUnitsRuleDefinition
Expand Down Expand Up @@ -46,6 +47,22 @@ const disallowedFontSizeKeywords = new Set([
"math",
]);

/**
* Checks whether a font size uses a unit or keyword that is not allowed.
* @param {CssNodePlain} value The font size value.
* @param {string[]} allowedFontUnits The relative units that are allowed.
* @returns {boolean} Whether the font size value is disallowed.
*/
function isDisallowedFontSize(value, allowedFontUnits) {
return (
(value.type === "Dimension" &&
!allowedFontUnits.includes(value.unit.toLowerCase())) ||
(value.type === "Identifier" &&
disallowedFontSizeKeywords.has(value.name.toLowerCase())) ||
(value.type === "Percentage" && !allowedFontUnits.includes("%"))
);
}

//-----------------------------------------------------------------------------
// Rule Definition
//-----------------------------------------------------------------------------
Expand Down Expand Up @@ -90,6 +107,7 @@ export default /** @satisfies {RelativeFontUnitsRuleDefinition} */ ({

create(context) {
const [{ allowUnits: allowedFontUnits }] = context.options;
const { lexer } = context.sourceCode;

return {
Declaration(node) {
Expand All @@ -100,18 +118,7 @@ export default /** @satisfies {RelativeFontUnitsRuleDefinition} */ ({
) {
const value = node.value.children[0];

if (
(value.type === "Dimension" &&
!allowedFontUnits.includes(
value.unit.toLowerCase(),
)) ||
(value.type === "Identifier" &&
disallowedFontSizeKeywords.has(
value.name.toLowerCase(),
)) ||
(value.type === "Percentage" &&
!allowedFontUnits.includes("%"))
) {
if (isDisallowedFontSize(value, allowedFontUnits)) {
context.report({
loc: value.loc,
messageId: "allowedFontUnits",
Expand All @@ -130,68 +137,22 @@ export default /** @satisfies {RelativeFontUnitsRuleDefinition} */ ({
node.value.children.length > 0
) {
const value = node.value;

const dimensionNode = value.children.find(
child => child.type === "Dimension",
);
const identifierNode = value.children.find(
child =>
child.type === "Identifier" &&
disallowedFontSizeKeywords.has(
child.name.toLowerCase(),
),
);
const percentageNode = value.children.find(
(child, index) => {
const isPercentage =
child.type === "Percentage";
const previousNode = value.children[index - 1];

const previousNodeIsSlashOperator =
previousNode &&
previousNode.type === "Operator" &&
previousNode.value === "/";

return (
isPercentage && !previousNodeIsSlashOperator
);
},
);

let location;
let shouldReport = false;

const conditions = [
{
check:
!allowedFontUnits.includes("%") &&
percentageNode,
loc: percentageNode?.loc,
},
{
check: identifierNode,
loc: identifierNode?.loc,
},
{
check:
dimensionNode &&
!allowedFontUnits.includes(
dimensionNode.unit.toLowerCase(),
const match = lexer.matchProperty("font", value);
const fontSizeNode = match.matched
? value.children.find(child =>
match.isProperty(
/** @type {CssNode} */ (child),
"font-size",
),
loc: dimensionNode?.loc,
},
];
for (const condition of conditions) {
if (condition.check) {
shouldReport = true;
location = condition.loc;
break;
}
}
)
: undefined;

if (shouldReport) {
if (
fontSizeNode &&
isDisallowedFontSize(fontSizeNode, allowedFontUnits)
) {
context.report({
loc: location,
loc: fontSizeNode.loc,
messageId: "allowedFontUnits",
data: {
allowedFontUnits:
Expand Down
15 changes: 15 additions & 0 deletions tests/rules/relative-font-units.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ ruleTester.run("relative-font-units", rule, {
"a { font-size: 1rem; }",
"a { font: 2rem Arial, sans-serif; }",
"a { font: 1.2rem/2 Arial, sans-serif; }",
"a { font: oblique 10deg 1rem serif; }",
"a { font: 1rem math; }",
"a { font-size: 1REM; }",
"a { font-size: 1Rem; }",
"a { font-size: 1rEm; }",
Expand Down Expand Up @@ -764,6 +766,19 @@ ruleTester.run("relative-font-units", rule, {
},
],
},
{
code: "a { font: oblique 10deg 1em serif; }",
errors: [
{
messageId: "allowedFontUnits",
line: 1,
column: 25,
endLine: 1,
endColumn: 28,
data: { allowedFontUnits: "rem" },
},
],
},
{
code: "a { font-size: xx-small; }",
errors: [
Expand Down