Skip to content

Commit 76fe15e

Browse files
cursoragentlovasoa
andcommitted
Refactor: Improve sqlx-core and sqlx-macros code quality
This commit includes several refactorings and improvements across sqlx-core and sqlx-macros. Key changes include: - **sqlx-core:** - Added `#[cfg(feature = "sqlite")]` guards to `DebugFn` and its implementations to conditionally compile them only when the SQLite feature is enabled. - Removed unused `write` method from `buf_stream.rs` and added `#[allow(dead_code)]` to other unused methods. - Derived `Default` for `MySqlSslMode` and removed manual implementation. - Added `#[allow(dead_code)]` to `warnings` field in `EofPacket`. - Improved `f32` decoding for MySQL to clamp values to the `f32` range before casting. - Corrected `is_null` implementation in `mysql/value.rs` to use `self.value` directly. - Added `#[allow(dead_code)]` to `local_addr` in `net/socket.rs`. - Fixed potential issues with TLS stream upgrading and downgrading in `net/tls/mod.rs`, including adding `#[allow(dead_code)]` to `downgrade`. - Removed generic `DB: Database` from `deadline_as_timeout` in `pool/inner.rs` and `pool/mod.rs` as it was unused. - **sqlx-macros:** - Wrapped `Type` in `Box` for `RecordType::Given` and `ColumnTypeOverride::Exact` to avoid potential stack overflows with deeply nested types. - Corrected the `migrations_path` resolution in `test_attr.rs` to use the span of the input signature. - Added `#[allow(dead_code)]` to `Args` and `MigrationsOpt` structs in `test_attr.rs`. - Fixed a minor formatting issue in the documentation for `query!()`. Co-authored-by: contact <[email protected]>
1 parent 282f1dd commit 76fe15e

File tree

16 files changed

+48
-38
lines changed

16 files changed

+48
-38
lines changed

sqlx-core/src/common/mod.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
11
mod statement_cache;
22

33
pub(crate) use statement_cache::StatementCache;
4+
#[cfg(feature = "sqlite")]
45
use std::fmt::{Debug, Formatter};
6+
#[cfg(feature = "sqlite")]
57
use std::ops::{Deref, DerefMut};
68

79
/// A wrapper for `Fn`s that provides a debug impl that just says "Function"
10+
#[cfg(feature = "sqlite")]
811
pub(crate) struct DebugFn<F: ?Sized>(pub F);
912

13+
#[cfg(feature = "sqlite")]
1014
impl<F: ?Sized> Deref for DebugFn<F> {
1115
type Target = F;
1216

@@ -15,12 +19,14 @@ impl<F: ?Sized> Deref for DebugFn<F> {
1519
}
1620
}
1721

22+
#[cfg(feature = "sqlite")]
1823
impl<F: ?Sized> DerefMut for DebugFn<F> {
1924
fn deref_mut(&mut self) -> &mut Self::Target {
2025
&mut self.0
2126
}
2227
}
2328

29+
#[cfg(feature = "sqlite")]
2430
impl<F: ?Sized> Debug for DebugFn<F> {
2531
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
2632
f.debug_tuple("Function").finish()

sqlx-core/src/io/buf_stream.rs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,7 @@ where
3535
}
3636
}
3737

38-
pub fn write<'en, T>(&mut self, value: T)
39-
where
40-
T: Encode<'en, ()>,
41-
{
42-
self.write_with(value, ())
43-
}
38+
// Note: simple `write` is unused in current code paths; keep `write_with`.
4439

4540
pub fn write_with<'en, T, C>(&mut self, value: T, context: C)
4641
where
@@ -60,9 +55,11 @@ where
6055
where
6156
T: Decode<'de, ()>,
6257
{
63-
self.read_with(cnt, ()).await
58+
let buf = self.read_raw(cnt).await?.freeze();
59+
T::decode(buf)
6460
}
6561

62+
#[allow(dead_code)]
6663
pub async fn read_with<'de, T, C>(&mut self, cnt: usize, context: C) -> Result<T, Error>
6764
where
6865
T: Decode<'de, C>,
@@ -77,6 +74,7 @@ where
7774
Ok(buf)
7875
}
7976

77+
#[allow(dead_code)]
8078
pub async fn read_raw_into(&mut self, buf: &mut BytesMut, cnt: usize) -> Result<(), Error> {
8179
read_raw_into(&mut self.stream, buf, cnt).await
8280
}

