Added LintedSpec type to return lint warnings

Signed-off-by: Nichol Yip <[email protected]>

Author: Nichol Yip

crates/spk-schema/src/v0/spec.rs

@@ -83,8 +83,8 @@ pub struct Spec<Ident> {
 
 #[derive(Debug, Clone, Hash, PartialEq, Eq, Ord, PartialOrd, Serialize)]
 pub struct LintedSpec<Ident> {
-    spec: Spec<Ident>,
-    lints: Vec<String>,
+    pub spec: Spec<Ident>,
+    pub lints: Vec<String>,
 }
 
 impl<Ident> Spec<Ident> {
@@ -730,7 +730,7 @@ where
     where
         D: serde::de::Deserializer<'de>,
     {
-        deserializer.deserialize_map(SpecVisitor::recipe())
+        Ok(std::convert::Into::<Spec<VersionIdent>>::into(deserializer.deserialize_map(SpecVisitor::recipe())?))
     }
 }
 
@@ -742,7 +742,7 @@ where
     where
         D: serde::de::Deserializer<'de>,
     {
-        let mut spec = deserializer.deserialize_map(SpecVisitor::default())?;
+        let mut spec: Spec<AnyIdent> = deserializer.deserialize_map(SpecVisitor::default())?.into();
         if spec.pkg.is_source() {
             // for backward-compatibility with older publishes, prune out anything
             // that is not relevant to a source package, since now source packages
@@ -761,7 +761,7 @@ where
     where
         D: serde::de::Deserializer<'de>,
     {
-        let mut spec = deserializer.deserialize_map(SpecVisitor::package())?;
+        let mut spec: Spec<BuildIdent> = deserializer.deserialize_map(SpecVisitor::package())?.into();
         if spec.pkg.is_source() {
             // for backward-compatibility with older publishes, prune out anything
             // that is not relevant to a source package, since now source packages
@@ -781,7 +781,7 @@ where
     where
         D: serde::de::Deserializer<'de>,
     {
-        deserializer.deserialize_map(SpecVisitor::recipe()).into()
+        Ok(std::convert::Into::<LintedSpec<VersionIdent>>::into(deserializer.deserialize_map(SpecVisitor::recipe())?))
     }
 }
 
@@ -793,7 +793,7 @@ where
     where
         D: serde::de::Deserializer<'de>,
     {
-        let mut spec: LintedSpec<AnyIdent> = deserializer.deserialize_map(SpecVisitor::default()).into();
+        let mut spec: LintedSpec<AnyIdent> = deserializer.deserialize_map(SpecVisitor::default())?.into();
         if spec.spec.pkg.is_source() {
             // for backward-compatibility with older publishes, prune out anything
             // that is not relevant to a source package, since now source packages
@@ -812,7 +812,7 @@ where
     where
         D: serde::de::Deserializer<'de>,
     {
-        let mut spec: LintedSpec<BuildIdent> = deserializer.deserialize_map(SpecVisitor::package()).into();
+        let mut spec: LintedSpec<BuildIdent> = deserializer.deserialize_map(SpecVisitor::package())?.into();
         if spec.spec.pkg.is_source() {
             // for backward-compatibility with older publishes, prune out anything
             // that is not relevant to a source package, since now source packages
@@ -854,59 +854,72 @@ impl<B, T> Default for SpecVisitor<B, T> {
     }
 }
 
-impl<'de, A, B, T> From<SpecVisitor<B, T>> for LintedSpec<Ident<B, T>>
+impl<B, T> From<SpecVisitor<B, T>> for LintedSpec<Ident<B, T>>
 where
-    A: serde::de::MapAccess<'de>,
     Ident<B, T>: serde::de::DeserializeOwned,
 {
-    fn from(mut map: A) -> std::result::Result<Self, A::Error> 
-    {   
-        let visitor = SpecVisitor::default();
-        while let Some(key) = map.next_key::<Stringified>()? {
-            match key.as_str() {
-                "pkg" => visitor.pkg = Some(map.next_value::<Ident<B, T>>()?),
-                "meta" => visitor.meta = Some(map.next_value::<Meta>()?),
-                "compat" => visitor.compat = Some(map.next_value::<Compat>()?),
-                "deprecated" => visitor.deprecated = Some(map.next_value::<bool>()?),
-                "sources" => visitor.sources = Some(map.next_value::<Vec<SourceSpec>>()?),
-                "build" => visitor.build = Some(map.next_value::<UncheckedBuildSpec>()?),
-                "tests" => visitor.tests = Some(map.next_value::<Vec<TestSpec>>()?),
-                "install" => visitor.install = Some(map.next_value::<InstallSpec>()?),
-                unknown_key => {
-                    visitor.lints.push(format!("Unrecognized key {unknown_key}"));
-                    map.next_value::<serde::de::IgnoredAny>()?;
-                }
-            }
-        }
-
-        let pkg = visitor
-            .pkg
-            .take()
-            .ok_or_else(|| serde::de::Error::missing_field("pkg"))?;
-
-        Ok(LintedSpec {
+    fn from(mut value: SpecVisitor<B, T>) -> Self {   
+        Self {
             spec: Spec {
-                meta: visitor.meta.take().unwrap_or_default(),
-                compat: visitor.compat.take().unwrap_or_default(),
-                deprecated: visitor.deprecated.take().unwrap_or_default(),
-                sources: visitor
+                pkg: value.pkg.expect("Missing field: pkg"),
+                meta: value.meta.take().unwrap_or_default(),
+                compat: value.compat.take().unwrap_or_default(),
+                deprecated: value.deprecated.take().unwrap_or_default(),
+                sources: value
                     .sources
                     .take()
                     .unwrap_or_else(|| vec![SourceSpec::Local(LocalSource::default())]),
-                build: match visitor.build.take() {
-                    Some(build_spec) if !visitor.check_build_spec => {
+                build: match value.build.take() {
+                    Some(build_spec) if !value.check_build_spec => {
                         // Safety: see the SpecVisitor::package constructor
                         unsafe { build_spec.into_inner() }
                     }
-                    Some(build_spec) => build_spec.try_into().map_err(serde::de::Error::custom)?,
+                    Some(build_spec) => {
+                        match build_spec.try_into() {
+                            Ok(b) => b,
+                            Err(_) => BuildSpec::default(),
+                        }
+                    },
                     None => Default::default(),
                 },
-                tests: visitor.tests.take().unwrap_or_default(),
-                install: visitor.install.take().unwrap_or_default(),
-                pkg,
+                tests: value.tests.take().unwrap_or_default(),
+                install: value.install.take().unwrap_or_default(),
+            },
+            lints: value.lints
+        }
+    }
+}
+
+impl<B, T> From<SpecVisitor<B, T>> for Spec<Ident<B, T>>
+where
+    Ident<B, T>: serde::de::DeserializeOwned,
+{
+    fn from(mut value: SpecVisitor<B, T>) -> Self {   
+        Self {
+            pkg: value.pkg.expect("Missing field pkg"),
+            meta: value.meta.take().unwrap_or_default(),
+            compat: value.compat.take().unwrap_or_default(),
+            deprecated: value.deprecated.take().unwrap_or_default(),
+            sources: value
+                .sources
+                .take()
+                .unwrap_or_else(|| vec![SourceSpec::Local(LocalSource::default())]),
+            build: match value.build.take() {
+                Some(build_spec) if !value.check_build_spec => {
+                    // Safety: see the SpecVisitor::package constructor
+                    unsafe { build_spec.into_inner() }
+                }
+                Some(build_spec) => {
+                    match build_spec.try_into() {
+                        Ok(b) => b,
+                        Err(_) => BuildSpec::default(),
+                    }
+                },
+                None => Default::default(),
             },
-            lints: visitor.lints
-        })
+            tests: value.tests.take().unwrap_or_default(),
+            install: value.install.take().unwrap_or_default(),
+        }
     }
 }
 
@@ -935,7 +948,7 @@ impl<'de, B, T> serde::de::Visitor<'de> for SpecVisitor<B, T>
 where
     Ident<B, T>: serde::de::DeserializeOwned,
 {
-    type Value = Spec<Ident<B, T>>;
+    type Value = SpecVisitor<B, T>;
 
     fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
         f.write_str("a package specification")
@@ -955,37 +968,50 @@ where
                 "build" => self.build = Some(map.next_value::<UncheckedBuildSpec>()?),
                 "tests" => self.tests = Some(map.next_value::<Vec<TestSpec>>()?),
                 "install" => self.install = Some(map.next_value::<InstallSpec>()?),
-                _ => {
+                unrecognized_string => {
+                    self.lints.push(format!("unrecognized string {unrecognized_string}"));
                     // ignore any unrecognized field, but consume the value anyway
                     // TODO: could we warn about fields that look like typos?
                     map.next_value::<serde::de::IgnoredAny>()?;
                 }
             }
         }
 
-        let pkg = self
-            .pkg
-            .take()
-            .ok_or_else(|| serde::de::Error::missing_field("pkg"))?;
-        Ok(Spec {
-            meta: self.meta.take().unwrap_or_default(),
-            compat: self.compat.take().unwrap_or_default(),
-            deprecated: self.deprecated.take().unwrap_or_default(),
-            sources: self
-                .sources
-                .take()
-                .unwrap_or_else(|| vec![SourceSpec::Local(LocalSource::default())]),
-            build: match self.build.take() {
-                Some(build_spec) if !self.check_build_spec => {
-                    // Safety: see the SpecVisitor::package constructor
-                    unsafe { build_spec.into_inner() }
-                }
-                Some(build_spec) => build_spec.try_into().map_err(serde::de::Error::custom)?,
-                None => Default::default(),
-            },
-            tests: self.tests.take().unwrap_or_default(),
-            install: self.install.take().unwrap_or_default(),
-            pkg,
-        })
+        self.pkg.as_ref().ok_or_else(|| serde::de::Error::missing_field("pkg"))?;
+        // match self.build {
+        //     Some(build_spec) if !self.check_build_spec => {
+        //         // Safety: see the SpecVisitor::package constructor
+        //         unsafe { build_spec.into_inner(); }
+        //     },
+        //     Some(build_spec) => build_spec.try_into().map_err(serde::de::Error::custom)?,
+        //     _ => (),
+        // }
+        Ok(self.into())
+
+        // let pkg = self
+        //     .pkg
+        //     .take()
+        //     .ok_or_else(|| serde::de::Error::missing_field("pkg"))?;
+        // Ok(S {
+        //     meta: self.meta.take().unwrap_or_default(),
+        //     compat: self.compat.take().unwrap_or_default(),
+        //     deprecated: self.deprecated.take().unwrap_or_default(),
+        //     sources: self
+        //         .sources
+        //         .take()
+        //         .unwrap_or_else(|| vec![SourceSpec::Local(LocalSource::default())]),
+        //     build: match self.build.take() {
+        //         Some(build_spec) if !self.check_build_spec => {
+        //             // Safety: see the SpecVisitor::package constructor
+        //             unsafe { build_spec.into_inner() }
+        //         }
+        //         Some(build_spec) => build_spec.try_into().map_err(serde::de::Error::custom)?,
+        //         None => Default::default(),
+        //     },
+        //     tests: self.tests.take().unwrap_or_default(),
+        //     install: self.install.take().unwrap_or_default(),
+        //     pkg,
+        // })
     }
 }
+