Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: make strict parsing even stricter #989

Merged
merged 4 commits into from
Jan 1, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions crates/rattler_conda_types/src/match_spec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,12 @@ use matcher::StringMatcher;
/// use std::sync::Arc;
///
/// let channel_config = ChannelConfig::default_with_root_dir(std::env::current_dir().unwrap());
/// let spec = MatchSpec::from_str("foo 1.0 py27_0", Strict).unwrap();
/// let spec = MatchSpec::from_str("foo 1.0.* py27_0", Strict).unwrap();
/// assert_eq!(spec.name, Some(PackageName::new_unchecked("foo")));
/// assert_eq!(spec.version, Some(VersionSpec::from_str("1.0", Strict).unwrap()));
/// assert_eq!(spec.version, Some(VersionSpec::from_str("1.0.*", Strict).unwrap()));
/// assert_eq!(spec.build, Some(StringMatcher::from_str("py27_0").unwrap()));
///
/// let spec = MatchSpec::from_str("foo 1.0 py27_0", Strict).unwrap();
/// let spec = MatchSpec::from_str("foo ==1.0 py27_0", Strict).unwrap();
/// assert_eq!(spec.name, Some(PackageName::new_unchecked("foo")));
/// assert_eq!(spec.version, Some(VersionSpec::from_str("==1.0", Strict).unwrap()));
/// assert_eq!(spec.build, Some(StringMatcher::from_str("py27_0").unwrap()));
Expand Down Expand Up @@ -702,12 +702,12 @@ mod tests {

#[test]
fn test_serialize_matchspec() {
let specs = ["mamba 1.0 py37_0",
"conda-forge::pytest[version=1.0, sha256=aaac4bc9c6916ecc0e33137431645b029ade22190c7144eead61446dcbcc6f97, md5=dede6252c964db3f3e41c7d30d07f6bf]",
let specs = ["mamba 1.0.* py37_0",
"conda-forge::pytest[version='==1.0', sha256=aaac4bc9c6916ecc0e33137431645b029ade22190c7144eead61446dcbcc6f97, md5=dede6252c964db3f3e41c7d30d07f6bf]",
"conda-forge/linux-64::pytest",
"conda-forge/linux-64::pytest[version=1.0]",
"conda-forge/linux-64::pytest[version=1.0, build=py37_0]",
"conda-forge/linux-64::pytest 1.2.3"];
"conda-forge/linux-64::pytest[version=1.0.*]",
"conda-forge/linux-64::pytest[version=1.0.*, build=py37_0]",
"conda-forge/linux-64::pytest ==1.2.3"];

assert_snapshot!(specs
.into_iter()
Expand Down
7 changes: 4 additions & 3 deletions crates/rattler_conda_types/src/match_spec/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -804,11 +804,12 @@ mod tests {
fn test_nameless_match_spec() {
insta::assert_yaml_snapshot!([
NamelessMatchSpec::from_str("3.8.* *_cpython", Strict).unwrap(),
NamelessMatchSpec::from_str("1.0 py27_0[fn=\"bla\"]", Strict).unwrap(),
NamelessMatchSpec::from_str("==1.0 py27_0[fn=\"bla\"]", Strict).unwrap(),
NamelessMatchSpec::from_str("=1.0 py27_0", Strict).unwrap(),
NamelessMatchSpec::from_str("*cpu*", Strict).unwrap(),
NamelessMatchSpec::from_str("conda-forge::foobar", Strict).unwrap(),
NamelessMatchSpec::from_str("foobar[channel=conda-forge]", Strict).unwrap(),
// the next two tests are a bit weird, the version is `foobar` and the channel is `conda-forge`
NamelessMatchSpec::from_str("conda-forge::==foobar", Strict).unwrap(),
NamelessMatchSpec::from_str("==foobar[channel=conda-forge]", Strict).unwrap(),
NamelessMatchSpec::from_str("* [build=foo]", Strict).unwrap(),
NamelessMatchSpec::from_str(">=1.2[build=foo]", Strict).unwrap(),
NamelessMatchSpec::from_str("[version='>=1.2', build=foo]", Strict).unwrap(),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
source: crates/rattler_conda_types/src/match_spec/parse.rs
expression: evaluated
snapshot_kind: text
---
blas *.* mkl:
name: blas
Expand Down Expand Up @@ -54,32 +55,13 @@ python=3.9:
python=*:
error: "invalid version constraint: '*' is incompatible with '=' operator'"
"https://software.repos.intel.com/python/conda::python[version=3.9]":
name: python
version: "==3.9"
channel:
base_url: "https://software.repos.intel.com/python/conda"
name: python/conda
error: "invalid version constraint: missing range specifier for '3.9'. Did you mean '==3.9' or '3.9.*'?"
"https://c.com/p/conda/linux-64::python[version=3.9]":
name: python
version: "==3.9"
channel:
base_url: "https://c.com/p/conda"
name: p/conda
subdir: linux-64
error: "invalid version constraint: missing range specifier for '3.9'. Did you mean '==3.9' or '3.9.*'?"
"https://c.com/p/conda::python[version=3.9, subdir=linux-64]":
name: python
version: "==3.9"
channel:
base_url: "https://c.com/p/conda"
name: p/conda
subdir: linux-64
error: "invalid version constraint: missing range specifier for '3.9'. Did you mean '==3.9' or '3.9.*'?"
"conda-forge/linux-32::python[version=3.9, subdir=linux-64]":
name: python
version: "==3.9"
channel:
base_url: "https://conda.anaconda.org/conda-forge"
name: conda-forge
subdir: linux-64
error: "invalid version constraint: missing range specifier for '3.9'. Did you mean '==3.9' or '3.9.*'?"
"conda-forge/linux-32::python ==3.9[subdir=linux-64, build_number=\"0\"]":
name: python
version: "==3.9"
Expand Down Expand Up @@ -119,11 +101,7 @@ rust ~=1.2.3:
base_url: "file://<CRATE>/relative/channel"
name: "./relative/channel"
"python[channel=https://conda.anaconda.org/python/conda,version=3.9]":
name: python
version: "==3.9"
channel:
base_url: "https://conda.anaconda.org/python/conda"
name: python/conda
error: "invalid version constraint: missing range specifier for '3.9'. Did you mean '==3.9' or '3.9.*'?"
"channel/win-64::foobar[channel=conda-forge, subdir=linux-64]":
name: foobar
channel:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
---
source: crates/rattler_conda_types/src/match_spec/parse.rs
expression: evaluated
snapshot_kind: text
---
2.7|>=3.6:
version: "==2.7|>=3.6"
error: "invalid version constraint: missing range specifier for '2.7'. Did you mean '==2.7' or '2.7.*'?"
"https://conda.anaconda.org/conda-forge/linux-64/_libgcc_mutex-0.1-conda_forge.tar.bz2":
url: "https://conda.anaconda.org/conda-forge/linux-64/_libgcc_mutex-0.1-conda_forge.tar.bz2"
~=1.2.3:
Expand Down Expand Up @@ -40,14 +41,13 @@ expression: evaluated
"==2.7.*.*|>=3.6":
error: "invalid version constraint: regex constraints are not supported"
"3.9":
version: "==3.9"
error: "invalid version constraint: missing range specifier for '3.9'. Did you mean '==3.9' or '3.9.*'?"
"*":
version: "*"
"[version=3.9]":
version: "==3.9"
error: "invalid version constraint: missing range specifier for '3.9'. Did you mean '==3.9' or '3.9.*'?"
"[version=3.9, subdir=linux-64]":
version: "==3.9"
subdir: linux-64
error: "invalid version constraint: missing range specifier for '3.9'. Did you mean '==3.9' or '3.9.*'?"
"==3.9[subdir=linux-64, build_number=\"0\"]":
version: "==3.9"
build_number:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
source: crates/rattler_conda_types/src/match_spec/mod.rs
expression: "specs.into_iter().map(|s|\nMatchSpec::from_str(s,\nStrict).unwrap()).map(|s| s.to_string()).collect::<Vec<String>>().join(\"\\n\")"
---
mamba ==1.0 py37_0
mamba 1.0.* py37_0
conda-forge::pytest ==1.0[md5="dede6252c964db3f3e41c7d30d07f6bf", sha256="aaac4bc9c6916ecc0e33137431645b029ade22190c7144eead61446dcbcc6f97"]
conda-forge/linux-64::pytest
conda-forge/linux-64::pytest ==1.0
conda-forge/linux-64::pytest ==1.0 py37_0
conda-forge/linux-64::pytest 1.0.*
conda-forge/linux-64::pytest 1.0.* py37_0
conda-forge/linux-64::pytest ==1.2.3
2 changes: 1 addition & 1 deletion crates/rattler_conda_types/src/version_spec/constraint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ mod test {
#[rstest]
fn test_exact(#[values(Lenient, Strict)] strictness: ParseStrictness) {
assert_eq!(
Constraint::from_str("1.2.3", strictness),
Constraint::from_str("==1.2.3", strictness),
Ok(Constraint::Exact(
EqualityOperator::Equals,
Version::from_str("1.2.3").unwrap(),
Expand Down
19 changes: 14 additions & 5 deletions crates/rattler_conda_types/src/version_spec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,15 +350,15 @@ mod tests {
use crate::{
version_spec::{
parse::ParseConstraintError, EqualityOperator, LogicalOperator, ParseVersionSpecError,
RangeOperator,
RangeOperator, StrictRangeOperator,
},
ParseStrictness, Version, VersionSpec,
ParseStrictness, StrictVersion, Version, VersionSpec,
};

#[test]
fn test_simple() {
assert_eq!(
VersionSpec::from_str("1.2.3", ParseStrictness::Strict),
VersionSpec::from_str("==1.2.3", ParseStrictness::Strict),
Ok(VersionSpec::Exact(
EqualityOperator::Equals,
Version::from_str("1.2.3").unwrap(),
Expand All @@ -371,6 +371,13 @@ mod tests {
Version::from_str("1.2.3").unwrap(),
))
);
assert_eq!(
VersionSpec::from_str("=1.2.3", ParseStrictness::Strict),
Ok(VersionSpec::StrictRange(
StrictRangeOperator::StartsWith,
StrictVersion::from_str("1.2.3").unwrap(),
))
);
}

#[test]
Expand Down Expand Up @@ -422,21 +429,23 @@ mod tests {
let vs1 = VersionSpec::from_str(">=1.2.3,<2.0.0", ParseStrictness::Strict).unwrap();
assert!(!vs1.matches(&v1));

let vs2 = VersionSpec::from_str("1.2", ParseStrictness::Strict).unwrap();
let vs2 = VersionSpec::from_str("==1.2.0", ParseStrictness::Strict).unwrap();
assert!(vs2.matches(&v1));

let v2 = Version::from_str("1.2.3").unwrap();
assert!(vs1.matches(&v2));
assert!(!vs2.matches(&v2));

let v3 = Version::from_str("1!1.2.3").unwrap();
println!("{v3:?}");

assert!(!vs1.matches(&v3));
assert!(!vs2.matches(&v3));

let vs3 = VersionSpec::from_str(">=1!1.2,<1!2", ParseStrictness::Strict).unwrap();
assert!(vs3.matches(&v3));

let vs4 = VersionSpec::from_str("1!1.2.*", ParseStrictness::Strict).unwrap();
assert!(vs4.matches(&v3));
}

#[test]
Expand Down
25 changes: 21 additions & 4 deletions crates/rattler_conda_types/src/version_spec/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ pub enum ParseConstraintError {
InvalidOperator(String),
#[error(transparent)]
InvalidVersion(#[from] ParseVersionError),
#[error("missing range specifier for '{0}'. Did you mean '=={0}' or '{0}.*'?")]
AmbiguousVersion(String),
/// Expected a version
#[error("expected a version")]
ExpectedVersion,
Expand Down Expand Up @@ -208,8 +210,15 @@ fn logical_constraint_parser(
let op = match (version_rest, op, strictness) {
// The version was successfully parsed
("", Some(op), _) => op,
("", None, _) => VersionOperators::Exact(EqualityOperator::Equals),

("", None, _) => {
if strictness == Strict {
return Err(nom::Err::Failure(ParseConstraintError::AmbiguousVersion(
version_str.to_owned(),
)));
} else {
VersionOperators::Exact(EqualityOperator::Equals)
}
}
// The version ends in a wildcard pattern
(
"*" | ".*",
Expand Down Expand Up @@ -316,6 +325,7 @@ mod test {
use std::str::FromStr;

use assert_matches::assert_matches;
use insta::assert_snapshot;
use rstest::rstest;

use super::*;
Expand Down Expand Up @@ -401,7 +411,7 @@ mod test {
#[rstest]
fn parse_logical_constraint(#[values(Lenient, Strict)] strictness: ParseStrictness) {
assert_eq!(
logical_constraint_parser(strictness)("3.1"),
logical_constraint_parser(strictness)("==3.1"),
Ok((
"",
Constraint::Exact(EqualityOperator::Equals, Version::from_str("3.1").unwrap())
Expand Down Expand Up @@ -510,7 +520,14 @@ mod test {

#[test]
fn pixi_issue_278() {
assert!(VersionSpec::from_str("1.8.1+g6b29558", Strict).is_ok());
assert!(VersionSpec::from_str("=1.8.1+g6b29558", Strict).is_ok());
}

#[test]
fn test_exact_strict() {
assert!(VersionSpec::from_str("==3.1", Strict).is_ok());
assert!(VersionSpec::from_str("3.1", Strict).is_err());
assert_snapshot!(VersionSpec::from_str("1.2.3", Strict).unwrap_err());
}

#[test]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
source: crates/rattler_conda_types/src/version_spec/parse.rs
expression: "VersionSpec::from_str(\"1.2.3\", Strict).unwrap_err()"
snapshot_kind: text
---
invalid version constraint: missing range specifier for '1.2.3'. Did you mean '==1.2.3' or '1.2.3.*'?
7 changes: 4 additions & 3 deletions crates/rattler_repodata_gateway/src/gateway/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,8 @@ mod test {
.query(
vec![index],
vec![Platform::Linux64],
vec![MatchSpec::from_str("openssl 3.3.1 h2466b09_1", Strict).unwrap()].into_iter(),
vec![MatchSpec::from_str("openssl ==3.3.1 h2466b09_1", Strict).unwrap()]
.into_iter(),
)
.recursive(true)
.await
Expand Down Expand Up @@ -454,7 +455,7 @@ mod test {
vec![index.clone()],
vec![Platform::Linux64],
vec![
MatchSpec::from_str("mamba 0.9.2 py39h951de11_0", Strict).unwrap(),
MatchSpec::from_str("mamba ==0.9.2 py39h951de11_0", Strict).unwrap(),
MatchSpec::from_str(openssl_url, Strict).unwrap(),
]
.into_iter(),
Expand Down Expand Up @@ -491,7 +492,7 @@ mod test {
.query(
vec![index.clone()],
vec![Platform::Linux64],
vec![MatchSpec::from_str("mamba 0.9.2 py39h951de11_0", Strict).unwrap()]
vec![MatchSpec::from_str("mamba ==0.9.2 py39h951de11_0", Strict).unwrap()]
.into_iter(),
)
.recursive(true)
Expand Down
Loading
Loading