Skip to content

Commit 9f67d0f

Browse files
HocuriHocuri
authored
refactor: Don't use traits where it's not necessary (#6567)
Traits are bad for readability and compile times.
1 parent c5cf16f commit 9f67d0f

File tree

8 files changed

+58
-51
lines changed

8 files changed

+58
-51
lines changed

‎deltachat-repl/src/cmdline.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ async fn reset_tables(context: &Context, bits: i32) {
9292
context.emit_msgs_changed_without_ids();
9393
}
9494

95-
async fn poke_eml_file(context: &Context, filename: impl AsRef<Path>) -> Result<()> {
95+
async fn poke_eml_file(context: &Context, filename: &Path) -> Result<()> {
9696
let data = read_file(context, filename).await?;
9797

9898
if let Err(err) = receive_imf(context, &data, false).await {
@@ -126,7 +126,7 @@ async fn poke_spec(context: &Context, spec: Option<&str>) -> bool {
126126
real_spec = rs.unwrap();
127127
}
128128
if let Some(suffix) = get_filesuffix_lc(&real_spec) {
129-
if suffix == "eml" && poke_eml_file(context, &real_spec).await.is_ok() {
129+
if suffix == "eml" && poke_eml_file(context, Path::new(&real_spec)).await.is_ok() {
130130
read_cnt += 1
131131
}
132132
} else {
@@ -140,7 +140,10 @@ async fn poke_spec(context: &Context, spec: Option<&str>) -> bool {
140140
if name.ends_with(".eml") {
141141
let path_plus_name = format!("{}/{}", &real_spec, name);
142142
println!("Import: {path_plus_name}");
143-
if poke_eml_file(context, path_plus_name).await.is_ok() {
143+
if poke_eml_file(context, Path::new(&path_plus_name))
144+
.await
145+
.is_ok()
146+
{
144147
read_cnt += 1
145148
}
146149
}
@@ -1278,7 +1281,7 @@ pub async fn cmdline(context: Context, line: &str, chat_id: &mut ChatId) -> Resu
12781281
"fileinfo" => {
12791282
ensure!(!arg1.is_empty(), "Argument <file> missing.");
12801283

1281-
if let Ok(buf) = read_file(&context, &arg1).await {
1284+
if let Ok(buf) = read_file(&context, Path::new(arg1)).await {
12821285
let (width, height) = get_filemeta(&buf)?;
12831286
println!("width={width}, height={height}");
12841287
} else {

‎src/blob.rs

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
//! # Blob directory management.
22
33
use core::cmp::max;
4-
use std::ffi::OsStr;
54
use std::io::{Cursor, Seek};
65
use std::iter::FusedIterator;
76
use std::mem;
@@ -151,10 +150,10 @@ impl<'a> BlobObject<'a> {
151150
let rel_path = path
152151
.strip_prefix(context.get_blobdir())
153152
.with_context(|| format!("wrong blobdir: {}", path.display()))?;
154-
if !BlobObject::is_acceptible_blob_name(rel_path) {
153+
let name = rel_path.to_str().context("wrong name")?;
154+
if !BlobObject::is_acceptible_blob_name(name) {
155155
return Err(format_err!("bad blob name: {}", rel_path.display()));
156156
}
157-
let name = rel_path.to_str().context("wrong name")?;
158157
BlobObject::from_name(context, name)
159158
}
160159

@@ -216,19 +215,17 @@ impl<'a> BlobObject<'a> {
216215
///
217216
/// This is slightly less strict than stanitise_name, presumably
218217
/// someone already created a file with such a name so we just
219-
/// ensure it's not actually a path in disguise is actually utf-8.
220-
fn is_acceptible_blob_name(name: impl AsRef<OsStr>) -> bool {
221-
let uname = match name.as_ref().to_str() {
222-
Some(name) => name,
223-
None => return false,
224-
};
225-
if uname.find('/').is_some() {
218+
/// ensure it's not actually a path in disguise.
219+
///
220+
/// Acceptible blob name always have to be valid utf-8.
221+
fn is_acceptible_blob_name(name: &str) -> bool {
222+
if name.find('/').is_some() {
226223
return false;
227224
}
228-
if uname.find('\\').is_some() {
225+
if name.find('\\').is_some() {
229226
return false;
230227
}
231-
if uname.find('\0').is_some() {
228+
if name.find('\0').is_some() {
232229
return false;
233230
}
234231
true

‎src/blob/blob_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ async fn test_create_from_name_long() {
120120
fn test_is_blob_name() {
121121
assert!(BlobObject::is_acceptible_blob_name("foo"));
122122
assert!(BlobObject::is_acceptible_blob_name("foo.txt"));
123-
assert!(BlobObject::is_acceptible_blob_name("f".repeat(128)));
123+
assert!(BlobObject::is_acceptible_blob_name(&"f".repeat(128)));
124124
assert!(!BlobObject::is_acceptible_blob_name("foo/bar"));
125125
assert!(!BlobObject::is_acceptible_blob_name("foo\\bar"));
126126
assert!(!BlobObject::is_acceptible_blob_name("foo\x00bar"));

‎src/imex.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ async fn imex_inner(
209209
.await
210210
.context("Cannot create private key or private key not available")?;
211211

212-
create_folder(context, &path).await?;
212+
create_folder(context, path).await?;
213213
}
214214

215215
match what {
@@ -600,7 +600,7 @@ where
600600

601601
/// Imports secret key from a file.
602602
async fn import_secret_key(context: &Context, path: &Path, set_default: bool) -> Result<()> {
603-
let buf = read_file(context, &path).await?;
603+
let buf = read_file(context, path).await?;
604604
let armored = std::string::String::from_utf8_lossy(&buf);
605605
set_self_key(context, &armored, set_default).await?;
606606
Ok(())

‎src/message.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -973,7 +973,7 @@ impl Message {
973973
}
974974

975975
if let Some(filename) = self.get_file(context) {
976-
if let Ok(ref buf) = read_file(context, filename).await {
976+
if let Ok(ref buf) = read_file(context, &filename).await {
977977
if let Ok((typ, headers, _)) = split_armored_data(buf) {
978978
if typ == pgp::armor::BlockType::Message {
979979
return headers.get(crate::pgp::HEADER_SETUPCODE).cloned();

‎src/test_utils.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -400,11 +400,11 @@ impl TestContext {
400400
/// Sets a name for this [`TestContext`] if one isn't yet set.
401401
///
402402
/// This will show up in events logged in the test output.
403-
pub fn set_name(&self, name: impl Into<String>) {
403+
pub fn set_name(&self, name: &str) {
404404
let mut context_names = CONTEXT_NAMES.write().unwrap();
405405
context_names
406406
.entry(self.ctx.get_id())
407-
.or_insert_with(|| name.into());
407+
.or_insert_with(|| name.to_string());
408408
}
409409

410410
/// Returns the name of this [`TestContext`].

‎src/tools.rs

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -348,14 +348,13 @@ pub(crate) fn get_abs_path(context: &Context, path: &Path) -> PathBuf {
348348
}
349349
}
350350

351-
pub(crate) async fn get_filebytes(context: &Context, path: impl AsRef<Path>) -> Result<u64> {
352-
let path_abs = get_abs_path(context, path.as_ref());
351+
pub(crate) async fn get_filebytes(context: &Context, path: &Path) -> Result<u64> {
352+
let path_abs = get_abs_path(context, path);
353353
let meta = fs::metadata(&path_abs).await?;
354354
Ok(meta.len())
355355
}
356356

357-
pub(crate) async fn delete_file(context: &Context, path: impl AsRef<Path>) -> Result<()> {
358-
let path = path.as_ref();
357+
pub(crate) async fn delete_file(context: &Context, path: &Path) -> Result<()> {
359358
let path_abs = get_abs_path(context, path);
360359
if !path_abs.exists() {
361360
bail!("path {} does not exist", path_abs.display());
@@ -443,19 +442,16 @@ impl AsRef<Path> for TempPathGuard {
443442
}
444443
}
445444

446-
pub(crate) async fn create_folder(
447-
context: &Context,
448-
path: impl AsRef<Path>,
449-
) -> Result<(), io::Error> {
450-
let path_abs = get_abs_path(context, path.as_ref());
445+
pub(crate) async fn create_folder(context: &Context, path: &Path) -> Result<(), io::Error> {
446+
let path_abs = get_abs_path(context, path);
451447
if !path_abs.exists() {
452448
match fs::create_dir_all(path_abs).await {
453449
Ok(_) => Ok(()),
454450
Err(err) => {
455451
warn!(
456452
context,
457453
"Cannot create directory \"{}\": {}",
458-
path.as_ref().display(),
454+
path.display(),
459455
err
460456
);
461457
Err(err)
@@ -469,50 +465,50 @@ pub(crate) async fn create_folder(
469465
/// Write a the given content to provided file path.
470466
pub(crate) async fn write_file(
471467
context: &Context,
472-
path: impl AsRef<Path>,
468+
path: &Path,
473469
buf: &[u8],
474470
) -> Result<(), io::Error> {
475-
let path_abs = get_abs_path(context, path.as_ref());
471+
let path_abs = get_abs_path(context, path);
476472
fs::write(&path_abs, buf).await.map_err(|err| {
477473
warn!(
478474
context,
479475
"Cannot write {} bytes to \"{}\": {}",
480476
buf.len(),
481-
path.as_ref().display(),
477+
path.display(),
482478
err
483479
);
484480
err
485481
})
486482
}
487483

488484
/// Reads the file and returns its context as a byte vector.
489-
pub async fn read_file(context: &Context, path: impl AsRef<Path>) -> Result<Vec<u8>> {
490-
let path_abs = get_abs_path(context, path.as_ref());
485+
pub async fn read_file(context: &Context, path: &Path) -> Result<Vec<u8>> {
486+
let path_abs = get_abs_path(context, path);
491487

492488
match fs::read(&path_abs).await {
493489
Ok(bytes) => Ok(bytes),
494490
Err(err) => {
495491
warn!(
496492
context,
497493
"Cannot read \"{}\" or file is empty: {}",
498-
path.as_ref().display(),
494+
path.display(),
499495
err
500496
);
501497
Err(err.into())
502498
}
503499
}
504500
}
505501

506-
pub async fn open_file(context: &Context, path: impl AsRef<Path>) -> Result<fs::File> {
507-
let path_abs = get_abs_path(context, path.as_ref());
502+
pub async fn open_file(context: &Context, path: &Path) -> Result<fs::File> {
503+
let path_abs = get_abs_path(context, path);
508504

509505
match fs::File::open(&path_abs).await {
510506
Ok(bytes) => Ok(bytes),
511507
Err(err) => {
512508
warn!(
513509
context,
514510
"Cannot read \"{}\" or file is empty: {}",
515-
path.as_ref().display(),
511+
path.display(),
516512
err
517513
);
518514
Err(err.into())

‎src/tools/tools_tests.rs

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -282,15 +282,22 @@ async fn test_file_handling() {
282282
};
283283
}
284284

285-
assert!(delete_file(context, "$BLOBDIR/lkqwjelqkwlje")
285+
assert!(delete_file(context, Path::new("$BLOBDIR/lkqwjelqkwlje"))
286286
.await
287287
.is_err());
288-
assert!(write_file(context, "$BLOBDIR/foobar", b"content")
289-
.await
290-
.is_ok());
288+
assert!(
289+
write_file(context, Path::new("$BLOBDIR/foobar"), b"content")
290+
.await
291+
.is_ok()
292+
);
291293
assert!(file_exist!(context, "$BLOBDIR/foobar"));
292294
assert!(!file_exist!(context, "$BLOBDIR/foobarx"));
293-
assert_eq!(get_filebytes(context, "$BLOBDIR/foobar").await.unwrap(), 7);
295+
assert_eq!(
296+
get_filebytes(context, Path::new("$BLOBDIR/foobar"))
297+
.await
298+
.unwrap(),
299+
7
300+
);
294301

295302
let abs_path = context
296303
.get_blobdir()
@@ -300,19 +307,23 @@ async fn test_file_handling() {
300307

301308
assert!(file_exist!(context, &abs_path));
302309

303-
assert!(delete_file(context, "$BLOBDIR/foobar").await.is_ok());
304-
assert!(create_folder(context, "$BLOBDIR/foobar-folder")
310+
assert!(delete_file(context, Path::new("$BLOBDIR/foobar"))
311+
.await
312+
.is_ok());
313+
assert!(create_folder(context, Path::new("$BLOBDIR/foobar-folder"))
305314
.await
306315
.is_ok());
307316
assert!(file_exist!(context, "$BLOBDIR/foobar-folder"));
308-
assert!(delete_file(context, "$BLOBDIR/foobar-folder")
317+
assert!(delete_file(context, Path::new("$BLOBDIR/foobar-folder"))
309318
.await
310319
.is_err());
311320

312321
let fn0 = "$BLOBDIR/data.data";
313-
assert!(write_file(context, &fn0, b"content").await.is_ok());
322+
assert!(write_file(context, Path::new(fn0), b"content")
323+
.await
324+
.is_ok());
314325

315-
assert!(delete_file(context, &fn0).await.is_ok());
326+
assert!(delete_file(context, Path::new(fn0)).await.is_ok());
316327
assert!(!file_exist!(context, &fn0));
317328
}
318329

0 commit comments

Comments
 (0)