refactor: Parse, don't validate, rust_version

By using the `PartialVersion` type with serde, we get the context for
the error automatically.

Author: epage

crates/resolver-tests/src/lib.rs

@@ -15,7 +15,7 @@ use cargo::core::resolver::{self, ResolveOpts, VersionPreferences};
 use cargo::core::source::{GitReference, QueryKind, SourceId};
 use cargo::core::Resolve;
 use cargo::core::{Dependency, PackageId, Registry, Summary};
-use cargo::util::{CargoResult, Config, Graph, IntoUrl};
+use cargo::util::{CargoResult, Config, Graph, IntoUrl, PartialVersion};
 
 use proptest::collection::{btree_map, vec};
 use proptest::prelude::*;
@@ -183,7 +183,7 @@ pub fn resolve_with_config_raw(
         deps,
         &BTreeMap::new(),
         None::<&String>,
-        None::<&String>,
+        None::<PartialVersion>,
     )
     .unwrap();
     let opts = ResolveOpts::everything();
@@ -584,7 +584,7 @@ pub fn pkg_dep<T: ToPkgId>(name: T, dep: Vec<Dependency>) -> Summary {
         dep,
         &BTreeMap::new(),
         link,
-        None::<&String>,
+        None::<PartialVersion>,
     )
     .unwrap()
 }
@@ -612,7 +612,7 @@ pub fn pkg_loc(name: &str, loc: &str) -> Summary {
         Vec::new(),
         &BTreeMap::new(),
         link,
-        None::<&String>,
+        None::<PartialVersion>,
     )
     .unwrap()
 }
@@ -626,7 +626,7 @@ pub fn remove_dep(sum: &Summary, ind: usize) -> Summary {
         deps,
         &BTreeMap::new(),
         sum.links().map(|a| a.as_str()),
-        None::<&String>,
+        None::<PartialVersion>,
     )
     .unwrap()
 }

src/cargo/core/compiler/compilation.rs

