Skip to content

Fix PSSA settings discovery + pick up changes to settings file #1206

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Feb 26, 2020
106 changes: 86 additions & 20 deletions src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
using System.Collections;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Runtime.InteropServices;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
Expand Down Expand Up @@ -91,10 +93,12 @@ internal static string GetUniqueIdFromDiagnostic(Diagnostic diagnostic)

private readonly ConcurrentDictionary<ScriptFile, CorrectionTableEntry> _mostRecentCorrectionsByFile;

private Lazy<PssaCmdletAnalysisEngine> _analysisEngine;
private Lazy<PssaCmdletAnalysisEngine> _analysisEngineLazy;

private CancellationTokenSource _diagnosticsCancellationTokenSource;

private string _pssaSettingsFilePath;

/// <summary>
/// Construct a new AnalysisService.
/// </summary>
Expand All @@ -115,13 +119,14 @@ public AnalysisService(
_workplaceService = workspaceService;
_analysisDelayMillis = 750;
_mostRecentCorrectionsByFile = new ConcurrentDictionary<ScriptFile, CorrectionTableEntry>();
_analysisEngine = new Lazy<PssaCmdletAnalysisEngine>(InstantiateAnalysisEngine);
_analysisEngineLazy = new Lazy<PssaCmdletAnalysisEngine>(InstantiateAnalysisEngine);
_pssaSettingsFilePath = null;
}

/// <summary>
/// The analysis engine to use for running script analysis.
/// </summary>
private PssaCmdletAnalysisEngine AnalysisEngine => _analysisEngine.Value;
private PssaCmdletAnalysisEngine AnalysisEngine => _analysisEngineLazy?.Value;

