Fix a bug in class file reader

Fix indentation writer
Improve testing harness to produce a much more helpful development
environment

Signed-off-by: Tin Svagelj <tin.svagelj@live.com>

Cargo.toml

@@ -29,3 +29,7 @@ zip = "0.6"
 serde = { version = "1.0", features = ["derive"] }
 
 clap = { version = "4.0", features = ["derive"], optional = true }
+
+[dev-dependencies]
+pretty_assertions = "1.4"
+tracing-subscriber = { version = "0.3" }

README.md

@@ -11,6 +11,10 @@ Pattern matching doesn't cover all instructions yet so a lot of the output is co
 
 Most command line arguments aren't properly handled yet.
 
+### Testing
+
+In order to run the test suite `JAVA_HOME` environment variable must be set or `javac` must be in path.
+
 ## JVM support status
 
 Latest version is currently being written to support any semi-recent Java version.

class_format/src/lib.rs

@@ -141,6 +141,14 @@ impl ClassPath {
         });
     }
 
+    pub fn from_class_index(
+        pool: &ConstantPool,
+        class_index: usize,
+    ) -> Result<Self, ClassPathError> {
+        let utf8_index = constant_match!(pool.get(class_index), Constant::Class { name_index } => { *name_index as usize })?;
+        ClassPath::try_from(pool.get(utf8_index))
+    }
+
     pub fn parse(name: impl AsRef<str>) -> Result<Self, ClassPathError> {
         let mut cursor = std::io::Cursor::new(name.as_ref());
         Self::read_from(&mut cursor)
@@ -230,26 +238,6 @@ pub struct Class {
     pub attributes: HashMap<String, AttributeValue>,
 }
 
-fn name_from_class_index(
-    index: usize,
-    constant_pool: &ConstantPool,
-) -> Result<ClassPath, ClassReadError> {
-    log::trace!("enter name_from_class_index({}, &constant_pool)", index);
-    let constant = constant_pool.try_get(index)?;
-
-    match constant {
-        Constant::Class { name_index } => match constant_pool.try_get(*name_index as usize)? {
-            Constant::Utf8 { value } => Ok(ClassPath::parse(value)?),
-            _ => Err(ClassReadError::InvalidClassNameReference),
-        },
-        other => Err(ConstantPoolError::UnexpectedType {
-            found: other.tag(),
-            expected: ConstantTag::Class,
-        }
-        .into()),
-    }
-}
-
 impl Class {
     pub fn open(path: impl AsRef<Path>) -> Result<Class, ClassReadError> {
         let mut file = File::open(path)?;
@@ -306,12 +294,18 @@ impl Class {
             "Class::try_from(impl Read)::class_name#{}",
             class_const_index
         );
-        let class_name = ClassPath::try_from(constant_pool.get(class_const_index))?;
+        let class_name = ClassPath::from_class_index(&constant_pool, class_const_index)?;
 
-        log::trace!("Class::read_from(impl Read)::super_name");
         let super_const_index = r.read_u16::<BE>()? as usize;
+        log::trace!(
+            "Class::read_from(impl Read)::super_name#{}",
+            super_const_index
+        );
         let super_name = if super_const_index != 0 {
-            Some(ClassPath::try_from(constant_pool.get(class_const_index))?)
+            Some(ClassPath::from_class_index(
+                &constant_pool,
+                super_const_index,
+            )?)
         } else {
             None
         };
@@ -322,7 +316,7 @@ impl Class {
 
         for _ in 0..interface_count {
             let interface_index = r.read_u16::<BE>()? as usize;
-            let interface_name = name_from_class_index(interface_index, &constant_pool)?;
+            let interface_name = ClassPath::from_class_index(&constant_pool, interface_index)?;
 
             interfaces.push(interface_name);
         }

src/gen/indent.rs

@@ -1,4 +1,4 @@
-use std::{io::Write, collections::HashSet};
+use std::{collections::HashSet, io::Write};
 
 #[derive(Debug, Copy, Clone, Eq, PartialEq)]
 #[repr(u8)]
@@ -20,23 +20,31 @@ pub struct Indented<W: Write> {
     inner: W,
     pub level: usize,
     pub indent: IndentKind,
+    pub pending: bool,
 
     pub enter_block_on: HashSet<u8>,
     pub exit_block_on: HashSet<u8>,
 }
 
 impl<W: Write> Indented<W> {
-    pub fn new(writer: W, indent: IndentKind, level: usize, enter_block_on: impl AsRef<[u8]>, exit_block_on: impl AsRef<[u8]>) -> Indented<W> {
+    pub fn new(
+        writer: W,
+        indent: IndentKind,
+        level: usize,
+        enter_block_on: impl AsRef<[u8]>,
+        exit_block_on: impl AsRef<[u8]>,
+    ) -> Indented<W> {
         Indented {
             inner: writer,
             level,
             indent,
+            pending: true,
 
             enter_block_on: HashSet::from_iter(enter_block_on.as_ref().iter().cloned()),
             exit_block_on: HashSet::from_iter(exit_block_on.as_ref().iter().cloned()),
         }
     }
-    
+
     #[inline]
     pub fn enter_block(&mut self) {
         self.level += 1;
@@ -68,36 +76,44 @@ impl<W: Write> Indented<W> {
 impl<W: Write> Write for Indented<W> {
     fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
         let mut total = 0;
+
         for byte in buf {
-            let count = match *byte as char {
+            total += match *byte as char {
                 '\n' => {
                     let nl_len = self.inner.write(b"\n")?;
-                    let indent_len = self.inner.write(self.indent_string().as_bytes())?;
-                    nl_len + indent_len
-                }
-                _ if self.enter_block_on.contains(byte) => {
-                    self.enter_block();
-                    self.inner.write(&[*byte])?
+                    self.pending = true;
+                    nl_len
                 }
-                _ if self.exit_block_on.contains(byte) => {
-                    self.exit_block();
+                _ => {
+                    if self.enter_block_on.contains(byte) {
+                        self.enter_block();
+                    } else if self.exit_block_on.contains(byte) {
+                        self.exit_block();
+                    }
+
+                    if self.pending {
+                        total += self.inner.write(self.indent_string().as_bytes())?;
+                        self.pending = false;
+                    }
+
                     self.inner.write(&[*byte])?
                 }
-                _ => self.inner.write(&[*byte])?,
             };
-            total += count;
         }
 
         Ok(total)
     }
-    
+
     fn write_all(&mut self, buf: &[u8]) -> std::io::Result<()> {
         let written = self.write(buf)?;
         // TODO: Do a proper check
         // needs to handle indentation levels
         // rope science - https://xi-editor.io/docs/rope_science_04.html
         if written < buf.len() {
-            return Err(std::io::Error::new(std::io::ErrorKind::Other, "didn't write enough code"))
+            return Err(std::io::Error::new(
+                std::io::ErrorKind::Other,
+                "didn't write enough code",
+            ));
         }
         Ok(())
     }

src/gen/java/class.rs

@@ -116,44 +116,60 @@ impl GenerateCode<Class> for JavaBackend {
                     }
                 }
             }
-            w.write_all(b" {\n")?;
+            w.write_all(b" {")?;
 
             // TODO: generate enum entries
 
-            tracing::debug!("- Generating fields for {}", class_name);
+            let contents = {
+                let mut content_buffer = Vec::with_capacity(512);
+                let mut w: Cursor<&mut Vec<u8>> = Cursor::new(&mut content_buffer);
 
-            let mut class_indent = Indented::new(&mut w, lang.indentation, 1, b"{", b"}");
+                tracing::debug!("- Generating fields for {}", class_name);
 
-            for field in &class.fields {
-                let field_requirements =
-                    self.write_value(&lang, &FieldContext, field, &mut class_indent)?;
-                req.add_import(field_requirements.imports);
-            }
+                let mut class_indent = Indented::new(&mut w, lang.indentation, 1, b"{", b"}");
 
-            tracing::debug!("- Generating methods for {}", class_name);
+                for field in &class.fields {
+                    let field_requirements =
+                        self.write_value(&lang, &FieldContext, field, &mut class_indent)?;
+                    req.add_import(field_requirements.imports);
+                }
 
-            for method in &class.methods {
-                let method_ctx = ClassContext {
-                    class_name: class.class_name.clone(),
-                    ..Default::default()
-                };
-                let method_requirements =
-                    self.write_value(&lang, &method_ctx, method, &mut class_indent)?;
-                req.add_import(method_requirements.imports);
-            }
+                tracing::debug!("- Generating methods for {}", class_name);
+
+                for method in &class.methods {
+                    let method_ctx = ClassContext {
+                        class_name: class.class_name.clone(),
+                        ..Default::default()
+                    };
+                    let method_requirements =
+                        self.write_value(&lang, &method_ctx, method, &mut class_indent)?;
+                    req.add_import(method_requirements.imports);
+                }
 
+                content_buffer
+            };
+
+            if !contents.is_empty() {
+                w.write_all(b"\n")?;
+            }
+            w.write_all(&contents)?;
             w.write_all(b"}\n")?;
             w.flush()?;
 
             result
         };
 
+        let mut has_imports = false;
         for import in req.imports.drain() {
+            has_imports = true;
             w.write_all(b"import ")?;
             w.write_all(import.full_path().as_bytes())?;
             w.write_all(b";\n")?;
         }
-        w.write_all(b"\n")?;
+
+        if has_imports {
+            w.write_all(b"\n")?;
+        }
 
         w.write_all(&delayed)?;
 

src/gen/java/code.rs

@@ -21,7 +21,8 @@ impl<'m, 'data> GenerateCode<Expression, CodeGenContext<'m, 'data>> for JavaBack
         match input {
             Expression::Comment(it) => self.write_value(lang, ctx, it, w),
             Expression::Super(it) => self.write_value(lang, ctx, it, w),
-            _ => todo!("unimplemented expression"),
+            Expression::EmptyConstructor(_) => Ok(Default::default()),
+            expr => todo!("unimplemented expression: {:?}", expr),
         }
     }
 }
@@ -40,7 +41,9 @@ impl<'m, 'data, B: GeneratorBackend> GenerateCode<EmptySuperCall, CodeGenContext
     }
 }
 
-impl<'m, 'data, B: GeneratorBackend> GenerateCode<InstructionComment, CodeGenContext<'m, 'data>> for B {
+impl<'m, 'data, B: GeneratorBackend> GenerateCode<InstructionComment, CodeGenContext<'m, 'data>>
+    for B
+{
     fn write_value<W: std::io::Write>(
         &self,
         _: &Self::LanguageContext,

src/gen/java/mod.rs

@@ -172,7 +172,7 @@ impl Default for JavaContext {
             header_message: Some(
                 "Generated file - do not edit, your changes will be lost.".to_string(),
             ),
-            indentation: IndentKind::Space(4),
+            indentation: IndentKind::Space(2),
             constant_pool: None,
         }
     }

src/ir/expression.rs

@@ -37,6 +37,7 @@ pub trait CheckExpression {
     ) -> Option<(usize, Expression)>;
 }
 
+#[derive(Debug)]
 pub enum Expression {
     EmptyConstructor(EmptyConstructor),
     ReturnStatement(ReturnStatement),
@@ -64,6 +65,7 @@ impl CheckExpression for InstructionComment {
     }
 }
 
+#[derive(Debug)]
 pub struct EmptyConstructor;
 
 impl CheckExpression for EmptyConstructor {
@@ -91,7 +93,8 @@ impl CheckExpression for EmptyConstructor {
     }
 }
 
-pub struct ReturnStatement; 
+#[derive(Debug)]
+pub struct ReturnStatement;
 
 impl CheckExpression for ReturnStatement {
     fn test<'cp, 'code>(
@@ -112,6 +115,7 @@ impl CheckExpression for ReturnStatement {
     }
 }
 
+#[derive(Debug)]
 pub struct EmptySuperCall;
 
 impl CheckExpression for EmptySuperCall {