Merge pull request #1757 from PowerShell/andschwa/format-check

Enable code analysis and formatting as errors on build

Author: andyleejordan

.editorconfig

@@ -22,6 +22,8 @@ csharp_style_expression_bodied_indexers = true
 csharp_style_expression_bodied_accessors = true
 csharp_style_expression_bodied_lambdas = when_on_single_line
 
+# TODO: Fix silenced and suggested rules!
+
 # CS0168: The variable 'var' is declared but never used
 dotnet_diagnostic.CS0168.severity = error
 # CS0169: The private field 'class member' is never used
@@ -30,10 +32,37 @@ dotnet_diagnostic.CS0169.severity = error
 dotnet_diagnostic.CS0219.severity = error
 # CS0414: The private field 'field' is assigned but its value is never used
 dotnet_diagnostic.CS0414.severity = error
+# CS0618: A class member was marked with the Obsolete attribute
+dotnet_diagnostic.CS0618.severity = suggestion
+# CS0649: Uninitialized private or internal field declaration that is never assigned a value
+dotnet_diagnostic.CS0649.severity = error
+# CS1998: This async method lacks 'await' operators and will run synchronously
+dotnet_diagnostic.CS1998.severity = suggestion
+# CS4014: Consider applying the await operator to the result of the call
+dotnet_diagnostic.CS4014.severity = suggestion
+
 # CA1067: Should override Equals because it implements IEquatable<T>
 dotnet_diagnostic.CA1067.severity = silent
 # CA1068: CancellationToken parameters must come last
 dotnet_diagnostic.CA1068.severity = error
+# CA1501: Avoid excessive inheritance
+dotnet_diagnostic.CA1501.severity = error
+# CA1502: Avoid excessive complexity
+dotnet_diagnostic.CA1502.severity = warning
+# CA1505: Avoid unmaintainable code
+dotnet_diagnostic.CA1505.severity = error
+# CA1506: Avoid excessive class coupling
+dotnet_diagnostic.CA1506.severity = warning
+# CA1507: Use nameof in place of string
+dotnet_diagnostic.CA1507.severity = error
+# CA1508: Avoid dead conditional code
+dotnet_diagnostic.CA1508.severity = error
+# CA1802: Use Literals Where Appropriate
+dotnet_diagnostic.CA1802.severity = error
+# CA1805: Do not initialize unnecessarily.
+dotnet_diagnostic.CA1805.severity = error
+# CA1820: Test for empty strings using string length
+dotnet_diagnostic.CA1820.severity = error
 # CA1822: Mark members as static
 dotnet_diagnostic.CA1822.severity = error
 # CA1823: Avoid unused private fields
@@ -42,40 +71,27 @@ dotnet_diagnostic.CA1823.severity = error
 dotnet_diagnostic.CA2007.severity = error
 # CA2016: Forward the CancellationToken parameter to methods that take one
 dotnet_diagnostic.CA2016.severity = error
+# CA2213: Disposable fields should be disposed
+dotnet_diagnostic.CA2213.severity = error
 # CA2254: The logging message template should not vary between calls to 'LoggerExtensions.*'
 dotnet_diagnostic.CA2254.severity = silent
 
-# TODO: Enable all maintainability issues (dead code etc.) and enforce
-# See: https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/maintainability-warnings
-dotnet_analyzer_diagnostic.category-Maintainability.severity = error
-
-# TODO: Fix all of these issues and explicitly ignore the intentional ones!
-
-# CA2016: Forward the CancellationToken parameter to methods that take one
-dotnet_diagnostic.CA2016.severity = error
-# CS0618: A class member was marked with the Obsolete attribute
-dotnet_diagnostic.CS0618.severity = suggestion
-# CS0649: Uninitialized private or internal field declaration that is never assigned a value
-dotnet_diagnostic.CS0649.severity = warning
-# CS1998: This async method lacks 'await' operators and will run synchronously
-dotnet_diagnostic.CS1998.severity = suggestion
-# CS4014: Consider applying the await operator to the result of the call
-dotnet_diagnostic.CS4014.severity = suggestion
-
+# RCS1049: Simplify boolean comparison
+dotnet_diagnostic.RCS1049.severity = error
 # RCS1102: Make class static
-dotnet_diagnostic.RCS1102.severity = warning
+dotnet_diagnostic.RCS1102.severity = error
 # RCS1139: Add summary element to documentation comment
 dotnet_diagnostic.RCS1139.severity = silent
 # RCS1194: Implement exception constructors
 dotnet_diagnostic.RCS1194.severity = suggestion
 # RCS1210: Return completed task instead of returning null
-dotnet_diagnostic.RCS1210.severity = suggestion
+dotnet_diagnostic.RCS1210.severity = error
 # RCS1036: Remove unnecessary blank line
-dotnet_diagnostic.RCS1036.severity = warning
+dotnet_diagnostic.RCS1036.severity = error
 # RCS1075: Avoid empty catch clause that catches System.Exception
 dotnet_diagnostic.RCS1075.severity = suggestion
 # RCS1170: Use read-only auto-implemented property
-dotnet_diagnostic.RCS1170.severity = warning
+dotnet_diagnostic.RCS1170.severity = error
 
 # VSTHRD002: Avoid problematic synchronous waits
 dotnet_diagnostic.VSTHRD002.severity = suggestion
@@ -90,123 +106,123 @@ dotnet_diagnostic.VSTHRD103.severity = suggestion
 # VSTHRD110: Observe result of async calls
 dotnet_diagnostic.VSTHRD110.severity = suggestion
 # VSTHRD114: Avoid returning a null Task
-dotnet_diagnostic.VSTHRD114.severity = suggestion
+dotnet_diagnostic.VSTHRD114.severity = error
 # VSTHRD200: Use "Async" suffix for awaitable methods
 dotnet_diagnostic.VSTHRD200.severity = silent
 
 # xUnit2013: Do not use equality check to check for collection size
-dotnet_diagnostic.xUnit2013.severity = warning
+dotnet_diagnostic.xUnit2013.severity = error
 # xUnit1004: Test methods should not be skipped
 dotnet_diagnostic.xUnit1004.severity = suggestion
 
 # IDE0001: Simplify name
-dotnet_diagnostic.IDE0001.severity = warning
+dotnet_diagnostic.IDE0001.severity = error
 # IDE0002: Simplify member access
-dotnet_diagnostic.IDE0001.severity = warning
+dotnet_diagnostic.IDE0001.severity = error
 # IDE0003: Remove this or Me qualification
-dotnet_diagnostic.IDE0003.severity = warning
+dotnet_diagnostic.IDE0003.severity = error
 # IDE0004: Remove unnecessary cast
-dotnet_diagnostic.IDE0004.severity = warning
+dotnet_diagnostic.IDE0004.severity = error
 # IDE0005: Remove unnecessary import
-dotnet_diagnostic.IDE0005.severity = suggestion
+dotnet_diagnostic.IDE0005.severity = error
 # IDE0008: Use explicit type instead of var
-dotnet_diagnostic.IDE0008.severity = warning
+dotnet_diagnostic.IDE0008.severity = error
 # IDE0011: Add braces
-dotnet_diagnostic.IDE0011.severity = warning
+dotnet_diagnostic.IDE0011.severity = error
 # IDE0016: Use throw expression
-dotnet_diagnostic.IDE0016.severity = warning
+dotnet_diagnostic.IDE0016.severity = error
 # IDE0017: Use object initializers
-dotnet_diagnostic.IDE0017.severity = warning
+dotnet_diagnostic.IDE0017.severity = error
 # IDE0018: Inline variable declaration
-dotnet_diagnostic.IDE0018.severity = warning
+dotnet_diagnostic.IDE0018.severity = error
 # IDE0019: Use pattern matching to avoid 'as' followed by a 'null' check
-dotnet_diagnostic.IDE0019.severity = warning
+dotnet_diagnostic.IDE0019.severity = error
 # IDE0020: Use pattern matching to avoid 'is' check followed by a cast (with variable)
-dotnet_diagnostic.IDE0020.severity = warning
+dotnet_diagnostic.IDE0020.severity = error
 # IDE0021: Use expression body for constructors
-dotnet_diagnostic.IDE0021.severity = warning
+dotnet_diagnostic.IDE0021.severity = error
 # IDE0022: Use expression body for methods
-dotnet_diagnostic.IDE0022.severity = warning
+dotnet_diagnostic.IDE0022.severity = error
 # IDE0023: Use expression body for conversion operators
-dotnet_diagnostic.IDE0023.severity = warning
+dotnet_diagnostic.IDE0023.severity = error
 # IDE0024: Use expression body for operators
-dotnet_diagnostic.IDE0024.severity = warning
+dotnet_diagnostic.IDE0024.severity = error
 # IDE0025: Use expression body for properties
-dotnet_diagnostic.IDE0025.severity = warning
+dotnet_diagnostic.IDE0025.severity = error
 # IDE0026: Use expression body for indexers
-dotnet_diagnostic.IDE0026.severity = warning
+dotnet_diagnostic.IDE0026.severity = error
 # IDE0027: Use expression body for accessors
-dotnet_diagnostic.IDE0027.severity = warning
+dotnet_diagnostic.IDE0027.severity = error
 # IDE0028: Use collection initializers
-dotnet_diagnostic.IDE0028.severity = warning
+dotnet_diagnostic.IDE0028.severity = error
 # IDE0029: Use coalesce expression (non-nullable types)
-dotnet_diagnostic.IDE0029.severity = warning
+dotnet_diagnostic.IDE0029.severity = error
 # IDE0030: Use coalesce expression (nullable types)
-dotnet_diagnostic.IDE0030.severity = warning
+dotnet_diagnostic.IDE0030.severity = error
 # IDE0031: Use null propagation
-dotnet_diagnostic.IDE0031.severity = warning
+dotnet_diagnostic.IDE0031.severity = error
 # IDE0032: Use auto property
-dotnet_diagnostic.IDE0032.severity = warning
+dotnet_diagnostic.IDE0032.severity = error
 # IDE0033: Use explicitly provided tuple name
-dotnet_diagnostic.IDE0033.severity = warning
+dotnet_diagnostic.IDE0033.severity = error
 # IDE0034: Simplify 'default' expression
-dotnet_diagnostic.IDE0034.severity = warning
+dotnet_diagnostic.IDE0034.severity = error
 # IDE0035: Remove unreachable code
-dotnet_diagnostic.IDE0035.severity = warning
+dotnet_diagnostic.IDE0035.severity = error
 # IDE0036: Order modifiers
-dotnet_diagnostic.IDE0036.severity = warning
+dotnet_diagnostic.IDE0036.severity = error
 # IDE0037: Use inferred member name
-dotnet_diagnostic.IDE0037.severity = warning
+dotnet_diagnostic.IDE0037.severity = error
 # IDE0038: Use pattern matching to avoid is check followed by a cast (without variable)
-dotnet_diagnostic.IDE0038.severity = suggestion
+dotnet_diagnostic.IDE0038.severity = error
 # IDE0040: Add accessibility modifiers
-dotnet_diagnostic.IDE0040.severity = warning
+dotnet_diagnostic.IDE0040.severity = error
 # IDE0041: Use is null check
-dotnet_diagnostic.IDE0041.severity = warning
+dotnet_diagnostic.IDE0041.severity = error
 # IDE0042: Deconstruct variable declaration
-dotnet_diagnostic.IDE0042.severity = warning
+dotnet_diagnostic.IDE0042.severity = error
 # IDE0044: Add readonly modifier
