Improve Sass spacing migration to cover interpolations (#7412)

<!--
  ☝️How to write a good PR title:
- Prefix it with [ComponentName] (if applicable), for example: [Button]
  - Start with a verb, for example: Add, Delete, Improve, Fix…
  - Give as much context as necessary and as little as possible
  - Prefix it with [WIP] while it’s a work in progress
-->

### WHY are these changes introduced?

We were missing some `spacing()` functions on the initial migrations
where it was being used within interpolations:

```scss
padding: -#{spacing()};
padding calc(100% - #{spacing()};
```

<!--
  Context about the problem that’s being addressed.
-->

### WHAT is this pull request doing?

This enhances the `replace-sass-spacing` to catch edge-cases of Sass
interpolations.

```scss
// Before
padding: -#{spacing()};
padding calc(100% - #{spacing()};

// After
padding: -1 * var(--p-space-4); // note this will also add a manual check comment
padding calc(100% - var(--p-space-4)};
```


<!--
  Summary of the changes committed.

Before / after screenshots are appreciated for UI changes. Make sure to
include alt text that describes the screenshot.

If you include an animated gif showing your change, wrapping it in a
details tag is recommended. Gifs usually autoplay, which can cause
accessibility issues for people reviewing your PR:

    <details>
      <summary>Summary of your gif(s)</summary>
      <img src="..." alt="Description of what the gif shows">
    </details>
-->

<!-- ℹ️ Delete the following for small / trivial changes -->

### How to 🎩

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development)
🗒 [General tophatting
guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md)
📄 [Changelog
guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)

<!--
  Give as much information as needed to experiment with the component
  in the playground.
-->

<details>
<summary>Copy-paste this code in
<code>playground/Playground.tsx</code>:</summary>

```jsx
import React from 'react';
import {Page} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      {/* Add the code you want to test in here */}
    </Page>
  );
}
```

</details>

### 🎩 checklist

- [ ] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [ ] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [ ] Updated the component's `README.md` with documentation changes
- [ ] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide

Author: sam-b-rose

.changeset/large-pillows-wave.md

@@ -0,0 +1,5 @@
+---
+'@shopify/polaris-migrator': patch
+---
+
+Enhance Sass spacing migration to catch Sass interpolations

polaris-migrator/src/migrations/replace-sass-spacing/replace-sass-spacing.ts

@@ -9,6 +9,11 @@ import {
   isSassFunction,
   hasSassFunction,
   hasNumericOperator,
+  hasSassInterpolation,
+  removeSassInterpolation,
+  hasNegativeSassInterpolation,
+  replaceNegativeSassInterpolation,
+  createInlineComment,
 } from '../../utilities/sass';
 import {isKeyOf} from '../../utilities/type-guards';
 
