Skip to content

Commit

Permalink
✨ - Show the path of the depency cycle
Browse files Browse the repository at this point in the history
  • Loading branch information
jfrolich committed Jul 28, 2023
1 parent 4ca5ed8 commit 44b5122
Show file tree
Hide file tree
Showing 11 changed files with 125 additions and 14 deletions.
92 changes: 85 additions & 7 deletions src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1155,6 +1155,74 @@ fn compute_file_hash(path: &str) -> Option<blake3::Hash> {
}
}

fn find_dependency_cycle(modules: &Vec<(&String, &Module)>) -> Vec<String> {
let mut visited: AHashSet<String> = AHashSet::new();
let mut stack: Vec<String> = vec![];

// we want to sort the module names so that we always return the same
// dependency cycle (there can be more than one)
let mut module_names = modules
.iter()
.map(|(name, _)| name.to_string())
.collect::<Vec<String>>();

module_names.sort_by(|a, b| a.cmp(b));
for module_name in module_names {
if find_dependency_cycle_helper(&module_name, &modules, &mut visited, &mut stack) {
return stack;
}
visited.clear();
stack.clear();
}
stack
}

fn find_dependency_cycle_helper(
module_name: &String,
modules: &Vec<(&String, &Module)>,
visited: &mut AHashSet<String>,
stack: &mut Vec<String>,
) -> bool {
if let Some(module) = modules
.iter()
.find(|(name, _)| *name == module_name)
.map(|(_, module)| module)
{
visited.insert(module_name.to_string());
// if the module is a mlmap (namespace), we don't want to show this in the path
// because the namespace is not a module the user created, so only add source files
// to the stack
if let SourceType::SourceFile(_) = module.source_type {
stack.push(module_name.to_string())
}
for dep in &module.deps {
if !visited.contains(dep) {
if find_dependency_cycle_helper(dep, modules, visited, stack) {
return true;
}
} else if stack.contains(dep) {
stack.push(dep.to_string());
return true;
}
}
// because we only pushed source files to the stack, we also only need to
// pop these from the stack if we don't find a dependency cycle
if let SourceType::SourceFile(_) = module.source_type {
let _ = stack.pop();
}
return false;
}
false
}

fn format_dependency_cycle(cycle: &Vec<String>) -> String {
cycle
.iter()
.map(|s| helpers::format_namespaced_module_name(s))
.collect::<Vec<String>>()
.join(" -> ")
}

