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 the SafariDriverService class and its associated Bazel build file to improve nullability handling and enhance type safety using @Nullable annotations. These changes aim to make the codebase more robust and compatible with tools that rely on nullability annotations.
Nullability Enhancements in SafariDriverService:
Added @Nullable annotations to method parameters: Updated multiple constructors and methods, including createDriverService, to mark parameters like File, Duration, List<String>, and Map<String, String> as nullable, improving clarity and type safety. ([[1]](https://github.com/SeleniumHQ/selenium/pull/16000/files#diff-5ee8455f18e8d287ed7899d66b590a2e6e13197ef14f9a5213e830ff32d0d656L67-R72), [[2]](https://github.com/SeleniumHQ/selenium/pull/16000/files#diff-5ee8455f18e8d287ed7899d66b590a2e6e13197ef14f9a5213e830ff32d0d656L172-R177))
Modified class fields and methods: Added @Nullable annotations to fields such as diagnose and methods like withLogging and withLogFile to indicate optional values. ([[1]](https://github.com/SeleniumHQ/selenium/pull/16000/files#diff-5ee8455f18e8d287ed7899d66b590a2e6e13197ef14f9a5213e830ff32d0d656L129-R130), [[2]](https://github.com/SeleniumHQ/selenium/pull/16000/files#diff-5ee8455f18e8d287ed7899d66b590a2e6e13197ef14f9a5213e830ff32d0d656L142-R149))
Build File Update:
Added @maven//:org_jspecify_jspecify dependency: Updated the Bazel build file to include the org.jspecify.jspecify library, enabling the use of @Nullable annotations in the project. ([java/src/org/openqa/selenium/safari/BUILD.bazelR17](https://github.com/SeleniumHQ/selenium/pull/16000/files#diff-0b0f5cc1965c89ca728657da296b7183ad9abed4555a906b6846798c506130a8R17))
Import Statement Update:
Imported @Nullable annotation: Added org.jspecify.annotations.Nullable to the imports in SafariDriverService.java to support nullability annotations. ([java/src/org/openqa/selenium/safari/SafariDriverService.javaR35](https://github.com/SeleniumHQ/selenium/pull/16000/files#diff-5ee8455f18e8d287ed7899d66b590a2e6e13197ef14f9a5213e830ff32d0d656R35))
💡 Additional Considerations
🔄 Types of changes
Cleanup (formatting, renaming)
PR Type
Enhancement
Description
Add @Nullable annotations to SafariDriverService parameters
Enhance type safety with JSpecify nullability annotations
Update Bazel build configuration for JSpecify dependency
Changes diagram
flowchart LR
A["SafariDriverService"] --> B["Add @Nullable annotations"]
B --> C["Constructor parameters"]
B --> D["Builder methods"]
B --> E["Class fields"]
F["BUILD.bazel"] --> G["Add JSpecify dependency"]
The diagnose field is annotated as @Nullable Boolean but the constructor and method parameters use @Nullable with primitive wrapper types. Verify that all nullable annotations are consistently applied and that the existing code properly handles null values for these parameters.
The method throws an exception regardless of the parameter value, making the @Nullable annotation misleading. Consider removing the annotation since the parameter is never actually used in the method body.
-public Builder withLogFile(@Nullable File logFile) {+public Builder withLogFile(File logFile) {
throw new WebDriverException(
"Can not set log location for Safari; use withLogging(true) and locate log in"
+ " ~/Library/Logs/com.apple.WebDriver/");
}
Apply / Chat
Suggestion importance[1-10]: 6
__
Why: The suggestion correctly identifies that the @Nullable annotation on the logFile parameter is misleading because the method unconditionally throws an exception, making the parameter unused.
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?
🔧 Implementation Notes
This pull request introduces changes to the
SafariDriverServiceclass and its associated Bazel build file to improve nullability handling and enhance type safety using@Nullableannotations. These changes aim to make the codebase more robust and compatible with tools that rely on nullability annotations.Nullability Enhancements in
SafariDriverService:@Nullableannotations to method parameters: Updated multiple constructors and methods, includingcreateDriverService, to mark parameters likeFile,Duration,List<String>, andMap<String, String>as nullable, improving clarity and type safety. ([[1]](https://github.com/SeleniumHQ/selenium/pull/16000/files#diff-5ee8455f18e8d287ed7899d66b590a2e6e13197ef14f9a5213e830ff32d0d656L67-R72),[[2]](https://github.com/SeleniumHQ/selenium/pull/16000/files#diff-5ee8455f18e8d287ed7899d66b590a2e6e13197ef14f9a5213e830ff32d0d656L172-R177))@Nullableannotations to fields such asdiagnoseand methods likewithLoggingandwithLogFileto indicate optional values. ([[1]](https://github.com/SeleniumHQ/selenium/pull/16000/files#diff-5ee8455f18e8d287ed7899d66b590a2e6e13197ef14f9a5213e830ff32d0d656L129-R130),[[2]](https://github.com/SeleniumHQ/selenium/pull/16000/files#diff-5ee8455f18e8d287ed7899d66b590a2e6e13197ef14f9a5213e830ff32d0d656L142-R149))Build File Update:
@maven//:org_jspecify_jspecifydependency: Updated the Bazel build file to include theorg.jspecify.jspecifylibrary, enabling the use of@Nullableannotations in the project. ([java/src/org/openqa/selenium/safari/BUILD.bazelR17](https://github.com/SeleniumHQ/selenium/pull/16000/files#diff-0b0f5cc1965c89ca728657da296b7183ad9abed4555a906b6846798c506130a8R17))Import Statement Update:
@Nullableannotation: Addedorg.jspecify.annotations.Nullableto the imports inSafariDriverService.javato support nullability annotations. ([java/src/org/openqa/selenium/safari/SafariDriverService.javaR35](https://github.com/SeleniumHQ/selenium/pull/16000/files#diff-5ee8455f18e8d287ed7899d66b590a2e6e13197ef14f9a5213e830ff32d0d656R35))💡 Additional Considerations
🔄 Types of changes
PR Type
Enhancement
Description
Add
@Nullableannotations to SafariDriverService parametersEnhance type safety with JSpecify nullability annotations
Update Bazel build configuration for JSpecify dependency
Changes diagram
Changes walkthrough 📝
SafariDriverService.java
Add nullable annotations to parameters and fieldsjava/src/org/openqa/selenium/safari/SafariDriverService.java
@Nullableannotation from JSpecify@Nullableto constructor parameters (File, Duration, List, Map)@Nullableto Builder class fielddiagnose@Nullableto Builder methodswithLoggingandwithLogFileBUILD.bazel
Add JSpecify dependency to build configurationjava/src/org/openqa/selenium/safari/BUILD.bazel