Skip to content

Commit 7aeda44

Browse files
author
Nichol Yip
committed
Implemented lint for the top level of the spec config
Signed-off-by: Nichol Yip <[email protected]>
1 parent 40cdb71 commit 7aeda44

File tree

6 files changed

+115
-38
lines changed

6 files changed

+115
-38
lines changed

‎Cargo.lock‎

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ use anyhow::Result;
88
use clap::Args;
99
use colored::Colorize;
1010
use spk_cli_common::{flags, CommandArgs, Run};
11-
use spk_schema::{SpecTemplate, Template, TemplateExt};
11+
use spk_schema::v0::LintedSpec;
12+
use spk_schema::{AnyIdent, Error};
1213

1314
/// Validate spk yaml files
1415
#[derive(Args)]
@@ -23,12 +24,29 @@ pub struct Lint {
2324
#[async_trait::async_trait]
2425
impl Run for Lint {
2526
async fn run(&mut self) -> Result<i32> {
26-
let options = self.options.get_options()?;
27+
// let options = self.options.get_options()?;
2728
let mut out = 0;
2829
for spec in self.packages.iter() {
29-
let result = SpecTemplate::from_file(spec).and_then(|t| t.render(&options));
30+
let file_path = spec
31+
.canonicalize()
32+
.map_err(|err| Error::InvalidPath(spec.to_owned(), err))?;
33+
let file = std::fs::File::open(&file_path)
34+
.map_err(|err| Error::FileOpenError(file_path.to_owned(), err))?;
35+
let rdr = std::io::BufReader::new(file);
36+
37+
let result: std::result::Result<LintedSpec<AnyIdent>, serde_yaml::Error> =
38+
serde_yaml::from_reader(rdr);
39+
3040
match result {
31-
Ok(_) => println!("{} {}", "OK".green(), spec.display()),
41+
Ok(s) => match s.lints.is_empty() {
42+
true => println!("{} {}", "OK".green(), spec.display()),
43+
false => {
44+
for lint in s.lints {
45+
tracing::error!(lint);
46+
}
47+
out = 1;
48+
}
49+
},
3250
Err(err) => {
3351
println!(
3452
"{} {}:\n{} {err}",

‎crates/spk-schema/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ format_serde_error = {version = "0.3", default_features = false, features = ["se
1414
ignore = "0.4.18"
1515
indexmap = "1.7"
1616
itertools = "0.10"
17+
ngrammatic = "0.4.0"
1718
nom = { workspace = true }
1819
regex = "1.5"
1920
relative-path = "1.3"

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

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,13 @@ impl Template for SpecTemplate {
161161
.map_err(Error::InvalidTemplate)?;
162162
Ok(SpecRecipe::from_yaml(rendered)?)
163163
}
164+
165+
// fn render_lint(&self, options: &OptionMap) -> Result<Self::Output> {
166+
// let data = super::TemplateData::new(options);
167+
// let rendered = spk_schema_liquid::render_template(&self.template, &data)
168+
// .map_err(Error::InvalidTemplate)?;
169+
// Ok(SpecRecipe::from_yaml(rendered)?)
170+
// }
164171
}
165172

166173
impl TemplateExt for SpecTemplate {
@@ -216,6 +223,29 @@ impl TemplateExt for SpecTemplate {
216223
}
217224
}
218225

226+
// #[derive(Debug, Clone, Hash, Eq, PartialEq, Serialize)]
227+
// // #[enum_dispatch(Deprecate, DeprecateMut)]
228+
// pub enum SpecRecipeKind {
229+
// DefaultSpec(super::v0::Spec<VersionIdent>),
230+
// LintSpec(super::v0::LintedSpec<VersionIdent>)
231+
// }
232+
233+
// impl SpecRecipeKind {
234+
// pub fn spec(&self) -> &super::v0::Spec<VersionIdent> {
235+
// match self {
236+
// SpecRecipeKind::DefaultSpec(s) => s,
237+
// SpecRecipeKind::LintSpec(lint) => &lint.spec,
238+
// }
239+
// }
240+
241+
// pub fn lints(&self) -> Vec<String> {
242+
// match self {
243+
// SpecRecipeKind::DefaultSpec(_) => Vec::default(),
244+
// SpecRecipeKind::LintSpec(lint) => lint.lints.clone(),
245+
// }
246+
// }
247+
// }
248+
219249
/// Specifies some buildable object within the spk ecosystem.
220250
///
221251
/// All build-able types have a recipe representation
@@ -357,7 +387,6 @@ impl FromYaml for SpecRecipe {
357387
Err(err) => return Err(SerdeError::new(yaml, err)),
358388
Ok(m) => m,
359389
};
360-
361390
match with_version.api {
362391
ApiVersion::V0Package => {
363392
let inner =

‎crates/spk-schema/src/v0/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ mod spec;
66
mod test_spec;
77
mod variant;
88

9-
pub use spec::Spec;
9+
pub use spec::{LintedSpec, Spec};
1010
pub use test_spec::TestSpec;
1111
pub use variant::Variant;
1212

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

Lines changed: 54 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use std::convert::TryInto;
88
use std::path::Path;
99

1010
use itertools::Itertools;
11+
use ngrammatic::CorpusBuilder;
1112
use serde::{Deserialize, Serialize};
1213
use spk_schema_foundation::name::PkgNameBuf;
1314
use spk_schema_foundation::option_map::Stringified;
@@ -730,7 +731,9 @@ where
730731
where
731732
D: serde::de::Deserializer<'de>,
732733
{
733-
Ok(std::convert::Into::<Spec<VersionIdent>>::into(deserializer.deserialize_map(SpecVisitor::recipe())?))
734+
Ok(std::convert::Into::<Spec<VersionIdent>>::into(
735+
deserializer.deserialize_map(SpecVisitor::recipe())?,
736+
))
734737
}
735738
}
736739

@@ -761,7 +764,8 @@ where
761764
where
762765
D: serde::de::Deserializer<'de>,
763766
{
764-
let mut spec: Spec<BuildIdent> = deserializer.deserialize_map(SpecVisitor::package())?.into();
767+
let mut spec: Spec<BuildIdent> =
768+
deserializer.deserialize_map(SpecVisitor::package())?.into();
765769
if spec.pkg.is_source() {
766770
// for backward-compatibility with older publishes, prune out anything
767771
// that is not relevant to a source package, since now source packages
@@ -781,7 +785,9 @@ where
781785
where
782786
D: serde::de::Deserializer<'de>,
783787
{
784-
Ok(std::convert::Into::<LintedSpec<VersionIdent>>::into(deserializer.deserialize_map(SpecVisitor::recipe())?))
788+
Ok(std::convert::Into::<LintedSpec<VersionIdent>>::into(
789+
deserializer.deserialize_map(SpecVisitor::recipe())?,
790+
))
785791
}
786792
}
787793

@@ -793,7 +799,8 @@ where
793799
where
794800
D: serde::de::Deserializer<'de>,
795801
{
796-
let mut spec: LintedSpec<AnyIdent> = deserializer.deserialize_map(SpecVisitor::default())?.into();
802+
let mut spec: LintedSpec<AnyIdent> =
803+
deserializer.deserialize_map(SpecVisitor::default())?.into();
797804
if spec.spec.pkg.is_source() {
798805
// for backward-compatibility with older publishes, prune out anything
799806
// that is not relevant to a source package, since now source packages
@@ -812,7 +819,8 @@ where
812819
where
813820
D: serde::de::Deserializer<'de>,
814821
{
815-
let mut spec: LintedSpec<BuildIdent> = deserializer.deserialize_map(SpecVisitor::package())?.into();
822+
let mut spec: LintedSpec<BuildIdent> =
823+
deserializer.deserialize_map(SpecVisitor::package())?.into();
816824
if spec.spec.pkg.is_source() {
817825
// for backward-compatibility with older publishes, prune out anything
818826
// that is not relevant to a source package, since now source packages
@@ -858,7 +866,7 @@ impl<B, T> From<SpecVisitor<B, T>> for LintedSpec<Ident<B, T>>
858866
where
859867
Ident<B, T>: serde::de::DeserializeOwned,
860868
{
861-
fn from(mut value: SpecVisitor<B, T>) -> Self {
869+
fn from(mut value: SpecVisitor<B, T>) -> Self {
862870
Self {
863871
spec: Spec {
864872
pkg: value.pkg.expect("Missing field: pkg"),
@@ -874,18 +882,16 @@ where
874882
// Safety: see the SpecVisitor::package constructor
875883
unsafe { build_spec.into_inner() }
876884
}
877-
Some(build_spec) => {
878-
match build_spec.try_into() {
879-
Ok(b) => b,
880-
Err(_) => BuildSpec::default(),
881-
}
885+
Some(build_spec) => match build_spec.try_into() {
886+
Ok(b) => b,
887+
Err(_) => BuildSpec::default(),
882888
},
883889
None => Default::default(),
884890
},
885891
tests: value.tests.take().unwrap_or_default(),
886892
install: value.install.take().unwrap_or_default(),
887893
},
888-
lints: value.lints
894+
lints: value.lints,
889895
}
890896
}
891897
}
@@ -894,7 +900,7 @@ impl<B, T> From<SpecVisitor<B, T>> for Spec<Ident<B, T>>
894900
where
895901
Ident<B, T>: serde::de::DeserializeOwned,
896902
{
897-
fn from(mut value: SpecVisitor<B, T>) -> Self {
903+
fn from(mut value: SpecVisitor<B, T>) -> Self {
898904
Self {
899905
pkg: value.pkg.expect("Missing field pkg"),
900906
meta: value.meta.take().unwrap_or_default(),
@@ -909,11 +915,9 @@ where
909915
// Safety: see the SpecVisitor::package constructor
910916
unsafe { build_spec.into_inner() }
911917
}
912-
Some(build_spec) => {
913-
match build_spec.try_into() {
914-
Ok(b) => b,
915-
Err(_) => BuildSpec::default(),
916-
}
918+
Some(build_spec) => match build_spec.try_into() {
919+
Ok(b) => b,
920+
Err(_) => BuildSpec::default(),
917921
},
918922
None => Default::default(),
919923
},
@@ -923,7 +927,6 @@ where
923927
}
924928
}
925929

926-
927930
impl SpecVisitor<PkgNameBuf, Version> {
928931
pub fn recipe() -> Self {
929932
Self::default()
@@ -968,25 +971,45 @@ where
968971
"build" => self.build = Some(map.next_value::<UncheckedBuildSpec>()?),
969972
"tests" => self.tests = Some(map.next_value::<Vec<TestSpec>>()?),
970973
"install" => self.install = Some(map.next_value::<InstallSpec>()?),
971-
unrecognized_string => {
972-
self.lints.push(format!("unrecognized string {unrecognized_string}"));
974+
"api" => {
975+
map.next_value::<serde::de::IgnoredAny>()?;
976+
}
977+
unknown_config => {
978+
self.lints.push(format!("Unknown config: {unknown_config}"));
979+
let mut corpus = CorpusBuilder::new().finish();
980+
981+
corpus.add_text("pkg");
982+
corpus.add_text("meta");
983+
corpus.add_text("compat");
984+
corpus.add_text("deprecated");
985+
corpus.add_text("sources");
986+
corpus.add_text("build");
987+
corpus.add_text("tests");
988+
corpus.add_text("install");
989+
corpus.add_text("api");
990+
991+
let results = corpus.search(unknown_config, 0.6);
992+
let no_match = format!("No similar config found for: {}", unknown_config);
993+
let top_match = match results.first() {
994+
Some(s) => &s.text,
995+
None => &no_match,
996+
};
997+
998+
self.lints
999+
.push(format!("The most similar config is: {}", top_match));
1000+
9731001
// ignore any unrecognized field, but consume the value anyway
9741002
// TODO: could we warn about fields that look like typos?
9751003
map.next_value::<serde::de::IgnoredAny>()?;
9761004
}
9771005
}
9781006
}
9791007

980-
self.pkg.as_ref().ok_or_else(|| serde::de::Error::missing_field("pkg"))?;
981-
// match self.build {
982-
// Some(build_spec) if !self.check_build_spec => {
983-
// // Safety: see the SpecVisitor::package constructor
984-
// unsafe { build_spec.into_inner(); }
985-
// },
986-
// Some(build_spec) => build_spec.try_into().map_err(serde::de::Error::custom)?,
987-
// _ => (),
988-
// }
989-
Ok(self.into())
1008+
self.pkg
1009+
.as_ref()
1010+
.ok_or_else(|| serde::de::Error::missing_field("pkg"))?;
1011+
1012+
Ok(self)
9901013

9911014
// let pkg = self
9921015
// .pkg
@@ -1014,4 +1037,3 @@ where
10141037
// })
10151038
}
10161039
}
1017-

0 commit comments

Comments
 (0)