Added lints for InstallSpec

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

Author: Nichol Yip

crates/spk-cli/group4/src/cmd_lint.rs

@@ -41,8 +41,9 @@ impl Run for Lint {
                 Ok(s) => match s.lints.is_empty() {
                     true => println!("{} {}", "OK".green(), spec.display()),
                     false => {
+                        println!("{} {}:", "Failed".red(), spec.display());
                         for lint in s.lints {
-                            tracing::error!(lint);
+                            println!("{} {lint}", "----->".red());
                         }
                         out = 1;
                     }

crates/spk-schema/src/install_spec.rs

@@ -1,7 +1,11 @@
 // Copyright (c) Sony Pictures Imageworks, et al.
 // SPDX-License-Identifier: Apache-2.0
 // https://github.com/imageworks/spk
+use std::marker::PhantomData;
+
+use ngrammatic::CorpusBuilder;
 use serde::{Deserialize, Serialize};
+use spk_schema_foundation::option_map::Stringified;
 use spk_schema_ident::BuildIdent;
 
 use super::{ComponentSpecList, EmbeddedPackagesList, EnvOp, OpKind, RequirementsList};
@@ -13,21 +17,66 @@ use crate::Result;
 mod install_spec_test;
 
 /// A set of structured installation parameters for a package.
-#[derive(Clone, Debug, Default, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)]
+#[derive(Clone, Debug, Default, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)]
 pub struct InstallSpec {
     #[serde(default, skip_serializing_if = "Vec::is_empty")]
     pub requirements: RequirementsList,
     #[serde(default, skip_serializing_if = "Vec::is_empty")]
     pub embedded: EmbeddedPackagesList,
     #[serde(default)]
     pub components: ComponentSpecList,
-    #[serde(
-        default,
-        deserialize_with = "deserialize_env_conf",
-        skip_serializing_if = "Vec::is_empty"
-    )]
+    #[serde(default, skip_serializing_if = "Vec::is_empty")]
     pub environment: Vec<EnvOp>,
 }
