From c576719b5cd61069c0cb98dd009f59e2018b8a46 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 | 4 ++++ lib/src/migrators/module.dart | 22 +++++++++++++++++++ pubspec.yaml | 2 +- .../namespace_references/loop_error.hrx | 21 ++++++++++++++++++ 4 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 test/migrators/module/namespace_references/loop_error.hrx diff --git a/CHANGELOG.md b/CHANGELOG.md index 69d943e0..9b8ae0f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## 1.8.2 + +* Detect dependency loops in module migrator fix. + ## 1.8.1 ### Calc Functions Interpolation Migrator diff --git a/lib/src/migrators/module.dart b/lib/src/migrators/module.dart index 76d15131..b2f2d546 100644 --- a/lib/src/migrators/module.dart +++ b/lib/src/migrators/module.dart @@ -200,6 +200,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]. @@ -1239,6 +1254,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 512ba702..ea71eca9 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: sass_migrator -version: 1.8.1 +version: 1.8.2 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!