From b3a68360a18b166bdc97d6a6a454cdd7a8191525 Mon Sep 17 00:00:00 2001 From: Pamela Lozano 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 | 22 +++++++++++++++++++ pubspec.yaml | 2 +- .../namespace_references/loop_error.hrx | 21 ++++++++++++++++++ 4 files changed, 47 insertions(+), 1 deletion(-) 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..930f3f92 100644 --- a/lib/src/migrators/module.dart +++ b/lib/src/migrators/module.dart @@ -199,6 +199,21 @@ class _ModuleMigrationVisitor extends MigrationVisitor { /// The values of the --forward flag. final Set forwards; + // Maps direct and indirect dependencies to prevent any potential loops. + final Map _dependencies = {}; + + void _addDependency(Uri source, Uri importedPath) { + if (_dependencies.containsKey(importedPath) && + _dependencies[importedPath] == source) { + // Throw an error indicating a potential loop. + var sourceUrl = _absoluteUrlToDependency(source); + var importedPathUrl = _absoluteUrlToDependency(importedPath); + throw MigrationException( + 'Dependency loop detected: ${sourceUrl.item1} -> ${importedPathUrl.item1}'); + } + _dependencies[source] = importedPath; + } + /// Constructs a new module migration visitor. /// /// [importCache] must be the same one used by [references]. @@ -1224,6 +1239,13 @@ class _ModuleMigrationVisitor extends MigrationVisitor { var url = declaration.sourceUrl; if (url == currentUrl) return null; + // Trace dependencies for loop detection. + try { + _addDependency(currentUrl, url); + } on Exception catch (e) { + throw MigrationSourceSpanException(e.toString(), 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/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..5895d3ff --- /dev/null +++ b/test/migrators/module/namespace_references/loop_error.hrx @@ -0,0 +1,21 @@ +<==> arguments +--migrate-deps + +<==> input/entrypoint.scss +@import "ejemplo"; +$var: $value; + +<==> input/_ejemplo.scss +$value: blue; +a { + color: $var; +} + +<==> error.txt +Error: Error: Dependency loop detected: entrypoint -> ejemplo + , +1 | $value: blue; + | ^^^^^^^^^^^^ + ' + _ejemplo.scss 1:1 root stylesheet +Migration failed!