Auto merge of rust-lang#13771 - epage:rust-version, r=weihanglo

fix(msrv): Error, rather than panic, on rust-version 'x' ### What does this PR try to resolve? Fixes rust-lang#13768 ### How should we test and review this PR? ### Additional information
weihanglo · Apr 18, 2024 · 39b8f17 · 39b8f17
2 parents 5afc53a + 6d8d3b6
commit 39b8f17
Show file tree

Hide file tree

Showing 5 changed files with 120 additions and 110 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/crates/cargo-util-schemas/Cargo.toml b/crates/cargo-util-schemas/Cargo.toml
@@ -20,3 +20,6 @@ url.workspace = true
 
 [lints]
 workspace = true
+
+[dev-dependencies]
+snapbox.workspace = true
diff --git a/crates/cargo-util-schemas/src/core/partial_version.rs b/crates/cargo-util-schemas/src/core/partial_version.rs
@@ -83,9 +83,6 @@ impl std::str::FromStr for PartialVersion {
     type Err = PartialVersionError;
 
     fn from_str(value: &str) -> Result<Self, Self::Err> {
-        if is_req(value) {
-            return Err(ErrorKind::VersionReq.into());
-        }
         match semver::Version::parse(value) {
             Ok(ver) => Ok(ver.into()),
             Err(_) => {
@@ -96,9 +93,16 @@ impl std::str::FromStr for PartialVersion {
                     Err(_) if value.contains('+') => return Err(ErrorKind::BuildMetadata.into()),
                     Err(_) => return Err(ErrorKind::Unexpected.into()),
                 };
-                assert_eq!(version_req.comparators.len(), 1, "guaranteed by is_req");
+                if version_req.comparators.len() != 1 {
+                    return Err(ErrorKind::VersionReq.into());
+                }
                 let comp = version_req.comparators.pop().unwrap();
-                assert_eq!(comp.op, semver::Op::Caret, "guaranteed by is_req");
+                if comp.op != semver::Op::Caret {
+                    return Err(ErrorKind::VersionReq.into());
+                } else if value.starts_with('^') {
+                    // Can't distinguish between `^` present or not
+                    return Err(ErrorKind::VersionReq.into());
+                }
                 let pre = if comp.pre.is_empty() {
                     None
                 } else {
@@ -179,9 +183,65 @@ enum ErrorKind {
     Unexpected,
 }
 
-fn is_req(value: &str) -> bool {
-    let Some(first) = value.chars().next() else {
-        return false;
-    };
-    "<>=^~".contains(first) || value.contains('*') || value.contains(',')
+#[cfg(test)]
+mod test {
+    use super::*;
+    use snapbox::str;
+
+    #[test]
+    fn parse_success() {
+        let cases = &[
+            // Valid pre-release
+            ("1.43.0-beta.1", str!["1.43.0-beta.1"]),
+            // Valid pre-release with wildcard
+            ("1.43.0-beta.1.x", str!["1.43.0-beta.1.x"]),
+        ];
+        for (input, expected) in cases {
+            let actual: Result<PartialVersion, _> = input.parse();
+            let actual = match actual {
+                Ok(result) => result.to_string(),
+                Err(err) => format!("didn't pass: {err}"),
+            };
+            snapbox::assert_eq(expected.clone(), actual);
+        }
+    }
+
+    #[test]
+    fn parse_errors() {
+        let cases = &[
+            // Disallow caret
+            (
+                "^1.43",
+                str![[r#"unexpected version requirement, expected a version like "1.32""#]],
+            ),
+            // Bad pre-release
+            (
+                "1.43-beta.1",
+                str![[r#"unexpected prerelease field, expected a version like "1.32""#]],
+            ),
+            // Weird wildcard
+            (
+                "x",
+                str![[r#"unexpected version requirement, expected a version like "1.32""#]],
+            ),
+            (
+                "1.x",
+                str![[r#"unexpected version requirement, expected a version like "1.32""#]],
+            ),
+            (
+                "1.1.x",
+                str![[r#"unexpected version requirement, expected a version like "1.32""#]],
+            ),
+            // Non-sense
+            ("foodaddle", str![[r#"expected a version like "1.32""#]]),
+        ];
+        for (input, expected) in cases {
+            let actual: Result<PartialVersion, _> = input.parse();
+            let actual = match actual {
+                Ok(result) => format!("didn't fail: {result:?}"),
+                Err(err) => err.to_string(),
+            };
+            snapbox::assert_eq(expected.clone(), actual);
+        }
+    }
 }
diff --git a/crates/cargo-util-schemas/src/manifest/rust_version.rs b/crates/cargo-util-schemas/src/manifest/rust_version.rs
@@ -106,6 +106,7 @@ enum RustVersionErrorKind {
 #[cfg(test)]
 mod test {
     use super::*;
+    use snapbox::str;
 
     #[test]
     fn is_compatible_with_rustc() {
@@ -170,4 +171,48 @@ mod test {
         }
         assert!(passed);
     }
+
+    #[test]
+    fn parse_errors() {
+        let cases = &[
+            // Disallow caret
+            (
+                "^1.43",
+                str![[r#"unexpected version requirement, expected a version like "1.32""#]],
+            ),
+            // Valid pre-release
+            (
+                "1.43.0-beta.1",
+                str![[r#"unexpected prerelease field, expected a version like "1.32""#]],
+            ),
+            // Bad pre-release
+            (
+                "1.43-beta.1",
+                str![[r#"unexpected prerelease field, expected a version like "1.32""#]],
+            ),
+            // Weird wildcard
+            (
+                "x",
+                str![[r#"unexpected version requirement, expected a version like "1.32""#]],
+            ),
+            (
+                "1.x",
+                str![[r#"unexpected version requirement, expected a version like "1.32""#]],
+            ),
+            (
+                "1.1.x",
+                str![[r#"unexpected version requirement, expected a version like "1.32""#]],
+            ),
+            // Non-sense
+            ("foodaddle", str![[r#"expected a version like "1.32""#]]),
+        ];
+        for (input, expected) in cases {
+            let actual: Result<RustVersion, _> = input.parse();
+            let actual = match actual {
+                Ok(result) => format!("didn't fail: {result:?}"),
+                Err(err) => err.to_string(),
+            };
+            snapbox::assert_eq(expected.clone(), actual);
+        }
+    }
 }
diff --git a/tests/testsuite/rust_version.rs b/tests/testsuite/rust_version.rs
@@ -26,7 +26,7 @@ fn rust_version_satisfied() {
 }
 
 #[cargo_test]
-fn rust_version_bad_caret() {
+fn rust_version_error() {
     project()
         .file(
             "Cargo.toml",
@@ -58,105 +58,6 @@ fn rust_version_bad_caret() {
         .run();
 }
 
-#[cargo_test]
-fn rust_version_good_pre_release() {
-    project()
-        .file(
-            "Cargo.toml",
-            r#"
-            [package]
-            name = "foo"
-            version = "0.0.1"
-            edition = "2015"
-            authors = []
-            rust-version = "1.43.0-beta.1"
-            [[bin]]
-            name = "foo"
-        "#,
-        )
-        .file("src/main.rs", "fn main() {}")
-        .build()
-        .cargo("check")
-        .with_status(101)
-        .with_stderr(
-            "\
-[ERROR] unexpected prerelease field, expected a version like \"1.32\"
- --> Cargo.toml:7:28
-  |
-7 |             rust-version = \"1.43.0-beta.1\"
-  |                            ^^^^^^^^^^^^^^^
-  |
-",
-        )
-        .run();
-}
-
-#[cargo_test]
-fn rust_version_bad_pre_release() {
-    project()
-        .file(
-            "Cargo.toml",
-            r#"
-            [package]
-            name = "foo"
-            version = "0.0.1"
-            edition = "2015"
-            authors = []
-            rust-version = "1.43-beta.1"
-            [[bin]]
-            name = "foo"
-        "#,
-        )
-        .file("src/main.rs", "fn main() {}")
-        .build()
-        .cargo("check")
-        .with_status(101)
-        .with_stderr(
-            "\
-[ERROR] unexpected prerelease field, expected a version like \"1.32\"
- --> Cargo.toml:7:28
-  |
-7 |             rust-version = \"1.43-beta.1\"
-  |                            ^^^^^^^^^^^^^
-  |
-",
-        )
-        .run();
-}
-
-#[cargo_test]
-fn rust_version_bad_nonsense() {
-    project()
-        .file(
-            "Cargo.toml",
-            r#"
-            [package]
-            name = "foo"
-            version = "0.0.1"
-            edition = "2015"
-            authors = []
-            rust-version = "foodaddle"
-            [[bin]]
-            name = "foo"
-        "#,
-        )
-        .file("src/main.rs", "fn main() {}")
-        .build()
-        .cargo("check")
-        .with_status(101)
-        .with_stderr(
-            "\
-[ERROR] expected a version like \"1.32\"
- --> Cargo.toml:7:28
-  |
-7 |             rust-version = \"foodaddle\"
-  |                            ^^^^^^^^^^^
-  |
-",
-        )
-        .run();
-}
-
 #[cargo_test]
 fn rust_version_older_than_edition() {
     project()