feat(check): java public-class ⇄ file-name mismatch — layout finding with a ready jmove mv repair
- 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
This commit is contained in:
parent
c0b06efe57
commit
f8004e669b
12 changed files with 334 additions and 80 deletions
|
|
@ -196,7 +196,8 @@ fn check(root: &Path, source_root: Option<&Path>, json: bool) -> Flow<i32> {
|
|||
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<i32> {
|
|||
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)
|
||||
}
|
||||
|
|
|
|||
133
src/cli/output/check.rs
Normal file
133
src/cli/output/check.rs
Normal file
|
|
@ -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<BrokenImport>,
|
||||
/// 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<NameMismatch>,
|
||||
}
|
||||
|
||||
/// 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<Vec<BrokenImport>> {
|
||||
let mut broken: Vec<BrokenImport> = 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<Vec<NameMismatch>> {
|
||||
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
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
@ -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<String> {
|
||||
pub(crate) fn read_file(root: &Path, rel: &Path) -> JmoveResult<String> {
|
||||
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<Vec<BrokenImport>> {
|
||||
let mut broken: Vec<BrokenImport> = 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<Vec<ChangedFile>> {
|
||||
|
|
@ -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<BrokenImport>,
|
||||
/// Number of broken imports (kept as an explicit counter for agents).
|
||||
pub total: usize,
|
||||
}
|
||||
128
src/parser/java/class_name.rs
Normal file
128
src/parser/java/class_name.rs
Normal file
|
|
@ -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<usize>,
|
||||
}
|
||||
|
||||
/// `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<Mismatch> {
|
||||
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<Node> = 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<String> {
|
||||
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");
|
||||
}
|
||||
}
|
||||
|
|
@ -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};
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue