Add --built-in-only flag to module migrator (#259)

Also fixes some built-in functions that weren't being migrated.

Author: jathak

CHANGELOG.md

@@ -1,3 +1,13 @@
+## 2.2.0
+
+### Module Migrator
+
+* 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
+  `selector-extend`) would not be migrated.
+
 ## 2.1.0
 
 ### Color Function Migrator

lib/src/migration_visitor.dart

@@ -66,7 +66,7 @@ abstract class MigrationVisitor extends ScopedAstVisitor {
   /// syntax, in which case this returns an empty string.
   String get semicolon => currentUrl.path.endsWith('.sass') ? "" : ";";
 
-  MigrationVisitor(this.importCache, this.migrateDependencies);
+  MigrationVisitor(this.importCache, {required this.migrateDependencies});
 
   /// Runs a new migration on [stylesheet] (and its dependencies, if
   /// [migrateDependencies] is true) and returns a map of migrated contents.

lib/src/migrators/calc_interpolation.dart

@@ -19,18 +19,17 @@ class CalculationInterpolationMigrator extends Migrator {
   @override
   Map<Uri, String> migrateFile(
       ImportCache importCache, Stylesheet stylesheet, Importer importer) {
-    var visitor =
-        _CalculationInterpolationVisitor(importCache, migrateDependencies);
+    var visitor = _CalculationInterpolationVisitor(importCache,
+        migrateDependencies: migrateDependencies);
     var result = visitor.run(stylesheet, importer);
     missingDependencies.addAll(visitor.missingDependencies);
     return result;
   }
 }
 
 class _CalculationInterpolationVisitor extends MigrationVisitor {
-  _CalculationInterpolationVisitor(
-      ImportCache importCache, bool migrateDependencies)
-      : super(importCache, migrateDependencies);
+  _CalculationInterpolationVisitor(super.importCache,
+      {required super.migrateDependencies});
 
   @override
   void visitFunctionExpression(FunctionExpression node) {

lib/src/migrators/color.dart

@@ -23,8 +23,8 @@ class ColorMigrator extends Migrator {
   Map<Uri, String> migrateFile(
       ImportCache importCache, Stylesheet stylesheet, Importer importer) {
     var references = References(importCache, stylesheet, importer);
-    var visitor =
-        _ColorMigrationVisitor(references, importCache, migrateDependencies);
+    var visitor = _ColorMigrationVisitor(references, importCache,
+        migrateDependencies: migrateDependencies);
     var result = visitor.run(stylesheet, importer);
     missingDependencies.addAll(visitor.missingDependencies);
     return result;
@@ -37,8 +37,8 @@ final _colorUrl = Uri(scheme: 'sass', path: 'color');
 class _ColorMigrationVisitor extends MigrationVisitor {
   final References references;
 
-  _ColorMigrationVisitor(
-      this.references, super.importCache, super.migrateDependencies);
+  _ColorMigrationVisitor(this.references, super.importCache,
+      {required super.migrateDependencies});
 
   /// The namespace of an existing `@use "sass:color"` rule in the current
   /// file, if any.

lib/src/migrators/division.dart

@@ -66,7 +66,8 @@ More info: https://sass-lang.com/d/slash-div""";
   Map<Uri, String> migrateFile(
       ImportCache importCache, Stylesheet stylesheet, Importer importer) {
     var visitor = _DivisionMigrationVisitor(
-        importCache, isPessimistic, useMultiplication, migrateDependencies);
+        importCache, isPessimistic, useMultiplication,
+        migrateDependencies: migrateDependencies);
     var result = visitor.run(stylesheet, importer);
     missingDependencies.addAll(visitor.missingDependencies);
     return result;
@@ -80,9 +81,9 @@ class _DivisionMigrationVisitor extends MigrationVisitor {
   final bool isPessimistic;
   final bool useMultiplication;
 
-  _DivisionMigrationVisitor(ImportCache importCache, this.isPessimistic,
-      this.useMultiplication, bool migrateDependencies)
-      : super(importCache, migrateDependencies);
+  _DivisionMigrationVisitor(
+      super.importCache, this.isPessimistic, this.useMultiplication,
+      {required super.migrateDependencies});
 
   /// True when division is allowed by the context the current node is in.
   var _isDivisionAllowed = false;

lib/src/migrators/module.dart

@@ -32,6 +32,8 @@ class ModuleMigrator extends Migrator {
 
   @override
   final argParser = ArgParser()
+    ..addFlag('built-in-only',
+        help: 'Migrates global functions without migrating @import.')
     ..addMultiOption('remove-prefix',
         abbr: 'p',
         help: 'Removes PREFIX from all migrated member names.\n'
@@ -77,6 +79,13 @@ 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!['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 --built-in-only.');
+    }
     if (forwards.contains(ForwardType.prefixed) &&
         !argResults!.wasParsed('remove-prefix')) {
       throw MigrationException(
@@ -85,8 +94,10 @@ class ModuleMigrator extends Migrator {
     }
 
     var references = References(importCache, stylesheet, importer);
-    var visitor = _ModuleMigrationVisitor(importCache, references,
-        globalResults!['load-path'] as List<String>, migrateDependencies,
+    var visitor = _ModuleMigrationVisitor(
+        importCache, references, globalResults!['load-path'] as List<String>,
+        migrateDependencies: migrateDependencies,
+        builtInOnly: builtInOnly,
         prefixesToRemove: (argResults!['remove-prefix'] as List<String>)
             .map((prefix) => prefix.replaceAll('_', '-')),
         forwards: forwards);
@@ -186,9 +197,6 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
   /// main migration pass is used.
   final References references;
 
-  /// Cache used to load stylesheets.
-  final ImportCache importCache;
-
   /// List of paths that stylesheets can be loaded from.
   final List<String> loadPaths;
 
@@ -199,6 +207,9 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
   /// The values of the --forward flag.
   final Set<ForwardType> forwards;
 
+  /// Whether to migrate only global functions, leaving `@import` rules as-is.
+  final bool builtInOnly;
+
   /// Constructs a new module migration visitor.
   ///
   /// [importCache] must be the same one used by [references].
@@ -210,22 +221,25 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
   /// the module migrator will filter out the dependencies' migration results.
   ///
   /// This converts the OS-specific relative [loadPaths] to absolute URL paths.
-  _ModuleMigrationVisitor(this.importCache, this.references,
-      List<String> loadPaths, bool migrateDependencies,
-      {Iterable<String> prefixesToRemove = const [], this.forwards = const {}})
+  _ModuleMigrationVisitor(
+      super.importCache, this.references, List<String> loadPaths,
+      {required super.migrateDependencies,
+      required this.builtInOnly,
+      Iterable<String> prefixesToRemove = const [],
+      this.forwards = const {}})
       : loadPaths = List.unmodifiable(
             loadPaths.map((path) => p.toUri(p.absolute(path)).path)),
-        prefixesToRemove = UnmodifiableSetView(prefixesToRemove.toSet()),
-        super(importCache, migrateDependencies);
+        prefixesToRemove = UnmodifiableSetView(prefixesToRemove.toSet());
 
   /// Checks which global declarations need to be renamed, then runs the
   /// migrator.
   @override
   Map<Uri, String> run(Stylesheet stylesheet, Importer importer) {
-    references.globalDeclarations.forEach(_renameDeclaration);
+    if (!builtInOnly) references.globalDeclarations.forEach(_renameDeclaration);
     var migrated = super.run(stylesheet, importer);
 
-    if (forwards.contains(ForwardType.importOnly) || _needsImportOnly) {
+    if (!builtInOnly &&
+        (forwards.contains(ForwardType.importOnly) || _needsImportOnly)) {
       var url = stylesheet.span.sourceUrl!;
       var importOnlyUrl = getImportOnlyUrl(url);
       var results = _generateImportOnly(url, importOnlyUrl);
@@ -597,7 +611,10 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
   /// Adds a namespace to any function call that requires it.
   @override
   void visitFunctionExpression(FunctionExpression node) {
-    if (node.namespace != null) {
+    var source = references.sources[node];
+    if (node.namespace != null ||
+        source is UseSource ||
+        builtInOnly && source is! BuiltInSource) {
       super.visitFunctionExpression(node);
       return;
     }
@@ -631,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),
@@ -776,14 +796,21 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
         }
       }
       if (ruleUrl != null) {
-        if (_useAllowed) {
+        if (builtInOnly) {
+          if (migrateDependencies) {
+            _upstreamStylesheets.add(currentUrl);
+            visitDependency(ruleUrl, import.span);
+            _upstreamStylesheets.remove(currentUrl);
+          }
+        } else if (_useAllowed) {
           migratedRules.addAll(_migrateImportToRules(ruleUrl, import.span));
         } else {
           migratedRules.add(_migrateImportToLoadCss(ruleUrl, import.span)
               .replaceAll('\n', '\n$indent'));
         }
       }
     }
+    if (builtInOnly) return;
 
     rulesText = migratedRules.join('$semicolon\n$indent');
     if (rulesText.isEmpty) {
@@ -1071,6 +1098,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
     _useAllowed = false;
     super.visitIncludeRule(node);
     if (node.namespace != null) return;
+    if (builtInOnly && references.sources[node] is! BuiltInSource) return;
 
     var declaration = references.mixins[node];
     if (declaration == null) {
@@ -1089,7 +1117,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
   @override
   void visitMixinRule(MixinRule node) {
     _useAllowed = false;
-    _renameReference(nameSpan(node), MemberDeclaration(node));
+    if (!builtInOnly) _renameReference(nameSpan(node), MemberDeclaration(node));
     super.visitMixinRule(node);
   }
 
@@ -1119,6 +1147,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
   @override
   void visitVariableExpression(VariableExpression node) {
     if (node.namespace != null) return;
+    if (builtInOnly && references.sources[node] is! BuiltInSource) return;
     var declaration = references.variables[node];
     if (declaration == null) {
       // TODO(jathak): Error here as part of fixing #182.
@@ -1145,6 +1174,10 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
   /// renaming or namespacing if necessary.
   @override
   void visitVariableDeclaration(VariableDeclaration node) {
+    if (builtInOnly) {
+      super.visitVariableDeclaration(node);
+      return;
+    }
     var declaration = MemberDeclaration(node);
     var defaultDeclaration =
         references.defaultVariableDeclarations[declaration];

lib/src/migrators/module/built_in_functions.dart

@@ -22,6 +22,7 @@ const builtInFunctionModules = {
   "complement": "color",
   "invert": "color",
   "alpha": "color",
+  "opacity": "color",
   "opacify": "color",
   "fade-in": "color",
   "transparentize": "color",
@@ -43,6 +44,7 @@ const builtInFunctionModules = {
   "is-superselector": "selector",
   "simple-selectors": "selector",
   "selector-parse": "selector",
+  "selector-extend": "selector",
   "percentage": "math",
   "round": "math",
   "ceil": "math",
@@ -62,6 +64,7 @@ const builtInFunctionModules = {
   "zip": "list",
   "index": "list",
   "list-separator": "list",
+  "is-bracketed": "list",
   "feature-exists": "meta",
   "variable-exists": "meta",
   "global-variable-exists": "meta",
@@ -100,6 +103,7 @@ const builtInFunctionNameChanges = {
   "selector-replace": "replace",
   "selector-unify": "unify",
   "selector-parse": "parse",
+  "selector-extend": "extend",
   "unitless": "is-unitless",
   "comparable": "compatible",
   "list-separator": "separator",

lib/src/migrators/module/references.dart

@@ -383,13 +383,19 @@ class _ReferenceVisitor extends ScopedAstVisitor {
   void visitUseRule(UseRule node) {
     super.visitUseRule(node);
     var namespace = node.namespace;
-    if (namespace == null) return;
     if (node.url.scheme == 'sass') {
-      _namespaces[namespace] = node.url;
+      if (namespace != null) _namespaces[namespace] = node.url;
       return;
     }
     var canonicalUrl = _loadUseOrForward(node.url, node);
-    _namespaces[namespace] = canonicalUrl;
+    if (namespace != null) {
+      _namespaces[namespace] = canonicalUrl;
+    } else {
+      var moduleScope = _moduleScopes[canonicalUrl]!;
+      currentScope.variables.addAll(moduleScope.variables);
+      currentScope.mixins.addAll(moduleScope.mixins);
+      currentScope.functions.addAll(moduleScope.functions);
+    }
 
     // `_moduleSources[canonicalUrl]` is set in `_loadUseOrForward`.
     var moduleSources = _moduleSources[canonicalUrl]!;

lib/src/migrators/namespace.dart

@@ -38,8 +38,9 @@ class NamespaceMigrator extends Migrator {
     var renamer = Renamer<UseRule>(argResults!['rename'].join('\n'),
         {'': ((rule) => rule.namespace!), 'url': (rule) => rule.url.toString()},
         sourceUrl: '--rename');
-    var visitor = _NamespaceMigrationVisitor(renamer,
-        argResults!['force'] as bool, importCache, migrateDependencies);
+    var visitor = _NamespaceMigrationVisitor(
+        renamer, argResults!['force'] as bool, importCache,
+        migrateDependencies: migrateDependencies);
     var result = visitor.run(stylesheet, importer);
     missingDependencies.addAll(visitor.missingDependencies);
     return result;
@@ -62,9 +63,8 @@ class _NamespaceMigrationVisitor extends MigrationVisitor {
       assertInStylesheet(__usedNamespaces, '_usedNamespaces');
   Set<String>? __usedNamespaces;
 
-  _NamespaceMigrationVisitor(this.renamer, this.forceRename,
-      ImportCache importCache, bool migrateDependencies)
-      : super(importCache, migrateDependencies);
+  _NamespaceMigrationVisitor(this.renamer, this.forceRename, super.importCache,
+      {required super.migrateDependencies});
 
   @override
   void visitStylesheet(Stylesheet node) {

lib/src/migrators/strict_unary.dart

@@ -19,16 +19,17 @@ class StrictUnaryMigrator extends Migrator {
   @override
   Map<Uri, String> migrateFile(
       ImportCache importCache, Stylesheet stylesheet, Importer importer) {
-    var visitor = _UnaryMigrationVisitor(importCache, migrateDependencies);
+    var visitor = _UnaryMigrationVisitor(importCache,
+        migrateDependencies: migrateDependencies);
     var result = visitor.run(stylesheet, importer);
     missingDependencies.addAll(visitor.missingDependencies);
     return result;
   }
 }
 
 class _UnaryMigrationVisitor extends MigrationVisitor {
-  _UnaryMigrationVisitor(ImportCache importCache, bool migrateDependencies)
-      : super(importCache, migrateDependencies);
+  _UnaryMigrationVisitor(super.importCache,
+      {required super.migrateDependencies});
 
   @override
   void visitBinaryOperationExpression(BinaryOperationExpression node) {