Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: address Rust 1.72 clippy lints #2011

Merged
merged 13 commits into from
Aug 28, 2023
Merged
2 changes: 1 addition & 1 deletion .cargo/config.toml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
[target.'cfg(all())']
[build]
rustflags = [
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With [target.'cfg(all())'], all allowed lints (such as as conversions, missing panics docs, etc.) were somehow still raising warnings.

I'm not sure if there are any cons of using the build table for this purpose, I'll raise this issue on EmbarkStudios/rust-ecosystem#59, will update this as required.

"-Funsafe_code",
"-Wclippy::as_conversions",
Expand Down
40 changes: 34 additions & 6 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions crates/data_models/src/errors.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
pub type StorageResult<T> = error_stack::Result<T, StorageError>;

#[derive(Debug, thiserror::Error)]
pub enum StorageError {
#[error("Initialization Error")]
InitializationError,
// TODO: deprecate this error type to use a domain error instead
#[error("DatabaseError: {0:?}")]
DatabaseError(String),
Expand Down
4 changes: 4 additions & 0 deletions crates/drainer/src/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ pub async fn redis_connection(
.expect("Failed to create Redis connection Pool")
}

// TODO: use stores defined in storage_impl instead
/// # Panics
///
/// Will panic if could not create a db pool
#[allow(clippy::expect_used)]
pub async fn diesel_make_pg_pool(
database: &Database,
Expand Down
1 change: 1 addition & 0 deletions crates/drainer/src/services.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ pub struct StoreConfig {
}

impl Store {
#[allow(clippy::expect_used)]
pub async fn new(config: &crate::settings::Settings, test_transaction: bool) -> Self {
Self {
master_pool: diesel_make_pg_pool(
Expand Down
4 changes: 4 additions & 0 deletions crates/router/src/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ pub type PgPool = bb8::Pool<async_bb8_diesel::ConnectionManager<PgConnection>>;

pub type PgPooledConn = async_bb8_diesel::Connection<PgConnection>;

///
/// # Panics
///
/// Panics if failed to create a redis pool
#[allow(clippy::expect_used)]
pub async fn redis_connection(
conf: &crate::configs::settings::Settings,
Expand Down
2 changes: 1 addition & 1 deletion crates/router/src/connector/checkout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -848,7 +848,7 @@ impl api::FileUpload for Checkout {
match purpose {
api::FilePurpose::DisputeEvidence => {
let supported_file_types =
vec!["image/jpeg", "image/jpg", "image/png", "application/pdf"];
["image/jpeg", "image/jpg", "image/png", "application/pdf"];
// 4 Megabytes (MB)
if file_size > 4000000 {
Err(errors::ConnectorError::FileValidationFailed {
Expand Down
2 changes: 1 addition & 1 deletion crates/router/src/connector/stripe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1311,7 +1311,7 @@ impl api::FileUpload for Stripe {
) -> CustomResult<(), errors::ConnectorError> {
match purpose {
api::FilePurpose::DisputeEvidence => {
let supported_file_types = vec!["image/jpeg", "image/png", "application/pdf"];
let supported_file_types = ["image/jpeg", "image/png", "application/pdf"];
// 5 Megabytes (MB)
if file_size > 5000000 {
Err(errors::ConnectorError::FileValidationFailed {
Expand Down
6 changes: 1 addition & 5 deletions crates/router/src/core/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -599,11 +599,7 @@ pub mod error_stack_parsing {
attachments: current_error.attachments,
}]
.into_iter()
.chain(
Into::<VecLinearErrorStack<'a>>::into(current_error.sources)
.0
.into_iter(),
)
.chain(Into::<VecLinearErrorStack<'a>>::into(current_error.sources).0)
})
.collect();
Self(multi_layered_errors)
Expand Down
2 changes: 1 addition & 1 deletion crates/router/src/core/payment_methods/cards.rs
Original file line number Diff line number Diff line change
Expand Up @@ -754,7 +754,7 @@ pub fn get_banks(

fn get_val(str: String, val: &serde_json::Value) -> Option<String> {
str.split('.')
.fold(Some(val), |acc, x| acc.and_then(|v| v.get(x)))
.try_fold(val, |acc, x| acc.get(x))
.and_then(|v| v.as_str())
.map(|s| s.to_string())
}
Expand Down
10 changes: 6 additions & 4 deletions crates/router/src/core/payments/operations/payment_status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ pub async fn get_payment_intent_payment_attempt(
merchant_id: &str,
storage_scheme: enums::MerchantStorageScheme,
) -> RouterResult<(storage::PaymentIntent, storage::PaymentAttempt)> {
(|| async {
let get_pi_pa = || async {
let (pi, pa);
match payment_id {
api_models::payments::PaymentIdType::PaymentIntentId(ref id) => {
Expand Down Expand Up @@ -482,7 +482,9 @@ pub async fn get_payment_intent_payment_attempt(
}
}
error_stack::Result::<_, errors::DataStorageError>::Ok((pi, pa))
})()
.await
.to_not_found_response(errors::ApiErrorResponse::PaymentNotFound)
};

get_pi_pa()
.await
.to_not_found_response(errors::ApiErrorResponse::PaymentNotFound)
}
2 changes: 1 addition & 1 deletion crates/router/src/core/refunds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -842,7 +842,7 @@ pub async fn sync_refund_with_gateway_workflow(
},
)
.await?;
let terminal_status = vec![
let terminal_status = [
enums::RefundStatus::Success,
enums::RefundStatus::Failure,
enums::RefundStatus::TransactionFailure,
Expand Down
14 changes: 9 additions & 5 deletions crates/router/src/routes/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ impl AppStateInfo for AppState {
}

impl AppState {
/// # Panics
///
/// Panics if Store can't be created or JWE decryption fails
#[allow(clippy::expect_used)]
pub async fn with_storage(
conf: settings::Settings,
storage_impl: StorageImpl,
Expand All @@ -68,14 +72,15 @@ impl AppState {
let kms_client = kms::get_kms_client(&conf.kms).await;
let testable = storage_impl == StorageImpl::PostgresqlTest;
let store: Box<dyn StorageInterface> = match storage_impl {
StorageImpl::Postgresql | StorageImpl::PostgresqlTest => {
Box::new(get_store(&conf, shut_down_signal, testable).await)
}
StorageImpl::Postgresql | StorageImpl::PostgresqlTest => Box::new(
get_store(&conf, shut_down_signal, testable)
.await
.expect("Failed to create store"),
),
StorageImpl::Mock => Box::new(MockDb::new(&conf).await),
};

#[cfg(feature = "kms")]
#[allow(clippy::expect_used)]
let kms_secrets = settings::ActiveKmsSecrets {
jwekey: conf.jwekey.clone().into(),
}
Expand All @@ -84,7 +89,6 @@ impl AppState {
.expect("Failed while performing KMS decryption");

#[cfg(feature = "email")]
#[allow(clippy::expect_used)]
let email_client = Box::new(AwsSes::new(&conf.email).await);
Self {
flow_name: String::from("default"),
Expand Down
2 changes: 1 addition & 1 deletion crates/router/src/scheduler/workflows/payment_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ impl ProcessTrackerWorkflow for PaymentsSyncWorkflow {
)
.await?;

let terminal_status = vec![
let terminal_status = [
enums::AttemptStatus::RouterDeclined,
enums::AttemptStatus::Charged,
enums::AttemptStatus::AutoRefunded,
Expand Down
19 changes: 11 additions & 8 deletions crates/router/src/services.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ pub mod authentication;
pub mod encryption;
pub mod logger;

#[cfg(feature = "kms")]
use data_models::errors::StorageError;
use data_models::errors::StorageResult;
use error_stack::{IntoReport, ResultExt};
#[cfg(feature = "kms")]
use external_services::kms::{self, decrypt::KmsDecrypt};
Expand Down Expand Up @@ -31,29 +34,29 @@ pub async fn get_store(
config: &settings::Settings,
shut_down_signal: oneshot::Sender<()>,
test_transaction: bool,
) -> Store {
) -> StorageResult<Store> {
#[cfg(feature = "kms")]
let kms_client = kms::get_kms_client(&config.kms).await;

#[cfg(feature = "kms")]
#[allow(clippy::expect_used)]
let master_config = config
.master_database
.clone()
.decrypt_inner(kms_client)
.await
.expect("Failed to decrypt master database config");
.change_context(StorageError::InitializationError)
.attach_printable("Failed to decrypt master database config")?;
#[cfg(not(feature = "kms"))]
let master_config = config.master_database.clone().into();

#[cfg(all(feature = "olap", feature = "kms"))]
#[allow(clippy::expect_used)]
let replica_config = config
.replica_database
.clone()
.decrypt_inner(kms_client)
.await
.expect("Failed to decrypt replica database config");
.change_context(StorageError::InitializationError)
.attach_printable("Failed to decrypt replica database config")?;

#[cfg(all(feature = "olap", not(feature = "kms")))]
let replica_config = config.replica_database.clone().into();
Expand All @@ -70,7 +73,7 @@ pub async fn get_store(
let conf = (master_config, replica_config);

let store: RouterStore<StoreType> = if test_transaction {
RouterStore::test_store(conf, &config.redis, master_enc_key).await
RouterStore::test_store(conf, &config.redis, master_enc_key).await?
} else {
RouterStore::from_config(
conf,
Expand All @@ -79,7 +82,7 @@ pub async fn get_store(
shut_down_signal,
consts::PUB_SUB_CHANNEL,
)
.await
.await?
};

#[cfg(feature = "kv_store")]
Expand All @@ -89,7 +92,7 @@ pub async fn get_store(
config.drainer.num_partitions,
);

store
Ok(store)
}

#[allow(clippy::expect_used)]
Expand Down
10 changes: 4 additions & 6 deletions crates/router/src/services/api/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,8 +275,8 @@ impl HeaderExt for Headers {
) -> CustomResult<reqwest::header::HeaderMap, errors::ApiClientError> {
use reqwest::header::{HeaderMap, HeaderName, HeaderValue};

self.into_iter().fold(
Ok(HeaderMap::new()),
self.into_iter().try_fold(
HeaderMap::new(),
|mut header_map, (header_name, header_value)| {
let header_name = HeaderName::from_str(&header_name)
.into_report()
Expand All @@ -285,10 +285,8 @@ impl HeaderExt for Headers {
let header_value = HeaderValue::from_str(&header_value)
.into_report()
.change_context(errors::ApiClientError::HeaderMapConstructionFailed)?;
if let Ok(map) = header_map.as_mut() {
map.append(header_name, header_value);
}
header_map
header_map.append(header_name, header_value);
Ok(header_map)
},
)
}
Expand Down
2 changes: 1 addition & 1 deletion crates/router/tests/connectors/adyen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ async fn should_fail_payment_for_invalid_exp_month() {
)
.await
.unwrap();
let errors = vec!["The provided Expiry Date is not valid.: Expiry month should be between 1 and 12 inclusive: 20","Refused"];
let errors = ["The provided Expiry Date is not valid.: Expiry month should be between 1 and 12 inclusive: 20","Refused"];
assert!(errors.contains(&response.response.unwrap_err().message.as_str()))
}

Expand Down
2 changes: 1 addition & 1 deletion crates/router/tests/payments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ async fn payments_todo() {
let client = awc::Client::default();
let mut response;
let mut response_body;
let _post_endpoints = vec!["123/update", "123/confirm", "cancel"];
let _post_endpoints = ["123/update", "123/confirm", "cancel"];
let get_endpoints = vec!["list"];

for endpoint in get_endpoints {
Expand Down
Loading
Loading