[java] specify nullability in package org.openqa.selenium.bidi.log

[asolntsev]

🔗 Related Issues

Part of #14291

💥 What does this PR do?

Marks the whole package org.openqa.selenium.bidi.log as non-nullable by default.

🔄 Types of changes

• Cleanup (formatting, renaming)

[qodo-code-review]

PR Type

Enhancement, Tests

---

Description

• Add nullability annotations to org.openqa.selenium.bidi.log package

• Enforce non-null validation using Require utility methods

• Improve JavaDoc formatting with proper block comments

• Refactor code for better null-safety and consistency

---

File Walkthrough

	Relevant files
Documentation	BaseLogEntry.java Convert comment to JavaDoc format                                                java/src/org/openqa/selenium/bidi/log/BaseLogEntry.java Convert inline comment to proper JavaDoc block format Update reference link to BiDi specification +3/-2      LogEntry.java Convert comment to JavaDoc format                                                java/src/org/openqa/selenium/bidi/log/LogEntry.java Convert inline comment to proper JavaDoc block format Update reference link to BiDi specification +3/-2
Enhancement	ConsoleLogEntry.java Add null validation and improve code consistency                  java/src/org/openqa/selenium/bidi/log/ConsoleLogEntry.java Add Require import for null validation Convert timestamp from long to Long for nullable handling Add null checks on all required fields in fromJson() method Replace super. calls with direct method calls in toJson() Add conditional check for optional stackTrace field +22/-10  GenericLogEntry.java Add null validation and improve code consistency                  java/src/org/openqa/selenium/bidi/log/GenericLogEntry.java Add Require import for null validation Convert timestamp from long to Long for nullable handling Add null checks on all required fields in fromJson() method Replace super. calls with direct method calls in toJson() +15/-7    JavascriptLogEntry.java Remove redundancy and add null validation                                java/src/org/openqa/selenium/bidi/log/JavascriptLogEntry.java Remove redundant type field and getType() method Add Require import for null validation Convert timestamp from long to Long for nullable handling Add null checks on all required fields in fromJson() method Replace super. calls with direct method calls in toJson() +18/-16  StackFrame.java Add validation and simplify map creation                                  java/src/org/openqa/selenium/bidi/log/StackFrame.java Add Require import for validation Add non-negative validation for lineNumber and columnNumber in constructor Convert lineNumber and columnNumber from int to Integer in fromJson() Add null checks on all required fields in fromJson() method Simplify toJson() to use Map.of() instead of TreeMap Remove unused imports +18/-18  StackTrace.java Improve immutability and null-safety                                          java/src/org/openqa/selenium/bidi/log/StackTrace.java Make callFrames field private and final Replace Collections.emptyList() with static import emptyList() Use readNonNull() instead of read() for callFrames deserialization Update JavaDoc to proper block format with correct BiDi spec link +7/-7

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🟡	🎫 #14291 🟢 Add JSpecify nullness annotations to Selenium Java code to make nullability of parameters/returns explicit to IDEs and static analyzers. Improve developer experience by making potential null values visible without relying solely on JavaDoc. ⚪ Improve interoperability with Kotlin via explicit nullness information.
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	PR does not match description The PR fails to mark the package as non-nullable because it omits the @NullMarked annotation in a package-info.java file. It incorrectly uses runtime null checks instead of enabling the static analysis described in the PR's goal. Examples: java/src/org/openqa/selenium/bidi/log/ConsoleLogEntry.java [115-123] return new ConsoleLogEntry( Require.nonNull("Log level", level), Require.nonNull("Log source", source), Require.nonNull("Log text", text), Require.nonNull("Log timestamp", timestamp), Require.nonNull("Log type", type), Require.nonNull("Log method", method), Require.nonNull("Log arguments", args), stackTrace); Solution Walkthrough: Before: // In ConsoleLogEntry.java public static ConsoleLogEntry fromJson(JsonInput input) { LogLevel level = null; Source source = null; String text = null; Long timestamp = null; // ... // ... (parse JSON) return new ConsoleLogEntry( Require.nonNull("Log level", level), Require.nonNull("Log source", source), Require.nonNull("Log text", text), Require.nonNull("Log timestamp", timestamp), // ... more runtime checks stackTrace); } After: // In a new file: java/src/org/openqa/selenium/bidi/log/package-info.java @NullMarked package org.openqa.selenium.bidi.log; import org.jspecify.annotations.NullMarked; // --- // With the @NullMarked annotation at the package level, // static analysis tools will enforce non-null contracts for the // entire package's public API, achieving the PR's stated goal. // The runtime checks in `fromJson` methods might still be useful for // defensive programming against malformed input, but they do not // replace the need for static analysis annotations. Suggestion importance[1-10]: 10 __ Why: The suggestion correctly identifies a critical flaw where the PR's implementation (runtime checks) completely contradicts its stated goal and the related ticket's objective (static analysis via @NullMarked annotation).	High
Possible issue	Validate log entry type in constructor In the JavascriptLogEntry constructor, validate that the type parameter is equal to "javascript" to ensure compliance with the WebDriver BiDi specification. java/src/org/openqa/selenium/bidi/log/JavascriptLogEntry.java [35-43] public JavascriptLogEntry( LogLevel level, Source source, String text, long timestamp, String type, @Nullable StackTrace stackTrace) { - super(level, source, text, timestamp, type, stackTrace); + super( + level, + source, + text, + timestamp, + Require.state("Javascript log entry type", type).equalTo("javascript"), + stackTrace); } Apply / Chat Suggestion importance[1-10]: 8 __ Why: The PR refactoring removed the hardcoded "javascript" type, but the new code in fromJson now passes the type from the JSON payload to the constructor without validation. This suggestion correctly points out that the type should be validated to be "javascript" to conform to the BiDi spec, fixing a potential issue introduced by the PR.	Medium
	Add null checks for constructor parameters Add Require.nonNull() checks for the scriptUrl and function parameters in the StackFrame constructor to ensure they are not null. java/src/org/openqa/selenium/bidi/log/StackFrame.java [34-39] public StackFrame(String scriptUrl, String function, int lineNumber, int columnNumber) { - this.url = scriptUrl; - this.functionName = function; + this.url = Require.nonNull("URL", scriptUrl); + this.functionName = Require.nonNull("Function name", function); this.lineNumber = Require.nonNegative("Line number", lineNumber); this.columnNumber = Require.nonNegative("Column number", columnNumber); } Apply / Chat Suggestion importance[1-10]: 7 __ Why: This suggestion correctly identifies that the StackFrame constructor lacks null checks for scriptUrl and function. While the fromJson method in the PR does perform these checks before calling the constructor, adding them to the constructor itself is a good practice for ensuring object invariants and making the class more robust against other instantiation paths.	Medium
