From 389f444163bf2a2b9f2702d017940e3804becc29 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 18 Mar 2021 08:51:22 +0100 Subject: Rewrite to use constructor Rewrite this function to use the constructor or PackageVersionConstraint instead of getting the parser and using it, because this shouldn't be allowed anyways. Signed-off-by: Matthias Beyer --- src/package/dependency/mod.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/package/dependency/mod.rs b/src/package/dependency/mod.rs index a4f6367..f2fd80f 100644 --- a/src/package/dependency/mod.rs +++ b/src/package/dependency/mod.rs @@ -52,15 +52,16 @@ pub(in crate::package::dependency) fn parse_package_dependency_string_into_name_ let name = caps .name("name") + .map(|m| String::from(m.as_str())) .ok_or_else(|| anyhow!("Could not parse name: '{}'", s))?; let vers = caps .name("version") + .map(|m| String::from(m.as_str())) .ok_or_else(|| anyhow!("Could not parse version: '{}'", s))?; - let v = PackageVersionConstraint::parser().parse(vers.as_str().as_bytes())?; - - Ok((PackageName::from(String::from(name.as_str())), v)) + let v = PackageVersionConstraint::new(vers)?; + Ok((PackageName::from(name), v)) } #[cfg(test)] -- cgit v1.2.3 From 13a85ddc5768a25f985eeb7b7dd1a8de5dce87d6 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 18 Mar 2021 08:52:19 +0100 Subject: Make ::parser() private Signed-off-by: Matthias Beyer --- src/package/version.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/package/version.rs b/src/package/version.rs index c27bf92..0c3f7f6 100644 --- a/src/package/version.rs +++ b/src/package/version.rs @@ -29,7 +29,7 @@ impl PackageVersionConstraint { Self::parser().parse(s.as_bytes()).map_err(Error::from) } - pub fn parser<'a>() -> PomParser<'a, u8, Self> { + fn parser<'a>() -> PomParser<'a, u8, Self> { (pom::parser::sym(b'=') + PackageVersion::parser()) .convert(|(constraint, version)| { String::from_utf8(vec![constraint]).map(|c| (c, version)) -- cgit v1.2.3 From 9870dd0967f33d9f8bf0279eaa03294da0b55148 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 18 Mar 2021 08:57:02 +0100 Subject: Rewrite PackageVersionConstraint constructing Use the TryFrom trait rather than a `::new()` constructor that can fail. This is way more idomatic. Signed-off-by: Matthias Beyer --- src/commands/env_of.rs | 5 +++-- src/commands/find_artifact.rs | 4 ++-- src/commands/find_pkg.rs | 5 +++-- src/commands/lint.rs | 4 ++-- src/commands/source.rs | 10 ++++------ src/commands/tree_of.rs | 5 +++-- src/package/dependency/mod.rs | 4 +++- src/package/version.rs | 22 ++++++++++++++++++---- 8 files changed, 38 insertions(+), 21 deletions(-) diff --git a/src/commands/env_of.rs b/src/commands/env_of.rs index 4ee48d1..c763a23 100644 --- a/src/commands/env_of.rs +++ b/src/commands/env_of.rs @@ -8,6 +8,8 @@ // SPDX-License-Identifier: EPL-2.0 // +use std::convert::TryFrom; + use anyhow::Result; use clap::ArgMatches; use log::trace; @@ -29,8 +31,7 @@ pub async fn env_of(matches: &ArgMatches, repo: Repository) -> Result<()> { .unwrap(); let constraint = matches .value_of("package_version_constraint") - .map(String::from) - .map(PackageVersionConstraint::new) + .map(PackageVersionConstraint::try_from) .unwrap()?; trace!( "Checking for package with name = {} and version = {:?}", diff --git a/src/commands/find_artifact.rs b/src/commands/find_artifact.rs index ee11229..17db563 100644 --- a/src/commands/find_artifact.rs +++ b/src/commands/find_artifact.rs @@ -11,6 +11,7 @@ use std::path::PathBuf; use std::io::Write; use std::sync::Arc; +use std::convert::TryFrom; use anyhow::Context; use anyhow::Error; @@ -37,8 +38,7 @@ pub async fn find_artifact(matches: &ArgMatches, config: &Configuration, progres let package_version_constraint = matches .value_of("package_version_constraint") - .map(String::from) - .map(PackageVersionConstraint::new) + .map(PackageVersionConstraint::try_from) .transpose() .context("Parsing package version constraint") .context("A valid package version constraint looks like this: '=1.0.0'")?; diff --git a/src/commands/find_pkg.rs b/src/commands/find_pkg.rs index e2f7b77..7190ef8 100644 --- a/src/commands/find_pkg.rs +++ b/src/commands/find_pkg.rs @@ -8,6 +8,8 @@ // SPDX-License-Identifier: EPL-2.0 // +use std::convert::TryFrom; + use anyhow::Context; use anyhow::Result; use clap::ArgMatches; @@ -31,8 +33,7 @@ pub async fn find_pkg( let package_version_constraint = matches .value_of("package_version_constraint") - .map(String::from) - .map(PackageVersionConstraint::new) + .map(PackageVersionConstraint::try_from) .transpose() .context("Parsing package version constraint") .context("A valid package version constraint looks like this: '=1.0.0'")?; diff --git a/src/commands/lint.rs b/src/commands/lint.rs index 9ad9c7d..2a54c62 100644 --- a/src/commands/lint.rs +++ b/src/commands/lint.rs @@ -8,6 +8,7 @@ // SPDX-License-Identifier: EPL-2.0 // +use std::convert::TryFrom; use std::path::Path; use anyhow::anyhow; @@ -36,8 +37,7 @@ pub async fn lint( .map(PackageName::from); let pvers = matches .value_of("package_version") - .map(String::from) - .map(PackageVersionConstraint::new) + .map(PackageVersionConstraint::try_from) .transpose()?; let bar = progressbars.bar(); diff --git a/src/commands/source.rs b/src/commands/source.rs index f29ce84..c3655cc 100644 --- a/src/commands/source.rs +++ b/src/commands/source.rs @@ -11,6 +11,7 @@ use std::io::Write; use std::path::PathBuf; use std::sync::Arc; +use std::convert::TryFrom; use anyhow::anyhow; use anyhow::Context; @@ -60,8 +61,7 @@ pub async fn verify( .map(PackageName::from); let pvers = matches .value_of("package_version") - .map(String::from) - .map(PackageVersionConstraint::new) + .map(PackageVersionConstraint::try_from) .transpose()?; let packages = repo @@ -183,8 +183,7 @@ pub async fn url(matches: &ArgMatches, repo: Repository) -> Result<()> { .map(PackageName::from); let pvers = matches .value_of("package_version") - .map(String::from) - .map(PackageVersionConstraint::new) + .map(PackageVersionConstraint::try_from) .transpose()?; repo.packages() @@ -225,8 +224,7 @@ pub async fn download( .map(PackageName::from); let pvers = matches .value_of("package_version") - .map(String::from) - .map(PackageVersionConstraint::new) + .map(PackageVersionConstraint::try_from) .transpose()?; let multi = { let mp = indicatif::MultiProgress::new(); diff --git a/src/commands/tree_of.rs b/src/commands/tree_of.rs index 30196b7..10e51d3 100644 --- a/src/commands/tree_of.rs +++ b/src/commands/tree_of.rs @@ -8,6 +8,8 @@ // SPDX-License-Identifier: EPL-2.0 // +use std::convert::TryFrom; + use anyhow::Error; use anyhow::Result; use clap::ArgMatches; @@ -31,8 +33,7 @@ pub async fn tree_of( .map(PackageName::from); let pvers = matches .value_of("package_version") - .map(String::from) - .map(PackageVersionConstraint::new) + .map(PackageVersionConstraint::try_from) .transpose()?; repo.packages() diff --git a/src/package/dependency/mod.rs b/src/package/dependency/mod.rs index f2fd80f..e58d0a0 100644 --- a/src/package/dependency/mod.rs +++ b/src/package/dependency/mod.rs @@ -8,6 +8,8 @@ // SPDX-License-Identifier: EPL-2.0 // +use std::convert::TryFrom; + use anyhow::anyhow; use anyhow::Result; use lazy_static::lazy_static; @@ -60,7 +62,7 @@ pub(in crate::package::dependency) fn parse_package_dependency_string_into_name_ .map(|m| String::from(m.as_str())) .ok_or_else(|| anyhow!("Could not parse version: '{}'", s))?; - let v = PackageVersionConstraint::new(vers)?; + let v = PackageVersionConstraint::try_from(vers)?; Ok((PackageName::from(name), v)) } diff --git a/src/package/version.rs b/src/package/version.rs index 0c3f7f6..9705c8d 100644 --- a/src/package/version.rs +++ b/src/package/version.rs @@ -25,10 +25,6 @@ pub struct PackageVersionConstraint { } impl PackageVersionConstraint { - pub fn new(s: String) -> Result { - Self::parser().parse(s.as_bytes()).map_err(Error::from) - } - fn parser<'a>() -> PomParser<'a, u8, Self> { (pom::parser::sym(b'=') + PackageVersion::parser()) .convert(|(constraint, version)| { @@ -53,6 +49,24 @@ impl PackageVersionConstraint { } } +impl std::convert::TryFrom for PackageVersionConstraint { + type Error = anyhow::Error; + + fn try_from(s: String) -> Result { + Self::try_from(&s as &str) + } +} + +impl std::convert::TryFrom<&str> for PackageVersionConstraint { + type Error = anyhow::Error; + + fn try_from(s: &str) -> Result { + PackageVersionConstraint::parser() + .parse(s.as_bytes()) + .map_err(Error::from) + } +} + #[derive( parse_display::Display, Serialize, -- cgit v1.2.3 From 586bca9267adf754042a9f4fc772d57da4d9ef6d Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 18 Mar 2021 08:59:37 +0100 Subject: Make error message more verbose Signed-off-by: Matthias Beyer --- src/package/version.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/package/version.rs b/src/package/version.rs index 9705c8d..359c80f 100644 --- a/src/package/version.rs +++ b/src/package/version.rs @@ -10,6 +10,7 @@ use std::ops::Deref; +use anyhow::Context; use anyhow::Error; use anyhow::Result; use pom::parser::Parser as PomParser; @@ -63,7 +64,10 @@ impl std::convert::TryFrom<&str> for PackageVersionConstraint { fn try_from(s: &str) -> Result { PackageVersionConstraint::parser() .parse(s.as_bytes()) + .context("Failed to parse package version constraint") + .context("A package version constraint must have a comparator and a version string, like so: =0.1.0") .map_err(Error::from) + } } -- cgit v1.2.3