Skip to content

Commit

Permalink
Implement more lints and integration tests
Browse files Browse the repository at this point in the history
- Lint oudated pak version.
- Lint empty archive.
- Lint archive containing only non-pak files.
- Lint archive containing multiple paks.
  • Loading branch information
jieyouxu committed Aug 13, 2023
1 parent e3d9856 commit 7b5ac28
Show file tree
Hide file tree
Showing 10 changed files with 209 additions and 4 deletions.
73 changes: 72 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ pub mod splice;
pub mod state;

use std::io::{Cursor, Read};
use std::str::FromStr;
use std::{
collections::HashSet,
path::{Path, PathBuf},
Expand Down Expand Up @@ -296,10 +297,80 @@ pub(crate) fn get_pak_from_data(mut data: Box<dyn ReadSeek>) -> Result<Box<dyn R
None => Ok(None),
}
})
.find_map(|e| e.transpose())
.find_map(Result::transpose)
.context("Zip does not contain pak")?
} else {
data.rewind()?;
Ok(data)
}
}

pub(crate) enum PakOrNotPak {
Pak(Box<dyn ReadSeek>),
NotPak(Box<dyn ReadSeek>),
}

pub(crate) enum GetAllFilesFromDataError {
EmptyArchive,
OnlyNonPakFiles,
Other(anyhow::Error),
}

pub(crate) fn lint_get_all_files_from_data(
mut data: Box<dyn ReadSeek>,
) -> Result<Vec<(PathBuf, PakOrNotPak)>, GetAllFilesFromDataError> {
if let Ok(mut archive) = zip::ZipArchive::new(&mut data) {
if archive.len() == 0 {

Check failure on line 323 in src/lib.rs

View workflow job for this annotation

GitHub Actions / clippy

length comparison to zero

error: length comparison to zero --> src/lib.rs:323:12 | 323 | if archive.len() == 0 { | ^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `archive.is_empty()` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#len_zero = note: `-D clippy::len-zero` implied by `-D warnings`

Check failure on line 323 in src/lib.rs

View workflow job for this annotation

GitHub Actions / clippy

length comparison to zero

error: length comparison to zero --> src/lib.rs:323:12 | 323 | if archive.len() == 0 { | ^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `archive.is_empty()` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#len_zero = note: `-D clippy::len-zero` implied by `-D warnings`
return Err(GetAllFilesFromDataError::EmptyArchive);
}

let mut files = Vec::new();
for i in 0..archive.len() {
let mut file = archive
.by_index(i)
.map_err(|e| GetAllFilesFromDataError::Other(e.into()))?;
match file.enclosed_name().map(Path::to_path_buf) {
Some(p) => {
if file.is_file() {
if p.extension().filter(|e| e == &"pak").is_some() {
let mut buf = vec![];
file.read_to_end(&mut buf)
.map_err(|e| GetAllFilesFromDataError::Other(e.into()))?;
files.push((
p.to_path_buf(),
PakOrNotPak::Pak(Box::new(Cursor::new(buf))),
));
} else {
let mut buf = vec![];
file.read_to_end(&mut buf)
.map_err(|e| GetAllFilesFromDataError::Other(e.into()))?;
files.push((
p.to_path_buf(),
PakOrNotPak::NotPak(Box::new(Cursor::new(buf))),
));
}
}
}
_ => {}
}

Check failure on line 355 in src/lib.rs

View workflow job for this annotation

GitHub Actions / clippy

you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`

error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let` --> src/lib.rs:332:13 | 332 | / match file.enclosed_name().map(Path::to_path_buf) { 333 | | Some(p) => { 334 | | if file.is_file() { 335 | | if p.extension().filter(|e| e == &"pak").is_some() { ... | 354 | | _ => {} 355 | | } | |_____________^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#single_match = note: `-D clippy::single-match` implied by `-D warnings` help: try | 332 ~ if let Some(p) = file.enclosed_name().map(Path::to_path_buf) { 333 + if file.is_file() { 334 + if p.extension().filter(|e| e == &"pak").is_some() { 335 + let mut buf = vec![]; 336 + file.read_to_end(&mut buf) 337 + .map_err(|e| GetAllFilesFromDataError::Other(e.into()))?; 338 + files.push(( 339 + p.to_path_buf(), 340 + PakOrNotPak::Pak(Box::new(Cursor::new(buf))), 341 + )); 342 + } else { 343 + let mut buf = vec![]; 344 + file.read_to_end(&mut buf) 345 + .map_err(|e| GetAllFilesFromDataError::Other(e.into()))?; 346 + files.push(( 347 + p.to_path_buf(), 348 + PakOrNotPak::NotPak(Box::new(Cursor::new(buf))), 349 + )); 350 + } 351 + } 352 + } |

Check failure on line 355 in src/lib.rs

View workflow job for this annotation

GitHub Actions / clippy

you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`

error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let` --> src/lib.rs:332:13 | 332 | / match file.enclosed_name().map(Path::to_path_buf) { 333 | | Some(p) => { 334 | | if file.is_file() { 335 | | if p.extension().filter(|e| e == &"pak").is_some() { ... | 354 | | _ => {} 355 | | } | |_____________^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#single_match = note: `-D clippy::single-match` implied by `-D warnings` help: try | 332 ~ if let Some(p) = file.enclosed_name().map(Path::to_path_buf) { 333 + if file.is_file() { 334 + if p.extension().filter(|e| e == &"pak").is_some() { 335 + let mut buf = vec![]; 336 + file.read_to_end(&mut buf) 337 + .map_err(|e| GetAllFilesFromDataError::Other(e.into()))?; 338 + files.push(( 339 + p.to_path_buf(), 340 + PakOrNotPak::Pak(Box::new(Cursor::new(buf))), 341 + )); 342 + } else { 343 + let mut buf = vec![]; 344 + file.read_to_end(&mut buf) 345 + .map_err(|e| GetAllFilesFromDataError::Other(e.into()))?; 346 + files.push(( 347 + p.to_path_buf(), 348 + PakOrNotPak::NotPak(Box::new(Cursor::new(buf))), 349 + )); 350 + } 351 + } 352 + } |
}

if files
.iter()
.filter(|(_, pak_or_not_pak)| matches!(pak_or_not_pak, PakOrNotPak::Pak(..)))
.count()
> 1
{
Ok(files)
} else {
Err(GetAllFilesFromDataError::OnlyNonPakFiles)
}
} else {
data.rewind()
.map_err(|e| GetAllFilesFromDataError::Other(e.into()))?;
Ok(vec![(
PathBuf::from_str(".").unwrap(),
PakOrNotPak::Pak(data),
)])
}
}
46 changes: 43 additions & 3 deletions src/mod_lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,17 @@ use anyhow::{Context, Result};
use tracing::{info, span, trace, Level};

use crate::providers::ModSpecification;
use crate::{get_pak_from_data, open_file};
use crate::{lint_get_all_files_from_data, open_file, GetAllFilesFromDataError, PakOrNotPak};

#[derive(Debug)]
pub struct ModLintReport {
pub conflicting_mods: BTreeMap<String, BTreeSet<ModSpecification>>,
pub asset_register_bin_mods: BTreeMap<ModSpecification, BTreeSet<String>>,
pub shader_file_mods: BTreeMap<ModSpecification, BTreeSet<String>>,
pub outdated_pak_version_mods: BTreeMap<ModSpecification, repak::Version>,
pub empty_archive_mods: BTreeSet<ModSpecification>,
pub archive_with_only_non_pak_files_mods: BTreeSet<ModSpecification>,
pub archive_with_multiple_paks_mods: BTreeSet<ModSpecification>,
}

pub fn lint(mods: &[(ModSpecification, PathBuf)]) -> Result<ModLintReport> {
Expand All @@ -26,12 +29,46 @@ pub fn lint(mods: &[(ModSpecification, PathBuf)]) -> Result<ModLintReport> {
let mut asset_register_bin_mods = BTreeMap::new();
let mut shader_file_mods = BTreeMap::new();
let mut outdated_pak_version_mods = BTreeMap::new();
let mut archive_with_only_non_pak_files_mods = BTreeSet::new();
let mut empty_archive_mods = BTreeSet::new();
let mut archive_with_multiple_paks_mods = BTreeSet::new();

for (mod_spec, mod_pak_path) in mods {
trace!(?mod_spec, ?mod_pak_path);

let mut buf = get_pak_from_data(Box::new(BufReader::new(open_file(mod_pak_path)?)))?;
let pak = repak::PakReader::new_any(&mut buf, None)?;
let bufs = match lint_get_all_files_from_data(Box::new(BufReader::new(open_file(
mod_pak_path,
)?))) {
Ok(bufs) => bufs,
Err(e) => match e {
GetAllFilesFromDataError::EmptyArchive => {
empty_archive_mods.insert(mod_spec.clone());
continue;
}
GetAllFilesFromDataError::OnlyNonPakFiles => {
archive_with_only_non_pak_files_mods.insert(mod_spec.clone());
continue;
}
GetAllFilesFromDataError::Other(e) => return Err(e),
},
};

let mut pak_bufs = bufs
.into_iter()
.filter_map(|(p, pak_or_non_pak)| match pak_or_non_pak {
PakOrNotPak::Pak(pak_buf) => Some((p, pak_buf)),
PakOrNotPak::NotPak(_) => None,
})
.collect::<Vec<_>>();

if pak_bufs.len() > 1 {
archive_with_multiple_paks_mods.insert(mod_spec.clone());
}

let (ref first_pak_path, ref mut first_pak_buf) = pak_bufs[0];
trace!(?first_pak_path);

let pak = repak::PakReader::new_any(first_pak_buf, None)?;

if pak.version() < repak::Version::V11 {
outdated_pak_version_mods.insert(mod_spec.clone(), pak.version());
Expand Down Expand Up @@ -85,5 +122,8 @@ pub fn lint(mods: &[(ModSpecification, PathBuf)]) -> Result<ModLintReport> {
asset_register_bin_mods,
shader_file_mods,
outdated_pak_version_mods,
empty_archive_mods,
archive_with_only_non_pak_files_mods,
archive_with_multiple_paks_mods,
})
}
Binary file added test_mod_batches/lints/empty_archive.zip
Binary file not shown.
Binary file added test_mod_batches/lints/multiple_pak_files.zip
Binary file not shown.
Binary file added test_mod_batches/lints/multiple_paks.zip
Binary file not shown.
Binary file added test_mod_batches/lints/only_non_pak_files.zip
Binary file not shown.
Empty file.
Binary file added test_mod_batches/lints/outdated_pak_version.pak
Binary file not shown.
Empty file.
94 changes: 94 additions & 0 deletions tests/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,97 @@ pub fn test_lint_asset_registry_bin() {
Some(&["fsd/content/assetregistry.bin".to_string()].into())
);
}

#[test]
pub fn test_lint_outdated_pak_version() {
let base_path = PathBuf::from_str("test_mod_batches/lints/").unwrap();
assert!(base_path.exists());
let outdated_pak_path = base_path.clone().join("outdated_pak_version.pak");
assert!(outdated_pak_path.exists());
let outdated_spec = ModSpecification {
url: "outdated".to_string(),
};
let mods = vec![(outdated_spec.clone(), outdated_pak_path)];

let ModLintReport {
outdated_pak_version_mods,
..
} = drg_mod_integration::mod_lint::lint(&mods).unwrap();

println!("{:#?}", outdated_pak_version_mods);

assert_eq!(
outdated_pak_version_mods.get(&outdated_spec),
Some(&repak::Version::V10)
);
}

#[test]
pub fn test_lint_empty_archive() {
let base_path = PathBuf::from_str("test_mod_batches/lints/").unwrap();
assert!(base_path.exists());
let empty_archive_path = base_path.clone().join("empty_archive.zip");
assert!(empty_archive_path.exists());
let empty_archive_spec = ModSpecification {
url: "empty".to_string(),
};
let mods = vec![(empty_archive_spec.clone(), empty_archive_path)];

let ModLintReport {
empty_archive_mods, ..
} = drg_mod_integration::mod_lint::lint(&mods).unwrap();

println!("{:#?}", empty_archive_mods);

assert!(empty_archive_mods.contains(&empty_archive_spec));
}

#[test]
pub fn test_lint_only_non_pak_files() {
let base_path = PathBuf::from_str("test_mod_batches/lints/").unwrap();
assert!(base_path.exists());
let a_path = base_path.clone().join("A.pak");
assert!(a_path.exists());
let only_non_pak_path = base_path.clone().join("only_non_pak_files.zip");
assert!(only_non_pak_path.exists());
let a_spec = ModSpecification {
url: "A".to_string(),
};
let only_non_pak_spec = ModSpecification {
url: "only_non_pak".to_string(),
};
let mods = vec![
(a_spec.clone(), a_path),
(only_non_pak_spec.clone(), only_non_pak_path),
];

let ModLintReport {
archive_with_only_non_pak_files_mods,
..
} = drg_mod_integration::mod_lint::lint(&mods).unwrap();

println!("{:#?}", archive_with_only_non_pak_files_mods);

assert!(archive_with_only_non_pak_files_mods.contains(&only_non_pak_spec));
}

#[test]
pub fn test_lint_multi_pak_archive() {
let base_path = PathBuf::from_str("test_mod_batches/lints/").unwrap();
assert!(base_path.exists());
let multiple_paks_archive_path = base_path.clone().join("multiple_paks.zip");
assert!(multiple_paks_archive_path.exists());
let multiple_paks_spec = ModSpecification {
url: "multiple_paks".to_string(),
};
let mods = vec![(multiple_paks_spec.clone(), multiple_paks_archive_path)];

let ModLintReport {
archive_with_multiple_paks_mods,
..
} = drg_mod_integration::mod_lint::lint(&mods).unwrap();

println!("{:#?}", archive_with_multiple_paks_mods);

assert!(archive_with_multiple_paks_mods.contains(&multiple_paks_spec));
}

0 comments on commit 7b5ac28

Please sign in to comment.