Skip to content

Commit 020b00a

Browse files
rjmholtrjmholt
authored
Implement textDocument/codeAction (#1004)
* Add initial handler * Add working codeAction implementation * Crash * Make tests work * Fix issues * Make tests work in WinPS
1 parent 2810c64 commit 020b00a

File tree

11 files changed

+346
-40
lines changed

11 files changed

+346
-40
lines changed

‎src/PowerShellEditorServices.Engine/LanguageServer/OmnisharpLanguageServer.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,8 @@ public async Task StartAsync()
9494
.WithHandler<DocumentSymbolHandler>()
9595
.WithHandler<DocumentHighlightHandler>()
9696
.WithHandler<PSHostProcessAndRunspaceHandlers>()
97-
.WithHandler<CodeLensHandlers>();
97+
.WithHandler<CodeLensHandlers>()
98+
.WithHandler<CodeActionHandler>();
9899

99100
logger.LogInformation("Handlers added");
100101
});

‎src/PowerShellEditorServices.Engine/PowerShellEditorServices.Engine.csproj‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212

1313
<ItemGroup>
1414
<PackageReference Include="Microsoft.Extensions.FileSystemGlobbing" Version="2.2.0" />
15-
<PackageReference Include="OmniSharp.Extensions.LanguageServer" Version="0.13.0-*" />
15+
<PackageReference Include="OmniSharp.Extensions.LanguageServer" Version="0.13.2-*" />
1616
<PackageReference Include="PowerShellStandard.Library" Version="5.1.1" />
1717
<PackageReference Include="Serilog.Extensions.Logging" Version="2.0.4" />
1818
<PackageReference Include="Serilog.Sinks.Console" Version="3.1.1" />

‎src/PowerShellEditorServices.Engine/Services/Analysis/AnalysisService.cs‎

Lines changed: 76 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,15 @@
1515
using OmniSharp.Extensions.LanguageServer.Protocol.Models;
1616
using OmniSharp.Extensions.LanguageServer.Protocol.Server;
1717
using System.Threading;
18+
using System.Collections.Concurrent;
1819

