Auto merge of #12667 - Eh2406:clippy, r=weihanglo

Clippy

### What does this PR try to resolve?

A few more small anti-patterns from our code base. Each in their own commit, found by turning on a clippy warning and manually reviewing the changes. Most of the changes are from switching to `let-else`.

### How should we test and review this PR?

Internal refactoring and tests still pass.

### Additional information

I know we don't like "fix clippy" PR's. Sorry.

On the other hand having reviewed for these lints, I wouldn't mind turning them to warn. 🤷

Author: bors

src/cargo/core/compiler/build_config.rs

@@ -7,7 +7,7 @@ use cargo_util::ProcessBuilder;
 use serde::ser;
 use std::cell::RefCell;
 use std::path::PathBuf;
-use std::sync::Arc;
+use std::rc::Rc;
 use std::thread::available_parallelism;
 
 /// Configuration information for a rustc build.
@@ -35,7 +35,7 @@ pub struct BuildConfig {
     pub primary_unit_rustc: Option<ProcessBuilder>,
     /// A thread used by `cargo fix` to receive messages on a socket regarding
     /// the success/failure of applying fixes.
-    pub rustfix_diagnostic_server: Arc<RefCell<Option<RustfixDiagnosticServer>>>,
+    pub rustfix_diagnostic_server: Rc<RefCell<Option<RustfixDiagnosticServer>>>,
     /// The directory to copy final artifacts to. Note that even if `out_dir` is
     /// set, a copy of artifacts still could be found a `target/(debug\release)`
     /// as usual.
@@ -113,7 +113,7 @@ impl BuildConfig {
             build_plan: false,
             unit_graph: false,
             primary_unit_rustc: None,
-            rustfix_diagnostic_server: Arc::new(RefCell::new(None)),
+            rustfix_diagnostic_server: Rc::new(RefCell::new(None)),
             export_dir: None,
             future_incompat_report: false,
             timing_outputs: Vec::new(),

src/cargo/core/compiler/build_context/target_info.rs

@@ -367,9 +367,8 @@ impl TargetInfo {
                 &*v.insert(value)
             }
         };
-        let (prefix, suffix) = match *crate_type_info {
-            Some((ref prefix, ref suffix)) => (prefix, suffix),
-            None => return Ok(None),
+        let Some((prefix, suffix)) = crate_type_info else {
+            return Ok(None);
         };
         let mut ret = vec![FileType {
             suffix: suffix.clone(),
@@ -612,13 +611,12 @@ fn parse_crate_type(
     if not_supported {
         return Ok(None);
     }
-    let line = match lines.next() {
-        Some(line) => line,
-        None => anyhow::bail!(
+    let Some(line) = lines.next() else {
+        anyhow::bail!(
             "malformed output when learning about crate-type {} information\n{}",
             crate_type,
             output_err_info(cmd, output, error)
-        ),
+        )
     };
     let mut parts = line.trim().split("___");
     let prefix = parts.next().unwrap();
@@ -978,9 +976,8 @@ impl<'cfg> RustcTargetData<'cfg> {
     pub fn dep_platform_activated(&self, dep: &Dependency, kind: CompileKind) -> bool {
         // If this dependency is only available for certain platforms,
         // make sure we're only enabling it for that platform.
-        let platform = match dep.platform() {
-            Some(p) => p,
-            None => return true,
+        let Some(platform) = dep.platform() else {
+            return true;
         };
         let name = self.short_name(&kind);
         platform.matches(name, self.cfg(kind))
@@ -1058,13 +1055,12 @@ impl RustDocFingerprint {
                 serde_json::to_string(&actual_rustdoc_target_data)?,
             )
         };
-        let rustdoc_data = match paths::read(&fingerprint_path) {
-            Ok(rustdoc_data) => rustdoc_data,
+        let Ok(rustdoc_data) = paths::read(&fingerprint_path) else {
             // If the fingerprint does not exist, do not clear out the doc
             // directories. Otherwise this ran into problems where projects
             // like rustbuild were creating the doc directory before running
             // `cargo doc` in a way that deleting it would break it.
-            Err(_) => return write_fingerprint(),
+            return write_fingerprint();
         };
         match serde_json::from_str::<RustDocFingerprint>(&rustdoc_data) {
             Ok(fingerprint) => {

src/cargo/core/compiler/build_plan.rs

@@ -86,10 +86,7 @@ impl Invocation {
             );
         }
         for (var, value) in cmd.get_envs() {
-            let value = match value {
-                Some(s) => s,
-                None => continue,
-            };
+            let Some(value) = value else { continue };
             self.env.insert(
                 var.clone(),
                 value

src/cargo/core/compiler/custom_build.rs

@@ -443,7 +443,7 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Job> {
                     ))
                 })?;
                 let data = &script_output.metadata;
-                for &(ref key, ref value) in data.iter() {
+                for (key, value) in data.iter() {
                     cmd.env(
                         &format!("DEP_{}_{}", super::envify(&name), super::envify(key)),
                         value,

src/cargo/core/compiler/fingerprint/mod.rs

@@ -811,9 +811,8 @@ impl LocalFingerprint {
             // rustc.
             LocalFingerprint::CheckDepInfo { dep_info } => {
                 let dep_info = target_root.join(dep_info);
-                let info = match parse_dep_info(pkg_root, target_root, &dep_info)? {
-                    Some(info) => info,
-                    None => return Ok(Some(StaleItem::MissingFile(dep_info))),
+                let Some(info) = parse_dep_info(pkg_root, target_root, &dep_info)? else {
+                    return Ok(Some(StaleItem::MissingFile(dep_info)));
                 };
                 for (key, previous) in info.env.iter() {
                     let current = if key == CARGO_ENV {
@@ -1038,16 +1037,16 @@ impl Fingerprint {
         for (a, b) in self.deps.iter().zip(old.deps.iter()) {
             if a.name != b.name {
                 return DirtyReason::UnitDependencyNameChanged {
-                    old: b.name.clone(),
-                    new: a.name.clone(),
+                    old: b.name,
+                    new: a.name,
                 };
             }
 
             if a.fingerprint.hash_u64() != b.fingerprint.hash_u64() {
                 return DirtyReason::UnitDependencyInfoChanged {
-                    new_name: a.name.clone(),
+                    new_name: a.name,
                     new_fingerprint: a.fingerprint.hash_u64(),
-                    old_name: b.name.clone(),
+                    old_name: b.name,
                     old_fingerprint: b.fingerprint.hash_u64(),
                 };
             }
@@ -1103,16 +1102,12 @@ impl Fingerprint {
         }
 
         let opt_max = mtimes.iter().max_by_key(|kv| kv.1);
-        let (max_path, max_mtime) = match opt_max {
-            Some(mtime) => mtime,
-
+        let Some((max_path, max_mtime)) = opt_max else {
             // We had no output files. This means we're an overridden build
             // script and we're just always up to date because we aren't
             // watching the filesystem.
-            None => {
-                self.fs_status = FsStatus::UpToDate { mtimes };
-                return Ok(());
-            }
+            self.fs_status = FsStatus::UpToDate { mtimes };
+            return Ok(());
         };
         debug!(
             "max output mtime for {:?} is {:?} {}",
@@ -1127,9 +1122,7 @@ impl Fingerprint {
                 | FsStatus::StaleItem(_)
                 | FsStatus::StaleDependency { .. }
                 | FsStatus::StaleDepFingerprint { .. } => {
-                    self.fs_status = FsStatus::StaleDepFingerprint {
-                        name: dep.name.clone(),
-                    };
+                    self.fs_status = FsStatus::StaleDepFingerprint { name: dep.name };
                     return Ok(());
                 }
             };
@@ -1171,7 +1164,7 @@ impl Fingerprint {
                 );
 
                 self.fs_status = FsStatus::StaleDependency {
-                    name: dep.name.clone(),
+                    name: dep.name,
                     dep_mtime: *dep_mtime,
                     max_mtime: *max_mtime,
                 };
@@ -1808,16 +1801,12 @@ pub fn parse_dep_info(
     target_root: &Path,
     dep_info: &Path,
 ) -> CargoResult<Option<RustcDepInfo>> {
-    let data = match paths::read_bytes(dep_info) {
-        Ok(data) => data,
-        Err(_) => return Ok(None),
+    let Ok(data) = paths::read_bytes(dep_info) else {
+        return Ok(None);
     };
-    let info = match EncodedDepInfo::parse(&data) {
-        Some(info) => info,
-        None => {
-            tracing::warn!("failed to parse cargo's dep-info at {:?}", dep_info);
-            return Ok(None);
-        }
+    let Some(info) = EncodedDepInfo::parse(&data) else {
+        tracing::warn!("failed to parse cargo's dep-info at {:?}", dep_info);
+        return Ok(None);
     };
     let mut ret = RustcDepInfo::default();
     ret.env = info.env;
@@ -1852,9 +1841,8 @@ where
     I: IntoIterator,
     I::Item: AsRef<Path>,
 {
-    let reference_mtime = match paths::mtime(reference) {
-        Ok(mtime) => mtime,
-        Err(..) => return Some(StaleItem::MissingFile(reference.to_path_buf())),
+    let Ok(reference_mtime) = paths::mtime(reference) else {
+        return Some(StaleItem::MissingFile(reference.to_path_buf()));
     };
 
     let skipable_dirs = if let Ok(cargo_home) = home::cargo_home() {
@@ -1882,9 +1870,8 @@ where
         let path_mtime = match mtime_cache.entry(path.to_path_buf()) {
             Entry::Occupied(o) => *o.get(),
             Entry::Vacant(v) => {
-                let mtime = match paths::mtime_recursive(path) {
-                    Ok(mtime) => mtime,
-                    Err(..) => return Some(StaleItem::MissingFile(path.to_path_buf())),
+                let Ok(mtime) = paths::mtime_recursive(path) else {
+                    return Some(StaleItem::MissingFile(path.to_path_buf()));
                 };
                 *v.insert(mtime)
             }
@@ -2156,9 +2143,8 @@ pub fn parse_rustc_dep_info(rustc_dep_info: &Path) -> CargoResult<RustcDepInfo>
     for line in contents.lines() {
         if let Some(rest) = line.strip_prefix("# env-dep:") {
             let mut parts = rest.splitn(2, '=');
-            let env_var = match parts.next() {
-                Some(s) => s,
-                None => continue,
+            let Some(env_var) = parts.next() else {
+                continue;
             };
             let env_val = match parts.next() {
                 Some(s) => Some(unescape_env(s)?),

src/cargo/core/compiler/future_incompat.rs

@@ -333,13 +333,11 @@ fn get_updates(ws: &Workspace<'_>, package_ids: &BTreeSet<PackageId>) -> Option<
     let mut summaries = Vec::new();
     while !package_ids.is_empty() {
         package_ids.retain(|&pkg_id| {
-            let source = match sources.get_mut(&pkg_id.source_id()) {
-                Some(s) => s,
-                None => return false,
+            let Some(source) = sources.get_mut(&pkg_id.source_id()) else {
+                return false;
             };
-            let dep = match Dependency::parse(pkg_id.name(), None, pkg_id.source_id()) {
-                Ok(dep) => dep,
-                Err(_) => return false,
+            let Ok(dep) = Dependency::parse(pkg_id.name(), None, pkg_id.source_id()) else {
+                return false;
             };
             match source.query_vec(&dep, QueryKind::Exact) {
                 Poll::Ready(Ok(sum)) => {

src/cargo/core/compiler/job_queue/mod.rs

@@ -941,9 +941,8 @@ impl<'cfg> DrainState<'cfg> {
         cx: &mut Context<'_, '_>,
     ) -> CargoResult<()> {
         let outputs = cx.build_script_outputs.lock().unwrap();
-        let metadata = match cx.find_build_script_metadata(unit) {
-            Some(metadata) => metadata,
-            None => return Ok(()),
+        let Some(metadata) = cx.find_build_script_metadata(unit) else {
+            return Ok(());
         };
         let bcx = &mut cx.bcx;
         if let Some(output) = outputs.get(metadata) {

src/cargo/core/compiler/links.rs

@@ -27,9 +27,8 @@ pub fn validate_links(resolve: &Resolve, unit_graph: &UnitGraph) -> CargoResult<
         if !validated.insert(unit.pkg.package_id()) {
             continue;
         }
-        let lib = match unit.pkg.manifest().links() {
-            Some(lib) => lib,
-            None => continue,
+        let Some(lib) = unit.pkg.manifest().links() else {
+            continue;
         };
         if let Some(&prev) = links.get(lib) {
             let prev_path = resolve

src/cargo/core/compiler/mod.rs

@@ -543,12 +543,9 @@ fn link_targets(cx: &mut Context<'_, '_>, unit: &Unit, fresh: bool) -> CargoResu
             if !src.exists() {
                 continue;
             }
-            let dst = match output.hardlink.as_ref() {
-                Some(dst) => dst,
-                None => {
-                    destinations.push(src.clone());
-                    continue;
-                }
+            let Some(dst) = output.hardlink.as_ref() else {
+                destinations.push(src.clone());
+                continue;
             };
             destinations.push(dst.clone());
             paths::link_or_copy(src, dst)?;
@@ -1321,7 +1318,7 @@ fn add_custom_flags(
                     cmd.arg("--check-cfg").arg(check_cfg);
                 }
             }
-            for &(ref name, ref value) in output.env.iter() {
+            for (name, value) in output.env.iter() {
                 cmd.env(name, value);
             }
         }

src/cargo/core/compiler/timings.rs

@@ -194,9 +194,8 @@ impl<'cfg> Timings<'cfg> {
         // `id` may not always be active. "fresh" units unconditionally
         // generate `Message::Finish`, but this active map only tracks dirty
         // units.
-        let unit_time = match self.active.get_mut(&id) {
-            Some(ut) => ut,
-            None => return,
+        let Some(unit_time) = self.active.get_mut(&id) else {
+            return;
         };
         let t = self.start.elapsed().as_secs_f64();
         unit_time.rmeta_time = Some(t - unit_time.start);
@@ -212,9 +211,8 @@ impl<'cfg> Timings<'cfg> {
             return;
         }
         // See note above in `unit_rmeta_finished`, this may not always be active.
-        let mut unit_time = match self.active.remove(&id) {
-            Some(ut) => ut,
-            None => return,
+        let Some(mut unit_time) = self.active.remove(&id) else {
+            return;
         };
         let t = self.start.elapsed().as_secs_f64();
         unit_time.duration = t - unit_time.start;
@@ -265,9 +263,8 @@ impl<'cfg> Timings<'cfg> {
         if !self.enabled {
             return;
         }
-        let prev = match &mut self.last_cpu_state {
-            Some(state) => state,
-            None => return,
+        let Some(prev) = &mut self.last_cpu_state else {
+            return;
         };
         // Don't take samples too frequently, even if requested.
         let now = Instant::now();