Repository: microsoft/griffel
Pull request #418 (2023-08-11 19:28:45)

eslint-plugin: Add autofix for no-shorthands
When migrating from Fluent v8 to v9, replacing shorthands with the equivalent function is somewhat tedious. We can leverage the existing linter rule to autofix all shorthands!

I wasn't sure how to instruct the fixer to import `shorthands` but this is pretty easily resolved in the editor.

github-actions[bot]: ## 📊 Bundle size report

<sub>🤖 This report was generated against <a href='https://github.com/microsoft/griffel/commit/adab62c6a9a54855bb518cabd54657622cc02314'>adab62c6a9a54855bb518cabd54657622cc02314</a></sub>

<!-- Sticky Pull Request Comment -->

Patch:
diff --git a/change/@griffel-eslint-plugin-878e2808-ab3e-46ad-b31c-15b299d18311.json b/change/@griffel-eslint-plugin-878e2808-ab3e-46ad-b31c-15b299d18311.json
new file mode 100644
index 0000000..3316f9d
--- /dev/null
+++ b/change/@griffel-eslint-plugin-878e2808-ab3e-46ad-b31c-15b299d18311.json
@@ -0,0 +1,7 @@
+{
+  "type": "minor",
+  "comment": "Add autofix for no-shorthands",
+  "packageName": "@griffel/eslint-plugin",
+  "email": "tigeroakes@microsoft.com",
+  "dependentChangeType": "patch"
+}
diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md
index 8b5308b..0a24a3e 100644
--- a/packages/eslint-plugin/README.md
+++ b/packages/eslint-plugin/README.md
@@ -45,6 +45,6 @@ You can find more info about enabled rules in the [Supported Rules section](#sup
 | Name                                                                     | Description                                                                                                     | 🔧  |
 | ------------------------------------------------------------------------ | --------------------------------------------------------------------------------------------------------------- | --- |
 | [`@griffel/hook-naming`](./src/rules/hook-naming.md)                     | Ensure that hooks returned by the `makeStyles()` function start with "use"                                      | ❌  |
-| [`@griffel/no-shorthands`](./src/rules/no-shorthands.md)                 | Enforce usage of CSS longhands                                                                                  | ❌  |
+| [`@griffel/no-shorthands`](./src/rules/no-shorthands.md)                 | Enforce usage of CSS longhands                                                                                  | ✅  |
 | [`@griffel/styles-file`](./src/rules/styles-file.md)                     | Ensures that all `makeStyles()` and `makeResetStyles()` calls are placed in a `.styles.js` or `.styles.ts` file | ❌  |
 | [`@griffel/pseudo-element-naming`](./src/rules/pseudo-element-naming.md) | Ensures that all Pseudo Elements start with two colons                                                          | ✅  |
diff --git a/packages/eslint-plugin/src/rules/no-shorthands.ts b/packages/eslint-plugin/src/rules/no-shorthands.ts
index cd56041..ed53028 100644
--- a/packages/eslint-plugin/src/rules/no-shorthands.ts
+++ b/packages/eslint-plugin/src/rules/no-shorthands.ts
@@ -2,7 +2,8 @@ import { ESLintUtils, TSESTree } from '@typescript-eslint/utils';
 import * as CSS from 'csstype';
 
 import { createRule } from '../utils/createRule';
-import { isIdentifier, isMakeStylesIdentifier, isObjectExpression, isProperty } from '../utils/helpers';
+import { isIdentifier, isLiteral, isMakeStylesIdentifier, isObjectExpression, isProperty } from '../utils/helpers';
+import { buildShorthandSplitter } from '../utils/buildShorthandSplitter';
 
 export const RULE_NAME = 'no-shorthands';
 
@@ -77,6 +78,38 @@ const UNSUPPORTED_CSS_PROPERTIES: Record<keyof CSS.StandardShorthandProperties,
   transition: true,
 };
 
