Skip to content

Commit

Permalink
chore: more clippy lint (#462)
Browse files Browse the repository at this point in the history
  • Loading branch information
baszalmstra authored Jan 2, 2024
1 parent 2c4c21e commit db99769
Show file tree
Hide file tree
Showing 83 changed files with 899 additions and 832 deletions.
78 changes: 78 additions & 0 deletions .cargo/config
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
[target.'cfg(all())']
rustflags = [
"-Wclippy::all",
"-Wclippy::await_holding_lock",
"-Wclippy::bool-to-int-with-if",
"-Wclippy::cast_lossless",
"-Wclippy::char_lit_as_u8",
"-Wclippy::checked_conversions",
"-Wclippy::debug_assert_with_mut_call",
"-Wclippy::default_trait_access",
"-Wclippy::doc_markdown",
"-Wclippy::empty_enum",
"-Wclippy::enum_glob_use",
"-Wclippy::expl_impl_clone_on_copy",
"-Wclippy::explicit_deref_methods",
"-Wclippy::explicit_into_iter_loop",
"-Wclippy::fallible_impl_from",
"-Wclippy::filter_map_next",
"-Wclippy::flat_map_option",
"-Wclippy::float_cmp_const",
"-Wclippy::fn_params_excessive_bools",
"-Wclippy::from_iter_instead_of_collect",
"-Wclippy::if-not-else",
"-Wclippy::implicit_clone",
"-Wclippy::imprecise_flops",
"-Wclippy::inconsistent_struct_constructor",
"-Wclippy::inefficient_to_string",
"-Wclippy::invalid_upcast_comparisons",
"-Wclippy::items-after-statements",
"-Wclippy::large_digit_groups",
"-Wclippy::large_stack_arrays",
"-Wclippy::large_types_passed_by_value",
"-Wclippy::let_unit_value",
"-Wclippy::linkedlist",
"-Wclippy::lossy_float_literal",
"-Wclippy::macro_use_imports",
"-Wclippy::manual-assert",
"-Wclippy::manual_ok_or",
"-Wclippy::map_err_ignore",
"-Wclippy::map_flatten",
"-Wclippy::map_unwrap_or",
"-Wclippy::match_on_vec_items",
"-Wclippy::match_same_arms",
"-Wclippy::match_wild_err_arm",
"-Wclippy::match_wildcard_for_single_variants",
"-Wclippy::mem_forget",
"-Wclippy::missing_enforced_import_renames",
"-Wclippy::mut_mut",
"-Wclippy::mutex_integer",
"-Wclippy::needless_borrow",
"-Wclippy::needless_continue",
"-Wclippy::needless_for_each",
"-Wclippy::option_option",
"-Wclippy::path_buf_push_overwrite",
"-Wclippy::ptr_as_ptr",
"-Wclippy::rc_mutex",
"-Wclippy::redundant_closure_for_method_calls",
"-Wclippy::ref_option_ref",
"-Wclippy::rest_pat_in_fully_bound_structs",
"-Wclippy::same_functions_in_if_condition",
"-Wclippy::semicolon_if_nothing_returned",
"-Wclippy::single_match_else",
"-Wclippy::string_add_assign",
"-Wclippy::string_lit_as_bytes",
"-Wclippy::string_to_string",
"-Wclippy::todo",
"-Wclippy::trait_duplication_in_bounds",
"-Wclippy::uninlined_format_args",
"-Wclippy::unnested_or_patterns",
"-Wclippy::unused_self",
"-Wclippy::useless_transmute",
"-Wclippy::verbose_file_reads",
"-Wclippy::wildcard-imports",
"-Wclippy::zero_sized_map_values",
"-Wfuture_incompatible",
"-Wnonstandard_style",
"-Wrust_2018_idioms",
]
34 changes: 16 additions & 18 deletions crates/rattler-bin/src/commands/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ pub async fn create(opt: Opt) -> anyhow::Result<()> {
Platform::current()
};

println!("installing for platform: {:?}", install_platform);
println!("installing for platform: {install_platform:?}");

// Parse the specs from the command line. We do this explicitly instead of allow clap to deal
// with this because we need to parse the `channel_config` when parsing matchspecs.
Expand Down Expand Up @@ -182,7 +182,7 @@ pub async fn create(opt: Opt) -> anyhow::Result<()> {
.map(|s| Version::from_str(s))
.unwrap_or(Version::from_str("0"))
.expect("Could not parse virtual package version"),
build_string: elems.get(2).unwrap_or(&"").to_string(),
build_string: (*elems.get(2).unwrap_or(&"")).to_string(),
})
})
.collect::<anyhow::Result<Vec<_>>>()?)
Expand All @@ -198,7 +198,7 @@ pub async fn create(opt: Opt) -> anyhow::Result<()> {
}
})?;

println!("virtual packages: {:?}", virtual_packages);
println!("virtual packages: {virtual_packages:?}");

