Skip to content

Conversation

@Havret
Copy link
Owner

@Havret Havret commented Nov 6, 2025

No description provided.

@Havret Havret requested a review from Copilot November 6, 2025 11:53
@Havret Havret merged commit 1302a05 into main Nov 6, 2025
8 checks passed
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds null handling support for primitive protobuf field deserialization. When JSON properties are explicitly set to null, the deserializer now treats them as default values rather than throwing an error.

Key Changes:

  • Added null token check in FieldConverter.Read() to skip processing when null values are encountered
  • Added comprehensive test coverage for null value handling across all primitive protobuf types

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/Protobuf.System.Text.Json/InternalConverters/FieldConverter.cs Added null token check to return early and use default values when converters don't handle nulls
test/Protobuf.System.Text.Json.Tests/SimpleMessageTests.cs Added test case verifying that null values deserialize to default values for all primitive types

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

Assert.Equal(0, deserialized.Sfixed32Property);
Assert.Equal(0L, deserialized.Sfixed64Property);
Assert.False(deserialized.BoolProperty);
}
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

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

Extra trailing space after closing brace should be removed.

Suggested change
}
}

Copilot uses AI. Check for mistakes.
@Havret Havret deleted the Handle-null-values-during-deserialization-of-primitive-types branch November 6, 2025 11:55
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