+#[derive(Clone, Debug, Default, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)]
+pub struct LintedInstallSpec {
+    pub install_spec: InstallSpec,
+    pub lints: Vec<String>,
+}
+
+#[derive(Default)]
+struct InstallSpecVisitor<D>
+where
+    D: Default,
+{
+    requirements: RequirementsList,
+    embedded: EmbeddedPackagesList,
+    components: ComponentSpecList,
+    environment: Vec<EnvOp>,
+    lints: Vec<String>,
+    _phantom: PhantomData<D>,
+}
+
+impl<D> From<InstallSpecVisitor<D>> for InstallSpec
+where
+    D: Default,
+{
+    fn from(value: InstallSpecVisitor<D>) -> Self {
+        Self {
+            requirements: value.requirements,
+            embedded: value.embedded,
+            components: value.components,
+            environment: value.environment,
+        }
+    }
+}
+
+impl<D> From<InstallSpecVisitor<D>> for LintedInstallSpec
+where
+    D: Default,
+{
+    fn from(value: InstallSpecVisitor<D>) -> Self {
+        Self {
+            install_spec: InstallSpec {
+                requirements: value.requirements,
+                embedded: value.embedded,
+                components: value.components,
+                environment: value.environment,
+            },
+            lints: value.lints,
+        }
+    }
+}
 
 impl InstallSpec {
     pub fn is_default(&self) -> bool {
@@ -44,37 +93,86 @@ impl InstallSpec {
     }
 }
 
-fn deserialize_env_conf<'de, D>(deserializer: D) -> std::result::Result<Vec<EnvOp>, D::Error>
+impl<'de> Deserialize<'de> for InstallSpec {
+    fn deserialize<D>(deserializer: D) -> std::result::Result<Self, D::Error>
+    where
+        D: serde::de::Deserializer<'de>,
+    {
+        Ok(std::convert::Into::<InstallSpec>::into(
+            deserializer.deserialize_map(InstallSpecVisitor::<InstallSpec>::default())?,
+        ))
+    }
+}
+
+impl<'de> Deserialize<'de> for LintedInstallSpec {
+    fn deserialize<D>(deserializer: D) -> std::result::Result<Self, D::Error>
+    where
+        D: serde::de::Deserializer<'de>,
+    {
+        Ok(std::convert::Into::<LintedInstallSpec>::into(
+            deserializer.deserialize_map(InstallSpecVisitor::<LintedInstallSpec>::default())?,
+        ))
+    }
+}
+
+impl<'de, D> serde::de::Visitor<'de> for InstallSpecVisitor<D>
 where
-    D: serde::Deserializer<'de>,
+    D: Default + From<InstallSpecVisitor<D>>,
 {
-    struct EnvConfVisitor;
+    type Value = D;
 
-    impl<'de> serde::de::Visitor<'de> for EnvConfVisitor {
-        type Value = Vec<EnvOp>;
+    fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        f.write_str("a package specification")
+    }
+
+    fn visit_map<A>(mut self, mut map: A) -> std::result::Result<Self::Value, A::Error>
+    where
+        A: serde::de::MapAccess<'de>,
+    {
+        while let Some(key) = map.next_key::<Stringified>()? {
+            match key.as_str() {
+                "requirements" => self.requirements = map.next_value::<RequirementsList>()?,
+                "embedded" => self.embedded = map.next_value::<EmbeddedPackagesList>()?,
+                "components" => self.components = map.next_value::<ComponentSpecList>()?,
+                "environment" => self.environment = map.next_value::<Vec<EnvOp>>()?,
+                unknown_config => {
+                    self.lints.push(format!("Unknown config: {unknown_config}"));
+                    let mut corpus = CorpusBuilder::new().finish();
+
+                    corpus.add_text("requirements");
+                    corpus.add_text("embedded");
+                    corpus.add_text("components");
+                    corpus.add_text("environment");
+
+                    let results = corpus.search(unknown_config, 0.6);
+                    let no_match = format!("No similar config found for: {}", unknown_config);
+                    let top_match = match results.first() {
+                        Some(s) => &s.text,
+                        None => &no_match,
+                    };
 
-        fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
-            f.write_str("an environment configuration")
+                    self.lints
+                        .push(format!("The most similar config is: {}", top_match));
+
+                    // 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>()?;
+                }
+            }
         }
 
-        fn visit_seq<A>(self, mut seq: A) -> std::result::Result<Self::Value, A::Error>
-        where
-            A: serde::de::SeqAccess<'de>,
+        if self
+            .environment
+            .iter()
+            .filter(|&e| e.kind() == OpKind::Priority)
+            .count()
+            > 1
         {
-            let mut vec = Vec::new();
-
-            while let Some(elem) = seq.next_element::<EnvOp>()? {
-                if vec.iter().any(|x: &EnvOp| x.kind() == OpKind::Priority)
-                    && elem.kind() == OpKind::Priority
-                {
-                    return Err(serde::de::Error::custom(
-                        "Multiple priority config cannot be set.",
-                    ));
-                };
-                vec.push(elem);
-            }
-            Ok(vec)
+            return Err(serde::de::Error::custom(
+                "Multiple priority configs cannot be set",
+            ));
         }
+
+        Ok(self.into())
     }
-    deserializer.deserialize_seq(EnvConfVisitor)
 }

crates/spk-schema/src/lib.rs

@@ -34,7 +34,7 @@ pub use deprecate::{Deprecate, DeprecateMut};
 pub use embedded_packages_list::EmbeddedPackagesList;
 pub use environ::{AppendEnv, EnvOp, OpKind, PrependEnv, SetEnv};
 pub use error::{Error, Result};
-pub use install_spec::InstallSpec;
+pub use install_spec::{InstallSpec, LintedInstallSpec};
 pub use option::{Inheritance, Opt};
 pub use package::{Package, PackageMut};
 pub use recipe::{BuildEnv, Recipe};

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

@@ -46,6 +46,7 @@ use crate::{
     Error,
     Inheritance,
     InstallSpec,
+    LintedInstallSpec,
     LocalSource,
     Opt,
     Package,
@@ -840,7 +841,7 @@ struct SpecVisitor<B, T> {
     sources: Option<Vec<SourceSpec>>,
     build: Option<UncheckedBuildSpec>,
     tests: Option<Vec<TestSpec>>,
-    install: Option<InstallSpec>,
+    install: Option<LintedInstallSpec>,
     check_build_spec: bool,
     lints: Vec<String>,
 }
@@ -867,6 +868,11 @@ where
     Ident<B, T>: serde::de::DeserializeOwned,
 {
     fn from(mut value: SpecVisitor<B, T>) -> Self {
+        match &value.install {
+            Some(l) => value.lints.append(&mut l.lints.clone()),
+            None => (),
+        }
+
         Self {
             spec: Spec {
                 pkg: value.pkg.expect("Missing field: pkg"),
@@ -889,7 +895,7 @@ where
                     None => Default::default(),
                 },
                 tests: value.tests.take().unwrap_or_default(),
-                install: value.install.take().unwrap_or_default(),
+                install: value.install.take().unwrap_or_default().install_spec,
             },
             lints: value.lints,
         }
@@ -922,7 +928,7 @@ where
                 None => Default::default(),
             },
             tests: value.tests.take().unwrap_or_default(),
-            install: value.install.take().unwrap_or_default(),
+            install: value.install.take().unwrap_or_default().install_spec,
         }
     }
 }
@@ -970,7 +976,7 @@ where
                 "sources" => self.sources = Some(map.next_value::<Vec<SourceSpec>>()?),
                 "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>()?),
+                "install" => self.install = Some(map.next_value::<LintedInstallSpec>()?),
                 "api" => {
                     map.next_value::<serde::de::IgnoredAny>()?;
                 }