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.

Author: nyurik

Cargo.lock

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

benches/benchsuite/benches/global_cache_tracker.rs

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

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

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

benches/benchsuite/src/lib.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/cargo-util-schemas/src/manifest/mod.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/resolver-tests/tests/resolve.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.

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

@@ -1,6 +1,6 @@
 [package]
 name = "cargo-credential-1password"
-version = "0.4.4"
+version = "0.5.0"
 rust-version.workspace = true
 edition.workspace = true
 license.workspace = true

credential/cargo-credential-1password/Cargo.toml

@@ -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)

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

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