[dotnet] Add nullability annotations to SessionId

[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

Add nullability annotations to SessionId. Now, new SessionId(null) throws an exception so we do a null check whenever we use it in WebDriver.

Since we override object.Equals, I also added some convenience interfaces and operators.

Motivation and Context

Contributes to #14640

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, bug_fix

---

Description

• Added nullability annotations to the SessionId class to improve type safety.
• Introduced a new static method SessionId.Create to handle potential null values gracefully.
• Implemented IEquatable<SessionId> and IEquatable<string> interfaces for better equality checks.
• Updated GetHashCode and Equals methods to handle null values safely.
• Modified WebDriver and test code to utilize the new SessionId.Create method.

---

Changes walkthrough 📝

	Relevant files
Enhancement	SessionId.cs Enhance `SessionId` with nullability and equality features dotnet/src/webdriver/SessionId.cs Added nullability annotations to SessionId. Introduced SessionId.Create method to handle null values. Implemented IEquatable interfaces for SessionId and string. Updated GetHashCode and Equals methods for null safety. +53/-16  WebDriver.cs Use `SessionId.Create` for session initialization                dotnet/src/webdriver/WebDriver.cs Replaced SessionId constructor with SessionId.Create method. +1/-1
Tests	CommandTests.cs Update tests to use `SessionId.Create` method                        dotnet/test/common/CommandTests.cs Updated test to use SessionId.Create. +1/-1

---

💡 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: 2 🔵🔵⚪⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Null Safety Verify that marking the constructor as obsolete and introducing SessionId.Create() doesn't break existing code that relies on null session IDs String Comparison Validate that using StringComparison.Ordinal for string equality comparison is the correct choice for session ID comparison Null Handling Ensure that the WebDriver class properly handles null session IDs returned from SessionId.Create()

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	Add null check in equality comparison to prevent potential runtime exceptions The Equals(string?) method should check if other is null before performing string comparison to avoid potential NullReferenceException when sessionOpaqueKey is not null but other is null. dotnet/src/webdriver/SessionId.cs [105-108] public bool Equals(string? other) { + if (other is null) + return this.sessionOpaqueKey is null; return string.Equals(this.sessionOpaqueKey, other, StringComparison.Ordinal); } Apply this suggestion Suggestion importance[1-10]: 8 Why: The suggestion addresses a potential bug by adding proper null handling in the Equals method, which is crucial for correct equality comparison behavior and preventing runtime exceptions.	8

💡 Need additional feedback ? start a PR chat

[nvborisenko]

What we have:

• SessionId class cannot accept null as id in ctor - OK
• This class starts to implement something to be opaque to the original ID. In general it is OK, but can we do it in separate PR please (it is kind of one more topic for discussion.)

[RenderMichael]

I think this whole class tries to be an opaque wrapper over a string. It already implements GetHashCode() and object.Equals. The changes here just finish that job. I can remove it if that's preferred, but it doesn't cause any changes.

[nvborisenko]

Not sure, Michael. Can you please research where this "opaque" functionality is used across whole code base? @jimevans do you remember why this class is better than native string?

[RenderMichael]

Looking through the history, this type used to live in the Remote namespace. It was introduced in 2011 and has remained completely unchanged since then. I think it doesn't add value, but it is also completely harmless.

[RenderMichael]

where this "opaque" functionality is used across whole code base?

Once in the code (the WebDriver line that is being changed) and once in tests. Both cases, it is exclusively to conform to the Command constructor:

selenium/dotnet/src/webdriver/Command.cs

Line 59 in 40ea8a4

public Command(SessionId sessionId, string name, Dictionary<string, object> parameters)

Which notably also accepts null.

[nvborisenko]

Big refactoring is definitely OOS. I propose to:

• SessionId class cannot accept null as id in ctor - OK

Nothing more.

[RenderMichael]

Fixed, all irrelevant changes are removed.

[nvborisenko]

Stop, not accepting null as a parameter is not a breaking change, so reverting the PR name back.

[nvborisenko]

Thanks @RenderMichael !