[dotnet] Solidify nullability of PinnedScript, add test

[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

Enforces nullability of the PinnedScript type, and adds a test. Luckily, much of the API surface of PinnedScript is internal, so this doesn't affect users at all.

Adds some null checks.

Motivation and Context

Contributes to #14640
Adds a test for something hitherto without any.

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

• Enhanced nullability checks across multiple methods, particularly for PinnedScript and related parameters.
• Added documentation for ArgumentNullException in several interfaces and classes.
• Refactored PinnedScript to enable nullability and improve script handling logic.
• Introduced a new test to verify the functionality of pinning and executing JavaScript code, including exception handling for unpinned scripts.

---

Changes walkthrough 📝

	Relevant files
Enhancement	EventFiringWebDriver.cs Add null check and exception documentation for ExecuteScript dotnet/src/support/Events/EventFiringWebDriver.cs Added null check for PinnedScript parameter. Documented ArgumentNullException for ExecuteScript method. +6/-0      IJavaScriptEngine.cs Add null checks and exception documentation in IJavaScriptEngine dotnet/src/webdriver/IJavaScriptEngine.cs Added null checks for scriptName and script parameters. Documented ArgumentNullException for relevant methods. +4/-0      JavaScriptEngine.cs Add null checks and update script handling logic                  dotnet/src/webdriver/JavaScriptEngine.cs Added null checks for script parameter. Updated script creation and removal logic. +21/-4    PinnedScript.cs Enable nullability and refactor PinnedScript methods          dotnet/src/webdriver/PinnedScript.cs Enabled nullability annotations. Refactored script handling methods. +18/-31  WebDriver.cs Add null check and exception documentation for WebDriver  dotnet/src/webdriver/WebDriver.cs Added null check for PinnedScript parameter. Documented ArgumentNullException for ExecuteScript method. +8/-1
Documentation	IJavascriptExecutor.cs Document ArgumentNullException for ExecuteScript method    dotnet/src/webdriver/IJavascriptExecutor.cs Documented ArgumentNullException for ExecuteScript method. +2/-0      ISearchContext.cs Document ArgumentNullException for FindElement method        dotnet/src/webdriver/ISearchContext.cs Documented ArgumentNullException for FindElement method. +2/-0
Tests	ExecutingJavascriptTest.cs Add test for pinning and executing JavaScript code              dotnet/test/common/ExecutingJavascriptTest.cs Added test for pinning and executing JavaScript code. Verified exception handling for unpinned scripts. +25/-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:

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Code Refactoring The PinnedScript class has been significantly refactored. Verify that all public methods and properties are still accessible and functioning as expected. Potential Performance Impact The PinScript method now creates a new GUID for each script. Assess if this could impact performance for applications that frequently pin scripts. Test Coverage A new test for pinning and executing JavaScript has been added. Ensure it covers all relevant scenarios, including error cases.

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Best practice	✅ Make PinnedScript class immutable for better thread safety and design Suggestion Impact: The commit implemented the suggestion by removing the setter for the ScriptId property, making it read-only. code diff: - internal string ScriptId { get; set; } + internal string ScriptId { get; } Consider making the PinnedScript class immutable by using read-only properties and removing the setter for ScriptId. dotnet/src/webdriver/PinnedScript.cs [53-83] public string Handle { get; } public string Source { get; } -internal string ScriptId { get; set; } +internal string ScriptId { get; } Apply this suggestion Suggestion importance[1-10]: 7 Why: This suggestion promotes better design practices by making the class immutable, which can improve thread safety. However, removing the setter for ScriptId might require adjustments in other parts of the code that currently set this property.	7
Enhancement	Simplify null check using null-coalescing operator Consider using the null-coalescing operator (??) to simplify the null check and throw statement for the 'script' parameter. dotnet/src/webdriver/JavaScriptEngine.cs [224-227] -if (script == null) -{ - throw new ArgumentNullException(nameof(script)); -} +script = script ?? throw new ArgumentNullException(nameof(script)); Apply this suggestion Suggestion importance[1-10]: 6 Why: This suggestion offers a more concise way to perform the null check and throw an exception. It's a valid improvement that enhances code readability without changing the functionality.	6
	Use a more specific exception type and message in the assertion for better test precision Consider using a more specific exception type in the assertion for executing an unpinned script, if possible, to make the test more precise. dotnet/test/common/ExecutingJavascriptTest.cs [471-474] Assert.That(() => { _ = ((IJavaScriptExecutor)driver).ExecuteScript(script); -}, Throws.TypeOf<JavaScriptException>()); +}, Throws.TypeOf<InvalidOperationException>().With.Message.Contains("Script is not pinned")); Apply this suggestion Suggestion importance[1-10]: 5 Why: This suggestion improves the test's precision by using a more specific exception type and message. However, it assumes that InvalidOperationException is the correct exception type, which may not be accurate without more context about the expected behavior of unpinned scripts.	5
	Simplify dictionary initialization using a collection initializer Consider using a dictionary field initializer for this.pinnedScripts to improve code readability and reduce the number of lines. dotnet/src/webdriver/JavaScriptEngine.cs [240-242] -PinnedScript pinnedScript = new PinnedScript(script, newScriptHandle, scriptId); -this.pinnedScripts[pinnedScript.Handle] = pinnedScript; +var pinnedScript = new PinnedScript(script, newScriptHandle, scriptId); +this.pinnedScripts = new Dictionary<string, PinnedScript> { { pinnedScript.Handle, pinnedScript } }; return pinnedScript; Apply this suggestion Suggestion importance[1-10]: 3 Why: The suggestion offers a minor improvement in code readability, but it's not entirely accurate. The existing code adds to the dictionary, while the suggested code replaces the entire dictionary, which could lead to unintended consequences if there were other entries.	3

💡 Need additional feedback ? start a PR chat

[RenderMichael]

Just calling out that the exception you get from using an unpinned looks like this:

We can add a better exception message here. Would that be alright?

[nvborisenko]

Just calling out that the exception you get from using an unpinned looks like this:

We can add a better exception message here. Would that be alright?

Current behavior is good in my opinion. It suites "minimize client state".

[nvborisenko]

Thank you, @RenderMichael !