fix: honor wildcard-key custom converters when writing CSV (#1045) (#1056)

[Mikkey-f]

What changed

Custom converters registered with supportExcelTypeKey() == null (the wildcard key (JavaType, null)) are now also registered under (JavaType, STRING). A single registerConverter(...) call now works for both xlsx and CSV writes.

Why

AbstractExcelWriteExecutor#doConvert resolves converters with the key (JavaType, targetCellDataType). The xlsx write path leaves the target type null, so a wildcard registration (JavaType, null) is found. The CSV write path forces the target type to STRING (csv is converted to string by default), so the same registration misses and the built-in string converter silently takes over: no warning is logged and the CSV output silently diverges from the xlsx output of the same code. See #1045 and #1056 (same root cause as EasyExcel#3054).

How

AbstractWriteHolder registers custom converters at two sites (own registration plus replay of workbook-level registrations when a sheet holder is created); both now delegate to a single putCustomConverter helper. When the declared excel type key is null, the converter is additionally registered under (JavaType, STRING) — the key the CSV lookup uses. No lookup code changes; the write path is untouched.

Impact

• Without custom converters: unaffected (registration loop is not executed).
• Explicit-key registrations: unaffected (registered under one key, as before).
• Wildcard (null-key) registrations: CSV now applies the custom converter instead of silently falling back to the built-in string converter — matching the declared wildcard intent (matches all cell data types). The template-fill write path shares the same registry and benefits automatically.
• Field-level converters (@ExcelProperty(converter = ...)) bypass the registry entirely and are unaffected.

Documentation

The null semantics of Converter#supportExcelTypeKey() are now documented on the method (javadoc) and on the website converter page (EN and zh-cn).

Tests

New WildcardConverterWriteTest (7 tests):

• xlsx wildcard converter applies (regression guard)
• CSV wildcard converter applies — red before the fix: output was true/false instead of the custom values
• CSV without custom converters unchanged (still true/false)
• explicit STRING-key registration unaffected
• explicit STRING-key registration before the wildcard still wins (registration-order guard)
• explicit STRING-key registration after the wildcard still wins (registration-order guard)
• wildcard converter present under both keys in workbook and sheet holders (covers both registration sites, including sheet-level replay)

Red/green verified: the failing cases were reproduced against the pre-fix code, then turned green by the fix.

Full suite: 1051/1051 green; spotless and javadoc pass.

Fixes #1045
Fixes #1056

[Copilot]

🟡 Changes recommended

The current wildcard registration logic can overwrite explicitly-registered (JavaType, STRING) converters depending on registration order, and one new test reads the “workbook” converter map via currentWriteHolder() which may not actually be the workbook holder after a write.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR fixes a long-standing inconsistency where custom write converters registered with a wildcard excel-type key (supportExcelTypeKey() == null) were applied for XLSX writes but silently ignored for CSV writes (which force converter lookup under (JavaType, STRING)). The approach is to duplicate wildcard registrations under the CSV lookup key without changing the converter resolution logic.

Changes:

• Register wildcard-key custom converters under both (JavaType, null) and (JavaType, STRING) during write-holder initialization.
• Add a new regression test suite covering wildcard vs explicit converter behavior across XLSX/CSV and across workbook/sheet holder registration paths.
• Document the supportExcelTypeKey() == null wildcard semantics in both EN and zh-cn docs and in the Converter javadoc.

File summaries

File	Description
fesod-sheet/src/main/java/org/apache/fesod/sheet/write/metadata/holder/AbstractWriteHolder.java	Adds helper-based converter registration and duplicates wildcard registrations for CSV compatibility.
fesod-sheet/src/main/java/org/apache/fesod/sheet/converters/Converter.java	Expands javadoc to document wildcard semantics and CSV behavior.
fesod-sheet/src/test/java/org/apache/fesod/sheet/converter/WildcardConverterWriteTest.java	Adds regression tests for wildcard converter application and registration in holder maps.
website/docs/sheet/write/converter.md	Documents wildcard-key behavior (EN).
website/i18n/zh-cn/docusaurus-plugin-content-docs/current/sheet/write/converter.md	Documents wildcard-key behavior (zh-cn).

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Mikkey-f]

Addressed: the (JavaType, STRING) registration now only replaces the occupant while it is still the built-in default converter (identity-checked against DefaultConverterLoader.loadDefaultWriteConverter()), so an explicitly registered converter is never displaced regardless of registration order; supportExcelTypeKey() is cached in a local. Regression test csvExplicitStringKeyRegisteredBeforeWildcardStillWins covers the order you described.

[Mikkey-f]

Addressed: workbookMap now reads writeWorkbookHolder().converterMap() — after write() the current holder is the sheet holder, so the two maps were aliased. The writer is now closed via try-with-resources.

[BigDataDZ]

