style: lint fuzz projects in CI (#283)

* ci: checks format for fuzz_read and fuzz_write * ci: add clippy checks for fuzz_read and fuzz_write it seems that they depends on cargo-afl, so we need to use `cargo afl clippy` instead of `cargo clippy`. * style: fix some clippy warnings In this commit I replaced `self.path.join("/")` with `self.path.join("")` because I think the expected result should be the original path with trailing "/". see: https://rust-lang.github.io/rust-clippy/master/index.html#join_absolute_paths
zip-rs · Feb 27, 2025 · c6ba201 · c6ba201
1 parent db69a4e
commit c6ba201
Show file tree

Hide file tree

Showing 2 changed files with 142 additions and 54 deletions.
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
@@ -45,8 +45,12 @@ jobs:
     steps:
       - uses: actions/checkout@v4
       - run: rustup toolchain add nightly && rustup default nightly && rustup component add rustfmt
-
-      - run: cargo fmt --all -- --check
+      - name: fmt
+        run: cargo fmt --all -- --check
+      - name: fmt fuzz_read
+        run: cargo fmt --manifest-path fuzz_read/Cargo.toml -- --check
+      - name: fmt fuzz_write
+        run: cargo fmt --manifest-path fuzz_write/Cargo.toml -- --check
 
   check_minimal_versions:
     if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name != github.event.pull_request.base.repo.full_name
@@ -84,12 +88,14 @@ jobs:
       - style_and_docs
     steps:
       - uses: actions/checkout@v4
-      - run: rustup toolchain add nightly && rustup default nightly
+      - run: rustup toolchain add nightly && rustup default nightly && rustup component add clippy
 
       - run: cargo install cargo-afl
 
       - name: cargo afl system-config
         run: cargo afl system-config
+      - name: clippy
+        run: cargo afl clippy --all-features --manifest-path ${{ github.workspace }}/fuzz_read/Cargo.toml -- -D warnings
       - name: compile fuzz
         run: cargo afl build --all-features --manifest-path ${{ github.workspace }}/fuzz_read/Cargo.toml
       - name: run fuzz
@@ -132,12 +138,14 @@ jobs:
       - style_and_docs
     steps:
       - uses: actions/checkout@v4
-      - run: rustup toolchain add nightly && rustup default nightly
+      - run: rustup toolchain add nightly && rustup default nightly && rustup component add clippy
 
       - run: cargo install cargo-afl
 
       - name: cargo afl system-config
         run: cargo afl system-config
+      - name: clippy
+        run: cargo afl clippy --no-default-features --manifest-path ${{ github.workspace }}/fuzz_read/Cargo.toml -- -D warnings
       - name: compile fuzz
         run: cargo afl build --manifest-path ${{ github.workspace }}/fuzz_read/Cargo.toml
       - name: run fuzz
@@ -172,12 +180,14 @@ jobs:
       - style_and_docs
     steps:
       - uses: actions/checkout@v4
-      - run: rustup toolchain add nightly && rustup default nightly
+      - run: rustup toolchain add nightly && rustup default nightly && rustup component add clippy
 
       - run: cargo install cargo-afl
 
       - name: cargo afl system-config
         run: cargo afl system-config
+      - name: clippy
+        run: cargo afl clippy --all-features --manifest-path ${{ github.workspace }}/fuzz_write/Cargo.toml -- -D warnings
       - name: compile fuzz
         run: cargo afl build --all-features --manifest-path ${{ github.workspace }}/fuzz_write/Cargo.toml
       - name: run fuzz
@@ -220,12 +230,14 @@ jobs:
       - style_and_docs
     steps:
       - uses: actions/checkout@v4
-      - run: rustup toolchain add nightly && rustup default nightly
+      - run: rustup toolchain add nightly && rustup default nightly && rustup component add clippy
 
       - run: cargo install cargo-afl
 
       - name: cargo afl system-config
         run: cargo afl system-config
+      - name: clippy
+        run: cargo afl clippy --no-default-features --manifest-path ${{ github.workspace }}/fuzz_write/Cargo.toml -- -D warnings
       - name: compile fuzz
         run: cargo afl build --all-features --manifest-path ${{ github.workspace }}/fuzz_write/Cargo.toml
       - name: run fuzz

diff --git a/fuzz_write/src/main.rs b/fuzz_write/src/main.rs
@@ -1,10 +1,10 @@
 use afl::fuzz;
 use arbitrary::{Arbitrary, Unstructured};
-use core::fmt::{Debug};
+use core::fmt::Debug;
 use replace_with::replace_with_or_abort;
 use std::fmt::{Arguments, Formatter, Write};
-use std::io::{Cursor, Seek, SeekFrom};
 use std::io::Write as IoWrite;
+use std::io::{Cursor, Seek, SeekFrom};
 use std::path::PathBuf;
 use tikv_jemallocator::Jemalloc;
 use zip::result::{ZipError, ZipResult};
@@ -49,11 +49,11 @@ pub struct FileOperation<'k> {
     // 'abort' flag is separate, to prevent trying to copy an aborted file
 }
 
