feat: directory moves — mirrored batch relocation of every indexed file under a dir
- MovePlan gains moves[] (file move = 1 relocation), left_behind[] for unindexable files that deliberately stay, prune_dirs[] applied after the batch (remove_dir only succeeds when empty, leftovers keep their home) - plan::dir: nesting/target-exists guards, merged per-file rewrites - apply: atomic N-rename batch (per-file git mv), rollback restores every move, rewrite and created dir newest-first - mv_reject accepts dirs; --json adds moved_files/would_move_files only for real dir moves (single-file contract byte-identical); summary shows (N files) and left-behind count - engine rollback/prune tests moved to tests/apply.rs (public API; a mid-batch failure cannot be forced through the CLI), java-rule e2e split into its own binary to keep every file under the 250-line budget
This commit is contained in:
parent
b96deeaf2f
commit
6f7a2190e6
GPG key ID:
253067914055423B
17 changed files with 842 additions and 293 deletions
170
tests/apply.rs
Normal file
170
tests/apply.rs
Normal file
|
|
@ -0,0 +1,170 @@
|
|||
//! Direct engine tests for `apply` and `apply_edits` via the public lib
|
||||
//! API: multi-move rollback and dir pruning are impossible to force
|
||||
//! through the CLI (pre-flight validation catches them first).
|
||||
|
||||
use jmove::core::JmoveResult;
|
||||
use jmove::core::apply::{GitMode, apply};
|
||||
use jmove::core::plan::{FileMove, MovePlan, Rewrite};
|
||||
use std::fs;
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
const OLD: &str = "import {\n fmt,\n} from '../lib/fmt';\n";
|
||||
const NEW: &str = "import {\n fmt,\n} from '../deep/fmt';\n";
|
||||
// Byte span of `../lib/fmt` (between the quotes) inside OLD.
|
||||
const SPAN: std::ops::Range<usize> = 24..34;
|
||||
|
||||
// Plan moving lib/fmt.ts -> deep/fmt.ts, rewriting src/app.ts.
|
||||
fn plan() -> MovePlan {
|
||||
let rewrite = Rewrite {
|
||||
file: "src/app.ts".into(),
|
||||
span: SPAN,
|
||||
old_text: "../lib/fmt".into(),
|
||||
new_text: "../deep/fmt".into(),
|
||||
};
|
||||
MovePlan {
|
||||
source: "lib/fmt.ts".into(),
|
||||
target: "deep/fmt.ts".into(),
|
||||
moves: vec![FileMove {
|
||||
source: "lib/fmt.ts".into(),
|
||||
target: "deep/fmt.ts".into(),
|
||||
}],
|
||||
rewrites: vec![rewrite],
|
||||
left_behind: Vec::new(),
|
||||
prune_dirs: Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
fn mk(dir: &Path, rel: &str, body: &str) -> JmoveResult<()> {
|
||||
let path = dir.join(rel);
|
||||
fs::create_dir_all(path.parent().unwrap())?;
|
||||
fs::write(path, body)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply_rewrites_spans_creates_dirs_and_moves_last() -> JmoveResult<()> {
|
||||
assert_eq!(&OLD[SPAN], "../lib/fmt"); // sanity: the span is real
|
||||
let dir = tempfile::TempDir::new()?;
|
||||
let root = dir.path();
|
||||
mk(root, "src/app.ts", OLD)?;
|
||||
mk(root, "lib/fmt.ts", "export const fmt = 1;\n")?;
|
||||
let applied = apply(root, &plan(), GitMode::Disabled)?;
|
||||
assert_eq!(
|
||||
(applied.files_rewritten, &applied.new_path),
|
||||
(1, &PathBuf::from("deep/fmt.ts"))
|
||||
);
|
||||
assert!(!root.join("lib/fmt.ts").exists());
|
||||
assert_eq!(
|
||||
fs::read_to_string(root.join("deep/fmt.ts"))?,
|
||||
"export const fmt = 1;\n"
|
||||
);
|
||||
// Only the specifier bytes changed; the layout is kept byte-exact.
|
||||
assert_eq!(fs::read_to_string(root.join("src/app.ts"))?, NEW);
|
||||
assert!(!root.join("src/app.ts.jmove-tmp").exists());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply_rolls_back_when_the_move_fails() -> JmoveResult<()> {
|
||||
// Missing source: the last rename fails after the rewrite landed.
|
||||
let dir = tempfile::TempDir::new()?;
|
||||
mk(dir.path(), "src/app.ts", OLD)?;
|
||||
let err = apply(dir.path(), &plan(), GitMode::Disabled).expect_err("missing source");
|
||||
assert!(matches!(err, jmove::core::JmoveError::Io(_)), "{err}");
|
||||
// Importer restored to its exact original bytes; created dirs gone.
|
||||
assert_eq!(fs::read_to_string(dir.path().join("src/app.ts"))?, OLD);
|
||||
assert!(!dir.path().join("deep").exists());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply_rejects_a_stale_plan_without_writing() -> JmoveResult<()> {
|
||||
// SPAN was computed on OLD's layout; a single-line importer has
|
||||
// different bytes there, so the run fails before any write.
|
||||
let other = "import { fmt } from '../lib/fmt';\n";
|
||||
let dir = tempfile::TempDir::new()?;
|
||||
let root = dir.path();
|
||||
mk(root, "src/app.ts", other)?;
|
||||
mk(root, "lib/fmt.ts", "export const fmt = 1;\n")?;
|
||||
let err = apply(root, &plan(), GitMode::Disabled).expect_err("span mismatch");
|
||||
assert!(
|
||||
matches!(err, jmove::core::JmoveError::StaleIndex(_)),
|
||||
"{err}"
|
||||
);
|
||||
assert_eq!(fs::read_to_string(root.join("src/app.ts"))?, other);
|
||||
assert!(root.join("lib/fmt.ts").exists());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dir_apply_prunes_emptied_source_dirs_last() -> JmoveResult<()> {
|
||||
let dir = tempfile::TempDir::new()?;
|
||||
let root = dir.path();
|
||||
mk(root, "src/a.ts", "x\n")?;
|
||||
mk(root, "src/nested/b.ts", "y\n")?;
|
||||
let plan = MovePlan {
|
||||
source: "src".into(),
|
||||
target: "lib".into(),
|
||||
moves: vec![
|
||||
FileMove {
|
||||
source: "src/a.ts".into(),
|
||||
target: "lib/a.ts".into(),
|
||||
},
|
||||
FileMove {
|
||||
source: "src/nested/b.ts".into(),
|
||||
target: "lib/nested/b.ts".into(),
|
||||
},
|
||||
],
|
||||
rewrites: Vec::new(),
|
||||
left_behind: Vec::new(),
|
||||
// shallowest last on purpose: apply must prune deepest first.
|
||||
prune_dirs: vec!["src/nested".into(), "src".into()],
|
||||
};
|
||||
apply(root, &plan, GitMode::Disabled)?;
|
||||
assert!(root.join("lib/nested/b.ts").is_file());
|
||||
assert!(!root.join("src").exists(), "emptied source tree must go");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dir_apply_rolls_back_every_move_when_a_later_one_fails() -> JmoveResult<()> {
|
||||
// "trap" is a file, so the second move's parent can never exist:
|
||||
// the first move must be undone and rewritten importers restored.
|
||||
let dir = tempfile::TempDir::new()?;
|
||||
let root = dir.path();
|
||||
mk(root, "src/a.ts", "a\n")?;
|
||||
mk(root, "src/b.ts", "b\n")?;
|
||||
mk(root, "trap", "I am a file\n")?;
|
||||
mk(root, "app.ts", "import './src/a';\n")?;
|
||||
let plan = MovePlan {
|
||||
source: "src".into(),
|
||||
target: "lib".into(),
|
||||
moves: vec![
|
||||
FileMove {
|
||||
source: "src/a.ts".into(),
|
||||
target: "lib/a.ts".into(),
|
||||
},
|
||||
FileMove {
|
||||
source: "src/b.ts".into(),
|
||||
target: "trap/b.ts".into(),
|
||||
},
|
||||
],
|
||||
rewrites: vec![Rewrite {
|
||||
file: "app.ts".into(),
|
||||
span: 8..15,
|
||||
old_text: "./src/a".into(),
|
||||
new_text: "./lib/a".into(),
|
||||
}],
|
||||
left_behind: Vec::new(),
|
||||
prune_dirs: vec!["src".into()],
|
||||
};
|
||||
let err = apply(root, &plan, GitMode::Disabled).expect_err("ENOTDIR");
|
||||
assert!(matches!(err, jmove::core::JmoveError::Io(_)), "{err}");
|
||||
assert!(root.join("src/a.ts").is_file(), "first move undone");
|
||||
assert!(!root.join("lib").exists(), "created dirs removed");
|
||||
assert_eq!(
|
||||
fs::read_to_string(root.join("app.ts"))?,
|
||||
"import './src/a';\n"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
98
tests/cli_dir.rs
Normal file
98
tests/cli_dir.rs
Normal file
|
|
@ -0,0 +1,98 @@
|
|||
//! End-to-end tests for moving whole directories: every indexed member
|
||||
//! relocates in one atomic batch and all importers follow.
|
||||
|
||||
mod common;
|
||||
|
||||
use common::{copy_fixture, in_root, jmove, read};
|
||||
use predicates::prelude::*;
|
||||
use tempfile::TempDir;
|
||||
|
||||
fn fixture(name: &str) -> TempDir {
|
||||
copy_fixture("typescript", name)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ts_dir_move_relocates_members_and_rewrites_importers() {
|
||||
let tmp = fixture("normal");
|
||||
jmove(&tmp, &["mv", "src/impl", "src/impl2"])
|
||||
.success()
|
||||
.stdout(predicate::str::contains("(2 files)"));
|
||||
assert!(!in_root(tmp.path(), "src/impl").exists());
|
||||
assert!(in_root(tmp.path(), "src/impl2/core.ts").is_file());
|
||||
assert!(in_root(tmp.path(), "src/impl2/index.ts").is_file());
|
||||
|
||||
let app = read(&in_root(tmp.path(), "src/app.ts"));
|
||||
assert!(app.contains("./impl2/core"), "{app}");
|
||||
// A barrel directory import keeps its conservative `.../index` form.
|
||||
let root = read(&in_root(tmp.path(), "src/index.ts"));
|
||||
assert!(root.contains("export * from \"./impl2/index\""), "{root}");
|
||||
jmove(&tmp, &["check"]).success();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn java_package_dir_move_rewrites_packages_and_imports() {
|
||||
let tmp = copy_fixture("java", "basic");
|
||||
jmove(
|
||||
&tmp,
|
||||
&[
|
||||
"mv",
|
||||
"src/main/java/com/example/util",
|
||||
"src/main/java/com/example/core",
|
||||
],
|
||||
)
|
||||
.success()
|
||||
.stdout(predicate::str::contains("updated 4 imports in 3 files"));
|
||||
|
||||
let text = read(&in_root(
|
||||
tmp.path(),
|
||||
"src/main/java/com/example/core/Text.java",
|
||||
));
|
||||
assert!(text.contains("package com.example.core;"), "{text}");
|
||||
let app = read(&in_root(
|
||||
tmp.path(),
|
||||
"src/main/java/com/example/app/App.java",
|
||||
));
|
||||
assert!(app.contains("import com.example.core.Text;"), "{app}");
|
||||
assert!(
|
||||
app.contains("import static com.example.core.Text.shout;"),
|
||||
"{app}"
|
||||
);
|
||||
jmove(&tmp, &["check"]).success();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dir_move_json_lists_every_file_and_unsupported_left_behind() {
|
||||
let tmp = fixture("normal");
|
||||
let note = in_root(tmp.path(), "src/impl/notes.txt");
|
||||
std::fs::write(¬e, "asset, not source\n").expect("write note");
|
||||
jmove(&tmp, &["mv", "src/impl", "src/impl2", "--json"])
|
||||
.success()
|
||||
.stdout(
|
||||
predicate::str::contains("\"moved\": 2")
|
||||
.and(predicate::str::contains("\"moved_files\""))
|
||||
.and(predicate::str::contains("\"to\": \"src/impl2/core.ts\""))
|
||||
.and(predicate::str::contains("left_behind"))
|
||||
.and(predicate::str::contains("src/impl/notes.txt")),
|
||||
);
|
||||
// The unindexable asset is reported, not silently dragged along.
|
||||
assert!(note.is_file());
|
||||
assert!(!in_root(tmp.path(), "src/impl2/notes.txt").exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dry_run_dir_move_previews_every_relocation() {
|
||||
let tmp = fixture("normal");
|
||||
jmove(
|
||||
&tmp,
|
||||
&["mv", "src/impl", "src/impl2", "--dry-run", "--json"],
|
||||
)
|
||||
.success()
|
||||
.stdout(
|
||||
predicate::str::contains("\"would_move_files\"")
|
||||
.and(predicate::str::contains("\"from\": \"src/impl/core.ts\"")),
|
||||
);
|
||||
assert!(
|
||||
in_root(tmp.path(), "src/impl/core.ts").is_file(),
|
||||
"dry-run writes nothing"
|
||||
);
|
||||
}
|
||||
|
|
@ -141,49 +141,6 @@ fn missing_project() -> tempfile::TempDir {
|
|||
common::copy_fixture("java", "fix_missing")
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn missing_import_adds_unique_and_reports_ambiguous() {
|
||||
let tmp = missing_project();
|
||||
jmove(&tmp, &["fix", "--rule", "java/missing-import", "--json"])
|
||||
.success()
|
||||
.stdout(
|
||||
predicate::str::contains("\"rule\": \"java/missing-import\"")
|
||||
.and(predicate::str::contains("\"applied\": false"))
|
||||
.and(predicate::str::contains("\"com.example.a.Config\""))
|
||||
.and(predicate::str::contains("\"com.example.b.Config\"")),
|
||||
);
|
||||
let calc = read(&in_root(
|
||||
tmp.path(),
|
||||
"src/main/java/com/example/app/Calc.java",
|
||||
));
|
||||
assert!(
|
||||
calc.contains("package com.example.app;\nimport com.example.util.Maths;\n"),
|
||||
"{calc}"
|
||||
);
|
||||
// The ambiguous `Config` is never guessed at.
|
||||
let refer = read(&in_root(
|
||||
tmp.path(),
|
||||
"src/main/java/com/example/c/Refer.java",
|
||||
));
|
||||
assert!(!refer.contains("import com.example."), "{refer}");
|
||||
jmove(&tmp, &["check"]).success();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn missing_import_dry_run_then_stable_reapply() {
|
||||
let tmp = missing_project();
|
||||
jmove(&tmp, &["fix", "--dry-run"])
|
||||
.success()
|
||||
.stdout(predicate::str::contains("+import com.example.util.Maths;"));
|
||||
jmove(&tmp, &["fix"])
|
||||
.success()
|
||||
.stdout(predicate::str::contains("fixed 3 issues in 2 files"));
|
||||
// Only manual (ambiguous) findings remain: nothing further applies.
|
||||
jmove(&tmp, &["fix"])
|
||||
.success()
|
||||
.stdout(predicate::str::contains("nothing to change"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unused_delete_and_missing_insert_coexist_in_one_file() {
|
||||
// Dual.java: the unused import is deleted while the missing `Maths`
|
||||
|
|
@ -198,56 +155,6 @@ fn unused_delete_and_missing_insert_coexist_in_one_file() {
|
|||
jmove(&tmp, &["check"]).success();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn three_rules_converge_on_one_project() {
|
||||
// App.java: unused import inside an out-of-order block (the order
|
||||
// rewrite overlaps the deletion, so it defers to run 2).
|
||||
// C.java: out-of-order block + a bare `Maths` reference (insertion at
|
||||
// the block boundary coexists with the rewrite in run 1).
|
||||
let tmp = common::copy_fixture("java", "order");
|
||||
jmove(&tmp, &["fix", "--dry-run", "--json"])
|
||||
.success()
|
||||
.stdout(
|
||||
predicate::str::contains("\"rule\": \"java/import-order\"")
|
||||
.and(predicate::str::contains("\"applied\": false"))
|
||||
.and(predicate::str::contains("skipped")),
|
||||
);
|
||||
let app = "src/main/java/com/example/app/App.java";
|
||||
let c = "src/main/java/com/example/app/C.java";
|
||||
jmove(&tmp, &["fix"])
|
||||
.success()
|
||||
.stdout(predicate::str::contains("fixed 3 issues in 2 files"));
|
||||
let app_text = read(&in_root(tmp.path(), app));
|
||||
// Unused is gone; the deferred order fix has not touched the block.
|
||||
assert!(!app_text.contains("Unneeded"), "{app_text}");
|
||||
assert!(
|
||||
app_text.contains("import java.util.List;\nimport com.example.util.Maths;"),
|
||||
"{app_text}"
|
||||
);
|
||||
jmove(&tmp, &["fix"])
|
||||
.success()
|
||||
.stdout(predicate::str::contains("fixed 2 issues in 2 files"));
|
||||
let c_text = read(&in_root(tmp.path(), c));
|
||||
assert!(
|
||||
c_text.contains(
|
||||
"import com.example.util.Maths;\nimport java.util.List;\nimport java.util.Map;\n"
|
||||
),
|
||||
"{c_text}"
|
||||
);
|
||||
let app_text = read(&in_root(tmp.path(), app));
|
||||
assert!(
|
||||
app_text.contains(
|
||||
"import com.example.util.Maths;\nimport java.util.List;\nimport java.util.Map;\n"
|
||||
),
|
||||
"{app_text}"
|
||||
);
|
||||
// Fixed point.
|
||||
jmove(&tmp, &["fix"])
|
||||
.success()
|
||||
.stdout(predicate::str::contains("nothing to change"));
|
||||
jmove(&tmp, &["check"]).success();
|
||||
}
|
||||
|
||||
fn ts_unused() -> tempfile::TempDir {
|
||||
common::copy_fixture("typescript", "unused")
|
||||
}
|
||||
|
|
|
|||
|
|
@ -113,3 +113,21 @@ fn mv_no_git_keeps_the_plain_rename_unstaged() {
|
|||
let target = git_output(tmp.path(), &["status", "--porcelain", "--", "utils/sum.ts"]);
|
||||
assert!(target.starts_with("??"), "{target}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dir_move_stages_every_rename_and_prunes_the_source_dir() {
|
||||
let tmp = git_fixture("basic");
|
||||
jmove(&tmp, &["mv", "lib", "vendor/lib"])
|
||||
.success()
|
||||
.stdout(predicate::str::contains("(via git mv)"));
|
||||
let staged = git_output(tmp.path(), &["diff", "--cached", "-M", "--name-status"]);
|
||||
assert!(
|
||||
staged.contains("vendor/lib/sum.ts") && staged.contains("lib/sum.ts"),
|
||||
"{staged}"
|
||||
);
|
||||
assert!(tmp.path().join("vendor/lib/sum.ts").is_file());
|
||||
assert!(
|
||||
!tmp.path().join("lib").exists(),
|
||||
"emptied source dir pruned"
|
||||
);
|
||||
}
|
||||
|
|
|
|||
96
tests/cli_missing_import.rs
Normal file
96
tests/cli_missing_import.rs
Normal file
|
|
@ -0,0 +1,96 @@
|
|||
//! End-to-end tests for the `java/missing-import` rule against fixtures.
|
||||
//! Self-contained copy of the tiny helpers it needs (each test file is its
|
||||
//! own crate; importing all of `common` would trip dead-code warnings).
|
||||
|
||||
use assert_cmd::Command;
|
||||
use predicates::prelude::*;
|
||||
use std::fs;
|
||||
use std::path::{Path, PathBuf};
|
||||
use tempfile::{TempDir, tempdir};
|
||||
|
||||
fn fixture(name: &str) -> TempDir {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
copy_dir(
|
||||
&Path::new(env!("CARGO_MANIFEST_DIR"))
|
||||
.join("tests")
|
||||
.join("java")
|
||||
.join(name),
|
||||
tmp.path(),
|
||||
);
|
||||
fs::create_dir(tmp.path().join(".git")).expect("git marker");
|
||||
tmp
|
||||
}
|
||||
|
||||
fn copy_dir(from: &Path, to: &Path) {
|
||||
fs::create_dir_all(to).expect("mkdir");
|
||||
for entry in fs::read_dir(from).expect("readdir") {
|
||||
let entry = entry.expect("entry");
|
||||
let target = to.join(entry.file_name());
|
||||
if entry.file_type().expect("filetype").is_dir() {
|
||||
copy_dir(&entry.path(), &target);
|
||||
} else {
|
||||
fs::copy(entry.path(), target).expect("copy");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn jmove(root: &TempDir, args: &[&str]) -> assert_cmd::assert::Assert {
|
||||
let mut cmd = Command::cargo_bin("jmove").expect("jmove binary");
|
||||
cmd.arg("--root").arg(root.path()).args(args);
|
||||
cmd.assert()
|
||||
}
|
||||
|
||||
fn in_root(root: &Path, rel: &str) -> PathBuf {
|
||||
root.join(rel)
|
||||
}
|
||||
|
||||
fn read(path: &PathBuf) -> String {
|
||||
fs::read_to_string(path).unwrap_or_else(|err| panic!("read {path:?}: {err}"))
|
||||
}
|
||||
|
||||
fn missing_project() -> tempfile::TempDir {
|
||||
fixture("fix_missing")
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn missing_import_adds_unique_and_reports_ambiguous() {
|
||||
let tmp = missing_project();
|
||||
jmove(&tmp, &["fix", "--rule", "java/missing-import", "--json"])
|
||||
.success()
|
||||
.stdout(
|
||||
predicate::str::contains("\"rule\": \"java/missing-import\"")
|
||||
.and(predicate::str::contains("\"applied\": false"))
|
||||
.and(predicate::str::contains("\"com.example.a.Config\""))
|
||||
.and(predicate::str::contains("\"com.example.b.Config\"")),
|
||||
);
|
||||
let calc = read(&in_root(
|
||||
tmp.path(),
|
||||
"src/main/java/com/example/app/Calc.java",
|
||||
));
|
||||
assert!(
|
||||
calc.contains("package com.example.app;\nimport com.example.util.Maths;\n"),
|
||||
"{calc}"
|
||||
);
|
||||
// The ambiguous `Config` is never guessed at.
|
||||
let refer = read(&in_root(
|
||||
tmp.path(),
|
||||
"src/main/java/com/example/c/Refer.java",
|
||||
));
|
||||
assert!(!refer.contains("import com.example."), "{refer}");
|
||||
jmove(&tmp, &["check"]).success();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn missing_import_dry_run_then_stable_reapply() {
|
||||
let tmp = missing_project();
|
||||
jmove(&tmp, &["fix", "--dry-run"])
|
||||
.success()
|
||||
.stdout(predicate::str::contains("+import com.example.util.Maths;"));
|
||||
jmove(&tmp, &["fix"])
|
||||
.success()
|
||||
.stdout(predicate::str::contains("fixed 3 issues in 2 files"));
|
||||
// Only manual (ambiguous) findings remain: nothing further applies.
|
||||
jmove(&tmp, &["fix"])
|
||||
.success()
|
||||
.stdout(predicate::str::contains("nothing to change"));
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue