- Implement test cases for spec types that has Lints implemented

- Fix how lints are outputted to CLI

Signed-off-by: Nichol Yip <nyip@imageworks.com>

Author: Nichol Yip

crates/spk-schema/src/build_spec.rs

@@ -437,6 +437,10 @@ struct BuildSpecVisitor {
 
 impl Lints for BuildSpecVisitor {
     fn lints(&mut self) -> Vec<Lint> {
+        for lint in self.lints.iter_mut() {
+            lint.update_key("build");
+        }
+
         std::mem::take(&mut self.lints)
     }
 }
@@ -503,11 +507,15 @@ impl<'de> serde::de::Visitor<'de> for BuildSpecVisitor {
                 "variants" => {
                     unchecked.raw_variants = map.next_value()?;
                 }
-                "validation" => unchecked.validation = map.next_value::<ValidationSpec>()?,
+                "validation" => {
+                    let linted_validations = map.next_value::<LintedItem<ValidationSpec>>()?;
+                    self.lints.extend(linted_validations.lints);
+                    unchecked.validation = linted_validations.item;
+                }
                 "auto_host_vars" => unchecked.auto_host_vars = map.next_value::<AutoHostVars>()?,
                 unknown_key => {
                     self.lints.push(Lint::Key(UnknownKey::new(
-                        &format!("build.{unknown_key}"),
+                        &unknown_key,
                         BuildSpec::FIELD_NAMES_AS_ARRAY.to_vec(),
                     )));
                     map.next_value::<serde::de::IgnoredAny>()?;

crates/spk-schema/src/build_spec_test.rs

@@ -7,6 +7,7 @@ use spk_schema_foundation::{option_map, pkg_name, FromYaml, IsDefault};
 
 use super::{AutoHostVars, BuildSpec};
 use crate::build_spec::UncheckedBuildSpec;
+use crate::LintedItem;
 
 #[rstest]
 fn test_auto_host_vars_default() {
@@ -223,3 +224,127 @@ options:
         .unwrap();
     assert_ne!(build_id1, build_id2);
 }
+
+#[rstest]
+fn test_build_script_lint() {
+    let build_spec: LintedItem<UncheckedBuildSpec> = serde_yaml::from_str(
+        r#"
+options:
+    - var: arch
+    - var: os
+    - var: centos
+    - pkg: python/3
+variants:
+    - {python: 2.7}
+    - {python: 3.7, gcc: 9.3}
+scripts:
+    - echo "Hello World!"
+    "#,
+    )
+    .unwrap();
+
+    assert_eq!(build_spec.lints.len(), 1);
+    for lint in build_spec.lints.iter() {
+        assert_eq!(lint.get_key(), "build.scripts");
+    }
+}
+
+#[rstest]
+fn test_build_variant_lint() {
+    let build_spec: LintedItem<UncheckedBuildSpec> = serde_yaml::from_str(
+        r#"
+options:
+    - var: arch
+    - var: os
+    - var: centos
+    - pkg: python/3
+variant:
+    - {python: 2.7}
+    - {python: 3.7, gcc: 9.3}
+script:
+    - echo "Hello World!"
+    "#,
+    )
+    .unwrap();
+
+    assert_eq!(build_spec.lints.len(), 1);
+    for lint in build_spec.lints.iter() {
+        assert_eq!(lint.get_key(), "build.variant");
+    }
+}
+
+#[rstest]
+fn test_build_options_lint() {
+    let build_spec: LintedItem<UncheckedBuildSpec> = serde_yaml::from_str(
+        r#"
+option:
+    - var: arch
+    - var: os
+    - var: centos
+    - pkg: python/3
+variants:
+    - {python: 2.7}
+    - {python: 3.7, gcc: 9.3}
+script:
+    - echo "Hello World!"
+    "#,
+    )
+    .unwrap();
+
+    assert_eq!(build_spec.lints.len(), 1);
+    for lint in build_spec.lints.iter() {
+        assert_eq!(lint.get_key(), "build.option");
+    }
+}
+
+#[rstest]
+fn test_build_auto_host_vars_lint() {
+    let build_spec: LintedItem<UncheckedBuildSpec> = serde_yaml::from_str(
+        r#"
+options:
+    - var: arch
+    - var: os
+    - var: centos
+    - pkg: python/3
+variants:
+    - {python: 2.7}
+    - {python: 3.7, gcc: 9.3}
+script:
+    - echo "Hello World!"
+auto_host_var: "None"
+    "#,
+    )
+    .unwrap();
+
+    assert_eq!(build_spec.lints.len(), 1);
+    for lint in build_spec.lints.iter() {
+        assert_eq!(lint.get_key(), "build.auto_host_var");
+    }
+}
+
+#[rstest]
+fn test_build_validation_lint() {
+    let build_spec: LintedItem<UncheckedBuildSpec> = serde_yaml::from_str(
+        r#"
+options:
+    - var: arch
+    - var: os
+    - var: centos
+    - pkg: python/3
+variants:
+    - {python: 2.7}
+    - {python: 3.7, gcc: 9.3}
+script:
+    - echo "Hello World!"
+validations: {
+    "rules": [{"allow": "EmptyPackage"}]
+}
+    "#,
+    )
+    .unwrap();
+
+    assert_eq!(build_spec.lints.len(), 1);
+    for lint in build_spec.lints.iter() {
+        assert_eq!(lint.get_key(), "build.validations");
+    }
+}

crates/spk-schema/src/embedded_packages_list.rs

@@ -69,6 +69,10 @@ struct EmbeddedPackagesListVisitor {
 
 impl Lints for EmbeddedPackagesListVisitor {
     fn lints(&mut self) -> Vec<Lint> {
+        for lint in self.lints.iter_mut() {
+            lint.update_key("embedded");
+        }
+
         std::mem::take(&mut self.lints)
     }
 }

crates/spk-schema/src/embedded_packages_list_test.rs

@@ -5,6 +5,7 @@
 use rstest::rstest;
 
 use super::EmbeddedPackagesList;
+use crate::LintedItem;
 
 #[rstest]
 fn test_install_embedded_build_options() {
@@ -45,3 +46,56 @@ fn test_embedded_nested_embedded() {
     )
     .unwrap();
 }
+
+#[rstest]
+fn test_embedded_install_lint() {
+    let _spec: LintedItem<EmbeddedPackagesList> = serde_yaml::from_str(
+        r#"
+          - pkg: "embedded/1.0.0"
+            install:
+              environments:
+                - priority: 99
+        "#,
+    )
+    .unwrap();
+
+    assert_eq!(_spec.lints.len(), 1);
+    for lint in _spec.lints.iter() {
+        assert_eq!(lint.get_key(), "embedded.install.environments")
+    }
+}
+
+#[rstest]
+fn test_embedded_components_lint() {
+    let _spec: LintedItem<EmbeddedPackagesList> = serde_yaml::from_str(
+        r#"
+        - pkg: "embedded/1.0.0"
+          component:
+            - name: run
+              embedded:
+                - pkg: "nested-embedded/2.0.0"
+      "#,
+    )
+    .unwrap();
+
+    assert_eq!(_spec.lints.len(), 1);
+    for lint in _spec.lints.iter() {
+        assert_eq!(lint.get_key(), "embedded.component")
+    }
+}
+
+#[rstest]
+fn test_embedded_build_lint() {
+    let _spec: LintedItem<EmbeddedPackagesList> = serde_yaml::from_str(
+        r#"
+        - pkg: "embedded/1.0.0"
+          build: {"option": [{"var": "python.abi", "static": "cp37"}]}
+      "#,
+    )
+    .unwrap();
+
+    assert_eq!(_spec.lints.len(), 1);
+    for lint in _spec.lints.iter() {
+        assert_eq!(lint.get_key(), "embedded.build.option")
+    }
+}

crates/spk-schema/src/install_spec.rs

@@ -63,6 +63,10 @@ where
 {
     fn lints(&mut self) -> Vec<Lint> {
         self.lints.extend(std::mem::take(&mut self.embedded.lints));
+        for lint in self.lints.iter_mut() {
+            lint.update_key("install");
+        }
+
         std::mem::take(&mut self.lints)
     }
 }
@@ -260,7 +264,7 @@ where
                 "environment" => self.environment = map.next_value::<EnvOpList>()?,
                 unknown_key => {
                     self.lints.push(Lint::Key(UnknownKey::new(
-                        &format!("install.{unknown_key}"),
+                        &unknown_key,
                         InstallSpecVisitor::<D>::FIELD_NAMES_AS_ARRAY.to_vec(),
                     )));
                     map.next_value::<serde::de::IgnoredAny>()?;

crates/spk-schema/src/install_spec_test.rs

@@ -11,7 +11,7 @@ use spk_schema_foundation::option_map::OptionMap;
 use spk_schema_foundation::version::{Version, BINARY_STR};
 use spk_schema_ident::{parse_ident_range, PkgRequest, Request, RequestedBy};
 
-use crate::{InstallSpec, Package, RequirementsList};
+use crate::{InstallSpec, LintedItem, Package, RawInstallSpec, RequirementsList};
 
 #[rstest]
 fn test_render_all_pins_renders_requirements_in_components() {
@@ -280,3 +280,74 @@ embedded:
         "expecting embedded to be expanded correctly"
     );
 }
+
+#[rstest]
+fn test_install_spec_lint_component_field() {
+    let install = serde_yaml::from_str::<LintedItem<RawInstallSpec>>(
+        r#"
+comzponents:
+  - name: comp1
+        "#,
+    )
+    .unwrap();
+
+    assert_eq!(install.lints.len(), 1);
+    for lint in install.lints.iter() {
+        assert_eq!(lint.get_key(), "install.comzponents");
+    }
+}
+
+#[rstest]
+fn test_install_spec_lint_embedded_field() {
+    let install = serde_yaml::from_str::<LintedItem<RawInstallSpec>>(
+        r#"
+components:
+  - name: comp1
+embeddsed:
+  - pkg: "embedded/1.0.0"
+    install:
+      components:
+        - name: comp1
+        "#,
+    )
+    .unwrap();
+
+    assert_eq!(install.lints.len(), 1);
+    for lint in install.lints.iter() {
+        assert_eq!(lint.get_key(), "install.embeddsed");
+    }
+}
+
+#[rstest]
+fn test_install_spec_lint_requirements_field() {
+    let install = serde_yaml::from_str::<LintedItem<RawInstallSpec>>(
+        r#"
+requirement:
+    - pkg: stdfs
+        "#,
+    )
+    .unwrap();
+
+    assert_eq!(install.lints.len(), 1);
+    for lint in install.lints.iter() {
+        assert_eq!(lint.get_key(), "install.requirement");
+    }
+}
+
+#[rstest]
+fn test_install_spec_lint_environments_field() {
+    let install = serde_yaml::from_str::<LintedItem<RawInstallSpec>>(
+        r#"
+environments:
+    - priority: 99
+    - set: PYVER1
+      value: $SPK_PKG_python_VERSION_BASE
+        "#,
+    )
+    .unwrap();
+
+    assert_eq!(install.lints.len(), 1);
+    for lint in install.lints.iter() {
+        assert_eq!(lint.get_key(), "install.environments");
+    }
+}

crates/spk-schema/src/lints.rs

@@ -22,7 +22,7 @@ impl UnknownKey {
 
     pub fn generate_message(&self) -> String {
         let key: &str = self.unknown_key.split('.').last().unwrap_or_default();
-        let mut message = format!("Unrecognized key: {}. ", self.unknown_key);
+        let mut message = format!("Unrecognized key: {} ", self.unknown_key);
         let mut corpus = CorpusBuilder::new().finish();
 
         for field in self.struct_fields.iter() {
@@ -31,7 +31,7 @@ impl UnknownKey {
 
         match corpus.search(key, 0.6).first() {
             Some(s) => message.push_str(format!("(Did you mean: '{}'?)", s.text).as_str()),
-            None => message.push_str(format!("(No similar keys found for: {}.)", key).as_str()),
+            None => message.push_str(format!("(No similar keys found for: {})", key).as_str()),
         };
 
         message.to_string()
@@ -43,6 +43,20 @@ pub enum Lint {
     Key(UnknownKey),
 }
 
+impl Lint {
+    pub fn get_key(&self) -> &str {
+        match self {
+            Lint::Key(k) => &k.unknown_key,
+        }
+    }
+
+    pub fn update_key(&mut self, key: &str) {
+        match self {
+            Lint::Key(k) => k.unknown_key = format!("{key}.{}", k.unknown_key),
+        }
+    }
+}
+
 #[derive(Debug, Default, Clone, Hash, PartialEq, Eq, Ord, PartialOrd)]
 pub struct LintedItem<T> {
     pub item: T,