-impl<'k> FileOperation<'k> {
+impl FileOperation<'_> {
     fn get_path(&self) -> Option<PathBuf> {
         match &self.basic {
             BasicFileOperation::SetArchiveComment(_) => None,
-            BasicFileOperation::WriteDirectory(_) => Some(self.path.join("/")),
+            BasicFileOperation::WriteDirectory(_) => Some(self.path.join("")),
             BasicFileOperation::MergeWithOtherFile { operations, .. } => operations
                 .iter()
                 .flat_map(|(op, abort)| if !abort { op.get_path() } else { None })
@@ -84,44 +84,66 @@ fn deduplicate_paths(copy: &mut PathBuf, original: &PathBuf) {
     }
 }
 
-fn do_operation<'k>(
+fn do_operation(
     writer: &mut zip::ZipWriter<Cursor<Vec<u8>>>,
-    operation: FileOperation<'k>,
+    operation: FileOperation<'_>,
     abort: bool,
     flush_on_finish_file: bool,
     files_added: &mut usize,
     stringifier: &mut impl Write,
-    panic_on_error: bool
+    panic_on_error: bool,
 ) -> Result<(), Box<dyn std::error::Error>> {
     writer.set_flush_on_finish_file(flush_on_finish_file);
-    let FileOperation { basic, mut path, reopen} = operation;
+    let FileOperation {
+        basic,
+        mut path,
+        reopen,
+    } = operation;
     match basic {
         BasicFileOperation::WriteNormalFile {
-            contents, mut options, ..
+            contents,
+            mut options,
+            ..
         } => {
             let uncompressed_size = contents.iter().map(|chunk| chunk.len()).sum::<usize>();
             if uncompressed_size >= u32::MAX as usize {
                 options = options.large_file(true);
             }
             if options == FullFileOptions::default() {
-                writeln!(stringifier, "writer.start_file_from_path({:?}, Default::default())?;", path)?;
+                writeln!(
+                    stringifier,
+                    "writer.start_file_from_path({:?}, Default::default())?;",
+                    path
+                )?;
             } else {
-                writeln!(stringifier, "writer.start_file_from_path({:?}, {:?})?;", path, options)?;
+                writeln!(
+                    stringifier,
+                    "writer.start_file_from_path({:?}, {:?})?;",
+                    path, options
+                )?;
             }
             writer.start_file_from_path(&*path, options)?;
             for chunk in contents.iter() {
                 writeln!(stringifier, "writer.write_all(&{:?})?;", chunk)?;
-                writer.write_all(&chunk)?;
+                writer.write_all(chunk)?;
             }
             *files_added += 1;
         }
         BasicFileOperation::WriteDirectory(options) => {
-            writeln!(stringifier, "writer.add_directory_from_path(&{:?}, {:?})?;", path, options)?;
+            writeln!(
+                stringifier,
+                "writer.add_directory_from_path(&{:?}, {:?})?;",
+                path, options
+            )?;
             writer.add_directory_from_path(&*path, options.to_owned())?;
             *files_added += 1;
         }
         BasicFileOperation::WriteSymlinkWithTarget { target, options } => {
-            writeln!(stringifier, "writer.add_symlink_from_path(&{:?}, {:?}, {:?});", path, target, options)?;
+            writeln!(
+                stringifier,
+                "writer.add_symlink_from_path(&{:?}, {:?}, {:?});",
+                path, target, options
+            )?;
             writer.add_symlink_from_path(&*path, target, options.to_owned())?;
             *files_added += 1;
         }
@@ -130,8 +152,20 @@ fn do_operation<'k>(
                 return Ok(());
             };
             deduplicate_paths(&mut path, &base_path);
-            do_operation(writer, *base, false, flush_on_finish_file, files_added, stringifier, panic_on_error)?;
-            writeln!(stringifier, "writer.shallow_copy_file_from_path({:?}, {:?});", base_path, path)?;
+            do_operation(
+                writer,
+                *base,
+                false,
+                flush_on_finish_file,
+                files_added,
+                stringifier,
+                panic_on_error,
+            )?;
+            writeln!(
+                stringifier,
+                "writer.shallow_copy_file_from_path({:?}, {:?});",
+                base_path, path
+            )?;
             writer.shallow_copy_file_from_path(&*base_path, &*path)?;
             *files_added += 1;
         }
@@ -140,38 +174,65 @@ fn do_operation<'k>(
                 return Ok(());
             };
             deduplicate_paths(&mut path, &base_path);
-            do_operation(writer, *base, false, flush_on_finish_file, files_added, stringifier, panic_on_error)?;
-            writeln!(stringifier, "writer.deep_copy_file_from_path({:?}, {:?});", base_path, path)?;
+            do_operation(
+                writer,
+                *base,
+                false,
+                flush_on_finish_file,
+                files_added,
+                stringifier,
+                panic_on_error,
+            )?;
+            writeln!(
+                stringifier,
+                "writer.deep_copy_file_from_path({:?}, {:?});",
+                base_path, path
+            )?;
             writer.deep_copy_file_from_path(&*base_path, path)?;
             *files_added += 1;
         }
