Support more complex forwards in import-only files

Fixes #129.

The migrator should now correctly identify forwards that are missing
when switching from an import-only file to the regular file.

This will allow libraries to require stricter dependencies for their
users when those users migrate to the module system, without breaking
`@import` users that depend on implicit dependencies.

lib/src/migrators/module.dart

@@ -9,6 +9,7 @@
 // https://github.com/sass/dart-sass/issues/236.
 import 'package:sass/src/ast/sass.dart';
 import 'package:sass/src/importer.dart';
+import 'package:sass/src/importer/utils.dart';
 import 'package:sass/src/import_cache.dart';
 
 import 'package:args/args.dart';
@@ -451,7 +452,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
       // namespace.
       var paths = {
         for (var source in sources)
-          source: Uri.parse(source.import.url).pathSegments.toList()
+          source: Uri.parse(source.ruleUrl).pathSegments.toList()
             ..removeLast()
             ..removeWhere((segment) => segment.contains('.'))
       };
@@ -507,12 +508,12 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
     return namespace;
   }
 
-  /// Given a set of import sources, groups them by the number of path segments
-  /// and sorts those groups from fewer to more segments.
+  /// Given a set of import-like sources, groups them by the number of path
+  /// segments and sorts those groups from fewer to more segments.
   List<Set<ImportSource>> _orderSources(Iterable<ImportSource> sources) {
     var byPathLength = <int, Set<ImportSource>>{};
     for (var source in sources) {
-      var pathSegments = Uri.parse(source.import.url).pathSegments;
+      var pathSegments = Uri.parse(source.ruleUrl).pathSegments;
       byPathLength.putIfAbsent(pathSegments.length, () => {}).add(source);
     }
     return [
@@ -746,7 +747,9 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
 
     // Associate the importer for this URL with the resolved URL so that we can
     // re-use this import URL later on.
-    var tuple = importCache.canonicalize(parsedUrl, importer, currentUrl);
+
+    var tuple = inUseRule(
+        () => importCache.canonicalize(parsedUrl, importer, currentUrl));
     var resolvedUrl = tuple.item2;
     _originalImports.putIfAbsent(
         resolvedUrl, () => Tuple2(import.url, tuple.item1));
@@ -1071,7 +1074,6 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
     if (libraryUrls != null) {
       url = minBy(libraryUrls, (url) => url.pathSegments.length);
     }
-
     if (!_usedUrls.contains(url)) {
       // Add new `@use` rule for indirect dependency
       var tuple = _absoluteUrlToDependency(url);

lib/src/migrators/module/member_declaration.dart

@@ -9,8 +9,6 @@
 // https://github.com/sass/dart-sass/issues/236.
 import 'package:sass/src/ast/sass.dart';
 
-import '../../utils.dart';
-
 /// A wrapper class for nodes that declare a variable, function, or mixin.
 ///
 /// The member this class wraps will always be a [VariableDeclaration],
@@ -32,14 +30,20 @@ class MemberDeclaration<T extends SassNode> {
   /// The URL this member came from.
   ///
   /// For un-forwarded members, this is the URL the member was declared in.
-  /// For forwarded members, this is the URL of the `@forward` rule.
+  /// For members forwarded through an import-only file, this is the [sourceUrl]
+  /// of the member prior to that forward.
+  /// For other forwarded members, this is the URL of the `@forward` rule.
   final Uri sourceUrl;
 
   /// The canonical URL forwarded by [forward].
   ///
   /// This is `null` when [forward] is.
   final Uri forwardedUrl;
 
+  /// True if this declaration is the result of a `@forward` rule within an
+  /// import-only stylesheet.
+  final bool isImportOnly;
+
   /// Constructs a MemberDefinition for [member], which must be a
   /// [VariableDeclaration], [Argument], [MixinRule], or [FunctionRule].
   MemberDeclaration(this.member)
@@ -54,21 +58,19 @@ class MemberDeclaration<T extends SassNode> {
         })(),
         sourceUrl = member.span.sourceUrl,
         forward = null,
-        forwardedUrl = null;
+        forwardedUrl = null,
+        isImportOnly = false;
 
   /// Constructs a forwarded MemberDefinition of [forwarding] based on
   /// [forward].
   MemberDeclaration.forward(
       MemberDeclaration forwarding, this.forward, this.forwardedUrl)
       : member = forwarding.member,
         name = '${forward.prefix ?? ""}${forwarding.name}',
-        sourceUrl = forward.span.sourceUrl;
-
-  /// Returns true if this declaration is the result of a `@forward` rule within
-  /// an import-only stylesheet.
-  bool get isImportOnly =>
-      forward != null &&
-      forward.span.sourceUrl == getImportOnlyUrl(forwardedUrl);
+        isImportOnly = _isImportOnly(forward),
+        sourceUrl = _isImportOnly(forward)
+            ? forwarding.sourceUrl
+            : forward.span.sourceUrl;
 
   operator ==(other) =>
       other is MemberDeclaration &&
@@ -77,3 +79,9 @@ class MemberDeclaration<T extends SassNode> {
 
   int get hashCode => member.hashCode;
 }
+
+/// Returns true if [forward] is non-null and inside an import-only file.
+bool _isImportOnly(ForwardRule forward) =>
+    forward != null &&
+    (forward.span.sourceUrl.path.endsWith('.import.scss') ||
+        forward.span.sourceUrl.path.endsWith('.import.sass'));

lib/src/migrators/module/reference_source.dart

@@ -20,11 +20,17 @@ abstract class ReferenceSource {
   String get defaultNamespace;
 }
 
-/// A source for references to members loaded by an `@import` rule.
+/// A source for references that should be migrated like an `@import` rule.
+///
+/// This includes both sources that directly come from an `@import` rule and
+/// those from a `@forward` rule within an import-only file.
 class ImportSource extends ReferenceSource {
   final Uri url;
 
-  /// The import that loaded the member being referenced.
+  /// The URL within the `@import` or `@forward` rule that loaded this member.
+  String ruleUrl;
+
+  /// Creates an [ImportSource] for [url] from [import].
   ///
   /// Note: This is the import that directly loaded the stylesheet defining
   /// the referenced member, not the immediate import within the referencing
@@ -33,15 +39,18 @@ class ImportSource extends ReferenceSource {
   /// For example, if A imports B and B imports C, and a member of C is
   /// referenced in A, than that reference's source should be the import in B
   /// that imports C, not the import in A that imports B.
-  final DynamicImport import;
+  ImportSource(this.url, DynamicImport import) : ruleUrl = import.url;
 
-  ImportSource(this.url, this.import);
+  /// Creates an [ImportSource] from an [ImportOnlySource].
+  ImportSource.fromImportOnlyForward(ImportOnlySource source)
+      : url = source.realSourceUrl,
+        ruleUrl = source.ruleUrl;
 
-  String get defaultNamespace => namespaceForPath(import.url);
+  String get defaultNamespace => namespaceForPath(ruleUrl);
 
   operator ==(other) =>
-      other is ImportSource && url == other.url && import == other.import;
-  int get hashCode => import.hashCode;
+      other is ImportSource && url == other.url && ruleUrl == other.ruleUrl;
+  int get hashCode => url.hashCode;
 }
 
 /// A source for references to members loaded by a `@use` rule.
@@ -101,6 +110,9 @@ class CurrentSource extends ReferenceSource {
 /// since forwarded members cannot be referenced in the stylesheet that forwards
 /// them, and this will be replaced by an [ImportSource] or [UseSource] in any
 /// context where a forwarded member can be referenced.
+///
+/// Members forwarded from an import-only file should use [ImportOnlySource]
+/// instead of this.
 class ForwardSource extends ReferenceSource {
   final Uri url;
 
@@ -115,3 +127,29 @@ class ForwardSource extends ReferenceSource {
       other is ForwardSource && url == other.url && forward == other.forward;
   int get hashCode => forward.hashCode;
 }
+
+/// A source for members forwarded through an import-only file.
+///
+/// This source is similar to [ForwardSource] in that it is only used by
+/// [_ReferenceVisitor] to track sources internally, and should not be present
+/// in the final [sources] property of [References].
+class ImportOnlySource extends ReferenceSource {
+  final Uri url;
+
+  final Uri realSourceUrl;
+
+  final String ruleUrl;
+
+  ImportOnlySource(this.realSourceUrl, ForwardRule forward)
+      : url = forward.span.sourceUrl,
+        ruleUrl = forward.url.toString();
+
+  String get defaultNamespace => null;
+
+  operator ==(other) =>
+      other is ImportOnlySource &&
+      url == other.url &&
+      realSourceUrl == other.realSourceUrl &&
+      ruleUrl == other.ruleUrl;
+  int get hashCode => realSourceUrl.hashCode;
+}

lib/src/migrators/module/references.dart

@@ -346,9 +346,13 @@ class _ReferenceVisitor extends RecursiveAstVisitor {
         var importSource = ImportSource(url, import);
         for (var declaration in _declarationSources.keys.toList()) {
           var source = _declarationSources[declaration];
-          if (source.url == url &&
-              (source is CurrentSource || source is ForwardSource)) {
-            _declarationSources[declaration] = importSource;
+          if (source.url == url) {
+            if (source is CurrentSource || source is ForwardSource) {
+              _declarationSources[declaration] = importSource;
+            } else if (source is ImportOnlySource) {
+              _declarationSources[declaration] =
+                  ImportSource.fromImportOnlyForward(source);
+            }
           }
           _importer = oldImporter;
         }
@@ -468,8 +472,15 @@ class _ReferenceVisitor extends RecursiveAstVisitor {
     _registerLibraryUrl(declaration);
     var prefix = forward.prefix ?? '';
     declarations['$prefix${forwarding.name}'] = declaration;
-    _declarationSources[declaration] =
-        ForwardSource(forward.span.sourceUrl, forward);
+
+    if (forward.span.sourceUrl.path.endsWith('.import.scss') ||
+        forward.span.sourceUrl.path.endsWith('.import.sass')) {
+      _declarationSources[declaration] =
+          ImportOnlySource(forwardedUrl, forward);
+    } else {
+      _declarationSources[declaration] =
+          ForwardSource(forward.span.sourceUrl, forward);
+    }
   }
 
   /// Visits each of [node]'s expressions and children.

test/migrators/module/partial_migration/implicit_dependency.hrx

@@ -0,0 +1,31 @@
+<==> arguments
+--migrate-deps
+
+<==> input/entrypoint.scss
+@import "direct";
+
+a {
+  b: $direct;
+  c: $indirect;
+}
+
+<==> input/_direct.scss
+@use "indirect";
+
+$direct: indirect.$indirect + 1;
+
+<==> input/_direct.import.scss
+@forward "direct";
+@forward "indirect";
+
+<==> input/_indirect.scss
+$indirect: 2;
+
+<==> output/entrypoint.scss
+@use "direct";
+@use "indirect";
+
+a {
+  b: direct.$direct;
+  c: indirect.$indirect;
+}