General	Make args list immutable In the ConsoleLogEntry constructor, wrap the args list parameter with List.copyOf() before assigning it to the field to ensure immutability. java/src/org/openqa/selenium/bidi/log/ConsoleLogEntry.java [40-52] public ConsoleLogEntry( LogLevel level, Source source, String text, long timestamp, String type, String method, List<RemoteValue> args, @Nullable StackTrace stackTrace) { super(level, source, text, timestamp, type, stackTrace); this.method = method; - this.args = args; + this.args = List.copyOf(args); } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 7 __ Why: This is a valid suggestion that improves the robustness of the ConsoleLogEntry class by making it immutable. Storing a defensive copy of the args list prevents external modifications to the object's internal state, which is a good practice for value objects.	Medium
	Make callFrames immutable In the StackTrace constructor, wrap the callFrames list parameter with List.copyOf() to ensure the internal list is immutable. java/src/org/openqa/selenium/bidi/log/StackTrace.java [36-38] public StackTrace(List<StackFrame> callFrames) { - this.callFrames = callFrames; + this.callFrames = List.copyOf(Require.nonNull("Call frames", callFrames)); } Apply / Chat Suggestion importance[1-10]: 7 __ Why: This is a valid suggestion that improves the robustness of the StackTrace class by making it immutable. Creating a defensive copy of the callFrames list prevents external modifications to the object's internal state, which is a good practice for value objects.	Medium
	Improve error message for missing property In StackTrace.fromJson, replace input.readNonNull() with input.read() followed by a Require.nonNull() check for the callFrames property to provide a more informative error message if it is missing. java/src/org/openqa/selenium/bidi/log/StackTrace.java [44-59] public static StackTrace fromJson(JsonInput input) { - - List<StackFrame> callFrames = emptyList(); + List<StackFrame> callFrames = null; input.beginObject(); while (input.hasNext()) { if ("callFrames".equals(input.nextName())) { - callFrames = input.readNonNull(new TypeToken<List<StackFrame>>() {}.getType()); + callFrames = input.read(new TypeToken<List<StackFrame>>() {}.getType()); } else { input.skipValue(); } } input.endObject(); - return new StackTrace(callFrames); + return new StackTrace(Require.nonNull("Call frames for stack trace", callFrames)); } Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion provides a valid way to improve error messaging by using Require.nonNull instead of readNonNull. This aligns with the pattern used elsewhere in the PR for other required fields, leading to more consistent and informative error handling.	Low
Update