Apply cleanups suggested by clippy and plug it into CI (messense#111)

Author: itsjunetime

.github/workflows/CI.yml

@@ -23,7 +23,7 @@ jobs:
           fetch-depth: 500
       - uses: dtolnay/rust-toolchain@stable
       - run: sudo apt-get -y install libfontconfig1-dev
-      - run: cargo check
+      - run: cargo clippy --tests -- -D warnings
 
   test:
     name: Test Suite

src/buffer.rs

@@ -2,6 +2,7 @@ use std::convert::TryFrom;
 use std::ffi::CString;
 use std::io;
 use std::ptr;
+use std::str::FromStr;
 
 use mupdf_sys::*;
 
@@ -15,6 +16,15 @@ pub struct Buffer {
     offset: usize,
 }
 
+impl FromStr for Buffer {
+    type Err = Error;
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        let c_str = CString::new(s)?;
+        let inner = unsafe { ffi_try!(mupdf_buffer_from_str(context(), c_str.as_ptr())) };
+        Ok(Self { inner, offset: 0 })
+    }
+}
+
 impl Buffer {
     pub(crate) unsafe fn from_raw(ptr: *mut fz_buffer) -> Self {
         Self {
@@ -27,12 +37,6 @@ impl Buffer {
         Self::with_capacity(0)
     }
 
-    pub fn from_str(str: &str) -> Result<Self, Error> {
-        let c_str = CString::new(str)?;
-        let inner = unsafe { ffi_try!(mupdf_buffer_from_str(context(), c_str.as_ptr())) };
-        Ok(Self { inner, offset: 0 })
-    }
-
     pub fn from_base64(str: &str) -> Result<Self, Error> {
         let c_str = CString::new(str)?;
         let inner = unsafe { ffi_try!(mupdf_buffer_from_base64(context(), c_str.as_ptr())) };
@@ -54,6 +58,10 @@ impl Buffer {
         unsafe { fz_buffer_storage(context(), self.inner, ptr::null_mut()) }
     }
 
+    pub fn is_empty(&self) -> bool {
+        self.len() == 0
+    }
+
     pub fn into_inner(mut self) -> *mut fz_buffer {
         let inner = self.inner;
         self.inner = ptr::null_mut();
@@ -152,7 +160,10 @@ impl TryFrom<Vec<u8>> for Buffer {
 #[cfg(test)]
 mod test {
     use super::Buffer;
-    use std::io::{Read, Write};
+    use std::{
+        io::{Read, Write},
+        str::FromStr,
+    };
 
     #[test]
     fn test_buffer_len() {
@@ -167,7 +178,7 @@ mod test {
         assert_eq!(n, 3);
 
         let mut output = [0; 3];
-        buf.read(&mut output).unwrap();
+        buf.read_exact(&mut output).unwrap();
         assert_eq!(output, [97, 98, 99]);
     }
 
@@ -225,15 +236,15 @@ mod test {
     fn test_buffer_from_str() {
         let mut buf = Buffer::from_str("abc").unwrap();
         let mut output = [0; 3];
-        buf.read(&mut output).unwrap();
+        buf.read_exact(&mut output).unwrap();
         assert_eq!(output, [97, 98, 99]);
     }
 
     #[test]
     fn test_buffer_from_base64() {
         let mut buf = Buffer::from_base64("YWJj").unwrap();
         let mut output = [0; 3];
-        buf.read(&mut output).unwrap();
+        buf.read_exact(&mut output).unwrap();
         assert_eq!(output, [97, 98, 99]);
     }
 }

src/context.rs

@@ -28,7 +28,7 @@ static BASE_CONTEXT: Lazy<Mutex<BaseContext>> = Lazy::new(|| {
 });
 
 thread_local! {
-    static LOCAL_CONTEXT: RefCell<RawContext> = RefCell::new(RawContext(ptr::null_mut()));
+    static LOCAL_CONTEXT: RefCell<RawContext> = const { RefCell::new(RawContext(ptr::null_mut())) };
 }
 
 #[derive(Debug)]

src/device.rs

@@ -76,6 +76,7 @@ impl Device {
         })
     }
 
+    #[allow(clippy::too_many_arguments)]
     pub fn fill_path(
         &self,
         path: &Path,
@@ -102,6 +103,7 @@ impl Device {
         Ok(())
     }
 
+    #[allow(clippy::too_many_arguments)]
     pub fn stroke_path(
         &self,
         path: &Path,
@@ -183,6 +185,7 @@ impl Device {
         Ok(())
     }
 
+    #[allow(clippy::too_many_arguments)]
     pub fn stroke_text(
         &self,
         text: &Text,

src/document.rs

@@ -66,7 +66,7 @@ impl Document {
         let c_magic = CString::new(magic)?;
         let len = bytes.len();
         let mut buf = Buffer::with_capacity(len);
-        buf.write(bytes)?;
+        buf.write_all(bytes)?;
         let inner = unsafe {
             ffi_try!(mupdf_open_document_from_bytes(
                 context(),
@@ -313,7 +313,7 @@ pub struct PageIter<'a> {
     doc: &'a Document,
 }
 
-impl<'a> Iterator for PageIter<'a> {
+impl Iterator for PageIter<'_> {
     type Item = Result<Page, Error>;
 
     fn next(&mut self) -> Option<Self::Item> {

src/error.rs

@@ -22,6 +22,11 @@ impl fmt::Display for MuPdfError {
 
 impl std::error::Error for MuPdfError {}
 
+/// # Safety
+///
+/// * `error` must point to a non-null, well-aligned initialized instance of `mupdf_error_t`.
+///
+/// * `(*error).message` must point to a null-terminated c-string.
 pub unsafe fn ffi_error(err: *mut mupdf_error_t) -> MuPdfError {
     use std::ffi::CStr;
 

src/pdf/document.rs

@@ -253,7 +253,7 @@ impl PdfDocument {
     pub fn from_bytes(bytes: &[u8]) -> Result<Self, Error> {
         let len = bytes.len();
         let mut buf = Buffer::with_capacity(len);
-        buf.write(bytes)?;
+        buf.write_all(bytes)?;
         unsafe {
             let inner = ffi_try!(mupdf_pdf_open_document_from_bytes(context(), buf.inner));
             Ok(Self::from_raw(inner))
@@ -645,7 +645,7 @@ impl PdfDocument {
 
         let mut refs = refs.into_iter();
         let first = refs.next().unwrap();
-        let last = refs.last().unwrap_or_else(|| first.clone());
+        let last = refs.next_back().unwrap_or_else(|| first.clone());
 
         parent.dict_put("First", first)?;
         parent.dict_put("Last", last)?;

src/pdf/object.rs

@@ -3,6 +3,7 @@ use std::ffi::{CStr, CString};
 use std::fmt;
 use std::io::{self, BufReader, Read, Write};
 use std::slice;
+use std::str::FromStr;
 
 use mupdf_sys::*;
 
@@ -317,6 +318,7 @@ impl PdfObject {
         Ok(Some(Self { inner }))
     }
 
+    #[allow(clippy::len_without_is_empty)]
     pub fn len(&self) -> Result<usize, Error> {
         let size = unsafe { ffi_try!(mupdf_pdf_array_len(context(), self.inner)) };
         Ok(size as usize)

src/stroke_state.rs

@@ -41,6 +41,7 @@ pub struct StrokeState {
 }
 
 impl StrokeState {
+    #[allow(clippy::too_many_arguments)]
     pub fn new(
         start_cap: LineCap,
         dash_cap: LineCap,

src/system_font.rs

@@ -1,4 +1,3 @@
-use std::convert::TryFrom;
 use std::ffi::CStr;
 use std::os::raw::{c_char, c_int};
 use std::ptr;
@@ -12,7 +11,7 @@ use num_enum::TryFromPrimitive;
 use crate::font::Font;
 use mupdf_sys::*;
 
-pub unsafe extern "C" fn load_system_font(
+pub(crate) unsafe extern "C" fn load_system_font(
     ctx: *mut fz_context,
     name: *const c_char,
     bold: c_int,
@@ -57,17 +56,14 @@ pub unsafe extern "C" fn load_system_font(
                 ),
                 Err(_) => return ptr::null_mut(),
             };
-            match font {
-                Ok(font) => {
-                    if needs_exact_metrics == 1
-                        && ((bold == 1 && !font.is_bold()) || (italic == 1 && !font.is_italic()))
-                    {
-                        return ptr::null_mut();
-                    }
-                    fz_keep_font(ctx, font.inner);
-                    return font.inner;
+            if let Ok(font) = font {
+                if needs_exact_metrics == 1
+                    && ((bold == 1 && !font.is_bold()) || (italic == 1 && !font.is_italic()))
+                {
+                    return ptr::null_mut();
                 }
-                Err(_) => {}
+                fz_keep_font(ctx, font.inner);
+                return font.inner;
             }
         }
     }
@@ -76,6 +72,7 @@ pub unsafe extern "C" fn load_system_font(
 
 #[derive(TryFromPrimitive)]
 #[repr(u32)]
+#[allow(clippy::enum_variant_names)]
 enum Ordering {
     AdobeCns = FZ_ADOBE_CNS as u32,
     AdobeGb = FZ_ADOBE_GB as u32,
@@ -146,38 +143,17 @@ pub unsafe extern "C" fn load_system_cjk_font(
 }
 
 #[cfg(not(windows))]
-pub unsafe extern "C" fn load_system_cjk_font(
+pub(crate) unsafe extern "C" fn load_system_cjk_font(
     ctx: *mut fz_context,
     name: *const c_char,
-    ordering: c_int,
-    serif: c_int,
+    _ordering: c_int,
+    _serif: c_int,
 ) -> *mut fz_font {
     // Try name first
-    let font = load_system_font(ctx, name, 0, 0, 0);
-    if !font.is_null() {
-        return font;
-    }
-    if serif == 1 {
-        match Ordering::try_from(ordering as u32) {
-            Ok(Ordering::AdobeCns) => {}
-            Ok(Ordering::AdobeGb) => {}
-            Ok(Ordering::AdobeJapan) => {}
-            Ok(Ordering::AdobeKorea) => {}
-            Err(_) => {}
-        }
-    } else {
-        match Ordering::try_from(ordering as u32) {
-            Ok(Ordering::AdobeCns) => {}
-            Ok(Ordering::AdobeGb) => {}
-            Ok(Ordering::AdobeJapan) => {}
-            Ok(Ordering::AdobeKorea) => {}
-            Err(_) => {}
-        }
-    }
-    ptr::null_mut()
+    load_system_font(ctx, name, 0, 0, 0)
 }
 
-pub unsafe extern "C" fn load_system_fallback_font(
+pub(crate) unsafe extern "C" fn load_system_fallback_font(
     _ctx: *mut fz_context,
     _script: c_int,
     _language: c_int,