From af9288f1b61ea3067770b031fced4d75a79e486c Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 17 Apr 2024 13:08:16 -0500 Subject: [PATCH 1/5] test(msrv): Show current parse behavior with X --- tests/testsuite/rust_version.rs | 102 ++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) diff --git a/tests/testsuite/rust_version.rs b/tests/testsuite/rust_version.rs index 01f49e32ff8..3df19e82406 100644 --- a/tests/testsuite/rust_version.rs +++ b/tests/testsuite/rust_version.rs @@ -124,6 +124,108 @@ fn rust_version_bad_pre_release() { .run(); } +#[cargo_test] +#[should_panic] +fn rust_version_x_wildcard() { + project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + edition = "2015" + authors = [] + rust-version = "x" + [[bin]] + name = "foo" + "#, + ) + .file("src/main.rs", "fn main() {}") + .build() + .cargo("check") + .with_status(101) + .with_stderr( + "\ +[ERROR] unexpected version requirement, expected a version like \"1.32\" + --> Cargo.toml:7:28 + | +7 | rust-version = \"^1.43\" + | ^^^^^^^ + | +", + ) + .run(); +} + +#[cargo_test] +#[should_panic] +fn rust_version_minor_x_wildcard() { + project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + edition = "2015" + authors = [] + rust-version = "1.x" + [[bin]] + name = "foo" + "#, + ) + .file("src/main.rs", "fn main() {}") + .build() + .cargo("check") + .with_status(101) + .with_stderr( + "\ +[ERROR] unexpected version requirement, expected a version like \"1.32\" + --> Cargo.toml:7:28 + | +7 | rust-version = \"1.x\" + | ^^^^^ + | +", + ) + .run(); +} + +#[cargo_test] +#[should_panic] +fn rust_version_patch_x_wildcard() { + project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + edition = "2015" + authors = [] + rust-version = "1.30.x" + [[bin]] + name = "foo" + "#, + ) + .file("src/main.rs", "fn main() {}") + .build() + .cargo("check") + .with_status(101) + .with_stderr( + "\ +[ERROR] unexpected version requirement, expected a version like \"1.32\" + --> Cargo.toml:7:28 + | +7 | rust-version = \"1.30.x\" + | ^^^^^^^^ + | +", + ) + .run(); +} + #[cargo_test] fn rust_version_bad_nonsense() { project() From 675d67d093c8daa33ae24aa048151b8562c7ff67 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 17 Apr 2024 13:15:09 -0500 Subject: [PATCH 2/5] fix(msrv): Error, rather than panic, on rust-version 'x' Fixes #13768 --- crates/cargo-util-schemas/src/core/partial_version.rs | 6 +++++- tests/testsuite/rust_version.rs | 7 ++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/cargo-util-schemas/src/core/partial_version.rs b/crates/cargo-util-schemas/src/core/partial_version.rs index b9c1db82ed1..63db0f1258e 100644 --- a/crates/cargo-util-schemas/src/core/partial_version.rs +++ b/crates/cargo-util-schemas/src/core/partial_version.rs @@ -183,5 +183,9 @@ fn is_req(value: &str) -> bool { let Some(first) = value.chars().next() else { return false; }; - "<>=^~".contains(first) || value.contains('*') || value.contains(',') + "<>=^~".contains(first) + || value.contains('*') + || value.contains(',') + || value.contains('x') + || value.contains('X') } diff --git a/tests/testsuite/rust_version.rs b/tests/testsuite/rust_version.rs index 3df19e82406..6dfeb202b01 100644 --- a/tests/testsuite/rust_version.rs +++ b/tests/testsuite/rust_version.rs @@ -125,7 +125,6 @@ fn rust_version_bad_pre_release() { } #[cargo_test] -#[should_panic] fn rust_version_x_wildcard() { project() .file( @@ -150,8 +149,8 @@ fn rust_version_x_wildcard() { [ERROR] unexpected version requirement, expected a version like \"1.32\" --> Cargo.toml:7:28 | -7 | rust-version = \"^1.43\" - | ^^^^^^^ +7 | rust-version = \"x\" + | ^^^ | ", ) @@ -159,7 +158,6 @@ fn rust_version_x_wildcard() { } #[cargo_test] -#[should_panic] fn rust_version_minor_x_wildcard() { project() .file( @@ -193,7 +191,6 @@ fn rust_version_minor_x_wildcard() { } #[cargo_test] -#[should_panic] fn rust_version_patch_x_wildcard() { project() .file( From 3a2cc8278992f59247cd589a33e78e831de8e362 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 17 Apr 2024 16:34:26 -0500 Subject: [PATCH 3/5] test(msrv): Migrate most parse tests to unit tests --- Cargo.lock | 1 + crates/cargo-util-schemas/Cargo.toml | 3 + .../src/manifest/rust_version.rs | 45 ++++ tests/testsuite/rust_version.rs | 200 +----------------- 4 files changed, 50 insertions(+), 199 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1511fbccee7..497222682f4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -472,6 +472,7 @@ dependencies = [ "serde", "serde-untagged", "serde-value", + "snapbox", "thiserror", "toml", "unicode-xid", diff --git a/crates/cargo-util-schemas/Cargo.toml b/crates/cargo-util-schemas/Cargo.toml index 0166672c0d0..1b72fd30bf1 100644 --- 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/manifest/rust_version.rs b/crates/cargo-util-schemas/src/manifest/rust_version.rs index 03ba94b3e71..5c40097737f 100644 --- 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 = 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 index 6dfeb202b01..1706bf4b485 100644 --- 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,204 +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_x_wildcard() { - project() - .file( - "Cargo.toml", - r#" - [package] - name = "foo" - version = "0.0.1" - edition = "2015" - authors = [] - rust-version = "x" - [[bin]] - name = "foo" - "#, - ) - .file("src/main.rs", "fn main() {}") - .build() - .cargo("check") - .with_status(101) - .with_stderr( - "\ -[ERROR] unexpected version requirement, expected a version like \"1.32\" - --> Cargo.toml:7:28 - | -7 | rust-version = \"x\" - | ^^^ - | -", - ) - .run(); -} - -#[cargo_test] -fn rust_version_minor_x_wildcard() { - project() - .file( - "Cargo.toml", - r#" - [package] - name = "foo" - version = "0.0.1" - edition = "2015" - authors = [] - rust-version = "1.x" - [[bin]] - name = "foo" - "#, - ) - .file("src/main.rs", "fn main() {}") - .build() - .cargo("check") - .with_status(101) - .with_stderr( - "\ -[ERROR] unexpected version requirement, expected a version like \"1.32\" - --> Cargo.toml:7:28 - | -7 | rust-version = \"1.x\" - | ^^^^^ - | -", - ) - .run(); -} - -#[cargo_test] -fn rust_version_patch_x_wildcard() { - project() - .file( - "Cargo.toml", - r#" - [package] - name = "foo" - version = "0.0.1" - edition = "2015" - authors = [] - rust-version = "1.30.x" - [[bin]] - name = "foo" - "#, - ) - .file("src/main.rs", "fn main() {}") - .build() - .cargo("check") - .with_status(101) - .with_stderr( - "\ -[ERROR] unexpected version requirement, expected a version like \"1.32\" - --> Cargo.toml:7:28 - | -7 | rust-version = \"1.30.x\" - | ^^^^^^^^ - | -", - ) - .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() From 6f22e9dbee2c86c0e99e91df306bc00d99e29892 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 17 Apr 2024 16:38:26 -0500 Subject: [PATCH 4/5] test(schemas): Add PartialVersion unit tests --- .../src/core/partial_version.rs | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/crates/cargo-util-schemas/src/core/partial_version.rs b/crates/cargo-util-schemas/src/core/partial_version.rs index 63db0f1258e..668a80b2dd1 100644 --- a/crates/cargo-util-schemas/src/core/partial_version.rs +++ b/crates/cargo-util-schemas/src/core/partial_version.rs @@ -189,3 +189,64 @@ fn is_req(value: &str) -> bool { || value.contains('x') || value.contains('X') } + +#[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"]), + ]; + for (input, expected) in cases { + let actual: Result = 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 = input.parse(); + let actual = match actual { + Ok(result) => format!("didn't fail: {result:?}"), + Err(err) => err.to_string(), + }; + snapbox::assert_eq(expected.clone(), actual); + } + } +} From 6d8d3b6420d00f42a6a38841c743a5d577422f71 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 17 Apr 2024 16:49:22 -0500 Subject: [PATCH 5/5] fix(schemas): Allow parsing pre-release with X --- .../src/core/partial_version.rs | 27 ++++++++----------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/crates/cargo-util-schemas/src/core/partial_version.rs b/crates/cargo-util-schemas/src/core/partial_version.rs index 668a80b2dd1..5057d6046e1 100644 --- 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 { - 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,17 +183,6 @@ enum ErrorKind { Unexpected, } -fn is_req(value: &str) -> bool { - let Some(first) = value.chars().next() else { - return false; - }; - "<>=^~".contains(first) - || value.contains('*') - || value.contains(',') - || value.contains('x') - || value.contains('X') -} - #[cfg(test)] mod test { use super::*; @@ -200,6 +193,8 @@ mod test { 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 = input.parse();