Skip to content

Conversation

asolntsev
Copy link
Contributor

@asolntsev asolntsev commented Oct 23, 2025

User description

So it's not needed to duplicate @NullMarked in every file.


PR Type

Enhancement


Description

  • Consolidate @NullMarked annotation to package level

  • Remove redundant per-class @NullMarked annotations

  • Create package-info.java with package-level nullability declaration

  • Reduce code duplication across org.openqa.selenium.net package


Diagram Walkthrough

flowchart LR
  A["Individual Classes<br/>with @NullMarked"] -->|Consolidate| B["package-info.java<br/>with @NullMarked"]
  B -->|Applies to| C["All classes in<br/>org.openqa.selenium.net"]
  C -->|Result| D["Cleaner codebase<br/>No duplication"]
Loading

File Walkthrough

Relevant files
Cleanup
9 files
DefaultNetworkInterfaceProvider.java
Remove class-level @NullMarked annotation                               
+0/-2     
HostIdentifier.java
Remove class-level @NullMarked annotation                               
+0/-2     
LinuxEphemeralPortRangeDetector.java
Remove class-level @NullMarked annotation                               
+0/-2     
NetworkInterface.java
Remove class-level @NullMarked annotation                               
+0/-2     
NetworkInterfaceProvider.java
Remove interface-level @NullMarked annotation                       
+0/-2     
NetworkUtils.java
Remove class-level @NullMarked annotation                               
+0/-2     
PortProber.java
Remove class-level @NullMarked annotation                               
+0/-2     
UrlChecker.java
Remove class-level @NullMarked annotation                               
+0/-2     
Urls.java
Remove class-level @NullMarked annotation                               
+0/-2     
Enhancement
1 files
package-info.java
Create package-info with package-level @NullMarked             
+21/-0   

So it's not needed to duplicate @NullMarked in every file.
@selenium-ci selenium-ci added the C-java Java Bindings label Oct 23, 2025
@qodo-merge-pro
Copy link
Contributor

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
🎫 No ticket provided
- [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true -->

</details></td></tr>
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

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

@qodo-merge-pro
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Learned
best practice
Ensure executor shutdown and await termination

Use try-with-resources for the InputStream (already done) and also ensure the
ExecutorService is shut down and awaited to terminate; prefer a bounded thread
pool for predictable lifecycle.

java/src/org/openqa/selenium/net/UrlChecker.java [65-104]

-ExecutorService service = Executors.newCachedThreadPool();
+ExecutorService service = Executors.newSingleThreadExecutor();
 try {
   // ...
   HttpURLConnection connection = (HttpURLConnection) url.openConnection();
   connection.setConnectTimeout((int) timeoutUnit.toMillis(timeout));
   connection.setReadTimeout((int) timeoutUnit.toMillis(timeout));
   connection.setInstanceFollowRedirects(followRedirects);
-  try (InputStream ignored =
-      connection.getInputStream()) { // Attempt to read to force a connection
+  try (InputStream ignored = connection.getInputStream()) {
     // ...
   } finally {
     connection.disconnect();
   }
   // ...
 } finally {
   service.shutdownNow();
+  service.awaitTermination(5, TimeUnit.SECONDS);
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Enforce deterministic resource cleanup with try-with-resources to close streams and executors on all paths.

Low
General
Move import before package declaration

In package-info.java, move the import statement before the annotated package
declaration to adhere to common Java conventions.

java/src/org/openqa/selenium/net/package-info.java [18-21]

+import org.jspecify.annotations.NullMarked;
+
 @NullMarked
 package org.openqa.selenium.net;
 
-import org.jspecify.annotations.NullMarked;
-
  • Apply / Chat
Suggestion importance[1-10]: 4

__

Why: The suggestion improves code style by aligning the import statement order with standard Java conventions for package-info.java files, enhancing readability and consistency with automated formatters.

Low
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-java Java Bindings I-cleanup Something needs to be tidied Review effort 1/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants