diff --git a/src/PowerShellEditorServices/Services/Workspace/Handlers/ConfigurationHandler.cs b/src/PowerShellEditorServices/Services/Workspace/Handlers/ConfigurationHandler.cs index 230a82d58..a7f36aab1 100644 --- a/src/PowerShellEditorServices/Services/Workspace/Handlers/ConfigurationHandler.cs +++ b/src/PowerShellEditorServices/Services/Workspace/Handlers/ConfigurationHandler.cs @@ -7,13 +7,9 @@ using System.Threading.Tasks; using MediatR; using Microsoft.Extensions.Logging; -using Microsoft.PowerShell.EditorServices.Logging; using Microsoft.PowerShell.EditorServices.Services; using Microsoft.PowerShell.EditorServices.Services.Configuration; -using Newtonsoft.Json.Linq; using OmniSharp.Extensions.LanguageServer.Protocol.Models; -using OmniSharp.Extensions.LanguageServer.Protocol.Server; -using OmniSharp.Extensions.LanguageServer.Protocol.Window; using OmniSharp.Extensions.LanguageServer.Protocol.Workspace; namespace Microsoft.PowerShell.EditorServices.Handlers @@ -23,19 +19,16 @@ internal class PsesConfigurationHandler : DidChangeConfigurationHandlerBase private readonly ILogger _logger; private readonly WorkspaceService _workspaceService; private readonly ConfigurationService _configurationService; - private readonly ILanguageServerFacade _languageServer; public PsesConfigurationHandler( ILoggerFactory factory, WorkspaceService workspaceService, AnalysisService analysisService, ConfigurationService configurationService, - ILanguageServerFacade languageServer, SymbolsService symbolsService) { _logger = factory.CreateLogger(); _workspaceService = workspaceService; _configurationService = configurationService; - _languageServer = languageServer; ConfigurationUpdated += analysisService.OnConfigurationUpdated; ConfigurationUpdated += symbolsService.OnConfigurationUpdated; @@ -51,8 +44,6 @@ public override async Task Handle(DidChangeConfigurationParams request, Ca return await Unit.Task.ConfigureAwait(false); } - SendFeatureChangesTelemetry(incomingSettings); - bool oldScriptAnalysisEnabled = _configurationService.CurrentSettings.ScriptAnalysis.Enable; string oldScriptAnalysisSettingsPath = _configurationService.CurrentSettings.ScriptAnalysis?.SettingsPath; @@ -109,59 +100,6 @@ public override async Task Handle(DidChangeConfigurationParams request, Ca return await Unit.Task.ConfigureAwait(false); } - private void SendFeatureChangesTelemetry(LanguageServerSettingsWrapper incomingSettings) - { - if (incomingSettings is null) - { - _logger.LogTrace("Incoming settings were null"); - return; - } - - Dictionary configChanges = new(); - // Send telemetry if the user opted-out of ScriptAnalysis - if (!incomingSettings.Powershell.ScriptAnalysis.Enable && - _configurationService.CurrentSettings.ScriptAnalysis.Enable != incomingSettings.Powershell.ScriptAnalysis.Enable) - { - configChanges["ScriptAnalysis"] = incomingSettings.Powershell.ScriptAnalysis.Enable; - } - - // Send telemetry if the user opted-out of CodeFolding - if (!incomingSettings.Powershell.CodeFolding.Enable && - _configurationService.CurrentSettings.CodeFolding.Enable != incomingSettings.Powershell.CodeFolding.Enable) - { - configChanges["CodeFolding"] = incomingSettings.Powershell.CodeFolding.Enable; - } - - // Send telemetry if the user opted-out of Profile loading - if (!incomingSettings.Powershell.EnableProfileLoading && - _configurationService.CurrentSettings.EnableProfileLoading != incomingSettings.Powershell.EnableProfileLoading) - { - configChanges["ProfileLoading"] = incomingSettings.Powershell.EnableProfileLoading; - } - - // Send telemetry if the user opted-in to Pester 5+ CodeLens - if (!incomingSettings.Powershell.Pester.UseLegacyCodeLens && - _configurationService.CurrentSettings.Pester.UseLegacyCodeLens != incomingSettings.Powershell.Pester.UseLegacyCodeLens) - { - // From our perspective we want to see how many people are opting in to this so we flip the value - configChanges["Pester5CodeLens"] = !incomingSettings.Powershell.Pester.UseLegacyCodeLens; - } - - // No need to send any telemetry since nothing changed - if (configChanges.Count == 0) - { - return; - } - - _languageServer.Window.SendTelemetryEvent(new TelemetryEventParams - { - ExtensionData = new PsesTelemetryEvent - { - EventName = "NonDefaultPsesFeatureConfiguration", - Data = JObject.FromObject(configChanges) - } - }); - } public event EventHandler ConfigurationUpdated; } diff --git a/test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs b/test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs index fd4854f04..1a227278d 100644 --- a/test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs +++ b/test/PowerShellEditorServices.Test.E2E/LanguageServerProtocolMessageTests.cs @@ -12,7 +12,6 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.PowerShell.EditorServices.Handlers; -using Microsoft.PowerShell.EditorServices.Logging; using Microsoft.PowerShell.EditorServices.Services.Configuration; using Microsoft.PowerShell.EditorServices.Services.PowerShell; using Microsoft.PowerShell.EditorServices.Services.Template; @@ -40,7 +39,6 @@ public class LanguageServerProtocolMessageTests : IClassFixture private readonly ILanguageClient PsesLanguageClient; private readonly List Messages; private readonly List Diagnostics; - private readonly List TelemetryEvents; private readonly string PwshExe; public LanguageServerProtocolMessageTests(ITestOutputHelper output, LSPTestsFixture data) @@ -51,15 +49,12 @@ public LanguageServerProtocolMessageTests(ITestOutputHelper output, LSPTestsFixt Messages.Clear(); Diagnostics = data.Diagnostics; Diagnostics.Clear(); - TelemetryEvents = data.TelemetryEvents; - TelemetryEvents.Clear(); PwshExe = PsesStdioProcess.PwshExe; } public void Dispose() { Diagnostics.Clear(); - TelemetryEvents.Clear(); GC.SuppressFinalize(this); } @@ -91,26 +86,12 @@ private async Task WaitForDiagnosticsAsync() // Wait for PSSA to finish. for (int i = 0; Diagnostics.Count == 0; i++) { - if (i >= 10) + if (i >= 30) { throw new InvalidDataException("No diagnostics showed up after 20s."); } - await Task.Delay(2000).ConfigureAwait(true); - } - } - - private async Task WaitForTelemetryEventsAsync() - { - // Wait for PSSA to finish. - for (int i = 0; TelemetryEvents.Count == 0; i++) - { - if (i >= 10) - { - throw new InvalidDataException("No telemetry events showed up after 20s."); - } - - await Task.Delay(2000).ConfigureAwait(true); + await Task.Delay(1000).ConfigureAwait(true); } } @@ -233,12 +214,6 @@ public async Task CanReceiveDiagnosticsFromConfigurationChangeAsync() Skip.If(PsesStdioProcess.RunningInConstrainedLanguageMode && PsesStdioProcess.IsWindowsPowerShell, "Windows PowerShell doesn't trust PSScriptAnalyzer by default so it won't load."); - NewTestFile("gci | % { $_ }"); - await WaitForDiagnosticsAsync().ConfigureAwait(true); - - // NewTestFile doesn't clear diagnostic notifications so we need to do that for this test. - Diagnostics.Clear(); - PsesLanguageClient.SendNotification("workspace/didChangeConfiguration", new DidChangeConfigurationParams { @@ -256,18 +231,12 @@ public async Task CanReceiveDiagnosticsFromConfigurationChangeAsync() }) }); - await WaitForTelemetryEventsAsync().ConfigureAwait(true); - PsesTelemetryEvent telemetryEvent = Assert.Single(TelemetryEvents); - Assert.Equal("NonDefaultPsesFeatureConfiguration", telemetryEvent.EventName); - Assert.False((bool)telemetryEvent.Data.GetValue("ScriptAnalysis")); + string filePath = NewTestFile("$a = 4"); - // We also shouldn't get any Diagnostics because ScriptAnalysis is disabled. + // Wait a bit to make sure no diagnostics came through + await Task.Delay(2000).ConfigureAwait(true); Assert.Empty(Diagnostics); - // Clear telemetry events so we can test to make sure telemetry doesn't - // come through with default settings. - TelemetryEvents.Clear(); - // Restore default configuration PsesLanguageClient.SendNotification("workspace/didChangeConfiguration", new DidChangeConfigurationParams @@ -280,10 +249,22 @@ public async Task CanReceiveDiagnosticsFromConfigurationChangeAsync() }) }); - // Wait a bit to make sure no telemetry events came through - await Task.Delay(2000).ConfigureAwait(true); - // Since we have default settings we should not get any telemetry events about - Assert.Empty(TelemetryEvents.Where(e => e.EventName == "NonDefaultPsesFeatureConfiguration")); + // That notification does not trigger re-analyzing open files. For that we have to send + // a textDocument/didChange notification. + PsesLanguageClient.SendNotification("textDocument/didChange", new DidChangeTextDocumentParams + { + ContentChanges = new Container(), + TextDocument = new OptionalVersionedTextDocumentIdentifier + { + Version = 4, + Uri = new Uri(filePath) + } + }); + + await WaitForDiagnosticsAsync().ConfigureAwait(true); + + Diagnostic diagnostic = Assert.Single(Diagnostics); + Assert.Equal("PSUseDeclaredVarsMoreThanAssignments", diagnostic.Code); } [Fact]