-dotnet_diagnostic.IDE0044.severity = warning
+dotnet_diagnostic.IDE0044.severity = error
 # IDE0045: Use conditional expression for assignment
-dotnet_diagnostic.IDE0045.severity = warning
+dotnet_diagnostic.IDE0045.severity = error
 # IDE0046: Use conditional expression for return
 dotnet_diagnostic.IDE0046.severity = silent
 # IDE0047: Remove unnecessary parentheses
-dotnet_diagnostic.IDE0047.severity = warning
+dotnet_diagnostic.IDE0047.severity = error
 # IDE0049: Use language keywords instead of framework type names for type references
-dotnet_diagnostic.IDE0049.severity = warning
+dotnet_diagnostic.IDE0049.severity = error
 # IDE0051: Remove unused private member
-dotnet_diagnostic.IDE0051.severity = warning
+dotnet_diagnostic.IDE0051.severity = error
 # IDE0052: Remove unread private member
-dotnet_diagnostic.IDE0052.severity = suggestion
+dotnet_diagnostic.IDE0052.severity = error
 # IDE0053: Use expression body for lambdas
-dotnet_diagnostic.IDE0053.severity = warning
+dotnet_diagnostic.IDE0053.severity = error
 # IDE0054: Use compound assignment
-dotnet_diagnostic.IDE0054.severity = warning
+dotnet_diagnostic.IDE0054.severity = error
 # IDE0063: Use simple 'using' statement
-dotnet_diagnostic.IDE0063.severity = warning
+dotnet_diagnostic.IDE0063.severity = error
 # IDE0066: Use switch expression
-dotnet_diagnostic.IDE0066.severity = warning
+dotnet_diagnostic.IDE0066.severity = error
 # IDE0071: Simplify interpolation
-dotnet_diagnostic.IDE0071.severity = warning
+dotnet_diagnostic.IDE0071.severity = error
 # IDE0073: Require file header
-dotnet_diagnostic.IDE0073.severity = warning
+dotnet_diagnostic.IDE0073.severity = error
 # IDE0075: Simplify conditional expression
-dotnet_diagnostic.IDE0075.severity = warning
+dotnet_diagnostic.IDE0075.severity = error
 # IDE0078: Use pattern matching
-dotnet_diagnostic.IDE0078.severity = warning
+dotnet_diagnostic.IDE0078.severity = error
 # IDE0082: Convert typeof to nameof
-dotnet_diagnostic.IDE0082.severity = warning
+dotnet_diagnostic.IDE0082.severity = error
 # IDE0083: Use pattern matching (not operator)
-dotnet_diagnostic.IDE0083.severity = warning
+dotnet_diagnostic.IDE0083.severity = error
 # IDE0090: Simplify new expression
-dotnet_diagnostic.IDE0090.severity = warning
+dotnet_diagnostic.IDE0090.severity = error
 # IDE0100: Remove unnecessary equality operator
-dotnet_diagnostic.IDE0100.severity = warning
+dotnet_diagnostic.IDE0100.severity = error
 # IDE0110: Remove unnecessary discard
-dotnet_diagnostic.IDE0110.severity = warning
+dotnet_diagnostic.IDE0110.severity = error
 # IDE1005: Use conditional delegate call
-dotnet_diagnostic.IDE1005.severity = warning
+dotnet_diagnostic.IDE1005.severity = error
 
 [*.{json}]
 indent_size = 2

PowerShellEditorServices.Common.props

@@ -14,6 +14,7 @@
     <!-- See: https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/overview -->
     <EnableNETAnalyzers>true</EnableNETAnalyzers>
     <EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
+    <!-- TODO: Enable <AnalysisMode>All</AnalysisMode> -->
     <!-- See: https://docs.microsoft.com/en-us/dotnet/core/compatibility/sdk/6.0/implicit-namespaces -->
     <DisableImplicitNamespaceImports>true</DisableImplicitNamespaceImports>
   </PropertyGroup>

src/PowerShellEditorServices.Hosting/Commands/StartEditorServicesCommand.cs

