diff --git a/README.md b/README.md index 3c733a9..b34066d 100644 --- a/README.md +++ b/README.md @@ -57,12 +57,21 @@ 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 183e684..84ad8df 100644 --- a/docs/EXAMPLES.md +++ b/docs/EXAMPLES.md @@ -5,8 +5,8 @@ consuming `--json`. All outputs below are captured from the real binary. Flags (see `jmove --help`): `mv [--dry-run] [--json] [--no-git]`, `check [--json]`, and the global `--root ` / -`--source-root ` (index one subtree only — the monorepo disambiguator -for duplicate Java packages). `.gitignore`d files are never indexed. Inside a git repo, `mv` of a tracked file uses `git mv` (the +`--source-root ` (monorepo subtree filter). `` may also be a +directory (mirrored batch move, emptied dirs pruned). Inside a git repo, `mv` of a tracked file uses `git mv` (the rename is staged); `--no-git` forces a plain filesystem rename. Exit codes: `0` ok · `1` operation error · `2` `check` found broken imports. @@ -21,9 +21,7 @@ $ jmove mv lib/sum.ts utils/sum.ts --dry-run @@ -1,4 +1,4 @@ -import { sum } from "./lib/sum"; +import { sum } from "./utils/sum"; - - export function main(): number { - return sum(1, 2); +... move lib/sum.ts -> utils/sum.ts ``` @@ -42,7 +40,7 @@ Rewrites happen first, the rename last; any failure rolls everything back. ```console $ jmove check -check: no broken imports found +check: no findings ``` When something does point at nothing, `check` prints one line per broken @@ -57,8 +55,7 @@ $ echo $? ### Operating on another project -`--root` points jmove at a project other than the current directory; all -path arguments stay relative to that root: +`--root` points jmove at another project; path arguments stay relative to it: ```console $ jmove --root ~/code/frontend mv src/old.ts src/new.ts --dry-run @@ -75,8 +72,8 @@ $ echo $? ``` jmove never overwrites: free the destination (or pick another name) and -retry. A missing source reports `SOURCE_NOT_FOUND` the same way, and a -file nobody imports simply moves with zero rewrites. +retry. A missing source reports `SOURCE_NOT_FOUND`; an unimported file +simply moves with zero rewrites. ### Java: package + imports + move in one step @@ -87,7 +84,6 @@ is three coordinated edits — jmove makes all of them: $ jmove mv src/main/java/com/example/util/Text.java src/main/java/com/example/core/Text.java --dry-run --- src/main/java/com/example/app/App.java +++ src/main/java/com/example/app/App.java -@@ -1,7 +1,7 @@ package com.example.app; -import com.example.util.Text; @@ -112,8 +108,6 @@ dry-run/atomic engine. Preview first, then apply: ```console $ cat src/main/java/com/example/app/App.java -package com.example.app; - import com.example.Text; import com.example.unused.Ghost; // never referenced @@ -121,7 +115,6 @@ import com.example.unused.Ghost; // never referenced $ jmove fix --dry-run --- src/main/java/com/example/app/App.java +++ src/main/java/com/example/app/App.java -@@ -1,6 +1,5 @@ package com.example.app; import com.example.Text; @@ -165,15 +158,13 @@ $ jmove mv lib/sum.ts utils/sum.ts --dry-run --json "would_move": "lib/sum.ts", "target": "utils/sum.ts", "would_update": 1, - "affected_files": [ - "app.ts" - ], + "affected_files": ["app.ts"], "would_move_via": "fs", "diff": "--- app.ts\n+++ app.ts\n@@ -1,4 +1,4 @@\n-import { sum } from \"./lib/sum\";\n+import { sum } from \"./utils/sum\";\n..." } ``` -Review `affected_files`; abort and ask the user if the blast radius is +Review `affected_files` and `non_import_refs`; abort and ask the user if the blast radius is unexpected. ### 2. Apply @@ -188,25 +179,26 @@ $ jmove mv lib/sum.ts utils/sum.ts --json "changed_files": [ { "path": "app.ts", - "changes": [ - { - "line": 1, - "old": "./lib/sum", - "new": "./utils/sum" - } - ] + "changes": [{ "line": 1, "old": "./lib/sum", "new": "./utils/sum" }] } ], "moved": 1, "updated_imports": 1, - "moved_via": "fs" + "moved_via": "fs", + "non_import_refs": [ + { "file": "README.md", "line": 1, "token": "lib/sum.ts", + "kind": "path", "text": "Use [sum](./lib/sum.ts) via `lib/sum`." } + ] } ``` `changed_files[].changes[]` lists every rewritten specifier with its 1-based line; `moved` and `updated_imports` are the counters, `moved_via` tells whether the rename went through git (`"git"`, staged) -or the plain filesystem (`"fs"`). +or the plain filesystem (`"fs"`). `non_import_refs` (omitted when empty, +also present in dry-run) names references the import graph cannot see — +markdown links, `package.json` fields, `jest.mock` strings. jmove never +edits those; report them to the user. ### 3. Verify @@ -227,10 +219,18 @@ $ jmove check --json } ``` -Note: this response keeps `status: "ok"` (the command itself succeeded) -while the process exits `2`; treat a non-zero `total` — or exit code `2` — -as a failed refactor. A clean project returns `"broken_imports": [], "total": 0` -and exit code `0`. +Note: the response keeps `status: "ok"` while the process exits `2`; treat a +non-zero `total` (or exit `2`) as a failed refactor; clean returns exit `0`. + +### CI report (same findings, machine formats) + +```console +$ jmove check --report build/jmove.sarif # SARIF 2.1.0, GitHub/CodeQL +$ jmove fix --report build/jmove.xml # Checkstyle XML, IDEs/Jenkins +``` + +`--report` never changes stdout or the exit code; clean runs write valid +empty documents. Rule ids match the `--json` ones. ### Error shape diff --git a/docs/SKILL.md b/docs/SKILL.md index c2b61b4..d2d9462 100644 --- a/docs/SKILL.md +++ b/docs/SKILL.md @@ -2,8 +2,8 @@ ## What this tool does -Moves or renames source files inside a project and updates every import -statement referencing them. Never breaks imports. Supported: TypeScript, +Moves or renames source files — or whole directories of them — inside a +project and updates every import statement referencing them. Never breaks imports. Supported: TypeScript, JavaScript, Java (package declaration + all importers + the file move are kept in sync); Python/Go on the roadmap. Single binary, no LSP needed. @@ -25,6 +25,13 @@ Always run `--dry-run` first and confirm the change set looks right. Moving onto an existing path fails with `TARGET_EXISTS` — choose another target (Phase 1 has no overwrite mode). +Directory moves: `mv ` relocates every indexed file under +`` mirrored under `` in one atomic batch. `--json` adds +`moved_files[]` (from/to per file) and `left_behind[]` — real files under +`` that are not indexable and deliberately stay where they are. +Emptied source directories are pruned; a directory with leftovers is not. +A source that is nested in its own target fails with `PLAN_REJECTED`. + Git integration: inside a git repository, a tracked file is renamed with `git mv` so the rename is staged (history-preserving `git log --follow` / `git diff -M` work). Untracked files, non-repositories and `--no-git` @@ -33,18 +40,47 @@ working tree unstaged either way — stage or commit them yourself. `--json` reports the choice as `moved_via` (`"git"`/`"fs"`) and, on a dry-run, `would_move_via`. -### check — find broken imports +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 ``` -jmove check [--root DIR] [--json] +jmove check [--root DIR] [--json] [--report FILE] ``` -Run after any move (or any edit) to validate project consistency. +Run after any move (or any edit) to validate project consistency. Reports +broken relative imports and Java files whose single public type is named +differently from the file (each finding carries the exact `jmove mv` that +renames it; exit code 2 covers both kinds). + +### report files — SARIF and Checkstyle for CI/IDE + +`check` and `fix` accept `--report FILE`; the format is chosen by the file +name suffix: `.sarif` → SARIF 2.1.0 (single run, `%SRCROOT%`-relative +artifact URIs, `properties.autoFixable`), `.xml` → Checkstyle XML +(`source="jmove."` with `/` mapped to `.`). Unknown suffixes fail +before any work with `INVALID_ARGUMENT`. The report mirrors what the run +already computed (fix: candidates with their applied/manual status; dry-run +included), stdout and exit codes stay as without `--report`, and clean runs +write valid *empty* documents. Rule ids: the four fix rules above, plus +`broken-import` and `java/class-name-mismatch`. ### fix — auto-repair import problems ``` -jmove fix [--root DIR] [--rule ID] [--dry-run] [--json] +jmove fix [--root DIR] [--rule ID] [--dry-run] [--json] [--report FILE] ``` Runs the deterministic rules over the whole project and applies the @@ -112,7 +148,7 @@ and a `hint` describing the next action. On success, `mv` reports - `0` — success - `1` — operation failed (read `--json` error or stderr) -- `2` — `check` found broken imports +- `2` — `check` found broken imports or Java name mismatches ## Rules of use diff --git a/src/cli/fix.rs b/src/cli/fix.rs index 40daca4..71f1d67 100644 --- a/src/cli/fix.rs +++ b/src/cli/fix.rs @@ -15,6 +15,7 @@ use crate::core::{JmoveError, JmoveResult, rel_str}; use crate::parser; use super::json::Envelope; +use super::report::{self, Report}; use super::{Flow, exit, fail, flow, json, output}; /// One reported candidate (JSON element). `applied` is false for @@ -77,9 +78,11 @@ 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", super::normalize_scope(&root, source_root))?; + let source_root = flow(json, "fix", Index::normalize_scope(&root, source_root))?; if let Some(rejected) = fix_reject(rule) { return Err(fail(json, "fix", rejected)); } @@ -87,6 +90,7 @@ pub fn fix( let index = flow(json, "fix", Index::build_scoped(&root, scope))?; let plan = plan_fix(&index, rule); if plan.is_empty() { + flow(json, "fix", Report::emit(&sink, &[]))?; if json { json::print(&Envelope::ok("fix", empty_data())); } else { @@ -95,13 +99,14 @@ pub fn fix( return Ok(exit::OK); } if dry_run { - return fix_dry_run(&root, json, &plan); + return fix_dry_run(&root, json, &plan, &sink); } // Line numbers use spans against the original contents, so the JSON // payload is assembled before any edit reaches the disk. let detail = flow(json, "fix", describe(&root, &plan))?; let edits = plan.auto_edits(); let files_changed = flow(json, "fix", apply::apply_edits(&root, &edits))?; + flow(json, "fix", Report::emit(&sink, &report::from_fix(&detail)))?; if json { let fixes = edits.values().map(Vec::len).sum(); json::print(&Envelope::ok( @@ -151,11 +156,16 @@ fn fix_reject(rule: Option<&str>) -> Option { } /// Dry-run branch: unified diff for humans, structured preview for agents. -fn fix_dry_run(root: &Path, json: bool, plan: &FixPlan) -> Flow { +fn fix_dry_run(root: &Path, json: bool, plan: &FixPlan, sink: &Option) -> Flow { let edits = plan.auto_edits(); let diff = flow(json, "fix", render_edits_diff(root, &edits))?; + let detail = if json || sink.is_some() { + flow(json, "fix", describe(root, plan))? + } else { + Vec::new() + }; + flow(json, "fix", Report::emit(sink, &report::from_fix(&detail)))?; if json { - let detail = flow(json, "fix", describe(root, plan))?; let affected = edits.keys().map(|p| rel_str(p)).collect(); json::print(&Envelope::dry_run( "fix", diff --git a/src/cli/json.rs b/src/cli/json.rs index 79f673d..cabe4fa 100644 --- a/src/cli/json.rs +++ b/src/cli/json.rs @@ -8,9 +8,11 @@ use serde::Serialize; use crate::core::plan::MovePlan; +use crate::core::refs::NonImportRef; use crate::core::{JmoveError, rel_str}; -use super::output; +use super::output::ChangedFile; +use super::output::{self}; /// Top-level envelope for every `--json` response. #[derive(Debug, Serialize)] @@ -107,26 +109,6 @@ impl ErrorData { } } -/// One rewritten import inside a changed file. -#[derive(Debug, Serialize)] -pub struct Change { - /// 1-based line of the rewritten specifier. - pub line: usize, - /// Specifier text before the move. - pub old: String, - /// Specifier text after the move. - pub new: String, -} - -/// A file whose imports were rewritten, with line-level change details. -#[derive(Debug, Serialize)] -pub struct ChangedFile { - /// Project-relative path of the importer. - pub path: String, - /// Rewritten specifiers, in source order. - pub changes: Vec, -} - /// Success payload of `mv --json` (flattened under `status: "ok"`). #[derive(Debug, Serialize)] pub struct MvData { @@ -136,25 +118,70 @@ pub struct MvData { pub target: String, /// Importer files touched by the move. pub changed_files: Vec, - /// Number of files moved (always 1 in Phase 1). + /// Number of files physically moved (1 for a file move, N for a dir). pub moved: usize, /// Total specifiers rewritten across all importers. pub updated_imports: usize, - /// Rename backend: `"git"` (staged in the index) or `"fs"`. + /// Rename backend: `"git"` (every rename staged in the index) or `"fs"`. pub moved_via: &'static str, + /// 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) -> Self { + pub fn new( + plan: &MovePlan, + changed_files: Vec, + via_git: bool, + non_import_refs: Vec, + ) -> Self { Self { source: rel_str(&plan.source), target: rel_str(&plan.target), changed_files, - moved: 1, + moved: plan.moves.len(), updated_imports: plan.rewrites.len(), moved_via: if via_git { "git" } else { "fs" }, + non_import_refs, + // A single-file move keeps the old contract: no extra fields. + moved_files: dir_moves(plan), + left_behind: plan.left_behind.iter().map(|p| rel_str(p)).collect(), } } } @@ -175,12 +202,23 @@ 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) -> Self { + pub fn new( + plan: &MovePlan, + diff: String, + via_git: bool, + non_import_refs: Vec, + ) -> Self { Self { would_move: rel_str(&plan.source), target: rel_str(&plan.target), @@ -191,32 +229,12 @@ 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 58d6049..25b4f43 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -6,6 +6,7 @@ pub mod fix; pub mod json; pub mod output; +pub mod report; use std::convert::identity; use std::path::{Path, PathBuf}; @@ -15,9 +16,10 @@ 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::{CheckData, Envelope, ErrorData, MvData, MvDryRunData}; +use json::{Envelope, ErrorData, MvData, MvDryRunData}; /// jmove — move source files, keep every import intact. #[derive(Debug, Parser)] @@ -59,6 +61,9 @@ pub enum Command { /// Machine-readable JSON output. #[arg(long)] json: bool, + /// Write findings to a report file: `.sarif` or `.xml` (checkstyle). + #[arg(long)] + report: Option, }, /// Auto-fix small import problems (same engine as `mv`). Fix { @@ -71,6 +76,9 @@ pub enum Command { /// Machine-readable JSON output (for AI agents). #[arg(long)] json: bool, + /// Write the candidate list to a report file: `.sarif` or `.xml`. + #[arg(long)] + report: Option, }, } @@ -106,19 +114,26 @@ pub fn run() -> anyhow::Result { &target, dry_run, json, - no_git, + apply::GitMode::from_no_git(no_git), + ), + Command::Check { json, report } => report::check( + &args.root, + args.source_root.as_deref(), + json, + report.as_deref(), ), - 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. @@ -133,11 +148,10 @@ fn mv( target: &Path, dry_run: bool, json: bool, - no_git: bool, + git: apply::GitMode, ) -> Flow { - let git = apply::GitMode::from_no_git(no_git); let root = flow(json, "mv", root.canonicalize().map_err(JmoveError::from))?; - let source_root = flow(json, "mv", normalize_scope(&root, source_root))?; + let source_root = flow(json, "mv", Index::normalize_scope(&root, source_root))?; let source = flow(json, "mv", core::rel_from_root(&root, source))?; let target = flow(json, "mv", core::rel_from_root(&root, target))?; if let Some(rejected) = output::mv_reject(&root, &source, &target) { @@ -147,11 +161,11 @@ fn mv( let scope = source_root.as_deref(); let index = flow(json, "mv", Index::build_scoped(&root, scope))?; let plan = flow(json, "mv", plan::plan_move(&index, &source, &target))?; + let hidden = refs::scan(&root, &index, &plan); if dry_run { - return mv_dry_run(&root, json, &plan, git); + return mv_dry_run(&root, json, &plan, git, hidden); } - // Line numbers use spans against the *original* contents, so the JSON - // payload is assembled before the rewrites hit the disk. + // Line numbers use spans against the *original* contents. let changed = if json { flow(json, "mv", output::changed_files(&root, &plan))? } else { @@ -160,76 +174,35 @@ fn mv( let applied = flow(json, "mv", apply::apply(&root, &plan, git))?; if json { - json::print(&Envelope::ok( - "mv", - MvData::new(&plan, changed, applied.via_git), - )); + let data = MvData::new(&plan, changed, applied.via_git, hidden); + json::print(&Envelope::ok("mv", data)); } else { println!("{}", output::mv_summary(&plan, applied.via_git)); + output::report_refs(&hidden); } Ok(exit::OK) } /// Dry-run branch: unified diff for humans, structured preview for agents. -fn mv_dry_run(root: &Path, json: bool, plan: &MovePlan, git: apply::GitMode) -> Flow { +fn mv_dry_run( + root: &Path, + json: bool, + plan: &MovePlan, + git: apply::GitMode, + hidden: Vec, +) -> Flow { let diff = flow(json, "mv", apply::render_diff(root, plan))?; let via_git = apply::would_use_git(root, &plan.source, git); if json { - json::print(&Envelope::dry_run( - "mv", - MvDryRunData::new(plan, diff, via_git), - )); + let data = MvDryRunData::new(plan, diff, via_git, hidden); + json::print(&Envelope::dry_run("mv", data)); } else { print!("{diff}"); + output::report_refs(&hidden); } Ok(exit::OK) } -/// `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/check.rs b/src/cli/output/check.rs new file mode 100644 index 0000000..10e0f75 --- /dev/null +++ b/src/cli/output/check.rs @@ -0,0 +1,133 @@ +//! `jmove check` collectors and payloads: unresolvable relative imports +//! plus Java file-name ⇄ public-class mismatches (layout errors that +//! `javac` rejects but import resolution cannot see). + +use serde::Serialize; +use std::path::Path; + +use crate::core::index::Index; +use crate::core::{JmoveResult, rel_str}; +use crate::parser::SourceLanguage; +use crate::parser::java::class_name; + +use super::{line_of, read_file}; + +/// `check` stdout line when the project has no findings. +const CHECK_CLEAN: &str = "check: no findings"; + +/// One unresolvable relative import found by `check`. +#[derive(Debug, Serialize)] +pub struct BrokenImport { + /// Project-relative file declaring the import. + pub file: String, + /// 1-based line of the specifier. + pub line: usize, + /// Specifier text as written. + pub import: String, + /// Stable reason code, currently always `"file_not_found"`. + pub reason: &'static str, +} + +/// A Java file whose single public top-level type is named differently +/// from the file — a `javac` error repaired by renaming the file (imports +/// stay valid: the class FQN does not change). +#[derive(Debug, Serialize)] +pub struct NameMismatch { + /// Project-relative file with the wrong name. + pub file: String, + /// 1-based line of the public type declaration. + pub line: usize, + /// Declared public type. + pub public_class: String, + /// Project-relative file it should live in. + pub expected_file: String, + /// Copy-paste repair command (paths are quoted for safety). + pub rename: String, +} + +/// Success payload of `check --json` (flattened under the envelope). +#[derive(Debug, Serialize)] +pub struct CheckData { + /// Broken imports, sorted by file then line. + pub broken_imports: Vec, + /// Number of broken imports (kept as an explicit counter for agents). + pub total: usize, + /// Java layout findings (omitted from JSON when clean). + #[serde(skip_serializing_if = "Vec::is_empty")] + pub name_mismatches: Vec, +} + +/// Collect every relative import that resolves to nothing in `index`. +/// +/// A specifier starting with `.` whose target is `None` is broken; a bare +/// package specifier without a target is an external dependency, not an +/// error. Results are sorted by file, then line. +pub fn broken_imports(root: &Path, index: &Index) -> JmoveResult> { + let mut broken: Vec = Vec::new(); + for (file, imports) in &index.imports { + for import in imports { + if import.target.is_some() || !import.record.specifier.starts_with('.') { + continue; + } + let text = read_file(root, file)?; + broken.push(BrokenImport { + file: rel_str(file), + line: line_of(&text, import.record.span.start), + import: import.record.specifier.clone(), + reason: "file_not_found", + }); + } + } + broken.sort_by(|a, b| (&a.file, a.line).cmp(&(&b.file, b.line))); + Ok(broken) +} + +/// Java files whose only public top-level type disagrees with the file +/// name; sorted by file, then line. +pub fn name_mismatches(root: &Path, index: &Index) -> JmoveResult> { + let mut out = Vec::new(); + for file in index.files.sorted() { + if SourceLanguage::for_path(&file) != Some(SourceLanguage::Java) { + continue; + } + let text = read_file(root, &file)?; + let Some(found) = class_name::mismatch(&file, &text) else { + continue; + }; + let dir = file.parent().unwrap_or(Path::new("")); + let ext = file.extension().and_then(|e| e.to_str()).unwrap_or("java"); + let expected = dir.join(format!("{}.{}", found.public_class, ext)); + let (old, new) = (rel_str(&file), rel_str(&expected)); + out.push(NameMismatch { + file: old.clone(), + line: line_of(&text, found.span.start), + public_class: found.public_class, + expected_file: new.clone(), + rename: format!("jmove mv '{old}' '{new}'"), + }); + } + out.sort_by_key(|m| (m.file.clone(), m.line)); + Ok(out) +} + +/// Print the human `check` report: the clean note, or one +/// `path:line: cannot resolve 'spec'` line per broken import and one +/// rename-line per Java layout mismatch. +pub fn report_check(broken: &[BrokenImport], mismatches: &[NameMismatch]) { + if broken.is_empty() && mismatches.is_empty() { + println!("{CHECK_CLEAN}"); + return; + } + for entry in broken { + println!( + "{}:{}: cannot resolve '{}'", + entry.file, entry.line, entry.import + ); + } + for m in mismatches { + println!( + "{}:{}: public class '{}' must live in '{}'; fix: {}", + m.file, m.line, m.public_class, m.expected_file, m.rename + ); + } +} diff --git a/src/cli/output.rs b/src/cli/output/mod.rs similarity index 66% rename from src/cli/output.rs rename to src/cli/output/mod.rs index 10cf3b1..01ccf7f 100644 --- a/src/cli/output.rs +++ b/src/cli/output/mod.rs @@ -1,5 +1,6 @@ //! Human-readable rendering plus the payload builders that both output -//! modes share: grouping rewrites, resolving broken imports, line lookup. +//! modes share: grouping rewrites, `mv` pre-flight validation, line lookup. +//! The `check` payloads and collectors live in [`check`]. //! //! Pure functions returning data, except [`report_check`] and //! [`print_error`] which perform the only I/O (stdout and stderr). @@ -7,29 +8,43 @@ 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}; -use super::json::{BrokenImport, Change, ChangedFile, ErrorData}; +mod check; -/// `check` stdout line when the project has no broken imports. -const CHECK_CLEAN: &str = "check: no broken imports found"; +use super::json::ErrorData; + +pub use check::{ + BrokenImport, CheckData, NameMismatch, broken_imports, name_mismatches, report_check, +}; /// Read a project file (project-relative path) as UTF-8 text. -fn read_file(root: &Path, rel: &Path) -> JmoveResult { +pub(crate) fn read_file(root: &Path, rel: &Path) -> JmoveResult { Ok(std::fs::read_to_string(root.join(rel))?) } -/// 1-based line containing the byte offset `byte` in `source`. -#[must_use] -pub fn line_of(source: &str, byte: usize) -> usize { - let upto = source.len().min(byte); - source.as_bytes()[..upto] - .iter() - .filter(|b| **b == b'\n') - .count() - + 1 +/// Re-exported so `output::line_of` call sites stay stable. +pub use crate::core::line_of; + +/// One rewritten import inside a changed file. +#[derive(Debug, serde::Serialize)] +pub struct Change { + /// 1-based line of the rewritten specifier. + pub line: usize, + /// Specifier text before the move. + pub old: String, + /// Specifier text after the move. + pub new: String, +} + +/// A file whose imports were rewritten, with line-level change details. +#[derive(Debug, serde::Serialize)] +pub struct ChangedFile { + /// Project-relative path of the importer. + pub path: String, + /// Rewritten specifiers, in source order. + pub changes: Vec, } /// Group rewrites by importer file; files in sorted order, rewrites of one @@ -42,32 +57,6 @@ pub fn group_by_file(rewrites: &[Rewrite]) -> Vec<(&Path, Vec<&Rewrite>)> { } map.into_iter().collect() } - -/// Collect every relative import that resolves to nothing in `index`. -/// -/// A specifier starting with `.` whose target is `None` is broken; a bare -/// package specifier without a target is an external dependency, not an -/// error. Results are sorted by file, then line. -pub fn broken_imports(root: &Path, index: &Index) -> JmoveResult> { - let mut broken: Vec = Vec::new(); - for (file, imports) in &index.imports { - for import in imports { - if import.target.is_some() || !import.record.specifier.starts_with('.') { - continue; - } - let text = read_file(root, file)?; - broken.push(BrokenImport { - file: rel_str(file), - line: line_of(&text, import.record.span.start), - import: import.record.specifier.clone(), - reason: "file_not_found", - }); - } - } - broken.sort_by(|a, b| (&a.file, a.line).cmp(&(&b.file, b.line))); - Ok(broken) -} - /// Build the `changed_files` payload: line-level specifier diffs grouped per /// importer, computed against the on-disk contents at call time. pub fn changed_files(root: &Path, plan: &MovePlan) -> JmoveResult> { @@ -107,7 +96,8 @@ pub fn mv_reject(root: &Path, source: &Path, target: &Path) -> Option let msg = "source and target are the same path".into(); return bad("INVALID_ARGUMENT", msg, "pick a different destination"); } - if !root.join(source).is_file() { + let src_path = root.join(source); + if !src_path.is_file() && !src_path.is_dir() { let msg = format!("source file '{}' does not exist", rel_str(source)); return bad( "SOURCE_NOT_FOUND", @@ -137,14 +127,23 @@ pub fn mv_reject(root: &Path, source: &Path, target: &Path) -> Option } /// `moved src -> tgt, updated N imports in M files` success summary, -/// noting when the rename went through `git mv`. +/// noting the `git mv` backend and, for directory moves, the file count +/// and anything unindexable that stays behind. #[must_use] pub fn mv_summary(plan: &MovePlan, via_git: bool) -> String { let imports = plan.rewrites.len(); let files = group_by_file(&plan.rewrites).len(); let git = if via_git { " (via git mv)" } else { "" }; + let batch = match plan.moves.len() { + 1 => String::new(), + n => format!(" ({n} files)"), + }; + let left = match plan.left_behind.len() { + 0 => String::new(), + n => format!(", {} unsupported {} left behind", n, plural(n, "file")), + }; format!( - "moved {} -> {}{git}, updated {} {} in {} {}", + "moved {} -> {}{batch}{git}, updated {} {} in {} {}{left}", rel_str(&plan.source), rel_str(&plan.target), imports, @@ -153,23 +152,6 @@ pub fn mv_summary(plan: &MovePlan, via_git: bool) -> String { plural(files, "file"), ) } - -/// Print the human `check` report: the clean note, or one -/// `path:line: cannot resolve 'spec'` line per broken import. -pub fn report_check(broken: &[BrokenImport]) { - if broken.is_empty() { - println!("{CHECK_CLEAN}"); - return; - } - for entry in broken { - let line = format!( - "{}:{}: cannot resolve '{}'", - entry.file, entry.line, entry.import - ); - println!("{line}"); - } -} - /// Print an operation error to stderr, with the hint on its own line. pub fn print_error(message: &str, hint: Option<&str>) { eprintln!("jmove: {message}"); @@ -178,6 +160,28 @@ 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/report/checkstyle.rs b/src/cli/report/checkstyle.rs new file mode 100644 index 0000000..c9ea088 --- /dev/null +++ b/src/cli/report/checkstyle.rs @@ -0,0 +1,115 @@ +//! Checkstyle XML emission: the de-facto report format for IDEs (IntelliJ, +//! VSCode), Jenkins warnings-ng and GitLab code-quality parsing. +//! +//! `source` carries the jmove rule id namespaced (`jmove.java.unused-import`) +//! so consumers can group by rule. Output is deterministic: files sorted +//! (input is already sorted), errors in source order. + +use std::collections::BTreeMap; +use std::fmt::Write as _; + +use super::Violation; + +/// Render `violations` as a Checkstyle XML document (always valid, even +/// with zero errors). +#[must_use] +pub fn build(violations: &[Violation]) -> String { + let mut out = + String::from("\n\n"); + let mut by_file: BTreeMap<&str, Vec<&Violation>> = BTreeMap::new(); + for v in violations { + by_file.entry(&v.file).or_default().push(v); + } + for (file, entries) in by_file { + let _ = writeln!(out, " ", escape(file)); + for v in entries { + let _ = writeln!( + out, + " ", + v.line, + v.severity, + escape(&v.message), + source(v.rule) + ); + } + out.push_str(" \n"); + } + out.push_str("\n"); + out +} + +// The rule as a dotted pseudo-class name, Checkstyle style. +fn source(rule: &str) -> String { + format!("jmove.{}", rule.replace('/', ".")) +} + +/// XML attribute escaping; `&` first so entities survive. +fn escape(text: &str) -> String { + let mut out = String::with_capacity(text.len()); + for c in text.chars() { + match c { + '&' => out.push_str("&"), + '<' => out.push_str("<"), + '>' => out.push_str(">"), + '"' => out.push_str("""), + _ => out.push(c), + } + } + out +} + +#[cfg(test)] +mod tests { + use super::*; + + fn violation(rule: &'static str, severity: &'static str, file: &str, line: usize) -> Violation { + Violation { + rule, + severity, + message: "he said \"fix & now\"".to_owned(), + file: file.to_owned(), + line, + fixable: true, + } + } + + #[test] + fn groups_files_and_renders_error_attributes() { + let xml = build(&[ + violation("java/unused-import", "warning", "a.java", 1), + violation("java/unused-import", "warning", "a.java", 4), + violation("broken-import", "error", "b.ts", 2), + ]); + assert!( + xml.starts_with("\n"), "{xml}"); + } + + #[test] + fn clean_run_is_a_valid_empty_document() { + let xml = build(&[]); + assert_eq!( + xml, + "\n\n\n" + ); + } +} diff --git a/src/cli/report/mod.rs b/src/cli/report/mod.rs new file mode 100644 index 0000000..f8aa74a --- /dev/null +++ b/src/cli/report/mod.rs @@ -0,0 +1,178 @@ +//! Machine-readable report files, and the `check` command that produces +//! their primary findings. +//! +//! `--report ` (on `check` and `fix`) writes the same findings a run +//! already computes into a CI-consumable format chosen by file extension: +//! SARIF 2.1.0 (`.sarif` — GitHub code scanning, CodeQL upload) or +//! Checkstyle XML (`.xml` — IDEs, Jenkins, GitLab). Unknown extensions +//! fail early with `INVALID_ARGUMENT`, before any indexing. +//! +//! Reports never change stdout or exit codes: `check` still exits `2` when +//! it finds something, and a clean run still writes a *valid empty* report +//! (CI parsers must not choke on green builds). + +use std::path::{Path, PathBuf}; + +use crate::core::index::Index; +use crate::core::{JmoveError, JmoveResult}; + +use crate::cli::fix::FixedFile; +use crate::cli::json::Envelope; +use crate::cli::output::{BrokenImport, NameMismatch}; +use crate::cli::{Flow, exit, flow, json, output}; + +mod checkstyle; +mod sarif; + +/// One finding, detached from the format that renders it. +#[derive(Debug)] +pub struct Violation { + /// Stable rule id, e.g. `java/unused-import` or `broken-import`. + pub rule: &'static str, + /// `error` | `warning` | `info` (mirrors `--json`). + pub severity: &'static str, + /// Human-readable message, same text the terminal output shows. + pub message: String, + /// Project-relative file path, `/` separated. + pub file: String, + /// 1-based line. + pub line: usize, + /// Whether jmove can resolve it itself (fix engine, or the suggested + /// `jmove mv` for layout findings). + pub fixable: bool, +} + +/// A parsed `--report` destination. +#[derive(Debug)] +pub enum Report { + /// SARIF 2.1.0 document. + Sarif(PathBuf), + /// Checkstyle XML document. + Checkstyle(PathBuf), +} + +impl Report { + /// Validate a requested report path (by extension) without touching it. + pub fn parse(requested: Option<&Path>) -> JmoveResult> { + let Some(path) = requested else { + return Ok(None); + }; + let name = path + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or_default(); + let report = if name.ends_with(".sarif") { + Self::Sarif(path.to_path_buf()) + } else if name.ends_with(".xml") { + Self::Checkstyle(path.to_path_buf()) + } else { + return Err(JmoveError::InvalidArgument(format!( + "--report '{name}': unsupported file name" + ))); + }; + Ok(Some(report)) + } + + /// Write the report when one was requested; always valid, also empty. + pub fn emit(sink: &Option, violations: &[Violation]) -> JmoveResult<()> { + let Some(report) = sink else { + return Ok(()); + }; + let (path, content) = match report { + Self::Sarif(path) => (path, sarif::build(violations)), + Self::Checkstyle(path) => (path, checkstyle::build(violations)), + }; + std::fs::write(path, content)?; + Ok(()) + } +} + +/// Findings of `check` as report violations. +#[must_use] +pub fn from_check(broken: &[BrokenImport], mismatches: &[NameMismatch]) -> Vec { + let mut out: Vec = broken + .iter() + .map(|b| Violation { + rule: "broken-import", + severity: "error", + message: format!("cannot resolve '{}'", b.import), + file: b.file.clone(), + line: b.line, + fixable: false, + }) + .collect(); + out.extend(mismatches.iter().map(|m| Violation { + rule: "java/class-name-mismatch", + severity: "error", + message: format!( + "public class '{}' must live in '{}'; {}", + m.public_class, m.expected_file, m.rename + ), + file: m.file.clone(), + line: m.line, + fixable: true, + })); + out +} + +/// Findings of `fix` (the per-file candidate detail) as report violations. +#[must_use] +pub fn from_fix(files: &[FixedFile]) -> Vec { + files + .iter() + .flat_map(|f| { + f.fixes.iter().map(|c| Violation { + rule: c.rule, + severity: c.severity, + message: c.message.clone(), + file: f.path.clone(), + line: c.line, + fixable: c.applied, + }) + }) + .collect() +} + +/// `check` handler: report relative imports that resolve to nothing. +/// +/// Exit code is `2` when at least one finding exists, in both the human +/// and the `--json` mode (the JSON `status` stays `"ok"` — the command +/// itself succeeded; agents read `total` or the exit code). A `--report` +/// file is written regardless of mode and does not alter the exit code. +pub fn check( + root: &Path, + source_root: Option<&Path>, + json: bool, + report: Option<&Path>, +) -> Flow { + let sink = flow(json, "check", Report::parse(report))?; + let root = flow(json, "check", root.canonicalize().map_err(JmoveError::from))?; + let source_root = flow(json, "check", Index::normalize_scope(&root, source_root))?; + let scope = source_root.as_deref(); + let index = flow(json, "check", Index::build_scoped(&root, scope))?; + let broken = flow(json, "check", output::broken_imports(&root, &index))?; + let mismatches = flow(json, "check", output::name_mismatches(&root, &index))?; + flow( + json, + "check", + Report::emit(&sink, &from_check(&broken, &mismatches)), + )?; + let code = if broken.is_empty() && mismatches.is_empty() { + exit::OK + } else { + exit::BROKEN + }; + + if json { + let total = broken.len(); + let data = output::CheckData { + broken_imports: broken, + total, + name_mismatches: mismatches, + }; + json::print(&Envelope::ok("check", data)); + } else { + output::report_check(&broken, &mismatches); + } + Ok(code) +} diff --git a/src/cli/report/sarif.rs b/src/cli/report/sarif.rs new file mode 100644 index 0000000..e742cdf --- /dev/null +++ b/src/cli/report/sarif.rs @@ -0,0 +1,247 @@ +//! SARIF 2.1.0 emission: single run, single tool driver, plain results. +//! +//! Only the subset consumed by GitHub code scanning / CodeQL upload / +//! VSCode is produced: `ruleId`, `level`, one physical `location` with a +//! start line, and a `properties.autoFixable` flag. `uriBaseId` is +//! `%SRCROOT%` so artifact URIs stay project-relative. + +use serde::Serialize; + +use super::Violation; + +const SCHEMA: &str = "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json"; + +/// Render `violations` as a pretty-printed SARIF document. +#[must_use] +pub fn build(violations: &[Violation]) -> String { + let results = violations + .iter() + .map(|v| SarifResult { + rule_id: v.rule, + level: level(v.severity), + message: Text { + text: v.message.clone(), + }, + locations: vec![Location { + physical_location: Physical { + artifact_location: Artifact { + uri: v.file.clone(), + uri_base_id: "%SRCROOT%", + }, + region: Region { start_line: v.line }, + }, + }], + properties: Props { + auto_fixable: v.fixable, + }, + }) + .collect(); + let doc = Sarif { + schema: SCHEMA, + version: "2.1.0", + runs: vec![Run { + tool: Tool { + driver: Driver { + name: "jmove", + version: env!("CARGO_PKG_VERSION"), + rules: driver_rules(violations), + }, + }, + results, + }], + }; + serde_json::to_string_pretty(&doc).expect("sarif shapes always serialize") +} + +/// SARIF `level`: the spec's vocabulary, mapped from jmove severities. +fn level(severity: &str) -> &'static str { + match severity { + "warning" => "warning", + "info" => "note", + _ => "error", + } +} + +// The driver rule registry: each distinct rule once, sorted, described. +fn driver_rules(violations: &[Violation]) -> Vec { + let mut ids: Vec<&'static str> = Vec::new(); + for v in violations { + if !ids.contains(&v.rule) { + ids.push(v.rule); + } + } + ids.sort_unstable(); + ids.into_iter() + .map(|id| Rule { + id, + short_description: Text { + text: description(id).to_owned(), + }, + }) + .collect() +} + +/// Stable one-line rule descriptions (`shortDescription.text`). +fn description(id: &str) -> &str { + match id { + "broken-import" => "Relative import specifier cannot be resolved", + "java/class-name-mismatch" => "File name must match the public Java type", + "java/unused-import" | "ts/unused-import" => "Import is never referenced", + "java/missing-import" => "Referenced type has no import", + "java/import-order" => "Imports violate the configured order", + other => other, + } +} + +#[derive(Serialize)] +struct Sarif { + #[serde(rename = "$schema")] + schema: &'static str, + version: &'static str, + runs: Vec, +} + +#[derive(Serialize)] +struct Tool { + driver: Driver, +} + +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +struct Driver { + name: &'static str, + version: &'static str, + rules: Vec, +} + +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +struct Rule { + id: &'static str, + short_description: Text, +} + +#[derive(Serialize)] +struct Text { + text: String, +} + +#[derive(Serialize)] +struct Run { + tool: Tool, + results: Vec, +} + +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +struct SarifResult { + rule_id: &'static str, + level: &'static str, + message: Text, + locations: Vec, + properties: Props, +} + +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +struct Location { + physical_location: Physical, +} + +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +struct Physical { + artifact_location: Artifact, + region: Region, +} + +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +struct Artifact { + uri: String, + uri_base_id: &'static str, +} + +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +struct Region { + start_line: usize, +} + +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +struct Props { + auto_fixable: bool, +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::parser; + + fn violation(rule: &'static str, severity: &'static str, file: &str, line: usize) -> Violation { + Violation { + rule, + severity, + message: format!("{rule} at {file}:{line}"), + file: file.to_owned(), + line, + fixable: severity != "error", + } + } + + fn parse(violations: &[Violation]) -> serde_json::Value { + serde_json::from_str(&build(violations)).expect("valid json") + } + + #[test] + fn envelope_and_result_shapes_follow_the_spec() { + let doc = parse(&[ + violation("java/unused-import", "warning", "a.java", 3), + violation("java/import-order", "info", "a.java", 9), + ]); + assert_eq!(doc["version"], "2.1.0"); + assert!(doc["$schema"].is_string()); + let run = &doc["runs"][0]; + assert_eq!(run["tool"]["driver"]["name"], "jmove"); + assert_eq!(run["results"][0]["ruleId"], "java/unused-import"); + assert_eq!(run["results"][0]["level"], "warning"); + let location = &run["results"][0]["locations"][0]["physicalLocation"]; + assert_eq!(location["artifactLocation"]["uri"], "a.java"); + assert_eq!(location["artifactLocation"]["uriBaseId"], "%SRCROOT%"); + assert_eq!(location["region"]["startLine"], 3); + assert_eq!(run["results"][0]["properties"]["autoFixable"], true); + assert_eq!(run["results"][1]["level"], "note"); + } + + #[test] + fn driver_lists_each_rule_once_sorted_with_descriptions() { + let doc = parse(&[ + violation("java/unused-import", "warning", "a.java", 1), + violation("broken-import", "error", "b.ts", 2), + violation("java/unused-import", "warning", "c.java", 3), + ]); + let ids: Vec<&str> = doc["runs"][0]["tool"]["driver"]["rules"] + .as_array() + .unwrap() + .iter() + .map(|r| r["id"].as_str().unwrap()) + .collect(); + assert_eq!(ids, ["broken-import", "java/unused-import"]); + assert_eq!(doc["runs"][0]["results"][1]["level"], "error"); + assert!( + doc["runs"][0]["tool"]["driver"]["rules"][0]["shortDescription"]["text"] + .as_str() + .unwrap() + .starts_with("Relative") + ); + } + + #[test] + fn every_shipped_rule_id_has_a_description() { + for id in parser::rule_ids() { + assert_ne!(description(id), *id, "missing description for {id}"); + } + assert!(!description("broken-import").starts_with("broken-import")); + } +} diff --git a/src/core/apply/diff.rs b/src/core/apply/diff.rs index 7d36dc6..2bc8c91 100644 --- a/src/core/apply/diff.rs +++ b/src/core/apply/diff.rs @@ -18,8 +18,10 @@ use crate::core::{JmoveResult, rel_str}; pub fn render_diff(root: &Path, plan: &MovePlan) -> JmoveResult { let mut out = render_edits_diff(root, &group_by_file(plan))?; if !out.is_empty() { - let (src, dst) = (rel_str(&plan.source), rel_str(&plan.target)); - out.push_str(&format!("move {src} -> {dst}\n")); + for m in &plan.moves { + let (src, dst) = (rel_str(&m.source), rel_str(&m.target)); + out.push_str(&format!("move {src} -> {dst}\n")); + } } Ok(out) } @@ -47,7 +49,7 @@ pub fn render_edits_diff( mod tests { use super::render_diff; use crate::core::JmoveResult; - use crate::core::plan::{MovePlan, Rewrite}; + use crate::core::plan::{FileMove, MovePlan, Rewrite}; use std::path::Path; const OLD: &str = "import {\n fmt,\n} from '../lib/fmt';\n"; @@ -56,12 +58,25 @@ mod tests { MovePlan { source: "lib/fmt.ts".into(), target: "deep/fmt.ts".into(), + moves: vec![ + FileMove { + source: "lib/fmt.ts".into(), + target: "deep/fmt.ts".into(), + }, + // A second move proves the dir flavour renders one line each. + FileMove { + source: "lib/gfx.ts".into(), + target: "deep/gfx.ts".into(), + }, + ], rewrites: vec![Rewrite { file: "src/app.ts".into(), span: 24..34, old_text: "../lib/fmt".into(), new_text: "../deep/fmt".into(), }], + left_behind: Vec::new(), + prune_dirs: Vec::new(), } } @@ -74,7 +89,10 @@ mod tests { assert!(diff.contains("--- src/app.ts") && diff.contains("+++ src/app.ts")); assert!(diff.contains("@@")); assert!(diff.contains("-} from '../lib/fmt';") && diff.contains("+} from '../deep/fmt';")); - assert!(diff.ends_with("move lib/fmt.ts -> deep/fmt.ts\n")); + assert!( + diff.ends_with("move lib/fmt.ts -> deep/fmt.ts\nmove lib/gfx.ts -> deep/gfx.ts\n"), + "{diff}" + ); let mut empty = plan(); empty.rewrites.clear(); assert_eq!(render_diff(dir.path(), &empty)?, ""); diff --git a/src/core/apply/mod.rs b/src/core/apply/mod.rs index 5e1f679..ad0c172 100644 --- a/src/core/apply/mod.rs +++ b/src/core/apply/mod.rs @@ -22,7 +22,7 @@ use crate::core::Edit; use crate::core::apply::fsops::{ create_missing_dirs, group_by_file, rewrite_bytes, sibling_temp, write_durable, }; -use crate::core::plan::MovePlan; +use crate::core::plan::{FileMove, MovePlan}; use crate::core::{JmoveError, JmoveResult, rel_str}; /// Summary of a successfully applied plan. @@ -30,22 +30,22 @@ use crate::core::{JmoveError, JmoveResult, rel_str}; pub struct Applied { /// Number of files whose imports were rewritten. pub files_rewritten: usize, - /// The moved file's new project-relative path. + /// The moved file's new project-relative path (directory moves: the + /// requested destination, mirroring [`crate::core::plan::MovePlan::target`]). pub new_path: PathBuf, - /// Whether the physical rename went through `git mv`. + /// Whether every physical rename went through `git mv`. pub via_git: bool, } // Rollback state for one run: originals of rewritten files (newest last), -// dirs created for the target, and the final rename once it happened. +// dirs created for the targets, and the renames once they happened. #[derive(Default)] struct Run { root: PathBuf, backups: Vec<(PathBuf, Vec)>, dirs: Vec, - // Root-relative (src, dst) of the executed rename. - moved: Option<(PathBuf, PathBuf)>, - via_git: bool, + // Executed root-relative renames (src, dst, went-through-git), oldest first. + moved: Vec<(PathBuf, PathBuf, bool)>, } /// Apply `plan` under `root` atomically (see module docs). Rollback is @@ -80,25 +80,45 @@ impl Run { fn try_apply(&mut self, plan: &MovePlan, mode: GitMode) -> JmoveResult { let by_file = group_by_file(plan); self.write_all(&by_file)?; - // The move comes last, after every importer was rewritten. - let (src, dst) = (self.root.join(&plan.source), self.root.join(&plan.target)); - self.dirs = create_missing_dirs(&dst)?; - // git mv needs the destination dir to exist; tracked sources are - // renamed through git so the change lands staged in the index. - self.via_git = would_use_git(&self.root, &plan.source, mode); - if self.via_git { - git::mv(&self.root, &plan.source, &plan.target)?; - } else { - fs::rename(&src, &dst)?; + // The moves come last, after every importer was rewritten. A + // directory plan is applied file by file in sorted order; any + // failure rolls the whole batch back. + for m in &plan.moves { + self.move_one(mode, m)?; + } + // Directory moves leave their emptied source dirs behind otherwise; + // remove_dir only succeeds when truly empty, so `left_behind` files + // keep their home. This is the last step: nothing can fail after it. + for d in &plan.prune_dirs { + let _ = fs::remove_dir(self.root.join(d)); } - self.moved = Some((plan.source.clone(), plan.target.clone())); Ok(Applied { files_rewritten: by_file.len(), new_path: plan.target.clone(), - via_git: self.via_git, + via_git: !self.moved.is_empty() && self.moved.iter().all(|(_, _, g)| *g), }) } + fn move_one(&mut self, mode: GitMode, m: &FileMove) -> JmoveResult<()> { + let (src, dst) = (self.root.join(&m.source), self.root.join(&m.target)); + for created in create_missing_dirs(&dst)? { + self.dirs.push(created); + } + // git mv needs the destination dir to exist; tracked sources are + // renamed through git so the change lands staged in the index. + let via_git = would_use_git(&self.root, &m.source, mode); + let done = if via_git { + git::mv(&self.root, &m.source, &m.target) + .map(|()| (m.source.clone(), m.target.clone(), true)) + } else { + fs::rename(&src, &dst) + .map(|()| (m.source.clone(), m.target.clone(), false)) + .map_err(Into::into) + }; + self.moved.push(done?); + Ok(()) + } + fn try_fix(&mut self, by_file: &BTreeMap>) -> JmoveResult { self.write_all(by_file)?; Ok(by_file.len()) @@ -131,8 +151,12 @@ impl Run { // problems to its message. fn undo(&mut self, err: JmoveError) -> JmoveError { let mut problems = Vec::new(); - if let Some((src, dst)) = self.moved.take() { - let back = if self.via_git { + for (src, dst, via_git) in self.moved.drain(..).rev() { + // The prune step never runs before a failure, but a later move + // can fail after git mv created target dirs that a rollback + // through git may expect; ensure the original parent exists. + let _ = fs::create_dir_all(self.root.join(&src).parent().unwrap()); + let back = if via_git { git::mv(&self.root, &dst, &src) } else { fs::rename(self.root.join(&dst), self.root.join(&src)).map_err(Into::into) @@ -156,94 +180,3 @@ impl Run { JmoveError::Io(std::io::Error::other(msg)) } } - -#[cfg(test)] -mod tests { - use super::{GitMode, apply}; - use crate::core::JmoveResult; - use crate::core::plan::{MovePlan, Rewrite}; - use std::fs; - use std::path::{Path, PathBuf}; - - const OLD: &str = "import {\n fmt,\n} from '../lib/fmt';\n"; - const NEW: &str = "import {\n fmt,\n} from '../deep/fmt';\n"; - // Byte span of `../lib/fmt` (between the quotes) inside OLD. - const SPAN: std::ops::Range = 24..34; - - // Plan moving lib/fmt.ts -> deep/fmt.ts, rewriting src/app.ts. - fn plan() -> MovePlan { - let rewrite = Rewrite { - file: "src/app.ts".into(), - span: SPAN, - old_text: "../lib/fmt".into(), - new_text: "../deep/fmt".into(), - }; - MovePlan { - source: "lib/fmt.ts".into(), - target: "deep/fmt.ts".into(), - rewrites: vec![rewrite], - } - } - - fn mk(dir: &Path, rel: &str, body: &str) -> JmoveResult<()> { - let path = dir.join(rel); - fs::create_dir_all(path.parent().unwrap())?; - fs::write(path, body)?; - Ok(()) - } - - #[test] - fn apply_rewrites_spans_creates_dirs_and_moves_last() -> JmoveResult<()> { - assert_eq!(&OLD[SPAN], "../lib/fmt"); // sanity: the span is real - let dir = tempfile::TempDir::new()?; - let root = dir.path(); - mk(root, "src/app.ts", OLD)?; - mk(root, "lib/fmt.ts", "export const fmt = 1;\n")?; - let applied = apply(root, &plan(), GitMode::Disabled)?; - assert_eq!( - (applied.files_rewritten, &applied.new_path), - (1, &PathBuf::from("deep/fmt.ts")) - ); - assert!(!root.join("lib/fmt.ts").exists()); - assert_eq!( - fs::read_to_string(root.join("deep/fmt.ts"))?, - "export const fmt = 1;\n" - ); - // Only the specifier bytes changed; the layout is kept byte-exact. - assert_eq!(fs::read_to_string(root.join("src/app.ts"))?, NEW); - assert!(!root.join("src/app.ts.jmove-tmp").exists()); - Ok(()) - } - - #[test] - fn apply_rolls_back_when_the_move_fails() -> JmoveResult<()> { - // Missing source: the last rename fails after the rewrite landed. - let dir = tempfile::TempDir::new()?; - mk(dir.path(), "src/app.ts", OLD)?; - let err = apply(dir.path(), &plan(), GitMode::Disabled).expect_err("missing source"); - assert!(matches!(err, crate::core::JmoveError::Io(_)), "{err}"); - // Importer restored to its exact original bytes; created dirs gone. - assert_eq!(fs::read_to_string(dir.path().join("src/app.ts"))?, OLD); - assert!(!dir.path().join("deep").exists()); - Ok(()) - } - - #[test] - fn apply_rejects_a_stale_plan_without_writing() -> JmoveResult<()> { - // SPAN was computed on OLD's layout; a single-line importer has - // different bytes there, so the run fails before any write. - let other = "import { fmt } from '../lib/fmt';\n"; - let dir = tempfile::TempDir::new()?; - let root = dir.path(); - mk(root, "src/app.ts", other)?; - mk(root, "lib/fmt.ts", "export const fmt = 1;\n")?; - let err = apply(root, &plan(), GitMode::Disabled).expect_err("span mismatch"); - assert!( - matches!(err, crate::core::JmoveError::StaleIndex(_)), - "{err}" - ); - assert_eq!(fs::read_to_string(root.join("src/app.ts"))?, other); - assert!(root.join("lib/fmt.ts").exists()); - Ok(()) - } -} diff --git a/src/core/index/mod.rs b/src/core/index/mod.rs index 83aa37e..01c77c2 100644 --- a/src/core/index/mod.rs +++ b/src/core/index/mod.rs @@ -9,6 +9,9 @@ mod files; #[cfg(test)] mod tests; +mod tsconfig; + +pub use tsconfig::PathAliases; use std::collections::{BTreeMap, HashMap}; use std::fs; @@ -16,7 +19,7 @@ use std::path::{Path, PathBuf}; use ignore::WalkBuilder; -use crate::core::{JmoveResult, normalize_rel_path}; +use crate::core::{JmoveError, JmoveResult, normalize_rel_path}; use crate::parser::java::JavaClassIndex; use crate::parser::resolve::resolve_module; use crate::parser::{ImportRecord, Language, PackageDecl, SourceLanguage, frontend_for}; @@ -48,6 +51,27 @@ 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 { @@ -64,12 +88,14 @@ impl Index { /// collision-free resolution (and therefore `mv`/`fix` rewrites) exact. pub fn build_scoped(root: &Path, source_root: Option<&Path>) -> JmoveResult { let root = root.canonicalize()?; + let aliases = PathAliases::load(&root); let mut index = Self { root, files: FileSet::default(), imports: HashMap::new(), packages: HashMap::new(), java_classes: JavaClassIndex::default(), + aliases, }; index.scan(source_root)?; // Resolution needs the complete file set (extension/index guessing) @@ -82,8 +108,14 @@ impl Index { java_classes .resolve(&resolved.record.specifier) .map(PathBuf::from) - } else { + } else if resolved.record.specifier.starts_with('.') { resolve_module(importer, &resolved.record.specifier, &index.files) + } else { + // Bare specifier: only tsconfig `paths` can map it into + // the project; anything else stays external. + index + .aliases + .resolve(&resolved.record.specifier, &index.files) }; } } diff --git a/src/core/index/tsconfig/mod.rs b/src/core/index/tsconfig/mod.rs new file mode 100644 index 0000000..a024931 --- /dev/null +++ b/src/core/index/tsconfig/mod.rs @@ -0,0 +1,212 @@ +//! tsconfig `compilerOptions.paths`: the alias table for bare specifiers. +//! +//! Most real TS projects import through aliases (`@/utils/x`, `@cfg`). +//! Without the mapping those imports are invisible to the graph, so `mv` +//! would leave them pointing at the old location. Loading is best-effort by +//! design: no tsconfig, invalid JSON or an unreadable file simply mean "no +//! aliases", never an error. `extends` chains are not followed (v1). +//! +//! tsconfig is JSONC: comments and trailing commas are legal, so the text +//! is sanitized before `serde_json` sees it. + +use std::fs; +use std::path::{Path, PathBuf}; + +use serde_json::Value; + +use crate::core::index::FileSet; +use crate::core::{normalize_rel_path, rel_str}; +use crate::parser::resolve::{MODULE_EXTS, resolve_base}; + +/// One `paths` entry. Star entries match by prefix, plain keys by equality. +#[derive(Debug, Clone, PartialEq, Eq)] +struct Entry { + /// Text the specifier must start with (star stripped), e.g. `"@utils/"`. + prefix: String, + /// `false` for an exact key (`"@cfg"`). + starred: bool, + /// Project-relative directory (star) or module base (exact) the alias maps into. + dir: PathBuf, +} + +/// Alias table; empty is a perfectly normal state. +#[derive(Debug, Default, Clone, PartialEq, Eq)] +pub struct PathAliases { + entries: Vec, +} + +impl PathAliases { + /// Load `root/tsconfig.json`; anything unreadable yields no aliases. + #[must_use] + pub fn load(root: &Path) -> Self { + let Ok(raw) = fs::read_to_string(root.join("tsconfig.json")) else { + return Self::default(); + }; + Self::parse(&sanitize_jsonc(&raw)) + } + + fn parse(json: &str) -> Self { + let Ok(value) = serde_json::from_str::(json) else { + return Self::default(); + }; + let options = value.get("compilerOptions"); + let base = options + .and_then(|o| o.get("baseUrl")) + .and_then(Value::as_str) + .and_then(|b| normalize_rel_path(Path::new(b))) + .unwrap_or_default(); + let Some(paths) = options + .and_then(|o| o.get("paths")) + .and_then(Value::as_object) + else { + return Self::default(); + }; + let mut entries = Vec::new(); + for (key, targets) in paths { + // Only the first candidate of a list is honoured (KISS). + let Some(first) = targets + .as_array() + .and_then(|a| a.first()) + .and_then(Value::as_str) + else { + continue; + }; + let starred = key.ends_with('*'); + let prefix = if starred { + key.trim_end_matches('*').to_owned() + } else { + key.clone() + }; + let stripped = first.trim_start_matches("./"); + let dir_src = if first.ends_with("/*") { + stripped.trim_end_matches("/*") + } else { + stripped + }; + let dir = normalize_rel_path(&base.join(dir_src)); + let Some(dir) = dir.filter(|d| !d.as_os_str().is_empty()) else { + continue; + }; + entries.push(Entry { + prefix, + starred, + dir, + }); + } + // Longest alias prefix wins when several match (`@a/b/*` over `@a/*`). + entries.sort_by(|a, b| b.prefix.cmp(&a.prefix)); + Self { entries } + } + + /// Resolve a bare (non-`.`-starting) specifier to an indexed file. + #[must_use] + pub fn resolve(&self, specifier: &str, files: &FileSet) -> Option { + let entry = self.find(specifier)?; + let base = if entry.starred { + entry.dir.join(&specifier[entry.prefix.len()..]) + } else { + entry.dir.clone() + }; + resolve_base(&normalize_rel_path(&base)?, files) + } + + /// Re-express `target` through the same alias `specifier` used before. + /// Star entries map morphologically (the plan runs before the file + /// exists at its destination, so no file-set check is possible): the + /// target must sit inside the alias directory and shed its module + /// extension the same way `resolve` would re-add it. `None` means + /// "fall back to a relative specifier". + #[must_use] + pub fn remap(&self, specifier: &str, target: &Path, files: &FileSet) -> Option { + let entry = self.find(specifier)?; + if !entry.starred { + // Exact key: it only keeps meaning while it maps to this file — + // after the move the old mapping points elsewhere, so this is + // normally `None` and the caller rewrites relatively. + return (self.resolve(specifier, files)? == target).then(|| specifier.to_owned()); + } + let rel = target.strip_prefix(&entry.dir).ok()?; + let text = rel_str(rel); + let stem = MODULE_EXTS + .iter() + .find(|ext| text.ends_with(**ext)) + .and_then(|ext| Some(text.strip_suffix(*ext)?.to_owned())) + .unwrap_or(text); + Some(format!("{}{}", entry.prefix, stem)) + } + + fn find(&self, specifier: &str) -> Option<&Entry> { + self.entries.iter().find(|e| { + if e.starred { + specifier.starts_with(&e.prefix) && specifier.len() > e.prefix.len() + } else { + specifier == e.prefix + } + }) + } +} + +/// Strip `//` + `/* */` comments and trailing commas from JSONC text. +/// String awareness keeps `"http://x"` and escaped quotes safe. +#[must_use] +pub(crate) fn sanitize_jsonc(src: &str) -> String { + let bytes = src.as_bytes(); + let mut out = String::with_capacity(src.len()); + let mut i = 0; + let mut in_string = false; + while i < bytes.len() { + let b = bytes[i]; + if in_string { + out.push(b as char); + if b == b'\\' && i + 1 < bytes.len() { + out.push(bytes[i + 1] as char); + i += 2; + continue; + } + if b == b'"' { + in_string = false; + } + i += 1; + continue; + } + match b { + b'"' => { + in_string = true; + out.push('"'); + i += 1; + } + b'/' if bytes.get(i + 1) == Some(&b'/') => { + i = bytes[i..] + .iter() + .position(|c| *c == b'\n') + .map_or(bytes.len(), |p| i + p); + } + b'/' if bytes.get(i + 1) == Some(&b'*') => { + let end = bytes[i + 2..] + .windows(2) + .position(|w| w == b"*/") + .map_or(bytes.len() - 2, |p| i + 2 + p + 2); + i = end; + } + b',' => { + // Drop only if the next non-space token closes a container. + let rest = &bytes[i + 1..]; + match rest.iter().find(|c| !c.is_ascii_whitespace()) { + Some(b'}') | Some(b']') => i += 1, + _ => { + out.push(','); + i += 1; + } + } + } + _ => { + out.push(b as char); + i += 1; + } + } + } + out +} + +#[cfg(test)] +mod tests; diff --git a/src/core/index/tsconfig/tests.rs b/src/core/index/tsconfig/tests.rs new file mode 100644 index 0000000..ef55f89 --- /dev/null +++ b/src/core/index/tsconfig/tests.rs @@ -0,0 +1,80 @@ +use super::{PathAliases, sanitize_jsonc}; +use crate::core::index::FileSet; +use std::path::{Path, PathBuf}; + +fn files(paths: &[&str]) -> FileSet { + let mut set = FileSet::default(); + for p in paths { + set.add(PathBuf::from(p)); + } + set +} + +fn aliases(json: &str) -> PathAliases { + PathAliases::parse(&sanitize_jsonc(json)) +} + +const TSCONFIG: &str = r#"{ + // comment with a brace } + "compilerOptions": { +"baseUrl": "./", +"paths": { + "@/*": ["src/*"], + "@utils/*": ["src/shared/utils/*"], + "@cfg": ["src/config.ts"], +}, + }, + "include": ["src/**/*"], +}"#; + +#[test] +fn jsonc_comments_and_trailing_commas_parse() { + let a = aliases(TSCONFIG); + assert_eq!(a.entries.len(), 3); +} + +#[test] +fn resolution_prefers_the_longest_matching_prefix() { + let set = files(&[ + "src/shared/utils/str.ts", + "src/utils/str.ts", + "src/config.ts", + ]); + let a = aliases(TSCONFIG); + assert_eq!( + a.resolve("@utils/str", &set).as_deref(), + Some(Path::new("src/shared/utils/str.ts")) + ); + assert_eq!( + a.resolve("@/utils/str", &set).as_deref(), + Some(Path::new("src/utils/str.ts")) + ); + assert_eq!( + a.resolve("@cfg", &set).as_deref(), + Some(Path::new("src/config.ts")) + ); + assert_eq!(a.resolve("react", &set), None); +} + +#[test] +fn remap_keeps_alias_shape_inside_and_falls_back_outside() { + let set = files(&["src/utils/str.ts", "src/deep/str.ts", "src/config.ts"]); + let a = aliases(r#"{"compilerOptions": {"paths": {"@u/*": ["src/utils/*"]}}}"#); + // Destination need not exist yet: the plan runs before the move. + assert_eq!( + a.remap("@u/str", Path::new("src/utils/other.ts"), &set) + .as_deref(), + Some("@u/other") + ); + // Target outside the alias tree: caller must fall back to relative. + assert!(a.remap("@u/str", Path::new("lib/other.ts"), &set).is_none()); +} + +#[test] +fn string_contents_survive_sanitizing() { + let json = r#"{"url": "http://x.com//y", "a": [1, 2,], /* c */ "b": "/*n*/"}"#; + let cleaned = sanitize_jsonc(json); + let value: serde_json::Value = serde_json::from_str(&cleaned).unwrap(); + assert_eq!(value["url"], "http://x.com//y"); + assert_eq!(value["b"], "/*n*/"); +} diff --git a/src/core/mod.rs b/src/core/mod.rs index 7d8279c..0c79130 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -10,6 +10,7 @@ pub mod apply; pub mod fix; pub mod index; pub mod plan; +pub mod refs; use std::ffi::OsStr; use std::io; @@ -123,6 +124,17 @@ pub fn rel_str(path: &Path) -> String { .join("/") } +/// 1-based line containing the byte offset `byte` in `source`. +#[must_use] +pub fn line_of(source: &str, byte: usize) -> usize { + let upto = source.len().min(byte); + source.as_bytes()[..upto] + .iter() + .filter(|b| **b == b'\n') + .count() + + 1 +} + /// Convert a user-supplied path to a normalized project-relative path. /// Relative paths are taken against `root`; absolute ones must live /// underneath it. Shared by CLI commands that accept paths. diff --git a/src/core/plan/dir.rs b/src/core/plan/dir.rs new file mode 100644 index 0000000..5118ecb --- /dev/null +++ b/src/core/plan/dir.rs @@ -0,0 +1,235 @@ +//! 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 4c0e580..8c03e79 100644 --- a/src/core/plan/mod.rs +++ b/src/core/plan/mod.rs @@ -2,12 +2,14 @@ //! //! A plan is pure data (no disk writes), so dry-run and `--json` can render //! it without touching the filesystem. Specifier arithmetic lives in -//! [`specifier`]; the Java package/directory flavour in [`java`]. +//! [`specifier`]; the Java package/directory flavour in [`java`], and the +//! mirrored batch move of a whole directory in [`dir`]. +mod dir; mod java; -mod specifier; #[cfg(test)] -pub(crate) mod tests_support; +pub(crate) use dir::tests_support; +mod specifier; pub use specifier::relative_specifier; @@ -42,24 +44,44 @@ impl From<&Rewrite> for Edit { } } -/// Complete plan for moving `source` to `target`. +/// One physical file relocation inside a plan. #[derive(Debug, Clone, PartialEq, Eq)] -pub struct MovePlan { - /// Project-relative path being moved. +pub struct FileMove { + /// Project-relative file being moved. pub source: PathBuf, /// Project-relative destination path. pub target: PathBuf, - /// Specifier rewrites, sorted by (file, span). - pub rewrites: Vec, } -/// Compute the rewrite plan for `source -> target`. +/// Complete plan for moving `source` to `target`: a file move produces one +/// [`FileMove`], a directory move one per indexed member (see [`dir`]). +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct MovePlan { + /// Requested source: the file, or the directory whose members move. + pub source: PathBuf, + /// Requested destination. + pub target: PathBuf, + /// Every physical relocation, in sorted source order. + pub moves: Vec, + /// Specifier/package rewrites, merged across moves, sorted by (file, span). + pub rewrites: Vec, + /// Directory moves only: real files under `source` that no plan step + /// moves (unindexable assets) — reported, never silently relocated. + pub left_behind: Vec, + /// Directory moves only: source directories to prune (deepest first) + /// once every move landed. Removal only succeeds when a directory is + /// empty, so `left_behind` files keep their home in place — exactly right. + pub prune_dirs: Vec, +} + +/// Compute the rewrite plan for `source -> target`; a `source` that is a +/// directory on disk becomes a mirrored move of all indexed members below it. /// -/// TS/JS: every indexed import whose resolved target equals `source` gets a -/// new relative specifier from the importer's directory to `target` (see -/// [`relative_specifier`]). Java moves additionally rewrite the moved file's -/// `package` declaration (see [`java`]). Rewrites whose result equals the -/// old specifier are dropped; the result is sorted by (file, span). +/// TS/JS: every indexed import whose resolved target equals a moved file gets +/// a new relative specifier from the importer's directory to its destination +/// (see [`relative_specifier`]). Java moves additionally rewrite the moved +/// file's `package` declaration (see [`java`]). Rewrites whose result equals +/// the old specifier are dropped; the result is sorted by (file, span). pub fn plan_move(index: &Index, source: &Path, target: &Path) -> JmoveResult { let rel = |label: &str, p: &Path| match normalize_rel_path(p) { Some(r) => Ok(r), @@ -69,36 +91,54 @@ pub fn plan_move(index: &Index, source: &Path, target: &Path) -> JmoveResult JmoveResult> { + Ok( + if SourceLanguage::for_path(source) == Some(SourceLanguage::Java) { + java::java_rewrites(index, source, target)? + } else { + ts_rewrites(index, source, target) + }, + ) +} + // Relative-specifier rewrites for the TS/JS flavour of the graph. fn ts_rewrites(index: &Index, source: &Path, target: &Path) -> Vec { let mut rewrites = Vec::new(); @@ -107,7 +147,13 @@ fn ts_rewrites(index: &Index, source: &Path, target: &Path) -> Vec { .iter() .filter(|e| e.target.as_deref() == Some(source)); for edge in edges { - let new_text = relative_specifier(&importer, target); + // Aliased imports keep their alias shape when the new location + // still round-trips through the same mapping; everything else + // gets the relative rewrite. + let new_text = index + .aliases + .remap(&edge.record.specifier, target, &index.files) + .unwrap_or_else(|| relative_specifier(&importer, target)); if new_text == edge.record.specifier { continue; // no-op rewrite, never reaches the plan } @@ -126,10 +172,10 @@ fn ts_rewrites(index: &Index, source: &Path, target: &Path) -> Vec { #[cfg(test)] mod tests { + use super::tests_support::edge; use super::{Rewrite, plan_move}; use crate::core::JmoveError; use crate::core::index::{Index, ResolvedImport}; - use crate::core::plan::tests_support::edge; use std::path::{Path, PathBuf}; fn index_with(files: &[&str], imports: &[(&str, Vec)]) -> Index { @@ -170,6 +216,7 @@ mod tests { new_text: "./sub/deep/s".into(), } ); + assert_eq!(plan.moves.len(), 1); } #[test] diff --git a/src/core/plan/tests_support.rs b/src/core/plan/tests_support.rs deleted file mode 100644 index ac157ef..0000000 --- a/src/core/plan/tests_support.rs +++ /dev/null @@ -1,20 +0,0 @@ -//! Shared `#[cfg(test)]` graph builders for the plan submodules. - -use std::ops::Range; -use std::path::PathBuf; - -use crate::core::index::ResolvedImport; -use crate::parser::ImportRecord; - -/// Hand-wired resolved edge: plan tests never touch the parser. -pub(crate) fn edge(spec: &str, span: Range, target: &str) -> ResolvedImport { - let record = ImportRecord { - specifier: spec.into(), - span, - is_dynamic: false, - }; - ResolvedImport { - record, - target: Some(PathBuf::from(target)), - } -} diff --git a/src/core/refs/mod.rs b/src/core/refs/mod.rs new file mode 100644 index 0000000..4daa320 --- /dev/null +++ b/src/core/refs/mod.rs @@ -0,0 +1,205 @@ +//! Non-import textual references to planned moves: report only. +//! +//! The import graph understands `import`/`export from`/`require` and +//! `import()` — but projects also reference files from markdown links, +//! `package.json` fields, `jest.mock("./x")` strings, tsconfig `files` +//! arrays and friends. Auto-rewriting those would require understanding +//! each format's semantics, so `jmove` never edits them; what it must not +//! do is move a file and stay silent while half a readme keeps pointing at +//! the old path. The scan runs before any write, on the same plan the +//! apply step consumes, and returns every suspicious occurrence with its +//! file and line. +//! +//! Matching is boundary-checked substring search over plain tokens (the +//! moved path, its extension-less module form, the exact specifier strings +//! importers used, and the directory prefix for batch moves). Tokens are +//! deliberately conservative: `lib/sum` matches `./lib/sum.ts` but not +//! `lib/summary`; occurrences inside the plan's own rewrite spans are the +//! import statements and are excluded. + +use std::path::{Path, PathBuf}; + +use serde::Serialize; + +use crate::core::index::Index; +use crate::core::plan::MovePlan; +use crate::core::{line_of, normalize_rel_path, rel_str}; + +/// One non-import occurrence of a moved path or specifier. +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] +pub struct NonImportRef { + /// Project-relative file containing the reference. + pub file: String, + /// 1-based line of the occurrence. + pub line: usize, + /// The matched token. + pub token: String, + /// Category of the token: `path`, `module`, `specifier` or `dir`. + /// Patterns are ranked in this order; one entry is kept per line. + pub kind: &'static str, + /// The matched line, trimmed and capped (context for humans/agents). + pub text: String, +} + +/// Text formats beyond source code that may name project files. +const TEXT_EXTS: &[&str] = &[ + "md", "mdx", "json", "json5", "yaml", "yml", "html", "htm", "xml", "txt", "pro", "gradle", +]; + +/// Lockfiles never reference project-local paths and are huge: skipped. +const LOCKFILES: &[&str] = &["package-lock.json", "yarn.lock", "pnpm-lock.yaml"]; + +const MAX_TEXT_BYTES: u64 = 512 * 1024; + +/// Scan the project for references to `plan`'s moves outside the import +/// statements the plan already rewrites. Infallible by design: unreadable +/// files are skipped (the scanner already ignored what it could not read). +pub fn scan(root: &Path, index: &Index, plan: &MovePlan) -> Vec { + let patterns = patterns(root, plan); + if patterns.is_empty() { + return Vec::new(); + } + let mut refs = Vec::new(); + for file in text_files(root, index) { + let Ok(source) = std::fs::read_to_string(root.join(&file)) else { + continue; // binary or vanished between walk and read + }; + let skip = skip_spans(plan, &file); + for (token, kind) in &patterns { + for at in token_occurrences(&source, token) { + if skip.iter().any(|s| s.contains(&at)) { + continue; // the import statements themselves + } + refs.push(NonImportRef { + file: rel_str(&file), + line: line_of(&source, at), + token: token.clone(), + kind, + text: excerpt(&source, at), + }); + } + } + } + // Stable sort keeps the first-inserted (highest-ranked) token per line: + // one warning per reference site, not one per matching pattern. + refs.sort_by(|a, b| (&a.file, a.line).cmp(&(&b.file, b.line))); + refs.dedup_by(|a, b| a.file == b.file && a.line == b.line); + refs +} + +// The search tokens of a plan. `source_root` scoping needs no special +// treatment: every token is already project-relative. +fn patterns(root: &Path, plan: &MovePlan) -> Vec<(String, &'static str)> { + let mut out: Vec<(String, &'static str)> = Vec::new(); + let mut push = |token: String, kind: &'static str| { + if token.len() > 1 && !out.iter().any(|(t, _)| t == &token) { + out.push((token, kind)); + } + }; + for m in &plan.moves { + let path = rel_str(&m.source); + push(path.clone(), "path"); + if let Some((stem, _)) = path.rsplit_once('.') { + push(stem.to_owned(), "module"); + } + } + for rewrite in &plan.rewrites { + push(rewrite.old_text.clone(), "specifier"); + } + if plan.moves.len() > 1 && root.join(&plan.source).is_dir() { + push(format!("{}/", rel_str(&plan.source)), "dir"); + } + out +} + +// Files whose rewrite the plan already performs legitimately: their spans. +fn skip_spans(plan: &MovePlan, file: &Path) -> Vec> { + plan.rewrites + .iter() + .filter(|r| r.file == file) + .map(|r| r.span.clone()) + .collect() +} + +/// Code files from the index plus non-hidden text files on disk. +fn text_files(root: &Path, index: &Index) -> Vec { + let mut out = index.files.sorted(); + let walker = ignore::WalkBuilder::new(root).require_git(false).build(); + for entry in walker.flatten() { + let path = entry.path(); + if entry.path_is_symlink() || !entry.file_type().is_some_and(|t| t.is_file()) { + continue; + } + let name = path + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or_default(); + if LOCKFILES.contains(&name) { + continue; + } + let ext = path + .extension() + .and_then(|e| e.to_str()) + .map(str::to_ascii_lowercase) + .unwrap_or_default(); + if !TEXT_EXTS.contains(&ext.as_str()) { + continue; + } + let Ok(stripped) = path.strip_prefix(root) else { + continue; + }; + let Some(rel) = normalize_rel_path(stripped) else { + continue; + }; + let hidden = rel + .components() + .any(|c| c.as_os_str().to_string_lossy().starts_with('.')); + let small = path + .metadata() + .map(|m| m.len() <= MAX_TEXT_BYTES) + .unwrap_or(false); + if !hidden && small && !out.contains(&rel) { + out.push(rel); + } + } + out.sort(); + out +} + +/// Every offset where `token` occurs with a word-free byte before it and +/// no word/joiner byte after: `lib/sum` matches `./lib/sum.ts` but never +/// `lib/summary` or `lib/sum-2`. +fn token_occurrences(source: &str, token: &str) -> Vec { + let bytes = source.as_bytes(); + let mut hits = Vec::new(); + let mut from = 0; + while let Some(found) = source[from..].find(token) { + let at = from + found; + let end = at + token.len(); + let before_ok = at == 0 || !is_word_byte(bytes[at - 1]); + let after_ok = !bytes + .get(end) + .is_some_and(|c| is_word_byte(*c) || matches!(c, b'-' | b'_')); + if before_ok && after_ok { + hits.push(at); + } + from = end; + } + hits +} + +fn is_word_byte(b: u8) -> bool { + b.is_ascii_alphanumeric() || b == b'_' || b == b'$' || b >= 0x80 +} + +/// The matched line, trimmed, with a hard cap. +fn excerpt(source: &str, at: usize) -> String { + let line_start = source[..at].rfind('\n').map_or(0, |i| i + 1); + let line_end = source[at..].find('\n').map_or(source.len(), |i| at + i); + let line = source[line_start..line_end].trim(); + let cut = line.char_indices().nth(100).map_or(line.len(), |(i, _)| i); + line[..cut].to_owned() +} + +#[cfg(test)] +mod tests; diff --git a/src/core/refs/tests.rs b/src/core/refs/tests.rs new file mode 100644 index 0000000..5dbe580 --- /dev/null +++ b/src/core/refs/tests.rs @@ -0,0 +1,123 @@ +//! Scanner tests: what may be referenced, what must never match. + +use std::path::Path; + +use crate::core::JmoveResult; +use crate::core::index::Index; +use crate::core::plan::plan_move; +use tempfile::TempDir; + +use super::{NonImportRef, scan, token_occurrences}; + +fn fixture() -> JmoveResult { + let dir = TempDir::new()?; + let root = dir.path(); + let write = |rel: &str, body: &str| -> JmoveResult<()> { + fs_create(rel, body, root)?; + Ok(()) + }; + write("lib/sum.ts", "export const sum = 3;\n")?; + write("lib/summary.ts", "export const summary = 's';\n")?; + write("app.ts", "import { sum } from './lib/sum';\n")?; + write( + "README.md", + "Use [sum](./lib/sum.ts) via `lib/sum`.\nSee also lib/summary for text.\nLayout note: lib/ holds helpers.\n", + )?; + write( + "package.json", + "{\"name\": \"refs\", \"main\": \"./lib/sum.ts\"}\n", + )?; + write("__tests__/sum.test.ts", "jest.mock('../lib/sum');\n")?; + write("package-lock.json", "{\"x\": \"./lib/sum\"}\n")?; + write(".notes/refs.md", "stale: ./lib/sum\n")?; + Ok(dir) +} + +fn fs_create(rel: &str, body: &str, root: &Path) -> std::io::Result<()> { + let p = root.join(rel); + std::fs::create_dir_all(p.parent().unwrap())?; + std::fs::write(p, body) +} + +fn files_of(refs: &[NonImportRef]) -> Vec { + refs.iter().map(|r| r.file.clone()).collect() +} + +fn scan_move(src: &str, dst: &str) -> JmoveResult<(TempDir, Vec)> { + let dir = fixture()?; + let index = Index::build(dir.path())?; + let plan = plan_move(&index, Path::new(src), Path::new(dst))?; + let refs = scan(dir.path(), &index, &plan); + Ok((dir, refs)) +} + +#[test] +fn doc_config_and_mock_references_are_reported() -> JmoveResult<()> { + let (_dir, refs) = scan_move("lib/sum.ts", "lib/total.ts")?; + let files = files_of(&refs); + for expected in ["README.md", "package.json", "__tests__/sum.test.ts"] { + assert!( + files.contains(&expected.to_string()), + "{expected} missing: {refs:?}" + ); + } + Ok(()) +} + +#[test] +fn the_import_statement_itself_is_never_reported() -> JmoveResult<()> { + let (_dir, refs) = scan_move("lib/sum.ts", "lib/total.ts")?; + assert!(!files_of(&refs).contains(&"app.ts".to_string()), "{refs:?}"); + assert!(!files_of(&refs).contains(&"lib/sum.ts".to_string())); + Ok(()) +} + +#[test] +fn decoys_lockfiles_and_hidden_dirs_are_not_reported() -> JmoveResult<()> { + let (_dir, refs) = scan_move("lib/sum.ts", "lib/total.ts")?; + let files = files_of(&refs); + for absent in ["lib/summary.ts", "package-lock.json", ".notes/refs.md"] { + assert!( + !files.contains(&absent.to_string()), + "{absent} leaked: {refs:?}" + ); + } + // README line 2 mentions only `lib/summary`: no ref may point there. + assert!( + refs.iter().all(|r| r.file != "README.md" || r.line != 2), + "{refs:?}" + ); + Ok(()) +} + +#[test] +fn one_entry_per_line_keeps_the_highest_ranked_token() -> JmoveResult<()> { + let (_dir, refs) = scan_move("lib/sum.ts", "lib/total.ts")?; + let readme = refs.iter().filter(|r| r.file == "README.md").count(); + // Line 1 (three matching tokens) and line 3 (`lib/` dir note is only a + // file-move token set away) collapse to one entry per line. + assert_eq!(readme, 1, "{refs:?}"); + assert_eq!(refs.iter().find(|r| r.file == "README.md").unwrap().line, 1); + Ok(()) +} + +#[test] +fn dir_move_reports_the_directory_prefix() -> JmoveResult<()> { + let (_dir, refs) = scan_move("lib", "pkg/lib")?; + assert!( + refs.iter() + .any(|r| r.file == "README.md" && r.line == 3 && r.kind == "dir"), + "{refs:?}" + ); + Ok(()) +} + +#[test] +fn boundary_rules_reject_longer_joined_and_prefixed_names() { + let none: Vec = Vec::new(); + assert_eq!(token_occurrences("lib/summary", "lib/sum"), none); + assert_eq!(token_occurrences("lib/sum-2", "lib/sum"), none); + assert_eq!(token_occurrences("mylib/sum", "lib/sum"), none); + assert_eq!(token_occurrences("./lib/sum.ts", "lib/sum"), vec![2]); + assert_eq!(token_occurrences("x/lib/sum", "lib/sum"), vec![2]); +} diff --git a/src/parser/java/class_name.rs b/src/parser/java/class_name.rs new file mode 100644 index 0000000..9ab1019 --- /dev/null +++ b/src/parser/java/class_name.rs @@ -0,0 +1,128 @@ +//! Java layout check: the single public top-level type must match the +//! file name (javac: "class Foo is public, should be declared in a file +//! named Foo.java"). +//! +//! This is a *finding*, not a fix rule: the repair is a file rename, and +//! the fix engine applies byte edits only. `jmove check` reports it with +//! the exact `jmove mv` command that repairs the layout — the moved class +//! keeps its FQN, so renaming the file rewrites no imports. + +use std::ops::Range; +use std::path::Path; + +use tree_sitter::Node; + +use super::{TreeSitterJava, text}; + +/// Kinds that introduce a top-level type in Java. +const TYPE_KINDS: &[&str] = &[ + "class_declaration", + "interface_declaration", + "enum_declaration", + "record_declaration", + "annotation_type_declaration", +]; + +/// The mismatch details: the public type's name, the span of its name +/// token (for line reporting) and the file stem it should live in. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Mismatch { + /// Name of the single public top-level type. + pub public_class: String, + /// Byte span of the class name token. + pub span: Range, +} + +/// `Some` when the file declares exactly one public top-level type whose +/// name differs from the file stem. Two public types (also illegal) or +/// zero are not this check's business. `package-info`/`module-info` files +/// never name a type and are skipped. +#[must_use] +pub fn mismatch(path: &Path, source: &str) -> Option { + let stem = path.file_stem()?.to_str()?; + if matches!(stem, "package-info" | "module-info") { + return None; + } + let tree = TreeSitterJava::parse(source)?; + let mut cursor = tree.root_node().walk(); + let publics: Vec = tree + .root_node() + .children(&mut cursor) + .filter(|n| TYPE_KINDS.contains(&n.kind())) + .filter(|n| has_public_modifier(*n)) + .collect(); + let [only] = publics.as_slice() else { + return None; // zero or many public types: not a naming mismatch + }; + let mut c = only.walk(); + let name = only.children(&mut c).find(|n| n.kind() == "identifier")?; + let name_text = text(name, source); + (name_text != stem).then(|| Mismatch { + public_class: name_text.to_owned(), + span: name.byte_range(), + }) +} + +// `public` is an anonymous token inside the `modifiers` child. +fn has_public_modifier(node: Node) -> bool { + let mut c = node.walk(); + node.children(&mut c) + .find(|n| n.kind() == "modifiers") + .is_some_and(|mods| { + let mut m = mods.walk(); + mods.children(&mut m).any(|t| t.kind() == "public") + }) +} + +#[cfg(test)] +mod tests { + use super::mismatch; + use std::path::Path; + + fn m(path: &str, src: &str) -> Option { + mismatch(Path::new(path), src).map(|f| f.public_class) + } + + #[test] + fn public_type_name_must_equal_file_stem() { + assert_eq!( + m("Foo.java", "package p;\npublic class Bar {}\n").as_deref(), + Some("Bar") + ); + assert_eq!(m("Foo.java", "package p;\npublic class Foo {}\n"), None); + } + + #[test] + fn every_public_top_level_kind_is_checked() { + assert_eq!( + m("A.java", "public interface B { int x(); }").as_deref(), + Some("B") + ); + assert_eq!(m("A.java", "public enum B { X }").as_deref(), Some("B")); + assert_eq!( + m("A.java", "public record B(int x) {}").as_deref(), + Some("B") + ); + assert_eq!(m("A.java", "public @interface B {}").as_deref(), Some("B")); + } + + #[test] + fn non_public_multiple_or_nested_types_are_not_reported() { + // Zero public types: legal package-private layout. + assert_eq!(m("Foo.java", "class Bar {}\nclass Baz {}"), None); + // Two public top-level types: a different (harder) error. + assert_eq!(m("Foo.java", "public class A {}\npublic class B {}"), None); + // Nested publics live inside a type: not top-level. + assert_eq!(m("A.java", "class A { public class B {} }"), None); + // Protected/private modifiers do not trigger the javac rule. + assert_eq!(m("A.java", "class B {}"), None); + } + + #[test] + fn info_files_are_skipped_and_span_names_the_class_token() { + assert_eq!(m("package-info.java", "package p;"), None); + let src = "package p;\n\npublic class Bar {}\n"; + let found = mismatch(Path::new("Foo.java"), src).unwrap(); + assert_eq!(&src[found.span.clone()], "Bar"); + } +} diff --git a/src/parser/java/mod.rs b/src/parser/java/mod.rs index 2fc62fc..2554092 100644 --- a/src/parser/java/mod.rs +++ b/src/parser/java/mod.rs @@ -12,6 +12,7 @@ use std::path::{Path, PathBuf}; use tree_sitter::{Node, Parser, Tree}; +pub mod class_name; pub mod rules; use super::{ImportRecord, Language, PackageDecl, SourceLanguage}; diff --git a/src/parser/resolve.rs b/src/parser/resolve.rs index b636895..3ac5986 100644 --- a/src/parser/resolve.rs +++ b/src/parser/resolve.rs @@ -14,6 +14,10 @@ use crate::core::normalize_rel_path; /// Supported extensions, in Node/TS resolution priority order. const EXTENSIONS: [&str; 6] = ["ts", "tsx", "js", "jsx", "mjs", "cjs"]; +/// Module extension *suffixes*, longest-first: `resolve_base` re-adds them +/// to a specifier, alias remapping sheds them again (the exact inverse). +pub const MODULE_EXTS: &[&str] = &[".d.ts", ".ts", ".tsx", ".js", ".jsx", ".mjs", ".cjs"]; + /// Ambient declaration files: only consulted after every real module /// candidate missed (last resort). const DECLARATION_EXT: &str = "d.ts"; @@ -45,9 +49,18 @@ pub fn resolve_module(importer: &Path, specifier: &str, files: &FileSet) -> Opti // `importer` is project-relative, so `..` segments that walk past the // root collapse to `None` here instead of escaping the index. let base = normalize_rel_path(&importer.parent()?.join(specifier))?; - if files.contains(&base) { - return Some(base); + resolve_base(&base, files) +} + +/// Resolve a project-relative module base path against the file set: +/// exact file, extension guessing, then `index.*` in the directory. +/// Shared by relative specifiers and tsconfig alias mapping. +#[must_use] +pub fn resolve_base(base: &Path, files: &FileSet) -> Option { + if files.contains(base) { + return Some(base.to_path_buf()); } + let base = base.to_path_buf(); for ext in EXTENSIONS { let candidate = with_ext(&base, ext); if files.contains(&candidate) { diff --git a/tests/apply.rs b/tests/apply.rs new file mode 100644 index 0000000..f6863ff --- /dev/null +++ b/tests/apply.rs @@ -0,0 +1,170 @@ +//! Direct engine tests for `apply` and `apply_edits` via the public lib +//! API: multi-move rollback and dir pruning are impossible to force +//! through the CLI (pre-flight validation catches them first). + +use jmove::core::JmoveResult; +use jmove::core::apply::{GitMode, apply}; +use jmove::core::plan::{FileMove, MovePlan, Rewrite}; +use std::fs; +use std::path::{Path, PathBuf}; + +const OLD: &str = "import {\n fmt,\n} from '../lib/fmt';\n"; +const NEW: &str = "import {\n fmt,\n} from '../deep/fmt';\n"; +// Byte span of `../lib/fmt` (between the quotes) inside OLD. +const SPAN: std::ops::Range = 24..34; + +// Plan moving lib/fmt.ts -> deep/fmt.ts, rewriting src/app.ts. +fn plan() -> MovePlan { + let rewrite = Rewrite { + file: "src/app.ts".into(), + span: SPAN, + old_text: "../lib/fmt".into(), + new_text: "../deep/fmt".into(), + }; + MovePlan { + source: "lib/fmt.ts".into(), + target: "deep/fmt.ts".into(), + moves: vec![FileMove { + source: "lib/fmt.ts".into(), + target: "deep/fmt.ts".into(), + }], + rewrites: vec![rewrite], + left_behind: Vec::new(), + prune_dirs: Vec::new(), + } +} + +fn mk(dir: &Path, rel: &str, body: &str) -> JmoveResult<()> { + let path = dir.join(rel); + fs::create_dir_all(path.parent().unwrap())?; + fs::write(path, body)?; + Ok(()) +} + +#[test] +fn apply_rewrites_spans_creates_dirs_and_moves_last() -> JmoveResult<()> { + assert_eq!(&OLD[SPAN], "../lib/fmt"); // sanity: the span is real + let dir = tempfile::TempDir::new()?; + let root = dir.path(); + mk(root, "src/app.ts", OLD)?; + mk(root, "lib/fmt.ts", "export const fmt = 1;\n")?; + let applied = apply(root, &plan(), GitMode::Disabled)?; + assert_eq!( + (applied.files_rewritten, &applied.new_path), + (1, &PathBuf::from("deep/fmt.ts")) + ); + assert!(!root.join("lib/fmt.ts").exists()); + assert_eq!( + fs::read_to_string(root.join("deep/fmt.ts"))?, + "export const fmt = 1;\n" + ); + // Only the specifier bytes changed; the layout is kept byte-exact. + assert_eq!(fs::read_to_string(root.join("src/app.ts"))?, NEW); + assert!(!root.join("src/app.ts.jmove-tmp").exists()); + Ok(()) +} + +#[test] +fn apply_rolls_back_when_the_move_fails() -> JmoveResult<()> { + // Missing source: the last rename fails after the rewrite landed. + let dir = tempfile::TempDir::new()?; + mk(dir.path(), "src/app.ts", OLD)?; + let err = apply(dir.path(), &plan(), GitMode::Disabled).expect_err("missing source"); + assert!(matches!(err, jmove::core::JmoveError::Io(_)), "{err}"); + // Importer restored to its exact original bytes; created dirs gone. + assert_eq!(fs::read_to_string(dir.path().join("src/app.ts"))?, OLD); + assert!(!dir.path().join("deep").exists()); + Ok(()) +} + +#[test] +fn apply_rejects_a_stale_plan_without_writing() -> JmoveResult<()> { + // SPAN was computed on OLD's layout; a single-line importer has + // different bytes there, so the run fails before any write. + let other = "import { fmt } from '../lib/fmt';\n"; + let dir = tempfile::TempDir::new()?; + let root = dir.path(); + mk(root, "src/app.ts", other)?; + mk(root, "lib/fmt.ts", "export const fmt = 1;\n")?; + let err = apply(root, &plan(), GitMode::Disabled).expect_err("span mismatch"); + assert!( + matches!(err, jmove::core::JmoveError::StaleIndex(_)), + "{err}" + ); + assert_eq!(fs::read_to_string(root.join("src/app.ts"))?, other); + assert!(root.join("lib/fmt.ts").exists()); + Ok(()) +} + +#[test] +fn dir_apply_prunes_emptied_source_dirs_last() -> JmoveResult<()> { + let dir = tempfile::TempDir::new()?; + let root = dir.path(); + mk(root, "src/a.ts", "x\n")?; + mk(root, "src/nested/b.ts", "y\n")?; + let plan = MovePlan { + source: "src".into(), + target: "lib".into(), + moves: vec![ + FileMove { + source: "src/a.ts".into(), + target: "lib/a.ts".into(), + }, + FileMove { + source: "src/nested/b.ts".into(), + target: "lib/nested/b.ts".into(), + }, + ], + rewrites: Vec::new(), + left_behind: Vec::new(), + // shallowest last on purpose: apply must prune deepest first. + prune_dirs: vec!["src/nested".into(), "src".into()], + }; + apply(root, &plan, GitMode::Disabled)?; + assert!(root.join("lib/nested/b.ts").is_file()); + assert!(!root.join("src").exists(), "emptied source tree must go"); + Ok(()) +} + +#[test] +fn dir_apply_rolls_back_every_move_when_a_later_one_fails() -> JmoveResult<()> { + // "trap" is a file, so the second move's parent can never exist: + // the first move must be undone and rewritten importers restored. + let dir = tempfile::TempDir::new()?; + let root = dir.path(); + mk(root, "src/a.ts", "a\n")?; + mk(root, "src/b.ts", "b\n")?; + mk(root, "trap", "I am a file\n")?; + mk(root, "app.ts", "import './src/a';\n")?; + let plan = MovePlan { + source: "src".into(), + target: "lib".into(), + moves: vec![ + FileMove { + source: "src/a.ts".into(), + target: "lib/a.ts".into(), + }, + FileMove { + source: "src/b.ts".into(), + target: "trap/b.ts".into(), + }, + ], + rewrites: vec![Rewrite { + file: "app.ts".into(), + span: 8..15, + old_text: "./src/a".into(), + new_text: "./lib/a".into(), + }], + left_behind: Vec::new(), + prune_dirs: vec!["src".into()], + }; + let err = apply(root, &plan, GitMode::Disabled).expect_err("ENOTDIR"); + assert!(matches!(err, jmove::core::JmoveError::Io(_)), "{err}"); + assert!(root.join("src/a.ts").is_file(), "first move undone"); + assert!(!root.join("lib").exists(), "created dirs removed"); + assert_eq!( + fs::read_to_string(root.join("app.ts"))?, + "import './src/a';\n" + ); + Ok(()) +} diff --git a/tests/cli.rs b/tests/cli.rs index 188f208..e1f04d8 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -105,7 +105,7 @@ fn check_passes_on_clean_and_fails_on_broken_fixture() { let clean = fixture("basic"); jmove(&clean, &["check"]) .success() - .stdout(predicate::str::contains("no broken imports")); + .stdout(predicate::str::contains("check: no findings")); let messy = fixture("complex"); jmove(&messy, &["check"]) diff --git a/tests/cli_alias.rs b/tests/cli_alias.rs new file mode 100644 index 0000000..f5f0948 --- /dev/null +++ b/tests/cli_alias.rs @@ -0,0 +1,37 @@ +//! End-to-end tests for tsconfig `paths` aliases: aliased imports take +//! part in `mv` and keep their alias shape inside the mapped tree. + +mod common; + +use common::{copy_fixture, in_root, jmove, read}; +use predicates::prelude::*; + +const APP: &str = "src/app.ts"; + +#[test] +fn aliased_imports_follow_the_move_and_keep_the_alias_shape() { + let tmp = copy_fixture("typescript", "aliased"); + jmove(&tmp, &["check"]).success(); // "@utils/str" resolves: not "broken" + jmove(&tmp, &["mv", "src/utils/str.ts", "src/utils/text.ts"]) + .success() + .stdout(predicate::str::contains("updated 1 import")); + let app = read(&in_root(tmp.path(), APP)); + assert!(app.contains("from \"@utils/text\""), "{app}"); + assert!(!app.contains("./utils/text"), "{app}"); + // Moving out of the alias tree falls back to a relative specifier. + jmove(&tmp, &["mv", "src/utils/text.ts", "src/core/text.ts"]).success(); + let app = read(&in_root(tmp.path(), APP)); + assert!(app.contains("from \"./core/text\""), "{app}"); + jmove(&tmp, &["check"]).success(); +} + +#[test] +fn exact_alias_key_falls_back_when_it_no_longer_matches() { + let tmp = copy_fixture("typescript", "aliased"); + jmove(&tmp, &["mv", "src/config.ts", "src/settings.ts"]).success(); + let app = read(&in_root(tmp.path(), APP)); + // "@cfg" would now resolve to nothing: the rewrite must be relative. + assert!(app.contains("from \"./settings\""), "{app}"); + assert!(!app.contains("@cfg"), "{app}"); + jmove(&tmp, &["check"]).success(); +} diff --git a/tests/cli_dir.rs b/tests/cli_dir.rs new file mode 100644 index 0000000..1da2797 --- /dev/null +++ b/tests/cli_dir.rs @@ -0,0 +1,98 @@ +//! End-to-end tests for moving whole directories: every indexed member +//! relocates in one atomic batch and all importers follow. + +mod common; + +use common::{copy_fixture, in_root, jmove, read}; +use predicates::prelude::*; +use tempfile::TempDir; + +fn fixture(name: &str) -> TempDir { + copy_fixture("typescript", name) +} + +#[test] +fn ts_dir_move_relocates_members_and_rewrites_importers() { + let tmp = fixture("normal"); + jmove(&tmp, &["mv", "src/impl", "src/impl2"]) + .success() + .stdout(predicate::str::contains("(2 files)")); + assert!(!in_root(tmp.path(), "src/impl").exists()); + assert!(in_root(tmp.path(), "src/impl2/core.ts").is_file()); + assert!(in_root(tmp.path(), "src/impl2/index.ts").is_file()); + + let app = read(&in_root(tmp.path(), "src/app.ts")); + assert!(app.contains("./impl2/core"), "{app}"); + // A barrel directory import keeps its conservative `.../index` form. + let root = read(&in_root(tmp.path(), "src/index.ts")); + assert!(root.contains("export * from \"./impl2/index\""), "{root}"); + jmove(&tmp, &["check"]).success(); +} + +#[test] +fn java_package_dir_move_rewrites_packages_and_imports() { + let tmp = copy_fixture("java", "basic"); + jmove( + &tmp, + &[ + "mv", + "src/main/java/com/example/util", + "src/main/java/com/example/core", + ], + ) + .success() + .stdout(predicate::str::contains("updated 4 imports in 3 files")); + + let text = read(&in_root( + tmp.path(), + "src/main/java/com/example/core/Text.java", + )); + assert!(text.contains("package com.example.core;"), "{text}"); + let app = read(&in_root( + tmp.path(), + "src/main/java/com/example/app/App.java", + )); + assert!(app.contains("import com.example.core.Text;"), "{app}"); + assert!( + app.contains("import static com.example.core.Text.shout;"), + "{app}" + ); + jmove(&tmp, &["check"]).success(); +} + +#[test] +fn dir_move_json_lists_every_file_and_unsupported_left_behind() { + let tmp = fixture("normal"); + let note = in_root(tmp.path(), "src/impl/notes.txt"); + std::fs::write(¬e, "asset, not source\n").expect("write note"); + jmove(&tmp, &["mv", "src/impl", "src/impl2", "--json"]) + .success() + .stdout( + predicate::str::contains("\"moved\": 2") + .and(predicate::str::contains("\"moved_files\"")) + .and(predicate::str::contains("\"to\": \"src/impl2/core.ts\"")) + .and(predicate::str::contains("left_behind")) + .and(predicate::str::contains("src/impl/notes.txt")), + ); + // The unindexable asset is reported, not silently dragged along. + assert!(note.is_file()); + assert!(!in_root(tmp.path(), "src/impl2/notes.txt").exists()); +} + +#[test] +fn dry_run_dir_move_previews_every_relocation() { + let tmp = fixture("normal"); + jmove( + &tmp, + &["mv", "src/impl", "src/impl2", "--dry-run", "--json"], + ) + .success() + .stdout( + predicate::str::contains("\"would_move_files\"") + .and(predicate::str::contains("\"from\": \"src/impl/core.ts\"")), + ); + assert!( + in_root(tmp.path(), "src/impl/core.ts").is_file(), + "dry-run writes nothing" + ); +} diff --git a/tests/cli_fix.rs b/tests/cli_fix.rs index 1ecb885..0a7d1b8 100644 --- a/tests/cli_fix.rs +++ b/tests/cli_fix.rs @@ -141,49 +141,6 @@ fn missing_project() -> tempfile::TempDir { common::copy_fixture("java", "fix_missing") } -#[test] -fn missing_import_adds_unique_and_reports_ambiguous() { - let tmp = missing_project(); - jmove(&tmp, &["fix", "--rule", "java/missing-import", "--json"]) - .success() - .stdout( - predicate::str::contains("\"rule\": \"java/missing-import\"") - .and(predicate::str::contains("\"applied\": false")) - .and(predicate::str::contains("\"com.example.a.Config\"")) - .and(predicate::str::contains("\"com.example.b.Config\"")), - ); - let calc = read(&in_root( - tmp.path(), - "src/main/java/com/example/app/Calc.java", - )); - assert!( - calc.contains("package com.example.app;\nimport com.example.util.Maths;\n"), - "{calc}" - ); - // The ambiguous `Config` is never guessed at. - let refer = read(&in_root( - tmp.path(), - "src/main/java/com/example/c/Refer.java", - )); - assert!(!refer.contains("import com.example."), "{refer}"); - jmove(&tmp, &["check"]).success(); -} - -#[test] -fn missing_import_dry_run_then_stable_reapply() { - let tmp = missing_project(); - jmove(&tmp, &["fix", "--dry-run"]) - .success() - .stdout(predicate::str::contains("+import com.example.util.Maths;")); - jmove(&tmp, &["fix"]) - .success() - .stdout(predicate::str::contains("fixed 3 issues in 2 files")); - // Only manual (ambiguous) findings remain: nothing further applies. - jmove(&tmp, &["fix"]) - .success() - .stdout(predicate::str::contains("nothing to change")); -} - #[test] fn unused_delete_and_missing_insert_coexist_in_one_file() { // Dual.java: the unused import is deleted while the missing `Maths` @@ -198,56 +155,6 @@ fn unused_delete_and_missing_insert_coexist_in_one_file() { jmove(&tmp, &["check"]).success(); } -#[test] -fn three_rules_converge_on_one_project() { - // App.java: unused import inside an out-of-order block (the order - // rewrite overlaps the deletion, so it defers to run 2). - // C.java: out-of-order block + a bare `Maths` reference (insertion at - // the block boundary coexists with the rewrite in run 1). - let tmp = common::copy_fixture("java", "order"); - jmove(&tmp, &["fix", "--dry-run", "--json"]) - .success() - .stdout( - predicate::str::contains("\"rule\": \"java/import-order\"") - .and(predicate::str::contains("\"applied\": false")) - .and(predicate::str::contains("skipped")), - ); - let app = "src/main/java/com/example/app/App.java"; - let c = "src/main/java/com/example/app/C.java"; - jmove(&tmp, &["fix"]) - .success() - .stdout(predicate::str::contains("fixed 3 issues in 2 files")); - let app_text = read(&in_root(tmp.path(), app)); - // Unused is gone; the deferred order fix has not touched the block. - assert!(!app_text.contains("Unneeded"), "{app_text}"); - assert!( - app_text.contains("import java.util.List;\nimport com.example.util.Maths;"), - "{app_text}" - ); - jmove(&tmp, &["fix"]) - .success() - .stdout(predicate::str::contains("fixed 2 issues in 2 files")); - let c_text = read(&in_root(tmp.path(), c)); - assert!( - c_text.contains( - "import com.example.util.Maths;\nimport java.util.List;\nimport java.util.Map;\n" - ), - "{c_text}" - ); - let app_text = read(&in_root(tmp.path(), app)); - assert!( - app_text.contains( - "import com.example.util.Maths;\nimport java.util.List;\nimport java.util.Map;\n" - ), - "{app_text}" - ); - // Fixed point. - jmove(&tmp, &["fix"]) - .success() - .stdout(predicate::str::contains("nothing to change")); - jmove(&tmp, &["check"]).success(); -} - fn ts_unused() -> tempfile::TempDir { common::copy_fixture("typescript", "unused") } diff --git a/tests/cli_git.rs b/tests/cli_git.rs index 8dc1566..2c38b17 100644 --- a/tests/cli_git.rs +++ b/tests/cli_git.rs @@ -113,3 +113,21 @@ fn mv_no_git_keeps_the_plain_rename_unstaged() { let target = git_output(tmp.path(), &["status", "--porcelain", "--", "utils/sum.ts"]); assert!(target.starts_with("??"), "{target}"); } + +#[test] +fn dir_move_stages_every_rename_and_prunes_the_source_dir() { + let tmp = git_fixture("basic"); + jmove(&tmp, &["mv", "lib", "vendor/lib"]) + .success() + .stdout(predicate::str::contains("(via git mv)")); + let staged = git_output(tmp.path(), &["diff", "--cached", "-M", "--name-status"]); + assert!( + staged.contains("vendor/lib/sum.ts") && staged.contains("lib/sum.ts"), + "{staged}" + ); + assert!(tmp.path().join("vendor/lib/sum.ts").is_file()); + assert!( + !tmp.path().join("lib").exists(), + "emptied source dir pruned" + ); +} diff --git a/tests/cli_missing_import.rs b/tests/cli_missing_import.rs new file mode 100644 index 0000000..4b7f017 --- /dev/null +++ b/tests/cli_missing_import.rs @@ -0,0 +1,96 @@ +//! End-to-end tests for the `java/missing-import` rule against fixtures. +//! Self-contained copy of the tiny helpers it needs (each test file is its +//! own crate; importing all of `common` would trip dead-code warnings). + +use assert_cmd::Command; +use predicates::prelude::*; +use std::fs; +use std::path::{Path, PathBuf}; +use tempfile::{TempDir, tempdir}; + +fn fixture(name: &str) -> TempDir { + let tmp = tempdir().expect("tempdir"); + copy_dir( + &Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests") + .join("java") + .join(name), + tmp.path(), + ); + fs::create_dir(tmp.path().join(".git")).expect("git marker"); + tmp +} + +fn copy_dir(from: &Path, to: &Path) { + fs::create_dir_all(to).expect("mkdir"); + for entry in fs::read_dir(from).expect("readdir") { + let entry = entry.expect("entry"); + let target = to.join(entry.file_name()); + if entry.file_type().expect("filetype").is_dir() { + copy_dir(&entry.path(), &target); + } else { + fs::copy(entry.path(), target).expect("copy"); + } + } +} + +fn jmove(root: &TempDir, args: &[&str]) -> assert_cmd::assert::Assert { + let mut cmd = Command::cargo_bin("jmove").expect("jmove binary"); + cmd.arg("--root").arg(root.path()).args(args); + cmd.assert() +} + +fn in_root(root: &Path, rel: &str) -> PathBuf { + root.join(rel) +} + +fn read(path: &PathBuf) -> String { + fs::read_to_string(path).unwrap_or_else(|err| panic!("read {path:?}: {err}")) +} + +fn missing_project() -> tempfile::TempDir { + fixture("fix_missing") +} + +#[test] +fn missing_import_adds_unique_and_reports_ambiguous() { + let tmp = missing_project(); + jmove(&tmp, &["fix", "--rule", "java/missing-import", "--json"]) + .success() + .stdout( + predicate::str::contains("\"rule\": \"java/missing-import\"") + .and(predicate::str::contains("\"applied\": false")) + .and(predicate::str::contains("\"com.example.a.Config\"")) + .and(predicate::str::contains("\"com.example.b.Config\"")), + ); + let calc = read(&in_root( + tmp.path(), + "src/main/java/com/example/app/Calc.java", + )); + assert!( + calc.contains("package com.example.app;\nimport com.example.util.Maths;\n"), + "{calc}" + ); + // The ambiguous `Config` is never guessed at. + let refer = read(&in_root( + tmp.path(), + "src/main/java/com/example/c/Refer.java", + )); + assert!(!refer.contains("import com.example."), "{refer}"); + jmove(&tmp, &["check"]).success(); +} + +#[test] +fn missing_import_dry_run_then_stable_reapply() { + let tmp = missing_project(); + jmove(&tmp, &["fix", "--dry-run"]) + .success() + .stdout(predicate::str::contains("+import com.example.util.Maths;")); + jmove(&tmp, &["fix"]) + .success() + .stdout(predicate::str::contains("fixed 3 issues in 2 files")); + // Only manual (ambiguous) findings remain: nothing further applies. + jmove(&tmp, &["fix"]) + .success() + .stdout(predicate::str::contains("nothing to change")); +} diff --git a/tests/cli_name_check.rs b/tests/cli_name_check.rs new file mode 100644 index 0000000..615d77d --- /dev/null +++ b/tests/cli_name_check.rs @@ -0,0 +1,43 @@ +//! `jmove check` finds Java files whose public type is misnamed, and the +//! suggested `jmove mv` repairs the layout. + +mod common; + +use common::{copy_fixture, in_root, jmove, read}; +use predicates::prelude::*; + +const BAD: &str = "src/main/java/com/example/Bad.java"; + +#[test] +fn check_reports_misnamed_public_class_and_clean_files_stay_quiet() { + let tmp = copy_fixture("java", "mismatch"); + jmove(&tmp, &["check"]) + .code(2) + .stdout( + predicate::str::contains( + "src/main/java/com/example/Bad.java:3: public class 'Wrong' must live in 'src/main/java/com/example/Wrong.java'", + ) + .and(predicate::str::contains("Good.java").not()), + ); + jmove(&tmp, &["check", "--json"]) + .code(2) + .stdout( + predicate::str::contains("\"name_mismatches\"") + .and(predicate::str::contains("\"public_class\": \"Wrong\"")) + .and(predicate::str::contains( + "\"rename\": \"jmove mv 'src/main/java/com/example/Bad.java' 'src/main/java/com/example/Wrong.java'\"", + )), + ); +} + +#[test] +fn running_the_suggested_rename_makes_check_pass() { + let tmp = copy_fixture("java", "mismatch"); + jmove(&tmp, &["mv", BAD, "src/main/java/com/example/Wrong.java"]).success(); + let moved = read(&in_root(tmp.path(), "src/main/java/com/example/Wrong.java")); + assert!(moved.contains("public class Wrong {}"), "{moved}"); + assert!(moved.contains("package com.example;"), "{moved}"); + jmove(&tmp, &["check"]) + .success() + .stdout(predicate::str::contains("no findings")); +} diff --git a/tests/cli_refs.rs b/tests/cli_refs.rs new file mode 100644 index 0000000..c679dba --- /dev/null +++ b/tests/cli_refs.rs @@ -0,0 +1,68 @@ +//! `mv` warns (never edits) about non-import references: markdown links, +//! `package.json` fields and `jest.mock` strings survive silent unless the +//! user sees them. JSON payload carries the same list for agents. + +mod common; + +use common::{copy_fixture, in_root, jmove, read}; +use predicates::prelude::*; + +fn dry_run_refs() -> String { + let tmp = copy_fixture("typescript", "refs"); + let out = jmove( + &tmp, + &["mv", "lib/sum.ts", "lib/total.ts", "--dry-run", "--json"], + ) + .success() + .stdout(predicate::str::contains("\"non_import_refs\"")) + .get_output() + .stdout + .clone(); + String::from_utf8(out).expect("json is utf-8") +} + +#[test] +fn json_dry_run_lists_hidden_references_and_skips_the_noise() { + let out = dry_run_refs(); + for expected in [ + "\"file\": \"README.md\"", + "\"file\": \"package.json\"", + "\"file\": \"__tests__/sum.test.ts\"", + ] { + assert!(out.contains(expected), "missing {expected} in {out}"); + } + for absent in [ + "\"file\": \"app.ts\"", + "\"file\": \"package-lock.json\"", + "\"file\": \".notes/refs.md\"", + "\"file\": \"lib/summary.ts\"", + ] { + assert!(!out.contains(absent), "leaked {absent} in {out}"); + } +} + +#[test] +fn human_dry_run_warns_on_stderr_and_exits_zero() { + let tmp = copy_fixture("typescript", "refs"); + jmove(&tmp, &["mv", "lib/sum.ts", "lib/total.ts", "--dry-run"]) + .success() + .stderr( + predicate::str::contains("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 new file mode 100644 index 0000000..a710859 --- /dev/null +++ b/tests/cli_report.rs @@ -0,0 +1,122 @@ +//! `--report ` interop: SARIF 2.1.0 and Checkstyle XML for CI/IDE +//! consumers. The reports mirror the findings of `check`/`fix` without +//! changing stdout or exit codes. + +mod common; + +use common::{copy_fixture, in_root, jmove, read}; +use predicates::prelude::*; + +fn report_path(tmp: &tempfile::TempDir, name: &str) -> String { + tmp.path() + .join(name) + .to_str() + .expect("utf-8 path") + .to_owned() +} + +fn sarif(tmp: &tempfile::TempDir, name: &str) -> serde_json::Value { + serde_json::from_str(&read(&tmp.path().join(name))).expect("valid sarif json") +} + +#[test] +fn check_writes_sarif_and_keeps_the_findings_exit_code() { + let tmp = copy_fixture("java", "mismatch"); + let report = report_path(&tmp, "out.sarif"); + jmove(&tmp, &["check", "--report", &report]).code(2); + let doc = sarif(&tmp, "out.sarif"); + let result = &doc["runs"][0]["results"][0]; + assert_eq!(result["ruleId"], "java/class-name-mismatch"); + assert_eq!(result["level"], "error"); + assert!( + result["message"]["text"] + .as_str() + .unwrap() + .contains("jmove mv") + ); + let location = &result["locations"][0]["physicalLocation"]; + assert_eq!( + location["artifactLocation"]["uri"], + "src/main/java/com/example/Bad.java" + ); + assert_eq!(location["region"]["startLine"], 3); + assert_eq!(result["properties"]["autoFixable"], true); +} + +#[test] +fn check_writes_checkstyle_for_broken_imports() { + let tmp = copy_fixture("typescript", "complex"); + let report = report_path(&tmp, "checkstyle.xml"); + jmove(&tmp, &["check", "--report", &report]).code(2); + let xml = read(&in_root(tmp.path(), "checkstyle.xml")); + assert!(xml.starts_with(""), "{xml}"); + assert!( + xml.contains( + "\n"), "{xml}"); +} + +#[test] +fn unknown_report_extension_fails_fast() { + let tmp = copy_fixture("typescript", "basic"); + let report = report_path(&tmp, "out.txt"); + jmove(&tmp, &["check", "--report", &report]) + .code(1) + .stderr(predicate::str::contains("unsupported file name")); + assert!(!tmp.path().join("out.txt").exists()); +} + +#[test] +fn fix_reports_candidates_in_both_formats_without_touching_stdout() { + let tmp = copy_fixture("typescript", "unused"); + let sarif_name = report_path(&tmp, "fix.sarif"); + let xml_name = report_path(&tmp, "fix.xml"); + jmove(&tmp, &["fix", "--dry-run", "--report", &sarif_name]) + .success() + .stdout(predicate::str::contains("-import type { Ghost }")); + jmove(&tmp, &["fix", "--dry-run", "--report", &xml_name]).success(); + let doc = sarif(&tmp, "fix.sarif"); + let result = &doc["runs"][0]["results"][0]; + assert_eq!(result["ruleId"], "ts/unused-import"); + assert_eq!(result["level"], "warning"); + assert_eq!(result["properties"]["autoFixable"], true); + let xml = read(&in_root(tmp.path(), "fix.xml")); + assert!(xml.contains("source=\"jmove.ts.unused-import\""), "{xml}"); + assert!(xml.contains("severity=\"warning\""), "{xml}"); +} + +#[test] +fn applied_fix_writes_report_before_stdout_summary() { + let tmp = copy_fixture("typescript", "unused"); + let report = report_path(&tmp, "applied.sarif"); + jmove(&tmp, &["fix", "--report", &report]) + .success() + .stdout(predicate::str::contains("fixed")); + let doc = sarif(&tmp, "applied.sarif"); + assert!(!doc["runs"][0]["results"].as_array().unwrap().is_empty()); +} diff --git a/tests/java/mismatch/src/main/java/com/example/Bad.java b/tests/java/mismatch/src/main/java/com/example/Bad.java new file mode 100644 index 0000000..c6d6e4c --- /dev/null +++ b/tests/java/mismatch/src/main/java/com/example/Bad.java @@ -0,0 +1,3 @@ +package com.example; + +public class Wrong {} diff --git a/tests/java/mismatch/src/main/java/com/example/Good.java b/tests/java/mismatch/src/main/java/com/example/Good.java new file mode 100644 index 0000000..c4e583a --- /dev/null +++ b/tests/java/mismatch/src/main/java/com/example/Good.java @@ -0,0 +1,3 @@ +package com.example; + +public class Good {} diff --git a/tests/typescript/aliased/package.json b/tests/typescript/aliased/package.json new file mode 100644 index 0000000..3f4a4c7 --- /dev/null +++ b/tests/typescript/aliased/package.json @@ -0,0 +1 @@ +{ "name": "aliased-fixture" } diff --git a/tests/typescript/aliased/src/app.ts b/tests/typescript/aliased/src/app.ts new file mode 100644 index 0000000..611b2ea --- /dev/null +++ b/tests/typescript/aliased/src/app.ts @@ -0,0 +1,7 @@ +import { shout } from "@utils/str"; +import cfg from "@cfg"; +import { log } from "./utils/log"; + +export function go(): string { + return shout(cfg) + log(); +} diff --git a/tests/typescript/aliased/src/config.ts b/tests/typescript/aliased/src/config.ts new file mode 100644 index 0000000..614457e --- /dev/null +++ b/tests/typescript/aliased/src/config.ts @@ -0,0 +1 @@ +export default "cfg"; diff --git a/tests/typescript/aliased/src/utils/log.ts b/tests/typescript/aliased/src/utils/log.ts new file mode 100644 index 0000000..2cf3ecd --- /dev/null +++ b/tests/typescript/aliased/src/utils/log.ts @@ -0,0 +1,3 @@ +export function log(): string { + return ""; +} diff --git a/tests/typescript/aliased/src/utils/str.ts b/tests/typescript/aliased/src/utils/str.ts new file mode 100644 index 0000000..2b5e1b1 --- /dev/null +++ b/tests/typescript/aliased/src/utils/str.ts @@ -0,0 +1,3 @@ +export function shout(s: string): string { + return s.toUpperCase(); +} diff --git a/tests/typescript/aliased/tsconfig.json b/tests/typescript/aliased/tsconfig.json new file mode 100644 index 0000000..26e34f3 --- /dev/null +++ b/tests/typescript/aliased/tsconfig.json @@ -0,0 +1,10 @@ +{ + // JSONC on purpose: comments and trailing commas must not break loading + "compilerOptions": { + "baseUrl": "./src", + "paths": { + "@utils/*": ["utils/*"], + "@cfg": ["config.ts"], + }, + }, +} diff --git a/tests/typescript/refs/.notes/refs.md b/tests/typescript/refs/.notes/refs.md new file mode 100644 index 0000000..6289495 --- /dev/null +++ b/tests/typescript/refs/.notes/refs.md @@ -0,0 +1 @@ +stale ref: ./lib/sum diff --git a/tests/typescript/refs/README.md b/tests/typescript/refs/README.md new file mode 100644 index 0000000..afa69e5 --- /dev/null +++ b/tests/typescript/refs/README.md @@ -0,0 +1,3 @@ +Use [sum](./lib/sum.ts) via `lib/sum`. +See also lib/summary for text. +Layout note: lib/ holds helpers. diff --git a/tests/typescript/refs/__tests__/sum.test.ts b/tests/typescript/refs/__tests__/sum.test.ts new file mode 100644 index 0000000..bf03682 --- /dev/null +++ b/tests/typescript/refs/__tests__/sum.test.ts @@ -0,0 +1 @@ +jest.mock('../lib/sum'); diff --git a/tests/typescript/refs/app.ts b/tests/typescript/refs/app.ts new file mode 100644 index 0000000..e28a9fa --- /dev/null +++ b/tests/typescript/refs/app.ts @@ -0,0 +1,2 @@ +import { sum } from './lib/sum'; +export const x = sum(1, 2); diff --git a/tests/typescript/refs/lib/sum.ts b/tests/typescript/refs/lib/sum.ts new file mode 100644 index 0000000..8073d27 --- /dev/null +++ b/tests/typescript/refs/lib/sum.ts @@ -0,0 +1 @@ +export const sum = (a: number, b: number): number => a + b; diff --git a/tests/typescript/refs/lib/summary.ts b/tests/typescript/refs/lib/summary.ts new file mode 100644 index 0000000..fc920fc --- /dev/null +++ b/tests/typescript/refs/lib/summary.ts @@ -0,0 +1 @@ +export const summary = 'text'; diff --git a/tests/typescript/refs/package-lock.json b/tests/typescript/refs/package-lock.json new file mode 100644 index 0000000..8c799c5 --- /dev/null +++ b/tests/typescript/refs/package-lock.json @@ -0,0 +1 @@ +{ "packages": { "x": "./lib/sum" } } diff --git a/tests/typescript/refs/package.json b/tests/typescript/refs/package.json new file mode 100644 index 0000000..21ec30c --- /dev/null +++ b/tests/typescript/refs/package.json @@ -0,0 +1 @@ +{ "name": "refs-fixture", "main": "./lib/sum.ts" } diff --git a/todo.md b/todo.md index 2d62afb..c4f7f18 100644 --- a/todo.md +++ b/todo.md @@ -55,7 +55,9 @@ AI оставляем СНАРУЖИ: при неоднозначности jmov - [~] Java v1: unused-imports (DONE, skip wildcard/ambiguous), missing-import (DONE: unique FQN candidate → insert `import pkg.Type;` at the import-block end; ambiguous/wildcard → `--json` `candidates`, applied:false; закрыт guava-разрыв «перенесли файл, соседняя ссылка без импорта умерла» — проверено mv+fix+javac SUCCESS), - import-order (DONE: Google-стиль — statics первыми, ASCII-сортировка, дедуп; конфликтующие с другими правилами откладываются (prune_overlaps по severity) и сходятся за 2-3 прогона), class-name-mismatch + import-order (DONE: Google-стиль — statics первыми, ASCII-сортировка, дедуп; конфликтующие с другими правилами откладываются (prune_overlaps по severity) и сходятся за 2-3 прогона), + class-name-mismatch (DONE как ПОВЕРХНОСТЬ check, не fix: починка = переименование файла, а fix-движок умеет только байтовые правки; + check отдаёт находку с готовой командой `jmove mv`, exit code 2; rename не меняет FQN → импорты не трогаются) - [x] TS v1: unused-imports (DONE: whole-statement delete, все биндинги мертвы → строка уходит; mixed used/unused НЕ трогаем — в ESM импорт исполняет побочные эффекты модуля, partial-удаление specifier'ов отложено осознанно) @@ -64,9 +66,9 @@ AI оставляем СНАРУЖИ: при неоднозначности jmov - [ ] Форматирование: свой cargo-fmt НЕ строим (вечный long-tail). Только «import formatting» (порядок/группировка — у нас уже есть spans). Опционально `--format-after ` (prettier / google-java-format), не зависимость -- [ ] Интероп PMD/Checkstyle/eslint (фаза 2.5): `jmove fix --report checkstyle.xml` маппит - violation(file,line,rule) на паттерны; на выход SARIF для CI/IDE. - Маркетинг: «auto-fix for what Checkstyle only reports» +- [x] Интероп Checkstyle/eslint (фаза 2.5): `check --report f.sarif|.xml` и `fix --report f.sarif|.xml` — + SARIF 2.1.0 (GitHub/CodeQL, autoFixable) и Checkstyle XML (source=jmove.); формат по расширению, + чистый прогон = валидный пустой файл, stdout/exit не меняются. Маркетинг: «auto-fix for what Checkstyle only reports» ## Guava real-world smoke test (google/guava @ main, JDK21, mvnw) — ПРОВЕРЕНО - [x] mv Primitives primitives→util: 5 правок (4 imports + package), `mvn -pl guava compile` @@ -75,10 +77,12 @@ AI оставляем СНАРУЖИ: при неоднозначности jmov явных импорта корректно, НО javac упал: сам перенесённый файл ссылался на соседний `GwtCompatible` БЕЗ импорта (тот же пакет) → после mv ссылка битая. jmove в v1 осознанно НЕ добавляет импорты. Это главный driver для fix/missing-import из Phase 1.6 выше -- [ ] (после fix) повторить обе перемещения как `mv` + авто-`fix` и добить compile до SUCCESS - (паттерн воспроизведён и закрыт локально: mv файла с bare-ссылкой на соседний пакет → - `fix` добавил импорт → javac SUCCESS; на реальном guava ещё не прогонялось) -- [x] Индексация в monorepo с дублями пакетов (guava vs android/guava в одном --root): +- [x] Обе перемещения повторены как `mv` + авто-`fix`, compile = BUILD SUCCESS (guava main, JDK21): + VisibleForTesting annotations→annotations.testing: mv без --source-root давал 1 правку (62 потеряны!), + с --source-root guava — 63/63 через git mv; fix добавил в перенесённый файл + `import com.google.common.annotations.GwtCompatible` (тот самый разрыв v1) + 36 import-order + (сошёлся за 2 прогона); Primitives primitives→util: 5 правок + fix; `jmove check` = 0 broken +- [x] Индексация в monorepo с дублями пакетов — ПРОВЕРЕНО НА guava (см. выше): глобальный `--source-root DIR` — индексирует (mv/check/fix) только поддерево, FQN-коллизии исчезают, соседнее дерево не трогается; авто-определение по mv-цели осознанно НЕ делаем (явный флаг предсказуемее, см. KISS) @@ -86,11 +90,15 @@ AI оставляем СНАРУЖИ: при неоднозначности jmov ## Phase 2 - [ ] Кэш индекса на диске (bincode/rkyv) → .jmove/index - [ ] Инкрементальная переиндексация (только изменённые файлы) -- [ ] Поддержка tsconfig paths / алиасов (@/...) +- [x] Поддержка tsconfig paths / алиасов (@/...): JSONC-парсер (комментарии/хвостовые запятые), + baseUrl + star/exact keys, longest-prefix wins; при mv алиас сохраняется, если файл остался + в дереве алиаса, иначе fallback на относительный; extends/2+ кандидаты — осознанно не делаем (v1) - [ ] Параллельная индексация через rayon - [x] --git интеграция (git mv для stage/истории): auto для tracked файлов, --no-git флаг, moved_via/would_move_via в --json -- [ ] Перенос директорий целиком (mv папки) -- [ ] Предупреждения о не-import ссылках: package.json exports, jest mocks, tsconfig includes, markdown links +- [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') не ловятся - [ ] prettier интеграция после rewrite (по желанию) ## Phase 3