Refactor JsonPropertyInfo initialization infrastructure and implement JsonPropertyInfo.AttributeProvider #71514

eiriktsarpalis · 2022-07-01T00:49:27Z

Makes the following changes:

Refactors JsonPropertyInfo initialization so that a minimal subset of required parameters are read-only and settable via constructors instead of property setters. Simplify initialization logic removing dead branches and unnecessary parameterization.
Simplify the converter resolution logic, removing dead branches and unnecessary parameterization.
Separate reflection-based JsonPropertyInfo metadata initialization into a separate method explicitly invoked by ReflectionJsonTypeInfo.
Apply consistent naming conventions e.g. renaming parentClassType to declaringType, ClrName to MemberName and memberType to typeToConvert. Remove unused parameters.
Implement JsonPropertyInfo.AttributeProvider and IsExtensionData properties as internal APIs.

Contributes to #63686.

ghost · 2022-07-01T00:49:55Z

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

Makes the following changes:

Refactors JsonPropertyInfo initialization so that a minimal subset of required parameters are read-only and settable via constructors instead of property setters. Simplify initialization logic removing dead branches and unnecessary parameterization.
Simplify the converter resolution logic, removing dead branches and unnecessary parameterization.
Separate reflection-based JsonPropertyInfo metadata initialization into a separate method explicitly invoked by ReflectionJsonTypeInfo.
Apply consistent naming conventions e.g. renaming parentClassType to declaringType, ClrName to MemberName and memberType to typeToConvert. Remove unused parameters.
Implement JsonPropertyInfo.AttributeProvider and IsExtensionData properties as internal APIs.

Contributes to #63686.

Author:	eiriktsarpalis
Assignees:	eiriktsarpalis
Labels:	`area-System.Text.Json`
Milestone:	-

...ibraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverter.cs

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs

…ion infrastructure

…ion/Metadata/JsonPropertyInfo.cs

…tion

krwq · 2022-07-04T09:33:40Z

src/libraries/System.Text.Json/src/System/ReflectionExtensions.cs

        private static bool HasJsonConstructorAttribute(ConstructorInfo constructorInfo)
            => constructorInfo.GetCustomAttribute<JsonConstructorAttribute>() != null;
+
+        public static TAttribute? GetUniqueCustomAttribute<TAttribute>(this MemberInfo memberInfo, bool inherit)


nit: consider ICustomAttributeProvider rather than MemberInfo

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs

...raries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/CustomJsonTypeInfoOfT.cs

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs

krwq · 2022-07-04T10:02:08Z

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs

+            DeclaringType = declaringType;
+            PropertyType = propertyType;
+            Options = options;
+            ParentTypeInfo = parentTypeInfo; // null parentTypeInfo means it's not tied yet


should we debug assert that if parentTypeInfo is not null then declaringType == parentTypeInfo.Type?

We could, I'll add in the next round of PRs

krwq · 2022-07-04T10:56:22Z

...es/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionJsonTypeInfoOfT.cs

        }

        private JsonPropertyInfo? AddProperty(
+            Type typeToConvert,


consider: propertyType

krwq

LGTM with couple of suggestions

eiriktsarpalis requested a review from krwq July 1, 2022 00:49

ghost assigned eiriktsarpalis Jul 1, 2022

ghost added the area-System.Text.Json label Jul 1, 2022

eiriktsarpalis added this to the 7.0.0 milestone Jul 1, 2022