refactor: use RegistryOrIndex enum to replace two booleans

weihanglo · weihanglo · commit 11754e992311 · 2023-09-16T01:10:16.000+08:00
diff --git a/src/bin/cargo/commands/install.rs b/src/bin/cargo/commands/install.rs
@@ -154,10 +154,11 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     } else if krates.is_empty() {
         from_cwd = true;
         SourceId::for_path(config.cwd())?
-    } else if let Some(index) = args.get_one::<String>("index") {
-        SourceId::for_registry(&index.into_url()?)?
-    } else if let Some(registry) = args.registry(config)? {
-        SourceId::alt_registry(config, &registry)?
+    } else if let Some(reg_or_index) = args.registry_or_index(config)? {
+        match reg_or_index {
+            ops::RegistryOrIndex::Registry(r) => SourceId::alt_registry(config, &r)?,
+            ops::RegistryOrIndex::Index(url) => SourceId::for_registry(&url)?,
+        }
     } else {
         SourceId::crates_io(config)?
     };
diff --git a/src/bin/cargo/commands/login.rs b/src/bin/cargo/commands/login.rs
@@ -1,6 +1,7 @@
-use crate::command_prelude::*;
-
 use cargo::ops;
+use cargo::ops::RegistryOrIndex;
+
+use crate::command_prelude::*;
 
 pub fn cli() -> Command {
     subcommand("login")
@@ -20,7 +21,12 @@ pub fn cli() -> Command {
 }
 
 pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
-    let registry = args.registry(config)?;
+    let reg = args.registry_or_index(config)?;
+    assert!(
+        !matches!(reg, Some(RegistryOrIndex::Index(..))),
+        "must not be index URL"
+    );
+
     let extra_args = args
         .get_many::<String>("args")
         .unwrap_or_default()
@@ -29,7 +35,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     ops::registry_login(
         config,
         args.get_one::<String>("token").map(|s| s.as_str().into()),
-        registry.as_deref(),
+        reg.as_ref(),
         &extra_args,
     )?;
     Ok(())
diff --git a/src/bin/cargo/commands/logout.rs b/src/bin/cargo/commands/logout.rs
@@ -1,5 +1,7 @@
-use crate::command_prelude::*;
 use cargo::ops;
+use cargo::ops::RegistryOrIndex;
+
+use crate::command_prelude::*;
 
 pub fn cli() -> Command {
     subcommand("logout")
@@ -12,7 +14,12 @@ pub fn cli() -> Command {
 }
 
 pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
-    let registry = args.registry(config)?;
-    ops::registry_logout(config, registry.as_deref())?;
+    let reg = args.registry_or_index(config)?;
+    assert!(
+        !matches!(reg, Some(RegistryOrIndex::Index(..))),
+        "must not be index URL"
+    );
+
+    ops::registry_logout(config, reg)?;
     Ok(())
 }
diff --git a/src/bin/cargo/commands/owner.rs b/src/bin/cargo/commands/owner.rs
@@ -34,19 +34,17 @@ pub fn cli() -> Command {
 }
 
 pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
-    let registry = args.registry(config)?;
     let opts = OwnersOptions {
         krate: args.get_one::<String>("crate").cloned(),
         token: args.get_one::<String>("token").cloned().map(Secret::from),
-        index: args.get_one::<String>("index").cloned(),
+        reg_or_index: args.registry_or_index(config)?,
         to_add: args
             .get_many::<String>("add")
             .map(|xs| xs.cloned().collect()),
         to_remove: args
             .get_many::<String>("remove")
             .map(|xs| xs.cloned().collect()),
         list: args.flag("list"),
-        registry,
     };
     ops::modify_owners(config, &opts)?;
     Ok(())
diff --git a/src/bin/cargo/commands/publish.rs b/src/bin/cargo/commands/publish.rs
@@ -30,7 +30,7 @@ pub fn cli() -> Command {
 }
 
 pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
-    let registry = args.registry(config)?;
+    let reg_or_index = args.registry_or_index(config)?;
     let ws = args.workspace(config)?;
     if ws.root_maybe().is_embedded() {
         return Err(anyhow::format_err!(
@@ -39,7 +39,6 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
         )
         .into());
     }
-    let index = args.index()?;
 
     ops::publish(
         &ws,
@@ -48,15 +47,14 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
             token: args
                 .get_one::<String>("token")
                 .map(|s| s.to_string().into()),
-            index,
+            reg_or_index,
             verify: !args.flag("no-verify"),
             allow_dirty: args.flag("allow-dirty"),
             to_publish: args.packages_from_flags()?,
             targets: args.targets()?,
             jobs: args.jobs()?,
             keep_going: args.keep_going(),
             dry_run: args.dry_run(),
-            registry,
             cli_features: args.cli_features()?,
         },
     )?;