@@ -38,6 +43,17 @@ const plugin = (options: PluginOptions = {}): Plugin => {
 
       const parsedValue = valueParser(decl.value);
 
+      // Convert -#{spacing()} to -1 * #{spacing()}
+      if (hasNegativeSassInterpolation(decl.value)) {
+        replaceNegativeSassInterpolation(parsedValue);
+      }
+
+      // Remove #{} from spacing()
+      if (hasSassInterpolation(parsedValue.toString())) {
+        removeSassInterpolation(namespacedSpacing, parsedValue);
+      }
+
+      // Now we can check if the value is a spacing() function
       if (!hasSassFunction(namespacedSpacing, parsedValue)) return;
 
       parsedValue.walk((node) => {
@@ -56,15 +72,16 @@ const plugin = (options: PluginOptions = {}): Plugin => {
             sourceIndex: node.nodes[0]?.sourceIndex ?? 0,
             sourceEndIndex: spacingCustomProperty.length,
           },
-          ...node.nodes.slice(1),
         ];
       });
 
       if (hasNumericOperator(parsedValue)) {
         // Insert comment if the declaration value contains calculations
-        decl.before(postcss.comment({text: POLARIS_MIGRATOR_COMMENT}));
         decl.before(
-          postcss.comment({text: `${decl.prop}: ${parsedValue.toString()};`}),
+          createInlineComment(POLARIS_MIGRATOR_COMMENT, {prose: true}),
+        );
+        decl.before(
+          createInlineComment(`${decl.prop}: ${parsedValue.toString()};`),
         );
       } else {
         decl.value = parsedValue.toString();

polaris-migrator/src/migrations/replace-sass-spacing/tests/replace-spacing.input.scss

@@ -1,14 +1,16 @@
-.Card {
-  z-index: z-index(content) + 1;
-  padding: spacing(loose) spacing(extra-loose);
-  border-radius: var(--p-border-radius-2, border-radius());
-  box-shadow: var(--p-shadow-card, shadow());
-  margin: spacing(tight) 0 0 (-1 * spacing());
-}
+@use 'global-styles/legacy-polaris-v8';
 
-.ButtonContainer {
-  right: spacing();
-  top: spacing(loose);
-  bottom: spacing() + spacing();
-  left: spacing('tight');
+.my-class {
+  // Standard replacement
+  padding: spacing();
+  padding: spacing(loose);
+  // With multiple values
+  padding: spacing() spacing();
+  padding: spacing() 0 spacing() 0;
+  // With interpolation
+  padding: calc(50% - #{spacing()});
+  // With negative interpolation
+  padding: -#{spacing()};
+  // With other functions
+  padding: calc(#{rem(16px)} + #{spacing()});
 }

polaris-migrator/src/migrations/replace-sass-spacing/tests/replace-spacing.output.scss

@@ -1,18 +1,22 @@
-.Card {
-  z-index: z-index(content) + 1;
-  padding: var(--p-space-5) var(--p-space-8);
-  border-radius: var(--p-border-radius-2, border-radius());
-  box-shadow: var(--p-shadow-card, shadow());
-  /* polaris-migrator: Unable to migrate the following expression. Please upgrade manually. */
-  /* margin: var(--p-space-2) 0 0 (-1 * var(--p-space-4)); */
-  margin: spacing(tight) 0 0 (-1 * spacing());
-}
+@use 'global-styles/legacy-polaris-v8';
 
-.ButtonContainer {
-  right: var(--p-space-4);
-  top: var(--p-space-5);
-  /* polaris-migrator: Unable to migrate the following expression. Please upgrade manually. */
-  /* bottom: var(--p-space-4) + var(--p-space-4); */
-  bottom: spacing() + spacing();
-  left: var(--p-space-2);
+.my-class {
+  // Standard replacement
+  padding: var(--p-space-4);
+  padding: var(--p-space-5);
+  // With multiple values
+  padding: var(--p-space-4) var(--p-space-4);
+  padding: var(--p-space-4) 0 var(--p-space-4) 0;
+  // With interpolation
+  // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
+  // padding: calc(50% - var(--p-space-4));
+  padding: calc(50% - #{spacing()});
+  // With negative interpolation
+  // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
+  // padding: -1 * var(--p-space-4);
+  padding: -#{spacing()};
+  // With other functions
+  // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
+  // padding: calc(#{rem(16px)} + var(--p-space-4));
+  padding: calc(#{rem(16px)} + #{spacing()});
 }

polaris-migrator/src/migrations/replace-sass-spacing/tests/with-namespace.input.scss

@@ -1,11 +1,18 @@
 @use 'global-styles/legacy-polaris-v8';
 
-.Card {
-  z-index: legacy-polaris-v8.z-index(content) + 1;
-  padding: legacy-polaris-v8.spacing(loose)
-    legacy-polaris-v8.spacing(extra-loose);
-  border-radius: var(--p-border-radius-2, legacy-polaris-v8.border-radius());
-  box-shadow: var(--p-shadow-card, legacy-polaris-v8.shadow());
-  margin: legacy-polaris-v8.spacing(tight) 0 0
-    (-1 * legacy-polaris-v8.spacing());
+.my-class {
+  // Standard replacement
+  padding: legacy-polaris-v8.spacing();
+  padding: legacy-polaris-v8.spacing(loose);
+  // With multiple values
+  padding: legacy-polaris-v8.spacing() legacy-polaris-v8.spacing();
+  padding: legacy-polaris-v8.spacing() 0 legacy-polaris-v8.spacing() 0;
+  // With interpolation
+  padding: calc(50% - #{legacy-polaris-v8.spacing()});
+  // With negative interpolation
+  padding: -#{legacy-polaris-v8.spacing()};
+  // With other functions
+  padding: calc(
+    #{legacy-polaris-v8.rem(16px)} + #{legacy-polaris-v8.spacing()}
+  );
 }

polaris-migrator/src/migrations/replace-sass-spacing/tests/with-namespace.output.scss

@@ -1,13 +1,24 @@
 @use 'global-styles/legacy-polaris-v8';
 
-.Card {
-  z-index: legacy-polaris-v8.z-index(content) + 1;
-  padding: var(--p-space-5) var(--p-space-8);
-  border-radius: var(--p-border-radius-2, legacy-polaris-v8.border-radius());
-  box-shadow: var(--p-shadow-card, legacy-polaris-v8.shadow());
-  /* polaris-migrator: Unable to migrate the following expression. Please upgrade manually. */
-  /* margin: var(--p-space-2) 0 0
-    (-1 * var(--p-space-4)); */
-  margin: legacy-polaris-v8.spacing(tight) 0 0
-    (-1 * legacy-polaris-v8.spacing());
+.my-class {
+  // Standard replacement
+  padding: var(--p-space-4);
+  padding: var(--p-space-5);
+  // With multiple values
+  padding: var(--p-space-4) var(--p-space-4);
+  padding: var(--p-space-4) 0 var(--p-space-4) 0;
+  // With interpolation
+  // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
+  // padding: calc(50% - var(--p-space-4));
+  padding: calc(50% - #{legacy-polaris-v8.spacing()});
+  // With negative interpolation
+  // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
+  // padding: -1 * var(--p-space-4);
+  padding: -#{legacy-polaris-v8.spacing()};
+  // With other functions
+  // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
+  // padding: calc(#{legacy-polaris-v8.rem(16px)} + var(--p-space-4));
+  padding: calc(
+    #{legacy-polaris-v8.rem(16px)} + #{legacy-polaris-v8.spacing()}
+  );
 }

polaris-migrator/src/utilities/sass.ts

@@ -1,11 +1,12 @@
-import type {Declaration} from 'postcss';
+import postcss, {Declaration} from 'postcss';
 import valueParser, {
   Node,
   ParsedValue,
   FunctionNode,
   Dimension,
 } from 'postcss-value-parser';
 import {toPx} from '@shopify/polaris-tokens';
+import prettier from 'prettier';
 
 import {isKeyOf} from './type-guards';
 
@@ -89,6 +90,75 @@ export function hasSassFunction(
   return containsSassFunction;
 }
 
+/**
+ * Check whether a string has Sass interpolation
+ */
+export function hasSassInterpolation(string: string) {
+  return /#\{.+?\}/.test(string);
+}
+
+/**
+ * Check whether a string has negative Sass interpolation
+ */
+export function hasNegativeSassInterpolation(string: string) {
+  return /-#\{.+?\}/.test(string);
+}
+
+/**
+ * Replace negative Sass interpolations with a multiplication operator and a negative number
+ *
+ * @example
+ * // Before
+ * -#{spacing()};
+ *
+ * // After
+ * -1 * ${spacing()};
+ */
+export function replaceNegativeSassInterpolation(parsedValue: ParsedValue) {
+  let newNodes: Node[] = [];
+  parsedValue.walk((node, index, nodes) => {
+    const containsInterpolation = /-#\{.+?/.test(node.value);
+    if (!containsInterpolation) return;
+
+    node.value = node.value.replace('-#{', '#{');
+
+    const left = index > 0 ? nodes.slice(0, index) : [];
+    const right = nodes.length - 1 > index ? nodes.slice(index + 1) : [];
+
+    newNodes = [
+      ...left,
+      {type: 'word', value: '-1'},
+      {type: 'space', value: ' '},
+      {type: 'word', value: '*'},
+      {type: 'space', value: ' '},
+      node,
+      ...right,
+    ] as Node[];
+  });
+  parsedValue.nodes = newNodes;
+}
+
+/**
+ * Remove the Sass interpolation from parsedValue
+ */
+export function removeSassInterpolation(
+  namespace: string,
+  parsedValue: ParsedValue,
+) {
+  parsedValue.walk((node, index, nodes) => {
+    const regexp = new RegExp(`#{${namespace}`);
+    const containsInterpolation = regexp.test(node.value);
+    if (containsInterpolation) {
+      node.value = node.value.replace(/#\{/, '');
+      nodes[index + 1].value = nodes[index + 1].value.replace(/}/, '');
+    }
+  });
+
+  parsedValue.nodes = parsedValue.nodes.filter(
+    (node) => !(node.type === 'word' && node.value === ''),
+  );
+}
+
 export function getFunctionArgs(node: FunctionNode): string[] {
   const args: string[] = [];
 
@@ -208,3 +278,19 @@ export function replaceRemFunction(
     },
   );
 }
+
+export function createInlineComment(text: string, options?: {prose?: boolean}) {
+  const formatted = prettier
+    .format(text, {
+      parser: options?.prose ? 'markdown' : 'scss',
+      proseWrap: 'never',
+      printWidth: 9999,
+    })
+    .trim();
+  const comment = postcss.comment({text: formatted});
+
+  comment.raws.left = ' ';
+  comment.raws.inline = true;
+
+  return comment;
+}

polaris-migrator/src/utilities/tests/sass.test.ts

@@ -1,6 +1,31 @@
 import valueParser from 'postcss-value-parser';
 
-import {getFunctionArgs} from '../sass';
+import {createInlineComment, getFunctionArgs} from '../sass';
+
+describe('createInlineComment', () => {
+  it('inline and contains starting space', () => {
+    const text = 'padding: var(--p-space-4);';
+    const comment = createInlineComment(text);
+    expect(comment.raws.inline).toBe(true);
+    expect(comment.raws.left).toBe(' ');
+  });
+
+  it('formats single line text', () => {
+    const text = 'padding: var(--p-space-4);';
+    const comment = createInlineComment(text);
+    expect(comment.text).toStrictEqual(text);
+  });
+
+  it('formats multiple line text', () => {
+    const text = `
+      padding: calc(-1 * var(--p-space-4))
+        var(--p-space-4);
+      `;
+    const formatted = 'padding: calc(-1 * var(--p-space-4)) var(--p-space-4);';
+    const comment = createInlineComment(text);
+    expect(comment.text).toStrictEqual(formatted);
+  });
+});
 
 describe('getFunctionArgs', () => {
   it('zero args', () => {