diff --git a/README.md b/README.md index 3c733a9..0a42c3c 100644 --- a/README.md +++ b/README.md @@ -57,6 +57,9 @@ jmove mv src/utils/parser.ts src/core/parser.ts # --no-git forces a plain rename jmove mv src/foo.ts src/bar/foo.ts --no-git +# Move a whole directory: every file relocates, every importer follows +jmove mv src/utils src/helpers + # Java: jmove updates `package`, all `import`s and moves the file jmove mv src/com/example/utils/Parser.java src/com/example/core/Parser.java diff --git a/docs/EXAMPLES.md b/docs/EXAMPLES.md index 183e684..7e79521 100644 --- a/docs/EXAMPLES.md +++ b/docs/EXAMPLES.md @@ -5,8 +5,8 @@ consuming `--json`. All outputs below are captured from the real binary. Flags (see `jmove --help`): `mv [--dry-run] [--json] [--no-git]`, `check [--json]`, and the global `--root ` / -`--source-root ` (index one subtree only — the monorepo disambiguator -for duplicate Java packages). `.gitignore`d files are never indexed. Inside a git repo, `mv` of a tracked file uses `git mv` (the +`--source-root ` (monorepo subtree filter). `` may also be a +directory (mirrored batch move, emptied dirs pruned). Inside a git repo, `mv` of a tracked file uses `git mv` (the rename is staged); `--no-git` forces a plain filesystem rename. Exit codes: `0` ok · `1` operation error · `2` `check` found broken imports. diff --git a/docs/SKILL.md b/docs/SKILL.md index c2b61b4..e05d4e6 100644 --- a/docs/SKILL.md +++ b/docs/SKILL.md @@ -2,8 +2,8 @@ ## What this tool does -Moves or renames source files inside a project and updates every import -statement referencing them. Never breaks imports. Supported: TypeScript, +Moves or renames source files — or whole directories of them — inside a +project and updates every import statement referencing them. Never breaks imports. Supported: TypeScript, JavaScript, Java (package declaration + all importers + the file move are kept in sync); Python/Go on the roadmap. Single binary, no LSP needed. @@ -25,6 +25,13 @@ Always run `--dry-run` first and confirm the change set looks right. Moving onto an existing path fails with `TARGET_EXISTS` — choose another target (Phase 1 has no overwrite mode). +Directory moves: `mv ` relocates every indexed file under +`` mirrored under `` in one atomic batch. `--json` adds +`moved_files[]` (from/to per file) and `left_behind[]` — real files under +`` that are not indexable and deliberately stay where they are. +Emptied source directories are pruned; a directory with leftovers is not. +A source that is nested in its own target fails with `PLAN_REJECTED`. + Git integration: inside a git repository, a tracked file is renamed with `git mv` so the rename is staged (history-preserving `git log --follow` / `git diff -M` work). Untracked files, non-repositories and `--no-git` diff --git a/src/cli/json.rs b/src/cli/json.rs index 79f673d..72aacb9 100644 --- a/src/cli/json.rs +++ b/src/cli/json.rs @@ -136,12 +136,45 @@ pub struct MvData { pub target: String, /// Importer files touched by the move. pub changed_files: Vec, - /// Number of files moved (always 1 in Phase 1). + /// Number of files physically moved (1 for a file move, N for a dir). pub moved: usize, /// Total specifiers rewritten across all importers. pub updated_imports: usize, - /// Rename backend: `"git"` (staged in the index) or `"fs"`. + /// Rename backend: `"git"` (every rename staged in the index) or `"fs"`. pub moved_via: &'static str, + /// Directory moves only: each `(from, to)` relocation (omitted for file moves). + #[serde(skip_serializing_if = "Vec::is_empty")] + pub moved_files: Vec, + /// Directory moves only: unindexable files staying in place (omitted when empty). + #[serde(skip_serializing_if = "Vec::is_empty")] + pub left_behind: Vec, +} + +/// One `(from, to)` relocation of a directory move. +#[derive(Debug, Serialize)] +pub struct FileMoveData { + /// Project-relative path moved away. + pub from: String, + /// Project-relative destination path. + pub to: String, +} + +impl FileMoveData { + fn from(m: &crate::core::plan::FileMove) -> Self { + Self { + from: rel_str(&m.source), + to: rel_str(&m.target), + } + } +} + +// Relocation list, present only when the plan moved more than one file. +fn dir_moves(plan: &MovePlan) -> Vec { + if plan.moves.len() > 1 { + plan.moves.iter().map(FileMoveData::from).collect() + } else { + Vec::new() + } } impl MvData { @@ -152,9 +185,12 @@ impl MvData { source: rel_str(&plan.source), target: rel_str(&plan.target), changed_files, - moved: 1, + moved: plan.moves.len(), updated_imports: plan.rewrites.len(), moved_via: if via_git { "git" } else { "fs" }, + // A single-file move keeps the old contract: no extra fields. + moved_files: dir_moves(plan), + left_behind: plan.left_behind.iter().map(|p| rel_str(p)).collect(), } } } @@ -175,6 +211,9 @@ pub struct MvDryRunData { pub diff: String, /// Rename backend a real run would use: `"git"` or `"fs"`. pub would_move_via: &'static str, + /// Directory moves only: every relocation that would happen. + #[serde(skip_serializing_if = "Vec::is_empty")] + pub would_move_files: Vec, } impl MvDryRunData { @@ -191,32 +230,11 @@ impl MvDryRunData { .collect(), diff, would_move_via: if via_git { "git" } else { "fs" }, + would_move_files: dir_moves(plan), } } } -/// One unresolvable relative import found by `check`. -#[derive(Debug, Serialize)] -pub struct BrokenImport { - /// Project-relative file declaring the import. - pub file: String, - /// 1-based line of the specifier. - pub line: usize, - /// Specifier text as written. - pub import: String, - /// Stable reason code, currently always `"file_not_found"`. - pub reason: &'static str, -} - -/// Success payload of `check --json` (flattened under the envelope). -#[derive(Debug, Serialize)] -pub struct CheckData { - /// Broken imports, sorted by file then line. - pub broken_imports: Vec, - /// Number of broken imports (kept as an explicit counter for agents). - pub total: usize, -} - /// Serialize `value` as pretty JSON to stdout. pub fn print(value: &T) { match serde_json::to_string_pretty(value) { diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 58d6049..68c4d56 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -17,7 +17,7 @@ use crate::core::index::Index; use crate::core::plan::{self, MovePlan}; use crate::core::{self, JmoveError, JmoveResult}; -use json::{CheckData, Envelope, ErrorData, MvData, MvDryRunData}; +use json::{Envelope, ErrorData, MvData, MvDryRunData}; /// jmove — move source files, keep every import intact. #[derive(Debug, Parser)] @@ -204,7 +204,7 @@ fn check(root: &Path, source_root: Option<&Path>, json: bool) -> Flow { if json { let total = broken.len(); - let data = CheckData { + let data = output::CheckData { broken_imports: broken, total, }; diff --git a/src/cli/output.rs b/src/cli/output.rs index 10cf3b1..b83a1e8 100644 --- a/src/cli/output.rs +++ b/src/cli/output.rs @@ -4,6 +4,8 @@ //! Pure functions returning data, except [`report_check`] and //! [`print_error`] which perform the only I/O (stdout and stderr). +use serde::Serialize; + use std::collections::BTreeMap; use std::path::{Path, PathBuf}; @@ -11,7 +13,7 @@ use crate::core::index::Index; use crate::core::plan::{MovePlan, Rewrite}; use crate::core::{JmoveResult, rel_str}; -use super::json::{BrokenImport, Change, ChangedFile, ErrorData}; +use super::json::{Change, ChangedFile, ErrorData}; /// `check` stdout line when the project has no broken imports. const CHECK_CLEAN: &str = "check: no broken imports found"; @@ -107,7 +109,8 @@ pub fn mv_reject(root: &Path, source: &Path, target: &Path) -> Option let msg = "source and target are the same path".into(); return bad("INVALID_ARGUMENT", msg, "pick a different destination"); } - if !root.join(source).is_file() { + let src_path = root.join(source); + if !src_path.is_file() && !src_path.is_dir() { let msg = format!("source file '{}' does not exist", rel_str(source)); return bad( "SOURCE_NOT_FOUND", @@ -137,14 +140,23 @@ pub fn mv_reject(root: &Path, source: &Path, target: &Path) -> Option } /// `moved src -> tgt, updated N imports in M files` success summary, -/// noting when the rename went through `git mv`. +/// noting the `git mv` backend and, for directory moves, the file count +/// and anything unindexable that stays behind. #[must_use] pub fn mv_summary(plan: &MovePlan, via_git: bool) -> String { let imports = plan.rewrites.len(); let files = group_by_file(&plan.rewrites).len(); let git = if via_git { " (via git mv)" } else { "" }; + let batch = match plan.moves.len() { + 1 => String::new(), + n => format!(" ({n} files)"), + }; + let left = match plan.left_behind.len() { + 0 => String::new(), + n => format!(", {} unsupported {} left behind", n, plural(n, "file")), + }; format!( - "moved {} -> {}{git}, updated {} {} in {} {}", + "moved {} -> {}{batch}{git}, updated {} {} in {} {}{left}", rel_str(&plan.source), rel_str(&plan.target), imports, @@ -186,3 +198,25 @@ pub(crate) fn plural(count: usize, noun: &str) -> String { format!("{noun}s") } } + +/// One unresolvable relative import found by `check`. +#[derive(Debug, Serialize)] +pub struct BrokenImport { + /// Project-relative file declaring the import. + pub file: String, + /// 1-based line of the specifier. + pub line: usize, + /// Specifier text as written. + pub import: String, + /// Stable reason code, currently always `"file_not_found"`. + pub reason: &'static str, +} + +/// Success payload of `check --json` (flattened under the envelope). +#[derive(Debug, Serialize)] +pub struct CheckData { + /// Broken imports, sorted by file then line. + pub broken_imports: Vec, + /// Number of broken imports (kept as an explicit counter for agents). + pub total: usize, +} diff --git a/src/core/apply/diff.rs b/src/core/apply/diff.rs index 7d36dc6..2bc8c91 100644 --- a/src/core/apply/diff.rs +++ b/src/core/apply/diff.rs @@ -18,8 +18,10 @@ use crate::core::{JmoveResult, rel_str}; pub fn render_diff(root: &Path, plan: &MovePlan) -> JmoveResult { let mut out = render_edits_diff(root, &group_by_file(plan))?; if !out.is_empty() { - let (src, dst) = (rel_str(&plan.source), rel_str(&plan.target)); - out.push_str(&format!("move {src} -> {dst}\n")); + for m in &plan.moves { + let (src, dst) = (rel_str(&m.source), rel_str(&m.target)); + out.push_str(&format!("move {src} -> {dst}\n")); + } } Ok(out) } @@ -47,7 +49,7 @@ pub fn render_edits_diff( mod tests { use super::render_diff; use crate::core::JmoveResult; - use crate::core::plan::{MovePlan, Rewrite}; + use crate::core::plan::{FileMove, MovePlan, Rewrite}; use std::path::Path; const OLD: &str = "import {\n fmt,\n} from '../lib/fmt';\n"; @@ -56,12 +58,25 @@ mod tests { MovePlan { source: "lib/fmt.ts".into(), target: "deep/fmt.ts".into(), + moves: vec![ + FileMove { + source: "lib/fmt.ts".into(), + target: "deep/fmt.ts".into(), + }, + // A second move proves the dir flavour renders one line each. + FileMove { + source: "lib/gfx.ts".into(), + target: "deep/gfx.ts".into(), + }, + ], rewrites: vec![Rewrite { file: "src/app.ts".into(), span: 24..34, old_text: "../lib/fmt".into(), new_text: "../deep/fmt".into(), }], + left_behind: Vec::new(), + prune_dirs: Vec::new(), } } @@ -74,7 +89,10 @@ mod tests { assert!(diff.contains("--- src/app.ts") && diff.contains("+++ src/app.ts")); assert!(diff.contains("@@")); assert!(diff.contains("-} from '../lib/fmt';") && diff.contains("+} from '../deep/fmt';")); - assert!(diff.ends_with("move lib/fmt.ts -> deep/fmt.ts\n")); + assert!( + diff.ends_with("move lib/fmt.ts -> deep/fmt.ts\nmove lib/gfx.ts -> deep/gfx.ts\n"), + "{diff}" + ); let mut empty = plan(); empty.rewrites.clear(); assert_eq!(render_diff(dir.path(), &empty)?, ""); diff --git a/src/core/apply/mod.rs b/src/core/apply/mod.rs index 5e1f679..ad0c172 100644 --- a/src/core/apply/mod.rs +++ b/src/core/apply/mod.rs @@ -22,7 +22,7 @@ use crate::core::Edit; use crate::core::apply::fsops::{ create_missing_dirs, group_by_file, rewrite_bytes, sibling_temp, write_durable, }; -use crate::core::plan::MovePlan; +use crate::core::plan::{FileMove, MovePlan}; use crate::core::{JmoveError, JmoveResult, rel_str}; /// Summary of a successfully applied plan. @@ -30,22 +30,22 @@ use crate::core::{JmoveError, JmoveResult, rel_str}; pub struct Applied { /// Number of files whose imports were rewritten. pub files_rewritten: usize, - /// The moved file's new project-relative path. + /// The moved file's new project-relative path (directory moves: the + /// requested destination, mirroring [`crate::core::plan::MovePlan::target`]). pub new_path: PathBuf, - /// Whether the physical rename went through `git mv`. + /// Whether every physical rename went through `git mv`. pub via_git: bool, } // Rollback state for one run: originals of rewritten files (newest last), -// dirs created for the target, and the final rename once it happened. +// dirs created for the targets, and the renames once they happened. #[derive(Default)] struct Run { root: PathBuf, backups: Vec<(PathBuf, Vec)>, dirs: Vec, - // Root-relative (src, dst) of the executed rename. - moved: Option<(PathBuf, PathBuf)>, - via_git: bool, + // Executed root-relative renames (src, dst, went-through-git), oldest first. + moved: Vec<(PathBuf, PathBuf, bool)>, } /// Apply `plan` under `root` atomically (see module docs). Rollback is @@ -80,25 +80,45 @@ impl Run { fn try_apply(&mut self, plan: &MovePlan, mode: GitMode) -> JmoveResult { let by_file = group_by_file(plan); self.write_all(&by_file)?; - // The move comes last, after every importer was rewritten. - let (src, dst) = (self.root.join(&plan.source), self.root.join(&plan.target)); - self.dirs = create_missing_dirs(&dst)?; - // git mv needs the destination dir to exist; tracked sources are - // renamed through git so the change lands staged in the index. - self.via_git = would_use_git(&self.root, &plan.source, mode); - if self.via_git { - git::mv(&self.root, &plan.source, &plan.target)?; - } else { - fs::rename(&src, &dst)?; + // The moves come last, after every importer was rewritten. A + // directory plan is applied file by file in sorted order; any + // failure rolls the whole batch back. + for m in &plan.moves { + self.move_one(mode, m)?; + } + // Directory moves leave their emptied source dirs behind otherwise; + // remove_dir only succeeds when truly empty, so `left_behind` files + // keep their home. This is the last step: nothing can fail after it. + for d in &plan.prune_dirs { + let _ = fs::remove_dir(self.root.join(d)); } - self.moved = Some((plan.source.clone(), plan.target.clone())); Ok(Applied { files_rewritten: by_file.len(), new_path: plan.target.clone(), - via_git: self.via_git, + via_git: !self.moved.is_empty() && self.moved.iter().all(|(_, _, g)| *g), }) } + fn move_one(&mut self, mode: GitMode, m: &FileMove) -> JmoveResult<()> { + let (src, dst) = (self.root.join(&m.source), self.root.join(&m.target)); + for created in create_missing_dirs(&dst)? { + self.dirs.push(created); + } + // git mv needs the destination dir to exist; tracked sources are + // renamed through git so the change lands staged in the index. + let via_git = would_use_git(&self.root, &m.source, mode); + let done = if via_git { + git::mv(&self.root, &m.source, &m.target) + .map(|()| (m.source.clone(), m.target.clone(), true)) + } else { + fs::rename(&src, &dst) + .map(|()| (m.source.clone(), m.target.clone(), false)) + .map_err(Into::into) + }; + self.moved.push(done?); + Ok(()) + } + fn try_fix(&mut self, by_file: &BTreeMap>) -> JmoveResult { self.write_all(by_file)?; Ok(by_file.len()) @@ -131,8 +151,12 @@ impl Run { // problems to its message. fn undo(&mut self, err: JmoveError) -> JmoveError { let mut problems = Vec::new(); - if let Some((src, dst)) = self.moved.take() { - let back = if self.via_git { + for (src, dst, via_git) in self.moved.drain(..).rev() { + // The prune step never runs before a failure, but a later move + // can fail after git mv created target dirs that a rollback + // through git may expect; ensure the original parent exists. + let _ = fs::create_dir_all(self.root.join(&src).parent().unwrap()); + let back = if via_git { git::mv(&self.root, &dst, &src) } else { fs::rename(self.root.join(&dst), self.root.join(&src)).map_err(Into::into) @@ -156,94 +180,3 @@ impl Run { JmoveError::Io(std::io::Error::other(msg)) } } - -#[cfg(test)] -mod tests { - use super::{GitMode, apply}; - use crate::core::JmoveResult; - use crate::core::plan::{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 = 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(), - rewrites: vec![rewrite], - } - } - - 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, crate::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, crate::core::JmoveError::StaleIndex(_)), - "{err}" - ); - assert_eq!(fs::read_to_string(root.join("src/app.ts"))?, other); - assert!(root.join("lib/fmt.ts").exists()); - Ok(()) - } -} diff --git a/src/core/plan/dir.rs b/src/core/plan/dir.rs new file mode 100644 index 0000000..fb02337 --- /dev/null +++ b/src/core/plan/dir.rs @@ -0,0 +1,226 @@ +//! Directory moves: relocate every indexed file under `source` into the +//! mirrored layout under `target`, merging the per-file rewrite plans. +//! +//! A directory move is exactly N file moves that must apply atomically +//! together, so it reuses the per-file planner and folds its rewrites; +//! a file that imports two moved classes from the same directory simply +//! gets both specifier edits. Files that exist on disk but are not +//! indexable (assets, binaries) cannot be rewritten and would silently +//! stay behind — they are reported as `left_behind` instead of being +//! moved blindly. + +use std::path::{Path, PathBuf}; + +use super::{FileMove, MovePlan, Rewrite}; +use crate::core::index::Index; +use crate::core::{JmoveError, JmoveResult, rel_str}; + +pub(super) fn plan_dir(index: &Index, source: &Path, target: &Path) -> JmoveResult { + let rejected = |msg: String| JmoveError::PlanRejected(format!("Directory move: {msg}")); + if source.starts_with(target) || target.starts_with(source) { + return Err(rejected(format!( + "'{}' and '{}' are nested; a directory cannot move into or out of itself", + rel_str(source), + rel_str(target) + ))); + } + let members: Vec = index + .files + .sorted() + .into_iter() + .filter(|f| f.starts_with(source)) + .collect(); + if members.is_empty() { + let s = rel_str(source); + return Err(JmoveError::InvalidArgument(format!( + "source '{s}' has no indexed source files" + ))); + } + let moves: Vec = members + .iter() + .map(|f| { + let dst = target.join(f.strip_prefix(source).unwrap_or(f)); + FileMove { + source: f.clone(), + target: dst, + } + }) + .collect(); + for m in &moves { + if index.files.contains(&m.target) { + let t = rel_str(&m.target); + return Err(rejected(format!("target '{t}' already exists"))); + } + } + let mut rewrites: Vec = moves + .iter() + .map(|m| super::file_rewrites(index, &m.source, &m.target)) + .collect::>>()? + .into_iter() + .flatten() + .collect(); + rewrites.sort_by_key(|r| (r.file.clone(), r.span.start)); + rewrites.dedup(); + let mut prune_dirs = prune_chain(source, &members); + prune_dirs.sort_by_key(|d| std::cmp::Reverse(d.components().count())); + Ok(MovePlan { + source: source.to_path_buf(), + target: target.to_path_buf(), + moves, + rewrites, + left_behind: unsupported_left_behind(index, source), + prune_dirs, + }) +} + +// Every directory that loses files: the moved members' parent chains up to +// and including `source` itself. +fn prune_chain(source: &Path, members: &[PathBuf]) -> Vec { + let mut out: Vec = Vec::new(); + for m in members { + let mut dir = m.parent().unwrap_or(Path::new("")).to_path_buf(); + loop { + if dir == source { + if !out.contains(&source.to_path_buf()) { + out.push(source.to_path_buf()); + } + break; + } + if !out.contains(&dir) { + out.push(dir.clone()); + } + let Some(up) = dir.parent() else { break }; + dir = up.to_path_buf(); + } + } + out +} + +// Real files anywhere under `source` that the index does not know (and +// therefore this plan will not move). Walked with the same gitignore rules +// as the scanner; hidden paths are nobody's source files and skipped. +fn unsupported_left_behind(index: &Index, source: &Path) -> Vec { + let mut out = Vec::new(); + let walk = ignore::WalkBuilder::new(index.root.join(source)) + .require_git(false) + .build(); + for entry in walk.flatten() { + if entry.path_is_symlink() || !entry.file_type().is_some_and(|t| t.is_file()) { + continue; + } + let Ok(stripped) = entry.path().strip_prefix(&index.root) else { + continue; + }; + let Some(rel) = crate::core::normalize_rel_path(stripped) else { + continue; + }; + let hidden = rel + .components() + .any(|c| c.as_os_str().to_string_lossy().starts_with('.')); + if !hidden && !index.files.contains(&rel) { + out.push(rel); + } + } + out.sort(); + out +} + +/// Shared `#[cfg(test)]` graph builders for the plan submodules. +#[cfg(test)] +pub(crate) mod tests_support { + use std::ops::Range; + use std::path::PathBuf; + + use crate::core::index::ResolvedImport; + use crate::parser::ImportRecord; + + /// Hand-wired resolved edge: plan tests never touch the parser. + pub(crate) fn edge(spec: &str, span: Range, target: &str) -> ResolvedImport { + let record = ImportRecord { + specifier: spec.into(), + span, + is_dynamic: false, + }; + ResolvedImport { + record, + target: Some(PathBuf::from(target)), + } + } +} + +#[cfg(test)] +mod tests { + use crate::core::JmoveResult; + use crate::core::index::Index; + use crate::core::plan::plan_move; + use std::fs; + use std::path::{Path, PathBuf}; + + fn project() -> JmoveResult { + let dir = tempfile::TempDir::new()?; + let root = dir.path().to_path_buf(); + let write = |rel: &str, body: &str| -> JmoveResult<()> { + let p = root.join(rel); + fs::create_dir_all(p.parent().unwrap())?; + fs::write(p, body)?; + Ok(()) + }; + write("docs/notes.md", "not source\n")?; + write("lib/a.ts", "export const a = 1;\n")?; + write("lib/b.ts", "export const b = 2;\n")?; + write("lib/data.json", "{}\n")?; + write( + "app.ts", + "import { a } from './lib/a';\nimport { b } from './lib/b';\n", + )?; + write("deep/c.ts", "import { a } from '../lib/a';\n")?; + Ok(dir) + } + + #[test] + fn dir_plan_moves_every_indexed_file_and_merges_rewrites() -> JmoveResult<()> { + let dir = project()?; + let index = Index::build(dir.path())?; + let plan = plan_move(&index, Path::new("lib"), Path::new("core/lib"))?; + assert_eq!(plan.prune_dirs, [PathBuf::from("lib")]); + let moves: Vec<(&str, &str)> = plan + .moves + .iter() + .map(|m| (m.source.to_str().unwrap(), m.target.to_str().unwrap())) + .collect(); + assert_eq!( + moves, + [("lib/a.ts", "core/lib/a.ts"), ("lib/b.ts", "core/lib/b.ts"),] + ); + // app.ts (root) imports both moved modules: two merged specifier + // edits; deep/c.ts gets its own relative rewrite. + let app: Vec<_> = plan + .rewrites + .iter() + .filter(|r| r.file == Path::new("app.ts")) + .map(|r| r.new_text.as_str()) + .collect(); + assert_eq!(app, ["./core/lib/a", "./core/lib/b"]); + // The unindexed JSON would stay on disk: report, never silently move. + assert_eq!(plan.left_behind, [PathBuf::from("lib/data.json")]); + Ok(()) + } + + #[test] + fn dir_plan_rejects_nested_and_empty_moves() -> JmoveResult<()> { + let dir = project()?; + let index = Index::build(dir.path())?; + let err = plan_move(&index, Path::new("lib"), Path::new("lib/sub")).unwrap_err(); + assert!( + matches!(err, crate::core::JmoveError::PlanRejected(_)), + "{err}" + ); + // `docs` exists but holds no indexable file: nothing to move. + let err = plan_move(&index, Path::new("docs"), Path::new("core")).unwrap_err(); + assert!( + matches!(err, crate::core::JmoveError::InvalidArgument(_)), + "{err}" + ); + Ok(()) + } +} diff --git a/src/core/plan/mod.rs b/src/core/plan/mod.rs index 4c0e580..8fae7b8 100644 --- a/src/core/plan/mod.rs +++ b/src/core/plan/mod.rs @@ -2,12 +2,14 @@ //! //! A plan is pure data (no disk writes), so dry-run and `--json` can render //! it without touching the filesystem. Specifier arithmetic lives in -//! [`specifier`]; the Java package/directory flavour in [`java`]. +//! [`specifier`]; the Java package/directory flavour in [`java`], and the +//! mirrored batch move of a whole directory in [`dir`]. +mod dir; mod java; -mod specifier; #[cfg(test)] -pub(crate) mod tests_support; +pub(crate) use dir::tests_support; +mod specifier; pub use specifier::relative_specifier; @@ -42,24 +44,44 @@ impl From<&Rewrite> for Edit { } } -/// Complete plan for moving `source` to `target`. +/// One physical file relocation inside a plan. #[derive(Debug, Clone, PartialEq, Eq)] -pub struct MovePlan { - /// Project-relative path being moved. +pub struct FileMove { + /// Project-relative file being moved. pub source: PathBuf, /// Project-relative destination path. pub target: PathBuf, - /// Specifier rewrites, sorted by (file, span). - pub rewrites: Vec, } -/// Compute the rewrite plan for `source -> target`. +/// Complete plan for moving `source` to `target`: a file move produces one +/// [`FileMove`], a directory move one per indexed member (see [`dir`]). +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct MovePlan { + /// Requested source: the file, or the directory whose members move. + pub source: PathBuf, + /// Requested destination. + pub target: PathBuf, + /// Every physical relocation, in sorted source order. + pub moves: Vec, + /// Specifier/package rewrites, merged across moves, sorted by (file, span). + pub rewrites: Vec, + /// Directory moves only: real files under `source` that no plan step + /// moves (unindexable assets) — reported, never silently relocated. + pub left_behind: Vec, + /// Directory moves only: source directories to prune (deepest first) + /// once every move landed. Removal only succeeds when a directory is + /// empty, so `left_behind` files keep their home in place — exactly right. + pub prune_dirs: Vec, +} + +/// Compute the rewrite plan for `source -> target`; a `source` that is a +/// directory on disk becomes a mirrored move of all indexed members below it. /// -/// TS/JS: every indexed import whose resolved target equals `source` gets a -/// new relative specifier from the importer's directory to `target` (see -/// [`relative_specifier`]). Java moves additionally rewrite the moved file's -/// `package` declaration (see [`java`]). Rewrites whose result equals the -/// old specifier are dropped; the result is sorted by (file, span). +/// TS/JS: every indexed import whose resolved target equals a moved file gets +/// a new relative specifier from the importer's directory to its destination +/// (see [`relative_specifier`]). Java moves additionally rewrite the moved +/// file's `package` declaration (see [`java`]). Rewrites whose result equals +/// the old specifier are dropped; the result is sorted by (file, span). pub fn plan_move(index: &Index, source: &Path, target: &Path) -> JmoveResult { let rel = |label: &str, p: &Path| match normalize_rel_path(p) { Some(r) => Ok(r), @@ -69,36 +91,54 @@ pub fn plan_move(index: &Index, source: &Path, target: &Path) -> JmoveResult JmoveResult> { + Ok( + if SourceLanguage::for_path(source) == Some(SourceLanguage::Java) { + java::java_rewrites(index, source, target)? + } else { + ts_rewrites(index, source, target) + }, + ) +} + // Relative-specifier rewrites for the TS/JS flavour of the graph. fn ts_rewrites(index: &Index, source: &Path, target: &Path) -> Vec { let mut rewrites = Vec::new(); @@ -126,10 +166,10 @@ fn ts_rewrites(index: &Index, source: &Path, target: &Path) -> Vec { #[cfg(test)] mod tests { + use super::tests_support::edge; use super::{Rewrite, plan_move}; use crate::core::JmoveError; use crate::core::index::{Index, ResolvedImport}; - use crate::core::plan::tests_support::edge; use std::path::{Path, PathBuf}; fn index_with(files: &[&str], imports: &[(&str, Vec)]) -> Index { @@ -170,6 +210,7 @@ mod tests { new_text: "./sub/deep/s".into(), } ); + assert_eq!(plan.moves.len(), 1); } #[test] diff --git a/src/core/plan/tests_support.rs b/src/core/plan/tests_support.rs deleted file mode 100644 index ac157ef..0000000 --- a/src/core/plan/tests_support.rs +++ /dev/null @@ -1,20 +0,0 @@ -//! Shared `#[cfg(test)]` graph builders for the plan submodules. - -use std::ops::Range; -use std::path::PathBuf; - -use crate::core::index::ResolvedImport; -use crate::parser::ImportRecord; - -/// Hand-wired resolved edge: plan tests never touch the parser. -pub(crate) fn edge(spec: &str, span: Range, target: &str) -> ResolvedImport { - let record = ImportRecord { - specifier: spec.into(), - span, - is_dynamic: false, - }; - ResolvedImport { - record, - target: Some(PathBuf::from(target)), - } -} diff --git a/tests/apply.rs b/tests/apply.rs new file mode 100644 index 0000000..f6863ff --- /dev/null +++ b/tests/apply.rs @@ -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 = 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(()) +} diff --git a/tests/cli_dir.rs b/tests/cli_dir.rs new file mode 100644 index 0000000..1da2797 --- /dev/null +++ b/tests/cli_dir.rs @@ -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" + ); +} diff --git a/tests/cli_fix.rs b/tests/cli_fix.rs index 1ecb885..0a7d1b8 100644 --- a/tests/cli_fix.rs +++ b/tests/cli_fix.rs @@ -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") } diff --git a/tests/cli_git.rs b/tests/cli_git.rs index 8dc1566..2c38b17 100644 --- a/tests/cli_git.rs +++ b/tests/cli_git.rs @@ -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" + ); +} diff --git a/tests/cli_missing_import.rs b/tests/cli_missing_import.rs new file mode 100644 index 0000000..4b7f017 --- /dev/null +++ b/tests/cli_missing_import.rs @@ -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")); +} diff --git a/todo.md b/todo.md index fc464a9..7573fc8 100644 --- a/todo.md +++ b/todo.md @@ -91,7 +91,7 @@ AI оставляем СНАРУЖИ: при неоднозначности jmov - [ ] Поддержка tsconfig paths / алиасов (@/...) - [ ] Параллельная индексация через rayon - [x] --git интеграция (git mv для stage/истории): auto для tracked файлов, --no-git флаг, moved_via/would_move_via в --json -- [ ] Перенос директорий целиком (mv папки) +- [x] Перенос директорий целиком (mv папки): зеркальный batch-move всех индексируемых файлов, merged rewrites, prune пустых исходных каталогов, left_behind для неиндексируемых - [ ] Предупреждения о не-import ссылках: package.json exports, jest mocks, tsconfig includes, markdown links - [ ] prettier интеграция после rewrite (по желанию)