-        BasicFileOperation::MergeWithOtherFile { operations, initial_junk } => {
+        BasicFileOperation::MergeWithOtherFile {
+            operations,
+            initial_junk,
+        } => {
             if initial_junk.is_empty() {
-                writeln!(stringifier, "let sub_writer = {{\n\
-                     let mut writer = ZipWriter::new(Cursor::new(Vec::new()));")?;
+                writeln!(
+                    stringifier,
+                    "let sub_writer = {{\n\
+                     let mut writer = ZipWriter::new(Cursor::new(Vec::new()));"
+                )?;
             } else {
-                writeln!(stringifier,
-                         "let sub_writer = {{\n\
+                writeln!(
+                    stringifier,
+                    "let sub_writer = {{\n\
                           let mut initial_junk = Cursor::new(vec!{:?});\n\
                           initial_junk.seek(SeekFrom::End(0))?;
-                          let mut writer = ZipWriter::new(initial_junk);", initial_junk)?;
+                          let mut writer = ZipWriter::new(initial_junk);",
+                    initial_junk
+                )?;
             }
             let mut initial_junk = Cursor::new(initial_junk.into_vec());
             initial_junk.seek(SeekFrom::End(0))?;
             let mut other_writer = zip::ZipWriter::new(initial_junk);
             let mut inner_files_added = 0;
-            operations.into_vec().into_iter().for_each(|(operation, abort)| {
-                let _ = do_operation(
-                    &mut other_writer,
-                    operation,
-                    abort,
-                    false,
-                    &mut inner_files_added,
-                    stringifier,
-                    panic_on_error
-                );
-            });
-            writeln!(stringifier, "writer\n}};\nwriter.merge_archive(sub_writer.finish_into_readable()?)?;")?;
+            operations
+                .into_vec()
+                .into_iter()
+                .for_each(|(operation, abort)| {
+                    let _ = do_operation(
+                        &mut other_writer,
+                        operation,
+                        abort,
+                        false,
+                        &mut inner_files_added,
+                        stringifier,
+                        panic_on_error,
+                    );
+                });
+            writeln!(
+                stringifier,
+                "writer\n}};\nwriter.merge_archive(sub_writer.finish_into_readable()?)?;"
+            )?;
             writer.merge_archive(other_writer.finish_into_readable()?)?;
             *files_added += inner_files_added;
         }
@@ -191,15 +252,19 @@ fn do_operation<'k>(
     match reopen {
         ReopenOption::DoNotReopen => {
             writeln!(stringifier, "writer")?;
-            return Ok(())
-        },
+            return Ok(());
+        }
         ReopenOption::ViaFinish => {
             let old_comment = writer.get_raw_comment().to_owned();
-            writeln!(stringifier, "let mut writer = ZipWriter::new_append(writer.finish()?)?;")?;
+            writeln!(
+                stringifier,
+                "let mut writer = ZipWriter::new_append(writer.finish()?)?;"
+            )?;
             replace_with_or_abort(writer, |old_writer: zip::ZipWriter<Cursor<Vec<u8>>>| {
                 (|| -> ZipResult<zip::ZipWriter<Cursor<Vec<u8>>>> {
                     zip::ZipWriter::new_append(old_writer.finish()?)
-                })().unwrap_or_else(|_| {
+                })()
+                .unwrap_or_else(|_| {
                     if panic_on_error {
                         panic!("Failed to create new ZipWriter")
                     }
@@ -212,11 +277,15 @@ fn do_operation<'k>(
         }
         ReopenOption::ViaFinishIntoReadable => {
             let old_comment = writer.get_raw_comment().to_owned();
-            writeln!(stringifier, "let mut writer = ZipWriter::new_append(writer.finish()?)?;")?;
+            writeln!(
+                stringifier,
+                "let mut writer = ZipWriter::new_append(writer.finish()?)?;"
+            )?;
             replace_with_or_abort(writer, |old_writer| {
                 (|| -> ZipResult<zip::ZipWriter<Cursor<Vec<u8>>>> {
                     zip::ZipWriter::new_append(old_writer.finish()?)
-                })().unwrap_or_else(|_| {
+                })()
+                .unwrap_or_else(|_| {
                     if panic_on_error {
                         panic!("Failed to create new ZipWriter")
                     }
@@ -229,7 +298,7 @@ fn do_operation<'k>(
     Ok(())
 }
 
-impl <'k> FuzzTestCase<'k> {
+impl FuzzTestCase<'_> {
     fn execute(self, stringifier: &mut impl Write, panic_on_error: bool) -> ZipResult<()> {
         let mut initial_junk = Cursor::new(self.initial_junk.into_vec());
         initial_junk.seek(SeekFrom::End(0))?;
@@ -251,7 +320,7 @@ impl <'k> FuzzTestCase<'k> {
                 self.flush_on_finish_file,
                 &mut files_added,
                 stringifier,
-                panic_on_error
+                panic_on_error,
             );
         }
         if final_reopen {
@@ -263,14 +332,21 @@ impl <'k> FuzzTestCase<'k> {
     }
 }
 
-impl <'k> Debug for FuzzTestCase<'k> {
+impl Debug for FuzzTestCase<'_> {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         if self.initial_junk.is_empty() {
-            writeln!(f, "let mut writer = ZipWriter::new(Cursor::new(Vec::new()));")?;
+            writeln!(
+                f,
+                "let mut writer = ZipWriter::new(Cursor::new(Vec::new()));"
+            )?;
         } else {
-            writeln!(f, "let mut initial_junk = Cursor::new(vec!{:?});\n\
+            writeln!(
+                f,
+                "let mut initial_junk = Cursor::new(vec!{:?});\n\
                          initial_junk.seek(SeekFrom::End(0))?;\n\
-                         let mut writer = ZipWriter::new(initial_junk);", &self.initial_junk)?;
+                         let mut writer = ZipWriter::new(initial_junk);",
+                &self.initial_junk
+            )?;
         }
         let _ = self.clone().execute(f, false);
         Ok(())