diff --git a/src/bin/cargo/commands/search.rs b/src/bin/cargo/commands/search.rs
@@ -24,8 +24,7 @@ pub fn cli() -> Command {
 }
 
 pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
-    let registry = args.registry(config)?;
-    let index = args.index()?;
+    let reg_or_index = args.registry_or_index(config)?;
     let limit = args.value_of_u32("limit")?;
     let limit = min(100, limit.unwrap_or(10));
     let query: Vec<&str> = args
@@ -34,6 +33,6 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
         .map(String::as_str)
         .collect();
     let query: String = query.join("+");
-    ops::search(&query, config, index, limit, registry)?;
+    ops::search(&query, config, reg_or_index, limit)?;
     Ok(())
 }
diff --git a/src/bin/cargo/commands/yank.rs b/src/bin/cargo/commands/yank.rs
@@ -26,8 +26,6 @@ pub fn cli() -> Command {
 }
 
 pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
-    let registry = args.registry(config)?;
-
     let (krate, version) = resolve_crate(
         args.get_one::<String>("crate").map(String::as_str),
         args.get_one::<String>("version").map(String::as_str),
@@ -41,9 +39,8 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
         krate.map(|s| s.to_string()),
         version.map(|s| s.to_string()),
         args.get_one::<String>("token").cloned().map(Secret::from),
-        args.get_one::<String>("index").cloned(),
+        args.registry_or_index(config)?,
         args.flag("undo"),
-        registry,
     )?;
     Ok(())
 }
diff --git a/src/cargo/ops/mod.rs b/src/cargo/ops/mod.rs
@@ -30,6 +30,7 @@ pub use self::registry::yank;
 pub use self::registry::OwnersOptions;
 pub use self::registry::PublishOpts;
 pub use self::registry::RegistryCredentialConfig;
