Auto merge of #12970 - Eh2406:MaybeSummary, r=epage

query{_vec} use IndexSummary

This builds on the work from #12749 and the discussion of the overall approach is at https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/better.20error.20messages.20for.20filtered.20versions.2E

### What does this PR try to resolve?

Changing the two traits `Registry` and `Source` to use the new `IndexSummary' involves a lot of changes all throughout the code base. This would be hard to review if it also included any logic changes. So this PR is just adding the type to the trait and immediately unwrapping every place it is used.

The hope is that reviewing changes to logic/ergonomics will be easier to review once the mechanical changes have been merged.

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

This is an internal re-factoring and all the tests still pass.

Author: bors

crates/resolver-tests/src/lib.rs

@@ -17,6 +17,7 @@ use cargo::core::Resolve;
 use cargo::core::{Dependency, PackageId, Registry, Summary};
 use cargo::core::{GitReference, SourceId};
 use cargo::sources::source::QueryKind;
+use cargo::sources::IndexSummary;
 use cargo::util::{CargoResult, Config, Graph, IntoUrl, RustVersion};
 
 use proptest::collection::{btree_map, vec};
@@ -131,7 +132,7 @@ pub fn resolve_with_config_raw(
             &mut self,
             dep: &Dependency,
             kind: QueryKind,
-            f: &mut dyn FnMut(Summary),
+            f: &mut dyn FnMut(IndexSummary),
         ) -> Poll<CargoResult<()>> {
             for summary in self.list.iter() {
                 let matched = match kind {
@@ -140,7 +141,7 @@ pub fn resolve_with_config_raw(
                 };
                 if matched {
                     self.used.insert(summary.package_id());
-                    f(summary.clone());
+                    f(IndexSummary::Candidate(summary.clone()));
                 }
             }
             Poll::Ready(Ok(()))

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

@@ -377,6 +377,7 @@ fn check_crates_io<'a>(
                 "`{name}@{current}` needs a bump because its should have a version newer than crates.io: {:?}`",
                 possibilities
                     .iter()
+                    .map(|s| s.as_summary())
                     .map(|s| format!("{}@{}", s.name(), s.version()))
                     .collect::<Vec<_>>(),
             );

src/cargo/core/compiler/future_incompat.rs

@@ -348,7 +348,7 @@ fn get_updates(ws: &Workspace<'_>, package_ids: &BTreeSet<PackageId>) -> Option<
     for (pkg_id, summaries) in summaries {
         let mut updated_versions: Vec<_> = summaries
             .iter()
-            .map(|summary| summary.version())
+            .map(|summary| summary.as_summary().version())
             .filter(|version| *version > pkg_id.version())
             .collect();
         updated_versions.sort();

src/cargo/core/registry.rs

@@ -7,6 +7,7 @@ use crate::sources::config::SourceConfigMap;
 use crate::sources::source::QueryKind;
 use crate::sources::source::Source;
 use crate::sources::source::SourceMap;
+use crate::sources::IndexSummary;
 use crate::util::errors::CargoResult;
 use crate::util::interning::InternedString;
 use crate::util::{CanonicalUrl, Config};
@@ -23,10 +24,14 @@ pub trait Registry {
         &mut self,
         dep: &Dependency,
         kind: QueryKind,
-        f: &mut dyn FnMut(Summary),
+        f: &mut dyn FnMut(IndexSummary),
     ) -> Poll<CargoResult<()>>;
 
-    fn query_vec(&mut self, dep: &Dependency, kind: QueryKind) -> Poll<CargoResult<Vec<Summary>>> {
+    fn query_vec(
+        &mut self,
+        dep: &Dependency,
+        kind: QueryKind,
+    ) -> Poll<CargoResult<Vec<IndexSummary>>> {
         let mut ret = Vec::new();
         self.query(dep, kind, &mut |s| ret.push(s)).map_ok(|()| ret)
     }
@@ -337,6 +342,8 @@ impl<'cfg> PackageRegistry<'cfg> {
                     }
                 };
 
+                let summaries = summaries.into_iter().map(|s| s.into_summary()).collect();
+
                 let (summary, should_unlock) =
                     match summary_for_patch(orig_patch, &locked, summaries, source) {
                         Poll::Ready(x) => x,
@@ -481,13 +488,15 @@ impl<'cfg> PackageRegistry<'cfg> {
         Ok(())
     }
 
-    fn query_overrides(&mut self, dep: &Dependency) -> Poll<CargoResult<Option<Summary>>> {
+    fn query_overrides(&mut self, dep: &Dependency) -> Poll<CargoResult<Option<IndexSummary>>> {
         for &s in self.overrides.iter() {
             let src = self.sources.get_mut(s).unwrap();
             let dep = Dependency::new_override(dep.package_name(), s);
-            let mut results = ready!(src.query_vec(&dep, QueryKind::Exact))?;
-            if !results.is_empty() {
-                return Poll::Ready(Ok(Some(results.remove(0))));
+
+            let mut results = None;
+            ready!(src.query(&dep, QueryKind::Exact, &mut |s| results = Some(s)))?;
+            if results.is_some() {
+                return Poll::Ready(Ok(results));
             }
         }
         Poll::Ready(Ok(None))
@@ -575,7 +584,7 @@ impl<'cfg> Registry for PackageRegistry<'cfg> {
         &mut self,
         dep: &Dependency,
         kind: QueryKind,
-        f: &mut dyn FnMut(Summary),
+        f: &mut dyn FnMut(IndexSummary),
     ) -> Poll<CargoResult<()>> {
         assert!(self.patches_locked);
         let (override_summary, n, to_warn) = {
@@ -607,9 +616,9 @@ impl<'cfg> Registry for PackageRegistry<'cfg> {
             if patches.len() == 1 && dep.is_locked() {
                 let patch = patches.remove(0);
                 match override_summary {
-                    Some(summary) => (summary, 1, Some(patch)),
+                    Some(summary) => (summary, 1, Some(IndexSummary::Candidate(patch))),
                     None => {
-                        f(patch);
+                        f(IndexSummary::Candidate(patch));
                         return Poll::Ready(Ok(()));
                     }
                 }
@@ -646,7 +655,7 @@ impl<'cfg> Registry for PackageRegistry<'cfg> {
                     // everything upstairs after locking the summary
                     (None, Some(source)) => {
                         for patch in patches.iter() {
-                            f(patch.clone());
+                            f(IndexSummary::Candidate(patch.clone()));
                         }
 
                         // Our sources shouldn't ever come back to us with two
@@ -658,14 +667,18 @@ impl<'cfg> Registry for PackageRegistry<'cfg> {
                         // already selected, then we skip this `summary`.
                         let locked = &self.locked;
                         let all_patches = &self.patches_available;
-                        let callback = &mut |summary: Summary| {
+                        let callback = &mut |summary: IndexSummary| {
                             for patch in patches.iter() {
                                 let patch = patch.package_id().version();
                                 if summary.package_id().version() == patch {
                                     return;
                                 }
                             }
-                            f(lock(locked, all_patches, summary))
+                            f(IndexSummary::Candidate(lock(
+                                locked,
+                                all_patches,
+                                summary.into_summary(),
+                            )))
                         };
                         return source.query(dep, kind, callback);
                     }
@@ -702,9 +715,12 @@ impl<'cfg> Registry for PackageRegistry<'cfg> {
                 "found an override with a non-locked list"
             )));
         } else if let Some(summary) = to_warn {
-            self.warn_bad_override(&override_summary, &summary)?;
+            self.warn_bad_override(override_summary.as_summary(), summary.as_summary())?;
         }
-        f(self.lock(override_summary));
+        f(IndexSummary::Candidate(
+            self.lock(override_summary.into_summary()),
+        ));
+
         Poll::Ready(Ok(()))
     }
 
@@ -887,6 +903,8 @@ fn summary_for_patch(
                 Vec::new()
             });
 
+        let orig_matches = orig_matches.into_iter().map(|s| s.into_summary()).collect();
+
         let summary = ready!(summary_for_patch(orig_patch, &None, orig_matches, source))?;
 
         // The unlocked version found a match. This returns a value to
@@ -907,7 +925,7 @@ fn summary_for_patch(
         });
     let mut vers = name_summaries
         .iter()
-        .map(|summary| summary.version())
+        .map(|summary| summary.as_summary().version())
         .collect::<Vec<_>>();
     let found = match vers.len() {
         0 => format!(""),

src/cargo/core/resolver/dep_cache.rs

@@ -105,7 +105,7 @@ impl<'a> RegistryQueryer<'a> {
 
         let mut ret = Vec::new();
         let ready = self.registry.query(dep, QueryKind::Exact, &mut |s| {
-            ret.push(s);
+            ret.push(s.into_summary());
         })?;
         if ready.is_pending() {
             self.registry_cache
@@ -135,16 +135,19 @@ impl<'a> RegistryQueryer<'a> {
                     return Poll::Pending;
                 }
             };
-            let s = summaries.next().ok_or_else(|| {
-                anyhow::format_err!(
-                    "no matching package for override `{}` found\n\
+            let s = summaries
+                .next()
+                .ok_or_else(|| {
+                    anyhow::format_err!(
+                        "no matching package for override `{}` found\n\
                      location searched: {}\n\
                      version required: {}",
-                    spec,
-                    dep.source_id(),
-                    dep.version_req()
-                )
-            })?;
+                        spec,
+                        dep.source_id(),
+                        dep.version_req()
+                    )
+                })?
+                .into_summary();
             let summaries = summaries.collect::<Vec<_>>();
             if !summaries.is_empty() {
                 let bullets = summaries

src/cargo/core/resolver/errors.rs

@@ -228,7 +228,7 @@ pub(super) fn activation_error(
     let mut new_dep = dep.clone();
     new_dep.set_version_req(OptVersionReq::Any);
 
-    let mut candidates = loop {
+    let candidates = loop {
         match registry.query_vec(&new_dep, QueryKind::Exact) {
             Poll::Ready(Ok(candidates)) => break candidates,
             Poll::Ready(Err(e)) => return to_resolve_err(e),
@@ -239,6 +239,8 @@ pub(super) fn activation_error(
         }
     };
 
+    let mut candidates: Vec<_> = candidates.into_iter().map(|s| s.into_summary()).collect();
+
     candidates.sort_unstable_by(|a, b| b.version().cmp(a.version()));
 
     let mut msg = if !candidates.is_empty() {
@@ -303,7 +305,7 @@ pub(super) fn activation_error(
     } else {
         // Maybe the user mistyped the name? Like `dep-thing` when `Dep_Thing`
         // was meant. So we try asking the registry for a `fuzzy` search for suggestions.
-        let mut candidates = loop {
+        let candidates = loop {
             match registry.query_vec(&new_dep, QueryKind::Fuzzy) {
                 Poll::Ready(Ok(candidates)) => break candidates,
                 Poll::Ready(Err(e)) => return to_resolve_err(e),
@@ -314,6 +316,8 @@ pub(super) fn activation_error(
             }
         };
 
+        let mut candidates: Vec<_> = candidates.into_iter().map(|s| s.into_summary()).collect();
+
         candidates.sort_unstable_by_key(|a| a.name());
         candidates.dedup_by(|a, b| a.name() == b.name());
         let mut candidates: Vec<_> = candidates

src/cargo/ops/cargo_add/mod.rs

@@ -545,7 +545,7 @@ fn get_latest_dependency(
             unreachable!("registry dependencies required, found a workspace dependency");
         }
         MaybeWorkspace::Other(query) => {
-            let mut possibilities = loop {
+            let possibilities = loop {
                 match registry.query_vec(&query, QueryKind::Fuzzy) {
                     std::task::Poll::Ready(res) => {
                         break res?;
@@ -554,6 +554,11 @@ fn get_latest_dependency(
                 }
             };
 
+            let mut possibilities: Vec<_> = possibilities
+                .into_iter()
+                .map(|s| s.into_summary())
+                .collect();
+
             possibilities.sort_by_key(|s| {
                 // Fallback to a pre-release if no official release is available by sorting them as
                 // less.
@@ -671,6 +676,12 @@ fn select_package(
                     std::task::Poll::Pending => registry.block_until_ready()?,
                 }
             };
+
+            let possibilities: Vec<_> = possibilities
+                .into_iter()
+                .map(|s| s.into_summary())
+                .collect();
+
             match possibilities.len() {
                 0 => {
                     let source = dependency
@@ -889,6 +900,7 @@ fn populate_available_features(
     // in the lock file for a given version requirement.
     let lowest_common_denominator = possibilities
         .iter()
+        .map(|s| s.as_summary())
         .min_by_key(|s| {
             // Fallback to a pre-release if no official release is available by sorting them as
             // more.

src/cargo/ops/common_for_install_and_uninstall.rs

@@ -552,7 +552,11 @@ where
             Poll::Pending => source.block_until_ready()?,
         }
     };
-    match deps.iter().max_by_key(|p| p.package_id()) {
+    match deps
+        .iter()
+        .map(|s| s.as_summary())
+        .max_by_key(|p| p.package_id())
+    {
         Some(summary) => {
             if let (Some(current), Some(msrv)) = (current_rust_version, summary.rust_version()) {
                 let msrv_req = msrv.to_caret_req();
@@ -571,6 +575,7 @@ where
                         };
                         if let Some(alt) = msrv_deps
                             .iter()
+                            .map(|s| s.as_summary())
                             .filter(|summary| {
                                 summary
                                     .rust_version()

src/cargo/sources/directory.rs

@@ -3,10 +3,11 @@ use std::fmt::{self, Debug, Formatter};
 use std::path::{Path, PathBuf};
 use std::task::Poll;
 
-use crate::core::{Dependency, Package, PackageId, SourceId, Summary};
+use crate::core::{Dependency, Package, PackageId, SourceId};
 use crate::sources::source::MaybePackage;
 use crate::sources::source::QueryKind;
 use crate::sources::source::Source;
+use crate::sources::IndexSummary;
 use crate::sources::PathSource;
 use crate::util::errors::CargoResult;
 use crate::util::Config;
@@ -99,7 +100,7 @@ impl<'cfg> Source for DirectorySource<'cfg> {
         &mut self,
         dep: &Dependency,
         kind: QueryKind,
-        f: &mut dyn FnMut(Summary),
+        f: &mut dyn FnMut(IndexSummary),
     ) -> Poll<CargoResult<()>> {
         if !self.updated {
             return Poll::Pending;
@@ -110,7 +111,7 @@ impl<'cfg> Source for DirectorySource<'cfg> {
             QueryKind::Fuzzy => true,
         });
         for summary in matches.map(|pkg| pkg.summary().clone()) {
-            f(summary);
+            f(IndexSummary::Candidate(summary));
         }
         Poll::Ready(Ok(()))
     }

src/cargo/sources/git/source.rs

@@ -3,11 +3,12 @@
 use crate::core::global_cache_tracker;
 use crate::core::GitReference;
 use crate::core::SourceId;
-use crate::core::{Dependency, Package, PackageId, Summary};
+use crate::core::{Dependency, Package, PackageId};
 use crate::sources::git::utils::GitRemote;
 use crate::sources::source::MaybePackage;
 use crate::sources::source::QueryKind;
 use crate::sources::source::Source;
+use crate::sources::IndexSummary;
 use crate::sources::PathSource;
 use crate::util::cache_lock::CacheLockMode;
 use crate::util::errors::CargoResult;
@@ -192,7 +193,7 @@ impl<'cfg> Source for GitSource<'cfg> {
         &mut self,
         dep: &Dependency,
         kind: QueryKind,
-        f: &mut dyn FnMut(Summary),
+        f: &mut dyn FnMut(IndexSummary),
     ) -> Poll<CargoResult<()>> {
         if let Some(src) = self.path_source.as_mut() {
             src.query(dep, kind, f)

src/cargo/sources/mod.rs

@@ -30,7 +30,9 @@ pub use self::config::SourceConfigMap;
 pub use self::directory::DirectorySource;
 pub use self::git::GitSource;
 pub use self::path::PathSource;
-pub use self::registry::{RegistrySource, CRATES_IO_DOMAIN, CRATES_IO_INDEX, CRATES_IO_REGISTRY};
+pub use self::registry::{
+    IndexSummary, RegistrySource, CRATES_IO_DOMAIN, CRATES_IO_INDEX, CRATES_IO_REGISTRY,
+};
 pub use self::replaced::ReplacedSource;
 
 pub mod config;