Add clippy and fix reported issues

Signed-off-by: Wiktor Kwapisiewicz <wiktor@metacode.biz>

.github/workflows/rust.yml

@@ -47,6 +47,8 @@ jobs:
         shell: bash
       - run: ci/fmt-check.bash cargo
         shell: bash
+      - run: ci/clippy.bash cargo
+        shell: bash
 
     strategy:
       fail-fast: true
@@ -84,6 +86,8 @@ jobs:
         if: matrix.target == 'x86_64-apple-darwin'
       - run: ci/fmt-check.bash cargo
         shell: bash
+      - run: ci/clippy.bash cargo
+        shell: bash
 
     strategy:
       fail-fast: true
@@ -124,6 +128,8 @@ jobs:
           matrix.target != 'sparc64-unknown-linux-gnu'
       - run: ci/fmt-check.bash cargo
         shell: bash
+      - run: ci/clippy.bash cargo
+        shell: bash
 
     strategy:
       fail-fast: true

ci/clippy.bash

@@ -0,0 +1,12 @@
+#!/usr/bin/env bash
+# Script for building your rust projects.
+set -e
+
+source ci/common.bash
+
+# $1 {path} = Path to cross/cargo executable
+CROSS=$1
+
+required_arg $CROSS 'CROSS'
+
+$CROSS clippy --all -- -D warnings

ci/set_rust_version.bash

@@ -3,4 +3,4 @@ set -e
 rustup update
 rustup default $1
 rustup target add $2
-rustup component add rustfmt
+rustup component add rustfmt clippy

examples/size_compare.rs

