Fix warnings in Polly project (#1514)

Got rid of SA1129; SA1204; S3928; CA1508; CA1822; CA2201; SA1642 warnings in Polly project.
App-vNext · Aug 28, 2023 · daf1529 · daf1529
1 parent f9b1afc
commit daf1529
Show file tree

Hide file tree

Showing 14 changed files with 152 additions and 147 deletions.
diff --git a/src/Polly/AsyncPolicy.ContextAndKeys.cs b/src/Polly/AsyncPolicy.ContextAndKeys.cs
@@ -9,7 +9,7 @@ public abstract partial class AsyncPolicy
     /// <param name="policyKey">The unique, used-definable key to assign to this <see cref="AsyncPolicy"/> instance.</param>
     public AsyncPolicy WithPolicyKey(string policyKey)
     {
-        if (policyKeyInternal != null) throw PolicyKeyMustBeImmutableException;
+        if (policyKeyInternal != null) throw PolicyKeyMustBeImmutableException(nameof(policyKey));
 
         policyKeyInternal = policyKey;
         return this;
@@ -22,7 +22,7 @@ public AsyncPolicy WithPolicyKey(string policyKey)
     /// <param name="policyKey">The unique, used-definable key to assign to this <see cref="IAsyncPolicy"/> instance.</param>
     IAsyncPolicy IAsyncPolicy.WithPolicyKey(string policyKey)
     {
-        if (policyKeyInternal != null) throw PolicyKeyMustBeImmutableException;
+        if (policyKeyInternal != null) throw PolicyKeyMustBeImmutableException(nameof(policyKey));
 
         policyKeyInternal = policyKey;
         return this;
@@ -38,7 +38,7 @@ public abstract partial class AsyncPolicy<TResult>
     /// <param name="policyKey">The unique, used-definable key to assign to this <see cref="AsyncPolicy{TResult}"/> instance.</param>
     public AsyncPolicy<TResult> WithPolicyKey(string policyKey)
     {
-        if (policyKeyInternal != null) throw PolicyKeyMustBeImmutableException;
+        if (policyKeyInternal != null) throw PolicyKeyMustBeImmutableException(nameof(policyKey));
 
         policyKeyInternal = policyKey;
         return this;
@@ -51,7 +51,7 @@ public AsyncPolicy<TResult> WithPolicyKey(string policyKey)
     /// <param name="policyKey">The unique, used-definable key to assign to this <see cref="IAsyncPolicy{TResult}"/> instance.</param>
     IAsyncPolicy<TResult> IAsyncPolicy<TResult>.WithPolicyKey(string policyKey)
     {
-        if (policyKeyInternal != null) throw PolicyKeyMustBeImmutableException;
+        if (policyKeyInternal != null) throw PolicyKeyMustBeImmutableException(nameof(policyKey));
 
         policyKeyInternal = policyKey;
         return this;

diff --git a/src/Polly/AsyncPolicy.ExecuteOverloads.cs b/src/Polly/AsyncPolicy.ExecuteOverloads.cs
@@ -103,7 +103,7 @@ public async Task ExecuteAsync(Func<Context, CancellationToken, Task> action, Co
         }
         finally
         {
-            RestorePolicyContext(context, priorPolicyWrapKey, priorPolicyKey);
+            PolicyBase.RestorePolicyContext(context, priorPolicyWrapKey, priorPolicyKey);
         }
     }
 
@@ -223,7 +223,7 @@ public async Task<TResult> ExecuteAsync<TResult>(Func<Context, CancellationToken
         }
         finally
         {
-            RestorePolicyContext(context, priorPolicyWrapKey, priorPolicyKey);
+            PolicyBase.RestorePolicyContext(context, priorPolicyWrapKey, priorPolicyKey);
         }
     }
 

diff --git a/src/Polly/AsyncPolicy.TResult.ExecuteOverloads.cs b/src/Polly/AsyncPolicy.TResult.ExecuteOverloads.cs
@@ -112,7 +112,7 @@ public async Task<TResult> ExecuteAsync(Func<Context, CancellationToken, Task<TR
         }
         finally
         {
-            RestorePolicyContext(context, priorPolicyWrapKey, priorPolicyKey);
+            PolicyBase.RestorePolicyContext(context, priorPolicyWrapKey, priorPolicyKey);
         }
     }
 

diff --git a/src/Polly/CircuitBreaker/CircuitStateController.cs b/src/Polly/CircuitBreaker/CircuitStateController.cs
@@ -38,11 +38,13 @@ public CircuitState CircuitState
 
             using var _ = TimedLock.Lock(_lock);
 
+#pragma warning disable CA1508 // Avoid dead conditional code. _circuitState is checked again in the lock
             if (_circuitState == CircuitState.Open && !IsInAutomatedBreak_NeedsLock)
             {
                 _circuitState = CircuitState.HalfOpen;
                 _onHalfOpen();
             }
+#pragma warning restore CA1508 // Avoid dead conditional code. _circuitState is checked again in the lock
 
             return _circuitState;
         }

diff --git a/src/Polly/Policy.ContextAndKeys.cs b/src/Polly/Policy.ContextAndKeys.cs
@@ -9,7 +9,7 @@ public abstract partial class Policy
     /// <param name="policyKey">The unique, used-definable key to assign to this <see cref="Policy"/> instance.</param>
     public Policy WithPolicyKey(string policyKey)
     {
-        if (policyKeyInternal != null) throw PolicyKeyMustBeImmutableException;
+        if (policyKeyInternal != null) throw PolicyKeyMustBeImmutableException(nameof(policyKey));
 
         policyKeyInternal = policyKey;
         return this;
@@ -22,7 +22,7 @@ public Policy WithPolicyKey(string policyKey)
     /// <param name="policyKey">The unique, used-definable key to assign to this <see cref="Policy"/> instance.</param>
     ISyncPolicy ISyncPolicy.WithPolicyKey(string policyKey)
     {
-        if (policyKeyInternal != null) throw PolicyKeyMustBeImmutableException;
+        if (policyKeyInternal != null) throw PolicyKeyMustBeImmutableException(nameof(policyKey));
 
         policyKeyInternal = policyKey;
         return this;
@@ -38,7 +38,7 @@ public abstract partial class Policy<TResult>
     /// <param name="policyKey">The unique, used-definable key to assign to this <see cref="Policy{TResult}"/> instance.</param>
     public Policy<TResult> WithPolicyKey(string policyKey)
     {
-        if (policyKeyInternal != null) throw PolicyKeyMustBeImmutableException;
+        if (policyKeyInternal != null) throw PolicyKeyMustBeImmutableException(nameof(policyKey));
 
         policyKeyInternal = policyKey;
         return this;
@@ -51,7 +51,7 @@ public Policy<TResult> WithPolicyKey(string policyKey)
     /// <param name="policyKey">The unique, used-definable key to assign to this <see cref="Policy{TResult}"/> instance.</param>
     ISyncPolicy<TResult> ISyncPolicy<TResult>.WithPolicyKey(string policyKey)
     {
-        if (policyKeyInternal != null) throw PolicyKeyMustBeImmutableException;
+        if (policyKeyInternal != null) throw PolicyKeyMustBeImmutableException(nameof(policyKey));
 
         policyKeyInternal = policyKey;
         return this;

diff --git a/src/Polly/Policy.ExecuteOverloads.cs b/src/Polly/Policy.ExecuteOverloads.cs
@@ -69,7 +69,7 @@ public void Execute(Action<Context, CancellationToken> action, Context context,
         }
         finally
         {
-            RestorePolicyContext(context, priorPolicyWrapKey, priorPolicyKey);
+            PolicyBase.RestorePolicyContext(context, priorPolicyWrapKey, priorPolicyKey);
         }
     }
 
@@ -160,7 +160,7 @@ public TResult Execute<TResult>(Func<Context, CancellationToken, TResult> action
         }
         finally
         {
-            RestorePolicyContext(context, priorPolicyWrapKey, priorPolicyKey);
+            PolicyBase.RestorePolicyContext(context, priorPolicyWrapKey, priorPolicyKey);
         }
     }
 

diff --git a/src/Polly/Policy.TResult.ExecuteOverloads.cs b/src/Polly/Policy.TResult.ExecuteOverloads.cs
@@ -83,7 +83,7 @@ public TResult Execute(Func<Context, CancellationToken, TResult> action, Context
         }
         finally
         {
-            RestorePolicyContext(context, priorPolicyWrapKey, priorPolicyKey);
+            PolicyBase.RestorePolicyContext(context, priorPolicyWrapKey, priorPolicyKey);
         }
     }
 

diff --git a/src/Polly/PolicyBase.ContextAndKeys.cs b/src/Polly/PolicyBase.ContextAndKeys.cs
@@ -12,7 +12,19 @@ public abstract partial class PolicyBase
     /// </summary>
     public string PolicyKey => policyKeyInternal ?? (policyKeyInternal = GetType().Name + "-" + KeyHelper.GuidPart());
 
-    internal static ArgumentException PolicyKeyMustBeImmutableException => new("PolicyKey cannot be changed once set; or (when using the default value after the PolicyKey property has been accessed.", "policyKey");
+    internal static ArgumentException PolicyKeyMustBeImmutableException(string policyKeyParamName) => new ("PolicyKey cannot be changed once set; or (when using the default value after the PolicyKey property has been accessed.", policyKeyParamName);
+
+    /// <summary>
+    /// Restores the supplied keys to the execution <see cref="Context"/>.
+    /// </summary>
+    /// <param name="executionContext">The execution <see cref="Context"/>.</param>
+    /// <param name="priorPolicyWrapKey">The <see cref="M:Context.PolicyWrapKey"/> prior to execution through this policy.</param>
+    /// <param name="priorPolicyKey">The <see cref="M:Context.PolicyKey"/> prior to execution through this policy.</param>
+    internal static void RestorePolicyContext(Context executionContext, string priorPolicyWrapKey, string priorPolicyKey)
+    {
+        executionContext.PolicyWrapKey = priorPolicyWrapKey;
+        executionContext.PolicyKey = priorPolicyKey;
+    }
 
     /// <summary>
     /// Updates the execution <see cref="Context"/> with context from the executing policy.
@@ -27,16 +39,4 @@ public abstract partial class PolicyBase
 
         executionContext.PolicyKey = PolicyKey;
     }
-
-    /// <summary>
-    /// Restores the supplied keys to the execution <see cref="Context"/>.
-    /// </summary>
-    /// <param name="executionContext">The execution <see cref="Context"/>.</param>
-    /// <param name="priorPolicyWrapKey">The <see cref="M:Context.PolicyWrapKey"/> prior to execution through this policy.</param>
-    /// <param name="priorPolicyKey">The <see cref="M:Context.PolicyKey"/> prior to execution through this policy.</param>
-    internal void RestorePolicyContext(Context executionContext, string priorPolicyWrapKey, string priorPolicyKey)
-    {
-        executionContext.PolicyWrapKey = priorPolicyWrapKey;
-        executionContext.PolicyKey = priorPolicyKey;
-    }
 }
