diff --git a/README.md b/README.md index b34066d..3c733a9 100644 --- a/README.md +++ b/README.md @@ -57,21 +57,12 @@ 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 - -# 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 # 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 84ad8df..183e684 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 ` (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 +`--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 rename is staged); `--no-git` forces a plain filesystem rename. Exit codes: `0` ok · `1` operation error · `2` `check` found broken imports. @@ -21,7 +21,9 @@ $ 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 ``` @@ -40,7 +42,7 @@ Rewrites happen first, the rename last; any failure rolls everything back. ```console $ jmove check -check: no findings +check: no broken imports found ``` When something does point at nothing, `check` prints one line per broken @@ -55,7 +57,8 @@ $ echo $? ### Operating on another project -`--root` points jmove at another project; path arguments stay relative to it: +`--root` points jmove at a project other than the current directory; all +path arguments stay relative to that root: ```console $ jmove --root ~/code/frontend mv src/old.ts src/new.ts --dry-run @@ -72,8 +75,8 @@ $ echo $? ``` jmove never overwrites: free the destination (or pick another name) and -retry. A missing source reports `SOURCE_NOT_FOUND`; an unimported file -simply moves with zero rewrites. +retry. A missing source reports `SOURCE_NOT_FOUND` the same way, and a +file nobody imports simply moves with zero rewrites. ### Java: package + imports + move in one step @@ -84,6 +87,7 @@ 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; @@ -108,6 +112,8 @@ 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 @@ -115,6 +121,7 @@ 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; @@ -158,13 +165,15 @@ $ 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` and `non_import_refs`; abort and ask the user if the blast radius is +Review `affected_files`; abort and ask the user if the blast radius is unexpected. ### 2. Apply @@ -179,26 +188,25 @@ $ 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", - "non_import_refs": [ - { "file": "README.md", "line": 1, "token": "lib/sum.ts", - "kind": "path", "text": "Use [sum](./lib/sum.ts) via `lib/sum`." } - ] + "moved_via": "fs" } ``` `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"`). `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. +or the plain filesystem (`"fs"`). ### 3. Verify @@ -219,18 +227,10 @@ $ jmove check --json } ``` -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. +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`. ### Error shape diff --git a/docs/SKILL.md b/docs/SKILL.md index d2d9462..c2b61b4 100644 --- a/docs/SKILL.md +++ b/docs/SKILL.md @@ -2,8 +2,8 @@ ## What this tool does -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, +Moves or renames source files 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,13 +25,6 @@ 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` @@ -40,47 +33,18 @@ 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". - -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 +### check — find broken imports ``` -jmove check [--root DIR] [--json] [--report FILE] +jmove check [--root DIR] [--json] ``` -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). - -### 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`. +Run after any move (or any edit) to validate project consistency. ### fix — auto-repair import problems ``` -jmove fix [--root DIR] [--rule ID] [--dry-run] [--json] [--report FILE] +jmove fix [--root DIR] [--rule ID] [--dry-run] [--json] ``` Runs the deterministic rules over the whole project and applies the @@ -148,7 +112,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 or Java name mismatches +- `2` — `check` found broken imports ## Rules of use diff --git a/src/cli/fix.rs b/src/cli/fix.rs index 71f1d67..40daca4 100644 --- a/src/cli/fix.rs +++ b/src/cli/fix.rs @@ -15,7 +15,6 @@ 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 @@ -78,11 +77,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))?; + let source_root = flow(json, "fix", super::normalize_scope(&root, source_root))?; if let Some(rejected) = fix_reject(rule) { return Err(fail(json, "fix", rejected)); } @@ -90,7 +87,6 @@ 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 { @@ -99,14 +95,13 @@ pub fn fix( return Ok(exit::OK); } if dry_run { - return fix_dry_run(&root, json, &plan, &sink); + return fix_dry_run(&root, json, &plan); } // 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( @@ -156,16 +151,11 @@ 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, sink: &Option) -> Flow { +fn fix_dry_run(root: &Path, json: bool, plan: &FixPlan) -> 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/json.rs b/src/cli/json.rs index cabe4fa..79f673d 100644 --- a/src/cli/json.rs +++ b/src/cli/json.rs @@ -8,11 +8,9 @@ use serde::Serialize; use crate::core::plan::MovePlan; -use crate::core::refs::NonImportRef; use crate::core::{JmoveError, rel_str}; -use super::output::ChangedFile; -use super::output::{self}; +use super::output; /// Top-level envelope for every `--json` response. #[derive(Debug, Serialize)] @@ -109,6 +107,26 @@ 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 { @@ -118,70 +136,25 @@ pub struct MvData { pub target: String, /// Importer files touched by the move. pub changed_files: Vec, - /// Number of files physically moved (1 for a file move, N for a dir). + /// Number of files moved (always 1 in Phase 1). pub moved: usize, /// Total specifiers rewritten across all importers. pub updated_imports: usize, - /// Rename backend: `"git"` (every rename staged in the index) or `"fs"`. + /// Rename backend: `"git"` (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, - /// 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 { /// Assemble the payload from an applied plan and its change details. #[must_use] - pub fn new( - plan: &MovePlan, - changed_files: Vec, - via_git: bool, - non_import_refs: Vec, - ) -> Self { + pub fn new(plan: &MovePlan, changed_files: Vec, via_git: bool) -> Self { Self { source: rel_str(&plan.source), target: rel_str(&plan.target), changed_files, - moved: plan.moves.len(), + moved: 1, 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(), } } } @@ -202,23 +175,12 @@ 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, } 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, - non_import_refs: Vec, - ) -> Self { + pub fn new(plan: &MovePlan, diff: String, via_git: bool) -> Self { Self { would_move: rel_str(&plan.source), target: rel_str(&plan.target), @@ -229,12 +191,32 @@ impl MvDryRunData { .collect(), diff, would_move_via: if via_git { "git" } else { "fs" }, - would_move_files: dir_moves(plan), - non_import_refs, } } } +/// 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 25b4f43..58d6049 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -6,7 +6,6 @@ pub mod fix; pub mod json; pub mod output; -pub mod report; use std::convert::identity; use std::path::{Path, PathBuf}; @@ -16,10 +15,9 @@ 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}; +use json::{CheckData, Envelope, ErrorData, MvData, MvDryRunData}; /// jmove — move source files, keep every import intact. #[derive(Debug, Parser)] @@ -61,9 +59,6 @@ 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 { @@ -76,9 +71,6 @@ 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, }, } @@ -114,26 +106,19 @@ pub fn run() -> anyhow::Result { &target, dry_run, json, - apply::GitMode::from_no_git(no_git), - ), - Command::Check { json, report } => report::check( - &args.root, - args.source_root.as_deref(), - json, - report.as_deref(), + no_git, ), + Command::Check { json } => check(&args.root, args.source_root.as_deref(), json), 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. @@ -148,10 +133,11 @@ fn mv( target: &Path, dry_run: bool, json: bool, - git: apply::GitMode, + no_git: bool, ) -> 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", Index::normalize_scope(&root, source_root))?; + let source_root = flow(json, "mv", normalize_scope(&root, source_root))?; let source = flow(json, "mv", core::rel_from_root(&root, source))?; let target = flow(json, "mv", core::rel_from_root(&root, target))?; if let Some(rejected) = output::mv_reject(&root, &source, &target) { @@ -161,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, hidden); + return mv_dry_run(&root, json, &plan, git); } - // Line numbers use spans against the *original* contents. + // Line numbers use spans against the *original* contents, so the JSON + // payload is assembled before the rewrites hit the disk. let changed = if json { flow(json, "mv", output::changed_files(&root, &plan))? } else { @@ -174,35 +160,76 @@ fn mv( let applied = flow(json, "mv", apply::apply(&root, &plan, git))?; if json { - let data = MvData::new(&plan, changed, applied.via_git, hidden); - json::print(&Envelope::ok("mv", data)); + json::print(&Envelope::ok( + "mv", + MvData::new(&plan, changed, applied.via_git), + )); } 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, - hidden: Vec, -) -> Flow { +fn mv_dry_run(root: &Path, json: bool, plan: &MovePlan, git: apply::GitMode) -> Flow { let diff = flow(json, "mv", apply::render_diff(root, plan))?; let via_git = apply::would_use_git(root, &plan.source, git); if json { - let data = MvDryRunData::new(plan, diff, via_git, hidden); - json::print(&Envelope::dry_run("mv", data)); + json::print(&Envelope::dry_run( + "mv", + MvDryRunData::new(plan, diff, via_git), + )); } else { print!("{diff}"); - output::report_refs(&hidden); } 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", normalize_scope(&root, source_root))?; + let scope = source_root.as_deref(); + let index = flow(json, "check", Index::build_scoped(&root, scope))?; + let broken = flow(json, "check", output::broken_imports(&root, &index))?; + let code = if broken.is_empty() { + exit::OK + } else { + exit::BROKEN + }; + + if json { + let total = broken.len(); + let data = CheckData { + broken_imports: broken, + total, + }; + json::print(&Envelope::ok("check", data)); + } else { + output::report_check(&broken); + } + 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.rs similarity index 66% rename from src/cli/output/mod.rs rename to src/cli/output.rs index 01ccf7f..10cf3b1 100644 --- a/src/cli/output/mod.rs +++ b/src/cli/output.rs @@ -1,6 +1,5 @@ //! Human-readable rendering plus the payload builders that both output -//! modes share: grouping rewrites, `mv` pre-flight validation, line lookup. -//! The `check` payloads and collectors live in [`check`]. +//! modes share: grouping rewrites, resolving broken imports, line lookup. //! //! Pure functions returning data, except [`report_check`] and //! [`print_error`] which perform the only I/O (stdout and stderr). @@ -8,43 +7,29 @@ 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::{BrokenImport, Change, ChangedFile, ErrorData}; -use super::json::ErrorData; - -pub use check::{ - BrokenImport, CheckData, NameMismatch, broken_imports, name_mismatches, report_check, -}; +/// `check` stdout line when the project has no broken imports. +const CHECK_CLEAN: &str = "check: no broken imports found"; /// Read a project file (project-relative path) as UTF-8 text. -pub(crate) fn read_file(root: &Path, rel: &Path) -> JmoveResult { +fn read_file(root: &Path, rel: &Path) -> JmoveResult { Ok(std::fs::read_to_string(root.join(rel))?) } -/// 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, +/// 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 } /// Group rewrites by importer file; files in sorted order, rewrites of one @@ -57,6 +42,32 @@ 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> { @@ -96,8 +107,7 @@ 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"); } - let src_path = root.join(source); - if !src_path.is_file() && !src_path.is_dir() { + if !root.join(source).is_file() { let msg = format!("source file '{}' does not exist", rel_str(source)); return bad( "SOURCE_NOT_FOUND", @@ -127,23 +137,14 @@ pub fn mv_reject(root: &Path, source: &Path, target: &Path) -> Option } /// `moved src -> tgt, updated N imports in M files` success summary, -/// noting the `git mv` backend and, for directory moves, the file count -/// and anything unindexable that stays behind. +/// noting when the rename went through `git mv`. #[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 {} -> {}{batch}{git}, updated {} {} in {} {}{left}", + "moved {} -> {}{git}, updated {} {} in {} {}", rel_str(&plan.source), rel_str(&plan.target), imports, @@ -152,6 +153,23 @@ 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}"); @@ -160,28 +178,6 @@ 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{} to moved files may need manual fixing:", - refs.len(), - if refs.len() == 1 { "" } else { "s" } - ); - 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/cli/output/check.rs b/src/cli/output/check.rs deleted file mode 100644 index 10e0f75..0000000 --- a/src/cli/output/check.rs +++ /dev/null @@ -1,133 +0,0 @@ -//! `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/report/checkstyle.rs b/src/cli/report/checkstyle.rs deleted file mode 100644 index c9ea088..0000000 --- a/src/cli/report/checkstyle.rs +++ /dev/null @@ -1,115 +0,0 @@ -//! 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 deleted file mode 100644 index f8aa74a..0000000 --- a/src/cli/report/mod.rs +++ /dev/null @@ -1,178 +0,0 @@ -//! 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 deleted file mode 100644 index e742cdf..0000000 --- a/src/cli/report/sarif.rs +++ /dev/null @@ -1,247 +0,0 @@ -//! 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/src/core/apply/diff.rs b/src/core/apply/diff.rs index 2bc8c91..7d36dc6 100644 --- a/src/core/apply/diff.rs +++ b/src/core/apply/diff.rs @@ -18,10 +18,8 @@ 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() { - for m in &plan.moves { - let (src, dst) = (rel_str(&m.source), rel_str(&m.target)); - out.push_str(&format!("move {src} -> {dst}\n")); - } + let (src, dst) = (rel_str(&plan.source), rel_str(&plan.target)); + out.push_str(&format!("move {src} -> {dst}\n")); } Ok(out) } @@ -49,7 +47,7 @@ pub fn render_edits_diff( mod tests { use super::render_diff; use crate::core::JmoveResult; - use crate::core::plan::{FileMove, MovePlan, Rewrite}; + use crate::core::plan::{MovePlan, Rewrite}; use std::path::Path; const OLD: &str = "import {\n fmt,\n} from '../lib/fmt';\n"; @@ -58,25 +56,12 @@ 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(), } } @@ -89,10 +74,7 @@ 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\nmove lib/gfx.ts -> deep/gfx.ts\n"), - "{diff}" - ); + assert!(diff.ends_with("move lib/fmt.ts -> deep/fmt.ts\n")); 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 ad0c172..5e1f679 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::{FileMove, MovePlan}; +use crate::core::plan::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 (directory moves: the - /// requested destination, mirroring [`crate::core::plan::MovePlan::target`]). + /// The moved file's new project-relative path. pub new_path: PathBuf, - /// Whether every physical rename went through `git mv`. + /// Whether the 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 targets, and the renames once they happened. +// dirs created for the target, and the final rename once it happened. #[derive(Default)] struct Run { root: PathBuf, backups: Vec<(PathBuf, Vec)>, dirs: Vec, - // Executed root-relative renames (src, dst, went-through-git), oldest first. - moved: Vec<(PathBuf, PathBuf, bool)>, + // Root-relative (src, dst) of the executed rename. + moved: Option<(PathBuf, PathBuf)>, + via_git: bool, } /// Apply `plan` under `root` atomically (see module docs). Rollback is @@ -80,45 +80,25 @@ 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 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)); + // 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)?; } + self.moved = Some((plan.source.clone(), plan.target.clone())); Ok(Applied { files_rewritten: by_file.len(), new_path: plan.target.clone(), - via_git: !self.moved.is_empty() && self.moved.iter().all(|(_, _, g)| *g), + via_git: self.via_git, }) } - 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()) @@ -151,12 +131,8 @@ impl Run { // problems to its message. fn undo(&mut self, err: JmoveError) -> JmoveError { let mut problems = Vec::new(); - 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 { + if let Some((src, dst)) = self.moved.take() { + let back = if self.via_git { git::mv(&self.root, &dst, &src) } else { fs::rename(self.root.join(&dst), self.root.join(&src)).map_err(Into::into) @@ -180,3 +156,94 @@ 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/index/mod.rs b/src/core/index/mod.rs index 01c77c2..83aa37e 100644 --- a/src/core/index/mod.rs +++ b/src/core/index/mod.rs @@ -9,9 +9,6 @@ mod files; #[cfg(test)] mod tests; -mod tsconfig; - -pub use tsconfig::PathAliases; use std::collections::{BTreeMap, HashMap}; use std::fs; @@ -19,7 +16,7 @@ use std::path::{Path, PathBuf}; use ignore::WalkBuilder; -use crate::core::{JmoveError, JmoveResult, normalize_rel_path}; +use crate::core::{JmoveResult, normalize_rel_path}; use crate::parser::java::JavaClassIndex; use crate::parser::resolve::resolve_module; use crate::parser::{ImportRecord, Language, PackageDecl, SourceLanguage, frontend_for}; @@ -51,27 +48,6 @@ 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, -} - -/// 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 { @@ -88,14 +64,12 @@ 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) @@ -108,14 +82,8 @@ impl Index { java_classes .resolve(&resolved.record.specifier) .map(PathBuf::from) - } 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) + resolve_module(importer, &resolved.record.specifier, &index.files) }; } } diff --git a/src/core/index/tsconfig/mod.rs b/src/core/index/tsconfig/mod.rs deleted file mode 100644 index a024931..0000000 --- a/src/core/index/tsconfig/mod.rs +++ /dev/null @@ -1,212 +0,0 @@ -//! 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 deleted file mode 100644 index ef55f89..0000000 --- a/src/core/index/tsconfig/tests.rs +++ /dev/null @@ -1,80 +0,0 @@ -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/mod.rs b/src/core/mod.rs index 0c79130..7d8279c 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -10,7 +10,6 @@ pub mod apply; pub mod fix; pub mod index; pub mod plan; -pub mod refs; use std::ffi::OsStr; use std::io; @@ -124,17 +123,6 @@ 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/plan/dir.rs b/src/core/plan/dir.rs deleted file mode 100644 index 5118ecb..0000000 --- a/src/core/plan/dir.rs +++ /dev/null @@ -1,235 +0,0 @@ -//! 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")]); - // rel_str: canonical '/' display, stable across platforms. - let moves: Vec<(String, String)> = plan - .moves - .iter() - .map(|m| { - ( - crate::core::rel_str(&m.source), - crate::core::rel_str(&m.target), - ) - }) - .collect(); - assert_eq!( - moves, - [ - ("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. - 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 8c03e79..4c0e580 100644 --- a/src/core/plan/mod.rs +++ b/src/core/plan/mod.rs @@ -2,14 +2,12 @@ //! //! 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`], and the -//! mirrored batch move of a whole directory in [`dir`]. +//! [`specifier`]; the Java package/directory flavour in [`java`]. -mod dir; mod java; -#[cfg(test)] -pub(crate) use dir::tests_support; mod specifier; +#[cfg(test)] +pub(crate) mod tests_support; pub use specifier::relative_specifier; @@ -44,44 +42,24 @@ impl From<&Rewrite> for Edit { } } -/// One physical file relocation inside a plan. +/// Complete plan for moving `source` to `target`. #[derive(Debug, Clone, PartialEq, Eq)] -pub struct FileMove { - /// Project-relative file being moved. +pub struct MovePlan { + /// Project-relative path being moved. pub source: PathBuf, /// Project-relative destination path. pub target: PathBuf, -} - -/// 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). + /// Specifier rewrites, 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. +/// Compute the rewrite plan for `source -> target`. /// -/// 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). +/// 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). 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), @@ -91,52 +69,34 @@ 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) - }, - ) + let rewrites = if SourceLanguage::for_path(&source) == Some(SourceLanguage::Java) { + java::java_rewrites(index, &source, &target)? + } else { + ts_rewrites(index, &source, &target) + }; + Ok(MovePlan { + source, + target, + rewrites, + }) } // Relative-specifier rewrites for the TS/JS flavour of the graph. @@ -147,13 +107,7 @@ fn ts_rewrites(index: &Index, source: &Path, target: &Path) -> Vec { .iter() .filter(|e| e.target.as_deref() == Some(source)); for edge in edges { - // 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)); + let new_text = relative_specifier(&importer, target); if new_text == edge.record.specifier { continue; // no-op rewrite, never reaches the plan } @@ -172,10 +126,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 { @@ -216,7 +170,6 @@ 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 new file mode 100644 index 0000000..ac157ef --- /dev/null +++ b/src/core/plan/tests_support.rs @@ -0,0 +1,20 @@ +//! 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/src/core/refs/mod.rs b/src/core/refs/mod.rs deleted file mode 100644 index 4daa320..0000000 --- a/src/core/refs/mod.rs +++ /dev/null @@ -1,205 +0,0 @@ -//! 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 deleted file mode 100644 index 5dbe580..0000000 --- a/src/core/refs/tests.rs +++ /dev/null @@ -1,123 +0,0 @@ -//! 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/src/parser/java/class_name.rs b/src/parser/java/class_name.rs deleted file mode 100644 index 9ab1019..0000000 --- a/src/parser/java/class_name.rs +++ /dev/null @@ -1,128 +0,0 @@ -//! 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 2554092..2fc62fc 100644 --- a/src/parser/java/mod.rs +++ b/src/parser/java/mod.rs @@ -12,7 +12,6 @@ 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/src/parser/resolve.rs b/src/parser/resolve.rs index 3ac5986..b636895 100644 --- a/src/parser/resolve.rs +++ b/src/parser/resolve.rs @@ -14,10 +14,6 @@ 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"; @@ -49,18 +45,9 @@ 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))?; - 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()); + if files.contains(&base) { + return Some(base); } - let base = base.to_path_buf(); for ext in EXTENSIONS { let candidate = with_ext(&base, ext); if files.contains(&candidate) { diff --git a/tests/apply.rs b/tests/apply.rs deleted file mode 100644 index f6863ff..0000000 --- a/tests/apply.rs +++ /dev/null @@ -1,170 +0,0 @@ -//! 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.rs b/tests/cli.rs index e1f04d8..188f208 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("check: no findings")); + .stdout(predicate::str::contains("no broken imports")); let messy = fixture("complex"); jmove(&messy, &["check"]) diff --git a/tests/cli_alias.rs b/tests/cli_alias.rs deleted file mode 100644 index f5f0948..0000000 --- a/tests/cli_alias.rs +++ /dev/null @@ -1,37 +0,0 @@ -//! 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/cli_dir.rs b/tests/cli_dir.rs deleted file mode 100644 index 1da2797..0000000 --- a/tests/cli_dir.rs +++ /dev/null @@ -1,98 +0,0 @@ -//! 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 0a7d1b8..1ecb885 100644 --- a/tests/cli_fix.rs +++ b/tests/cli_fix.rs @@ -141,6 +141,49 @@ 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` @@ -155,6 +198,56 @@ 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 2c38b17..8dc1566 100644 --- a/tests/cli_git.rs +++ b/tests/cli_git.rs @@ -113,21 +113,3 @@ 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 deleted file mode 100644 index 4b7f017..0000000 --- a/tests/cli_missing_import.rs +++ /dev/null @@ -1,96 +0,0 @@ -//! 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/tests/cli_name_check.rs b/tests/cli_name_check.rs deleted file mode 100644 index 615d77d..0000000 --- a/tests/cli_name_check.rs +++ /dev/null @@ -1,43 +0,0 @@ -//! `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/cli_refs.rs b/tests/cli_refs.rs deleted file mode 100644 index c679dba..0000000 --- a/tests/cli_refs.rs +++ /dev/null @@ -1,68 +0,0 @@ -//! `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("3 non-import references to moved files") - .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/cli_report.rs b/tests/cli_report.rs deleted file mode 100644 index a710859..0000000 --- a/tests/cli_report.rs +++ /dev/null @@ -1,122 +0,0 @@ -//! `--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/tests/java/mismatch/src/main/java/com/example/Bad.java b/tests/java/mismatch/src/main/java/com/example/Bad.java deleted file mode 100644 index c6d6e4c..0000000 --- a/tests/java/mismatch/src/main/java/com/example/Bad.java +++ /dev/null @@ -1,3 +0,0 @@ -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 deleted file mode 100644 index c4e583a..0000000 --- a/tests/java/mismatch/src/main/java/com/example/Good.java +++ /dev/null @@ -1,3 +0,0 @@ -package com.example; - -public class Good {} diff --git a/tests/typescript/aliased/package.json b/tests/typescript/aliased/package.json deleted file mode 100644 index 3f4a4c7..0000000 --- a/tests/typescript/aliased/package.json +++ /dev/null @@ -1 +0,0 @@ -{ "name": "aliased-fixture" } diff --git a/tests/typescript/aliased/src/app.ts b/tests/typescript/aliased/src/app.ts deleted file mode 100644 index 611b2ea..0000000 --- a/tests/typescript/aliased/src/app.ts +++ /dev/null @@ -1,7 +0,0 @@ -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 deleted file mode 100644 index 614457e..0000000 --- a/tests/typescript/aliased/src/config.ts +++ /dev/null @@ -1 +0,0 @@ -export default "cfg"; diff --git a/tests/typescript/aliased/src/utils/log.ts b/tests/typescript/aliased/src/utils/log.ts deleted file mode 100644 index 2cf3ecd..0000000 --- a/tests/typescript/aliased/src/utils/log.ts +++ /dev/null @@ -1,3 +0,0 @@ -export function log(): string { - return ""; -} diff --git a/tests/typescript/aliased/src/utils/str.ts b/tests/typescript/aliased/src/utils/str.ts deleted file mode 100644 index 2b5e1b1..0000000 --- a/tests/typescript/aliased/src/utils/str.ts +++ /dev/null @@ -1,3 +0,0 @@ -export function shout(s: string): string { - return s.toUpperCase(); -} diff --git a/tests/typescript/aliased/tsconfig.json b/tests/typescript/aliased/tsconfig.json deleted file mode 100644 index 26e34f3..0000000 --- a/tests/typescript/aliased/tsconfig.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - // JSONC on purpose: comments and trailing commas must not break loading - "compilerOptions": { - "baseUrl": "./src", - "paths": { - "@utils/*": ["utils/*"], - "@cfg": ["config.ts"], - }, - }, -} diff --git a/tests/typescript/refs/.notes/refs.md b/tests/typescript/refs/.notes/refs.md deleted file mode 100644 index 6289495..0000000 --- a/tests/typescript/refs/.notes/refs.md +++ /dev/null @@ -1 +0,0 @@ -stale ref: ./lib/sum diff --git a/tests/typescript/refs/README.md b/tests/typescript/refs/README.md deleted file mode 100644 index afa69e5..0000000 --- a/tests/typescript/refs/README.md +++ /dev/null @@ -1,3 +0,0 @@ -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 deleted file mode 100644 index bf03682..0000000 --- a/tests/typescript/refs/__tests__/sum.test.ts +++ /dev/null @@ -1 +0,0 @@ -jest.mock('../lib/sum'); diff --git a/tests/typescript/refs/app.ts b/tests/typescript/refs/app.ts deleted file mode 100644 index e28a9fa..0000000 --- a/tests/typescript/refs/app.ts +++ /dev/null @@ -1,2 +0,0 @@ -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 deleted file mode 100644 index 8073d27..0000000 --- a/tests/typescript/refs/lib/sum.ts +++ /dev/null @@ -1 +0,0 @@ -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 deleted file mode 100644 index fc920fc..0000000 --- a/tests/typescript/refs/lib/summary.ts +++ /dev/null @@ -1 +0,0 @@ -export const summary = 'text'; diff --git a/tests/typescript/refs/package-lock.json b/tests/typescript/refs/package-lock.json deleted file mode 100644 index 8c799c5..0000000 --- a/tests/typescript/refs/package-lock.json +++ /dev/null @@ -1 +0,0 @@ -{ "packages": { "x": "./lib/sum" } } diff --git a/tests/typescript/refs/package.json b/tests/typescript/refs/package.json deleted file mode 100644 index 21ec30c..0000000 --- a/tests/typescript/refs/package.json +++ /dev/null @@ -1 +0,0 @@ -{ "name": "refs-fixture", "main": "./lib/sum.ts" } diff --git a/todo.md b/todo.md index c4f7f18..2d62afb 100644 --- a/todo.md +++ b/todo.md @@ -55,9 +55,7 @@ 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 (DONE как ПОВЕРХНОСТЬ check, не fix: починка = переименование файла, а fix-движок умеет только байтовые правки; - check отдаёт находку с готовой командой `jmove mv`, exit code 2; rename не меняет FQN → импорты не трогаются) + import-order (DONE: Google-стиль — statics первыми, ASCII-сортировка, дедуп; конфликтующие с другими правилами откладываются (prune_overlaps по severity) и сходятся за 2-3 прогона), class-name-mismatch - [x] TS v1: unused-imports (DONE: whole-statement delete, все биндинги мертвы → строка уходит; mixed used/unused НЕ трогаем — в ESM импорт исполняет побочные эффекты модуля, partial-удаление specifier'ов отложено осознанно) @@ -66,9 +64,9 @@ AI оставляем СНАРУЖИ: при неоднозначности jmov - [ ] Форматирование: свой cargo-fmt НЕ строим (вечный long-tail). Только «import formatting» (порядок/группировка — у нас уже есть spans). Опционально `--format-after ` (prettier / google-java-format), не зависимость -- [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» +- [ ] Интероп PMD/Checkstyle/eslint (фаза 2.5): `jmove fix --report checkstyle.xml` маппит + violation(file,line,rule) на паттерны; на выход SARIF для CI/IDE. + Маркетинг: «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` @@ -77,12 +75,10 @@ AI оставляем СНАРУЖИ: при неоднозначности jmov явных импорта корректно, НО javac упал: сам перенесённый файл ссылался на соседний `GwtCompatible` БЕЗ импорта (тот же пакет) → после mv ссылка битая. jmove в v1 осознанно НЕ добавляет импорты. Это главный driver для fix/missing-import из Phase 1.6 выше -- [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 (см. выше): +- [ ] (после fix) повторить обе перемещения как `mv` + авто-`fix` и добить compile до SUCCESS + (паттерн воспроизведён и закрыт локально: mv файла с bare-ссылкой на соседний пакет → + `fix` добавил импорт → javac SUCCESS; на реальном guava ещё не прогонялось) +- [x] Индексация в monorepo с дублями пакетов (guava vs android/guava в одном --root): глобальный `--source-root DIR` — индексирует (mv/check/fix) только поддерево, FQN-коллизии исчезают, соседнее дерево не трогается; авто-определение по mv-цели осознанно НЕ делаем (явный флаг предсказуемее, см. KISS) @@ -90,15 +86,11 @@ AI оставляем СНАРУЖИ: при неоднозначности jmov ## Phase 2 - [ ] Кэш индекса на диске (bincode/rkyv) → .jmove/index - [ ] Инкрементальная переиндексация (только изменённые файлы) -- [x] Поддержка tsconfig paths / алиасов (@/...): JSONC-парсер (комментарии/хвостовые запятые), - baseUrl + star/exact keys, longest-prefix wins; при mv алиас сохраняется, если файл остался - в дереве алиаса, иначе fallback на относительный; extends/2+ кандидаты — осознанно не делаем (v1) +- [ ] Поддержка tsconfig paths / алиасов (@/...) - [ ] Параллельная индексация через 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 для неиндексируемых -- [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') не ловятся +- [ ] Перенос директорий целиком (mv папки) +- [ ] Предупреждения о не-import ссылках: package.json exports, jest mocks, tsconfig includes, markdown links - [ ] prettier интеграция после rewrite (по желанию) ## Phase 3