Skip to content

Conversation

@RenderMichael
Copy link
Contributor

@RenderMichael RenderMichael commented Mar 1, 2025

User description

Description

Annotate nullability on capabilities types

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


Description

  • Enabled nullable reference types in multiple files for better null safety.

  • Updated method signatures and properties to support nullable types.

  • Simplified logic using null-coalescing operators and pattern matching.

  • Improved internal data structures with readonly collections and HashSet.


Changes walkthrough 📝

Relevant files
Enhancement
IHasCapabilities.cs
Enable nullable reference types in interface                         

dotnet/src/webdriver/IHasCapabilities.cs

  • Enabled nullable reference types.
+2/-0     
IWritableCapabilities.cs
Enable nullable reference types in writable capabilities 

dotnet/src/webdriver/IWritableCapabilities.cs

  • Enabled nullable reference types.
+2/-0     
DesiredCapabilities.cs
Improve null safety in DesiredCapabilities                             

dotnet/src/webdriver/Remote/DesiredCapabilities.cs

  • Updated constructor to accept nullable dictionary.
  • Improved null safety in methods and properties.
  • Simplified equality checks using pattern matching.
  • +1/-1     
    ReadOnlyDesiredCapabilities.cs
    Enhance null safety in ReadOnlyDesiredCapabilities             

    dotnet/src/webdriver/Remote/ReadOnlyDesiredCapabilities.cs

  • Enabled nullable reference types.
  • Simplified property logic with null-coalescing operators.
  • Improved dictionary handling with TryGetValue.
  • Updated method signatures to support nullable types.
  • +20/-40 
    RemoteSessionSettings.cs
    Improve null safety in RemoteSessionSettings                         

    dotnet/src/webdriver/Remote/RemoteSessionSettings.cs

  • Enabled nullable reference types.
  • Updated private fields to readonly or nullable.
  • Improved dictionary and list handling with nullable types.
  • Enhanced internal methods for null safety.
  • +17/-14 

    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-merge-pro
    Copy link
    Contributor

    qodo-merge-pro bot commented Mar 1, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Null Reference

    The GetCapability method returns nullable object but AcceptInsecureCerts property assumes non-null when casting to bool. This could cause runtime exceptions.

    bool acceptSSLCerts = false; object? capabilityValue = this.GetCapability(CapabilityType.AcceptInsecureCertificates); if (capabilityValue != null) { acceptSSLCerts = (bool)capabilityValue; }
    @qodo-merge-pro
    Copy link
    Contributor

    qodo-merge-pro bot commented Mar 1, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Prevent potential invalid type casting

    Add null check before casting to bool to prevent potential InvalidCastException
    if capabilityValue is not a boolean.

    dotnet/src/webdriver/Remote/ReadOnlyDesiredCapabilities.cs [96-99]

    -if (capabilityValue != null) +if (capabilityValue is bool boolValue) { - acceptSSLCerts = (bool)capabilityValue; + acceptSSLCerts = boolValue; }
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    __

    Why: The suggestion improves type safety by using pattern matching instead of direct casting, preventing potential runtime InvalidCastException if capabilityValue is not a boolean type.

    Medium
    Improve null safety handling

    Use pattern matching to safely handle null case in
    GetAlwaysMatchOptionsAsSerializableDictionary to prevent potential
    NullReferenceException.

    dotnet/src/webdriver/Remote/RemoteSessionSettings.cs [267-270]

     private IDictionary<string, object>? GetAlwaysMatchOptionsAsSerializableDictionary() { - return this.mustMatchDriverOptions?.ToDictionary(); + return this.mustMatchDriverOptions is DriverOptions options ? options.ToDictionary() : null; }
    • Apply this suggestion
    Suggestion importance[1-10]: 4

    __

    Why: While the suggestion uses pattern matching for null checking, the existing null-conditional operator (?.) already provides adequate null safety. The improvement is mainly stylistic with minimal impact on functionality.

    Low
    • More
    @RenderMichael RenderMichael merged commit fc0a3b4 into SeleniumHQ:trunk Mar 1, 2025
    10 checks passed
    @RenderMichael RenderMichael deleted the nullable-capabilities branch March 1, 2025 08:10
    sandeepsuryaprasad pushed a commit to sandeepsuryaprasad/selenium that referenced this pull request Mar 23, 2025
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    1 participant