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

Refactor/fix clippy errors #2056

Merged
merged 20 commits into from
Jan 10, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
allow-unwrap-in-tests = true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added this file to configure clippy.

Copy link
Member

Choose a reason for hiding this comment

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

May be a #![allow(clippy::unwrap_used, clippy::expect_used)] directive in test files would be better?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can do that. It might be better because clippy config file like clippy.toml is unstable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed
889d250

1 change: 0 additions & 1 deletion sea-orm-macros/src/strum/helpers/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
pub use self::case_style::CaseStyleHelpers;
pub use self::type_props::HasTypeProperties;
pub use self::variant_props::HasStrumVariantProperties;

Expand Down
18 changes: 9 additions & 9 deletions src/database/db_connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,23 +19,23 @@ use std::sync::Arc;
pub enum DatabaseConnection {
/// Create a MYSQL database connection and pool
#[cfg(feature = "sqlx-mysql")]
SqlxMySqlPoolConnection(crate::SqlxMySqlPoolConnection),
SqlxMySqlPoolConnection(crate::driver::SqlxMySqlPoolConnection),

/// Create a PostgreSQL database connection and pool
#[cfg(feature = "sqlx-postgres")]
SqlxPostgresPoolConnection(crate::SqlxPostgresPoolConnection),
SqlxPostgresPoolConnection(crate::driver::SqlxPostgresPoolConnection),

/// Create a SQLite database connection and pool
#[cfg(feature = "sqlx-sqlite")]
SqlxSqlitePoolConnection(crate::SqlxSqlitePoolConnection),
SqlxSqlitePoolConnection(crate::driver::SqlxSqlitePoolConnection),

/// Create a Mock database connection useful for testing
#[cfg(feature = "mock")]
MockDatabaseConnection(Arc<crate::MockDatabaseConnection>),
MockDatabaseConnection(Arc<crate::driver::MockDatabaseConnection>),

/// Create a Proxy database connection useful for proxying
#[cfg(feature = "proxy")]
ProxyDatabaseConnection(Arc<crate::ProxyDatabaseConnection>),
ProxyDatabaseConnection(Arc<crate::driver::ProxyDatabaseConnection>),

/// The connection to the database has been severed
Disconnected,
Expand Down Expand Up @@ -74,9 +74,9 @@ pub(crate) enum InnerConnection {
#[cfg(feature = "sqlx-sqlite")]
Sqlite(PoolConnection<sqlx::Sqlite>),
#[cfg(feature = "mock")]
Mock(Arc<crate::MockDatabaseConnection>),
Mock(Arc<crate::driver::MockDatabaseConnection>),
#[cfg(feature = "proxy")]
Proxy(Arc<crate::ProxyDatabaseConnection>),
Proxy(Arc<crate::driver::ProxyDatabaseConnection>),
}

impl std::fmt::Debug for DatabaseConnection {
Expand Down Expand Up @@ -397,7 +397,7 @@ impl DatabaseConnection {
/// # Panics
///
/// Panics if [DbConn] is not a mock connection.
pub fn as_mock_connection(&self) -> &crate::MockDatabaseConnection {
pub fn as_mock_connection(&self) -> &crate::driver::MockDatabaseConnection {
match self {
DatabaseConnection::MockDatabaseConnection(mock_conn) => mock_conn,
_ => panic!("Not mock connection"),
Expand Down Expand Up @@ -426,7 +426,7 @@ impl DatabaseConnection {
/// # Panics
///
/// Panics if [DbConn] is not a proxy connection.
pub fn as_proxy_connection(&self) -> &crate::ProxyDatabaseConnection {
pub fn as_proxy_connection(&self) -> &crate::driver::ProxyDatabaseConnection {
match self {
DatabaseConnection::ProxyDatabaseConnection(proxy_conn) => proxy_conn,
_ => panic!("Not proxy connection"),
Expand Down
7 changes: 4 additions & 3 deletions src/database/mock.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use crate::{
error::*, DatabaseConnection, DbBackend, EntityTrait, ExecResult, ExecResultHolder, Iden,
IdenStatic, Iterable, MockDatabaseConnection, MockDatabaseTrait, ModelTrait, QueryResult,
QueryResultRow, SelectA, SelectB, Statement,
driver::{MockDatabaseConnection, MockDatabaseTrait},
error::*,
DatabaseConnection, DbBackend, EntityTrait, ExecResult, ExecResultHolder, Iden, IdenStatic,
Iterable, ModelTrait, QueryResult, QueryResultRow, SelectA, SelectB, Statement,
};
use sea_query::{Value, ValueType, Values};
use std::{collections::BTreeMap, sync::Arc};
Expand Down
16 changes: 8 additions & 8 deletions src/database/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,19 +71,19 @@ impl Database {

#[cfg(feature = "sqlx-mysql")]
if DbBackend::MySql.is_prefix_of(&opt.url) {
return crate::SqlxMySqlConnector::connect(opt).await;
return crate::driver::SqlxMySqlConnector::connect(opt).await;
}
#[cfg(feature = "sqlx-postgres")]
if DbBackend::Postgres.is_prefix_of(&opt.url) {
return crate::SqlxPostgresConnector::connect(opt).await;
return crate::driver::SqlxPostgresConnector::connect(opt).await;
}
#[cfg(feature = "sqlx-sqlite")]
if DbBackend::Sqlite.is_prefix_of(&opt.url) {
return crate::SqlxSqliteConnector::connect(opt).await;
return crate::driver::SqlxSqliteConnector::connect(opt).await;
}
#[cfg(feature = "mock")]
if crate::MockDatabaseConnector::accepts(&opt.url) {
return crate::MockDatabaseConnector::connect(&opt.url).await;
if crate::driver::MockDatabaseConnector::accepts(&opt.url) {
return crate::driver::MockDatabaseConnector::connect(&opt.url).await;
}

Err(conn_err(format!(
Expand All @@ -101,19 +101,19 @@ impl Database {
) -> Result<DatabaseConnection, DbErr> {
match db_type {
DbBackend::MySql => {
return crate::ProxyDatabaseConnector::connect(
return crate::driver::ProxyDatabaseConnector::connect(
DbBackend::MySql,
proxy_func_arc.to_owned(),
);
}
DbBackend::Postgres => {
return crate::ProxyDatabaseConnector::connect(
return crate::driver::ProxyDatabaseConnector::connect(
DbBackend::Postgres,
proxy_func_arc.to_owned(),
);
}
DbBackend::Sqlite => {
return crate::ProxyDatabaseConnector::connect(
return crate::driver::ProxyDatabaseConnector::connect(
DbBackend::Sqlite,
proxy_func_arc.to_owned(),
);
Expand Down
8 changes: 4 additions & 4 deletions src/database/stream/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,14 +89,14 @@ impl
#[cfg(feature = "mock")]
impl
From<(
Arc<crate::MockDatabaseConnection>,
Arc<crate::driver::MockDatabaseConnection>,
Statement,
Option<crate::metric::Callback>,
)> for QueryStream
{
fn from(
(conn, stmt, metric_callback): (
Arc<crate::MockDatabaseConnection>,
Arc<crate::driver::MockDatabaseConnection>,
Statement,
Option<crate::metric::Callback>,
),
Expand All @@ -108,14 +108,14 @@ impl
#[cfg(feature = "proxy")]
impl
From<(
Arc<crate::ProxyDatabaseConnection>,
Arc<crate::driver::ProxyDatabaseConnection>,
Statement,
Option<crate::metric::Callback>,
)> for QueryStream
{
fn from(
(conn, stmt, metric_callback): (
Arc<crate::ProxyDatabaseConnection>,
Arc<crate::driver::ProxyDatabaseConnection>,
Statement,
Option<crate::metric::Callback>,
),
Expand Down
8 changes: 4 additions & 4 deletions src/database/transaction.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
#[cfg(feature = "sqlx-dep")]
use crate::driver::{sqlx_error_to_exec_err, sqlx_error_to_query_err};
use crate::{
debug_print, error::*, AccessMode, ConnectionTrait, DbBackend, DbErr, ExecResult,
InnerConnection, IsolationLevel, QueryResult, Statement, StreamTrait, TransactionStream,
TransactionTrait,
};
#[cfg(feature = "sqlx-dep")]
use crate::{sqlx_error_to_exec_err, sqlx_error_to_query_err};
use futures::lock::Mutex;
#[cfg(feature = "sqlx-dep")]
use sqlx::{pool::PoolConnection, TransactionManager};
Expand Down Expand Up @@ -81,7 +81,7 @@ impl DatabaseTransaction {

#[cfg(feature = "mock")]
pub(crate) async fn new_mock(
inner: Arc<crate::MockDatabaseConnection>,
inner: Arc<crate::driver::MockDatabaseConnection>,
metric_callback: Option<crate::metric::Callback>,
) -> Result<DatabaseTransaction, DbErr> {
let backend = inner.get_database_backend();
Expand All @@ -97,7 +97,7 @@ impl DatabaseTransaction {

#[cfg(feature = "proxy")]
pub(crate) async fn new_proxy(
inner: Arc<crate::ProxyDatabaseConnection>,
inner: Arc<crate::driver::ProxyDatabaseConnection>,
metric_callback: Option<crate::metric::Callback>,
) -> Result<DatabaseTransaction, DbErr> {
let backend = inner.get_database_backend();
Expand Down
10 changes: 7 additions & 3 deletions src/driver/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,22 +84,24 @@ impl MockDatabaseConnector {
}

#[cfg(feature = "sqlx-mysql")]
if crate::SqlxMySqlConnector::accepts(string) {
if crate::driver::SqlxMySqlConnector::accepts(string) {
return connect_mock_db!(DbBackend::MySql);
}
#[cfg(feature = "sqlx-postgres")]
if crate::SqlxPostgresConnector::accepts(string) {
if crate::driver::SqlxPostgresConnector::accepts(string) {
return connect_mock_db!(DbBackend::Postgres);
}
#[cfg(feature = "sqlx-sqlite")]
if crate::SqlxSqliteConnector::accepts(string) {
if crate::driver::SqlxSqliteConnector::accepts(string) {
return connect_mock_db!(DbBackend::Sqlite);
}
connect_mock_db!(DbBackend::Postgres)
}
}

impl MockDatabaseConnection {
/// # Panics
///
/// Create a connection to the [MockDatabase]
pub fn new<M: 'static>(m: M) -> Self
where
Expand All @@ -116,6 +118,8 @@ impl MockDatabaseConnection {
&self.mocker
}

/// # Panics
///
/// Get the [DatabaseBackend](crate::DatabaseBackend) being used by the [MockDatabase]
pub fn get_database_backend(&self) -> DbBackend {
self.mocker
Expand Down
11 changes: 1 addition & 10 deletions src/driver/sqlx_mysql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ impl std::fmt::Debug for SqlxMySqlPoolConnection {

impl SqlxMySqlConnector {
/// Check if the URI provided corresponds to `mysql://` for a MySQL database
#[cfg(feature = "mock")]
Copy link
Member

@tyt2y3 tyt2y3 Jan 10, 2024

Choose a reason for hiding this comment

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

This is what confuses me, this accepts method should be in the public API, why does clippy think it's redundant?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that is because the accepts method is not used from anywhere in this workspace except for mock. But if we want this method to be public, we need #[allow(unused_variables)] instead so I will replace them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove feature flag and I put #[allow(unused_variables)] instead
09f845a

pub fn accepts(string: &str) -> bool {
string.starts_with("mysql://") && string.parse::<MySqlConnectOptions>().is_ok()
}
Expand Down Expand Up @@ -65,16 +66,6 @@ impl SqlxMySqlConnector {
}
}

impl SqlxMySqlConnector {
/// Instantiate a sqlx pool connection to a [DatabaseConnection]
pub fn from_sqlx_mysql_pool(pool: MySqlPool) -> DatabaseConnection {
DatabaseConnection::SqlxMySqlPoolConnection(SqlxMySqlPoolConnection {
pool,
metric_callback: None,
})
}
}

impl SqlxMySqlPoolConnection {
/// Execute a [Statement] on a MySQL backend
#[instrument(level = "trace")]
Expand Down
11 changes: 1 addition & 10 deletions src/driver/sqlx_postgres.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ impl std::fmt::Debug for SqlxPostgresPoolConnection {

impl SqlxPostgresConnector {
/// Check if the URI provided corresponds to `postgres://` for a PostgreSQL database
#[cfg(feature = "mock")]
pub fn accepts(string: &str) -> bool {
string.starts_with("postgres://") && string.parse::<PgConnectOptions>().is_ok()
}
Expand Down Expand Up @@ -80,16 +81,6 @@ impl SqlxPostgresConnector {
}
}

impl SqlxPostgresConnector {
/// Instantiate a sqlx pool connection to a [DatabaseConnection]
pub fn from_sqlx_postgres_pool(pool: PgPool) -> DatabaseConnection {
DatabaseConnection::SqlxPostgresPoolConnection(SqlxPostgresPoolConnection {
pool,
metric_callback: None,
})
}
}

impl SqlxPostgresPoolConnection {
/// Execute a [Statement] on a PostgreSQL backend
#[instrument(level = "trace")]
Expand Down
11 changes: 1 addition & 10 deletions src/driver/sqlx_sqlite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ impl std::fmt::Debug for SqlxSqlitePoolConnection {

impl SqlxSqliteConnector {
/// Check if the URI provided corresponds to `sqlite:` for a SQLite database
#[cfg(feature = "mock")]
pub fn accepts(string: &str) -> bool {
string.starts_with("sqlite:") && string.parse::<SqliteConnectOptions>().is_ok()
}
Expand Down Expand Up @@ -72,16 +73,6 @@ impl SqlxSqliteConnector {
}
}

impl SqlxSqliteConnector {
/// Instantiate a sqlx pool connection to a [DatabaseConnection]
pub fn from_sqlx_sqlite_pool(pool: SqlitePool) -> DatabaseConnection {
DatabaseConnection::SqlxSqlitePoolConnection(SqlxSqlitePoolConnection {
pool,
metric_callback: None,
})
}
}

impl SqlxSqlitePoolConnection {
/// Execute a [Statement] on a SQLite backend
#[instrument(level = "trace")]
Expand Down
1 change: 0 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,6 @@ pub mod tests_cfg;
mod util;

pub use database::*;
pub use driver::*;
Copy link
Member

@tyt2y3 tyt2y3 Jan 10, 2024

Choose a reason for hiding this comment

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

Instead of removing this line, I think we should add #[allow(clippy::unused_xxx)], because the content of the re-export depends on feature flags

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed
138b9dc

I will add #[allow(clippy::unused_xxx)] later because I could not see clippy error in my local somehow...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added #[allow(unused_imports)]
20dc881

pub use entity::*;
pub use error::*;
pub use executor::*;
Expand Down
1 change: 0 additions & 1 deletion src/query/combine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use crate::{
ColumnTrait, EntityTrait, IdenStatic, Iterable, QueryTrait, Select, SelectTwo, SelectTwoMany,
};
use core::marker::PhantomData;
pub use sea_query::JoinType;
Copy link
Member

Choose a reason for hiding this comment

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

can confirm this is already exported in query.rs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, it is not exported so I have reverted this change. e4f4a12

use sea_query::{Alias, ColumnRef, Iden, Order, SeaRc, SelectExpr, SelectStatement, SimpleExpr};

macro_rules! select_def {
Expand Down
1 change: 0 additions & 1 deletion src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ pub use combine::{SelectA, SelectB};
pub use delete::*;
pub use helper::*;
pub use insert::*;
pub use join::*;
Copy link
Member

Choose a reason for hiding this comment

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

The file contains only impl, so nothing to export

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In that case, do we need to keep pub use join::*;?

#[cfg(feature = "with-json")]
pub use json::*;
pub use loader::*;
Expand Down
1 change: 0 additions & 1 deletion src/query/select.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use crate::{ColumnTrait, EntityTrait, Iterable, QueryFilter, QueryOrder, QuerySelect, QueryTrait};
use core::fmt::Debug;
use core::marker::PhantomData;
pub use sea_query::JoinType;
use sea_query::{Expr, IntoColumnRef, SelectStatement, SimpleExpr};

/// Defines a structure to perform select operations
Expand Down
Loading
Loading