Moved crates to top level cargo.toml

Nichol Yip · Nichol Yip · commit e903423797c6 · 2024-01-03T10:57:37.000-08:00
Changed linting feature to return an UnknownKey object instead of a generated string

Modified new method for UnknownKey to store values needed to generated lint message

Added enum for lints to support different kinds of lints

Removed commented code

Refactored render method and renamed render_lint -&gt; render_to_string

Signed-off-by: Nichol Yip &lt;nyip@imageworks.com&gt;
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -52,8 +52,10 @@ futures-core = "0.3.28"
 fuser = "0.12"
 glob = "0.3"
 indicatif = "0.17.5"
+itertools = "0.10"
 libc = "0.2.80"
 miette = "5.10"
+ngrammatic = "0.4.0"
 nix = "0.26.2"
 nom = "7.1"
 nom-supreme = "0.8"
@@ -72,6 +74,7 @@ serde_yaml = "0.9.25"
 shellexpand = "3.1.0"
 static_assertions = "1.1"
 strip-ansi-escapes = "0.1.1"
+struct-field-names-as-array = "0.3.0"
 strum = { version = "0.24", features = ["derive"] }
 thiserror = "1.0"
 tempfile = "3.3"
diff --git a/crates/spk-cli/group4/src/cmd_lint.rs b/crates/spk-cli/group4/src/cmd_lint.rs
@@ -9,6 +9,7 @@ use colored::Colorize;
 use miette::Result;
 use spk_cli_common::{flags, CommandArgs, Run};
 use spk_schema::v0::Spec;
+use spk_schema::Lint::Key;
 use spk_schema::{AnyIdent, LintedItem, SpecTemplate, Template, TemplateExt};
 
 /// Validate spk yaml files
