From 9f28137b38b32ebcd8128b8e68e38a4260610fde Mon Sep 17 00:00:00 2001 From: Jess Telford Date: Thu, 27 Oct 2022 16:15:02 +1100 Subject: [PATCH] Utilities to handle the Partial Fix case in SASS Migrations --- .changeset/odd-pans-poke.md | 5 + polaris-migrator/README.md | 5 +- .../replace-spacing-lengths.ts | 19 +-- polaris-migrator/src/utilities/sass.ts | 129 ++++++++++++++++-- .../{{kebabCase migrationName}}.ts.hbs | 5 +- 5 files changed, 127 insertions(+), 36 deletions(-) create mode 100644 .changeset/odd-pans-poke.md diff --git a/.changeset/odd-pans-poke.md b/.changeset/odd-pans-poke.md new file mode 100644 index 00000000000..64414f06ceb --- /dev/null +++ b/.changeset/odd-pans-poke.md @@ -0,0 +1,5 @@ +--- +'@shopify/polaris-migrator': minor +--- + +Expose utilities for SASS Migrations to leverage the Suggestion-on-partial-fix pattern diff --git a/polaris-migrator/README.md b/polaris-migrator/README.md index 7c00c476d3b..02750a8397a 100644 --- a/polaris-migrator/README.md +++ b/polaris-migrator/README.md @@ -293,7 +293,7 @@ import type {PolarisMigrator} from '../../utilities/sass'; const replaceHelloWorld: PolarisMigrator = (_, {methods}, context) => { return (root) => { - root.walkDecls((decl) => { + methods.walkDecls(root, (decl) => { const parsedValue = valueParser(decl.value); parsedValue.walk((node) => { if (isSassFunction('hello', node)) { @@ -311,12 +311,9 @@ const replaceHelloWorld: PolarisMigrator = (_, {methods}, context) => { return StopWalkingFunctionNodes; } }); - if (context.fix) { decl.value = parsedValue.toString(); } - - methods.flushReports(); }); }; }; diff --git a/polaris-migrator/src/migrations/replace-spacing-lengths/replace-spacing-lengths.ts b/polaris-migrator/src/migrations/replace-spacing-lengths/replace-spacing-lengths.ts index 5a6d892ad84..088f09cf8b1 100644 --- a/polaris-migrator/src/migrations/replace-spacing-lengths/replace-spacing-lengths.ts +++ b/polaris-migrator/src/migrations/replace-spacing-lengths/replace-spacing-lengths.ts @@ -18,30 +18,17 @@ export default createSassMigrator( const namespacedRem = namespace('rem', options); return (root) => { - root.walkDecls((decl) => { + methods.walkDecls(root, (decl) => { if (!spaceProps.has(decl.prop)) return; const parsedValue = valueParser(decl.value); handleSpaceProps(); - const newValue = parsedValue.toString(); - - if (context.fix && newValue !== decl.value) { - if (methods.getReportsForNode(decl)) { - // The "partial fix" case: When there's a new value AND a report. - methods.report({ - node: decl, - severity: 'suggestion', - message: `${decl.prop}: ${parsedValue.toString()}`, - }); - } else { - decl.value = parsedValue.toString(); - } + if (context.fix) { + decl.value = parsedValue.toString(); } - methods.flushReports(); - function handleSpaceProps() { parsedValue.walk((node) => { if (isNumericOperator(node)) { diff --git a/polaris-migrator/src/utilities/sass.ts b/polaris-migrator/src/utilities/sass.ts index a9e61bc1e0e..3242a1b9aa5 100644 --- a/polaris-migrator/src/utilities/sass.ts +++ b/polaris-migrator/src/utilities/sass.ts @@ -1,5 +1,15 @@ import type {FileInfo, API, Options} from 'jscodeshift'; -import postcss, {Root, Result, Plugin, Node as PostCSSNode} from 'postcss'; +import postcss, { + Root, + Result, + Plugin, + Container, + Declaration, + Node as PostCSSNode, + Rule as PostCSSRule, + Comment as PostCSSComment, + AtRule, +} from 'postcss'; import valueParser, { Node, ParsedValue, @@ -259,7 +269,7 @@ interface PluginOptions extends Options, NamespaceOptions {} interface Report { node: PostCSSNode; - severity: 'warning' | 'error' | 'suggestion'; + severity: 'warning' | 'error'; message: string; } @@ -286,14 +296,32 @@ type StylelintRule

= StylelintRuleBase & { }; // End: Extracted from stylelint +type Walker = (node: N) => false | void; + export type PolarisMigrator = ( primaryOption: true, secondaryOptions: { options: {[key: string]: any}; methods: { report: (report: Report) => void; - flushReports: () => void; - getReportsForNode: (node: PostCSSNode) => Report[] | undefined; + each: (root: T, walker: Walker) => void; + walk: (root: T, walker: Walker) => void; + walkComments: ( + root: T, + walker: Walker, + ) => void; + walkAtRules: ( + root: T, + atRuleWalker: Walker, + ) => void; + walkDecls: ( + root: T, + declWalker: Walker, + ) => void; + walkRules: ( + root: T, + ruleWalker: Walker, + ) => void; }; }, context: PluginContext, @@ -376,18 +404,49 @@ export function createSassMigrator(name: string, ruleFn: PolarisMigrator) { for (const report of reportsForNode) { node.before( - report.severity === 'suggestion' - ? createInlineComment(report.message) - : createInlineComment(`${report.severity}: ${report.message}`, { - prose: true, - }), + createInlineComment(`${report.severity}: ${report.message}`, { + prose: true, + }), ); } } reports.clear(); }; - const getReportsForNode = (node: PostCSSNode) => reports.get(node); + function createWalker(args: { + walker: (node: T) => false | void; + serialiseSuggestion: (node: T) => string; + }): (node: T) => false | void { + const {walker, serialiseSuggestion} = args; + + return (node: T) => { + let oldNode: T; + if (context.fix) { + oldNode = node.clone(); + } + + const result = walker(node); + + const isPartialFix = + context.fix && + reports.has(node) && + node.toString() !== oldNode!.toString(); + + flushReportsAsComments(); + + // Our migrations have an opinion on partial fixes (when multiple + // issues are found in a single node, and some but not all can be + // fixed): We dump out the partial fix result as a comment + // immediately above the node. + if (isPartialFix) { + node.before(createInlineComment(serialiseSuggestion(node))); + // Undo changes + node.replaceWith(oldNode!); + } + + return result; + }; + } return ruleFn( primary, @@ -399,8 +458,54 @@ export function createSassMigrator(name: string, ruleFn: PolarisMigrator) { options: secondaryOptions, methods: { report: addDedupedReport, - flushReports: flushReportsAsComments, - getReportsForNode, + each(root, walker) { + root.each( + createWalker({ + walker, + serialiseSuggestion: (node) => node.toString(), + }), + ); + }, + walk(root, walker) { + root.walk( + createWalker({ + walker, + serialiseSuggestion: (node) => node.toString(), + }), + ); + }, + walkAtRules(root, walker) { + root.walkAtRules( + createWalker({ + walker, + serialiseSuggestion: (node) => `@${node.name} ${node.params}`, + }), + ); + }, + walkComments(root, walker) { + root.walkComments( + createWalker({ + walker, + serialiseSuggestion: (node) => node.text, + }), + ); + }, + walkDecls(root, walker) { + root.walkDecls( + createWalker({ + walker, + serialiseSuggestion: (node) => `${node.prop}: ${node.value}`, + }), + ); + }, + walkRules(root, walker) { + root.walkRules( + createWalker({ + walker, + serialiseSuggestion: (node) => node.selector, + }), + ); + }, }, }, context, diff --git a/polaris-migrator/templates/sass-migration/{{kebabCase migrationName}}/{{kebabCase migrationName}}.ts.hbs b/polaris-migrator/templates/sass-migration/{{kebabCase migrationName}}/{{kebabCase migrationName}}.ts.hbs index 488c77a7c19..214c559ab08 100644 --- a/polaris-migrator/templates/sass-migration/{{kebabCase migrationName}}/{{kebabCase migrationName}}.ts.hbs +++ b/polaris-migrator/templates/sass-migration/{{kebabCase migrationName}}/{{kebabCase migrationName}}.ts.hbs @@ -16,7 +16,7 @@ const {{camelCase migrationName}}: PolarisMigrator = ( context, ) => { return (root) => { - root.walkDecls((decl) => { + methods.walkDecls(root, (decl) => { // Using the parsedValue allows easy detection of individual functions and // properties. Particularly useful when dealing with shorthand // declarations such as `border`, `padding`, etc. @@ -45,9 +45,6 @@ const {{camelCase migrationName}}: PolarisMigrator = ( if (context.fix) { decl.value = parsedValue.toString(); } - - // Ensure all generated reports are flushed to the appropriate output - methods.flushReports(); }); }; };