Skip to content

Provide quickfix functionality #304

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Dec 5, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions src/PowerShellEditorServices.Protocol/LanguageServer/CodeAction.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
using Microsoft.PowerShell.EditorServices.Protocol.MessageProtocol;
using Newtonsoft.Json.Linq;

namespace Microsoft.PowerShell.EditorServices.Protocol.LanguageServer
{
public class CodeActionRequest
{
public static readonly
RequestType<CodeActionRequest, CodeActionCommand[]> Type =
RequestType<CodeActionRequest, CodeActionCommand[]>.Create("textDocument/codeAction");

public TextDocumentIdentifier TextDocument { get; set; }

public Range Range { get; set; }

public CodeActionContext Context { get; set; }
}

public class CodeActionContext
{
public Diagnostic[] Diagnostics { get; set; }
}

public class CodeActionCommand
{
public string Title { get; set; }

public string Command { get; set; }

public JArray Arguments { get; set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ public class ServerCapabilities
public bool? DocumentSymbolProvider { get; set; }

public bool? WorkspaceSymbolProvider { get; set; }

public bool? CodeActionProvider { get; set; }
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
<Compile Include="DebugAdapter\SetFunctionBreakpointsRequest.cs" />
<Compile Include="DebugAdapter\SetVariableRequest.cs" />
<Compile Include="LanguageServer\EditorCommands.cs" />
<Compile Include="LanguageServer\CodeAction.cs" />
<Compile Include="LanguageServer\FindModuleRequest.cs" />
<Compile Include="LanguageServer\InstallModuleRequest.cs" />
<Compile Include="LanguageServer\PowerShellVersionRequest.cs" />
Expand Down Expand Up @@ -155,7 +156,7 @@
</PropertyGroup>
<Error Condition="!Exists('$(SolutionDir)\.nuget\NuGet.targets')" Text="$([System.String]::Format('$(ErrorText)', '$(SolutionDir)\.nuget\NuGet.targets'))" />
</Target>
<!-- To modify your build process, add your task inside one of the targets below and uncomment it.
<!-- To modify your build process, add your task inside one of the targets below and uncomment it.
Other similar extension points exist, see Microsoft.Common.targets.
<Target Name="BeforeBuild">
</Target>
Expand Down
83 changes: 69 additions & 14 deletions src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Microsoft.PowerShell.EditorServices.Protocol.MessageProtocol.Channel;
using Microsoft.PowerShell.EditorServices.Session;
using Microsoft.PowerShell.EditorServices.Utility;
using Newtonsoft.Json.Linq;
using System;
using System.Collections.Generic;
using System.IO;
Expand All @@ -30,6 +31,8 @@ public class LanguageServer : LanguageServerBase
private OutputDebouncer outputDebouncer;
private LanguageServerEditorOperations editorOperations;
private LanguageServerSettings currentSettings = new LanguageServerSettings();
private Dictionary<string, Dictionary<string, MarkerCorrection>> codeActionsPerFile =
new Dictionary<string, Dictionary<string, MarkerCorrection>>();

/// <param name="hostDetails">
/// Provides details about the host application.
Expand Down Expand Up @@ -92,6 +95,7 @@ protected override void Initialize()
this.SetRequestHandler(HoverRequest.Type, this.HandleHoverRequest);
this.SetRequestHandler(DocumentSymbolRequest.Type, this.HandleDocumentSymbolRequest);
this.SetRequestHandler(WorkspaceSymbolRequest.Type, this.HandleWorkspaceSymbolRequest);
this.SetRequestHandler(CodeActionRequest.Type, this.HandleCodeActionRequest);

this.SetRequestHandler(ShowOnlineHelpRequest.Type, this.HandleShowOnlineHelpRequest);
this.SetRequestHandler(ExpandAliasRequest.Type, this.HandleExpandAliasRequest);
Expand Down Expand Up @@ -146,6 +150,7 @@ await requestContext.SendResult(
DocumentSymbolProvider = true,
WorkspaceSymbolProvider = true,
HoverProvider = true,
CodeActionProvider = true,
CompletionProvider = new CompletionOptions
{
ResolveProvider = true,
Expand Down Expand Up @@ -226,17 +231,17 @@ function __Expand-Alias {
param($targetScript)

[ref]$errors=$null
$tokens = [System.Management.Automation.PsParser]::Tokenize($targetScript, $errors).Where({$_.type -eq 'command'}) |

$tokens = [System.Management.Automation.PsParser]::Tokenize($targetScript, $errors).Where({$_.type -eq 'command'}) |
Sort Start -Descending

foreach ($token in $tokens) {
$definition=(Get-Command ('`'+$token.Content) -CommandType Alias -ErrorAction SilentlyContinue).Definition

if($definition) {
if($definition) {
$lhs=$targetScript.Substring(0, $token.Start)
$rhs=$targetScript.Substring($token.Start + $token.Length)

$targetScript=$lhs + $definition + $rhs
}
}
Expand Down Expand Up @@ -346,13 +351,13 @@ protected async Task HandleDidChangeConfigurationNotification(
EventContext eventContext)
{
bool oldLoadProfiles = this.currentSettings.EnableProfileLoading;
bool oldScriptAnalysisEnabled =
bool oldScriptAnalysisEnabled =
this.currentSettings.ScriptAnalysis.Enable.HasValue;
string oldScriptAnalysisSettingsPath =
this.currentSettings.ScriptAnalysis.SettingsPath;

this.currentSettings.Update(
configChangeParams.Settings.Powershell,
configChangeParams.Settings.Powershell,
this.editorSession.Workspace.WorkspacePath);

if (!this.profilesLoaded &&
Expand Down Expand Up @@ -386,6 +391,7 @@ protected async Task HandleDidChangeConfigurationNotification(
await PublishScriptDiagnostics(
scriptFile,
emptyAnalysisDiagnostics,
this.codeActionsPerFile,
eventContext);
}
}
Expand Down Expand Up @@ -815,6 +821,35 @@ private bool IsQueryMatch(string query, string symbolName)
return symbolName.IndexOf(query, StringComparison.OrdinalIgnoreCase) >= 0;
}

protected async Task HandleCodeActionRequest(
CodeActionRequest codeActionParams,
RequestContext<CodeActionCommand[]> requestContext)
{
MarkerCorrection correction = null;
Dictionary<string, MarkerCorrection> markerIndex = null;
List<CodeActionCommand> codeActionCommands = new List<CodeActionCommand>();

if (this.codeActionsPerFile.TryGetValue(codeActionParams.TextDocument.Uri, out markerIndex))
{
foreach (var diagnostic in codeActionParams.Context.Diagnostics)
{
if (markerIndex.TryGetValue(diagnostic.Code, out correction))
{
codeActionCommands.Add(
new CodeActionCommand
{
Title = correction.Name,
Command = "PowerShell.ApplyCodeActionEdits",
Arguments = JArray.FromObject(correction.Edits)
});
}
}
}

await requestContext.SendResult(
codeActionCommands.ToArray());
}

protected Task HandleEvaluateRequest(
DebugAdapterMessages.EvaluateRequestArguments evaluateParams,
RequestContext<DebugAdapterMessages.EvaluateResponseBody> requestContext)
Expand Down Expand Up @@ -974,6 +1009,7 @@ private Task RunScriptDiagnostics(
DelayThenInvokeDiagnostics(
750,
filesToAnalyze,
this.codeActionsPerFile,
editorSession,
eventContext,
existingRequestCancellation.Token),
Expand All @@ -987,6 +1023,7 @@ private Task RunScriptDiagnostics(
private static async Task DelayThenInvokeDiagnostics(
int delayMilliseconds,
ScriptFile[] filesToAnalyze,
Dictionary<string, Dictionary<string, MarkerCorrection>> correctionIndex,
EditorSession editorSession,
EventContext eventContext,
CancellationToken cancellationToken)
Expand Down Expand Up @@ -1034,28 +1071,45 @@ private static async Task DelayThenInvokeDiagnostics(
await PublishScriptDiagnostics(
scriptFile,
semanticMarkers,
correctionIndex,
eventContext);
}
}

private static async Task PublishScriptDiagnostics(
ScriptFile scriptFile,
ScriptFileMarker[] semanticMarkers,
Dictionary<string, Dictionary<string, MarkerCorrection>> correctionIndex,
EventContext eventContext)
{
var allMarkers = scriptFile.SyntaxMarkers.Concat(semanticMarkers);
List<Diagnostic> diagnostics = new List<Diagnostic>();

// Hold on to any corrections that may need to be applied later
Dictionary<string, MarkerCorrection> fileCorrections =
new Dictionary<string, MarkerCorrection>();

foreach (var marker in scriptFile.SyntaxMarkers.Concat(semanticMarkers))
{
// Does the marker contain a correction?
Diagnostic markerDiagnostic = GetDiagnosticFromMarker(marker);
if (marker.Correction != null)
{
fileCorrections.Add(markerDiagnostic.Code, marker.Correction);
}

diagnostics.Add(markerDiagnostic);
}

correctionIndex[scriptFile.ClientFilePath] = fileCorrections;

// Always send syntax and semantic errors. We want to
// Always send syntax and semantic errors. We want to
// make sure no out-of-date markers are being displayed.
await eventContext.SendEvent(
PublishDiagnosticsNotification.Type,
new PublishDiagnosticsNotification
{
Uri = scriptFile.ClientFilePath,
Diagnostics =
allMarkers
.Select(GetDiagnosticFromMarker)
.ToArray()
Diagnostics = diagnostics.ToArray()
});
}

Expand All @@ -1065,6 +1119,7 @@ private static Diagnostic GetDiagnosticFromMarker(ScriptFileMarker scriptFileMar
{
Severity = MapDiagnosticSeverity(scriptFileMarker.Level),
Message = scriptFileMarker.Message,
Code = Guid.NewGuid().ToString(),
Range = new Range
{
// TODO: What offsets should I use?
Expand Down Expand Up @@ -1145,7 +1200,7 @@ private static CompletionItem CreateCompletionItem(
if (!completionDetails.ListItemText.Equals(
completionDetails.ToolTipText,
StringComparison.OrdinalIgnoreCase) &&
!Regex.IsMatch(completionDetails.ToolTipText,
!Regex.IsMatch(completionDetails.ToolTipText,
@"^\s*" + escapedToolTipText + @"\s+\["))
{
detailString = completionDetails.ToolTipText;
Expand All @@ -1158,7 +1213,7 @@ private static CompletionItem CreateCompletionItem(
// default (with common params at the end). We just need to make sure the default
// order also be the lexicographical order which we do by prefixig the ListItemText
// with a leading 0's four digit index. This would not sort correctly for a list
// > 999 parameters but surely we won't have so many items in the "parameter name"
// > 999 parameters but surely we won't have so many items in the "parameter name"
// completion list. Technically we don't need the ListItemText at all but it may come
// in handy during debug.
var sortText = (completionDetails.CompletionType == CompletionType.ParameterName)
Expand Down
18 changes: 11 additions & 7 deletions src/PowerShellEditorServices/Analysis/AnalysisService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
namespace Microsoft.PowerShell.EditorServices
{
/// <summary>
/// Provides a high-level service for performing semantic analysis
/// Provides a high-level service for performing semantic analysis
/// of PowerShell scripts.
/// </summary>
public class AnalysisService : IDisposable
Expand All @@ -31,15 +31,19 @@ public class AnalysisService : IDisposable
/// Defines the list of Script Analyzer rules to include by default if
/// no settings file is specified.
/// </summary>
private static readonly string[] IncludedRules = {
private static readonly string[] IncludedRules = new string[]
{
"PSUseToExportFieldsInManifest",
"PSMisleadingBacktick",
"PSAvoidUsingCmdletAliases",
"PSUseApprovedVerbs",
"PSAvoidUsingPlainTextForPassword",
"PSReservedCmdletChar",
"PSReservedParams",
"PSShouldProcess",
"PSMissingModuleManifestField",
"PSAvoidDefaultValueSwitchParameter",
"PSUseDeclaredVarsMoreThanAssigments",
"PSMisleadingBacktick",
"PSUseDeclaredVarsMoreThanAssigments"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should test the new rule additions. We got a lot of blow-back from the community early on because of lots of green squiggles in folks' scripts. Also, we should add a note in the comments for this section, that any updates here should be also made in the https://github.com/PowerShell/vscode-powershell/blob/master/examples/PSScriptAnalyzerSettings.psd1 file as well.

BTW it is more palatable for folks when the green squiggles underline just the bare minimum of the script as opposed to say entire functions. :-) BTW that also helps with not having "overlapping" script extents for multiple rule violations.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was testing PSUseDeclaredVarsMoreThanAssigments so I added it there and forgot to remove it. The rule is still a bit noisy and I would rather remove it. Thanks for catching it.

};

#endregion // Private Fields
Expand All @@ -64,7 +68,7 @@ public string SettingsPath
/// <summary>
/// Creates an instance of the AnalysisService class.
/// </summary>
/// <param name="consoleHost">An object that implements IConsoleHost in which to write errors/warnings
/// <param name="consoleHost">An object that implements IConsoleHost in which to write errors/warnings
/// from analyzer.</param>
/// <param name="settingsPath">Path to a PSScriptAnalyzer settings file.</param>
public AnalysisService(IConsoleHost consoleHost, string settingsPath = null)
Expand Down Expand Up @@ -107,8 +111,8 @@ public ScriptFileMarker[] GetSemanticMarkers(ScriptFile file)
Task.Factory.StartNew<ScriptFileMarker[]>(
() =>
{
return
GetDiagnosticRecords(file)
return
GetDiagnosticRecords(file)
.Select(ScriptFileMarker.FromDiagnosticRecord)
.ToArray();
},
Expand Down
Loading