Detect dependency loops in module migrator

Author: pamelalozano16

CHANGELOG.md

@@ -1,3 +1,6 @@
+## 2.0.4
+* Detect dependency loops in module migrator fix.
+
 ## 2.0.3
 
 ### Module Migrator

lib/src/migrators/module.dart

@@ -15,6 +15,7 @@ import '../migration_visitor.dart';
 import '../migrator.dart';
 import '../patch.dart';
 import '../utils.dart';
+import '../util/dependency_graph.dart';
 import '../util/member_declaration.dart';
 import '../util/node_modules_importer.dart';
 
@@ -199,6 +200,33 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
   /// The values of the --forward flag.
   final Set<ForwardType> forwards;
 
+  /// Dependencies where keys represent source URIs and values represent imported URIs.
+  final DependencyGraph _dependencies = DependencyGraph();
+
+  /// Checks for dependency loops between source and imported paths.
+  ///
+  /// This method verifies whether importing a path introduces a circular dependency
+  /// by checking if the imported path is already mapped as a dependency of the source path.
+  ///
+  /// Throws a [MigrationException] if a dependency loop is detected.
+  ///
+  /// The [source] parameter is the path where the dependency is checked.
+  /// The [importedPath] parameter is the path being imported.
+  void _checkDependency(Uri source, Uri importedPath, FileSpan span) {
+    if (_dependencies.hasDependency(importedPath, source)) {
+      // Throw an error indicating a potential loop.
+      var (sourceUrl, _) = _absoluteUrlToDependency(source);
+      var (importedPathUrl, _) = _absoluteUrlToDependency(importedPath);
+      throw MigrationSourceSpanException(
+          'Dependency loop detected: ${sourceUrl} -> ${importedPathUrl}.\n'
+          'To resolve this issue, consider either of the following:\n'
+          '1. Remove the import statement that causes the dependency loop.\n'
+          '2. Declare the variables used in the other stylesheet within the same stylesheet.\n',
+          span);
+    }
+    _dependencies.add(source, importedPath);
+  }
+
   /// Constructs a new module migration visitor.
   ///
   /// [importCache] must be the same one used by [references].
@@ -848,6 +876,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
         withClause.isNotEmpty ||
         references.anyMemberReferenced(canonicalUrl, currentUrl)) {
       _usedUrls.add(canonicalUrl);
+      _checkDependency(currentUrl, canonicalUrl, context);
       rules.add('@use $quotedUrl$asClause$withClause');
     }
     if (normalForwardRules != null) rules.addAll(normalForwardRules);
@@ -1223,6 +1252,8 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
   String? _namespaceForDeclaration(MemberDeclaration declaration) {
     var url = declaration.sourceUrl;
     if (url == currentUrl) return null;
+    // Trace dependencies for loop detection.
+    _checkDependency(currentUrl, url, declaration.member.span);
 
     // If we can load [declaration] from a library entrypoint URL, do so. Choose
     // the shortest one if there are multiple options.

lib/src/util/dependency_graph.dart

@@ -0,0 +1,33 @@
+// Copyright 2023 Google LLC
+//
+// Use of this source code is governed by an MIT-style
+// license that can be found in the LICENSE file or at
+// https://opensource.org/licenses/MIT.
+
+/// A graph data structure to manage dependencies between URIs.
+///
+/// This class allows adding, removing, and querying dependencies
+/// between source URIs and their imported paths.
+class DependencyGraph {
+  final Map<Uri, Set<Uri>> _graph = {};
+
+  /// Adds a dependency relationship between source and importedPath.
+  void add(Uri source, Uri importedPath) {
+    _graph.putIfAbsent(source, () => {}).add(importedPath);
+  }
+
+  /// Removes a dependency relationship between source and importedPath.
+  void remove(Uri source, Uri importedPath) {
+    _graph[source]?.remove(importedPath);
+  }
+
+  /// Finds all dependencies of a given source.
+  Set<Uri>? find(Uri source) {
+    return _graph[source];
+  }
+
+  /// Checks if a specific dependency exists.
+  bool hasDependency(Uri source, Uri importedPath) {
+    return _graph.containsKey(source) && _graph[source]!.contains(importedPath);
+  }
+}

pubspec.yaml

@@ -1,5 +1,5 @@
 name: sass_migrator
-version: 2.0.3
+version: 2.0.4
 description: A tool for running migrations on Sass files
 homepage: https://github.com/sass/migrator
 

test/migrators/module/namespace_references/loop_error.hrx

@@ -0,0 +1,28 @@
+<==> arguments
+--migrate-deps
+
+<==> input/entrypoint.scss
+$a: red;
+@import "ejemplo";
+a {
+  background: $b;
+}
+
+<==> input/_ejemplo.scss
+$b: blue;
+a {
+  color: $a;
+}
+
+<==> error.txt
+Error: Dependency loop detected: entrypoint -> ejemplo.
+To resolve this issue, consider either of the following:
+1. Remove the import statement that causes the dependency loop.
+2. Declare the variables used in the other stylesheet within the same stylesheet.
+
+  ,
+2 | @import "ejemplo";
+  |         ^^^^^^^^^
+  '
+  entrypoint.scss 2:9  root stylesheet
+Migration failed!

test/migrators/module/namespace_references/loop_error_one_reference.hrx

@@ -0,0 +1,24 @@
+<==> arguments
+--migrate-deps
+
+<==> input/entrypoint.scss
+$var1: 2;
+@import "library";
+
+<==> input/_library.scss
+a {
+  margin: $var1;
+}
+
+<==> error.txt
+Error: Dependency loop detected: entrypoint -> library.
+To resolve this issue, consider either of the following:
+1. Remove the import statement that causes the dependency loop.
+2. Declare the variables used in the other stylesheet within the same stylesheet.
+
+  ,
+2 | @import "library";
+  |         ^^^^^^^^^
+  '
+  entrypoint.scss 2:9  root stylesheet
+Migration failed!