// Now that we parsed and downloaded all information, construct the packaging problem that we
// need to solve. We do this by constructing a `SolverProblem`. This encapsulates all the
Expand Down Expand Up @@ -262,27 +262,27 @@ pub async fn create(opt: Opt) -> anyhow::Result<()> {
);
}
TransactionOperation::Reinstall(r) => {
println!("* Reinstall: {}", format_record(&r.repodata_record))
println!("* Reinstall: {}", format_record(&r.repodata_record));
}
TransactionOperation::Remove(r) => {
println!("* Remove: {}", format_record(&r.repodata_record))
println!("* Remove: {}", format_record(&r.repodata_record));
}
}
}

return Ok(());
}

if !transaction.operations.is_empty() {
// Execute the operations that are returned by the solver.
execute_transaction(transaction, target_prefix, cache_dir, download_client).await?;
if transaction.operations.is_empty() {
println!(
"{} Successfully updated the environment",
"{} Already up to date",
console::style(console::Emoji("✔", "")).green(),
);
} else {
// Execute the operations that are returned by the solver.
execute_transaction(transaction, target_prefix, cache_dir, download_client).await?;
println!(
"{} Already up to date",
"{} Successfully updated the environment",
console::style(console::Emoji("✔", "")).green(),
);
}
Expand Down Expand Up @@ -644,17 +644,16 @@ async fn fetch_repo_data_records_with_progress(
progress_bar.finish_with_message("Error");
Err(err.into())
}
Err(err) => match err.try_into_panic() {
Ok(panic) => {
Err(err) => {
if let Ok(panic) = err.try_into_panic() {
std::panic::resume_unwind(panic);
}
Err(_) => {
} else {
progress_bar.set_style(errored_progress_style());
progress_bar.finish_with_message("Cancelled..");
// Since the task was cancelled most likely the whole async stack is being cancelled.
Err(anyhow::anyhow!("cancelled"))
}
},
}
}
}

Expand All @@ -663,8 +662,7 @@ fn friendly_channel_name(channel: &Channel) -> String {
channel
.name
.as_ref()
.map(String::from)
.unwrap_or_else(|| channel.canonical_name())
.map_or_else(|| channel.canonical_name(), String::from)
}

