fix(dart): keep top-level fromJson/toJson names with --from-map#3000
Merged
Conversation
Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
With
--from-map(methodNamesWithMap), quicktype's Dart output renamed thetop-level free functions
sensordataFromJson/sensordataToJsontosensordataFromMap/sensordataToMap, even though those top-level functionstake/return a JSON
String(viajson.decode/json.encode), not aMap.Only the class-level factory constructor and instance method — which really
do operate on
Map<String, dynamic>— should change name when--from-mapis used.
Repro:
Before the fix:
After the fix:
Root cause
DartRenderer.makeTopLevelDependencyNamesbuilt the top-level encoder/decodernames using the
toJson/fromJsongetters, which are conditional onmethodNamesWithMapand returntoMap/fromMapwhen the option is set.Those getters are also (correctly) used for the class-level
fromMap/toMapmethods emitted by_emitMapEncoderDecoder, so thetop-level functions inherited the same renaming even though they don't
operate on a
Map.Fix
makeTopLevelDependencyNamesnow hard-codes the_toJson/_fromJsonsuffixes for the top-level function names, independent of
methodNamesWithMap. The class-level Map methods are untouched and keepusing the conditional naming, matching the fix suggested by a maintainer in
the issue thread.
Test coverage
test/unit/dart-method-names.test.ts(new): renders Dart both with andwithout the
from-maprenderer option and asserts the top-level functionsare always named
..FromJson/..ToJsonwhile the class-levelfactory/method are named
fromMap/toMaponly when the option is set(and
fromJson/toJsonotherwise, i.e. no regression to the defaultcase).
test/languages.ts: added afrom-maprenderer-option variant to the Dartfixture's
quickTestRendererOptions(usingsimple-object.json) so theend-to-end fixture also exercises code generation with this option.
Verification
unfixed code, then applied the fix and confirmed it passes.
npm run buildpasses.npm run test:unit— 165/165 tests pass.fromJson/toJsontop-level names while class-levelfromMap/toMapnames are preserved, and confirmed no regression when
--from-mapisomitted.
dartbinary) is not available in this sandboxedenvironment, so the
FIXTURE=dart script/testend-to-end run itselfwasn't executed locally; the new
from-mapfixture variant will bevalidated by CI.
Fixes #1719
🤖 Generated with Claude Code