From 76e48bdaca8c4e8d943f325db9794ce44fb90fdb Mon Sep 17 00:00:00 2001
From: Pamela Lozano <pamelalozano16@gmail.com>
Date: Mon, 11 Dec 2023 16:15:35 -0800
Subject: [PATCH] Detect dependency loops in module migrator

---
 CHANGELOG.md                                  |  3 ++
 lib/src/migrators/module.dart                 | 27 +++++++++++++++
 lib/src/util/dependency_graph.dart            | 33 +++++++++++++++++++
 pubspec.yaml                                  |  2 +-
 .../namespace_references/loop_error.hrx       | 24 ++++++++++++++
 5 files changed, 88 insertions(+), 1 deletion(-)
 create mode 100644 lib/src/util/dependency_graph.dart
 create mode 100644 test/migrators/module/namespace_references/loop_error.hrx

diff --git a/CHANGELOG.md b/CHANGELOG.md
index f2a876be..27bd1429 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,6 @@
+## 2.0.4
+* Detect dependency loops in module migrator fix.
+
 ## 2.0.3
 
 ### Module Migrator
diff --git a/lib/src/migrators/module.dart b/lib/src/migrators/module.dart
index e1afbf80..65801985 100644
--- a/lib/src/migrators/module.dart
+++ b/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,29 @@ 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}', span);
+    }
+    _dependencies.add(source, importedPath);
+  }
+
   /// Constructs a new module migration visitor.
   ///
   /// [importCache] must be the same one used by [references].
@@ -1224,6 +1248,9 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
     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.
     var libraryUrls = references.libraries[declaration];
diff --git a/lib/src/util/dependency_graph.dart b/lib/src/util/dependency_graph.dart
new file mode 100644
index 00000000..2541e314
--- /dev/null
+++ b/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);
+  }
+}
\ No newline at end of file
diff --git a/pubspec.yaml b/pubspec.yaml
index df61c114..0b6ecd3e 100644
--- a/pubspec.yaml
+++ b/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
 
diff --git a/test/migrators/module/namespace_references/loop_error.hrx b/test/migrators/module/namespace_references/loop_error.hrx
new file mode 100644
index 00000000..4cbd4a3d
--- /dev/null
+++ b/test/migrators/module/namespace_references/loop_error.hrx
@@ -0,0 +1,24 @@
+<==> 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
+  ,
+1 | $b: blue;
+  | ^^^^^^^^
+  '
+  _ejemplo.scss 1:1  root stylesheet
+Migration failed!