[dotnet] Add trimming attributes, address some AOT warnings

[RenderMichael]

User description

Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

This improves the story for trimming/AOT using Selenium, specifically addressing non-JSON related warnings.

Motivation and Context

In furtherance of #14480

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• x] I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

PR Type

Enhancement, Other

---

Description

• Enhanced the DevToolsDomains class to improve AOT compatibility by introducing a DomainType struct and using DynamicallyAccessedMembers attributes.
• Improved the JsonEnumMemberConverter class by adding attributes for public fields and updating enum value retrieval for .NET 5.0 or greater.
• Added a new file TrimmingAttributes.cs with custom attributes to support trimming and AOT compatibility, including conditional compilation for different .NET versions.
• Updated the WebDriver.csproj file to include a conditional AOT compatibility property for future .NET versions.

---

Changes walkthrough 📝

	Relevant files
Enhancement	DevToolsDomains.cs Enhance DevToolsDomains for AOT compatibility                        dotnet/src/webdriver/DevTools/DevToolsDomains.cs Introduced DomainType struct to handle trimming. Added DynamicallyAccessedMembers attribute for public constructors. Updated dictionary to use DomainType instead of Type. +18/-8    JsonEnumMemberConverter.cs Improve JsonEnumMemberConverter for AOT compatibility        dotnet/src/webdriver/DevTools/Json/JsonEnumMemberConverter.cs Added DynamicallyAccessedMembers attribute for public fields. Updated enum value retrieval for .NET 5.0 or greater. +9/-4      TrimmingAttributes.cs Add custom trimming attributes for AOT support                      dotnet/src/webdriver/Internal/TrimmingAttributes.cs Added custom attributes for trimming support. Defined DynamicallyAccessedMembers and related attributes. Implemented conditional compilation for different .NET versions. +420/-0
Configuration changes	WebDriver.csproj Update project file for potential AOT compatibility            dotnet/src/webdriver/WebDriver.csproj Added conditional AOT compatibility property for future .NET versions. +5/-0

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 14480 - Partially compliant Fully compliant requirements: Partially addresses AOT compatibility by adding trimming attributes and addressing some trim warnings Not compliant requirements: Full AOT compatibility is not yet achieved Specific components (W3C WebDriver, CDP, W3C BiDi) are not explicitly addressed in this PR

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Potential Performance Impact The introduction of the DomainType struct and the use of DynamicallyAccessedMembers attributes may have an impact on runtime performance. This should be validated to ensure it doesn't introduce significant overhead. Conditional Compilation The use of conditional compilation (#if NET5_0_OR_GREATER) may lead to different behavior across .NET versions. Ensure that this doesn't introduce unexpected issues in older .NET versions. Incomplete Implementation The AOT compatibility property is commented out. This suggests that the AOT compatibility work is not complete and may need further attention in future PRs.

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Best practice	Use a read-only dictionary to prevent unintended modifications Consider using a readonly dictionary for SupportedDevToolsVersions to prevent accidental modifications during runtime. dotnet/src/webdriver/DevTools/DevToolsDomains.cs [37-43] -private static readonly Dictionary<int, DomainType> SupportedDevToolsVersions = new Dictionary<int, DomainType>() +private static readonly IReadOnlyDictionary<int, DomainType> SupportedDevToolsVersions = new Dictionary<int, DomainType>() { { 127, new DomainType(typeof(V127.V127Domains)) }, { 129, new DomainType(typeof(V129.V129Domains)) }, { 128, new DomainType(typeof(V128.V128Domains)) }, { 85, new DomainType(typeof(V85.V85Domains)) } }; Apply this suggestion Suggestion importance[1-10]: 7 Why: Changing the dictionary to a read-only dictionary enhances code safety by preventing accidental modifications at runtime. This is a good practice for maintaining the integrity of static data structures.	7
Enhancement	Use a more concise conditional compilation structure for better readability Consider using a switch expression for better readability and maintainability when handling different .NET versions. dotnet/src/webdriver/DevTools/Json/JsonEnumMemberConverter.cs [20-24] +var values = #if NET5_0_OR_GREATER -TEnum[] values = Enum.GetValues<TEnum>(); + Enum.GetValues<TEnum>(); #else -Array values = Enum.GetValues(type); + Enum.GetValues(type); #endif Apply this suggestion Suggestion importance[1-10]: 6 Why: The suggestion improves readability by simplifying the conditional compilation structure. It makes the code cleaner and easier to maintain without changing its functionality.	6
	Use a conditional property group for future AOT compatibility Consider using a conditional property group for future AOT compatibility instead of commenting it out. dotnet/src/webdriver/WebDriver.csproj [45-48] -<!--TODO when AOT is ready https://github.com/SeleniumHQ/selenium/issues/14480--> -<!--<PropertyGroup Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net8.0'))"> +<PropertyGroup Condition="'$(EnableAOTCompilation)' == 'true' AND $([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net8.0'))"> <IsAotCompatible>true</IsAotCompatible> -</PropertyGroup>--> +</PropertyGroup> Apply this suggestion Suggestion importance[1-10]: 5 Why: The suggestion provides a more structured approach to handle future AOT compatibility, which could be beneficial when the feature becomes available. However, it is speculative and depends on future developments, which slightly reduces its immediate relevance.	5

