diff --git a/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs b/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs index eafe34fdb..0ae0ff27b 100644 --- a/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs +++ b/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs @@ -575,7 +575,7 @@ protected async Task HandleDidChangeConfigurationNotification( bool oldScriptAnalysisEnabled = this.currentSettings.ScriptAnalysis.Enable.HasValue ? this.currentSettings.ScriptAnalysis.Enable.Value : false; string oldScriptAnalysisSettingsPath = - this.currentSettings.ScriptAnalysis.SettingsPath; + this.currentSettings.ScriptAnalysis?.SettingsPath; this.currentSettings.Update( configChangeParams.Settings.Powershell, @@ -604,12 +604,15 @@ protected async Task HandleDidChangeConfigurationNotification( string newSettingsPath = this.currentSettings.ScriptAnalysis.SettingsPath; if (!string.Equals(oldScriptAnalysisSettingsPath, newSettingsPath, StringComparison.OrdinalIgnoreCase)) { - this.editorSession.AnalysisService.SettingsPath = newSettingsPath; - settingsPathChanged = true; + if (this.editorSession.AnalysisService != null) + { + this.editorSession.AnalysisService.SettingsPath = newSettingsPath; + settingsPathChanged = true; + } } // If script analysis settings have changed we need to clear & possibly update the current diagnostic records. - if ((oldScriptAnalysisEnabled != this.currentSettings.ScriptAnalysis.Enable) || settingsPathChanged) + if ((oldScriptAnalysisEnabled != this.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. @@ -1443,7 +1446,7 @@ private Task RunScriptDiagnostics( DelayThenInvokeDiagnostics( 750, filesToAnalyze, - this.currentSettings.ScriptAnalysis.Enable.Value, + this.currentSettings.ScriptAnalysis?.Enable.Value ?? false, this.codeActionsPerFile, editorSession, eventSender, diff --git a/src/PowerShellEditorServices/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Analysis/AnalysisService.cs index 67bec2aa4..00d2be6a4 100644 --- a/src/PowerShellEditorServices/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Analysis/AnalysisService.cs @@ -14,6 +14,7 @@ using System.Text; using System.Collections; using System.IO; +using Microsoft.PowerShell.Commands; namespace Microsoft.PowerShell.EditorServices { @@ -25,23 +26,38 @@ public class AnalysisService : IDisposable { #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; - private RunspacePool _analysisRunspacePool; - private bool _hasScriptAnalyzerModule; + /// + /// Runspace pool to generate runspaces for script analysis and handle + /// ansynchronous analysis requests. + /// + private RunspacePool _analysisRunspacePool; - private string[] activeRules; - private string settingsPath; + /// + /// Info object describing the PSScriptAnalyzer module that has been loaded in + /// to provide analysis services. + /// + private PSModuleInfo _pssaModuleInfo; /// /// Defines the list of Script Analyzer rules to include by default if /// no settings file is specified. /// - private static readonly string[] IncludedRules = new string[] + private static readonly string[] s_includedRules = new string[] { "PSUseToExportFieldsInManifest", "PSMisleadingBacktick", @@ -66,90 +82,118 @@ public class AnalysisService : IDisposable #region Properties /// - /// Set of PSScriptAnalyzer rules used for analysis + /// Set of PSScriptAnalyzer rules used for analysis. /// - public string[] ActiveRules - { - get - { - return activeRules; - } - - set - { - activeRules = value; - } - } + public string[] ActiveRules { get; set; } /// /// Gets or sets the path to a settings file (.psd1) /// containing PSScriptAnalyzer settings. /// - public string SettingsPath - { - get - { - return settingsPath; - } - set - { - settingsPath = value; - } - } + public string SettingsPath { get; set; } #endregion #region Constructors /// - /// Creates an instance of the AnalysisService class. + /// Construct a new AnalysisService object. /// - /// Path to a PSScriptAnalyzer settings file. - /// An ILogger implementation used for writing log messages. - public AnalysisService(string settingsPath, ILogger logger) + /// + /// 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, + ILogger logger, + PSModuleInfo pssaModuleInfo = null) { - this._logger = logger; + _analysisRunspacePool = analysisRunspacePool; + SettingsPath = pssaSettingsPath; + ActiveRules = activeRules.ToArray(); + _logger = logger; + _pssaModuleInfo = pssaModuleInfo; + } + + #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(string settingsPath, ILogger logger) + { try { - this.SettingsPath = settingsPath; + 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); - if (!(_hasScriptAnalyzerModule = VerifyPSScriptAnalyzerAvailable())) + } + catch (Exception e) { - throw new Exception("PSScriptAnalyzer module not available"); + throw new AnalysisServiceLoadException("PSScriptAnalyzer runspace pool could not be created", e); } - // Create a base session state with PSScriptAnalyzer loaded - InitialSessionState sessionState = InitialSessionState.CreateDefault2(); - sessionState.ImportPSModule(new [] { PSSA_MODULE_NAME }); - - // runspacepool takes care of queuing commands for us so we do not - // need to worry about executing concurrent commands - this._analysisRunspacePool = RunspaceFactory.CreateRunspacePool(sessionState); + 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 + // Having more than one runspace doesn't block code formatting if one // runspace is occupied for diagnostics - this._analysisRunspacePool.SetMaxRunspaces(NumRunspaces); - this._analysisRunspacePool.ThreadOptions = PSThreadOptions.ReuseThread; - this._analysisRunspacePool.Open(); + analysisRunspacePool.SetMaxRunspaces(NumRunspaces); + analysisRunspacePool.ThreadOptions = PSThreadOptions.ReuseThread; + analysisRunspacePool.Open(); + + var analysisService = new AnalysisService( + analysisRunspacePool, + settingsPath, + s_includedRules, + logger, + pssaModuleInfo); - ActiveRules = IncludedRules.ToArray(); - EnumeratePSScriptAnalyzerCmdlets(); - EnumeratePSScriptAnalyzerRules(); + // Log what features are available in PSSA here + analysisService.LogAvailablePssaFeatures(); + + return analysisService; + } + catch (AnalysisServiceLoadException e) + { + logger.WriteException("PSScriptAnalyzer cannot be imported, AnalysisService will be disabled", e); + return null; } catch (Exception e) { - var sb = new StringBuilder(); - sb.AppendLine("PSScriptAnalyzer cannot be imported, AnalysisService will be disabled."); - sb.AppendLine(e.Message); - this._logger.Write(LogLevel.Warning, sb.ToString()); + logger.WriteException("AnalysisService could not be started due to an unexpected exception", e); + return null; } } - #endregion // constructors - - #region Public Methods - /// /// Get PSScriptAnalyzer settings hashtable for PSProvideCommentHelp rule. /// @@ -206,7 +250,7 @@ public static Hashtable GetPSSASettingsHashtable(IDictionary /// An array of ScriptFileMarkers containing semantic analysis results. public async Task GetSemanticMarkersAsync(ScriptFile file) { - return await GetSemanticMarkersAsync(file, activeRules, settingsPath); + return await GetSemanticMarkersAsync(file, ActiveRules, SettingsPath); } /// @@ -239,13 +283,10 @@ public async Task GetSemanticMarkersAsync( public IEnumerable GetPSScriptAnalyzerRules() { List ruleNames = new List(); - if (_hasScriptAnalyzerModule) + var ruleObjects = InvokePowerShell("Get-ScriptAnalyzerRule", new Dictionary()); + foreach (var rule in ruleObjects) { - var ruleObjects = InvokePowerShell("Get-ScriptAnalyzerRule", new Dictionary()); - foreach (var rule in ruleObjects) - { - ruleNames.Add((string)rule.Members["RuleName"].Value); - } + ruleNames.Add((string)rule.Members["RuleName"].Value); } return ruleNames; @@ -265,7 +306,7 @@ public async Task Format( { // 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 (!_hasScriptAnalyzerModule || string.IsNullOrEmpty(scriptDefinition)) + if (string.IsNullOrEmpty(scriptDefinition)) { return null; } @@ -283,19 +324,6 @@ public async Task Format( return result?.Select(r => r?.ImmediateBaseObject as string).FirstOrDefault(); } - /// - /// Disposes the runspace being used by the analysis service. - /// - public void Dispose() - { - if (this._analysisRunspacePool != null) - { - this._analysisRunspacePool.Close(); - this._analysisRunspacePool.Dispose(); - this._analysisRunspacePool = null; - } - } - #endregion // public methods #region Private Methods @@ -305,8 +333,7 @@ private async Task GetSemanticMarkersAsync( string[] rules, TSettings settings) where TSettings : class { - if (_hasScriptAnalyzerModule - && file.IsAnalysisEnabled) + if (file.IsAnalysisEnabled) { return await GetSemanticMarkersAsync( file.Contents, @@ -338,43 +365,61 @@ private async Task GetSemanticMarkersAsync( } } - private void EnumeratePSScriptAnalyzerCmdlets() + /// + /// Log the features available from the PSScriptAnalyzer module that has been imported + /// for use with the AnalysisService. + /// + private void LogAvailablePssaFeatures() { - if (_hasScriptAnalyzerModule) + // Save ourselves some work here + var featureLogLevel = LogLevel.Verbose; + if (_logger.MinimumConfiguredLogLevel > featureLogLevel) { - var sb = new StringBuilder(); - var commands = InvokePowerShell( - "Get-Command", - new Dictionary - { - {"Module", "PSScriptAnalyzer"} - }); + return; + } - var commandNames = commands? - .Select(c => c.ImmediateBaseObject as CmdletInfo) - .Where(c => c != null) - .Select(c => c.Name) ?? Enumerable.Empty(); + // If we already know the module that was imported, save some work + if (_pssaModuleInfo == null) + { + PSObject[] modules = InvokePowerShell( + "Get-Module", + new Dictionary{ {"Name", PSSA_MODULE_NAME} }); + + _pssaModuleInfo = modules + .Select(m => m.BaseObject) + .OfType() + .FirstOrDefault(); + } - sb.AppendLine("The following cmdlets are available in the imported PSScriptAnalyzer module:"); - sb.AppendLine(String.Join(Environment.NewLine, commandNames.Select(s => " " + s))); - this._logger.Write(LogLevel.Verbose, sb.ToString()); + if (_pssaModuleInfo == null) + { + throw new AnalysisServiceLoadException("Unable to find loaded PSScriptAnalyzer module for logging"); } - } - private void EnumeratePSScriptAnalyzerRules() - { - if (_hasScriptAnalyzerModule) + 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)) { - var rules = GetPSScriptAnalyzerRules(); - var sb = new StringBuilder(); - sb.AppendLine("Available PSScriptAnalyzer Rules:"); - foreach (var rule in rules) - { - sb.AppendLine(rule); - } + sb.Append(" "); + sb.AppendLine(cmdletName); + } - this._logger.Write(LogLevel.Verbose, sb.ToString()); + // Log available rules + sb.AppendLine(" Available Rules:"); + foreach (string ruleName in GetPSScriptAnalyzerRules()) + { + sb.Append(" "); + sb.AppendLine(ruleName); } + + _logger.Write(featureLogLevel, sb.ToString()); } private async Task GetDiagnosticRecordsAsync( @@ -392,9 +437,7 @@ private async Task GetDiagnosticRecordsAsync( return diagnosticRecords; } - if (_hasScriptAnalyzerModule - && (typeof(TSettings) == typeof(string) - || typeof(TSettings) == typeof(Hashtable))) + if (typeof(TSettings) == typeof(string) || typeof(TSettings) == typeof(Hashtable)) { //Use a settings file if one is provided, otherwise use the default rule list. string settingParameter; @@ -419,19 +462,18 @@ private async Task GetDiagnosticRecordsAsync( }); } - this._logger.Write( + _logger.Write( LogLevel.Verbose, String.Format("Found {0} violations", diagnosticRecords.Count())); return diagnosticRecords; } - private PSObject[] InvokePowerShell(string command, IDictionary paramArgMap) { using (var powerShell = System.Management.Automation.PowerShell.Create()) { - powerShell.RunspacePool = this._analysisRunspacePool; + powerShell.RunspacePool = _analysisRunspacePool; powerShell.AddCommand(command); foreach (var kvp in paramArgMap) { @@ -447,7 +489,7 @@ private PSObject[] InvokePowerShell(string command, IDictionary { // This exception is possible if the module path loaded // is wrong even though PSScriptAnalyzer is available as a module - this._logger.Write(LogLevel.Error, ex.Message); + _logger.Write(LogLevel.Error, ex.Message); } catch (CmdletInvocationException ex) { @@ -455,7 +497,7 @@ private PSObject[] InvokePowerShell(string command, IDictionary // 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" - this._logger.Write(LogLevel.Error, ex.Message); + _logger.Write(LogLevel.Error, ex.Message); } return result; @@ -472,25 +514,107 @@ private async Task InvokePowerShellAsync(string command, IDictionary return await task; } - private bool VerifyPSScriptAnalyzerAvailable() + /// + /// 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 { - return ps.Invoke()?.Any() ?? false; + // Get the latest version of PSScriptAnalyzer we can find + pssaModuleInfo = ps.Invoke()? + .Select(psObj => psObj.BaseObject) + .OfType() + .OrderBy(moduleInfo => moduleInfo.Version) + .FirstOrDefault(); + } + catch (Exception e) + { + throw new AnalysisServiceLoadException("Unable to find PSScriptAnalyzer module on the module path", e); } - catch (Exception) + + if (pssaModuleInfo == null) { - return false; + throw new AnalysisServiceLoadException("Unable to find PSScriptAnalyzer module on the module path"); } + + // Create a base session state with PSScriptAnalyzer loaded + InitialSessionState 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 + } + + /// + /// 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/Extensions/EditorObject.cs b/src/PowerShellEditorServices/Extensions/EditorObject.cs index 54605ebd4..e8f50d589 100644 --- a/src/PowerShellEditorServices/Extensions/EditorObject.cs +++ b/src/PowerShellEditorServices/Extensions/EditorObject.cs @@ -93,7 +93,7 @@ public void UnregisterCommand(string commandName) /// /// Returns all registered EditorCommands. /// - /// An Array of all registered EditorCommands. + /// An Array of all registered EditorCommands. public EditorCommand[] GetCommands() { return this.extensionService.GetCommands(); diff --git a/src/PowerShellEditorServices/Extensions/ExtensionService.cs b/src/PowerShellEditorServices/Extensions/ExtensionService.cs index 82b948931..d9defe6a3 100644 --- a/src/PowerShellEditorServices/Extensions/ExtensionService.cs +++ b/src/PowerShellEditorServices/Extensions/ExtensionService.cs @@ -173,7 +173,7 @@ public void UnregisterCommand(string commandName) /// /// Returns all registered EditorCommands. /// - /// An Array of all registered EditorCommands. + /// An Array of all registered EditorCommands. public EditorCommand[] GetCommands() { EditorCommand[] commands = new EditorCommand[this.editorCommands.Count]; diff --git a/src/PowerShellEditorServices/Session/EditorSession.cs b/src/PowerShellEditorServices/Session/EditorSession.cs index 566b2135e..8bc751a9f 100644 --- a/src/PowerShellEditorServices/Session/EditorSession.cs +++ b/src/PowerShellEditorServices/Session/EditorSession.cs @@ -160,18 +160,8 @@ public void StartDebugService(IEditorOperations editorOperations) internal void InstantiateAnalysisService(string settingsPath = null) { - // AnalysisService will throw FileNotFoundException if - // Script Analyzer binaries are not included. - try - { - this.AnalysisService = new AnalysisService(settingsPath, this.logger); - } - catch (FileNotFoundException) - { - this.logger.Write( - LogLevel.Warning, - "Script Analyzer binaries not found, AnalysisService will be disabled."); - } + // Create the analysis service. If this fails, the result will be null -- any exceptions are caught and logged + this.AnalysisService = AnalysisService.Create(settingsPath, this.logger); } #endregion diff --git a/src/PowerShellEditorServices/Utility/Logging.cs b/src/PowerShellEditorServices/Utility/Logging.cs index dd7f0a1be..1a0f7d6fe 100644 --- a/src/PowerShellEditorServices/Utility/Logging.cs +++ b/src/PowerShellEditorServices/Utility/Logging.cs @@ -44,6 +44,11 @@ public enum LogLevel /// public interface ILogger : IDisposable { + /// + /// The minimum log level that this logger instance is configured to log at. + /// + LogLevel MinimumConfiguredLogLevel { get; } + /// /// Write a message with the given severity to the logs. /// @@ -180,7 +185,7 @@ public ILogger Build() ); } - return new PsesLogger(configuration.CreateLogger()); + return new PsesLogger(configuration.CreateLogger(), _logLevel); } } @@ -210,7 +215,7 @@ public static Builder CreateLogger() /// Convert an EditorServices log level to a Serilog log level. /// /// The EditorServices log level. - /// The Serilog LogEventLevel corresponding to the EditorServices log level. + /// The Serilog LogEventLevel corresponding to the EditorServices log level. private static LogEventLevel ConvertLogLevel(LogLevel logLevel) { switch (logLevel) diff --git a/src/PowerShellEditorServices/Utility/PsesLogger.cs b/src/PowerShellEditorServices/Utility/PsesLogger.cs index 85777a813..2d00e57b2 100644 --- a/src/PowerShellEditorServices/Utility/PsesLogger.cs +++ b/src/PowerShellEditorServices/Utility/PsesLogger.cs @@ -24,11 +24,18 @@ public class PsesLogger : ILogger /// Construct a new logger around a Serilog ILogger. /// /// The Serilog logger to use internally. - internal PsesLogger(Logger logger) + /// The minimum severity level the logger is configured to log messages at. + internal PsesLogger(Logger logger, LogLevel minimumLogLevel) { _logger = logger; + MinimumConfiguredLogLevel = minimumLogLevel; } + /// + /// The minimum log level that this logger is configured to log at. + /// + public LogLevel MinimumConfiguredLogLevel { get; } + /// /// Write a message with the given severity to the logs. ///