You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This pull request introduces changes to improve nullability annotations in the ChromeDriverService class and its associated methods, enhancing type safety and clarity in handling nullable parameters. Additionally, a dependency on org_jspecify_jspecify is added to the Bazel build file to support these annotations.
Nullability Enhancements in ChromeDriverService:
Constructor and Method Parameters: Updated the ChromeDriverService constructor and several builder methods to include @Nullable annotations for parameters that can accept null values, ensuring better clarity and type safety. (java/src/org/openqa/selenium/chrome/ChromeDriverService.java) [1][2][3][4][5]
Builder Class Fields: Added @Nullable annotations to fields in the Builder class to reflect their potential nullability. (java/src/org/openqa/selenium/chrome/ChromeDriverService.java)
Dependency Updates:
Bazel Build File: Added a dependency on @maven//:org_jspecify_jspecify in the Bazel build file to support @Nullable annotations. (java/src/org/openqa/selenium/chrome/BUILD.bazel)
Import Statements:
Added Import for @Nullable: Included the necessary import statement for org.jspecify.annotations.Nullable in the ChromeDriverService class. (java/src/org/openqa/selenium/chrome/ChromeDriverService.java)
🔧 Implementation Notes
💡 Additional Considerations
🔄 Types of changes
Cleanup (formatting, renaming)
PR Type
Enhancement
Description
Add JSpecify nullable annotations to ChromeDriverService parameters
Update constructor and builder methods with @nullable annotations
Add JSpecify dependency to Bazel build configuration
Improve type safety for nullable parameters
Changes diagram
flowchart LR
A["ChromeDriverService"] --> B["Add @Nullable annotations"]
B --> C["Constructor parameters"]
B --> D["Builder methods"]
B --> E["Builder fields"]
F["BUILD.bazel"] --> G["Add JSpecify dependency"]
The withVerbose method sets this.verbose = false unconditionally, even when the parameter is true. This appears to be a bug that should be addressed - it should set this.verbose = verbose instead.
Adding @nullable annotations to method parameters like withLogLevel and withAllowedListIps may break existing code that expects non-null parameters, and the methods don't appear to handle null values appropriately.
publicBuilderwithLogLevel(@NullableChromiumDriverLogLevellogLevel) {
this.logLevel = logLevel;
this.silent = false;
this.verbose = false;
returnthis;
}
/** * Configures the driver server for silent output. * * @param silent Log no output for true, no changes made if false. * @return A self reference. */publicBuilderwithSilent(booleansilent) {
if (silent) {
this.logLevel = ChromiumDriverLogLevel.OFF;
}
this.silent = false;
returnthis;
}
/** * Configures the driver server verbosity. * * @param verbose Log all output for true, no changes made if false. * @return A self reference. */publicBuilderwithVerbose(booleanverbose) {
if (verbose) {
this.logLevel = ChromiumDriverLogLevel.ALL;
}
this.verbose = false;
returnthis;
}
/** * Configures the comma-separated list of remote IPv4 addresses which are allowed to connect to * the driver server. * * @param allowedListIps Comma-separated list of remote IPv4 addresses. * @return A self reference. */publicBuilderwithAllowedListIps(@NullableStringallowedListIps) {
The verbose field is always set to false regardless of the input parameter value. This appears to be a bug where the field should be set to the parameter value instead.
[To ensure code accuracy, apply this suggestion manually]
Suggestion importance[1-10]: 8
__
Why: The suggestion correctly identifies a pre-existing bug in the withVerbose method where this.verbose is always set to false, rendering the verbose parameter ineffective.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
🔗 Related Issues
fixes #14291
💥 What does this PR do?
This pull request introduces changes to improve nullability annotations in the
ChromeDriverServiceclass and its associated methods, enhancing type safety and clarity in handling nullable parameters. Additionally, a dependency onorg_jspecify_jspecifyis added to the Bazel build file to support these annotations.Nullability Enhancements in
ChromeDriverService:ChromeDriverServiceconstructor and several builder methods to include@Nullableannotations for parameters that can accept null values, ensuring better clarity and type safety. (java/src/org/openqa/selenium/chrome/ChromeDriverService.java) [1] [2] [3] [4] [5]@Nullableannotations to fields in theBuilderclass to reflect their potential nullability. (java/src/org/openqa/selenium/chrome/ChromeDriverService.java)Dependency Updates:
@maven//:org_jspecify_jspecifyin the Bazel build file to support@Nullableannotations. (java/src/org/openqa/selenium/chrome/BUILD.bazel)Import Statements:
@Nullable: Included the necessary import statement fororg.jspecify.annotations.Nullablein theChromeDriverServiceclass. (java/src/org/openqa/selenium/chrome/ChromeDriverService.java)🔧 Implementation Notes
💡 Additional Considerations
🔄 Types of changes
PR Type
Enhancement
Description
Add JSpecify nullable annotations to ChromeDriverService parameters
Update constructor and builder methods with @nullable annotations
Add JSpecify dependency to Bazel build configuration
Improve type safety for nullable parameters
Changes diagram
Changes walkthrough 📝
ChromeDriverService.java
Add nullable annotations to parameters and fieldsjava/src/org/openqa/selenium/chrome/ChromeDriverService.java
environment)
readableTimestamp, etc.)
readableTimestamp)
BUILD.bazel
Add JSpecify dependencyjava/src/org/openqa/selenium/chrome/BUILD.bazel