Skip to content

Commit

Permalink
Fix bug when modules are renamed
Browse files Browse the repository at this point in the history
This fixes a "modules defined in multiple files" bug when a module is
renamed. The rename is interpreted as one module being deleted (which
triggers a restart) and one module being added (which triggers an
`:add`).

The bug is that it would restart `ghci`, thereby loading the new module,
and then attempt to `:add` the new module anyways, which would fail.
  • Loading branch information
9999years committed Oct 4, 2023
1 parent 8ec0c17 commit 9007839
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 1 deletion.
9 changes: 9 additions & 0 deletions src/ghci/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,9 @@ impl Ghci {
tracing::info!(%command, "Running after-restart command");
self.stdin.run_command(&mut self.stdout, command).await?;
}
// Once we restart, everything is freshly loaded. We don't need to add or
// reload any other modules.
return Ok(());
}

if actions.needs_add_or_reload() {
Expand Down Expand Up @@ -551,6 +554,11 @@ impl Ghci {
/// Optionally returns a compilation result.
#[instrument(skip(self), level = "debug")]
async fn add_module(&mut self, path: &NormalPath) -> miette::Result<Option<CompilationResult>> {
if self.targets.contains_source_path(path.absolute())? {
tracing::debug!(%path, "Skipping `:add`ing already-loaded path");
return Ok(None);
}

let messages = self
.stdin
.add_module(&mut self.stdout, path.relative())
Expand Down Expand Up @@ -662,6 +670,7 @@ impl Display for Mode {
/// Actions needed to perform a reload.
///
/// See [`Ghci::reload`].
#[derive(Debug)]
struct ReloadActions {
/// Paths to modules which need a full `ghci` restart.
needs_restart: Vec<NormalPath>,
Expand Down
2 changes: 2 additions & 0 deletions src/ghci/parse/show_paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ impl ShowPaths {
if is_haskell_source_file(target_path) {
// The target is already a path.
if let Some(path) = self.target_path_to_path(target_path) {
tracing::trace!(%path, %target, "Target is path");
return Ok(path);
}
} else {
Expand All @@ -45,6 +46,7 @@ impl ShowPaths {
path.set_extension(haskell_source_extension);

if let Some(path) = self.target_path_to_path(&path) {
tracing::trace!(%path, %target, "Found path for target");
return Ok(path);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/ghci/stdin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ impl GhciStdin {
stdout.show_paths().await
}

#[instrument(skip(self, stdout), level = "debug")]
#[instrument(skip_all, level = "debug")]
pub async fn show_targets(
&mut self,
stdout: &mut GhciStdout,
Expand Down
45 changes: 45 additions & 0 deletions tests/rename.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
use test_harness::fs;
use test_harness::test;
use test_harness::GhcidNg;
use test_harness::Matcher;

/// Test that `ghcid-ng` can restart correctly when modules are removed and added (i.e., renamed)
/// at the same time.
#[test]
async fn can_compile_renamed_module() {
let mut session = GhcidNg::new("tests/data/simple")
.await
.expect("ghcid-ng starts");
session
.wait_until_ready()
.await
.expect("ghcid-ng loads ghci");

let module_path = session.path("src/MyModule.hs");
let new_module_path = session.path("src/MyCoolModule.hs");
fs::rename(&module_path, &new_module_path).await.unwrap();

session
.wait_until_restart()
.await
.expect("ghcid-ng restarts on module move");

session
.assert_logged(Matcher::message("Compilation failed").in_span("reload"))
.await
.unwrap();

fs::replace(new_module_path, "module MyModule", "module MyCoolModule")
.await
.unwrap();

session
.wait_until_reload()
.await
.expect("ghcid-ng reloads on module change");

session
.assert_logged(Matcher::message("Compilation succeeded").in_span("reload"))
.await
.unwrap();
}

0 comments on commit 9007839

Please sign in to comment.