feat(hashmap): add FromJson impl for HashMap[String, V] to match Map

[bobzhang]

Summary

Adds impl FromJson for HashMap[String, V] to mirror the same impl that Map[String, V] already has in json/from_json.mbt.

Motivation

Map and HashMap should have a consistent API. HashMap was missing JSON deserialization support.

Changes

• Added impl @json.FromJson for HashMap[String, V] in hashmap/json.mbt
• Added moonbitlang/core/json as a non-test import in hashmap/moon.pkg
• Uses the public JsonPath::add_key API (enum variants are private)
• Updated pkg.generated.mbti

[coveralls]

Coverage Report for CI Build 6265

Coverage increased (+0.002%) to 90.793%

Details

• Coverage increased (+0.002%) from the base build.
• Patch coverage: 5 of 5 lines across 1 file are fully covered (100%).
• No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.

---

Coverage Stats

Relevant Lines:	18280
Covered Lines:	16597
Line Coverage:	90.79%
Coverage Strength:	305775.97 hits per line

---

💛 - Coveralls

[Copilot]

Pull request overview

Adds JSON deserialization support for HashMap[String, V] to align HashMap’s API with the existing Map[String, V] FromJson implementation in moonbitlang/core/json.

Changes:

• Added impl @json.FromJson for HashMap[String, V] in hashmap/json.mbt.
• Added moonbitlang/core/json as a non-test dependency for the hashmap package.
• Updated generated package interface metadata (pkg.generated.mbti) to reflect the new import and impl.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
hashmap/json.mbt	Implements @json.FromJson for HashMap[String, V] and documents usage.
hashmap/moon.pkg	Adds moonbitlang/core/json to non-test imports so the new impl can compile/use @json.
hashmap/pkg.generated.mbti	Regenerates package interface to include the new json import and FromJson impl.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[bobzhang]

Rebased onto main and modernized. The premise still holds: HashMap has had ToJson for a while with no FromJson, so it could be serialized but not read back, while Map, SortedMap and @immut/sorted_map all carry the object-decoding impl already.

What the rebase changed

The only conflicting file was the generated pkg.generated.mbti, regenerated rather than merged. The impl itself needed bringing up to current conventions, taking @sorted_map's equivalent as the model:

• with from_json(...) → with fn from_json(...).
• raise @json.JsonDecodeError(...) → raise JsonDecodeError(...), and the message "HashMap::from_json: expected object" → "@hashmap.from_json: expected object", matching @sorted_map's wording.
• @json.FromJson::from_json(v, ...) → V::from_json(v, ...).
• Added the extend declaration in hashmap/extends.mbt, #deprecated + #doc(hidden) exactly as sorted_map/extends.mbt does for the same impl. Without it the build now emits warning 0079 (implicit_impl_as_method) — that warning did not exist when this PR was opened.
• Pre-sized the table with capacity_for_length(obj.length()), since the object's size is known up front and the entries would otherwise rehash on the way in.
• Added hashmap/json_test.mbt. The original had only a docstring example: now a generated round trip, an empty object, nested values, a non-object input, and the decode path reported when a single member fails. Replacing path.add_key(k) with path fails that last test.

pkg.generated.mbti gains exactly one line. moon test passes 7518/7518, and 7459/7459 on js.

Codex CLI review (xhigh reasoning effort)

Approved, with two P3s — both fixed in the pushed branch.

No P0/P1/P2 findings. I would approve.

• P3 — hashmap/json_test.mbt:17 incorrectly says HashMap has no Arbitrary implementation; one exists in quickcheck/arbitrary_collections.mbt. Remove the comment or generate HashMap samples directly.
• P3 — hashmap/json.mbt:38 teaches .to_json(), contrary to the repository's stated ToJson API direction. The object literal can be passed directly to @json.from_json.

The judgment calls look sound:

• Object decoding is correct because it round-trips the existing encoding for HashMap[String, V]. vector_map uses pairs specifically to preserve order and non-string keys; adopting that shape here would require a breaking ToJson redesign.
• For object length n, the selected capacity is at least 2n, so the n insertions cannot hit the 50% grow condition. Empty objects produce capacity 1, which is valid. The theoretical length * 2 overflow requires an object size beyond what the current Map representation can practically support and mirrors existing constructor logic.
• The deprecated, hidden extend is correct for a trait owned by @json.
• JSON objects are already Map[String, Json]. Parsing repeated names updates that map, so the last value wins before this decoder runs — matching Map and both sorted-map decoders. A duplicate-key test here would document end-to-end behavior but would not discriminate this implementation.
• path.add_key(k) constructs the same observable path as Map's reused mutable node. Its allocation is one transient node per decoded member and is not material enough to expand the public JSON-path API.
• A mutation that removes the capacity argument and uses HashMap([]) would survive every new test. That means the functional behavior is well covered, while the pre-sizing optimization is not directly tested; asserting public capacity() could cover it, although that would deliberately couple the test to the optimization.

Signed-off-by: Codex CLI codex@openai.com

Both P3s were right, and the second is worth spelling out: hashmap/extends.mbt marks to_json deprecated in favour of @json.to_json, so the docstring was teaching users a call that warns. The example now passes the object literal straight to @json.from_json. The round-trip test generates HashMap samples directly instead of going through an array of pairs.

I left the pre-sizing untested, per the reviewer's own reasoning — it is a capacity hint with no observable behaviour, and pinning it would couple the test to the optimization rather than to the contract.

I also asked whether the [String, V] shape is still the right convention, given @immut/vector_map instead decodes an array of [key, value] pairs with a fully generic key and round-trips its own ToJson exactly. The answer above is the reason to keep this one as it is: matching vector_map would mean redesigning HashMap's existing ToJson, which is a breaking change and a separate discussion.