Fix clippy warnings; fix class local reference leaks

wuwbobo2021 · Dirbaio · commit 745d02c07e87 · 2025-07-03T23:47:14.000+02:00
diff --git a/java-spaghetti-gen/src/config/runtime.rs b/java-spaghetti-gen/src/config/runtime.rs
@@ -167,7 +167,7 @@ fn expand_vars(string: String) -> String {
             if let Ok(replacement) = std::env::var(segment) {
                 buf.push_str(&replacement[..]);
             } else {
-                println!("cargo:rerun-if-env-changed={}", segment);
+                println!("cargo:rerun-if-env-changed={segment}");
                 buf.push('%');
                 buf.push_str(segment);
                 buf.push('%');
diff --git a/java-spaghetti-gen/src/emit_rust/class_proxy.rs b/java-spaghetti-gen/src/emit_rust/class_proxy.rs
@@ -13,6 +13,7 @@ use crate::emit_rust::fields::emit_rust_type;
 use crate::parser_util::Id;
 
 impl Class {
+    #[allow(clippy::vec_init_then_push)]
     pub(crate) fn write_proxy(&self, context: &Context, methods: &[Method]) -> anyhow::Result<TokenStream> {
         let mut emit_reject_reasons = Vec::new();
 
@@ -204,7 +205,7 @@ fn mangle_native_method(path: &str, name: &str, args: &[FieldDescriptor]) -> Str
     let mut res = String::new();
     res.push_str("Java_");
     res.push_str(&mangle_native(path));
-    res.push_str("_");
+    res.push('_');
     res.push_str(&mangle_native(name));
     res.push_str("__");
     for d in args {
@@ -219,7 +220,7 @@ fn mangle_native(s: &str) -> String {
     for c in s.chars() {
         match c {
             '0'..='9' | 'a'..='z' | 'A'..='Z' => res.push(c),
-            '/' => res.push_str("_"),
+            '/' => res.push('_'),
             '_' => res.push_str("_1"),
             ';' => res.push_str("_2"),
             '[' => res.push_str("_3"),
diff --git a/java-spaghetti-gen/src/emit_rust/classes.rs b/java-spaghetti-gen/src/emit_rust/classes.rs
@@ -114,8 +114,8 @@ impl Class {
         };
 
         let docs = match KnownDocsUrl::from_class(context, self.java.path()) {
-            Some(url) => format!("{} {} {}", visibility, keyword, url),
-            None => format!("{} {} {}", visibility, keyword, self.java.path().as_str()),
+            Some(url) => format!("{visibility} {keyword} {url}"),
+            None => format!("{visibility} {keyword} {}", self.java.path().as_str()),
         };
 
         let rust_name = format_ident!("{}", &self.rust.struct_name);
@@ -235,7 +235,7 @@ impl Class {
 
         out.extend(quote!(impl #rust_name { #contents }));
 
-        if context.proxy_included(&self.java.path().as_str()) {
+        if context.proxy_included(self.java.path().as_str()) {
             out.extend(self.write_proxy(context, &methods)?);
         }
 
diff --git a/java-spaghetti-gen/src/emit_rust/fields.rs b/java-spaghetti-gen/src/emit_rust/fields.rs
@@ -120,7 +120,7 @@ impl<'a> Field<'a> {
         match self.rust_names.as_ref().map_err(|e| anyhow!("bad mangling: {e}"))? {
             FieldMangling::ConstValue(constant, value) => {
                 let constant = format_ident!("{}", constant);
-                let value = emit_constant(&value, descriptor);
+                let value = emit_constant(value, descriptor);
                 let ty = if descriptor.dimensions == 0
                     && let FieldType::Object(cls) = &descriptor.field_type
                     && Id::from(cls).is_string_class()
@@ -208,14 +208,14 @@ pub fn emit_constant(constant: &LiteralConstant<'_>, descriptor: &FieldDescripto
                 let value = *value as i16;
                 quote!(#value)
             }
-            _ => panic!("invalid constant for char {:?}", constant),
+            _ => panic!("invalid constant for char {constant:?}"),
         };
     }
     if descriptor.field_type == FieldType::Boolean && descriptor.dimensions == 0 {
         return match constant {
             LiteralConstant::Integer(0) => quote!(false),
             LiteralConstant::Integer(1) => quote!(true),
-            _ => panic!("invalid constant for boolean {:?}", constant),
+            _ => panic!("invalid constant for boolean {constant:?}"),
         };
     }
 
diff --git a/java-spaghetti-gen/src/emit_rust/methods.rs b/java-spaghetti-gen/src/emit_rust/methods.rs
@@ -147,7 +147,7 @@ impl<'a> Method<'a> {
 
         let docs = match KnownDocsUrl::from_method(context, self) {
             Some(url) => format!("{url}"),
-            None => format!("{}", self.java.name()),
+            None => self.java.name().to_string(),
         };
 
         let throwable = context.throwable_rust_path(mod_);
diff --git a/java-spaghetti-gen/src/emit_rust/mod.rs b/java-spaghetti-gen/src/emit_rust/mod.rs
@@ -52,7 +52,7 @@ impl<'a> Context<'a> {
     pub fn java_to_rust_path(&self, java_class: parser_util::Id, mod_: &str) -> Result<TokenStream, Box<dyn Error>> {
         let m = Class::mod_for(self, java_class)?;
         let s = Class::name_for(self, java_class)?;
-        let fqn = format!("{}::{}", m, s);
+        let fqn = format!("{m}::{s}");
 
         // Calculate relative path from B to A.
         let b: Vec<&str> = mod_.split("::").collect();
diff --git a/java-spaghetti-gen/src/emit_rust/modules.rs b/java-spaghetti-gen/src/emit_rust/modules.rs
@@ -21,14 +21,14 @@ impl Module {
         for (name, module) in self.modules.iter() {
             writeln!(out)?;
 
-            writeln!(out, "pub mod {} {{", name)?;
+            writeln!(out, "pub mod {name} {{")?;
             module.write(context, out)?;
             writeln!(out, "}}")?;
         }
 
         for (_, class) in self.classes.iter() {
             let res = class.write(context)?;
-            out.write(dumb_format(res).as_bytes())?;
+            out.write_all(dumb_format(res).as_bytes())?;
         }
 
         Ok(())
@@ -61,7 +61,7 @@ struct DumbFormatter {
 
 impl DumbFormatter {
     fn newline(&mut self) {
-        self.f.push_str("\n");
+        self.f.push('\n');
         for _ in 0..self.indent {
             self.f.push_str("    ");
         }
@@ -71,7 +71,7 @@ impl DumbFormatter {
 
     fn pre_write(&mut self) {
         if self.space && !self.after_newline {
-            self.f.push_str(" ");
+            self.f.push(' ');
         }
         self.space = false;
         self.after_newline = false;
diff --git a/java-spaghetti-gen/src/run/mod.rs b/java-spaghetti-gen/src/run/mod.rs
@@ -1,5 +1,7 @@
 //! Core generation logic
 
+// XXX: rename this submodule
+#[allow(clippy::module_inception)]
 mod run;
 
 pub use run::run;
diff --git a/java-spaghetti-gen/src/run/run.rs b/java-spaghetti-gen/src/run/run.rs
@@ -76,10 +76,7 @@ fn gather_file(context: &mut emit_rust::Context, path: &Path) -> Result<(), Box<
         unknown => {
             Err(io::Error::new(
                 io::ErrorKind::InvalidInput,
-                format!(
-                    "Input files must have a '.class' or '.jar' extension, not a '.{}' extension",
-                    unknown
-                ),
+                format!("Input files must have a '.class' or '.jar' extension, not a '.{unknown}' extension",),
             ))?;
         }
     }
diff --git a/java-spaghetti-gen/src/util/progress.rs b/java-spaghetti-gen/src/util/progress.rs
@@ -19,7 +19,7 @@ impl Progress {
 
     pub fn force_update(&mut self, msg: &str) {
         self.can_next_log = Instant::now() + self.debounce;
-        println!("{}", msg);
+        println!("{msg}");
     }
 
     pub fn update(&mut self, msg: &str) {
diff --git a/java-spaghetti/Cargo.toml b/java-spaghetti/Cargo.toml
@@ -1,7 +1,7 @@
 [package]
 name = "java-spaghetti"
 version = "0.2.0"
-edition = "2021"
+edition = "2024"
 description = "Glue code to accompany the java-spaghetti code generator for binding to JVM APIs from Rust"
 documentation = "https://docs.rs/java-spaghetti/"
 repository = "https://github.com/Dirbaio/java-spaghetti"
diff --git a/java-spaghetti/src/array.rs b/java-spaghetti/src/array.rs
@@ -34,6 +34,11 @@ where
     /// Uses JNI `GetArrayLength` to get the length of the Java array.
     fn len(self: &Ref<'_, Self>) -> usize;
 
+    /// Uses JNI `GetArrayLength` to get the length of the Java array, returns `true` if it is 0.
+    fn is_empty(self: &Ref<'_, Self>) -> bool {
+        self.len() == 0
+    }
+
     /// Uses JNI `Get{Type}ArrayRegion` to read the contents of the Java array within `[start .. start + elements.len()]`.
     ///
     /// Panics if the index is out of bound.
@@ -100,7 +105,7 @@ macro_rules! primitive_array {
 
         impl PrimitiveArray<$type> for $name {
             fn new<'env>(env: Env<'env>, size: usize) -> Local<'env, Self> {
-                assert!(size <= std::i32::MAX as usize); // jsize == jint == i32
+                assert!(size <= i32::MAX as usize); // jsize == jint == i32
                 let size = size as jsize;
                 let jnienv = env.as_raw();
                 unsafe {
@@ -127,8 +132,8 @@ macro_rules! primitive_array {
             }
 
             fn get_region(self: &Ref<'_, Self>, start: usize, elements: &mut [$type]) {
-                assert!(start <= std::i32::MAX as usize); // jsize == jint == i32
-                assert!(elements.len() <= std::i32::MAX as usize); // jsize == jint == i32
+                assert!(start <= i32::MAX as usize); // jsize == jint == i32
+                assert!(elements.len() <= i32::MAX as usize); // jsize == jint == i32
                 let self_len = self.len() as jsize;
                 let elements_len = elements.len() as jsize;
 
@@ -150,8 +155,8 @@ macro_rules! primitive_array {
             }
 
             fn set_region(self: &Ref<'_, Self>, start: usize, elements: &[$type]) {
-                assert!(start <= std::i32::MAX as usize); // jsize == jint == i32
-                assert!(elements.len() <= std::i32::MAX as usize); // jsize == jint == i32
+                assert!(start <= i32::MAX as usize); // jsize == jint == i32
+                assert!(elements.len() <= i32::MAX as usize); // jsize == jint == i32
                 let self_len = self.len() as jsize;
                 let elements_len = elements.len() as jsize;
 
@@ -210,7 +215,7 @@ unsafe impl<T: ReferenceType, E: ThrowableType> JniType for ObjectArray<T, E> {
 impl<T: ReferenceType, E: ThrowableType> ObjectArray<T, E> {
     /// Uses JNI `NewObjectArray` to create a new Java object array.
     pub fn new<'env>(env: Env<'env>, size: usize) -> Local<'env, Self> {
-        assert!(size <= std::i32::MAX as usize); // jsize == jint == i32
+        assert!(size <= i32::MAX as usize); // jsize == jint == i32
         let class = T::static_with_jni_type(|t| unsafe { env.require_class(t) });
         let size = size as jsize;
 
@@ -221,6 +226,11 @@ impl<T: ReferenceType, E: ThrowableType> ObjectArray<T, E> {
         };
         // Only sane exception here is an OOM exception
         env.exception_check::<E>().map_err(|_| "OOM").unwrap();
+
+        unsafe {
+            let env = env.as_raw();
+            ((**env).v1_2.DeleteLocalRef)(env, class);
+        }
         unsafe { Local::from_raw(env, object) }
     }
 
@@ -235,10 +245,7 @@ impl<T: ReferenceType, E: ThrowableType> ObjectArray<T, E> {
 
     /// Uses JNI `NewObjectArray` to create a new Java object array of the exact size, then sets its items
     /// with the iterator of JNI (null?) references.
-    pub fn new_from<'env>(
-        env: Env<'env>,
-        elements: impl ExactSizeIterator + Iterator<Item = impl AsArg<T>>,
-    ) -> Local<'env, Self> {
+    pub fn new_from<'env>(env: Env<'env>, elements: impl ExactSizeIterator<Item = impl AsArg<T>>) -> Local<'env, Self> {
         let size = elements.len();
         let array = Self::new(env, size);
         let env = array.env().as_raw();
@@ -260,7 +267,7 @@ impl<T: ReferenceType, E: ThrowableType> ObjectArray<T, E> {
     ///
     /// XXX: Expose this via `std::ops::Index`.
     pub fn get<'env>(self: &Ref<'env, Self>, index: usize) -> Result<Option<Local<'env, T>>, Local<'env, E>> {
-        assert!(index <= std::i32::MAX as usize); // jsize == jint == i32 XXX: Should maybe be treated as an exception?
+        assert!(index <= i32::MAX as usize); // jsize == jint == i32 XXX: Should maybe be treated as an exception?
         let index = index as jsize;
         let env = self.env();
         let result = unsafe {
@@ -279,7 +286,7 @@ impl<T: ReferenceType, E: ThrowableType> ObjectArray<T, E> {
     ///
     /// XXX: I don't think there's a way to expose this via `std::ops::IndexMut` sadly?
     pub fn set<'env>(self: &Ref<'env, Self>, index: usize, value: impl AsArg<T>) -> Result<(), Local<'env, E>> {
-        assert!(index <= std::i32::MAX as usize); // jsize == jint == i32 XXX: Should maybe be treated as an exception?
+        assert!(index <= i32::MAX as usize); // jsize == jint == i32 XXX: Should maybe be treated as an exception?
         let index = index as jsize;
         let env = self.env();
         unsafe {
diff --git a/java-spaghetti/src/env.rs b/java-spaghetti/src/env.rs
@@ -1,8 +1,8 @@
 use std::ffi::CStr;
 use std::marker::PhantomData;
 use std::ptr::{self, null_mut};
-use std::sync::atomic::{AtomicPtr, Ordering};
 use std::sync::OnceLock;
+use std::sync::atomic::{AtomicPtr, Ordering};
 
 use jni_sys::*;
 
@@ -69,6 +69,7 @@ pub struct Env<'env> {
 static CLASS_LOADER: AtomicPtr<_jobject> = AtomicPtr::new(null_mut());
 
 #[allow(clippy::missing_safety_doc)]
+#[allow(unsafe_op_in_unsafe_fn)]
 impl<'env> Env<'env> {
     pub unsafe fn from_raw(ptr: *mut JNIEnv) -> Self {
         Self {
@@ -232,7 +233,7 @@ impl<'env> Env<'env> {
         }
 
         // If neither found the class, panic.
-        panic!("couldn't load class {:?}", class);
+        panic!("couldn't load class {class:?}");
     }
 
     unsafe fn require_class_jni(self, class: &CStr) -> jclass {
diff --git a/java-spaghetti/src/jni_type.rs b/java-spaghetti/src/jni_type.rs
@@ -4,6 +4,8 @@ use jni_sys::*;
 
 /// JNI bindings rely on this type being accurate.
 ///
+/// # Safety
+///
 /// **unsafe**: Passing the wrong type can cause unsoundness, since the code that interacts with JNI blindly trusts it's correct.
 ///
 /// Why the awkward callback style instead of returning `&'static CStr`?  Arrays of arrays may need to dynamically
diff --git a/java-spaghetti/src/lib.rs b/java-spaghetti/src/lib.rs
@@ -62,6 +62,7 @@ pub trait ThrowableType: ReferenceType {}
 
 /// You should generally not be interacting with this type directly, but it must be public for codegen.
 #[doc(hidden)]
+#[warn(clippy::missing_safety_doc)]
 pub unsafe trait ReferenceType: JniType + Sized + 'static {}
 
 /// Marker trait indicating `Self` can be assigned to `T`.
diff --git a/java-spaghetti/src/refs/arg.rs b/java-spaghetti/src/refs/arg.rs
@@ -42,7 +42,7 @@ impl<T: ReferenceType> Arg<T> {
         if self.object.is_null() {
             None
         } else {
-            Some(Ref::from_raw(env, self.object))
+            Some(unsafe { Ref::from_raw(env, self.object) })
         }
     }
 
@@ -52,7 +52,7 @@ impl<T: ReferenceType> Arg<T> {
     /// the intended use case of immediately converting any [Arg] into [Local] at the start of a JNI callback,
     /// where Java directly invoked your function with an [Env] + arguments, is sound.
     pub unsafe fn into_local<'env>(self, env: Env<'env>) -> Option<Local<'env, T>> {
-        self.into_ref(env).map(|r| r.as_local())
+        unsafe { self.into_ref(env) }.map(|r| r.as_local())
     }
 
     /// This equals [Arg::into_ref] + [Ref::as_global].
@@ -61,6 +61,6 @@ impl<T: ReferenceType> Arg<T> {
     ///
     /// **unsafe**:  The same as [Arg::into_ref].
     pub unsafe fn into_global(self, env: Env) -> Option<Global<T>> {
-        self.into_ref(env).as_ref().map(Ref::as_global)
+        unsafe { self.into_ref(env) }.as_ref().map(Ref::as_global)
     }
 }
diff --git a/java-spaghetti/src/refs/local.rs b/java-spaghetti/src/refs/local.rs
@@ -43,7 +43,7 @@ impl<'env, T: ReferenceType> Local<'env, T> {
     /// - `object` references an instance of type `T`.
     pub unsafe fn from_raw(env: Env<'env>, object: jobject) -> Self {
         Self {
-            ref_: Ref::from_raw(env, object),
+            ref_: unsafe { Ref::from_raw(env, object) },
         }
     }
 
diff --git a/java-spaghetti/src/refs/ref_.rs b/java-spaghetti/src/refs/ref_.rs
@@ -88,10 +88,12 @@ impl<'env, T: ReferenceType> Ref<'env, T> {
         let env = self.env();
         let jnienv = env.as_raw();
         let class = U::static_with_jni_type(|t| unsafe { env.require_class(t) });
-        if !unsafe { ((**jnienv).v1_2.IsInstanceOf)(jnienv, self.as_raw(), class) } {
-            return Err(crate::CastError);
-        }
-        Ok(())
+        let assignable = unsafe {
+            let ret = ((**jnienv).v1_2.IsInstanceOf)(jnienv, self.as_raw(), class);
+            ((**jnienv).v1_2.DeleteLocalRef)(jnienv, class);
+            ret
+        };
+        if assignable { Ok(()) } else { Err(crate::CastError) }
     }
 
     /// Casts itself to a JNI reference of type `U` forcefully, without the cost of runtime checking.
@@ -100,7 +102,7 @@ impl<'env, T: ReferenceType> Ref<'env, T> {
     ///
     /// - `self` references an instance of type `U`.
     pub unsafe fn cast_unchecked<U: ReferenceType>(self) -> Ref<'env, U> {
-        transmute(self)
+        unsafe { transmute(self) }
     }
 
     /// Tries to cast itself to a JNI reference of type `U`.
@@ -123,7 +125,7 @@ impl<'env, T: ReferenceType> Ref<'env, T> {
     ///
     /// - `self` references an instance of type `U`.
     pub unsafe fn cast_ref_unchecked<U: ReferenceType>(&self) -> &Ref<'env, U> {
-        transmute(self)
+        unsafe { transmute(self) }
     }
 
     /// Tries to cast the borrowed `Ref` to a JNI reference of type `U`.
diff --git a/java-spaghetti/src/string_chars.rs b/java-spaghetti/src/string_chars.rs
@@ -25,8 +25,8 @@ impl<'env> StringChars<'env> {
     pub unsafe fn from_env_jstring(env: Env<'env>, string: jstring) -> Self {
         debug_assert!(!string.is_null());
 
-        let chars = env.get_string_chars(string);
-        let length = env.get_string_length(string);
+        let chars = unsafe { env.get_string_chars(string) };
+        let length = unsafe { env.get_string_length(string) };
 
         Self {
             env,
@@ -42,7 +42,7 @@ impl<'env> StringChars<'env> {
     }
 
     /// [std::char::decode_utf16]\(...\)s these string characters.
-    pub fn decode(&self) -> char::DecodeUtf16<iter::Cloned<slice::Iter<u16>>> {
+    pub fn decode(&self) -> char::DecodeUtf16<iter::Cloned<slice::Iter<'_, u16>>> {
         char::decode_utf16(self.chars().iter().cloned())
     }
 
diff --git a/java-spaghetti/src/vm.rs b/java-spaghetti/src/vm.rs

Original file line number	Diff line number	Diff line change
`@@ -76,10 +76,7 @@ fn gather_file(context: &mut emit_rust::Context, path: &Path) -> Result<(), Box<`
`76`	`76`	`unknown => {`
`77`	`77`	`Err(io::Error::new(`
`78`	`78`	`io::ErrorKind::InvalidInput,`
`79`		`- format!(`
`80`		`- "Input files must have a '.class' or '.jar' extension, not a '.{}' extension",`
`81`		`- unknown`
`82`		`- ),`
	`79`	`+ format!("Input files must have a '.class' or '.jar' extension, not a '.{unknown}' extension",),`
`83`	`80`	`))?;`
`84`	`81`	`}`
`85`	`82`	`}`
Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,7 @@ impl Progress {`
`19`	`19`
`20`	`20`	`pub fn force_update(&mut self, msg: &str) {`
`21`	`21`	`self.can_next_log = Instant::now() + self.debounce;`
`22`		`- println!("{}", msg);`
	`22`	`+ println!("{msg}");`
`23`	`23`	`}`
`24`	`24`
`25`	`25`	`pub fn update(&mut self, msg: &str) {`