More robust stylelint shim to support reporting

Author: jesstelford

polaris-migrator/README.md

@@ -278,37 +278,47 @@ migrations
 
 #### The SASS migration function
 
-Each migrator has a default export adhering to the [PostCSS Plugin API](https://github.com/postcss/postcss/blob/main/docs/writing-a-plugin.md) with one main difference: events are only executed once.
+Each migrator has a default export adhering to the [Stylelint Rule API](https://github.com/postcss/postcss/blob/main/docs/writing-a-plugin.md). A PostCSS AST is passed as the `root` and can be mutated inline, or emit warning/error reports.
 
 Continuing the example, here is what the migration may look like if our goal is to replace the Sass function `hello()` with `world()`.
 
 ```ts
 // polaris-migrator/src/migrations/replace-sass-function/replace-sass-function.ts
-import valueParser from 'postcss-value-parser';
-
 import {
   isSassFunction,
-  StopWalkingFunctionNodes,
   createSassMigrator,
+  StopWalkingFunctionNodes,
 } from '../../utilities/sass';
+import type {PolarisMigrator} from '../../utilities/sass';
 
-// options can be passed in from cli / config.
-export default createSassMigrator('replace-sass-function', (/* options */) => {
-  return {
-    Declaration(decl) {
+const replaceHelloWorld: PolarisMigrator = (_, {methods}, context) => {
+  return (root) => {
+    methods.walkDecls(root, (decl) => {
       const parsedValue = valueParser(decl.value);
-
       parsedValue.walk((node) => {
         if (isSassFunction('hello', node)) {
-          node.value = 'world';
+          if (context.fix) {
+            node.value = 'world';
+          } else {
+            methods.report({
+              node: decl,
+              severity: 'error',
+              message:
+                'Method hello() is no longer supported. Please migrate to world().',
+            });
+          }
+
           return StopWalkingFunctionNodes;
         }
       });
-
-      decl.value = parsedValue.toString();
-    },
+      if (context.fix) {
+        decl.value = parsedValue.toString();
+      }
+    });
   };
-});
+};
+
+export default createSassMigrator('replace-hello-world', replaceHelloWorld);
 ```
 
 A more complete example can be seen in [`replace-spacing-lengths.ts`](https://github.com/Shopify/polaris/blob/main/polaris-migrator/src/migrations/replace-spacing-lengths/replace-spacing-lengths.ts).

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

@@ -1,8 +1,6 @@
 import valueParser from 'postcss-value-parser';
 
-import {POLARIS_MIGRATOR_COMMENT} from '../../constants';
 import {
-  createInlineComment,
   getFunctionArgs,
   isNumericOperator,
   isSassFunction,
@@ -16,39 +14,43 @@ import {isKeyOf} from '../../utilities/type-guards';
 
 export default createSassMigrator(
   'replace-sass-space',
-  (_, options, context) => {
+  (_, {methods, options}, context) => {
     const namespacedRem = namespace('rem', options);
 
     return (root) => {
       root.walkDecls((decl) => {
         if (!spaceProps.has(decl.prop)) return;
 
-        /**
-         * A collection of transformable values to migrate (e.g. decl lengths, functions, etc.)
-         *
-         * Note: This is evaluated at the end of each visitor execution to determine whether
-         * or not to replace the declaration or insert a comment.
-         */
-        const targets: {replaced: boolean}[] = [];
         let hasNumericOperator = false;
+
         const parsedValue = valueParser(decl.value);
 
         handleSpaceProps();
 
-        if (targets.some(({replaced}) => !replaced || hasNumericOperator)) {
-          decl.before(
-            createInlineComment(POLARIS_MIGRATOR_COMMENT, {prose: true}),
-          );
-          decl.before(
-            createInlineComment(`${decl.prop}: ${parsedValue.toString()};`),
-          );
-        } else if (context.fix) {
+        if (hasNumericOperator) {
+          methods.report({
+            node: decl,
+            severity: 'warning',
+            message: 'Numeric operator detected.',
+          });
+        }
+
+        if (context.fix && !methods.getReportsForNode(decl)) {
           decl.value = parsedValue.toString();
+        } else {
+          // The "partial fix" case means we want to insert a suggestion if
+          // there were changes, but not actually change the line of code.
+          const newValue = parsedValue.toString();
+          if (newValue !== decl.value) {
+            methods.report({
+              node: decl,
+              severity: 'suggestion',
+              message: `${decl.prop}: ${parsedValue.toString()}`,
+            });
+          }
         }
 
-        //
-        // Handlers
-        //
+        methods.flushReports();
 
         function handleSpaceProps() {
           parsedValue.walk((node) => {
@@ -64,41 +66,71 @@ export default createSassMigrator(
 
               if (!isTransformableLength(dimension)) return;
 
-              targets.push({replaced: false});
-
               const valueInPx = toTransformablePx(node.value);
 
-              if (!isKeyOf(spaceMap, valueInPx)) return;
+              if (!isKeyOf(spaceMap, valueInPx)) {
+                methods.report({
+                  node: decl,
+                  severity: 'error',
+                  message: `Non-tokenizable value '${node.value}'`,
+                });
+                return;
+              }
 
-              targets[targets.length - 1]!.replaced = true;
+              if (context.fix) {
+                node.value = `var(${spaceMap[valueInPx]})`;
+                return;
+              }
 
-              node.value = `var(${spaceMap[valueInPx]})`;
+              methods.report({
+                node: decl,
+                severity: 'error',
+                message: `Prefer var(${spaceMap[valueInPx]}) Polaris token.`,
+              });
               return;
             }
 
             if (node.type === 'function') {
               if (isSassFunction(namespacedRem, node)) {
-                targets.push({replaced: false});
-
                 const args = getFunctionArgs(node);
 
-                if (args.length !== 1) return;
+                if (args.length !== 1) {
+                  methods.report({
+                    node: decl,
+                    severity: 'error',
+                    message: `Expected 1 argument, got ${args.length}`,
+                  });
+                  return;
+                }
 
                 const valueInPx = toTransformablePx(args[0]);
 
-                if (!isKeyOf(spaceMap, valueInPx)) return;
-
-                targets[targets.length - 1]!.replaced = true;
-
-                node.value = 'var';
-                node.nodes = [
-                  {
-                    type: 'word',
-                    value: spaceMap[valueInPx],
-                    sourceIndex: node.nodes[0]?.sourceIndex ?? 0,
-                    sourceEndIndex: spaceMap[valueInPx].length,
-                  },
-                ];
+                if (!isKeyOf(spaceMap, valueInPx)) {
+                  methods.report({
+                    node: decl,
+                    severity: 'error',
+                    message: `Non-tokenizable value '${args[0].trim()}'`,
+                  });
+                  return;
+                }
+
+                if (context.fix) {
+                  node.value = 'var';
+                  node.nodes = [
+                    {
+                      type: 'word',
+                      value: spaceMap[valueInPx],
+                      sourceIndex: node.nodes[0]?.sourceIndex ?? 0,
+                      sourceEndIndex: spaceMap[valueInPx].length,
+                    },
+                  ];
+                  return;
+                }
+                methods.report({
+                  node: decl,
+                  severity: 'error',
+                  message: `Prefer var(${spaceMap[valueInPx]}) Polaris token.`,
+                });
               }
 
               return StopWalkingFunctionNodes;

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

@@ -15,6 +15,8 @@
   padding: 0;
   padding: 1;
   padding: 2;
+  // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
+  // warning: Numeric operator detected.
   padding: #{16px + 16px};
   padding: layout-width(nav);
   padding: 10em;
@@ -23,55 +25,65 @@
   padding: var(--p-space-4, 16px);
   // Comment
   // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
-  // padding: 10px;
+  // error: Non-tokenizable value '10px'
   padding: 10px;
   // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
-  // padding: 10rem;
+  // error: Non-tokenizable value '10rem'
   padding: 10rem;
   // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
-  // padding: 10px 10px;
+  // error: Non-tokenizable value '10px'
   padding: 10px 10px;
   // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
+  // error: Non-tokenizable value '10px'
   // padding: var(--p-space-4) 10px;
   padding: 16px 10px;
   // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
-  // padding: 10rem 10rem;
+  // error: Non-tokenizable value '10rem'
   padding: 10rem 10rem;
   // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
+  // error: Non-tokenizable value '10rem'
   // padding: var(--p-space-4) 10rem;
   padding: 1rem 10rem;
   // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
-  // padding: 10px 10rem;
+  // error: Non-tokenizable value '10px'
+  // error: Non-tokenizable value '10rem'
   padding: 10px 10rem;
   // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
+  // error: Non-tokenizable value '10rem'
   // padding: var(--p-space-4) 10rem;
   padding: 16px 10rem;
   // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
+  // error: Non-tokenizable value '10px'
   // padding: 10px var(--p-space-4);
   padding: 10px 1rem;
   // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
+  // error: Non-tokenizable value '-16px'
   // padding: var(--p-space-4) -16px;
   padding: 16px -16px;
   // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
+  // warning: Numeric operator detected.
   // padding: var(--p-space-4) + var(--p-space-4);
   padding: 16px + 16px;
   // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
+  // warning: Numeric operator detected.
   // padding: var(--p-space-4) + var(--p-space-4) var(--p-space-4);
   padding: 16px + 16px 16px;
   // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
+  // warning: Numeric operator detected.
   // padding: $var + var(--p-space-4);
   padding: $var + 16px;
   padding: calc(16px + 16px);
   // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
+  // warning: Numeric operator detected.
   // padding: var(--p-space-4) + #{16px + 16px};
   padding: 16px + #{16px + 16px};
   // Skip negative lengths. Need to discuss replacement strategy e.g.
   // calc(-1 * var(--p-space-*)) vs var(--p-space--*)
   // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
-  // padding: -16px;
+  // error: Non-tokenizable value '-16px'
   padding: -16px;
   // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
-  // padding: -10px;
+  // error: Non-tokenizable value '-10px'
   padding: -10px;
 
   // REM FUNCTION
@@ -86,21 +98,26 @@
   padding: calc(10rem + var(--p-choice-size, #{rem(10px)}));
   // Comment
   // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
-  // padding: rem(10px);
+  // error: Non-tokenizable value '10px'
   padding: rem(10px);
   // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
-  // padding: rem(10px) rem(10px);
+  // error: Non-tokenizable value '10px'
   padding: rem(10px) rem(10px);
   // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
+  // error: Non-tokenizable value '10px'
   // padding: var(--p-space-4) rem(10px);
   padding: rem(16px) rem(10px);
   // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
+  // error: Non-tokenizable value '-16px'
   // padding: var(--p-space-4) -16px;
   padding: rem(16px) -16px;
   // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
+  // warning: Numeric operator detected.
   // padding: var(--p-space-4) + var(--p-space-4);
   padding: rem(16px) + 16px;
   // polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
+  // error: Non-tokenizable value '$var \* 16px'
+  // warning: Numeric operator detected.
   // padding: rem($var * var(--p-space-4));
   padding: rem($var * 16px);
 }