+const pxSplitter = buildShorthandSplitter({ numberUnit: 'px' });
+
+// Transforms shorthand string into args for griffel shorthands.<name>() function.
+const SHORTHAND_FUNCTIONS: Partial<
+  Record<keyof typeof UNSUPPORTED_CSS_PROPERTIES, ReturnType<typeof buildShorthandSplitter>>
+> = {
+  border: pxSplitter,
+  borderLeft: pxSplitter,
+  borderBottom: pxSplitter,
+  borderRight: pxSplitter,
+  borderTop: pxSplitter,
+  borderColor: pxSplitter,
+  borderStyle: pxSplitter,
+  borderRadius: pxSplitter,
+  borderWidth: pxSplitter,
+  // Instead of converting to pixels, convert to flex-grow value.
+  flex: buildShorthandSplitter({ numberUnit: '' }),
+  gap: pxSplitter,
+  // Split every `/` character (and trim whitespace)
+  gridArea: buildShorthandSplitter({ separator: '/' }),
+  margin: pxSplitter,
+  marginBlock: pxSplitter,
+  marginInline: pxSplitter,
+  padding: pxSplitter,
+  paddingBlock: pxSplitter,
+  paddingInline: pxSplitter,
+  overflow: pxSplitter,
+  inset: pxSplitter,
+  outline: pxSplitter,
+  textDecoration: pxSplitter,
+};
+
 function findShorthandProperties(
   node: TSESTree.ObjectExpression,
   isRoot = false,
@@ -107,14 +140,11 @@ export const noShorthandsRule: ReturnType<ReturnType<typeof ESLintUtils.RuleCrea
       description: 'Disallow using CSS shorthands in makeStyles() calls',
       recommended: 'error',
     },
+    fixable: 'code',
     messages: {
       shorthandFound: 'Usage of shorthands is prohibited',
     },
-    schema: [
-      {
-        type: 'string',
-      },
-    ],
+    schema: [],
   },
   defaultOptions: [],
 
@@ -128,9 +158,27 @@ export const noShorthandsRule: ReturnType<ReturnType<typeof ESLintUtils.RuleCrea
             const shorthandProperties = findShorthandProperties(argument, true);
 
             shorthandProperties.forEach(shorthandProperty => {
+              const propertyNode = shorthandProperty.parent as TSESTree.Property;
+              let autoFixArguments: string[] | null = null;
+              if (isLiteral(propertyNode.value)) {
+                const { value } = propertyNode.value;
+                const shorthandToArguments =
+                  SHORTHAND_FUNCTIONS[shorthandProperty.name as keyof typeof UNSUPPORTED_CSS_PROPERTIES];
+                if (shorthandToArguments !== undefined && (typeof value === 'string' || typeof value === 'number')) {
+                  autoFixArguments = shorthandToArguments(value);
+                }
+              }
+
               context.report({
                 node: shorthandProperty,
                 messageId: 'shorthandFound',
+                fix:
+                  autoFixArguments != null
+                    ? fixer => {
+                        const args = autoFixArguments!.map(arg => `'${arg}'`).join(', ');
+                        return fixer.replaceText(propertyNode, `...shorthands.${shorthandProperty.name}(${args})`);
+                      }
+                    : undefined,
               });
             });
           }
