Skip to content

Commit b41517d

Browse files
khodzhaJake-Shadle
authored andcommitted
Add label to license gathering when a clarification file is not found
1 parent e0a3ba0 commit b41517d

3 files changed

Lines changed: 61 additions & 28 deletions

File tree

src/licenses/cfg.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@ pub struct Private {
7777
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
7878
pub struct FileSource {
7979
/// The crate relative path of the LICENSE file
80-
pub path: PathBuf,
80+
/// Spanned so we can report typos on it in case it never matches anything.
81+
pub path: Spanned<PathBuf>,
8182
/// The hash of the LICENSE text. If the `path`'s hash
8283
/// differs from the contents of the path, the file is
8384
/// parsed to determine if the license(s) contained in
@@ -390,14 +391,15 @@ mod test {
390391
version: semver::VersionReq::parse("0.1.1").unwrap(),
391392
}]
392393
);
394+
let p: PathBuf = "LICENSE".into();
393395
assert_eq!(
394396
validated.clarifications,
395397
vec![ValidClarification {
396398
name: "ring".to_owned(),
397399
version: semver::VersionReq::parse("*").unwrap(),
398400
expression: spdx::Expression::parse("MIT AND ISC AND OpenSSL").unwrap(),
399401
license_files: vec![FileSource {
400-
path: "LICENSE".into(),
402+
path: p.fake(),
401403
hash: 0xbd0e_ed23,
402404
}],
403405
expr_offset: 415,

src/licenses/diags.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,3 +64,15 @@ impl Into<Diag> for UnmatchedLicenseAllowance {
6464
.into()
6565
}
6666
}
67+
68+
pub(crate) struct MissingClarificationFile<'a> {
69+
pub(crate) expected: &'a crate::cfg::Spanned<std::path::PathBuf>,
70+
pub(crate) cfg_file_id: crate::diag::FileId,
71+
}
72+
73+
impl<'a> Into<Label> for MissingClarificationFile<'a> {
74+
fn into(self) -> Label {
75+
Label::secondary(self.cfg_file_id, self.expected.span.clone())
76+
.with_message("unable to locate specified license file")
77+
}
78+
}

src/licenses/gather.rs

Lines changed: 45 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,15 @@ struct PackFile {
158158
data: PackFileData,
159159
}
160160

161+
enum MismatchReason<'a> {
162+
/// The specified file was not found when gathering license files
163+
FileNotFound,
164+
/// Encountered an I/O error trying to read the file contents
165+
Error(&'a std::io::Error),
166+
/// The hash of the license file doesn't match the expected hash
167+
HashDiffers,
168+
}
169+
161170
struct LicensePack {
162171
license_files: Vec<PackFile>,
163172
err: Option<std::io::Error>,
@@ -199,29 +208,26 @@ impl LicensePack {
199208
}
200209
}
201210

202-
fn matches(&self, hashes: &[FileSource]) -> bool {
203-
if self.license_files.len() != hashes.len() {
204-
return false;
205-
}
206-
207-
for (expected, actual) in self.license_files.iter().zip(hashes.iter()) {
208-
if !expected.path.ends_with(&actual.path) {
209-
return false;
210-
}
211-
212-
match &expected.data {
213-
PackFileData::Bad(_) => {
214-
return false;
215-
}
216-
PackFileData::Good(lf) => {
217-
if lf.hash != actual.hash {
218-
return false;
211+
fn license_files_match(&self, expected: &FileSource) -> Result<(), MismatchReason<'_>> {
212+
let err = match self
213+
.license_files
214+
.iter()
215+
.find(|lf| lf.path.ends_with(&expected.path.value))
216+
{
217+
Some(lf) => match &lf.data {
218+
PackFileData::Bad(e) => MismatchReason::Error(e),
219+
PackFileData::Good(file_data) => {
220+
if file_data.hash != expected.hash {
221+
MismatchReason::HashDiffers
222+
} else {
223+
return Ok(());
219224
}
220225
}
221-
}
222-
}
226+
},
227+
None => MismatchReason::FileNotFound,
228+
};
223229

224-
true
230+
Err(err)
225231
}
226232

227233
fn get_expression(
@@ -591,14 +597,27 @@ impl Gatherer {
591597
}
592598
};
593599

594-
// pub name: String,
595-
// pub version: VersionReq,
596-
// pub expression: spdx::Expression,
597-
// pub license_files: Vec<FileSource>,
598600
// Check to see if the clarification provided exactly matches
599601
// the set of detected licenses, if they do, we use the clarification's
600-
// license expression as the license requirement's for this crate
601-
if lp.matches(&clarification.license_files) {
602+
// license expression as the license requirements for this crate
603+
if clarification.license_files.iter().all(|clf| {
604+
match lp.license_files_match(&clf) {
605+
Ok(_) => true,
606+
Err(reason) => {
607+
if let MismatchReason::FileNotFound = reason {
608+
labels.push(
609+
super::diags::MissingClarificationFile {
610+
expected: &clf.path,
611+
cfg_file_id: cfg.file_id,
612+
}
613+
.into(),
614+
);
615+
}
616+
617+
false
618+
}
619+
}
620+
}) {
602621
return KrateLicense {
603622
krate,
604623
lic_info: LicenseInfo::SPDXExpression {

0 commit comments

Comments
 (0)