Ensure CI covers examples and unit tests, fix clippy findings (#191)

While reviewing #188 I wanted to
confirm that example code was being built in CI. It turns out that it
wasn't. Similarly we haven't been running `clippy` against test code,
and so there was a number of findings to address.

This branch updates CI to:

* Remove `--all`. This is a deprecated alias for `--workspace`, and
`--workspace` is the default for a directory containing a workspace so
it can be omitted.
* Use `--all-targets` whenever we run `cargo check`, `cargo test` or
`cargo clippy`. This ensures coverage for both examples and unit tests.

In order for the `cargo clippy ... --all-targets` to succeed this branch
addresses each of the findings that were present. I've done this with a
separate commit per class of finding to make it easier to review. In one
case (7bfe0ef) I allowed the finding
instead of fixing it since it seemed like the choice of digit groupings
was done intentionally.

.github/workflows/ci.yml

@@ -38,8 +38,8 @@ jobs:
         uses: dtolnay/rust-toolchain@stable
         with:
           components: clippy
-      - run: cargo clippy --all-features
-      - run: cargo clippy --no-default-features
+      - run: cargo clippy --all-features --all-targets
+      - run: cargo clippy --no-default-features --all-targets
 
   rustdoc:
     name: Documentation
@@ -57,7 +57,7 @@ jobs:
         with:
           toolchain: ${{ matrix.toolchain }}
       - name: cargo doc (all features)
-        run: cargo doc --all --all-features --document-private-items
+        run: cargo doc --all-features --document-private-items
         env:
           RUSTDOCFLAGS: ${{ matrix.rust_channel == 'nightly' && '-Dwarnings --cfg=docsrs' || '-Dwarnings' }}
 
@@ -89,14 +89,14 @@ jobs:
       uses: dtolnay/rust-toolchain@stable
     - run: echo "VCPKG_ROOT=$env:VCPKG_INSTALLATION_ROOT" | Out-File -FilePath $env:GITHUB_ENV -Append
     - run: vcpkg install openssl:x64-windows-static-md
-    - name: Run cargo check --all
-      run: cargo check --all
+    - name: Run cargo check
+      run: cargo check --all-targets
     - name: Run the tests
-      run: cargo test --all
+      run: cargo test --all-targets
     - name: Run the tests with x509-parser enabled
-      run: cargo test --verbose --features x509-parser
+      run: cargo test --verbose --features x509-parser --all-targets
     - name: Run the tests with no default features enabled
-      run: cargo test --verbose --no-default-features
+      run: cargo test --verbose --no-default-features --all-targets
 
   build:
     strategy:
@@ -128,14 +128,14 @@ jobs:
       uses: dtolnay/rust-toolchain@master
       with:
         toolchain: ${{ matrix.toolchain }}
-    - name: Run cargo check --all
-      run: cargo check --all
+    - name: Run cargo check
+      run: cargo check --all-targets
     - name: Run the tests
-      run: cargo test --all
+      run: cargo test --all-targets
     - name: Run the tests with x509-parser enabled
-      run: cargo test --verbose --features x509-parser
+      run: cargo test --verbose --features x509-parser --all-targets
     - name: Run the tests with no default features enabled
-      run: cargo test --verbose --no-default-features
+      run: cargo test --verbose --no-default-features --all-targets
 
   coverage:
     name: Measure coverage

rcgen/examples/rsa-irc-openssl.rs

@@ -1,12 +1,11 @@
-#![allow(clippy::complexity, clippy::style, clippy::pedantic)]
-
 fn main() -> Result<(), Box<dyn std::error::Error>> {
 	use rcgen::{date_time_ymd, Certificate, CertificateParams, DistinguishedName};
+	use std::fmt::Write;
 	use std::fs;
 
 	let mut params: CertificateParams = Default::default();
-	params.not_before = date_time_ymd(2021, 05, 19);
-	params.not_after = date_time_ymd(4096, 01, 01);
+	params.not_before = date_time_ymd(2021, 5, 19);
+	params.not_after = date_time_ymd(4096, 1, 1);
 	params.distinguished_name = DistinguishedName::new();
 
 	params.alg = &rcgen::PKCS_RSA_SHA256;
@@ -20,18 +19,18 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
 	let pem_serialized = cert.serialize_pem()?;
 	let pem = pem::parse(&pem_serialized)?;
 	let der_serialized = pem.contents();
-	let hash = ring::digest::digest(&ring::digest::SHA512, &der_serialized);
-	let hash_hex: String = hash.as_ref().iter().map(|b| format!("{b:02x}")).collect();
+	let hash = ring::digest::digest(&ring::digest::SHA512, der_serialized);
+	let hash_hex = hash.as_ref().iter().fold(String::new(), |mut output, b| {
+		let _ = write!(output, "{b:02x}");
+		output
+	});
 	println!("sha-512 fingerprint: {hash_hex}");
 	println!("{pem_serialized}");
 	println!("{}", cert.serialize_private_key_pem());
 	std::fs::create_dir_all("certs/")?;
-	fs::write("certs/cert.pem", &pem_serialized.as_bytes())?;
-	fs::write("certs/cert.der", &der_serialized)?;
-	fs::write(
-		"certs/key.pem",
-		&cert.serialize_private_key_pem().as_bytes(),
-	)?;
-	fs::write("certs/key.der", &cert.serialize_private_key_der())?;
+	fs::write("certs/cert.pem", pem_serialized.as_bytes())?;
+	fs::write("certs/cert.der", der_serialized)?;
+	fs::write("certs/key.pem", cert.serialize_private_key_pem().as_bytes())?;
+	fs::write("certs/key.der", cert.serialize_private_key_der())?;
 	Ok(())
 }

rcgen/examples/rsa-irc.rs

@@ -1,16 +1,15 @@
-#![allow(clippy::complexity, clippy::style, clippy::pedantic)]
-
 fn main() -> Result<(), Box<dyn std::error::Error>> {
 	use rand::rngs::OsRng;
 	use rsa::pkcs8::EncodePrivateKey;
 	use rsa::RsaPrivateKey;
 
 	use rcgen::{date_time_ymd, Certificate, CertificateParams, DistinguishedName};
+	use std::fmt::Write;
 	use std::fs;
 
 	let mut params: CertificateParams = Default::default();
-	params.not_before = date_time_ymd(2021, 05, 19);
-	params.not_after = date_time_ymd(4096, 01, 01);
+	params.not_before = date_time_ymd(2021, 5, 19);
+	params.not_after = date_time_ymd(4096, 1, 1);
 	params.distinguished_name = DistinguishedName::new();
 
 	params.alg = &rcgen::PKCS_RSA_SHA256;
@@ -26,18 +25,18 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
 	let pem_serialized = cert.serialize_pem()?;
 	let pem = pem::parse(&pem_serialized)?;
 	let der_serialized = pem.contents();
-	let hash = ring::digest::digest(&ring::digest::SHA512, &der_serialized);
-	let hash_hex: String = hash.as_ref().iter().map(|b| format!("{:02x}", b)).collect();
+	let hash = ring::digest::digest(&ring::digest::SHA512, der_serialized);
+	let hash_hex = hash.as_ref().iter().fold(String::new(), |mut output, b| {
+		let _ = write!(output, "{b:02x}");
+		output
+	});
 	println!("sha-512 fingerprint: {hash_hex}");
 	println!("{pem_serialized}");
 	println!("{}", cert.serialize_private_key_pem());
 	std::fs::create_dir_all("certs/")?;
-	fs::write("certs/cert.pem", &pem_serialized.as_bytes())?;
-	fs::write("certs/cert.der", &der_serialized)?;
-	fs::write(
-		"certs/key.pem",
-		&cert.serialize_private_key_pem().as_bytes(),
-	)?;
-	fs::write("certs/key.der", &cert.serialize_private_key_der())?;
+	fs::write("certs/cert.pem", pem_serialized.as_bytes())?;
+	fs::write("certs/cert.der", der_serialized)?;
+	fs::write("certs/key.pem", cert.serialize_private_key_pem().as_bytes())?;
+	fs::write("certs/key.der", cert.serialize_private_key_der())?;
 	Ok(())
 }

rcgen/src/lib.rs

@@ -1869,7 +1869,7 @@ mod tests {
 		fn test_not_windows_line_endings() {
 			let cert = Certificate::from_params(CertificateParams::default()).unwrap();
 			let pem = cert.serialize_pem().expect("Failed to serialize pem");
-			assert!(!pem.contains("\r"));
+			assert!(!pem.contains('\r'));
 		}
 	}
 

rcgen/tests/botan.rs

@@ -13,22 +13,22 @@ mod util;
 fn default_params() -> CertificateParams {
 	let mut params = util::default_params();
 	// Botan has a sanity check that enforces a maximum expiration date
-	params.not_after = rcgen::date_time_ymd(3016, 01, 01);
+	params.not_after = rcgen::date_time_ymd(3016, 1, 1);
 	params
 }
 
-fn check_cert<'a, 'b>(cert_der: &[u8], cert: &'a Certificate) {
+fn check_cert(cert_der: &[u8], cert: &Certificate) {
 	println!("{}", cert.serialize_pem().unwrap());
 	check_cert_ca(cert_der, cert, cert_der);
 }
 
-fn check_cert_ca<'a, 'b>(cert_der: &[u8], _cert: &'a Certificate, ca_der: &[u8]) {
+fn check_cert_ca(cert_der: &[u8], _cert: &Certificate, ca_der: &[u8]) {
 	println!(
 		"botan version: {}",
 		botan::Version::current().unwrap().string
 	);
-	let trust_anchor = botan::Certificate::load(&ca_der).unwrap();
-	let end_entity_cert = botan::Certificate::load(&cert_der).unwrap();
+	let trust_anchor = botan::Certificate::load(ca_der).unwrap();
+	let end_entity_cert = botan::Certificate::load(cert_der).unwrap();
 
 	// Set time to Jan 10, 2004
 	const REFERENCE_TIME: Option<u64> = Some(0x40_00_00_00);
@@ -161,7 +161,7 @@ fn test_botan_separate_ca() {
 		.distinguished_name
 		.push(DnType::CommonName, "Dev domain");
 	// Botan has a sanity check that enforces a maximum expiration date
-	params.not_after = rcgen::date_time_ymd(3016, 01, 01);
+	params.not_after = rcgen::date_time_ymd(3016, 1, 1);
 
 	let cert = Certificate::from_params(params).unwrap();
 	let cert_der = cert.serialize_der_with_signer(&ca_cert).unwrap();
@@ -195,7 +195,7 @@ fn test_botan_imported_ca() {
 		.distinguished_name
 		.push(DnType::CommonName, "Dev domain");
 	// Botan has a sanity check that enforces a maximum expiration date
-	params.not_after = rcgen::date_time_ymd(3016, 01, 01);
+	params.not_after = rcgen::date_time_ymd(3016, 1, 1);
 	let cert = Certificate::from_params(params).unwrap();
 	let cert_der = cert.serialize_der_with_signer(&imported_ca_cert).unwrap();
 
@@ -232,7 +232,7 @@ fn test_botan_imported_ca_with_printable_string() {
 		.distinguished_name
 		.push(DnType::CommonName, "Dev domain");
 	// Botan has a sanity check that enforces a maximum expiration date
-	params.not_after = rcgen::date_time_ymd(3016, 01, 01);
+	params.not_after = rcgen::date_time_ymd(3016, 1, 1);
 	let cert = Certificate::from_params(params).unwrap();
 	let cert_der = cert.serialize_der_with_signer(&imported_ca_cert).unwrap();
 
@@ -259,7 +259,7 @@ fn test_botan_crl_parse() {
 	ee.is_ca = IsCa::NoCa;
 	ee.serial_number = Some(SerialNumber::from(99999));
 	// Botan has a sanity check that enforces a maximum expiration date
-	ee.not_after = rcgen::date_time_ymd(3016, 01, 01);
+	ee.not_after = rcgen::date_time_ymd(3016, 1, 1);
 	let ee = Certificate::from_params(ee).unwrap();
 	let ee_der = ee.serialize_der_with_signer(&issuer).unwrap();
 	let botan_ee = botan::Certificate::load(ee_der.as_ref()).unwrap();

rcgen/tests/generic.rs

@@ -127,7 +127,7 @@ mod test_x509_custom_ext {
 			.get_extension_unique(&test_oid)
 			.expect("invalid extensions")
 			.expect("missing custom extension");
-		assert_eq!(favorite_drink_ext.critical, true);
+		assert!(favorite_drink_ext.critical);
 		assert_eq!(favorite_drink_ext.value, test_ext);
 
 		// Generate a CSR with the custom extension, parse it with x509-parser.
@@ -154,7 +154,7 @@ mod test_x509_custom_ext {
 			.iter()
 			.find(|ext| ext.oid == test_oid)
 			.expect("missing requested custom extension");
-		assert_eq!(custom_ext.critical, true);
+		assert!(custom_ext.critical);
 		assert_eq!(custom_ext.value, test_ext);
 	}
 }
@@ -223,7 +223,7 @@ mod test_x509_parser_crl {
 		// TODO: x509-parser does not yet parse the CRL issuing DP extension for further examination.
 
 		// We should be able to verify the CRL signature with the issuer.
-		assert!(x509_crl.verify_signature(&x509_issuer.public_key()).is_ok());
+		assert!(x509_crl.verify_signature(x509_issuer.public_key()).is_ok());
 	}
 }
 

rcgen/tests/openssl.rs

@@ -20,15 +20,15 @@ fn verify_cert_basic(cert: &Certificate) {
 	let cert_pem = cert.serialize_pem().unwrap();
 	println!("{cert_pem}");
 
-	let x509 = X509::from_pem(&cert_pem.as_bytes()).unwrap();
+	let x509 = X509::from_pem(cert_pem.as_bytes()).unwrap();
 	let mut builder = X509StoreBuilder::new().unwrap();
 	builder.add_cert(x509.clone()).unwrap();
 
 	let store: X509Store = builder.build();
 	let mut ctx = X509StoreContext::new().unwrap();
 	let mut stack = Stack::new().unwrap();
 	stack.push(x509.clone()).unwrap();
-	ctx.init(&store, &x509, &stack.as_ref(), |ctx| {
+	ctx.init(&store, &x509, stack.as_ref(), |ctx| {
 		ctx.verify_cert().unwrap();
 		Ok(())
 	})
@@ -79,7 +79,7 @@ impl Read for PipeEnd {
 	fn read(&mut self, mut buf: &mut [u8]) -> ioResult<usize> {
 		let inner = self.inner.borrow_mut();
 		let r_sl = &inner.0[1 - self.end_idx][self.read_pos..];
-		if r_sl.len() == 0 {
+		if r_sl.is_empty() {
 			return Err(Error::new(ErrorKind::WouldBlock, "oh no!"));
 		}
 		let r = buf.len().min(r_sl.len());
@@ -101,9 +101,9 @@ fn verify_cert_ca(cert_pem: &str, key: &[u8], ca_cert_pem: &str) {
 	println!("{cert_pem}");
 	println!("{ca_cert_pem}");
 
-	let x509 = X509::from_pem(&cert_pem.as_bytes()).unwrap();
+	let x509 = X509::from_pem(cert_pem.as_bytes()).unwrap();
 
-	let ca_x509 = X509::from_pem(&ca_cert_pem.as_bytes()).unwrap();
+	let ca_x509 = X509::from_pem(ca_cert_pem.as_bytes()).unwrap();
 
 	let mut builder = X509StoreBuilder::new().unwrap();
 	builder.add_cert(ca_x509).unwrap();
@@ -113,7 +113,7 @@ fn verify_cert_ca(cert_pem: &str, key: &[u8], ca_cert_pem: &str) {
 	let srv = SslMethod::tls_server();
 	let mut ssl_srv_ctx = SslAcceptor::mozilla_modern(srv).unwrap();
 	//let key = cert.serialize_private_key_der();
-	let pkey = PKey::private_key_from_der(&key).unwrap();
+	let pkey = PKey::private_key_from_der(key).unwrap();
 	ssl_srv_ctx.set_private_key(&pkey).unwrap();
 
 	ssl_srv_ctx.set_certificate(&x509).unwrap();
@@ -168,7 +168,7 @@ fn verify_csr(cert: &Certificate) {
 	let key = cert.serialize_private_key_der();
 	let pkey = PKey::private_key_from_der(&key).unwrap();
 
-	let req = X509Req::from_pem(&csr.as_bytes()).unwrap();
+	let req = X509Req::from_pem(csr.as_bytes()).unwrap();
 	req.verify(&pkey).unwrap();
 }
 
@@ -241,6 +241,7 @@ fn test_openssl_25519_v1_given() {
 	// Now verify the certificate as well as CSR,
 	// but only on OpenSSL >= 1.1.1
 	// On prior versions, only do basic verification
+	#[allow(clippy::unusual_byte_groupings)]
 	if openssl::version::number() >= 0x1_01_01_00_f {
 		verify_cert(&cert);
 		verify_csr(&cert);