Apply analyzer suggestions (#2617)

* Avoid double dictionary lookup

* Use const string strings

This enable constant string interpolation

* Use coalescing

* No need to capture `client`

* Use string.Concat when the separator is empty

* Fix typo

* Use await inside try/catch

* Bring latest analyzer

* Ignore CA1711 AND CA1725

* ignore CA1848

* Fix a rule

* Ignore CA2254

* More fixes

* Build green

* Update lib/.editorconfig

* Ignore CA1860

* Fix CA1860

* Update lib/PuppeteerSharp/PuppeteerSharp.csproj

* Fix analysis level

---------

Co-authored-by: Darío Kondratiuk <[email protected]>

Author: jnyrup

lib/.editorconfig

@@ -249,14 +249,32 @@ dotnet_diagnostic.CA1056.severity = none
 dotnet_diagnostic.CA1040.severity = none
 # Use ConfigureAwait
 dotnet_diagnostic.CA2007.severity = error
+# CA1711: Identifiers should not have incorrect suffix
+# We need many suffixes coming from Puppeteer.
+dotnet_diagnostic.CA1711.severity = none
+# CA1725: Parameter names should match base declaration
+# I found that in some places it doesn't detect overloads.
+dotnet_diagnostic.CA1725.severity = suggestion
 # Do not initialize unnecessarily.
 dotnet_diagnostic.CA1805.severity = none
 # Properties should not return arrays.
 dotnet_diagnostic.CA1819.severity = suggestion
 # should override equality and inequality.
 dotnet_diagnostic.CA1815.severity = suggestion
+# CA1848: Use the LoggerMessage delegates
+# TODO: REMOVE
+dotnet_diagnostic.CA1848.severity = none
+# CA1859: Use concrete types when possible for improved performance
+# It collides with https://www.jetbrains.com/help/rider/ReturnTypeCanBeEnumerable.Local.html
+dotnet_diagnostic.CA1859.severity = none
+# CA1862: Use the 'StringComparison' method overloads to perform case-insensitive string comparisons
+# This rule is not compatible with .netstandard
+dotnet_diagnostic.CA1862.severity = none
 # Change '...' to be read-only by removing the property setter.
 dotnet_diagnostic.CA2227.severity = suggestion
+# CA2254: Template should be a static expression
+# TODO: REMOVE
+dotnet_diagnostic.CA2254.severity = none
 # Avoid using cref tags with a prefix
 dotnet_diagnostic.CA1200.severity = suggestion
 # Marshaling for P/Invoke
@@ -297,6 +315,8 @@ dotnet_diagnostic.CA1806.severity = error
 dotnet_diagnostic.CA1816.severity = error
 # CA1836: Prefer IsEmpty over Count when available.
 dotnet_diagnostic.CA1836.severity = error
+# CA1860: Avoid using 'Enumerable.Any()' extension method
+dotnet_diagnostic.CA1860.severity = error
 # CA2000: Call System.IDisposable.Dispose on object created by 'new Process()' before all references to it are out of scope.
 dotnet_diagnostic.CA2000.severity = error
 # CA2008: Pass TaskScheduler
@@ -326,9 +346,9 @@ dotnet_diagnostic.IL3000.severity = none
 
 # Default severity for analyzer diagnostics with category 'Usage'
 dotnet_analyzer_diagnostic.category-Usage.severity = none
-
-# Default severity for analyzer diagnostics with category 'CodeQuality'
-dotnet_analyzer_diagnostic.category-CodeQuality.severity = error
+
+# Default severity for analyzer diagnostics with category 'CodeQuality'
+dotnet_analyzer_diagnostic.category-CodeQuality.severity = error
 
 [*.{cpp,h,in}]
 curly_bracket_next_line = true

lib/PuppeteerSharp/BrowserData/Firefox.cs

@@ -36,13 +36,13 @@ internal static async Task<string> ResolveBuildIdAsync(string channel)
                 .GetAsync<Dictionary<string, string>>("https://product-details.mozilla.org/1.0/firefox_versions.json")
                 .ConfigureAwait(false);
 
-            if (!version.ContainsKey(channel))
+            if (!version.TryGetValue(channel, out var buildId))
             {
                 throw new PuppeteerException($"Channel {channel} not found.");
             }
 
-            _cachedBuildIds[channel] = version[channel];
-            return version[channel];
+            _cachedBuildIds[channel] = buildId;
+            return buildId;
         }
 
         internal static string RelativeExecutablePath(Platform platform, string buildId)
