improve errors for circular dependencies (#7940)

* improve errors for circular dependencies * changelog * fix * Update rewatch/src/build/compile/dependency_cycle.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * remove * fix error message * remove replace --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Author: zth

CHANGELOG.md

@@ -24,6 +24,8 @@
 
 #### :nail_care: Polish
 
+- Improve circular dependency errors, and make sure they end up in the compiler log so the editor tooling can surface them. https://github.com/rescript-lang/rescript/pull/7940
+
 #### :house: Internal
 
 - Use AST nodes with locations for fn arguments in the typed tree. https://github.com/rescript-lang/rescript/pull/7873

rewatch/src/build/compile.rs

@@ -356,11 +356,28 @@ pub fn compile(
  .collect::<Vec<(&String, &Module)>>(),
  );
 
- compile_errors.push_str(&format!(
- "\n{}\n{}\n",
+ let guidance = "Possible solutions:\n- Extract shared code into a new module both depend on.\n";
+ let message = format!(
+ "\n{}\n{}\n{}",
  style("Can't continue... Found a circular dependency in your code:").red(),
- dependency_cycle::format(&cycle)
- ))
+ dependency_cycle::format(&cycle, build_state),
+ guidance
+ );
+
+ // Append the error to the logs of all packages involved in the cycle,
+ // so editor tooling can surface it from .compiler.log
+ let mut touched_packages = AHashSet::<String>::new();
+ for module_name in cycle.iter() {
+ if let Some(module) = build_state.get_module(module_name) {
+ if touched_packages.insert(module.package_name.clone()) {
+ if let Some(package) = build_state.get_package(&module.package_name) {
+ logs::append(package, &message);
+ }
+ }
+ }
+ }
+
+ compile_errors.push_str(&message)
  }
  if !compile_errors.is_empty() {
  break;

rewatch/src/build/compile/dependency_cycle.rs

@@ -1,7 +1,10 @@
 use super::super::build_types::*;
 use crate::helpers;
 use ahash::AHashSet;
-use std::collections::{HashMap, HashSet, VecDeque};
+use std::{
+ collections::{HashMap, HashSet, VecDeque},
+ path::Path,
+};
 
 pub fn find(modules: &Vec<(&String, &Module)>) -> Vec<String> {
  find_shortest_cycle(modules)
@@ -139,15 +142,43 @@ fn find_cycle_bfs(
  None
 }
 
-pub fn format(cycle: &[String]) -> String {
- let mut cycle = cycle.to_vec();
- cycle.reverse();
- // add the first module to the end of the cycle
- cycle.push(cycle[0].clone());
+pub fn format(cycle: &[String], build_state: &BuildCommandState) -> String {
+ let mut nodes = cycle.to_vec();
+ if nodes.is_empty() {
+ return String::new();
+ }
+ nodes.reverse();
+ nodes.push(nodes[0].clone());
+
+ let root_path = &build_state.get_root_config().path;
+ let root = root_path.parent().unwrap_or(root_path.as_path());
 
- cycle
+ nodes
  .iter()
- .map(|s| helpers::format_namespaced_module_name(s))
+ .map(|name| {
+ let display_name = helpers::format_namespaced_module_name(name);
+
+ match build_state.get_module(name) {
+ Some(Module {
+ source_type: SourceType::SourceFile(source_file),
+ package_name,
+ ..
+ }) => {
+ if let Some(package) = build_state.get_package(package_name) {
+ let abs_path = Path::new(&package.path).join(&source_file.implementation.path);
+ let rel_path = abs_path.strip_prefix(root).unwrap_or(&abs_path).to_string_lossy();
+ format!("{display_name} ({rel_path})")
+ } else {
+ display_name
+ }
+ }
+ Some(Module {
+ source_type: SourceType::MlMap(_),
+ ..
+ })
+ | None => display_name,
+ }
+ })
  .collect::<Vec<String>>()
  .join("\n → ")
 }

rewatch/tests/snapshots/dependency-cycle.txt

@@ -12,10 +12,12 @@ The field 'bsc-flags' found in the package config of '@testrepo/deprecated-confi
 Use 'compiler-flags' instead.
 
 Can't continue... Found a circular dependency in your code:
-Dep01
- → Dep02
- → NS
- → NewNamespace.NS_alias
- → Dep01
+Dep01 (packages/dep01/src/Dep01.res)
+ → Dep02 (packages/dep02/src/Dep02.res)
+ → NS (packages/new-namespace/src/NS.res)
+ → NewNamespace.NS_alias (packages/new-namespace/src/NS_alias.res)
+ → Dep01 (packages/dep01/src/Dep01.res)
+Possible solutions:
+- Extract shared code into a new module both depend on.
 
 Incremental build failed. Error:  Failed to Compile. See Errors Above