Nullability for browsers

java/src/org/openqa/selenium/chrome/ChromeDriverService.java

@@ -18,7 +18,6 @@
 package org.openqa.selenium.chrome;
 
 import static java.util.Collections.unmodifiableList;
-import static java.util.Collections.unmodifiableMap;
 import static org.openqa.selenium.remote.Browser.CHROME;
 
 import com.google.auto.service.AutoService;
@@ -27,7 +26,6 @@
 import java.io.OutputStream;
 import java.time.Duration;
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -110,12 +108,7 @@ public ChromeDriverService(
       @Nullable List<String> args,
       @Nullable Map<String, String> environment)
       throws IOException {
-    super(
-        executable,
-        port,
-        timeout,
-        unmodifiableList(new ArrayList<>(args)),
-        unmodifiableMap(new HashMap<>(environment)));
+    super(executable, port, timeout, args, environment);
   }
 
   public String getDriverName() {

java/src/org/openqa/selenium/edge/EdgeDriverInfo.java

@@ -21,7 +21,6 @@
 
 import com.google.auto.service.AutoService;
 import java.util.Optional;
-import java.util.logging.Logger;
 import org.openqa.selenium.Capabilities;
 import org.openqa.selenium.ImmutableCapabilities;
 import org.openqa.selenium.SessionNotCreatedException;
@@ -34,8 +33,6 @@
 
 @AutoService(WebDriverInfo.class)
 public class EdgeDriverInfo extends ChromiumDriverInfo {
-  private static final Logger LOG = Logger.getLogger(EdgeDriverInfo.class.getName());
-
   @Override
   public String getDisplayName() {
     return "Edge";

java/src/org/openqa/selenium/edge/EdgeDriverService.java

@@ -104,7 +104,7 @@ public EdgeDriverService(
       @Nullable List<String> args,
       @Nullable Map<String, String> environment)
       throws IOException {
-    super(executable, port, timeout, List.copyOf(args), Map.copyOf(environment));
+    super(executable, port, timeout, args, environment);
   }
 
   public String getDriverName() {

java/src/org/openqa/selenium/edge/package-info.java

@@ -0,0 +1,50 @@
+// Licensed to the Software Freedom Conservancy (SFC) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The SFC licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+/**
+ * Mechanisms to configure and run selenium via the command line. There are two key classes {@link
+ * org.openqa.selenium.cli.CliCommand} and {@link org.openqa.selenium.grid.config.HasRoles}.
+ * Ultimately, these are used to build a {@link org.openqa.selenium.grid.config.Config} instance,
+ * for which there are strongly-typed role-specific classes that use a {@code Config}, such as
+ * {@link org.openqa.selenium.grid.node.docker.DockerOptions}.
+ *
+ * <p>Assuming your {@code CliCommand} extends {@link org.openqa.selenium.grid.TemplateGridCommand},
+ * the process for building the set of flags to use is:
+ *
+ * <ol>
+ *   <li>The default flags are added (these are {@link org.openqa.selenium.grid.server.HelpFlags}
+ *       and {@link org.openqa.selenium.grid.config.ConfigFlags}
+ *   <li>{@link java.util.ServiceLoader} is used to find all implementations of {@link
+ *       org.openqa.selenium.grid.config.HasRoles} where {@link
+ *       org.openqa.selenium.grid.config.HasRoles#getRoles()} is contained within {@link
+ *       org.openqa.selenium.cli.CliCommand#getConfigurableRoles()}.
+ *   <li>Finally all flags returned by {@link
+ *       org.openqa.selenium.grid.TemplateGridCommand#getFlagObjects()} are added.
+ * </ol>
+ *
+ * <p>The flags are then used by JCommander to parse the command arguments. Once that's done, the
+ * raw flags are converted to a {@link org.openqa.selenium.grid.config.Config} by combining all of
+ * the flag objects with system properties and environment variables. This implies that each flag
+ * object has annotated each field with {@link org.openqa.selenium.grid.config.ConfigValue}.
+ *
+ * <p>Ultimately, this means that flag objects have all (most?) fields annotated with JCommander's
+ * {@link com.beust.jcommander.Parameter} annotation as well as {@code ConfigValue}.
+ */
+@NullMarked
+package org.openqa.selenium.edge;
+
+import org.jspecify.annotations.NullMarked;

java/src/org/openqa/selenium/federatedcredentialmanagement/FederatedCredentialManagementAccount.java

@@ -18,7 +18,6 @@
 package org.openqa.selenium.federatedcredentialmanagement;
 
 import java.util.Map;
-import org.jspecify.annotations.NullMarked;
 import org.jspecify.annotations.Nullable;
 
 /**
@@ -29,7 +28,6 @@
  * @see <a href="https://w3c-fedid.github.io/FedCM/#webdriver-accountlist">
  *     https://w3c-fedid.github.io/FedCM/#webdriver-accountlist</a>
  */
-@NullMarked
 public class FederatedCredentialManagementAccount {
   private final @Nullable String accountId;
   private final @Nullable String email;
@@ -57,16 +55,16 @@ public class FederatedCredentialManagementAccount {
   public static final String LOGIN_STATE_SIGNIN = "SignIn";
   public static final String LOGIN_STATE_SIGNUP = "SignUp";
 
-  public FederatedCredentialManagementAccount(Map<String, String> dict) {
-    accountId = (String) dict.getOrDefault("accountId", null);
-    email = (String) dict.getOrDefault("email", null);
-    name = (String) dict.getOrDefault("name", null);
-    givenName = (String) dict.getOrDefault("givenName", null);
-    pictureUrl = (String) dict.getOrDefault("pictureUrl", null);
-    idpConfigUrl = (String) dict.getOrDefault("idpConfigUrl", null);
-    loginState = (String) dict.getOrDefault("loginState", null);
-    termsOfServiceUrl = (String) dict.getOrDefault("termsOfServiceUrl", null);
-    privacyPolicyUrl = (String) dict.getOrDefault("privacyPolicyUrl", null);
+  public FederatedCredentialManagementAccount(Map<String, @Nullable String> dict) {
+    accountId = dict.getOrDefault("accountId", null);
+    email = dict.getOrDefault("email", null);
+    name = dict.getOrDefault("name", null);
+    givenName = dict.getOrDefault("givenName", null);
+    pictureUrl = dict.getOrDefault("pictureUrl", null);
+    idpConfigUrl = dict.getOrDefault("idpConfigUrl", null);
+    loginState = dict.getOrDefault("loginState", null);
+    termsOfServiceUrl = dict.getOrDefault("termsOfServiceUrl", null);
+    privacyPolicyUrl = dict.getOrDefault("privacyPolicyUrl", null);
   }
 
   public @Nullable String getAccountid() {

java/src/org/openqa/selenium/federatedcredentialmanagement/FederatedCredentialManagementDialog.java

@@ -18,15 +18,13 @@
 package org.openqa.selenium.federatedcredentialmanagement;
 
 import java.util.List;
-import org.jspecify.annotations.NullMarked;
 import org.jspecify.annotations.Nullable;
 
 /**
  * Represents an open dialog of the Federated Credential Management API.
  *
  * @see <a href="https://w3c-fedid.github.io/FedCM/">https://w3c-fedid.github.io/FedCM/</a>
  */
-@NullMarked
 public interface FederatedCredentialManagementDialog {
 
   String DIALOG_TYPE_ACCOUNT_LIST = "AccountChooser";

java/src/org/openqa/selenium/federatedcredentialmanagement/HasFederatedCredentialManagement.java

@@ -17,13 +17,11 @@
 
 package org.openqa.selenium.federatedcredentialmanagement;
 
-import org.jspecify.annotations.NullMarked;
 import org.jspecify.annotations.Nullable;
 import org.openqa.selenium.Beta;
 
 /** Used by classes to indicate that they can interact with FedCM dialogs. */
 @Beta
-@NullMarked
 public interface HasFederatedCredentialManagement {
   /**
    * Disables the promise rejection delay.

java/src/org/openqa/selenium/federatedcredentialmanagement/package-info.java

@@ -0,0 +1,50 @@
+// Licensed to the Software Freedom Conservancy (SFC) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The SFC licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+/**
+ * Mechanisms to configure and run selenium via the command line. There are two key classes {@link
+ * org.openqa.selenium.cli.CliCommand} and {@link org.openqa.selenium.grid.config.HasRoles}.
+ * Ultimately, these are used to build a {@link org.openqa.selenium.grid.config.Config} instance,
+ * for which there are strongly-typed role-specific classes that use a {@code Config}, such as
+ * {@link org.openqa.selenium.grid.node.docker.DockerOptions}.
+ *
+ * <p>Assuming your {@code CliCommand} extends {@link org.openqa.selenium.grid.TemplateGridCommand},
+ * the process for building the set of flags to use is:
+ *
+ * <ol>
+ *   <li>The default flags are added (these are {@link org.openqa.selenium.grid.server.HelpFlags}
+ *       and {@link org.openqa.selenium.grid.config.ConfigFlags}
+ *   <li>{@link java.util.ServiceLoader} is used to find all implementations of {@link
+ *       org.openqa.selenium.grid.config.HasRoles} where {@link
+ *       org.openqa.selenium.grid.config.HasRoles#getRoles()} is contained within {@link
+ *       org.openqa.selenium.cli.CliCommand#getConfigurableRoles()}.
+ *   <li>Finally all flags returned by {@link
+ *       org.openqa.selenium.grid.TemplateGridCommand#getFlagObjects()} are added.
+ * </ol>
+ *
+ * <p>The flags are then used by JCommander to parse the command arguments. Once that's done, the
+ * raw flags are converted to a {@link org.openqa.selenium.grid.config.Config} by combining all of
+ * the flag objects with system properties and environment variables. This implies that each flag
+ * object has annotated each field with {@link org.openqa.selenium.grid.config.ConfigValue}.
+ *
+ * <p>Ultimately, this means that flag objects have all (most?) fields annotated with JCommander's
+ * {@link com.beust.jcommander.Parameter} annotation as well as {@code ConfigValue}.
+ */
+@NullMarked
+package org.openqa.selenium.federatedcredentialmanagement;
+
+import org.jspecify.annotations.NullMarked;

java/src/org/openqa/selenium/firefox/AddHasContext.java

@@ -69,8 +69,7 @@ public void setContext(FirefoxCommandContext context) {
 
       @Override
       public FirefoxCommandContext getContext() {
-        String context = (String) executeMethod.execute(GET_CONTEXT, null);
-
+        String context = executeMethod.executeRequired(GET_CONTEXT, null);
         return FirefoxCommandContext.fromString(context);
       }
     };

java/src/org/openqa/selenium/firefox/AddHasExtensions.java

@@ -95,17 +95,16 @@ public String installExtension(Path path, Boolean temporary) {
           throw new InvalidArgumentException(path + " is an invalid path", e);
         }
 
-        return (String)
-            executeMethod.execute(
-                INSTALL_EXTENSION, Map.of("addon", encoded, "temporary", temporary));
+        return executeMethod.executeRequired(
+            INSTALL_EXTENSION, Map.of("addon", encoded, "temporary", temporary));
       }
 
       private byte[] zipDirectory(Path path) throws IOException {
         ByteArrayOutputStream baos = new ByteArrayOutputStream();
         try (ZipOutputStream zos = new ZipOutputStream(baos)) {
           Files.walkFileTree(
               path,
-              new SimpleFileVisitor<Path>() {
+              new SimpleFileVisitor<>() {
                 @Override
                 public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
                     throws IOException {

java/src/org/openqa/selenium/firefox/FileExtension.java

@@ -18,6 +18,7 @@
 package org.openqa.selenium.firefox;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
+import static java.util.Objects.requireNonNull;
 import static org.openqa.selenium.json.Json.MAP_TYPE;
 
 import java.io.BufferedInputStream;
@@ -131,7 +132,7 @@ private String readIdFromManifestJson(File root) {
         JsonInput json = new Json().newInput(reader)) {
       String addOnId = null;
 
-      Map<String, Object> manifestObject = json.read(MAP_TYPE);
+      Map<String, Object> manifestObject = json.readNonNull(MAP_TYPE);
       if (manifestObject.get("applications") instanceof Map) {
         Map<?, ?> applicationObj = (Map<?, ?>) manifestObject.get("applications");
         if (applicationObj.get("gecko") instanceof Map) {
@@ -143,10 +144,14 @@ private String readIdFromManifestJson(File root) {
       }
 
       if (addOnId == null || addOnId.isEmpty()) {
-        addOnId =
-            ((String) manifestObject.get("name")).replaceAll(" ", "")
-                + "@"
-                + manifestObject.get("version");
+        String name =
+            (String)
+                requireNonNull(
+                    manifestObject.get("name"),
+                    () ->
+                        "Manifest should contain either 'applications' or 'name', but received: "
+                            + manifestObject);
+        addOnId = name.replaceAll(" ", "") + "@" + manifestObject.get("version");
       }
 
       return addOnId;
@@ -158,9 +163,8 @@ private String readIdFromManifestJson(File root) {
   }
 
   private String readIdFromInstallRdf(File root) {
+    File installRdf = new File(root, "install.rdf");
     try {
-      File installRdf = new File(root, "install.rdf");
-
       DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
       factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
       factory.setNamespaceAware(true);
@@ -214,7 +218,7 @@ public Iterator<String> getPrefixes(String uri) {
       }
       return id;
     } catch (Exception e) {
-      throw new WebDriverException(e);
+      throw new WebDriverException("Failed to read id from " + installRdf.getAbsolutePath(), e);
     }
   }
 }

java/src/org/openqa/selenium/firefox/FirefoxCommandContext.java

@@ -32,17 +32,15 @@ public enum FirefoxCommandContext {
 
   @Override
   public String toString() {
-    return String.valueOf(text);
+    return text;
   }
 
   public static FirefoxCommandContext fromString(String text) {
-    if (text != null) {
-      for (FirefoxCommandContext b : FirefoxCommandContext.values()) {
-        if (text.equalsIgnoreCase(b.text)) {
-          return b;
-        }
+    for (FirefoxCommandContext b : FirefoxCommandContext.values()) {
+      if (text.equalsIgnoreCase(b.text)) {
+        return b;
       }
     }
-    return null;
+    throw new IllegalArgumentException("Unknown Firefox context: " + text);
   }
 }

java/src/org/openqa/selenium/firefox/FirefoxDriver.java

@@ -27,7 +27,6 @@
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
-import org.jspecify.annotations.NonNull;
 import org.openqa.selenium.Beta;
 import org.openqa.selenium.Capabilities;
 import org.openqa.selenium.ImmutableCapabilities;
@@ -169,6 +168,8 @@ public static RemoteWebDriverBuilder builder() {
 
   /** Check capabilities and proxy if it is set */
   private static Capabilities checkCapabilitiesAndProxy(Capabilities capabilities) {
+    // TODO I think we can remove this null check
+    //noinspection ConstantValue
     if (capabilities == null) {
       return new ImmutableCapabilities();
     }
@@ -184,7 +185,6 @@ private static Capabilities checkCapabilitiesAndProxy(Capabilities capabilities)
     return caps;
   }
 
-  @NonNull
   @Override
   public Capabilities getCapabilities() {
     return capabilities;

java/src/org/openqa/selenium/firefox/FirefoxDriverLogLevel.java

@@ -21,6 +21,7 @@
 import java.util.Locale;
 import java.util.Map;
 import java.util.logging.Level;
+import org.jspecify.annotations.Nullable;
 
 /**
  * <a href="https://github.com/mozilla/geckodriver#log-object">Log levels</a> defined by GeckoDriver
@@ -51,7 +52,8 @@ public String toString() {
     return super.toString().toLowerCase(Locale.ENGLISH);
   }
 
-  public static FirefoxDriverLogLevel fromString(String text) {
+  @Nullable
+  public static FirefoxDriverLogLevel fromString(@Nullable String text) {
     if (text != null) {
       for (FirefoxDriverLogLevel b : FirefoxDriverLogLevel.values()) {
         if (text.equalsIgnoreCase(b.toString())) {
@@ -70,6 +72,7 @@ Map<String, String> toJson() {
     return Collections.singletonMap("level", toString());
   }
 
+  @Nullable
   static FirefoxDriverLogLevel fromJson(Map<String, String> json) {
     return fromString(json.get("level"));
   }