Migrate Cargo's &Option<T> into Option<&T>

As part of this, also modify `Option<&String>` to `Option<&str>`, same for `PathBuf`

Trying out my new lint rust-lang/rust-clippy#13336 - according to the [video](https://www.youtube.com/watch?v=6c7pZYP_iIE), this could lead to some performance and memory optimizations.

Basic thoughts expressed in the video that seem to make sense:
* `&Option<T>` in an API breaks encapsulation:
  * caller must own T and move it into an Option to call with it
  * if returned, the owner must store it as Option<T> internally in order to return it
* Performance is subject to compiler optimization, but at the basics, `&Option<T>` points to memory that has `presence` flag + value, whereas `Option<&T>` by specification is always optimized to a single pointer.

benches/benchsuite/benches/global_cache_tracker.rs

@@ -43,7 +43,7 @@ fn initialize_context() -> GlobalContext {
         false,
         false,
         false,
-        &None,
+        None,
         &["gc".to_string()],
         &[],
     )

benches/benchsuite/src/bin/capture-last-use.rs

@@ -36,7 +36,7 @@ fn main() {
         false,
         false,
         false,
-        &None,
+        None,
         &["gc".to_string()],
         &[],
     )

benches/benchsuite/src/lib.rs

@@ -188,7 +188,7 @@ impl Fixtures {
             false,
             false,
             false,
-            &Some(self.target_dir()),
+            Some(&self.target_dir()),
             &[],
             &[],
         )

crates/cargo-util-schemas/src/manifest/mod.rs

@@ -239,8 +239,11 @@ impl TomlPackage {
         }
     }
 
-    pub fn normalized_edition(&self) -> Result<Option<&String>, UnresolvedError> {
-        self.edition.as_ref().map(|v| v.normalized()).transpose()
+    pub fn normalized_edition(&self) -> Result<Option<&str>, UnresolvedError> {
+        self.edition
+            .as_ref()
+            .map(|v| v.normalized().map(String::as_str))
+            .transpose()
     }
 
     pub fn normalized_rust_version(&self) -> Result<Option<&RustVersion>, UnresolvedError> {
@@ -258,7 +261,7 @@ impl TomlPackage {
         self.authors.as_ref().map(|v| v.normalized()).transpose()
     }
 
-    pub fn normalized_build(&self) -> Result<Option<&String>, UnresolvedError> {
+    pub fn normalized_build(&self) -> Result<Option<&str>, UnresolvedError> {
         let readme = self.build.as_ref().ok_or(UnresolvedError)?;
         match readme {
             StringOrBool::Bool(false) => Ok(None),
@@ -279,30 +282,33 @@ impl TomlPackage {
         self.publish.as_ref().map(|v| v.normalized()).transpose()
     }
 
-    pub fn normalized_description(&self) -> Result<Option<&String>, UnresolvedError> {
+    pub fn normalized_description(&self) -> Result<Option<&str>, UnresolvedError> {
         self.description
             .as_ref()
-            .map(|v| v.normalized())
+            .map(|v| v.normalized().map(String::as_str))
             .transpose()
     }
 
-    pub fn normalized_homepage(&self) -> Result<Option<&String>, UnresolvedError> {
-        self.homepage.as_ref().map(|v| v.normalized()).transpose()
+    pub fn normalized_homepage(&self) -> Result<Option<&str>, UnresolvedError> {
+        self.homepage
+            .as_ref()
+            .map(|v| v.normalized().map(String::as_str))
+            .transpose()
     }
 
-    pub fn normalized_documentation(&self) -> Result<Option<&String>, UnresolvedError> {
+    pub fn normalized_documentation(&self) -> Result<Option<&str>, UnresolvedError> {
         self.documentation
             .as_ref()
-            .map(|v| v.normalized())
+            .map(|v| v.normalized().map(String::as_str))
             .transpose()
     }
 
-    pub fn normalized_readme(&self) -> Result<Option<&String>, UnresolvedError> {
+    pub fn normalized_readme(&self) -> Result<Option<&str>, UnresolvedError> {
         let readme = self.readme.as_ref().ok_or(UnresolvedError)?;
         readme.normalized().and_then(|sb| match sb {
             StringOrBool::Bool(false) => Ok(None),
             StringOrBool::Bool(true) => Err(UnresolvedError),
-            StringOrBool::String(value) => Ok(Some(value)),
+            StringOrBool::String(value) => Ok(Some(value.as_str())),
         })
     }
 
@@ -314,19 +320,25 @@ impl TomlPackage {
         self.categories.as_ref().map(|v| v.normalized()).transpose()
     }
 
-    pub fn normalized_license(&self) -> Result<Option<&String>, UnresolvedError> {
-        self.license.as_ref().map(|v| v.normalized()).transpose()
+    pub fn normalized_license(&self) -> Result<Option<&str>, UnresolvedError> {
+        self.license
+            .as_ref()
+            .map(|v| v.normalized().map(String::as_str))
+            .transpose()
     }
 
-    pub fn normalized_license_file(&self) -> Result<Option<&String>, UnresolvedError> {
+    pub fn normalized_license_file(&self) -> Result<Option<&str>, UnresolvedError> {
         self.license_file
             .as_ref()
-            .map(|v| v.normalized())
+            .map(|v| v.normalized().map(String::as_str))
             .transpose()
     }
 
-    pub fn normalized_repository(&self) -> Result<Option<&String>, UnresolvedError> {
-        self.repository.as_ref().map(|v| v.normalized()).transpose()
+    pub fn normalized_repository(&self) -> Result<Option<&str>, UnresolvedError> {
+        self.repository
+            .as_ref()
+            .map(|v| v.normalized().map(String::as_str))
+            .transpose()
     }
 }
 

crates/resolver-tests/tests/resolve.rs

@@ -69,7 +69,7 @@ proptest! {
                 false,
                 false,
                 false,
-                &None,
+                None,
                 &["minimal-versions".to_string()],
                 &[],
             )
@@ -110,7 +110,7 @@ proptest! {
                 false,
                 false,
                 false,
-                &None,
+                None,
                 &["direct-minimal-versions".to_string()],
                 &[],
             )
@@ -436,7 +436,7 @@ fn test_resolving_minimum_version_with_transitive_deps() {
         false,
         false,
         false,
-        &None,
+        None,
         &["minimal-versions".to_string()],
         &[],
     )

crates/xtask-bump-check/src/xtask.rs

@@ -98,7 +98,7 @@ fn global_context_configure(gctx: &mut GlobalContext, args: &ArgMatches) -> CliR
         frozen,
         locked,
         offline,
-        &None,
+        None,
         &unstable_flags,
         &config_args,
     )?;
@@ -333,7 +333,7 @@ fn changed<'r, 'ws>(
     let ws_members = ws
         .members()
         .filter(|pkg| pkg.name() != root_pkg_name) // Only take care of sub crates here.
-        .filter(|pkg| pkg.publish() != &Some(vec![])) // filter out `publish = false`
+        .filter(|pkg| pkg.publish() != Some(&vec![])) // filter out `publish = false`
         .map(|pkg| {
             // Having relative package root path so that we can compare with
             // paths of changed files to determine which package has changed.

credential/cargo-credential-1password/src/main.rs

@@ -114,7 +114,7 @@ impl OnePasswordKeychain {
         Ok(Some(buffer))
     }
 
-    fn make_cmd(&self, session: &Option<String>, args: &[&str]) -> Command {
+    fn make_cmd(&self, session: Option<&str>, args: &[&str]) -> Command {
         let mut cmd = Command::new("op");
         cmd.args(args);
         if let Some(account) = &self.account {
@@ -153,7 +153,7 @@ impl OnePasswordKeychain {
         Ok(buffer)
     }
 
-    fn search(&self, session: &Option<String>, index_url: &str) -> Result<Option<String>, Error> {
+    fn search(&self, session: Option<&str>, index_url: &str) -> Result<Option<String>, Error> {
         let cmd = self.make_cmd(
             session,
             &[
@@ -192,7 +192,7 @@ impl OnePasswordKeychain {
 
     fn modify(
         &self,
-        session: &Option<String>,
+        session: Option<&str>,
         id: &str,
         token: Secret<&str>,
         _name: Option<&str>,
@@ -207,7 +207,7 @@ impl OnePasswordKeychain {
 
     fn create(
         &self,
-        session: &Option<String>,
+        session: Option<&str>,
         index_url: &str,
         token: Secret<&str>,
         name: Option<&str>,
@@ -235,7 +235,7 @@ impl OnePasswordKeychain {
         Ok(())
     }
 
-    fn get_token(&self, session: &Option<String>, id: &str) -> Result<Secret<String>, Error> {
+    fn get_token(&self, session: Option<&str>, id: &str) -> Result<Secret<String>, Error> {
         let cmd = self.make_cmd(session, &["item", "get", "--format=json", id]);
         let buffer = self.run_cmd(cmd)?;
         let item: Login = serde_json::from_str(&buffer)
@@ -250,7 +250,7 @@ impl OnePasswordKeychain {
         }
     }
 
-    fn delete(&self, session: &Option<String>, id: &str) -> Result<(), Error> {
+    fn delete(&self, session: Option<&str>, id: &str) -> Result<(), Error> {
         let cmd = self.make_cmd(session, &["item", "delete", id]);
         self.run_cmd(cmd)?;
         Ok(())
@@ -270,8 +270,8 @@ impl Credential for OnePasswordCredential {
         match action {
             Action::Get(_) => {
                 let session = op.signin()?;
-                if let Some(id) = op.search(&session, registry.index_url)? {
-                    op.get_token(&session, &id)
+                if let Some(id) = op.search(session.as_deref(), registry.index_url)? {
+                    op.get_token(session.as_deref(), &id)
                         .map(|token| CredentialResponse::Get {
                             token,
                             cache: CacheControl::Session,
@@ -284,21 +284,26 @@ impl Credential for OnePasswordCredential {
             Action::Login(options) => {
                 let session = op.signin()?;
                 // Check if an item already exists.
-                if let Some(id) = op.search(&session, registry.index_url)? {
+                if let Some(id) = op.search(session.as_deref(), registry.index_url)? {
                     eprintln!("note: token already exists for `{}`", registry.index_url);
                     let token = cargo_credential::read_token(options, registry)?;
-                    op.modify(&session, &id, token.as_deref(), None)?;
+                    op.modify(session.as_deref(), &id, token.as_deref(), None)?;
                 } else {
                     let token = cargo_credential::read_token(options, registry)?;
-                    op.create(&session, registry.index_url, token.as_deref(), None)?;
+                    op.create(
+                        session.as_deref(),
+                        registry.index_url,
+                        token.as_deref(),
+                        None,
+                    )?;
                 }
                 Ok(CredentialResponse::Login)
             }
             Action::Logout => {
                 let session = op.signin()?;
                 // Check if an item already exists.
-                if let Some(id) = op.search(&session, registry.index_url)? {
-                    op.delete(&session, &id)?;
+                if let Some(id) = op.search(session.as_deref(), registry.index_url)? {
+                    op.delete(session.as_deref(), &id)?;
                     Ok(CredentialResponse::Logout)
                 } else {
                     Err(Error::NotFound)

src/bin/cargo/cli.rs

@@ -436,7 +436,7 @@ fn configure_gctx(
         frozen,
         locked,
         offline,
-        arg_target_dir,
+        arg_target_dir.as_deref(),
         &unstable_flags,
         &config_args,
     )?;

src/cargo/core/compiler/build_plan.rs

@@ -64,10 +64,10 @@ impl Invocation {
         }
     }
 
-    pub fn add_output(&mut self, path: &Path, link: &Option<PathBuf>) {
+    pub fn add_output(&mut self, path: &Path, link: Option<&Path>) {
         self.outputs.push(path.to_path_buf());
-        if let Some(ref link) = *link {
-            self.links.insert(link.clone(), path.to_path_buf());
+        if let Some(link) = link {
+            self.links.insert(link.to_path_buf(), path.to_path_buf());
         }
     }
 
@@ -134,7 +134,7 @@ impl BuildPlan {
 
         invocation.update_cmd(cmd)?;
         for output in outputs.iter() {
-            invocation.add_output(&output.path, &output.hardlink);
+            invocation.add_output(&output.path, output.hardlink.as_deref());
         }
 
         Ok(())

src/cargo/core/compiler/custom_build.rs

@@ -555,7 +555,7 @@ fn build_work(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> CargoResul
             &script_out_dir,
             nightly_features_allowed,
             &targets,
-            &msrv,
+            msrv.as_ref(),
         )?;
 
         if json_messages {
@@ -583,7 +583,7 @@ fn build_work(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> CargoResul
                 &script_out_dir,
                 nightly_features_allowed,
                 &targets_fresh,
-                &msrv_fresh,
+                msrv_fresh.as_ref(),
             )?,
         };
 
@@ -639,7 +639,7 @@ impl BuildOutput {
         script_out_dir: &Path,
         nightly_features_allowed: bool,
         targets: &[Target],
-        msrv: &Option<RustVersion>,
+        msrv: Option<&RustVersion>,
     ) -> CargoResult<BuildOutput> {
         let contents = paths::read_bytes(path)?;
         BuildOutput::parse(
@@ -667,7 +667,7 @@ impl BuildOutput {
         script_out_dir: &Path,
         nightly_features_allowed: bool,
         targets: &[Target],
-        msrv: &Option<RustVersion>,
+        msrv: Option<&RustVersion>,
     ) -> CargoResult<BuildOutput> {
         let mut library_paths = Vec::new();
         let mut library_links = Vec::new();
@@ -717,7 +717,7 @@ impl BuildOutput {
 
         fn check_minimum_supported_rust_version_for_new_syntax(
             pkg_descr: &str,
-            msrv: &Option<RustVersion>,
+            msrv: Option<&RustVersion>,
             flag: &str,
         ) -> CargoResult<()> {
             if let Some(msrv) = msrv {
@@ -1250,7 +1250,7 @@ fn prev_build_output(
             &script_out_dir,
             build_runner.bcx.gctx.nightly_features_allowed,
             unit.pkg.targets(),
-            &unit.pkg.rust_version().cloned(),
+            unit.pkg.rust_version(),
         )
         .ok(),
         prev_script_out_dir,

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

@@ -117,7 +117,7 @@ use std::cell::RefCell;
 use std::collections::{HashMap, HashSet};
 use std::fmt::Write as _;
 use std::io;
-use std::path::{Path, PathBuf};
+use std::path::Path;
 use std::sync::Arc;
 use std::thread::{self, Scope};
 use std::time::Duration;
@@ -499,7 +499,7 @@ impl<'gctx> JobQueue<'gctx> {
             pending_queue: Vec::new(),
             print: DiagnosticPrinter::new(
                 build_runner.bcx.gctx,
-                &build_runner.bcx.rustc().workspace_wrapper,
+                build_runner.bcx.rustc().workspace_wrapper.as_deref(),
             ),
             finished: 0,
             per_package_future_incompat_reports: Vec::new(),
@@ -661,7 +661,7 @@ impl<'gctx> DrainState<'gctx> {
                         self.report_warning_count(
                             build_runner.bcx.gctx,
                             id,
-                            &build_runner.bcx.rustc().workspace_wrapper,
+                            build_runner.bcx.rustc().workspace_wrapper.as_deref(),
                         );
                         self.active.remove(&id).unwrap()
                     }
@@ -807,7 +807,10 @@ impl<'gctx> DrainState<'gctx> {
         }
 
         let time_elapsed = util::elapsed(build_runner.bcx.gctx.creation_time().elapsed());
-        if let Err(e) = self.timings.finished(build_runner, &errors.to_error()) {
+        if let Err(e) = self
+            .timings
+            .finished(build_runner, errors.to_error().as_ref())
+        {
             self.handle_error(&mut build_runner.bcx.gctx.shell(), &mut errors, e);
         }
         if build_runner.bcx.build_config.emit_json() {
@@ -1021,7 +1024,7 @@ impl<'gctx> DrainState<'gctx> {
         &mut self,
         gctx: &GlobalContext,
         id: JobId,
-        rustc_workspace_wrapper: &Option<PathBuf>,
+        rustc_workspace_wrapper: Option<&Path>,
     ) {
         let count = match self.warning_count.remove(&id) {
             // An error could add an entry for a `Unit`