fix: deserialize package reqs as normalized (#49)

Author: dsherret

src/common.rs

@@ -0,0 +1,159 @@
+// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
+
+use monch::*;
+
+use crate::CowVec;
+use crate::Partial;
+use crate::VersionBoundKind;
+use crate::VersionPreOrBuild;
+use crate::VersionRange;
+use crate::XRange;
+
+// logical-or ::= ( ' ' ) * '||' ( ' ' ) *
+pub fn logical_or(input: &str) -> ParseResult<&str> {
+  delimited(skip_whitespace, tag("||"), skip_whitespace)(input)
+}
+
+// logical-and ::= ( ' ' ) * '&&' ( ' ' ) *
+pub fn logical_and(input: &str) -> ParseResult<&str> {
+  delimited(skip_whitespace, tag("&&"), skip_whitespace)(input)
+}
+
+// partial ::= xr ( '.' xr ( '.' xr qualifier ? )? )?
+pub fn partial<'a>(
+  xr: impl Fn(&'a str) -> ParseResult<'a, XRange>,
+) -> impl Fn(&'a str) -> ParseResult<'a, Partial> {
+  move |input| {
+    let (input, major) = xr(input)?;
+    let (input, maybe_minor) = maybe(preceded(ch('.'), &xr))(input)?;
+    let (input, maybe_patch) = if maybe_minor.is_some() {
+      maybe(preceded(ch('.'), &xr))(input)?
+    } else {
+      (input, None)
+    };
+    let (input, qual) = if maybe_patch.is_some() {
+      maybe(qualifier)(input)?
+    } else {
+      (input, None)
+    };
+    let qual = qual.unwrap_or_default();
+    Ok((
+      input,
+      Partial {
+        major,
+        minor: maybe_minor.unwrap_or(XRange::Wildcard),
+        patch: maybe_patch.unwrap_or(XRange::Wildcard),
+        pre: qual.pre,
+        build: qual.build,
+      },
+    ))
+  }
+}
+
+#[derive(Debug, Clone, Default)]
+pub struct Qualifier {
+  pub pre: CowVec<VersionPreOrBuild>,
+  pub build: CowVec<VersionPreOrBuild>,
+}
+
+// qualifier ::= ( '-' pre )? ( '+' build )?
+pub fn qualifier(input: &str) -> ParseResult<Qualifier> {
+  let (input, pre_parts) = maybe(pre)(input)?;
+  let (input, build_parts) = maybe(build)(input)?;
+  Ok((
+    input,
+    Qualifier {
+      pre: pre_parts.unwrap_or_default(),
+      build: build_parts.unwrap_or_default(),
+    },
+  ))
+}
+
+// pre ::= parts
+fn pre(input: &str) -> ParseResult<CowVec<VersionPreOrBuild>> {
+  preceded(maybe(ch('-')), parts)(input)
+}
+
+// build ::= parts
+fn build(input: &str) -> ParseResult<CowVec<VersionPreOrBuild>> {
+  preceded(ch('+'), parts)(input)
+}
+
+// parts ::= part ( '.' part ) *
+fn parts(input: &str) -> ParseResult<CowVec<VersionPreOrBuild>> {
+  if_true(
+    map(separated_list(part, ch('.')), |text| {
+      text
+        .into_iter()
+        .map(VersionPreOrBuild::from_str)
+        .collect::<CowVec<_>>()
+    }),
+    |items| !items.is_empty(),
+  )(input)
+}
+
+// part ::= nr | [-0-9A-Za-z]+
+fn part(input: &str) -> ParseResult<&str> {
+  // nr is in the other set, so don't bother checking for it
+  if_true(
+    take_while(|c| c.is_ascii_alphanumeric() || c == '-'),
+    |result| !result.is_empty(),
+  )(input)
+}
+
+#[derive(Debug, Clone, Copy)]
+pub enum PrimitiveKind {
+  GreaterThan,
+  LessThan,
+  GreaterThanOrEqual,
+  LessThanOrEqual,
+  Equal,
+}
+
+pub fn primitive_kind(input: &str) -> ParseResult<PrimitiveKind> {
+  or5(
+    map(tag(">="), |_| PrimitiveKind::GreaterThanOrEqual),
+    map(tag("<="), |_| PrimitiveKind::LessThanOrEqual),
+    map(ch('<'), |_| PrimitiveKind::LessThan),
+    map(ch('>'), |_| PrimitiveKind::GreaterThan),
+    map(ch('='), |_| PrimitiveKind::Equal),
+  )(input)
+}
+
+#[derive(Debug, Clone)]
+pub struct Primitive {
+  pub kind: PrimitiveKind,
+  pub partial: Partial,
+}
+
+impl Primitive {
+  pub fn into_version_range(self) -> VersionRange {
+    let partial = self.partial;
+    match self.kind {
+      PrimitiveKind::Equal => partial.as_equal_range(),
+      PrimitiveKind::GreaterThan => {
+        partial.as_greater_than(VersionBoundKind::Exclusive)
+      }
+      PrimitiveKind::GreaterThanOrEqual => {
+        partial.as_greater_than(VersionBoundKind::Inclusive)
+      }
+      PrimitiveKind::LessThan => {
+        partial.as_less_than(VersionBoundKind::Exclusive)
+      }
+      PrimitiveKind::LessThanOrEqual => {
+        partial.as_less_than(VersionBoundKind::Inclusive)
+      }
+    }
+  }
+}
+
+pub fn primitive<'a>(
+  partial: impl Fn(&'a str) -> ParseResult<'a, Partial>,
+) -> impl Fn(&'a str) -> ParseResult<'a, Primitive> {
+  move |input| {
+    let (input, kind) = primitive_kind(input)?;
+    let (input, _) = skip_whitespace(input)?;
+    let (input, partial) = partial(input)?;
+    Ok((input, Primitive { kind, partial }))
+  }
+}

src/jsr.rs

@@ -213,7 +213,7 @@ impl<'de> Deserialize<'de> for JsrDepPackageReq {
     D: serde::Deserializer<'de>,
   {
     let text: Cow<'de, str> = Deserialize::deserialize(deserializer)?;
-    match Self::from_str_loose(&text) {
+    match Self::from_str_normalized(&text) {
       Ok(req) => Ok(req),
       Err(err) => Err(serde::de::Error::custom(err)),
     }
@@ -240,13 +240,18 @@ impl JsrDepPackageReq {
     Self::from_str_inner(text, PackageReq::from_str)
   }
 
-  #[allow(clippy::should_implement_trait)]
   pub fn from_str_loose(
     text: &str,
   ) -> Result<Self, JsrDepPackageReqParseError> {
     Self::from_str_inner(text, PackageReq::from_str_loose)
   }
 
+  pub fn from_str_normalized(
+    text: &str,
+  ) -> Result<Self, JsrDepPackageReqParseError> {
+    Self::from_str_inner(text, PackageReq::from_str_normalized)
+  }
+
   fn from_str_inner(
     text: &str,
     parse_req: impl FnOnce(&str) -> Result<PackageReq, PackageReqParseError>,
@@ -443,4 +448,16 @@ mod test {
     run_test("jsr:a@^1.0", "jsr:a@1");
     run_test("jsr:a@1.2.3 || 1.4.5", "jsr:a@1.2.3 || 1.4.5"); // note: this is the serialized form--it's not a url
   }
+
+  #[test]
+  fn serialize_deserialize_tag_package_req_with_v() {
+    // note: this specifier is a tag and not a version
+    let package_req = JsrDepPackageReq::from_str("npm:test@v1.0").unwrap();
+    assert!(package_req.req.version_req.tag().is_some());
+    let json = serde_json::to_string(&package_req).unwrap();
+    assert_eq!(json, "\"npm:test@v1.0\"");
+    let result = serde_json::from_str::<JsrDepPackageReq>(&json).unwrap();
+    assert!(result.req.version_req.tag().is_some());
+    assert_eq!(result, package_req);
+  }
 }

src/lib.rs

@@ -17,10 +17,12 @@ use serde::Deserialize;
 use serde::Serialize;
 use thiserror::Error;
 
+mod common;
 pub mod jsr;
 pub mod npm;
 pub mod package;
 mod range;
+mod range_set_or_tag;
 mod specifier;
 mod string;
 
@@ -38,6 +40,8 @@ pub use self::range::VersionBoundKind;
 pub use self::range::VersionRange;
 pub use self::range::VersionRangeSet;
 pub use self::range::XRange;
+pub use self::range_set_or_tag::PackageTag;
+pub use self::range_set_or_tag::RangeSetOrTag;
 
 /// Specifier that points to the wildcard version.
 pub static WILDCARD_VERSION_REQ: Lazy<VersionReq> =
@@ -225,39 +229,12 @@ pub(crate) fn is_valid_tag(value: &str) -> bool {
   npm::is_valid_npm_tag(value)
 }
 
-pub type PackageTag = SmallStackString;
-
-#[derive(
-  Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize, CapacityDisplay,
-)]
-pub enum RangeSetOrTag {
-  RangeSet(VersionRangeSet),
-  Tag(PackageTag),
-}
-
-impl<'a> StringAppendable<'a> for &'a RangeSetOrTag {
-  fn append_to_builder<TString: StringType>(
-    self,
-    builder: &mut StringBuilder<'a, TString>,
-  ) {
-    match self {
-      RangeSetOrTag::RangeSet(range_set) => builder.append(range_set),
-      RangeSetOrTag::Tag(tag) => builder.append(tag),
-    }
-  }
-}
-
-impl RangeSetOrTag {
-  pub fn intersects(&self, other: &RangeSetOrTag) -> bool {
-    match (self, other) {
-      (RangeSetOrTag::RangeSet(a), RangeSetOrTag::RangeSet(b)) => {
-        a.intersects_set(b)
-      }
-      (RangeSetOrTag::RangeSet(_), RangeSetOrTag::Tag(_))
-      | (RangeSetOrTag::Tag(_), RangeSetOrTag::RangeSet(_)) => false,
-      (RangeSetOrTag::Tag(a), RangeSetOrTag::Tag(b)) => a == b,
-    }
-  }
+#[derive(Error, Debug, Clone, deno_error::JsError, PartialEq, Eq)]
+#[class(type)]
+#[error("Invalid normalized version requirement")]
+pub struct VersionReqNormalizedParseError {
+  #[source]
+  source: monch::ParseErrorFailureError,
 }
 
 /// A version constraint.
@@ -290,6 +267,22 @@ impl VersionReq {
     Self { raw_text, inner }
   }
 
+  pub fn parse_from_normalized(
+    text: &str,
+  ) -> Result<Self, VersionReqNormalizedParseError> {
+    use monch::*;
+
+    with_failure_handling(|input| {
+      map(RangeSetOrTag::parse, |inner| {
+        VersionReq::from_raw_text_and_inner(
+          crate::SmallStackString::from_str(input),
+          inner,
+        )
+      })(input)
+    })(text)
+    .map_err(|err| VersionReqNormalizedParseError { source: err })
+  }
+
   pub fn parse_from_specifier(
     specifier: &str,
   ) -> Result<Self, VersionReqSpecifierParseError> {