diff --git a/src/Polly/Polly.csproj b/src/Polly/Polly.csproj
@@ -11,8 +11,8 @@
     <NoWarn>$(NoWarn);IDE0044;IDE1006;CA1062;S107;CA1068;S4039;SA1121;CA1000;CA1063;SA1113;CA1031;CA1051;CA1200</NoWarn>
     <NoWarn>$(NoWarn);SA1629;SA1612;CA2211;S2223;CA1032;CA1815;CA1816;S4457;SA1615;IDE0250;S109;SA1618;SA1407;CA1033</NoWarn>
     <NoWarn>$(NoWarn);SA1515;S4023;CA1010;IDE0063;S2681;S3442;S3880;CA1064;SA1110;SA1203;SA1649;SA1625;SA1623;SA1118</NoWarn>
-    <NoWarn>$(NoWarn);S3253;S3971;S6605;CA1724;CA1716;SA1108;CA1710;S4049;S3246;SA1204;S3928;CA1508;CA1822;CA2201;SA1642</NoWarn>
-    <NoWarn>$(NoWarn);SA1805;SA1129;SA1805;CA1805;CA1821</NoWarn>
+    <NoWarn>$(NoWarn);S3253;S3971;S6605;CA1724;CA1716;SA1108;CA1710;S4049;S3246</NoWarn>
+    <NoWarn>$(NoWarn);SA1805;SA1805;CA1805;CA1821</NoWarn>
 
     <!--Pulic API Analyzers: We do not need to fix these as it would break compatibility with released Polly versions-->
     <NoWarn>$(NoWarn);RS0037;</NoWarn>

