refactor ♻️: improve code quality and test coverage

- Fix handling of missing groupOrder configuration
- Refactor negative conditions to positive ones with optional chaining
- Add comprehensive tests to achieve total coverage
This commit is contained in:
Ante Budimir 2025-03-12 06:06:40 +02:00
parent 5557409368
commit 46751da51b
15 changed files with 310 additions and 186 deletions

View file

@ -16,13 +16,11 @@ import { enforceAlphabeticalCSSOrderInStyleObject } from './style-object-process
* 3. For each relevant property (e.g., 'base', 'variants'), it applies alphabetical ordering to the CSS properties.
*/
export const enforceAlphabeticalCSSOrderInRecipe = (node: TSESTree.CallExpression, context: Rule.RuleContext): void => {
if (!node.arguments[0] || node.arguments[0].type !== 'ObjectExpression') {
return;
if (node.arguments[0]?.type === 'ObjectExpression') {
const recipeObject = node.arguments[0];
processRecipeProperties(context, recipeObject, (context, object) =>
processStyleNode(context, object, enforceAlphabeticalCSSOrderInStyleObject),
);
}
const recipeObject = node.arguments[0];
processRecipeProperties(context, recipeObject, (context, object) =>
processStyleNode(context, object, enforceAlphabeticalCSSOrderInStyleObject),
);
};

View file

@ -20,18 +20,16 @@ export const enforceAlphabeticalCSSOrderInStyleObject = (
ruleContext: Rule.RuleContext,
styleObject: TSESTree.ObjectExpression,
): void => {
if (!styleObject || styleObject.type !== AST_NODE_TYPES.ObjectExpression) {
return;
}
if (styleObject?.type === AST_NODE_TYPES.ObjectExpression) {
if (isSelectorsObject(styleObject)) {
processNestedSelectors(ruleContext, styleObject, enforceAlphabeticalCSSOrderInStyleObject);
return;
}
const { regularProperties } = separateProperties(styleObject.properties);
enforceAlphabeticalCSSOrder(ruleContext, regularProperties);
if (isSelectorsObject(styleObject)) {
processNestedSelectors(ruleContext, styleObject, enforceAlphabeticalCSSOrderInStyleObject);
return;
}
const { regularProperties } = separateProperties(styleObject.properties);
enforceAlphabeticalCSSOrder(ruleContext, regularProperties);
processNestedSelectors(ruleContext, styleObject, enforceAlphabeticalCSSOrderInStyleObject);
};

View file

@ -65,21 +65,19 @@ export const enforceConcentricCSSOrder = (
ruleContext: Rule.RuleContext,
cssPropertyInfoList: CSSPropertyInfo[],
): void => {
if (cssPropertyInfoList.length <= 1) {
return;
}
if (cssPropertyInfoList.length > 1) {
// Create pairs of consecutive properties
const propertyPairs = cssPropertyInfoList.slice(0, -1).map((currentProperty, index) => ({
currentProperty,
nextProperty: cssPropertyInfoList[index + 1] as CSSPropertyInfo,
}));
// Create pairs of consecutive properties
const propertyPairs = cssPropertyInfoList.slice(0, -1).map((currentProperty, index) => ({
currentProperty,
nextProperty: cssPropertyInfoList[index + 1] as CSSPropertyInfo,
}));
const violatingPair = propertyPairs.find(
({ currentProperty, nextProperty }) => compareProperties(currentProperty, nextProperty) > 0,
);
const violatingPair = propertyPairs.find(
({ currentProperty, nextProperty }) => compareProperties(currentProperty, nextProperty) > 0,
);
if (violatingPair) {
reportOrderingIssue(ruleContext, violatingPair.currentProperty, violatingPair.nextProperty, cssPropertyInfoList);
if (violatingPair) {
reportOrderingIssue(ruleContext, violatingPair.currentProperty, violatingPair.nextProperty, cssPropertyInfoList);
}
}
};

View file

@ -19,13 +19,11 @@ export const enforceConcentricCSSOrderInRecipe = (
ruleContext: Rule.RuleContext,
callExpression: TSESTree.CallExpression,
): void => {
if (!callExpression.arguments[0] || callExpression.arguments[0].type !== 'ObjectExpression') {
return;
if (callExpression.arguments[0]?.type === 'ObjectExpression') {
const recipeObjectExpression = callExpression.arguments[0];
processRecipeProperties(ruleContext, recipeObjectExpression, (currentContext, styleObject) =>
processStyleNode(currentContext, styleObject, enforceConcentricCSSOrderInStyleObject),
);
}
const recipeObjectExpression = callExpression.arguments[0];
processRecipeProperties(ruleContext, recipeObjectExpression, (currentContext, styleObject) =>
processStyleNode(currentContext, styleObject, enforceConcentricCSSOrderInStyleObject),
);
};

View file

@ -50,23 +50,21 @@ export const enforceConcentricCSSOrderInStyleObject = (
ruleContext: Rule.RuleContext,
styleObject: TSESTree.ObjectExpression,
): void => {
if (!styleObject || styleObject.type !== AST_NODE_TYPES.ObjectExpression) {
return;
if (styleObject?.type === AST_NODE_TYPES.ObjectExpression) {
if (isSelectorsObject(styleObject)) {
styleObject.properties.forEach((property) => {
if (property.type === AST_NODE_TYPES.Property && property.value.type === AST_NODE_TYPES.ObjectExpression) {
enforceConcentricCSSOrderInStyleObject(ruleContext, property.value);
}
});
return;
}
const { regularProperties } = separateProperties(styleObject.properties);
const cssPropertyInfoList = buildCSSPropertyInfoList(regularProperties);
enforceConcentricCSSOrder(ruleContext, cssPropertyInfoList);
processNestedSelectors(ruleContext, styleObject, enforceConcentricCSSOrderInStyleObject);
}
if (isSelectorsObject(styleObject)) {
styleObject.properties.forEach((property) => {
if (property.type === AST_NODE_TYPES.Property && property.value.type === AST_NODE_TYPES.ObjectExpression) {
enforceConcentricCSSOrderInStyleObject(ruleContext, property.value);
}
});
return;
}
const { regularProperties } = separateProperties(styleObject.properties);
const cssPropertyInfoList = buildCSSPropertyInfoList(regularProperties);
enforceConcentricCSSOrder(ruleContext, cssPropertyInfoList);
processNestedSelectors(ruleContext, styleObject, enforceConcentricCSSOrderInStyleObject);
};

View file

@ -0,0 +1,115 @@
import tsParser from '@typescript-eslint/parser';
import { run } from 'eslint-vitest-rule-tester';
import customGroupOrderRule from '../rule-definition.js';
run({
name: 'vanilla-extract/custom-order/defaults',
rule: customGroupOrderRule,
languageOptions: {
parser: tsParser,
parserOptions: {
ecmaVersion: 2022,
sourceType: 'module',
},
},
valid: [
// Test with no options provided - should use defaults
`
import { style } from '@vanilla-extract/css';
const myStyle = style({
alignItems: 'center',
backgroundColor: 'red',
color: 'blue',
display: 'flex',
margin: '10px',
padding: '20px',
zIndex: 1
});
`,
// Test with empty groupOrder array - should use defaults
{
code: `
import { style } from '@vanilla-extract/css';
const myStyle = style({
alignItems: 'center',
backgroundColor: 'red',
color: 'blue',
display: 'flex',
margin: '10px',
padding: '20px',
zIndex: 1
});
`,
options: [
{
groupOrder: [],
},
],
},
],
invalid: [
// Test with no options provided - should use alphabetical ordering by default
{
code: `
import { style } from '@vanilla-extract/css';
const myStyle = style({
zIndex: 1,
padding: '20px',
margin: '10px',
display: 'flex',
color: 'blue',
backgroundColor: 'red',
alignItems: 'center'
});
`,
errors: [{ messageId: 'alphabeticalOrder' }],
output: `
import { style } from '@vanilla-extract/css';
const myStyle = style({
alignItems: 'center',
backgroundColor: 'red',
color: 'blue',
display: 'flex',
margin: '10px',
padding: '20px',
zIndex: 1
});
`,
},
// Test with empty groupOrder array - should use alphabetical ordering by default
{
code: `
import { style } from '@vanilla-extract/css';
const myStyle = style({
zIndex: 1,
padding: '20px',
margin: '10px',
display: 'flex',
color: 'blue',
backgroundColor: 'red',
alignItems: 'center'
});
`,
options: [
{
groupOrder: [],
},
],
errors: [{ messageId: 'alphabeticalOrder' }],
output: `
import { style } from '@vanilla-extract/css';
const myStyle = style({
alignItems: 'center',
backgroundColor: 'red',
color: 'blue',
display: 'flex',
margin: '10px',
padding: '20px',
zIndex: 1
});
`,
},
],
});

View file

@ -23,69 +23,74 @@ export const enforceCustomGroupOrder = (
userDefinedGroups: string[] = [],
sortRemainingProperties?: 'alphabetical' | 'concentric',
): void => {
if (cssPropertyInfoList.length <= 1) {
return;
}
if (cssPropertyInfoList.length > 1) {
const cssPropertyPriorityMap = createCSSPropertyPriorityMap(userDefinedGroups);
const cssPropertyPriorityMap = createCSSPropertyPriorityMap(userDefinedGroups);
const compareProperties = (firstProperty: CSSPropertyInfo, secondProperty: CSSPropertyInfo) => {
const firstPropertyInfo = cssPropertyPriorityMap.get(firstProperty.name) || {
groupIndex: Infinity,
positionInGroup: Infinity,
inUserGroup: false,
};
const secondPropertyInfo = cssPropertyPriorityMap.get(secondProperty.name) || {
groupIndex: Infinity,
positionInGroup: Infinity,
inUserGroup: false,
};
const compareProperties = (firstProperty: CSSPropertyInfo, secondProperty: CSSPropertyInfo) => {
const firstPropertyInfo = cssPropertyPriorityMap.get(firstProperty.name) || {
groupIndex: Infinity,
positionInGroup: Infinity,
inUserGroup: false,
};
const secondPropertyInfo = cssPropertyPriorityMap.get(secondProperty.name) || {
groupIndex: Infinity,
positionInGroup: Infinity,
inUserGroup: false,
};
if (firstPropertyInfo.inUserGroup !== secondPropertyInfo.inUserGroup) {
return firstPropertyInfo.inUserGroup ? -1 : 1;
}
if (firstPropertyInfo.inUserGroup) {
if (firstPropertyInfo.groupIndex !== secondPropertyInfo.groupIndex) {
return firstPropertyInfo.groupIndex - secondPropertyInfo.groupIndex;
if (firstPropertyInfo.inUserGroup !== secondPropertyInfo.inUserGroup) {
return firstPropertyInfo.inUserGroup ? -1 : 1;
}
return firstPropertyInfo.positionInGroup - secondPropertyInfo.positionInGroup;
if (firstPropertyInfo.inUserGroup) {
if (firstPropertyInfo.groupIndex !== secondPropertyInfo.groupIndex) {
return firstPropertyInfo.groupIndex - secondPropertyInfo.groupIndex;
}
return firstPropertyInfo.positionInGroup - secondPropertyInfo.positionInGroup;
}
// For properties not in user-defined groups
if (sortRemainingProperties === 'alphabetical') {
return firstProperty.name.localeCompare(secondProperty.name);
} else {
return (
firstPropertyInfo.groupIndex - secondPropertyInfo.groupIndex ||
firstPropertyInfo.positionInGroup - secondPropertyInfo.positionInGroup
);
}
};
const sortedPropertyList = [...cssPropertyInfoList].sort(compareProperties);
// Find the first pair that violates the new ordering
const violatingProperty = cssPropertyInfoList
.slice(0, -1)
.find((currentProperty, index) => currentProperty.name !== sortedPropertyList[index]?.name);
if (violatingProperty) {
const indexInSorted = cssPropertyInfoList.indexOf(violatingProperty);
const sortedProperty = sortedPropertyList[indexInSorted];
// Defensive programming - sortedProperty will always exist and have a name because sortedPropertyList is derived from cssPropertyInfoList and cssPropertyInfoList exists and is non-empty
// Therefore, we can exclude the next line from coverage because it's unreachable in practice
/* v8 ignore next */
const nextPropertyName = sortedProperty?.name ?? '';
ruleContext.report({
node: violatingProperty.node as Rule.Node,
messageId: 'incorrectOrder',
data: {
currentProperty: violatingProperty.name,
nextProperty: nextPropertyName,
},
fix: (fixer) =>
generateFixesForCSSOrder(
fixer,
ruleContext,
cssPropertyInfoList,
compareProperties,
(propertyInfo) => propertyInfo.node as Rule.Node,
),
});
}
// For properties not in user-defined groups
if (sortRemainingProperties === 'alphabetical') {
return firstProperty.name.localeCompare(secondProperty.name);
} else {
return (
firstPropertyInfo.groupIndex - secondPropertyInfo.groupIndex ||
firstPropertyInfo.positionInGroup - secondPropertyInfo.positionInGroup
);
}
};
const sortedPropertyList = [...cssPropertyInfoList].sort(compareProperties);
// Find the first pair that violates the new ordering
const violatingProperty = cssPropertyInfoList
.slice(0, -1)
.find((currentProperty, index) => currentProperty.name !== sortedPropertyList[index]?.name);
if (violatingProperty) {
ruleContext.report({
node: violatingProperty.node as Rule.Node,
messageId: 'incorrectOrder',
data: {
currentProperty: violatingProperty.name,
nextProperty: sortedPropertyList[cssPropertyInfoList.indexOf(violatingProperty)]?.name || '',
},
fix: (fixer) =>
generateFixesForCSSOrder(
fixer,
ruleContext,
cssPropertyInfoList,
compareProperties,
(propertyInfo) => propertyInfo.node as Rule.Node,
),
});
}
};

View file

@ -23,20 +23,18 @@ export const enforceUserDefinedGroupOrderInRecipe = (
userDefinedGroups: string[],
sortRemainingPropertiesMethod?: 'alphabetical' | 'concentric',
): void => {
if (!callExpression.arguments[0] || callExpression.arguments[0].type !== 'ObjectExpression') {
return;
}
if (callExpression.arguments[0]?.type === 'ObjectExpression') {
const recipeObjectExpression = callExpression.arguments[0];
const recipeObjectExpression = callExpression.arguments[0];
processRecipeProperties(ruleContext, recipeObjectExpression, (currentContext, styleObject) =>
processStyleNode(currentContext, styleObject, (styleContext, styleObjectNode) =>
enforceUserDefinedGroupOrderInStyleObject(
styleContext,
styleObjectNode,
userDefinedGroups,
sortRemainingPropertiesMethod,
processRecipeProperties(ruleContext, recipeObjectExpression, (currentContext, styleObject) =>
processStyleNode(currentContext, styleObject, (styleContext, styleObjectNode) =>
enforceUserDefinedGroupOrderInStyleObject(
styleContext,
styleObjectNode,
userDefinedGroups,
sortRemainingPropertiesMethod,
),
),
),
);
);
}
};

View file

@ -34,10 +34,13 @@ const customGroupOrderRule: Rule.RuleModule = {
},
],
messages: {
incorrectOrder:
"Property '{{nextProperty}}' should come before '{{currentProperty}}' according to custom CSS group ordering.",
// default message for ordering in case no groupOrder is provided
alphabeticalOrder:
"Property '{{nextProperty}}' should come before '{{currentProperty}}' according to alphabetical ordering.",
fontFaceOrder:
"Properties in fontFace should be ordered with 'src' first, followed by other properties in alphabetical order. Property '{{nextProperty}}' should come before '{{currentProperty}}'.",
incorrectOrder:
"Property '{{nextProperty}}' should come before '{{currentProperty}}' according to custom CSS group ordering.",
},
},
create(ruleContext: Rule.RuleContext) {

View file

@ -29,51 +29,49 @@ export const enforceUserDefinedGroupOrderInStyleObject = (
userDefinedGroups: string[],
sortRemainingPropertiesMethod: 'alphabetical' | 'concentric' = 'concentric',
): void => {
if (!styleObject || styleObject.type !== AST_NODE_TYPES.ObjectExpression) {
return;
}
if (styleObject?.type === AST_NODE_TYPES.ObjectExpression) {
if (isSelectorsObject(styleObject)) {
styleObject.properties.forEach((property) => {
if (property.type === AST_NODE_TYPES.Property && property.value.type === AST_NODE_TYPES.ObjectExpression) {
enforceUserDefinedGroupOrderInStyleObject(
ruleContext,
property.value,
userDefinedGroups,
sortRemainingPropertiesMethod,
);
}
});
return;
}
if (isSelectorsObject(styleObject)) {
styleObject.properties.forEach((property) => {
if (property.type === AST_NODE_TYPES.Property && property.value.type === AST_NODE_TYPES.ObjectExpression) {
enforceUserDefinedGroupOrderInStyleObject(
ruleContext,
property.value,
userDefinedGroups,
sortRemainingPropertiesMethod,
);
}
const cssPropertyPriorityMap = createCSSPropertyPriorityMap(userDefinedGroups);
const { regularProperties } = separateProperties(styleObject.properties);
const cssPropertyInfoList: CSSPropertyInfo[] = regularProperties.map((property) => {
const propertyName = getPropertyName(property);
const propertyInfo = cssPropertyPriorityMap.get(propertyName);
const group =
userDefinedGroups.find((groupName) => concentricGroups[groupName]?.includes(propertyName)) || 'remaining';
return {
name: propertyName,
node: property,
priority: propertyInfo?.groupIndex ?? Number.MAX_SAFE_INTEGER,
positionInGroup: propertyInfo?.positionInGroup ?? Number.MAX_SAFE_INTEGER,
group,
inUserGroup: propertyInfo?.inUserGroup ?? false,
};
});
return;
enforceCustomGroupOrder(ruleContext, cssPropertyInfoList, userDefinedGroups, sortRemainingPropertiesMethod);
processNestedSelectors(ruleContext, styleObject, (nestedContext, nestedNode) =>
enforceUserDefinedGroupOrderInStyleObject(
nestedContext,
nestedNode,
userDefinedGroups,
sortRemainingPropertiesMethod,
),
);
}
const cssPropertyPriorityMap = createCSSPropertyPriorityMap(userDefinedGroups);
const { regularProperties } = separateProperties(styleObject.properties);
const cssPropertyInfoList: CSSPropertyInfo[] = regularProperties.map((property) => {
const propertyName = getPropertyName(property);
const propertyInfo = cssPropertyPriorityMap.get(propertyName);
const group =
userDefinedGroups.find((groupName) => concentricGroups[groupName]?.includes(propertyName)) || 'remaining';
return {
name: propertyName,
node: property,
priority: propertyInfo?.groupIndex ?? Number.MAX_SAFE_INTEGER,
positionInGroup: propertyInfo?.positionInGroup ?? Number.MAX_SAFE_INTEGER,
group,
inUserGroup: propertyInfo?.inUserGroup ?? false,
};
});
enforceCustomGroupOrder(ruleContext, cssPropertyInfoList, userDefinedGroups, sortRemainingPropertiesMethod);
processNestedSelectors(ruleContext, styleObject, (nestedContext, nestedNode) =>
enforceUserDefinedGroupOrderInStyleObject(
nestedContext,
nestedNode,
userDefinedGroups,
sortRemainingPropertiesMethod,
),
);
};

View file

@ -40,7 +40,7 @@ export const createNodeVisitors = (
return enforceConcentricCSSOrderInStyleObject;
case 'userDefinedGroupOrder':
if (!userDefinedGroupOrder || userDefinedGroupOrder.length === 0) {
throw new Error('💥 👿 User-defined group order must be provided for userDefinedGroupOrder strategy');
return enforceAlphabeticalCSSOrderInStyleObject;
}
return (ruleContext: Rule.RuleContext, node: TSESTree.Node) =>
enforceUserDefinedGroupOrderInStyleObject(

View file

@ -5,7 +5,7 @@ import customOrderRule from './css-rules/custom-order/rule-definition.js';
export const vanillaExtract = {
meta: {
name: '@antebudimir/eslint-plugin-vanilla-extract',
version: '1.4.7',
version: '1.5.0',
},
rules: {
'alphabetical-order': alphabeticalOrderRule,