all: fix clippy warnings and add CI check (#191)

It's nice to fix these, and they make the code simpler in many cases.

Fixes #185.

Author: kevinburke

.github/workflows/test.yaml

@@ -115,3 +115,24 @@ jobs:
         with:
           command: fmt
           args: --all -- --check
+
+  clippy:
+    name: Clippy
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout sources
+        uses: actions/checkout@v2
+        with:
+          persist-credentials: false
+      - name: Install rust toolchain
+        uses: actions-rs/toolchain@v1
+        with:
+          toolchain: stable
+          override: true
+          default: true
+          components: clippy
+      - name: Check formatting
+        uses: actions-rs/cargo@v1
+        with:
+          command: clippy
+          args: --workspace

build.rs

@@ -17,10 +17,9 @@ fn main() {
     let dest_path = out_dir.join("version.rs");
     let mut f = File::create(&dest_path).expect("Could not create file");
     let pkg_version = env!("CARGO_PKG_VERSION");
-    write!(
+    writeln!(
         &mut f,
-        r#"const RUSTLS_FFI_VERSION: &'static str = "rustls-ffi/{}/rustls/{}";
-"#,
+        r#"const RUSTLS_FFI_VERSION: &str = "rustls-ffi/{}/rustls/{}";"#,
         pkg_version, RUSTLS_CRATE_VERSION
     )
     .expect("Could not write file");

src/cipher.rs

@@ -178,7 +178,7 @@ impl rustls_certified_key {
             };
             let certified_key = Arc::into_raw(Arc::new(*certified_key)) as *const _;
             *certified_key_out = certified_key;
-            return rustls_result::Ok
+            rustls_result::Ok
         }
     }
 
@@ -228,7 +228,7 @@ impl rustls_certified_key {
                 new_key.ocsp = None;
             }
             *cloned_key_out = ArcCastPtr::to_const_ptr(new_key);
-            return rustls_result::Ok
+            rustls_result::Ok
         }
     }
 

src/client.rs