diff --git a/src/Polly/RateLimit/LockFreeTokenBucketRateLimiter.cs b/src/Polly/RateLimit/LockFreeTokenBucketRateLimiter.cs
@@ -15,7 +15,7 @@ internal sealed class LockFreeTokenBucketRateLimiter : IRateLimiter
     private long addNextTokenAtTicks;
 
 #if !NETSTANDARD2_0
-    private SpinWait spinner = new();
+    private SpinWait spinner = default;
 #endif
 
     /// <summary>

diff --git a/src/Polly/RateLimit/RateLimitRejectedException.cs b/src/Polly/RateLimit/RateLimitRejectedException.cs
@@ -54,15 +54,15 @@ public RateLimitRejectedException(TimeSpan retryAfter, string message) : base(me
     public RateLimitRejectedException(TimeSpan retryAfter, string message, Exception innerException) : base(message, innerException) =>
         SetRetryAfter(retryAfter);
 
+    private static string DefaultMessage(TimeSpan retryAfter) =>
+        $"The operation has been rate-limited and should be retried after {retryAfter}";
+
     private void SetRetryAfter(TimeSpan retryAfter)
     {
         if (retryAfter < TimeSpan.Zero) throw new ArgumentOutOfRangeException(nameof(retryAfter), retryAfter, $"The {nameof(retryAfter)} parameter must be a TimeSpan greater than or equal to TimeSpan.Zero.");
         RetryAfter = retryAfter;
     }
 
-    private static string DefaultMessage(TimeSpan retryAfter) =>
-        $"The operation has been rate-limited and should be retried after {retryAfter}";
-
 #if NETSTANDARD2_0
     /// <summary>
     /// Initializes a new instance of the <see cref="RateLimitRejectedException"/> class.

diff --git a/src/Polly/Registry/PolicyRegistry.cs b/src/Polly/Registry/PolicyRegistry.cs
@@ -10,7 +10,7 @@ public class PolicyRegistry : IConcurrentPolicyRegistry<string>
     private readonly IDictionary<string, IsPolicy> _registry = new ConcurrentDictionary<string, IsPolicy>();
 
     /// <summary>
-    /// Creates a registry of policies with <see cref="string"/> keys.
+    /// Initializes a new instance of the <see cref="PolicyRegistry"/> class, with <see cref="string"/> keys.
     /// </summary>
     public PolicyRegistry()
     {
@@ -21,12 +21,12 @@ public PolicyRegistry()
     }
 
     /// <summary>
-    /// Creates a registry of policies with <see cref="string"/> keys.
+    /// Initializes a new instance of the <see cref="PolicyRegistry"/> class, with <see cref="IDictionary{string, IsPolicy}"/> dictionary.
     /// <remarks>This internal constructor exists solely to facilitate testing of the GetEnumerator() methods, which allow us to support collection initialisation syntax.</remarks>
     /// </summary>
     /// <param name="registry">a dictionary containing keys and policies used for testing.</param>
     internal PolicyRegistry(IDictionary<string, IsPolicy> registry) =>
-        _registry = registry ?? throw new NullReferenceException(nameof(registry));
+        _registry = registry ?? throw new ArgumentNullException(nameof(registry));
 
     private ConcurrentDictionary<string, IsPolicy> ThrowIfNotConcurrentImplementation()
     {
@@ -158,11 +158,14 @@ public bool TryRemove<TPolicy>(string key, out TPolicy policy) where TPolicy : I
     /// <summary>
     /// Compares the existing policy for the specified key with a specified policy, and if they are equal, updates the policy with a third value.
     /// </summary>
-    /// <typeparam name="TPolicy"></typeparam>
+    /// <typeparam name="TPolicy">The type of the policy.</typeparam>
     /// <param name="key">The key whose value is compared with comparisonPolicy, and possibly replaced.</param>
     /// <param name="newPolicy">The policy that replaces the value for the specified <paramref name="key"/>, if the comparison results in equality.</param>
     /// <param name="comparisonPolicy">The policy that is compared to the existing policy at the specified key.</param>
-    /// <returns></returns>
+    /// <returns>
+    /// <see langword="true"/> if the value with <paramref name="key"/> was equal to <paramref name="comparisonPolicy"/> and
+    /// replaced with <paramref name="newPolicy"/>; otherwise, <see langword="false"/>.
+    /// </returns>
     public bool TryUpdate<TPolicy>(string key, TPolicy newPolicy, TPolicy comparisonPolicy) where TPolicy : IsPolicy
     {
         var registry = ThrowIfNotConcurrentImplementation();

diff --git a/src/Polly/Wrap/AsyncPolicyWrapSyntax.cs b/src/Polly/Wrap/AsyncPolicyWrapSyntax.cs
@@ -1,5 +1,59 @@
 namespace Polly;
 
+/// <summary>
+/// Defines extensions for configuring <see cref="PolicyWrap"/> instances on an <see cref="IAsyncPolicy"/> or <see cref="IAsyncPolicy{TResult}"/>.
+/// </summary>
+public static class IAsyncPolicyPolicyWrapExtensions
+{
+    /// <summary>
+    /// Wraps the specified outer policy round the inner policy.
+    /// </summary>
+    /// <param name="outerPolicy">The outer policy.</param>
+    /// <param name="innerPolicy">The inner policy.</param>
+    /// <returns>A <see cref="PolicyWrap"/> instance representing the combined wrap.</returns>
+    public static AsyncPolicyWrap WrapAsync(this IAsyncPolicy outerPolicy, IAsyncPolicy innerPolicy)
+    {
+        if (outerPolicy == null) throw new ArgumentNullException(nameof(outerPolicy));
+        return ((AsyncPolicy)outerPolicy).WrapAsync(innerPolicy);
+    }
+
+    /// <summary>
+    /// Wraps the specified outer policy round the inner policy.
+    /// </summary>
+    /// <param name="outerPolicy">The outer policy.</param>
+    /// <param name="innerPolicy">The inner policy.</param>
+    /// <returns>A <see cref="PolicyWrap"/> instance representing the combined wrap.</returns>
+    public static AsyncPolicyWrap<TResult> WrapAsync<TResult>(this IAsyncPolicy outerPolicy, IAsyncPolicy<TResult> innerPolicy)
+    {
+        if (outerPolicy == null) throw new ArgumentNullException(nameof(outerPolicy));
+        return ((AsyncPolicy)outerPolicy).WrapAsync(innerPolicy);
+    }
+
+    /// <summary>
+    /// Wraps the specified outer policy round the inner policy.
+    /// </summary>
+    /// <param name="outerPolicy">The outer policy.</param>
+    /// <param name="innerPolicy">The inner policy.</param>
+    /// <returns>A <see cref="PolicyWrap"/> instance representing the combined wrap.</returns>
+    public static AsyncPolicyWrap<TResult> WrapAsync<TResult>(this IAsyncPolicy<TResult> outerPolicy, IAsyncPolicy innerPolicy)
+    {
+        if (outerPolicy == null) throw new ArgumentNullException(nameof(outerPolicy));
+        return ((AsyncPolicy<TResult>)outerPolicy).WrapAsync(innerPolicy);
+    }
+
+    /// <summary>
+    /// Wraps the specified outer policy round the inner policy.
+    /// </summary>
+    /// <param name="outerPolicy">The outer policy.</param>
+    /// <param name="innerPolicy">The inner policy.</param>
+    /// <returns>A <see cref="PolicyWrap"/> instance representing the combined wrap.</returns>
+    public static AsyncPolicyWrap<TResult> WrapAsync<TResult>(this IAsyncPolicy<TResult> outerPolicy, IAsyncPolicy<TResult> innerPolicy)
+    {
+        if (outerPolicy == null) throw new ArgumentNullException(nameof(outerPolicy));
+        return ((AsyncPolicy<TResult>)outerPolicy).WrapAsync(innerPolicy);
+    }
+}
+
 public partial class AsyncPolicy
 {
     /// <summary>
@@ -97,7 +151,7 @@ public static AsyncPolicyWrap WrapAsync(params IAsyncPolicy[] policies)
     /// <typeparam name="TResult">The return type of delegates which may be executed through the policy.</typeparam>
     /// <returns>The PolicyWrap.</returns>
     /// <exception cref="ArgumentException">The enumerable of policies to form the wrap must contain at least two policies.</exception>
-    public  static AsyncPolicyWrap<TResult> WrapAsync<TResult>(params IAsyncPolicy<TResult>[] policies)
+    public static AsyncPolicyWrap<TResult> WrapAsync<TResult>(params IAsyncPolicy<TResult>[] policies)
     {
         switch (policies.Length)
         {
@@ -112,57 +166,3 @@ public  static AsyncPolicyWrap<TResult> WrapAsync<TResult>(params IAsyncPolicy<T
         }
     }
 }
-
-/// <summary>
-/// Defines extensions for configuring <see cref="PolicyWrap"/> instances on an <see cref="IAsyncPolicy"/> or <see cref="IAsyncPolicy{TResult}"/>.
-/// </summary>
-public static class IAsyncPolicyPolicyWrapExtensions
-{
-    /// <summary>
-    /// Wraps the specified outer policy round the inner policy.
-    /// </summary>
-    /// <param name="outerPolicy">The outer policy.</param>
-    /// <param name="innerPolicy">The inner policy.</param>
-    /// <returns>A <see cref="PolicyWrap"/> instance representing the combined wrap.</returns>
-    public static AsyncPolicyWrap WrapAsync(this IAsyncPolicy outerPolicy, IAsyncPolicy innerPolicy)
-    {
-        if (outerPolicy == null) throw new ArgumentNullException(nameof(outerPolicy));
-        return ((AsyncPolicy)outerPolicy).WrapAsync(innerPolicy);
-    }
-
-    /// <summary>
-    /// Wraps the specified outer policy round the inner policy.
-    /// </summary>
-    /// <param name="outerPolicy">The outer policy.</param>
-    /// <param name="innerPolicy">The inner policy.</param>
-    /// <returns>A <see cref="PolicyWrap"/> instance representing the combined wrap.</returns>
-    public  static AsyncPolicyWrap<TResult> WrapAsync<TResult>(this IAsyncPolicy outerPolicy, IAsyncPolicy<TResult> innerPolicy)
-    {
-        if (outerPolicy == null) throw new ArgumentNullException(nameof(outerPolicy));
-        return ((AsyncPolicy)outerPolicy).WrapAsync(innerPolicy);
-    }
-
-    /// <summary>
-    /// Wraps the specified outer policy round the inner policy.
-    /// </summary>
-    /// <param name="outerPolicy">The outer policy.</param>
-    /// <param name="innerPolicy">The inner policy.</param>
-    /// <returns>A <see cref="PolicyWrap"/> instance representing the combined wrap.</returns>
-    public  static AsyncPolicyWrap<TResult> WrapAsync<TResult>(this IAsyncPolicy<TResult> outerPolicy, IAsyncPolicy innerPolicy)
-    {
-        if (outerPolicy == null) throw new ArgumentNullException(nameof(outerPolicy));
-        return ((AsyncPolicy<TResult>)outerPolicy).WrapAsync(innerPolicy);
-    }
-
-    /// <summary>
-    /// Wraps the specified outer policy round the inner policy.
-    /// </summary>
-    /// <param name="outerPolicy">The outer policy.</param>
-    /// <param name="innerPolicy">The inner policy.</param>
-    /// <returns>A <see cref="PolicyWrap"/> instance representing the combined wrap.</returns>
-    public  static AsyncPolicyWrap<TResult> WrapAsync<TResult>(this IAsyncPolicy<TResult> outerPolicy, IAsyncPolicy<TResult> innerPolicy)
-    {
-        if (outerPolicy == null) throw new ArgumentNullException(nameof(outerPolicy));
-        return ((AsyncPolicy<TResult>)outerPolicy).WrapAsync(innerPolicy);
-    }
-}