@@ -191,6 +191,7 @@ public StartEditorServicesCommand()
         [Parameter]
         public string StartupBanner { get; set; }
 
+#pragma warning disable IDE0022
         protected override void BeginProcessing()
         {
 #if DEBUG
@@ -203,10 +204,10 @@ protected override void BeginProcessing()
                 }
             }
 #endif
-
             // Set up logging now for use throughout startup
             StartLogging();
         }
+#pragma warning restore IDE0022
 
         [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Uses ThrowTerminatingError() instead")]
         protected override void EndProcessing()

src/PowerShellEditorServices.Hosting/Configuration/EditorServicesConfig.cs

@@ -76,12 +76,12 @@ public EditorServicesConfig(
         /// <summary>
         /// Names of or paths to any additional modules to load on startup.
         /// </summary>
-        public IReadOnlyList<string> AdditionalModules { get; set; } = null;
+        public IReadOnlyList<string> AdditionalModules { get; set; }
 
         /// <summary>
         /// Flags of features to enable on startup.
         /// </summary>
-        public IReadOnlyList<string> FeatureFlags { get; set; } = null;
+        public IReadOnlyList<string> FeatureFlags { get; set; }
 
         /// <summary>
         /// The console REPL experience to use in the integrated console
@@ -97,12 +97,12 @@ public EditorServicesConfig(
         /// <summary>
         /// Configuration for the language server protocol transport to use.
         /// </summary>
-        public ITransportConfig LanguageServiceTransport { get; set; } = null;
+        public ITransportConfig LanguageServiceTransport { get; set; }
 
         /// <summary>
         /// Configuration for the debug adapter protocol transport to use.
         /// </summary>
-        public ITransportConfig DebugServiceTransport { get; set; } = null;
+        public ITransportConfig DebugServiceTransport { get; set; }
 
         /// <summary>
         /// PowerShell profile locations for Editor Services to use for its profiles.

src/PowerShellEditorServices.Hosting/Configuration/TransportConfig.cs

@@ -49,7 +49,7 @@ public sealed class StdioTransportConfig : ITransportConfig
 
         public string SessionFileTransportName => "Stdio";
 
-        public IReadOnlyDictionary<string, object> SessionFileEntries { get; } = null;
+        public IReadOnlyDictionary<string, object> SessionFileEntries { get; }
 
         public Task<(Stream inStream, Stream outStream)> ConnectStreamsAsync()
         {

src/PowerShellEditorServices.Hosting/Internal/NamedPipeUtils.cs

@@ -4,11 +4,12 @@
 using System.Collections.Generic;
 using System.IO;
 using System.IO.Pipes;
-using System.Runtime.InteropServices;
 
 #if !CoreCLR
 using System.Security.Principal;
 using System.Security.AccessControl;
+#else
+using System.Runtime.InteropServices;
 #endif
 
 namespace Microsoft.PowerShell.EditorServices.Hosting

src/PowerShellEditorServices/Hosting/EditorServicesServerFactory.cs

@@ -58,7 +58,8 @@ public static EditorServicesServerFactory Create(string logPath, int minimumLogL
             SelfLog.Enable(msg => Debug.WriteLine(msg));
 #endif
 
-            ILoggerFactory loggerFactory = new LoggerFactory().AddSerilog();
+            LoggerFactory loggerFactory = new();
+            loggerFactory.AddSerilog();
 
             // Hook up logging from the host so that its recorded in the log file
             hostLogger.Subscribe(new HostLoggerAdapter(loggerFactory));

src/PowerShellEditorServices/Server/PsesDebugServer.cs

@@ -124,6 +124,7 @@ public void Dispose()
             _debugAdapterServer.Dispose();
             _inputStream.Dispose();
             _outputStream.Dispose();
+            _loggerFactory.Dispose();
             _serverStopped.SetResult(true);
             // TODO: If the debugger has stopped, should we clear the breakpoints?
         }