1920
namespace Microsoft.PowerShell.EditorServices
2021
{
2122
/// <summary>
2223
/// Provides a high-level service for performing semantic analysis
2324
/// of PowerShell scripts.
2425
/// </summary>
25-
public class AnalysisService : IDisposable
26+
internal class AnalysisService : IDisposable
2627
{
2728
#region Static fields
2829

@@ -54,9 +55,6 @@ public class AnalysisService : IDisposable
5455

5556
private static readonly string[] s_emptyGetRuleResult = new string[0];
5657

57-
private Dictionary<string, Dictionary<string, MarkerCorrection>> codeActionsPerFile =
58-
new Dictionary<string, Dictionary<string, MarkerCorrection>>();
59-
6058
private static CancellationTokenSource s_existingRequestCancellation;
6159

6260
/// <summary>
@@ -96,8 +94,11 @@ public class AnalysisService : IDisposable
9694
private PSModuleInfo _pssaModuleInfo;
9795

9896
private readonly ILanguageServer _languageServer;
97+
9998
private readonly ConfigurationService _configurationService;
10099

100+
private readonly ConcurrentDictionary<string, (SemaphoreSlim, Dictionary<string, MarkerCorrection>)> _mostRecentCorrectionsByFile;
101+
101102
#endregion // Private Fields
102103

103104
#region Properties
@@ -149,6 +150,7 @@ private AnalysisService(
149150
_configurationService = configurationService;
150151
_logger = logger;
151152
_pssaModuleInfo = pssaModuleInfo;
153+
_mostRecentCorrectionsByFile = new ConcurrentDictionary<string, (SemaphoreSlim, Dictionary<string, MarkerCorrection>)>();
152154
}
153155

154156
#endregion // constructors
@@ -813,26 +815,57 @@ private void PublishScriptDiagnostics(
813815
ScriptFile scriptFile,
814816
List<ScriptFileMarker> markers)
815817
{
816-
List<Diagnostic> diagnostics = new List<Diagnostic>();
818+
var diagnostics = new List<Diagnostic>();
817819

818-
// Hold on to any corrections that may need to be applied later
819-
Dictionary<string, MarkerCorrection> fileCorrections =
820-
new Dictionary<string, MarkerCorrection>();
820+
// Create the entry for this file if it does not already exist
821+
SemaphoreSlim fileLock;
822+
Dictionary<string, MarkerCorrection> fileCorrections;
823+
bool newEntryNeeded = false;
824+
if (_mostRecentCorrectionsByFile.TryGetValue(scriptFile.DocumentUri, out (SemaphoreSlim, Dictionary<string, MarkerCorrection>) fileCorrectionsEntry))
825+
{
826+
fileLock = fileCorrectionsEntry.Item1;
827+
fileCorrections = fileCorrectionsEntry.Item2;
828+
}
829+
else
830+
{
831+
fileLock = new SemaphoreSlim(initialCount: 1, maxCount: 1);
832+
fileCorrections = new Dictionary<string, MarkerCorrection>();
833+
newEntryNeeded = true;
834+
}
821835

822-
foreach (var marker in markers)
836+
fileLock.Wait();
837+
try
823838
{
824-
// Does the marker contain a correction?
825-
Diagnostic markerDiagnostic = GetDiagnosticFromMarker(marker);
826-
if (marker.Correction != null)
839+
if (newEntryNeeded)
827840
{
828-
string diagnosticId = GetUniqueIdFromDiagnostic(markerDiagnostic);
829-
fileCorrections[diagnosticId] = marker.Correction;
841+
// If we create a new entry, we should do it after acquiring the lock we just created
842+
// to ensure a competing thread can never acquire it first and read invalid information from it
843+
_mostRecentCorrectionsByFile[scriptFile.DocumentUri] = (fileLock, fileCorrections);
830844
}
845+
else
846+
{
847+
// Otherwise we need to clear the stale corrections
848+
fileCorrections.Clear();
849+
}
850+
851+
foreach (ScriptFileMarker marker in markers)
852+
{
853+
// Does the marker contain a correction?
854+
Diagnostic markerDiagnostic = GetDiagnosticFromMarker(marker);
855+
if (marker.Correction != null)
856+
{
857+
string diagnosticId = GetUniqueIdFromDiagnostic(markerDiagnostic);
858+
fileCorrections[diagnosticId] = marker.Correction;
859+
}
831860

832-
diagnostics.Add(markerDiagnostic);
861+
diagnostics.Add(markerDiagnostic);
862+
}
863+
}
864+
finally
865+
{
866+
fileLock.Release();
833867
}
834868

835-
codeActionsPerFile[scriptFile.DocumentUri] = fileCorrections;
836869

837870
var uriBuilder = new UriBuilder()
838871
{
@@ -850,17 +883,42 @@ private void PublishScriptDiagnostics(
850883
});
851884
}
852885

886+
public async Task<IReadOnlyDictionary<string, MarkerCorrection>> GetMostRecentCodeActionsForFileAsync(string documentUri)
887+
{
888+
if (!_mostRecentCorrectionsByFile.TryGetValue(documentUri, out (SemaphoreSlim fileLock, Dictionary<string, MarkerCorrection> corrections) fileCorrectionsEntry))
889+
{
890+
return null;
891+
}
892+
893+
await fileCorrectionsEntry.fileLock.WaitAsync();
894+
// We must copy the dictionary for thread safety
895+
var corrections = new Dictionary<string, MarkerCorrection>(fileCorrectionsEntry.corrections.Count);
896+
try
897+
{
898+
foreach (KeyValuePair<string, MarkerCorrection> correction in fileCorrectionsEntry.corrections)
899+
{
900+
corrections.Add(correction.Key, correction.Value);
901+
}
902+
903+
return corrections;
904+
}
905+
finally
906+
{
907+
fileCorrectionsEntry.fileLock.Release();
908+
}
909+
}
910+
853911
// Generate a unique id that is used as a key to look up the associated code action (code fix) when
854912
// we receive and process the textDocument/codeAction message.
855-
private static string GetUniqueIdFromDiagnostic(Diagnostic diagnostic)
913+
internal static string GetUniqueIdFromDiagnostic(Diagnostic diagnostic)
856914
{
857915
Position start = diagnostic.Range.Start;
858916
Position end = diagnostic.Range.End;
859917

860918
var sb = new StringBuilder(256)
861919
.Append(diagnostic.Source ?? "?")
862920
.Append("_")
863-
.Append(diagnostic.Code.ToString())
921+
.Append(diagnostic.Code.IsString ? diagnostic.Code.String : diagnostic.Code.Long.ToString())
864922
.Append("_")
865923
.Append(diagnostic.Severity?.ToString() ?? "?")
866924
.Append("_")

‎src/PowerShellEditorServices.Engine/Services/TextDocument/Handlers/CodeActionHandler.cs‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
using System;
2+
using System.Collections.Concurrent;
3+
using System.Collections.Generic;
4+
using System.ComponentModel;
5+
using System.Threading;
6+
using System.Threading.Tasks;
7+
using Microsoft.Extensions.Logging;
8+
using Newtonsoft.Json.Linq;
9+
using OmniSharp.Extensions.JsonRpc.Client;
10+
using OmniSharp.Extensions.LanguageServer.Protocol.Client.Capabilities;
11+
using OmniSharp.Extensions.LanguageServer.Protocol.Models;
12+
using OmniSharp.Extensions.LanguageServer.Protocol.Server;
13+
using PowerShellEditorServices.Engine.Services.Handlers;
14+
15+
namespace Microsoft.PowerShell.EditorServices.TextDocument
16+
{
17+
internal class CodeActionHandler : ICodeActionHandler
18+
{
19+
private static readonly CodeActionKind[] s_supportedCodeActions = new[]
20+
{
21+
CodeActionKind.QuickFix
22+
};
23+
24+
private readonly CodeActionRegistrationOptions _registrationOptions;
25+
26+
private readonly ILogger _logger;
27+
28+
private readonly AnalysisService _analysisService;
29+
30+
private CodeActionCapability _capability;
31+
32+
public CodeActionHandler(ILoggerFactory factory, AnalysisService analysisService)
33+
{
34+
_logger = factory.CreateLogger<TextDocumentHandler>();
35+
_analysisService = analysisService;
36+
_registrationOptions = new CodeActionRegistrationOptions()
37+
{
38+
DocumentSelector = new DocumentSelector(new DocumentFilter() { Pattern = "**/*.ps*1" }),
39+
CodeActionKinds = s_supportedCodeActions
40+
};
41+
}
42+
43+
public CodeActionRegistrationOptions GetRegistrationOptions()
44+
{
45+
return _registrationOptions;
46+
}
47+
48+
public void SetCapability(CodeActionCapability capability)
49+
{
50+
_capability = capability;
51+
}
52+
53+
public async Task<CommandOrCodeActionContainer> Handle(CodeActionParams request, CancellationToken cancellationToken)
54+
{
55+
IReadOnlyDictionary<string, MarkerCorrection> corrections = await _analysisService.GetMostRecentCodeActionsForFileAsync(request.TextDocument.Uri.ToString());
56+
57+
if (corrections == null)
58+
{
59+
// TODO: Find out if we can cache this empty value
60+
return new CommandOrCodeActionContainer();
61+
}
62+
63+
var codeActions = new List<CommandOrCodeAction>();
64+
65+
// If there are any code fixes, send these commands first so they appear at top of "Code Fix" menu in the client UI.
66+
foreach (Diagnostic diagnostic in request.Context.Diagnostics)
67+
{
68+
if (diagnostic.Code.IsLong)
69+
{
70+
_logger.LogWarning(
71+
$"textDocument/codeAction skipping diagnostic with non-string code {diagnostic.Code.Long}: {diagnostic.Source} {diagnostic.Message}");
72+
}
73+
else if (string.IsNullOrEmpty(diagnostic.Code.String))
74+
{
75+
_logger.LogWarning(
76+
$"textDocument/codeAction skipping diagnostic with empty Code field: {diagnostic.Source} {diagnostic.Message}");
77+
78+
continue;
79+
}
80+
81+
82+
string diagnosticId = AnalysisService.GetUniqueIdFromDiagnostic(diagnostic);
83+
if (corrections.TryGetValue(diagnosticId, out MarkerCorrection correction))
84+
{
85+
codeActions.Add(new Command()
86+
{
87+
Title = correction.Name,
88+
Name = "PowerShell.ApplyCodeActionEdits",
89+
Arguments = JArray.FromObject(correction.Edits)
90+
});
91+
}
92+
}
93+
94+
// Add "show documentation" commands last so they appear at the bottom of the client UI.
95+
// These commands do not require code fixes. Sometimes we get a batch of diagnostics
96+
// to create commands for. No need to create multiple show doc commands for the same rule.
97+
var ruleNamesProcessed = new HashSet<string>();
98+
foreach (Diagnostic diagnostic in request.Context.Diagnostics)
99+
{
100+
if (!diagnostic.Code.IsString || string.IsNullOrEmpty(diagnostic.Code.String)) { continue; }
101+
102+
if (string.Equals(diagnostic.Source, "PSScriptAnalyzer", StringComparison.OrdinalIgnoreCase) &&
103+
!ruleNamesProcessed.Contains(diagnostic.Code.String))
104+
{
105+
ruleNamesProcessed.Add(diagnostic.Code.String);
106+
107+
codeActions.Add(
108+
new Command
109+
{
110+
Title = $"Show documentation for \"{diagnostic.Code}\"",
111+
Name = "PowerShell.ShowCodeActionDocumentation",
112+
Arguments = JArray.FromObject(new[] { diagnostic.Code })
113+
});
114+
}
115+
}
116+
117+
return codeActions;
118+
}
119+
}
120+
}

‎src/PowerShellEditorServices.Engine/Services/TextDocument/Handlers/FormattingHandlers.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
namespace PowerShellEditorServices.Engine.Services.Handlers
1212
{
13-
public class DocumentFormattingHandler : IDocumentFormattingHandler
13+
internal class DocumentFormattingHandler : IDocumentFormattingHandler
1414
{
1515
private readonly DocumentSelector _documentSelector = new DocumentSelector(
1616
new DocumentFilter()
@@ -88,7 +88,7 @@ public void SetCapability(DocumentFormattingCapability capability)
8888
}
8989
}
9090

91-
public class DocumentRangeFormattingHandler : IDocumentRangeFormattingHandler
91+
internal class DocumentRangeFormattingHandler : IDocumentRangeFormattingHandler
9292
{
9393
private readonly DocumentSelector _documentSelector = new DocumentSelector(
9494
new DocumentFilter()

‎src/PowerShellEditorServices.Engine/Services/TextDocument/ScriptFileMarker.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -147,11 +147,11 @@ internal static ScriptFileMarker FromDiagnosticRecord(PSObject psObject)
147147
new ScriptRegion(
148148
diagnosticRecord.ScriptPath,
149149
suggestedCorrection.Text,
150-
suggestedCorrection.StartLineNumber,
151-
suggestedCorrection.StartColumnNumber,
150+
startLineNumber: suggestedCorrection.StartLineNumber,
151+
startColumnNumber: suggestedCorrection.StartColumnNumber,
152+
endLineNumber: suggestedCorrection.EndLineNumber,
153+
endColumnNumber: suggestedCorrection.EndColumnNumber,
152154
startOffset: -1,
153-
suggestedCorrection.EndLineNumber,
154-
suggestedCorrection.EndColumnNumber,
155155
endOffset: -1));
156156

157157
correctionMessage = suggestedCorrection.Description;

‎src/PowerShellEditorServices.Engine/Services/Workspace/Handlers/ConfigurationHandler.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
namespace Microsoft.PowerShell.EditorServices
1212
{
13-
public class ConfigurationHandler : IDidChangeConfigurationHandler
13+
internal class ConfigurationHandler : IDidChangeConfigurationHandler
1414
{
1515
private readonly ILogger _logger;
1616
private readonly AnalysisService _analysisService;

‎src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1732,7 +1732,6 @@ await DelayThenInvokeDiagnosticsAsync(
17321732
cancellationToken);
17331733
}
17341734

1735-
17361735
private static async Task DelayThenInvokeDiagnosticsAsync(
17371736
int delayMilliseconds,
17381737
ScriptFile[] filesToAnalyze,

0 commit comments

Comments
 (0)