@@ -2,7 +2,7 @@ use rasn_pkix::Certificate;
 
 fn main() {
     const DER_BYTES: &[u8] = include_bytes!("../standards/pkix/tests/data/letsencrypt-x3.crt");
-    let original = rasn::der::decode::<Certificate>(&DER_BYTES).unwrap();
+    let original = rasn::der::decode::<Certificate>(DER_BYTES).unwrap();
     let ber = rasn::ber::encode(&original).unwrap();
     let cer = rasn::cer::encode(&original).unwrap();
     let uper = rasn::uper::encode(&original).unwrap();

macros/src/tag.rs

@@ -34,7 +34,7 @@ impl Class {
 }
 
 impl Class {
-    pub fn to_tokens(&self, crate_root: &syn::Path) -> proc_macro2::TokenStream {
+    pub fn as_tokens(&self, crate_root: &syn::Path) -> proc_macro2::TokenStream {
         match self {
             Self::Universal => quote!(#crate_root::types::Class::Universal),
             Self::Application => quote!(#crate_root::types::Class::Application),
@@ -158,7 +158,7 @@ impl Tag {
     pub fn to_tokens(&self, crate_root: &syn::Path) -> proc_macro2::TokenStream {
         match self {
             Self::Value { class, value, .. } => {
-                let cls = class.to_tokens(crate_root);
+                let cls = class.as_tokens(crate_root);
                 quote!(#crate_root::Tag::new(#cls, #value))
             }
             Self::Delegate { ty } => quote!(<#ty as #crate_root::AsnType>::TAG),

src/ber/de.rs

@@ -168,7 +168,7 @@ impl<'input> Decoder<'input> {
         // Helper function to deal with fractions and without timezone
         let parse_without_timezone = |string: &str| -> Result<NaiveDateTime, DecodeError> {
             // Handle both decimal cases (dot . and comma , )
-            let string: &str = &string.replace(",", ".");
+            let string: &str = &string.replace(',', ".");
             if string.contains('.') {
                 // Use chrono to parse the string every time, since we don't the know the number of decimals places
                 NaiveDateTime::parse_from_str(string, "%Y%m%d%H%.f")
@@ -530,13 +530,9 @@ impl<'input> crate::Decoder for Decoder<'input> {
         tag: Tag,
         constraints: Constraints,
     ) -> Result<types::TeletexString> {
-        types::TeletexString::try_from(self.decode_octet_string(tag, constraints)?).map_err(|e| {
-            DecodeError::string_conversion_failed(
-                types::Tag::TELETEX_STRING,
-                e.to_string(),
-                self.codec(),
-            )
-        })
+        Ok(types::TeletexString::from(
+            self.decode_octet_string(tag, constraints)?,
+        ))
     }
 
     fn decode_bmp_string(&mut self, _: Tag, _constraints: Constraints) -> Result<types::BmpString> {

src/bits.rs

@@ -22,6 +22,7 @@ pub(crate) fn range_from_len(bit_length: u32) -> i128 {
 /// *The encodings of the component values of a set-of value shall appear in ascending order,*
 /// *the encodings being compared as octet strings with the shorter components being padded*
 /// *at their trailing end with 0-octets.*
+#[allow(clippy::ptr_arg)] // used in comparison function
 pub(crate) fn octet_string_ascending(a: &Vec<u8>, b: &Vec<u8>) -> Ordering {
     let min_length = b.len().min(a.len());
     for i in 0..min_length {
@@ -30,13 +31,7 @@ pub(crate) fn octet_string_ascending(a: &Vec<u8>, b: &Vec<u8>) -> Ordering {
             o => return o,
         }
     }
-    if b.len() > a.len() {
-        return Ordering::Less;
-    } else if a.len() > b.len() {
-        return Ordering::Greater;
-    } else {
-        Ordering::Equal
-    }
+    a.len().cmp(&b.len())
 }
 
 // Workaround for https://github.com/ferrilab/bitvec/issues/228

src/codec.rs

@@ -73,7 +73,7 @@ impl Codec {
                         crate::error::DecodeErrorKind::Custom {
                             msg: alloc::format!("Failed to decode JER from UTF8 bytes: {e:?}"),
                         },
-                        self.clone(),
+                        *self,
                     ))
                 },
                 |s| crate::jer::decode(&s),
@@ -115,7 +115,7 @@ impl Codec {
                 crate::error::DecodeErrorKind::Custom {
                     msg: alloc::format!("{codec} is a text-based encoding. Call `Codec::decode_from_binary` instead."),
                 },
-                codec.clone(),
+                *codec,
             )),
         }
     }

src/error/encode.rs

@@ -63,41 +63,38 @@ impl From<CodecEncodeError> for EncodeError {
 ///     VisibleString,
 /// );
 ///
-/// fn main() {
-///     // Below sample requires that `backtraces` feature is enabled
+/// // Below sample requires that `backtraces` feature is enabled
 ///
-///     let constrained_str = MyConstrainedString(VisibleString::try_from("abcD").unwrap());
-///     let encoded = Codec::Uper.encode_to_binary(&constrained_str);
-///     match encoded {
-///         Ok(succ) => {
-///             println!("Successful encoding!");
-///             dbg!(succ);
-///         }
-///         Err(e) => {
-///             // e is EncodeError, Kind is boxed
-///             match *e.kind {
-///                 EncodeErrorKind::AlphabetConstraintNotSatisfied {
-///                     reason: PermittedAlphabetError::CharacterNotFound {character },
-///                 } => {
-///                     println!("Codec: {}", e.codec);
-///                     println!("Character {} not found from the permitted list.",
-///                              char::from_u32(character).unwrap());
-///                     #[cfg(feature = "backtraces")]
-///                     println!("Backtrace:\n{}", e.backtrace);
-///                 }
-///                 _ => {
-///                     panic!("Unexpected error!");
-///                 }
+/// let constrained_str = MyConstrainedString(VisibleString::try_from("abcD").unwrap());
+/// let encoded = Codec::Uper.encode_to_binary(&constrained_str);
+/// match encoded {
+///     Ok(succ) => {
+///         println!("Successful encoding!");
+///         dbg!(succ);
+///     }
+///     Err(e) => {
+///         // e is EncodeError, Kind is boxed
+///         match *e.kind {
+///             EncodeErrorKind::AlphabetConstraintNotSatisfied {
+///                reason: PermittedAlphabetError::CharacterNotFound {character },
+///             } => {
+///                 println!("Codec: {}", e.codec);
+///                 println!("Character {} not found from the permitted list.",
+///                          char::from_u32(character).unwrap());
+///                 #[cfg(feature = "backtraces")]
+///                 println!("Backtrace:\n{}", e.backtrace);
+///             }
+///             _ => {
+///                 panic!("Unexpected error!");
 ///             }
 ///         }
 ///     }
-///     // Should print ->
-///     //
-///     // Codec: UPER
-///     // Character D not found from the permitted list.
-///     // Backtrace: ...
-///
 /// }
+/// // Should print ->
+/// //
+/// // Codec: UPER
+/// // Character D not found from the permitted list.
+/// // Backtrace: ...
 /// ```
 ///
 #[derive(Debug)]

src/jer.rs

@@ -4,7 +4,7 @@ pub mod enc;
 /// Attempts to decode `T` from `input` using JER.
 /// # Errors
 /// Returns error specific to JER decoder if decoding is not possible.
-pub fn decode<'de, T: crate::Decode>(input: &'de str) -> Result<T, crate::error::DecodeError> {
+pub fn decode<T: crate::Decode>(input: &str) -> Result<T, crate::error::DecodeError> {
     T::decode(&mut de::Decoder::new(input)?)
 }
 

src/jer/de.rs

@@ -91,12 +91,12 @@ impl crate::Decoder for Decoder {
         D: Constructed,
         F: FnOnce(&mut Self) -> Result<D, Self::Error>,
     {
-        let mut last = self.stack.pop().ok_or_else(|| JerDecodeErrorKind::eoi())?;
+        let mut last = self.stack.pop().ok_or_else(JerDecodeErrorKind::eoi)?;
         let value_map = last
             .as_object_mut()
             .ok_or_else(|| JerDecodeErrorKind::TypeMismatch {
                 needed: "object",
-                found: alloc::format!("unknown"),
+                found: "unknown".into(),
             })?;
         let mut field_names = [D::FIELDS, D::EXTENDED_FIELDS.unwrap_or(Fields::empty())]
             .iter()
@@ -275,12 +275,12 @@ impl crate::Decoder for Decoder {
         D: Fn(&mut Self, usize, crate::Tag) -> Result<FIELDS, Self::Error>,
         F: FnOnce(alloc::vec::Vec<FIELDS>) -> Result<SET, Self::Error>,
     {
-        let mut last = self.stack.pop().ok_or_else(|| JerDecodeErrorKind::eoi())?;
+        let mut last = self.stack.pop().ok_or_else(JerDecodeErrorKind::eoi)?;
         let value_map = last
             .as_object_mut()
             .ok_or_else(|| JerDecodeErrorKind::TypeMismatch {
                 needed: "object",
-                found: alloc::format!("unknown"),
+                found: "unknown".into(),
             })?;
         let mut field_indices = SET::FIELDS
             .iter()
@@ -316,7 +316,7 @@ impl crate::Decoder for Decoder {
     }
 
     fn decode_optional<D: crate::Decode>(&mut self) -> Result<Option<D>, Self::Error> {
-        let last = self.stack.pop().ok_or_else(|| JerDecodeErrorKind::eoi())?;
+        let last = self.stack.pop().ok_or_else(JerDecodeErrorKind::eoi)?;
         match last {
             JsonValue::Null => Ok(None),
             v => {
@@ -417,8 +417,7 @@ impl Decoder {
             .try_into()
             .ok();
         Ok(discriminant
-            .map(|i| E::from_discriminant(i))
-            .flatten()
+            .and_then(|i| E::from_discriminant(i))
             .ok_or_else(|| JerDecodeErrorKind::InvalidEnumDiscriminant {
                 discriminant: discriminant.unwrap_or(isize::MIN),
             })?)
@@ -437,7 +436,7 @@ impl Decoder {
     fn null_from_value(value: JsonValue) -> Result<(), DecodeError> {
         Ok(value
             .is_null()
-            .then(|| ())
+            .then_some(())
             .ok_or_else(|| JerDecodeErrorKind::TypeMismatch {
                 needed: "null",
                 found: alloc::format!("{value}"),
@@ -451,17 +450,17 @@ impl Decoder {
                 needed: "number array",
                 found: alloc::format!("{value}"),
             })?
-            .split(".")
-            .into_iter()
+            .split('.')
             .map(|arc| {
-                u32::from_str_radix(arc, 10).map_err(|_| JerDecodeErrorKind::TypeMismatch {
-                    needed: "OID arc number",
-                    found: alloc::format!("{arc}"),
-                })
+                arc.parse::<u32>()
+                    .map_err(|_| JerDecodeErrorKind::TypeMismatch {
+                        needed: "OID arc number",
+                        found: arc.into(),
+                    })
             })
             .collect::<Result<alloc::vec::Vec<u32>, _>>()
             .ok()
-            .and_then(|arcs| Oid::new(&arcs).map(|oid| ObjectIdentifier::from(oid)))
+            .and_then(|arcs| Oid::new(&arcs).map(ObjectIdentifier::from))
             .ok_or_else(|| JerDecodeErrorKind::InvalidOIDString { value })?)
     }
 
@@ -529,7 +528,7 @@ impl Decoder {
                 D::IDENTIFIERS
                     .iter()
                     .enumerate()
-                    .find(|id| id.1.eq_ignore_ascii_case(&k))
+                    .find(|id| id.1.eq_ignore_ascii_case(k))
                     .map(|(i, _)| (i, v))
             })
             .map_or(Tag::EOC, |(i, v)| {
@@ -540,7 +539,7 @@ impl Decoder {
                 {
                     Some(t) => {
                         self.stack.push(v.clone());
-                        t.clone()
+                        *t
                     }
                     None => Tag::EOC,
                 }

src/jer/enc.rs

@@ -13,6 +13,12 @@ pub struct Encoder {
     root_value: Option<JsonValue>,
 }
 
+impl Default for Encoder {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 impl Encoder {
     pub fn new() -> Self {
         Self {
@@ -25,7 +31,7 @@ impl Encoder {
     pub fn root_value(self) -> Result<JsonValue, EncodeError> {
         Ok(self
             .root_value
-            .ok_or_else(|| JerEncodeErrorKind::NoRootValueFound)?)
+            .ok_or(JerEncodeErrorKind::NoRootValueFound)?)
     }
 
     pub fn to_json(self) -> alloc::string::String {
@@ -40,7 +46,7 @@ impl Encoder {
                     .ok_or_else(|| JerEncodeErrorKind::JsonEncoder {
                         msg: "Internal stack mismatch!".into(),
                     })?
-                    .insert(id.into(), value);
+                    .insert(id, value);
             }
             None => {
                 self.root_value = Some(value);

src/per/enc.rs

@@ -198,7 +198,7 @@ impl Encoder {
         let mut buffer = BitString::default();
         let string_length = value.len();
 
-        let is_extended_value = self.encode_extensible_bit(&constraints, &mut buffer, || {
+        let is_extended_value = self.encode_extensible_bit(constraints, &mut buffer, || {
             constraints.size().map_or(false, |size_constraint| {
                 size_constraint.extensible.is_some()
                     && size_constraint.constraint.contains(&string_length)

src/types/strings/constrained.rs

@@ -97,6 +97,7 @@ pub(crate) trait StaticPermittedAlphabet: Sized + Default {
         self.chars().count()
     }
 
+    #[allow(clippy::box_collection)]
     fn build_index_map() -> Box<alloc::collections::BTreeMap<u32, u32>> {
         Box::new(
             Self::CHARACTER_SET
@@ -108,6 +109,7 @@ pub(crate) trait StaticPermittedAlphabet: Sized + Default {
         )
     }
 
+    #[allow(clippy::box_collection)]
     fn build_character_map() -> Box<alloc::collections::BTreeMap<u32, u32>> {
         Box::new(
             Self::CHARACTER_SET
@@ -162,7 +164,7 @@ pub(crate) fn try_from_permitted_alphabet<S: StaticPermittedAlphabet>(
             string.push_char(
                 *alphabet
                     .get(&index)
-                    .ok_or_else(|| PermittedAlphabetError::IndexNotFound { index })?,
+                    .ok_or(PermittedAlphabetError::IndexNotFound { index })?,
             );
         }
     } else {
@@ -176,11 +178,7 @@ pub(crate) fn try_from_permitted_alphabet<S: StaticPermittedAlphabet>(
 }
 pub(crate) fn should_be_indexed(width: u32, character_set: &[u32]) -> bool {
     let largest_value = character_set.iter().copied().max().unwrap_or(0);
-    if 2u32.pow(width) > largest_value {
-        false
-    } else {
-        true
-    }
+    2u32.pow(width) <= largest_value
 }
 
 #[derive(Debug, Default, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)]