@@ -312,6 +312,7 @@ impl<'cfg> Compilation<'cfg> {
         // crate properties which might require rebuild upon change
         // consider adding the corresponding properties to the hash
         // in BuildContext::target_metadata()
+        let rust_version = pkg.rust_version().as_ref().map(ToString::to_string);
         cmd.env("CARGO_MANIFEST_DIR", pkg.root())
             .env("CARGO_PKG_VERSION_MAJOR", &pkg.version().major.to_string())
             .env("CARGO_PKG_VERSION_MINOR", &pkg.version().minor.to_string())
@@ -342,7 +343,7 @@ impl<'cfg> Compilation<'cfg> {
             .env("CARGO_PKG_AUTHORS", &pkg.authors().join(":"))
             .env(
                 "CARGO_PKG_RUST_VERSION",
-                &pkg.rust_version().unwrap_or(&String::new()),
+                &rust_version.as_deref().unwrap_or_default(),
             )
             .env(
                 "CARGO_PKG_README",

src/cargo/core/manifest.rs

@@ -19,7 +19,7 @@ use crate::core::{Edition, Feature, Features, WorkspaceConfig};
 use crate::util::errors::*;
 use crate::util::interning::InternedString;
 use crate::util::toml::{TomlManifest, TomlProfiles};
-use crate::util::{short_hash, Config, Filesystem};
+use crate::util::{short_hash, Config, Filesystem, PartialVersion};
 
 pub enum EitherManifest {
     Real(Manifest),
@@ -58,7 +58,7 @@ pub struct Manifest {
     original: Rc<TomlManifest>,
     unstable_features: Features,
     edition: Edition,
-    rust_version: Option<String>,
+    rust_version: Option<PartialVersion>,
     im_a_teapot: Option<bool>,
     default_run: Option<String>,
     metabuild: Option<Vec<String>>,
@@ -112,7 +112,7 @@ pub struct ManifestMetadata {
     pub documentation: Option<String>, // URL
     pub badges: BTreeMap<String, BTreeMap<String, String>>,
     pub links: Option<String>,
-    pub rust_version: Option<String>,
+    pub rust_version: Option<PartialVersion>,
 }
 
 #[derive(Clone, Hash, PartialEq, Eq, PartialOrd, Ord)]
@@ -401,7 +401,7 @@ impl Manifest {
         workspace: WorkspaceConfig,
         unstable_features: Features,
         edition: Edition,
-        rust_version: Option<String>,
+        rust_version: Option<PartialVersion>,
         im_a_teapot: Option<bool>,
         default_run: Option<String>,
         original: Rc<TomlManifest>,
@@ -570,8 +570,8 @@ impl Manifest {
         self.edition
     }
 
-    pub fn rust_version(&self) -> Option<&str> {
-        self.rust_version.as_deref()
+    pub fn rust_version(&self) -> Option<PartialVersion> {
+        self.rust_version
     }
 
     pub fn custom_metadata(&self) -> Option<&toml::Value> {

src/cargo/core/package.rs

@@ -31,6 +31,7 @@ use crate::util::network::http::http_handle_and_timeout;
 use crate::util::network::http::HttpTimeout;
 use crate::util::network::retry::{Retry, RetryResult};
 use crate::util::network::sleep::SleepTracker;
+use crate::util::PartialVersion;
 use crate::util::{self, internal, Config, Progress, ProgressStyle};
 
 pub const MANIFEST_PREAMBLE: &str = "\
@@ -103,7 +104,7 @@ pub struct SerializedPackage {
     #[serde(skip_serializing_if = "Option::is_none")]
     metabuild: Option<Vec<String>>,
     default_run: Option<String>,
-    rust_version: Option<String>,
+    rust_version: Option<PartialVersion>,
 }
 
 impl Package {
@@ -177,7 +178,7 @@ impl Package {
         self.targets().iter().any(|target| target.proc_macro())
     }
     /// Gets the package's minimum Rust version.
-    pub fn rust_version(&self) -> Option<&str> {
+    pub fn rust_version(&self) -> Option<PartialVersion> {
         self.manifest().rust_version()
     }
 
@@ -262,7 +263,7 @@ impl Package {
             metabuild: self.manifest().metabuild().cloned(),
             publish: self.publish().as_ref().cloned(),
             default_run: self.manifest().default_run().map(|s| s.to_owned()),
-            rust_version: self.rust_version().map(|s| s.to_owned()),
+            rust_version: self.rust_version(),
         }
     }
 }

src/cargo/core/resolver/version_prefs.rs

@@ -81,6 +81,7 @@ impl VersionPreferences {
 mod test {
     use super::*;
     use crate::core::SourceId;
+    use crate::util::PartialVersion;
     use std::collections::BTreeMap;
 
     fn pkgid(name: &str, version: &str) -> PackageId {
@@ -103,7 +104,7 @@ mod test {
             Vec::new(),
             &features,
             None::<&String>,
-            None::<&String>,
+            None::<PartialVersion>,
         )
         .unwrap()
     }

src/cargo/core/summary.rs

@@ -1,6 +1,7 @@
 use crate::core::{Dependency, PackageId, SourceId};
 use crate::util::interning::InternedString;
 use crate::util::CargoResult;
+use crate::util::PartialVersion;
 use anyhow::bail;
 use semver::Version;
 use std::collections::{BTreeMap, HashMap, HashSet};
@@ -25,7 +26,7 @@ struct Inner {
     features: Rc<FeatureMap>,
     checksum: Option<String>,
     links: Option<InternedString>,
-    rust_version: Option<InternedString>,
+    rust_version: Option<PartialVersion>,
 }
 
 impl Summary {
@@ -34,7 +35,7 @@ impl Summary {
         dependencies: Vec<Dependency>,
         features: &BTreeMap<InternedString, Vec<InternedString>>,
         links: Option<impl Into<InternedString>>,
-        rust_version: Option<impl Into<InternedString>>,
+        rust_version: Option<PartialVersion>,
     ) -> CargoResult<Summary> {
         // ****CAUTION**** If you change anything here that may raise a new
         // error, be sure to coordinate that change with either the index
@@ -56,7 +57,7 @@ impl Summary {
                 features: Rc::new(feature_map),
                 checksum: None,
                 links: links.map(|l| l.into()),
-                rust_version: rust_version.map(|l| l.into()),
+                rust_version,
             }),
         })
     }
@@ -87,7 +88,7 @@ impl Summary {
         self.inner.links
     }
 
-    pub fn rust_version(&self) -> Option<InternedString> {
+    pub fn rust_version(&self) -> Option<PartialVersion> {
         self.inner.rust_version
     }
 

src/cargo/ops/cargo_add/mod.rs

@@ -568,11 +568,7 @@ fn get_latest_dependency(
             })?;
 
             if config.cli_unstable().msrv_policy && honor_rust_version {
-                fn parse_msrv(rust_version: impl AsRef<str>) -> (u64, u64, u64) {
-                    let comp = rust_version
-                        .as_ref()
-                        .parse::<PartialVersion>()
-                        .expect("validated on parsing of manifest");
+                fn parse_msrv(comp: PartialVersion) -> (u64, u64, u64) {
                     (comp.major, comp.minor.unwrap_or(0), comp.patch.unwrap_or(0))
                 }
 
@@ -632,7 +628,7 @@ fn get_latest_dependency(
 
 fn rust_version_incompat_error(
     dep: &str,
-    rust_version: &str,
+    rust_version: PartialVersion,
     lowest_rust_version: Option<&Summary>,
 ) -> anyhow::Error {
     let mut error_msg = format!(

src/cargo/ops/cargo_compile/mod.rs

@@ -492,7 +492,7 @@ pub fn create_bcx<'a, 'cfg>(
                 None => continue,
             };
 
-            let req = semver::VersionReq::parse(version).unwrap();
+            let req = version.caret_req();
             if req.matches(&untagged_version) {
                 continue;
             }

src/cargo/ops/registry/publish.rs

@@ -372,6 +372,7 @@ fn transmit(
         ref links,
         ref rust_version,
     } = *manifest.metadata();
+    let rust_version = rust_version.as_ref().map(ToString::to_string);
     let readme_content = readme
         .as_ref()
         .map(|readme| {
@@ -424,7 +425,7 @@ fn transmit(
                 license_file: license_file.clone(),
                 badges: badges.clone(),
                 links: links.clone(),
-                rust_version: rust_version.clone(),
+                rust_version: rust_version,
             },
             tarball,
         )

src/cargo/sources/registry/index.rs

@@ -91,7 +91,9 @@ use crate::core::{PackageId, SourceId, Summary};
 use crate::sources::registry::{LoadResponse, RegistryData};
 use crate::util::interning::InternedString;
 use crate::util::IntoUrl;
-use crate::util::{internal, CargoResult, Config, Filesystem, OptVersionReq, ToSemver};
+use crate::util::{
+    internal, CargoResult, Config, Filesystem, OptVersionReq, PartialVersion, ToSemver,
+};
 use anyhow::bail;
 use cargo_util::{paths, registry::make_dep_path};
 use semver::Version;
@@ -305,7 +307,7 @@ pub struct IndexPackage<'a> {
     ///
     /// Added in 2023 (see <https://github.com/rust-lang/crates.io/pull/6267>),
     /// can be `None` if published before then or if not set in the manifest.
-    rust_version: Option<InternedString>,
+    rust_version: Option<PartialVersion>,
     /// The schema version for this entry.
     ///
     /// If this is None, it defaults to version `1`. Entries with unknown

src/cargo/util/semver_ext.rs

@@ -108,13 +108,27 @@ impl From<VersionReq> for OptVersionReq {
     }
 }
 
-#[derive(PartialEq, Eq, Hash, Clone, Debug)]
+#[derive(PartialEq, Eq, Hash, Copy, Clone, Debug)]
 pub struct PartialVersion {
     pub major: u64,
     pub minor: Option<u64>,
     pub patch: Option<u64>,
 }
 
+impl PartialVersion {
+    pub fn caret_req(&self) -> VersionReq {
+        VersionReq {
+            comparators: vec![Comparator {
+                op: semver::Op::Caret,
+                major: self.major,
+                minor: self.minor,
+                patch: self.patch,
+                pre: Default::default(),
+            }],
+        }
+    }
+}
+
 impl std::str::FromStr for PartialVersion {
     type Err = anyhow::Error;
 
@@ -125,7 +139,7 @@ impl std::str::FromStr for PartialVersion {
         let version_req = match semver::VersionReq::parse(value) {
             // Exclude semver operators like `^` and pre-release identifiers
             Ok(req) if value.chars().all(|c| c.is_ascii_digit() || c == '.') => req,
-            _ => anyhow::bail!("`rust-version` must be a value like \"1.32\""),
+            _ => anyhow::bail!("expected a version like \"1.32\""),
         };
         assert_eq!(
             version_req.comparators.len(),
@@ -162,6 +176,41 @@ impl Display for PartialVersion {
     }
 }
 
+impl serde::Serialize for PartialVersion {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        serializer.collect_str(self)
+    }
+}
+
+impl<'de> serde::Deserialize<'de> for PartialVersion {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        struct VersionVisitor;
+
+        impl<'de> serde::de::Visitor<'de> for VersionVisitor {
+            type Value = PartialVersion;
+
+            fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
+                formatter.write_str("semver version")
+            }
+
+            fn visit_str<E>(self, string: &str) -> Result<Self::Value, E>
+            where
+                E: serde::de::Error,
+            {
+                string.parse().map_err(serde::de::Error::custom)
+            }
+        }
+
+        deserializer.deserialize_str(VersionVisitor)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;