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 59a593431..5fd9f5ef0 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -4,20 +4,19 @@ // using System; -using System.Linq; -using System.Threading.Tasks; -using System.Management.Automation.Runspaces; -using System.Management.Automation; +using System.Collections; +using System.Collections.Concurrent; using System.Collections.Generic; +using System.Linq; using System.Text; -using System.Collections; +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; -using System.Threading; -using System.Collections.Concurrent; -using Microsoft.PowerShell.EditorServices.Services.TextDocument; -using Microsoft.PowerShell.EditorServices.Utility; namespace Microsoft.PowerShell.EditorServices.Services { @@ -25,15 +24,42 @@ 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 + /// + /// 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; + 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. /// - private static readonly string[] s_includedRules = { + private static readonly string[] s_defaultRules = { "PSAvoidAssignmentToAutomaticVariable", "PSUseToExportFieldsInManifest", "PSMisleadingBacktick", @@ -51,711 +77,254 @@ 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(); - - private static CancellationTokenSource s_existingRequestCancellation; - - /// - /// The indentation to add when the logger lists errors. - /// - private static readonly string s_indentJoin = Environment.NewLine + " "; - - #endregion // Static fields - - #region Private Fields - - /// - /// Maximum number of runspaces we allow to be in use for script analysis. - /// - private const int NumRunspaces = 1; - - /// - /// Name of the PSScriptAnalyzer module, to be used for PowerShell module interactions. - /// - private const string PSSA_MODULE_NAME = "PSScriptAnalyzer"; - - /// - /// Provides logging. - /// - private ILogger _logger; - - /// - /// Runspace pool to generate runspaces for script analysis and handle - /// ansynchronous analysis requests. - /// - private RunspacePool _analysisRunspacePool; + private readonly ILoggerFactory _loggerFactory; - /// - /// Info object describing the PSScriptAnalyzer module that has been loaded in - /// to provide analysis services. - /// - private PSModuleInfo _pssaModuleInfo; + private readonly ILogger _logger; private readonly ILanguageServer _languageServer; private readonly ConfigurationService _configurationService; - private readonly ConcurrentDictionary)> _mostRecentCorrectionsByFile; - - #endregion // Private Fields + private readonly WorkspaceService _workplaceService; - #region Properties + private readonly int _analysisDelayMillis; - /// - /// Set of PSScriptAnalyzer rules used for analysis. - /// - public string[] ActiveRules { get; set; } - - /// - /// Gets or sets the path to a settings file (.psd1) - /// containing PSScriptAnalyzer settings. - /// - public string SettingsPath { get; set; } + private readonly ConcurrentDictionary _mostRecentCorrectionsByFile; - #endregion + private Lazy _analysisEngine; - #region Constructors + private CancellationTokenSource _diagnosticsCancellationTokenSource; /// - /// Construct a new AnalysisService object. + /// Construct a new AnalysisService. /// - /// - /// 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, + /// 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, ConfigurationService configurationService, - ILogger logger, - PSModuleInfo pssaModuleInfo = null) + WorkspaceService workspaceService) { - _analysisRunspacePool = analysisRunspacePool; - SettingsPath = pssaSettingsPath; - ActiveRules = activeRules.ToArray(); + _loggerFactory = loggerFactory; + _logger = loggerFactory.CreateLogger(); _languageServer = languageServer; _configurationService = configurationService; - _logger = logger; - _pssaModuleInfo = pssaModuleInfo; - _mostRecentCorrectionsByFile = new ConcurrentDictionary)>(); + _workplaceService = workspaceService; + _analysisDelayMillis = 750; + _mostRecentCorrectionsByFile = new ConcurrentDictionary(); + _analysisEngine = new Lazy(InstantiateAnalysisEngine); } - #endregion // constructors - - #region Public Methods + /// + /// The analysis engine to use for running script analysis. + /// + private PssaCmdletAnalysisEngine AnalysisEngine => _analysisEngine.Value; /// - /// Factory method for producing AnalysisService instances. Handles loading of the PSScriptAnalyzer module - /// and runspace pool instantiation before creating the service instance. + /// Sets up a script analysis run, eventually returning the result. /// - /// 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) + /// 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 void RunScriptDiagnostics( + ScriptFile[] filesToAnalyze, + CancellationToken cancellationToken) { - Validate.IsNotNull(nameof(configurationService), configurationService); - string settingsPath = configurationService.CurrentSettings.ScriptAnalysis.SettingsPath; - try + if (AnalysisEngine == null) + { + return; + } + + // Create a cancellation token source that will cancel if we do or if the caller does + var cancellationSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + + // If there's an existing task, we want to cancel it here; + CancellationTokenSource oldTaskCancellation = Interlocked.Exchange(ref _diagnosticsCancellationTokenSource, cancellationSource); + if (oldTaskCancellation != null) { - 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); - + oldTaskCancellation.Cancel(); + oldTaskCancellation.Dispose(); } 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"); + _logger.LogError(e, "Exception occurred while cancelling analysis task"); } - - // 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; - } - } - - /// - /// 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) + if (filesToAnalyze.Length == 0) { - ruleSettingsHashtable.Add(kvp.Key, kvp.Value); + return; } - 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); - } + var analysisTask = Task.Run(() => DelayThenInvokeDiagnosticsAsync(filesToAnalyze, _diagnosticsCancellationTokenSource.Token), _diagnosticsCancellationTokenSource.Token); - /// - /// 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); - } + // Ensure that any next corrections request will wait for this diagnostics publication + foreach (ScriptFile file in filesToAnalyze) + { + CorrectionTableEntry fileCorrectionsEntry = _mostRecentCorrectionsByFile.GetOrAdd( + file, + CorrectionTableEntry.CreateForFile); - /// - /// 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); + fileCorrectionsEntry.DiagnosticPublish = analysisTask; + } } /// - /// Returns a list of builtin-in PSScriptAnalyzer rules + /// Formats a PowerShell script with the given settings. /// - public IEnumerable GetPSScriptAnalyzerRules() + /// 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) { - PowerShellResult getRuleResult = InvokePowerShell("Get-ScriptAnalyzerRule"); - if (getRuleResult == null) + if (AnalysisEngine == 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 null; } - return ruleNames; + return AnalysisEngine.FormatAsync(scriptFileContents, formatSettings, formatRange); } /// - /// Format a given script text with default codeformatting settings. + /// Get comment help text for a PowerShell function definition. /// - /// 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) + /// 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) { - // 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)) + if (AnalysisEngine == null) { return null; } - var argsDict = new Dictionary { - {"ScriptDefinition", scriptDefinition}, - {"Settings", settings} - }; - if (rangeList != null) - { - argsDict.Add("Range", rangeList); - } + Hashtable commentHelpSettings = AnalysisService.GetCommentHelpRuleSettings(helpLocation, forBlockComment); - PowerShellResult result = await InvokePowerShellAsync("Invoke-Formatter", argsDict).ConfigureAwait(false); + ScriptFileMarker[] analysisResults = await AnalysisEngine.AnalyzeScriptAsync(functionText, commentHelpSettings).ConfigureAwait(false); - if (result == null) + if (analysisResults.Length == 0 + || analysisResults[0]?.Correction?.Edits == null + || analysisResults[0].Correction.Edits.Count() == 0) { - _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; + return analysisResults[0].Correction.Edits[0].Text; } - #endregion // public methods - - #region Private Methods - - private async Task> GetSemanticMarkersAsync( - ScriptFile file, - string[] rules, - TSettings settings) where TSettings : class + /// + /// 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 async Task> GetMostRecentCodeActionsForFileAsync(ScriptFile scriptFile) { - if (file.IsAnalysisEnabled) - { - return await GetSemanticMarkersAsync( - file.Contents, - rules, - settings).ConfigureAwait(false); - } - else + if (!_mostRecentCorrectionsByFile.TryGetValue(scriptFile, out CorrectionTableEntry corrections)) { - // Return an empty marker list - return new List(); + return null; } + + // Wait for diagnostics to be published for this file + await corrections.DiagnosticPublish.ConfigureAwait(false); + + return corrections.Corrections; } - private async Task> GetSemanticMarkersAsync( - string scriptContent, - string[] rules, - TSettings settings) where TSettings : class + /// + /// 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 void ClearMarkers(ScriptFile file) { - 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(); - } + PublishScriptDiagnostics(file, Array.Empty()); } /// - /// Log the features available from the PSScriptAnalyzer module that has been imported - /// for use with the AnalysisService. + /// Event subscription method to be run when PSES configuration has been updated. /// - private void LogAvailablePssaFeatures() + /// The sender of the configuration update event. + /// The new language server settings. + public void OnConfigurationUpdated(object sender, LanguageServerSettings settings) { - // 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()); + ClearOpenFileMarkers(); + _analysisEngine = new Lazy(InstantiateAnalysisEngine); } - private async Task GetDiagnosticRecordsAsync( - string scriptContent, - string[] rules, - TSettings settings) where TSettings : class + private PssaCmdletAnalysisEngine InstantiateAnalysisEngine() { - // 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))) + if (!(_configurationService.CurrentSettings.ScriptAnalysis.Enable ?? false)) { - return s_emptyDiagnosticResult; + return null; } - //Use a settings file if one is provided, otherwise use the default rule list. - string settingParameter; - object settingArgument; - if (settings != null) + var pssaCmdletEngineBuilder = new PssaCmdletAnalysisEngine.Builder(_loggerFactory); + + // If there's a settings file use that + if (TryFindSettingsFile(out string settingsFilePath)) { - settingParameter = "Settings"; - settingArgument = settings; + _logger.LogInformation($"Configuring PSScriptAnalyzer with rules at '{settingsFilePath}'"); + pssaCmdletEngineBuilder.WithSettingsFile(settingsFilePath); } else { - settingParameter = "IncludeRule"; - settingArgument = rules; + _logger.LogInformation("PSScriptAnalyzer settings file not found. Falling back to default rules"); + pssaCmdletEngineBuilder.WithIncludedRules(s_defaultRules); } - 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; - } + return pssaCmdletEngineBuilder.Build(); } - private Task InvokePowerShellAsync(string command, IDictionary paramArgMap = null) + private bool TryFindSettingsFile(out string settingsFilePath) { - return Task.Run(() => InvokePowerShell(command, paramArgMap)); - } + string configuredPath = _configurationService.CurrentSettings.ScriptAnalysis.SettingsPath; - /// - /// 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()) + if (!string.IsNullOrEmpty(configuredPath)) { - // 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); - } + settingsFilePath = _workplaceService.ResolveWorkspacePath(configuredPath); - if (pssaModuleInfo == null) + if (settingsFilePath == null) { - throw new AnalysisServiceLoadException("Unable to find PSScriptAnalyzer module on the module path"); + _logger.LogError($"Unable to find PSSA settings file at '{configuredPath}'. Loading default rules."); } - // 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); + return settingsFilePath != null; } - } - - #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; - } - } + // TODO: Could search for a default here - /// - /// 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); + settingsFilePath = null; + return false; } - #endregion - - /// - /// Wraps the result of an execution of PowerShell to send back through - /// asynchronous calls. - /// - private class PowerShellResult + private void ClearOpenFileMarkers() { - public PowerShellResult( - PSObject[] output, - ErrorRecord[] errors, - bool hasErrors) + foreach (ScriptFile file in _workplaceService.GetOpenedFiles()) { - Output = output; - Errors = errors; - HasErrors = hasErrors; + ClearMarkers(file); } - - public PSObject[] Output { get; } - - public ErrorRecord[] Errors { get; } - - public bool HasErrors { get; } } - internal Task RunScriptDiagnosticsAsync( - ScriptFile[] filesToAnalyze) + private async Task DelayThenInvokeDiagnosticsAsync(ScriptFile[] filesToAnalyze, CancellationToken cancellationToken) { - // 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) + if (cancellationToken.IsCancellationRequested) { - return Task.CompletedTask; + return; } - // 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); + await Task.Delay(_analysisDelayMillis, cancellationToken).ConfigureAwait(false); } catch (TaskCanceledException) { - // If the task is cancelled, exit directly - foreach (var script in filesToAnalyze) - { - PublishScriptDiagnostics( - script, - script.DiagnosticMarkers); - } - return; } @@ -766,151 +335,53 @@ private async Task DelayThenInvokeDiagnosticsAsync( // 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(); - } + ScriptFileMarker[] semanticMarkers = await AnalysisEngine.AnalyzeScriptAsync(scriptFile.Contents).ConfigureAwait(false); scriptFile.DiagnosticMarkers.AddRange(semanticMarkers); - PublishScriptDiagnostics( - scriptFile, - // Concat script analysis errors to any existing parse errors - scriptFile.DiagnosticMarkers); + PublishScriptDiagnostics(scriptFile); } } - 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) => PublishScriptDiagnostics(scriptFile, scriptFile.DiagnosticMarkers); - private void PublishScriptDiagnostics( - ScriptFile scriptFile, - List markers) + private void PublishScriptDiagnostics(ScriptFile scriptFile, IReadOnlyList markers) { - var diagnostics = new List(); + var diagnostics = new Diagnostic[scriptFile.DiagnosticMarkers.Count]; - // 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; - } + CorrectionTableEntry fileCorrections = _mostRecentCorrectionsByFile.GetOrAdd( + scriptFile, + CorrectionTableEntry.CreateForFile); - fileLock.Wait(); - try + fileCorrections.Corrections.Clear(); + + for (int i = 0; i < markers.Count; i++) { - 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(); - } + ScriptFileMarker marker = markers[i]; - 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; - } + Diagnostic diagnostic = GetDiagnosticFromMarker(marker); - diagnostics.Add(markerDiagnostic); + if (marker.Correction != null) + { + string diagnosticId = GetUniqueIdFromDiagnostic(diagnostic); + fileCorrections.Corrections[diagnosticId] = marker.Correction; } - } - finally - { - fileLock.Release(); + + diagnostics[i] = diagnostic; } - // Always send syntax and semantic errors. We want to - // make sure no out-of-date markers are being displayed. - _languageServer.Document.PublishDiagnostics(new PublishDiagnosticsParams() + _languageServer.Document.PublishDiagnostics(new PublishDiagnosticsParams { Uri = new Uri(scriptFile.DocumentUri), - Diagnostics = new Container(diagnostics), + Diagnostics = new Container(diagnostics) }); } - public async Task> GetMostRecentCodeActionsForFileAsync(string documentUri) + private static ConcurrentDictionary CreateFileCorrectionsEntry(string fileUri) { - 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; + return new ConcurrentDictionary(); } private static Diagnostic GetDiagnosticFromMarker(ScriptFileMarker scriptFileMarker) @@ -941,43 +412,75 @@ private static DiagnosticSeverity MapDiagnosticSeverity(ScriptFileMarkerLevel ma { switch (markerLevel) { - case ScriptFileMarkerLevel.Error: - return DiagnosticSeverity.Error; + case ScriptFileMarkerLevel.Error: return DiagnosticSeverity.Error; + case ScriptFileMarkerLevel.Warning: return DiagnosticSeverity.Warning; + case ScriptFileMarkerLevel.Information: return DiagnosticSeverity.Information; + default: return DiagnosticSeverity.Error; + }; + } - case ScriptFileMarkerLevel.Warning: - return DiagnosticSeverity.Warning; + 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 }} + }} + }}; + } - case ScriptFileMarkerLevel.Information: - return DiagnosticSeverity.Information; + #region IDisposable Support + private bool disposedValue = false; // To detect redundant calls - default: - return DiagnosticSeverity.Error; + protected virtual void Dispose(bool disposing) + { + if (!disposedValue) + { + if (disposing) + { + if (_analysisEngine.IsValueCreated) { _analysisEngine.Value.Dispose(); } + _diagnosticsCancellationTokenSource?.Dispose(); + } + + disposedValue = true; } } - } - /// - /// 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) + // 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 /// - /// Instantiate an AnalysisService error based on another error that occurred internally. + /// 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. /// - /// 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) + private class CorrectionTableEntry { + public static CorrectionTableEntry CreateForFile(ScriptFile file) + { + return new CorrectionTableEntry(); + } + + public CorrectionTableEntry() + { + Corrections = new ConcurrentDictionary(); + DiagnosticPublish = Task.CompletedTask; + } + + public ConcurrentDictionary Corrections { get; } + + public Task DiagnosticPublish { get; set; } } } } diff --git a/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs new file mode 100644 index 000000000..7eb7e9c02 --- /dev/null +++ b/src/PowerShellEditorServices/Services/Analysis/PssaCmdletAnalysisEngine.cs @@ -0,0 +1,537 @@ +// +// 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 +{ + /// + /// PowerShell script analysis engine that uses PSScriptAnalyzer + /// cmdlets run through a PowerShell API to drive analysis. + /// + internal class PssaCmdletAnalysisEngine : IDisposable + { + /// + /// Builder for the PssaCmdletAnalysisEngine allowing settings configuration. + /// + public class Builder + { + private readonly ILoggerFactory _loggerFactory; + + private object _settingsParameter; + + private string[] _rules; + + /// + /// Create a builder for PssaCmdletAnalysisEngine construction. + /// + /// The logger to use. + public Builder(ILoggerFactory loggerFactory) + { + _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); + + cmdletAnalysisEngine.LogAvailablePssaFeatures(); + return cmdletAnalysisEngine; + } + catch (FileNotFoundException e) + { + logger.LogError(e, $"Unable to find PSScriptAnalyzer. Disabling script analysis. PSModulePath: '{Environment.GetEnvironmentVariable("PSModulePath")}'"); + return null; + } + } + } + + 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 + }; + + private readonly ILogger _logger; + + private readonly RunspacePool _analysisRunspacePool; + + private readonly PSModuleInfo _pssaModuleInfo; + + private readonly object _settingsParameter; + + private readonly string[] _rulesToInclude; + + private PssaCmdletAnalysisEngine( + ILogger logger, + RunspacePool analysisRunspacePool, + PSModuleInfo pssaModuleInfo, + string[] rulesToInclude) + : this(logger, analysisRunspacePool, pssaModuleInfo) + { + _rulesToInclude = rulesToInclude; + } + + private PssaCmdletAnalysisEngine( + ILogger logger, + 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; + } + + /// + /// 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. + // 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", formatSettings); + + 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; + } + + /// + /// 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. + // Furthermore, if you call Invoke-ScriptAnalyzer with an empty ScriptDefinition + // it will generate a ParameterBindingValidationException. + if (string.IsNullOrEmpty(scriptContent)) + { + return Task.FromResult(Array.Empty()); + } + + var command = new PSCommand() + .AddCommand("Invoke-ScriptAnalyzer") + .AddParameter("ScriptDefinition", scriptContent) + .AddParameter("Severity", s_scriptMarkerLevels); + + object settingsValue = settings ?? _settingsParameter; + if (settingsValue != null) + { + command.AddParameter("Settings", settingsValue); + } + else + { + command.AddParameter("IncludeRule", _rulesToInclude); + } + + 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 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; + } + } + + /// + /// 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) + { + using (PSModulePathPreserver.Take()) + { + return powershell.Invoke(); + } + } + + /// + /// 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 (_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 + { + using (PSModulePathPreserver.Take()) + { + // 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"); + } + + // 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() + : InitialSessionState.CreateDefault2(); +#else + InitialSessionState sessionState = InitialSessionState.CreateDefault2(); +#endif + + sessionState.ImportPSModule(new [] { pssaModuleInfo.ModuleBase }); + + 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()) + { + 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; } + } + + /// + /// 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(); + + 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/PowerShellContext/Handlers/GetCommentHelpHandler.cs b/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/GetCommentHelpHandler.cs index 13f4deac0..c3b3134b4 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).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 ec1a8bd2d..aa302eafa 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeActionHandler.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeActionHandler.cs @@ -5,6 +5,8 @@ using System; using System.Collections.Generic; +using System.Diagnostics; +using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.Logging; @@ -30,12 +32,15 @@ internal class CodeActionHandler : ICodeActionHandler private readonly AnalysisService _analysisService; + private readonly WorkspaceService _workspaceService; + private CodeActionCapability _capability; - public CodeActionHandler(ILoggerFactory factory, AnalysisService analysisService) + public CodeActionHandler(ILoggerFactory factory, AnalysisService analysisService, WorkspaceService workspaceService) { _logger = factory.CreateLogger(); _analysisService = analysisService; + _workspaceService = workspaceService; _registrationOptions = new CodeActionRegistrationOptions { DocumentSelector = new DocumentSelector(new DocumentFilter() { Language = "powershell" }), @@ -55,7 +60,9 @@ public void SetCapability(CodeActionCapability capability) public async Task Handle(CodeActionParams request, CancellationToken cancellationToken) { - IReadOnlyDictionary corrections = await _analysisService.GetMostRecentCodeActionsForFileAsync(request.TextDocument.Uri.OriginalString).ConfigureAwait(false); + // On Windows, VSCode still gives us file URIs like "file:///c%3a/...", so we need to escape them + IReadOnlyDictionary corrections = await _analysisService.GetMostRecentCodeActionsForFileAsync( + _workspaceService.GetFile(request.TextDocument.Uri)).ConfigureAwait(false); if (corrections == null) { @@ -68,12 +75,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}"); @@ -81,7 +83,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 e9f844386..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.RunScriptDiagnosticsAsync(new ScriptFile[] { changedFile }); -#pragma warning restore CS4014 + _analysisService.RunScriptDiagnostics(new ScriptFile[] { changedFile }, token); 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.RunScriptDiagnosticsAsync(new ScriptFile[] { openedFile }); -#pragma warning restore CS4014 + _analysisService.RunScriptDiagnostics(new ScriptFile[] { openedFile }, token); _logger.LogTrace("Finished opening document."); return Unit.Task; diff --git a/src/PowerShellEditorServices/Services/TextDocument/ScriptFileMarker.cs b/src/PowerShellEditorServices/Services/TextDocument/ScriptFileMarker.cs index 9609b2cae..11356aa35 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/ScriptFileMarker.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/ScriptFileMarker.cs @@ -108,21 +108,10 @@ internal static ScriptFileMarker FromParseError( Source = "PowerShell" }; } - private static string GetIfExistsString(PSObject psobj, string memberName) - { - if (psobj.Members.Match(memberName).Count > 0) - { - return psobj.Members[memberName].Value != null ? (string)psobj.Members[memberName].Value : ""; - } - else - { - return ""; - } - } 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 @@ -160,7 +149,7 @@ internal static ScriptFileMarker FromDiagnosticRecord(PSObject psObject) correction = new MarkerCorrection { - Name = correctionMessage == null ? diagnosticRecord.Message : correctionMessage, + Name = correctionMessage ?? diagnosticRecord.Message, Edits = editRegions.ToArray() }; } @@ -175,8 +164,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, diff --git a/src/PowerShellEditorServices/Services/Workspace/Handlers/ConfigurationHandler.cs b/src/PowerShellEditorServices/Services/Workspace/Handlers/ConfigurationHandler.cs index 949ba04dd..f56faf4e0 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,8 @@ 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); - } - } - } + // 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): @@ -146,5 +123,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..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 _); } /// @@ -482,6 +483,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; diff --git a/test/PowerShellEditorServices.Test.E2E/LSPTestsFixures.cs b/test/PowerShellEditorServices.Test.E2E/LSPTestsFixures.cs index fcf10bee3..55eea94ef 100644 --- a/test/PowerShellEditorServices.Test.E2E/LSPTestsFixures.cs +++ b/test/PowerShellEditorServices.Test.E2E/LSPTestsFixures.cs @@ -1,5 +1,6 @@ using System.Collections.Generic; using System.IO; +using System.Linq; using System.Threading.Tasks; using Microsoft.Extensions.Logging; using Newtonsoft.Json.Linq; @@ -41,7 +42,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 1ea7a0aea..d9e9f735d 100644 --- a/test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs +++ b/test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs @@ -35,12 +35,12 @@ public class LanguageServerProtocolMessageTests : IClassFixture public LanguageServerProtocolMessageTests(ITestOutputHelper output, LSPTestsFixture data) { - Diagnostics = new List(); LanguageClient = data.LanguageClient; Diagnostics = data.Diagnostics; - PwshExe = TestsFixture.PwshExe; Diagnostics.Clear(); + PwshExe = TestsFixture.PwshExe; + _output = output; if (!s_registeredOnLogMessage)