diff --git a/packages/eslint-plugin/src/utils/buildShorthandSplitter.ts b/packages/eslint-plugin/src/utils/buildShorthandSplitter.ts
new file mode 100644
index 0000000..54ac30e
--- /dev/null
+++ b/packages/eslint-plugin/src/utils/buildShorthandSplitter.ts
@@ -0,0 +1,65 @@
+interface ShorthandSplitterOptions {
+  /**
+   * Character to split on.
+   * @default ' '
+   */
+  separator?: string;
+  /**
+   * Unit to use for numbers, such as `px` or `em`.
+   * @default '' for unitless number
+   */
+  numberUnit?: string;
+}
+
+/**
+ * Splits a string into an array of CSS functions and values.
+ * If a separator is encountered inside of a function, it is ignored.
+ * If a number is provided, we convert it to a string using the given unit.
+ */
+export function buildShorthandSplitter(options: ShorthandSplitterOptions = {}) {
+  const { separator = ' ', numberUnit = '' } = options;
+
+  return function split(value: string | number): string[] {
+    // If a number is provided, we convert it to a string and append the number suffix.
+    if (typeof value !== 'string') {
+      if (value === 0) {
+        value = '0';
+      } else {
+        value = `${value}${numberUnit}`;
+      }
+      // Numbers are always a single value
+      return [value];
+    }
+
+    // How many `(` have been opened but not closed.
+    let unclosedParenthesis = 0;
+    // If whitespace was just encountered, we don't want to split on the next separator.
+    let justSplit = false;
+    let partStartIndex = 0;
+    const parts = [];
+
+    for (let i = 0; i < value.length; i++) {
+      switch (value[i]) {
+        case separator:
+          if (!justSplit && unclosedParenthesis === 0) {
+            parts.push(value.slice(partStartIndex, i));
+            partStartIndex = i + 1;
+            justSplit = true;
+          }
+          continue;
+        case '(':
+          unclosedParenthesis++;
+          break;
+        case ')':
+          unclosedParenthesis--;
+          break;
+      }
+      justSplit = false;
+    }
+
+    if (!justSplit) {
+      parts.push(value.slice(partStartIndex));
+    }
+    return parts.map(part => part.trim());
+  };
+}
diff --git a/packages/eslint-plugin/src/utils/helpers.ts b/packages/eslint-plugin/src/utils/helpers.ts
index d33c84b..d472a65 100644
--- a/packages/eslint-plugin/src/utils/helpers.ts
+++ b/packages/eslint-plugin/src/utils/helpers.ts
@@ -4,8 +4,8 @@ type IsHelper<NodeType extends AST_NODE_TYPES> = (node: TSESTree.Node | null | u
   type: NodeType;
 };
 
-const isLiteral: IsHelper<AST_NODE_TYPES.Literal> = ASTUtils.isNodeOfType(AST_NODE_TYPES.Literal);
 export const isIdentifier: IsHelper<AST_NODE_TYPES.Identifier> = ASTUtils.isIdentifier;
+export const isLiteral: IsHelper<AST_NODE_TYPES.Literal> = ASTUtils.isNodeOfType(AST_NODE_TYPES.Literal);
 export const isObjectExpression: IsHelper<AST_NODE_TYPES.ObjectExpression> = ASTUtils.isNodeOfType(
   AST_NODE_TYPES.ObjectExpression,
 );


