Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix beginStructure in JsonTreeDecoder when inner structre descriptor is same as outer #2346

Merged
merged 1 commit into from
Jun 29, 2023

Conversation

ionspin
Copy link
Contributor

@ionspin ionspin commented Jun 23, 2023

Attempt 2 (previous pull request and discussion here #2344) at solving #2343. This time instead of returning the same instance of decoder (with invalid position) a new instance is created, and polyDiscriminator and polyDescriptor are passed to the new instance.

This way check for unknown keys in endStructure can properly filter out polymorphic discriminator (by default "type) from potential unknown keys.

Added a simple test that fails with the original code and passes with the fix, but I'm not sure what to call it or if it's in the right place.

@sandwwraith sandwwraith changed the base branch from master to dev June 27, 2023 13:24
@sandwwraith sandwwraith changed the base branch from dev to master June 27, 2023 13:25
Copy link
Member

@sandwwraith sandwwraith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull requests with code changes should be opened against dev. You have to rebase your commit on dev and then change base in the PR (just changing base results in unrelated commits added). Thank you!

@ionspin ionspin changed the base branch from master to dev June 28, 2023 11:23
@ionspin ionspin requested a review from sandwwraith June 28, 2023 11:46
…stance of decoder when the polyDescriptor is same returns a new instance with the same polyDiscriminaton. Fixes Kotlin#2343
@ionspin
Copy link
Contributor Author

ionspin commented Jun 29, 2023

I also squashed the commits.

@ionspin ionspin requested a review from sandwwraith June 29, 2023 09:43
@sandwwraith sandwwraith merged commit 5bba108 into Kotlin:dev Jun 29, 2023
@sandwwraith
Copy link
Member

Great job, thanks!

JesusMcCloud pushed a commit to a-sit-plus/kotlinx.serialization that referenced this pull request Jul 5, 2023
… is same as outer (Kotlin#2346)

Instead of returning the same instance of decoder (with invalid position) a new instance is created, and polyDiscriminator and polyDescriptor are passed to the new instance.

This way check for unknown keys in endStructure can properly filter out polymorphic discriminator (by default "type) from potential unknown keys.

Fixes Kotlin#2343
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants