Code review

CHANGELOG.md

@@ -2,7 +2,7 @@
 
 ### Module Migrator
 
-* Add a new `--builtin-only` flag, which migrates global functions to their
+* Add a new `--built-in-only` flag, which migrates global functions to their
   `sass:` module equivalents, while leaving `@import` rules unchanged.
 
 * Fix bug where some functions (`opacity`, `is-bracketed`, and

lib/src/migrators/module.dart

@@ -32,7 +32,7 @@ class ModuleMigrator extends Migrator {
 
   @override
   final argParser = ArgParser()
-    ..addFlag('builtin-only',
+    ..addFlag('built-in-only',
         help: 'Migrates global functions without migrating @import.')
     ..addMultiOption('remove-prefix',
         abbr: 'p',
@@ -79,12 +79,12 @@ class ModuleMigrator extends Migrator {
   Map<Uri, String> migrateFile(
       ImportCache importCache, Stylesheet stylesheet, Importer importer) {
     var forwards = {for (var arg in argResults!['forward']) ForwardType(arg)};
-    var builtInOnly = argResults!['builtin-only'] as bool;
+    var builtInOnly = argResults!['built-in-only'] as bool;
     if (builtInOnly &&
         (argResults!.wasParsed('forward') ||
             argResults!.wasParsed('remove-prefix'))) {
       throw MigrationException('--forward and --remove-prefix may not be '
-          'passed with --builtin-only.');
+          'passed with --built-in-only.');
     }
     if (forwards.contains(ForwardType.prefixed) &&
         !argResults!.wasParsed('remove-prefix')) {
@@ -630,7 +630,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
       });
     }
 
-    if (node.name == "get-function" && !builtInOnly) {
+    if (node.name == "get-function") {
       var declaration = references.getFunctionReferences[node];
       if (declaration != null) {
         _unreferencable.check(declaration, node);
@@ -648,12 +648,15 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
       // Warn for get-function calls without a static name.
       var nameArg =
           node.arguments.named['name'] ?? node.arguments.positional.first;
-      if (nameArg is! StringExpression || nameArg.text.asPlain == null) {
+      if ((nameArg is! StringExpression || nameArg.text.asPlain == null) &&
+          !builtInOnly) {
         emitWarning(
             "get-function call may require \$module parameter", nameArg.span);
         return;
       }
 
+      if (builtInOnly && declaration != null) return;
+
       _patchNamespaceForFunction(node, declaration, (namespace) {
         var beforeParen = node.span.end.offset - 1;
         addPatch(Patch(node.span.file.span(beforeParen, beforeParen),
@@ -794,9 +797,11 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
       }
       if (ruleUrl != null) {
         if (builtInOnly) {
-          _upstreamStylesheets.add(currentUrl);
-          if (migrateDependencies) visitDependency(ruleUrl, import.span);
-          _upstreamStylesheets.remove(currentUrl);
+          if (migrateDependencies) {
+            _upstreamStylesheets.add(currentUrl);
+            visitDependency(ruleUrl, import.span);
+            _upstreamStylesheets.remove(currentUrl);
+          }
         } else if (_useAllowed) {
           migratedRules.addAll(_migrateImportToRules(ruleUrl, import.span));
         } else {

test/migrators/module/built_in_functions/builtin_only/basic.hrx → test/migrators/module/built_in_functions/built_in_only/basic.hrx

@@ -1,5 +1,5 @@
 <==> arguments
---builtin-only
+--built-in-only
 
 <==> input/entrypoint.scss
 @import "library";

test/migrators/module/built_in_functions/built_in_only/get_function.hrx

@@ -0,0 +1,25 @@
+<==> arguments
+--built-in-only
+
+<==> input/entrypoint.scss
+@import "library";
+
+a {
+  b: call(get-function(mix), red, blue);
+  c: call(get-function(fn), green);
+}
+
+<==> input/_library.scss
+@function fn($x) {
+  @return $x;
+}
+
+<==> output/entrypoint.scss
+@use "sass:color";
+@use "sass:meta";
+@import "library";
+
+a {
+  b: meta.call(meta.get-function(mix, $module: "color"), red, blue);
+  c: meta.call(meta.get-function(fn), green);
+}

test/migrators/module/built_in_functions/builtin_only/local_shadow.hrx → test/migrators/module/built_in_functions/built_in_only/local_shadow.hrx

@@ -1,5 +1,5 @@
 <==> arguments
---builtin-only
+--built-in-only
 
 <==> input/entrypoint.scss
 @import "library";

test/migrators/module/built_in_functions/builtin_only/migrate_deps.hrx → test/migrators/module/built_in_functions/built_in_only/migrate_deps.hrx

@@ -1,5 +1,5 @@
 <==> arguments
---builtin-only --migrate-deps
+--built-in-only --migrate-deps
 
 <==> input/entrypoint.scss
 @import "library";

test/migrators/module/built_in_functions/builtin_only/module_already_exists.hrx → test/migrators/module/built_in_functions/built_in_only/module_already_exists.hrx

@@ -1,5 +1,5 @@
 <==> arguments
---builtin-only
+--built-in-only
 
 <==> input/entrypoint.scss
 @use "sass:color";

test/migrators/module/built_in_functions/builtin_only/multiple_imports.hrx → test/migrators/module/built_in_functions/built_in_only/multiple_imports.hrx

@@ -1,5 +1,5 @@
 <==> arguments
---builtin-only --migrate-deps
+--built-in-only --migrate-deps
 
 <==> input/entrypoint.scss
 @import "a", "b";

test/migrators/module/built_in_functions/builtin_only/partially_migrated.hrx → test/migrators/module/built_in_functions/built_in_only/partially_migrated.hrx

@@ -1,5 +1,5 @@
 <==> arguments
---builtin-only
+--built-in-only
 
 <==> input/entrypoint.scss
 @use "sass:math";

test/migrators/module/built_in_functions/builtin_only/shadowed_by_import.hrx → test/migrators/module/built_in_functions/built_in_only/shadowed_by_import.hrx

@@ -1,5 +1,5 @@
 <==> arguments
---builtin-only
+--built-in-only
 
 <==> input/entrypoint.scss
 @import "library";

test/migrators/module/built_in_functions/builtin_only/shadowed_by_use.hrx → test/migrators/module/built_in_functions/built_in_only/shadowed_by_use.hrx

@@ -1,5 +1,5 @@
 <==> arguments
---builtin-only
+--built-in-only
 
 <==> input/entrypoint.scss
 @use "library" as *;

test/migrators/module/built_in_functions/namespacing.hrx

@@ -2,6 +2,7 @@
 $a: mix(red, blue);
 $b: str-length("hello");
 $c: scale-color(blue, $lightness: -10%);
+$d: call(get-function(mix), red, blue);
 
 @function fn($args...) {
   @return keywords($args);
@@ -14,6 +15,7 @@ $c: scale-color(blue, $lightness: -10%);
 $a: color.mix(red, blue);
 $b: string.length("hello");
 $c: color.scale(blue, $lightness: -10%);
+$d: meta.call(meta.get-function(mix, $module: "color"), red, blue);
 
 @function fn($args...) {
   @return meta.keywords($args);