@@ -90,7 +90,7 @@ private static string GetArchive(Platform platform, string buildId)
 
         private static Dictionary<string, object> GetDefaultPreferences(Dictionary<string, object> preferences)
         {
-            var server = "dummy.test";
+            const string server = "dummy.test";
             var prefs = new Dictionary<string, object>()
             {
                 // Make sure Shield doesn't hit the network.

lib/PuppeteerSharp/BrowserFetcher.cs

@@ -247,7 +247,7 @@ private async Task<InstalledBrowser> DownloadAsync(SupportedBrowser browser, str
             return new InstalledBrowser(cache, browser, buildId, Platform);
         }
 
-        private Task InstallDmgAsync(string dmgPath, string folderPath)
+        private async Task InstallDmgAsync(string dmgPath, string folderPath)
         {
             try
             {
@@ -301,7 +301,7 @@ private Task InstallDmgAsync(string dmgPath, string folderPath)
                 process.BeginOutputReadLine();
                 process.WaitForExit();
 
-                return mountAndCopyTcs.Task.WithTimeout(Puppeteer.DefaultTimeout);
+                await mountAndCopyTcs.Task.WithTimeout(Puppeteer.DefaultTimeout).ConfigureAwait(false);
             }
             finally
             {
@@ -400,7 +400,7 @@ private async Task UnpackArchiveAsync(string archivePath, string outputPath, str
 
                         if (code != 0)
                         {
-                            throw new Exception("Chmod operation failed");
+                            throw new PuppeteerException("Chmod operation failed");
                         }
                     }
                 }

lib/PuppeteerSharp/Cdp/CdpElementHandle.cs

@@ -107,7 +107,7 @@ public override Task UploadFileAsync(bool resolveFilePaths, params string[] file
                 // The zero-length array is a special case, it seems that
                 // DOM.setFileInputFiles does not actually update the files in that case, so
                 // the solution is to eval the element value to a new FileList directly.
-                if (!filePaths.Any())
+                if (filePaths.Length == 0)
                 {
                     await handle.EvaluateFunctionAsync(@"(element) => {
                         element.files = new DataTransfer().files;

lib/PuppeteerSharp/Cdp/CdpJSHandle.cs

@@ -58,7 +58,7 @@ public override async Task<T> JsonValueAsync<T>()
 
         var value = await EvaluateFunctionAsync<T>("object => object").ConfigureAwait(false);
 
-        return value == null ? throw new PuppeteerException("Could not serialize referenced object") : value;
+        return value ?? throw new PuppeteerException("Could not serialize referenced object");
     }
 
     /// <inheritdoc/>

lib/PuppeteerSharp/Cdp/CdpPage.cs

@@ -615,7 +615,7 @@ async void ResponseEventListener(object sender, ResponseCreatedEventArgs e)
             }
             catch (Exception ex)
             {
-                responseTcs.TrySetException(new Exception("Predicated failed", ex));
+                responseTcs.TrySetException(new PuppeteerException("Predicated failed", ex));
             }
         }
 

lib/PuppeteerSharp/Cdp/FrameManager.cs

@@ -107,7 +107,7 @@ internal ExecutionContext GetExecutionContextById(int contextId, CDPSession sess
         }
 
         internal DeviceRequestPromptManager GetDeviceRequestPromptManager(CDPSession client)
-            => _deviceRequestPromptManagerMap.GetOrAdd(client, _ => new DeviceRequestPromptManager(client, TimeoutSettings));
+            => _deviceRequestPromptManagerMap.GetOrAdd(client, client => new DeviceRequestPromptManager(client, TimeoutSettings));
 
         internal Frame[] GetFrames() => FrameTree.Frames;
 
@@ -425,7 +425,7 @@ private void OnFrameNavigatedWithinDocument(NavigatedWithinDocumentResponse e)
 
         private void RemoveFramesRecursively(Frame frame)
         {
-            while (frame.ChildFrames.Any())
+            while (frame.ChildFrames.Count != 0)
             {
                 RemoveFramesRecursively(frame.ChildFrames.First() as Frame);
             }

lib/PuppeteerSharp/Cdp/Messaging/FetchFulfillRequest.cs

@@ -79,9 +79,9 @@ public string ResponsePhrase
         {
             get
             {
-                if (StatusTexts.ContainsKey(ResponseCode))
+                if (StatusTexts.TryGetValue(ResponseCode, out var statusText))
                 {
-                    return StatusTexts[ResponseCode];
+                    return statusText;
                 }
 
                 return "Unknown Status Code";

lib/PuppeteerSharp/Helpers/AsyncMessageQueue.cs

@@ -51,7 +51,7 @@ public void Enqueue(MessageTask callback, ConnectionResponse obj)
             task.ContinueWith(
                 t =>
                 {
-                    _logger.LogError(t.Exception, "Failed to complete async handling of SendAsync for {callback}", callback.Method);
+                    _logger.LogError(t.Exception, "Failed to complete async handling of SendAsync for {Method}", callback.Method);
                     callback.TaskWrapper.TrySetException(t.Exception!); // t.Exception is available since this runs only on faulted
                 },
                 CancellationToken.None,

lib/PuppeteerSharp/Helpers/RemoteObjectHelper.cs

@@ -126,7 +126,7 @@ private static object ValueFromUnserializableValue(RemoteObject remoteObject, st
                 case "-Infinity":
                     return double.NegativeInfinity;
                 default:
-                    throw new Exception("Unsupported unserializable value: " + unserializableValue);
+                    throw new PuppeteerException("Unsupported unserializable value: " + unserializableValue);
             }
         }
     }