Add better logging for formatter and refactor it into 1 class (#1228)

PowerShell · Mar 11, 2020 · 487454a · 487454a
1 parent 5b7af3f
commit 487454a
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 43 deletions.
diff --git a/src/PowerShellEditorServices/Server/PsesLanguageServer.cs b/src/PowerShellEditorServices/Server/PsesLanguageServer.cs
@@ -73,8 +73,7 @@ public async Task StartAsync()
                     .WithHandler<GetVersionHandler>()
                     .WithHandler<ConfigurationHandler>()
                     .WithHandler<FoldingRangeHandler>()
-                    .WithHandler<DocumentFormattingHandler>()
-                    .WithHandler<DocumentRangeFormattingHandler>()
+                    .WithHandler<DocumentFormattingHandlers>()
                     .WithHandler<ReferencesHandler>()
                     .WithHandler<DocumentSymbolHandler>()
                     .WithHandler<DocumentHighlightHandler>()

diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/FormattingHandlers.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/FormattingHandlers.cs
@@ -14,17 +14,24 @@
 
 namespace Microsoft.PowerShell.EditorServices.Handlers
 {
-    internal class DocumentFormattingHandler : IDocumentFormattingHandler
+    // TODO: Add IDocumentOnTypeFormatHandler to support on-type formatting.
+    internal class DocumentFormattingHandlers : IDocumentFormattingHandler, IDocumentRangeFormattingHandler
     {
         private readonly ILogger _logger;
         private readonly AnalysisService _analysisService;
         private readonly ConfigurationService _configurationService;
         private readonly WorkspaceService _workspaceService;
-        private DocumentFormattingCapability _capability;
 
-        public DocumentFormattingHandler(ILoggerFactory factory, AnalysisService analysisService, ConfigurationService configurationService, WorkspaceService workspaceService)
+        private DocumentFormattingCapability _documentFormattingCapability;
+        private DocumentRangeFormattingCapability _documentRangeFormattingCapability;
+
+        public DocumentFormattingHandlers(
+            ILoggerFactory factory,
+            AnalysisService analysisService,
+            ConfigurationService configurationService,
+            WorkspaceService workspaceService)
         {
-            _logger = factory.CreateLogger<DocumentFormattingHandler>();
+            _logger = factory.CreateLogger<DocumentFormattingHandlers>();
             _analysisService = analysisService;
             _configurationService = configurationService;
             _workspaceService = workspaceService;
@@ -43,7 +50,8 @@ public async Task<TextEditContainer> Handle(DocumentFormattingParams request, Ca
             var scriptFile = _workspaceService.GetFile(request.TextDocument.Uri);
             var pssaSettings = _configurationService.CurrentSettings.CodeFormatting.GetPSSASettingsHashtable(
                 (int)request.Options.TabSize,
-                request.Options.InsertSpaces);
+                request.Options.InsertSpaces,
+                _logger);
 
 
             // TODO raise an error event in case format returns null
@@ -79,42 +87,13 @@ public async Task<TextEditContainer> Handle(DocumentFormattingParams request, Ca
             });
         }
 
-        public void SetCapability(DocumentFormattingCapability capability)
-        {
-            _capability = capability;
-        }
-    }
-
-    internal class DocumentRangeFormattingHandler : IDocumentRangeFormattingHandler
-    {
-        private readonly ILogger _logger;
-        private readonly AnalysisService _analysisService;
-        private readonly ConfigurationService _configurationService;
-        private readonly WorkspaceService _workspaceService;
-        private DocumentRangeFormattingCapability _capability;
-
-        public DocumentRangeFormattingHandler(ILoggerFactory factory, AnalysisService analysisService, ConfigurationService configurationService, WorkspaceService workspaceService)
-        {
-            _logger = factory.CreateLogger<DocumentRangeFormattingHandler>();
-            _analysisService = analysisService;
-            _configurationService = configurationService;
-            _workspaceService = workspaceService;
-        }
-
-        public TextDocumentRegistrationOptions GetRegistrationOptions()
-        {
-            return new TextDocumentRegistrationOptions
-            {
-                DocumentSelector = LspUtils.PowerShellDocumentSelector
-            };
-        }
-
         public async Task<TextEditContainer> Handle(DocumentRangeFormattingParams request, CancellationToken cancellationToken)
         {
             var scriptFile = _workspaceService.GetFile(request.TextDocument.Uri);
             var pssaSettings = _configurationService.CurrentSettings.CodeFormatting.GetPSSASettingsHashtable(
                 (int)request.Options.TabSize,
-                request.Options.InsertSpaces);
+                request.Options.InsertSpaces,
+                _logger);
 
             // TODO raise an error event in case format returns null;
             string formattedScript;
@@ -137,17 +116,24 @@ public async Task<TextEditContainer> Handle(DocumentRangeFormattingParams reques
             };
 
             Range range = request.Range;
-            var rangeList = range == null ? null : new int[] {
+            var rangeList = range == null ? null : new int[]
+            {
                 (int)range.Start.Line + 1,
                 (int)range.Start.Character + 1,
                 (int)range.End.Line + 1,
-                (int)range.End.Character + 1};
+                (int)range.End.Character + 1
+            };
 
             formattedScript = await _analysisService.FormatAsync(
                 scriptFile.Contents,
                 pssaSettings,
                 rangeList).ConfigureAwait(false);
-            formattedScript = formattedScript ?? scriptFile.Contents;
+
+            if (formattedScript == null)
+            {
+                _logger.LogWarning("Formatting returned null. Returning original contents for file: {0}", scriptFile.DocumentUri);
+                formattedScript = scriptFile.Contents;
+            }
 
             return new TextEditContainer(new TextEdit
             {
@@ -156,9 +142,14 @@ public async Task<TextEditContainer> Handle(DocumentRangeFormattingParams reques
             });
         }
 
+        public void SetCapability(DocumentFormattingCapability capability)
+        {
+            _documentFormattingCapability = capability;
+        }
+
         public void SetCapability(DocumentRangeFormattingCapability capability)
         {
-            _capability = capability;
+            _documentRangeFormattingCapability = capability;
         }
     }
 }
diff --git a/src/PowerShellEditorServices/Services/Workspace/LanguageServerSettings.cs b/src/PowerShellEditorServices/Services/Workspace/LanguageServerSettings.cs
@@ -11,6 +11,7 @@
 using System.Security;
 using Microsoft.Extensions.Logging;
 using Microsoft.PowerShell.EditorServices.Logging;
+using Newtonsoft.Json;
 
 namespace Microsoft.PowerShell.EditorServices.Services.Configuration
 {
@@ -223,7 +224,8 @@ public CodeFormattingSettings(CodeFormattingSettings codeFormattingSettings)
         /// <returns></returns>
         public Hashtable GetPSSASettingsHashtable(
             int tabSize,
-            bool insertSpaces)
+            bool insertSpaces,
+            ILogger logger)
         {
             var settings = GetCustomPSSASettingsHashtable(tabSize, insertSpaces);
             var ruleSettings = (Hashtable)(settings["Rules"]);
@@ -253,6 +255,7 @@ public Hashtable GetPSSASettingsHashtable(
                     break;
             }
 
+            logger.LogDebug("Created formatting hashtable: {0}", JsonConvert.SerializeObject(settings));
             return settings;
         }