+pub use self::registry::RegistryOrIndex;
 pub use self::resolve::{
     add_overrides, get_resolved_packages, resolve_with_previous, resolve_ws, resolve_ws_with_opts,
     WorkspaceResolve,
diff --git a/src/cargo/ops/registry/login.rs b/src/cargo/ops/registry/login.rs
@@ -16,16 +16,23 @@ use cargo_credential::Secret;
 
 use super::get_source_id;
 use super::registry;
+use super::RegistryOrIndex;
 
 pub fn registry_login(
     config: &Config,
     token_from_cmdline: Option<Secret<&str>>,
-    reg: Option<&str>,
+    reg_or_index: Option<&RegistryOrIndex>,
     args: &[&str],
 ) -> CargoResult<()> {
-    let source_ids = get_source_id(config, None, reg)?;
-
-    let login_url = match registry(config, token_from_cmdline.clone(), None, reg, false, None) {
+    let source_ids = get_source_id(config, reg_or_index)?;
+
+    let login_url = match registry(
+        config,
+        token_from_cmdline.clone(),
+        reg_or_index,
+        false,
+        None,
+    ) {
         Ok((registry, _)) => Some(format!("{}/me", registry.host())),
         Err(e) if e.is::<AuthorizationError>() => e
             .downcast::<AuthorizationError>()
diff --git a/src/cargo/ops/registry/logout.rs b/src/cargo/ops/registry/logout.rs
@@ -8,9 +8,10 @@ use crate::CargoResult;
 use crate::Config;
 
 use super::get_source_id;
+use super::RegistryOrIndex;
 
-pub fn registry_logout(config: &Config, reg: Option<&str>) -> CargoResult<()> {
-    let source_ids = get_source_id(config, None, reg)?;
+pub fn registry_logout(config: &Config, reg_or_index: Option<RegistryOrIndex>) -> CargoResult<()> {
+    let source_ids = get_source_id(config, reg_or_index.as_ref())?;
     auth::logout(config, &source_ids.original)?;
     Ok(())
 }
diff --git a/src/cargo/ops/registry/mod.rs b/src/cargo/ops/registry/mod.rs
@@ -16,6 +16,7 @@ use std::task::Poll;
 use anyhow::{bail, format_err, Context as _};
 use cargo_credential::{Operation, Secret};
 use crates_io::{self, Registry};
+use url::Url;
 
 use crate::core::SourceId;
 use crate::sources::source::Source;
@@ -24,7 +25,6 @@ use crate::util::auth;
 use crate::util::config::{Config, PathAndArgs};
 use crate::util::errors::CargoResult;
 use crate::util::network::http::http_handle;
-use crate::util::IntoUrl;
 
 pub use self::login::registry_login;
 pub use self::logout::registry_logout;
@@ -35,6 +35,19 @@ pub use self::publish::PublishOpts;
 pub use self::search::search;
 pub use self::yank::yank;
 
+/// Represents either `--registry` or `--index` argument, which is mutually exclusive.
+#[derive(Debug, Clone)]
+pub enum RegistryOrIndex {
+    Registry(String),
+    Index(Url),
+}
+
+impl RegistryOrIndex {
+    fn is_index(&self) -> bool {
+        matches!(self, RegistryOrIndex::Index(..))
+    }
+}
+
 /// Registry settings loaded from config files.
 ///
 /// This is loaded based on the `--registry` flag and the config settings.
@@ -103,14 +116,14 @@ impl RegistryCredentialConfig {
 fn registry(
     config: &Config,
     token_from_cmdline: Option<Secret<&str>>,
-    index: Option<&str>,
-    registry: Option<&str>,
+    reg_or_index: Option<&RegistryOrIndex>,
     force_update: bool,
     token_required: Option<Operation<'_>>,
 ) -> CargoResult<(Registry, RegistrySourceIds)> {
-    let source_ids = get_source_id(config, index, registry)?;
+    let source_ids = get_source_id(config, reg_or_index)?;
 
-    if token_required.is_some() && index.is_some() && token_from_cmdline.is_none() {
+    let is_index = reg_or_index.map(|v| v.is_index()).unwrap_or_default();
+    if is_index && token_required.is_some() && token_from_cmdline.is_none() {
         bail!("command-line argument --index requires --token to be specified");
     }
     if let Some(token) = token_from_cmdline {
@@ -171,13 +184,12 @@ fn registry(
 /// crates.io (such as index.crates.io), while the second is always the original source.
 fn get_source_id(
     config: &Config,
-    index: Option<&str>,
-    reg: Option<&str>,
+    reg_or_index: Option<&RegistryOrIndex>,
 ) -> CargoResult<RegistrySourceIds> {
-    let sid = match (reg, index) {
-        (None, None) => SourceId::crates_io(config)?,
-        (_, Some(i)) => SourceId::for_registry(&i.into_url()?)?,
-        (Some(r), None) => SourceId::alt_registry(config, r)?,
+    let sid = match reg_or_index {
+        None => SourceId::crates_io(config)?,
+        Some(RegistryOrIndex::Index(url)) => SourceId::for_registry(url)?,
+        Some(RegistryOrIndex::Registry(r)) => SourceId::alt_registry(config, r)?,
     };
     // Load source replacements that are built-in to Cargo.
     let builtin_replacement_sid = SourceConfigMap::empty(config)?
@@ -186,7 +198,7 @@ fn get_source_id(
     let replacement_sid = SourceConfigMap::new(config)?
         .load(sid, &HashSet::new())?
         .replaced_source_id();
-    if reg.is_none() && index.is_none() && replacement_sid != builtin_replacement_sid {
+    if reg_or_index.is_none() && replacement_sid != builtin_replacement_sid {
         // Neither --registry nor --index was passed and the user has configured source-replacement.
         if let Some(replacement_name) = replacement_sid.alt_registry_key() {
             bail!("crates-io is replaced with remote registry {replacement_name};\ninclude `--registry {replacement_name}` or `--registry crates-io`");
diff --git a/src/cargo/ops/registry/owner.rs b/src/cargo/ops/registry/owner.rs
@@ -13,14 +13,15 @@ use crate::util::important_paths::find_root_manifest_for_wd;
 use crate::CargoResult;
 use crate::Config;
 
+use super::RegistryOrIndex;
+
 pub struct OwnersOptions {
     pub krate: Option<String>,
     pub token: Option<Secret<String>>,
-    pub index: Option<String>,
+    pub reg_or_index: Option<RegistryOrIndex>,
     pub to_add: Option<Vec<String>>,
     pub to_remove: Option<Vec<String>>,
     pub list: bool,
-    pub registry: Option<String>,
 }
 
 pub fn modify_owners(config: &Config, opts: &OwnersOptions) -> CargoResult<()> {
@@ -38,8 +39,7 @@ pub fn modify_owners(config: &Config, opts: &OwnersOptions) -> CargoResult<()> {
     let (mut registry, _) = super::registry(
         config,
         opts.token.as_ref().map(Secret::as_deref),
-        opts.index.as_deref(),
-        opts.registry.as_deref(),
+        opts.reg_or_index.as_ref(),
         true,
         Some(operation),
     )?;
diff --git a/src/cargo/ops/registry/publish.rs b/src/cargo/ops/registry/publish.rs
@@ -37,19 +37,19 @@ use crate::CargoResult;
 use crate::Config;
 
 use super::super::check_dep_has_version;
+use super::RegistryOrIndex;
 
 pub struct PublishOpts<'cfg> {
     pub config: &'cfg Config,
     pub token: Option<Secret<String>>,
-    pub index: Option<String>,
+    pub reg_or_index: Option<RegistryOrIndex>,
     pub verify: bool,
     pub allow_dirty: bool,
     pub jobs: Option<JobsConfig>,
     pub keep_going: bool,
     pub to_publish: ops::Packages,
     pub targets: Vec<String>,
     pub dry_run: bool,
-    pub registry: Option<String>,
     pub cli_features: CliFeatures,
 }
 
@@ -76,7 +76,10 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> {
 
     let (pkg, cli_features) = pkgs.pop().unwrap();
 
-    let mut publish_registry = opts.registry.clone();
+    let mut publish_registry = match opts.reg_or_index.as_ref() {
+        Some(RegistryOrIndex::Registry(registry)) => Some(registry.clone()),
+        _ => None,
+    };
     if let Some(ref allowed_registries) = *pkg.publish() {
         if publish_registry.is_none() && allowed_registries.len() == 1 {
             // If there is only one allowed registry, push to that one directly,
@@ -116,11 +119,16 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> {
     let ver = pkg.version().to_string();
     let operation = Operation::Read;
 
+    let reg_or_index = match opts.reg_or_index.clone() {
+        Some(RegistryOrIndex::Registry(_)) | None => {
+            publish_registry.map(RegistryOrIndex::Registry)
+        }
+        val => val,
+    };
     let (mut registry, reg_ids) = super::registry(
         opts.config,
         opts.token.as_ref().map(Secret::as_deref),
-        opts.index.as_deref(),
-        publish_registry.as_deref(),
+        reg_or_index.as_ref(),
         true,
         Some(operation).filter(|_| !opts.dry_run),
     )?;
diff --git a/src/cargo/ops/registry/search.rs b/src/cargo/ops/registry/search.rs
@@ -13,15 +13,16 @@ use crate::util::truncate_with_ellipsis;
 use crate::CargoResult;
 use crate::Config;
 
+use super::RegistryOrIndex;
+
 pub fn search(
     query: &str,
     config: &Config,
-    index: Option<String>,
+    reg_or_index: Option<RegistryOrIndex>,
     limit: u32,
-    reg: Option<String>,
 ) -> CargoResult<()> {
     let (mut registry, source_ids) =
-        super::registry(config, None, index.as_deref(), reg.as_deref(), false, None)?;
+        super::registry(config, None, reg_or_index.as_ref(), false, None)?;
     let (crates, total_crates) = registry.search(query, limit).with_context(|| {
         format!(
             "failed to retrieve search results from the registry at {}",
diff --git a/src/cargo/ops/registry/yank.rs b/src/cargo/ops/registry/yank.rs
diff --git a/src/cargo/util/command_prelude.rs b/src/cargo/util/command_prelude.rs
