From b96deeaf2f42700b85ea078d56c62a495822a00f Mon Sep 17 00:00:00 2001 From: loki5512344 Date: Tue, 15 Sep 2026 18:02:45 +0200 Subject: [PATCH 01/10] =?UTF-8?q?docs:=20guava=20real-world=20re-run=20gre?= =?UTF-8?q?en=20=E2=80=94=20mv+git-mv+fix+compile=20SUCCESS=20with=20--sou?= =?UTF-8?q?rce-root?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- todo.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/todo.md b/todo.md index 2d62afb..fc464a9 100644 --- a/todo.md +++ b/todo.md @@ -75,10 +75,12 @@ AI оставляем СНАРУЖИ: при неоднозначности jmov явных импорта корректно, НО javac упал: сам перенесённый файл ссылался на соседний `GwtCompatible` БЕЗ импорта (тот же пакет) → после mv ссылка битая. jmove в v1 осознанно НЕ добавляет импорты. Это главный driver для fix/missing-import из Phase 1.6 выше -- [ ] (после fix) повторить обе перемещения как `mv` + авто-`fix` и добить compile до SUCCESS - (паттерн воспроизведён и закрыт локально: mv файла с bare-ссылкой на соседний пакет → - `fix` добавил импорт → javac SUCCESS; на реальном guava ещё не прогонялось) -- [x] Индексация в monorepo с дублями пакетов (guava vs android/guava в одном --root): +- [x] Обе перемещения повторены как `mv` + авто-`fix`, compile = BUILD SUCCESS (guava main, JDK21): + VisibleForTesting annotations→annotations.testing: mv без --source-root давал 1 правку (62 потеряны!), + с --source-root guava — 63/63 через git mv; fix добавил в перенесённый файл + `import com.google.common.annotations.GwtCompatible` (тот самый разрыв v1) + 36 import-order + (сошёлся за 2 прогона); Primitives primitives→util: 5 правок + fix; `jmove check` = 0 broken +- [x] Индексация в monorepo с дублями пакетов — ПРОВЕРЕНО НА guava (см. выше): глобальный `--source-root DIR` — индексирует (mv/check/fix) только поддерево, FQN-коллизии исчезают, соседнее дерево не трогается; авто-определение по mv-цели осознанно НЕ делаем (явный флаг предсказуемее, см. KISS) From 6f7a2190e6a92a5435bea92308ba4fa3267d95b6 Mon Sep 17 00:00:00 2001 From: loki5512344 Date: Tue, 15 Sep 2026 18:30:33 +0200 Subject: [PATCH 02/10] =?UTF-8?q?feat:=20directory=20moves=20=E2=80=94=20m?= =?UTF-8?q?irrored=20batch=20relocation=20of=20every=20indexed=20file=20un?= =?UTF-8?q?der=20a=20dir?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- README.md | 3 + docs/EXAMPLES.md | 4 +- docs/SKILL.md | 11 +- src/cli/json.rs | 68 ++++++---- src/cli/mod.rs | 4 +- src/cli/output.rs | 42 +++++- src/core/apply/diff.rs | 26 +++- src/core/apply/mod.rs | 157 +++++++---------------- src/core/plan/dir.rs | 226 +++++++++++++++++++++++++++++++++ src/core/plan/mod.rs | 97 ++++++++++---- src/core/plan/tests_support.rs | 20 --- tests/apply.rs | 170 +++++++++++++++++++++++++ tests/cli_dir.rs | 98 ++++++++++++++ tests/cli_fix.rs | 93 -------------- tests/cli_git.rs | 18 +++ tests/cli_missing_import.rs | 96 ++++++++++++++ todo.md | 2 +- 17 files changed, 842 insertions(+), 293 deletions(-) create mode 100644 src/core/plan/dir.rs delete mode 100644 src/core/plan/tests_support.rs create mode 100644 tests/apply.rs create mode 100644 tests/cli_dir.rs create mode 100644 tests/cli_missing_import.rs 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 (по желанию) From 4e78a1c2b810a54fa8baf82f174ceae7f6c57bb9 Mon Sep 17 00:00:00 2001 From: loki5512344 Date: Tue, 15 Sep 2026 18:33:24 +0200 Subject: [PATCH 03/10] test(dir): compare planned paths via rel_str so Windows backslash display cannot leak into assertions --- src/core/plan/dir.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/core/plan/dir.rs b/src/core/plan/dir.rs index fb02337..ef04c1c 100644 --- a/src/core/plan/dir.rs +++ b/src/core/plan/dir.rs @@ -183,10 +183,16 @@ mod tests { 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 + // rel_str: canonical '/' display, stable across platforms. + let moves: Vec<(String, String)> = plan .moves .iter() - .map(|m| (m.source.to_str().unwrap(), m.target.to_str().unwrap())) + .map(|m| { + ( + crate::core::rel_str(&m.source), + crate::core::rel_str(&m.target), + ) + }) .collect(); assert_eq!( moves, From c0b06efe57c37fd814fb9f52123bab55a20db92d Mon Sep 17 00:00:00 2001 From: loki5512344 Date: Tue, 15 Sep 2026 18:35:56 +0200 Subject: [PATCH 04/10] fix(test): make dir-plan assertions type-comparable string tuples (missed in previous commit) --- src/core/plan/dir.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/core/plan/dir.rs b/src/core/plan/dir.rs index ef04c1c..5118ecb 100644 --- a/src/core/plan/dir.rs +++ b/src/core/plan/dir.rs @@ -196,7 +196,10 @@ mod tests { .collect(); assert_eq!( moves, - [("lib/a.ts", "core/lib/a.ts"), ("lib/b.ts", "core/lib/b.ts"),] + [ + ("lib/a.ts".to_string(), "core/lib/a.ts".to_string()), + ("lib/b.ts".into(), "core/lib/b.ts".into()), + ] ); // app.ts (root) imports both moved modules: two merged specifier // edits; deep/c.ts gets its own relative rewrite. From f8004e669bc3639c4cee6a8da68e0133baf0f1a1 Mon Sep 17 00:00:00 2001 From: loki5512344 Date: Tue, 15 Sep 2026 19:05:54 +0200 Subject: [PATCH 05/10] =?UTF-8?q?feat(check):=20java=20public-class=20?= =?UTF-8?q?=E2=87=84=20file-name=20mismatch=20=E2=80=94=20layout=20finding?= =?UTF-8?q?=20with=20a=20ready=20jmove=20mv=20repair?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - parser::java::class_name: exactly-one-public-top-level-type rule (package-info/module-info and 0-or-2-public files skipped) - check gains name_mismatches[] (JSON, omitted when clean) + human lines; exit 2 covers both kinds; fix engine untouched by design — a file rename is not a byte edit, and mv keeps the FQN so no imports change - output split into output/{mod,check} to stay under the 250-line rule - e2e: reports Bad.java, stays quiet on Good.java, suggested rename makes check pass and preserves the file bytes --- docs/EXAMPLES.md | 2 +- docs/SKILL.md | 9 +- src/cli/mod.rs | 6 +- src/cli/output/check.rs | 133 ++++++++++++++++++ src/cli/{output.rs => output/mod.rs} | 80 ++--------- src/parser/java/class_name.rs | 128 +++++++++++++++++ src/parser/java/mod.rs | 1 + tests/cli.rs | 2 +- tests/cli_name_check.rs | 43 ++++++ .../src/main/java/com/example/Bad.java | 3 + .../src/main/java/com/example/Good.java | 3 + todo.md | 4 +- 12 files changed, 334 insertions(+), 80 deletions(-) create mode 100644 src/cli/output/check.rs rename src/cli/{output.rs => output/mod.rs} (67%) create mode 100644 src/parser/java/class_name.rs create mode 100644 tests/cli_name_check.rs create mode 100644 tests/java/mismatch/src/main/java/com/example/Bad.java create mode 100644 tests/java/mismatch/src/main/java/com/example/Good.java diff --git a/docs/EXAMPLES.md b/docs/EXAMPLES.md index 7e79521..6ea482d 100644 --- a/docs/EXAMPLES.md +++ b/docs/EXAMPLES.md @@ -42,7 +42,7 @@ Rewrites happen first, the rename last; any failure rolls everything back. ```console $ jmove check -check: no broken imports found +check: no findings ``` When something does point at nothing, `check` prints one line per broken diff --git a/docs/SKILL.md b/docs/SKILL.md index e05d4e6..2486cb6 100644 --- a/docs/SKILL.md +++ b/docs/SKILL.md @@ -40,13 +40,16 @@ working tree unstaged either way — stage or commit them yourself. `--json` reports the choice as `moved_via` (`"git"`/`"fs"`) and, on a dry-run, `would_move_via`. -### check — find broken imports +### check — find broken imports and Java layout errors ``` jmove check [--root DIR] [--json] ``` -Run after any move (or any edit) to validate project consistency. +Run after any move (or any edit) to validate project consistency. Reports +broken relative imports and Java files whose single public type is named +differently from the file (each finding carries the exact `jmove mv` that +renames it; exit code 2 covers both kinds). ### fix — auto-repair import problems @@ -119,7 +122,7 @@ and a `hint` describing the next action. On success, `mv` reports - `0` — success - `1` — operation failed (read `--json` error or stderr) -- `2` — `check` found broken imports +- `2` — `check` found broken imports or Java name mismatches ## Rules of use diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 68c4d56..f18956e 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -196,7 +196,8 @@ fn check(root: &Path, source_root: Option<&Path>, json: bool) -> Flow { 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() { + let mismatches = flow(json, "check", output::name_mismatches(&root, &index))?; + let code = if broken.is_empty() && mismatches.is_empty() { exit::OK } else { exit::BROKEN @@ -207,10 +208,11 @@ fn check(root: &Path, source_root: Option<&Path>, json: bool) -> Flow { let data = output::CheckData { broken_imports: broken, total, + name_mismatches: mismatches, }; json::print(&Envelope::ok("check", data)); } else { - output::report_check(&broken); + output::report_check(&broken, &mismatches); } Ok(code) } diff --git a/src/cli/output/check.rs b/src/cli/output/check.rs new file mode 100644 index 0000000..10e0f75 --- /dev/null +++ b/src/cli/output/check.rs @@ -0,0 +1,133 @@ +//! `jmove check` collectors and payloads: unresolvable relative imports +//! plus Java file-name ⇄ public-class mismatches (layout errors that +//! `javac` rejects but import resolution cannot see). + +use serde::Serialize; +use std::path::Path; + +use crate::core::index::Index; +use crate::core::{JmoveResult, rel_str}; +use crate::parser::SourceLanguage; +use crate::parser::java::class_name; + +use super::{line_of, read_file}; + +/// `check` stdout line when the project has no findings. +const CHECK_CLEAN: &str = "check: no findings"; + +/// 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, +} + +/// A Java file whose single public top-level type is named differently +/// from the file — a `javac` error repaired by renaming the file (imports +/// stay valid: the class FQN does not change). +#[derive(Debug, Serialize)] +pub struct NameMismatch { + /// Project-relative file with the wrong name. + pub file: String, + /// 1-based line of the public type declaration. + pub line: usize, + /// Declared public type. + pub public_class: String, + /// Project-relative file it should live in. + pub expected_file: String, + /// Copy-paste repair command (paths are quoted for safety). + pub rename: String, +} + +/// 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, + /// Java layout findings (omitted from JSON when clean). + #[serde(skip_serializing_if = "Vec::is_empty")] + pub name_mismatches: Vec, +} + +/// Collect every relative import that resolves to nothing in `index`. +/// +/// A specifier starting with `.` whose target is `None` is broken; a bare +/// package specifier without a target is an external dependency, not an +/// error. Results are sorted by file, then line. +pub fn broken_imports(root: &Path, index: &Index) -> JmoveResult> { + let mut broken: Vec = Vec::new(); + for (file, imports) in &index.imports { + for import in imports { + if import.target.is_some() || !import.record.specifier.starts_with('.') { + continue; + } + let text = read_file(root, file)?; + broken.push(BrokenImport { + file: rel_str(file), + line: line_of(&text, import.record.span.start), + import: import.record.specifier.clone(), + reason: "file_not_found", + }); + } + } + broken.sort_by(|a, b| (&a.file, a.line).cmp(&(&b.file, b.line))); + Ok(broken) +} + +/// Java files whose only public top-level type disagrees with the file +/// name; sorted by file, then line. +pub fn name_mismatches(root: &Path, index: &Index) -> JmoveResult> { + let mut out = Vec::new(); + for file in index.files.sorted() { + if SourceLanguage::for_path(&file) != Some(SourceLanguage::Java) { + continue; + } + let text = read_file(root, &file)?; + let Some(found) = class_name::mismatch(&file, &text) else { + continue; + }; + let dir = file.parent().unwrap_or(Path::new("")); + let ext = file.extension().and_then(|e| e.to_str()).unwrap_or("java"); + let expected = dir.join(format!("{}.{}", found.public_class, ext)); + let (old, new) = (rel_str(&file), rel_str(&expected)); + out.push(NameMismatch { + file: old.clone(), + line: line_of(&text, found.span.start), + public_class: found.public_class, + expected_file: new.clone(), + rename: format!("jmove mv '{old}' '{new}'"), + }); + } + out.sort_by_key(|m| (m.file.clone(), m.line)); + Ok(out) +} + +/// Print the human `check` report: the clean note, or one +/// `path:line: cannot resolve 'spec'` line per broken import and one +/// rename-line per Java layout mismatch. +pub fn report_check(broken: &[BrokenImport], mismatches: &[NameMismatch]) { + if broken.is_empty() && mismatches.is_empty() { + println!("{CHECK_CLEAN}"); + return; + } + for entry in broken { + println!( + "{}:{}: cannot resolve '{}'", + entry.file, entry.line, entry.import + ); + } + for m in mismatches { + println!( + "{}:{}: public class '{}' must live in '{}'; fix: {}", + m.file, m.line, m.public_class, m.expected_file, m.rename + ); + } +} diff --git a/src/cli/output.rs b/src/cli/output/mod.rs similarity index 67% rename from src/cli/output.rs rename to src/cli/output/mod.rs index b83a1e8..a9ad44a 100644 --- a/src/cli/output.rs +++ b/src/cli/output/mod.rs @@ -1,25 +1,26 @@ //! Human-readable rendering plus the payload builders that both output -//! modes share: grouping rewrites, resolving broken imports, line lookup. +//! modes share: grouping rewrites, `mv` pre-flight validation, line lookup. +//! The `check` payloads and collectors live in [`check`]. //! //! 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}; -use crate::core::index::Index; use crate::core::plan::{MovePlan, Rewrite}; use crate::core::{JmoveResult, rel_str}; +mod check; + 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"; +pub use check::{ + BrokenImport, CheckData, NameMismatch, broken_imports, name_mismatches, report_check, +}; /// Read a project file (project-relative path) as UTF-8 text. -fn read_file(root: &Path, rel: &Path) -> JmoveResult { +pub(crate) fn read_file(root: &Path, rel: &Path) -> JmoveResult { Ok(std::fs::read_to_string(root.join(rel))?) } @@ -44,32 +45,6 @@ pub fn group_by_file(rewrites: &[Rewrite]) -> Vec<(&Path, Vec<&Rewrite>)> { } map.into_iter().collect() } - -/// Collect every relative import that resolves to nothing in `index`. -/// -/// A specifier starting with `.` whose target is `None` is broken; a bare -/// package specifier without a target is an external dependency, not an -/// error. Results are sorted by file, then line. -pub fn broken_imports(root: &Path, index: &Index) -> JmoveResult> { - let mut broken: Vec = Vec::new(); - for (file, imports) in &index.imports { - for import in imports { - if import.target.is_some() || !import.record.specifier.starts_with('.') { - continue; - } - let text = read_file(root, file)?; - broken.push(BrokenImport { - file: rel_str(file), - line: line_of(&text, import.record.span.start), - import: import.record.specifier.clone(), - reason: "file_not_found", - }); - } - } - broken.sort_by(|a, b| (&a.file, a.line).cmp(&(&b.file, b.line))); - Ok(broken) -} - /// Build the `changed_files` payload: line-level specifier diffs grouped per /// importer, computed against the on-disk contents at call time. pub fn changed_files(root: &Path, plan: &MovePlan) -> JmoveResult> { @@ -165,23 +140,6 @@ pub fn mv_summary(plan: &MovePlan, via_git: bool) -> String { plural(files, "file"), ) } - -/// Print the human `check` report: the clean note, or one -/// `path:line: cannot resolve 'spec'` line per broken import. -pub fn report_check(broken: &[BrokenImport]) { - if broken.is_empty() { - println!("{CHECK_CLEAN}"); - return; - } - for entry in broken { - let line = format!( - "{}:{}: cannot resolve '{}'", - entry.file, entry.line, entry.import - ); - println!("{line}"); - } -} - /// Print an operation error to stderr, with the hint on its own line. pub fn print_error(message: &str, hint: Option<&str>) { eprintln!("jmove: {message}"); @@ -198,25 +156,3 @@ 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/parser/java/class_name.rs b/src/parser/java/class_name.rs new file mode 100644 index 0000000..9ab1019 --- /dev/null +++ b/src/parser/java/class_name.rs @@ -0,0 +1,128 @@ +//! Java layout check: the single public top-level type must match the +//! file name (javac: "class Foo is public, should be declared in a file +//! named Foo.java"). +//! +//! This is a *finding*, not a fix rule: the repair is a file rename, and +//! the fix engine applies byte edits only. `jmove check` reports it with +//! the exact `jmove mv` command that repairs the layout — the moved class +//! keeps its FQN, so renaming the file rewrites no imports. + +use std::ops::Range; +use std::path::Path; + +use tree_sitter::Node; + +use super::{TreeSitterJava, text}; + +/// Kinds that introduce a top-level type in Java. +const TYPE_KINDS: &[&str] = &[ + "class_declaration", + "interface_declaration", + "enum_declaration", + "record_declaration", + "annotation_type_declaration", +]; + +/// The mismatch details: the public type's name, the span of its name +/// token (for line reporting) and the file stem it should live in. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Mismatch { + /// Name of the single public top-level type. + pub public_class: String, + /// Byte span of the class name token. + pub span: Range, +} + +/// `Some` when the file declares exactly one public top-level type whose +/// name differs from the file stem. Two public types (also illegal) or +/// zero are not this check's business. `package-info`/`module-info` files +/// never name a type and are skipped. +#[must_use] +pub fn mismatch(path: &Path, source: &str) -> Option { + let stem = path.file_stem()?.to_str()?; + if matches!(stem, "package-info" | "module-info") { + return None; + } + let tree = TreeSitterJava::parse(source)?; + let mut cursor = tree.root_node().walk(); + let publics: Vec = tree + .root_node() + .children(&mut cursor) + .filter(|n| TYPE_KINDS.contains(&n.kind())) + .filter(|n| has_public_modifier(*n)) + .collect(); + let [only] = publics.as_slice() else { + return None; // zero or many public types: not a naming mismatch + }; + let mut c = only.walk(); + let name = only.children(&mut c).find(|n| n.kind() == "identifier")?; + let name_text = text(name, source); + (name_text != stem).then(|| Mismatch { + public_class: name_text.to_owned(), + span: name.byte_range(), + }) +} + +// `public` is an anonymous token inside the `modifiers` child. +fn has_public_modifier(node: Node) -> bool { + let mut c = node.walk(); + node.children(&mut c) + .find(|n| n.kind() == "modifiers") + .is_some_and(|mods| { + let mut m = mods.walk(); + mods.children(&mut m).any(|t| t.kind() == "public") + }) +} + +#[cfg(test)] +mod tests { + use super::mismatch; + use std::path::Path; + + fn m(path: &str, src: &str) -> Option { + mismatch(Path::new(path), src).map(|f| f.public_class) + } + + #[test] + fn public_type_name_must_equal_file_stem() { + assert_eq!( + m("Foo.java", "package p;\npublic class Bar {}\n").as_deref(), + Some("Bar") + ); + assert_eq!(m("Foo.java", "package p;\npublic class Foo {}\n"), None); + } + + #[test] + fn every_public_top_level_kind_is_checked() { + assert_eq!( + m("A.java", "public interface B { int x(); }").as_deref(), + Some("B") + ); + assert_eq!(m("A.java", "public enum B { X }").as_deref(), Some("B")); + assert_eq!( + m("A.java", "public record B(int x) {}").as_deref(), + Some("B") + ); + assert_eq!(m("A.java", "public @interface B {}").as_deref(), Some("B")); + } + + #[test] + fn non_public_multiple_or_nested_types_are_not_reported() { + // Zero public types: legal package-private layout. + assert_eq!(m("Foo.java", "class Bar {}\nclass Baz {}"), None); + // Two public top-level types: a different (harder) error. + assert_eq!(m("Foo.java", "public class A {}\npublic class B {}"), None); + // Nested publics live inside a type: not top-level. + assert_eq!(m("A.java", "class A { public class B {} }"), None); + // Protected/private modifiers do not trigger the javac rule. + assert_eq!(m("A.java", "class B {}"), None); + } + + #[test] + fn info_files_are_skipped_and_span_names_the_class_token() { + assert_eq!(m("package-info.java", "package p;"), None); + let src = "package p;\n\npublic class Bar {}\n"; + let found = mismatch(Path::new("Foo.java"), src).unwrap(); + assert_eq!(&src[found.span.clone()], "Bar"); + } +} diff --git a/src/parser/java/mod.rs b/src/parser/java/mod.rs index 2fc62fc..2554092 100644 --- a/src/parser/java/mod.rs +++ b/src/parser/java/mod.rs @@ -12,6 +12,7 @@ use std::path::{Path, PathBuf}; use tree_sitter::{Node, Parser, Tree}; +pub mod class_name; pub mod rules; use super::{ImportRecord, Language, PackageDecl, SourceLanguage}; diff --git a/tests/cli.rs b/tests/cli.rs index 188f208..e1f04d8 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -105,7 +105,7 @@ fn check_passes_on_clean_and_fails_on_broken_fixture() { let clean = fixture("basic"); jmove(&clean, &["check"]) .success() - .stdout(predicate::str::contains("no broken imports")); + .stdout(predicate::str::contains("check: no findings")); let messy = fixture("complex"); jmove(&messy, &["check"]) diff --git a/tests/cli_name_check.rs b/tests/cli_name_check.rs new file mode 100644 index 0000000..615d77d --- /dev/null +++ b/tests/cli_name_check.rs @@ -0,0 +1,43 @@ +//! `jmove check` finds Java files whose public type is misnamed, and the +//! suggested `jmove mv` repairs the layout. + +mod common; + +use common::{copy_fixture, in_root, jmove, read}; +use predicates::prelude::*; + +const BAD: &str = "src/main/java/com/example/Bad.java"; + +#[test] +fn check_reports_misnamed_public_class_and_clean_files_stay_quiet() { + let tmp = copy_fixture("java", "mismatch"); + jmove(&tmp, &["check"]) + .code(2) + .stdout( + predicate::str::contains( + "src/main/java/com/example/Bad.java:3: public class 'Wrong' must live in 'src/main/java/com/example/Wrong.java'", + ) + .and(predicate::str::contains("Good.java").not()), + ); + jmove(&tmp, &["check", "--json"]) + .code(2) + .stdout( + predicate::str::contains("\"name_mismatches\"") + .and(predicate::str::contains("\"public_class\": \"Wrong\"")) + .and(predicate::str::contains( + "\"rename\": \"jmove mv 'src/main/java/com/example/Bad.java' 'src/main/java/com/example/Wrong.java'\"", + )), + ); +} + +#[test] +fn running_the_suggested_rename_makes_check_pass() { + let tmp = copy_fixture("java", "mismatch"); + jmove(&tmp, &["mv", BAD, "src/main/java/com/example/Wrong.java"]).success(); + let moved = read(&in_root(tmp.path(), "src/main/java/com/example/Wrong.java")); + assert!(moved.contains("public class Wrong {}"), "{moved}"); + assert!(moved.contains("package com.example;"), "{moved}"); + jmove(&tmp, &["check"]) + .success() + .stdout(predicate::str::contains("no findings")); +} diff --git a/tests/java/mismatch/src/main/java/com/example/Bad.java b/tests/java/mismatch/src/main/java/com/example/Bad.java new file mode 100644 index 0000000..c6d6e4c --- /dev/null +++ b/tests/java/mismatch/src/main/java/com/example/Bad.java @@ -0,0 +1,3 @@ +package com.example; + +public class Wrong {} diff --git a/tests/java/mismatch/src/main/java/com/example/Good.java b/tests/java/mismatch/src/main/java/com/example/Good.java new file mode 100644 index 0000000..c4e583a --- /dev/null +++ b/tests/java/mismatch/src/main/java/com/example/Good.java @@ -0,0 +1,3 @@ +package com.example; + +public class Good {} diff --git a/todo.md b/todo.md index 7573fc8..e7bbd10 100644 --- a/todo.md +++ b/todo.md @@ -55,7 +55,9 @@ AI оставляем СНАРУЖИ: при неоднозначности jmov - [~] Java v1: unused-imports (DONE, skip wildcard/ambiguous), missing-import (DONE: unique FQN candidate → insert `import pkg.Type;` at the import-block end; ambiguous/wildcard → `--json` `candidates`, applied:false; закрыт guava-разрыв «перенесли файл, соседняя ссылка без импорта умерла» — проверено mv+fix+javac SUCCESS), - import-order (DONE: Google-стиль — statics первыми, ASCII-сортировка, дедуп; конфликтующие с другими правилами откладываются (prune_overlaps по severity) и сходятся за 2-3 прогона), class-name-mismatch + import-order (DONE: Google-стиль — statics первыми, ASCII-сортировка, дедуп; конфликтующие с другими правилами откладываются (prune_overlaps по severity) и сходятся за 2-3 прогона), + class-name-mismatch (DONE как ПОВЕРХНОСТЬ check, не fix: починка = переименование файла, а fix-движок умеет только байтовые правки; + check отдаёт находку с готовой командой `jmove mv`, exit code 2; rename не меняет FQN → импорты не трогаются) - [x] TS v1: unused-imports (DONE: whole-statement delete, все биндинги мертвы → строка уходит; mixed used/unused НЕ трогаем — в ESM импорт исполняет побочные эффекты модуля, partial-удаление specifier'ов отложено осознанно) From b1af375763d900add139a16b0c91ebebb94250f3 Mon Sep 17 00:00:00 2001 From: loki5512344 Date: Tue, 15 Sep 2026 19:17:06 +0200 Subject: [PATCH 06/10] =?UTF-8?q?feat:=20tsconfig=20compilerOptions.paths?= =?UTF-8?q?=20=E2=80=94=20aliased=20bare=20specifiers=20join=20the=20graph?= =?UTF-8?q?=20and=20follow=20moves?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - index/tsconfig: JSONC sanitize (comments, trailing commas), baseUrl + star/exact path keys, first candidate only, longest-prefix wins; invalid or missing tsconfig silently means no aliases - resolution pass: '.'-prefixed specifiers resolve relatively as before, bare ones go through the alias table and join the importer graph - planner: remap keeps the alias shape while the moved file stays inside the mapped tree (morphological, since destinations do not exist yet), exact keys keep meaning only while still resolving to the target; otherwise the previous relative rewrite applies unchanged - resolve_base extracted from resolve_module (shared extension/index guessing); MODULE_EXTS is the exact inverse for remapping - e2e: @utils/str.ts -> @utils/text.ts inside alias, -> ./core/text.ts when leaving it, @cfg falls back to ./settings; check passes on aliases --- docs/SKILL.md | 7 + src/core/index/mod.rs | 15 +- src/core/index/tsconfig/mod.rs | 212 ++++++++++++++++++++++ src/core/index/tsconfig/tests.rs | 80 ++++++++ src/core/plan/mod.rs | 8 +- src/parser/resolve.rs | 17 +- tests/cli_alias.rs | 37 ++++ tests/typescript/aliased/package.json | 1 + tests/typescript/aliased/src/app.ts | 7 + tests/typescript/aliased/src/config.ts | 1 + tests/typescript/aliased/src/utils/log.ts | 3 + tests/typescript/aliased/src/utils/str.ts | 3 + tests/typescript/aliased/tsconfig.json | 10 + todo.md | 4 +- 14 files changed, 400 insertions(+), 5 deletions(-) create mode 100644 src/core/index/tsconfig/mod.rs create mode 100644 src/core/index/tsconfig/tests.rs create mode 100644 tests/cli_alias.rs create mode 100644 tests/typescript/aliased/package.json create mode 100644 tests/typescript/aliased/src/app.ts create mode 100644 tests/typescript/aliased/src/config.ts create mode 100644 tests/typescript/aliased/src/utils/log.ts create mode 100644 tests/typescript/aliased/src/utils/str.ts create mode 100644 tests/typescript/aliased/tsconfig.json diff --git a/docs/SKILL.md b/docs/SKILL.md index 2486cb6..e16325f 100644 --- a/docs/SKILL.md +++ b/docs/SKILL.md @@ -40,6 +40,13 @@ working tree unstaged either way — stage or commit them yourself. `--json` reports the choice as `moved_via` (`"git"`/`"fs"`) and, on a dry-run, `would_move_via`. +tsconfig `paths`: `compilerOptions.paths` prefixes (`@/*`, `@cfg` exact +keys, `baseUrl`-relative) map bare specifiers into the project graph, so +aliased imports participate in `mv` — the rewrite keeps the alias shape +while the new file stays inside the mapped tree and falls back to a +relative specifier otherwise. `extends` chains and 2nd+ candidate lists +are not followed (v1); an invalid tsconfig silently means "no aliases". + ### check — find broken imports and Java layout errors ``` diff --git a/src/core/index/mod.rs b/src/core/index/mod.rs index 83aa37e..92b3419 100644 --- a/src/core/index/mod.rs +++ b/src/core/index/mod.rs @@ -9,6 +9,9 @@ mod files; #[cfg(test)] mod tests; +mod tsconfig; + +pub use tsconfig::PathAliases; use std::collections::{BTreeMap, HashMap}; use std::fs; @@ -48,6 +51,8 @@ pub struct Index { /// FQN → file map for every indexed Java class; the fix rules use it /// for candidate lookup (empty when the project has no Java sources). pub java_classes: JavaClassIndex, + /// tsconfig `compilerOptions.paths` alias table (empty without one). + pub aliases: PathAliases, } impl Index { @@ -64,12 +69,14 @@ impl Index { /// collision-free resolution (and therefore `mv`/`fix` rewrites) exact. pub fn build_scoped(root: &Path, source_root: Option<&Path>) -> JmoveResult { let root = root.canonicalize()?; + let aliases = PathAliases::load(&root); let mut index = Self { root, files: FileSet::default(), imports: HashMap::new(), packages: HashMap::new(), java_classes: JavaClassIndex::default(), + aliases, }; index.scan(source_root)?; // Resolution needs the complete file set (extension/index guessing) @@ -82,8 +89,14 @@ impl Index { java_classes .resolve(&resolved.record.specifier) .map(PathBuf::from) - } else { + } else if resolved.record.specifier.starts_with('.') { resolve_module(importer, &resolved.record.specifier, &index.files) + } else { + // Bare specifier: only tsconfig `paths` can map it into + // the project; anything else stays external. + index + .aliases + .resolve(&resolved.record.specifier, &index.files) }; } } diff --git a/src/core/index/tsconfig/mod.rs b/src/core/index/tsconfig/mod.rs new file mode 100644 index 0000000..a024931 --- /dev/null +++ b/src/core/index/tsconfig/mod.rs @@ -0,0 +1,212 @@ +//! tsconfig `compilerOptions.paths`: the alias table for bare specifiers. +//! +//! Most real TS projects import through aliases (`@/utils/x`, `@cfg`). +//! Without the mapping those imports are invisible to the graph, so `mv` +//! would leave them pointing at the old location. Loading is best-effort by +//! design: no tsconfig, invalid JSON or an unreadable file simply mean "no +//! aliases", never an error. `extends` chains are not followed (v1). +//! +//! tsconfig is JSONC: comments and trailing commas are legal, so the text +//! is sanitized before `serde_json` sees it. + +use std::fs; +use std::path::{Path, PathBuf}; + +use serde_json::Value; + +use crate::core::index::FileSet; +use crate::core::{normalize_rel_path, rel_str}; +use crate::parser::resolve::{MODULE_EXTS, resolve_base}; + +/// One `paths` entry. Star entries match by prefix, plain keys by equality. +#[derive(Debug, Clone, PartialEq, Eq)] +struct Entry { + /// Text the specifier must start with (star stripped), e.g. `"@utils/"`. + prefix: String, + /// `false` for an exact key (`"@cfg"`). + starred: bool, + /// Project-relative directory (star) or module base (exact) the alias maps into. + dir: PathBuf, +} + +/// Alias table; empty is a perfectly normal state. +#[derive(Debug, Default, Clone, PartialEq, Eq)] +pub struct PathAliases { + entries: Vec, +} + +impl PathAliases { + /// Load `root/tsconfig.json`; anything unreadable yields no aliases. + #[must_use] + pub fn load(root: &Path) -> Self { + let Ok(raw) = fs::read_to_string(root.join("tsconfig.json")) else { + return Self::default(); + }; + Self::parse(&sanitize_jsonc(&raw)) + } + + fn parse(json: &str) -> Self { + let Ok(value) = serde_json::from_str::(json) else { + return Self::default(); + }; + let options = value.get("compilerOptions"); + let base = options + .and_then(|o| o.get("baseUrl")) + .and_then(Value::as_str) + .and_then(|b| normalize_rel_path(Path::new(b))) + .unwrap_or_default(); + let Some(paths) = options + .and_then(|o| o.get("paths")) + .and_then(Value::as_object) + else { + return Self::default(); + }; + let mut entries = Vec::new(); + for (key, targets) in paths { + // Only the first candidate of a list is honoured (KISS). + let Some(first) = targets + .as_array() + .and_then(|a| a.first()) + .and_then(Value::as_str) + else { + continue; + }; + let starred = key.ends_with('*'); + let prefix = if starred { + key.trim_end_matches('*').to_owned() + } else { + key.clone() + }; + let stripped = first.trim_start_matches("./"); + let dir_src = if first.ends_with("/*") { + stripped.trim_end_matches("/*") + } else { + stripped + }; + let dir = normalize_rel_path(&base.join(dir_src)); + let Some(dir) = dir.filter(|d| !d.as_os_str().is_empty()) else { + continue; + }; + entries.push(Entry { + prefix, + starred, + dir, + }); + } + // Longest alias prefix wins when several match (`@a/b/*` over `@a/*`). + entries.sort_by(|a, b| b.prefix.cmp(&a.prefix)); + Self { entries } + } + + /// Resolve a bare (non-`.`-starting) specifier to an indexed file. + #[must_use] + pub fn resolve(&self, specifier: &str, files: &FileSet) -> Option { + let entry = self.find(specifier)?; + let base = if entry.starred { + entry.dir.join(&specifier[entry.prefix.len()..]) + } else { + entry.dir.clone() + }; + resolve_base(&normalize_rel_path(&base)?, files) + } + + /// Re-express `target` through the same alias `specifier` used before. + /// Star entries map morphologically (the plan runs before the file + /// exists at its destination, so no file-set check is possible): the + /// target must sit inside the alias directory and shed its module + /// extension the same way `resolve` would re-add it. `None` means + /// "fall back to a relative specifier". + #[must_use] + pub fn remap(&self, specifier: &str, target: &Path, files: &FileSet) -> Option { + let entry = self.find(specifier)?; + if !entry.starred { + // Exact key: it only keeps meaning while it maps to this file — + // after the move the old mapping points elsewhere, so this is + // normally `None` and the caller rewrites relatively. + return (self.resolve(specifier, files)? == target).then(|| specifier.to_owned()); + } + let rel = target.strip_prefix(&entry.dir).ok()?; + let text = rel_str(rel); + let stem = MODULE_EXTS + .iter() + .find(|ext| text.ends_with(**ext)) + .and_then(|ext| Some(text.strip_suffix(*ext)?.to_owned())) + .unwrap_or(text); + Some(format!("{}{}", entry.prefix, stem)) + } + + fn find(&self, specifier: &str) -> Option<&Entry> { + self.entries.iter().find(|e| { + if e.starred { + specifier.starts_with(&e.prefix) && specifier.len() > e.prefix.len() + } else { + specifier == e.prefix + } + }) + } +} + +/// Strip `//` + `/* */` comments and trailing commas from JSONC text. +/// String awareness keeps `"http://x"` and escaped quotes safe. +#[must_use] +pub(crate) fn sanitize_jsonc(src: &str) -> String { + let bytes = src.as_bytes(); + let mut out = String::with_capacity(src.len()); + let mut i = 0; + let mut in_string = false; + while i < bytes.len() { + let b = bytes[i]; + if in_string { + out.push(b as char); + if b == b'\\' && i + 1 < bytes.len() { + out.push(bytes[i + 1] as char); + i += 2; + continue; + } + if b == b'"' { + in_string = false; + } + i += 1; + continue; + } + match b { + b'"' => { + in_string = true; + out.push('"'); + i += 1; + } + b'/' if bytes.get(i + 1) == Some(&b'/') => { + i = bytes[i..] + .iter() + .position(|c| *c == b'\n') + .map_or(bytes.len(), |p| i + p); + } + b'/' if bytes.get(i + 1) == Some(&b'*') => { + let end = bytes[i + 2..] + .windows(2) + .position(|w| w == b"*/") + .map_or(bytes.len() - 2, |p| i + 2 + p + 2); + i = end; + } + b',' => { + // Drop only if the next non-space token closes a container. + let rest = &bytes[i + 1..]; + match rest.iter().find(|c| !c.is_ascii_whitespace()) { + Some(b'}') | Some(b']') => i += 1, + _ => { + out.push(','); + i += 1; + } + } + } + _ => { + out.push(b as char); + i += 1; + } + } + } + out +} + +#[cfg(test)] +mod tests; diff --git a/src/core/index/tsconfig/tests.rs b/src/core/index/tsconfig/tests.rs new file mode 100644 index 0000000..ef55f89 --- /dev/null +++ b/src/core/index/tsconfig/tests.rs @@ -0,0 +1,80 @@ +use super::{PathAliases, sanitize_jsonc}; +use crate::core::index::FileSet; +use std::path::{Path, PathBuf}; + +fn files(paths: &[&str]) -> FileSet { + let mut set = FileSet::default(); + for p in paths { + set.add(PathBuf::from(p)); + } + set +} + +fn aliases(json: &str) -> PathAliases { + PathAliases::parse(&sanitize_jsonc(json)) +} + +const TSCONFIG: &str = r#"{ + // comment with a brace } + "compilerOptions": { +"baseUrl": "./", +"paths": { + "@/*": ["src/*"], + "@utils/*": ["src/shared/utils/*"], + "@cfg": ["src/config.ts"], +}, + }, + "include": ["src/**/*"], +}"#; + +#[test] +fn jsonc_comments_and_trailing_commas_parse() { + let a = aliases(TSCONFIG); + assert_eq!(a.entries.len(), 3); +} + +#[test] +fn resolution_prefers_the_longest_matching_prefix() { + let set = files(&[ + "src/shared/utils/str.ts", + "src/utils/str.ts", + "src/config.ts", + ]); + let a = aliases(TSCONFIG); + assert_eq!( + a.resolve("@utils/str", &set).as_deref(), + Some(Path::new("src/shared/utils/str.ts")) + ); + assert_eq!( + a.resolve("@/utils/str", &set).as_deref(), + Some(Path::new("src/utils/str.ts")) + ); + assert_eq!( + a.resolve("@cfg", &set).as_deref(), + Some(Path::new("src/config.ts")) + ); + assert_eq!(a.resolve("react", &set), None); +} + +#[test] +fn remap_keeps_alias_shape_inside_and_falls_back_outside() { + let set = files(&["src/utils/str.ts", "src/deep/str.ts", "src/config.ts"]); + let a = aliases(r#"{"compilerOptions": {"paths": {"@u/*": ["src/utils/*"]}}}"#); + // Destination need not exist yet: the plan runs before the move. + assert_eq!( + a.remap("@u/str", Path::new("src/utils/other.ts"), &set) + .as_deref(), + Some("@u/other") + ); + // Target outside the alias tree: caller must fall back to relative. + assert!(a.remap("@u/str", Path::new("lib/other.ts"), &set).is_none()); +} + +#[test] +fn string_contents_survive_sanitizing() { + let json = r#"{"url": "http://x.com//y", "a": [1, 2,], /* c */ "b": "/*n*/"}"#; + let cleaned = sanitize_jsonc(json); + let value: serde_json::Value = serde_json::from_str(&cleaned).unwrap(); + assert_eq!(value["url"], "http://x.com//y"); + assert_eq!(value["b"], "/*n*/"); +} diff --git a/src/core/plan/mod.rs b/src/core/plan/mod.rs index 8fae7b8..8c03e79 100644 --- a/src/core/plan/mod.rs +++ b/src/core/plan/mod.rs @@ -147,7 +147,13 @@ fn ts_rewrites(index: &Index, source: &Path, target: &Path) -> Vec { .iter() .filter(|e| e.target.as_deref() == Some(source)); for edge in edges { - let new_text = relative_specifier(&importer, target); + // Aliased imports keep their alias shape when the new location + // still round-trips through the same mapping; everything else + // gets the relative rewrite. + let new_text = index + .aliases + .remap(&edge.record.specifier, target, &index.files) + .unwrap_or_else(|| relative_specifier(&importer, target)); if new_text == edge.record.specifier { continue; // no-op rewrite, never reaches the plan } diff --git a/src/parser/resolve.rs b/src/parser/resolve.rs index b636895..3ac5986 100644 --- a/src/parser/resolve.rs +++ b/src/parser/resolve.rs @@ -14,6 +14,10 @@ use crate::core::normalize_rel_path; /// Supported extensions, in Node/TS resolution priority order. const EXTENSIONS: [&str; 6] = ["ts", "tsx", "js", "jsx", "mjs", "cjs"]; +/// Module extension *suffixes*, longest-first: `resolve_base` re-adds them +/// to a specifier, alias remapping sheds them again (the exact inverse). +pub const MODULE_EXTS: &[&str] = &[".d.ts", ".ts", ".tsx", ".js", ".jsx", ".mjs", ".cjs"]; + /// Ambient declaration files: only consulted after every real module /// candidate missed (last resort). const DECLARATION_EXT: &str = "d.ts"; @@ -45,9 +49,18 @@ pub fn resolve_module(importer: &Path, specifier: &str, files: &FileSet) -> Opti // `importer` is project-relative, so `..` segments that walk past the // root collapse to `None` here instead of escaping the index. let base = normalize_rel_path(&importer.parent()?.join(specifier))?; - if files.contains(&base) { - return Some(base); + resolve_base(&base, files) +} + +/// Resolve a project-relative module base path against the file set: +/// exact file, extension guessing, then `index.*` in the directory. +/// Shared by relative specifiers and tsconfig alias mapping. +#[must_use] +pub fn resolve_base(base: &Path, files: &FileSet) -> Option { + if files.contains(base) { + return Some(base.to_path_buf()); } + let base = base.to_path_buf(); for ext in EXTENSIONS { let candidate = with_ext(&base, ext); if files.contains(&candidate) { diff --git a/tests/cli_alias.rs b/tests/cli_alias.rs new file mode 100644 index 0000000..f5f0948 --- /dev/null +++ b/tests/cli_alias.rs @@ -0,0 +1,37 @@ +//! End-to-end tests for tsconfig `paths` aliases: aliased imports take +//! part in `mv` and keep their alias shape inside the mapped tree. + +mod common; + +use common::{copy_fixture, in_root, jmove, read}; +use predicates::prelude::*; + +const APP: &str = "src/app.ts"; + +#[test] +fn aliased_imports_follow_the_move_and_keep_the_alias_shape() { + let tmp = copy_fixture("typescript", "aliased"); + jmove(&tmp, &["check"]).success(); // "@utils/str" resolves: not "broken" + jmove(&tmp, &["mv", "src/utils/str.ts", "src/utils/text.ts"]) + .success() + .stdout(predicate::str::contains("updated 1 import")); + let app = read(&in_root(tmp.path(), APP)); + assert!(app.contains("from \"@utils/text\""), "{app}"); + assert!(!app.contains("./utils/text"), "{app}"); + // Moving out of the alias tree falls back to a relative specifier. + jmove(&tmp, &["mv", "src/utils/text.ts", "src/core/text.ts"]).success(); + let app = read(&in_root(tmp.path(), APP)); + assert!(app.contains("from \"./core/text\""), "{app}"); + jmove(&tmp, &["check"]).success(); +} + +#[test] +fn exact_alias_key_falls_back_when_it_no_longer_matches() { + let tmp = copy_fixture("typescript", "aliased"); + jmove(&tmp, &["mv", "src/config.ts", "src/settings.ts"]).success(); + let app = read(&in_root(tmp.path(), APP)); + // "@cfg" would now resolve to nothing: the rewrite must be relative. + assert!(app.contains("from \"./settings\""), "{app}"); + assert!(!app.contains("@cfg"), "{app}"); + jmove(&tmp, &["check"]).success(); +} diff --git a/tests/typescript/aliased/package.json b/tests/typescript/aliased/package.json new file mode 100644 index 0000000..3f4a4c7 --- /dev/null +++ b/tests/typescript/aliased/package.json @@ -0,0 +1 @@ +{ "name": "aliased-fixture" } diff --git a/tests/typescript/aliased/src/app.ts b/tests/typescript/aliased/src/app.ts new file mode 100644 index 0000000..611b2ea --- /dev/null +++ b/tests/typescript/aliased/src/app.ts @@ -0,0 +1,7 @@ +import { shout } from "@utils/str"; +import cfg from "@cfg"; +import { log } from "./utils/log"; + +export function go(): string { + return shout(cfg) + log(); +} diff --git a/tests/typescript/aliased/src/config.ts b/tests/typescript/aliased/src/config.ts new file mode 100644 index 0000000..614457e --- /dev/null +++ b/tests/typescript/aliased/src/config.ts @@ -0,0 +1 @@ +export default "cfg"; diff --git a/tests/typescript/aliased/src/utils/log.ts b/tests/typescript/aliased/src/utils/log.ts new file mode 100644 index 0000000..2cf3ecd --- /dev/null +++ b/tests/typescript/aliased/src/utils/log.ts @@ -0,0 +1,3 @@ +export function log(): string { + return ""; +} diff --git a/tests/typescript/aliased/src/utils/str.ts b/tests/typescript/aliased/src/utils/str.ts new file mode 100644 index 0000000..2b5e1b1 --- /dev/null +++ b/tests/typescript/aliased/src/utils/str.ts @@ -0,0 +1,3 @@ +export function shout(s: string): string { + return s.toUpperCase(); +} diff --git a/tests/typescript/aliased/tsconfig.json b/tests/typescript/aliased/tsconfig.json new file mode 100644 index 0000000..26e34f3 --- /dev/null +++ b/tests/typescript/aliased/tsconfig.json @@ -0,0 +1,10 @@ +{ + // JSONC on purpose: comments and trailing commas must not break loading + "compilerOptions": { + "baseUrl": "./src", + "paths": { + "@utils/*": ["utils/*"], + "@cfg": ["config.ts"], + }, + }, +} diff --git a/todo.md b/todo.md index e7bbd10..580817c 100644 --- a/todo.md +++ b/todo.md @@ -90,7 +90,9 @@ AI оставляем СНАРУЖИ: при неоднозначности jmov ## Phase 2 - [ ] Кэш индекса на диске (bincode/rkyv) → .jmove/index - [ ] Инкрементальная переиндексация (только изменённые файлы) -- [ ] Поддержка tsconfig paths / алиасов (@/...) +- [x] Поддержка tsconfig paths / алиасов (@/...): JSONC-парсер (комментарии/хвостовые запятые), + baseUrl + star/exact keys, longest-prefix wins; при mv алиас сохраняется, если файл остался + в дереве алиаса, иначе fallback на относительный; extends/2+ кандидаты — осознанно не делаем (v1) - [ ] Параллельная индексация через rayon - [x] --git интеграция (git mv для stage/истории): auto для tracked файлов, --no-git флаг, moved_via/would_move_via в --json - [x] Перенос директорий целиком (mv папки): зеркальный batch-move всех индексируемых файлов, merged rewrites, prune пустых исходных каталогов, left_behind для неиндексируемых From 1ddf79b508ce22fb7558e8f500822582e3a9004c Mon Sep 17 00:00:00 2001 From: loki5512344 Date: Tue, 15 Sep 2026 19:48:18 +0200 Subject: [PATCH 07/10] feat: warn about non-import references to moved files The import graph only rewrites import/export-from/require. Files are also named from markdown links, package.json fields, jest.mock strings and tsconfig file lists, and a move that dangles those must not pass silently. mv now scans text formats plus source strings for the moved paths, module names and old specifier forms (word-boundary matched, lockfiles/hidden dirs skipped) and reports each occurrence once per line on stderr and as non_import_refs[] in --json (dry-run included). References are never edited; exit codes are unchanged. Also moves normalize_scope into Index (its only consumer) and the Change/ChangedFile JSON structs into output where they are built, to stay inside the 250-line-per-file budget. --- README.md | 3 + docs/EXAMPLES.md | 25 ++- docs/SKILL.md | 7 + src/cli/fix.rs | 2 +- src/cli/json.rs | 46 ++--- src/cli/mod.rs | 53 ++--- src/cli/output/mod.rs | 58 +++++- src/core/index/mod.rs | 21 +- src/core/mod.rs | 12 ++ src/core/refs/mod.rs | 205 ++++++++++++++++++++ src/core/refs/tests.rs | 123 ++++++++++++ tests/cli_refs.rs | 68 +++++++ tests/typescript/refs/.notes/refs.md | 1 + tests/typescript/refs/README.md | 3 + tests/typescript/refs/__tests__/sum.test.ts | 1 + tests/typescript/refs/app.ts | 2 + tests/typescript/refs/lib/sum.ts | 1 + tests/typescript/refs/lib/summary.ts | 1 + tests/typescript/refs/package-lock.json | 1 + tests/typescript/refs/package.json | 1 + todo.md | 4 +- 21 files changed, 557 insertions(+), 81 deletions(-) create mode 100644 src/core/refs/mod.rs create mode 100644 src/core/refs/tests.rs create mode 100644 tests/cli_refs.rs create mode 100644 tests/typescript/refs/.notes/refs.md create mode 100644 tests/typescript/refs/README.md create mode 100644 tests/typescript/refs/__tests__/sum.test.ts create mode 100644 tests/typescript/refs/app.ts create mode 100644 tests/typescript/refs/lib/sum.ts create mode 100644 tests/typescript/refs/lib/summary.ts create mode 100644 tests/typescript/refs/package-lock.json create mode 100644 tests/typescript/refs/package.json diff --git a/README.md b/README.md index 0a42c3c..c9f9669 100644 --- a/README.md +++ b/README.md @@ -60,6 +60,9 @@ 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 +# Moves also warn (never edit) about references the import graph cannot +# see: markdown links, package.json fields, jest.mock strings + # 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 6ea482d..75117f3 100644 --- a/docs/EXAMPLES.md +++ b/docs/EXAMPLES.md @@ -165,15 +165,13 @@ $ jmove mv lib/sum.ts utils/sum.ts --dry-run --json "would_move": "lib/sum.ts", "target": "utils/sum.ts", "would_update": 1, - "affected_files": [ - "app.ts" - ], + "affected_files": ["app.ts"], "would_move_via": "fs", "diff": "--- app.ts\n+++ app.ts\n@@ -1,4 +1,4 @@\n-import { sum } from \"./lib/sum\";\n+import { sum } from \"./utils/sum\";\n..." } ``` -Review `affected_files`; abort and ask the user if the blast radius is +Review `affected_files` and `non_import_refs`; abort and ask the user if the blast radius is unexpected. ### 2. Apply @@ -188,25 +186,26 @@ $ jmove mv lib/sum.ts utils/sum.ts --json "changed_files": [ { "path": "app.ts", - "changes": [ - { - "line": 1, - "old": "./lib/sum", - "new": "./utils/sum" - } - ] + "changes": [{ "line": 1, "old": "./lib/sum", "new": "./utils/sum" }] } ], "moved": 1, "updated_imports": 1, - "moved_via": "fs" + "moved_via": "fs", + "non_import_refs": [ + { "file": "README.md", "line": 1, "token": "lib/sum.ts", + "kind": "path", "text": "Use [sum](./lib/sum.ts) via `lib/sum`." } + ] } ``` `changed_files[].changes[]` lists every rewritten specifier with its 1-based line; `moved` and `updated_imports` are the counters, `moved_via` tells whether the rename went through git (`"git"`, staged) -or the plain filesystem (`"fs"`). +or the plain filesystem (`"fs"`). `non_import_refs` (omitted when empty, +also present in dry-run) names references the import graph cannot see — +markdown links, `package.json` fields, `jest.mock` strings. jmove never +edits those; report them to the user. ### 3. Verify diff --git a/docs/SKILL.md b/docs/SKILL.md index e16325f..dbffb3a 100644 --- a/docs/SKILL.md +++ b/docs/SKILL.md @@ -47,6 +47,13 @@ while the new file stays inside the mapped tree and falls back to a relative specifier otherwise. `extends` chains and 2nd+ candidate lists are not followed (v1); an invalid tsconfig silently means "no aliases". +Non-import references: before writing, `mv` scans markdown/config/text +files and source strings for mentions of moved paths and old specifiers. +Such references are never rewritten (each format has its own semantics); +they are reported as `non_import_refs[]` (`file`, `line`, `token`, `kind`, +`text`) in `mv --json` — dry-run included — and on stderr for humans. +Exit code never changes; surface the list to the user. + ### check — find broken imports and Java layout errors ``` diff --git a/src/cli/fix.rs b/src/cli/fix.rs index 40daca4..fde7cdd 100644 --- a/src/cli/fix.rs +++ b/src/cli/fix.rs @@ -79,7 +79,7 @@ pub fn fix( json: bool, ) -> Flow { let root = flow(json, "fix", root.canonicalize().map_err(JmoveError::from))?; - let source_root = flow(json, "fix", super::normalize_scope(&root, source_root))?; + let source_root = flow(json, "fix", Index::normalize_scope(&root, source_root))?; if let Some(rejected) = fix_reject(rule) { return Err(fail(json, "fix", rejected)); } diff --git a/src/cli/json.rs b/src/cli/json.rs index 72aacb9..cabe4fa 100644 --- a/src/cli/json.rs +++ b/src/cli/json.rs @@ -8,9 +8,11 @@ use serde::Serialize; use crate::core::plan::MovePlan; +use crate::core::refs::NonImportRef; use crate::core::{JmoveError, rel_str}; -use super::output; +use super::output::ChangedFile; +use super::output::{self}; /// Top-level envelope for every `--json` response. #[derive(Debug, Serialize)] @@ -107,26 +109,6 @@ impl ErrorData { } } -/// One rewritten import inside a changed file. -#[derive(Debug, Serialize)] -pub struct Change { - /// 1-based line of the rewritten specifier. - pub line: usize, - /// Specifier text before the move. - pub old: String, - /// Specifier text after the move. - pub new: String, -} - -/// A file whose imports were rewritten, with line-level change details. -#[derive(Debug, Serialize)] -pub struct ChangedFile { - /// Project-relative path of the importer. - pub path: String, - /// Rewritten specifiers, in source order. - pub changes: Vec, -} - /// Success payload of `mv --json` (flattened under `status: "ok"`). #[derive(Debug, Serialize)] pub struct MvData { @@ -142,6 +124,9 @@ pub struct MvData { pub updated_imports: usize, /// Rename backend: `"git"` (every rename staged in the index) or `"fs"`. pub moved_via: &'static str, + /// Non-import textual references left unfixed (omitted when none). + #[serde(skip_serializing_if = "Vec::is_empty")] + pub non_import_refs: Vec, /// Directory moves only: each `(from, to)` relocation (omitted for file moves). #[serde(skip_serializing_if = "Vec::is_empty")] pub moved_files: Vec, @@ -180,7 +165,12 @@ fn dir_moves(plan: &MovePlan) -> Vec { impl MvData { /// Assemble the payload from an applied plan and its change details. #[must_use] - pub fn new(plan: &MovePlan, changed_files: Vec, via_git: bool) -> Self { + pub fn new( + plan: &MovePlan, + changed_files: Vec, + via_git: bool, + non_import_refs: Vec, + ) -> Self { Self { source: rel_str(&plan.source), target: rel_str(&plan.target), @@ -188,6 +178,7 @@ impl MvData { moved: plan.moves.len(), updated_imports: plan.rewrites.len(), moved_via: if via_git { "git" } else { "fs" }, + non_import_refs, // 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(), @@ -211,6 +202,9 @@ pub struct MvDryRunData { pub diff: String, /// Rename backend a real run would use: `"git"` or `"fs"`. pub would_move_via: &'static str, + /// Non-import references a real run would leave unfixed. + #[serde(skip_serializing_if = "Vec::is_empty")] + pub non_import_refs: Vec, /// Directory moves only: every relocation that would happen. #[serde(skip_serializing_if = "Vec::is_empty")] pub would_move_files: Vec, @@ -219,7 +213,12 @@ pub struct MvDryRunData { impl MvDryRunData { /// Assemble the preview payload from a plan and its rendered diff. #[must_use] - pub fn new(plan: &MovePlan, diff: String, via_git: bool) -> Self { + pub fn new( + plan: &MovePlan, + diff: String, + via_git: bool, + non_import_refs: Vec, + ) -> Self { Self { would_move: rel_str(&plan.source), target: rel_str(&plan.target), @@ -231,6 +230,7 @@ impl MvDryRunData { diff, would_move_via: if via_git { "git" } else { "fs" }, would_move_files: dir_moves(plan), + non_import_refs, } } } diff --git a/src/cli/mod.rs b/src/cli/mod.rs index f18956e..c627d5b 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -15,6 +15,7 @@ use clap::{Parser, Subcommand}; use crate::core::apply; use crate::core::index::Index; use crate::core::plan::{self, MovePlan}; +use crate::core::refs; use crate::core::{self, JmoveError, JmoveResult}; use json::{Envelope, ErrorData, MvData, MvDryRunData}; @@ -106,7 +107,7 @@ pub fn run() -> anyhow::Result { &target, dry_run, json, - no_git, + apply::GitMode::from_no_git(no_git), ), Command::Check { json } => check(&args.root, args.source_root.as_deref(), json), Command::Fix { @@ -133,11 +134,10 @@ fn mv( target: &Path, dry_run: bool, json: bool, - no_git: bool, + git: apply::GitMode, ) -> Flow { - 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_root = flow(json, "mv", Index::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) = output::mv_reject(&root, &source, &target) { @@ -147,11 +147,11 @@ fn mv( 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))?; + let hidden = refs::scan(&root, &index, &plan); if dry_run { - return mv_dry_run(&root, json, &plan, git); + return mv_dry_run(&root, json, &plan, git, hidden); } - // Line numbers use spans against the *original* contents, so the JSON - // payload is assembled before the rewrites hit the disk. + // Line numbers use spans against the *original* contents. let changed = if json { flow(json, "mv", output::changed_files(&root, &plan))? } else { @@ -160,27 +160,31 @@ fn mv( let applied = flow(json, "mv", apply::apply(&root, &plan, git))?; if json { - json::print(&Envelope::ok( - "mv", - MvData::new(&plan, changed, applied.via_git), - )); + let data = MvData::new(&plan, changed, applied.via_git, hidden); + json::print(&Envelope::ok("mv", data)); } else { println!("{}", output::mv_summary(&plan, applied.via_git)); + output::report_refs(&hidden); } Ok(exit::OK) } /// 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 { +fn mv_dry_run( + root: &Path, + json: bool, + plan: &MovePlan, + git: apply::GitMode, + hidden: Vec, +) -> Flow { let diff = flow(json, "mv", apply::render_diff(root, plan))?; let via_git = apply::would_use_git(root, &plan.source, git); if json { - json::print(&Envelope::dry_run( - "mv", - MvDryRunData::new(plan, diff, via_git), - )); + let data = MvDryRunData::new(plan, diff, via_git, hidden); + json::print(&Envelope::dry_run("mv", data)); } else { print!("{diff}"); + output::report_refs(&hidden); } Ok(exit::OK) } @@ -192,7 +196,7 @@ fn mv_dry_run(root: &Path, json: bool, plan: &MovePlan, git: apply::GitMode) -> /// command itself succeeded; agents read `total` or the exit code). fn check(root: &Path, source_root: Option<&Path>, json: bool) -> Flow { let root = flow(json, "check", root.canonicalize().map_err(JmoveError::from))?; - let source_root = flow(json, "check", normalize_scope(&root, source_root))?; + let source_root = flow(json, "check", Index::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))?; @@ -217,21 +221,6 @@ fn check(root: &Path, source_root: Option<&Path>, json: bool) -> Flow { Ok(code) } -/// Validate the global `--source-root`: project-relative, existing dir. -fn normalize_scope(root: &Path, scope: Option<&Path>) -> JmoveResult> { - 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(json: bool, operation: &'static str, result: JmoveResult) -> Flow { result.map_err(|err| fail(json, operation, ErrorData::from_core(&err))) diff --git a/src/cli/output/mod.rs b/src/cli/output/mod.rs index a9ad44a..2c22443 100644 --- a/src/cli/output/mod.rs +++ b/src/cli/output/mod.rs @@ -13,7 +13,7 @@ use crate::core::{JmoveResult, rel_str}; mod check; -use super::json::{Change, ChangedFile, ErrorData}; +use super::json::ErrorData; pub use check::{ BrokenImport, CheckData, NameMismatch, broken_imports, name_mismatches, report_check, @@ -24,15 +24,27 @@ pub(crate) fn read_file(root: &Path, rel: &Path) -> JmoveResult { Ok(std::fs::read_to_string(root.join(rel))?) } -/// 1-based line containing the byte offset `byte` in `source`. -#[must_use] -pub fn line_of(source: &str, byte: usize) -> usize { - let upto = source.len().min(byte); - source.as_bytes()[..upto] - .iter() - .filter(|b| **b == b'\n') - .count() - + 1 +/// Re-exported so `output::line_of` call sites stay stable. +pub use crate::core::line_of; + +/// One rewritten import inside a changed file. +#[derive(Debug, serde::Serialize)] +pub struct Change { + /// 1-based line of the rewritten specifier. + pub line: usize, + /// Specifier text before the move. + pub old: String, + /// Specifier text after the move. + pub new: String, +} + +/// A file whose imports were rewritten, with line-level change details. +#[derive(Debug, serde::Serialize)] +pub struct ChangedFile { + /// Project-relative path of the importer. + pub path: String, + /// Rewritten specifiers, in source order. + pub changes: Vec, } /// Group rewrites by importer file; files in sorted order, rewrites of one @@ -148,6 +160,32 @@ pub fn print_error(message: &str, hint: Option<&str>) { } } +/// Human warning block for non-import references (stderr, after the mv +/// result, so stdout stays clean for scripts). Silent when there are none. +pub fn report_refs(refs: &[crate::core::refs::NonImportRef]) { + if refs.is_empty() { + return; + } + eprintln!( + "warning: {} non-import reference {} may need manual fixing:", + refs.len(), + if refs.len() == 1 { + "to the moved file" + } else { + "s to moved files" + } + ); + for r in refs.iter().take(10) { + eprintln!(" {}:{}", r.file, r.line); + } + if refs.len() > 10 { + eprintln!( + " ... and {} more (see --json non_import_refs)", + refs.len() - 10 + ); + } +} + /// `N noun` with a naive English plural. pub(crate) fn plural(count: usize, noun: &str) -> String { if count == 1 { diff --git a/src/core/index/mod.rs b/src/core/index/mod.rs index 92b3419..01c77c2 100644 --- a/src/core/index/mod.rs +++ b/src/core/index/mod.rs @@ -19,7 +19,7 @@ use std::path::{Path, PathBuf}; use ignore::WalkBuilder; -use crate::core::{JmoveResult, normalize_rel_path}; +use crate::core::{JmoveError, JmoveResult, normalize_rel_path}; use crate::parser::java::JavaClassIndex; use crate::parser::resolve::resolve_module; use crate::parser::{ImportRecord, Language, PackageDecl, SourceLanguage, frontend_for}; @@ -55,6 +55,25 @@ pub struct Index { pub aliases: PathAliases, } +/// Validate the global `--source-root`: project-relative, existing dir. +/// Kept next to [`Index::build_scoped`], its only consumer. +impl Index { + /// Resolve `--source-root` against the project root. + pub fn normalize_scope(root: &Path, scope: Option<&Path>) -> JmoveResult> { + let Some(scope) = scope else { + return Ok(None); + }; + let rel = crate::core::rel_from_root(root, scope)?; + if !root.join(&rel).is_dir() { + return Err(JmoveError::InvalidArgument(format!( + "--source-root '{}' is not a directory", + crate::core::rel_str(&rel) + ))); + } + Ok(Some(rel)) + } +} + impl Index { /// Scan `root`, parse every supported source file and build the graph. /// Unreadable or unparseable files are skipped, not fatal. diff --git a/src/core/mod.rs b/src/core/mod.rs index 7d8279c..0c79130 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -10,6 +10,7 @@ pub mod apply; pub mod fix; pub mod index; pub mod plan; +pub mod refs; use std::ffi::OsStr; use std::io; @@ -123,6 +124,17 @@ pub fn rel_str(path: &Path) -> String { .join("/") } +/// 1-based line containing the byte offset `byte` in `source`. +#[must_use] +pub fn line_of(source: &str, byte: usize) -> usize { + let upto = source.len().min(byte); + source.as_bytes()[..upto] + .iter() + .filter(|b| **b == b'\n') + .count() + + 1 +} + /// Convert a user-supplied path to a normalized project-relative path. /// Relative paths are taken against `root`; absolute ones must live /// underneath it. Shared by CLI commands that accept paths. diff --git a/src/core/refs/mod.rs b/src/core/refs/mod.rs new file mode 100644 index 0000000..4daa320 --- /dev/null +++ b/src/core/refs/mod.rs @@ -0,0 +1,205 @@ +//! Non-import textual references to planned moves: report only. +//! +//! The import graph understands `import`/`export from`/`require` and +//! `import()` — but projects also reference files from markdown links, +//! `package.json` fields, `jest.mock("./x")` strings, tsconfig `files` +//! arrays and friends. Auto-rewriting those would require understanding +//! each format's semantics, so `jmove` never edits them; what it must not +//! do is move a file and stay silent while half a readme keeps pointing at +//! the old path. The scan runs before any write, on the same plan the +//! apply step consumes, and returns every suspicious occurrence with its +//! file and line. +//! +//! Matching is boundary-checked substring search over plain tokens (the +//! moved path, its extension-less module form, the exact specifier strings +//! importers used, and the directory prefix for batch moves). Tokens are +//! deliberately conservative: `lib/sum` matches `./lib/sum.ts` but not +//! `lib/summary`; occurrences inside the plan's own rewrite spans are the +//! import statements and are excluded. + +use std::path::{Path, PathBuf}; + +use serde::Serialize; + +use crate::core::index::Index; +use crate::core::plan::MovePlan; +use crate::core::{line_of, normalize_rel_path, rel_str}; + +/// One non-import occurrence of a moved path or specifier. +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] +pub struct NonImportRef { + /// Project-relative file containing the reference. + pub file: String, + /// 1-based line of the occurrence. + pub line: usize, + /// The matched token. + pub token: String, + /// Category of the token: `path`, `module`, `specifier` or `dir`. + /// Patterns are ranked in this order; one entry is kept per line. + pub kind: &'static str, + /// The matched line, trimmed and capped (context for humans/agents). + pub text: String, +} + +/// Text formats beyond source code that may name project files. +const TEXT_EXTS: &[&str] = &[ + "md", "mdx", "json", "json5", "yaml", "yml", "html", "htm", "xml", "txt", "pro", "gradle", +]; + +/// Lockfiles never reference project-local paths and are huge: skipped. +const LOCKFILES: &[&str] = &["package-lock.json", "yarn.lock", "pnpm-lock.yaml"]; + +const MAX_TEXT_BYTES: u64 = 512 * 1024; + +/// Scan the project for references to `plan`'s moves outside the import +/// statements the plan already rewrites. Infallible by design: unreadable +/// files are skipped (the scanner already ignored what it could not read). +pub fn scan(root: &Path, index: &Index, plan: &MovePlan) -> Vec { + let patterns = patterns(root, plan); + if patterns.is_empty() { + return Vec::new(); + } + let mut refs = Vec::new(); + for file in text_files(root, index) { + let Ok(source) = std::fs::read_to_string(root.join(&file)) else { + continue; // binary or vanished between walk and read + }; + let skip = skip_spans(plan, &file); + for (token, kind) in &patterns { + for at in token_occurrences(&source, token) { + if skip.iter().any(|s| s.contains(&at)) { + continue; // the import statements themselves + } + refs.push(NonImportRef { + file: rel_str(&file), + line: line_of(&source, at), + token: token.clone(), + kind, + text: excerpt(&source, at), + }); + } + } + } + // Stable sort keeps the first-inserted (highest-ranked) token per line: + // one warning per reference site, not one per matching pattern. + refs.sort_by(|a, b| (&a.file, a.line).cmp(&(&b.file, b.line))); + refs.dedup_by(|a, b| a.file == b.file && a.line == b.line); + refs +} + +// The search tokens of a plan. `source_root` scoping needs no special +// treatment: every token is already project-relative. +fn patterns(root: &Path, plan: &MovePlan) -> Vec<(String, &'static str)> { + let mut out: Vec<(String, &'static str)> = Vec::new(); + let mut push = |token: String, kind: &'static str| { + if token.len() > 1 && !out.iter().any(|(t, _)| t == &token) { + out.push((token, kind)); + } + }; + for m in &plan.moves { + let path = rel_str(&m.source); + push(path.clone(), "path"); + if let Some((stem, _)) = path.rsplit_once('.') { + push(stem.to_owned(), "module"); + } + } + for rewrite in &plan.rewrites { + push(rewrite.old_text.clone(), "specifier"); + } + if plan.moves.len() > 1 && root.join(&plan.source).is_dir() { + push(format!("{}/", rel_str(&plan.source)), "dir"); + } + out +} + +// Files whose rewrite the plan already performs legitimately: their spans. +fn skip_spans(plan: &MovePlan, file: &Path) -> Vec> { + plan.rewrites + .iter() + .filter(|r| r.file == file) + .map(|r| r.span.clone()) + .collect() +} + +/// Code files from the index plus non-hidden text files on disk. +fn text_files(root: &Path, index: &Index) -> Vec { + let mut out = index.files.sorted(); + let walker = ignore::WalkBuilder::new(root).require_git(false).build(); + for entry in walker.flatten() { + let path = entry.path(); + if entry.path_is_symlink() || !entry.file_type().is_some_and(|t| t.is_file()) { + continue; + } + let name = path + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or_default(); + if LOCKFILES.contains(&name) { + continue; + } + let ext = path + .extension() + .and_then(|e| e.to_str()) + .map(str::to_ascii_lowercase) + .unwrap_or_default(); + if !TEXT_EXTS.contains(&ext.as_str()) { + continue; + } + let Ok(stripped) = path.strip_prefix(root) else { + continue; + }; + let Some(rel) = normalize_rel_path(stripped) else { + continue; + }; + let hidden = rel + .components() + .any(|c| c.as_os_str().to_string_lossy().starts_with('.')); + let small = path + .metadata() + .map(|m| m.len() <= MAX_TEXT_BYTES) + .unwrap_or(false); + if !hidden && small && !out.contains(&rel) { + out.push(rel); + } + } + out.sort(); + out +} + +/// Every offset where `token` occurs with a word-free byte before it and +/// no word/joiner byte after: `lib/sum` matches `./lib/sum.ts` but never +/// `lib/summary` or `lib/sum-2`. +fn token_occurrences(source: &str, token: &str) -> Vec { + let bytes = source.as_bytes(); + let mut hits = Vec::new(); + let mut from = 0; + while let Some(found) = source[from..].find(token) { + let at = from + found; + let end = at + token.len(); + let before_ok = at == 0 || !is_word_byte(bytes[at - 1]); + let after_ok = !bytes + .get(end) + .is_some_and(|c| is_word_byte(*c) || matches!(c, b'-' | b'_')); + if before_ok && after_ok { + hits.push(at); + } + from = end; + } + hits +} + +fn is_word_byte(b: u8) -> bool { + b.is_ascii_alphanumeric() || b == b'_' || b == b'$' || b >= 0x80 +} + +/// The matched line, trimmed, with a hard cap. +fn excerpt(source: &str, at: usize) -> String { + let line_start = source[..at].rfind('\n').map_or(0, |i| i + 1); + let line_end = source[at..].find('\n').map_or(source.len(), |i| at + i); + let line = source[line_start..line_end].trim(); + let cut = line.char_indices().nth(100).map_or(line.len(), |(i, _)| i); + line[..cut].to_owned() +} + +#[cfg(test)] +mod tests; diff --git a/src/core/refs/tests.rs b/src/core/refs/tests.rs new file mode 100644 index 0000000..5dbe580 --- /dev/null +++ b/src/core/refs/tests.rs @@ -0,0 +1,123 @@ +//! Scanner tests: what may be referenced, what must never match. + +use std::path::Path; + +use crate::core::JmoveResult; +use crate::core::index::Index; +use crate::core::plan::plan_move; +use tempfile::TempDir; + +use super::{NonImportRef, scan, token_occurrences}; + +fn fixture() -> JmoveResult { + let dir = TempDir::new()?; + let root = dir.path(); + let write = |rel: &str, body: &str| -> JmoveResult<()> { + fs_create(rel, body, root)?; + Ok(()) + }; + write("lib/sum.ts", "export const sum = 3;\n")?; + write("lib/summary.ts", "export const summary = 's';\n")?; + write("app.ts", "import { sum } from './lib/sum';\n")?; + write( + "README.md", + "Use [sum](./lib/sum.ts) via `lib/sum`.\nSee also lib/summary for text.\nLayout note: lib/ holds helpers.\n", + )?; + write( + "package.json", + "{\"name\": \"refs\", \"main\": \"./lib/sum.ts\"}\n", + )?; + write("__tests__/sum.test.ts", "jest.mock('../lib/sum');\n")?; + write("package-lock.json", "{\"x\": \"./lib/sum\"}\n")?; + write(".notes/refs.md", "stale: ./lib/sum\n")?; + Ok(dir) +} + +fn fs_create(rel: &str, body: &str, root: &Path) -> std::io::Result<()> { + let p = root.join(rel); + std::fs::create_dir_all(p.parent().unwrap())?; + std::fs::write(p, body) +} + +fn files_of(refs: &[NonImportRef]) -> Vec { + refs.iter().map(|r| r.file.clone()).collect() +} + +fn scan_move(src: &str, dst: &str) -> JmoveResult<(TempDir, Vec)> { + let dir = fixture()?; + let index = Index::build(dir.path())?; + let plan = plan_move(&index, Path::new(src), Path::new(dst))?; + let refs = scan(dir.path(), &index, &plan); + Ok((dir, refs)) +} + +#[test] +fn doc_config_and_mock_references_are_reported() -> JmoveResult<()> { + let (_dir, refs) = scan_move("lib/sum.ts", "lib/total.ts")?; + let files = files_of(&refs); + for expected in ["README.md", "package.json", "__tests__/sum.test.ts"] { + assert!( + files.contains(&expected.to_string()), + "{expected} missing: {refs:?}" + ); + } + Ok(()) +} + +#[test] +fn the_import_statement_itself_is_never_reported() -> JmoveResult<()> { + let (_dir, refs) = scan_move("lib/sum.ts", "lib/total.ts")?; + assert!(!files_of(&refs).contains(&"app.ts".to_string()), "{refs:?}"); + assert!(!files_of(&refs).contains(&"lib/sum.ts".to_string())); + Ok(()) +} + +#[test] +fn decoys_lockfiles_and_hidden_dirs_are_not_reported() -> JmoveResult<()> { + let (_dir, refs) = scan_move("lib/sum.ts", "lib/total.ts")?; + let files = files_of(&refs); + for absent in ["lib/summary.ts", "package-lock.json", ".notes/refs.md"] { + assert!( + !files.contains(&absent.to_string()), + "{absent} leaked: {refs:?}" + ); + } + // README line 2 mentions only `lib/summary`: no ref may point there. + assert!( + refs.iter().all(|r| r.file != "README.md" || r.line != 2), + "{refs:?}" + ); + Ok(()) +} + +#[test] +fn one_entry_per_line_keeps_the_highest_ranked_token() -> JmoveResult<()> { + let (_dir, refs) = scan_move("lib/sum.ts", "lib/total.ts")?; + let readme = refs.iter().filter(|r| r.file == "README.md").count(); + // Line 1 (three matching tokens) and line 3 (`lib/` dir note is only a + // file-move token set away) collapse to one entry per line. + assert_eq!(readme, 1, "{refs:?}"); + assert_eq!(refs.iter().find(|r| r.file == "README.md").unwrap().line, 1); + Ok(()) +} + +#[test] +fn dir_move_reports_the_directory_prefix() -> JmoveResult<()> { + let (_dir, refs) = scan_move("lib", "pkg/lib")?; + assert!( + refs.iter() + .any(|r| r.file == "README.md" && r.line == 3 && r.kind == "dir"), + "{refs:?}" + ); + Ok(()) +} + +#[test] +fn boundary_rules_reject_longer_joined_and_prefixed_names() { + let none: Vec = Vec::new(); + assert_eq!(token_occurrences("lib/summary", "lib/sum"), none); + assert_eq!(token_occurrences("lib/sum-2", "lib/sum"), none); + assert_eq!(token_occurrences("mylib/sum", "lib/sum"), none); + assert_eq!(token_occurrences("./lib/sum.ts", "lib/sum"), vec![2]); + assert_eq!(token_occurrences("x/lib/sum", "lib/sum"), vec![2]); +} diff --git a/tests/cli_refs.rs b/tests/cli_refs.rs new file mode 100644 index 0000000..1493bc3 --- /dev/null +++ b/tests/cli_refs.rs @@ -0,0 +1,68 @@ +//! `mv` warns (never edits) about non-import references: markdown links, +//! `package.json` fields and `jest.mock` strings survive silent unless the +//! user sees them. JSON payload carries the same list for agents. + +mod common; + +use common::{copy_fixture, in_root, jmove, read}; +use predicates::prelude::*; + +fn dry_run_refs() -> String { + let tmp = copy_fixture("typescript", "refs"); + let out = jmove( + &tmp, + &["mv", "lib/sum.ts", "lib/total.ts", "--dry-run", "--json"], + ) + .success() + .stdout(predicate::str::contains("\"non_import_refs\"")) + .get_output() + .stdout + .clone(); + String::from_utf8(out).expect("json is utf-8") +} + +#[test] +fn json_dry_run_lists_hidden_references_and_skips_the_noise() { + let out = dry_run_refs(); + for expected in [ + "\"file\": \"README.md\"", + "\"file\": \"package.json\"", + "\"file\": \"__tests__/sum.test.ts\"", + ] { + assert!(out.contains(expected), "missing {expected} in {out}"); + } + for absent in [ + "\"file\": \"app.ts\"", + "\"file\": \"package-lock.json\"", + "\"file\": \".notes/refs.md\"", + "\"file\": \"lib/summary.ts\"", + ] { + assert!(!out.contains(absent), "leaked {absent} in {out}"); + } +} + +#[test] +fn human_dry_run_warns_on_stderr_and_exits_zero() { + let tmp = copy_fixture("typescript", "refs"); + jmove(&tmp, &["mv", "lib/sum.ts", "lib/total.ts", "--dry-run"]) + .success() + .stderr( + predicate::str::contains("non-import reference") + .and(predicate::str::contains("README.md:1")) + .and(predicate::str::contains("__tests__/sum.test.ts:1")), + ); +} + +#[test] +fn real_move_still_succeeds_and_still_warns() { + let tmp = copy_fixture("typescript", "refs"); + jmove(&tmp, &["mv", "lib/sum.ts", "lib/total.ts"]) + .success() + .stderr(predicate::str::contains("may need manual fixing")); + assert!(in_root(tmp.path(), "lib/total.ts").exists()); + let app = read(&in_root(tmp.path(), "app.ts")); + assert!(app.contains("from './lib/total'"), "{app}"); + // The scanner warns; only the import graph is rewritten. + let readme = read(&in_root(tmp.path(), "README.md")); + assert!(readme.contains("./lib/sum.ts"), "{readme}"); +} diff --git a/tests/typescript/refs/.notes/refs.md b/tests/typescript/refs/.notes/refs.md new file mode 100644 index 0000000..6289495 --- /dev/null +++ b/tests/typescript/refs/.notes/refs.md @@ -0,0 +1 @@ +stale ref: ./lib/sum diff --git a/tests/typescript/refs/README.md b/tests/typescript/refs/README.md new file mode 100644 index 0000000..afa69e5 --- /dev/null +++ b/tests/typescript/refs/README.md @@ -0,0 +1,3 @@ +Use [sum](./lib/sum.ts) via `lib/sum`. +See also lib/summary for text. +Layout note: lib/ holds helpers. diff --git a/tests/typescript/refs/__tests__/sum.test.ts b/tests/typescript/refs/__tests__/sum.test.ts new file mode 100644 index 0000000..bf03682 --- /dev/null +++ b/tests/typescript/refs/__tests__/sum.test.ts @@ -0,0 +1 @@ +jest.mock('../lib/sum'); diff --git a/tests/typescript/refs/app.ts b/tests/typescript/refs/app.ts new file mode 100644 index 0000000..e28a9fa --- /dev/null +++ b/tests/typescript/refs/app.ts @@ -0,0 +1,2 @@ +import { sum } from './lib/sum'; +export const x = sum(1, 2); diff --git a/tests/typescript/refs/lib/sum.ts b/tests/typescript/refs/lib/sum.ts new file mode 100644 index 0000000..8073d27 --- /dev/null +++ b/tests/typescript/refs/lib/sum.ts @@ -0,0 +1 @@ +export const sum = (a: number, b: number): number => a + b; diff --git a/tests/typescript/refs/lib/summary.ts b/tests/typescript/refs/lib/summary.ts new file mode 100644 index 0000000..fc920fc --- /dev/null +++ b/tests/typescript/refs/lib/summary.ts @@ -0,0 +1 @@ +export const summary = 'text'; diff --git a/tests/typescript/refs/package-lock.json b/tests/typescript/refs/package-lock.json new file mode 100644 index 0000000..8c799c5 --- /dev/null +++ b/tests/typescript/refs/package-lock.json @@ -0,0 +1 @@ +{ "packages": { "x": "./lib/sum" } } diff --git a/tests/typescript/refs/package.json b/tests/typescript/refs/package.json new file mode 100644 index 0000000..21ec30c --- /dev/null +++ b/tests/typescript/refs/package.json @@ -0,0 +1 @@ +{ "name": "refs-fixture", "main": "./lib/sum.ts" } diff --git a/todo.md b/todo.md index 580817c..1a8d707 100644 --- a/todo.md +++ b/todo.md @@ -96,7 +96,9 @@ AI оставляем СНАРУЖИ: при неоднозначности jmov - [ ] Параллельная индексация через rayon - [x] --git интеграция (git mv для stage/истории): auto для tracked файлов, --no-git флаг, moved_via/would_move_via в --json - [x] Перенос директорий целиком (mv папки): зеркальный batch-move всех индексируемых файлов, merged rewrites, prune пустых исходных каталогов, left_behind для неиндексируемых -- [ ] Предупреждения о не-import ссылках: package.json exports, jest mocks, tsconfig includes, markdown links +- [x] Предупреждения о не-import ссылках: скан text/md/json/yaml/html + строк в коде на path/module/specifier/dir токены (с word-границами); + никогда не правит, только stderr + non_import_refs[] в --json; lockfiles/hidden/>512KiB пропускаются. + Ограничение v1: ссылки из чужих директорий в своей относительной форме ('./sum' из __tests__/ при переносе 'lib/sum') не ловятся - [ ] prettier интеграция после rewrite (по желанию) ## Phase 3 From 98e1f4a3d3a7b46cfdab49fc1e71a1d9fa813b34 Mon Sep 17 00:00:00 2001 From: loki5512344 Date: Tue, 15 Sep 2026 19:50:54 +0200 Subject: [PATCH 08/10] fix: correct pluralization in the non-import reference warning Live smoke printed 'reference s to moved files' (the plural 's' was separated from the noun by the format placeholder). The e2e now pins the exact warning text. --- tests/cli_refs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cli_refs.rs b/tests/cli_refs.rs index 1493bc3..c679dba 100644 --- a/tests/cli_refs.rs +++ b/tests/cli_refs.rs @@ -47,7 +47,7 @@ fn human_dry_run_warns_on_stderr_and_exits_zero() { jmove(&tmp, &["mv", "lib/sum.ts", "lib/total.ts", "--dry-run"]) .success() .stderr( - predicate::str::contains("non-import reference") + predicate::str::contains("3 non-import references to moved files") .and(predicate::str::contains("README.md:1")) .and(predicate::str::contains("__tests__/sum.test.ts:1")), ); From 47395069cb15c84b95c622625cddbae254e82b2d Mon Sep 17 00:00:00 2001 From: loki5512344 Date: Tue, 15 Sep 2026 19:51:43 +0200 Subject: [PATCH 09/10] fix: actually apply the warning pluralization fix The previous commit shipped a no-op: cargo fmt had reflowed the match into multiline form, so the scripted replacement never matched and the failing e2e it introduced caught exactly that. Verified with a full cargo test this time (not a grep-masked pipeline). --- src/cli/output/mod.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/cli/output/mod.rs b/src/cli/output/mod.rs index 2c22443..01ccf7f 100644 --- a/src/cli/output/mod.rs +++ b/src/cli/output/mod.rs @@ -167,13 +167,9 @@ pub fn report_refs(refs: &[crate::core::refs::NonImportRef]) { return; } eprintln!( - "warning: {} non-import reference {} may need manual fixing:", + "warning: {} non-import reference{} to moved files may need manual fixing:", refs.len(), - if refs.len() == 1 { - "to the moved file" - } else { - "s to moved files" - } + if refs.len() == 1 { "" } else { "s" } ); for r in refs.iter().take(10) { eprintln!(" {}:{}", r.file, r.line); From ad1ad4f999ee77e22dffdc809987082d794d25f1 Mon Sep 17 00:00:00 2001 From: loki5512344 Date: Tue, 15 Sep 2026 20:23:20 +0200 Subject: [PATCH 10/10] feat: SARIF 2.1.0 and Checkstyle XML reports for check/fix --report FILE on check and fix writes the findings a run already computed into CI-consumable formats chosen by extension: .sarif (single run, %SRCROOT%-relative URIs, properties.autoFixable, rule registry with descriptions) and .xml (Checkstyle, source=jmove.). Unknown suffixes fail fast with INVALID_ARGUMENT; stdout and exit codes are untouched, and clean runs write valid empty documents. The check handler moved into cli::report (its format siblings); fix emits the candidate list in dry-run and apply modes. Marketing angle from the plan: auto-fix for what Checkstyle only reports. --- README.md | 3 + docs/EXAMPLES.md | 31 ++--- docs/SKILL.md | 16 ++- src/cli/fix.rs | 16 ++- src/cli/mod.rs | 48 +++---- src/cli/report/checkstyle.rs | 115 ++++++++++++++++ src/cli/report/mod.rs | 178 +++++++++++++++++++++++++ src/cli/report/sarif.rs | 247 +++++++++++++++++++++++++++++++++++ tests/cli_report.rs | 122 +++++++++++++++++ todo.md | 6 +- 10 files changed, 726 insertions(+), 56 deletions(-) create mode 100644 src/cli/report/checkstyle.rs create mode 100644 src/cli/report/mod.rs create mode 100644 src/cli/report/sarif.rs create mode 100644 tests/cli_report.rs diff --git a/README.md b/README.md index c9f9669..b34066d 100644 --- a/README.md +++ b/README.md @@ -69,6 +69,9 @@ jmove mv src/com/example/utils/Parser.java src/com/example/core/Parser.java # Find broken imports (exit code 2 if any) jmove check +# Feed CI/IDE: SARIF 2.1.0 or Checkstyle XML (never changes stdout/exit code) +jmove check --report build/jmove.sarif + # Auto-repair import problems (unused + missing imports today) — same dry-run/atomic engine jmove fix --dry-run jmove fix diff --git a/docs/EXAMPLES.md b/docs/EXAMPLES.md index 75117f3..84ad8df 100644 --- a/docs/EXAMPLES.md +++ b/docs/EXAMPLES.md @@ -21,9 +21,7 @@ $ jmove mv lib/sum.ts utils/sum.ts --dry-run @@ -1,4 +1,4 @@ -import { sum } from "./lib/sum"; +import { sum } from "./utils/sum"; - - export function main(): number { - return sum(1, 2); +... move lib/sum.ts -> utils/sum.ts ``` @@ -57,8 +55,7 @@ $ echo $? ### Operating on another project -`--root` points jmove at a project other than the current directory; all -path arguments stay relative to that root: +`--root` points jmove at another project; path arguments stay relative to it: ```console $ jmove --root ~/code/frontend mv src/old.ts src/new.ts --dry-run @@ -75,8 +72,8 @@ $ echo $? ``` jmove never overwrites: free the destination (or pick another name) and -retry. A missing source reports `SOURCE_NOT_FOUND` the same way, and a -file nobody imports simply moves with zero rewrites. +retry. A missing source reports `SOURCE_NOT_FOUND`; an unimported file +simply moves with zero rewrites. ### Java: package + imports + move in one step @@ -87,7 +84,6 @@ is three coordinated edits — jmove makes all of them: $ jmove mv src/main/java/com/example/util/Text.java src/main/java/com/example/core/Text.java --dry-run --- src/main/java/com/example/app/App.java +++ src/main/java/com/example/app/App.java -@@ -1,7 +1,7 @@ package com.example.app; -import com.example.util.Text; @@ -112,8 +108,6 @@ dry-run/atomic engine. Preview first, then apply: ```console $ cat src/main/java/com/example/app/App.java -package com.example.app; - import com.example.Text; import com.example.unused.Ghost; // never referenced @@ -121,7 +115,6 @@ import com.example.unused.Ghost; // never referenced $ jmove fix --dry-run --- src/main/java/com/example/app/App.java +++ src/main/java/com/example/app/App.java -@@ -1,6 +1,5 @@ package com.example.app; import com.example.Text; @@ -226,10 +219,18 @@ $ jmove check --json } ``` -Note: this response keeps `status: "ok"` (the command itself succeeded) -while the process exits `2`; treat a non-zero `total` — or exit code `2` — -as a failed refactor. A clean project returns `"broken_imports": [], "total": 0` -and exit code `0`. +Note: the response keeps `status: "ok"` while the process exits `2`; treat a +non-zero `total` (or exit `2`) as a failed refactor; clean returns exit `0`. + +### CI report (same findings, machine formats) + +```console +$ jmove check --report build/jmove.sarif # SARIF 2.1.0, GitHub/CodeQL +$ jmove fix --report build/jmove.xml # Checkstyle XML, IDEs/Jenkins +``` + +`--report` never changes stdout or the exit code; clean runs write valid +empty documents. Rule ids match the `--json` ones. ### Error shape diff --git a/docs/SKILL.md b/docs/SKILL.md index dbffb3a..d2d9462 100644 --- a/docs/SKILL.md +++ b/docs/SKILL.md @@ -57,7 +57,7 @@ Exit code never changes; surface the list to the user. ### check — find broken imports and Java layout errors ``` -jmove check [--root DIR] [--json] +jmove check [--root DIR] [--json] [--report FILE] ``` Run after any move (or any edit) to validate project consistency. Reports @@ -65,10 +65,22 @@ broken relative imports and Java files whose single public type is named differently from the file (each finding carries the exact `jmove mv` that renames it; exit code 2 covers both kinds). +### report files — SARIF and Checkstyle for CI/IDE + +`check` and `fix` accept `--report FILE`; the format is chosen by the file +name suffix: `.sarif` → SARIF 2.1.0 (single run, `%SRCROOT%`-relative +artifact URIs, `properties.autoFixable`), `.xml` → Checkstyle XML +(`source="jmove."` with `/` mapped to `.`). Unknown suffixes fail +before any work with `INVALID_ARGUMENT`. The report mirrors what the run +already computed (fix: candidates with their applied/manual status; dry-run +included), stdout and exit codes stay as without `--report`, and clean runs +write valid *empty* documents. Rule ids: the four fix rules above, plus +`broken-import` and `java/class-name-mismatch`. + ### fix — auto-repair import problems ``` -jmove fix [--root DIR] [--rule ID] [--dry-run] [--json] +jmove fix [--root DIR] [--rule ID] [--dry-run] [--json] [--report FILE] ``` Runs the deterministic rules over the whole project and applies the diff --git a/src/cli/fix.rs b/src/cli/fix.rs index fde7cdd..71f1d67 100644 --- a/src/cli/fix.rs +++ b/src/cli/fix.rs @@ -15,6 +15,7 @@ use crate::core::{JmoveError, JmoveResult, rel_str}; use crate::parser; use super::json::Envelope; +use super::report::{self, Report}; use super::{Flow, exit, fail, flow, json, output}; /// One reported candidate (JSON element). `applied` is false for @@ -77,7 +78,9 @@ pub fn fix( rule: Option<&str>, dry_run: bool, json: bool, + report: Option<&Path>, ) -> Flow { + let sink = flow(json, "fix", Report::parse(report))?; let root = flow(json, "fix", root.canonicalize().map_err(JmoveError::from))?; let source_root = flow(json, "fix", Index::normalize_scope(&root, source_root))?; if let Some(rejected) = fix_reject(rule) { @@ -87,6 +90,7 @@ pub fn fix( let index = flow(json, "fix", Index::build_scoped(&root, scope))?; let plan = plan_fix(&index, rule); if plan.is_empty() { + flow(json, "fix", Report::emit(&sink, &[]))?; if json { json::print(&Envelope::ok("fix", empty_data())); } else { @@ -95,13 +99,14 @@ pub fn fix( return Ok(exit::OK); } if dry_run { - return fix_dry_run(&root, json, &plan); + return fix_dry_run(&root, json, &plan, &sink); } // Line numbers use spans against the original contents, so the JSON // payload is assembled before any edit reaches the disk. let detail = flow(json, "fix", describe(&root, &plan))?; let edits = plan.auto_edits(); let files_changed = flow(json, "fix", apply::apply_edits(&root, &edits))?; + flow(json, "fix", Report::emit(&sink, &report::from_fix(&detail)))?; if json { let fixes = edits.values().map(Vec::len).sum(); json::print(&Envelope::ok( @@ -151,11 +156,16 @@ fn fix_reject(rule: Option<&str>) -> Option { } /// Dry-run branch: unified diff for humans, structured preview for agents. -fn fix_dry_run(root: &Path, json: bool, plan: &FixPlan) -> Flow { +fn fix_dry_run(root: &Path, json: bool, plan: &FixPlan, sink: &Option) -> Flow { let edits = plan.auto_edits(); let diff = flow(json, "fix", render_edits_diff(root, &edits))?; + let detail = if json || sink.is_some() { + flow(json, "fix", describe(root, plan))? + } else { + Vec::new() + }; + flow(json, "fix", Report::emit(sink, &report::from_fix(&detail)))?; if json { - let detail = flow(json, "fix", describe(root, plan))?; let affected = edits.keys().map(|p| rel_str(p)).collect(); json::print(&Envelope::dry_run( "fix", diff --git a/src/cli/mod.rs b/src/cli/mod.rs index c627d5b..25b4f43 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -6,6 +6,7 @@ pub mod fix; pub mod json; pub mod output; +pub mod report; use std::convert::identity; use std::path::{Path, PathBuf}; @@ -60,6 +61,9 @@ pub enum Command { /// Machine-readable JSON output. #[arg(long)] json: bool, + /// Write findings to a report file: `.sarif` or `.xml` (checkstyle). + #[arg(long)] + report: Option, }, /// Auto-fix small import problems (same engine as `mv`). Fix { @@ -72,6 +76,9 @@ pub enum Command { /// Machine-readable JSON output (for AI agents). #[arg(long)] json: bool, + /// Write the candidate list to a report file: `.sarif` or `.xml`. + #[arg(long)] + report: Option, }, } @@ -109,17 +116,24 @@ pub fn run() -> anyhow::Result { json, apply::GitMode::from_no_git(no_git), ), - Command::Check { json } => check(&args.root, args.source_root.as_deref(), json), + Command::Check { json, report } => report::check( + &args.root, + args.source_root.as_deref(), + json, + report.as_deref(), + ), Command::Fix { rule, dry_run, json, + report, } => fix::fix( &args.root, args.source_root.as_deref(), rule.as_deref(), dry_run, json, + report.as_deref(), ), }; // Handlers report their own failures; both arms carry an exit code. @@ -189,38 +203,6 @@ fn mv_dry_run( Ok(exit::OK) } -/// `check` handler: report relative imports that resolve to nothing. -/// -/// 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, source_root: Option<&Path>, json: bool) -> Flow { - let root = flow(json, "check", root.canonicalize().map_err(JmoveError::from))?; - let source_root = flow(json, "check", Index::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 mismatches = flow(json, "check", output::name_mismatches(&root, &index))?; - let code = if broken.is_empty() && mismatches.is_empty() { - exit::OK - } else { - exit::BROKEN - }; - - if json { - let total = broken.len(); - let data = output::CheckData { - broken_imports: broken, - total, - name_mismatches: mismatches, - }; - json::print(&Envelope::ok("check", data)); - } else { - output::report_check(&broken, &mismatches); - } - Ok(code) -} - /// Unwrap a core result, routing failures through the CLI error channel. fn flow(json: bool, operation: &'static str, result: JmoveResult) -> Flow { result.map_err(|err| fail(json, operation, ErrorData::from_core(&err))) diff --git a/src/cli/report/checkstyle.rs b/src/cli/report/checkstyle.rs new file mode 100644 index 0000000..c9ea088 --- /dev/null +++ b/src/cli/report/checkstyle.rs @@ -0,0 +1,115 @@ +//! Checkstyle XML emission: the de-facto report format for IDEs (IntelliJ, +//! VSCode), Jenkins warnings-ng and GitLab code-quality parsing. +//! +//! `source` carries the jmove rule id namespaced (`jmove.java.unused-import`) +//! so consumers can group by rule. Output is deterministic: files sorted +//! (input is already sorted), errors in source order. + +use std::collections::BTreeMap; +use std::fmt::Write as _; + +use super::Violation; + +/// Render `violations` as a Checkstyle XML document (always valid, even +/// with zero errors). +#[must_use] +pub fn build(violations: &[Violation]) -> String { + let mut out = + String::from("\n\n"); + let mut by_file: BTreeMap<&str, Vec<&Violation>> = BTreeMap::new(); + for v in violations { + by_file.entry(&v.file).or_default().push(v); + } + for (file, entries) in by_file { + let _ = writeln!(out, " ", escape(file)); + for v in entries { + let _ = writeln!( + out, + " ", + v.line, + v.severity, + escape(&v.message), + source(v.rule) + ); + } + out.push_str(" \n"); + } + out.push_str("\n"); + out +} + +// The rule as a dotted pseudo-class name, Checkstyle style. +fn source(rule: &str) -> String { + format!("jmove.{}", rule.replace('/', ".")) +} + +/// XML attribute escaping; `&` first so entities survive. +fn escape(text: &str) -> String { + let mut out = String::with_capacity(text.len()); + for c in text.chars() { + match c { + '&' => out.push_str("&"), + '<' => out.push_str("<"), + '>' => out.push_str(">"), + '"' => out.push_str("""), + _ => out.push(c), + } + } + out +} + +#[cfg(test)] +mod tests { + use super::*; + + fn violation(rule: &'static str, severity: &'static str, file: &str, line: usize) -> Violation { + Violation { + rule, + severity, + message: "he said \"fix & now\"".to_owned(), + file: file.to_owned(), + line, + fixable: true, + } + } + + #[test] + fn groups_files_and_renders_error_attributes() { + let xml = build(&[ + violation("java/unused-import", "warning", "a.java", 1), + violation("java/unused-import", "warning", "a.java", 4), + violation("broken-import", "error", "b.ts", 2), + ]); + assert!( + xml.starts_with("\n"), "{xml}"); + } + + #[test] + fn clean_run_is_a_valid_empty_document() { + let xml = build(&[]); + assert_eq!( + xml, + "\n\n\n" + ); + } +} diff --git a/src/cli/report/mod.rs b/src/cli/report/mod.rs new file mode 100644 index 0000000..f8aa74a --- /dev/null +++ b/src/cli/report/mod.rs @@ -0,0 +1,178 @@ +//! Machine-readable report files, and the `check` command that produces +//! their primary findings. +//! +//! `--report ` (on `check` and `fix`) writes the same findings a run +//! already computes into a CI-consumable format chosen by file extension: +//! SARIF 2.1.0 (`.sarif` — GitHub code scanning, CodeQL upload) or +//! Checkstyle XML (`.xml` — IDEs, Jenkins, GitLab). Unknown extensions +//! fail early with `INVALID_ARGUMENT`, before any indexing. +//! +//! Reports never change stdout or exit codes: `check` still exits `2` when +//! it finds something, and a clean run still writes a *valid empty* report +//! (CI parsers must not choke on green builds). + +use std::path::{Path, PathBuf}; + +use crate::core::index::Index; +use crate::core::{JmoveError, JmoveResult}; + +use crate::cli::fix::FixedFile; +use crate::cli::json::Envelope; +use crate::cli::output::{BrokenImport, NameMismatch}; +use crate::cli::{Flow, exit, flow, json, output}; + +mod checkstyle; +mod sarif; + +/// One finding, detached from the format that renders it. +#[derive(Debug)] +pub struct Violation { + /// Stable rule id, e.g. `java/unused-import` or `broken-import`. + pub rule: &'static str, + /// `error` | `warning` | `info` (mirrors `--json`). + pub severity: &'static str, + /// Human-readable message, same text the terminal output shows. + pub message: String, + /// Project-relative file path, `/` separated. + pub file: String, + /// 1-based line. + pub line: usize, + /// Whether jmove can resolve it itself (fix engine, or the suggested + /// `jmove mv` for layout findings). + pub fixable: bool, +} + +/// A parsed `--report` destination. +#[derive(Debug)] +pub enum Report { + /// SARIF 2.1.0 document. + Sarif(PathBuf), + /// Checkstyle XML document. + Checkstyle(PathBuf), +} + +impl Report { + /// Validate a requested report path (by extension) without touching it. + pub fn parse(requested: Option<&Path>) -> JmoveResult> { + let Some(path) = requested else { + return Ok(None); + }; + let name = path + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or_default(); + let report = if name.ends_with(".sarif") { + Self::Sarif(path.to_path_buf()) + } else if name.ends_with(".xml") { + Self::Checkstyle(path.to_path_buf()) + } else { + return Err(JmoveError::InvalidArgument(format!( + "--report '{name}': unsupported file name" + ))); + }; + Ok(Some(report)) + } + + /// Write the report when one was requested; always valid, also empty. + pub fn emit(sink: &Option, violations: &[Violation]) -> JmoveResult<()> { + let Some(report) = sink else { + return Ok(()); + }; + let (path, content) = match report { + Self::Sarif(path) => (path, sarif::build(violations)), + Self::Checkstyle(path) => (path, checkstyle::build(violations)), + }; + std::fs::write(path, content)?; + Ok(()) + } +} + +/// Findings of `check` as report violations. +#[must_use] +pub fn from_check(broken: &[BrokenImport], mismatches: &[NameMismatch]) -> Vec { + let mut out: Vec = broken + .iter() + .map(|b| Violation { + rule: "broken-import", + severity: "error", + message: format!("cannot resolve '{}'", b.import), + file: b.file.clone(), + line: b.line, + fixable: false, + }) + .collect(); + out.extend(mismatches.iter().map(|m| Violation { + rule: "java/class-name-mismatch", + severity: "error", + message: format!( + "public class '{}' must live in '{}'; {}", + m.public_class, m.expected_file, m.rename + ), + file: m.file.clone(), + line: m.line, + fixable: true, + })); + out +} + +/// Findings of `fix` (the per-file candidate detail) as report violations. +#[must_use] +pub fn from_fix(files: &[FixedFile]) -> Vec { + files + .iter() + .flat_map(|f| { + f.fixes.iter().map(|c| Violation { + rule: c.rule, + severity: c.severity, + message: c.message.clone(), + file: f.path.clone(), + line: c.line, + fixable: c.applied, + }) + }) + .collect() +} + +/// `check` handler: report relative imports that resolve to nothing. +/// +/// Exit code is `2` when at least one finding exists, in both the human +/// and the `--json` mode (the JSON `status` stays `"ok"` — the command +/// itself succeeded; agents read `total` or the exit code). A `--report` +/// file is written regardless of mode and does not alter the exit code. +pub fn check( + root: &Path, + source_root: Option<&Path>, + json: bool, + report: Option<&Path>, +) -> Flow { + let sink = flow(json, "check", Report::parse(report))?; + let root = flow(json, "check", root.canonicalize().map_err(JmoveError::from))?; + let source_root = flow(json, "check", Index::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 mismatches = flow(json, "check", output::name_mismatches(&root, &index))?; + flow( + json, + "check", + Report::emit(&sink, &from_check(&broken, &mismatches)), + )?; + let code = if broken.is_empty() && mismatches.is_empty() { + exit::OK + } else { + exit::BROKEN + }; + + if json { + let total = broken.len(); + let data = output::CheckData { + broken_imports: broken, + total, + name_mismatches: mismatches, + }; + json::print(&Envelope::ok("check", data)); + } else { + output::report_check(&broken, &mismatches); + } + Ok(code) +} diff --git a/src/cli/report/sarif.rs b/src/cli/report/sarif.rs new file mode 100644 index 0000000..e742cdf --- /dev/null +++ b/src/cli/report/sarif.rs @@ -0,0 +1,247 @@ +//! SARIF 2.1.0 emission: single run, single tool driver, plain results. +//! +//! Only the subset consumed by GitHub code scanning / CodeQL upload / +//! VSCode is produced: `ruleId`, `level`, one physical `location` with a +//! start line, and a `properties.autoFixable` flag. `uriBaseId` is +//! `%SRCROOT%` so artifact URIs stay project-relative. + +use serde::Serialize; + +use super::Violation; + +const SCHEMA: &str = "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json"; + +/// Render `violations` as a pretty-printed SARIF document. +#[must_use] +pub fn build(violations: &[Violation]) -> String { + let results = violations + .iter() + .map(|v| SarifResult { + rule_id: v.rule, + level: level(v.severity), + message: Text { + text: v.message.clone(), + }, + locations: vec![Location { + physical_location: Physical { + artifact_location: Artifact { + uri: v.file.clone(), + uri_base_id: "%SRCROOT%", + }, + region: Region { start_line: v.line }, + }, + }], + properties: Props { + auto_fixable: v.fixable, + }, + }) + .collect(); + let doc = Sarif { + schema: SCHEMA, + version: "2.1.0", + runs: vec![Run { + tool: Tool { + driver: Driver { + name: "jmove", + version: env!("CARGO_PKG_VERSION"), + rules: driver_rules(violations), + }, + }, + results, + }], + }; + serde_json::to_string_pretty(&doc).expect("sarif shapes always serialize") +} + +/// SARIF `level`: the spec's vocabulary, mapped from jmove severities. +fn level(severity: &str) -> &'static str { + match severity { + "warning" => "warning", + "info" => "note", + _ => "error", + } +} + +// The driver rule registry: each distinct rule once, sorted, described. +fn driver_rules(violations: &[Violation]) -> Vec { + let mut ids: Vec<&'static str> = Vec::new(); + for v in violations { + if !ids.contains(&v.rule) { + ids.push(v.rule); + } + } + ids.sort_unstable(); + ids.into_iter() + .map(|id| Rule { + id, + short_description: Text { + text: description(id).to_owned(), + }, + }) + .collect() +} + +/// Stable one-line rule descriptions (`shortDescription.text`). +fn description(id: &str) -> &str { + match id { + "broken-import" => "Relative import specifier cannot be resolved", + "java/class-name-mismatch" => "File name must match the public Java type", + "java/unused-import" | "ts/unused-import" => "Import is never referenced", + "java/missing-import" => "Referenced type has no import", + "java/import-order" => "Imports violate the configured order", + other => other, + } +} + +#[derive(Serialize)] +struct Sarif { + #[serde(rename = "$schema")] + schema: &'static str, + version: &'static str, + runs: Vec, +} + +#[derive(Serialize)] +struct Tool { + driver: Driver, +} + +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +struct Driver { + name: &'static str, + version: &'static str, + rules: Vec, +} + +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +struct Rule { + id: &'static str, + short_description: Text, +} + +#[derive(Serialize)] +struct Text { + text: String, +} + +#[derive(Serialize)] +struct Run { + tool: Tool, + results: Vec, +} + +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +struct SarifResult { + rule_id: &'static str, + level: &'static str, + message: Text, + locations: Vec, + properties: Props, +} + +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +struct Location { + physical_location: Physical, +} + +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +struct Physical { + artifact_location: Artifact, + region: Region, +} + +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +struct Artifact { + uri: String, + uri_base_id: &'static str, +} + +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +struct Region { + start_line: usize, +} + +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +struct Props { + auto_fixable: bool, +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::parser; + + fn violation(rule: &'static str, severity: &'static str, file: &str, line: usize) -> Violation { + Violation { + rule, + severity, + message: format!("{rule} at {file}:{line}"), + file: file.to_owned(), + line, + fixable: severity != "error", + } + } + + fn parse(violations: &[Violation]) -> serde_json::Value { + serde_json::from_str(&build(violations)).expect("valid json") + } + + #[test] + fn envelope_and_result_shapes_follow_the_spec() { + let doc = parse(&[ + violation("java/unused-import", "warning", "a.java", 3), + violation("java/import-order", "info", "a.java", 9), + ]); + assert_eq!(doc["version"], "2.1.0"); + assert!(doc["$schema"].is_string()); + let run = &doc["runs"][0]; + assert_eq!(run["tool"]["driver"]["name"], "jmove"); + assert_eq!(run["results"][0]["ruleId"], "java/unused-import"); + assert_eq!(run["results"][0]["level"], "warning"); + let location = &run["results"][0]["locations"][0]["physicalLocation"]; + assert_eq!(location["artifactLocation"]["uri"], "a.java"); + assert_eq!(location["artifactLocation"]["uriBaseId"], "%SRCROOT%"); + assert_eq!(location["region"]["startLine"], 3); + assert_eq!(run["results"][0]["properties"]["autoFixable"], true); + assert_eq!(run["results"][1]["level"], "note"); + } + + #[test] + fn driver_lists_each_rule_once_sorted_with_descriptions() { + let doc = parse(&[ + violation("java/unused-import", "warning", "a.java", 1), + violation("broken-import", "error", "b.ts", 2), + violation("java/unused-import", "warning", "c.java", 3), + ]); + let ids: Vec<&str> = doc["runs"][0]["tool"]["driver"]["rules"] + .as_array() + .unwrap() + .iter() + .map(|r| r["id"].as_str().unwrap()) + .collect(); + assert_eq!(ids, ["broken-import", "java/unused-import"]); + assert_eq!(doc["runs"][0]["results"][1]["level"], "error"); + assert!( + doc["runs"][0]["tool"]["driver"]["rules"][0]["shortDescription"]["text"] + .as_str() + .unwrap() + .starts_with("Relative") + ); + } + + #[test] + fn every_shipped_rule_id_has_a_description() { + for id in parser::rule_ids() { + assert_ne!(description(id), *id, "missing description for {id}"); + } + assert!(!description("broken-import").starts_with("broken-import")); + } +} diff --git a/tests/cli_report.rs b/tests/cli_report.rs new file mode 100644 index 0000000..a710859 --- /dev/null +++ b/tests/cli_report.rs @@ -0,0 +1,122 @@ +//! `--report ` interop: SARIF 2.1.0 and Checkstyle XML for CI/IDE +//! consumers. The reports mirror the findings of `check`/`fix` without +//! changing stdout or exit codes. + +mod common; + +use common::{copy_fixture, in_root, jmove, read}; +use predicates::prelude::*; + +fn report_path(tmp: &tempfile::TempDir, name: &str) -> String { + tmp.path() + .join(name) + .to_str() + .expect("utf-8 path") + .to_owned() +} + +fn sarif(tmp: &tempfile::TempDir, name: &str) -> serde_json::Value { + serde_json::from_str(&read(&tmp.path().join(name))).expect("valid sarif json") +} + +#[test] +fn check_writes_sarif_and_keeps_the_findings_exit_code() { + let tmp = copy_fixture("java", "mismatch"); + let report = report_path(&tmp, "out.sarif"); + jmove(&tmp, &["check", "--report", &report]).code(2); + let doc = sarif(&tmp, "out.sarif"); + let result = &doc["runs"][0]["results"][0]; + assert_eq!(result["ruleId"], "java/class-name-mismatch"); + assert_eq!(result["level"], "error"); + assert!( + result["message"]["text"] + .as_str() + .unwrap() + .contains("jmove mv") + ); + let location = &result["locations"][0]["physicalLocation"]; + assert_eq!( + location["artifactLocation"]["uri"], + "src/main/java/com/example/Bad.java" + ); + assert_eq!(location["region"]["startLine"], 3); + assert_eq!(result["properties"]["autoFixable"], true); +} + +#[test] +fn check_writes_checkstyle_for_broken_imports() { + let tmp = copy_fixture("typescript", "complex"); + let report = report_path(&tmp, "checkstyle.xml"); + jmove(&tmp, &["check", "--report", &report]).code(2); + let xml = read(&in_root(tmp.path(), "checkstyle.xml")); + assert!(xml.starts_with(""), "{xml}"); + assert!( + xml.contains( + "\n"), "{xml}"); +} + +#[test] +fn unknown_report_extension_fails_fast() { + let tmp = copy_fixture("typescript", "basic"); + let report = report_path(&tmp, "out.txt"); + jmove(&tmp, &["check", "--report", &report]) + .code(1) + .stderr(predicate::str::contains("unsupported file name")); + assert!(!tmp.path().join("out.txt").exists()); +} + +#[test] +fn fix_reports_candidates_in_both_formats_without_touching_stdout() { + let tmp = copy_fixture("typescript", "unused"); + let sarif_name = report_path(&tmp, "fix.sarif"); + let xml_name = report_path(&tmp, "fix.xml"); + jmove(&tmp, &["fix", "--dry-run", "--report", &sarif_name]) + .success() + .stdout(predicate::str::contains("-import type { Ghost }")); + jmove(&tmp, &["fix", "--dry-run", "--report", &xml_name]).success(); + let doc = sarif(&tmp, "fix.sarif"); + let result = &doc["runs"][0]["results"][0]; + assert_eq!(result["ruleId"], "ts/unused-import"); + assert_eq!(result["level"], "warning"); + assert_eq!(result["properties"]["autoFixable"], true); + let xml = read(&in_root(tmp.path(), "fix.xml")); + assert!(xml.contains("source=\"jmove.ts.unused-import\""), "{xml}"); + assert!(xml.contains("severity=\"warning\""), "{xml}"); +} + +#[test] +fn applied_fix_writes_report_before_stdout_summary() { + let tmp = copy_fixture("typescript", "unused"); + let report = report_path(&tmp, "applied.sarif"); + jmove(&tmp, &["fix", "--report", &report]) + .success() + .stdout(predicate::str::contains("fixed")); + let doc = sarif(&tmp, "applied.sarif"); + assert!(!doc["runs"][0]["results"].as_array().unwrap().is_empty()); +} diff --git a/todo.md b/todo.md index 1a8d707..c4f7f18 100644 --- a/todo.md +++ b/todo.md @@ -66,9 +66,9 @@ AI оставляем СНАРУЖИ: при неоднозначности jmov - [ ] Форматирование: свой cargo-fmt НЕ строим (вечный long-tail). Только «import formatting» (порядок/группировка — у нас уже есть spans). Опционально `--format-after ` (prettier / google-java-format), не зависимость -- [ ] Интероп PMD/Checkstyle/eslint (фаза 2.5): `jmove fix --report checkstyle.xml` маппит - violation(file,line,rule) на паттерны; на выход SARIF для CI/IDE. - Маркетинг: «auto-fix for what Checkstyle only reports» +- [x] Интероп Checkstyle/eslint (фаза 2.5): `check --report f.sarif|.xml` и `fix --report f.sarif|.xml` — + SARIF 2.1.0 (GitHub/CodeQL, autoFixable) и Checkstyle XML (source=jmove.); формат по расширению, + чистый прогон = валидный пустой файл, stdout/exit не меняются. Маркетинг: «auto-fix for what Checkstyle only reports» ## Guava real-world smoke test (google/guava @ main, JDK21, mvnw) — ПРОВЕРЕНО - [x] mv Primitives primitives→util: 5 правок (4 imports + package), `mvn -pl guava compile`