@@ -132,10 +132,10 @@ impl rustls_client_config_builder {
         ffi_panic_boundary! {
             let cipher_suites: &[*const rustls_supported_ciphersuite] = try_slice!(cipher_suites, cipher_suites_len);
             let mut cs_vec: Vec<SupportedCipherSuite> = Vec::new();
-            for &cs in cipher_suites.into_iter() {
+            for &cs in cipher_suites.iter() {
                 let cs = try_ref_from_ptr!(cs);
                 match ALL_CIPHER_SUITES.iter().find(|&acs| cs.eq(acs)) {
-                    Some(scs) => cs_vec.push(scs.clone()),
+                    Some(scs) => cs_vec.push(*scs),
                     None => return InvalidParameter,
                 }
             }
@@ -157,7 +157,7 @@ impl rustls_client_config_builder {
                 Err(_) => return rustls_result::InvalidParameter,
             };
             let config_builder = ClientConfigBuilder {
-                base: base,
+                base,
                 verifier: Arc::new(NoneVerifier),
                 cert_resolver: None,
                 alpn_protocols: vec![],
@@ -230,18 +230,15 @@ impl rustls::client::ServerCertVerifier for Verifier {
     ) -> Result<ServerCertVerified, rustls::Error> {
         let cb = self.callback;
         let dns_name: &str = match server_name {
-            rustls::ServerName::DnsName(n) => n.as_ref().into(),
+            rustls::ServerName::DnsName(n) => n.as_ref(),
             _ => return Err(rustls::Error::General("unknown name type".to_string())),
         };
         let dns_name: rustls_str = match dns_name.try_into() {
             Ok(r) => r,
             Err(NulByte {}) => return Err(rustls::Error::General("NUL byte in SNI".to_string())),
         };
 
-        let intermediates: Vec<_> = intermediates
-            .into_iter()
-            .map(|cert| cert.as_ref())
-            .collect();
+        let intermediates: Vec<_> = intermediates.iter().map(|cert| cert.as_ref()).collect();
 
         let intermediates = rustls_slice_slice_bytes {
             inner: &*intermediates,
@@ -250,7 +247,7 @@ impl rustls::client::ServerCertVerifier for Verifier {
         let params = rustls_verify_server_cert_params {
             end_entity_cert_der: end_entity.as_ref().into(),
             intermediate_certs_der: &intermediates,
-            dns_name: dns_name.into(),
+            dns_name,
             ocsp_response: ocsp_response.into(),
         };
         let userdata = userdata_get().map_err(|_| {
@@ -312,7 +309,7 @@ impl rustls_client_config_builder {
                 None => return rustls_result::InvalidParameter,
             };
 
-            let verifier: Verifier = Verifier{callback: callback};
+            let verifier: Verifier = Verifier{callback};
             config_builder.verifier = Arc::new(verifier);
             rustls_result::Ok
         }

src/connection.rs

@@ -248,7 +248,7 @@ impl rustls_connection {
             };
             match guard.try_drop() {
                 Ok(()) => result,
-                Err(_) => return rustls_result::Panic,
+                Err(_) => rustls_result::Panic,
             }
         }
     }

src/error.rs

@@ -47,14 +47,14 @@ impl rustls_result {
     #[no_mangle]
     pub extern "C" fn rustls_result_is_cert_error(result: rustls_result) -> bool {
         match result_to_error(&result) {
-            Either::Error(e) => match e {
-                Error::InvalidCertificateData(_) => true,
-                Error::InvalidCertificateEncoding => true,
-                Error::InvalidCertificateSignature => true,
-                Error::InvalidCertificateSignatureType => true,
-                Error::InvalidSct(_) => true,
-                _ => false,
-            },
+            Either::Error(e) => matches!(
+                e,
+                Error::InvalidCertificateData(_)
+                    | Error::InvalidCertificateEncoding
+                    | Error::InvalidCertificateSignature
+                    | Error::InvalidCertificateSignatureType
+                    | Error::InvalidSct(_)
+            ),
             _ => false,
         }
     }
@@ -241,27 +241,27 @@ pub(crate) enum Either {
     Error(rustls::Error),
 }
 
-impl Into<Either> for String {
-    fn into(self) -> Either {
-        Either::String(self)
+impl From<String> for Either {
+    fn from(s: String) -> Either {
+        Either::String(s)
     }
 }
 
-impl Into<Either> for &str {
-    fn into(self) -> Either {
-        Either::String(self.to_string())
+impl From<&str> for Either {
+    fn from(s: &str) -> Either {
+        Either::String(s.to_string())
     }
 }
 
-impl Into<Either> for webpki::Error {
-    fn into(self) -> Either {
-        Either::String(self.to_string())
+impl From<webpki::Error> for Either {
+    fn from(e: webpki::Error) -> Either {
+        Either::String(e.to_string())
     }
 }
 
-impl Into<Either> for rustls::Error {
-    fn into(self) -> Either {
-        Either::Error(self)
+impl From<rustls::Error> for Either {
+    fn from(e: rustls::Error) -> Either {
+        Either::Error(e)
     }
 }
 

src/lib.rs

@@ -1,11 +1,11 @@
 #![crate_type = "staticlib"]
 #![allow(non_camel_case_types)]
+#![allow(clippy::not_unsafe_ptr_arg_deref)]
 use crate::rslice::rustls_str;
 use libc::{c_void, size_t};
 use std::cell::RefCell;
 use std::mem;
 use std::sync::Arc;
-use std::thread::AccessError;
 
 pub mod cipher;
 pub mod client;
@@ -99,7 +99,7 @@ impl UserdataGuard {
                     },
                 )
             })
-            .unwrap_or_else(|_: AccessError| Err(UserdataError::AccessError))
+            .unwrap_or(Err(UserdataError::AccessError))
     }
 }
 
@@ -410,14 +410,14 @@ where
     unsafe { F::cast_mut_ptr(from).as_mut() }
 }
 
-pub(crate) fn try_box_from<'a, F, T>(from: *mut F) -> Option<Box<T>>
+pub(crate) fn try_box_from<F, T>(from: *mut F) -> Option<Box<T>>
 where
     F: BoxCastPtr<RustType = T>,
 {
     F::to_box(from)
 }
 
-pub(crate) fn try_arc_from<'a, F, T>(from: *const F) -> Option<Arc<T>>
+pub(crate) fn try_arc_from<F, T>(from: *const F) -> Option<Arc<T>>
 where
     F: ArcCastPtr<RustType = T>,
 {
@@ -496,7 +496,7 @@ macro_rules! try_callback {
 /// not need to be freed.
 #[no_mangle]
 pub extern "C" fn rustls_version() -> rustls_str<'static> {
-    return rustls_str::from_str_unchecked(RUSTLS_FFI_VERSION);
+    rustls_str::from_str_unchecked(RUSTLS_FFI_VERSION)
 }
 
 #[test]

src/rslice.rs

@@ -77,10 +77,10 @@ pub extern "C" fn rustls_slice_slice_bytes_len(input: *const rustls_slice_slice_
 /// pointer is NULL, or n is greater than the length of the
 /// rustls_slice_slice_bytes, returns rustls_slice_bytes{NULL, 0}.
 #[no_mangle]
-pub extern "C" fn rustls_slice_slice_bytes_get<'a>(
-    input: *const rustls_slice_slice_bytes<'a>,
+pub extern "C" fn rustls_slice_slice_bytes_get(
+    input: *const rustls_slice_slice_bytes,
     n: size_t,
-) -> rustls_slice_bytes<'a> {
+) -> rustls_slice_bytes {
     let input: &rustls_slice_slice_bytes = unsafe {
         match input.as_ref() {
             Some(c) => c,

src/server.rs

@@ -121,10 +121,10 @@ impl rustls_server_config_builder {
         ffi_panic_boundary! {
             let cipher_suites: &[*const rustls_supported_ciphersuite] = try_slice!(cipher_suites, cipher_suites_len);
             let mut cs_vec: Vec<SupportedCipherSuite> = Vec::new();
-            for &cs in cipher_suites.into_iter() {
+            for &cs in cipher_suites.iter() {
                 let cs = try_ref_from_ptr!(cs);
                 match ALL_CIPHER_SUITES.iter().find(|&acs| cs.eq(acs)) {
-                    Some(scs) => cs_vec.push(scs.clone()),
+                    Some(scs) => cs_vec.push(*scs),
                     None => return InvalidParameter,
                 }
             }
@@ -387,7 +387,7 @@ pub extern "C" fn rustls_server_connection_get_sni_hostname(
         if len > write_buf.len() {
             return rustls_result::InsufficientSize;
         }
-        write_buf[..len].copy_from_slice(&sni_hostname.as_bytes());
+        write_buf[..len].copy_from_slice(sni_hostname.as_bytes());
         *out_n = len;
         rustls_result::Ok
     }
@@ -497,7 +497,7 @@ impl ResolvesServerCert for ClientHelloResolver {
     fn resolve(&self, client_hello: ClientHello) -> Option<Arc<CertifiedKey>> {
         let sni_name: &str = {
             match client_hello.server_name() {
-                Some(c) => c.into(),
+                Some(c) => c,
                 None => "",
             }
         };

src/session.rs

@@ -107,7 +107,7 @@ impl SessionStoreBroker {
             ) {
                 rustls_result::Ok => {
                     data.set_len(out_n);
-                    return Some(data);
+                    Some(data)
                 }
                 _ => None,
             }
@@ -122,12 +122,7 @@ impl SessionStoreBroker {
             Ok(u) => u,
             Err(_) => return false,
         };
-        unsafe {
-            match cb(userdata, &key, &value) {
-                rustls_result::Ok => true,
-                _ => false,
-            }
-        }
+        unsafe { matches!(cb(userdata, &key, &value), rustls_result::Ok) }
     }
 }
 
@@ -137,11 +132,11 @@ impl rustls::server::StoresServerSessions for SessionStoreBroker {
     }
 
     fn get(&self, key: &[u8]) -> Option<Vec<u8>> {
-        return self.retrieve(key, false);
+        self.retrieve(key, false)
     }
 
     fn take(&self, key: &[u8]) -> Option<Vec<u8>> {
-        return self.retrieve(key, true);
+        self.retrieve(key, true)
     }
 
     fn can_cache(&self) -> bool {
@@ -155,7 +150,7 @@ impl rustls::client::StoresClientSessions for SessionStoreBroker {
     }
 
     fn get(&self, key: &[u8]) -> Option<Vec<u8>> {
-        return self.retrieve(key, false);
+        self.retrieve(key, false)
     }
 }