💡 Need additional feedback ? start a PR chat

[RenderMichael]

This is the kind of workaround necessary when using a Type as a generic argument that requires trimming annotations. Hope this is acceptable.

[RenderMichael]

var here is actually TEnum in .NET, but object in .NET Standard. All use of value is either cast or are ToString()-ed, so there's no runtime difference.

[nvborisenko]

I will start to review after basic #14574 is merged. I thought that CDP requires deeper knowledge, so on hold for now.

[RenderMichael]

@nvborisenko Thanks!

It looks like a lot of the work you're doing could be addressed if System.Text.Json implemented this feature request: dotnet/runtime#98038

It doesn't have much support because this kind of conversion isn't roundtrippable. Maybe Selenium can make the case for this functionality there?

[nvborisenko]

No, Source Generation in Json is very good. The problem I am trying to resolve is that many many years ago wire objects were serialized as objects. And the issue is that object is not compile time available meta type. Microsoft wrote great docs about it.

You referenced to the issue, which is in open state...

[RenderMichael]

@nvborisenko Yeah the object propagation makes things harder. I guess a deeper fix could be to propagate everything as JsonNode (or the derived JsonObject in place of Dictionary<string, object>. There's only a handful of places where the command parameters are exposed to users, like the the Command constructor.

But, that would involve a lot of legwork. I would love to contribute any PRs if that's the direction this project wants to go down.

[nvborisenko]

I am in communication with https://github.com/appium/appium team, they are testing #14574

We should migrate "classic" before moving further.

[nvborisenko]

could be to propagate everything as JsonNode

Unfortunately it is big boom breaking change :(

[RenderMichael]

Assembly.CodeBase always throws an exception in modern .NET.

Luckily, AppDomain.ShadowCopyFiles always returns false in modern .NET; the concept of shadow copying does not exist anymore.

These changes fix warnings and simplify the code path.

[nvborisenko]

We cannot use preprocessor directives. netstandard2.0 can be consumed by .net6. I am not sure what alternatives I can propose, probably just to suppress the warning (only if we cannot find out better way).

[nvborisenko]

Ah, it is even not warning?

[RenderMichael]

There's no problem with including this in .NET standard to be consumed by .NET 6. AppDomain.CurrentDomain.ShadowCopyFiles will just return false like before.

This is just addressing the warning when compiling against .NET 8, produced by Assembly.CodeBase.

[RenderMichael]

It would be nice if modern .NET recognized at compile time, that anything hidden behind AppDomain.CurrentDomain.ShadowCopyFiles is dead code. But that's unfortunately not the case, so this directive is necessary to address warnings

[RenderMichael]

I removed the devtools changes from this PR. This PR doesn't need to handle the AOT compat for that scenario, such changes can come after (maybe paired with some generator changes, who knows).