pub fn build(filter: &Option<regex::Regex>, path: &str, no_timing: bool) -> Result<BuildState, ()> {
let timing_total = Instant::now();
let project_root = helpers::get_abs_path(path);
Expand Down Expand Up @@ -1322,8 +1390,7 @@ pub fn build(filter: &Option<regex::Regex>, path: &str, no_timing: bool) -> Resu
// we get this by traversing from the dirty modules to all the modules that
// are dependent on them
let mut compile_universe = dirty_modules.clone();

let mut current_step_modules = dirty_modules.clone();
let mut current_step_modules = compile_universe.clone();
loop {
let mut dependents: AHashSet<String> = AHashSet::new();
for dirty_module in current_step_modules.iter() {
Expand All @@ -1334,7 +1401,7 @@ pub fn build(filter: &Option<regex::Regex>, path: &str, no_timing: bool) -> Resu
.map(|s| s.to_string())
.collect::<AHashSet<String>>();

compile_universe.extend(current_step_modules.clone());
compile_universe.extend(current_step_modules.to_owned());
if current_step_modules.is_empty() {
break;
}
Expand Down Expand Up @@ -1371,8 +1438,9 @@ pub fn build(filter: &Option<regex::Regex>, path: &str, no_timing: bool) -> Resu
loop_count,
);

in_progress_modules
.clone()
let current_in_progres_modules = in_progress_modules.clone();

current_in_progres_modules
.par_iter()
.map(|module_name| {
let module = build_state.get_module(module_name).unwrap();
Expand Down Expand Up @@ -1602,8 +1670,18 @@ pub fn build(filter: &Option<regex::Regex>, path: &str, no_timing: bool) -> Resu
break;
}
if in_progress_modules.len() == 0 {
// we probably want to find the cycle(s), and give a helpful error message here
compile_errors.push_str("Can't continue... Dependency cycle\n")
// find the dependency cycle
let cycle = find_dependency_cycle(
&compile_universe
.iter()
.map(|s| (s, build_state.get_module(s).unwrap()))
.collect::<Vec<(&String, &Module)>>(),
);
compile_errors.push_str(&format!(
"\n{}\n{}\n",
style("Can't continue... Found a circular dependency in your code:").red(),
format_dependency_cycle(&cycle)
))
}
if compile_errors.len() > 0 {
break;
Expand Down
13 changes: 13 additions & 0 deletions src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,3 +314,16 @@ pub fn get_extension(path: &str) -> String {
.expect("Could not get extension 2")
.to_string();
}

pub fn format_namespaced_module_name(module_name: &str) -> String {
// from ModuleName-Namespace to Namespace.ModuleName
// also format ModuleName-@Namespace to Namespace.ModuleName
let mut split = module_name.split("-");
let module_name = split.next().unwrap();
let namespace = split.next();
let namespace = namespace.map(|ns| ns.trim_start_matches("@"));
return match namespace {
None => module_name.to_string(),
Some(ns) => ns.to_string() + "." + module_name,
};
}
4 changes: 1 addition & 3 deletions testrepo/packages/dep01/bsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,5 @@
"in-source": true
},
"suffix": ".bs.js",
"bs-dependencies": [
"@testrepo/dep02"
]
"bs-dependencies": ["@testrepo/dep02"]
}
2 changes: 1 addition & 1 deletion testrepo/packages/dep01/src/Dep01.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ function log(param) {
export {
log ,
}
/* No side effect */
/* Dep02 Not a pure module */
2 changes: 1 addition & 1 deletion testrepo/packages/dep02/bsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@
"in-source": true
},
"suffix": ".bs.js",
"bs-dependencies": []
"bs-dependencies": ["@testrepo/new-namespace"]
}
5 changes: 4 additions & 1 deletion testrepo/packages/dep02/src/Dep02.mjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Generated by ReScript, PLEASE EDIT WITH CARE

import * as $$Array from "./Array.mjs";
import * as NS_alias$$atNewNamespace from "@testrepo/new-namespace/src/NS_alias.mjs";

function log(param) {
$$Array.forEach([
Expand All @@ -11,7 +12,9 @@ function log(param) {
}));
}

console.log(NS_alias$$atNewNamespace.hello_world(undefined));

export {
log ,
}
/* No side effect */
/* Not a pure module */
3 changes: 2 additions & 1 deletion testrepo/packages/dep02/src/Dep02.res
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
open Array;
open Array
let log = () => ["a", "b"]->forEach(Js.log)
Js.log(NS.Alias.hello_world())
1 change: 1 addition & 0 deletions testrepo/packages/main/src/output.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
Hello world
01
02
a
Expand Down
1 change: 1 addition & 0 deletions testrepo/packages/new-namespace/bsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,6 @@
"module": "es6",
"in-source": true
},
"bs-dependencies": ["@testrepo/dep01"],
"suffix": ".bs.js"
}
7 changes: 7 additions & 0 deletions tests/compile.sh
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#!/bin/bash
cd $(dirname $0)
source "./utils.sh"
cd ../testrepo
Expand Down Expand Up @@ -52,6 +53,12 @@ replace "s/$(pwd | sed "s/\//\\\\\//g")//g" ../tests/snapshots/remove-file.txt
git checkout -- packages/dep02/src/Dep02.res
rewatch build &> /dev/null

# it should show an error when we have a dependency cycle
echo 'Dep01.log()' >> packages/new-namespace/src/NS_alias.res
rewatch build --no-timing=true &> ../tests/snapshots/dependency-cycle.txt
git checkout -- packages/new-namespace/src/NS_alias.res
rewatch build &> /dev/null

# make sure we don't have changes in the test repo
if git diff --exit-code ./;
then
Expand Down
9 changes: 9 additions & 0 deletions tests/snapshots/dependency-cycle.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[1/7] 🌴 Building package tree...[1/7] ️✅ Built package tree in 0.00s
[2/7] 🔍 Finding source files...[2/7] ️✅ Found source files in 0.00s
[3/7] 🧹 Cleaning up previous build...[3/7] ️✅ Cleaned 0/9 0.00s
[4/7] ️✅ Parsed 1 source files in 0.00s
[5/7] ️✅ Collected deps in 0.00s
[6/7] ️🛑 Compiled 0 modules in 0.00s

Can't continue... Found a circular dependency in your code:
Dep01 -> Dep02 -> NS -> NewNamespace.NS_alias -> Dep01
Expand Down

0 comments on commit 44b5122

Please sign in to comment.