fix: make zip crate safer and more readable

[Its-Just-Nans]

fix: make create safer and more readable

Less #[cfg(feature...)] for the imports

Enforce no unwrap with clippy lints

[gemini-code-assist]

Summary of Changes

Hello @Its-Just-Nans, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the robustness and readability of the codebase by implementing stricter error handling, enforcing comprehensive Clippy lints, and refining type conversions. The changes aim to make the code safer and more maintainable, particularly in areas involving cryptographic operations, data decompression, and string conversions, without altering the core functionality.

Highlights

• Improved Error Handling: Replaced numerous instances of .unwrap() and .expect() with more robust error handling mechanisms like .map_err() and ok_or_else(), particularly in src/aes.rs, src/compression.rs, src/read.rs, src/unstable.rs, and src/write.rs. This makes the code safer by explicitly handling potential failure points.
• Stricter Clippy Lints: The project now enforces a wider range of Clippy lints, including clippy::unwrap_used, clippy::expect_used, and clippy::cast_lossless, to improve code quality and prevent common pitfalls. Specific test modules have been annotated to allow unwrap_used and expect_used where appropriate for testing contexts.
• Enhanced Readability and Maintainability: Documentation comments have been updated to use backticks for code and type names, improving clarity. Imports for feature-gated modules (like AES crypto) have been refactored to use fully qualified paths, reducing #[cfg] proliferation and making the code easier to follow.
• Safer Type Conversions: Explicit type conversions now frequently use u32::from() and u64::from() instead of as u32 or as u64 casts, promoting type safety and preventing potential data loss or unexpected behavior, especially in src/compression.rs, src/spec.rs, and src/types.rs.
• Refactored CP437 Decoding: The FromCp437 trait implementations in src/cp437.rs now return Result types, allowing for proper error propagation when decoding CP437 byte sequences to strings. The to_char helper function also now returns Option<char> for safer character conversion.
• Lifetime Annotation Improvements: Lifetime annotations have been added or refined in several function signatures, such as make_reader in src/read.rs and various FileOptions parameters in src/types.rs and src/write.rs, leading to more precise type inference and compile-time correctness checks.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[amazon-q-developer]

Summary

This PR makes significant improvements to code safety by eliminating unwrap() calls and adding strict clippy lints. However, there are several critical issues that need to be addressed before merging:

Critical Issues ⚠️

1. Breaking API Change: The data_start() method in ZipFile now returns Option<u64> instead of u64, which is a breaking change requiring a major version bump.

2. Logic Error in CP437 Conversion: The change from map to filter_map in src/cp437.rs silently drops invalid characters instead of properly handling conversion errors, potentially causing data loss.

Positive Changes ✅

• Proper error handling replacing unwrap() calls throughout the codebase
• Addition of strict clippy lints to prevent future unsafe code
• Improved error messages in many places
• Better use of explicit type conversions

Recommendations

1. For the API breaking change: Either revert the data_start() signature change or document this as a breaking change requiring a major version bump.

2. For CP437 conversion: Fix the logic to properly handle conversion errors instead of silently dropping invalid characters.

3. Error messages: Consider providing more descriptive error messages in places like clamp_size_field().

The overall direction of this PR is excellent for improving code safety, but the critical issues need to be resolved before merging.

---

You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.

[amazon-q-developer]

🛑 Breaking API Change: Changing data_start() from returning u64 to Option<u64> is a breaking change that will cause compilation failures for existing users of this public API.

This method is part of the public API and changing its return type requires a major version bump. Consider deprecating the old method and adding a new one with a different name, or document this as a breaking change in the changelog.

[gemini-code-assist]

Code Review

This pull request does a great job of improving code safety and readability by removing unwrap() calls and enforcing stricter clippy lints. The changes are well-executed across the codebase. I've found a couple of remaining unwrap() calls that were likely missed and a minor inconsistency with the new casting style. Addressing these will fully align the code with the goals of this PR.

[Its-Just-Nans]

@Pr0methean

I'm still working on it until all tests pass

Just note that this cannot be inside the 7.2.1 but inside at least the 7.3.0
There is an API change #536 (comment)

[Its-Just-Nans]

@Pr0methean

If successful, this is mergeable

note: need a bump to 7.3.0 (the Cargo.toml is already modified for the CI to pass)