Merge pull request #286 from image-rs/clippy

Resolve all the clippy lints and add it to CI

.github/workflows/rust.yml

@@ -45,6 +45,10 @@ jobs:
     - name: Cargo check
       run: cargo check --features "${{ matrix.feature }}" --no-default-features
 
+    - name: Cargo clippy
+      if: ${{ matrix.feature != '' }}
+      run: cargo clippy --no-deps --all-features --all-targets -- -D warnings
+
   other_archs:
     # github actions does not support 32-bit or big endian systems directly, but
     # it does support QEMU. so we install qemu, then build and run the tests on

src/bytecast.rs

@@ -10,6 +10,9 @@
 //! the unsafe code guidelines).
 //!
 //! TODO: Would like to use std-lib here.
+// Until we implement predictors for f16, we don't need f16_as_ne_bytes. (Due to the macro we do
+// not apply this directly to the functions). And rust-version is not new enough for `expect`.
+#![allow(dead_code)]
 use std::{mem, slice};
 
 use half::f16;

src/decoder/ifd.rs

@@ -445,7 +445,7 @@ impl Entry {
             // 2b: the value is at most 4 bytes or doesn't fit in the offset field.
             return Ok(match self.type_ {
                 Type::BYTE => Byte(self.offset[0]),
-                Type::SBYTE => SignedByte(i8::from(self.offset[0] as i8)),
+                Type::SBYTE => SignedByte(self.offset[0] as i8),
                 Type::UNDEFINED => Byte(self.offset[0]),
                 Type::SHORT => Short(self.r(bo).read_u16()?),
                 Type::SSHORT => SignedShort(self.r(bo).read_i16()?),

src/decoder/image.rs

@@ -29,10 +29,10 @@ pub(crate) struct TileAttributes {
 
 impl TileAttributes {
     pub fn tiles_across(&self) -> usize {
-        (self.image_width + self.tile_width - 1) / self.tile_width
+        self.image_width.div_ceil(self.tile_width)
     }
     pub fn tiles_down(&self) -> usize {
-        (self.image_height + self.tile_length - 1) / self.tile_length
+        self.image_height.div_ceil(self.tile_length)
     }
     fn padding_right(&self) -> usize {
         (self.tile_width - self.image_width % self.tile_width) % self.tile_width
@@ -625,12 +625,12 @@ impl Image {
         let chunk_row_bits = (u64::from(chunk_dims.0) * u64::from(self.bits_per_sample))
             .checked_mul(samples as u64)
             .ok_or(TiffError::LimitsExceeded)?;
-        let chunk_row_bytes: usize = ((chunk_row_bits + 7) / 8).try_into()?;
+        let chunk_row_bytes: usize = chunk_row_bits.div_ceil(8).try_into()?;
 
         let data_row_bits = (u64::from(data_dims.0) * u64::from(self.bits_per_sample))
             .checked_mul(samples as u64)
             .ok_or(TiffError::LimitsExceeded)?;
-        let data_row_bytes: usize = ((data_row_bits + 7) / 8).try_into()?;
+        let data_row_bytes: usize = data_row_bits.div_ceil(8).try_into()?;
 
         // TODO: Should these return errors instead?
         assert!(output_row_stride >= data_row_bytes);
@@ -643,11 +643,11 @@ impl Image {
             self.jpeg_tables.as_deref().map(|a| &**a),
         )?;
 
-        if output_row_stride == chunk_row_bytes as usize {
+        if output_row_stride == chunk_row_bytes {
             let tile = &mut buf[..chunk_row_bytes * data_dims.1 as usize];
             reader.read_exact(tile)?;
 
-            for row in tile.chunks_mut(chunk_row_bytes as usize) {
+            for row in tile.chunks_mut(chunk_row_bytes) {
                 super::fix_endianness_and_predict(
                     row,
                     color_type.bit_depth(),
@@ -678,11 +678,7 @@ impl Image {
                 }
             }
         } else {
-            for (_i, row) in buf
+            for row in buf.chunks_mut(output_row_stride).take(data_dims.1 as usize) {
-                .chunks_mut(output_row_stride)
-                .take(data_dims.1 as usize)
-                .enumerate()
-            {
                 let row = &mut row[..data_row_bytes];
                 reader.read_exact(row)?;
 

src/decoder/mod.rs

@@ -206,6 +206,7 @@ pub enum ChunkType {
 
 /// Decoding limits
 #[derive(Clone, Debug)]
+#[non_exhaustive]
 pub struct Limits {
     /// The maximum size of any `DecodingResult` in bytes, the default is
     /// 256MiB. If the entire image is decoded at once, then this will
@@ -218,10 +219,6 @@ pub struct Limits {
     /// Maximum size for intermediate buffer which may be used to limit the amount of data read per
     /// segment even if the entire image is decoded at once.
     pub intermediate_buffer_size: usize,
-    /// The purpose of this is to prevent all the fields of the struct from
-    /// being public, as this would make adding new fields a major version
-    /// bump.
-    _non_exhaustive: (),
 }
 
 impl Limits {
@@ -234,10 +231,9 @@ impl Limits {
     /// naturally, the machine running the program does not have infinite memory.
     pub fn unlimited() -> Limits {
         Limits {
-            decoding_buffer_size: usize::max_value(),
+            decoding_buffer_size: usize::MAX,
-            ifd_value_size: usize::max_value(),
+            ifd_value_size: usize::MAX,
-            intermediate_buffer_size: usize::max_value(),
+            intermediate_buffer_size: usize::MAX,
-            _non_exhaustive: (),
         }
     }
 }
@@ -248,7 +244,6 @@ impl Default for Limits {
             decoding_buffer_size: 256 * 1024 * 1024,
             intermediate_buffer_size: 128 * 1024 * 1024,
             ifd_value_size: 1024 * 1024,
-            _non_exhaustive: (),
         }
     }
 }
@@ -861,7 +856,8 @@ impl<R: Read + Seek> Decoder<R> {
         let row_samples = if bits_per_sample >= 8 {
             width
         } else {
-            ((((width as u64) * bits_per_sample as u64) + 7) / 8)
+            ((width as u64) * bits_per_sample as u64)
+                .div_ceil(8)
                 .try_into()
                 .map_err(|_| TiffError::LimitsExceeded)?
         };
@@ -960,12 +956,12 @@ impl<R: Read + Seek> Decoder<R> {
         let output_row_bits = (width as u64 * self.image.bits_per_sample as u64)
             .checked_mul(samples as u64)
             .ok_or(TiffError::LimitsExceeded)?;
-        let output_row_stride: usize = ((output_row_bits + 7) / 8).try_into()?;
+        let output_row_stride: usize = output_row_bits.div_ceil(8).try_into()?;
 
         let chunk_row_bits = (chunk_dimensions.0 as u64 * self.image.bits_per_sample as u64)
             .checked_mul(samples as u64)
             .ok_or(TiffError::LimitsExceeded)?;
-        let chunk_row_bytes: usize = ((chunk_row_bits + 7) / 8).try_into()?;
+        let chunk_row_bytes: usize = chunk_row_bits.div_ceil(8).try_into()?;
 
         let chunks_across = ((width - 1) / chunk_dimensions.0 + 1) as usize;
 

src/decoder/stream.rs

@@ -135,20 +135,17 @@ impl<R: Read> EndianReader<R> {
     }
 }
 
-///
+//
-/// # READERS
+// # READERS
-///
+//
-
-///
-/// ## Deflate Reader
-///
 
+/// Type alias for the deflate Reader
 #[cfg(feature = "deflate")]
 pub type DeflateReader<R> = flate2::read::ZlibDecoder<R>;
 
-///
+//
-/// ## LZW Reader
+// ## LZW Reader
-///
+//
 
 /// Reader that decompresses LZW streams
 #[cfg(feature = "lzw")]
@@ -207,10 +204,11 @@ impl<R: Read> Read for LZWReader<R> {
     }
 }
 
-///
+//
-/// ## PackBits Reader
+// ## PackBits Reader
-///
+//
 
+/// Internal state machine for the PackBitsReader.
 enum PackBitsReaderState {
     Header,
     Literal,

src/directory.rs

@@ -27,6 +27,8 @@ pub struct Directory {
 }
 
 impl Directory {
+    /// Create a directory in an initial state without entries. Note that an empty directory can
+    /// not be encoded in a file, it must contain at least one entry.
     pub fn empty() -> Self {
         Directory {
             entries: BTreeMap::new(),
@@ -72,6 +74,12 @@ impl Directory {
         self.entries.len()
     }
 
+    /// Check if there are any entries in this directory. Note that an empty directory can not be
+    /// encoded in the file, it must contain at least one entry.
+    pub fn is_empty(&self) -> bool {
+        self.entries.is_empty()
+    }
+
     /// Get the pointer to the next IFD, if it was defined.
     pub fn next(&self) -> Option<IfdPointer> {
         self.next_ifd.map(|n| IfdPointer(n.get()))

src/encoder/compression/deflate.rs

@@ -11,21 +11,17 @@ pub struct Deflate {
 /// It allows trading compression ratio for compression speed.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
 #[non_exhaustive]
+#[derive(Default)]
 pub enum DeflateLevel {
     /// The fastest possible compression mode.
     Fast = 1,
     /// The conserative choice between speed and ratio.
+    #[default]
     Balanced = 6,
     /// The best compression available with Deflate.
     Best = 9,
 }
 
-impl Default for DeflateLevel {
-    fn default() -> Self {
-        DeflateLevel::Balanced
-    }
-}
-
 impl Deflate {
     /// Create a new deflate compressor with a specific level of compression.
     pub fn with_level(level: DeflateLevel) -> Self {

src/encoder/mod.rs

@@ -296,7 +296,7 @@ impl<'a, W: 'a + Write + Seek, K: TiffKind> DirectoryEncoder<'a, W, K> {
         }
 
         let entry = Self::write_value(
-            &mut self.writer,
+            self.writer,
             &DirectoryEntry {
                 data_type: <T>::FIELD_TYPE,
                 count: value.count().try_into()?,
@@ -370,14 +370,14 @@ impl<'a, W: 'a + Write + Seek, K: TiffKind> DirectoryEncoder<'a, W, K> {
 
         if bytes.len() > in_entry_bytes {
             let offset = writer.offset();
-            writer.write_bytes(&bytes)?;
+            writer.write_bytes(bytes)?;
 
             let offset = K::convert_offset(offset)?;
             offset_bytes[..offset.bytes()].copy_from_slice(&offset.data());
         } else {
             // Note: we have indicated our native byte order in the header, hence this
             // corresponds to our byte order no matter the value type.
-            offset_bytes[..bytes.len()].copy_from_slice(&bytes);
+            offset_bytes[..bytes.len()].copy_from_slice(bytes);
         }
 
         // Undoing some hidden type. Offset is either u32 or u64. Due to the trait API being public
@@ -532,11 +532,11 @@ impl<'a, W: 'a + Write + Seek, T: ColorType, K: TiffKind> ImageEncoder<'a, W, T,
         let rows_per_strip = {
             match compression.tag() {
                 CompressionMethod::PackBits => 1, // Each row must be packed separately. Do not compress across row boundaries
-                _ => (1_000_000 + row_bytes - 1) / row_bytes,
+                _ => 1_000_000_u64.div_ceil(row_bytes),
             }
         };
 
-        let strip_count = (u64::from(height) + rows_per_strip - 1) / rows_per_strip;
+        let strip_count = u64::from(height).div_ceil(rows_per_strip);
 
         encoder.write_tag(Tag::ImageWidth, width)?;
         encoder.write_tag(Tag::ImageLength, height)?;
@@ -705,7 +705,7 @@ impl<'a, W: 'a + Write + Seek, T: ColorType, K: TiffKind> ImageEncoder<'a, W, T,
         self.encoder.write_tag(Tag::RowsPerStrip, value)?;
 
         let value: u64 = value as u64;
-        self.strip_count = (self.height as u64 + value - 1) / value;
+        self.strip_count = (self.height as u64).div_ceil(value);
         self.rows_per_strip = value;
 
         Ok(())

src/encoder/tiff_value.rs

@@ -450,7 +450,7 @@ impl TiffValue for str {
     }
 }
 
-impl<'a, T: TiffValue + ?Sized> TiffValue for &'a T {
+impl<T: TiffValue + ?Sized> TiffValue for &'_ T {
     const BYTE_LEN: u8 = T::BYTE_LEN;
     const FIELD_TYPE: Type = T::FIELD_TYPE;
 

src/tags.rs

@@ -2,7 +2,7 @@ macro_rules! tags {
     {
         // Permit arbitrary meta items, which include documentation.
         $( #[$enum_attr:meta] )*
-        $vis:vis enum $name:ident($ty:tt) $(unknown($unknown_doc:literal))* {
+        $vis:vis enum $name:ident($ty:tt) $(unknown(#[$unknown_meta:meta] $unknown_doc:ident))* {
             // Each of the `Name = Val,` permitting documentation.
             $($(#[$ident_attr:meta])* $tag:ident = $val:expr,)*
         }
@@ -13,7 +13,7 @@ macro_rules! tags {
         pub enum $name {
             $($(#[$ident_attr])* $tag,)*
             $(
-                #[doc = $unknown_doc]
+                #[$unknown_meta]
                 Unknown($ty),
             )*
         }
@@ -31,7 +31,7 @@ macro_rules! tags {
             fn __to_inner_type(&self) -> $ty {
                 match *self {
                     $( $name::$tag => $val, )*
-                    $( $name::Unknown(n) => { $unknown_doc; n }, )*
+                    $( $name::Unknown($unknown_doc) => { $unknown_doc }, )*
                 }
             }
         }
@@ -39,7 +39,7 @@ macro_rules! tags {
         tags!($name, $ty, $($unknown_doc)*);
     };
     // For u16 tags, provide direct inherent primitive conversion methods.
-    ($name:tt, u16, $($unknown_doc:literal)*) => {
+    ($name:tt, u16, $($unknown_doc:ident)*) => {
         impl $name {
             #[inline(always)]
             pub fn from_u16(val: u16) -> Option<Self> {
@@ -48,9 +48,8 @@ macro_rules! tags {
 
             $(
             #[inline(always)]
-            pub fn from_u16_exhaustive(val: u16) -> Self {
+            pub fn from_u16_exhaustive($unknown_doc: u16) -> Self {
-                $unknown_doc;
+                Self::__from_inner_type($unknown_doc).unwrap_or_else(|_| $name::Unknown($unknown_doc))
-                Self::__from_inner_type(val).unwrap_or_else(|_| $name::Unknown(val))
             }
             )*
 
@@ -68,7 +67,10 @@ macro_rules! tags {
 // Note: These tags appear in the order they are mentioned in the TIFF reference
 tags! {
 /// TIFF tags
-pub enum Tag(u16) unknown("A private or extension tag") {
+pub enum Tag(u16) unknown(
+    /// A private or extension tag
+    unknown
+) {
     // Baseline tags:
     Artist = 315,
     // grayscale images PhotometricInterpretation 1 or 3
@@ -192,7 +194,10 @@ pub enum Type(u16) {
 tags! {
 /// See [TIFF compression tags](https://www.awaresystems.be/imaging/tiff/tifftags/compression.html)
 /// for reference.
-pub enum CompressionMethod(u16) unknown("A custom compression method") {
+pub enum CompressionMethod(u16) unknown(
+    /// A custom compression method
+    unknown
+) {
     None = 1,
     Huffman = 2,
     Fax3 = 3,
@@ -254,7 +259,10 @@ pub enum ResolutionUnit(u16) {
 }
 
 tags! {
-pub enum SampleFormat(u16) unknown("An unknown extension sample format") {
+pub enum SampleFormat(u16) unknown(
+    /// An unknown extension sample format
+    unknown
+) {
     Uint = 1,
     Int = 2,
     IEEEFP = 3,

tests/decode_images.rs

@@ -45,12 +45,12 @@ fn test_image_color_type_unsupported(file: &str, expected_type: ColorType) {
     let img_file = File::open(path).expect("Cannot find test image!");
     let mut decoder = Decoder::new(img_file).expect("Cannot create decoder");
     assert_eq!(decoder.colortype().unwrap(), expected_type);
-    assert!(match decoder.read_image() {
+    assert!(matches!(
+        decoder.read_image(),
         Err(tiff::TiffError::UnsupportedError(
             tiff::TiffUnsupportedError::UnsupportedColorType(_),
-        )) => true,
+        ))
-        _ => false,
+    ));
-    });
 }
 
 #[test]
@@ -172,7 +172,7 @@ fn test_string_tags() {
                 &s,
                 "GraphicsMagick 1.2 unreleased Q16 http://www.GraphicsMagick.org/"
             ),
-            _ => assert!(false),
+            other => unreachable!("Expected Ascii tag from file, got {other:?}"),
         };
     }
 }
@@ -348,7 +348,7 @@ fn test_div_zero() {
 
     match err {
         TiffError::FormatError(TiffFormatError::StripTileTagConflict) => {}
-        unexpected => panic!("Unexpected error {}", unexpected),
+        unexpected => panic!("Unexpected error {unexpected}"),
     }
 }
 
@@ -366,7 +366,7 @@ fn test_too_many_value_bytes() {
 
     match error {
         tiff::TiffError::LimitsExceeded => {}
-        unexpected => panic!("Unexpected error {}", unexpected),
+        unexpected => panic!("Unexpected error {unexpected}"),
     }
 }
 
@@ -497,7 +497,7 @@ fn timeout() {
 
     match error {
         TiffError::FormatError(TiffFormatError::CycleInOffsets) => {}
-        e => panic!("Unexpected error {:?}", e),
+        e => panic!("Unexpected error {e:?}"),
     }
 }
 

tests/encode_images.rs

@@ -621,7 +621,7 @@ fn test_rows_per_strip() {
             let img2 = [i; 2 * 100];
             match decoder.read_chunk(i as u32).unwrap() {
                 DecodingResult::U8(data) => assert_eq!(&img2[..], &data[..]),
-                other => panic!("Incorrect strip type {:?}", other),
+                other => panic!("Incorrect strip type {other:?}"),
             }
         }
     }
@@ -652,8 +652,6 @@ fn test_auxiliary_directory() {
         encoder.write_tag(Tag::ExifDirectory, exif.offset).unwrap();
     }
 
-    drop(tiff);
-
     data.set_position(0);
     let mut decoder = Decoder::new(&mut data).unwrap();
 

tests/encode_images_with_compression.rs

@@ -116,7 +116,7 @@ fn encode_decode_with_compression(compression: Compression) {
         assert_eq!(
             match decoder.read_image() {
                 Ok(DecodingResult::U16(image_data)) => image_data,
-                unexpected => panic!("Descoding RGB failed: {:?}", unexpected),
+                unexpected => panic!("Descoding RGB failed: {unexpected:?}"),
             },
             image_rgb.reference_data()
         );
@@ -126,7 +126,7 @@ fn encode_decode_with_compression(compression: Compression) {
         assert_eq!(
             match decoder.read_image() {
                 Ok(DecodingResult::U8(image_data)) => image_data,
-                unexpected => panic!("Decoding grayscale failed: {:?}", unexpected),
+                unexpected => panic!("Decoding grayscale failed: {unexpected:?}"),
             },
             image_grayscale.reference_data()
         );

tests/predict.rs

@@ -1,7 +1,7 @@
 extern crate tiff;
 
 use tiff::decoder::{Decoder, DecodingResult};
-use tiff::encoder::{colortype, Compression, Predictor, TiffEncoder};
+use tiff::encoder::{colortype, Predictor, TiffEncoder};
 use tiff::ColorType;
 
 use std::fs::File;