From 90b97007484e782994120587e751108819b7aa50 Mon Sep 17 00:00:00 2001 From: Piotr Maks Date: Mon, 31 Aug 2020 12:04:57 +0200 Subject: [PATCH] Clippy (#146) * Bump minimum required Rust version to 1.40 Motivation for this change is use of `#[non_exhaustive]` attribute that was stabilized in Rust 1.40.0 * Migrate benchmarks to criterion Remove use of unstable features in favour of criterion benchmarks * Enable clippy in Github Actions * Fix clippy::manual_non_exhaustive Remove manual implementations of the non-exhaustive pattern. Instead use the `#[non_exhaustive]` attribute. * Allow reexport of deprecated function Silence rustc `deprecated` working in function reexport * Remove redundant clone * Fix various clippy warnings * Remove redundant pattern * Use `unreachable!()` to fail test * No need to add `&` to all patterns --- .github/workflows/ci.yml | 18 ++++++++++++++- Cargo.toml | 5 +++++ README.md | 2 +- benches/jwt.rs | 23 +++++++++++-------- examples/custom_chrono.rs | 2 +- src/errors.rs | 15 +++++-------- src/lib.rs | 6 +++-- src/validation.rs | 47 ++++++++++++++++++--------------------- 8 files changed, 69 insertions(+), 49 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ec4f4cb..d4ef727 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -26,6 +26,22 @@ jobs: command: fmt args: -- --check + clippy: + name: Clippy + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: stable + override: true + components: clippy + - uses: actions-rs/cargo@v1 + with: + command: clippy + args: --all-targets --all-features -- -D warnings + tests: name: Tests runs-on: ${{ matrix.os }} @@ -35,7 +51,7 @@ jobs: include: - build: pinned os: ubuntu-18.04 - rust: 1.39.0 + rust: 1.40.0 - build: stable os: ubuntu-18.04 rust: stable diff --git a/Cargo.toml b/Cargo.toml index 5b88897..b7d8fe5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,6 +23,11 @@ simple_asn1 = "0.4" [dev-dependencies] # For the custom chrono example chrono = "0.4" +criterion = "0.3" + +[[bench]] +name = "jwt" +harness = false [badges] maintenance = { status = "passively-maintained" } diff --git a/README.md b/README.md index 254eff1..eb74474 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ jsonwebtoken = "7" serde = {version = "1.0", features = ["derive"] } ``` -The minimum required Rust version is 1.39. +The minimum required Rust version is 1.40. ## Algorithms This library currently supports the following: diff --git a/benches/jwt.rs b/benches/jwt.rs index 41a4fc9..640b53c 100644 --- a/benches/jwt.rs +++ b/benches/jwt.rs @@ -1,6 +1,4 @@ -#![feature(test)] -extern crate test; - +use criterion::{black_box, criterion_group, criterion_main, Criterion}; use jsonwebtoken::{decode, encode, DecodingKey, EncodingKey, Header, Validation}; use serde::{Deserialize, Serialize}; @@ -10,18 +8,25 @@ struct Claims { company: String, } -#[bench] -fn bench_encode(b: &mut test::Bencher) { +fn bench_encode(c: &mut Criterion) { let claim = Claims { sub: "b@b.com".to_owned(), company: "ACME".to_owned() }; let key = EncodingKey::from_secret("secret".as_ref()); - b.iter(|| encode(&Header::default(), &claim, &key)); + c.bench_function("bench_encode", |b| { + b.iter(|| encode(black_box(&Header::default()), black_box(&claim), black_box(&key))) + }); } -#[bench] -fn bench_decode(b: &mut test::Bencher) { +fn bench_decode(c: &mut Criterion) { let token = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWV9.TJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQ"; let key = DecodingKey::from_secret("secret".as_ref()); - b.iter(|| decode::(token, &key, &Validation::default())); + c.bench_function("bench_decode", |b| { + b.iter(|| { + decode::(black_box(token), black_box(&key), black_box(&Validation::default())) + }) + }); } + +criterion_group!(benches, bench_encode, bench_decode); +criterion_main!(benches); diff --git a/examples/custom_chrono.rs b/examples/custom_chrono.rs index 081cad4..a5950d4 100644 --- a/examples/custom_chrono.rs +++ b/examples/custom_chrono.rs @@ -126,7 +126,7 @@ fn main() -> Result<(), Box> { let iat = Utc::now(); let exp = iat + chrono::Duration::days(1); - let claims = Claims::new(sub.clone(), iat, exp); + let claims = Claims::new(sub, iat, exp); let token = jsonwebtoken::encode( &Header::default(), diff --git a/src/errors.rs b/src/errors.rs index d54d0cb..c7e7ed6 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -27,6 +27,11 @@ impl Error { } /// The specific type of an error. +/// +/// This enum may grow additional variants, the `#[non_exhaustive]` +/// attribute makes sure clients don't count on exhaustive matching. +/// (Otherwise, adding a new variant could break existing code.) +#[non_exhaustive] #[derive(Debug)] pub enum ErrorKind { /// When a token doesn't have a valid JWT shape @@ -66,14 +71,6 @@ pub enum ErrorKind { Utf8(::std::string::FromUtf8Error), /// Something unspecified went wrong with crypto Crypto(::ring::error::Unspecified), - - /// Hints that destructuring should not be exhaustive. - /// - /// This enum may grow additional variants, so this makes sure clients - /// don't count on exhaustive matching. (Otherwise, adding a new variant - /// could break existing code.) - #[doc(hidden)] - __Nonexhaustive, } impl StdError for Error { @@ -95,7 +92,6 @@ impl StdError for Error { ErrorKind::Json(ref err) => Some(err), ErrorKind::Utf8(ref err) => Some(err), ErrorKind::Crypto(ref err) => Some(err), - ErrorKind::__Nonexhaustive => None, } } } @@ -119,7 +115,6 @@ impl fmt::Display for Error { ErrorKind::Utf8(ref err) => write!(f, "UTF-8 error: {}", err), ErrorKind::Crypto(ref err) => write!(f, "Crypto error: {}", err), ErrorKind::Base64(ref err) => write!(f, "Base64 error: {}", err), - ErrorKind::__Nonexhaustive => write!(f, "Unknown error"), } } } diff --git a/src/lib.rs b/src/lib.rs index 9e86000..41b10f9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -16,9 +16,11 @@ mod serialization; mod validation; pub use algorithms::Algorithm; +#[allow(deprecated)] +pub use decoding::dangerous_unsafe_decode; pub use decoding::{ - dangerous_insecure_decode, dangerous_insecure_decode_with_validation, dangerous_unsafe_decode, - decode, decode_header, DecodingKey, TokenData, + dangerous_insecure_decode, dangerous_insecure_decode_with_validation, decode, decode_header, + DecodingKey, TokenData, }; pub use encoding::{encode, EncodingKey}; pub use header::Header; diff --git a/src/validation.rs b/src/validation.rs index b6805aa..4cf7726 100644 --- a/src/validation.rs +++ b/src/validation.rs @@ -194,8 +194,8 @@ mod tests { assert!(res.is_err()); match res.unwrap_err().kind() { - &ErrorKind::ExpiredSignature => (), - _ => assert!(false), + ErrorKind::ExpiredSignature => (), + _ => unreachable!(), }; } @@ -215,8 +215,8 @@ mod tests { let res = validate(&claims, &Validation::default()); assert!(res.is_err()); match res.unwrap_err().kind() { - &ErrorKind::ExpiredSignature => (), - _ => assert!(false), + ErrorKind::ExpiredSignature => (), + _ => unreachable!(), }; } @@ -240,8 +240,8 @@ mod tests { assert!(res.is_err()); match res.unwrap_err().kind() { - &ErrorKind::ImmatureSignature => (), - _ => assert!(false), + ErrorKind::ImmatureSignature => (), + _ => unreachable!(), }; } @@ -285,8 +285,8 @@ mod tests { assert!(res.is_err()); match res.unwrap_err().kind() { - &ErrorKind::InvalidIssuer => (), - _ => assert!(false), + ErrorKind::InvalidIssuer => (), + _ => unreachable!(), }; } @@ -302,8 +302,8 @@ mod tests { assert!(res.is_err()); match res.unwrap_err().kind() { - &ErrorKind::InvalidIssuer => (), - _ => assert!(false), + ErrorKind::InvalidIssuer => (), + _ => unreachable!(), }; } @@ -333,8 +333,8 @@ mod tests { assert!(res.is_err()); match res.unwrap_err().kind() { - &ErrorKind::InvalidSubject => (), - _ => assert!(false), + ErrorKind::InvalidSubject => (), + _ => unreachable!(), }; } @@ -350,8 +350,8 @@ mod tests { assert!(res.is_err()); match res.unwrap_err().kind() { - &ErrorKind::InvalidSubject => (), - _ => assert!(false), + ErrorKind::InvalidSubject => (), + _ => unreachable!(), }; } @@ -385,8 +385,8 @@ mod tests { assert!(res.is_err()); match res.unwrap_err().kind() { - &ErrorKind::InvalidAudience => (), - _ => assert!(false), + ErrorKind::InvalidAudience => (), + _ => unreachable!(), }; } @@ -400,8 +400,8 @@ mod tests { assert!(res.is_err()); match res.unwrap_err().kind() { - &ErrorKind::InvalidAudience => (), - _ => assert!(false), + ErrorKind::InvalidAudience => (), + _ => unreachable!(), }; } @@ -414,8 +414,8 @@ mod tests { assert!(res.is_err()); match res.unwrap_err().kind() { - &ErrorKind::InvalidAudience => (), - _ => assert!(false), + ErrorKind::InvalidAudience => (), + _ => unreachable!(), }; } @@ -435,11 +435,8 @@ mod tests { // It errors because it needs to validate iss/sub which are missing assert!(res.is_err()); match res.unwrap_err().kind() { - &ErrorKind::InvalidIssuer => (), - t @ _ => { - println!("{:?}", t); - assert!(false) - } + ErrorKind::InvalidIssuer => (), + t => panic!("{:?}", t), }; }