Skip to content

Fix serialization priority and improve deserialization in ArrayReflectionConvertor#30

Merged
IvanMurzak merged 6 commits into
mainfrom
fix/array-reflection-convertor
Nov 28, 2025
Merged

Fix serialization priority and improve deserialization in ArrayReflectionConvertor#30
IvanMurzak merged 6 commits into
mainfrom
fix/array-reflection-convertor

Conversation

@IvanMurzak
Copy link
Copy Markdown
Owner

Adjust serialization priority for string types and enhance the array deserialization logic by adding a new method for parsing elements to members.

@IvanMurzak IvanMurzak self-assigned this Nov 28, 2025
@IvanMurzak IvanMurzak requested a review from Copilot November 28, 2025 09:16
@IvanMurzak IvanMurzak added the bug Something isn't working label Nov 28, 2025
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 28, 2025

Test Results

    3 files      3 suites   3m 5s ⏱️
  578 tests   578 ✅ 0 💤 0 ❌
1 696 runs  1 696 ✅ 0 💤 0 ❌

Results for commit 908b5e4.

♻️ This comment has been updated with latest results.

Copy link
Copy Markdown
Contributor

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 refactors array deserialization logic in the ArrayReflectionConvertor to work directly with JSON arrays instead of deserializing through SerializedMemberList, and adjusts the serialization priority to properly exclude strings from array handling.

Key Changes:

  • Strings now explicitly return priority 0 before the general IEnumerable check, preventing them from being treated as arrays
  • Array deserialization refactored to parse JsonElement arrays directly rather than first deserializing to SerializedMemberList
  • New ParseElementToMember method intelligently handles both structured SerializedMember objects and simple JSON values

Reviewed changes

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

File Description
ReflectorNet/src/Convertor/Reflection/ArrayReflectionConvertor.cs Fixed serialization priority for strings by explicitly checking for typeof(string) before the IEnumerable check, preventing strings from being treated as collection types
ReflectorNet/src/Convertor/Reflection/ArrayReflectionConvertor.Deserialize.cs Refactored deserialization to work directly with JSON arrays, removed System.Linq dependency, and added ParseElementToMember method to handle both structured and simple array elements
Comments suppressed due to low confidence (1)

ReflectorNet/src/Convertor/Reflection/ArrayReflectionConvertor.Deserialize.cs:167

                foreach (var element in jsonArray.EnumerateArray())
                {
                    var member = ParseElementToMember(element);

                    var deserializedElement = reflector.Deserialize(
                        member,
                        fallbackType: elementType,
                        depth: depth + 1,
                        stringBuilder: stringBuilder,
                        logger: logger);

                    addMethod.Invoke(list, new[] { deserializedElement });
                }

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

Comment thread ReflectorNet/src/Convertor/Reflection/ArrayReflectionConvertor.Deserialize.cs Outdated
Comment thread ReflectorNet/src/Convertor/Reflection/ArrayReflectionConvertor.Deserialize.cs Outdated
Copy link
Copy Markdown
Contributor

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

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

Comments suppressed due to low confidence (1)

ReflectorNet/src/Convertor/Reflection/ArrayReflectionConvertor.Deserialize.cs:167

                foreach (var element in jsonArray.EnumerateArray())
                {
                    var member = ParseElementToMember(element);

                    var deserializedElement = reflector.Deserialize(
                        member,
                        fallbackType: elementType,
                        depth: depth + 1,
                        stringBuilder: stringBuilder,
                        logger: logger);

                    addMethod.Invoke(list, new[] { deserializedElement });
                }

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

@IvanMurzak IvanMurzak merged commit d912cf1 into main Nov 28, 2025
2 checks passed
@IvanMurzak IvanMurzak deleted the fix/array-reflection-convertor branch November 28, 2025 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants