Project: Enable StyleCop and fix existing rules to make PRs easier going forward (#2757)

* Project: Enable StyleCop and fix existing rules to make PRs easier going forward

This adopts StyleCop and fixes most issues (and disables things we wouldn't want) to make PRs more consistent in an automated way and prevent formatting problems affecting git history, etc. It also does all the documentation enforcement.

Note: since I had to fix up many anyway, I finally did the `<inheritdoc />` minimization on `IDatabase`/`IDatabaseAsync` to remove 10% of duplicate documentation. That should also make PRs and maintenance less monotonous.

* Fix stylecop issues

Author: NickCraver

.editorconfig

@@ -138,23 +138,73 @@ csharp_space_between_method_call_empty_parameter_list_parentheses = false
 csharp_preserve_single_line_statements = true
 csharp_preserve_single_line_blocks = true
 
-# IDE0090: Use 'new(...)'
-dotnet_diagnostic.IDE0090.severity = silent
 
-# RCS1037: Remove trailing white-space.
-dotnet_diagnostic.RCS1037.severity = error
+# IDE preferences
+dotnet_diagnostic.IDE0090.severity = silent # IDE0090: Use 'new(...)'
+
+#Roslynator preferences
+dotnet_diagnostic.RCS1037.severity = error # RCS1037: Remove trailing white-space.
+dotnet_diagnostic.RCS1098.severity = none # RCS1098: Constant values should be placed on right side of comparisons.
+
+dotnet_diagnostic.RCS1194.severity = none # RCS1194: Implement exception constructors.
+dotnet_diagnostic.RCS1229.severity = none # RCS1229: Use async/await when necessary.
+dotnet_diagnostic.RCS1233.severity = none # RCS1233: Use short-circuiting operator.
+dotnet_diagnostic.RCS1234.severity = none # RCS1234: Duplicate enum value.
+
+# StyleCop preferences
+dotnet_diagnostic.SA0001.severity = none # SA0001: XML comment analysis is disabled
+
+dotnet_diagnostic.SA1101.severity = none # SA1101: Prefix local calls with this
+dotnet_diagnostic.SA1108.severity = none # SA1108: Block statements should not contain embedded comments
+dotnet_diagnostic.SA1122.severity = none # SA1122: Use string.Empty for empty strings
+dotnet_diagnostic.SA1127.severity = none # SA1127: Generic type constraints should be on their own line
+dotnet_diagnostic.SA1128.severity = none # SA1128: Put constructor initializers on their own line
+dotnet_diagnostic.SA1132.severity = none # SA1132: Do not combine fields
+dotnet_diagnostic.SA1133.severity = none # SA1133: Do not combine attributes
+
+dotnet_diagnostic.SA1200.severity = none # SA1200: Using directives should be placed correctly
+dotnet_diagnostic.SA1201.severity = none # SA1201: Elements should appear in the correct order
+dotnet_diagnostic.SA1202.severity = none # SA1202: Elements should be ordered by access
+dotnet_diagnostic.SA1203.severity = none # SA1203: Constants should appear before fields
+
+dotnet_diagnostic.SA1306.severity = none # SA1306: Field names should begin with lower-case letter
+dotnet_diagnostic.SA1309.severity = none # SA1309: Field names should not begin with underscore
+dotnet_diagnostic.SA1310.severity = silent # SA1310: Field names should not contain underscore
+dotnet_diagnostic.SA1311.severity = none # SA1311: Static readonly fields should begin with upper-case letter
+dotnet_diagnostic.SA1312.severity = none # SA1312: Variable names should begin with lower-case letter
+
+dotnet_diagnostic.SA1401.severity = silent # SA1401: Fields should be private
+dotnet_diagnostic.SA1402.severity = suggestion # SA1402: File may only contain a single type
+
+dotnet_diagnostic.SA1503.severity = silent # SA1503: Braces should not be omitted
+dotnet_diagnostic.SA1516.severity = silent # SA1516: Elements should be separated by blank line
+
+dotnet_diagnostic.SA1600.severity = none # SA1600: Elements should be documented
+dotnet_diagnostic.SA1601.severity = none # SA1601: Partial elements should be documented
+dotnet_diagnostic.SA1602.severity = none # SA1602: Enumeration items should be documented
+dotnet_diagnostic.SA1615.severity = none # SA1615: Element return value should be documented
+dotnet_diagnostic.SA1623.severity = none # SA1623: Property summary documentation should match accessors
+dotnet_diagnostic.SA1633.severity = none # SA1633: File should have header
+dotnet_diagnostic.SA1642.severity = none # SA1642: Constructor summary documentation should begin with standard text
+dotnet_diagnostic.SA1643.severity = none # SA1643: Destructor summary documentation should begin with standard text
+
+
+# To Fix:
+dotnet_diagnostic.SA1204.severity = none # SA1204: Static elements should appear before instance elements
+dotnet_diagnostic.SA1214.severity = none # SA1214: Readonly fields should appear before non-readonly fields
+dotnet_diagnostic.SA1304.severity = none # SA1304: Non-private readonly fields should begin with upper-case letter
+dotnet_diagnostic.SA1307.severity = none # SA1307: Accessible fields should begin with upper-case letter
+dotnet_diagnostic.SA1308.severity = suggestion # SA1308: Variable names should not be prefixed
+dotnet_diagnostic.SA1131.severity = none # SA1131: Use readable conditions
+dotnet_diagnostic.SA1405.severity = none # SA1405: Debug.Assert should provide message text
+dotnet_diagnostic.SA1501.severity = none # SA1501: Statement should not be on a single line
+dotnet_diagnostic.SA1502.severity = suggestion # SA1502: Element should not be on a single line
+dotnet_diagnostic.SA1513.severity = none # SA1513: Closing brace should be followed by blank line
+dotnet_diagnostic.SA1515.severity = none # SA1515: Single-line comment should be preceded by blank line
+dotnet_diagnostic.SA1611.severity = suggestion # SA1611: Element parameters should be documented
+dotnet_diagnostic.SA1649.severity = suggestion # SA1649: File name should match first type name
 
-# RCS1098: Constant values should be placed on right side of comparisons.
-dotnet_diagnostic.RCS1098.severity = none
 
-# RCS1194: Implement exception constructors.
-dotnet_diagnostic.RCS1194.severity = none
 
-# RCS1229: Use async/await when necessary.
-dotnet_diagnostic.RCS1229.severity = none
 
-# RCS1233: Use short-circuiting operator.
-dotnet_diagnostic.RCS1233.severity = none
 
-# RCS1234: Duplicate enum value.
-dotnet_diagnostic.RCS1234.severity = none

Directory.Build.props

@@ -33,10 +33,13 @@
     <Deterministic>true</Deterministic>
     <EmbedUntrackedSources>true</EmbedUntrackedSources>
   </PropertyGroup>
+
   <ItemGroup Condition="'$(Configuration)' == 'Release' and '$(SourceRoot)'==''">
     <SourceRoot Include="$(MSBuildThisFileDirectory)/"/>
   </ItemGroup>
+
   <ItemGroup>
     <PackageReference Include="Microsoft.NETFramework.ReferenceAssemblies" PrivateAssets="all" />
+    <PackageReference Include="StyleCop.Analyzers" PrivateAssets="All" />
   </ItemGroup>
 </Project>

Directory.Packages.props

@@ -20,6 +20,7 @@
     <PackageVersion Include="Newtonsoft.Json" Version="13.0.1" />
     <PackageVersion Include="NSubstitute" Version="5.0.0" />
     <PackageVersion Include="StackExchange.Redis" Version="2.6.96" />
+    <PackageVersion Include="StyleCop.Analyzers" Version="1.2.0-beta.556" />
     <!-- For binding redirect testing, main package gets this transitively -->
     <PackageVersion Include="System.IO.Pipelines" Version="5.0.1" />
     <PackageVersion Include="System.Runtime.Caching" Version="5.0.0" />

src/StackExchange.Redis/APITypes/ClientKillFilter.cs

@@ -113,7 +113,7 @@ public ClientKillFilter WithSkipMe(bool? skipMe)
     /// <summary>
     /// Set the MaxAgeInSeconds filter.
     /// </summary>
-    /// <param name="maxAgeInSeconds">Age of connection in seconds</param>
+    /// <param name="maxAgeInSeconds">Age of connection in seconds.</param>
     public ClientKillFilter WithMaxAgeInSeconds(long? maxAgeInSeconds)
     {
         MaxAgeInSeconds = maxAgeInSeconds;
@@ -123,9 +123,9 @@ public ClientKillFilter WithMaxAgeInSeconds(long? maxAgeInSeconds)
     internal List<RedisValue> ToList(bool withReplicaCommands)
     {
         var parts = new List<RedisValue>(15)
-            {
-                RedisLiterals.KILL
-            };
+        {
+            RedisLiterals.KILL,
+        };
         if (Id != null)
         {
             parts.Add(RedisLiterals.ID);

src/StackExchange.Redis/APITypes/GeoRadiusOptions.cs

@@ -13,22 +13,26 @@ public enum GeoRadiusOptions
     /// No Options.
     /// </summary>
     None = 0,
+
     /// <summary>
     /// Redis will return the coordinates of any results.
     /// </summary>
     WithCoordinates = 1,
+
     /// <summary>
     /// Redis will return the distance from center for all results.
     /// </summary>
     WithDistance = 2,
+
     /// <summary>
     /// Redis will return the geo hash value as an integer. (This is the score in the sorted set).
     /// </summary>
     WithGeoHash = 4,
+
     /// <summary>
     /// Populates the commonly used values from the entry (the integer hash is not returned as it is not commonly useful).
     /// </summary>
-    Default = WithCoordinates | WithDistance
+    Default = WithCoordinates | WithDistance,
 }
 
 internal static class GeoRadiusOptionsExtensions

src/StackExchange.Redis/APITypes/GeoRadiusResult.cs

@@ -23,7 +23,7 @@ public readonly struct GeoRadiusResult
     /// <summary>
     /// The hash value of the matched member as an integer. (The key in the sorted set).
     /// </summary>
-    /// <remarks>Note that this is not the same as the hash returned from GeoHash</remarks>
+    /// <remarks>Note that this is not the same as the hash returned from GeoHash.</remarks>
     public long? Hash { get; }
 
     /// <summary>

src/StackExchange.Redis/APITypes/GeoSearchShape.cs

@@ -3,7 +3,7 @@
 namespace StackExchange.Redis;
 
 /// <summary>
-/// A Shape that you can use for a GeoSearch
+/// A Shape that you can use for a GeoSearch.
 /// </summary>
 public abstract class GeoSearchShape
 {
@@ -18,9 +18,9 @@ public abstract class GeoSearchShape
     internal abstract int ArgCount { get; }
 
     /// <summary>
-    /// constructs a <see cref="GeoSearchShape"/>
+    /// constructs a <see cref="GeoSearchShape"/>.
     /// </summary>
-    /// <param name="unit"></param>
+    /// <param name="unit">The geography unit to use.</param>
     public GeoSearchShape(GeoUnit unit)
     {
         Unit = unit;
@@ -30,7 +30,7 @@ public GeoSearchShape(GeoUnit unit)
 }
 
 /// <summary>
-/// A circle drawn on a map bounding
+/// A circle drawn on a map bounding.
 /// </summary>
 public class GeoSearchCircle : GeoSearchShape
 {
@@ -41,17 +41,16 @@ public class GeoSearchCircle : GeoSearchShape
     /// </summary>
     /// <param name="radius">The radius of the circle.</param>
     /// <param name="unit">The distance unit the circle will use, defaults to Meters.</param>
-    public GeoSearchCircle(double radius, GeoUnit unit = GeoUnit.Meters) : base (unit)
+    public GeoSearchCircle(double radius, GeoUnit unit = GeoUnit.Meters) : base(unit)
     {
         _radius = radius;
     }
 
     internal override int ArgCount => 3;
 
     /// <summary>
-    /// Gets the <exception cref="RedisValue"/>s for this shape
+    /// Gets the <see cref="RedisValue"/>s for this shape.
     /// </summary>
-    /// <returns></returns>
     internal override void AddArgs(List<RedisValue> args)
     {
         args.Add(RedisLiterals.BYRADIUS);
@@ -61,7 +60,7 @@ internal override void AddArgs(List<RedisValue> args)
 }
 
 /// <summary>
-/// A box drawn on a map
+/// A box drawn on a map.
 /// </summary>
 public class GeoSearchBox : GeoSearchShape
 {

src/StackExchange.Redis/APITypes/LatencyHistoryEntry.cs

@@ -3,7 +3,7 @@
 namespace StackExchange.Redis;
 
 /// <summary>
-/// A latency entry as reported by the built-in LATENCY HISTORY command
+/// A latency entry as reported by the built-in LATENCY HISTORY command.
 /// </summary>
 public readonly struct LatencyHistoryEntry
 {
@@ -30,12 +30,12 @@ protected override bool TryParse(in RawResult raw, out LatencyHistoryEntry parse
     }
 
     /// <summary>
-    /// The time at which this entry was recorded
+    /// The time at which this entry was recorded.
     /// </summary>
     public DateTime Timestamp { get; }
 
     /// <summary>
-    /// The latency recorded for this event
+    /// The latency recorded for this event.
     /// </summary>
     public int DurationMilliseconds { get; }
 

src/StackExchange.Redis/APITypes/LatencyLatestEntry.cs

@@ -3,7 +3,7 @@
 namespace StackExchange.Redis;
 
 /// <summary>
-/// A latency entry as reported by the built-in LATENCY LATEST command
+/// A latency entry as reported by the built-in LATENCY LATEST command.
 /// </summary>
 public readonly struct LatencyLatestEntry
 {
@@ -31,22 +31,22 @@ protected override bool TryParse(in RawResult raw, out LatencyLatestEntry parsed
     }
 
     /// <summary>
-    /// The name of this event
+    /// The name of this event.
     /// </summary>
     public string EventName { get; }
 
     /// <summary>
-    /// The time at which this entry was recorded
+    /// The time at which this entry was recorded.
     /// </summary>
     public DateTime Timestamp { get; }
 
     /// <summary>
-    /// The latency recorded for this event
+    /// The latency recorded for this event.
     /// </summary>
     public int DurationMilliseconds { get; }
 
     /// <summary>
-    /// The max latency recorded for all events
+    /// The max latency recorded for all events.
     /// </summary>
     public int MaxDurationMilliseconds { get; }
 

src/StackExchange.Redis/APITypes/StreamPendingMessageInfo.cs

@@ -1,5 +1,4 @@
-
-namespace StackExchange.Redis;
+namespace StackExchange.Redis;
 
 /// <summary>
 /// Describes properties of a pending message.

src/StackExchange.Redis/BufferReader.cs

@@ -41,7 +41,8 @@ private bool FetchNextSegment()
                 _current = _iterator.Current.Span;
                 OffsetThisSpan = 0;
                 RemainingThisSpan = _current.Length;
-            } while (IsEmpty); // skip empty segments, they don't help us!
+            }
+            while (IsEmpty); // skip empty segments, they don't help us!
 
             return true;
         }
@@ -59,7 +60,7 @@ public BufferReader(scoped in ReadOnlySequence<byte> buffer)
         }
 
         /// <summary>
-        /// Note that in results other than success, no guarantees are made about final state; if you care: snapshot
+        /// Note that in results other than success, no guarantees are made about final state; if you care: snapshot.
         /// </summary>
         public ConsumeResult TryConsumeCRLF()
         {
@@ -82,6 +83,7 @@ public ConsumeResult TryConsumeCRLF()
                     return result;
             }
         }
+
         public bool TryConsume(int count)
         {
             if (count < 0) throw new ArgumentOutOfRangeException(nameof(count));
@@ -102,7 +104,8 @@ public bool TryConsume(int count)
                 // consume all of this span
                 _totalConsumed += available;
                 count -= available;
-            } while (FetchNextSegment());
+            }
+            while (FetchNextSegment());
             return false;
         }
 
@@ -127,13 +130,18 @@ public ReadOnlySequence<byte> ConsumeAsBuffer(int count)
             if (!TryConsumeAsBuffer(count, out var buffer)) throw new EndOfStreamException();
             return buffer;
         }
+
         public ReadOnlySequence<byte> ConsumeToEnd()
         {
             var from = SnapshotPosition();
             var result = _buffer.Slice(from);
-            while (FetchNextSegment()) { } // consume all
+            while (FetchNextSegment())
+            {
+                // consume all
+            }
             return result;
         }
+
         public bool TryConsumeAsBuffer(int count, out ReadOnlySequence<byte> buffer)
         {
             var from = SnapshotPosition();
@@ -146,6 +154,7 @@ public bool TryConsumeAsBuffer(int count, out ReadOnlySequence<byte> buffer)
             buffer = _buffer.Slice(from, to);
             return true;
         }
+
         public void Consume(int count)
         {
             if (!TryConsume(count)) throw new EndOfStreamException();
@@ -163,13 +172,14 @@ public void Consume(int count)
                 if (found >= 0) return totalSkipped + found;
 
                 totalSkipped += span.Length;
-            } while (reader.FetchNextSegment());
+            }
+            while (reader.FetchNextSegment());
             return -1;
         }
+
         internal static int FindNextCrLf(BufferReader reader) // very deliberately not ref; want snapshot
         {
             // is it in the current span? (we need to handle the offsets differently if so)
-
             int totalSkipped = 0;
             bool haveTrailingCR = false;
             do