Repository navigation
Fix JAR manifest lookup and complete typed Attributes Map support - #215
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a0ee698da
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #215 +/- ##
==========================================
+ Coverage 87.20% 87.29% +0.09%
==========================================
Files 309 310 +1
Lines 46332 46685 +353
==========================================
+ Hits 40403 40756 +353
Misses 5929 5929 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🔵 Needs a closer look
It changes shared iterator semantics across HashMap/HashSet views plus JAR manifest lookup behavior, which is broad enough to warrant human review despite no functional defects found.
1 open finding
What changed in this PR
This PR improves RustJava's java.util.jar compatibility and extends the shared HashMap iterator infrastructure. It fixes JAR manifest discovery to be case-insensitive (while keeping ordinary entry lookup case-sensitive), introduces a typed Attributes$Name key class with validation and case-insensitive equality/hashing, makes Attributes expose a live entrySet via the Map interface, and adds Iterator.remove() support to the HashMap entry/key/value view iterators (which also benefits HashSet).
Changes:
- Case-insensitive manifest lookup with canonical-filename preference and
nullwhen absent; invalid manifest attribute names surface asIOException. - New
Attributes$Name(validation, case-insensitive equals/hashCode, Java 8 constants);Attributesnow implementsMap, stores typed keys, supportsgetValue(Name)and a liveentrySet. - Shared
HashMap$HashIterator.remove()implementation (via amap/lastReturnedfield) withsnapshot_entriesalways snapshotting entries; iterators now receive the backing map in their constructor.
| File | Description |
|---|---|
src/classes/java/util/jar/attributes_name.rs |
New typed Attributes$Name class with validation, equality/hashing, and constants. |
src/classes/java/util/jar/attributes.rs |
Implements Map, adds getValue(Name)/entrySet, wraps keys as Name. |
src/classes/java/util/jar/manifest.rs |
Converts invalid-attribute IllegalArgumentException into IOException. |
src/classes/java/util/jar/jar_file.rs |
Case-insensitive manifest fallback using the zip archive's file names. |
src/classes/java/util/jar.rs |
Exports the new AttributesName. |
src/classes/java/util/zip/zip_file.rs |
Exposes get_zip_archive as pub(crate). |
src/classes/java/util/hash_map.rs |
snapshot_entries always snapshots entries; passes this to iterators; drops SnapshotKind. |
src/classes/java/util/hash_map_hash_iterator.rs |
Adds map/lastReturned fields and a working remove(). |
src/classes/java/util/hash_map_{key,value,entry}_iterator.rs |
New constructor signature; key/value iterators override next to extract key/value. |
src/loader.rs |
Registers AttributesName proto. |
tests/.../jar/test_attributes.rs, test_jar_file.rs, test_manifest.rs |
Cover typed names, entrySet removal, case-insensitive manifest, invalid-name IOException. |
tests/.../util/test_hash_map.rs, test_hash_set.rs |
Cover view-iterator removal and backing-map updates. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
dlunch
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8234ff9869
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Changes
Verification