Test patch:
diff --git a/packages/eslint-plugin/src/rules/no-shorthands.test.ts b/packages/eslint-plugin/src/rules/no-shorthands.test.ts
index 36a39c3..1467d76 100644
--- a/packages/eslint-plugin/src/rules/no-shorthands.test.ts
+++ b/packages/eslint-plugin/src/rules/no-shorthands.test.ts
@@ -60,5 +60,85 @@ export const useStyles = makeStyles({
 `,
       errors: [{ messageId: 'shorthandFound' }],
     },
+    {
+      name: 'border shorthand',
+      code: `
+import { makeStyles } from '@griffel/react';
+
+export const useStyles = makeStyles({
+  root: { border: '1px solid rgb(0 0 0)' },
+  icon: { borderLeft: '1px solid' },
+});
+`,
+      output: `
+import { makeStyles } from '@griffel/react';
+
+export const useStyles = makeStyles({
+  root: { ...shorthands.border('1px', 'solid', 'rgb(0 0 0)') },
+  icon: { ...shorthands.borderLeft('1px', 'solid') },
+});
+`,
+      errors: [{ messageId: 'shorthandFound' }, { messageId: 'shorthandFound' }],
+    },
+    {
+      name: 'flex shorthand',
+      code: `
+import { makeStyles } from '@griffel/react';
+
+export const useStyles = makeStyles({
+  root: { flex: '1 2 30px' },
+  icon: { flex: 0 },
+});
+`,
+      output: `
+import { makeStyles } from '@griffel/react';
+
+export const useStyles = makeStyles({
+  root: { ...shorthands.flex('1', '2', '30px') },
+  icon: { ...shorthands.flex('0') },
+});
+`,
+      errors: [{ messageId: 'shorthandFound' }, { messageId: 'shorthandFound' }],
+    },
+    {
+      name: 'margin shorthand',
+      code: `
+import { makeStyles } from '@griffel/react';
+
+export const useStyles = makeStyles({
+  root: { margin: '1em var(--margin-x) 30px' },
+  icon: { margin: 20 },
+});
+`,
+      output: `
+import { makeStyles } from '@griffel/react';
+
+export const useStyles = makeStyles({
+  root: { ...shorthands.margin('1em', 'var(--margin-x)', '30px') },
+  icon: { ...shorthands.margin('20px') },
+});
+`,
+      errors: [{ messageId: 'shorthandFound' }, { messageId: 'shorthandFound' }],
+    },
+    {
+      name: 'grid-area shorthand',
+      code: `
+import { makeStyles } from '@griffel/react';
+
+export const useStyles = makeStyles({
+  root: { gridArea: 'box' },
+  icon: { gridArea: '1 icon / span 2' },
+});
+`,
+      output: `
+import { makeStyles } from '@griffel/react';
+
+export const useStyles = makeStyles({
+  root: { ...shorthands.gridArea('box') },
+  icon: { ...shorthands.gridArea('1 icon', 'span 2') },
+});
+`,
+      errors: [{ messageId: 'shorthandFound' }, { messageId: 'shorthandFound' }],
+    },
   ],
 });
diff --git a/packages/eslint-plugin/src/utils/buildShorthandSplitter.test.ts b/packages/eslint-plugin/src/utils/buildShorthandSplitter.test.ts
new file mode 100644
index 0000000..6df1353
--- /dev/null
+++ b/packages/eslint-plugin/src/utils/buildShorthandSplitter.test.ts
@@ -0,0 +1,44 @@
+import { buildShorthandSplitter } from './buildShorthandSplitter';
+
+describe('buildShorthandSplitter', () => {
+  test('should convert numbers to px', () => {
+    const pxSplitter = buildShorthandSplitter({ numberUnit: 'px' });
+    expect(pxSplitter('1 2 3 4')).toEqual(['1', '2', '3', '4']);
+    expect(pxSplitter(10)).toEqual(['10px']);
+    expect(pxSplitter(15)).toEqual(['15px']);
+    expect(pxSplitter(0)).toEqual(['0']);
+  });
+
+  test('should split around spaces', () => {
+    const splitter = buildShorthandSplitter();
+    expect(splitter('1px solid black')).toEqual(['1px', 'solid', 'black']);
+    expect(splitter('1px solid')).toEqual(['1px', 'solid']);
+    expect(splitter('1px')).toEqual(['1px']);
+    expect(splitter('1 2 30px')).toEqual(['1', '2', '30px']);
+  });
+
+  test('should split around slashes', () => {
+    const splitter = buildShorthandSplitter({ separator: '/' });
+    expect(splitter('1 icon / span 2')).toEqual(['1 icon', 'span 2']);
+    expect(splitter('span 2')).toEqual(['span 2']);
+  });
+
+  test('should not split CSS functions', () => {
+    const splitter = buildShorthandSplitter();
+    expect(splitter('var(--color) rgba(0 0 0 / 0.5) rgb(0, 0, 0)')).toEqual([
+      'var(--color)',
+      'rgba(0 0 0 / 0.5)',
+      'rgb(0, 0, 0)',
+    ]);
+    expect(splitter('var(--color)   rgba(0   0 0 /  0.5) rgb(0,  0, 0)')).toEqual([
+      'var(--color)',
+      'rgba(0   0 0 /  0.5)',
+      'rgb(0,  0, 0)',
+    ]);
+    expect(splitter('var(--color, var(--fallback)) rgba(0 0 0 / 0.5) rgb(0, 0, 0)  ')).toEqual([
+      'var(--color, var(--fallback))',
+      'rgba(0 0 0 / 0.5)',
+      'rgb(0, 0, 0)',
+    ]);
+  });
+});