[dotnet] Simplify and nullable annotate DriverFinder (SeleniumHQ#15232)

RenderMichael · sandeepsuryaprasad · commit a2c6c41a33c5 · 2025-03-23T15:07:38.000+05:30
* [dotnet] Simplify and nullable annotate `DriverFinder`

* Minimize diffs

* Maintain previous logic

* Better local name

* Minimize diffs

* Minimize diffs further

* Better code style

* Modernate minimal diffs with code style

* Go back to dictionary style

* Use more consistent consts

* Make name the same

* Use single path

* minimize more diffs
diff --git a/dotnet/src/webdriver/Chromium/ChromiumDriver.cs b/dotnet/src/webdriver/Chromium/ChromiumDriver.cs
@@ -168,9 +168,9 @@ private static ICommandExecutor GenerateDriverServiceCommandExecutor(DriverServi
                 string fullServicePath = finder.GetDriverPath();
                 service.DriverServicePath = Path.GetDirectoryName(fullServicePath);
                 service.DriverServiceExecutableName = Path.GetFileName(fullServicePath);
-                if (finder.HasBrowserPath())
+                if (finder.TryGetBrowserPath(out string browserPath))
                 {
-                    options.BinaryLocation = finder.GetBrowserPath();
+                    options.BinaryLocation = browserPath;
                     options.BrowserVersion = null;
                 }
             }
diff --git a/dotnet/src/webdriver/DriverFinder.cs b/dotnet/src/webdriver/DriverFinder.cs
@@ -19,10 +19,13 @@
 
 using System;
 using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.IO;
 using System.Text;
 
+#nullable enable
+
 namespace OpenQA.Selenium
 {
     /// <summary>
@@ -31,17 +34,16 @@ namespace OpenQA.Selenium
     /// </summary>
     public class DriverFinder
     {
-        private DriverOptions options;
+        private readonly DriverOptions options;
         private Dictionary<string, string> paths = new Dictionary<string, string>();
-        private const string BrowserPathKey = "browser_path";
-        private const string DriverPathKey = "driver_path";
 
         /// <summary>
         /// Initializes a new instance of the <see cref="DriverFinder"/> class.
         /// </summary>
+        /// <exception cref="ArgumentNullException">If <paramref name="options"/> is <see langword="null"/>.</exception>
         public DriverFinder(DriverOptions options)
         {
-            this.options = options;
+            this.options = options ?? throw new ArgumentNullException(nameof(options));
         }
 
         /// <summary>
@@ -52,7 +54,7 @@ public DriverFinder(DriverOptions options)
         /// </returns>
         public string GetBrowserPath()
         {
-            return BinaryPaths()[BrowserPathKey];
+            return BinaryPaths()[SeleniumManager.BrowserPathKey];
         }
 
         /// <summary>
@@ -63,14 +65,36 @@ public string GetBrowserPath()
         /// </returns>
         public string GetDriverPath()
         {
-            return BinaryPaths()[DriverPathKey];
+            return BinaryPaths()[SeleniumManager.DriverPathKey];
         }
 
+        /// <summary>
+        /// Gets whether there is a browser path for the given browser on this platform.
+        /// </summary>
+        /// <returns><see langword="true"/> if a browser path exists; otherwise, <see langword="false"/>.</returns>
         public bool HasBrowserPath()
         {
             return !string.IsNullOrWhiteSpace(GetBrowserPath());
         }
 
+        /// <summary>
+        /// Tries to get the browser path, as retrieved by Selenium Manager.
+        /// </summary>
+        /// <param name="browserPath">If the method returns <see langword="true"/>, the full browser path.</param>
+        /// <returns><see langword="true"/> if a browser path exists; otherwise, <see langword="false"/>.</returns>
+        public bool TryGetBrowserPath([NotNullWhen(true)] out string? browserPath)
+        {
+            string? path = GetBrowserPath();
+            if (!string.IsNullOrWhiteSpace(path))
+            {
+                browserPath = path;
+                return true;
+            }
+
+            browserPath = null;
+            return false;
+        }
+
         /// <summary>
         /// Invokes Selenium Manager to get the binaries paths and validates if they exist.
         /// </summary>
@@ -80,29 +104,33 @@ public bool HasBrowserPath()
         /// <exception cref="NoSuchDriverException">If one of the paths does not exist.</exception>
         private Dictionary<string, string> BinaryPaths()
         {
-            if (paths.ContainsKey(DriverPathKey) && !string.IsNullOrWhiteSpace(paths[DriverPathKey]))
+            if (paths.ContainsKey(SeleniumManager.DriverPathKey) && !string.IsNullOrWhiteSpace(paths[SeleniumManager.DriverPathKey]))
             {
                 return paths;
             }
+
             Dictionary<string, string> binaryPaths = SeleniumManager.BinaryPaths(CreateArguments());
-            string driverPath = binaryPaths[DriverPathKey];
-            string browserPath = binaryPaths[BrowserPathKey];
+            string driverPath = binaryPaths[SeleniumManager.DriverPathKey];
+            string browserPath = binaryPaths[SeleniumManager.BrowserPathKey];
+
             if (File.Exists(driverPath))
             {
-                paths.Add(DriverPathKey, driverPath);
+                paths.Add(SeleniumManager.DriverPathKey, driverPath);
             }
             else
             {
                 throw new NoSuchDriverException($"The driver path is not a valid file: {driverPath}");
             }
+
             if (File.Exists(browserPath))
             {
-                paths.Add(BrowserPathKey, browserPath);
+                paths.Add(SeleniumManager.BrowserPathKey, browserPath);
             }
             else
             {
                 throw new NoSuchDriverException($"The browser path is not a valid file: {browserPath}");
             }
+
             return paths;
         }
 
@@ -123,7 +151,7 @@ private string CreateArguments()
                 argsBuilder.AppendFormat(CultureInfo.InvariantCulture, " --browser-version {0}", options.BrowserVersion);
             }
 
-            string browserBinary = options.BinaryLocation;
+            string? browserBinary = options.BinaryLocation;
             if (!string.IsNullOrEmpty(browserBinary))
             {
                 argsBuilder.AppendFormat(CultureInfo.InvariantCulture, " --browser-path \"{0}\"", browserBinary);
diff --git a/dotnet/src/webdriver/Firefox/FirefoxDriver.cs b/dotnet/src/webdriver/Firefox/FirefoxDriver.cs
@@ -221,9 +221,9 @@ private static ICommandExecutor GenerateDriverServiceCommandExecutor(DriverServi
                 string fullServicePath = finder.GetDriverPath();
                 service.DriverServicePath = Path.GetDirectoryName(fullServicePath);
                 service.DriverServiceExecutableName = Path.GetFileName(fullServicePath);
-                if (finder.HasBrowserPath())
+                if (finder.TryGetBrowserPath(out string browserPath))
                 {
-                    options.BinaryLocation = finder.GetBrowserPath();
+                    options.BinaryLocation = browserPath;
                     options.BrowserVersion = null;
                 }
             }
diff --git a/dotnet/src/webdriver/SeleniumManager.cs b/dotnet/src/webdriver/SeleniumManager.cs
@@ -38,6 +38,9 @@ namespace OpenQA.Selenium
     /// </summary>
     public static class SeleniumManager
     {
+        internal const string DriverPathKey = "driver_path";
+        internal const string BrowserPathKey = "browser_path";
+
         private static readonly ILogger _logger = Log.GetLogger(typeof(SeleniumManager));
 
         private static readonly Lazy<string> _lazyBinaryFullPath = new(() =>
@@ -93,14 +96,14 @@ public static Dictionary<string, string> BinaryPaths(string arguments)
             var smCommandResult = RunCommand(_lazyBinaryFullPath.Value, argsBuilder.ToString());
             Dictionary<string, string> binaryPaths = new()
             {
-                { "browser_path", smCommandResult.BrowserPath },
-                { "driver_path", smCommandResult.DriverPath }
+                { BrowserPathKey, smCommandResult.BrowserPath },
+                { DriverPathKey, smCommandResult.DriverPath }
             };
 
             if (_logger.IsEnabled(LogEventLevel.Trace))
             {
-                _logger.Trace($"Driver path: {binaryPaths["driver_path"]}");
-                _logger.Trace($"Browser path: {binaryPaths["browser_path"]}");
+                _logger.Trace($"Driver path: {binaryPaths[DriverPathKey]}");
+                _logger.Trace($"Browser path: {binaryPaths[BrowserPathKey]}");
             }
 
             return binaryPaths;
@@ -214,9 +217,9 @@ public sealed record LogEntryResponse(string Level, string Message);
 
         public sealed record ResultResponse
         (
-            [property: JsonPropertyName("driver_path")]
+            [property: JsonPropertyName(SeleniumManager.DriverPathKey)]
             string DriverPath,
-            [property: JsonPropertyName("browser_path")]
+            [property: JsonPropertyName(SeleniumManager.BrowserPathKey)]
             string BrowserPath
         );
     }

Original file line number	Diff line number	Diff line change
`@@ -168,9 +168,9 @@ private static ICommandExecutor GenerateDriverServiceCommandExecutor(DriverServi`
`168`	`168`	`string fullServicePath = finder.GetDriverPath();`
`169`	`169`	`service.DriverServicePath = Path.GetDirectoryName(fullServicePath);`
`170`	`170`	`service.DriverServiceExecutableName = Path.GetFileName(fullServicePath);`
`171`		`- if (finder.HasBrowserPath())`
	`171`	`+ if (finder.TryGetBrowserPath(out string browserPath))`
`172`	`172`	`{`
`173`		`- options.BinaryLocation = finder.GetBrowserPath();`
	`173`	`+ options.BinaryLocation = browserPath;`
`174`	`174`	`options.BrowserVersion = null;`
`175`	`175`	`}`
`176`	`176`	`}`
Original file line number	Diff line number	Diff line change
`@@ -19,10 +19,13 @@`
`19`	`19`
`20`	`20`	`using System;`
`21`	`21`	`using System.Collections.Generic;`
	`22`	`+using System.Diagnostics.CodeAnalysis;`
`22`	`23`	`using System.Globalization;`
`23`	`24`	`using System.IO;`
`24`	`25`	`using System.Text;`
`25`	`26`
	`27`	`+#nullable enable`
	`28`	`+`
`26`	`29`	`namespace OpenQA.Selenium`
`27`	`30`	`{`
`28`	`31`	`/// <summary>`
`@@ -31,17 +34,16 @@ namespace OpenQA.Selenium`
`31`	`34`	`/// </summary>`
`32`	`35`	`public class DriverFinder`
`33`	`36`	`{`
`34`		`- private DriverOptions options;`
	`37`	`+ private readonly DriverOptions options;`
`35`	`38`	`private Dictionary<string, string> paths = new Dictionary<string, string>();`
`36`		`- private const string BrowserPathKey = "browser_path";`
`37`		`- private const string DriverPathKey = "driver_path";`
`38`	`39`
`39`	`40`	`/// <summary>`
`40`	`41`	`/// Initializes a new instance of the <see cref="DriverFinder"/> class.`
`41`	`42`	`/// </summary>`
	`43`	`+ /// <exception cref="ArgumentNullException">If <paramref name="options"/> is <see langword="null"/>.</exception>`
`42`	`44`	`public DriverFinder(DriverOptions options)`
`43`	`45`	`{`
`44`		`- this.options = options;`
	`46`	`+ this.options = options ?? throw new ArgumentNullException(nameof(options));`
`45`	`47`	`}`
`46`	`48`
`47`	`49`	`/// <summary>`
`@@ -52,7 +54,7 @@ public DriverFinder(DriverOptions options)`
`52`	`54`	`/// </returns>`
`53`	`55`	`public string GetBrowserPath()`
`54`	`56`	`{`
`55`		`- return BinaryPaths()[BrowserPathKey];`
	`57`	`+ return BinaryPaths()[SeleniumManager.BrowserPathKey];`
`56`	`58`	`}`
`57`	`59`
`58`	`60`	`/// <summary>`
`@@ -63,14 +65,36 @@ public string GetBrowserPath()`
`63`	`65`	`/// </returns>`
`64`	`66`	`public string GetDriverPath()`
`65`	`67`	`{`
`66`		`- return BinaryPaths()[DriverPathKey];`
	`68`	`+ return BinaryPaths()[SeleniumManager.DriverPathKey];`
`67`	`69`	`}`
`68`	`70`
	`71`	`+ /// <summary>`
	`72`	`+ /// Gets whether there is a browser path for the given browser on this platform.`
	`73`	`+ /// </summary>`
	`74`	`+ /// <returns><see langword="true"/> if a browser path exists; otherwise, <see langword="false"/>.</returns>`
`69`	`75`	`public bool HasBrowserPath()`
`70`	`76`	`{`
`71`	`77`	`return !string.IsNullOrWhiteSpace(GetBrowserPath());`
`72`	`78`	`}`
`73`	`79`
	`80`	`+ /// <summary>`
	`81`	`+ /// Tries to get the browser path, as retrieved by Selenium Manager.`
	`82`	`+ /// </summary>`
	`83`	`+ /// <param name="browserPath">If the method returns <see langword="true"/>, the full browser path.</param>`
	`84`	`+ /// <returns><see langword="true"/> if a browser path exists; otherwise, <see langword="false"/>.</returns>`
	`85`	`+ public bool TryGetBrowserPath([NotNullWhen(true)] out string? browserPath)`
	`86`	`+ {`
	`87`	`+ string? path = GetBrowserPath();`
	`88`	`+ if (!string.IsNullOrWhiteSpace(path))`
	`89`	`+ {`
	`90`	`+ browserPath = path;`
	`91`	`+ return true;`
	`92`	`+ }`
	`93`	`+`
	`94`	`+ browserPath = null;`
	`95`	`+ return false;`
	`96`	`+ }`
	`97`	`+`
`74`	`98`	`/// <summary>`
`75`	`99`	`/// Invokes Selenium Manager to get the binaries paths and validates if they exist.`
`76`	`100`	`/// </summary>`
`@@ -80,29 +104,33 @@ public bool HasBrowserPath()`
`80`	`104`	`/// <exception cref="NoSuchDriverException">If one of the paths does not exist.</exception>`
`81`	`105`	`private Dictionary<string, string> BinaryPaths()`
`82`	`106`	`{`
`83`		`- if (paths.ContainsKey(DriverPathKey) && !string.IsNullOrWhiteSpace(paths[DriverPathKey]))`
	`107`	`+ if (paths.ContainsKey(SeleniumManager.DriverPathKey) && !string.IsNullOrWhiteSpace(paths[SeleniumManager.DriverPathKey]))`
`84`	`108`	`{`
`85`	`109`	`return paths;`
`86`	`110`	`}`
	`111`	`+`
`87`	`112`	`Dictionary<string, string> binaryPaths = SeleniumManager.BinaryPaths(CreateArguments());`
`88`		`- string driverPath = binaryPaths[DriverPathKey];`
`89`		`- string browserPath = binaryPaths[BrowserPathKey];`
	`113`	`+ string driverPath = binaryPaths[SeleniumManager.DriverPathKey];`
	`114`	`+ string browserPath = binaryPaths[SeleniumManager.BrowserPathKey];`
	`115`	`+`
`90`	`116`	`if (File.Exists(driverPath))`
`91`	`117`	`{`
`92`		`- paths.Add(DriverPathKey, driverPath);`
	`118`	`+ paths.Add(SeleniumManager.DriverPathKey, driverPath);`
`93`	`119`	`}`
`94`	`120`	`else`
`95`	`121`	`{`
`96`	`122`	`throw new NoSuchDriverException($"The driver path is not a valid file: {driverPath}");`
`97`	`123`	`}`
	`124`	`+`
`98`	`125`	`if (File.Exists(browserPath))`
`99`	`126`	`{`
`100`		`- paths.Add(BrowserPathKey, browserPath);`
	`127`	`+ paths.Add(SeleniumManager.BrowserPathKey, browserPath);`
`101`	`128`	`}`
`102`	`129`	`else`
`103`	`130`	`{`
`104`	`131`	`throw new NoSuchDriverException($"The browser path is not a valid file: {browserPath}");`
`105`	`132`	`}`
	`133`	`+`
`106`	`134`	`return paths;`
`107`	`135`	`}`
`108`	`136`
`@@ -123,7 +151,7 @@ private string CreateArguments()`
`123`	`151`	`argsBuilder.AppendFormat(CultureInfo.InvariantCulture, " --browser-version {0}", options.BrowserVersion);`
`124`	`152`	`}`
`125`	`153`
`126`		`- string browserBinary = options.BinaryLocation;`
	`154`	`+ string? browserBinary = options.BinaryLocation;`
`127`	`155`	`if (!string.IsNullOrEmpty(browserBinary))`
`128`	`156`	`{`
`129`	`157`	`argsBuilder.AppendFormat(CultureInfo.InvariantCulture, " --browser-path \"{0}\"", browserBinary);`
Original file line number	Diff line number	Diff line change
`@@ -221,9 +221,9 @@ private static ICommandExecutor GenerateDriverServiceCommandExecutor(DriverServi`
`221`	`221`	`string fullServicePath = finder.GetDriverPath();`
`222`	`222`	`service.DriverServicePath = Path.GetDirectoryName(fullServicePath);`
`223`	`223`	`service.DriverServiceExecutableName = Path.GetFileName(fullServicePath);`
`224`		`- if (finder.HasBrowserPath())`
	`224`	`+ if (finder.TryGetBrowserPath(out string browserPath))`
`225`	`225`	`{`
`226`		`- options.BinaryLocation = finder.GetBrowserPath();`
	`226`	`+ options.BinaryLocation = browserPath;`
`227`	`227`	`options.BrowserVersion = null;`
`228`	`228`	`}`
`229`	`229`	`}`