@@ -27,7 +28,7 @@ impl Run for Lint {
         let mut out = 0;
         let options = self.options.get_options()?;
         for spec in self.packages.iter() {
-            let yaml = SpecTemplate::from_file(spec).and_then(|t| t.render_lint(&options))?;
+            let yaml = SpecTemplate::from_file(spec).and_then(|t| t.render_to_string(&options))?;
             let lints: std::result::Result<LintedItem<Spec<AnyIdent>>, serde_yaml::Error> =
                 serde_yaml::from_str(&yaml);
 
@@ -37,7 +38,9 @@ impl Run for Lint {
                     false => {
                         println!("{} {}:", "Failed".red(), spec.display());
                         for lint in s.lints {
-                            println!("{} {}", "----->".red(), lint);
+                            match lint {
+                                Key(k) => println!("{} {}", "----->".red(), k.generate_message()),
+                            }
                         }
                         out = 1;
                     }
diff --git a/crates/spk-schema/Cargo.toml b/crates/spk-schema/Cargo.toml
@@ -20,8 +20,8 @@ format_serde_error = { version = "0.3", default_features = false, features = [
 ] }
 ignore = "0.4.18"
 indexmap = "1.7"
-itertools = "0.10"
-ngrammatic = "0.4.0"
+itertools = { workspace = true }
+ngrammatic = { workspace = true }
 nom = { workspace = true }
 regex = "1.5"
 relative-path = "1.3"
@@ -36,6 +36,7 @@ spk-schema-ident = { path = "./crates/ident" }
 spk-schema-validators = { path = "./crates/validators" }
 spk-schema-liquid = { path = "./crates/liquid" }
 strum = { workspace = true }
+struct-field-names-as-array = { workspace = true }
 sys-info = "0.9.0"
 tempfile = { workspace = true }
 thiserror = { workspace = true }
diff --git a/crates/spk-schema/src/build_spec.rs b/crates/spk-schema/src/build_spec.rs
@@ -13,7 +13,7 @@ use super::{v0, Opt, ValidationSpec};
 use crate::name::{OptName, OptNameBuf};
 use crate::option::VarOpt;
 
-use crate::{LintedItem, Lints, Result, UnknownKey, Variant};
+use crate::{Lint, LintedItem, Lints, Result, UnknownKey, Variant};
 
 #[cfg(test)]
 #[path = "./build_spec_test.rs"]
@@ -309,6 +309,24 @@ impl UncheckedBuildSpec {
     }
 }
 
+#[derive(Default)]
+struct BuildSpecVisitor {
+    build_spec: UncheckedBuildSpec,
+    lints: Vec<Lint>,
+}
+
+impl Lints for BuildSpecVisitor {
+    fn lints(&mut self) -> Vec<Lint> {
+        std::mem::take(&mut self.lints)
+    }
+}
+
+impl From<BuildSpecVisitor> for UncheckedBuildSpec {
+    fn from(value: BuildSpecVisitor) -> Self {
+        value.build_spec
+    }
+}
+
 impl<'de> Deserialize<'de> for UncheckedBuildSpec {
     fn deserialize<D>(deserializer: D) -> std::result::Result<Self, D::Error>
     where
diff --git a/crates/spk-schema/src/environ.rs b/crates/spk-schema/src/environ.rs
@@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize};
 use spk_schema_foundation::option_map::Stringified;
 use struct_field_names_as_array::FieldNamesAsArray;
 
-use crate::{LintedItem, Lints, UnknownKey};
+use crate::{Lint, LintedItem, Lints, UnknownKey};
 
 #[cfg(test)]
 #[path = "./environ_test.rs"]
@@ -191,11 +191,11 @@ struct EnvOpVisitor {
     value: Option<String>,
     separator: Option<String>,
     #[field_names_as_array(skip)]
-    lints: Vec<String>,
+    lints: Vec<Lint>,
 }
 
 impl Lints for EnvOpVisitor {
-    fn lints(&mut self) -> Vec<String> {
+    fn lints(&mut self) -> Vec<Lint> {
         std::mem::take(&mut self.lints)
     }
 }
@@ -304,8 +304,8 @@ impl<'de> serde::de::Visitor<'de> for EnvOpVisitor {
                     field_names.extend(EnvPriority::FIELD_NAMES_AS_ARRAY.to_vec());
                     field_names.extend(PrependEnv::FIELD_NAMES_AS_ARRAY.to_vec());
                     field_names.extend(SetEnv::FIELD_NAMES_AS_ARRAY.to_vec());
-                    let lint = UnknownKey::new(unknown_key, field_names);
-                    self.lints.push(lint.message.to_string());
+                    self.lints
+                        .push(Lint::Key(UnknownKey::new(unknown_key, field_names)));
                     map.next_value::<serde::de::IgnoredAny>()?;
                 }
             }
diff --git a/crates/spk-schema/src/install_spec.rs b/crates/spk-schema/src/install_spec.rs
@@ -12,7 +12,7 @@ use struct_field_names_as_array::FieldNamesAsArray;
 
 use super::{ComponentSpecList, EmbeddedPackagesList, EnvOp, OpKind, RequirementsList};
 use crate::foundation::option_map::OptionMap;
-use crate::{LintedItem, Lints, Result, UnknownKey};
+use crate::{Lint, LintedItem, Lints, Result, UnknownKey};
 
 #[cfg(test)]
 #[path = "./install_spec_test.rs"]
@@ -35,7 +35,7 @@ impl<D> Lints for InstallSpecVisitor<D>
 where
     D: Default,
 {
-    fn lints(&mut self) -> Vec<String> {
+    fn lints(&mut self) -> Vec<Lint> {
         for env in self.environment.iter_mut() {
             self.lints.extend(std::mem::take(&mut env.lints));
         }
@@ -54,7 +54,7 @@ where
     components: ComponentSpecList,
     environment: Vec<LintedItem<EnvOp>>,
     #[field_names_as_array(skip)]
-    lints: Vec<String>,
+    lints: Vec<Lint>,
     #[field_names_as_array(skip)]
     _phantom: PhantomData<D>,
 }
@@ -139,11 +139,10 @@ where
                 "components" => self.components = map.next_value::<ComponentSpecList>()?,
                 "environment" => self.environment = map.next_value::<Vec<LintedItem<EnvOp>>>()?,
                 unknown_key => {
-                    let lint = UnknownKey::new(
+                    self.lints.push(Lint::Key(UnknownKey::new(
                         unknown_key,
                         InstallSpecVisitor::<D>::FIELD_NAMES_AS_ARRAY.to_vec(),
-                    );
-                    self.lints.push(lint.message.to_string());
+                    )));
                     map.next_value::<serde::de::IgnoredAny>()?;
                 }
             }
diff --git a/crates/spk-schema/src/lib.rs b/crates/spk-schema/src/lib.rs
@@ -35,7 +35,7 @@ pub use environ::{AppendEnv, EnvComment, EnvOp, EnvPriority, OpKind, PrependEnv,
 pub use error::{Error, Result};
 pub use input_variant::InputVariant;
 pub use install_spec::InstallSpec;
-pub use lints::{LintedItem, Lints, UnknownKey};
+pub use lints::{Lint, LintedItem, Lints, UnknownKey};
 pub use option::{Inheritance, Opt};
 pub use package::{Package, PackageMut};
 pub use recipe::{BuildEnv, Recipe};
diff --git a/crates/spk-schema/src/lints.rs b/crates/spk-schema/src/lints.rs
@@ -2,46 +2,55 @@
 // SPDX-License-Identifier: Apache-2.0
 // https://github.com/imageworks/spk
 
+use std::sync::Arc;
+
 use ngrammatic::CorpusBuilder;
-use serde::Serialize;
 
-#[derive(Debug, Default, Clone, Hash, PartialEq, Eq, Ord, PartialOrd, Serialize)]
+#[derive(Debug, Default, Clone, Hash, PartialEq, Eq, Ord, PartialOrd)]
 pub struct UnknownKey {
-    pub key: String,
-    pub message: String,
+    unknown_key: String,
+    struct_fields: Vec<Arc<str>>,
 }
 
 impl UnknownKey {
     pub fn new(unknown_key: &str, struct_fields: Vec<&str>) -> Self {
-        let mut message = format!("Unrecognized key: {unknown_key}. ");
+        Self {
+            unknown_key: unknown_key.to_string(),
+            struct_fields: struct_fields.iter().map(|v| Arc::from(*v)).collect(),
+        }
+    }
+
+    pub fn generate_message(&self) -> String {
+        let mut message = format!("Unrecognized key: {}. ", self.unknown_key);
         let mut corpus = CorpusBuilder::new().finish();
 
-        for field in struct_fields.iter() {
+        for field in self.struct_fields.iter() {
             corpus.add_text(field);
         }
 
-        match corpus.search(unknown_key, 0.6).first() {
+        match corpus.search(&self.unknown_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: {}.)", unknown_key).as_str())
-            }
+            None => message
+                .push_str(format!("(No similar keys found for: {}.)", self.unknown_key).as_str()),
         };
 
-        Self {
-            key: std::mem::take(&mut unknown_key.to_string()),
-            message: message.to_string(),
-        }
+        message.to_string()
     }
 }
 
-#[derive(Debug, Default, Clone, Hash, PartialEq, Eq, Ord, PartialOrd, Serialize)]
+#[derive(Debug, Clone, Hash, PartialEq, Eq, Ord, PartialOrd)]
+pub enum Lint {
+    Key(UnknownKey),
+}
+
+#[derive(Debug, Default, Clone, Hash, PartialEq, Eq, Ord, PartialOrd)]
 pub struct LintedItem<T> {
     pub item: T,
-    pub lints: Vec<String>,
+    pub lints: Vec<Lint>,
 }
 
 pub trait Lints {
-    fn lints(&mut self) -> Vec<String>;
+    fn lints(&mut self) -> Vec<Lint>;
 }
 
 impl<T, V> From<V> for LintedItem<T>
diff --git a/crates/spk-schema/src/metadata/meta.rs b/crates/spk-schema/src/metadata/meta.rs
@@ -10,7 +10,7 @@ use spk_config::Metadata;
 use spk_schema_foundation::option_map::Stringified;
 use struct_field_names_as_array::FieldNamesAsArray;
 
-use crate::{Error, LintedItem, Lints, Result, UnknownKey};
+use crate::{Error, Lint, LintedItem, Lints, Result, UnknownKey};
 
 #[cfg(test)]
 #[path = "./meta_test.rs"]
@@ -103,11 +103,11 @@ struct MetaVisitor {
     license: Option<String>,
     labels: Option<BTreeMap<String, String>>,
     #[field_names_as_array(skip)]
-    lints: Vec<String>,
+    lints: Vec<Lint>,
 }
 
 impl Lints for MetaVisitor {
-    fn lints(&mut self) -> Vec<String> {
+    fn lints(&mut self) -> Vec<Lint> {
         std::mem::take(&mut self.lints)
     }
 }
@@ -159,9 +159,10 @@ impl<'de> serde::de::Visitor<'de> for MetaVisitor {
                 "license" => self.license = Some(map.next_value::<Stringified>()?.0),
                 "labels" => self.labels = Some(map.next_value::<BTreeMap<String, String>>()?),
                 unknown_key => {
-                    let lint =
-                        UnknownKey::new(unknown_key, MetaVisitor::FIELD_NAMES_AS_ARRAY.to_vec());
-                    self.lints.push(lint.message.to_string());
+                    self.lints.push(Lint::Key(UnknownKey::new(
+                        unknown_key,
+                        MetaVisitor::FIELD_NAMES_AS_ARRAY.to_vec(),
+                    )));
                     map.next_value::<serde::de::IgnoredAny>()?;
                 }
             }
diff --git a/crates/spk-schema/src/requirements_list.rs b/crates/spk-schema/src/requirements_list.rs
@@ -24,7 +24,7 @@ mod requirements_list_test;
 /// same name, requiring instead that they be combined into a single
 /// request as needed.
 #[derive(Clone, Debug, Default, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)]
-// #[serde(transparent)]
+#[serde(transparent)]
 pub struct RequirementsList(Vec<Request>);
 
 #[derive(Clone, Debug, Default, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)]
diff --git a/crates/spk-schema/src/source_spec.rs b/crates/spk-schema/src/source_spec.rs
@@ -11,7 +11,7 @@ use serde::{Deserialize, Serialize};
 use spk_schema_foundation::option_map::Stringified;
 use struct_field_names_as_array::FieldNamesAsArray;
 
-use crate::{Error, LintedItem, Lints, Result, Script, UnknownKey};
+use crate::{Error, Lint, LintedItem, Lints, Result, Script, UnknownKey};
 
 #[cfg(test)]
 #[path = "./source_spec_test.rs"]
@@ -74,11 +74,11 @@ struct SourceSpecVisitor {
     depth: Option<u32>,
     subdir: Option<String>,
     #[field_names_as_array(skip)]
-    lints: Vec<String>,
+    lints: Vec<Lint>,
 }
 
 impl Lints for SourceSpecVisitor {
-    fn lints(&mut self) -> Vec<String> {
+    fn lints(&mut self) -> Vec<Lint> {
         std::mem::take(&mut self.lints)
     }
 }
@@ -174,11 +174,10 @@ impl<'de> serde::de::Visitor<'de> for SourceSpecVisitor {
                 "depth" => self.depth = Some(map.next_value::<u32>()?),
                 "subdir" => self.subdir = Some(map.next_value::<Stringified>()?.0),
                 unknown_key => {
-                    let lint = UnknownKey::new(
+                    self.lints.push(Lint::Key(UnknownKey::new(
                         unknown_key,
                         SourceSpecVisitor::FIELD_NAMES_AS_ARRAY.to_vec(),
-                    );
-                    self.lints.push(lint.message.to_string());
+                    )));
                     map.next_value::<serde::de::IgnoredAny>()?;
                 }
             }
diff --git a/crates/spk-schema/src/spec.rs b/crates/spk-schema/src/spec.rs
@@ -157,14 +157,7 @@ impl Template for SpecTemplate {
         &self.file_path
     }
 
-    fn render(&self, options: &OptionMap) -> Result<Self::Output> {
-        let data = super::TemplateData::new(options);
-        let rendered = spk_schema_liquid::render_template(&self.template, &data)
-            .map_err(Error::InvalidTemplate)?;
-        Ok(SpecRecipe::from_yaml(rendered)?)
-    }
-
-    fn render_lint(&self, options: &OptionMap) -> Result<String> {
+    fn render_to_string(&self, options: &OptionMap) -> Result<String> {
         let data = super::TemplateData::new(options);
         spk_schema_liquid::render_template(&self.template, &data).map_err(Error::InvalidTemplate)
     }
diff --git a/crates/spk-schema/src/template.rs b/crates/spk-schema/src/template.rs
diff --git a/crates/spk-schema/src/v0/spec.rs b/crates/spk-schema/src/v0/spec.rs
diff --git a/crates/spk-schema/src/v0/test_spec.rs b/crates/spk-schema/src/v0/test_spec.rs

Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize};`
`8`	`8`	`use spk_schema_foundation::option_map::Stringified;`
`9`	`9`	`use struct_field_names_as_array::FieldNamesAsArray;`
`10`	`10`
`11`		`-use crate::{LintedItem, Lints, UnknownKey};`
	`11`	`+use crate::{Lint, LintedItem, Lints, UnknownKey};`
`12`	`12`
`13`	`13`	`#[cfg(test)]`
`14`	`14`	`#[path = "./environ_test.rs"]`
`@@ -191,11 +191,11 @@ struct EnvOpVisitor {`
`191`	`191`	`value: Option<String>,`
`192`	`192`	`separator: Option<String>,`
`193`	`193`	`#[field_names_as_array(skip)]`
`194`		`- lints: Vec<String>,`
	`194`	`+ lints: Vec<Lint>,`
`195`	`195`	`}`
`196`	`196`
`197`	`197`	`impl Lints for EnvOpVisitor {`
`198`		`- fn lints(&mut self) -> Vec<String> {`
	`198`	`+ fn lints(&mut self) -> Vec<Lint> {`
`199`	`199`	`std::mem::take(&mut self.lints)`
`200`	`200`	`}`
`201`	`201`	`}`
`@@ -304,8 +304,8 @@ impl<'de> serde::de::Visitor<'de> for EnvOpVisitor {`
`304`	`304`	`field_names.extend(EnvPriority::FIELD_NAMES_AS_ARRAY.to_vec());`
`305`	`305`	`field_names.extend(PrependEnv::FIELD_NAMES_AS_ARRAY.to_vec());`
`306`	`306`	`field_names.extend(SetEnv::FIELD_NAMES_AS_ARRAY.to_vec());`
`307`		`- let lint = UnknownKey::new(unknown_key, field_names);`
`308`		`- self.lints.push(lint.message.to_string());`
	`307`	`+ self.lints`
	`308`	`+ .push(Lint::Key(UnknownKey::new(unknown_key, field_names)));`
`309`	`309`	`map.next_value::<serde::de::IgnoredAny>()?;`
`310`	`310`	`}`
`311`	`311`	`}`