From d4136eb6f6664ccca728f6bc177763a32a84a501 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kh=E1=BA=A3i?= Date: Fri, 22 May 2026 13:48:50 +0700 Subject: [PATCH] chore(pacquet/lint): more clippy (#11839) Co-authored-by: Claude --- Cargo.toml | 5 +++++ dylint.toml | 12 +++--------- pacquet/CODE_STYLE_GUIDE.md | 2 +- pacquet/crates/cmd-shim/src/link_bins.rs | 8 +++----- .../src/node_resolver.rs | 6 +----- pacquet/crates/executor/src/lifecycle.rs | 7 ++----- .../package-manager/src/create_virtual_store.rs | 8 +++----- pacquet/crates/registry/src/package_version.rs | 2 +- .../resolving-deps-resolver/src/resolve_peers.rs | 4 ++-- .../src/resolve_from_workspace.rs | 6 +----- 10 files changed, 22 insertions(+), 38 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 6c58d3198d..2659a729f2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -146,6 +146,11 @@ allow_branch = "main" [workspace.lints.rust] unexpected_cfgs = { level = "warn", check-cfg = ['cfg(dylint_lib, values("perfectionist"))'] } +[workspace.lints.clippy] +clone_on_ref_ptr = "warn" +if_then_some_else_none = "warn" +unnecessary_lazy_evaluations = "warn" + [profile.release] opt-level = 3 lto = "fat" diff --git a/dylint.toml b/dylint.toml index 83cb214c92..282f1edc3c 100644 --- a/dylint.toml +++ b/dylint.toml @@ -40,17 +40,11 @@ ignore = [ "params", ] -# Enforce a single canonical ordering inside every `#[derive(...)]` -# attribute. The codebase has ~270 derive sites in 79 distinct -# orderings before this rule; the prefix below encodes the -# conventions that already dominate (Debug first, error-chain -# `Display→Error→Diagnostic` together, supertraits before subtraits, -# serde at the end) and alphabetises everything past the long tail. -# Trait matching is on the final path segment, so `derive_more::Display` -# / `miette::Diagnostic` / `clap::Parser` match `Display` / `Diagnostic` -# / `Parser` respectively. [perfectionist] enable = ["derive_ordering"] +disable = [ + { name = "arc_rc_clone", reason = "`arc_rc_clone` is enforced by `clippy::clone_on_ref_ptr` instead" }, +] ["perfectionist::derive_ordering"] style = "prefix_then_alphabetical" diff --git a/pacquet/CODE_STYLE_GUIDE.md b/pacquet/CODE_STYLE_GUIDE.md index b597e56127..83ce6d2752 100644 --- a/pacquet/CODE_STYLE_GUIDE.md +++ b/pacquet/CODE_STYLE_GUIDE.md @@ -502,7 +502,7 @@ Do not flatten the tests into a sibling file such as `src/foo_tests.rs`, and do ### Cloning `Arc` and `Rc` -Prefer `Arc::clone(&x)` / `Rc::clone(&x)` over `x.clone()` for reference-counted types. The qualified form makes the O(1) refcount bump visible at the call site and fails to compile if a refactor changes the binding's type to something whose `Clone` is an arbitrarily expensive deep copy. Enforced by [`perfectionist::arc_rc_clone`](https://github.com/KSXGitHub/perfectionist/blob/0.0.0-rc.15/rules/arc_rc_clone.md). +Prefer `Arc::clone(&x)` / `Rc::clone(&x)` over `x.clone()` for reference-counted types. The qualified form makes the O(1) refcount bump visible at the call site and fails to compile if a refactor changes the binding's type to something whose `Clone` is an arbitrarily expensive deep copy. Enforced by [`clippy::clone_on_ref_ptr`](https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_ref_ptr), wired into `[workspace.lints.clippy]` in the root `Cargo.toml`. ### Reading process state diff --git a/pacquet/crates/cmd-shim/src/link_bins.rs b/pacquet/crates/cmd-shim/src/link_bins.rs index 26be3126ee..b280d469fd 100644 --- a/pacquet/crates/cmd-shim/src/link_bins.rs +++ b/pacquet/crates/cmd-shim/src/link_bins.rs @@ -453,15 +453,13 @@ where // themselves still get computed inside the `cfg!(windows)` branch // below — moving the `generate_*` calls there keeps Unix builds // off the `relative_target_windows` allocation path entirely. - let windows_shims = if cfg!(windows) { + let windows_shims = cfg!(windows).then(|| { let cmd_path = with_extension_appended(shim_path, "cmd"); let ps1_path = with_extension_appended(shim_path, "ps1"); let cmd_body = generate_cmd_shim(target_path, &cmd_path, runtime.as_ref()); let ps1_body = generate_pwsh_shim(target_path, &ps1_path, runtime.as_ref()); - Some((cmd_path, cmd_body, ps1_path, ps1_body)) - } else { - None - }; + (cmd_path, cmd_body, ps1_path, ps1_body) + }); // Idempotent skip fires only when every flavor that *should* be // present is present and pointing at the right target. The `.sh` diff --git a/pacquet/crates/engine-runtime-node-resolver/src/node_resolver.rs b/pacquet/crates/engine-runtime-node-resolver/src/node_resolver.rs index 041525b933..e10e582c72 100644 --- a/pacquet/crates/engine-runtime-node-resolver/src/node_resolver.rs +++ b/pacquet/crates/engine-runtime-node-resolver/src/node_resolver.rs @@ -294,11 +294,7 @@ async fn read_node_assets_from_mirror( error: Arc::new(error), }) as ResolveError })?; - let prefix = if matches!(archive, BinaryArchive::Zip) { - Some(address.basename.clone()) - } else { - None - }; + let prefix = matches!(archive, BinaryArchive::Zip).then(|| address.basename.clone()); let binary = BinaryResolution { url, integrity, diff --git a/pacquet/crates/executor/src/lifecycle.rs b/pacquet/crates/executor/src/lifecycle.rs index 18cae14ed9..861e0bd330 100644 --- a/pacquet/crates/executor/src/lifecycle.rs +++ b/pacquet/crates/executor/src/lifecycle.rs @@ -164,11 +164,8 @@ pub fn run_postinstall_hooks( } let install_script = get_script("install").map(String::from).or_else(|| { - if get_script("preinstall").is_none() && opts.pkg_root.join("binding.gyp").exists() { - Some("node-gyp rebuild".to_string()) - } else { - None - } + (get_script("preinstall").is_none() && opts.pkg_root.join("binding.gyp").exists()) + .then(|| "node-gyp rebuild".to_string()) }); if let Some(script) = &install_script && script != "npx only-allow pnpm" diff --git a/pacquet/crates/package-manager/src/create_virtual_store.rs b/pacquet/crates/package-manager/src/create_virtual_store.rs index 3aab6ca91c..7f544c1453 100644 --- a/pacquet/crates/package-manager/src/create_virtual_store.rs +++ b/pacquet/crates/package-manager/src/create_virtual_store.rs @@ -624,16 +624,14 @@ impl<'a> CreateVirtualStore<'a> { // need to reason across the two branches. Cold-batch // entries are appended at the bottom of the function once // the cold-batch fetch finishes. - let mut cas_paths_by_pkg_id: Option = if is_hoisted { + let mut cas_paths_by_pkg_id: Option = is_hoisted.then(|| { let mut map = CasPathsByPkgId::with_capacity(warm.len()); for (snapshot_key, _snapshot, cas_paths) in &warm { let pkg_id = PkgIdWithPatchHash::from(snapshot_key.to_string()); map.entry(pkg_id).or_insert_with(|| (***cas_paths).clone()); } - Some(map) - } else { - None - }; + map + }); let import_method = config.package_import_method; if !is_hoisted { diff --git a/pacquet/crates/registry/src/package_version.rs b/pacquet/crates/registry/src/package_version.rs index 01a512f79c..08c2a2c213 100644 --- a/pacquet/crates/registry/src/package_version.rs +++ b/pacquet/crates/registry/src/package_version.rs @@ -152,7 +152,7 @@ where Ok(Some(value)) } fn visit_bool(self, value: bool) -> Result { - Ok(if value { Some(String::new()) } else { None }) + Ok(value.then(String::new)) } fn visit_none(self) -> Result { Ok(None) diff --git a/pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs b/pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs index a6f4275d0b..0a7530cc94 100644 --- a/pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs +++ b/pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs @@ -335,7 +335,7 @@ impl<'tree> Walker<'tree> { let parent_ref = ParentRef { version: child_version, node_id: Some(child_node_id.clone()), - alias: if alias != &child_real_name { Some(alias.clone()) } else { None }, + alias: (alias != &child_real_name).then(|| alias.clone()), }; if alias_relevant { child_parent_refs.insert(alias.clone(), parent_ref.clone()); @@ -610,7 +610,7 @@ fn insert_parent_ref( let parent_ref = ParentRef { version: version.clone(), node_id: Some(direct.node_id.clone()), - alias: if direct.alias != real_name { Some(direct.alias.clone()) } else { None }, + alias: (direct.alias != real_name).then(|| direct.alias.clone()), }; if alias_relevant { refs.insert(direct.alias.clone(), parent_ref.clone()); diff --git a/pacquet/crates/resolving-npm-resolver/src/resolve_from_workspace.rs b/pacquet/crates/resolving-npm-resolver/src/resolve_from_workspace.rs index d66dd4501e..7d4a551e86 100644 --- a/pacquet/crates/resolving-npm-resolver/src/resolve_from_workspace.rs +++ b/pacquet/crates/resolving-npm-resolver/src/resolve_from_workspace.rs @@ -221,11 +221,7 @@ fn pick_matching_local_version_or_null( resolve_workspace_range("*", &raw) } RegistryPackageSpecType::Version => { - if versions.contains_key(&spec.fetch_spec) { - Some(spec.fetch_spec.clone()) - } else { - None - } + versions.contains_key(&spec.fetch_spec).then(|| spec.fetch_spec.clone()) } RegistryPackageSpecType::Range => { let raw: Vec = versions.keys().cloned().collect();