Reduce IParseNode Enum methods DynamicallyAccessedMembers scope

[hwoodiwiss]

• Update DynamicallyAccessMembers for IParseNode Enum methods
• Update version and changelog

This will need to be manually updated across the other repos that implement IParseNode

fixes #201

[andrueastman]

Thanks for this @hwoodiwiss

Would you also be able to submit PRs for the serialization libs taking a dependency on IParseNode? If not, I can probably do that once we merge this one in.

[hwoodiwiss]

Thanks for this @hwoodiwiss

Would you also be able to submit PRs for the serialization libs taking a dependency on IParseNode? If not, I can probably do that once we merge this one in.

I'm happy to update the Serialization libraries for this

[hwoodiwiss]

I'm going to need to take a deeper look at this, looking at the usage of GetEnumName we're always going to pass Enum as the generic type, as generic type information isn't available above ReplaceEnumValueByStringRepresentation so we'll have to rely on Runtime type information which will make this incompatible with AOT.

I'll revert this part of the change for now.

[hwoodiwiss]

I think fully supporting AOT compilation is going to need some deeper changes that will likely also require changes to the public API, specifically around RequestInformation.QueryParameters. If you think it makes sense, I'll open an issue that can run long to track and discuss fully supporting AOT.

[baywet]

Please go ahead with the issue. Do you think we should hold to merge this PR to get resolution? or do you believe this PR is already an improvement over the current situation?

[hwoodiwiss]

Please go ahead with the issue. Do you think we should hold to merge this PR to get resolution? or do you believe this PR is already an improvement over the current situation?

Cool, I'll open something this evening.

I think this is still an improvement, it gives a better base going forward, and this was by far the largest producer of trim warnings I saw when compiling AOT, just due to how many times it's used, plus nothing the serialization libraries are doing at the moment requires more than DynamicallyAccessedMemberTypes.PublicFields, and shouldn't need to.

[MichalStrehovsky]

Enumerating fields on enums is always trimming/AOT safe. If the T is restricted to be Enum, it's fine to access public static fields on it and suppress the warning. See dotnet/runtime#97737.

I.e. even if this is GetNames<T>(T o) where T : Enum { o.GetType().GetFields(BindingFlags.Static | .Public) will work and the warning can be suppressed. This guarantees o is some enum and that's enough.

[baywet]

Alright, thanks everyone for the context! I'll hand it over to @andrueastman for a final review, merge and publish!

[hwoodiwiss]

Enumerating fields on enums is always trimming/AOT safe. If the T is restricted to be Enum, it's fine to access public static fields on it and suppress the warning. See dotnet/runtime#97737.

I.e. even if this is GetNames<T>(T o) where T : Enum { o.GetType().GetFields(BindingFlags.Static | .Public) will work and the warning can be suppressed. This guarantees o is some enum and that's enough.

Ahh, okay, that makes sense. I'll amend this to take that into account this evening. Thank you for the info. In which case, after this, the abstractions library should be trimming safe.