Skip to content

Commit

Permalink
Detect dependency loops in module migrator
Browse files Browse the repository at this point in the history
  • Loading branch information
pamelalozano16 committed Jun 18, 2024
1 parent dce67db commit 76e48bd
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 1 deletion.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
## 2.0.4
* Detect dependency loops in module migrator fix.

## 2.0.3

### Module Migrator
Expand Down
27 changes: 27 additions & 0 deletions lib/src/migrators/module.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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].
Expand Down Expand Up @@ -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];
Expand Down
33 changes: 33 additions & 0 deletions lib/src/util/dependency_graph.dart
Original file line number Diff line number Diff line change
@@ -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);
}
}
2 changes: 1 addition & 1 deletion pubspec.yaml
Original file line number Diff line number Diff line change
@@ -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

Expand Down
24 changes: 24 additions & 0 deletions test/migrators/module/namespace_references/loop_error.hrx
Original file line number Diff line number Diff line change
@@ -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!

0 comments on commit 76e48bd

Please sign in to comment.