/// <summary>
/// Sets up a script analysis run, eventually returning the result.
Expand All @@ -133,6 +138,8 @@ public void RunScriptDiagnostics(
ScriptFile[] filesToAnalyze,
CancellationToken cancellationToken)
{
EnsureEngineSettingsCurrent();

if (AnalysisEngine == null)
{
return;
Expand Down Expand Up @@ -183,6 +190,8 @@ public void RunScriptDiagnostics(
/// <returns>The text of the formatted PowerShell script.</returns>
public Task<string> FormatAsync(string scriptFileContents, Hashtable formatSettings, int[] formatRange = null)
{
EnsureEngineSettingsCurrent();

if (AnalysisEngine == null)
{
return null;
Expand Down Expand Up @@ -254,23 +263,61 @@ public void ClearMarkers(ScriptFile file)
/// <param name="settings">The new language server settings.</param>
public void OnConfigurationUpdated(object sender, LanguageServerSettings settings)
{
ClearOpenFileMarkers();
_analysisEngine = new Lazy<PssaCmdletAnalysisEngine>(InstantiateAnalysisEngine);
InitializeAnalysisEngineToCurrentSettings();
}

private PssaCmdletAnalysisEngine InstantiateAnalysisEngine()
private void EnsureEngineSettingsCurrent()
{
if (!(_configurationService.CurrentSettings.ScriptAnalysis.Enable ?? false))
if (_pssaSettingsFilePath != null
&& !File.Exists(_pssaSettingsFilePath))
{
return null;
InitializeAnalysisEngineToCurrentSettings();
}
}

private void InitializeAnalysisEngineToCurrentSettings()
{
// If script analysis has been disabled, just return null
if (_configurationService.CurrentSettings.ScriptAnalysis.Enable != true)
{
if (_analysisEngineLazy != null && _analysisEngineLazy.IsValueCreated)
{
_analysisEngineLazy.Value.Dispose();
}

_analysisEngineLazy = null;
return;
}

// We may be triggered after the lazy factory is set,
// but before it's been able to instantiate
if (_analysisEngineLazy == null)
{
_analysisEngineLazy = new Lazy<PssaCmdletAnalysisEngine>(InstantiateAnalysisEngine);
return;
}
else if (!_analysisEngineLazy.IsValueCreated)
{
return;
}

// Retrieve the current script analysis engine so we can recreate it after we've overridden it
PssaCmdletAnalysisEngine currentAnalysisEngine = AnalysisEngine;

// Clear the open file markers and set the new engine factory
ClearOpenFileMarkers();
_analysisEngineLazy = new Lazy<PssaCmdletAnalysisEngine>(() => RecreateAnalysisEngine(currentAnalysisEngine));
}

private PssaCmdletAnalysisEngine InstantiateAnalysisEngine()
{
var pssaCmdletEngineBuilder = new PssaCmdletAnalysisEngine.Builder(_loggerFactory);

// If there's a settings file use that
if (TryFindSettingsFile(out string settingsFilePath))
{
_logger.LogInformation($"Configuring PSScriptAnalyzer with rules at '{settingsFilePath}'");
_pssaSettingsFilePath = settingsFilePath;
pssaCmdletEngineBuilder.WithSettingsFile(settingsFilePath);
}
else
Expand All @@ -282,26 +329,40 @@ private PssaCmdletAnalysisEngine InstantiateAnalysisEngine()
return pssaCmdletEngineBuilder.Build();
}

private PssaCmdletAnalysisEngine RecreateAnalysisEngine(PssaCmdletAnalysisEngine oldAnalysisEngine)
{
if (TryFindSettingsFile(out string settingsFilePath))
{
_logger.LogInformation($"Recreating analysis engine with rules at '{settingsFilePath}'");
_pssaSettingsFilePath = settingsFilePath;
return oldAnalysisEngine.RecreateWithNewSettings(settingsFilePath);
}

_logger.LogInformation("PSScriptAnalyzer settings file not found. Falling back to default rules");
return oldAnalysisEngine.RecreateWithRules(s_defaultRules);
}

private bool TryFindSettingsFile(out string settingsFilePath)
{
string configuredPath = _configurationService.CurrentSettings.ScriptAnalysis.SettingsPath;

if (!string.IsNullOrEmpty(configuredPath))
if (string.IsNullOrEmpty(configuredPath))
{
settingsFilePath = _workplaceService.ResolveWorkspacePath(configuredPath);
settingsFilePath = null;
return false;
}

if (settingsFilePath == null)
{
_logger.LogError($"Unable to find PSSA settings file at '{configuredPath}'. Loading default rules.");
}
settingsFilePath = _workplaceService.ResolveWorkspacePath(configuredPath);

return settingsFilePath != null;
if (settingsFilePath == null
|| !File.Exists(settingsFilePath))
{
_logger.LogWarning($"Unable to find PSSA settings file at '{configuredPath}'. Loading default rules.");
settingsFilePath = null;
return false;
}

// TODO: Could search for a default here

settingsFilePath = null;
return false;
return true;
}

private void ClearOpenFileMarkers()
Expand Down Expand Up @@ -442,7 +503,12 @@ protected virtual void Dispose(bool disposing)
{
if (disposing)
{
if (_analysisEngine.IsValueCreated) { _analysisEngine.Value.Dispose(); }
if (_analysisEngineLazy != null
&& _analysisEngineLazy.IsValueCreated)
{
_analysisEngineLazy.Value.Dispose();
}

_diagnosticsCancellationTokenSource?.Dispose();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,21 @@ public Task<ScriptFileMarker[]> AnalyzeScriptAsync(string scriptContent, Hashtab
return GetSemanticMarkersFromCommandAsync(command);
}

public PssaCmdletAnalysisEngine RecreateWithNewSettings(string settingsPath)
{
return new PssaCmdletAnalysisEngine(_logger, _analysisRunspacePool, _pssaModuleInfo, settingsPath);
}

public PssaCmdletAnalysisEngine RecreateWithNewSettings(Hashtable settingsHashtable)
{
return new PssaCmdletAnalysisEngine(_logger, _analysisRunspacePool, _pssaModuleInfo, settingsHashtable);
}

public PssaCmdletAnalysisEngine RecreateWithRules(string[] rules)
{
return new PssaCmdletAnalysisEngine(_logger, _analysisRunspacePool, _pssaModuleInfo, rules);
}

#region IDisposable Support
private bool disposedValue = false; // To detect redundant calls

Expand Down