Reviewed as the author of the read-side counterpart (#1086) - this is the right shape for the write side, and I can confirm the registration semantics here are consistent with what the read-side fix ended up doing.

What stands out as particularly well done:

• The putCustomConverter guard is the part I was worried about before reading it: expanding a wildcard (JavaType, null) registration into (JavaType, STRING) would naively displace an explicitly registered (JavaType, STRING) converter depending on registration order. Taking over the slot only when the occupant is absent or identity-equal to the built-in default (== DefaultConverterLoader.loadDefaultWriteConverter().get(stringKey)) makes the outcome order-independent, and the shallow parent copy keeps the identity check valid for inherited holders too.
• The wildcard's (JavaType, STRING) slot is also found by the xlsx path when a field's target cell type is explicitly STRING, so behavior is consistent for both formats regardless of which key the executor builds - a bonus beyond the CSV fix itself.
• The Converter javadoc + bilingual docs make the null contract explicit, which was my main gripe as a user hitting this in #1056's repro.

Two non-blocking thoughts:

1. Once this and #1086 both land, the "Converter Resolution Priority" docs section could use a short paragraph describing the read/write asymmetry: read resolves by the concrete cell type key (xlsx/xls included), write resolves wildcard on xlsx/xls but forced STRING on CSV. Happy to draft that paragraph when either merges, so the two sides are documented together rather than in two places.
2. The class-level javadoc on XlsFormatTrackingListener-style details aside, WildcardConverterWriteTest covering explicit-priority preservation (explicit (JavaType, STRING) registered before AND after a wildcard registration) would lock the order-independence claim - if it is already in there, even better.

Nice work - this unblocks the converter story for CSV writes that #1045/#1056 have been waiting on.

[Mikkey-f]

Thanks for the careful read — and for checking the guard against the shallow parent copy specifically. Order-independence was the exact thing Copilot's earlier review flagged, so it's good to have it confirmed from the read-side perspective as well.

On your two points:

1. Docs paragraph — agreed it should read as one story rather than two, and I'd rather you draft it: you have the read-side resolution order freshest, and it will come out more naturally from that side. Happy to review or co-write from the write side whenever you want to start.

2. Registration-order test — added csvExplicitStringKeyRegisteredAfterWildcardStillWins (explicit (JavaType, STRING) registered after the wildcard; both rows asserted), so the invariant is now locked in both orders alongside the existing csvExplicitStringKeyRegisteredBeforeWildcardStillWins.

Also merged current main into the branch, so it's no longer behind.

[BigDataDZ]

Great - here is a draft for the shared paragraph, written to drop into the "Converter Resolution Priority" section of both docs pages (read and write). Wording follows what our two implementations actually do:

---

Converter resolution across read and write

Registered converters are looked up by the key (JavaType, CellDataType), but the two directions build that key differently, and both give priority to explicit registrations over wildcard ones.

Write. The target cell type is null for xlsx/xls (the converter decides the cell type it produces) and forced to STRING for CSV. A converter registered with supportExcelTypeKey() == null (the wildcard key (JavaType, null)) therefore wins on xlsx/xls writes directly, and is additionally registered under (JavaType, STRING) so CSV honors it too. That extra registration only takes over the STRING slot when it is still held by the built-in default, so an explicitly registered converter is never displaced, regardless of registration order.

Read. Cells carry a concrete type from the file, so every format resolves (JavaType, cellData.getType()) - e.g. (Boolean, STRING) for a text cell. A wildcard read registration is expanded under every concrete cell-type key (overwriting built-in defaults, skipping EMPTY), while keys already held by an explicitly registered converter are left untouched - again, explicit wins.

In short: one wildcard registration covers every format and every cell type, an explicit registration always beats the wildcard, and user registrations always beat built-ins.

---

If the wording works for you, I will add it to the read-side converter docs in #1086 (both EN and zh-CN, mirroring what you did for the write page); if #1069 merges first, feel free to fold the same text into the write page so neither ships a half story. Edits welcome, it is meant as a starting point.

[Mikkey-f]

Thanks — this reads well, and I went through it against both implementations line by line. The write-side description matches #1069 exactly (including why the extra (JavaType, STRING) registration is order-independent: it only takes the slot while the built-in default still holds it), and the read-side description matches the expansion in #1086 (explicit keys claimed first, EMPTY skipped, built-ins overridden).

One addition worth considering: with the header-path fix in #1098, the header row resolves against the built-in converters only, so "user registrations always beat built-ins" could be read as universal. A closing sentence would keep the summary honest, e.g. "The header row is resolved against the built-in converters only: registered converters apply to data cells, never to the header text used for @ExcelProperty name matching." It only becomes accurate once #1098 lands, so adding it after that merges may be best — your call.

On placement: agreed on one story rather than two. Go ahead and add it to the read page in #1086. I'll fold the same paragraph (plus that header note once #1098 is in) into the write page after either PR lands, so no page documents behaviour that has not shipped yet — but if you'd rather cover both pages in #1086, that works for me too.

[BigDataDZ]

Done — added the shared paragraph to the read page in #1086 (EN and zh-CN), under a "Converter Resolution Priority" section so it drops into the write page's same-named section unchanged. Commit: e717b5d.

On the header note: agreed it should wait until #1098 lands, so I left it out — the read page now only describes shipped behaviour. Over to you for the write-page fold-in once either PR merges.