From 2710077825e831de599580cbd42f5515242f5041 Mon Sep 17 00:00:00 2001 From: Rob Holt Date: Wed, 5 Feb 2020 11:39:33 -0800 Subject: [PATCH 01/33] Fix AnalysisService initialization --- .../Services/Analysis/AnalysisService.cs | 1051 ++++------------- .../Services/Analysis/IAnalysisEngine.cs | 36 + .../Services/Analysis/NullAnalysisEngine.cs | 48 + .../Analysis/PssaCmdletAnalysisEngine.cs | 452 +++++++ .../Services/TextDocument/ScriptFileMarker.cs | 16 +- 5 files changed, 785 insertions(+), 818 deletions(-) create mode 100644 src/PowerShellEditorServices/Services/Analysis/IAnalysisEngine.cs create mode 100644 src/PowerShellEditorServices/Services/Analysis/NullAnalysisEngine.cs create mode 100644 src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs diff --git a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs index 59a593431..1ac55be01 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -18,22 +18,18 @@ using System.Collections.Concurrent; using Microsoft.PowerShell.EditorServices.Services.TextDocument; using Microsoft.PowerShell.EditorServices.Utility; +using System.Collections.ObjectModel; +using Microsoft.PowerShell.EditorServices.Services.Analysis; namespace Microsoft.PowerShell.EditorServices.Services { - /// - /// Provides a high-level service for performing semantic analysis - /// of PowerShell scripts. - /// - public class AnalysisService : IDisposable + internal class AnalysisService : IDisposable { - #region Static fields - /// /// Defines the list of Script Analyzer rules to include by default if /// no settings file is specified. /// - private static readonly string[] s_includedRules = { + private static readonly string[] s_defaultRules = { "PSAvoidAssignmentToAutomaticVariable", "PSUseToExportFieldsInManifest", "PSMisleadingBacktick", @@ -51,76 +47,265 @@ public class AnalysisService : IDisposable "PSPossibleIncorrectUsageOfRedirectionOperator" }; - /// - /// An empty diagnostic result to return when a script fails analysis. - /// - private static readonly PSObject[] s_emptyDiagnosticResult = Array.Empty(); - private static readonly string[] s_emptyGetRuleResult = Array.Empty(); + public static AnalysisService Create(ILogger logger, ILanguageServer languageServer) + { + IAnalysisEngine analysisEngine = PssaCmdletAnalysisEngine.Create(logger); - private static CancellationTokenSource s_existingRequestCancellation; + // ?? doesn't work above sadly + if (analysisEngine == null) + { + analysisEngine = new NullAnalysisEngine(); + } - /// - /// The indentation to add when the logger lists errors. - /// - private static readonly string s_indentJoin = Environment.NewLine + " "; + var analysisService = new AnalysisService(logger, languageServer, analysisEngine) + { + EnabledRules = s_defaultRules, + }; - #endregion // Static fields + return analysisService; + } - #region Private Fields + private readonly ILogger _logger; - /// - /// Maximum number of runspaces we allow to be in use for script analysis. - /// - private const int NumRunspaces = 1; + private readonly ILanguageServer _languageServer; - /// - /// Name of the PSScriptAnalyzer module, to be used for PowerShell module interactions. - /// - private const string PSSA_MODULE_NAME = "PSScriptAnalyzer"; + private readonly IAnalysisEngine _analysisEngine; - /// - /// Provides logging. - /// - private ILogger _logger; + private readonly int _analysisDelayMillis; - /// - /// Runspace pool to generate runspaces for script analysis and handle - /// ansynchronous analysis requests. - /// - private RunspacePool _analysisRunspacePool; + private readonly ConcurrentDictionary)> _mostRecentCorrectionsByFile; - /// - /// Info object describing the PSScriptAnalyzer module that has been loaded in - /// to provide analysis services. - /// - private PSModuleInfo _pssaModuleInfo; + private CancellationTokenSource _diagnosticsCancellationTokenSource; - private readonly ILanguageServer _languageServer; + public AnalysisService(ILogger logger, ILanguageServer languageServer, IAnalysisEngine analysisEngine) + { + _logger = logger; + _languageServer = languageServer; + _analysisEngine = analysisEngine; + _analysisDelayMillis = 750; + _mostRecentCorrectionsByFile = new ConcurrentDictionary)>(); + } - private readonly ConfigurationService _configurationService; + public string[] EnabledRules { get; set; } - private readonly ConcurrentDictionary)> _mostRecentCorrectionsByFile; + public string SettingsPath { get; set; } - #endregion // Private Fields + public Task RunScriptDiagnosticsAsync( + ScriptFile[] filesToAnalyze, + CancellationToken cancellationToken) + { + // Create a cancellation token source that will cancel if we do or if the caller does + var cancellationSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - #region Properties + CancellationTokenSource oldTaskCancellation; + // If there's an existing task, we want to cancel it here + if ((oldTaskCancellation = Interlocked.Exchange(ref _diagnosticsCancellationTokenSource, cancellationSource)) != null) + { + try + { + oldTaskCancellation.Cancel(); + oldTaskCancellation.Dispose(); + } + catch (Exception e) + { + _logger.LogError(e, "Exception occurred while cancelling analysis task"); + } + } - /// - /// Set of PSScriptAnalyzer rules used for analysis. - /// - public string[] ActiveRules { get; set; } + if (filesToAnalyze.Length == 0) + { + return Task.CompletedTask; + } - /// - /// Gets or sets the path to a settings file (.psd1) - /// containing PSScriptAnalyzer settings. - /// - public string SettingsPath { get; set; } + return Task.Run(() => DelayThenInvokeDiagnosticsAsync(filesToAnalyze, _diagnosticsCancellationTokenSource.Token), _diagnosticsCancellationTokenSource.Token); + } + + private async Task DelayThenInvokeDiagnosticsAsync(ScriptFile[] filesToAnalyze, CancellationToken cancellationToken) + { + if (cancellationToken.IsCancellationRequested) + { + return; + } + + try + { + await Task.Delay(_analysisDelayMillis, cancellationToken).ConfigureAwait(false); + } + catch (TaskCanceledException) + { + // Ensure no stale markers are displayed + foreach (ScriptFile script in filesToAnalyze) + { + await PublishScriptDiagnosticsAsync(script).ConfigureAwait(false); + } + + return; + } + + // If we've made it past the delay period then we don't care + // about the cancellation token anymore. This could happen + // when the user stops typing for long enough that the delay + // period ends but then starts typing while analysis is going + // on. It makes sense to send back the results from the first + // delay period while the second one is ticking away. + + foreach (ScriptFile scriptFile in filesToAnalyze) + { + ScriptFileMarker[] semanticMarkers = SettingsPath != null + ? await _analysisEngine.GetSemanticMarkersAsync(scriptFile.Contents, SettingsPath).ConfigureAwait(false) + : await _analysisEngine.GetSemanticMarkersAsync(scriptFile.Contents, EnabledRules).ConfigureAwait(false); + + scriptFile.DiagnosticMarkers.AddRange(semanticMarkers); + + await PublishScriptDiagnosticsAsync(scriptFile).ConfigureAwait(false); + } + } + + private async Task PublishScriptDiagnosticsAsync(ScriptFile scriptFile) + { + (SemaphoreSlim fileLock, Dictionary fileCorrections) = _mostRecentCorrectionsByFile.GetOrAdd( + scriptFile.DocumentUri, + CreateFileCorrectionsEntry); + + var diagnostics = new Diagnostic[scriptFile.DiagnosticMarkers.Count]; + + await fileLock.WaitAsync(); + try + { + fileCorrections.Clear(); + + for (int i = 0; i < scriptFile.DiagnosticMarkers.Count; i++) + { + ScriptFileMarker marker = scriptFile.DiagnosticMarkers[i]; + + Diagnostic diagnostic = GetDiagnosticFromMarker(marker); + + if (marker.Correction != null) + { + string diagnosticId = GetUniqueIdFromDiagnostic(diagnostic); + fileCorrections[diagnosticId] = marker.Correction; + } + + diagnostics[i] = diagnostic; + } + } + finally + { + fileLock.Release(); + } + + _languageServer.Document.PublishDiagnostics(new PublishDiagnosticsParams + { + Uri = new Uri(scriptFile.DocumentUri), + Diagnostics = new Container(diagnostics) + }); + } + + private static (SemaphoreSlim, Dictionary) CreateFileCorrectionsEntry(string fileUri) + { + return (AsyncUtils.CreateSimpleLockingSemaphore(), new Dictionary()); + } + + private static Diagnostic GetDiagnosticFromMarker(ScriptFileMarker scriptFileMarker) + { + return new Diagnostic + { + Severity = MapDiagnosticSeverity(scriptFileMarker.Level), + Message = scriptFileMarker.Message, + Code = scriptFileMarker.RuleName, + Source = scriptFileMarker.Source, + Range = new Range + { + Start = new Position + { + Line = scriptFileMarker.ScriptRegion.StartLineNumber - 1, + Character = scriptFileMarker.ScriptRegion.StartColumnNumber - 1 + }, + End = new Position + { + Line = scriptFileMarker.ScriptRegion.EndLineNumber - 1, + Character = scriptFileMarker.ScriptRegion.EndColumnNumber - 1 + } + } + }; + } + + private static DiagnosticSeverity MapDiagnosticSeverity(ScriptFileMarkerLevel markerLevel) + { + switch (markerLevel) + { + case ScriptFileMarkerLevel.Error: + return DiagnosticSeverity.Error; + + case ScriptFileMarkerLevel.Warning: + return DiagnosticSeverity.Warning; + + case ScriptFileMarkerLevel.Information: + return DiagnosticSeverity.Information; + + default: + return DiagnosticSeverity.Error; + } + } + + internal static string GetUniqueIdFromDiagnostic(Diagnostic diagnostic) + { + Position start = diagnostic.Range.Start; + Position end = diagnostic.Range.End; + + var sb = new StringBuilder(256) + .Append(diagnostic.Source ?? "?") + .Append("_") + .Append(diagnostic.Code.IsString ? diagnostic.Code.String : diagnostic.Code.Long.ToString()) + .Append("_") + .Append(diagnostic.Severity?.ToString() ?? "?") + .Append("_") + .Append(start.Line) + .Append(":") + .Append(start.Character) + .Append("-") + .Append(end.Line) + .Append(":") + .Append(end.Character); + + var id = sb.ToString(); + return id; + } + + #region IDisposable Support + private bool disposedValue = false; // To detect redundant calls + + protected virtual void Dispose(bool disposing) + { + if (!disposedValue) + { + if (disposing) + { + _analysisEngine.Dispose(); + _diagnosticsCancellationTokenSource?.Dispose(); + } + + disposedValue = true; + } + } + // This code added to correctly implement the disposable pattern. + public void Dispose() + { + // Do not change this code. Put cleanup code in Dispose(bool disposing) above. + Dispose(true); + } #endregion - #region Constructors + } + /// + /// Provides a high-level service for performing semantic analysis + /// of PowerShell scripts. + /// + public class OldOldAnalysisService + { /// /// Construct a new AnalysisService object. /// @@ -227,757 +412,5 @@ public static AnalysisService Create(ConfigurationService configurationService, return null; } } - - /// - /// Get PSScriptAnalyzer settings hashtable for PSProvideCommentHelp rule. - /// - /// Enable the rule. - /// Analyze only exported functions/cmdlets. - /// Use block comment or line comment. - /// Return a vscode snipped correction should be returned. - /// Place comment help at the given location relative to the function definition. - /// A PSScriptAnalyzer settings hashtable. - public static Hashtable GetCommentHelpRuleSettings( - bool enable, - bool exportedOnly, - bool blockComment, - bool vscodeSnippetCorrection, - string placement) - { - var settings = new Dictionary(); - var ruleSettings = new Hashtable(); - ruleSettings.Add("Enable", enable); - ruleSettings.Add("ExportedOnly", exportedOnly); - ruleSettings.Add("BlockComment", blockComment); - ruleSettings.Add("VSCodeSnippetCorrection", vscodeSnippetCorrection); - ruleSettings.Add("Placement", placement); - settings.Add("PSProvideCommentHelp", ruleSettings); - return GetPSSASettingsHashtable(settings); - } - - /// - /// Construct a PSScriptAnalyzer settings hashtable - /// - /// A settings hashtable - /// - public static Hashtable GetPSSASettingsHashtable(IDictionary ruleSettingsMap) - { - Validate.IsNotNull(nameof(ruleSettingsMap), ruleSettingsMap); - - var hashtable = new Hashtable(); - var ruleSettingsHashtable = new Hashtable(); - - hashtable["IncludeRules"] = ruleSettingsMap.Keys.ToArray(); - hashtable["Rules"] = ruleSettingsHashtable; - - foreach (var kvp in ruleSettingsMap) - { - ruleSettingsHashtable.Add(kvp.Key, kvp.Value); - } - - return hashtable; - } - - /// - /// Perform semantic analysis on the given ScriptFile and returns - /// an array of ScriptFileMarkers. - /// - /// The ScriptFile which will be analyzed for semantic markers. - /// An array of ScriptFileMarkers containing semantic analysis results. - public Task> GetSemanticMarkersAsync(ScriptFile file) - { - return GetSemanticMarkersAsync(file, ActiveRules, SettingsPath); - } - - /// - /// Perform semantic analysis on the given ScriptFile with the given settings. - /// - /// The ScriptFile to be analyzed. - /// ScriptAnalyzer settings - /// - public Task> GetSemanticMarkersAsync(ScriptFile file, Hashtable settings) - { - return GetSemanticMarkersAsync(file, null, settings); - } - - /// - /// Perform semantic analysis on the given script with the given settings. - /// - /// The script content to be analyzed. - /// ScriptAnalyzer settings - /// - public Task> GetSemanticMarkersAsync( - string scriptContent, - Hashtable settings) - { - return GetSemanticMarkersAsync(scriptContent, null, settings); - } - - /// - /// Returns a list of builtin-in PSScriptAnalyzer rules - /// - public IEnumerable GetPSScriptAnalyzerRules() - { - PowerShellResult getRuleResult = InvokePowerShell("Get-ScriptAnalyzerRule"); - if (getRuleResult == null) - { - _logger.LogWarning("Get-ScriptAnalyzerRule returned null result"); - return s_emptyGetRuleResult; - } - - var ruleNames = new List(); - foreach (var rule in getRuleResult.Output) - { - ruleNames.Add((string)rule.Members["RuleName"].Value); - } - - return ruleNames; - } - - /// - /// Format a given script text with default codeformatting settings. - /// - /// Script text to be formatted - /// ScriptAnalyzer settings - /// The range within which formatting should be applied. - /// The formatted script text. - public async Task FormatAsync( - string scriptDefinition, - Hashtable settings, - int[] rangeList) - { - // We cannot use Range type therefore this workaround of using -1 default value. - // Invoke-Formatter throws a ParameterBinderValidationException if the ScriptDefinition is an empty string. - if (string.IsNullOrEmpty(scriptDefinition)) - { - return null; - } - - var argsDict = new Dictionary { - {"ScriptDefinition", scriptDefinition}, - {"Settings", settings} - }; - if (rangeList != null) - { - argsDict.Add("Range", rangeList); - } - - PowerShellResult result = await InvokePowerShellAsync("Invoke-Formatter", argsDict).ConfigureAwait(false); - - if (result == null) - { - _logger.LogError("Formatter returned null result"); - return null; - } - - if (result.HasErrors) - { - var errorBuilder = new StringBuilder().Append(s_indentJoin); - foreach (ErrorRecord err in result.Errors) - { - errorBuilder.Append(err).Append(s_indentJoin); - } - _logger.LogWarning($"Errors found while formatting file: {errorBuilder}"); - return null; - } - - foreach (PSObject resultObj in result.Output) - { - string formatResult = resultObj?.BaseObject as string; - if (formatResult != null) - { - return formatResult; - } - } - - return null; - } - - #endregion // public methods - - #region Private Methods - - private async Task> GetSemanticMarkersAsync( - ScriptFile file, - string[] rules, - TSettings settings) where TSettings : class - { - if (file.IsAnalysisEnabled) - { - return await GetSemanticMarkersAsync( - file.Contents, - rules, - settings).ConfigureAwait(false); - } - else - { - // Return an empty marker list - return new List(); - } - } - - private async Task> GetSemanticMarkersAsync( - string scriptContent, - string[] rules, - TSettings settings) where TSettings : class - { - if ((typeof(TSettings) == typeof(string) || typeof(TSettings) == typeof(Hashtable)) - && (rules != null || settings != null)) - { - var scriptFileMarkers = await GetDiagnosticRecordsAsync(scriptContent, rules, settings).ConfigureAwait(false); - return scriptFileMarkers.Select(ScriptFileMarker.FromDiagnosticRecord).ToList(); - } - else - { - // Return an empty marker list - return new List(); - } - } - - /// - /// Log the features available from the PSScriptAnalyzer module that has been imported - /// for use with the AnalysisService. - /// - private void LogAvailablePssaFeatures() - { - // Save ourselves some work here - if (!_logger.IsEnabled(LogLevel.Debug)) - { - return; - } - - // If we already know the module that was imported, save some work - if (_pssaModuleInfo == null) - { - PowerShellResult getModuleResult = InvokePowerShell( - "Get-Module", - new Dictionary{ {"Name", PSSA_MODULE_NAME} }); - - if (getModuleResult == null) - { - throw new AnalysisServiceLoadException("Get-Module call to find PSScriptAnalyzer module failed"); - } - - _pssaModuleInfo = getModuleResult.Output - .Select(m => m.BaseObject) - .OfType() - .FirstOrDefault(); - } - - if (_pssaModuleInfo == null) - { - throw new AnalysisServiceLoadException("Unable to find loaded PSScriptAnalyzer module for logging"); - } - - var sb = new StringBuilder(); - sb.AppendLine("PSScriptAnalyzer successfully imported:"); - - // Log version - sb.Append(" Version: "); - sb.AppendLine(_pssaModuleInfo.Version.ToString()); - - // Log exported cmdlets - sb.AppendLine(" Exported Cmdlets:"); - foreach (string cmdletName in _pssaModuleInfo.ExportedCmdlets.Keys.OrderBy(name => name)) - { - sb.Append(" "); - sb.AppendLine(cmdletName); - } - - // Log available rules - sb.AppendLine(" Available Rules:"); - foreach (string ruleName in GetPSScriptAnalyzerRules()) - { - sb.Append(" "); - sb.AppendLine(ruleName); - } - - _logger.LogDebug(sb.ToString()); - } - - private async Task GetDiagnosticRecordsAsync( - string scriptContent, - string[] rules, - TSettings settings) where TSettings : class - { - // When a new, empty file is created there are by definition no issues. - // Furthermore, if you call Invoke-ScriptAnalyzer with an empty ScriptDefinition - // it will generate a ParameterBindingValidationException. - if (string.IsNullOrEmpty(scriptContent) - || !(typeof(TSettings) == typeof(string) || typeof(TSettings) == typeof(Hashtable))) - { - return s_emptyDiagnosticResult; - } - - //Use a settings file if one is provided, otherwise use the default rule list. - string settingParameter; - object settingArgument; - if (settings != null) - { - settingParameter = "Settings"; - settingArgument = settings; - } - else - { - settingParameter = "IncludeRule"; - settingArgument = rules; - } - - PowerShellResult result = await InvokePowerShellAsync( - "Invoke-ScriptAnalyzer", - new Dictionary - { - { "ScriptDefinition", scriptContent }, - { settingParameter, settingArgument }, - // We ignore ParseErrors from PSSA because we already send them when we parse the file. - { "Severity", new [] { ScriptFileMarkerLevel.Error, ScriptFileMarkerLevel.Information, ScriptFileMarkerLevel.Warning }} - }).ConfigureAwait(false); - - var diagnosticResults = result?.Output ?? s_emptyDiagnosticResult; - _logger.LogDebug(String.Format("Found {0} violations", diagnosticResults.Length)); - return diagnosticResults; - } - - private PowerShellResult InvokePowerShell(string command, IDictionary paramArgMap = null) - { - using (var powerShell = System.Management.Automation.PowerShell.Create()) - { - powerShell.RunspacePool = _analysisRunspacePool; - powerShell.AddCommand(command); - if (paramArgMap != null) - { - foreach (KeyValuePair kvp in paramArgMap) - { - powerShell.AddParameter(kvp.Key, kvp.Value); - } - } - - PowerShellResult result = null; - try - { - PSObject[] output = powerShell.Invoke().ToArray(); - ErrorRecord[] errors = powerShell.Streams.Error.ToArray(); - result = new PowerShellResult(output, errors, powerShell.HadErrors); - } - catch (CommandNotFoundException ex) - { - // This exception is possible if the module path loaded - // is wrong even though PSScriptAnalyzer is available as a module - _logger.LogError(ex.Message); - } - catch (CmdletInvocationException ex) - { - // We do not want to crash EditorServices for exceptions caused by cmdlet invocation. - // Two main reasons that cause the exception are: - // * PSCmdlet.WriteOutput being called from another thread than Begin/Process - // * CompositionContainer.ComposeParts complaining that "...Only one batch can be composed at a time" - _logger.LogError(ex.Message); - } - - return result; - } - } - - private Task InvokePowerShellAsync(string command, IDictionary paramArgMap = null) - { - return Task.Run(() => InvokePowerShell(command, paramArgMap)); - } - - /// - /// Create a new runspace pool around a PSScriptAnalyzer module for asynchronous script analysis tasks. - /// This looks for the latest version of PSScriptAnalyzer on the path and loads that. - /// - /// A runspace pool with PSScriptAnalyzer loaded for running script analysis tasks. - private static RunspacePool CreatePssaRunspacePool(out PSModuleInfo pssaModuleInfo) - { - using (var ps = System.Management.Automation.PowerShell.Create()) - { - // Run `Get-Module -ListAvailable -Name "PSScriptAnalyzer"` - ps.AddCommand("Get-Module") - .AddParameter("ListAvailable") - .AddParameter("Name", PSSA_MODULE_NAME); - - try - { - // Get the latest version of PSScriptAnalyzer we can find - pssaModuleInfo = ps.Invoke()? - .Select(psObj => psObj.BaseObject) - .OfType() - .OrderByDescending(moduleInfo => moduleInfo.Version) - .FirstOrDefault(); - } - catch (Exception e) - { - throw new AnalysisServiceLoadException("Unable to find PSScriptAnalyzer module on the module path", e); - } - - if (pssaModuleInfo == null) - { - throw new AnalysisServiceLoadException("Unable to find PSScriptAnalyzer module on the module path"); - } - - // Create a base session state with PSScriptAnalyzer loaded - InitialSessionState sessionState; - if (Environment.GetEnvironmentVariable("PSES_TEST_USE_CREATE_DEFAULT") == "1") { - sessionState = InitialSessionState.CreateDefault(); - } else { - sessionState = InitialSessionState.CreateDefault2(); - } - sessionState.ImportPSModule(new [] { pssaModuleInfo.ModuleBase }); - - // RunspacePool takes care of queuing commands for us so we do not - // need to worry about executing concurrent commands - return RunspaceFactory.CreateRunspacePool(sessionState); - } - } - - #endregion //private methods - - #region IDisposable Support - - private bool _disposedValue = false; // To detect redundant calls - - /// - /// Dispose of this object. - /// - /// True if the method is called by the Dispose method, false if called by the finalizer. - protected virtual void Dispose(bool disposing) - { - if (!_disposedValue) - { - if (disposing) - { - _analysisRunspacePool.Dispose(); - _analysisRunspacePool = null; - } - - _disposedValue = true; - } - } - - /// - /// Clean up all internal resources and dispose of the analysis service. - /// - public void Dispose() - { - // Do not change this code. Put cleanup code in Dispose(bool disposing) above. - Dispose(true); - } - - #endregion - - /// - /// Wraps the result of an execution of PowerShell to send back through - /// asynchronous calls. - /// - private class PowerShellResult - { - public PowerShellResult( - PSObject[] output, - ErrorRecord[] errors, - bool hasErrors) - { - Output = output; - Errors = errors; - HasErrors = hasErrors; - } - - public PSObject[] Output { get; } - - public ErrorRecord[] Errors { get; } - - public bool HasErrors { get; } - } - - internal Task RunScriptDiagnosticsAsync( - ScriptFile[] filesToAnalyze) - { - // If there's an existing task, attempt to cancel it - try - { - if (s_existingRequestCancellation != null) - { - // Try to cancel the request - s_existingRequestCancellation.Cancel(); - - // If cancellation didn't throw an exception, - // clean up the existing token - s_existingRequestCancellation.Dispose(); - s_existingRequestCancellation = null; - } - } - catch (Exception e) - { - // TODO: Catch a more specific exception! - _logger.LogError( - string.Format( - "Exception while canceling analysis task:\n\n{0}", - e.ToString())); - - TaskCompletionSource cancelTask = new TaskCompletionSource(); - cancelTask.SetCanceled(); - return Task.CompletedTask; - } - - // If filesToAnalzye is empty, nothing to do so return early. - if (filesToAnalyze.Length == 0) - { - return Task.CompletedTask; - } - - // Create a fresh cancellation token and then start the task. - // We create this on a different TaskScheduler so that we - // don't block the main message loop thread. - // TODO: Is there a better way to do this? - s_existingRequestCancellation = new CancellationTokenSource(); - bool scriptAnalysisEnabled = _configurationService.CurrentSettings.ScriptAnalysis.Enable ?? false; - return Task.Run(() => DelayThenInvokeDiagnosticsAsync(delayMilliseconds: 750, filesToAnalyze, scriptAnalysisEnabled, s_existingRequestCancellation.Token)); - } - - private async Task DelayThenInvokeDiagnosticsAsync( - int delayMilliseconds, - ScriptFile[] filesToAnalyze, - bool isScriptAnalysisEnabled, - CancellationToken cancellationToken) - { - // First of all, wait for the desired delay period before - // analyzing the provided list of files - try - { - await Task.Delay(delayMilliseconds, cancellationToken).ConfigureAwait(false); - } - catch (TaskCanceledException) - { - // If the task is cancelled, exit directly - foreach (var script in filesToAnalyze) - { - PublishScriptDiagnostics( - script, - script.DiagnosticMarkers); - } - - return; - } - - // If we've made it past the delay period then we don't care - // about the cancellation token anymore. This could happen - // when the user stops typing for long enough that the delay - // period ends but then starts typing while analysis is going - // on. It makes sense to send back the results from the first - // delay period while the second one is ticking away. - - // Get the requested files - foreach (ScriptFile scriptFile in filesToAnalyze) - { - List semanticMarkers = null; - if (isScriptAnalysisEnabled) - { - semanticMarkers = await GetSemanticMarkersAsync(scriptFile).ConfigureAwait(false); - } - else - { - // Semantic markers aren't available if the AnalysisService - // isn't available - semanticMarkers = new List(); - } - - scriptFile.DiagnosticMarkers.AddRange(semanticMarkers); - - PublishScriptDiagnostics( - scriptFile, - // Concat script analysis errors to any existing parse errors - scriptFile.DiagnosticMarkers); - } - } - - internal void ClearMarkers(ScriptFile scriptFile) - { - // send empty diagnostic markers to clear any markers associated with the given file - PublishScriptDiagnostics( - scriptFile, - new List()); - } - - private void PublishScriptDiagnostics( - ScriptFile scriptFile, - List markers) - { - var diagnostics = new List(); - - // Create the entry for this file if it does not already exist - SemaphoreSlim fileLock; - Dictionary fileCorrections; - bool newEntryNeeded = false; - if (_mostRecentCorrectionsByFile.TryGetValue(scriptFile.DocumentUri, out (SemaphoreSlim, Dictionary) fileCorrectionsEntry)) - { - fileLock = fileCorrectionsEntry.Item1; - fileCorrections = fileCorrectionsEntry.Item2; - } - else - { - fileLock = new SemaphoreSlim(initialCount: 1, maxCount: 1); - fileCorrections = new Dictionary(); - newEntryNeeded = true; - } - - fileLock.Wait(); - try - { - if (newEntryNeeded) - { - // If we create a new entry, we should do it after acquiring the lock we just created - // to ensure a competing thread can never acquire it first and read invalid information from it - _mostRecentCorrectionsByFile[scriptFile.DocumentUri] = (fileLock, fileCorrections); - } - else - { - // Otherwise we need to clear the stale corrections - fileCorrections.Clear(); - } - - foreach (ScriptFileMarker marker in markers) - { - // Does the marker contain a correction? - Diagnostic markerDiagnostic = GetDiagnosticFromMarker(marker); - if (marker.Correction != null) - { - string diagnosticId = GetUniqueIdFromDiagnostic(markerDiagnostic); - fileCorrections[diagnosticId] = marker.Correction; - } - - diagnostics.Add(markerDiagnostic); - } - } - finally - { - fileLock.Release(); - } - - // Always send syntax and semantic errors. We want to - // make sure no out-of-date markers are being displayed. - _languageServer.Document.PublishDiagnostics(new PublishDiagnosticsParams() - { - Uri = new Uri(scriptFile.DocumentUri), - Diagnostics = new Container(diagnostics), - }); - } - - public async Task> GetMostRecentCodeActionsForFileAsync(string documentUri) - { - if (!_mostRecentCorrectionsByFile.TryGetValue(documentUri, out (SemaphoreSlim fileLock, Dictionary corrections) fileCorrectionsEntry)) - { - return null; - } - - await fileCorrectionsEntry.fileLock.WaitAsync().ConfigureAwait(false); - // We must copy the dictionary for thread safety - var corrections = new Dictionary(fileCorrectionsEntry.corrections.Count); - try - { - foreach (KeyValuePair correction in fileCorrectionsEntry.corrections) - { - corrections.Add(correction.Key, correction.Value); - } - - return corrections; - } - finally - { - fileCorrectionsEntry.fileLock.Release(); - } - } - - // Generate a unique id that is used as a key to look up the associated code action (code fix) when - // we receive and process the textDocument/codeAction message. - internal static string GetUniqueIdFromDiagnostic(Diagnostic diagnostic) - { - Position start = diagnostic.Range.Start; - Position end = diagnostic.Range.End; - - var sb = new StringBuilder(256) - .Append(diagnostic.Source ?? "?") - .Append("_") - .Append(diagnostic.Code.IsString ? diagnostic.Code.String : diagnostic.Code.Long.ToString()) - .Append("_") - .Append(diagnostic.Severity?.ToString() ?? "?") - .Append("_") - .Append(start.Line) - .Append(":") - .Append(start.Character) - .Append("-") - .Append(end.Line) - .Append(":") - .Append(end.Character); - - var id = sb.ToString(); - return id; - } - - private static Diagnostic GetDiagnosticFromMarker(ScriptFileMarker scriptFileMarker) - { - return new Diagnostic - { - Severity = MapDiagnosticSeverity(scriptFileMarker.Level), - Message = scriptFileMarker.Message, - Code = scriptFileMarker.RuleName, - Source = scriptFileMarker.Source, - Range = new Range - { - Start = new Position - { - Line = scriptFileMarker.ScriptRegion.StartLineNumber - 1, - Character = scriptFileMarker.ScriptRegion.StartColumnNumber - 1 - }, - End = new Position - { - Line = scriptFileMarker.ScriptRegion.EndLineNumber - 1, - Character = scriptFileMarker.ScriptRegion.EndColumnNumber - 1 - } - } - }; - } - - private static DiagnosticSeverity MapDiagnosticSeverity(ScriptFileMarkerLevel markerLevel) - { - switch (markerLevel) - { - case ScriptFileMarkerLevel.Error: - return DiagnosticSeverity.Error; - - case ScriptFileMarkerLevel.Warning: - return DiagnosticSeverity.Warning; - - case ScriptFileMarkerLevel.Information: - return DiagnosticSeverity.Information; - - default: - return DiagnosticSeverity.Error; - } - } - } - - /// - /// Class to catch known failure modes for starting the AnalysisService. - /// - public class AnalysisServiceLoadException : Exception - { - /// - /// Instantiate an AnalysisService error based on a simple message. - /// - /// The message to display to the user detailing the error. - public AnalysisServiceLoadException(string message) - : base(message) - { - } - - /// - /// Instantiate an AnalysisService error based on another error that occurred internally. - /// - /// The message to display to the user detailing the error. - /// The inner exception that occurred to trigger this error. - public AnalysisServiceLoadException(string message, Exception innerException) - : base(message, innerException) - { - } } } diff --git a/src/PowerShellEditorServices/Services/Analysis/IAnalysisEngine.cs b/src/PowerShellEditorServices/Services/Analysis/IAnalysisEngine.cs new file mode 100644 index 000000000..8ea89f2f9 --- /dev/null +++ b/src/PowerShellEditorServices/Services/Analysis/IAnalysisEngine.cs @@ -0,0 +1,36 @@ +// +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. +// + +using Microsoft.PowerShell.EditorServices.Services.TextDocument; +using System; +using System.Collections; +using System.Threading.Tasks; + +namespace Microsoft.PowerShell.EditorServices.Services.Analysis +{ + internal interface IAnalysisEngine : IDisposable + { + bool IsEnabled { get; } + + Task FormatAsync( + string scriptDefinition, + Hashtable settings, + int[] rangeList); + + Task GetSemanticMarkersAsync( + string scriptFileContent, + Hashtable settings); + + Task GetSemanticMarkersAsync( + string scriptFileContent, + string settingsFilePath); + + Task GetSemanticMarkersAsync( + string scriptFileContent, + string[] rules); + + } + +} diff --git a/src/PowerShellEditorServices/Services/Analysis/NullAnalysisEngine.cs b/src/PowerShellEditorServices/Services/Analysis/NullAnalysisEngine.cs new file mode 100644 index 000000000..875d28464 --- /dev/null +++ b/src/PowerShellEditorServices/Services/Analysis/NullAnalysisEngine.cs @@ -0,0 +1,48 @@ +// +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. +// + +using Microsoft.PowerShell.EditorServices.Services.TextDocument; +using System; +using System.Collections; +using System.Threading.Tasks; + +namespace Microsoft.PowerShell.EditorServices.Services.Analysis +{ + internal class NullAnalysisEngine : IAnalysisEngine + { + public bool IsEnabled => false; + + public Task FormatAsync(string scriptDefinition, Hashtable settings, int[] rangeList) + { + throw CreateInvocationException(); + } + + public Task GetSemanticMarkersAsync(string scriptFileContent, Hashtable settings) + { + throw CreateInvocationException(); + } + + public Task GetSemanticMarkersAsync(string scriptFileContent, string settingsFilePath) + { + throw CreateInvocationException(); + } + + public Task GetSemanticMarkersAsync(string scriptFileContent, string[] rules) + { + throw CreateInvocationException(); + } + + private Exception CreateInvocationException() + { + return new InvalidOperationException($"{nameof(NullAnalysisEngine)} implements no functionality and its methods should not be called"); + } + + public void Dispose() + { + // Nothing to dispose + } + } + +} diff --git a/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs new file mode 100644 index 000000000..0de09b6e9 --- /dev/null +++ b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs @@ -0,0 +1,452 @@ +// +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. +// + +using Microsoft.Extensions.Logging; +using Microsoft.PowerShell.EditorServices.Services.TextDocument; +using System; +using System.Collections; +using System.Collections.Generic; +using System.Collections.ObjectModel; +using System.IO; +using System.Linq; +using System.Management.Automation; +using System.Management.Automation.Runspaces; +using System.Text; +using System.Threading; +using System.Threading.Tasks; + +namespace Microsoft.PowerShell.EditorServices.Services.Analysis +{ + internal class PssaCmdletAnalysisEngine : IAnalysisEngine + { + private const string PSSA_MODULE_NAME = "PSScriptAnalyzer"; + + /// + /// The indentation to add when the logger lists errors. + /// + private static readonly string s_indentJoin = Environment.NewLine + " "; + + private static readonly IReadOnlyCollection s_emptyDiagnosticResult = new Collection(); + + private static readonly ScriptFileMarkerLevel[] s_scriptMarkerLevels = new[] + { + ScriptFileMarkerLevel.Error, + ScriptFileMarkerLevel.Warning, + ScriptFileMarkerLevel.Information + }; + + public static PssaCmdletAnalysisEngine Create(ILogger logger) + { + // RunspacePool takes care of queuing commands for us so we do not + // need to worry about executing concurrent commands + RunspacePool pssaRunspacePool = CreatePssaRunspacePool(out PSModuleInfo pssaModuleInfo); + + var cmdletAnalysisEngine = new PssaCmdletAnalysisEngine(logger, pssaRunspacePool, pssaModuleInfo); + + cmdletAnalysisEngine.LogAvailablePssaFeatures(); + + return cmdletAnalysisEngine; + } + + private readonly ILogger _logger; + + private readonly RunspacePool _analysisRunspacePool; + + private readonly PSModuleInfo _pssaModuleInfo; + + private bool _alreadyRun; + + private PssaCmdletAnalysisEngine( + ILogger logger, + RunspacePool analysisRunspacePool, + PSModuleInfo pssaModuleInfo) + { + _logger = logger; + _analysisRunspacePool = analysisRunspacePool; + _pssaModuleInfo = pssaModuleInfo; + _alreadyRun = false; + } + + public bool IsEnabled => true; + + public async Task FormatAsync(string scriptDefinition, Hashtable settings, int[] rangeList) + { + // We cannot use Range type therefore this workaround of using -1 default value. + // Invoke-Formatter throws a ParameterBinderValidationException if the ScriptDefinition is an empty string. + if (string.IsNullOrEmpty(scriptDefinition)) + { + return null; + } + + var psCommand = new PSCommand() + .AddCommand("Invoke-Formatter") + .AddParameter("ScriptDefinition", scriptDefinition) + .AddParameter("Settings", settings); + + if (rangeList != null) + { + psCommand.AddParameter("Range", rangeList); + } + + PowerShellResult result = await InvokePowerShellAsync(psCommand).ConfigureAwait(false); + + if (result == null) + { + _logger.LogError("Formatter returned null result"); + return null; + } + + if (result.HasErrors) + { + var errorBuilder = new StringBuilder().Append(s_indentJoin); + foreach (ErrorRecord err in result.Errors) + { + errorBuilder.Append(err).Append(s_indentJoin); + } + _logger.LogWarning($"Errors found while formatting file: {errorBuilder}"); + return null; + } + + foreach (PSObject resultObj in result.Output) + { + if (resultObj?.BaseObject is string formatResult) + { + return formatResult; + } + } + + return null; + } + + public Task GetSemanticMarkersAsync(string scriptContent, Hashtable settings) + { + PSCommand command = CreateInitialScriptAnalyzerInvocation(scriptContent); + + if (command == null) + { + return Task.FromResult(Array.Empty()); + } + + command.AddParameter("Settings", settings); + + return GetSemanticMarkersFromCommandAsync(command); + } + + public Task GetSemanticMarkersAsync(string scriptContent, string settingsFilePath) + { + PSCommand command = CreateInitialScriptAnalyzerInvocation(scriptContent); + + if (command == null) + { + return Task.FromResult(Array.Empty()); + } + + command.AddParameter("Settings", settingsFilePath); + + return GetSemanticMarkersFromCommandAsync(command); + } + + public Task GetSemanticMarkersAsync(string scriptContent, string[] rules) + { + PSCommand command = CreateInitialScriptAnalyzerInvocation(scriptContent); + + if (command == null) + { + return Task.FromResult(Array.Empty()); + } + + command.AddParameter("IncludeRules", rules); + + return GetSemanticMarkersFromCommandAsync(command); + } + + #region IDisposable Support + private bool disposedValue = false; // To detect redundant calls + + protected virtual void Dispose(bool disposing) + { + if (!disposedValue) + { + if (disposing) + { + _analysisRunspacePool.Dispose(); + } + + disposedValue = true; + } + } + + // This code added to correctly implement the disposable pattern. + public void Dispose() + { + // Do not change this code. Put cleanup code in Dispose(bool disposing) above. + Dispose(true); + } + + #endregion + + private PSCommand CreateInitialScriptAnalyzerInvocation(string scriptContent) + { + // When a new, empty file is created there are by definition no issues. + // Furthermore, if you call Invoke-ScriptAnalyzer with an empty ScriptDefinition + // it will generate a ParameterBindingValidationException. + if (string.IsNullOrEmpty(scriptContent)) + { + return null; + } + + return new PSCommand() + .AddCommand("Invoke-ScriptAnalyzer") + .AddParameter("ScriptDefinition", scriptContent) + .AddParameter("Severity", s_scriptMarkerLevels); + } + + private async Task GetSemanticMarkersFromCommandAsync(PSCommand command) + { + PowerShellResult result = await InvokePowerShellAsync(command).ConfigureAwait(false); + + IReadOnlyCollection diagnosticResults = result?.Output ?? s_emptyDiagnosticResult; + _logger.LogDebug(String.Format("Found {0} violations", diagnosticResults.Count)); + + var scriptMarkers = new ScriptFileMarker[diagnosticResults.Count]; + int i = 0; + foreach (PSObject diagnostic in diagnosticResults) + { + scriptMarkers[i] = ScriptFileMarker.FromDiagnosticRecord(diagnostic); + i++; + } + + return scriptMarkers; + } + + private Task InvokePowerShellAsync(PSCommand command) + { + return Task.Run(() => InvokePowerShell(command)); + } + + private PowerShellResult InvokePowerShell(PSCommand command) + { + using (var powerShell = System.Management.Automation.PowerShell.Create()) + { + powerShell.RunspacePool = _analysisRunspacePool; + powerShell.Commands = command; + PowerShellResult result = null; + try + { + Collection output = InvokePowerShellWithModulePathPreservation(powerShell); + PSDataCollection errors = powerShell.Streams.Error; + result = new PowerShellResult(output, errors, powerShell.HadErrors); + } + catch (CommandNotFoundException ex) + { + // This exception is possible if the module path loaded + // is wrong even though PSScriptAnalyzer is available as a module + _logger.LogError(ex.Message); + } + catch (CmdletInvocationException ex) + { + // We do not want to crash EditorServices for exceptions caused by cmdlet invocation. + // Two main reasons that cause the exception are: + // * PSCmdlet.WriteOutput being called from another thread than Begin/Process + // * CompositionContainer.ComposeParts complaining that "...Only one batch can be composed at a time" + _logger.LogError(ex.Message); + } + + return result; + } + } + + private Collection InvokePowerShellWithModulePathPreservation(System.Management.Automation.PowerShell powershell) + { + if (_alreadyRun) + { + return powershell.Invoke(); + } + + // PSScriptAnalyzer uses a runspace pool underneath and we need to stop the PSModulePath from being changed by it + try + { + using (PSModulePathPreserver.Take()) + { + return powershell.Invoke(); + } + } + finally + { + _alreadyRun = true; + } + } + + /// + /// Log the features available from the PSScriptAnalyzer module that has been imported + /// for use with the AnalysisService. + /// + private void LogAvailablePssaFeatures() + { + // Save ourselves some work here + if (!_logger.IsEnabled(LogLevel.Debug)) + { + return; + } + + // If we already know the module that was imported, save some work + if (_pssaModuleInfo == null) + { + } + + if (_pssaModuleInfo == null) + { + throw new FileNotFoundException("Unable to find loaded PSScriptAnalyzer module for logging"); + } + + var sb = new StringBuilder(); + sb.AppendLine("PSScriptAnalyzer successfully imported:"); + + // Log version + sb.Append(" Version: "); + sb.AppendLine(_pssaModuleInfo.Version.ToString()); + + // Log exported cmdlets + sb.AppendLine(" Exported Cmdlets:"); + foreach (string cmdletName in _pssaModuleInfo.ExportedCmdlets.Keys.OrderBy(name => name)) + { + sb.Append(" "); + sb.AppendLine(cmdletName); + } + + // Log available rules + sb.AppendLine(" Available Rules:"); + foreach (string ruleName in GetPSScriptAnalyzerRules()) + { + sb.Append(" "); + sb.AppendLine(ruleName); + } + + _logger.LogDebug(sb.ToString()); + } + + /// + /// Returns a list of builtin-in PSScriptAnalyzer rules + /// + private IEnumerable GetPSScriptAnalyzerRules() + { + PowerShellResult getRuleResult = InvokePowerShell(new PSCommand().AddCommand("Get-ScriptAnalyzerRule")); + if (getRuleResult == null) + { + _logger.LogWarning("Get-ScriptAnalyzerRule returned null result"); + return Enumerable.Empty(); + } + + var ruleNames = new List(getRuleResult.Output.Count); + foreach (var rule in getRuleResult.Output) + { + ruleNames.Add((string)rule.Members["RuleName"].Value); + } + + return ruleNames; + } + + /// + /// Create a new runspace pool around a PSScriptAnalyzer module for asynchronous script analysis tasks. + /// This looks for the latest version of PSScriptAnalyzer on the path and loads that. + /// + /// A runspace pool with PSScriptAnalyzer loaded for running script analysis tasks. + private static RunspacePool CreatePssaRunspacePool(out PSModuleInfo pssaModuleInfo) + { + using (var ps = System.Management.Automation.PowerShell.Create()) + { + // Run `Get-Module -ListAvailable -Name "PSScriptAnalyzer"` + ps.AddCommand("Get-Module") + .AddParameter("ListAvailable") + .AddParameter("Name", PSSA_MODULE_NAME); + + try + { + // Get the latest version of PSScriptAnalyzer we can find + pssaModuleInfo = ps.Invoke()? + .OrderByDescending(moduleInfo => moduleInfo.Version) + .FirstOrDefault(); + } + catch (Exception e) + { + throw new FileNotFoundException("Unable to find PSScriptAnalyzer module on the module path", e); + } + + if (pssaModuleInfo == null) + { + throw new FileNotFoundException("Unable to find PSScriptAnalyzer module on the module path"); + } + + // Create a base session state with PSScriptAnalyzer loaded +#if DEBUG + InitialSessionState sessionState = Environment.GetEnvironmentVariable("PSES_TEST_USE_CREATE_DEFAULT") == "1" + ? InitialSessionState.CreateDefault() + : InitialSessionState.CreateDefault2(); +#else + InitialSessionState sessionState = InitialSessionState.CreateDefault2(); +#endif + + sessionState.ImportPSModule(new [] { pssaModuleInfo.ModuleBase }); + + RunspacePool runspacePool = RunspaceFactory.CreateRunspacePool(sessionState); + + // Open the runspace pool here so we can deterministically handle the PSModulePath change issue + using (PSModulePathPreserver.Take()) + { + runspacePool.Open(); + } + + return runspacePool; + } + } + + /// + /// Wraps the result of an execution of PowerShell to send back through + /// asynchronous calls. + /// + private class PowerShellResult + { + public PowerShellResult( + Collection output, + PSDataCollection errors, + bool hasErrors) + { + Output = output; + Errors = errors; + HasErrors = hasErrors; + } + + public Collection Output { get; } + + public PSDataCollection Errors { get; } + + public bool HasErrors { get; } + } + + private struct PSModulePathPreserver : IDisposable + { + private static object s_psModulePathMutationLock = new object(); + + public static PSModulePathPreserver Take() + { + Monitor.Enter(s_psModulePathMutationLock); + return new PSModulePathPreserver(Environment.GetEnvironmentVariable("PSModulePath")); + } + + private readonly string _psModulePath; + + private PSModulePathPreserver(string psModulePath) + { + _psModulePath = psModulePath; + } + + public void Dispose() + { + Environment.SetEnvironmentVariable("PSModulePath", _psModulePath); + Monitor.Exit(s_psModulePathMutationLock); + } + } + } +} diff --git a/src/PowerShellEditorServices/Services/TextDocument/ScriptFileMarker.cs b/src/PowerShellEditorServices/Services/TextDocument/ScriptFileMarker.cs index 9609b2cae..9ab80cb0b 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/ScriptFileMarker.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/ScriptFileMarker.cs @@ -110,19 +110,17 @@ internal static ScriptFileMarker FromParseError( } private static string GetIfExistsString(PSObject psobj, string memberName) { - if (psobj.Members.Match(memberName).Count > 0) + if (psobj.Members.Match(memberName).Count == 0) { - return psobj.Members[memberName].Value != null ? (string)psobj.Members[memberName].Value : ""; - } - else - { - return ""; + return string.Empty; } + + return psobj.Members[memberName].Value as string ?? string.Empty; } internal static ScriptFileMarker FromDiagnosticRecord(PSObject psObject) { - Validate.IsNotNull("psObject", psObject); + Validate.IsNotNull(nameof(psObject), psObject); MarkerCorrection correction = null; // make sure psobject is of type DiagnosticRecord @@ -175,8 +173,8 @@ internal static ScriptFileMarker FromDiagnosticRecord(PSObject psObject) return new ScriptFileMarker { - Message = $"{diagnosticRecord.Message as string}", - RuleName = $"{diagnosticRecord.RuleName as string}", + Message = diagnosticRecord.Message as string ?? string.Empty, + RuleName = diagnosticRecord.RuleName as string ?? string.Empty, Level = level, ScriptRegion = ScriptRegion.Create(diagnosticRecord.Extent as IScriptExtent), Correction = correction, From 104da3ffe45e82be5e9e5dc6f7a2e5cc2642a0e2 Mon Sep 17 00:00:00 2001 From: Rob Holt Date: Wed, 5 Feb 2020 11:43:39 -0800 Subject: [PATCH 02/33] Remove old analysis service --- .../Services/Analysis/AnalysisService.cs | 128 ++---------------- 1 file changed, 14 insertions(+), 114 deletions(-) diff --git a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs index 1ac55be01..704624bc7 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -23,6 +23,10 @@ namespace Microsoft.PowerShell.EditorServices.Services { + /// + /// Provides a high-level service for performing semantic analysis + /// of PowerShell scripts. + /// internal class AnalysisService : IDisposable { /// @@ -48,6 +52,16 @@ internal class AnalysisService : IDisposable }; + /// + /// Factory method for producing AnalysisService instances. Handles loading of the PSScriptAnalyzer module + /// and runspace pool instantiation before creating the service instance. + /// + /// EditorServices logger for logging information. + /// The language server instance to use for messaging. + /// + /// A new analysis service instance with a freshly imported PSScriptAnalyzer module and runspace pool. + /// Returns null if problems occur. This method should never throw. + /// public static AnalysisService Create(ILogger logger, ILanguageServer languageServer) { IAnalysisEngine analysisEngine = PssaCmdletAnalysisEngine.Create(logger); @@ -299,118 +313,4 @@ public void Dispose() #endregion } - - /// - /// Provides a high-level service for performing semantic analysis - /// of PowerShell scripts. - /// - public class OldOldAnalysisService - { - /// - /// Construct a new AnalysisService object. - /// - /// - /// The runspace pool with PSScriptAnalyzer module loaded that will handle - /// analysis tasks. - /// - /// - /// The path to the PSScriptAnalyzer settings file to handle analysis settings. - /// - /// An array of rules to be used for analysis. - /// Maintains logs for the analysis service. - /// - /// Optional module info of the loaded PSScriptAnalyzer module. If not provided, - /// the analysis service will populate it, but it can be given here to save time. - /// - private AnalysisService( - RunspacePool analysisRunspacePool, - string pssaSettingsPath, - IEnumerable activeRules, - ILanguageServer languageServer, - ConfigurationService configurationService, - ILogger logger, - PSModuleInfo pssaModuleInfo = null) - { - _analysisRunspacePool = analysisRunspacePool; - SettingsPath = pssaSettingsPath; - ActiveRules = activeRules.ToArray(); - _languageServer = languageServer; - _configurationService = configurationService; - _logger = logger; - _pssaModuleInfo = pssaModuleInfo; - _mostRecentCorrectionsByFile = new ConcurrentDictionary)>(); - } - - #endregion // constructors - - #region Public Methods - - /// - /// Factory method for producing AnalysisService instances. Handles loading of the PSScriptAnalyzer module - /// and runspace pool instantiation before creating the service instance. - /// - /// Path to the PSSA settings file to be used for this service instance. - /// EditorServices logger for logging information. - /// - /// A new analysis service instance with a freshly imported PSScriptAnalyzer module and runspace pool. - /// Returns null if problems occur. This method should never throw. - /// - public static AnalysisService Create(ConfigurationService configurationService, ILanguageServer languageServer, ILogger logger) - { - Validate.IsNotNull(nameof(configurationService), configurationService); - string settingsPath = configurationService.CurrentSettings.ScriptAnalysis.SettingsPath; - try - { - RunspacePool analysisRunspacePool; - PSModuleInfo pssaModuleInfo; - try - { - // Try and load a PSScriptAnalyzer module with the required version - // by looking on the script path. Deep down, this internally runs Get-Module -ListAvailable, - // so we'll use this to check whether such a module exists - analysisRunspacePool = CreatePssaRunspacePool(out pssaModuleInfo); - - } - catch (Exception e) - { - throw new AnalysisServiceLoadException("PSScriptAnalyzer runspace pool could not be created", e); - } - - if (analysisRunspacePool == null) - { - throw new AnalysisServiceLoadException("PSScriptAnalyzer runspace pool failed to be created"); - } - - // Having more than one runspace doesn't block code formatting if one - // runspace is occupied for diagnostics - analysisRunspacePool.SetMaxRunspaces(NumRunspaces); - analysisRunspacePool.ThreadOptions = PSThreadOptions.ReuseThread; - analysisRunspacePool.Open(); - - var analysisService = new AnalysisService( - analysisRunspacePool, - settingsPath, - s_includedRules, - languageServer, - configurationService, - logger, - pssaModuleInfo); - - // Log what features are available in PSSA here - analysisService.LogAvailablePssaFeatures(); - - return analysisService; - } - catch (AnalysisServiceLoadException e) - { - logger.LogWarning("PSScriptAnalyzer cannot be imported, AnalysisService will be disabled", e); - return null; - } - catch (Exception e) - { - logger.LogWarning("AnalysisService could not be started due to an unexpected exception", e); - return null; - } - } - } } From 617ba319d704ddb49c2df5a3f668eef7ba9e3c9f Mon Sep 17 00:00:00 2001 From: Rob Holt Date: Wed, 5 Feb 2020 11:43:54 -0800 Subject: [PATCH 03/33] Restore old runspace pool settings --- .../Services/Analysis/PssaCmdletAnalysisEngine.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs index 0de09b6e9..8cc948d1b 100644 --- a/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs +++ b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs @@ -392,6 +392,9 @@ private static RunspacePool CreatePssaRunspacePool(out PSModuleInfo pssaModuleIn RunspacePool runspacePool = RunspaceFactory.CreateRunspacePool(sessionState); + runspacePool.SetMaxRunspaces(1); + runspacePool.ThreadOptions = PSThreadOptions.ReuseThread; + // Open the runspace pool here so we can deterministically handle the PSModulePath change issue using (PSModulePathPreserver.Take()) { From ba36e4548fbd1dcc2b5518f793ff6f2734204c34 Mon Sep 17 00:00:00 2001 From: Rob Holt Date: Wed, 5 Feb 2020 11:52:59 -0800 Subject: [PATCH 04/33] Change analysis engine throwing to null analysis engine --- .../Analysis/PssaCmdletAnalysisEngine.cs | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs index 8cc948d1b..efbaf8ceb 100644 --- a/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs +++ b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs @@ -41,13 +41,18 @@ public static PssaCmdletAnalysisEngine Create(ILogger logger) { // RunspacePool takes care of queuing commands for us so we do not // need to worry about executing concurrent commands - RunspacePool pssaRunspacePool = CreatePssaRunspacePool(out PSModuleInfo pssaModuleInfo); - - var cmdletAnalysisEngine = new PssaCmdletAnalysisEngine(logger, pssaRunspacePool, pssaModuleInfo); - - cmdletAnalysisEngine.LogAvailablePssaFeatures(); - - return cmdletAnalysisEngine; + try + { + RunspacePool pssaRunspacePool = CreatePssaRunspacePool(out PSModuleInfo pssaModuleInfo); + var cmdletAnalysisEngine = new PssaCmdletAnalysisEngine(logger, pssaRunspacePool, pssaModuleInfo); + cmdletAnalysisEngine.LogAvailablePssaFeatures(); + return cmdletAnalysisEngine; + } + catch (FileNotFoundException e) + { + logger.LogError(e, "Unable to find PSScriptAnalyzer. Disabling script analysis."); + return null; + } } private readonly ILogger _logger; @@ -291,11 +296,6 @@ private void LogAvailablePssaFeatures() return; } - // If we already know the module that was imported, save some work - if (_pssaModuleInfo == null) - { - } - if (_pssaModuleInfo == null) { throw new FileNotFoundException("Unable to find loaded PSScriptAnalyzer module for logging"); @@ -428,6 +428,13 @@ public PowerShellResult( public bool HasErrors { get; } } + /// + /// Struct to manage a call that may change the PSModulePath, so that it can be safely reset afterward. + /// + /// + /// If the user manages to set the module path at the same time, using this struct may override that. + /// But this happening is less likely than the current issue where the PSModulePath is always reset. + /// private struct PSModulePathPreserver : IDisposable { private static object s_psModulePathMutationLock = new object(); From d3d22a9519fd93127e53d6781a853fb2dd88070d Mon Sep 17 00:00:00 2001 From: Rob Holt Date: Wed, 5 Feb 2020 11:58:43 -0800 Subject: [PATCH 05/33] Add some comments --- .../Services/Analysis/AnalysisService.cs | 4 +- .../Services/Analysis/IAnalysisEngine.cs | 37 +++++++++++++++++-- .../Services/Analysis/NullAnalysisEngine.cs | 6 +-- .../Analysis/PssaCmdletAnalysisEngine.cs | 6 +-- 4 files changed, 42 insertions(+), 11 deletions(-) diff --git a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs index 704624bc7..16425fa7a 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -167,8 +167,8 @@ private async Task DelayThenInvokeDiagnosticsAsync(ScriptFile[] filesToAnalyze, foreach (ScriptFile scriptFile in filesToAnalyze) { ScriptFileMarker[] semanticMarkers = SettingsPath != null - ? await _analysisEngine.GetSemanticMarkersAsync(scriptFile.Contents, SettingsPath).ConfigureAwait(false) - : await _analysisEngine.GetSemanticMarkersAsync(scriptFile.Contents, EnabledRules).ConfigureAwait(false); + ? await _analysisEngine.AnalyzeScriptAsync(scriptFile.Contents, SettingsPath).ConfigureAwait(false) + : await _analysisEngine.AnalyzeScriptAsync(scriptFile.Contents, EnabledRules).ConfigureAwait(false); scriptFile.DiagnosticMarkers.AddRange(semanticMarkers); diff --git a/src/PowerShellEditorServices/Services/Analysis/IAnalysisEngine.cs b/src/PowerShellEditorServices/Services/Analysis/IAnalysisEngine.cs index 8ea89f2f9..5b92534cd 100644 --- a/src/PowerShellEditorServices/Services/Analysis/IAnalysisEngine.cs +++ b/src/PowerShellEditorServices/Services/Analysis/IAnalysisEngine.cs @@ -10,24 +10,55 @@ namespace Microsoft.PowerShell.EditorServices.Services.Analysis { + /// + /// An engine to run PowerShell script analysis. + /// internal interface IAnalysisEngine : IDisposable { + /// + /// If false, other methods on this object will have no meaningful implementation. + /// bool IsEnabled { get; } + /// + /// Format a PowerShell script. + /// + /// The string of the PowerShell script to format. + /// The formatter settings. + /// An optional list of ranges to format in the script. + /// Task FormatAsync( string scriptDefinition, Hashtable settings, int[] rangeList); - Task GetSemanticMarkersAsync( + /// + /// Analyze a PowerShell script file using a settings object. + /// + /// The script to analyze in string form. + /// The settings hashtable to configure the analyzer with. + /// Markers for any diagnostics in the script. + Task AnalyzeScriptAsync( string scriptFileContent, Hashtable settings); - Task GetSemanticMarkersAsync( + /// + /// Analyze a PowerShell script file with a path to a settings file. + /// + /// The script to analyze in string form. + /// The path to the settings file with which to configure the analyzer. + /// Markers for any diagnostics in the script. + Task AnalyzeScriptAsync( string scriptFileContent, string settingsFilePath); - Task GetSemanticMarkersAsync( + /// + /// Analyze a PowerShell script file using a set of script analysis rules. + /// + /// The script to analyze in string form. + /// The rules to run on the script for analysis. + /// Markers for any diagnostics in the script. + Task AnalyzeScriptAsync( string scriptFileContent, string[] rules); diff --git a/src/PowerShellEditorServices/Services/Analysis/NullAnalysisEngine.cs b/src/PowerShellEditorServices/Services/Analysis/NullAnalysisEngine.cs index 875d28464..60cae1e1a 100644 --- a/src/PowerShellEditorServices/Services/Analysis/NullAnalysisEngine.cs +++ b/src/PowerShellEditorServices/Services/Analysis/NullAnalysisEngine.cs @@ -19,17 +19,17 @@ public Task FormatAsync(string scriptDefinition, Hashtable settings, int throw CreateInvocationException(); } - public Task GetSemanticMarkersAsync(string scriptFileContent, Hashtable settings) + public Task AnalyzeScriptAsync(string scriptFileContent, Hashtable settings) { throw CreateInvocationException(); } - public Task GetSemanticMarkersAsync(string scriptFileContent, string settingsFilePath) + public Task AnalyzeScriptAsync(string scriptFileContent, string settingsFilePath) { throw CreateInvocationException(); } - public Task GetSemanticMarkersAsync(string scriptFileContent, string[] rules) + public Task AnalyzeScriptAsync(string scriptFileContent, string[] rules) { throw CreateInvocationException(); } diff --git a/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs index efbaf8ceb..0435fdb34 100644 --- a/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs +++ b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs @@ -125,7 +125,7 @@ public async Task FormatAsync(string scriptDefinition, Hashtable setting return null; } - public Task GetSemanticMarkersAsync(string scriptContent, Hashtable settings) + public Task AnalyzeScriptAsync(string scriptContent, Hashtable settings) { PSCommand command = CreateInitialScriptAnalyzerInvocation(scriptContent); @@ -139,7 +139,7 @@ public Task GetSemanticMarkersAsync(string scriptContent, Ha return GetSemanticMarkersFromCommandAsync(command); } - public Task GetSemanticMarkersAsync(string scriptContent, string settingsFilePath) + public Task AnalyzeScriptAsync(string scriptContent, string settingsFilePath) { PSCommand command = CreateInitialScriptAnalyzerInvocation(scriptContent); @@ -153,7 +153,7 @@ public Task GetSemanticMarkersAsync(string scriptContent, st return GetSemanticMarkersFromCommandAsync(command); } - public Task GetSemanticMarkersAsync(string scriptContent, string[] rules) + public Task AnalyzeScriptAsync(string scriptContent, string[] rules) { PSCommand command = CreateInitialScriptAnalyzerInvocation(scriptContent); From ccb4caa8b4023f2e5d901db2e3e8f17950e73df0 Mon Sep 17 00:00:00 2001 From: Rob Holt Date: Wed, 5 Feb 2020 15:25:10 -0800 Subject: [PATCH 06/33] Allow AnalysisService to reinstantiate its engine --- .../Services/Analysis/AnalysisService.cs | 172 +++++++++++------- .../Services/Analysis/IAnalysisEngine.cs | 31 +--- .../Services/Analysis/NullAnalysisEngine.cs | 16 +- .../Analysis/PssaCmdletAnalysisEngine.cs | 155 +++++++++------- .../Handlers/ConfigurationHandler.cs | 32 +--- .../Services/Workspace/WorkspaceService.cs | 5 + 6 files changed, 217 insertions(+), 194 deletions(-) diff --git a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs index 16425fa7a..0ac8a23a3 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -20,6 +20,9 @@ using Microsoft.PowerShell.EditorServices.Utility; using System.Collections.ObjectModel; using Microsoft.PowerShell.EditorServices.Services.Analysis; +using Microsoft.PowerShell.EditorServices.Handlers; +using System.IO; +using Microsoft.PowerShell.EditorServices.Services.Configuration; namespace Microsoft.PowerShell.EditorServices.Services { @@ -29,6 +32,30 @@ namespace Microsoft.PowerShell.EditorServices.Services /// internal class AnalysisService : IDisposable { + internal static string GetUniqueIdFromDiagnostic(Diagnostic diagnostic) + { + Position start = diagnostic.Range.Start; + Position end = diagnostic.Range.End; + + var sb = new StringBuilder(256) + .Append(diagnostic.Source ?? "?") + .Append("_") + .Append(diagnostic.Code.IsString ? diagnostic.Code.String : diagnostic.Code.Long.ToString()) + .Append("_") + .Append(diagnostic.Severity?.ToString() ?? "?") + .Append("_") + .Append(start.Line) + .Append(":") + .Append(start.Character) + .Append("-") + .Append(end.Line) + .Append(":") + .Append(end.Character); + + var id = sb.ToString(); + return id; + } + /// /// Defines the list of Script Analyzer rules to include by default if /// no settings file is specified. @@ -51,52 +78,32 @@ internal class AnalysisService : IDisposable "PSPossibleIncorrectUsageOfRedirectionOperator" }; - - /// - /// Factory method for producing AnalysisService instances. Handles loading of the PSScriptAnalyzer module - /// and runspace pool instantiation before creating the service instance. - /// - /// EditorServices logger for logging information. - /// The language server instance to use for messaging. - /// - /// A new analysis service instance with a freshly imported PSScriptAnalyzer module and runspace pool. - /// Returns null if problems occur. This method should never throw. - /// - public static AnalysisService Create(ILogger logger, ILanguageServer languageServer) - { - IAnalysisEngine analysisEngine = PssaCmdletAnalysisEngine.Create(logger); - - // ?? doesn't work above sadly - if (analysisEngine == null) - { - analysisEngine = new NullAnalysisEngine(); - } - - var analysisService = new AnalysisService(logger, languageServer, analysisEngine) - { - EnabledRules = s_defaultRules, - }; - - return analysisService; - } - private readonly ILogger _logger; private readonly ILanguageServer _languageServer; - private readonly IAnalysisEngine _analysisEngine; + private readonly ConfigurationService _configurationService; + + private readonly WorkspaceService _workplaceService; private readonly int _analysisDelayMillis; private readonly ConcurrentDictionary)> _mostRecentCorrectionsByFile; + private IAnalysisEngine _analysisEngine; + private CancellationTokenSource _diagnosticsCancellationTokenSource; - public AnalysisService(ILogger logger, ILanguageServer languageServer, IAnalysisEngine analysisEngine) + public AnalysisService( + ILogger logger, + ILanguageServer languageServer, + ConfigurationService configurationService, + WorkspaceService workspaceService) { _logger = logger; _languageServer = languageServer; - _analysisEngine = analysisEngine; + _configurationService = configurationService; + _workplaceService = workspaceService; _analysisDelayMillis = 750; _mostRecentCorrectionsByFile = new ConcurrentDictionary)>(); } @@ -105,6 +112,19 @@ public AnalysisService(ILogger logger, ILanguageServer languageServer, IAnalysis public string SettingsPath { get; set; } + private IAnalysisEngine AnalysisEngine + { + get + { + if (_analysisEngine == null) + { + _analysisEngine = InstantiateAnalysisEngine(); + } + + return _analysisEngine; + } + } + public Task RunScriptDiagnosticsAsync( ScriptFile[] filesToAnalyze, CancellationToken cancellationToken) @@ -135,6 +155,58 @@ public Task RunScriptDiagnosticsAsync( return Task.Run(() => DelayThenInvokeDiagnosticsAsync(filesToAnalyze, _diagnosticsCancellationTokenSource.Token), _diagnosticsCancellationTokenSource.Token); } + public void OnConfigurationUpdated(object sender, LanguageServerSettings settings) + { + ClearMarkers(); + _analysisEngine = InstantiateAnalysisEngine(); + } + + private IAnalysisEngine InstantiateAnalysisEngine() + { + if (_configurationService.CurrentSettings.ScriptAnalysis.Enable ?? false) + { + return new NullAnalysisEngine(); + } + + var pssaCmdletEngineBuilder = new PssaCmdletAnalysisEngine.Builder(_logger); + + if (TryFindSettingsFile(out string settingsFilePath)) + { + _logger.LogInformation($"Configuring PSScriptAnalyzer with rules at '{settingsFilePath}'"); + pssaCmdletEngineBuilder.WithSettingsFile(settingsFilePath); + } + else + { + _logger.LogInformation("PSScriptAnalyzer settings file not found. Falling back to default rules"); + pssaCmdletEngineBuilder.WithIncludedRules(s_defaultRules); + } + + return pssaCmdletEngineBuilder.Build(); + } + + private bool TryFindSettingsFile(out string settingsFilePath) + { + string configuredPath = _configurationService.CurrentSettings.ScriptAnalysis.SettingsPath; + + if (!string.IsNullOrEmpty(configuredPath)) + { + settingsFilePath = _workplaceService.ResolveWorkspacePath(configuredPath); + return settingsFilePath != null; + } + + // TODO: Could search for a default here + + settingsFilePath = null; + return false; + } + + private Task ClearMarkers() + { + return Task.WhenAll( + _workplaceService.GetOpenedFiles() + .Select(file => PublishScriptDiagnosticsAsync(file, Array.Empty()))); + } + private async Task DelayThenInvokeDiagnosticsAsync(ScriptFile[] filesToAnalyze, CancellationToken cancellationToken) { if (cancellationToken.IsCancellationRequested) @@ -166,9 +238,7 @@ private async Task DelayThenInvokeDiagnosticsAsync(ScriptFile[] filesToAnalyze, foreach (ScriptFile scriptFile in filesToAnalyze) { - ScriptFileMarker[] semanticMarkers = SettingsPath != null - ? await _analysisEngine.AnalyzeScriptAsync(scriptFile.Contents, SettingsPath).ConfigureAwait(false) - : await _analysisEngine.AnalyzeScriptAsync(scriptFile.Contents, EnabledRules).ConfigureAwait(false); + ScriptFileMarker[] semanticMarkers = await _analysisEngine.AnalyzeScriptAsync(scriptFile.Contents).ConfigureAwait(false); scriptFile.DiagnosticMarkers.AddRange(semanticMarkers); @@ -176,7 +246,9 @@ private async Task DelayThenInvokeDiagnosticsAsync(ScriptFile[] filesToAnalyze, } } - private async Task PublishScriptDiagnosticsAsync(ScriptFile scriptFile) + private Task PublishScriptDiagnosticsAsync(ScriptFile scriptFile) => PublishScriptDiagnosticsAsync(scriptFile, scriptFile.DiagnosticMarkers); + + private async Task PublishScriptDiagnosticsAsync(ScriptFile scriptFile, IReadOnlyList markers) { (SemaphoreSlim fileLock, Dictionary fileCorrections) = _mostRecentCorrectionsByFile.GetOrAdd( scriptFile.DocumentUri, @@ -189,9 +261,9 @@ private async Task PublishScriptDiagnosticsAsync(ScriptFile scriptFile) { fileCorrections.Clear(); - for (int i = 0; i < scriptFile.DiagnosticMarkers.Count; i++) + for (int i = 0; i < markers.Count; i++) { - ScriptFileMarker marker = scriptFile.DiagnosticMarkers[i]; + ScriptFileMarker marker = markers[i]; Diagnostic diagnostic = GetDiagnosticFromMarker(marker); @@ -263,30 +335,6 @@ private static DiagnosticSeverity MapDiagnosticSeverity(ScriptFileMarkerLevel ma } } - internal static string GetUniqueIdFromDiagnostic(Diagnostic diagnostic) - { - Position start = diagnostic.Range.Start; - Position end = diagnostic.Range.End; - - var sb = new StringBuilder(256) - .Append(diagnostic.Source ?? "?") - .Append("_") - .Append(diagnostic.Code.IsString ? diagnostic.Code.String : diagnostic.Code.Long.ToString()) - .Append("_") - .Append(diagnostic.Severity?.ToString() ?? "?") - .Append("_") - .Append(start.Line) - .Append(":") - .Append(start.Character) - .Append("-") - .Append(end.Line) - .Append(":") - .Append(end.Character); - - var id = sb.ToString(); - return id; - } - #region IDisposable Support private bool disposedValue = false; // To detect redundant calls diff --git a/src/PowerShellEditorServices/Services/Analysis/IAnalysisEngine.cs b/src/PowerShellEditorServices/Services/Analysis/IAnalysisEngine.cs index 5b92534cd..b2de7b9c7 100644 --- a/src/PowerShellEditorServices/Services/Analysis/IAnalysisEngine.cs +++ b/src/PowerShellEditorServices/Services/Analysis/IAnalysisEngine.cs @@ -6,6 +6,7 @@ using Microsoft.PowerShell.EditorServices.Services.TextDocument; using System; using System.Collections; +using System.Collections.Generic; using System.Threading.Tasks; namespace Microsoft.PowerShell.EditorServices.Services.Analysis @@ -24,44 +25,20 @@ internal interface IAnalysisEngine : IDisposable /// Format a PowerShell script. /// /// The string of the PowerShell script to format. - /// The formatter settings. + /// The settings to apply when formatting. /// An optional list of ranges to format in the script. /// Task FormatAsync( string scriptDefinition, - Hashtable settings, + Hashtable formatSettings, int[] rangeList); /// /// Analyze a PowerShell script file using a settings object. /// /// The script to analyze in string form. - /// The settings hashtable to configure the analyzer with. /// Markers for any diagnostics in the script. - Task AnalyzeScriptAsync( - string scriptFileContent, - Hashtable settings); - - /// - /// Analyze a PowerShell script file with a path to a settings file. - /// - /// The script to analyze in string form. - /// The path to the settings file with which to configure the analyzer. - /// Markers for any diagnostics in the script. - Task AnalyzeScriptAsync( - string scriptFileContent, - string settingsFilePath); - - /// - /// Analyze a PowerShell script file using a set of script analysis rules. - /// - /// The script to analyze in string form. - /// The rules to run on the script for analysis. - /// Markers for any diagnostics in the script. - Task AnalyzeScriptAsync( - string scriptFileContent, - string[] rules); - + Task AnalyzeScriptAsync(string scriptFileContent); } } diff --git a/src/PowerShellEditorServices/Services/Analysis/NullAnalysisEngine.cs b/src/PowerShellEditorServices/Services/Analysis/NullAnalysisEngine.cs index 60cae1e1a..77ab4daa9 100644 --- a/src/PowerShellEditorServices/Services/Analysis/NullAnalysisEngine.cs +++ b/src/PowerShellEditorServices/Services/Analysis/NullAnalysisEngine.cs @@ -6,6 +6,7 @@ using Microsoft.PowerShell.EditorServices.Services.TextDocument; using System; using System.Collections; +using System.Collections.Generic; using System.Threading.Tasks; namespace Microsoft.PowerShell.EditorServices.Services.Analysis @@ -14,26 +15,15 @@ internal class NullAnalysisEngine : IAnalysisEngine { public bool IsEnabled => false; - public Task FormatAsync(string scriptDefinition, Hashtable settings, int[] rangeList) + public Task FormatAsync(string scriptDefinition, Hashtable formatSettings, int[] rangeList) { throw CreateInvocationException(); } - public Task AnalyzeScriptAsync(string scriptFileContent, Hashtable settings) + public Task AnalyzeScriptAsync(string scriptFileContent) { throw CreateInvocationException(); } - - public Task AnalyzeScriptAsync(string scriptFileContent, string settingsFilePath) - { - throw CreateInvocationException(); - } - - public Task AnalyzeScriptAsync(string scriptFileContent, string[] rules) - { - throw CreateInvocationException(); - } - private Exception CreateInvocationException() { return new InvalidOperationException($"{nameof(NullAnalysisEngine)} implements no functionality and its methods should not be called"); diff --git a/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs index 0435fdb34..f0e808100 100644 --- a/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs +++ b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs @@ -12,6 +12,7 @@ using System.IO; using System.Linq; using System.Management.Automation; +using System.Management.Automation.Language; using System.Management.Automation.Runspaces; using System.Text; using System.Threading; @@ -21,6 +22,60 @@ namespace Microsoft.PowerShell.EditorServices.Services.Analysis { internal class PssaCmdletAnalysisEngine : IAnalysisEngine { + public class Builder + { + private readonly ILogger _logger; + + private object _settingsParameter; + + private string[] _rules; + + public Builder(ILogger logger) + { + _logger = logger; + } + + public Builder WithSettingsFile(string settingsPath) + { + _settingsParameter = settingsPath; + return this; + } + + public Builder WithSettings(Hashtable settings) + { + _settingsParameter = settings; + return this; + } + + public Builder WithIncludedRules(string[] rules) + { + _rules = rules; + return this; + } + + public PssaCmdletAnalysisEngine Build() + { + // RunspacePool takes care of queuing commands for us so we do not + // need to worry about executing concurrent commands + try + { + RunspacePool pssaRunspacePool = CreatePssaRunspacePool(out PSModuleInfo pssaModuleInfo); + + PssaCmdletAnalysisEngine cmdletAnalysisEngine = _settingsParameter != null + ? new PssaCmdletAnalysisEngine(_logger, pssaRunspacePool, pssaModuleInfo, _settingsParameter) + : new PssaCmdletAnalysisEngine(_logger, pssaRunspacePool, pssaModuleInfo, _rules); + + cmdletAnalysisEngine.LogAvailablePssaFeatures(); + return cmdletAnalysisEngine; + } + catch (FileNotFoundException e) + { + _logger.LogError(e, "Unable to find PSScriptAnalyzer. Disabling script analysis."); + return null; + } + } + } + private const string PSSA_MODULE_NAME = "PSScriptAnalyzer"; /// @@ -37,46 +92,47 @@ internal class PssaCmdletAnalysisEngine : IAnalysisEngine ScriptFileMarkerLevel.Information }; - public static PssaCmdletAnalysisEngine Create(ILogger logger) - { - // RunspacePool takes care of queuing commands for us so we do not - // need to worry about executing concurrent commands - try - { - RunspacePool pssaRunspacePool = CreatePssaRunspacePool(out PSModuleInfo pssaModuleInfo); - var cmdletAnalysisEngine = new PssaCmdletAnalysisEngine(logger, pssaRunspacePool, pssaModuleInfo); - cmdletAnalysisEngine.LogAvailablePssaFeatures(); - return cmdletAnalysisEngine; - } - catch (FileNotFoundException e) - { - logger.LogError(e, "Unable to find PSScriptAnalyzer. Disabling script analysis."); - return null; - } - } - private readonly ILogger _logger; private readonly RunspacePool _analysisRunspacePool; private readonly PSModuleInfo _pssaModuleInfo; + private readonly object _settingsParameter; + + private readonly string[] _rulesToInclude; + private bool _alreadyRun; private PssaCmdletAnalysisEngine( ILogger logger, RunspacePool analysisRunspacePool, - PSModuleInfo pssaModuleInfo) + PSModuleInfo pssaModuleInfo, + string[] rulesToInclude) + { + _logger = logger; + _analysisRunspacePool = analysisRunspacePool; + _pssaModuleInfo = pssaModuleInfo; + _rulesToInclude = rulesToInclude; + _alreadyRun = false; + } + + private PssaCmdletAnalysisEngine( + ILogger logger, + RunspacePool analysisRunspacePool, + PSModuleInfo pssaModuleInfo, + object analysisSettingsParameter) { _logger = logger; _analysisRunspacePool = analysisRunspacePool; _pssaModuleInfo = pssaModuleInfo; + _settingsParameter = analysisSettingsParameter; _alreadyRun = false; } public bool IsEnabled => true; - public async Task FormatAsync(string scriptDefinition, Hashtable settings, int[] rangeList) + public async Task FormatAsync(string scriptDefinition, Hashtable formatSettings, int[] rangeList) { // We cannot use Range type therefore this workaround of using -1 default value. // Invoke-Formatter throws a ParameterBinderValidationException if the ScriptDefinition is an empty string. @@ -88,7 +144,7 @@ public async Task FormatAsync(string scriptDefinition, Hashtable setting var psCommand = new PSCommand() .AddCommand("Invoke-Formatter") .AddParameter("ScriptDefinition", scriptDefinition) - .AddParameter("Settings", settings); + .AddParameter("Settings", formatSettings); if (rangeList != null) { @@ -125,45 +181,30 @@ public async Task FormatAsync(string scriptDefinition, Hashtable setting return null; } - public Task AnalyzeScriptAsync(string scriptContent, Hashtable settings) + public Task AnalyzeScriptAsync(string scriptContent) { - PSCommand command = CreateInitialScriptAnalyzerInvocation(scriptContent); - - if (command == null) + // When a new, empty file is created there are by definition no issues. + // Furthermore, if you call Invoke-ScriptAnalyzer with an empty ScriptDefinition + // it will generate a ParameterBindingValidationException. + if (string.IsNullOrEmpty(scriptContent)) { return Task.FromResult(Array.Empty()); } - command.AddParameter("Settings", settings); - - return GetSemanticMarkersFromCommandAsync(command); - } - - public Task AnalyzeScriptAsync(string scriptContent, string settingsFilePath) - { - PSCommand command = CreateInitialScriptAnalyzerInvocation(scriptContent); + var command = new PSCommand() + .AddCommand("Invoke-ScriptAnalyzer") + .AddParameter("ScriptDefinition", scriptContent) + .AddParameter("Severity", s_scriptMarkerLevels); - if (command == null) + if (_settingsParameter != null) { - return Task.FromResult(Array.Empty()); + command.AddParameter("Settings", _settingsParameter); } - - command.AddParameter("Settings", settingsFilePath); - - return GetSemanticMarkersFromCommandAsync(command); - } - - public Task AnalyzeScriptAsync(string scriptContent, string[] rules) - { - PSCommand command = CreateInitialScriptAnalyzerInvocation(scriptContent); - - if (command == null) + else { - return Task.FromResult(Array.Empty()); + command.AddParameter("IncludeRules", _rulesToInclude); } - command.AddParameter("IncludeRules", rules); - return GetSemanticMarkersFromCommandAsync(command); } @@ -192,22 +233,6 @@ public void Dispose() #endregion - private PSCommand CreateInitialScriptAnalyzerInvocation(string scriptContent) - { - // When a new, empty file is created there are by definition no issues. - // Furthermore, if you call Invoke-ScriptAnalyzer with an empty ScriptDefinition - // it will generate a ParameterBindingValidationException. - if (string.IsNullOrEmpty(scriptContent)) - { - return null; - } - - return new PSCommand() - .AddCommand("Invoke-ScriptAnalyzer") - .AddParameter("ScriptDefinition", scriptContent) - .AddParameter("Severity", s_scriptMarkerLevels); - } - private async Task GetSemanticMarkersFromCommandAsync(PSCommand command) { PowerShellResult result = await InvokePowerShellAsync(command).ConfigureAwait(false); diff --git a/src/PowerShellEditorServices/Services/Workspace/Handlers/ConfigurationHandler.cs b/src/PowerShellEditorServices/Services/Workspace/Handlers/ConfigurationHandler.cs index 949ba04dd..10240a9ec 100644 --- a/src/PowerShellEditorServices/Services/Workspace/Handlers/ConfigurationHandler.cs +++ b/src/PowerShellEditorServices/Services/Workspace/Handlers/ConfigurationHandler.cs @@ -20,7 +20,6 @@ namespace Microsoft.PowerShell.EditorServices.Handlers internal class ConfigurationHandler : IDidChangeConfigurationHandler { private readonly ILogger _logger; - private readonly AnalysisService _analysisService; private readonly WorkspaceService _workspaceService; private readonly ConfigurationService _configurationService; private readonly PowerShellContextService _powerShellContextService; @@ -37,9 +36,10 @@ public ConfigurationHandler( { _logger = factory.CreateLogger(); _workspaceService = workspaceService; - _analysisService = analysisService; _configurationService = configurationService; _powerShellContextService = powerShellContextService; + + ConfigurationUpdated += analysisService.OnConfigurationUpdated; } public object GetRegistrationOptions() @@ -83,31 +83,7 @@ public async Task Handle(DidChangeConfigurationParams request, Cancellatio this._consoleReplStarted = true; } - // If there is a new settings file path, restart the analyzer with the new settings. - bool settingsPathChanged = false; - string newSettingsPath = _configurationService.CurrentSettings.ScriptAnalysis.SettingsPath; - if (!string.Equals(oldScriptAnalysisSettingsPath, newSettingsPath, StringComparison.OrdinalIgnoreCase)) - { - if (_analysisService != null) - { - _analysisService.SettingsPath = newSettingsPath; - settingsPathChanged = true; - } - } - - // If script analysis settings have changed we need to clear & possibly update the current diagnostic records. - if ((oldScriptAnalysisEnabled != _configurationService.CurrentSettings.ScriptAnalysis?.Enable) || settingsPathChanged) - { - // If the user just turned off script analysis or changed the settings path, send a diagnostics - // event to clear the analysis markers that they already have. - if (!_configurationService.CurrentSettings.ScriptAnalysis.Enable.Value || settingsPathChanged) - { - foreach (var scriptFile in _workspaceService.GetOpenedFiles()) - { - _analysisService.ClearMarkers(scriptFile); - } - } - } + ConfigurationUpdated.Invoke(this, _configurationService.CurrentSettings); // Convert the editor file glob patterns into an array for the Workspace // Both the files.exclude and search.exclude hash tables look like (glob-text, is-enabled): @@ -146,5 +122,7 @@ public void SetCapability(DidChangeConfigurationCapability capability) { _capability = capability; } + + public event EventHandler ConfigurationUpdated; } } diff --git a/src/PowerShellEditorServices/Services/Workspace/WorkspaceService.cs b/src/PowerShellEditorServices/Services/Workspace/WorkspaceService.cs index 5da819232..9f2f065df 100644 --- a/src/PowerShellEditorServices/Services/Workspace/WorkspaceService.cs +++ b/src/PowerShellEditorServices/Services/Workspace/WorkspaceService.cs @@ -482,6 +482,11 @@ internal static bool IsPathInMemory(string filePath) return isInMemory; } + internal string ResolveWorkspacePath(string path) + { + return ResolveRelativeScriptPath(WorkspacePath, path); + } + internal string ResolveRelativeScriptPath(string baseFilePath, string relativePath) { string combinedPath = null; From 660e8950c77143229c0e55cd00e37b036cbf13e9 Mon Sep 17 00:00:00 2001 From: Rob Holt Date: Wed, 5 Feb 2020 18:46:15 -0800 Subject: [PATCH 07/33] Move more functionality behind AnalysisService --- .../Server/PsesServiceCollectionExtensions.cs | 9 +- .../Services/Analysis/AnalysisService.cs | 106 ++++++++++++++---- .../Services/Analysis/IAnalysisEngine.cs | 8 ++ .../Services/Analysis/NullAnalysisEngine.cs | 3 + .../Analysis/PssaCmdletAnalysisEngine.cs | 9 +- .../Handlers/GetCommentHelpHandler.cs | 18 +-- .../Handlers/CodeActionHandler.cs | 2 +- .../Handlers/TextDocumentHandler.cs | 4 +- 8 files changed, 106 insertions(+), 53 deletions(-) diff --git a/src/PowerShellEditorServices/Server/PsesServiceCollectionExtensions.cs b/src/PowerShellEditorServices/Server/PsesServiceCollectionExtensions.cs index 7ad24f36b..4754e8722 100644 --- a/src/PowerShellEditorServices/Server/PsesServiceCollectionExtensions.cs +++ b/src/PowerShellEditorServices/Server/PsesServiceCollectionExtensions.cs @@ -43,14 +43,7 @@ public static IServiceCollection AddPsesLanguageServices( .Wait(); return extensionService; }) - .AddSingleton( - (provider) => - { - return AnalysisService.Create( - provider.GetService(), - provider.GetService(), - provider.GetService().CreateLogger()); - }); + .AddSingleton(); } public static IServiceCollection AddPsesDebugServices( diff --git a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs index 0ac8a23a3..a8a810f76 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -6,11 +6,8 @@ using System; using System.Linq; using System.Threading.Tasks; -using System.Management.Automation.Runspaces; -using System.Management.Automation; using System.Collections.Generic; using System.Text; -using System.Collections; using Microsoft.Extensions.Logging; using OmniSharp.Extensions.LanguageServer.Protocol.Models; using OmniSharp.Extensions.LanguageServer.Protocol.Server; @@ -18,11 +15,9 @@ using System.Collections.Concurrent; using Microsoft.PowerShell.EditorServices.Services.TextDocument; using Microsoft.PowerShell.EditorServices.Utility; -using System.Collections.ObjectModel; using Microsoft.PowerShell.EditorServices.Services.Analysis; -using Microsoft.PowerShell.EditorServices.Handlers; -using System.IO; using Microsoft.PowerShell.EditorServices.Services.Configuration; +using System.Collections; namespace Microsoft.PowerShell.EditorServices.Services { @@ -88,24 +83,24 @@ internal static string GetUniqueIdFromDiagnostic(Diagnostic diagnostic) private readonly int _analysisDelayMillis; - private readonly ConcurrentDictionary)> _mostRecentCorrectionsByFile; + private readonly ConcurrentDictionary)> _mostRecentCorrectionsByFile; - private IAnalysisEngine _analysisEngine; + private IAnalysisEngine _analysisEngineField; private CancellationTokenSource _diagnosticsCancellationTokenSource; public AnalysisService( - ILogger logger, + ILoggerFactory loggerFactory, ILanguageServer languageServer, ConfigurationService configurationService, WorkspaceService workspaceService) { - _logger = logger; + _logger = loggerFactory.CreateLogger(); _languageServer = languageServer; _configurationService = configurationService; _workplaceService = workspaceService; _analysisDelayMillis = 750; - _mostRecentCorrectionsByFile = new ConcurrentDictionary)>(); + _mostRecentCorrectionsByFile = new ConcurrentDictionary)>(); } public string[] EnabledRules { get; set; } @@ -116,12 +111,12 @@ private IAnalysisEngine AnalysisEngine { get { - if (_analysisEngine == null) + if (_analysisEngineField == null) { - _analysisEngine = InstantiateAnalysisEngine(); + _analysisEngineField = InstantiateAnalysisEngine(); } - return _analysisEngine; + return _analysisEngineField; } } @@ -129,6 +124,11 @@ public Task RunScriptDiagnosticsAsync( ScriptFile[] filesToAnalyze, CancellationToken cancellationToken) { + if (!AnalysisEngine.IsEnabled) + { + return Task.CompletedTask; + } + // Create a cancellation token source that will cancel if we do or if the caller does var cancellationSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); @@ -155,10 +155,56 @@ public Task RunScriptDiagnosticsAsync( return Task.Run(() => DelayThenInvokeDiagnosticsAsync(filesToAnalyze, _diagnosticsCancellationTokenSource.Token), _diagnosticsCancellationTokenSource.Token); } + public Task FormatAsync(string scriptFileContents, Hashtable formatSettings, int[] formatRange = null) + { + if (!AnalysisEngine.IsEnabled) + { + return null; + } + + return AnalysisEngine.FormatAsync(scriptFileContents, formatSettings, formatRange); + } + + public async Task GetCommentHelpText(string functionText, string helpLocation, bool forBlockComment) + { + if (!AnalysisEngine.IsEnabled) + { + return null; + } + + Hashtable commentHelpSettings = AnalysisService.GetCommentHelpRuleSettings(helpLocation, forBlockComment); + + ScriptFileMarker[] analysisResults = await AnalysisEngine.AnalyzeScriptAsync(functionText, commentHelpSettings); + + if (analysisResults.Length == 0 + || analysisResults[0]?.Correction?.Edits == null + || analysisResults[0].Correction.Edits.Count() == 0) + { + return null; + } + + return analysisResults[0].Correction.Edits[0].Text; + } + + public IReadOnlyDictionary GetMostRecentCodeActionsForFile(string documentUri) + { + if (!_mostRecentCorrectionsByFile.TryGetValue(documentUri, out (SemaphoreSlim fileLock, ConcurrentDictionary corrections) fileCorrectionsEntry)) + { + return null; + } + + return fileCorrectionsEntry.corrections; + } + + public Task ClearMarkers(ScriptFile file) + { + return PublishScriptDiagnosticsAsync(file, Array.Empty()); + } + public void OnConfigurationUpdated(object sender, LanguageServerSettings settings) { - ClearMarkers(); - _analysisEngine = InstantiateAnalysisEngine(); + ClearOpenFileMarkers(); + _analysisEngineField = InstantiateAnalysisEngine(); } private IAnalysisEngine InstantiateAnalysisEngine() @@ -200,11 +246,11 @@ private bool TryFindSettingsFile(out string settingsFilePath) return false; } - private Task ClearMarkers() + private Task ClearOpenFileMarkers() { return Task.WhenAll( _workplaceService.GetOpenedFiles() - .Select(file => PublishScriptDiagnosticsAsync(file, Array.Empty()))); + .Select(file => ClearMarkers(file))); } private async Task DelayThenInvokeDiagnosticsAsync(ScriptFile[] filesToAnalyze, CancellationToken cancellationToken) @@ -238,7 +284,7 @@ private async Task DelayThenInvokeDiagnosticsAsync(ScriptFile[] filesToAnalyze, foreach (ScriptFile scriptFile in filesToAnalyze) { - ScriptFileMarker[] semanticMarkers = await _analysisEngine.AnalyzeScriptAsync(scriptFile.Contents).ConfigureAwait(false); + ScriptFileMarker[] semanticMarkers = await AnalysisEngine.AnalyzeScriptAsync(scriptFile.Contents).ConfigureAwait(false); scriptFile.DiagnosticMarkers.AddRange(semanticMarkers); @@ -250,7 +296,7 @@ private async Task DelayThenInvokeDiagnosticsAsync(ScriptFile[] filesToAnalyze, private async Task PublishScriptDiagnosticsAsync(ScriptFile scriptFile, IReadOnlyList markers) { - (SemaphoreSlim fileLock, Dictionary fileCorrections) = _mostRecentCorrectionsByFile.GetOrAdd( + (SemaphoreSlim fileLock, ConcurrentDictionary fileCorrections) = _mostRecentCorrectionsByFile.GetOrAdd( scriptFile.DocumentUri, CreateFileCorrectionsEntry); @@ -288,9 +334,9 @@ private async Task PublishScriptDiagnosticsAsync(ScriptFile scriptFile, IReadOnl }); } - private static (SemaphoreSlim, Dictionary) CreateFileCorrectionsEntry(string fileUri) + private static (SemaphoreSlim, ConcurrentDictionary) CreateFileCorrectionsEntry(string fileUri) { - return (AsyncUtils.CreateSimpleLockingSemaphore(), new Dictionary()); + return (AsyncUtils.CreateSimpleLockingSemaphore(), new ConcurrentDictionary()); } private static Diagnostic GetDiagnosticFromMarker(ScriptFileMarker scriptFileMarker) @@ -335,6 +381,20 @@ private static DiagnosticSeverity MapDiagnosticSeverity(ScriptFileMarkerLevel ma } } + private static Hashtable GetCommentHelpRuleSettings(string helpLocation, bool forBlockComment) + { + return new Hashtable { + { "Rules", new Hashtable { + { "PSProvideCommentHelp", new Hashtable { + { "Enable", true }, + { "ExportedOnly", false }, + { "BlockComment", forBlockComment }, + { "VSCodeSnippetCorrection", true }, + { "Placement", helpLocation }} + }} + }}; + } + #region IDisposable Support private bool disposedValue = false; // To detect redundant calls @@ -344,7 +404,7 @@ protected virtual void Dispose(bool disposing) { if (disposing) { - _analysisEngine.Dispose(); + _analysisEngineField?.Dispose(); _diagnosticsCancellationTokenSource?.Dispose(); } diff --git a/src/PowerShellEditorServices/Services/Analysis/IAnalysisEngine.cs b/src/PowerShellEditorServices/Services/Analysis/IAnalysisEngine.cs index b2de7b9c7..165d88c07 100644 --- a/src/PowerShellEditorServices/Services/Analysis/IAnalysisEngine.cs +++ b/src/PowerShellEditorServices/Services/Analysis/IAnalysisEngine.cs @@ -39,6 +39,14 @@ Task FormatAsync( /// The script to analyze in string form. /// Markers for any diagnostics in the script. Task AnalyzeScriptAsync(string scriptFileContent); + + /// + /// Analyze a PowerShell script file using a settings object. + /// + /// The script to analyze in string form. + /// A settings object to use as engine settings in this call. + /// Markers for any diagnostics in the script. + Task AnalyzeScriptAsync(string scriptFileContent, Hashtable settings); } } diff --git a/src/PowerShellEditorServices/Services/Analysis/NullAnalysisEngine.cs b/src/PowerShellEditorServices/Services/Analysis/NullAnalysisEngine.cs index 77ab4daa9..5eeb1fefe 100644 --- a/src/PowerShellEditorServices/Services/Analysis/NullAnalysisEngine.cs +++ b/src/PowerShellEditorServices/Services/Analysis/NullAnalysisEngine.cs @@ -24,6 +24,9 @@ public Task AnalyzeScriptAsync(string scriptFileContent) { throw CreateInvocationException(); } + + public Task AnalyzeScriptAsync(string scriptFileContent, Hashtable settings) => AnalyzeScriptAsync(scriptFileContent); + private Exception CreateInvocationException() { return new InvalidOperationException($"{nameof(NullAnalysisEngine)} implements no functionality and its methods should not be called"); diff --git a/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs index f0e808100..f78d0605d 100644 --- a/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs +++ b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs @@ -181,7 +181,9 @@ public async Task FormatAsync(string scriptDefinition, Hashtable formatS return null; } - public Task AnalyzeScriptAsync(string scriptContent) + public Task AnalyzeScriptAsync(string scriptContent) => AnalyzeScriptAsync(scriptContent, settings: null); + + public Task AnalyzeScriptAsync(string scriptContent, Hashtable settings) { // When a new, empty file is created there are by definition no issues. // Furthermore, if you call Invoke-ScriptAnalyzer with an empty ScriptDefinition @@ -196,9 +198,10 @@ public Task AnalyzeScriptAsync(string scriptContent) .AddParameter("ScriptDefinition", scriptContent) .AddParameter("Severity", s_scriptMarkerLevels); - if (_settingsParameter != null) + object settingsValue = settings ?? _settingsParameter; + if (settingsValue != null) { - command.AddParameter("Settings", _settingsParameter); + command.AddParameter("Settings", settingsValue); } else { diff --git a/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/GetCommentHelpHandler.cs b/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/GetCommentHelpHandler.cs index 13f4deac0..77697ebff 100644 --- a/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/GetCommentHelpHandler.cs +++ b/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/GetCommentHelpHandler.cs @@ -14,7 +14,7 @@ namespace Microsoft.PowerShell.EditorServices.Handlers { - public class GetCommentHelpHandler : IGetCommentHelpHandler + internal class GetCommentHelpHandler : IGetCommentHelpHandler { private readonly ILogger _logger; private readonly WorkspaceService _workspaceService; @@ -70,21 +70,7 @@ public async Task Handle(CommentHelpRequestParams requ funcText = string.Join("\n", lines); } - List analysisResults = await _analysisService.GetSemanticMarkersAsync( - funcText, - AnalysisService.GetCommentHelpRuleSettings( - enable: true, - exportedOnly: false, - blockComment: request.BlockComment, - vscodeSnippetCorrection: true, - placement: helpLocation)); - - if (analysisResults == null || analysisResults.Count == 0) - { - return result; - } - - string helpText = analysisResults[0]?.Correction?.Edits[0].Text; + string helpText = await _analysisService.GetCommentHelpText(funcText, helpLocation, forBlockComment: request.BlockComment); if (helpText == null) { diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeActionHandler.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeActionHandler.cs index ec1a8bd2d..6796c7064 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeActionHandler.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeActionHandler.cs @@ -55,7 +55,7 @@ public void SetCapability(CodeActionCapability capability) public async Task Handle(CodeActionParams request, CancellationToken cancellationToken) { - IReadOnlyDictionary corrections = await _analysisService.GetMostRecentCodeActionsForFileAsync(request.TextDocument.Uri.OriginalString).ConfigureAwait(false); + IReadOnlyDictionary corrections = _analysisService.GetMostRecentCodeActionsForFile(request.TextDocument.Uri.OriginalString); if (corrections == null) { diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/TextDocumentHandler.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/TextDocumentHandler.cs index e9f844386..41aac15fb 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/TextDocumentHandler.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/TextDocumentHandler.cs @@ -65,7 +65,7 @@ public Task Handle(DidChangeTextDocumentParams notification, CancellationT #pragma warning disable CS4014 // Kick off script diagnostics without blocking the response // TODO: Get all recently edited files in the workspace - _analysisService.RunScriptDiagnosticsAsync(new ScriptFile[] { changedFile }); + _analysisService.RunScriptDiagnosticsAsync(new ScriptFile[] { changedFile }, token); #pragma warning restore CS4014 return Unit.Task; } @@ -94,7 +94,7 @@ public Task Handle(DidOpenTextDocumentParams notification, CancellationTok #pragma warning disable CS4014 // Kick off script diagnostics without blocking the response // TODO: Get all recently edited files in the workspace - _analysisService.RunScriptDiagnosticsAsync(new ScriptFile[] { openedFile }); + _analysisService.RunScriptDiagnosticsAsync(new ScriptFile[] { openedFile }, token); #pragma warning restore CS4014 _logger.LogTrace("Finished opening document."); From e3dee1adf31ee308bab8287e276b10499f32fa76 Mon Sep 17 00:00:00 2001 From: Rob Holt Date: Wed, 5 Feb 2020 20:08:43 -0800 Subject: [PATCH 08/33] Move help API into AnalysisService --- .../Services/Analysis/AnalysisService.cs | 7 ++-- .../Analysis/PssaCmdletAnalysisEngine.cs | 2 +- .../Handlers/GetCommentHelpHandler.cs | 2 +- .../PowerShellContextService.cs | 32 ++++++++++++------- .../Handlers/CodeActionHandler.cs | 1 + .../LanguageServerProtocolMessageTests.cs | 2 +- 6 files changed, 29 insertions(+), 17 deletions(-) diff --git a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs index a8a810f76..808b742b4 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -18,6 +18,7 @@ using Microsoft.PowerShell.EditorServices.Services.Analysis; using Microsoft.PowerShell.EditorServices.Services.Configuration; using System.Collections; +using System.Diagnostics; namespace Microsoft.PowerShell.EditorServices.Services { @@ -174,7 +175,7 @@ public async Task GetCommentHelpText(string functionText, string helpLoc Hashtable commentHelpSettings = AnalysisService.GetCommentHelpRuleSettings(helpLocation, forBlockComment); - ScriptFileMarker[] analysisResults = await AnalysisEngine.AnalyzeScriptAsync(functionText, commentHelpSettings); + ScriptFileMarker[] analysisResults = await AnalysisEngine.AnalyzeScriptAsync(functionText, commentHelpSettings).ConfigureAwait(false); if (analysisResults.Length == 0 || analysisResults[0]?.Correction?.Edits == null @@ -209,7 +210,7 @@ public void OnConfigurationUpdated(object sender, LanguageServerSettings setting private IAnalysisEngine InstantiateAnalysisEngine() { - if (_configurationService.CurrentSettings.ScriptAnalysis.Enable ?? false) + if (!(_configurationService.CurrentSettings.ScriptAnalysis.Enable ?? false)) { return new NullAnalysisEngine(); } @@ -302,7 +303,7 @@ private async Task PublishScriptDiagnosticsAsync(ScriptFile scriptFile, IReadOnl var diagnostics = new Diagnostic[scriptFile.DiagnosticMarkers.Count]; - await fileLock.WaitAsync(); + await fileLock.WaitAsync().ConfigureAwait(false); try { fileCorrections.Clear(); diff --git a/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs index f78d0605d..190872e3f 100644 --- a/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs +++ b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs @@ -205,7 +205,7 @@ public Task AnalyzeScriptAsync(string scriptContent, Hashtab } else { - command.AddParameter("IncludeRules", _rulesToInclude); + command.AddParameter("IncludeRule", _rulesToInclude); } return GetSemanticMarkersFromCommandAsync(command); diff --git a/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/GetCommentHelpHandler.cs b/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/GetCommentHelpHandler.cs index 77697ebff..c3b3134b4 100644 --- a/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/GetCommentHelpHandler.cs +++ b/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/GetCommentHelpHandler.cs @@ -70,7 +70,7 @@ public async Task Handle(CommentHelpRequestParams requ funcText = string.Join("\n", lines); } - string helpText = await _analysisService.GetCommentHelpText(funcText, helpLocation, forBlockComment: request.BlockComment); + string helpText = await _analysisService.GetCommentHelpText(funcText, helpLocation, forBlockComment: request.BlockComment).ConfigureAwait(false); if (helpText == null) { diff --git a/src/PowerShellEditorServices/Services/PowerShellContext/PowerShellContextService.cs b/src/PowerShellEditorServices/Services/PowerShellContext/PowerShellContextService.cs index bed51685a..063422352 100644 --- a/src/PowerShellEditorServices/Services/PowerShellContext/PowerShellContextService.cs +++ b/src/PowerShellEditorServices/Services/PowerShellContext/PowerShellContextService.cs @@ -1153,20 +1153,30 @@ internal static TResult ExecuteScriptAndGetItem(string scriptToExecute, /// A Task that can be awaited for completion. public async Task LoadHostProfilesAsync() { - if (this.profilePaths != null) + if (this.profilePaths == null) { - // Load any of the profile paths that exist - var command = new PSCommand(); - foreach (var profilePath in GetLoadableProfilePaths(this.profilePaths)) - { - command.AddCommand(profilePath, false).AddStatement(); - } - await ExecuteCommandAsync(command, sendOutputToHost: true).ConfigureAwait(false); + return; + } - // Gather the session details (particularly the prompt) after - // loading the user's profiles. - await this.GetSessionDetailsInRunspaceAsync().ConfigureAwait(false); + // Load any of the profile paths that exist + var command = new PSCommand(); + bool hasLoadablePath = false; + foreach (var profilePath in GetLoadableProfilePaths(this.profilePaths)) + { + hasLoadablePath = true; + command.AddCommand(profilePath, false).AddStatement(); } + + if (!hasLoadablePath) + { + return; + } + + await ExecuteCommandAsync(command, sendOutputToHost: true).ConfigureAwait(false); + + // Gather the session details (particularly the prompt) after + // loading the user's profiles. + await this.GetSessionDetailsInRunspaceAsync().ConfigureAwait(false); } /// diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeActionHandler.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeActionHandler.cs index 6796c7064..63d5edc90 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeActionHandler.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeActionHandler.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.Logging; diff --git a/test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs b/test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs index 1ea7a0aea..93577de31 100644 --- a/test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs +++ b/test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs @@ -35,7 +35,6 @@ public class LanguageServerProtocolMessageTests : IClassFixture public LanguageServerProtocolMessageTests(ITestOutputHelper output, LSPTestsFixture data) { - Diagnostics = new List(); LanguageClient = data.LanguageClient; Diagnostics = data.Diagnostics; PwshExe = TestsFixture.PwshExe; @@ -61,6 +60,7 @@ public void Dispose() private string NewTestFile(string script, bool isPester = false) { + Diagnostics.Clear(); string fileExt = isPester ? ".Tests.ps1" : ".ps1"; string filePath = Path.Combine(s_binDir, Path.GetRandomFileName() + fileExt); File.WriteAllText(filePath, script); From 0a488c289c7ba8828c4ea57372cf046d9963efb4 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Thu, 6 Feb 2020 11:07:16 -0800 Subject: [PATCH 09/33] Add comments --- .../Services/Analysis/AnalysisService.cs | 80 +++++++++++++++---- 1 file changed, 63 insertions(+), 17 deletions(-) diff --git a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs index 808b742b4..1591b048f 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -28,6 +28,11 @@ namespace Microsoft.PowerShell.EditorServices.Services /// internal class AnalysisService : IDisposable { + /// + /// Reliably generate an ID for a diagnostic record to track it. + /// + /// The diagnostic to generate an ID for. + /// A string unique to this diagnostic given where and what kind it is. internal static string GetUniqueIdFromDiagnostic(Diagnostic diagnostic) { Position start = diagnostic.Range.Start; @@ -90,6 +95,13 @@ internal static string GetUniqueIdFromDiagnostic(Diagnostic diagnostic) private CancellationTokenSource _diagnosticsCancellationTokenSource; + /// + /// Construct a new AnalysisService. + /// + /// Logger factory to create logger instances with. + /// The LSP language server for notifications. + /// The configuration service to query for configurations. + /// The workspace service for file handling within a workspace. public AnalysisService( ILoggerFactory loggerFactory, ILanguageServer languageServer, @@ -104,10 +116,9 @@ public AnalysisService( _mostRecentCorrectionsByFile = new ConcurrentDictionary)>(); } - public string[] EnabledRules { get; set; } - - public string SettingsPath { get; set; } - + /// + /// The analysis engine to use for running script analysis. + /// private IAnalysisEngine AnalysisEngine { get @@ -121,6 +132,12 @@ private IAnalysisEngine AnalysisEngine } } + /// + /// Sets up a script analysis run, eventually returning the result. + /// + /// The files to run script analysis on. + /// A cancellation token to cancel this call with. + /// A task that finishes when script diagnostics have been published. public Task RunScriptDiagnosticsAsync( ScriptFile[] filesToAnalyze, CancellationToken cancellationToken) @@ -156,6 +173,13 @@ public Task RunScriptDiagnosticsAsync( return Task.Run(() => DelayThenInvokeDiagnosticsAsync(filesToAnalyze, _diagnosticsCancellationTokenSource.Token), _diagnosticsCancellationTokenSource.Token); } + /// + /// Formats a PowerShell script with the given settings. + /// + /// The script to format. + /// The settings to use with the formatter. + /// Optionally, the range that should be formatted. + /// The text of the formatted PowerShell script. public Task FormatAsync(string scriptFileContents, Hashtable formatSettings, int[] formatRange = null) { if (!AnalysisEngine.IsEnabled) @@ -166,6 +190,13 @@ public Task FormatAsync(string scriptFileContents, Hashtable formatSetti return AnalysisEngine.FormatAsync(scriptFileContents, formatSettings, formatRange); } + /// + /// Get comment help text for a PowerShell function definition. + /// + /// The text of the function to get comment help for. + /// A string referring to which location comment help should be placed around the function. + /// If true, block comment help will be supplied. + /// public async Task GetCommentHelpText(string functionText, string helpLocation, bool forBlockComment) { if (!AnalysisEngine.IsEnabled) @@ -187,6 +218,11 @@ public async Task GetCommentHelpText(string functionText, string helpLoc return analysisResults[0].Correction.Edits[0].Text; } + /// + /// Get the most recent corrections computed for a given script file. + /// + /// The URI string of the file to get code actions for. + /// A threadsafe readonly dictionary of the code actions of the particular file. public IReadOnlyDictionary GetMostRecentCodeActionsForFile(string documentUri) { if (!_mostRecentCorrectionsByFile.TryGetValue(documentUri, out (SemaphoreSlim fileLock, ConcurrentDictionary corrections) fileCorrectionsEntry)) @@ -197,11 +233,21 @@ public IReadOnlyDictionary GetMostRecentCodeActionsFor return fileCorrectionsEntry.corrections; } + /// + /// Clear all diagnostic markers for a given file. + /// + /// The file to clear markers in. + /// A task that ends when all markers in the file have been cleared. public Task ClearMarkers(ScriptFile file) { return PublishScriptDiagnosticsAsync(file, Array.Empty()); } + /// + /// Event subscription method to be run when PSES configuration has been updated. + /// + /// The sender of the configuration update event. + /// The new language server settings. public void OnConfigurationUpdated(object sender, LanguageServerSettings settings) { ClearOpenFileMarkers(); @@ -217,6 +263,7 @@ private IAnalysisEngine InstantiateAnalysisEngine() var pssaCmdletEngineBuilder = new PssaCmdletAnalysisEngine.Builder(_logger); + // If there's a settings file use that if (TryFindSettingsFile(out string settingsFilePath)) { _logger.LogInformation($"Configuring PSScriptAnalyzer with rules at '{settingsFilePath}'"); @@ -238,6 +285,12 @@ private bool TryFindSettingsFile(out string settingsFilePath) if (!string.IsNullOrEmpty(configuredPath)) { settingsFilePath = _workplaceService.ResolveWorkspacePath(configuredPath); + + if (settingsFilePath == null) + { + _logger.LogError($"Unable to find PSSA settings file at '{configuredPath}'. Loading default rules."); + } + return settingsFilePath != null; } @@ -366,20 +419,13 @@ private static Diagnostic GetDiagnosticFromMarker(ScriptFileMarker scriptFileMar private static DiagnosticSeverity MapDiagnosticSeverity(ScriptFileMarkerLevel markerLevel) { - switch (markerLevel) + 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, + }; } private static Hashtable GetCommentHelpRuleSettings(string helpLocation, bool forBlockComment) From 0b10f9ab26d494a915fc1a929cd9445f1c53b563 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Thu, 6 Feb 2020 11:34:40 -0800 Subject: [PATCH 10/33] Simplify design and add more comments --- .../Services/Analysis/AnalysisService.cs | 28 ++++--- .../Services/Analysis/IAnalysisEngine.cs | 52 ------------- .../Services/Analysis/NullAnalysisEngine.cs | 41 ---------- .../Analysis/PssaCmdletAnalysisEngine.cs | 74 ++++++++++++++++--- 4 files changed, 80 insertions(+), 115 deletions(-) delete mode 100644 src/PowerShellEditorServices/Services/Analysis/IAnalysisEngine.cs delete mode 100644 src/PowerShellEditorServices/Services/Analysis/NullAnalysisEngine.cs diff --git a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs index 1591b048f..c011ed819 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -91,7 +91,9 @@ internal static string GetUniqueIdFromDiagnostic(Diagnostic diagnostic) private readonly ConcurrentDictionary)> _mostRecentCorrectionsByFile; - private IAnalysisEngine _analysisEngineField; + private bool _hasInstantiatedAnalysisEngine; + + private PssaCmdletAnalysisEngine _analysisEngine; private CancellationTokenSource _diagnosticsCancellationTokenSource; @@ -114,21 +116,23 @@ public AnalysisService( _workplaceService = workspaceService; _analysisDelayMillis = 750; _mostRecentCorrectionsByFile = new ConcurrentDictionary)>(); + _hasInstantiatedAnalysisEngine = false; } /// /// The analysis engine to use for running script analysis. /// - private IAnalysisEngine AnalysisEngine + private PssaCmdletAnalysisEngine AnalysisEngine { get { - if (_analysisEngineField == null) + if (!_hasInstantiatedAnalysisEngine) { - _analysisEngineField = InstantiateAnalysisEngine(); + _analysisEngine = InstantiateAnalysisEngine(); + _hasInstantiatedAnalysisEngine = true; } - return _analysisEngineField; + return _analysisEngine; } } @@ -142,7 +146,7 @@ public Task RunScriptDiagnosticsAsync( ScriptFile[] filesToAnalyze, CancellationToken cancellationToken) { - if (!AnalysisEngine.IsEnabled) + if (AnalysisEngine == null) { return Task.CompletedTask; } @@ -182,7 +186,7 @@ public Task RunScriptDiagnosticsAsync( /// The text of the formatted PowerShell script. public Task FormatAsync(string scriptFileContents, Hashtable formatSettings, int[] formatRange = null) { - if (!AnalysisEngine.IsEnabled) + if (AnalysisEngine == null) { return null; } @@ -199,7 +203,7 @@ public Task FormatAsync(string scriptFileContents, Hashtable formatSetti /// public async Task GetCommentHelpText(string functionText, string helpLocation, bool forBlockComment) { - if (!AnalysisEngine.IsEnabled) + if (AnalysisEngine == null) { return null; } @@ -251,14 +255,14 @@ public Task ClearMarkers(ScriptFile file) public void OnConfigurationUpdated(object sender, LanguageServerSettings settings) { ClearOpenFileMarkers(); - _analysisEngineField = InstantiateAnalysisEngine(); + _analysisEngine = InstantiateAnalysisEngine(); } - private IAnalysisEngine InstantiateAnalysisEngine() + private PssaCmdletAnalysisEngine InstantiateAnalysisEngine() { if (!(_configurationService.CurrentSettings.ScriptAnalysis.Enable ?? false)) { - return new NullAnalysisEngine(); + return null; } var pssaCmdletEngineBuilder = new PssaCmdletAnalysisEngine.Builder(_logger); @@ -451,7 +455,7 @@ protected virtual void Dispose(bool disposing) { if (disposing) { - _analysisEngineField?.Dispose(); + _analysisEngine?.Dispose(); _diagnosticsCancellationTokenSource?.Dispose(); } diff --git a/src/PowerShellEditorServices/Services/Analysis/IAnalysisEngine.cs b/src/PowerShellEditorServices/Services/Analysis/IAnalysisEngine.cs deleted file mode 100644 index 165d88c07..000000000 --- a/src/PowerShellEditorServices/Services/Analysis/IAnalysisEngine.cs +++ /dev/null @@ -1,52 +0,0 @@ -// -// Copyright (c) Microsoft. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. -// - -using Microsoft.PowerShell.EditorServices.Services.TextDocument; -using System; -using System.Collections; -using System.Collections.Generic; -using System.Threading.Tasks; - -namespace Microsoft.PowerShell.EditorServices.Services.Analysis -{ - /// - /// An engine to run PowerShell script analysis. - /// - internal interface IAnalysisEngine : IDisposable - { - /// - /// If false, other methods on this object will have no meaningful implementation. - /// - bool IsEnabled { get; } - - /// - /// Format a PowerShell script. - /// - /// The string of the PowerShell script to format. - /// The settings to apply when formatting. - /// An optional list of ranges to format in the script. - /// - Task FormatAsync( - string scriptDefinition, - Hashtable formatSettings, - int[] rangeList); - - /// - /// Analyze a PowerShell script file using a settings object. - /// - /// The script to analyze in string form. - /// Markers for any diagnostics in the script. - Task AnalyzeScriptAsync(string scriptFileContent); - - /// - /// Analyze a PowerShell script file using a settings object. - /// - /// The script to analyze in string form. - /// A settings object to use as engine settings in this call. - /// Markers for any diagnostics in the script. - Task AnalyzeScriptAsync(string scriptFileContent, Hashtable settings); - } - -} diff --git a/src/PowerShellEditorServices/Services/Analysis/NullAnalysisEngine.cs b/src/PowerShellEditorServices/Services/Analysis/NullAnalysisEngine.cs deleted file mode 100644 index 5eeb1fefe..000000000 --- a/src/PowerShellEditorServices/Services/Analysis/NullAnalysisEngine.cs +++ /dev/null @@ -1,41 +0,0 @@ -// -// Copyright (c) Microsoft. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. -// - -using Microsoft.PowerShell.EditorServices.Services.TextDocument; -using System; -using System.Collections; -using System.Collections.Generic; -using System.Threading.Tasks; - -namespace Microsoft.PowerShell.EditorServices.Services.Analysis -{ - internal class NullAnalysisEngine : IAnalysisEngine - { - public bool IsEnabled => false; - - public Task FormatAsync(string scriptDefinition, Hashtable formatSettings, int[] rangeList) - { - throw CreateInvocationException(); - } - - public Task AnalyzeScriptAsync(string scriptFileContent) - { - throw CreateInvocationException(); - } - - public Task AnalyzeScriptAsync(string scriptFileContent, Hashtable settings) => AnalyzeScriptAsync(scriptFileContent); - - private Exception CreateInvocationException() - { - return new InvalidOperationException($"{nameof(NullAnalysisEngine)} implements no functionality and its methods should not be called"); - } - - public void Dispose() - { - // Nothing to dispose - } - } - -} diff --git a/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs index 190872e3f..0cdd43014 100644 --- a/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs +++ b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs @@ -12,7 +12,6 @@ using System.IO; using System.Linq; using System.Management.Automation; -using System.Management.Automation.Language; using System.Management.Automation.Runspaces; using System.Text; using System.Threading; @@ -20,57 +19,89 @@ namespace Microsoft.PowerShell.EditorServices.Services.Analysis { - internal class PssaCmdletAnalysisEngine : IAnalysisEngine + /// + /// PowerShell script analysis engine that uses PSScriptAnalyzer + /// cmdlets run through a PowerShell API to drive analysis. + /// + internal class PssaCmdletAnalysisEngine { + /// + /// Builder for the PssaCmdletAnalysisEngine allowing settings configuration. + /// public class Builder { - private readonly ILogger _logger; + private readonly ILoggerFactory _loggerFactory; private object _settingsParameter; private string[] _rules; - public Builder(ILogger logger) + /// + /// Create a builder for PssaCmdletAnalysisEngine construction. + /// + /// The logger to use. + public Builder(ILoggerFactory loggerFactory) { - _logger = logger; + _loggerFactory = loggerFactory; } + /// + /// Uses a settings file for PSSA rule configuration. + /// + /// The absolute path to the settings file. + /// The builder for chaining. public Builder WithSettingsFile(string settingsPath) { _settingsParameter = settingsPath; return this; } + /// + /// Uses a settings hashtable for PSSA rule configuration. + /// + /// The settings hashtable to pass to PSSA. + /// The builder for chaining. public Builder WithSettings(Hashtable settings) { _settingsParameter = settings; return this; } + /// + /// Uses a set of unconfigured rules for PSSA configuration. + /// + /// The rules for PSSA to run. + /// The builder for chaining. public Builder WithIncludedRules(string[] rules) { _rules = rules; return this; } + /// + /// Attempts to build a PssaCmdletAnalysisEngine with the given configuration. + /// If PSScriptAnalyzer cannot be found, this will return null. + /// + /// A newly configured PssaCmdletAnalysisEngine, or null if PSScriptAnalyzer cannot be found. public PssaCmdletAnalysisEngine Build() { // RunspacePool takes care of queuing commands for us so we do not // need to worry about executing concurrent commands + ILogger logger = _loggerFactory.CreateLogger(); try { RunspacePool pssaRunspacePool = CreatePssaRunspacePool(out PSModuleInfo pssaModuleInfo); PssaCmdletAnalysisEngine cmdletAnalysisEngine = _settingsParameter != null - ? new PssaCmdletAnalysisEngine(_logger, pssaRunspacePool, pssaModuleInfo, _settingsParameter) - : new PssaCmdletAnalysisEngine(_logger, pssaRunspacePool, pssaModuleInfo, _rules); + ? new PssaCmdletAnalysisEngine(logger, pssaRunspacePool, pssaModuleInfo, _settingsParameter) + : new PssaCmdletAnalysisEngine(logger, pssaRunspacePool, pssaModuleInfo, _rules); cmdletAnalysisEngine.LogAvailablePssaFeatures(); return cmdletAnalysisEngine; } catch (FileNotFoundException e) { - _logger.LogError(e, "Unable to find PSScriptAnalyzer. Disabling script analysis."); + logger.LogError(e, $"Unable to find PSScriptAnalyzer. Disabling script analysis. PSModulePath: '{Environment.GetEnvironmentVariable("PSModulePath")}'"); return null; } } @@ -130,8 +161,13 @@ private PssaCmdletAnalysisEngine( _alreadyRun = false; } - public bool IsEnabled => true; - + /// + /// Format a script given its contents. + /// + /// The full text of a script. + /// The formatter settings to use. + /// A possible range over which to run the formatter. + /// public async Task FormatAsync(string scriptDefinition, Hashtable formatSettings, int[] rangeList) { // We cannot use Range type therefore this workaround of using -1 default value. @@ -181,8 +217,20 @@ public async Task FormatAsync(string scriptDefinition, Hashtable formatS return null; } + /// + /// Analyze a given script using PSScriptAnalyzer. + /// + /// The contents of the script to analyze. + /// An array of markers indicating script analysis diagnostics. public Task AnalyzeScriptAsync(string scriptContent) => AnalyzeScriptAsync(scriptContent, settings: null); + + /// + /// Analyze a given script using PSScriptAnalyzer. + /// + /// The contents of the script to analyze. + /// The settings file to use in this instance of analysis. + /// An array of markers indicating script analysis diagnostics. public Task AnalyzeScriptAsync(string scriptContent, Hashtable settings) { // When a new, empty file is created there are by definition no issues. @@ -291,6 +339,12 @@ private PowerShellResult InvokePowerShell(PSCommand command) } } + /// + /// Execute PSScriptAnalyzer cmdlets in PowerShell while preserving the PSModulePath. + /// Attempts to prevent PSModulePath mutation by runspace creation within the PSScriptAnalyzer module. + /// + /// The PowerShell instance to execute. + /// The output of PowerShell execution. private Collection InvokePowerShellWithModulePathPreservation(System.Management.Automation.PowerShell powershell) { if (_alreadyRun) From 03fdc3c511d7066be9d40125e669edc2c75c7933 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Thu, 6 Feb 2020 11:50:07 -0800 Subject: [PATCH 11/33] Fix logger instantiation --- .../Services/Analysis/AnalysisService.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs index c011ed819..8bd2fdba2 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -79,6 +79,8 @@ internal static string GetUniqueIdFromDiagnostic(Diagnostic diagnostic) "PSPossibleIncorrectUsageOfRedirectionOperator" }; + private readonly ILoggerFactory _loggerFactory; + private readonly ILogger _logger; private readonly ILanguageServer _languageServer; @@ -110,6 +112,7 @@ public AnalysisService( ConfigurationService configurationService, WorkspaceService workspaceService) { + _loggerFactory = loggerFactory; _logger = loggerFactory.CreateLogger(); _languageServer = languageServer; _configurationService = configurationService; @@ -265,7 +268,7 @@ private PssaCmdletAnalysisEngine InstantiateAnalysisEngine() return null; } - var pssaCmdletEngineBuilder = new PssaCmdletAnalysisEngine.Builder(_logger); + var pssaCmdletEngineBuilder = new PssaCmdletAnalysisEngine.Builder(_loggerFactory); // If there's a settings file use that if (TryFindSettingsFile(out string settingsFilePath)) From da2491cb57b0f712dd84cb50a110c1690a6f5399 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Thu, 6 Feb 2020 11:52:32 -0800 Subject: [PATCH 12/33] Undo new switch syntax --- .../Services/Analysis/AnalysisService.cs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs index 8bd2fdba2..0dbf8d44e 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -112,6 +112,7 @@ public AnalysisService( ConfigurationService configurationService, WorkspaceService workspaceService) { + Debugger.Launch(); _loggerFactory = loggerFactory; _logger = loggerFactory.CreateLogger(); _languageServer = languageServer; @@ -426,12 +427,12 @@ private static Diagnostic GetDiagnosticFromMarker(ScriptFileMarker scriptFileMar private static DiagnosticSeverity MapDiagnosticSeverity(ScriptFileMarkerLevel markerLevel) { - return markerLevel switch + switch (markerLevel) { - ScriptFileMarkerLevel.Error => DiagnosticSeverity.Error, - ScriptFileMarkerLevel.Warning => DiagnosticSeverity.Warning, - ScriptFileMarkerLevel.Information => DiagnosticSeverity.Information, - _ => DiagnosticSeverity.Error, + case ScriptFileMarkerLevel.Error: return DiagnosticSeverity.Error; + case ScriptFileMarkerLevel.Warning: return DiagnosticSeverity.Warning; + case ScriptFileMarkerLevel.Information: return DiagnosticSeverity.Information; + default: return DiagnosticSeverity.Error; }; } From 5154ed7a8d72d0dcb7acc0b59071fef2eab8ec07 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Thu, 6 Feb 2020 14:11:53 -0800 Subject: [PATCH 13/33] Fix code action out-of-order issue --- .../Services/Analysis/AnalysisService.cs | 75 ++++++++----------- .../Handlers/CodeActionHandler.cs | 8 +- .../Handlers/TextDocumentHandler.cs | 12 +-- .../LanguageServerProtocolMessageTests.cs | 3 +- 4 files changed, 42 insertions(+), 56 deletions(-) diff --git a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs index 0dbf8d44e..ee709c907 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -91,7 +91,7 @@ internal static string GetUniqueIdFromDiagnostic(Diagnostic diagnostic) private readonly int _analysisDelayMillis; - private readonly ConcurrentDictionary)> _mostRecentCorrectionsByFile; + private readonly ConcurrentDictionary> _mostRecentCorrectionsByFile; private bool _hasInstantiatedAnalysisEngine; @@ -112,14 +112,13 @@ public AnalysisService( ConfigurationService configurationService, WorkspaceService workspaceService) { - Debugger.Launch(); _loggerFactory = loggerFactory; _logger = loggerFactory.CreateLogger(); _languageServer = languageServer; _configurationService = configurationService; _workplaceService = workspaceService; _analysisDelayMillis = 750; - _mostRecentCorrectionsByFile = new ConcurrentDictionary)>(); + _mostRecentCorrectionsByFile = new ConcurrentDictionary>(); _hasInstantiatedAnalysisEngine = false; } @@ -233,12 +232,12 @@ public async Task GetCommentHelpText(string functionText, string helpLoc /// A threadsafe readonly dictionary of the code actions of the particular file. public IReadOnlyDictionary GetMostRecentCodeActionsForFile(string documentUri) { - if (!_mostRecentCorrectionsByFile.TryGetValue(documentUri, out (SemaphoreSlim fileLock, ConcurrentDictionary corrections) fileCorrectionsEntry)) + if (!_mostRecentCorrectionsByFile.TryGetValue(documentUri, out ConcurrentDictionary corrections)) { return null; } - return fileCorrectionsEntry.corrections; + return corrections; } /// @@ -246,9 +245,9 @@ public IReadOnlyDictionary GetMostRecentCodeActionsFor /// /// The file to clear markers in. /// A task that ends when all markers in the file have been cleared. - public Task ClearMarkers(ScriptFile file) + public void ClearMarkers(ScriptFile file) { - return PublishScriptDiagnosticsAsync(file, Array.Empty()); + PublishScriptDiagnostics(file, Array.Empty()); } /// @@ -308,11 +307,12 @@ private bool TryFindSettingsFile(out string settingsFilePath) return false; } - private Task ClearOpenFileMarkers() + private void ClearOpenFileMarkers() { - return Task.WhenAll( - _workplaceService.GetOpenedFiles() - .Select(file => ClearMarkers(file))); + foreach (ScriptFile file in _workplaceService.GetOpenedFiles()) + { + ClearMarkers(file); + } } private async Task DelayThenInvokeDiagnosticsAsync(ScriptFile[] filesToAnalyze, CancellationToken cancellationToken) @@ -329,10 +329,10 @@ private async Task DelayThenInvokeDiagnosticsAsync(ScriptFile[] filesToAnalyze, catch (TaskCanceledException) { // Ensure no stale markers are displayed - foreach (ScriptFile script in filesToAnalyze) - { - await PublishScriptDiagnosticsAsync(script).ConfigureAwait(false); - } + //foreach (ScriptFile script in filesToAnalyze) + //{ + // PublishScriptDiagnosticsAsync(script); + //} return; } @@ -350,43 +350,35 @@ private async Task DelayThenInvokeDiagnosticsAsync(ScriptFile[] filesToAnalyze, scriptFile.DiagnosticMarkers.AddRange(semanticMarkers); - await PublishScriptDiagnosticsAsync(scriptFile).ConfigureAwait(false); + PublishScriptDiagnostics(scriptFile); } } - private Task PublishScriptDiagnosticsAsync(ScriptFile scriptFile) => PublishScriptDiagnosticsAsync(scriptFile, scriptFile.DiagnosticMarkers); + private void PublishScriptDiagnostics(ScriptFile scriptFile) => PublishScriptDiagnostics(scriptFile, scriptFile.DiagnosticMarkers); - private async Task PublishScriptDiagnosticsAsync(ScriptFile scriptFile, IReadOnlyList markers) + private void PublishScriptDiagnostics(ScriptFile scriptFile, IReadOnlyList markers) { - (SemaphoreSlim fileLock, ConcurrentDictionary fileCorrections) = _mostRecentCorrectionsByFile.GetOrAdd( + var diagnostics = new Diagnostic[scriptFile.DiagnosticMarkers.Count]; + + ConcurrentDictionary fileCorrections = _mostRecentCorrectionsByFile.GetOrAdd( scriptFile.DocumentUri, CreateFileCorrectionsEntry); - var diagnostics = new Diagnostic[scriptFile.DiagnosticMarkers.Count]; + fileCorrections.Clear(); - await fileLock.WaitAsync().ConfigureAwait(false); - try + for (int i = 0; i < markers.Count; i++) { - fileCorrections.Clear(); - - for (int i = 0; i < markers.Count; i++) - { - ScriptFileMarker marker = markers[i]; + ScriptFileMarker marker = markers[i]; - Diagnostic diagnostic = GetDiagnosticFromMarker(marker); + Diagnostic diagnostic = GetDiagnosticFromMarker(marker); - if (marker.Correction != null) - { - string diagnosticId = GetUniqueIdFromDiagnostic(diagnostic); - fileCorrections[diagnosticId] = marker.Correction; - } - - diagnostics[i] = diagnostic; + if (marker.Correction != null) + { + string diagnosticId = GetUniqueIdFromDiagnostic(diagnostic); + fileCorrections[diagnosticId] = marker.Correction; } - } - finally - { - fileLock.Release(); + + diagnostics[i] = diagnostic; } _languageServer.Document.PublishDiagnostics(new PublishDiagnosticsParams @@ -396,9 +388,9 @@ private async Task PublishScriptDiagnosticsAsync(ScriptFile scriptFile, IReadOnl }); } - private static (SemaphoreSlim, ConcurrentDictionary) CreateFileCorrectionsEntry(string fileUri) + private static ConcurrentDictionary CreateFileCorrectionsEntry(string fileUri) { - return (AsyncUtils.CreateSimpleLockingSemaphore(), new ConcurrentDictionary()); + return new ConcurrentDictionary(); } private static Diagnostic GetDiagnosticFromMarker(ScriptFileMarker scriptFileMarker) @@ -474,6 +466,5 @@ public void Dispose() Dispose(true); } #endregion - } } diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeActionHandler.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeActionHandler.cs index 63d5edc90..f424057b7 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeActionHandler.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeActionHandler.cs @@ -69,12 +69,7 @@ public async Task Handle(CodeActionParams request, // If there are any code fixes, send these commands first so they appear at top of "Code Fix" menu in the client UI. foreach (Diagnostic diagnostic in request.Context.Diagnostics) { - if (diagnostic.Code.IsLong) - { - _logger.LogWarning( - $"textDocument/codeAction skipping diagnostic with non-string code {diagnostic.Code.Long}: {diagnostic.Source} {diagnostic.Message}"); - } - else if (string.IsNullOrEmpty(diagnostic.Code.String)) + if (string.IsNullOrEmpty(diagnostic.Code.String)) { _logger.LogWarning( $"textDocument/codeAction skipping diagnostic with empty Code field: {diagnostic.Source} {diagnostic.Message}"); @@ -82,7 +77,6 @@ public async Task Handle(CodeActionParams request, continue; } - string diagnosticId = AnalysisService.GetUniqueIdFromDiagnostic(diagnostic); if (corrections.TryGetValue(diagnosticId, out MarkerCorrection correction)) { diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/TextDocumentHandler.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/TextDocumentHandler.cs index 41aac15fb..b993b8107 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/TextDocumentHandler.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/TextDocumentHandler.cs @@ -49,7 +49,7 @@ public TextDocumentHandler( _remoteFileManagerService = remoteFileManagerService; } - public Task Handle(DidChangeTextDocumentParams notification, CancellationToken token) + public async Task Handle(DidChangeTextDocumentParams notification, CancellationToken token) { ScriptFile changedFile = _workspaceService.GetFile(notification.TextDocument.Uri); @@ -65,9 +65,9 @@ public Task Handle(DidChangeTextDocumentParams notification, CancellationT #pragma warning disable CS4014 // Kick off script diagnostics without blocking the response // TODO: Get all recently edited files in the workspace - _analysisService.RunScriptDiagnosticsAsync(new ScriptFile[] { changedFile }, token); + await _analysisService.RunScriptDiagnosticsAsync(new ScriptFile[] { changedFile }, token); #pragma warning restore CS4014 - return Unit.Task; + return Unit.Value; } TextDocumentChangeRegistrationOptions IRegistration.GetRegistrationOptions() @@ -84,7 +84,7 @@ public void SetCapability(SynchronizationCapability capability) _capability = capability; } - public Task Handle(DidOpenTextDocumentParams notification, CancellationToken token) + public async Task Handle(DidOpenTextDocumentParams notification, CancellationToken token) { ScriptFile openedFile = _workspaceService.GetFileBuffer( @@ -94,11 +94,11 @@ public Task Handle(DidOpenTextDocumentParams notification, CancellationTok #pragma warning disable CS4014 // Kick off script diagnostics without blocking the response // TODO: Get all recently edited files in the workspace - _analysisService.RunScriptDiagnosticsAsync(new ScriptFile[] { openedFile }, token); + await _analysisService.RunScriptDiagnosticsAsync(new ScriptFile[] { openedFile }, token); #pragma warning restore CS4014 _logger.LogTrace("Finished opening document."); - return Unit.Task; + return Unit.Value; } TextDocumentRegistrationOptions IRegistration.GetRegistrationOptions() diff --git a/test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs b/test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs index 93577de31..d36139417 100644 --- a/test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs +++ b/test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs @@ -60,7 +60,6 @@ public void Dispose() private string NewTestFile(string script, bool isPester = false) { - Diagnostics.Clear(); string fileExt = isPester ? ".Tests.ps1" : ".ps1"; string filePath = Path.Combine(s_binDir, Path.GetRandomFileName() + fileExt); File.WriteAllText(filePath, script); @@ -636,6 +635,8 @@ function CanSendReferencesCodeLensRequest { [Fact] public async Task CanSendCodeActionRequest() { + Debugger.Launch(); + Diagnostics.Clear(); string filePath = NewTestFile("gci"); await WaitForDiagnostics(); From 062ca7508f0f042ca4f3ee5a2f7b5afe0efa0605 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Thu, 6 Feb 2020 16:52:29 -0800 Subject: [PATCH 14/33] Fix codeAction issue --- .../Services/Analysis/AnalysisService.cs | 68 +++++++++++++++---- .../Handlers/CodeActionHandler.cs | 3 +- .../Handlers/TextDocumentHandler.cs | 12 ++-- .../Services/TextDocument/ScriptFileMarker.cs | 2 +- .../Services/Workspace/WorkspaceService.cs | 2 +- .../LanguageServerProtocolMessageTests.cs | 1 - 6 files changed, 65 insertions(+), 23 deletions(-) diff --git a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs index ee709c907..ed2427721 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -19,6 +19,7 @@ using Microsoft.PowerShell.EditorServices.Services.Configuration; using System.Collections; using System.Diagnostics; +using System.Runtime.InteropServices; namespace Microsoft.PowerShell.EditorServices.Services { @@ -91,7 +92,7 @@ internal static string GetUniqueIdFromDiagnostic(Diagnostic diagnostic) private readonly int _analysisDelayMillis; - private readonly ConcurrentDictionary> _mostRecentCorrectionsByFile; + private readonly ConcurrentDictionary _mostRecentCorrectionsByFile; private bool _hasInstantiatedAnalysisEngine; @@ -118,7 +119,7 @@ public AnalysisService( _configurationService = configurationService; _workplaceService = workspaceService; _analysisDelayMillis = 750; - _mostRecentCorrectionsByFile = new ConcurrentDictionary>(); + _mostRecentCorrectionsByFile = new ConcurrentDictionary(); _hasInstantiatedAnalysisEngine = false; } @@ -145,13 +146,13 @@ private PssaCmdletAnalysisEngine AnalysisEngine /// The files to run script analysis on. /// A cancellation token to cancel this call with. /// A task that finishes when script diagnostics have been published. - public Task RunScriptDiagnosticsAsync( + public void RunScriptDiagnostics( ScriptFile[] filesToAnalyze, CancellationToken cancellationToken) { if (AnalysisEngine == null) { - return Task.CompletedTask; + return; } // Create a cancellation token source that will cancel if we do or if the caller does @@ -174,10 +175,20 @@ public Task RunScriptDiagnosticsAsync( if (filesToAnalyze.Length == 0) { - return Task.CompletedTask; + return; } - return Task.Run(() => DelayThenInvokeDiagnosticsAsync(filesToAnalyze, _diagnosticsCancellationTokenSource.Token), _diagnosticsCancellationTokenSource.Token); + var analysisTask = Task.Run(() => DelayThenInvokeDiagnosticsAsync(filesToAnalyze, _diagnosticsCancellationTokenSource.Token), _diagnosticsCancellationTokenSource.Token); + + // Ensure that any next corrections request will wait for this diagnostics publication + foreach (ScriptFile file in filesToAnalyze) + { + CorrectionTableEntry fileCorrectionsEntry = _mostRecentCorrectionsByFile.GetOrAdd( + file.DocumentUri, + CorrectionTableEntry.CreateForFile); + + fileCorrectionsEntry.DiagnosticPublish = analysisTask; + } } /// @@ -230,14 +241,27 @@ public async Task GetCommentHelpText(string functionText, string helpLoc /// /// The URI string of the file to get code actions for. /// A threadsafe readonly dictionary of the code actions of the particular file. - public IReadOnlyDictionary GetMostRecentCodeActionsForFile(string documentUri) + public async Task> GetMostRecentCodeActionsForFileAsync(string documentUri) { - if (!_mostRecentCorrectionsByFile.TryGetValue(documentUri, out ConcurrentDictionary corrections)) + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + var uri = new Uri(documentUri); + if (uri.Scheme == "file") + { + documentUri = WorkspaceService.UnescapeDriveColon(uri).AbsoluteUri; + } + } + + + if (!_mostRecentCorrectionsByFile.TryGetValue(documentUri, out CorrectionTableEntry corrections)) { return null; } - return corrections; + // Wait for diagnostics to be published for this file + await corrections.DiagnosticPublish.ConfigureAwait(false); + + return corrections.Corrections; } /// @@ -360,11 +384,11 @@ private void PublishScriptDiagnostics(ScriptFile scriptFile, IReadOnlyList fileCorrections = _mostRecentCorrectionsByFile.GetOrAdd( + CorrectionTableEntry fileCorrections = _mostRecentCorrectionsByFile.GetOrAdd( scriptFile.DocumentUri, - CreateFileCorrectionsEntry); + CorrectionTableEntry.CreateForFile); - fileCorrections.Clear(); + fileCorrections.Corrections.Clear(); for (int i = 0; i < markers.Count; i++) { @@ -375,7 +399,7 @@ private void PublishScriptDiagnostics(ScriptFile scriptFile, IReadOnlyList(); + DiagnosticPublish = Task.CompletedTask; + } + + public ConcurrentDictionary Corrections { get; } + + public Task DiagnosticPublish { get; set; } + } } } diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeActionHandler.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeActionHandler.cs index f424057b7..10bd37958 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeActionHandler.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeActionHandler.cs @@ -6,6 +6,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.Logging; @@ -56,7 +57,7 @@ public void SetCapability(CodeActionCapability capability) public async Task Handle(CodeActionParams request, CancellationToken cancellationToken) { - IReadOnlyDictionary corrections = _analysisService.GetMostRecentCodeActionsForFile(request.TextDocument.Uri.OriginalString); + IReadOnlyDictionary corrections = await _analysisService.GetMostRecentCodeActionsForFileAsync(request.TextDocument.Uri.OriginalString); if (corrections == null) { diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/TextDocumentHandler.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/TextDocumentHandler.cs index b993b8107..afe38345a 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/TextDocumentHandler.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/TextDocumentHandler.cs @@ -49,7 +49,7 @@ public TextDocumentHandler( _remoteFileManagerService = remoteFileManagerService; } - public async Task Handle(DidChangeTextDocumentParams notification, CancellationToken token) + public Task Handle(DidChangeTextDocumentParams notification, CancellationToken token) { ScriptFile changedFile = _workspaceService.GetFile(notification.TextDocument.Uri); @@ -65,9 +65,9 @@ public async Task Handle(DidChangeTextDocumentParams notification, Cancell #pragma warning disable CS4014 // Kick off script diagnostics without blocking the response // TODO: Get all recently edited files in the workspace - await _analysisService.RunScriptDiagnosticsAsync(new ScriptFile[] { changedFile }, token); + _analysisService.RunScriptDiagnostics(new ScriptFile[] { changedFile }, token); #pragma warning restore CS4014 - return Unit.Value; + return Unit.Task; } TextDocumentChangeRegistrationOptions IRegistration.GetRegistrationOptions() @@ -84,7 +84,7 @@ public void SetCapability(SynchronizationCapability capability) _capability = capability; } - public async Task Handle(DidOpenTextDocumentParams notification, CancellationToken token) + public Task Handle(DidOpenTextDocumentParams notification, CancellationToken token) { ScriptFile openedFile = _workspaceService.GetFileBuffer( @@ -94,11 +94,11 @@ public async Task Handle(DidOpenTextDocumentParams notification, Cancellat #pragma warning disable CS4014 // Kick off script diagnostics without blocking the response // TODO: Get all recently edited files in the workspace - await _analysisService.RunScriptDiagnosticsAsync(new ScriptFile[] { openedFile }, token); + _analysisService.RunScriptDiagnostics(new ScriptFile[] { openedFile }, token); #pragma warning restore CS4014 _logger.LogTrace("Finished opening document."); - return Unit.Value; + return Unit.Task; } TextDocumentRegistrationOptions IRegistration.GetRegistrationOptions() diff --git a/src/PowerShellEditorServices/Services/TextDocument/ScriptFileMarker.cs b/src/PowerShellEditorServices/Services/TextDocument/ScriptFileMarker.cs index 9ab80cb0b..ddbf8a8c1 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/ScriptFileMarker.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/ScriptFileMarker.cs @@ -158,7 +158,7 @@ internal static ScriptFileMarker FromDiagnosticRecord(PSObject psObject) correction = new MarkerCorrection { - Name = correctionMessage == null ? diagnosticRecord.Message : correctionMessage, + Name = correctionMessage ?? diagnosticRecord.Message, Edits = editRegions.ToArray() }; } diff --git a/src/PowerShellEditorServices/Services/Workspace/WorkspaceService.cs b/src/PowerShellEditorServices/Services/Workspace/WorkspaceService.cs index 9f2f065df..2da7881f2 100644 --- a/src/PowerShellEditorServices/Services/Workspace/WorkspaceService.cs +++ b/src/PowerShellEditorServices/Services/Workspace/WorkspaceService.cs @@ -535,7 +535,7 @@ internal string ResolveRelativeScriptPath(string baseFilePath, string relativePa return combinedPath; } - private static Uri UnescapeDriveColon(Uri fileUri) + internal static Uri UnescapeDriveColon(Uri fileUri) { if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { diff --git a/test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs b/test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs index d36139417..9ddae1eb0 100644 --- a/test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs +++ b/test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs @@ -635,7 +635,6 @@ function CanSendReferencesCodeLensRequest { [Fact] public async Task CanSendCodeActionRequest() { - Debugger.Launch(); Diagnostics.Clear(); string filePath = NewTestFile("gci"); await WaitForDiagnostics(); From 983b3bb2587022241fa0c399351fa4303274a908 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Thu, 6 Feb 2020 18:34:38 -0800 Subject: [PATCH 15/33] Add comment about URI fix --- .../Services/Analysis/AnalysisService.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs index ed2427721..71be38130 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -243,6 +243,7 @@ public async Task GetCommentHelpText(string functionText, string helpLoc /// A threadsafe readonly dictionary of the code actions of the particular file. public async Task> GetMostRecentCodeActionsForFileAsync(string documentUri) { + // On Windows, VSCode still gives us file URIs like "file:///c%3a/...", so we need to escape them if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { var uri = new Uri(documentUri); From c9edb7a42bf3163ab4a642d59d619b8e11ca8732 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Thu, 6 Feb 2020 18:34:48 -0800 Subject: [PATCH 16/33] Remove null diagnostics from test --- test/PowerShellEditorServices.Test.E2E/LSPTestsFixures.cs | 6 ++++-- .../LanguageServerProtocolMessageTests.cs | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/test/PowerShellEditorServices.Test.E2E/LSPTestsFixures.cs b/test/PowerShellEditorServices.Test.E2E/LSPTestsFixures.cs index fcf10bee3..c82c375c3 100644 --- a/test/PowerShellEditorServices.Test.E2E/LSPTestsFixures.cs +++ b/test/PowerShellEditorServices.Test.E2E/LSPTestsFixures.cs @@ -1,5 +1,7 @@ -using System.Collections.Generic; +using System.Collections.Concurrent; +using System.Collections.Generic; using System.IO; +using System.Linq; using System.Threading.Tasks; using Microsoft.Extensions.Logging; using Newtonsoft.Json.Linq; @@ -41,7 +43,7 @@ public async override Task CustomInitializeAsync( Diagnostics = new List(); LanguageClient.TextDocument.OnPublishDiagnostics((uri, diagnostics) => { - Diagnostics.AddRange(diagnostics); + Diagnostics.AddRange(diagnostics.Where(d => d != null)); }); } diff --git a/test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs b/test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs index 9ddae1eb0..d9e9f735d 100644 --- a/test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs +++ b/test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs @@ -37,9 +37,10 @@ public LanguageServerProtocolMessageTests(ITestOutputHelper output, LSPTestsFixt { LanguageClient = data.LanguageClient; Diagnostics = data.Diagnostics; - PwshExe = TestsFixture.PwshExe; Diagnostics.Clear(); + PwshExe = TestsFixture.PwshExe; + _output = output; if (!s_registeredOnLogMessage) @@ -635,7 +636,6 @@ function CanSendReferencesCodeLensRequest { [Fact] public async Task CanSendCodeActionRequest() { - Diagnostics.Clear(); string filePath = NewTestFile("gci"); await WaitForDiagnostics(); From 62fd954ca42b28e533b3d089fb6dd33e5bae2cec Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Thu, 6 Feb 2020 18:45:50 -0800 Subject: [PATCH 17/33] Remove unused imports --- .../Services/Analysis/AnalysisService.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs index 71be38130..a23cb3fb3 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -14,11 +14,9 @@ using System.Threading; using System.Collections.Concurrent; using Microsoft.PowerShell.EditorServices.Services.TextDocument; -using Microsoft.PowerShell.EditorServices.Utility; using Microsoft.PowerShell.EditorServices.Services.Analysis; using Microsoft.PowerShell.EditorServices.Services.Configuration; using System.Collections; -using System.Diagnostics; using System.Runtime.InteropServices; namespace Microsoft.PowerShell.EditorServices.Services From a0d006cf759b335c806678e7fa22aaf3a304f78b Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Thu, 6 Feb 2020 19:04:00 -0800 Subject: [PATCH 18/33] Explicitly get PssaCmdletAnalysisEngine to implement IDisposable --- .../Services/Analysis/PssaCmdletAnalysisEngine.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs index 0cdd43014..8bdef4cb9 100644 --- a/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs +++ b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs @@ -23,7 +23,7 @@ namespace Microsoft.PowerShell.EditorServices.Services.Analysis /// PowerShell script analysis engine that uses PSScriptAnalyzer /// cmdlets run through a PowerShell API to drive analysis. /// - internal class PssaCmdletAnalysisEngine + internal class PssaCmdletAnalysisEngine : IDisposable { /// /// Builder for the PssaCmdletAnalysisEngine allowing settings configuration. From 92d79b70182c373689db1244a848e35a70389d4b Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Thu, 6 Feb 2020 19:06:29 -0800 Subject: [PATCH 19/33] Add ConfigureAwait(false) --- .../Services/TextDocument/Handlers/CodeActionHandler.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeActionHandler.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeActionHandler.cs index 10bd37958..23fb79538 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeActionHandler.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeActionHandler.cs @@ -57,7 +57,8 @@ public void SetCapability(CodeActionCapability capability) public async Task Handle(CodeActionParams request, CancellationToken cancellationToken) { - IReadOnlyDictionary corrections = await _analysisService.GetMostRecentCodeActionsForFileAsync(request.TextDocument.Uri.OriginalString); + IReadOnlyDictionary corrections = await _analysisService.GetMostRecentCodeActionsForFileAsync( + request.TextDocument.Uri.OriginalString).ConfigureAwait(false); if (corrections == null) { From 7c0cee5a9948c71470b04a2990eea063234480fb Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Thu, 6 Feb 2020 19:14:10 -0800 Subject: [PATCH 20/33] Remove commented out code --- .../Services/Analysis/AnalysisService.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs index a23cb3fb3..9875a00e3 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -351,12 +351,6 @@ private async Task DelayThenInvokeDiagnosticsAsync(ScriptFile[] filesToAnalyze, } catch (TaskCanceledException) { - // Ensure no stale markers are displayed - //foreach (ScriptFile script in filesToAnalyze) - //{ - // PublishScriptDiagnosticsAsync(script); - //} - return; } From a4e65ad33624fdcbc12597e8c08dc860096befc1 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Thu, 6 Feb 2020 19:16:34 -0800 Subject: [PATCH 21/33] Better event invocation --- .../Services/Workspace/Handlers/ConfigurationHandler.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/PowerShellEditorServices/Services/Workspace/Handlers/ConfigurationHandler.cs b/src/PowerShellEditorServices/Services/Workspace/Handlers/ConfigurationHandler.cs index 10240a9ec..f56faf4e0 100644 --- a/src/PowerShellEditorServices/Services/Workspace/Handlers/ConfigurationHandler.cs +++ b/src/PowerShellEditorServices/Services/Workspace/Handlers/ConfigurationHandler.cs @@ -83,7 +83,8 @@ public async Task Handle(DidChangeConfigurationParams request, Cancellatio this._consoleReplStarted = true; } - ConfigurationUpdated.Invoke(this, _configurationService.CurrentSettings); + // Run any events subscribed to configuration updates + ConfigurationUpdated(this, _configurationService.CurrentSettings); // Convert the editor file glob patterns into an array for the Workspace // Both the files.exclude and search.exclude hash tables look like (glob-text, is-enabled): From 5d6cf78db2ef5a4bea28fa43bcc03e030dccf2b9 Mon Sep 17 00:00:00 2001 From: Rob Holt Date: Tue, 11 Feb 2020 11:12:09 -0800 Subject: [PATCH 22/33] Change codeaction dict to use scriptfiles --- .../Services/Analysis/AnalysisService.cs | 25 ++++++------------- .../Handlers/CodeActionHandler.cs | 8 ++++-- .../Services/Workspace/WorkspaceService.cs | 9 ++++--- 3 files changed, 18 insertions(+), 24 deletions(-) diff --git a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs index 9875a00e3..23ffef339 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -90,7 +90,7 @@ internal static string GetUniqueIdFromDiagnostic(Diagnostic diagnostic) private readonly int _analysisDelayMillis; - private readonly ConcurrentDictionary _mostRecentCorrectionsByFile; + private readonly ConcurrentDictionary _mostRecentCorrectionsByFile; private bool _hasInstantiatedAnalysisEngine; @@ -117,7 +117,7 @@ public AnalysisService( _configurationService = configurationService; _workplaceService = workspaceService; _analysisDelayMillis = 750; - _mostRecentCorrectionsByFile = new ConcurrentDictionary(); + _mostRecentCorrectionsByFile = new ConcurrentDictionary(); _hasInstantiatedAnalysisEngine = false; } @@ -182,7 +182,7 @@ public void RunScriptDiagnostics( foreach (ScriptFile file in filesToAnalyze) { CorrectionTableEntry fileCorrectionsEntry = _mostRecentCorrectionsByFile.GetOrAdd( - file.DocumentUri, + file, CorrectionTableEntry.CreateForFile); fileCorrectionsEntry.DiagnosticPublish = analysisTask; @@ -239,20 +239,9 @@ public async Task GetCommentHelpText(string functionText, string helpLoc /// /// The URI string of the file to get code actions for. /// A threadsafe readonly dictionary of the code actions of the particular file. - public async Task> GetMostRecentCodeActionsForFileAsync(string documentUri) + public async Task> GetMostRecentCodeActionsForFileAsync(ScriptFile scriptFile) { - // On Windows, VSCode still gives us file URIs like "file:///c%3a/...", so we need to escape them - if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - { - var uri = new Uri(documentUri); - if (uri.Scheme == "file") - { - documentUri = WorkspaceService.UnescapeDriveColon(uri).AbsoluteUri; - } - } - - - if (!_mostRecentCorrectionsByFile.TryGetValue(documentUri, out CorrectionTableEntry corrections)) + if (!_mostRecentCorrectionsByFile.TryGetValue(scriptFile, out CorrectionTableEntry corrections)) { return null; } @@ -378,7 +367,7 @@ private void PublishScriptDiagnostics(ScriptFile scriptFile, IReadOnlyList(); _analysisService = analysisService; + _workspaceService = workspaceService; _registrationOptions = new CodeActionRegistrationOptions { DocumentSelector = new DocumentSelector(new DocumentFilter() { Language = "powershell" }), @@ -57,8 +60,9 @@ public void SetCapability(CodeActionCapability capability) public async Task Handle(CodeActionParams request, CancellationToken cancellationToken) { + // On Windows, VSCode still gives us file URIs like "file:///c%3a/...", so we need to escape them IReadOnlyDictionary corrections = await _analysisService.GetMostRecentCodeActionsForFileAsync( - request.TextDocument.Uri.OriginalString).ConfigureAwait(false); + _workspaceService.GetFile(request.TextDocument.Uri)).ConfigureAwait(false); if (corrections == null) { diff --git a/src/PowerShellEditorServices/Services/Workspace/WorkspaceService.cs b/src/PowerShellEditorServices/Services/Workspace/WorkspaceService.cs index 2da7881f2..6cff2008e 100644 --- a/src/PowerShellEditorServices/Services/Workspace/WorkspaceService.cs +++ b/src/PowerShellEditorServices/Services/Workspace/WorkspaceService.cs @@ -15,6 +15,7 @@ using Microsoft.PowerShell.EditorServices.Utility; using Microsoft.PowerShell.EditorServices.Services.Workspace; using Microsoft.PowerShell.EditorServices.Services.TextDocument; +using System.Collections.Concurrent; namespace Microsoft.PowerShell.EditorServices.Services { @@ -53,7 +54,7 @@ public class WorkspaceService private readonly ILogger logger; private readonly Version powerShellVersion; - private readonly Dictionary workspaceFiles = new Dictionary(); + private readonly ConcurrentDictionary workspaceFiles = new ConcurrentDictionary(); #endregion @@ -146,7 +147,7 @@ public ScriptFile GetFile(Uri fileUri) streamReader, this.powerShellVersion); - this.workspaceFiles.Add(keyName, scriptFile); + this.workspaceFiles[keyName] = scriptFile; } this.logger.LogDebug("Opened file on disk: " + resolvedFileUri.OriginalString); @@ -277,7 +278,7 @@ public void CloseFile(ScriptFile scriptFile) { Validate.IsNotNull("scriptFile", scriptFile); - this.workspaceFiles.Remove(scriptFile.Id); + this.workspaceFiles.TryRemove(scriptFile.Id, out ScriptFile _); } /// @@ -535,7 +536,7 @@ internal string ResolveRelativeScriptPath(string baseFilePath, string relativePa return combinedPath; } - internal static Uri UnescapeDriveColon(Uri fileUri) + private static Uri UnescapeDriveColon(Uri fileUri) { if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { From 2aed0a6799482807079d6facfaefc13f4a960e17 Mon Sep 17 00:00:00 2001 From: Rob Holt Date: Tue, 11 Feb 2020 12:09:54 -0800 Subject: [PATCH 23/33] Address @TylerLeonhardt's comments --- .../Services/Analysis/AnalysisService.cs | 6 +++--- .../Services/TextDocument/Handlers/TextDocumentHandler.cs | 4 ---- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs index 23ffef339..dc34c39e9 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -156,9 +156,9 @@ public void RunScriptDiagnostics( // Create a cancellation token source that will cancel if we do or if the caller does var cancellationSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - CancellationTokenSource oldTaskCancellation; - // If there's an existing task, we want to cancel it here - if ((oldTaskCancellation = Interlocked.Exchange(ref _diagnosticsCancellationTokenSource, cancellationSource)) != null) + // If there's an existing task, we want to cancel it here; + CancellationTokenSource oldTaskCancellation = Interlocked.Exchange(ref _diagnosticsCancellationTokenSource, cancellationSource); + if (oldTaskCancellation != null) { try { diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/TextDocumentHandler.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/TextDocumentHandler.cs index afe38345a..a8d10fbae 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/TextDocumentHandler.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/TextDocumentHandler.cs @@ -62,11 +62,9 @@ public Task Handle(DidChangeTextDocumentParams notification, CancellationT textChange.Text)); } -#pragma warning disable CS4014 // Kick off script diagnostics without blocking the response // TODO: Get all recently edited files in the workspace _analysisService.RunScriptDiagnostics(new ScriptFile[] { changedFile }, token); -#pragma warning restore CS4014 return Unit.Task; } @@ -91,11 +89,9 @@ public Task Handle(DidOpenTextDocumentParams notification, CancellationTok notification.TextDocument.Uri, notification.TextDocument.Text); -#pragma warning disable CS4014 // Kick off script diagnostics without blocking the response // TODO: Get all recently edited files in the workspace _analysisService.RunScriptDiagnostics(new ScriptFile[] { openedFile }, token); -#pragma warning restore CS4014 _logger.LogTrace("Finished opening document."); return Unit.Task; From 47c8b0b72223fa6abdb1247aaa2da29bd9d5f27f Mon Sep 17 00:00:00 2001 From: Rob Holt Date: Thu, 13 Feb 2020 13:47:23 -0800 Subject: [PATCH 24/33] Ensure PSModulePath is preserved when looking for PSSA --- .../Services/Analysis/PssaCmdletAnalysisEngine.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs index 8bdef4cb9..0334f2099 100644 --- a/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs +++ b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs @@ -446,10 +446,13 @@ private static RunspacePool CreatePssaRunspacePool(out PSModuleInfo pssaModuleIn try { - // Get the latest version of PSScriptAnalyzer we can find - pssaModuleInfo = ps.Invoke()? - .OrderByDescending(moduleInfo => moduleInfo.Version) - .FirstOrDefault(); + using (PSModulePathPreserver.Take()) + { + // Get the latest version of PSScriptAnalyzer we can find + pssaModuleInfo = ps.Invoke()? + .OrderByDescending(moduleInfo => moduleInfo.Version) + .FirstOrDefault(); + } } catch (Exception e) { From 81267d18c66a69fbfb41477cdae6019e7fece675 Mon Sep 17 00:00:00 2001 From: Rob Holt Date: Thu, 13 Feb 2020 13:52:17 -0800 Subject: [PATCH 25/33] Remove unused method --- .../Services/TextDocument/ScriptFileMarker.cs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/PowerShellEditorServices/Services/TextDocument/ScriptFileMarker.cs b/src/PowerShellEditorServices/Services/TextDocument/ScriptFileMarker.cs index ddbf8a8c1..11356aa35 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/ScriptFileMarker.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/ScriptFileMarker.cs @@ -108,15 +108,6 @@ internal static ScriptFileMarker FromParseError( Source = "PowerShell" }; } - private static string GetIfExistsString(PSObject psobj, string memberName) - { - if (psobj.Members.Match(memberName).Count == 0) - { - return string.Empty; - } - - return psobj.Members[memberName].Value as string ?? string.Empty; - } internal static ScriptFileMarker FromDiagnosticRecord(PSObject psObject) { From 393bd59a765d1e578531d3ba158b396dafb8cdc5 Mon Sep 17 00:00:00 2001 From: Rob Holt Date: Thu, 13 Feb 2020 13:53:51 -0800 Subject: [PATCH 26/33] Remove unneeded using --- test/PowerShellEditorServices.Test.E2E/LSPTestsFixures.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/PowerShellEditorServices.Test.E2E/LSPTestsFixures.cs b/test/PowerShellEditorServices.Test.E2E/LSPTestsFixures.cs index c82c375c3..55eea94ef 100644 --- a/test/PowerShellEditorServices.Test.E2E/LSPTestsFixures.cs +++ b/test/PowerShellEditorServices.Test.E2E/LSPTestsFixures.cs @@ -1,5 +1,4 @@ -using System.Collections.Concurrent; -using System.Collections.Generic; +using System.Collections.Generic; using System.IO; using System.Linq; using System.Threading.Tasks; From da558b46f9e81f6a1dc3d2058c8eb349351780cb Mon Sep 17 00:00:00 2001 From: Rob Holt Date: Thu, 13 Feb 2020 14:00:26 -0800 Subject: [PATCH 27/33] Add comment explaining correction table entry class --- .../Services/Analysis/AnalysisService.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs index dc34c39e9..8ecc3ec5a 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -473,6 +473,13 @@ public void Dispose() } #endregion + /// + /// Tracks corrections suggested by PSSA for a given file, + /// so that after a diagnostics request has fired, + /// a code action request can look up that file, + /// wait for analysis to finish if needed, + /// and then fetch the corrections set in the table entry by PSSA. + /// private class CorrectionTableEntry { public static CorrectionTableEntry CreateForFile(ScriptFile file) From 7dd68e26afeab6cacabe0455431fc9f6d9ad46ca Mon Sep 17 00:00:00 2001 From: Rob Holt Date: Thu, 13 Feb 2020 14:01:37 -0800 Subject: [PATCH 28/33] Enhance PSSA ISS comment --- .../Services/Analysis/PssaCmdletAnalysisEngine.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs index 0334f2099..3cb6126a1 100644 --- a/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs +++ b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs @@ -464,7 +464,8 @@ private static RunspacePool CreatePssaRunspacePool(out PSModuleInfo pssaModuleIn throw new FileNotFoundException("Unable to find PSScriptAnalyzer module on the module path"); } - // Create a base session state with PSScriptAnalyzer loaded + // Now that we know where the PSScriptAnalyzer we want to use is, + // create a base session state with PSScriptAnalyzer loaded #if DEBUG InitialSessionState sessionState = Environment.GetEnvironmentVariable("PSES_TEST_USE_CREATE_DEFAULT") == "1" ? InitialSessionState.CreateDefault() From 23dbbe395579fe6a5515ed4d2d836c38b2ff47bb Mon Sep 17 00:00:00 2001 From: Rob Holt Date: Thu, 13 Feb 2020 16:22:50 -0800 Subject: [PATCH 29/33] Address issues --- .../Services/Analysis/AnalysisService.cs | 48 +++++++------------ .../Analysis/PssaCmdletAnalysisEngine.cs | 40 ++++++++++------ 2 files changed, 43 insertions(+), 45 deletions(-) diff --git a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs index 8ecc3ec5a..a2d5f521d 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -3,21 +3,20 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. // -using System; -using System.Linq; -using System.Threading.Tasks; -using System.Collections.Generic; -using System.Text; using Microsoft.Extensions.Logging; -using OmniSharp.Extensions.LanguageServer.Protocol.Models; -using OmniSharp.Extensions.LanguageServer.Protocol.Server; -using System.Threading; -using System.Collections.Concurrent; -using Microsoft.PowerShell.EditorServices.Services.TextDocument; using Microsoft.PowerShell.EditorServices.Services.Analysis; using Microsoft.PowerShell.EditorServices.Services.Configuration; +using Microsoft.PowerShell.EditorServices.Services.TextDocument; +using OmniSharp.Extensions.LanguageServer.Protocol.Models; +using OmniSharp.Extensions.LanguageServer.Protocol.Server; +using System; using System.Collections; -using System.Runtime.InteropServices; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading; +using System.Threading.Tasks; namespace Microsoft.PowerShell.EditorServices.Services { @@ -92,12 +91,13 @@ internal static string GetUniqueIdFromDiagnostic(Diagnostic diagnostic) private readonly ConcurrentDictionary _mostRecentCorrectionsByFile; - private bool _hasInstantiatedAnalysisEngine; - - private PssaCmdletAnalysisEngine _analysisEngine; + private Lazy _analysisEngine; private CancellationTokenSource _diagnosticsCancellationTokenSource; + #region Engine Initialization + #endregion + /// /// Construct a new AnalysisService. /// @@ -118,25 +118,13 @@ public AnalysisService( _workplaceService = workspaceService; _analysisDelayMillis = 750; _mostRecentCorrectionsByFile = new ConcurrentDictionary(); - _hasInstantiatedAnalysisEngine = false; + _analysisEngine = new Lazy(InstantiateAnalysisEngine); } /// /// The analysis engine to use for running script analysis. /// - private PssaCmdletAnalysisEngine AnalysisEngine - { - get - { - if (!_hasInstantiatedAnalysisEngine) - { - _analysisEngine = InstantiateAnalysisEngine(); - _hasInstantiatedAnalysisEngine = true; - } - - return _analysisEngine; - } - } + private PssaCmdletAnalysisEngine AnalysisEngine => _analysisEngine.Value; /// /// Sets up a script analysis run, eventually returning the result. @@ -270,7 +258,7 @@ public void ClearMarkers(ScriptFile file) public void OnConfigurationUpdated(object sender, LanguageServerSettings settings) { ClearOpenFileMarkers(); - _analysisEngine = InstantiateAnalysisEngine(); + _analysisEngine = new Lazy(InstantiateAnalysisEngine); } private PssaCmdletAnalysisEngine InstantiateAnalysisEngine() @@ -457,7 +445,7 @@ protected virtual void Dispose(bool disposing) { if (disposing) { - _analysisEngine?.Dispose(); + if (_analysisEngine.IsValueCreated) { _analysisEngine.Value.Dispose(); } _diagnosticsCancellationTokenSource?.Dispose(); } diff --git a/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs index 3cb6126a1..5b4333db4 100644 --- a/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs +++ b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs @@ -107,6 +107,8 @@ public PssaCmdletAnalysisEngine Build() } } + private const int PSSA_RUNPOOL_SIZE = 10; + private const string PSSA_MODULE_NAME = "PSScriptAnalyzer"; /// @@ -133,19 +135,17 @@ public PssaCmdletAnalysisEngine Build() private readonly string[] _rulesToInclude; - private bool _alreadyRun; + // Used to mitigate the side effects of PSSA opening runspaces with its runspace pool under the hood + private int _pssaRunCount; private PssaCmdletAnalysisEngine( ILogger logger, RunspacePool analysisRunspacePool, PSModuleInfo pssaModuleInfo, string[] rulesToInclude) + : this(logger, analysisRunspacePool, pssaModuleInfo) { - _logger = logger; - _analysisRunspacePool = analysisRunspacePool; - _pssaModuleInfo = pssaModuleInfo; _rulesToInclude = rulesToInclude; - _alreadyRun = false; } private PssaCmdletAnalysisEngine( @@ -153,12 +153,20 @@ private PssaCmdletAnalysisEngine( RunspacePool analysisRunspacePool, PSModuleInfo pssaModuleInfo, object analysisSettingsParameter) + : this(logger, analysisRunspacePool, pssaModuleInfo) + { + _settingsParameter = analysisSettingsParameter; + } + + private PssaCmdletAnalysisEngine( + ILogger logger, + RunspacePool analysisRunspacePool, + PSModuleInfo pssaModuleInfo) { _logger = logger; _analysisRunspacePool = analysisRunspacePool; _pssaModuleInfo = pssaModuleInfo; - _settingsParameter = analysisSettingsParameter; - _alreadyRun = false; + _pssaRunCount = 0; } /// @@ -347,22 +355,24 @@ private PowerShellResult InvokePowerShell(PSCommand command) /// The output of PowerShell execution. private Collection InvokePowerShellWithModulePathPreservation(System.Management.Automation.PowerShell powershell) { - if (_alreadyRun) + if (_pssaRunCount > PSSA_RUNPOOL_SIZE) { return powershell.Invoke(); } - // PSScriptAnalyzer uses a runspace pool underneath and we need to stop the PSModulePath from being changed by it - try + // PSScriptAnalyzer uses a runspace pool underneath and we need to stop the PSModulePath from being changed by it on invocation + // Each time a new runspace is opened we need to take this precaution, which is effectively a lock + // But when the runspace pool is at capacity and stops opening new runspaces, we can stop + using (PSModulePathPreserver.Take()) { - using (PSModulePathPreserver.Take()) + try { return powershell.Invoke(); } - } - finally - { - _alreadyRun = true; + finally + { + Interlocked.Increment(ref _pssaRunCount); + } } } From de071f956fc0110e220a7aebf049a9ecc2633a9f Mon Sep 17 00:00:00 2001 From: Rob Holt Date: Thu, 13 Feb 2020 17:15:12 -0800 Subject: [PATCH 30/33] Remove cleverness with PSSA runspace management --- .../Analysis/PssaCmdletAnalysisEngine.cs | 21 +------------------ 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs index 5b4333db4..82818a467 100644 --- a/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs +++ b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs @@ -135,9 +135,6 @@ public PssaCmdletAnalysisEngine Build() private readonly string[] _rulesToInclude; - // Used to mitigate the side effects of PSSA opening runspaces with its runspace pool under the hood - private int _pssaRunCount; - private PssaCmdletAnalysisEngine( ILogger logger, RunspacePool analysisRunspacePool, @@ -166,7 +163,6 @@ private PssaCmdletAnalysisEngine( _logger = logger; _analysisRunspacePool = analysisRunspacePool; _pssaModuleInfo = pssaModuleInfo; - _pssaRunCount = 0; } /// @@ -355,24 +351,9 @@ private PowerShellResult InvokePowerShell(PSCommand command) /// The output of PowerShell execution. private Collection InvokePowerShellWithModulePathPreservation(System.Management.Automation.PowerShell powershell) { - if (_pssaRunCount > PSSA_RUNPOOL_SIZE) - { - return powershell.Invoke(); - } - - // PSScriptAnalyzer uses a runspace pool underneath and we need to stop the PSModulePath from being changed by it on invocation - // Each time a new runspace is opened we need to take this precaution, which is effectively a lock - // But when the runspace pool is at capacity and stops opening new runspaces, we can stop using (PSModulePathPreserver.Take()) { - try - { - return powershell.Invoke(); - } - finally - { - Interlocked.Increment(ref _pssaRunCount); - } + return powershell.Invoke(); } } From 2d7fd1dcf75fc8ddee335f649a74573ff3977dc4 Mon Sep 17 00:00:00 2001 From: Rob Holt Date: Thu, 13 Feb 2020 17:16:10 -0800 Subject: [PATCH 31/33] Remove unused constant --- .../Services/Analysis/PssaCmdletAnalysisEngine.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs index 82818a467..7eb7e9c02 100644 --- a/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs +++ b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs @@ -107,8 +107,6 @@ public PssaCmdletAnalysisEngine Build() } } - private const int PSSA_RUNPOOL_SIZE = 10; - private const string PSSA_MODULE_NAME = "PSScriptAnalyzer"; /// From c9b2a4cf6b29a3bed8e638463aeab3c3c3a6fd10 Mon Sep 17 00:00:00 2001 From: Rob Holt Date: Thu, 13 Feb 2020 17:27:52 -0800 Subject: [PATCH 32/33] Remove unused region --- .../Services/Analysis/AnalysisService.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs index a2d5f521d..e5d7e1121 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -95,9 +95,6 @@ internal static string GetUniqueIdFromDiagnostic(Diagnostic diagnostic) private CancellationTokenSource _diagnosticsCancellationTokenSource; - #region Engine Initialization - #endregion - /// /// Construct a new AnalysisService. /// From e3dbcd460f12dcfeab5e1fe46d33815eba775f16 Mon Sep 17 00:00:00 2001 From: Rob Holt Date: Thu, 13 Feb 2020 17:28:50 -0800 Subject: [PATCH 33/33] Change using order --- .../Services/Analysis/AnalysisService.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs index e5d7e1121..5fd9f5ef0 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -3,12 +3,6 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. // -using Microsoft.Extensions.Logging; -using Microsoft.PowerShell.EditorServices.Services.Analysis; -using Microsoft.PowerShell.EditorServices.Services.Configuration; -using Microsoft.PowerShell.EditorServices.Services.TextDocument; -using OmniSharp.Extensions.LanguageServer.Protocol.Models; -using OmniSharp.Extensions.LanguageServer.Protocol.Server; using System; using System.Collections; using System.Collections.Concurrent; @@ -17,6 +11,12 @@ using System.Text; using System.Threading; using System.Threading.Tasks; +using Microsoft.Extensions.Logging; +using Microsoft.PowerShell.EditorServices.Services.Analysis; +using Microsoft.PowerShell.EditorServices.Services.Configuration; +using Microsoft.PowerShell.EditorServices.Services.TextDocument; +using OmniSharp.Extensions.LanguageServer.Protocol.Models; +using OmniSharp.Extensions.LanguageServer.Protocol.Server; namespace Microsoft.PowerShell.EditorServices.Services {