From f8004e669bc3639c4cee6a8da68e0133baf0f1a1 Mon Sep 17 00:00:00 2001 From: loki5512344 Date: Tue, 15 Sep 2026 19:05:54 +0200 Subject: [PATCH] =?UTF-8?q?feat(check):=20java=20public-class=20=E2=87=84?= =?UTF-8?q?=20file-name=20mismatch=20=E2=80=94=20layout=20finding=20with?= =?UTF-8?q?=20a=20ready=20jmove=20mv=20repair?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - parser::java::class_name: exactly-one-public-top-level-type rule (package-info/module-info and 0-or-2-public files skipped) - check gains name_mismatches[] (JSON, omitted when clean) + human lines; exit 2 covers both kinds; fix engine untouched by design — a file rename is not a byte edit, and mv keeps the FQN so no imports change - output split into output/{mod,check} to stay under the 250-line rule - e2e: reports Bad.java, stays quiet on Good.java, suggested rename makes check pass and preserves the file bytes --- docs/EXAMPLES.md | 2 +- docs/SKILL.md | 9 +- src/cli/mod.rs | 6 +- src/cli/output/check.rs | 133 ++++++++++++++++++ src/cli/{output.rs => output/mod.rs} | 80 ++--------- src/parser/java/class_name.rs | 128 +++++++++++++++++ src/parser/java/mod.rs | 1 + tests/cli.rs | 2 +- tests/cli_name_check.rs | 43 ++++++ .../src/main/java/com/example/Bad.java | 3 + .../src/main/java/com/example/Good.java | 3 + todo.md | 4 +- 12 files changed, 334 insertions(+), 80 deletions(-) create mode 100644 src/cli/output/check.rs rename src/cli/{output.rs => output/mod.rs} (67%) create mode 100644 src/parser/java/class_name.rs create mode 100644 tests/cli_name_check.rs create mode 100644 tests/java/mismatch/src/main/java/com/example/Bad.java create mode 100644 tests/java/mismatch/src/main/java/com/example/Good.java diff --git a/docs/EXAMPLES.md b/docs/EXAMPLES.md index 7e79521..6ea482d 100644 --- a/docs/EXAMPLES.md +++ b/docs/EXAMPLES.md @@ -42,7 +42,7 @@ Rewrites happen first, the rename last; any failure rolls everything back. ```console $ jmove check -check: no broken imports found +check: no findings ``` When something does point at nothing, `check` prints one line per broken diff --git a/docs/SKILL.md b/docs/SKILL.md index e05d4e6..2486cb6 100644 --- a/docs/SKILL.md +++ b/docs/SKILL.md @@ -40,13 +40,16 @@ working tree unstaged either way — stage or commit them yourself. `--json` reports the choice as `moved_via` (`"git"`/`"fs"`) and, on a dry-run, `would_move_via`. -### check — find broken imports +### check — find broken imports and Java layout errors ``` jmove check [--root DIR] [--json] ``` -Run after any move (or any edit) to validate project consistency. +Run after any move (or any edit) to validate project consistency. Reports +broken relative imports and Java files whose single public type is named +differently from the file (each finding carries the exact `jmove mv` that +renames it; exit code 2 covers both kinds). ### fix — auto-repair import problems @@ -119,7 +122,7 @@ and a `hint` describing the next action. On success, `mv` reports - `0` — success - `1` — operation failed (read `--json` error or stderr) -- `2` — `check` found broken imports +- `2` — `check` found broken imports or Java name mismatches ## Rules of use diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 68c4d56..f18956e 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -196,7 +196,8 @@ fn check(root: &Path, source_root: Option<&Path>, json: bool) -> Flow { let scope = source_root.as_deref(); let index = flow(json, "check", Index::build_scoped(&root, scope))?; let broken = flow(json, "check", output::broken_imports(&root, &index))?; - let code = if broken.is_empty() { + let mismatches = flow(json, "check", output::name_mismatches(&root, &index))?; + let code = if broken.is_empty() && mismatches.is_empty() { exit::OK } else { exit::BROKEN @@ -207,10 +208,11 @@ fn check(root: &Path, source_root: Option<&Path>, json: bool) -> Flow { let data = output::CheckData { broken_imports: broken, total, + name_mismatches: mismatches, }; json::print(&Envelope::ok("check", data)); } else { - output::report_check(&broken); + output::report_check(&broken, &mismatches); } Ok(code) } diff --git a/src/cli/output/check.rs b/src/cli/output/check.rs new file mode 100644 index 0000000..10e0f75 --- /dev/null +++ b/src/cli/output/check.rs @@ -0,0 +1,133 @@ +//! `jmove check` collectors and payloads: unresolvable relative imports +//! plus Java file-name ⇄ public-class mismatches (layout errors that +//! `javac` rejects but import resolution cannot see). + +use serde::Serialize; +use std::path::Path; + +use crate::core::index::Index; +use crate::core::{JmoveResult, rel_str}; +use crate::parser::SourceLanguage; +use crate::parser::java::class_name; + +use super::{line_of, read_file}; + +/// `check` stdout line when the project has no findings. +const CHECK_CLEAN: &str = "check: no findings"; + +/// One unresolvable relative import found by `check`. +#[derive(Debug, Serialize)] +pub struct BrokenImport { + /// Project-relative file declaring the import. + pub file: String, + /// 1-based line of the specifier. + pub line: usize, + /// Specifier text as written. + pub import: String, + /// Stable reason code, currently always `"file_not_found"`. + pub reason: &'static str, +} + +/// A Java file whose single public top-level type is named differently +/// from the file — a `javac` error repaired by renaming the file (imports +/// stay valid: the class FQN does not change). +#[derive(Debug, Serialize)] +pub struct NameMismatch { + /// Project-relative file with the wrong name. + pub file: String, + /// 1-based line of the public type declaration. + pub line: usize, + /// Declared public type. + pub public_class: String, + /// Project-relative file it should live in. + pub expected_file: String, + /// Copy-paste repair command (paths are quoted for safety). + pub rename: String, +} + +/// Success payload of `check --json` (flattened under the envelope). +#[derive(Debug, Serialize)] +pub struct CheckData { + /// Broken imports, sorted by file then line. + pub broken_imports: Vec, + /// Number of broken imports (kept as an explicit counter for agents). + pub total: usize, + /// Java layout findings (omitted from JSON when clean). + #[serde(skip_serializing_if = "Vec::is_empty")] + pub name_mismatches: Vec, +} + +/// Collect every relative import that resolves to nothing in `index`. +/// +/// A specifier starting with `.` whose target is `None` is broken; a bare +/// package specifier without a target is an external dependency, not an +/// error. Results are sorted by file, then line. +pub fn broken_imports(root: &Path, index: &Index) -> JmoveResult> { + let mut broken: Vec = Vec::new(); + for (file, imports) in &index.imports { + for import in imports { + if import.target.is_some() || !import.record.specifier.starts_with('.') { + continue; + } + let text = read_file(root, file)?; + broken.push(BrokenImport { + file: rel_str(file), + line: line_of(&text, import.record.span.start), + import: import.record.specifier.clone(), + reason: "file_not_found", + }); + } + } + broken.sort_by(|a, b| (&a.file, a.line).cmp(&(&b.file, b.line))); + Ok(broken) +} + +/// Java files whose only public top-level type disagrees with the file +/// name; sorted by file, then line. +pub fn name_mismatches(root: &Path, index: &Index) -> JmoveResult> { + let mut out = Vec::new(); + for file in index.files.sorted() { + if SourceLanguage::for_path(&file) != Some(SourceLanguage::Java) { + continue; + } + let text = read_file(root, &file)?; + let Some(found) = class_name::mismatch(&file, &text) else { + continue; + }; + let dir = file.parent().unwrap_or(Path::new("")); + let ext = file.extension().and_then(|e| e.to_str()).unwrap_or("java"); + let expected = dir.join(format!("{}.{}", found.public_class, ext)); + let (old, new) = (rel_str(&file), rel_str(&expected)); + out.push(NameMismatch { + file: old.clone(), + line: line_of(&text, found.span.start), + public_class: found.public_class, + expected_file: new.clone(), + rename: format!("jmove mv '{old}' '{new}'"), + }); + } + out.sort_by_key(|m| (m.file.clone(), m.line)); + Ok(out) +} + +/// Print the human `check` report: the clean note, or one +/// `path:line: cannot resolve 'spec'` line per broken import and one +/// rename-line per Java layout mismatch. +pub fn report_check(broken: &[BrokenImport], mismatches: &[NameMismatch]) { + if broken.is_empty() && mismatches.is_empty() { + println!("{CHECK_CLEAN}"); + return; + } + for entry in broken { + println!( + "{}:{}: cannot resolve '{}'", + entry.file, entry.line, entry.import + ); + } + for m in mismatches { + println!( + "{}:{}: public class '{}' must live in '{}'; fix: {}", + m.file, m.line, m.public_class, m.expected_file, m.rename + ); + } +} diff --git a/src/cli/output.rs b/src/cli/output/mod.rs similarity index 67% rename from src/cli/output.rs rename to src/cli/output/mod.rs index b83a1e8..a9ad44a 100644 --- a/src/cli/output.rs +++ b/src/cli/output/mod.rs @@ -1,25 +1,26 @@ //! Human-readable rendering plus the payload builders that both output -//! modes share: grouping rewrites, resolving broken imports, line lookup. +//! modes share: grouping rewrites, `mv` pre-flight validation, line lookup. +//! The `check` payloads and collectors live in [`check`]. //! //! Pure functions returning data, except [`report_check`] and //! [`print_error`] which perform the only I/O (stdout and stderr). -use serde::Serialize; - use std::collections::BTreeMap; use std::path::{Path, PathBuf}; -use crate::core::index::Index; use crate::core::plan::{MovePlan, Rewrite}; use crate::core::{JmoveResult, rel_str}; +mod check; + use super::json::{Change, ChangedFile, ErrorData}; -/// `check` stdout line when the project has no broken imports. -const CHECK_CLEAN: &str = "check: no broken imports found"; +pub use check::{ + BrokenImport, CheckData, NameMismatch, broken_imports, name_mismatches, report_check, +}; /// Read a project file (project-relative path) as UTF-8 text. -fn read_file(root: &Path, rel: &Path) -> JmoveResult { +pub(crate) fn read_file(root: &Path, rel: &Path) -> JmoveResult { Ok(std::fs::read_to_string(root.join(rel))?) } @@ -44,32 +45,6 @@ pub fn group_by_file(rewrites: &[Rewrite]) -> Vec<(&Path, Vec<&Rewrite>)> { } map.into_iter().collect() } - -/// Collect every relative import that resolves to nothing in `index`. -/// -/// A specifier starting with `.` whose target is `None` is broken; a bare -/// package specifier without a target is an external dependency, not an -/// error. Results are sorted by file, then line. -pub fn broken_imports(root: &Path, index: &Index) -> JmoveResult> { - let mut broken: Vec = Vec::new(); - for (file, imports) in &index.imports { - for import in imports { - if import.target.is_some() || !import.record.specifier.starts_with('.') { - continue; - } - let text = read_file(root, file)?; - broken.push(BrokenImport { - file: rel_str(file), - line: line_of(&text, import.record.span.start), - import: import.record.specifier.clone(), - reason: "file_not_found", - }); - } - } - broken.sort_by(|a, b| (&a.file, a.line).cmp(&(&b.file, b.line))); - Ok(broken) -} - /// Build the `changed_files` payload: line-level specifier diffs grouped per /// importer, computed against the on-disk contents at call time. pub fn changed_files(root: &Path, plan: &MovePlan) -> JmoveResult> { @@ -165,23 +140,6 @@ pub fn mv_summary(plan: &MovePlan, via_git: bool) -> String { plural(files, "file"), ) } - -/// Print the human `check` report: the clean note, or one -/// `path:line: cannot resolve 'spec'` line per broken import. -pub fn report_check(broken: &[BrokenImport]) { - if broken.is_empty() { - println!("{CHECK_CLEAN}"); - return; - } - for entry in broken { - let line = format!( - "{}:{}: cannot resolve '{}'", - entry.file, entry.line, entry.import - ); - println!("{line}"); - } -} - /// Print an operation error to stderr, with the hint on its own line. pub fn print_error(message: &str, hint: Option<&str>) { eprintln!("jmove: {message}"); @@ -198,25 +156,3 @@ pub(crate) fn plural(count: usize, noun: &str) -> String { format!("{noun}s") } } - -/// One unresolvable relative import found by `check`. -#[derive(Debug, Serialize)] -pub struct BrokenImport { - /// Project-relative file declaring the import. - pub file: String, - /// 1-based line of the specifier. - pub line: usize, - /// Specifier text as written. - pub import: String, - /// Stable reason code, currently always `"file_not_found"`. - pub reason: &'static str, -} - -/// Success payload of `check --json` (flattened under the envelope). -#[derive(Debug, Serialize)] -pub struct CheckData { - /// Broken imports, sorted by file then line. - pub broken_imports: Vec, - /// Number of broken imports (kept as an explicit counter for agents). - pub total: usize, -} diff --git a/src/parser/java/class_name.rs b/src/parser/java/class_name.rs new file mode 100644 index 0000000..9ab1019 --- /dev/null +++ b/src/parser/java/class_name.rs @@ -0,0 +1,128 @@ +//! Java layout check: the single public top-level type must match the +//! file name (javac: "class Foo is public, should be declared in a file +//! named Foo.java"). +//! +//! This is a *finding*, not a fix rule: the repair is a file rename, and +//! the fix engine applies byte edits only. `jmove check` reports it with +//! the exact `jmove mv` command that repairs the layout — the moved class +//! keeps its FQN, so renaming the file rewrites no imports. + +use std::ops::Range; +use std::path::Path; + +use tree_sitter::Node; + +use super::{TreeSitterJava, text}; + +/// Kinds that introduce a top-level type in Java. +const TYPE_KINDS: &[&str] = &[ + "class_declaration", + "interface_declaration", + "enum_declaration", + "record_declaration", + "annotation_type_declaration", +]; + +/// The mismatch details: the public type's name, the span of its name +/// token (for line reporting) and the file stem it should live in. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Mismatch { + /// Name of the single public top-level type. + pub public_class: String, + /// Byte span of the class name token. + pub span: Range, +} + +/// `Some` when the file declares exactly one public top-level type whose +/// name differs from the file stem. Two public types (also illegal) or +/// zero are not this check's business. `package-info`/`module-info` files +/// never name a type and are skipped. +#[must_use] +pub fn mismatch(path: &Path, source: &str) -> Option { + let stem = path.file_stem()?.to_str()?; + if matches!(stem, "package-info" | "module-info") { + return None; + } + let tree = TreeSitterJava::parse(source)?; + let mut cursor = tree.root_node().walk(); + let publics: Vec = tree + .root_node() + .children(&mut cursor) + .filter(|n| TYPE_KINDS.contains(&n.kind())) + .filter(|n| has_public_modifier(*n)) + .collect(); + let [only] = publics.as_slice() else { + return None; // zero or many public types: not a naming mismatch + }; + let mut c = only.walk(); + let name = only.children(&mut c).find(|n| n.kind() == "identifier")?; + let name_text = text(name, source); + (name_text != stem).then(|| Mismatch { + public_class: name_text.to_owned(), + span: name.byte_range(), + }) +} + +// `public` is an anonymous token inside the `modifiers` child. +fn has_public_modifier(node: Node) -> bool { + let mut c = node.walk(); + node.children(&mut c) + .find(|n| n.kind() == "modifiers") + .is_some_and(|mods| { + let mut m = mods.walk(); + mods.children(&mut m).any(|t| t.kind() == "public") + }) +} + +#[cfg(test)] +mod tests { + use super::mismatch; + use std::path::Path; + + fn m(path: &str, src: &str) -> Option { + mismatch(Path::new(path), src).map(|f| f.public_class) + } + + #[test] + fn public_type_name_must_equal_file_stem() { + assert_eq!( + m("Foo.java", "package p;\npublic class Bar {}\n").as_deref(), + Some("Bar") + ); + assert_eq!(m("Foo.java", "package p;\npublic class Foo {}\n"), None); + } + + #[test] + fn every_public_top_level_kind_is_checked() { + assert_eq!( + m("A.java", "public interface B { int x(); }").as_deref(), + Some("B") + ); + assert_eq!(m("A.java", "public enum B { X }").as_deref(), Some("B")); + assert_eq!( + m("A.java", "public record B(int x) {}").as_deref(), + Some("B") + ); + assert_eq!(m("A.java", "public @interface B {}").as_deref(), Some("B")); + } + + #[test] + fn non_public_multiple_or_nested_types_are_not_reported() { + // Zero public types: legal package-private layout. + assert_eq!(m("Foo.java", "class Bar {}\nclass Baz {}"), None); + // Two public top-level types: a different (harder) error. + assert_eq!(m("Foo.java", "public class A {}\npublic class B {}"), None); + // Nested publics live inside a type: not top-level. + assert_eq!(m("A.java", "class A { public class B {} }"), None); + // Protected/private modifiers do not trigger the javac rule. + assert_eq!(m("A.java", "class B {}"), None); + } + + #[test] + fn info_files_are_skipped_and_span_names_the_class_token() { + assert_eq!(m("package-info.java", "package p;"), None); + let src = "package p;\n\npublic class Bar {}\n"; + let found = mismatch(Path::new("Foo.java"), src).unwrap(); + assert_eq!(&src[found.span.clone()], "Bar"); + } +} diff --git a/src/parser/java/mod.rs b/src/parser/java/mod.rs index 2fc62fc..2554092 100644 --- a/src/parser/java/mod.rs +++ b/src/parser/java/mod.rs @@ -12,6 +12,7 @@ use std::path::{Path, PathBuf}; use tree_sitter::{Node, Parser, Tree}; +pub mod class_name; pub mod rules; use super::{ImportRecord, Language, PackageDecl, SourceLanguage}; diff --git a/tests/cli.rs b/tests/cli.rs index 188f208..e1f04d8 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -105,7 +105,7 @@ fn check_passes_on_clean_and_fails_on_broken_fixture() { let clean = fixture("basic"); jmove(&clean, &["check"]) .success() - .stdout(predicate::str::contains("no broken imports")); + .stdout(predicate::str::contains("check: no findings")); let messy = fixture("complex"); jmove(&messy, &["check"]) diff --git a/tests/cli_name_check.rs b/tests/cli_name_check.rs new file mode 100644 index 0000000..615d77d --- /dev/null +++ b/tests/cli_name_check.rs @@ -0,0 +1,43 @@ +//! `jmove check` finds Java files whose public type is misnamed, and the +//! suggested `jmove mv` repairs the layout. + +mod common; + +use common::{copy_fixture, in_root, jmove, read}; +use predicates::prelude::*; + +const BAD: &str = "src/main/java/com/example/Bad.java"; + +#[test] +fn check_reports_misnamed_public_class_and_clean_files_stay_quiet() { + let tmp = copy_fixture("java", "mismatch"); + jmove(&tmp, &["check"]) + .code(2) + .stdout( + predicate::str::contains( + "src/main/java/com/example/Bad.java:3: public class 'Wrong' must live in 'src/main/java/com/example/Wrong.java'", + ) + .and(predicate::str::contains("Good.java").not()), + ); + jmove(&tmp, &["check", "--json"]) + .code(2) + .stdout( + predicate::str::contains("\"name_mismatches\"") + .and(predicate::str::contains("\"public_class\": \"Wrong\"")) + .and(predicate::str::contains( + "\"rename\": \"jmove mv 'src/main/java/com/example/Bad.java' 'src/main/java/com/example/Wrong.java'\"", + )), + ); +} + +#[test] +fn running_the_suggested_rename_makes_check_pass() { + let tmp = copy_fixture("java", "mismatch"); + jmove(&tmp, &["mv", BAD, "src/main/java/com/example/Wrong.java"]).success(); + let moved = read(&in_root(tmp.path(), "src/main/java/com/example/Wrong.java")); + assert!(moved.contains("public class Wrong {}"), "{moved}"); + assert!(moved.contains("package com.example;"), "{moved}"); + jmove(&tmp, &["check"]) + .success() + .stdout(predicate::str::contains("no findings")); +} diff --git a/tests/java/mismatch/src/main/java/com/example/Bad.java b/tests/java/mismatch/src/main/java/com/example/Bad.java new file mode 100644 index 0000000..c6d6e4c --- /dev/null +++ b/tests/java/mismatch/src/main/java/com/example/Bad.java @@ -0,0 +1,3 @@ +package com.example; + +public class Wrong {} diff --git a/tests/java/mismatch/src/main/java/com/example/Good.java b/tests/java/mismatch/src/main/java/com/example/Good.java new file mode 100644 index 0000000..c4e583a --- /dev/null +++ b/tests/java/mismatch/src/main/java/com/example/Good.java @@ -0,0 +1,3 @@ +package com.example; + +public class Good {} diff --git a/todo.md b/todo.md index 7573fc8..e7bbd10 100644 --- a/todo.md +++ b/todo.md @@ -55,7 +55,9 @@ AI оставляем СНАРУЖИ: при неоднозначности jmov - [~] Java v1: unused-imports (DONE, skip wildcard/ambiguous), missing-import (DONE: unique FQN candidate → insert `import pkg.Type;` at the import-block end; ambiguous/wildcard → `--json` `candidates`, applied:false; закрыт guava-разрыв «перенесли файл, соседняя ссылка без импорта умерла» — проверено mv+fix+javac SUCCESS), - import-order (DONE: Google-стиль — statics первыми, ASCII-сортировка, дедуп; конфликтующие с другими правилами откладываются (prune_overlaps по severity) и сходятся за 2-3 прогона), class-name-mismatch + import-order (DONE: Google-стиль — statics первыми, ASCII-сортировка, дедуп; конфликтующие с другими правилами откладываются (prune_overlaps по severity) и сходятся за 2-3 прогона), + class-name-mismatch (DONE как ПОВЕРХНОСТЬ check, не fix: починка = переименование файла, а fix-движок умеет только байтовые правки; + check отдаёт находку с готовой командой `jmove mv`, exit code 2; rename не меняет FQN → импорты не трогаются) - [x] TS v1: unused-imports (DONE: whole-statement delete, все биндинги мертвы → строка уходит; mixed used/unused НЕ трогаем — в ESM импорт исполняет побочные эффекты модуля, partial-удаление specifier'ов отложено осознанно)