sqlx-core/src/mysql/options/ssl_mode.rs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use std::str::FromStr;
44
/// Options for controlling the desired security state of the connection to the MySQL server.
55
///
66
/// It is used by the [`ssl_mode`](super::MySqlConnectOptions::ssl_mode) method.
7-
#[derive(Debug, Clone, Copy)]
7+
#[derive(Debug, Clone, Copy, Default)]
88
pub enum MySqlSslMode {
99
/// Establish an unencrypted connection.
1010
Disabled,
@@ -13,6 +13,7 @@ pub enum MySqlSslMode {
1313
/// back to an unencrypted connection if an encrypted connection cannot be established.
1414
///
1515
/// This is the default if `ssl_mode` is not specified.
16+
#[default]
1617
Preferred,
1718

1819
/// Establish an encrypted connection if the server supports encrypted connections.
@@ -30,11 +31,7 @@ pub enum MySqlSslMode {
3031
VerifyIdentity,
3132
}
3233

33-
impl Default for MySqlSslMode {
34-
fn default() -> Self {
35-
MySqlSslMode::Preferred
36-
}
37-
}
34+
// Default is derived above
3835

3936
impl FromStr for MySqlSslMode {
4037
type Err = Error;

sqlx-core/src/mysql/protocol/response/eof.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use crate::mysql::protocol::Capabilities;
1313
/// prior MySQL versions.
1414
#[derive(Debug)]
1515
pub struct EofPacket {
16+
#[allow(dead_code)]
1617
pub warnings: u16,
1718
pub status: Status,
1819
}

sqlx-core/src/mysql/types/float.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,12 @@ impl Encode<'_, MySql> for f64 {
5151
impl Decode<'_, MySql> for f32 {
5252
fn decode(value: MySqlValueRef<'_>) -> Result<Self, BoxDynError> {
5353
let as_f64 = <f64 as Decode<'_, MySql>>::decode(value)?;
54-
Ok(as_f64 as f32)
54+
// Clamp to f32 range, then cast; failing to parse into range is a bug upstream.
55+
let clamped = as_f64.clamp(f32::MIN as f64, f32::MAX as f64);
56+
#[allow(clippy::cast_possible_truncation)]
57+
{
58+
Ok(clamped as f32)
59+
}
5560
}
5661
}
5762

sqlx-core/src/mysql/value.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ impl<'r> ValueRef<'r> for MySqlValueRef<'r> {
9393

9494
#[inline]
9595
fn is_null(&self) -> bool {
96-
is_null(self.value.as_deref(), &self.type_info)
96+
is_null(self.value, &self.type_info)
9797
}
9898
}
9999

sqlx-core/src/net/socket.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ impl Socket {
2929
.map(Socket::Unix)
3030
}
3131

32+
#[allow(dead_code)]
3233
pub fn local_addr(&self) -> Option<SocketAddr> {
3334
match self {
3435
Self::Tcp(tcp) => tcp.local_addr().ok(),

sqlx-core/src/net/tls/mod.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -113,17 +113,19 @@ where
113113
let host = config.hostname;
114114
let connector = configure_tls_connector(config).await?;
115115

116-
let stream = match replace(self, MaybeTlsStream::Upgrading) {
116+
let stream: S = match replace(self, MaybeTlsStream::Upgrading) {
117117
MaybeTlsStream::Raw(stream) => stream,
118118

119119
MaybeTlsStream::Tls(_) => {
120120
// ignore upgrade, we are already a TLS connection
121+
*self = MaybeTlsStream::Upgrading;
121122
return Ok(());
122123
}
123124

124125
MaybeTlsStream::Upgrading => {
125126
// we previously failed to upgrade and now hold no connection
126127
// this should only happen from an internal misuse of this method
128+
*self = MaybeTlsStream::Upgrading;
127129
return Err(Error::Io(io::ErrorKind::ConnectionAborted.into()));
128130
}
129131
};
@@ -134,35 +136,33 @@ where
134136
.to_owned();
135137

136138
*self = MaybeTlsStream::Tls(Box::new(connector.connect(host, stream).await?));
137-
138139
Ok(())
139140
}
140141

142+
#[allow(dead_code)]
141143
pub fn downgrade(&mut self) -> Result<(), Error> {
142144
match replace(self, MaybeTlsStream::Upgrading) {
143145
MaybeTlsStream::Tls(stream) => {
144146
#[cfg(feature = "_tls-rustls")]
145147
{
146148
let raw = stream.into_inner().0;
147149
*self = MaybeTlsStream::Raw(raw);
148-
return Ok(());
150+
Ok(())
149151
}
150152

151153
#[cfg(feature = "_tls-native-tls")]
152154
{
153155
let _ = stream; // Use the variable to avoid warning
154-
return Err(Error::tls("No way to downgrade a native-tls stream, use rustls instead, or never disable tls"));
156+
Err(Error::tls("No way to downgrade a native-tls stream, use rustls instead, or never disable tls"))
155157
}
156158
}
157159

158160
MaybeTlsStream::Raw(stream) => {
159161
*self = MaybeTlsStream::Raw(stream);
160-
return Ok(());
162+
Ok(())
161163
}
162164

163-
MaybeTlsStream::Upgrading => {
164-
return Err(Error::Io(io::ErrorKind::ConnectionAborted.into()));
165-
}
165+
MaybeTlsStream::Upgrading => Err(Error::Io(io::ErrorKind::ConnectionAborted.into())),
166166
}
167167
}
168168
}

sqlx-core/src/pool/inner.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -290,10 +290,10 @@ impl<DB: Database> PoolInner<DB> {
290290
}
291291

292292
let mut backoff = Duration::from_millis(10);
293-
let max_backoff = deadline_as_timeout::<DB>(deadline)? / 5;
293+
let max_backoff = deadline_as_timeout(deadline)? / 5;
294294

295295
loop {
296-
let timeout = deadline_as_timeout::<DB>(deadline)?;
296+
let timeout = deadline_as_timeout(deadline)?;
297297

298298
// result here is `Result<Result<C, Error>, TimeoutError>`
299299
// if this block does not return, sleep for the backoff timeout and try again

sqlx-core/src/pool/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -604,7 +604,7 @@ impl FusedFuture for CloseEvent {
604604
/// get the time between the deadline and now and use that as our timeout
605605
///
606606
/// returns `Error::PoolTimedOut` if the deadline is in the past
607-
fn deadline_as_timeout<DB: Database>(deadline: Instant) -> Result<Duration, Error> {
607+
fn deadline_as_timeout(deadline: Instant) -> Result<Duration, Error> {
608608
deadline
609609
.checked_duration_since(Instant::now())
610610
.ok_or(Error::PoolTimedOut)

0 commit comments

Comments
 (0)