[dotnet] [bidi] Simplify conversion to LocalValue

[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.

Motivation and Context

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

• 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, Tests

---

Description

• Simplified the usage of LocalValue by introducing static factory methods.

• Added comprehensive type conversion support for LocalValue, including JSON nodes and .NET types.

• Enhanced test coverage for LocalValue conversions and usage in function calls.

• Improved readability and maintainability of LocalValue-related code.

---

Changes walkthrough 📝

	Relevant files
Enhancement	LocalValue.cs Enhanced `LocalValue` with static methods and type conversions dotnet/src/webdriver/BiDi/Modules/Script/LocalValue.cs Introduced static factory methods for creating LocalValue instances. Added support for converting JSON nodes and .NET types to LocalValue. Implemented utility methods for handling various LocalValue types. Improved error handling and validation for unsupported cases. +219/-10
Tests	CallFunctionLocalValueTest.cs Updated and extended tests for `LocalValue` functionality dotnet/test/common/BiDi/Script/CallFunctionLocalValueTest.cs Updated tests to use new LocalValue static factory methods. Added new test cases for LocalValue types like True, False, and BigInt. Improved test readability and consistency with updated LocalValue usage. +34/-18

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[qodo-code-review]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Potential Null Reference The String method throws an ArgumentNullException if value is null, but the implicit operator for string doesn't perform this check, potentially leading to inconsistent behavior. public static implicit operator LocalValue(bool? value) { return value is bool b ? (b ? True : False) : Null; } public static implicit operator LocalValue(int? value) { return value is int i ? Number(i) : Null; } public static implicit operator LocalValue(double? value) { return value is double d ? Number(d) : Null; } public static implicit operator LocalValue(string? value) { return value is null ? Null : String(value); } Incomplete Error Handling The ConvertFrom method for objects doesn't handle all possible object types and might throw unexpected exceptions for unsupported types. // TODO: Extend converting from types public static LocalValue ConvertFrom(object? value) { switch (value) { case LocalValue localValue: return localValue; case null: return Null; case bool b: return b ? True : False; case int i: return Number(i); case double d: return Number(d); case string str: return String(str); case IEnumerable<object?> list: return Array(list.Select(ConvertFrom).ToList()); case object: { var type = value.GetType(); var properties = type.GetProperties(System.Reflection.BindingFlags.Public | System.Reflection.BindingFlags.Instance); List<List<LocalValue>> values = []; foreach (var property in properties) { values.Add([property.Name, ConvertFrom(property.GetValue(value))]); } return Object(values); } } }

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Use regex.Pattern instead of ToString The Regex method uses regex.ToString() to get the pattern, but this returns the pattern with options which could lead to duplicate flags. Use regex.Pattern instead to get just the pattern without options. dotnet/src/webdriver/BiDi/Modules/Script/LocalValue.cs [230-282] public static RegExpLocalValue Regex(Regex regex) { RegexOptions options = regex.Options; if (options == RegexOptions.None) { - return new RegExpLocalValue(new RegExpValue(regex.ToString())); + return new RegExpLocalValue(new RegExpValue(regex.Pattern)); } string flags = string.Empty; const RegexOptions NonBacktracking = (RegexOptions)1024; #if NET8_0_OR_GREATER Debug.Assert(NonBacktracking == RegexOptions.NonBacktracking); #endif const RegexOptions NonApplicableOptions = RegexOptions.Compiled | NonBacktracking; const RegexOptions UnsupportedOptions = RegexOptions.ExplicitCapture | RegexOptions.IgnorePatternWhitespace | RegexOptions.RightToLeft | RegexOptions.CultureInvariant; options &= ~NonApplicableOptions; if ((options & UnsupportedOptions) != 0) { throw new NotSupportedException($"The selected RegEx options are not supported in BiDi: {options & UnsupportedOptions}"); } if ((options & RegexOptions.IgnoreCase) != 0) { flags += "i"; options = options & ~RegexOptions.IgnoreCase; } if ((options & RegexOptions.Multiline) != 0) { options = options & ~RegexOptions.Multiline; flags += "m"; } if ((options & RegexOptions.Singleline) != 0) { options = options & ~RegexOptions.Singleline; flags += "s"; } Debug.Assert(options == RegexOptions.None); - return new RegExpLocalValue(new RegExpValue(regex.ToString()) { Flags = flags }); + return new RegExpLocalValue(new RegExpValue(regex.Pattern) { Flags = flags }); } Apply this suggestion Suggestion importance[1-10]: 9 __ Why: Using regex.ToString() returns the pattern with options included, which could lead to duplicate flags when the method explicitly adds flags. Using regex.Pattern provides just the pattern without options, preventing potential issues with duplicate or conflicting flags.	High
	Use round-trip format for BigInteger The BigInt method currently converts a BigInteger to string without any format specification, which could lead to incorrect representation. Use ToString("R") to ensure the round-trip format is used for accurate numeric representation. dotnet/src/webdriver/BiDi/Modules/Script/LocalValue.cs [188-191] public static BigIntLocalValue BigInt(BigInteger value) { - return new BigIntLocalValue(value.ToString()); + return new BigIntLocalValue(value.ToString("R")); } Apply this suggestion Suggestion importance[1-10]: 7 __ Why: Using ToString("R") for BigInteger ensures accurate numeric representation with the round-trip format, preventing potential data loss or incorrect representation when converting between .NET and JavaScript BigInt values.	Medium
Update

[nvborisenko]

I am concerned in general to introduce "implicit/explicit" casting. In my mind .NET types and BiDi types have to be mapped 1 to 1. I feel it should be a rule, exactly 1 to 1. I agree that this feature is amazing and very helpful, but I guess we are missing something that will be painful in the future. Do you feel the same?

(object casting is off-topic)

[nvborisenko]

Thanks Mike!