/// Returns the style to use for a progressbar that is currently in progress.
Expand All @@ -676,7 +674,7 @@ fn default_bytes_style() -> indicatif::ProgressStyle {
"smoothed_bytes_per_sec",
|s: &ProgressState, w: &mut dyn Write| match (s.pos(), s.elapsed().as_millis()) {
(pos, elapsed_ms) if elapsed_ms > 0 => {
write!(w, "{}/s", HumanBytes((pos as f64 * 1000_f64 / elapsed_ms as f64) as u64)).unwrap()
write!(w, "{}/s", HumanBytes((pos as f64 * 1000_f64 / elapsed_ms as f64) as u64)).unwrap();
}
_ => write!(w, "-").unwrap(),
},
Expand Down
4 changes: 2 additions & 2 deletions crates/rattler/src/install/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ impl InstallDriver {
// Await the result being send back. If an error occurs during receive it means that the
// sending end of the channel was closed. This can only really happen when the task has been
// dropped. We assume that that means the task has been cancelled.
rx.await.map_err(|_| InstallError::Cancelled)?
rx.await.map_err(|_err| InstallError::Cancelled)?
}

/// Spawns a blocking operation on another thread but does not wait for it to complete. This is
Expand All @@ -134,6 +134,6 @@ impl InstallDriver {

impl Drop for InstallDriverInner {
fn drop(&mut self) {
self.join_handle.abort()
self.join_handle.abort();
}
}
3 changes: 2 additions & 1 deletion crates/rattler/src/install/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,8 @@ impl AsRef<[u8]> for MmapOrBytes {
///
/// This fallback exists because we've seen that in some particular situations memory mapping is not
/// allowed. A particular dubious case we've encountered is described in the this issue:
/// https://github.com/prefix-dev/pixi/issues/234
/// <https://github.com/prefix-dev/pixi/issues/234>
#[allow(clippy::verbose_file_reads)]
fn map_or_read_source_file(source_path: &Path) -> Result<MmapOrBytes, LinkFileError> {
let mut file =
std::fs::File::open(source_path).map_err(LinkFileError::FailedToOpenSourceFile)?;
Expand Down
103 changes: 50 additions & 53 deletions crates/rattler/src/install/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ pub async fn link_package(

// Parse the `link.json` file and extract entry points from it.
let link_json = if index_json.noarch.is_python() {
read_link_json(package_dir, driver, options.link_json).await?
read_link_json(package_dir, driver, options.link_json.flatten()).await?
} else {
None
};
Expand Down Expand Up @@ -250,10 +250,10 @@ pub async fn link_package(

// Start linking all package files in parallel
let mut number_of_paths_entries = 0;
for entry in paths_json.paths.into_iter() {
for entry in paths_json.paths {
let package_dir = package_dir.to_owned();
let target_dir = target_dir.to_owned();
let target_prefix = target_prefix.to_owned();
let target_prefix = target_prefix.clone();
let python_info = python_info.clone();

// Spawn a task to link the specific file. Note that these tasks are throttled by the
Expand Down Expand Up @@ -325,7 +325,7 @@ pub async fn link_package(
let tx = tx.clone();
let python_info = python_info.clone();
let target_dir = target_dir.to_owned();
let target_prefix = target_prefix.to_owned();
let target_prefix = target_prefix.clone();

if platform.is_windows() {
driver.spawn_throttled_and_forget(move || {
Expand All @@ -352,7 +352,7 @@ pub async fn link_package(
}
}
});
number_of_paths_entries += 2
number_of_paths_entries += 2;
} else {
driver.spawn_throttled_and_forget(move || {
// Return immediately if the receiver was closed. This can happen if a previous step
Expand Down Expand Up @@ -433,17 +433,16 @@ async fn read_paths_json(
driver: &InstallDriver,
paths_json: Option<PathsJson>,
) -> Result<PathsJson, InstallError> {
match paths_json {
Some(paths) => Ok(paths),
None => {
let package_dir = package_dir.to_owned();
driver
.spawn_throttled(move || {
PathsJson::from_package_directory_with_deprecated_fallback(&package_dir)
.map_err(InstallError::FailedToReadPathsJson)
})
.await
}
if let Some(paths_json) = paths_json {
Ok(paths_json)
} else {
let package_dir = package_dir.to_owned();
driver
.spawn_throttled(move || {
PathsJson::from_package_directory_with_deprecated_fallback(&package_dir)
.map_err(InstallError::FailedToReadPathsJson)
})
.await
}
}

Expand All @@ -454,17 +453,16 @@ async fn read_index_json(
driver: &InstallDriver,
index_json: Option<IndexJson>,
) -> Result<IndexJson, InstallError> {
match index_json {
Some(index) => Ok(index),
None => {
let package_dir = package_dir.to_owned();
driver
.spawn_throttled(move || {
IndexJson::from_package_directory(package_dir)
.map_err(InstallError::FailedToReadIndexJson)
})
.await
}
if let Some(index) = index_json {
Ok(index)
} else {
let package_dir = package_dir.to_owned();
driver
.spawn_throttled(move || {
IndexJson::from_package_directory(package_dir)
.map_err(InstallError::FailedToReadIndexJson)
})
.await
}
}

Expand All @@ -473,34 +471,33 @@ async fn read_index_json(
async fn read_link_json(
package_dir: &Path,
driver: &InstallDriver,
index_json: Option<Option<LinkJson>>,
index_json: Option<LinkJson>,
) -> Result<Option<LinkJson>, InstallError> {
match index_json {
Some(index) => Ok(index),
None => {
let package_dir = package_dir.to_owned();
driver
.spawn_throttled(move || {
LinkJson::from_package_directory(package_dir)
.map_or_else(
|e| {
// Its ok if the file is not present.
if e.kind() == ErrorKind::NotFound {
Ok(None)
} else {
Err(e)
}
},
|link_json| Ok(Some(link_json)),
)
.map_err(InstallError::FailedToReadLinkJson)
})
.await
}
if let Some(index) = index_json {
Ok(Some(index))
} else {
let package_dir = package_dir.to_owned();
driver
.spawn_throttled(move || {
LinkJson::from_package_directory(package_dir)
.map_or_else(
|e| {
// Its ok if the file is not present.
if e.kind() == ErrorKind::NotFound {
Ok(None)
} else {
Err(e)
}
},
|link_json| Ok(Some(link_json)),
)
.map_err(InstallError::FailedToReadLinkJson)
})
.await
}
}

/// A helper struct for a BinaryHeap to provides ordering to items that are otherwise unordered.
/// A helper struct for a `BinaryHeap` to provides ordering to items that are otherwise unordered.
struct OrderWrapper<T> {
index: usize,
data: T,
Expand Down Expand Up @@ -530,7 +527,7 @@ impl<T> Ord for OrderWrapper<T> {
/// Returns true if it is possible to create symlinks in the target directory.
async fn can_create_symlinks(target_dir: &Path) -> bool {
let uuid = uuid::Uuid::new_v4();
let symlink_path = target_dir.join(format!("symtest_{}", uuid));
let symlink_path = target_dir.join(format!("symtest_{uuid}"));
#[cfg(windows)]
let result = tokio::fs::symlink_file("./", &symlink_path).await;
#[cfg(unix)]
Expand All @@ -541,7 +538,7 @@ async fn can_create_symlinks(target_dir: &Path) -> bool {
tracing::warn!(
"failed to delete temporary file '{}': {e}",
symlink_path.display()
)
);
}
true
}
Expand Down
4 changes: 2 additions & 2 deletions crates/rattler/src/install/python.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,14 @@ impl PythonInfo {
let path = if platform.is_windows() {
PathBuf::from("python.exe")
} else {
PathBuf::from(format!("bin/python{}.{}", major, minor))
PathBuf::from(format!("bin/python{major}.{minor}"))
};

// Find the location of the site packages
let site_packages_path = if platform.is_windows() {
PathBuf::from("Lib/site-packages")
} else {
PathBuf::from(format!("lib/python{}.{}/site-packages", major, minor))
PathBuf::from(format!("lib/python{major}.{minor}/site-packages"))
};

// Binary directory
Expand Down
Loading

0 comments on commit db99769

Please sign in to comment.