feat: global --source-root — index/mv/check/fix one subtree; deterministic Java FQN resolution in monorepos with duplicate packages (guava vs android/guava); move mv_reject to output.rs

This commit is contained in:
loki5512344 2026-09-15 16:48:57 +02:00
parent 7f568fd0c9
commit d5d34afb00
Signed by: boba
GPG key ID: 253067914055423B
13 changed files with 278 additions and 64 deletions

View file

@ -4,9 +4,9 @@ Concrete examples for both audiences: humans at a terminal and AI agents
consuming `--json`. All outputs below are captured from the real binary.
Flags (see `jmove --help`): `mv <source> <target> [--dry-run] [--json]
[--no-git]`, `check [--json]`, and the global `--root <DIR>`. Paths may be
relative to the root or absolute inside it; `.gitignore`d files are never
indexed. Inside a git repo, `mv` of a tracked file uses `git mv` (the
[--no-git]`, `check [--json]`, and the global `--root <DIR>` /
`--source-root <DIR>` (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
rename is staged); `--no-git` forces a plain filesystem rename.
Exit codes: `0` ok · `1` operation error · `2` `check` found broken imports.

View file

@ -18,7 +18,7 @@ kept in sync); Python/Go on the roadmap. Single binary, no LSP needed.
### mv — move a file and rewrite its importers
```
jmove mv <source> <target> [--root DIR] [--dry-run] [--json] [--no-git]
jmove mv <source> <target> [--root DIR] [--source-root DIR] [--dry-run] [--json] [--no-git]
```
Always run `--dry-run` first and confirm the change set looks right.
@ -80,6 +80,12 @@ without `--dry-run`. For Java projects the practical loop is: `mv` →
- `check` never reports unresolved Java imports (jdk, third-party,
`pkg.*`) as broken — they are external by design, like TS bare
specifiers.
- Monorepos with duplicate packages (`guava` vs `android/guava` under one
`--root`): the same FQN exists in parallel trees, so resolution picks the
sorted-first copy and a move rewrites the wrong files. Pass the global
`--source-root DIR` to index (and fix/move) only that subtree; each tree
is then self-contained and correct. Without the flag behavior is
unchanged; `--source-root` on a non-directory exits 1.
## Recommended agent workflow

View file

@ -71,12 +71,20 @@ pub struct FixDryRunData {
}
/// `fix` handler: validate rule, index, plan, then dry-run or apply.
pub fn fix(root: &Path, rule: Option<&str>, dry_run: bool, json: bool) -> Flow<i32> {
pub fn fix(
root: &Path,
source_root: Option<&Path>,
rule: Option<&str>,
dry_run: bool,
json: bool,
) -> Flow<i32> {
let root = flow(json, "fix", root.canonicalize().map_err(JmoveError::from))?;
let source_root = flow(json, "fix", super::normalize_scope(&root, source_root))?;
if let Some(rejected) = fix_reject(rule) {
return Err(fail(json, "fix", rejected));
}
let index = flow(json, "fix", Index::build(&root))?;
let scope = source_root.as_deref();
let index = flow(json, "fix", Index::build_scoped(&root, scope))?;
let plan = plan_fix(&index, rule);
if plan.is_empty() {
if json {

View file

@ -29,6 +29,10 @@ pub struct Args {
/// Project root (defaults to the current directory).
#[arg(long, global = true, default_value = ".")]
pub root: PathBuf,
/// Index only files under this root-relative directory: the monorepo
/// disambiguator for duplicate Java packages (`guava` vs `android/guava`).
#[arg(long, global = true)]
pub source_root: Option<PathBuf>,
}
/// Available subcommands (MVP: `mv`, `check`).
@ -95,13 +99,27 @@ pub fn run() -> anyhow::Result<i32> {
dry_run,
json,
no_git,
} => mv(&args.root, &source, &target, dry_run, json, no_git),
Command::Check { json } => check(&args.root, json),
} => mv(
&args.root,
args.source_root.as_deref(),
&source,
&target,
dry_run,
json,
no_git,
),
Command::Check { json } => check(&args.root, args.source_root.as_deref(), json),
Command::Fix {
rule,
dry_run,
json,
} => fix::fix(&args.root, rule.as_deref(), dry_run, json),
} => fix::fix(
&args.root,
args.source_root.as_deref(),
rule.as_deref(),
dry_run,
json,
),
};
// Handlers report their own failures; both arms carry an exit code.
Ok(outcome.unwrap_or_else(identity))
@ -110,6 +128,7 @@ pub fn run() -> anyhow::Result<i32> {
/// `mv` handler: normalize paths, validate, index, plan, then dry-run or apply.
fn mv(
root: &Path,
source_root: Option<&Path>,
source: &Path,
target: &Path,
dry_run: bool,
@ -118,13 +137,15 @@ fn mv(
) -> Flow<i32> {
let git = apply::GitMode::from_no_git(no_git);
let root = flow(json, "mv", root.canonicalize().map_err(JmoveError::from))?;
let source_root = flow(json, "mv", normalize_scope(&root, source_root))?;
let source = flow(json, "mv", core::rel_from_root(&root, source))?;
let target = flow(json, "mv", core::rel_from_root(&root, target))?;
if let Some(rejected) = mv_reject(&root, &source, &target) {
if let Some(rejected) = output::mv_reject(&root, &source, &target) {
return Err(fail(json, "mv", rejected));
}
let index = flow(json, "mv", Index::build(&root))?;
let scope = source_root.as_deref();
let index = flow(json, "mv", Index::build_scoped(&root, scope))?;
let plan = flow(json, "mv", plan::plan_move(&index, &source, &target))?;
if dry_run {
return mv_dry_run(&root, json, &plan, git);
@ -149,48 +170,6 @@ fn mv(
Ok(exit::OK)
}
/// Pre-flight `mv` validation. A file that exists on disk but is absent
/// from the import index stays moveable: its plan simply has no rewrites.
fn mv_reject(root: &Path, source: &Path, target: &Path) -> Option<ErrorData> {
let bad = |code: &str, message: String, hint: &str| {
Some(ErrorData::new(code, message, Some(hint.into())))
};
if source == target {
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 msg = format!("source file '{}' does not exist", core::rel_str(source));
return bad(
"SOURCE_NOT_FOUND",
msg,
"check the path or run `jmove check`",
);
}
if root.join(target).exists() {
let msg = format!("target path '{}' already exists", core::rel_str(target));
return bad(
"TARGET_EXISTS",
msg,
"remove or rename the existing target first",
);
}
// `target` names a file, so `parent()` always yields the directory part.
let parent = root.join(target.parent().unwrap_or(Path::new("")));
if parent.exists() && !parent.is_dir() {
let msg = format!(
"target parent of '{}' is not a directory",
core::rel_str(target)
);
return bad(
"INVALID_ARGUMENT",
msg,
"pick a destination inside a directory",
);
}
None
}
/// Dry-run branch: unified diff for humans, structured preview for agents.
fn mv_dry_run(root: &Path, json: bool, plan: &MovePlan, git: apply::GitMode) -> Flow<i32> {
let diff = flow(json, "mv", apply::render_diff(root, plan))?;
@ -211,9 +190,11 @@ fn mv_dry_run(root: &Path, json: bool, plan: &MovePlan, git: apply::GitMode) ->
/// Exit code is `2` when at least one broken import was found, in both the
/// human and the `--json` mode (the JSON `status` stays `"ok"` — the
/// command itself succeeded; agents read `total` or the exit code).
fn check(root: &Path, json: bool) -> Flow<i32> {
fn check(root: &Path, source_root: Option<&Path>, json: bool) -> Flow<i32> {
let root = flow(json, "check", root.canonicalize().map_err(JmoveError::from))?;
let index = flow(json, "check", Index::build(&root))?;
let source_root = flow(json, "check", normalize_scope(&root, source_root))?;
let scope = source_root.as_deref();
let index = flow(json, "check", Index::build_scoped(&root, scope))?;
let broken = flow(json, "check", output::broken_imports(&root, &index))?;
let code = if broken.is_empty() {
exit::OK
@ -234,6 +215,21 @@ fn check(root: &Path, json: bool) -> Flow<i32> {
Ok(code)
}
/// Validate the global `--source-root`: project-relative, existing dir.
fn normalize_scope(root: &Path, scope: Option<&Path>) -> JmoveResult<Option<PathBuf>> {
let Some(scope) = scope else {
return Ok(None);
};
let rel = core::rel_from_root(root, scope)?;
if !root.join(&rel).is_dir() {
return Err(JmoveError::InvalidArgument(format!(
"--source-root '{}' is not a directory",
core::rel_str(&rel)
)));
}
Ok(Some(rel))
}
/// Unwrap a core result, routing failures through the CLI error channel.
fn flow<T>(json: bool, operation: &'static str, result: JmoveResult<T>) -> Flow<T> {
result.map_err(|err| fail(json, operation, ErrorData::from_core(&err)))

View file

@ -11,7 +11,7 @@ use crate::core::index::Index;
use crate::core::plan::{MovePlan, Rewrite};
use crate::core::{JmoveResult, rel_str};
use super::json::{BrokenImport, Change, ChangedFile};
use super::json::{BrokenImport, Change, ChangedFile, ErrorData};
/// `check` stdout line when the project has no broken imports.
const CHECK_CLEAN: &str = "check: no broken imports found";
@ -95,6 +95,47 @@ pub fn changed_files(root: &Path, plan: &MovePlan) -> JmoveResult<Vec<ChangedFil
.collect())
}
/// Pre-flight `mv` validation: `(root, source, target)` are project-relative
/// and normalized. A file that exists on disk but is absent from the import
/// index stays moveable: its plan simply has no rewrites.
#[must_use]
pub fn mv_reject(root: &Path, source: &Path, target: &Path) -> Option<ErrorData> {
let bad = |code: &str, message: String, hint: &str| {
Some(ErrorData::new(code, message, Some(hint.into())))
};
if source == target {
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 msg = format!("source file '{}' does not exist", rel_str(source));
return bad(
"SOURCE_NOT_FOUND",
msg,
"check the path or run `jmove check`",
);
}
if root.join(target).exists() {
let msg = format!("target path '{}' already exists", rel_str(target));
return bad(
"TARGET_EXISTS",
msg,
"remove or rename the existing target first",
);
}
// `target` names a file, so `parent()` always yields the directory part.
let parent = root.join(target.parent().unwrap_or(Path::new("")));
if parent.exists() && !parent.is_dir() {
let msg = format!("target parent of '{}' is not a directory", rel_str(target));
return bad(
"INVALID_ARGUMENT",
msg,
"pick a destination inside a directory",
);
}
None
}
/// `moved src -> tgt, updated N imports in M files` success summary,
/// noting when the rename went through `git mv`.
#[must_use]

View file

@ -54,6 +54,15 @@ impl Index {
/// Scan `root`, parse every supported source file and build the graph.
/// Unreadable or unparseable files are skipped, not fatal.
pub fn build(root: &Path) -> JmoveResult<Self> {
Self::build_scoped(root, None)
}
/// Like [`build`](Self::build), but when `source_root` (project-relative,
/// normalized) is `Some`, only files under that subtree are indexed. This
/// is the monorepo escape hatch: `guava` vs `android/guava` declare the
/// same FQNs, and scoping the index to one self-contained copy makes
/// collision-free resolution (and therefore `mv`/`fix` rewrites) exact.
pub fn build_scoped(root: &Path, source_root: Option<&Path>) -> JmoveResult<Self> {
let root = root.canonicalize()?;
let mut index = Self {
root,
@ -62,7 +71,7 @@ impl Index {
packages: HashMap::new(),
java_classes: JavaClassIndex::default(),
};
index.scan()?;
index.scan(source_root)?;
// Resolution needs the complete file set (extension/index guessing)
// and the full package map, so it runs as a second pass.
let java_classes = JavaClassIndex::new(&index.files, &index.packages);
@ -83,11 +92,17 @@ impl Index {
}
// Walk the project and parse each supported source file, staging the
// raw records with `target: None` for the resolution pass above.
fn scan(&mut self) -> JmoveResult<()> {
// Sorted map: deterministic discovery order.
// raw records with `target: None` for the resolution pass above. When
// `source_root` (project-relative, normalized) is set, only files under
// that subtree are indexed — see [`Index::build_scoped`].
fn scan(&mut self, source_root: Option<&Path>) -> JmoveResult<()> {
// Sorted map: deterministic discovery order. Walking starts at the
// scoped subtree when set, so the rest of the monorepo is not even
// opened; paths stay root-relative because the prefix removed is
// always `self.root`.
let mut found: BTreeMap<PathBuf, SourceLanguage> = BTreeMap::new();
for entry in WalkBuilder::new(&self.root).require_git(false).build() {
let base = source_root.map_or(self.root.clone(), |scope| self.root.join(scope));
for entry in WalkBuilder::new(base).require_git(false).build() {
// Walker errors (unreadable dirs, etc.) simply skip the entry.
let Ok(entry) = entry else { continue };
if entry.path_is_symlink() || !entry.file_type().is_some_and(|t| t.is_file()) {

View file

@ -119,3 +119,81 @@ fn importers_of_returns_sorted_reverse_edges() -> JmoveResult<()> {
assert!(index.importers_of(Path::new("missing.ts")).is_empty());
Ok(())
}
// The guava monorepo shape: `android/guava` and `guava` both declare
// `com.example.App`, so an unscoped index resolves every importer to the
// same (sorted-first) copy and a move in one tree rewrites the wrong files.
fn monorepo() -> JmoveResult<tempfile::TempDir> {
let dir = tempfile::TempDir::new()?;
let root = dir.path();
for tree in ["guava", "android/guava"] {
write_file(
root,
&format!("{tree}/src/com/example/App.java"),
"package com.example;\npublic class App {}\n",
)?;
write_file(
root,
&format!("{tree}/src/com/example/Use.java"),
"package com.example;\nimport com.example.App;\nclass Use { App a; }\n",
)?;
}
Ok(dir)
}
fn resolved_target(index: &Index, importer: &str) -> Option<PathBuf> {
index
.imports
.get(Path::new(importer))?
.iter()
.find_map(|r| r.target.clone())
}
#[test]
fn unscoped_index_resolves_the_duplicate_fqn_to_the_sorted_first_tree() -> JmoveResult<()> {
let dir = monorepo()?;
let index = Index::build(dir.path())?;
// Both trees' `App` collide; the class index keeps `android/guava` (sorts first).
assert_eq!(
resolved_target(&index, "guava/src/com/example/Use.java").as_deref(),
Some(Path::new("android/guava/src/com/example/App.java")),
"demonstrates the collision the flag exists to fix"
);
Ok(())
}
#[test]
fn source_root_scopes_indexing_so_the_fqn_resolves_within_the_tree() -> JmoveResult<()> {
let dir = monorepo()?;
let index = Index::build_scoped(dir.path(), Some(Path::new("guava")))?;
// Only the guava tree is indexed at all.
assert!(
index
.files
.contains(Path::new("guava/src/com/example/App.java"))
);
assert!(
!index
.files
.contains(Path::new("android/guava/src/com/example/App.java"))
);
assert_eq!(
resolved_target(&index, "guava/src/com/example/Use.java").as_deref(),
Some(Path::new("guava/src/com/example/App.java")),
"the scoped index resolves to the same tree"
);
Ok(())
}
#[test]
fn source_root_does_not_leak_siblings_sharing_a_name_prefix() -> JmoveResult<()> {
// `--source-root guava` must match the directory `guava`, not `guavaX`.
let dir = tempfile::TempDir::new()?;
let root = dir.path();
write_file(root, "guava/src/A.java", "package p;\npublic class A {}\n")?;
write_file(root, "guavax/src/B.java", "package p;\npublic class B {}\n")?;
let index = Index::build_scoped(root, Some(Path::new("guava")))?;
assert!(index.files.contains(Path::new("guava/src/A.java")));
assert!(!index.files.contains(Path::new("guavax/src/B.java")));
Ok(())
}

View file

@ -149,3 +149,56 @@ fn default_package_file_cannot_change_directory() {
.success();
assert!(in_root(tmp.path(), "src/main/java/Run.java").is_file());
}
fn monorepo() -> tempfile::TempDir {
fixture("monorepo")
}
#[test]
fn source_root_scopes_the_move_to_one_duplicate_tree() {
let tmp = monorepo();
jmove(
&tmp,
&[
"mv",
"--source-root",
"guava",
"guava/src/com/example/Primitives.java",
"guava/src/com/example/util/Primitives.java",
],
)
.success()
.stdout(predicate::str::contains("updated 2 imports in 2 files"));
// The scoped tree moved and its importer followed the new package.
let app = read(&in_root(tmp.path(), "guava/src/com/example/app/App.java"));
assert!(app.contains("import com.example.util.Primitives;"), "{app}");
let moved = read(&in_root(
tmp.path(),
"guava/src/com/example/util/Primitives.java",
));
assert!(moved.contains("package com.example.util;"), "{moved}");
// The sibling copy is a self-contained tree: not one byte touched.
let android = read(&in_root(
tmp.path(),
"android/guava/src/com/example/app/App.java",
));
assert!(
android.contains("import com.example.Primitives;"),
"{android}"
);
assert!(in_root(tmp.path(), "android/guava/src/com/example/Primitives.java").is_file());
jmove(&tmp, &["check", "--source-root", "guava"]).success();
}
#[test]
fn unknown_source_root_is_rejected() {
let tmp = monorepo();
jmove(&tmp, &["check", "--source-root", "nope"])
.failure()
.code(1)
.stderr(predicate::str::contains(
"--source-root 'nope' is not a directory",
));
}

View file

@ -0,0 +1,3 @@
package com.example;
public class Primitives {}

View file

@ -0,0 +1,5 @@
package com.example.app;
import com.example.Primitives;
public class App { Primitives p; }

View file

@ -0,0 +1,3 @@
package com.example;
public class Primitives {}

View file

@ -0,0 +1,5 @@
package com.example.app;
import com.example.Primitives;
public class App { Primitives p; }

View file

@ -74,9 +74,10 @@ AI оставляем СНАРУЖИ: при неоднозначности jmov
- [ ] (после fix) повторить обе перемещения как `mv` + авто-`fix` и добить compile до SUCCESS
(паттерн воспроизведён и закрыт локально: mv файла с bare-ссылкой на соседний пакет →
`fix` добавил импорт → javac SUCCESS; на реальном guava ещё не прогонялось)
- [ ] Индексация в monorepo с дублями пакетов (guava vs android/guava в одном --root):
FQN-коллизии → class_index оставляет первый по сортировке, импорт резолвится не туда.
Нужен выбор/фильтр source root (например `--source-root` или авто-определение по mv-цели)
- [x] Индексация в monorepo с дублями пакетов (guava vs android/guava в одном --root):
глобальный `--source-root DIR` — индексирует (mv/check/fix) только поддерево,
FQN-коллизии исчезают, соседнее дерево не трогается; авто-определение по mv-цели
осознанно НЕ делаем (явный флаг предсказуемее, см. KISS)
## Phase 2
- [ ] Кэш индекса на диске (bincode/rkyv) → .jmove/index