From c91e4114494e6d940c4ce9633989d0d75a95cac9 Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Thu, 31 Oct 2019 07:11:00 -0400 Subject: [PATCH 1/7] minimum change needed --- .../PowerShellEditorServices.psm1 | 3 + .../PowerShellEditorServices.csproj | 1 + .../Server/PsesServiceCollectionExtensions.cs | 9 +- .../Services/Analysis/AnalysisService.cs | 816 +++--------------- .../Handlers/TextDocumentHandler.cs | 4 +- .../Services/TextDocument/ScriptFileMarker.cs | 18 +- 6 files changed, 131 insertions(+), 720 deletions(-) diff --git a/module/PowerShellEditorServices/PowerShellEditorServices.psm1 b/module/PowerShellEditorServices/PowerShellEditorServices.psm1 index 770e516f8..66f8878a9 100644 --- a/module/PowerShellEditorServices/PowerShellEditorServices.psm1 +++ b/module/PowerShellEditorServices/PowerShellEditorServices.psm1 @@ -92,6 +92,9 @@ function Start-EditorServicesHost { $WaitForDebugger ) + # Make sure PSScriptAnalyzer dlls are loaded. + Import-Module PSScriptAnalyzer + $editorServicesHost = $null $hostDetails = Microsoft.PowerShell.Utility\New-Object Microsoft.PowerShell.EditorServices.Hosting.HostDetails @( diff --git a/src/PowerShellEditorServices/PowerShellEditorServices.csproj b/src/PowerShellEditorServices/PowerShellEditorServices.csproj index 843ee3d7f..17c60b711 100644 --- a/src/PowerShellEditorServices/PowerShellEditorServices.csproj +++ b/src/PowerShellEditorServices/PowerShellEditorServices.csproj @@ -31,5 +31,6 @@ + diff --git a/src/PowerShellEditorServices/Server/PsesServiceCollectionExtensions.cs b/src/PowerShellEditorServices/Server/PsesServiceCollectionExtensions.cs index 074178567..4542b8597 100644 --- a/src/PowerShellEditorServices/Server/PsesServiceCollectionExtensions.cs +++ b/src/PowerShellEditorServices/Server/PsesServiceCollectionExtensions.cs @@ -55,14 +55,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 a44e73427..0bc342a70 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -1,4 +1,4 @@ -// +// // Copyright (c) Microsoft. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. // @@ -6,8 +6,6 @@ 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; @@ -17,6 +15,9 @@ using System.Threading; using System.Collections.Concurrent; using Microsoft.PowerShell.EditorServices.Services.TextDocument; +using Microsoft.Windows.PowerShell.ScriptAnalyzer.Hosting; +using Microsoft.Windows.PowerShell.ScriptAnalyzer; +using Microsoft.PowerShell.EditorServices.Utility; namespace Microsoft.PowerShell.EditorServices.Services { @@ -24,9 +25,9 @@ namespace Microsoft.PowerShell.EditorServices.Services /// Provides a high-level service for performing semantic analysis /// of PowerShell scripts. /// - public class AnalysisService : IDisposable + public class AnalysisService { - #region Static fields + #region Fields /// /// Defines the list of Script Analyzer rules to include by default if @@ -50,182 +51,53 @@ public class AnalysisService : IDisposable "PSPossibleIncorrectUsageOfRedirectionOperator" }; - /// - /// An empty diagnostic result to return when a script fails analysis. - /// - private static readonly PSObject[] s_emptyDiagnosticResult = new PSObject[0]; - - private static readonly string[] s_emptyGetRuleResult = new string[0]; - - 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; - - /// - /// Info object describing the PSScriptAnalyzer module that has been loaded in - /// to provide analysis services. - /// - private PSModuleInfo _pssaModuleInfo; - + private readonly ILogger _logger; + private readonly HostedAnalyzer _analyzer; + private readonly Settings _analyzerSettings; private readonly ILanguageServer _languageServer; - private readonly ConfigurationService _configurationService; - private readonly ConcurrentDictionary)> _mostRecentCorrectionsByFile; - #endregion // Private Fields + private CancellationTokenSource _existingRequestCancellation; + private readonly SemaphoreSlim _existingRequestCancellationLock; + + #endregion #region Properties /// /// Set of PSScriptAnalyzer rules used for analysis. /// - public string[] ActiveRules { get; set; } + public string[] ActiveRules => s_includedRules; /// /// Gets or sets the path to a settings file (.psd1) /// containing PSScriptAnalyzer settings. /// - public string SettingsPath { get; set; } + public string SettingsPath { get; internal set; } #endregion #region Constructors - /// - /// 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) + public AnalysisService(ConfigurationService configurationService, ILanguageServer languageServer, ILoggerFactory factory) { - _analysisRunspacePool = analysisRunspacePool; - SettingsPath = pssaSettingsPath; - ActiveRules = activeRules.ToArray(); - _languageServer = languageServer; + SettingsPath = configurationService.CurrentSettings.ScriptAnalysis.SettingsPath; + _logger = factory.CreateLogger(); + _analyzer = new HostedAnalyzer(); + // TODO: use our rules + _analyzerSettings = _analyzer.CreateSettings("PSGallery"); _configurationService = configurationService; - _logger = logger; - _pssaModuleInfo = pssaModuleInfo; + _languageServer = languageServer; _mostRecentCorrectionsByFile = new ConcurrentDictionary)>(); + _existingRequestCancellation = new CancellationTokenSource(); + _existingRequestCancellationLock = AsyncUtils.CreateSimpleLockingSemaphore(); } - #endregion // constructors + #endregion #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) - { - 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; - } - } - /// /// Get PSScriptAnalyzer settings hashtable for PSProvideCommentHelp rule. /// @@ -250,23 +122,14 @@ public static Hashtable GetCommentHelpRuleSettings( 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) - { var hashtable = new Hashtable(); var ruleSettingsHashtable = new Hashtable(); - hashtable["IncludeRules"] = ruleSettingsMap.Keys.ToArray(); + hashtable["IncludeRules"] = settings.Keys.ToArray(); hashtable["Rules"] = ruleSettingsHashtable; - foreach (var kvp in ruleSettingsMap) + foreach (var kvp in settings) { ruleSettingsHashtable.Add(kvp.Key, kvp.Value); } @@ -274,28 +137,6 @@ public static Hashtable GetPSSASettingsHashtable(IDictionary 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 async Task> GetSemanticMarkersAsync(ScriptFile file) - { - return await GetSemanticMarkersAsync(file, ActiveRules, SettingsPath); - } - - /// - /// Perform semantic analysis on the given ScriptFile with the given settings. - /// - /// The ScriptFile to be analyzed. - /// ScriptAnalyzer settings - /// - public async Task> GetSemanticMarkersAsync(ScriptFile file, Hashtable settings) - { - return await GetSemanticMarkersAsync(file, null, settings); - } - /// /// Perform semantic analysis on the given script with the given settings. /// @@ -306,28 +147,11 @@ public async Task> GetSemanticMarkersAsync( string scriptContent, Hashtable settings) { - return await 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; - } + AnalyzerResult analyzerResult = await _analyzer.AnalyzeAsync( + scriptContent, + _analyzer.CreateSettings(settings)); - var ruleNames = new List(); - foreach (var rule in getRuleResult.Output) - { - ruleNames.Add((string)rule.Members["RuleName"].Value); - } - - return ruleNames; + return analyzerResult.Result.Select(ScriptFileMarker.FromDiagnosticRecord).ToList(); } /// @@ -349,469 +173,147 @@ public async Task FormatAsync( return null; } - var argsDict = new Dictionary { - {"ScriptDefinition", scriptDefinition}, - {"Settings", settings} - }; - if (rangeList != null) - { - argsDict.Add("Range", rangeList); - } - - PowerShellResult result = await InvokePowerShellAsync("Invoke-Formatter", argsDict); - - 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; + // TODO: support rangeList + return await _analyzer.FormatAsync(scriptDefinition, _analyzer.CreateSettings(settings)); } - #endregion // public methods - - #region Private Methods - - private async Task> GetSemanticMarkersAsync( - ScriptFile file, - string[] rules, - TSettings settings) where TSettings : class + public async Task RunScriptDiagnosticsAsync( + ScriptFile[] filesToAnalyze, + CancellationToken token) { - if (file.IsAnalysisEnabled) - { - return await GetSemanticMarkersAsync( - file.Contents, - rules, - settings); - } - else - { - // Return an empty marker list - return new List(); - } - } + // This token will be cancelled (typically via LSP cancellation) if the token passed in is cancelled or if + // multiple requests come in (the last one wins). + CancellationToken ct; - 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); - return scriptFileMarkers.Select(ScriptFileMarker.FromDiagnosticRecord).ToList(); - } - else + // If there's an existing task, attempt to cancel it + try { - // Return an empty marker list - return new List(); - } - } + await _existingRequestCancellationLock.WaitAsync(); + // Try to cancel the request + _existingRequestCancellation.Cancel(); - /// - /// 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 cancellation didn't throw an exception, + // clean up the existing token + _existingRequestCancellation.Dispose(); + _existingRequestCancellation = CancellationTokenSource.CreateLinkedTokenSource(token); + ct = _existingRequestCancellation.Token; } - - // If we already know the module that was imported, save some work - if (_pssaModuleInfo == null) + catch (Exception e) { - 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(); - } + // TODO: Catch a more specific exception! + _logger.LogError( + string.Format( + "Exception while canceling analysis task:\n\n{0}", + e.ToString())); - if (_pssaModuleInfo == null) - { - throw new AnalysisServiceLoadException("Unable to find loaded PSScriptAnalyzer module for logging"); + TaskCompletionSource cancelTask = new TaskCompletionSource(); + cancelTask.SetCanceled(); + return; } - - 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)) + finally { - sb.Append(" "); - sb.AppendLine(cmdletName); + _existingRequestCancellationLock.Release(); } - // Log available rules - sb.AppendLine(" Available Rules:"); - foreach (string ruleName in GetPSScriptAnalyzerRules()) + // If filesToAnalyze is empty, nothing to do so return early. + if (filesToAnalyze.Length == 0) { - sb.Append(" "); - sb.AppendLine(ruleName); + return; } - _logger.LogDebug(sb.ToString()); - } - - private async Task GetDiagnosticRecordsAsync( - string scriptContent, - string[] rules, - TSettings settings) where TSettings : class - { - var diagnosticRecords = s_emptyDiagnosticResult; - - // 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)) + try { - return diagnosticRecords; - } + // Wait for the desired delay period before analyzing the provided list of files. + await Task.Delay(750, ct); - 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; - object settingArgument; - if (settings != null) + foreach (ScriptFile file in filesToAnalyze) { - settingParameter = "Settings"; - settingArgument = settings; - } - else - { - settingParameter = "IncludeRule"; - settingArgument = rules; - } + AnalyzerResult analyzerResult = await _analyzer.AnalyzeAsync( + file.ScriptAst, + file.ScriptTokens, + _analyzerSettings, + file.FilePath); - 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 }} - }); - - diagnosticRecords = result?.Output; - } - - _logger.LogDebug(String.Format("Found {0} violations", diagnosticRecords.Count())); - - return diagnosticRecords; - } - - 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) + if (!ct.CanBeCanceled || ct.IsCancellationRequested) { - powerShell.AddParameter(kvp.Key, kvp.Value); + break; } - } - 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); + IEnumerable scriptFileMarkers = analyzerResult.Result.Select(ScriptFileMarker.FromDiagnosticRecord); + file.DiagnosticMarkers.AddRange(scriptFileMarkers); } - 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 async Task InvokePowerShellAsync(string command, IDictionary paramArgMap = null) - { - var task = Task.Run(() => - { - return InvokePowerShell(command, paramArgMap); - }); - - return await task; - } - - /// - /// 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()) + catch (TaskCanceledException) { - // 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); + // If a cancellation was requested, then publish what we have. } - } - - #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) + foreach (var file in filesToAnalyze) { - if (disposing) - { - _analysisRunspacePool.Dispose(); - _analysisRunspacePool = null; - } - - _disposedValue = true; + PublishScriptDiagnostics( + file, + file.DiagnosticMarkers); } } - /// - /// Clean up all internal resources and dispose of the analysis service. - /// - public void Dispose() + public void ClearMarkers(ScriptFile scriptFile) { - // Do not change this code. Put cleanup code in Dispose(bool disposing) above. - Dispose(true); + // send empty diagnostic markers to clear any markers associated with the given file. + PublishScriptDiagnostics( + scriptFile, + new List()); } - #endregion - - /// - /// Wraps the result of an execution of PowerShell to send back through - /// asynchronous calls. - /// - private class PowerShellResult + public async Task> GetMostRecentCodeActionsForFileAsync(string documentUri) { - public PowerShellResult( - PSObject[] output, - ErrorRecord[] errors, - bool hasErrors) + if (!_mostRecentCorrectionsByFile.TryGetValue(documentUri, out (SemaphoreSlim fileLock, Dictionary corrections) fileCorrectionsEntry)) { - Output = output; - Errors = errors; - HasErrors = hasErrors; + return null; } - public PSObject[] Output { get; } - - public ErrorRecord[] Errors { get; } - - public bool HasErrors { get; } - } - - internal async Task RunScriptDiagnosticsAsync( - ScriptFile[] filesToAnalyze) - { - // If there's an existing task, attempt to cancel it + await fileCorrectionsEntry.fileLock.WaitAsync(); + // We must copy the dictionary for thread safety + var corrections = new Dictionary(fileCorrectionsEntry.corrections.Count); try { - if (s_existingRequestCancellation != null) + foreach (KeyValuePair correction in fileCorrectionsEntry.corrections) { - // 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; + corrections.Add(correction.Key, correction.Value); } - } - 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; + return corrections; } - - // If filesToAnalzye is empty, nothing to do so return early. - if (filesToAnalyze.Length == 0) + finally { - return; + fileCorrectionsEntry.fileLock.Release(); } - - // 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(); - await Task.Factory.StartNew( - () => - DelayThenInvokeDiagnosticsAsync( - 750, - filesToAnalyze, - _configurationService.CurrentSettings.ScriptAnalysis.Enable ?? false, - s_existingRequestCancellation.Token), - CancellationToken.None, - TaskCreationOptions.None, - TaskScheduler.Default); } - private async Task DelayThenInvokeDiagnosticsAsync( - int delayMilliseconds, - ScriptFile[] filesToAnalyze, - bool isScriptAnalysisEnabled, - CancellationToken cancellationToken) + internal static string GetUniqueIdFromDiagnostic(Diagnostic diagnostic) { - // First of all, wait for the desired delay period before - // analyzing the provided list of files - try - { - await Task.Delay(delayMilliseconds, cancellationToken); - } - 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); - } - else - { - // Semantic markers aren't available if the AnalysisService - // isn't available - semanticMarkers = new List(); - } + OmniSharp.Extensions.LanguageServer.Protocol.Models.Position start = diagnostic.Range.Start; + OmniSharp.Extensions.LanguageServer.Protocol.Models.Position end = diagnostic.Range.End; - scriptFile.DiagnosticMarkers.AddRange(semanticMarkers); + 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); - PublishScriptDiagnostics( - scriptFile, - // Concat script analysis errors to any existing parse errors - scriptFile.DiagnosticMarkers); - } + var id = sb.ToString(); + return id; } - internal void ClearMarkers(ScriptFile scriptFile) - { - // send empty diagnostic markers to clear any markers associated with the given file - PublishScriptDiagnostics( - scriptFile, - new List()); - } + #endregion private void PublishScriptDiagnostics( ScriptFile scriptFile, @@ -877,57 +379,6 @@ private void PublishScriptDiagnostics( }); } - public async Task> GetMostRecentCodeActionsForFileAsync(string documentUri) - { - if (!_mostRecentCorrectionsByFile.TryGetValue(documentUri, out (SemaphoreSlim fileLock, Dictionary corrections) fileCorrectionsEntry)) - { - return null; - } - - await fileCorrectionsEntry.fileLock.WaitAsync(); - // 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 @@ -936,14 +387,14 @@ private static Diagnostic GetDiagnosticFromMarker(ScriptFileMarker scriptFileMar Message = scriptFileMarker.Message, Code = scriptFileMarker.RuleName, Source = scriptFileMarker.Source, - Range = new Range + Range = new OmniSharp.Extensions.LanguageServer.Protocol.Models.Range { - Start = new Position + Start = new OmniSharp.Extensions.LanguageServer.Protocol.Models.Position { Line = scriptFileMarker.ScriptRegion.StartLineNumber - 1, Character = scriptFileMarker.ScriptRegion.StartColumnNumber - 1 }, - End = new Position + End = new OmniSharp.Extensions.LanguageServer.Protocol.Models.Position { Line = scriptFileMarker.ScriptRegion.EndLineNumber - 1, Character = scriptFileMarker.ScriptRegion.EndColumnNumber - 1 @@ -970,29 +421,4 @@ private static DiagnosticSeverity MapDiagnosticSeverity(ScriptFileMarkerLevel ma } } } - - /// - /// 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/TextDocument/Handlers/TextDocumentHandler.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/TextDocumentHandler.cs index 9bf4546eb..9dc8cbc5f 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/TextDocumentHandler.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/TextDocumentHandler.cs @@ -63,7 +63,7 @@ public Task Handle(DidChangeTextDocumentParams notification, CancellationT } // TODO: Get all recently edited files in the workspace - _analysisService.RunScriptDiagnosticsAsync(new ScriptFile[] { changedFile }); + _analysisService.RunScriptDiagnosticsAsync(new ScriptFile[] { changedFile }, token); return Unit.Task; } @@ -89,7 +89,7 @@ public Task Handle(DidOpenTextDocumentParams notification, CancellationTok notification.TextDocument.Text); // TODO: Get all recently edited files in the workspace - _analysisService.RunScriptDiagnosticsAsync(new ScriptFile[] { openedFile }); + _analysisService.RunScriptDiagnosticsAsync(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..b5c03208c 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/ScriptFileMarker.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/ScriptFileMarker.cs @@ -9,6 +9,7 @@ using System.Linq; using System.Management.Automation.Language; using Microsoft.PowerShell.EditorServices.Utility; +using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; namespace Microsoft.PowerShell.EditorServices.Services.TextDocument { @@ -120,24 +121,11 @@ private static string GetIfExistsString(PSObject psobj, string memberName) } } - internal static ScriptFileMarker FromDiagnosticRecord(PSObject psObject) + internal static ScriptFileMarker FromDiagnosticRecord(DiagnosticRecord diagnosticRecord) { - Validate.IsNotNull("psObject", psObject); + Validate.IsNotNull("diagnosticRecord", diagnosticRecord); MarkerCorrection correction = null; - // make sure psobject is of type DiagnosticRecord - if (!psObject.TypeNames.Contains( - "Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord", - StringComparer.OrdinalIgnoreCase)) - { - throw new ArgumentException("Input PSObject must of DiagnosticRecord type."); - } - - // casting psobject to dynamic allows us to access - // the diagnostic record's properties directly i.e. . - // without having to go through PSObject's Members property. - var diagnosticRecord = psObject as dynamic; - if (diagnosticRecord.SuggestedCorrections != null) { var editRegions = new List(); From a038c32d9a3a46ca93fc5f80415215b65ff17a0e Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Wed, 27 Nov 2019 08:33:51 -0800 Subject: [PATCH 2/7] remove ScriptFileMarker type and refactor comment-based help --- NuGet.Config | 1 + .../Services/Analysis/AnalysisService.cs | 249 +++++++++--------- .../Analysis/DiagnosticCreationHelper.cs | 88 +++++++ .../Services/Analysis/MarkerCorrection.cs | 25 ++ .../Handlers/GetCommentHelpHandler.cs | 17 +- .../Handlers/CodeActionHandler.cs | 2 +- .../Services/TextDocument/ScriptFile.cs | 6 +- .../Services/TextDocument/ScriptFileMarker.cs | 178 ------------- 8 files changed, 239 insertions(+), 327 deletions(-) create mode 100644 src/PowerShellEditorServices/Services/Analysis/DiagnosticCreationHelper.cs create mode 100644 src/PowerShellEditorServices/Services/Analysis/MarkerCorrection.cs delete mode 100644 src/PowerShellEditorServices/Services/TextDocument/ScriptFileMarker.cs diff --git a/NuGet.Config b/NuGet.Config index 2f64451ed..45dd090e9 100644 --- a/NuGet.Config +++ b/NuGet.Config @@ -6,5 +6,6 @@ + diff --git a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs index 0bc342a70..ebd0d4b5e 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -18,6 +18,8 @@ using Microsoft.Windows.PowerShell.ScriptAnalyzer.Hosting; using Microsoft.Windows.PowerShell.ScriptAnalyzer; using Microsoft.PowerShell.EditorServices.Utility; +using Microsoft.PowerShell.EditorServices.Services.Analysis; +using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; namespace Microsoft.PowerShell.EditorServices.Services { @@ -99,42 +101,31 @@ public AnalysisService(ConfigurationService configurationService, ILanguageServe #region Public Methods /// - /// Get PSScriptAnalyzer settings hashtable for PSProvideCommentHelp rule. + /// Get PSScriptAnalyzer settings 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( + /// A PSScriptAnalyzer settings. + public Settings 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); - - var hashtable = new Hashtable(); - var ruleSettingsHashtable = new Hashtable(); - - hashtable["IncludeRules"] = settings.Keys.ToArray(); - hashtable["Rules"] = ruleSettingsHashtable; - - foreach (var kvp in settings) - { - ruleSettingsHashtable.Add(kvp.Key, kvp.Value); - } + var pssaSettings = _analyzer.CreateSettings(); + pssaSettings.AddRuleArgument("PSProvideCommentHelp", new Dictionary{ + { "Enable", enable }, + { "ExportedOnly", exportedOnly }, + { "BlockComment", blockComment }, + { "VSCodeSnippetCorrection", vscodeSnippetCorrection }, + { "Placement", placement } + }); - return hashtable; + return pssaSettings; } /// @@ -143,15 +134,15 @@ public static Hashtable GetCommentHelpRuleSettings( /// The script content to be analyzed. /// ScriptAnalyzer settings /// - public async Task> GetSemanticMarkersAsync( + public async Task> GetSemanticMarkersAsync( string scriptContent, - Hashtable settings) + Settings settings) { AnalyzerResult analyzerResult = await _analyzer.AnalyzeAsync( scriptContent, - _analyzer.CreateSettings(settings)); + settings); - return analyzerResult.Result.Select(ScriptFileMarker.FromDiagnosticRecord).ToList(); + return analyzerResult.Result.Select(DiagnosticCreationHelper.FromDiagnosticRecord).ToList(); } /// @@ -228,6 +219,11 @@ public async Task RunScriptDiagnosticsAsync( foreach (ScriptFile file in filesToAnalyze) { + if (!ct.CanBeCanceled || ct.IsCancellationRequested) + { + break; + } + AnalyzerResult analyzerResult = await _analyzer.AnalyzeAsync( file.ScriptAst, file.ScriptTokens, @@ -239,8 +235,77 @@ public async Task RunScriptDiagnosticsAsync( break; } - IEnumerable scriptFileMarkers = analyzerResult.Result.Select(ScriptFileMarker.FromDiagnosticRecord); - file.DiagnosticMarkers.AddRange(scriptFileMarkers); + // Create the entry for this file if it does not already exist + SemaphoreSlim fileLock; + Dictionary fileCorrections; + bool newEntryNeeded = false; + if (_mostRecentCorrectionsByFile.TryGetValue(file.DocumentUri, out (SemaphoreSlim, Dictionary) fileCorrectionsEntry)) + { + fileLock = fileCorrectionsEntry.Item1; + fileCorrections = fileCorrectionsEntry.Item2; + } + else + { + fileLock = AsyncUtils.CreateSimpleLockingSemaphore(); + fileCorrections = new Dictionary(); + newEntryNeeded = true; + } + + await fileLock.WaitAsync(); + 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[file.DocumentUri] = (fileLock, fileCorrections); + } + else + { + // Otherwise we need to clear the stale corrections + fileCorrections.Clear(); + } + + foreach (DiagnosticRecord diagnosticRecord in analyzerResult.Result) + { + var diagnostic = DiagnosticCreationHelper.FromDiagnosticRecord(diagnosticRecord); + file.DiagnosticMarkers.Add(diagnostic); + + // Does the marker contain a correction? + //Diagnostic markerDiagnostic = GetDiagnosticFromMarker(marker); + if (diagnosticRecord.SuggestedCorrections != null) + { + var editRegions = new List(); + string correctionMessage = null; + foreach (dynamic suggestedCorrection in diagnosticRecord.SuggestedCorrections) + { + editRegions.Add( + new ScriptRegion( + diagnosticRecord.ScriptPath, + suggestedCorrection.Text, + startLineNumber: suggestedCorrection.StartLineNumber, + startColumnNumber: suggestedCorrection.StartColumnNumber, + endLineNumber: suggestedCorrection.EndLineNumber, + endColumnNumber: suggestedCorrection.EndColumnNumber, + startOffset: -1, + endOffset: -1)); + + correctionMessage = suggestedCorrection.Description; + } + + string diagnosticId = GetUniqueIdFromDiagnostic(diagnostic); + fileCorrections[diagnosticId] = new MarkerCorrection + { + Name = correctionMessage == null ? diagnosticRecord.Message : correctionMessage, + Edits = editRegions.ToArray() + }; + } + } + } + finally + { + fileLock.Release(); + } } } catch (TaskCanceledException) @@ -256,13 +321,8 @@ public async Task RunScriptDiagnosticsAsync( } } - public void ClearMarkers(ScriptFile scriptFile) - { - // send empty diagnostic markers to clear any markers associated with the given file. - PublishScriptDiagnostics( - scriptFile, - new List()); - } + // send empty diagnostic markers to clear any markers associated with the given file. + public void ClearMarkers(ScriptFile scriptFile) => PublishScriptDiagnostics(scriptFile, new List()); public async Task> GetMostRecentCodeActionsForFileAsync(string documentUri) { @@ -313,63 +373,34 @@ internal static string GetUniqueIdFromDiagnostic(Diagnostic diagnostic) return id; } + /// + /// Uses the PSScriptAnalyzer rule 'PSProvideCommentHelp' to get the comment-based help for a function string passed in. + /// + /// The string representation of the function we will get help for. + /// Use block comment or line comment. + /// Place comment help at the given location relative to the function definition. + /// A PSScriptAnalyzer settings. + internal async Task GetCommentHelpCorrectionTextAsync(string funcText, bool blockComment, string placement) + { + Settings commentHelpSettings = GetCommentHelpRuleSettings( + enable: true, + exportedOnly: false, + blockComment: blockComment, + vscodeSnippetCorrection: true, + placement: placement); + + AnalyzerResult analyzerResult = await _analyzer.AnalyzeAsync(funcText, commentHelpSettings); + return analyzerResult.Result + .Single(record => record.RuleName == "PSProvideCommentHelp") + .SuggestedCorrections.Single().Text; + } + #endregion private void PublishScriptDiagnostics( ScriptFile scriptFile, - List markers) + List diagnostics) { - 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() @@ -378,47 +409,5 @@ private void PublishScriptDiagnostics( Diagnostics = new Container(diagnostics), }); } - - private static Diagnostic GetDiagnosticFromMarker(ScriptFileMarker scriptFileMarker) - { - return new Diagnostic - { - Severity = MapDiagnosticSeverity(scriptFileMarker.Level), - Message = scriptFileMarker.Message, - Code = scriptFileMarker.RuleName, - Source = scriptFileMarker.Source, - Range = new OmniSharp.Extensions.LanguageServer.Protocol.Models.Range - { - Start = new OmniSharp.Extensions.LanguageServer.Protocol.Models.Position - { - Line = scriptFileMarker.ScriptRegion.StartLineNumber - 1, - Character = scriptFileMarker.ScriptRegion.StartColumnNumber - 1 - }, - End = new OmniSharp.Extensions.LanguageServer.Protocol.Models.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; - } - } } } diff --git a/src/PowerShellEditorServices/Services/Analysis/DiagnosticCreationHelper.cs b/src/PowerShellEditorServices/Services/Analysis/DiagnosticCreationHelper.cs new file mode 100644 index 000000000..ec7f4b62f --- /dev/null +++ b/src/PowerShellEditorServices/Services/Analysis/DiagnosticCreationHelper.cs @@ -0,0 +1,88 @@ +// +// 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.Utility; +using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; +using OmniSharp.Extensions.LanguageServer.Protocol.Models; +using System.Management.Automation.Language; +using OmnisharpDiagnosticSeverity = OmniSharp.Extensions.LanguageServer.Protocol.Models.DiagnosticSeverity; +using ScriptAnalyzerDiagnosticSeverity = Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticSeverity; + +namespace Microsoft.PowerShell.EditorServices.Services.Analysis +{ + internal static class DiagnosticCreationHelper + { + // Converts a ParseError from PowerShell into a Diagnostic type. + internal static Diagnostic FromParseError(ParseError parseError) + { + Validate.IsNotNull("parseError", parseError); + + return new Diagnostic + { + Message = parseError.Message, + Severity = OmnisharpDiagnosticSeverity.Error, + Range = new OmniSharp.Extensions.LanguageServer.Protocol.Models.Range + { + Start = new OmniSharp.Extensions.LanguageServer.Protocol.Models.Position + { + Line = parseError.Extent.StartLineNumber - 1, + Character = parseError.Extent.StartColumnNumber - 1 + }, + End = new OmniSharp.Extensions.LanguageServer.Protocol.Models.Position + { + Line = parseError.Extent.EndLineNumber - 1, + Character = parseError.Extent.EndColumnNumber - 1 + } + }, + Source = "PowerShell" + }; + } + + // Converts a DiagnosticRecord from PSScriptAnalyzer into a Diagnostic type. + internal static Diagnostic FromDiagnosticRecord(DiagnosticRecord diagnosticRecord) + { + Validate.IsNotNull("diagnosticRecord", diagnosticRecord); + + return new Diagnostic + { + Message = $"{diagnosticRecord.Message as string}", + Code = $"{diagnosticRecord.RuleName as string}", + Severity = MapDiagnosticSeverity(diagnosticRecord.Severity), + Source = "PSScriptAnalyzer", + Range = new OmniSharp.Extensions.LanguageServer.Protocol.Models.Range + { + Start = new OmniSharp.Extensions.LanguageServer.Protocol.Models.Position + { + Line = diagnosticRecord.Extent.StartLineNumber - 1, + Character = diagnosticRecord.Extent.StartColumnNumber - 1 + }, + End = new OmniSharp.Extensions.LanguageServer.Protocol.Models.Position + { + Line = diagnosticRecord.Extent.EndLineNumber - 1, + Character = diagnosticRecord.Extent.EndColumnNumber - 1 + } + } + }; + } + + private static OmnisharpDiagnosticSeverity MapDiagnosticSeverity(ScriptAnalyzerDiagnosticSeverity diagnosticSeverity) + { + switch (diagnosticSeverity) + { + case ScriptAnalyzerDiagnosticSeverity.Error: + return OmnisharpDiagnosticSeverity.Error; + + case ScriptAnalyzerDiagnosticSeverity.Warning: + return OmnisharpDiagnosticSeverity.Warning; + + case ScriptAnalyzerDiagnosticSeverity.Information: + return OmnisharpDiagnosticSeverity.Information; + + default: + return OmnisharpDiagnosticSeverity.Error; + } + } + } +} diff --git a/src/PowerShellEditorServices/Services/Analysis/MarkerCorrection.cs b/src/PowerShellEditorServices/Services/Analysis/MarkerCorrection.cs new file mode 100644 index 000000000..49a3f4b65 --- /dev/null +++ b/src/PowerShellEditorServices/Services/Analysis/MarkerCorrection.cs @@ -0,0 +1,25 @@ +// +// 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; + +namespace Microsoft.PowerShell.EditorServices.Services.Analysis +{ + /// + /// Contains details for a code correction which can be applied from a ScriptFileMarker. + /// + public class MarkerCorrection + { + /// + /// Gets or sets the display name of the code correction. + /// + public string Name { get; set; } + + /// + /// Gets or sets the list of ScriptRegions that define the edits to be made by the correction. + /// + public ScriptRegion[] Edits { get; set; } + } +} diff --git a/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/GetCommentHelpHandler.cs b/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/GetCommentHelpHandler.cs index 13f4deac0..29db33327 100644 --- a/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/GetCommentHelpHandler.cs +++ b/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/GetCommentHelpHandler.cs @@ -70,22 +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.GetCommentHelpCorrectionTextAsync(funcText, request.BlockComment, helpLocation); if (helpText == null) { return result; diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeActionHandler.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeActionHandler.cs index 80180ca61..085c798d6 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeActionHandler.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeActionHandler.cs @@ -9,7 +9,7 @@ using System.Threading.Tasks; using Microsoft.Extensions.Logging; using Microsoft.PowerShell.EditorServices.Services; -using Microsoft.PowerShell.EditorServices.Services.TextDocument; +using Microsoft.PowerShell.EditorServices.Services.Analysis; using Newtonsoft.Json.Linq; using OmniSharp.Extensions.LanguageServer.Protocol.Client.Capabilities; using OmniSharp.Extensions.LanguageServer.Protocol.Models; diff --git a/src/PowerShellEditorServices/Services/TextDocument/ScriptFile.cs b/src/PowerShellEditorServices/Services/TextDocument/ScriptFile.cs index 5b1b58bd5..8d2dd7107 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/ScriptFile.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/ScriptFile.cs @@ -9,8 +9,10 @@ using System.Linq; using System.Management.Automation; using System.Management.Automation.Language; +using Microsoft.PowerShell.EditorServices.Services.Analysis; using Microsoft.PowerShell.EditorServices.Services.Symbols; using Microsoft.PowerShell.EditorServices.Utility; +using OmniSharp.Extensions.LanguageServer.Protocol.Models; namespace Microsoft.PowerShell.EditorServices.Services.TextDocument { @@ -100,7 +102,7 @@ public string Contents /// Gets the list of syntax markers found by parsing this /// file's contents. /// - public List DiagnosticMarkers + public List DiagnosticMarkers { get; private set; @@ -656,7 +658,7 @@ private void ParseFileContents() // Translate parse errors into syntax markers this.DiagnosticMarkers = parseErrors - .Select(ScriptFileMarker.FromParseError) + .Select(DiagnosticCreationHelper.FromParseError) .ToList(); // Untitled files have no directory diff --git a/src/PowerShellEditorServices/Services/TextDocument/ScriptFileMarker.cs b/src/PowerShellEditorServices/Services/TextDocument/ScriptFileMarker.cs deleted file mode 100644 index b5c03208c..000000000 --- a/src/PowerShellEditorServices/Services/TextDocument/ScriptFileMarker.cs +++ /dev/null @@ -1,178 +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 System; -using System.Management.Automation; -using System.Collections.Generic; -using System.Linq; -using System.Management.Automation.Language; -using Microsoft.PowerShell.EditorServices.Utility; -using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; - -namespace Microsoft.PowerShell.EditorServices.Services.TextDocument -{ - /// - /// Contains details for a code correction which can be applied from a ScriptFileMarker. - /// - public class MarkerCorrection - { - /// - /// Gets or sets the display name of the code correction. - /// - public string Name { get; set; } - - /// - /// Gets or sets the list of ScriptRegions that define the edits to be made by the correction. - /// - public ScriptRegion[] Edits { get; set; } - } - - /// - /// Defines the message level of a script file marker. - /// - public enum ScriptFileMarkerLevel - { - ///  -        /// Information: This warning is trivial, but may be useful. They are recommended by PowerShell best practice. -        ///  -        Information = 0, -        ///  -        /// WARNING: This warning may cause a problem or does not follow PowerShell's recommended guidelines. -        ///  -        Warning = 1, -        ///  -        /// ERROR: This warning is likely to cause a problem or does not follow PowerShell's required guidelines. -        ///  -        Error = 2, -        ///  -        /// ERROR: This diagnostic is caused by an actual parsing error, and is generated only by the engine. -        ///  -        ParseError = 3 - }; - - /// - /// Contains details about a marker that should be displayed - /// for the a script file. The marker information could come - /// from syntax parsing or semantic analysis of the script. - /// - public class ScriptFileMarker - { - #region Properties - - /// - /// Gets or sets the marker's message string. - /// - public string Message { get; set; } - - /// - /// Gets or sets the ruleName associated with this marker. - /// - public string RuleName { get; set; } - - /// - /// Gets or sets the marker's message level. - /// - public ScriptFileMarkerLevel Level { get; set; } - - /// - /// Gets or sets the ScriptRegion where the marker should appear. - /// - public ScriptRegion ScriptRegion { get; set; } - - /// - /// Gets or sets an optional code correction that can be applied based on this marker. - /// - public MarkerCorrection Correction { get; set; } - - /// - /// Gets or sets the name of the marker's source like "PowerShell" - /// or "PSScriptAnalyzer". - /// - public string Source { get; set; } - - #endregion - - #region Public Methods - - internal static ScriptFileMarker FromParseError( - ParseError parseError) - { - Validate.IsNotNull("parseError", parseError); - - return new ScriptFileMarker - { - Message = parseError.Message, - Level = ScriptFileMarkerLevel.Error, - ScriptRegion = ScriptRegion.Create(parseError.Extent), - 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(DiagnosticRecord diagnosticRecord) - { - Validate.IsNotNull("diagnosticRecord", diagnosticRecord); - MarkerCorrection correction = null; - - if (diagnosticRecord.SuggestedCorrections != null) - { - var editRegions = new List(); - string correctionMessage = null; - foreach (dynamic suggestedCorrection in diagnosticRecord.SuggestedCorrections) - { - editRegions.Add( - new ScriptRegion( - diagnosticRecord.ScriptPath, - suggestedCorrection.Text, - startLineNumber: suggestedCorrection.StartLineNumber, - startColumnNumber: suggestedCorrection.StartColumnNumber, - endLineNumber: suggestedCorrection.EndLineNumber, - endColumnNumber: suggestedCorrection.EndColumnNumber, - startOffset: -1, - endOffset: -1)); - - correctionMessage = suggestedCorrection.Description; - } - - correction = new MarkerCorrection - { - Name = correctionMessage == null ? diagnosticRecord.Message : correctionMessage, - Edits = editRegions.ToArray() - }; - } - - string severity = diagnosticRecord.Severity.ToString(); - if (!Enum.TryParse(severity, out ScriptFileMarkerLevel level)) - { - throw new ArgumentException( - $"The provided DiagnosticSeverity value '{severity}' is unknown.", - "diagnosticSeverity"); - } - - return new ScriptFileMarker - { - Message = $"{diagnosticRecord.Message as string}", - RuleName = $"{diagnosticRecord.RuleName as string}", - Level = level, - ScriptRegion = ScriptRegion.Create(diagnosticRecord.Extent as IScriptExtent), - Correction = correction, - Source = "PSScriptAnalyzer" - }; - } - - #endregion - } -} - From ef7a9e2b35717f2d70784d79e1ae24b07bc33cd4 Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Wed, 4 Dec 2019 17:59:04 -0500 Subject: [PATCH 3/7] use Settings in LanguageServerSettings --- .../Services/Analysis/AnalysisService.cs | 18 ++- .../Handlers/FormattingHandlers.cs | 23 +-- .../Workspace/LanguageServerSettings.cs | 133 ++++++++---------- 3 files changed, 77 insertions(+), 97 deletions(-) diff --git a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs index ebd0d4b5e..60a69a340 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -87,8 +87,7 @@ public AnalysisService(ConfigurationService configurationService, ILanguageServe SettingsPath = configurationService.CurrentSettings.ScriptAnalysis.SettingsPath; _logger = factory.CreateLogger(); _analyzer = new HostedAnalyzer(); - // TODO: use our rules - _analyzerSettings = _analyzer.CreateSettings("PSGallery"); + _analyzerSettings = _analyzer.CreateSettings(s_includedRules); _configurationService = configurationService; _languageServer = languageServer; _mostRecentCorrectionsByFile = new ConcurrentDictionary)>(); @@ -154,8 +153,9 @@ public async Task> GetSemanticMarkersAsync( /// The formatted script text. public async Task FormatAsync( string scriptDefinition, - Hashtable settings, - int[] rangeList) + int tabSize, + bool insertSpaces, + Windows.PowerShell.ScriptAnalyzer.Range range) { // 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. @@ -164,8 +164,14 @@ public async Task FormatAsync( return null; } - // TODO: support rangeList - return await _analyzer.FormatAsync(scriptDefinition, _analyzer.CreateSettings(settings)); + Settings settings = _configurationService.CurrentSettings.CodeFormatting.GetFormatterSettings( + _analyzer, + tabSize, + insertSpaces); + + return range == null + ? await _analyzer.FormatAsync(scriptDefinition, settings) + : await _analyzer.FormatAsync(scriptDefinition, settings, range); } public async Task RunScriptDiagnosticsAsync( diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/FormattingHandlers.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/FormattingHandlers.cs index aabdeb615..010b60ca2 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/FormattingHandlers.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/FormattingHandlers.cs @@ -24,7 +24,6 @@ internal class DocumentFormattingHandler : IDocumentFormattingHandler private readonly ILogger _logger; private readonly AnalysisService _analysisService; - private readonly ConfigurationService _configurationService; private readonly WorkspaceService _workspaceService; private DocumentFormattingCapability _capability; @@ -32,7 +31,6 @@ public DocumentFormattingHandler(ILoggerFactory factory, AnalysisService analysi { _logger = factory.CreateLogger(); _analysisService = analysisService; - _configurationService = configurationService; _workspaceService = workspaceService; } @@ -47,10 +45,6 @@ public TextDocumentRegistrationOptions GetRegistrationOptions() public async Task Handle(DocumentFormattingParams request, CancellationToken cancellationToken) { var scriptFile = _workspaceService.GetFile(request.TextDocument.Uri.ToString()); - var pssaSettings = _configurationService.CurrentSettings.CodeFormatting.GetPSSASettingsHashtable( - (int)request.Options.TabSize, - request.Options.InsertSpaces); - // TODO raise an error event in case format returns null string formattedScript; @@ -74,7 +68,8 @@ public async Task Handle(DocumentFormattingParams request, Ca formattedScript = await _analysisService.FormatAsync( scriptFile.Contents, - pssaSettings, + (int)request.Options.TabSize, + request.Options.InsertSpaces, null); formattedScript = formattedScript ?? scriptFile.Contents; @@ -102,7 +97,6 @@ internal class DocumentRangeFormattingHandler : IDocumentRangeFormattingHandler private readonly ILogger _logger; private readonly AnalysisService _analysisService; - private readonly ConfigurationService _configurationService; private readonly WorkspaceService _workspaceService; private DocumentRangeFormattingCapability _capability; @@ -110,7 +104,6 @@ public DocumentRangeFormattingHandler(ILoggerFactory factory, AnalysisService an { _logger = factory.CreateLogger(); _analysisService = analysisService; - _configurationService = configurationService; _workspaceService = workspaceService; } @@ -125,9 +118,6 @@ public TextDocumentRegistrationOptions GetRegistrationOptions() public async Task Handle(DocumentRangeFormattingParams request, CancellationToken cancellationToken) { var scriptFile = _workspaceService.GetFile(request.TextDocument.Uri.ToString()); - var pssaSettings = _configurationService.CurrentSettings.CodeFormatting.GetPSSASettingsHashtable( - (int)request.Options.TabSize, - request.Options.InsertSpaces); // TODO raise an error event in case format returns null; string formattedScript; @@ -150,16 +140,17 @@ public async Task Handle(DocumentRangeFormattingParams reques }; Range range = request.Range; - var rangeList = range == null ? null : new int[] { + Windows.PowerShell.ScriptAnalyzer.Range pssaRange = new Windows.PowerShell.ScriptAnalyzer.Range( (int)range.Start.Line + 1, (int)range.Start.Character + 1, (int)range.End.Line + 1, - (int)range.End.Character + 1}; + (int)range.End.Character + 1); formattedScript = await _analysisService.FormatAsync( scriptFile.Contents, - pssaSettings, - rangeList); + (int)request.Options.TabSize, + request.Options.InsertSpaces, + pssaRange); formattedScript = formattedScript ?? scriptFile.Contents; return new TextEditContainer(new TextEdit diff --git a/src/PowerShellEditorServices/Services/Workspace/LanguageServerSettings.cs b/src/PowerShellEditorServices/Services/Workspace/LanguageServerSettings.cs index dbdefc9de..8b272e6f3 100644 --- a/src/PowerShellEditorServices/Services/Workspace/LanguageServerSettings.cs +++ b/src/PowerShellEditorServices/Services/Workspace/LanguageServerSettings.cs @@ -11,6 +11,8 @@ using System.Security; using Microsoft.Extensions.Logging; using Microsoft.PowerShell.EditorServices.Logging; +using Microsoft.Windows.PowerShell.ScriptAnalyzer; +using Microsoft.Windows.PowerShell.ScriptAnalyzer.Hosting; namespace Microsoft.PowerShell.EditorServices.Services.Configuration { @@ -214,107 +216,88 @@ public CodeFormattingSettings(CodeFormattingSettings codeFormattingSettings) public bool AlignPropertyValuePairs { get; set; } public bool UseCorrectCasing { get; set; } - /// - /// Get the settings hashtable that will be consumed by PSScriptAnalyzer. + /// Get the settings that will be consumed by PSScriptAnalyzer. /// + /// The analyzer used to create the settings. /// The tab size in the number spaces. /// If true, insert spaces otherwise insert tabs for indentation. /// - public Hashtable GetPSSASettingsHashtable( - int tabSize, - bool insertSpaces) + public Settings GetFormatterSettings(HostedAnalyzer analyzer, int tabSize, bool insertSpaces) { - var settings = GetCustomPSSASettingsHashtable(tabSize, insertSpaces); - var ruleSettings = (Hashtable)(settings["Rules"]); - var closeBraceSettings = (Hashtable)ruleSettings["PSPlaceCloseBrace"]; - var openBraceSettings = (Hashtable)ruleSettings["PSPlaceOpenBrace"]; + bool openBraceOnSameLine, newLineAfterOpenBrace, newLineAfterCloseBrace; switch(Preset) { case CodeFormattingPreset.Allman: - openBraceSettings["OnSameLine"] = false; - openBraceSettings["NewLineAfter"] = true; - closeBraceSettings["NewLineAfter"] = true; + openBraceOnSameLine = false; + newLineAfterOpenBrace = true; + newLineAfterCloseBrace = true; break; case CodeFormattingPreset.OTBS: - openBraceSettings["OnSameLine"] = true; - openBraceSettings["NewLineAfter"] = true; - closeBraceSettings["NewLineAfter"] = false; + openBraceOnSameLine = true; + newLineAfterOpenBrace = true; + newLineAfterCloseBrace = false; break; case CodeFormattingPreset.Stroustrup: - openBraceSettings["OnSameLine"] = true; - openBraceSettings["NewLineAfter"] = true; - closeBraceSettings["NewLineAfter"] = true; + openBraceOnSameLine = true; + newLineAfterOpenBrace = true; + newLineAfterCloseBrace = true; break; default: + openBraceOnSameLine = OpenBraceOnSameLine; + newLineAfterOpenBrace = NewLineAfterOpenBrace; + newLineAfterCloseBrace = NewLineAfterCloseBrace; break; } - return settings; - } + Settings settings = analyzer.CreateSettings(); + settings.AddRuleArgument("PSPlaceOpenBrace", new Dictionary + { + { "Enable", true }, + { "OnSameLine", openBraceOnSameLine }, + { "NewLineAfter", newLineAfterOpenBrace }, + { "IgnoreOneLineBlock", IgnoreOneLineBlock } + }); - private Hashtable GetCustomPSSASettingsHashtable(int tabSize, bool insertSpaces) - { - var ruleConfigurations = new Hashtable + settings.AddRuleArgument("PSPlaceCloseBrace", new Dictionary { - { "PSPlaceOpenBrace", new Hashtable { - { "Enable", true }, - { "OnSameLine", OpenBraceOnSameLine }, - { "NewLineAfter", NewLineAfterOpenBrace }, - { "IgnoreOneLineBlock", IgnoreOneLineBlock } - }}, - { "PSPlaceCloseBrace", new Hashtable { - { "Enable", true }, - { "NewLineAfter", NewLineAfterCloseBrace }, - { "IgnoreOneLineBlock", IgnoreOneLineBlock } - }}, - { "PSUseConsistentIndentation", new Hashtable { - { "Enable", true }, - { "IndentationSize", tabSize }, - { "PipelineIndentation", PipelineIndentationStyle }, - { "Kind", insertSpaces ? "space" : "tab" } - }}, - { "PSUseConsistentWhitespace", new Hashtable { - { "Enable", true }, - { "CheckOpenBrace", WhitespaceBeforeOpenBrace }, - { "CheckOpenParen", WhitespaceBeforeOpenParen }, - { "CheckOperator", WhitespaceAroundOperator }, - { "CheckSeparator", WhitespaceAfterSeparator }, - { "CheckInnerBrace", WhitespaceInsideBrace }, - { "CheckPipe", WhitespaceAroundPipe }, - }}, - { "PSAlignAssignmentStatement", new Hashtable { - { "Enable", true }, - { "CheckHashtable", AlignPropertyValuePairs } - }}, - { "PSUseCorrectCasing", new Hashtable { - { "Enable", UseCorrectCasing } - }}, - }; - - if (AutoCorrectAliases) + { "Enable", true }, + { "NewLineAfter", newLineAfterCloseBrace }, + { "IgnoreOneLineBlock", IgnoreOneLineBlock } + }); + + settings.AddRuleArgument("PSUseConsistentIndentation", new Dictionary { - // Empty hashtable required to activate the rule, - // since PSAvoidUsingCmdletAliases inherits from IScriptRule and not ConfigurableRule - ruleConfigurations.Add("PSAvoidUsingCmdletAliases", new Hashtable()); - } + { "Enable", true }, + { "IndentationSize", tabSize }, + { "PipelineIndentation", PipelineIndentationStyle }, + { "Kind", insertSpaces ? "space" : "tab" } + }); - return new Hashtable() + settings.AddRuleArgument("PSUseConsistentWhitespace", new Dictionary { - { "IncludeRules", new string[] { - "PSPlaceCloseBrace", - "PSPlaceOpenBrace", - "PSUseConsistentWhitespace", - "PSUseConsistentIndentation", - "PSAlignAssignmentStatement" - }}, - { - "Rules", ruleConfigurations - } - }; + { "Enable", true }, + { "CheckOpenBrace", WhitespaceBeforeOpenBrace }, + { "CheckOpenParen", WhitespaceBeforeOpenParen }, + { "CheckOperator", WhitespaceAroundOperator }, + { "CheckSeparator", WhitespaceAfterSeparator }, + { "CheckInnerBrace", WhitespaceInsideBrace }, + { "CheckPipe", WhitespaceAroundPipe }, + }); + + settings.AddRuleArgument("PSAlignAssignmentStatement", new Dictionary + { + { "Enable", true }, + { "CheckHashtable", AlignPropertyValuePairs } + }); + + settings.AddRuleArgument("PSUseCorrectCasing", "Enable", UseCorrectCasing); + settings.AddRuleArgument("PSAvoidUsingCmdletAliases", "Enable", AutoCorrectAliases); + + return settings; } } From 555200552f2d6285044ec7bcfaab9c1bd7326cef Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Thu, 5 Dec 2019 09:54:52 -0800 Subject: [PATCH 4/7] fix and then format --- .../Services/Analysis/AnalysisService.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs index 60a69a340..11ff28024 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -170,8 +170,8 @@ public async Task FormatAsync( insertSpaces); return range == null - ? await _analyzer.FormatAsync(scriptDefinition, settings) - : await _analyzer.FormatAsync(scriptDefinition, settings, range); + ? await _analyzer.FormatAsync(_analyzer.Fix(scriptDefinition, settings), settings) + : await _analyzer.FormatAsync(_analyzer.Fix(scriptDefinition, settings), settings, range); } public async Task RunScriptDiagnosticsAsync( From b42a707e5fd9685e5436bfd65c3b5772633181a4 Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Thu, 5 Dec 2019 10:51:29 -0800 Subject: [PATCH 5/7] codacy --- .../Services/Analysis/AnalysisService.cs | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs index 11ff28024..875cc4416 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -8,7 +8,6 @@ using System.Threading.Tasks; 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; @@ -27,7 +26,7 @@ namespace Microsoft.PowerShell.EditorServices.Services /// Provides a high-level service for performing semantic analysis /// of PowerShell scripts. /// - public class AnalysisService + public class AnalysisService : IDisposable { #region Fields @@ -67,11 +66,6 @@ public class AnalysisService #region Properties - /// - /// Set of PSScriptAnalyzer rules used for analysis. - /// - public string[] ActiveRules => s_includedRules; - /// /// Gets or sets the path to a settings file (.psd1) /// containing PSScriptAnalyzer settings. @@ -99,6 +93,16 @@ public AnalysisService(ConfigurationService configurationService, ILanguageServe #region Public Methods + /// + /// Clean up resources. + /// + public void Dispose() + { + _existingRequestCancellation.Dispose(); + _analyzer.Dispose(); + _existingRequestCancellationLock.Dispose(); + } + /// /// Get PSScriptAnalyzer settings for PSProvideCommentHelp rule. /// @@ -221,7 +225,7 @@ public async Task RunScriptDiagnosticsAsync( try { // Wait for the desired delay period before analyzing the provided list of files. - await Task.Delay(750, ct); + await Task.Delay(750, ct).ConfigureAwait(continueOnCapturedContext: false); foreach (ScriptFile file in filesToAnalyze) { @@ -257,7 +261,7 @@ public async Task RunScriptDiagnosticsAsync( newEntryNeeded = true; } - await fileLock.WaitAsync(); + await fileLock.WaitAsync().ConfigureAwait(continueOnCapturedContext: false); try { if (newEntryNeeded) @@ -278,7 +282,6 @@ public async Task RunScriptDiagnosticsAsync( file.DiagnosticMarkers.Add(diagnostic); // Does the marker contain a correction? - //Diagnostic markerDiagnostic = GetDiagnosticFromMarker(marker); if (diagnosticRecord.SuggestedCorrections != null) { var editRegions = new List(); From 1b8e6e84f0cb2e3e108ba967fe7a833992fdabb8 Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Thu, 5 Dec 2019 11:12:28 -0800 Subject: [PATCH 6/7] codacy 2 --- .../Services/Analysis/AnalysisService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs index 875cc4416..45cfea934 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -189,7 +189,7 @@ public async Task RunScriptDiagnosticsAsync( // If there's an existing task, attempt to cancel it try { - await _existingRequestCancellationLock.WaitAsync(); + await _existingRequestCancellationLock.WaitAsync().ConfigureAwait(continueOnCapturedContext: false); // Try to cancel the request _existingRequestCancellation.Cancel(); From 996cb85e26abdf84781af4820cd9b30548018b0e Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Mon, 9 Dec 2019 13:36:44 -0800 Subject: [PATCH 7/7] last feature gaps --- .../Services/Analysis/AnalysisService.cs | 9 ++- .../Workspace/LanguageServerSettings.cs | 80 +++++++++---------- 2 files changed, 45 insertions(+), 44 deletions(-) diff --git a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs index 45cfea934..b27ee9cfa 100644 --- a/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs +++ b/src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs @@ -1,4 +1,4 @@ -// +// // Copyright (c) Microsoft. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. // @@ -82,6 +82,11 @@ public AnalysisService(ConfigurationService configurationService, ILanguageServe _logger = factory.CreateLogger(); _analyzer = new HostedAnalyzer(); _analyzerSettings = _analyzer.CreateSettings(s_includedRules); + _analyzerSettings.Severities.AddRange(new [] { + RuleSeverity.Error.ToString(), + RuleSeverity.Information.ToString(), + RuleSeverity.Information.ToString() + }); _configurationService = configurationService; _languageServer = languageServer; _mostRecentCorrectionsByFile = new ConcurrentDictionary)>(); @@ -175,7 +180,7 @@ public async Task FormatAsync( return range == null ? await _analyzer.FormatAsync(_analyzer.Fix(scriptDefinition, settings), settings) - : await _analyzer.FormatAsync(_analyzer.Fix(scriptDefinition, settings), settings, range); + : await _analyzer.FormatAsync(_analyzer.Fix(scriptDefinition, range, settings), settings, range); } public async Task RunScriptDiagnosticsAsync( diff --git a/src/PowerShellEditorServices/Services/Workspace/LanguageServerSettings.cs b/src/PowerShellEditorServices/Services/Workspace/LanguageServerSettings.cs index 8b272e6f3..83ee77cb9 100644 --- a/src/PowerShellEditorServices/Services/Workspace/LanguageServerSettings.cs +++ b/src/PowerShellEditorServices/Services/Workspace/LanguageServerSettings.cs @@ -254,48 +254,44 @@ public Settings GetFormatterSettings(HostedAnalyzer analyzer, int tabSize, bool } Settings settings = analyzer.CreateSettings(); - settings.AddRuleArgument("PSPlaceOpenBrace", new Dictionary - { - { "Enable", true }, - { "OnSameLine", openBraceOnSameLine }, - { "NewLineAfter", newLineAfterOpenBrace }, - { "IgnoreOneLineBlock", IgnoreOneLineBlock } - }); - - settings.AddRuleArgument("PSPlaceCloseBrace", new Dictionary - { - { "Enable", true }, - { "NewLineAfter", newLineAfterCloseBrace }, - { "IgnoreOneLineBlock", IgnoreOneLineBlock } - }); - - settings.AddRuleArgument("PSUseConsistentIndentation", new Dictionary - { - { "Enable", true }, - { "IndentationSize", tabSize }, - { "PipelineIndentation", PipelineIndentationStyle }, - { "Kind", insertSpaces ? "space" : "tab" } - }); - - settings.AddRuleArgument("PSUseConsistentWhitespace", new Dictionary - { - { "Enable", true }, - { "CheckOpenBrace", WhitespaceBeforeOpenBrace }, - { "CheckOpenParen", WhitespaceBeforeOpenParen }, - { "CheckOperator", WhitespaceAroundOperator }, - { "CheckSeparator", WhitespaceAfterSeparator }, - { "CheckInnerBrace", WhitespaceInsideBrace }, - { "CheckPipe", WhitespaceAroundPipe }, - }); - - settings.AddRuleArgument("PSAlignAssignmentStatement", new Dictionary - { - { "Enable", true }, - { "CheckHashtable", AlignPropertyValuePairs } - }); - - settings.AddRuleArgument("PSUseCorrectCasing", "Enable", UseCorrectCasing); - settings.AddRuleArgument("PSAvoidUsingCmdletAliases", "Enable", AutoCorrectAliases); + settings + .AddRuleArgument("PSPlaceOpenBrace", new Dictionary + { + { "Enable", true }, + { "OnSameLine", openBraceOnSameLine }, + { "NewLineAfter", newLineAfterOpenBrace }, + { "IgnoreOneLineBlock", IgnoreOneLineBlock } + }) + .AddRuleArgument("PSPlaceCloseBrace", new Dictionary + { + { "Enable", true }, + { "NewLineAfter", newLineAfterCloseBrace }, + { "IgnoreOneLineBlock", IgnoreOneLineBlock } + }) + .AddRuleArgument("PSUseConsistentIndentation", new Dictionary + { + { "Enable", true }, + { "IndentationSize", tabSize }, + { "PipelineIndentation", PipelineIndentationStyle }, + { "Kind", insertSpaces ? "space" : "tab" } + }) + .AddRuleArgument("PSUseConsistentWhitespace", new Dictionary + { + { "Enable", true }, + { "CheckOpenBrace", WhitespaceBeforeOpenBrace }, + { "CheckOpenParen", WhitespaceBeforeOpenParen }, + { "CheckOperator", WhitespaceAroundOperator }, + { "CheckSeparator", WhitespaceAfterSeparator }, + { "CheckInnerBrace", WhitespaceInsideBrace }, + { "CheckPipe", WhitespaceAroundPipe }, + }) + .AddRuleArgument("PSAlignAssignmentStatement", new Dictionary + { + { "Enable", true }, + { "CheckHashtable", AlignPropertyValuePairs } + }) + .AddRuleArgument("PSUseCorrectCasing", "Enable", UseCorrectCasing) + .AddRuleArgument("PSAvoidUsingCmdletAliases", "Enable", AutoCorrectAliases); return settings; }