Skip to content

Commit

Permalink
Print diagnostic for missing clarification files (#249)
Browse files Browse the repository at this point in the history
* Add label to license gathering when a clarification file is not found

* Placate clippy

Co-authored-by: Jake Shadle <jake.shadle@embark-studios.com>
  • Loading branch information
khodzha and Jake-Shadle authored Oct 13, 2020
1 parent e0a3ba0 commit 4e068f7
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 28 deletions.
6 changes: 4 additions & 2 deletions src/licenses/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ pub struct Private {
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
pub struct FileSource {
/// The crate relative path of the LICENSE file
pub path: PathBuf,
/// Spanned so we can report typos on it in case it never matches anything.
pub path: Spanned<PathBuf>,
/// The hash of the LICENSE text. If the `path`'s hash
/// differs from the contents of the path, the file is
/// parsed to determine if the license(s) contained in
Expand Down Expand Up @@ -390,14 +391,15 @@ mod test {
version: semver::VersionReq::parse("0.1.1").unwrap(),
}]
);
let p: PathBuf = "LICENSE".into();
assert_eq!(
validated.clarifications,
vec![ValidClarification {
name: "ring".to_owned(),
version: semver::VersionReq::parse("*").unwrap(),
expression: spdx::Expression::parse("MIT AND ISC AND OpenSSL").unwrap(),
license_files: vec![FileSource {
path: "LICENSE".into(),
path: p.fake(),
hash: 0xbd0e_ed23,
}],
expr_offset: 415,
Expand Down
12 changes: 12 additions & 0 deletions src/licenses/diags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,15 @@ impl Into<Diag> for UnmatchedLicenseAllowance {
.into()
}
}

pub(crate) struct MissingClarificationFile<'a> {
pub(crate) expected: &'a crate::cfg::Spanned<std::path::PathBuf>,
pub(crate) cfg_file_id: crate::diag::FileId,
}

impl<'a> Into<Label> for MissingClarificationFile<'a> {
fn into(self) -> Label {
Label::secondary(self.cfg_file_id, self.expected.span.clone())
.with_message("unable to locate specified license file")
}
}
73 changes: 47 additions & 26 deletions src/licenses/gather.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,15 @@ struct PackFile {
data: PackFileData,
}

enum MismatchReason<'a> {
/// The specified file was not found when gathering license files
FileNotFound,
/// Encountered an I/O error trying to read the file contents
Error(&'a std::io::Error),
/// The hash of the license file doesn't match the expected hash
HashDiffers,
}

struct LicensePack {
license_files: Vec<PackFile>,
err: Option<std::io::Error>,
Expand Down Expand Up @@ -199,29 +208,26 @@ impl LicensePack {
}
}

fn matches(&self, hashes: &[FileSource]) -> bool {
if self.license_files.len() != hashes.len() {
return false;
}

for (expected, actual) in self.license_files.iter().zip(hashes.iter()) {
if !expected.path.ends_with(&actual.path) {
return false;
}

match &expected.data {
PackFileData::Bad(_) => {
return false;
}
PackFileData::Good(lf) => {
if lf.hash != actual.hash {
return false;
fn license_files_match(&self, expected: &FileSource) -> Result<(), MismatchReason<'_>> {
let err = match self
.license_files
.iter()
.find(|lf| lf.path.ends_with(&expected.path.value))
{
Some(lf) => match &lf.data {
PackFileData::Bad(e) => MismatchReason::Error(e),
PackFileData::Good(file_data) => {
if file_data.hash != expected.hash {
MismatchReason::HashDiffers
} else {
return Ok(());
}
}
}
}
},
None => MismatchReason::FileNotFound,
};

true
Err(err)
}

fn get_expression(
Expand Down Expand Up @@ -591,14 +597,29 @@ impl Gatherer {
}
};

// pub name: String,
// pub version: VersionReq,
// pub expression: spdx::Expression,
// pub license_files: Vec<FileSource>,
// Check to see if the clarification provided exactly matches
// the set of detected licenses, if they do, we use the clarification's
// license expression as the license requirement's for this crate
if lp.matches(&clarification.license_files) {
// license expression as the license requirements for this crate
let clarifications_match = clarification.license_files.iter().all(|clf| {
match lp.license_files_match(&clf) {
Ok(_) => true,
Err(reason) => {
if let MismatchReason::FileNotFound = reason {
labels.push(
super::diags::MissingClarificationFile {
expected: &clf.path,
cfg_file_id: cfg.file_id,
}
.into(),
);
}

false
}
}
});

if clarifications_match {
return KrateLicense {
krate,
lic_info: LicenseInfo::SPDXExpression {
Expand Down

0 comments on commit 4e068f7

Please sign in to comment.