From 9f5e86a73f15c11049c9dbda5fa449af40afc588 Mon Sep 17 00:00:00 2001 From: Nichol Yip Date: Tue, 22 Aug 2023 14:34:51 -0700 Subject: [PATCH 01/16] WIP: Updating spec deserialize method to handle lints. Signed-off-by: Nichol Yip --- crates/spk-schema/src/v0/spec.rs | 59 ++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/crates/spk-schema/src/v0/spec.rs b/crates/spk-schema/src/v0/spec.rs index 280680f3c..55a6a1146 100644 --- a/crates/spk-schema/src/v0/spec.rs +++ b/crates/spk-schema/src/v0/spec.rs @@ -87,6 +87,12 @@ pub struct Spec { pub install: InstallSpec, } +#[derive(Debug, Clone, Hash, PartialEq, Eq, Ord, PartialOrd, Serialize)] +pub struct LintedSpec { + spec: Spec, + lints: Vec, +} + impl Spec { /// Create an empty spec for the identified package pub fn new(ident: Ident) -> Self { @@ -907,6 +913,57 @@ where } } +impl<'de> Deserialize<'de> for LintedSpec +where + VersionIdent: serde::de::DeserializeOwned, +{ + fn deserialize(deserializer: D) -> std::result::Result + where + D: serde::de::Deserializer<'de>, + { + deserializer.deserialize_map(SpecVisitor::recipe()).into() + } +} + +impl<'de> Deserialize<'de> for LintedSpec +where + AnyIdent: serde::de::DeserializeOwned, +{ + fn deserialize(deserializer: D) -> std::result::Result + where + D: serde::de::Deserializer<'de>, + { + let mut spec: LintedSpec = 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 + // can technically have their own requirements, etc. + spec.spec.prune_for_source_build(); + } + Ok(spec) + } +} + +impl<'de> Deserialize<'de> for LintedSpec +where + BuildIdent: serde::de::DeserializeOwned, +{ + fn deserialize(deserializer: D) -> std::result::Result + where + D: serde::de::Deserializer<'de>, + { + let mut spec: LintedSpec = 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 + // can technically have their own requirements, etc. + spec.spec.prune_for_source_build(); + } + + Ok(spec) + } +} + struct SpecVisitor { pkg: Option>, meta: Option, @@ -917,6 +974,7 @@ struct SpecVisitor { tests: Option>, install: Option, check_build_spec: bool, + lints: Vec, } impl SpecVisitor { @@ -931,6 +989,7 @@ impl SpecVisitor { build: None, tests: None, install: None, + lints: None, check_build_spec, } } From 1b484689775d560a581635b0b076bdd4e2c41fa3 Mon Sep 17 00:00:00 2001 From: Nichol Yip Date: Wed, 23 Aug 2023 16:24:47 -0700 Subject: [PATCH 02/16] Added LintedSpec type to return lint warnings Signed-off-by: Nichol Yip --- crates/spk-schema/src/v0/spec.rs | 150 ++++++++++++++++++++++++------- 1 file changed, 116 insertions(+), 34 deletions(-) diff --git a/crates/spk-schema/src/v0/spec.rs b/crates/spk-schema/src/v0/spec.rs index 55a6a1146..d75732d80 100644 --- a/crates/spk-schema/src/v0/spec.rs +++ b/crates/spk-schema/src/v0/spec.rs @@ -89,8 +89,8 @@ pub struct Spec { #[derive(Debug, Clone, Hash, PartialEq, Eq, Ord, PartialOrd, Serialize)] pub struct LintedSpec { - spec: Spec, - lints: Vec, + pub spec: Spec, + pub lints: Vec, } impl Spec { @@ -870,7 +870,7 @@ where where D: serde::de::Deserializer<'de>, { - deserializer.deserialize_map(SpecVisitor::recipe()) + Ok(std::convert::Into::>::into(deserializer.deserialize_map(SpecVisitor::recipe())?)) } } @@ -882,7 +882,7 @@ where where D: serde::de::Deserializer<'de>, { - let mut spec = deserializer.deserialize_map(SpecVisitor::default())?; + let mut spec: Spec = 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 @@ -901,7 +901,7 @@ where where D: serde::de::Deserializer<'de>, { - let mut spec = deserializer.deserialize_map(SpecVisitor::package())?; + let mut spec: Spec = 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 @@ -921,7 +921,7 @@ where where D: serde::de::Deserializer<'de>, { - deserializer.deserialize_map(SpecVisitor::recipe()).into() + Ok(std::convert::Into::>::into(deserializer.deserialize_map(SpecVisitor::recipe())?)) } } @@ -933,7 +933,7 @@ where where D: serde::de::Deserializer<'de>, { - let mut spec: LintedSpec = deserializer.deserialize_map(SpecVisitor::default()).into(); + let mut spec: LintedSpec = 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 @@ -952,7 +952,7 @@ where where D: serde::de::Deserializer<'de>, { - let mut spec: LintedSpec = deserializer.deserialize_map(SpecVisitor::package()).into(); + let mut spec: LintedSpec = 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 @@ -995,6 +995,75 @@ impl SpecVisitor { } } +impl From> for LintedSpec> +where + Ident: serde::de::DeserializeOwned, +{ + fn from(mut value: SpecVisitor) -> Self { + Self { + spec: Spec { + 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(), + }, + tests: value.tests.take().unwrap_or_default(), + install: value.install.take().unwrap_or_default(), + }, + lints: value.lints + } + } +} + +impl From> for Spec> +where + Ident: serde::de::DeserializeOwned, +{ + fn from(mut value: SpecVisitor) -> 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(), + }, + tests: value.tests.take().unwrap_or_default(), + install: value.install.take().unwrap_or_default(), + } + } +} + impl Default for SpecVisitor { #[inline] fn default() -> Self { @@ -1021,7 +1090,7 @@ impl<'de, B, T> serde::de::Visitor<'de> for SpecVisitor where Ident: Named + serde::de::DeserializeOwned, { - type Value = Spec>; + type Value = SpecVisitor; fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { f.write_str("a package specification") @@ -1041,7 +1110,8 @@ where "build" => self.build = Some(map.next_value::()?), "tests" => self.tests = Some(map.next_value::>()?), "install" => self.install = Some(map.next_value::()?), - _ => { + 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::()?; @@ -1049,29 +1119,41 @@ where } } - 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, + // }) } } + From 817199447235714cba58a4fa6dfba1dd0c42c46d Mon Sep 17 00:00:00 2001 From: Nichol Yip Date: Thu, 24 Aug 2023 14:00:00 -0700 Subject: [PATCH 03/16] Implemented lint for the top level of the spec config Signed-off-by: Nichol Yip --- Cargo.lock | 7 +++ crates/spk-cli/group4/src/cmd_lint.rs | 24 +++++++- crates/spk-schema/Cargo.toml | 1 + crates/spk-schema/src/spec.rs | 31 +++++++++- crates/spk-schema/src/v0/mod.rs | 1 + crates/spk-schema/src/v0/spec.rs | 85 +++++++++++++++++---------- 6 files changed, 114 insertions(+), 35 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 10e357aad..e9810afb2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2073,6 +2073,12 @@ dependencies = [ "tempfile", ] +[[package]] +name = "ngrammatic" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3c6f2f987e82da7fb8a290e959bba528638bc0e2629e38647591e845ecb5f6fe" + [[package]] name = "nix" version = "0.27.1" @@ -4353,6 +4359,7 @@ dependencies = [ "indexmap 2.2.3", "itertools 0.12.0", "miette", + "ngrammatic", "nom", "proptest", "regex", diff --git a/crates/spk-cli/group4/src/cmd_lint.rs b/crates/spk-cli/group4/src/cmd_lint.rs index 37aeca5c0..71977d62c 100644 --- a/crates/spk-cli/group4/src/cmd_lint.rs +++ b/crates/spk-cli/group4/src/cmd_lint.rs @@ -8,7 +8,8 @@ use clap::Args; use colored::Colorize; use miette::Result; use spk_cli_common::{flags, CommandArgs, Run}; -use spk_schema::{SpecTemplate, Template, TemplateExt}; +use spk_schema::v0::LintedSpec; +use spk_schema::{AnyIdent, Error}; /// Validate spk yaml files #[derive(Args)] @@ -28,9 +29,26 @@ impl Run for Lint { let options = self.options.get_options()?; let mut out = 0; for spec in self.packages.iter() { - let result = SpecTemplate::from_file(spec).and_then(|t| t.render(&options)); + let file_path = spec + .canonicalize() + .map_err(|err| Error::InvalidPath(spec.to_owned(), err))?; + let file = std::fs::File::open(&file_path) + .map_err(|err| Error::FileOpenError(file_path.to_owned(), err))?; + let rdr = std::io::BufReader::new(file); + + let result: std::result::Result, serde_yaml::Error> = + serde_yaml::from_reader(rdr); + match result { - Ok(_) => println!("{} {}", "OK".green(), spec.display()), + Ok(s) => match s.lints.is_empty() { + true => println!("{} {}", "OK".green(), spec.display()), + false => { + for lint in s.lints { + tracing::error!(lint); + } + out = 1; + } + }, Err(err) => { println!( "{} {}:\n{} {err}", diff --git a/crates/spk-schema/Cargo.toml b/crates/spk-schema/Cargo.toml index 2144874b6..fd55528ee 100644 --- a/crates/spk-schema/Cargo.toml +++ b/crates/spk-schema/Cargo.toml @@ -27,6 +27,7 @@ format_serde_error = { version = "0.3", default-features = false, features = [ ignore = "0.4.18" indexmap = { workspace = true } itertools = { workspace = true } +ngrammatic = "0.4.0" nom = { workspace = true } regex = { workspace = true } relative-path = { workspace = true } diff --git a/crates/spk-schema/src/spec.rs b/crates/spk-schema/src/spec.rs index 20c0bcc93..3a20cfb75 100644 --- a/crates/spk-schema/src/spec.rs +++ b/crates/spk-schema/src/spec.rs @@ -170,6 +170,13 @@ impl Template for SpecTemplate { .map_err(Error::InvalidTemplate)?; Ok(SpecRecipe::from_yaml(rendered)?) } + + // fn render_lint(&self, options: &OptionMap) -> Result { + // 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)?) + // } } impl TemplateExt for SpecTemplate { @@ -237,6 +244,29 @@ impl TemplateExt for SpecTemplate { } } +// #[derive(Debug, Clone, Hash, Eq, PartialEq, Serialize)] +// // #[enum_dispatch(Deprecate, DeprecateMut)] +// pub enum SpecRecipeKind { +// DefaultSpec(super::v0::Spec), +// LintSpec(super::v0::LintedSpec) +// } + +// impl SpecRecipeKind { +// pub fn spec(&self) -> &super::v0::Spec { +// match self { +// SpecRecipeKind::DefaultSpec(s) => s, +// SpecRecipeKind::LintSpec(lint) => &lint.spec, +// } +// } + +// pub fn lints(&self) -> Vec { +// match self { +// SpecRecipeKind::DefaultSpec(_) => Vec::default(), +// SpecRecipeKind::LintSpec(lint) => lint.lints.clone(), +// } +// } +// } + /// Specifies some buildable object within the spk ecosystem. /// /// All build-able types have a recipe representation @@ -434,7 +464,6 @@ impl FromYaml for SpecRecipe { } Ok(m) => m, }; - match with_version.api { ApiVersion::V0Package => { let inner = serde_yaml::from_str(&yaml) diff --git a/crates/spk-schema/src/v0/mod.rs b/crates/spk-schema/src/v0/mod.rs index ba52ff8e4..6eb8acc83 100644 --- a/crates/spk-schema/src/v0/mod.rs +++ b/crates/spk-schema/src/v0/mod.rs @@ -10,6 +10,7 @@ mod variant_spec; pub use platform::Platform; pub use spec::Spec; +pub use spec::{LintedSpec, Spec}; pub use test_spec::TestSpec; pub use variant::Variant; pub use variant_spec::VariantSpec; diff --git a/crates/spk-schema/src/v0/spec.rs b/crates/spk-schema/src/v0/spec.rs index d75732d80..8d30f1ec2 100644 --- a/crates/spk-schema/src/v0/spec.rs +++ b/crates/spk-schema/src/v0/spec.rs @@ -9,6 +9,7 @@ use std::path::Path; use std::str::FromStr; use itertools::Itertools; +use ngrammatic::CorpusBuilder; use serde::{Deserialize, Serialize}; use spk_schema_foundation::ident_build::BuildId; use spk_schema_foundation::ident_component::ComponentBTreeSet; @@ -870,7 +871,9 @@ where where D: serde::de::Deserializer<'de>, { - Ok(std::convert::Into::>::into(deserializer.deserialize_map(SpecVisitor::recipe())?)) + Ok(std::convert::Into::>::into( + deserializer.deserialize_map(SpecVisitor::recipe())?, + )) } } @@ -901,7 +904,8 @@ where where D: serde::de::Deserializer<'de>, { - let mut spec: Spec = deserializer.deserialize_map(SpecVisitor::package())?.into(); + let mut spec: Spec = + 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 @@ -921,7 +925,9 @@ where where D: serde::de::Deserializer<'de>, { - Ok(std::convert::Into::>::into(deserializer.deserialize_map(SpecVisitor::recipe())?)) + Ok(std::convert::Into::>::into( + deserializer.deserialize_map(SpecVisitor::recipe())?, + )) } } @@ -933,7 +939,8 @@ where where D: serde::de::Deserializer<'de>, { - let mut spec: LintedSpec = deserializer.deserialize_map(SpecVisitor::default())?.into(); + let mut spec: LintedSpec = + 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 @@ -952,7 +959,8 @@ where where D: serde::de::Deserializer<'de>, { - let mut spec: LintedSpec = deserializer.deserialize_map(SpecVisitor::package())?.into(); + let mut spec: LintedSpec = + 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 @@ -999,7 +1007,7 @@ impl From> for LintedSpec> where Ident: serde::de::DeserializeOwned, { - fn from(mut value: SpecVisitor) -> Self { + fn from(mut value: SpecVisitor) -> Self { Self { spec: Spec { pkg: value.pkg.expect("Missing field: pkg"), @@ -1015,18 +1023,16 @@ where // 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(), - } + Some(build_spec) => match build_spec.try_into() { + Ok(b) => b, + Err(_) => BuildSpec::default(), }, None => Default::default(), }, tests: value.tests.take().unwrap_or_default(), install: value.install.take().unwrap_or_default(), }, - lints: value.lints + lints: value.lints, } } } @@ -1035,7 +1041,7 @@ impl From> for Spec> where Ident: serde::de::DeserializeOwned, { - fn from(mut value: SpecVisitor) -> Self { + fn from(mut value: SpecVisitor) -> Self { Self { pkg: value.pkg.expect("Missing field pkg"), meta: value.meta.take().unwrap_or_default(), @@ -1050,11 +1056,9 @@ where // 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(), - } + Some(build_spec) => match build_spec.try_into() { + Ok(b) => b, + Err(_) => BuildSpec::default(), }, None => Default::default(), }, @@ -1110,8 +1114,33 @@ where "build" => self.build = Some(map.next_value::()?), "tests" => self.tests = Some(map.next_value::>()?), "install" => self.install = Some(map.next_value::()?), - unrecognized_string => { - self.lints.push(format!("unrecognized string {unrecognized_string}")); + "api" => { + map.next_value::()?; + } + unknown_config => { + self.lints.push(format!("Unknown config: {unknown_config}")); + let mut corpus = CorpusBuilder::new().finish(); + + corpus.add_text("pkg"); + corpus.add_text("meta"); + corpus.add_text("compat"); + corpus.add_text("deprecated"); + corpus.add_text("sources"); + corpus.add_text("build"); + corpus.add_text("tests"); + corpus.add_text("install"); + corpus.add_text("api"); + + 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, + }; + + 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::()?; @@ -1119,16 +1148,11 @@ where } } - 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()) + self.pkg + .as_ref() + .ok_or_else(|| serde::de::Error::missing_field("pkg"))?; + + Ok(self) // let pkg = self // .pkg @@ -1156,4 +1180,3 @@ where // }) } } - From 29d710bc9956d5203dc9dc03ba6308583e68917c Mon Sep 17 00:00:00 2001 From: Nichol Yip Date: Tue, 29 Aug 2023 11:08:15 -0700 Subject: [PATCH 04/16] Added lints for InstallSpec Signed-off-by: Nichol Yip Fixed error message when no similar configs are found. Added linting for EnvOp Signed-off-by: Nichol Yip Removed redudancy and simplified code. WIP: Generalizing LintedItem struct across all config types. Signed-off-by: Nichol Yip Added blanket implementation for LintedItem to generalize lints across all configs Signed-off-by: Nichol Yip Moved lint logic into separate file. Signed-off-by: Nichol Yip WIP: enum for lint messages Signed-off-by: Nichol Yip Moved lint error message inside new lint file. Fixed error message when unknown key is found. Implemented LintMessage enum for current variantions of lint error types. Signed-off-by: Nichol Yip --- crates/spk-cli/group4/src/cmd_lint.rs | 9 +- crates/spk-schema/src/environ.rs | 39 +++++-- crates/spk-schema/src/install_spec.rs | 125 ++++++++++++++++----- crates/spk-schema/src/lib.rs | 2 + crates/spk-schema/src/lints.rs | 112 ++++++++++++++++++ crates/spk-schema/src/requirements_list.rs | 6 + crates/spk-schema/src/v0/spec.rs | 104 +++++------------ 7 files changed, 276 insertions(+), 121 deletions(-) create mode 100644 crates/spk-schema/src/lints.rs diff --git a/crates/spk-cli/group4/src/cmd_lint.rs b/crates/spk-cli/group4/src/cmd_lint.rs index 71977d62c..4dac93845 100644 --- a/crates/spk-cli/group4/src/cmd_lint.rs +++ b/crates/spk-cli/group4/src/cmd_lint.rs @@ -8,8 +8,8 @@ use clap::Args; use colored::Colorize; use miette::Result; use spk_cli_common::{flags, CommandArgs, Run}; -use spk_schema::v0::LintedSpec; -use spk_schema::{AnyIdent, Error}; +use spk_schema::v0::Spec; +use spk_schema::{AnyIdent, Error, LintedItem}; /// Validate spk yaml files #[derive(Args)] @@ -36,15 +36,16 @@ impl Run for Lint { .map_err(|err| Error::FileOpenError(file_path.to_owned(), err))?; let rdr = std::io::BufReader::new(file); - let result: std::result::Result, serde_yaml::Error> = + let result: std::result::Result>, serde_yaml::Error> = serde_yaml::from_reader(rdr); match result { 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!("{} {}", "----->".red(), lint.message()); } out = 1; } diff --git a/crates/spk-schema/src/environ.rs b/crates/spk-schema/src/environ.rs index 74515e25c..c8d91155e 100644 --- a/crates/spk-schema/src/environ.rs +++ b/crates/spk-schema/src/environ.rs @@ -4,6 +4,7 @@ use std::collections::HashMap; +use ngrammatic::CorpusBuilder; use serde::{Deserialize, Serialize}; use spk_schema_foundation::option_map::Stringified; @@ -21,7 +22,7 @@ const OP_COMMENT: &str = "comment"; const OP_PREPEND: &str = "prepend"; const OP_PRIORITY: &str = "priority"; const OP_SET: &str = "set"; -const OP_NAMES: &[&str] = &[OP_APPEND, OP_COMMENT, OP_PREPEND, OP_SET]; +const OP_NAMES: &[&str] = &[OP_APPEND, OP_COMMENT, OP_PREPEND, OP_SET, OP_PRIORITY]; /// The set of operation types for use in deserialization #[derive(Copy, Clone, Debug, PartialEq, strum::Display)] @@ -182,6 +183,7 @@ impl<'de> Deserialize<'de> for EnvOp { op_and_var: Option<(OpKind, ConfKind)>, value: Option, separator: Option, + lints: Vec, } impl<'de> serde::de::Visitor<'de> for EnvOpVisitor { @@ -256,7 +258,24 @@ impl<'de> Deserialize<'de> for EnvOp { "separator" => { self.separator = map.next_value::>()?.map(|s| s.0) } - _ => { + unknown_config => { + self.lints + .push(format!("Unknown config: {unknown_config}. ")); + let mut corpus = CorpusBuilder::new().finish(); + for op_name in OP_NAMES.iter() { + corpus.add_text(op_name); + } + + match corpus.search(unknown_config, 0.6).first() { + Some(s) => self + .lints + .push(format!("The most similar config is: {}", s.text)), + None => self.lints.push(format!( + "No similar config found for: {}.", + unknown_config + )), + }; + // ignore any unknown field for the sake of // forward compatibility map.next_value::()?; @@ -280,17 +299,17 @@ impl<'de> Deserialize<'de> for EnvOp { match self.op_and_var.take() { Some((op, var)) => match op { - OpKind::Prepend => Ok(EnvOp::Prepend(PrependEnv{ + OpKind::Prepend => Ok(EnvOp::Prepend(PrependEnv { prepend: var.get_op(), separator: self.separator.take(), value: value.unwrap_or_default() })), - OpKind::Append => Ok(EnvOp::Append(AppendEnv{ + OpKind::Append => Ok(EnvOp::Append(AppendEnv { append: var.get_op(), separator: self.separator.take(), value: value.unwrap_or_default() })), - OpKind::Set => Ok(EnvOp::Set(SetEnv{ + OpKind::Set => Ok(EnvOp::Set(SetEnv { set: var.get_op(), value: value.unwrap_or_default() })), @@ -301,9 +320,13 @@ impl<'de> Deserialize<'de> for EnvOp { priority: var.get_priority() })) }, - None => Err(serde::de::Error::custom(format!( - "missing field to define operation and variable, expected one of {OP_NAMES:?}", - ))), + None => { + let mut err_msg = String::from(""); + for lint in self.lints.iter() { + err_msg = err_msg + lint; + } + Err(serde::de::Error::custom(err_msg)) + } } } } diff --git a/crates/spk-schema/src/install_spec.rs b/crates/spk-schema/src/install_spec.rs index 7c3e5807e..c56c619a6 100644 --- a/crates/spk-schema/src/install_spec.rs +++ b/crates/spk-schema/src/install_spec.rs @@ -1,20 +1,22 @@ // Copyright (c) Contributors to the SPK project. // SPDX-License-Identifier: Apache-2.0 // https://github.com/spkenv/spk +use std::marker::PhantomData; use serde::{Deserialize, Serialize}; +use spk_schema_foundation::option_map::Stringified; use spk_schema_ident::BuildIdent; use super::{ComponentSpecList, EmbeddedPackagesList, EnvOp, OpKind, RequirementsList}; use crate::foundation::option_map::OptionMap; -use crate::Result; +use crate::{InstallSpecKey, LintMessage, LintedItem, Lints, Result}; #[cfg(test)] #[path = "./install_spec_test.rs"] 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, @@ -22,14 +24,46 @@ pub struct InstallSpec { 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, } +impl Lints for InstallSpecVisitor +where + D: Default, +{ + fn lints(&mut self) -> Vec { + std::mem::take(&mut self.lints) + } +} + +#[derive(Default)] +struct InstallSpecVisitor +where + D: Default, +{ + requirements: RequirementsList, + embedded: EmbeddedPackagesList, + components: ComponentSpecList, + environment: Vec, + lints: Vec, + _phantom: PhantomData, +} + +impl From> for InstallSpec +where + D: Default, +{ + fn from(value: InstallSpecVisitor) -> Self { + Self { + requirements: value.requirements, + embedded: value.embedded, + components: value.components, + environment: value.environment, + } + } +} + impl InstallSpec { pub fn is_default(&self) -> bool { self.requirements.is_empty() && self.embedded.is_empty() && self.components.is_default() @@ -53,37 +87,66 @@ impl InstallSpec { } } -fn deserialize_env_conf<'de, D>(deserializer: D) -> std::result::Result, D::Error> +impl<'de> Deserialize<'de> for InstallSpec { + fn deserialize(deserializer: D) -> std::result::Result + where + D: serde::de::Deserializer<'de>, + { + deserializer.deserialize_map(InstallSpecVisitor::::default()) + } +} + +impl<'de> Deserialize<'de> for LintedItem { + fn deserialize(deserializer: D) -> std::result::Result + where + D: serde::de::Deserializer<'de>, + { + deserializer.deserialize_map(InstallSpecVisitor::>::default()) + } +} + +impl<'de, D> serde::de::Visitor<'de> for InstallSpecVisitor where - D: serde::Deserializer<'de>, + D: Default + From>, { - struct EnvConfVisitor; + type Value = D; - impl<'de> serde::de::Visitor<'de> for EnvConfVisitor { - type Value = Vec; + fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + f.write_str("a package specification") + } - fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - f.write_str("an environment configuration") + fn visit_map(mut self, mut map: A) -> std::result::Result + where + A: serde::de::MapAccess<'de>, + { + while let Some(key) = map.next_key::()? { + match key.as_str() { + "requirements" => self.requirements = map.next_value::()?, + "embedded" => self.embedded = map.next_value::()?, + "components" => self.components = map.next_value::()?, + "environment" => self.environment = map.next_value::>()?, + unknown_config => { + self.lints + .push(LintMessage::UnknownInstallSpecKey(InstallSpecKey::new( + unknown_config, + ))); + map.next_value::()?; + } + } } - fn visit_seq(self, mut seq: A) -> std::result::Result - 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::()? { - 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) } diff --git a/crates/spk-schema/src/lib.rs b/crates/spk-schema/src/lib.rs index 6f00bf873..1e337786e 100644 --- a/crates/spk-schema/src/lib.rs +++ b/crates/spk-schema/src/lib.rs @@ -12,6 +12,7 @@ mod error; mod input_variant; mod install_spec; mod metadata; +mod lints; mod option; mod package; pub mod prelude; @@ -34,6 +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::{InstallSpecKey, LintKind, LintMessage, LintedItem, Lints, V0SpecKey}; 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 new file mode 100644 index 000000000..7a587a019 --- /dev/null +++ b/crates/spk-schema/src/lints.rs @@ -0,0 +1,112 @@ +// Copyright (c) Sony Pictures Imageworks, et al. +// SPDX-License-Identifier: Apache-2.0 +// https://github.com/imageworks/spk +use ngrammatic::CorpusBuilder; +use serde::{Deserialize, Serialize}; + +#[derive(Debug, Clone, Hash, PartialEq, Eq, Ord, PartialOrd, Deserialize, Serialize)] +pub enum LintKind { + UnknownV0SpecKey, + UnknownInstallSpecKey, +} + +#[derive(Debug, Clone, Hash, PartialEq, Eq, Ord, PartialOrd, Deserialize, Serialize)] +pub enum LintMessage { + UnknownV0SpecKey(V0SpecKey), + UnknownInstallSpecKey(InstallSpecKey), +} + +impl LintMessage { + pub fn message(&self) -> String { + match self { + Self::UnknownV0SpecKey(key) => key.message.clone(), + Self::UnknownInstallSpecKey(key) => key.message.clone(), + } + } +} + +#[derive(Debug, Default, Clone, Hash, PartialEq, Eq, Ord, PartialOrd, Deserialize, Serialize)] +pub struct V0SpecKey { + key: String, + message: String, +} + +impl V0SpecKey { + pub fn new(unknown_key: &str) -> Self { + let mut message = format!("Unrecognized V0 Spec key: {unknown_key}. "); + let mut corpus = CorpusBuilder::new().finish(); + + corpus.add_text("pkg"); + corpus.add_text("meta"); + corpus.add_text("compat"); + corpus.add_text("deprecated"); + corpus.add_text("sources"); + corpus.add_text("build"); + corpus.add_text("tests"); + corpus.add_text("install"); + corpus.add_text("api"); + + match corpus.search(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()) + } + }; + + Self { + key: std::mem::take(&mut unknown_key.to_string()), + message: message.to_string(), + } + } +} + +#[derive(Debug, Default, Clone, Hash, PartialEq, Eq, Ord, PartialOrd, Deserialize, Serialize)] +pub struct InstallSpecKey { + key: String, + message: String, +} + +impl InstallSpecKey { + pub fn new(unknown_key: &str) -> Self { + let mut message = format!("Unrecognized InstallSpec key: {unknown_key}. "); + let mut corpus = CorpusBuilder::new().finish(); + + corpus.add_text("requirements"); + corpus.add_text("embedded"); + corpus.add_text("components"); + corpus.add_text("environment"); + + match corpus.search(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()) + } + }; + Self { + key: std::mem::take(&mut unknown_key.to_string()), + message: message.to_string(), + } + } +} + +#[derive(Debug, Default, Clone, Hash, PartialEq, Eq, Ord, PartialOrd, Serialize)] +pub struct LintedItem { + pub item: T, + pub lints: Vec, +} + +pub trait Lints { + fn lints(&mut self) -> Vec; +} + +impl From for LintedItem +where + V: Lints + Into, +{ + fn from(mut value: V) -> Self { + Self { + lints: value.lints(), + item: value.into(), + } + } +} diff --git a/crates/spk-schema/src/requirements_list.rs b/crates/spk-schema/src/requirements_list.rs index 8dad7595c..8ed9a6e21 100644 --- a/crates/spk-schema/src/requirements_list.rs +++ b/crates/spk-schema/src/requirements_list.rs @@ -27,6 +27,12 @@ mod requirements_list_test; #[serde(transparent)] pub struct RequirementsList(Vec); +#[derive(Clone, Debug, Default, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] +pub struct LintedRequirementsList { + pub requirements: Vec, + pub lints: Vec, +} + impl std::ops::Deref for RequirementsList { type Target = Vec; fn deref(&self) -> &Self::Target { diff --git a/crates/spk-schema/src/v0/spec.rs b/crates/spk-schema/src/v0/spec.rs index 8d30f1ec2..c94035723 100644 --- a/crates/spk-schema/src/v0/spec.rs +++ b/crates/spk-schema/src/v0/spec.rs @@ -9,7 +9,6 @@ use std::path::Path; use std::str::FromStr; use itertools::Itertools; -use ngrammatic::CorpusBuilder; use serde::{Deserialize, Serialize}; use spk_schema_foundation::ident_build::BuildId; use spk_schema_foundation::ident_component::ComponentBTreeSet; @@ -52,6 +51,9 @@ use crate::{ Inheritance, InputVariant, InstallSpec, + LintMessage, + LintedItem, + Lints, LocalSource, Opt, Package, @@ -61,6 +63,7 @@ use crate::{ Result, SourceSpec, TestStage, + V0SpecKey, ValidationSpec, Variant, }; @@ -88,12 +91,6 @@ pub struct Spec { pub install: InstallSpec, } -#[derive(Debug, Clone, Hash, PartialEq, Eq, Ord, PartialOrd, Serialize)] -pub struct LintedSpec { - pub spec: Spec, - pub lints: Vec, -} - impl Spec { /// Create an empty spec for the identified package pub fn new(ident: Ident) -> Self { @@ -917,7 +914,7 @@ where } } -impl<'de> Deserialize<'de> for LintedSpec +impl<'de> Deserialize<'de> for LintedItem> where VersionIdent: serde::de::DeserializeOwned, { @@ -925,13 +922,11 @@ where where D: serde::de::Deserializer<'de>, { - Ok(std::convert::Into::>::into( - deserializer.deserialize_map(SpecVisitor::recipe())?, - )) + Ok(deserializer.deserialize_map(SpecVisitor::recipe())?.into()) } } -impl<'de> Deserialize<'de> for LintedSpec +impl<'de> Deserialize<'de> for LintedItem> where AnyIdent: serde::de::DeserializeOwned, { @@ -939,19 +934,19 @@ where where D: serde::de::Deserializer<'de>, { - let mut spec: LintedSpec = + let mut spec: LintedItem> = deserializer.deserialize_map(SpecVisitor::default())?.into(); - if spec.spec.pkg.is_source() { + if spec.item.pkg.is_source() { // for backward-compatibility with older publishes, prune out anything // that is not relevant to a source package, since now source packages // can technically have their own requirements, etc. - spec.spec.prune_for_source_build(); + spec.item.prune_for_source_build(); } Ok(spec) } } -impl<'de> Deserialize<'de> for LintedSpec +impl<'de> Deserialize<'de> for LintedItem> where BuildIdent: serde::de::DeserializeOwned, { @@ -959,13 +954,13 @@ where where D: serde::de::Deserializer<'de>, { - let mut spec: LintedSpec = + let mut spec: LintedItem> = deserializer.deserialize_map(SpecVisitor::package())?.into(); - if spec.spec.pkg.is_source() { + if spec.item.pkg.is_source() { // for backward-compatibility with older publishes, prune out anything // that is not relevant to a source package, since now source packages // can technically have their own requirements, etc. - spec.spec.prune_for_source_build(); + spec.item.prune_for_source_build(); } Ok(spec) @@ -980,9 +975,9 @@ struct SpecVisitor { sources: Option>, build: Option, tests: Option>, - install: Option, + install: Option>, check_build_spec: bool, - lints: Vec, + lints: Vec, } impl SpecVisitor { @@ -1003,37 +998,12 @@ impl SpecVisitor { } } -impl From> for LintedSpec> -where - Ident: serde::de::DeserializeOwned, -{ - fn from(mut value: SpecVisitor) -> Self { - Self { - spec: Spec { - 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(), - }, - tests: value.tests.take().unwrap_or_default(), - install: value.install.take().unwrap_or_default(), - }, - lints: value.lints, +impl Lints for SpecVisitor { + fn lints(&mut self) -> Vec { + if let Some(l) = self.install.as_mut() { + self.lints.extend(std::mem::take(&mut l.lints)); } + std::mem::take(&mut self.lints) } } @@ -1063,7 +1033,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().item, } } } @@ -1113,36 +1083,15 @@ where "sources" => self.sources = Some(map.next_value::>()?), "build" => self.build = Some(map.next_value::()?), "tests" => self.tests = Some(map.next_value::>()?), - "install" => self.install = Some(map.next_value::()?), + "install" => self.install = Some(map.next_value::>()?), "api" => { map.next_value::()?; } unknown_config => { - self.lints.push(format!("Unknown config: {unknown_config}")); - let mut corpus = CorpusBuilder::new().finish(); - - corpus.add_text("pkg"); - corpus.add_text("meta"); - corpus.add_text("compat"); - corpus.add_text("deprecated"); - corpus.add_text("sources"); - corpus.add_text("build"); - corpus.add_text("tests"); - corpus.add_text("install"); - corpus.add_text("api"); - - 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, - }; - 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? + .push(LintMessage::UnknownV0SpecKey(V0SpecKey::new( + unknown_config, + ))); map.next_value::()?; } } @@ -1153,7 +1102,6 @@ where .ok_or_else(|| serde::de::Error::missing_field("pkg"))?; Ok(self) - // let pkg = self // .pkg // .take() From 85d4aa5207a551334fa4d3e9c7c15fc3097ac13f Mon Sep 17 00:00:00 2001 From: Nichol Yip Date: Wed, 6 Sep 2023 16:19:53 -0700 Subject: [PATCH 05/16] Implemented lint feature for EnvOp Signed-off-by: Nichol Yip --- crates/spk-schema/src/environ.rs | 361 ++++++++++++++------------ crates/spk-schema/src/install_spec.rs | 16 +- crates/spk-schema/src/lib.rs | 2 +- crates/spk-schema/src/lints.rs | 33 +++ 4 files changed, 240 insertions(+), 172 deletions(-) diff --git a/crates/spk-schema/src/environ.rs b/crates/spk-schema/src/environ.rs index c8d91155e..cd856b5b3 100644 --- a/crates/spk-schema/src/environ.rs +++ b/crates/spk-schema/src/environ.rs @@ -8,6 +8,8 @@ use ngrammatic::CorpusBuilder; use serde::{Deserialize, Serialize}; use spk_schema_foundation::option_map::Stringified; +use crate::{EnvOpKey, LintMessage, LintedItem, Lints}; + #[cfg(test)] #[path = "./environ_test.rs"] mod environ_test; @@ -22,7 +24,6 @@ const OP_COMMENT: &str = "comment"; const OP_PREPEND: &str = "prepend"; const OP_PRIORITY: &str = "priority"; const OP_SET: &str = "set"; -const OP_NAMES: &[&str] = &[OP_APPEND, OP_COMMENT, OP_PREPEND, OP_SET, OP_PRIORITY]; /// The set of operation types for use in deserialization #[derive(Copy, Clone, Debug, PartialEq, strum::Display)] @@ -32,6 +33,7 @@ pub enum OpKind { Comment, Prepend, Priority, + NoOp, Set, } @@ -44,6 +46,7 @@ pub enum EnvOp { Prepend(PrependEnv), Priority(EnvPriority), Set(SetEnv), + NoOp(NoOp), } impl EnvOp { @@ -54,6 +57,7 @@ impl EnvOp { EnvOp::Prepend(_) => OpKind::Prepend, EnvOp::Priority(_) => OpKind::Priority, EnvOp::Set(_) => OpKind::Set, + EnvOp::NoOp(_) => OpKind::NoOp, } } @@ -131,6 +135,7 @@ impl EnvOp { Self::Prepend(op) => op.bash_source(), Self::Priority(op) => op.bash_source(), Self::Set(op) => op.bash_source(), + Self::NoOp(op) => op.bash_source(), } } @@ -142,6 +147,7 @@ impl EnvOp { Self::Prepend(op) => op.tcsh_source(), Self::Priority(op) => op.tcsh_source(), Self::Set(op) => op.tcsh_source(), + Self::NoOp(op) => op.tcsh_source(), } } @@ -151,186 +157,193 @@ impl EnvOp { } } -impl<'de> Deserialize<'de> for EnvOp { - fn deserialize(deserializer: D) -> std::result::Result - where - D: serde::Deserializer<'de>, - { - #[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize, Deserialize)] - enum ConfKind { - Priority(u8), - Operation(String), - } +#[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize, Deserialize)] +enum ConfKind { + Priority(u8), + Operation(String), +} - impl ConfKind { - pub fn get_op(&self) -> String { - match self { - ConfKind::Priority(_) => String::from(""), - ConfKind::Operation(o) => o.clone(), - } - } +impl ConfKind { + pub fn get_op(&self) -> String { + match self { + ConfKind::Priority(_) => String::from(""), + ConfKind::Operation(o) => o.clone(), + } + } - pub fn get_priority(&self) -> u8 { - match self { - ConfKind::Priority(p) => *p, - ConfKind::Operation(_) => 0, - } - } + pub fn get_priority(&self) -> u8 { + match self { + ConfKind::Priority(p) => *p, + ConfKind::Operation(_) => 0, } + } +} + +#[derive(Default, Debug)] +struct EnvOpVisitor { + op_and_var: Option<(OpKind, ConfKind)>, + value: Option, + separator: Option, + lints: Vec, +} + +impl Lints for EnvOpVisitor { + fn lints(&mut self) -> Vec { + std::mem::take(&mut self.lints) + } +} - #[derive(Default)] - struct EnvOpVisitor { - op_and_var: Option<(OpKind, ConfKind)>, - value: Option, - separator: Option, - lints: Vec, +impl From for EnvOp { + fn from(mut value: EnvOpVisitor) -> Self { + match value.op_and_var.take() { + Some((op, var)) => match op { + OpKind::Prepend => EnvOp::Prepend(PrependEnv { + prepend: var.get_op(), + separator: value.separator.take(), + value: value.value.expect("an environment value"), + }), + OpKind::Append => EnvOp::Append(AppendEnv { + append: var.get_op(), + separator: value.separator.take(), + value: value.value.expect("an environment value"), + }), + OpKind::Set => EnvOp::Set(SetEnv { + set: var.get_op(), + value: value.value.expect("an environment value"), + }), + OpKind::Comment => EnvOp::Comment(CommentEnv { + comment: var.get_op(), + }), + OpKind::Priority => EnvOp::Priority(Priority { + priority: var.get_priority(), + }), + OpKind::NoOp => EnvOp::NoOp(NoOp), + }, + None => EnvOp::NoOp(NoOp), } + } +} - impl<'de> serde::de::Visitor<'de> for EnvOpVisitor { - type Value = EnvOp; +impl<'de> Deserialize<'de> for EnvOp { + fn deserialize(deserializer: D) -> std::result::Result + where + D: serde::de::Deserializer<'de>, + { + Ok(deserializer + .deserialize_map(EnvOpVisitor::default())? + .into()) + } +} - fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - f.write_str("an environment operation") - } +impl<'de> Deserialize<'de> for LintedItem { + fn deserialize(deserializer: D) -> std::result::Result + where + D: serde::de::Deserializer<'de>, + { + Ok(deserializer + .deserialize_map(EnvOpVisitor::default())? + .into()) + } +} + +impl<'de> serde::de::Visitor<'de> for EnvOpVisitor { + type Value = EnvOpVisitor; + + fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + f.write_str("an environment operation") + } - fn visit_map(mut self, mut map: A) -> std::result::Result - where - A: serde::de::MapAccess<'de>, - { - while let Some(key) = map.next_key::()? { - match key.as_str() { - OP_PREPEND => { - if let Some((existing_op, _)) = &self.op_and_var { - return Err(serde::de::Error::custom(format!( - "encountered {key} but operation already defined as {existing_op}", - ))); - } - self.op_and_var = Some(( - OpKind::Prepend, - ConfKind::Operation(map.next_value::()?.0), - )); - } - OP_PRIORITY => { - if let Some((existing_op, _)) = &self.op_and_var { - return Err(serde::de::Error::custom(format!( - "encountered {key} but operation already defined as {existing_op}", - ))); - } - self.op_and_var = Some(( - OpKind::Priority, - ConfKind::Priority(map.next_value::()?), - )); - } - OP_COMMENT => { - if let Some((existing_op, _)) = &self.op_and_var { - return Err(serde::de::Error::custom(format!( - "encountered {key} but operation already defined as {existing_op}", - ))); - } - self.op_and_var = Some(( - OpKind::Comment, - ConfKind::Operation(map.next_value::()?.0), - )); - } - OP_APPEND => { - if let Some((existing_op, _)) = &self.op_and_var { - return Err(serde::de::Error::custom(format!( - "encountered {key} but operation already defined as {existing_op}", - ))); - } - self.op_and_var = Some(( - OpKind::Append, - ConfKind::Operation(map.next_value::()?.0), - )); - } - OP_SET => { - if let Some((existing_op, _)) = &self.op_and_var { - return Err(serde::de::Error::custom(format!( - "encountered {key} but operation already defined as {existing_op}", - ))); - } - self.op_and_var = Some(( - OpKind::Set, - ConfKind::Operation(map.next_value::()?.0), - )); - } - "value" => self.value = Some(map.next_value::()?.0), - "separator" => { - self.separator = map.next_value::>()?.map(|s| s.0) - } - unknown_config => { - self.lints - .push(format!("Unknown config: {unknown_config}. ")); - let mut corpus = CorpusBuilder::new().finish(); - for op_name in OP_NAMES.iter() { - corpus.add_text(op_name); - } - - match corpus.search(unknown_config, 0.6).first() { - Some(s) => self - .lints - .push(format!("The most similar config is: {}", s.text)), - None => self.lints.push(format!( - "No similar config found for: {}.", - unknown_config - )), - }; - - // ignore any unknown field for the sake of - // forward compatibility - map.next_value::()?; - } + fn visit_map(mut self, mut map: A) -> std::result::Result + where + A: serde::de::MapAccess<'de>, + { + while let Some(key) = map.next_key::()? { + match key.as_str() { + OP_PREPEND => { + if let Some((existing_op, _)) = &self.op_and_var { + return Err(serde::de::Error::custom(format!( + "encountered {key} but operation already defined as {existing_op}", + ))); } + self.op_and_var = Some(( + OpKind::Prepend, + ConfKind::Operation(map.next_value::()?.0), + )); } - - // Comments and priority configs don't have any values. - let value = self - .op_and_var - .as_ref() - .and_then(|(op_kind, _)| match op_kind { - OpKind::Comment | OpKind::Priority => None, - _ => Some( - self.value - .take() - .ok_or_else(|| serde::de::Error::missing_field("value")), - ), - }) - .transpose()?; - - match self.op_and_var.take() { - Some((op, var)) => match op { - OpKind::Prepend => Ok(EnvOp::Prepend(PrependEnv { - prepend: var.get_op(), - separator: self.separator.take(), - value: value.unwrap_or_default() - })), - OpKind::Append => Ok(EnvOp::Append(AppendEnv { - append: var.get_op(), - separator: self.separator.take(), - value: value.unwrap_or_default() - })), - OpKind::Set => Ok(EnvOp::Set(SetEnv { - set: var.get_op(), - value: value.unwrap_or_default() - })), - OpKind::Comment => Ok(EnvOp::Comment(EnvComment{ - comment: var.get_op() - })), - OpKind::Priority => Ok(EnvOp::Priority(EnvPriority{ - priority: var.get_priority() - })) - }, - None => { - let mut err_msg = String::from(""); - for lint in self.lints.iter() { - err_msg = err_msg + lint; - } - Err(serde::de::Error::custom(err_msg)) + OP_PRIORITY => { + if let Some((existing_op, _)) = &self.op_and_var { + return Err(serde::de::Error::custom(format!( + "encountered {key} but operation already defined as {existing_op}", + ))); + } + self.op_and_var = Some(( + OpKind::Priority, + ConfKind::Priority(map.next_value::()?), + )); + } + OP_COMMENT => { + if let Some((existing_op, _)) = &self.op_and_var { + return Err(serde::de::Error::custom(format!( + "encountered {key} but operation already defined as {existing_op}", + ))); + } + self.op_and_var = Some(( + OpKind::Comment, + ConfKind::Operation(map.next_value::()?.0), + )); + } + OP_APPEND => { + if let Some((existing_op, _)) = &self.op_and_var { + return Err(serde::de::Error::custom(format!( + "encountered {key} but operation already defined as {existing_op}", + ))); } + self.op_and_var = Some(( + OpKind::Append, + ConfKind::Operation(map.next_value::()?.0), + )); + } + OP_SET => { + if let Some((existing_op, _)) = &self.op_and_var { + return Err(serde::de::Error::custom(format!( + "encountered {key} but operation already defined as {existing_op}", + ))); + } + self.op_and_var = Some(( + OpKind::Set, + ConfKind::Operation(map.next_value::()?.0), + )); + } + "value" => self.value = Some(map.next_value::()?.0), + "separator" => { + self.separator = map.next_value::>()?.map(|s| s.0) + } + unknown_config => { + self.lints + .push(LintMessage::UnknownEnvOpKey(EnvOpKey::new(unknown_config))); + map.next_value::()?; } } } - deserializer.deserialize_map(EnvOpVisitor::default()) + + // Comments and priority configs don't have any values. + let value = match self.op_and_var.as_ref() { + Some(v) => match v.0 { + OpKind::Comment | OpKind::Priority => String::from(""), + _ => self + .value + .take() + .ok_or_else(|| serde::de::Error::missing_field("value"))?, + }, + None => String::from(""), + }; + + let value = shellexpand::env(&value) + .unwrap_or(std::borrow::Cow::Borrowed("")) + .to_string(); + + self.value = Some(value); + Ok(self) } } @@ -489,3 +502,17 @@ impl SetEnv { format!("setenv {} \"{}\"", self.set, self.value) } } + +#[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] +pub struct NoOp; + +/// Returns empty string from methods when the operation is not found in deserialize +impl NoOp { + pub fn bash_source(&self) -> String { + String::from("") + } + + pub fn tcsh_source(&self) -> String { + String::from("") + } +} diff --git a/crates/spk-schema/src/install_spec.rs b/crates/spk-schema/src/install_spec.rs index c56c619a6..343de62a5 100644 --- a/crates/spk-schema/src/install_spec.rs +++ b/crates/spk-schema/src/install_spec.rs @@ -3,6 +3,7 @@ // https://github.com/spkenv/spk use std::marker::PhantomData; +use itertools::Itertools; use serde::{Deserialize, Serialize}; use spk_schema_foundation::option_map::Stringified; use spk_schema_ident::BuildIdent; @@ -33,6 +34,9 @@ where D: Default, { fn lints(&mut self) -> Vec { + for env in self.environment.iter_mut() { + self.lints.extend(std::mem::take(&mut env.lints)); + } std::mem::take(&mut self.lints) } } @@ -45,7 +49,7 @@ where requirements: RequirementsList, embedded: EmbeddedPackagesList, components: ComponentSpecList, - environment: Vec, + environment: Vec>, lints: Vec, _phantom: PhantomData, } @@ -59,7 +63,11 @@ where requirements: value.requirements, embedded: value.embedded, components: value.components, - environment: value.environment, + environment: value + .environment + .iter() + .map(|l| l.item.clone()) + .collect_vec(), } } } @@ -124,7 +132,7 @@ where "requirements" => self.requirements = map.next_value::()?, "embedded" => self.embedded = map.next_value::()?, "components" => self.components = map.next_value::()?, - "environment" => self.environment = map.next_value::>()?, + "environment" => self.environment = map.next_value::>>()?, unknown_config => { self.lints .push(LintMessage::UnknownInstallSpecKey(InstallSpecKey::new( @@ -138,7 +146,7 @@ where if self .environment .iter() - .filter(|&e| e.kind() == OpKind::Priority) + .filter(|&e| e.item.kind() == OpKind::Priority) .count() > 1 { diff --git a/crates/spk-schema/src/lib.rs b/crates/spk-schema/src/lib.rs index 1e337786e..1720119a4 100644 --- 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::{InstallSpecKey, LintKind, LintMessage, LintedItem, Lints, V0SpecKey}; +pub use lints::{EnvOpKey, InstallSpecKey, LintKind, LintMessage, LintedItem, Lints, V0SpecKey}; 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 index 7a587a019..fe41dba5a 100644 --- a/crates/spk-schema/src/lints.rs +++ b/crates/spk-schema/src/lints.rs @@ -8,12 +8,14 @@ use serde::{Deserialize, Serialize}; pub enum LintKind { UnknownV0SpecKey, UnknownInstallSpecKey, + UnknownEnvOpKey, } #[derive(Debug, Clone, Hash, PartialEq, Eq, Ord, PartialOrd, Deserialize, Serialize)] pub enum LintMessage { UnknownV0SpecKey(V0SpecKey), UnknownInstallSpecKey(InstallSpecKey), + UnknownEnvOpKey(EnvOpKey), } impl LintMessage { @@ -21,6 +23,7 @@ impl LintMessage { match self { Self::UnknownV0SpecKey(key) => key.message.clone(), Self::UnknownInstallSpecKey(key) => key.message.clone(), + Self::UnknownEnvOpKey(key) => key.message.clone(), } } } @@ -60,6 +63,36 @@ impl V0SpecKey { } } +#[derive(Debug, Default, Clone, Hash, PartialEq, Eq, Ord, PartialOrd, Deserialize, Serialize)] +pub struct EnvOpKey { + key: String, + message: String, +} + +impl EnvOpKey { + pub fn new(unknown_key: &str) -> Self { + let mut message = format!("Unrecognized EnvOp key: {unknown_key}. "); + let mut corpus = CorpusBuilder::new().finish(); + + corpus.add_text("append"); + corpus.add_text("comment"); + corpus.add_text("prepend"); + corpus.add_text("priority"); + corpus.add_text("set"); + + match corpus.search(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()) + } + }; + Self { + key: std::mem::take(&mut unknown_key.to_string()), + message: message.to_string(), + } + } +} + #[derive(Debug, Default, Clone, Hash, PartialEq, Eq, Ord, PartialOrd, Deserialize, Serialize)] pub struct InstallSpecKey { key: String, From 14778229872a5e221f8b0fb438c54138184248be Mon Sep 17 00:00:00 2001 From: Nichol Yip Date: Fri, 8 Sep 2023 11:09:36 -0700 Subject: [PATCH 06/16] Reworked NoOp type for EnvOp to UnrecognizedKey. Signed-off-by: Nichol Yip WIP: Adding lint feature to source spec Signed-off-by: Nichol Yip --- crates/spk-schema/src/environ.rs | 93 ++++--- crates/spk-schema/src/lints.rs | 8 - crates/spk-schema/src/source_spec.rs | 359 ++++++++++++++++++++++++++- 3 files changed, 409 insertions(+), 51 deletions(-) diff --git a/crates/spk-schema/src/environ.rs b/crates/spk-schema/src/environ.rs index cd856b5b3..35364826a 100644 --- a/crates/spk-schema/src/environ.rs +++ b/crates/spk-schema/src/environ.rs @@ -24,6 +24,7 @@ const OP_COMMENT: &str = "comment"; const OP_PREPEND: &str = "prepend"; const OP_PRIORITY: &str = "priority"; const OP_SET: &str = "set"; +const OP_NAMES: &[&str] = &[OP_APPEND, OP_COMMENT, OP_PREPEND, OP_SET]; /// The set of operation types for use in deserialization #[derive(Copy, Clone, Debug, PartialEq, strum::Display)] @@ -33,8 +34,8 @@ pub enum OpKind { Comment, Prepend, Priority, - NoOp, Set, + UnrecognizedKey, } /// An operation performed to the environment @@ -46,7 +47,7 @@ pub enum EnvOp { Prepend(PrependEnv), Priority(EnvPriority), Set(SetEnv), - NoOp(NoOp), + UnrecognizedKey(UnrecognizedKey), } impl EnvOp { @@ -57,7 +58,18 @@ impl EnvOp { EnvOp::Prepend(_) => OpKind::Prepend, EnvOp::Priority(_) => OpKind::Priority, EnvOp::Set(_) => OpKind::Set, - EnvOp::NoOp(_) => OpKind::NoOp, + EnvOp::UnrecognizedKey(_) => OpKind::UnrecognizedKey, + } + } + + pub fn error(&self) -> &str { + match self { + EnvOp::Append(_) => "", + EnvOp::Comment(_) => "", + EnvOp::Prepend(_) => "", + EnvOp::Priority(_) => "", + EnvOp::Set(_) => "", + EnvOp::UnrecognizedKey(e) => e.error.as_str(), } } @@ -135,7 +147,7 @@ impl EnvOp { Self::Prepend(op) => op.bash_source(), Self::Priority(op) => op.bash_source(), Self::Set(op) => op.bash_source(), - Self::NoOp(op) => op.bash_source(), + Self::UnrecognizedKey(op) => op.bash_source(), } } @@ -147,7 +159,7 @@ impl EnvOp { Self::Prepend(op) => op.tcsh_source(), Self::Priority(op) => op.tcsh_source(), Self::Set(op) => op.tcsh_source(), - Self::NoOp(op) => op.tcsh_source(), + Self::UnrecognizedKey(op) => op.tcsh_source(), } } @@ -195,31 +207,31 @@ impl Lints for EnvOpVisitor { impl From for EnvOp { fn from(mut value: EnvOpVisitor) -> Self { - match value.op_and_var.take() { - Some((op, var)) => match op { - OpKind::Prepend => EnvOp::Prepend(PrependEnv { - prepend: var.get_op(), - separator: value.separator.take(), - value: value.value.expect("an environment value"), - }), - OpKind::Append => EnvOp::Append(AppendEnv { - append: var.get_op(), - separator: value.separator.take(), - value: value.value.expect("an environment value"), - }), - OpKind::Set => EnvOp::Set(SetEnv { - set: var.get_op(), - value: value.value.expect("an environment value"), - }), - OpKind::Comment => EnvOp::Comment(CommentEnv { - comment: var.get_op(), - }), - OpKind::Priority => EnvOp::Priority(Priority { - priority: var.get_priority(), - }), - OpKind::NoOp => EnvOp::NoOp(NoOp), - }, - None => EnvOp::NoOp(NoOp), + let (op, var) = value.op_and_var.expect("an operation and variable"); + match op { + OpKind::Prepend => EnvOp::Prepend(PrependEnv { + prepend: var.get_op(), + separator: value.separator.take(), + value: value.value.expect("an environment value"), + }), + OpKind::Append => EnvOp::Append(AppendEnv { + append: var.get_op(), + separator: value.separator.take(), + value: value.value.expect("an environment value"), + }), + OpKind::Set => EnvOp::Set(SetEnv { + set: var.get_op(), + value: value.value.expect("an environment value"), + }), + OpKind::Comment => EnvOp::Comment(CommentEnv { + comment: var.get_op(), + }), + OpKind::Priority => EnvOp::Priority(Priority { + priority: var.get_priority(), + }), + OpKind::UnrecognizedKey => EnvOp::UnrecognizedKey(UnrecognizedKey { + error: var.get_op(), + }), } } } @@ -318,9 +330,13 @@ impl<'de> serde::de::Visitor<'de> for EnvOpVisitor { "separator" => { self.separator = map.next_value::>()?.map(|s| s.0) } - unknown_config => { + unknown_key => { self.lints - .push(LintMessage::UnknownEnvOpKey(EnvOpKey::new(unknown_config))); + .push(LintMessage::UnknownEnvOpKey(EnvOpKey::new(unknown_key))); + self.op_and_var = Some(( + OpKind::UnrecognizedKey, + ConfKind::Operation(format!("missing field to define operation and variable, expected one of {OP_NAMES:?}")), + )); map.next_value::()?; } } @@ -329,7 +345,7 @@ impl<'de> serde::de::Visitor<'de> for EnvOpVisitor { // Comments and priority configs don't have any values. let value = match self.op_and_var.as_ref() { Some(v) => match v.0 { - OpKind::Comment | OpKind::Priority => String::from(""), + OpKind::Comment | OpKind::Priority | OpKind::UnrecognizedKey => String::from(""), _ => self .value .take() @@ -503,15 +519,18 @@ impl SetEnv { } } +/// Stores the error message of the unrecognized key #[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] -pub struct NoOp; +pub struct UnrecognizedKey { + pub error: String, +} -/// Returns empty string from methods when the operation is not found in deserialize -impl NoOp { +impl UnrecognizedKey { + /// Empty bash source pub fn bash_source(&self) -> String { String::from("") } - + /// Empty tcsh source pub fn tcsh_source(&self) -> String { String::from("") } diff --git a/crates/spk-schema/src/lints.rs b/crates/spk-schema/src/lints.rs index fe41dba5a..5b31ecc8f 100644 --- a/crates/spk-schema/src/lints.rs +++ b/crates/spk-schema/src/lints.rs @@ -16,14 +16,6 @@ pub enum LintMessage { UnknownV0SpecKey(V0SpecKey), UnknownInstallSpecKey(InstallSpecKey), UnknownEnvOpKey(EnvOpKey), -} - -impl LintMessage { - pub fn message(&self) -> String { - match self { - Self::UnknownV0SpecKey(key) => key.message.clone(), - Self::UnknownInstallSpecKey(key) => key.message.clone(), - Self::UnknownEnvOpKey(key) => key.message.clone(), } } } diff --git a/crates/spk-schema/src/source_spec.rs b/crates/spk-schema/src/source_spec.rs index 17e4a0722..c395d80b2 100644 --- a/crates/spk-schema/src/source_spec.rs +++ b/crates/spk-schema/src/source_spec.rs @@ -8,8 +8,20 @@ use std::path::{Path, PathBuf}; use relative_path::RelativePathBuf; use serde::{Deserialize, Serialize}; - -use crate::{Error, Result, Script}; +use spk_schema_foundation::option_map::Stringified; + +use crate::{ + Error, + GitSourceKey, + LintMessage, + LintedItem, + Lints, + LocalSourceKey, + Result, + Script, + ScriptSourceKey, + TarSourceKey, +}; #[cfg(test)] #[path = "./source_spec_test.rs"] @@ -47,10 +59,19 @@ impl SourceSpec { SourceSpec::Script(source) => source.collect(dirname, env), } } + + // pub fn lints(&self) -> Vec { + // match self { + // SourceSpec::Local(source) => source.lints.clone(), + // SourceSpec::Git(source) => source.lints.clone(), + // SourceSpec::Tar(source) => source.lints.clone(), + // SourceSpec::Script(source) => source.lints.clone(), + // } + // } } /// Package source files in a local directory or file path. -#[derive(Clone, Debug, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] +#[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] pub struct LocalSource { pub path: PathBuf, #[serde( @@ -78,6 +99,97 @@ impl Default for LocalSource { } } +#[derive(Default, Debug)] +struct LocalSourceVisitor { + path: PathBuf, + exclude: Vec, + filter: Vec, + subdir: Option, + lints: Vec, +} + +impl Lints for LocalSourceVisitor { + fn lints(&mut self) -> Vec { + std::mem::take(&mut self.lints) + } +} + +impl From for LocalSource { + fn from(value: LocalSourceVisitor) -> Self { + Self { + path: value.path, + exclude: value.exclude, + filter: value.filter, + subdir: value.subdir, + } + } +} + +impl<'de> Deserialize<'de> for LocalSource { + fn deserialize(deserializer: D) -> std::result::Result + where + D: serde::de::Deserializer<'de>, + { + Ok(deserializer + .deserialize_map(LocalSourceVisitor::default())? + .into()) + } +} + +impl<'de> Deserialize<'de> for LintedItem { + fn deserialize(deserializer: D) -> std::result::Result + where + D: serde::de::Deserializer<'de>, + { + Ok(deserializer + .deserialize_map(LocalSourceVisitor::default())? + .into()) + } +} + +impl<'de> serde::de::Visitor<'de> for LocalSourceVisitor { + type Value = LocalSourceVisitor; + + fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + f.write_str("a local source spec") + } + + fn visit_map(mut self, mut map: A) -> std::result::Result + where + A: serde::de::MapAccess<'de>, + { + while let Some(key) = map.next_key::()? { + match key.as_str() { + "path" => self.path = map.next_value::()?, + "exclude" => { + self.exclude = map + .next_value::>()? + .into_iter() + .map(|s| s.0) + .collect() + } + "filter" => { + self.filter = map + .next_value::>()? + .into_iter() + .map(|s| s.0) + .collect() + } + "subdir" => self.subdir = Some(map.next_value::()?.0), + unknown_key => { + self.lints + .push(LintMessage::UnknownLocalSourceKey(LocalSourceKey::new( + unknown_key, + ))); + + map.next_value::()?; + } + } + } + Ok(self) + } +} + impl LocalSource { /// Create a new local source for the given path. pub fn new>(path: P) -> Self { @@ -153,7 +265,7 @@ impl LocalSource { } /// Package source files from a remote git repository. -#[derive(Clone, Debug, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] +#[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] pub struct GitSource { pub git: String, #[serde(default, rename = "ref", skip_serializing_if = "String::is_empty")] @@ -167,6 +279,85 @@ pub struct GitSource { pub subdir: Option, } +#[derive(Default, Debug)] +struct GitSourceVisitor { + git: String, + reference: String, + depth: u32, + subdir: Option, + lints: Vec, +} + +impl Lints for GitSourceVisitor { + fn lints(&mut self) -> Vec { + std::mem::take(&mut self.lints) + } +} + +impl From for GitSource { + fn from(value: GitSourceVisitor) -> Self { + Self { + git: value.git, + reference: value.reference, + depth: value.depth, + subdir: value.subdir, + } + } +} + +impl<'de> Deserialize<'de> for GitSource { + fn deserialize(deserializer: D) -> std::result::Result + where + D: serde::de::Deserializer<'de>, + { + Ok(deserializer + .deserialize_map(GitSourceVisitor::default())? + .into()) + } +} + +impl<'de> Deserialize<'de> for LintedItem { + fn deserialize(deserializer: D) -> std::result::Result + where + D: serde::de::Deserializer<'de>, + { + Ok(deserializer + .deserialize_map(GitSourceVisitor::default())? + .into()) + } +} + +impl<'de> serde::de::Visitor<'de> for GitSourceVisitor { + type Value = GitSourceVisitor; + + fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + f.write_str("a git source spec") + } + + fn visit_map(mut self, mut map: A) -> std::result::Result + where + A: serde::de::MapAccess<'de>, + { + while let Some(key) = map.next_key::()? { + match key.as_str() { + "git" => self.git = map.next_value::()?.0, + "reference" => self.reference = map.next_value::()?.0, + "depth" => self.depth = map.next_value::()?, + "subdir" => self.subdir = Some(map.next_value::()?.0), + unknown_key => { + self.lints + .push(LintMessage::UnknownGitSourceKey(GitSourceKey::new( + unknown_key, + ))); + + map.next_value::()?; + } + } + } + Ok(self) + } +} + impl GitSource { /// Collect the represented sources files into the given directory. pub fn collect(&self, dirname: &Path) -> Result<()> { @@ -215,13 +406,86 @@ impl GitSource { } /// Package source files from a local or remote tar archive. -#[derive(Clone, Debug, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] +#[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] pub struct TarSource { pub tar: String, #[serde(default, skip_serializing_if = "Option::is_none")] pub subdir: Option, } +#[derive(Default, Debug)] +struct TarSourceVisitor { + tar: String, + subdir: Option, + lints: Vec, +} + +impl Lints for TarSourceVisitor { + fn lints(&mut self) -> Vec { + std::mem::take(&mut self.lints) + } +} + +impl From for TarSource { + fn from(value: TarSourceVisitor) -> Self { + Self { + tar: value.tar, + subdir: value.subdir, + } + } +} + +impl<'de> Deserialize<'de> for TarSource { + fn deserialize(deserializer: D) -> std::result::Result + where + D: serde::de::Deserializer<'de>, + { + Ok(deserializer + .deserialize_map(TarSourceVisitor::default())? + .into()) + } +} + +impl<'de> Deserialize<'de> for LintedItem { + fn deserialize(deserializer: D) -> std::result::Result + where + D: serde::de::Deserializer<'de>, + { + Ok(deserializer + .deserialize_map(TarSourceVisitor::default())? + .into()) + } +} + +impl<'de> serde::de::Visitor<'de> for TarSourceVisitor { + type Value = TarSourceVisitor; + + fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + f.write_str("a tar source spec") + } + + fn visit_map(mut self, mut map: A) -> std::result::Result + where + A: serde::de::MapAccess<'de>, + { + while let Some(key) = map.next_key::()? { + match key.as_str() { + "tar" => self.tar = map.next_value::()?.0, + "subdir" => self.subdir = Some(map.next_value::()?.0), + unknown_key => { + self.lints + .push(LintMessage::UnknownTarSourceKey(TarSourceKey::new( + unknown_key, + ))); + + map.next_value::()?; + } + } + } + Ok(self) + } +} + impl TarSource { /// Collect the represented sources files into the given directory. pub fn collect(&self, dirname: &Path) -> Result<()> { @@ -287,13 +551,96 @@ impl TarSource { } /// Package source files collected via arbitrary shell script. -#[derive(Clone, Debug, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] +#[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] pub struct ScriptSource { pub script: Script, #[serde(default, skip_serializing_if = "Option::is_none")] pub subdir: Option, } +#[derive(Debug)] +struct ScriptSourceVisitor { + script: Script, + subdir: Option, + lints: Vec, +} + +impl Default for ScriptSourceVisitor { + fn default() -> Self { + Self { + script: Script::new(vec![""]), + subdir: None, + lints: Vec::default(), + } + } +} + +impl Lints for ScriptSourceVisitor { + fn lints(&mut self) -> Vec { + std::mem::take(&mut self.lints) + } +} + +impl From for ScriptSource { + fn from(value: ScriptSourceVisitor) -> Self { + Self { + script: value.script, + subdir: value.subdir, + } + } +} + +impl<'de> Deserialize<'de> for ScriptSource { + fn deserialize(deserializer: D) -> std::result::Result + where + D: serde::de::Deserializer<'de>, + { + Ok(deserializer + .deserialize_map(ScriptSourceVisitor::default())? + .into()) + } +} + +impl<'de> Deserialize<'de> for LintedItem { + fn deserialize(deserializer: D) -> std::result::Result + where + D: serde::de::Deserializer<'de>, + { + Ok(deserializer + .deserialize_map(ScriptSourceVisitor::default())? + .into()) + } +} + +impl<'de> serde::de::Visitor<'de> for ScriptSourceVisitor { + type Value = ScriptSourceVisitor; + + fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + f.write_str("a script source spec") + } + + fn visit_map(mut self, mut map: A) -> std::result::Result + where + A: serde::de::MapAccess<'de>, + { + while let Some(key) = map.next_key::()? { + match key.as_str() { + "script" => self.script = map.next_value::