Use `dotnet format` and `roslynator fix` to automatically apply fixes #1743

andyleejordan · 2022-03-14T22:16:32Z

It's not quite perfect, but it's automated! If everything passes then I suggest we merge this PR and keep running these tools manually until we can integrate them into CI (which is pending upstream package fixes for dotnet format). Annoyingly, I haven't figured out how to get either tool to auto-apply some fixes, such as RCS1036 and RCS1170, and I'm not sure there are automatic fixes for rules that remove unused code like IDE0005 and IDE0060. But this gets us a lot of the way there!

With `--ignored-diagnostics CA1067 CS0659 VSTHRD002 VSTHRD100 VSTHRD103 VSTHRD114 VSTHRD200 xUnit1004`

JustinGrote · 2022-03-14T23:04:13Z

Seems like a good time to do this ahead of the stable release for sure, hopefully stuff doesn't get too broken.

andyleejordan · 2022-03-14T23:14:00Z

src/PowerShellEditorServices.Hosting/Configuration/HostLogger.cs

@@ -260,7 +236,211 @@ internal class StreamLogger : IObserver<(PsesLogLevel logLevel, string message)>
    {
        public static StreamLogger CreateWithNewFile(string path)
        {
-            var fileStream = new FileStream(
+
+            /* Unmerged change from project 'PowerShellEditorServices.Hosting(net461)'


WTF is this?

andyleejordan · 2022-03-14T23:14:48Z

src/PowerShellEditorServices.Hosting/Configuration/TransportConfig.cs

-            return new DuplexNamedPipeTransportConfig(logger, NamedPipeUtils.GenerateValidNamedPipeName());
-        }
+
+        /* Unmerged change from project 'PowerShellEditorServices.Hosting(net461)'


And another.

andyleejordan · 2022-03-14T23:18:12Z

src/PowerShellEditorServices.Hosting/Internal/NamedPipeUtils.cs

@@ -137,6 +137,7 @@ public static bool IsPipeNameValid(string pipeName)
        /// </summary>
        /// <param name="pipeName">The simple name of the named pipe.</param>
        /// <returns>The full path of the named pipe.</returns>
+#pragma warning disable IDE0022


The #if CoreCLR confused the formatter, so I disabled the rule.

andyleejordan · 2022-03-14T23:18:41Z

src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs

@@ -408,10 +402,12 @@ private static object GetPSVersion()
            // which is expensive.
            // Rather than do that, we instead go straight to the source,
            // which is a static property, internal in WinPS and public in PS 6+
+#pragma warning disable CA1825
            return typeof(PSObject).Assembly
                .GetType("System.Management.Automation.PSVersionInfo")
                .GetMethod("get_PSVersion", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic)
                .Invoke(null, new object[0] /* Cannot use Array.Empty, since it must work in net452 */);


This is CA1825, hence the disabling.

andyleejordan · 2022-03-14T23:19:01Z

src/PowerShellEditorServices.VSCode/Cmdlets/VSCodeHtmlContentViewCommands.cs

@@ -59,7 +59,8 @@ protected override void BeginProcessing()
                .GetAwaiter()
                .GetResult();

-            if (_showInColumn != null) {
+            if (_showInColumn != null)


Not sure if there's a rule to convert != null to is not null...

You'd think https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0078 would do it.

andyleejordan · 2022-03-15T19:53:25Z

src/PowerShellEditorServices/Server/PsesLanguageServer.cs

@@ -136,7 +136,7 @@ public async Task StartAsync()
                            {
                                // If RootUri isn't set, try to use the first WorkspaceFolder.
                                // TODO: Support multi-workspace.
-                                foreach (var workspaceFolder in request.WorkspaceFolders)
+                                foreach (OmniSharp.Extensions.LanguageServer.Protocol.Models.WorkspaceFolder workspaceFolder in request.WorkspaceFolders)


Slightly annoying that it defaulted to the full name like this, but we'll fix it some other time.

andyleejordan · 2022-03-15T19:54:13Z

src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs

+            return markerLevel switch
            {
-                case ScriptFileMarkerLevel.Error:       return DiagnosticSeverity.Error;
-                case ScriptFileMarkerLevel.Warning:     return DiagnosticSeverity.Warning;
-                case ScriptFileMarkerLevel.Information: return DiagnosticSeverity.Information;
-                default:                                return DiagnosticSeverity.Error;
+                ScriptFileMarkerLevel.Error => DiagnosticSeverity.Error,
+                ScriptFileMarkerLevel.Warning => DiagnosticSeverity.Warning,
+                ScriptFileMarkerLevel.Information => DiagnosticSeverity.Information,
+                _ => DiagnosticSeverity.Error,
            };
+            ;


I love that it auto converted these, damn!

andyleejordan · 2022-03-15T20:01:31Z

src/PowerShellEditorServices/Services/CodeLens/PesterCodeLensProvider.cs

            foreach (SymbolReference symbol in _symbolProvider.ProvideDocumentSymbols(scriptFile))
            {
-                if (!(symbol is PesterSymbolReference pesterSymbol))
+                if (symbol is not PesterSymbolReference pesterSymbol)


Hm so it does correct these...

andyleejordan · 2022-03-15T20:02:26Z

src/PowerShellEditorServices/Services/DebugAdapter/Debugging/BreakpointApiUtils.cs

@@ -224,7 +218,7 @@ public static ScriptBlock GetBreakpointActionScriptBlock(string condition, strin
                    // Check for "advanced" condition syntax i.e. if the user has specified
                    // a "break" or  "continue" statement anywhere in their scriptblock,
                    // pass their scriptblock through to the Action parameter as-is.
-                    if (parsed.Ast.Find(ast => ast is BreakStatementAst || ast is ContinueStatementAst, true) is not null)
+                    if (parsed.Ast.Find(ast => ast is BreakStatementAst or ContinueStatementAst, true) is not null)


And it does these...

andyleejordan · 2022-03-15T20:04:53Z

src/PowerShellEditorServices/Services/DebugAdapter/Debugging/VariableDetails.cs

@@ -133,26 +132,23 @@ private static bool GetIsExpandable(object valueObject)
            }

            // If a PSObject, unwrap it
-            var psobject = valueObject as PSObject;
-            if (psobject != null)
+            if (valueObject is PSObject psobject)


Damn it's funny that it can fix these but not == null to is null etc.

andyleejordan · 2022-03-15T20:05:16Z

src/PowerShellEditorServices/Services/DebugAdapter/Debugging/VariableDetails.cs

@@ -133,26 +132,23 @@ private static bool GetIsExpandable(object valueObject)
            }

            // If a PSObject, unwrap it
-            var psobject = valueObject as PSObject;
-            if (psobject != null)
+            if (valueObject is PSObject psobject)
            {
                valueObject = psobject.BaseObject;
            }

            Type valueType =


I wish it would auto-squash the lines.

andyleejordan · 2022-03-15T20:08:41Z

src/PowerShellEditorServices/Services/PowerShell/Console/ConsoleProxy.cs

@@ -1,5 +1,5 @@
-// Copyright (c) Microsoft. All rights reserved.
-// Licensed under the MIT license.
+// Copyright (c) Microsoft Corporation.


Yay, I love that these can be auto-fixed now! Because we did it semi-manually last time in #1580 and clearly missed some.

andyleejordan · 2022-03-15T20:09:35Z

src/PowerShellEditorServices/Services/PowerShell/Console/UnixConsoleOperations.cs

+                while (!WaitForKeyAvailable(cancellationToken))
+                {
+                    ;
+                }


Hm funny, but technically correct.

andyleejordan

@SeeminglyScience I'm good with this if you are! I enabled auto-merge.

SeeminglyScience

LGTM!

Yeah the is null thing makes sense unfortunately. Even if rare (and usually unintentional) it would need too much context to determine if there was a behavioral change.

andyleejordan requested a review from SeeminglyScience March 14, 2022 22:16

andyleejordan added the Ignore Exclude from the changelog. label Mar 14, 2022

andyleejordan added 2 commits March 14, 2022 15:23

Update .editorconfig with IDE rules

021e24d

Run dotnet format style until no changes were made

44387b3

andyleejordan force-pushed the andschwa/dotnet-format branch from 669986b to 1b4cd89 Compare March 14, 2022 22:23

andyleejordan marked this pull request as ready for review March 14, 2022 22:26

andyleejordan force-pushed the andschwa/dotnet-format branch from 1b4cd89 to 589e625 Compare March 14, 2022 22:49

andyleejordan added 2 commits March 14, 2022 15:55

Run roslynator fix

7233182

With `--ignored-diagnostics CA1067 CS0659 VSTHRD002 VSTHRD100 VSTHRD103 VSTHRD114 VSTHRD200 xUnit1004`

Run dotnet format whitespace

15cdde3

andyleejordan force-pushed the andschwa/dotnet-format branch from 589e625 to 15cdde3 Compare March 14, 2022 22:55

andyleejordan commented Mar 14, 2022

View reviewed changes

Remove 'unmerged changes' from dotnet format

21b1f35

andyleejordan commented Mar 14, 2022

View reviewed changes

andyleejordan commented Mar 15, 2022

View reviewed changes

andyleejordan enabled auto-merge March 15, 2022 21:05

andyleejordan commented Mar 15, 2022

View reviewed changes

andyleejordan merged commit 2c879ca into master Mar 16, 2022

SeeminglyScience approved these changes Mar 16, 2022

View reviewed changes

andyleejordan deleted the andschwa/dotnet-format branch March 16, 2022 04:03

andyleejordan mentioned this pull request Mar 24, 2022

Release v3.1.6 #1747

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use `dotnet format` and `roslynator fix` to automatically apply fixes #1743

Use `dotnet format` and `roslynator fix` to automatically apply fixes #1743

andyleejordan commented Mar 14, 2022

JustinGrote commented Mar 14, 2022

andyleejordan Mar 14, 2022

andyleejordan Mar 14, 2022

andyleejordan Mar 14, 2022

andyleejordan Mar 14, 2022

andyleejordan Mar 14, 2022

andyleejordan Mar 14, 2022

andyleejordan Mar 14, 2022

andyleejordan Mar 15, 2022

andyleejordan Mar 15, 2022

andyleejordan Mar 15, 2022

andyleejordan Mar 15, 2022

andyleejordan Mar 15, 2022

andyleejordan Mar 15, 2022

andyleejordan Mar 15, 2022

andyleejordan Mar 15, 2022

andyleejordan Mar 15, 2022

andyleejordan left a comment

SeeminglyScience left a comment

Use dotnet format and roslynator fix to automatically apply fixes #1743

Use dotnet format and roslynator fix to automatically apply fixes #1743

Conversation

andyleejordan commented Mar 14, 2022

JustinGrote commented Mar 14, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andyleejordan left a comment

Choose a reason for hiding this comment

SeeminglyScience left a comment

Choose a reason for hiding this comment

Use `dotnet format` and `roslynator fix` to automatically apply fixes #1743

Use `dotnet format` and `roslynator fix` to automatically apply fixes #1743