Skip to content

Commit

Permalink
Merge pull request #54426 from dotnet/merges/main-to-main-vs-deps
Browse files Browse the repository at this point in the history
Merge main to main-vs-deps
  • Loading branch information
msftbot[bot] authored Jun 27, 2021
2 parents cf20979 + 457d6e6 commit 03a07d1
Show file tree
Hide file tree
Showing 9 changed files with 105 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public abstract class AbstractLanguageServerProtocolTests
private static readonly TestComposition s_composition = EditorTestCompositions.LanguageServerProtocolWpf
.AddParts(typeof(TestLspWorkspaceRegistrationService))
.AddParts(typeof(TestDocumentTrackingService))
.AddParts(typeof(TestExperimentationService))
.RemoveParts(typeof(MockWorkspaceEventListenerProvider));

[Export(typeof(ILspWorkspaceRegistrationService)), PartNotDiscoverable]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,32 +5,31 @@
using System;
using System.Collections.Generic;
using System.Composition;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Experiments;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.Options;

namespace Microsoft.VisualStudio.LanguageServices.Implementation.Diagnostics
namespace Microsoft.CodeAnalysis.Diagnostics
{
[ExportWorkspaceServiceFactory(typeof(IDiagnosticModeService), ServiceLayer.Host), Shared]
internal class VisualStudioDiagnosticModeServiceFactory : IWorkspaceServiceFactory
[ExportWorkspaceServiceFactory(typeof(IDiagnosticModeService), ServiceLayer.Default), Shared]
internal class DefaultDiagnosticModeServiceFactory : IWorkspaceServiceFactory
{
[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public VisualStudioDiagnosticModeServiceFactory()
public DefaultDiagnosticModeServiceFactory()
{
}

public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices)
=> new VisualStudioDiagnosticModeService(workspaceServices.Workspace);
=> new DefaultDiagnosticModeService(workspaceServices.Workspace);

private class VisualStudioDiagnosticModeService : IDiagnosticModeService
private class DefaultDiagnosticModeService : IDiagnosticModeService
{
private readonly Workspace _workspace;
private readonly Dictionary<Option2<DiagnosticMode>, Lazy<DiagnosticMode>> _optionToMode = new();

public VisualStudioDiagnosticModeService(Workspace workspace)
public DefaultDiagnosticModeService(Workspace workspace)
{
_workspace = workspace;
}
Expand Down Expand Up @@ -65,11 +64,26 @@ private DiagnosticMode ComputeDiagnosticMode(Option2<DiagnosticMode> option)
if (inCodeSpacesServer)
return DiagnosticMode.Pull;

var diagnosticModeOption = _workspace.Options.GetOption(option);

// If the workspace diagnostic mode is set to Default, defer to the feature flag service.
if (diagnosticModeOption == DiagnosticMode.Default)
{
return GetDiagnosticModeFromFeatureFlag();
}

// Otherwise, defer to the workspace+option to determine what mode we're in.
return _workspace.Options.GetOption(option);
return diagnosticModeOption;
}

private DiagnosticMode GetDiagnosticModeFromFeatureFlag()
{
var featureFlagService = _workspace.Services.GetRequiredService<IExperimentationService>();
var isPullDiagnosticExperimentEnabled = featureFlagService.IsExperimentEnabled(WellKnownExperimentNames.LspPullDiagnosticsFeatureFlag);
return isPullDiagnosticExperimentEnabled ? DiagnosticMode.Pull : DiagnosticMode.Push;
}

private bool IsInCodeSpacesServer()
private static bool IsInCodeSpacesServer()
{
// hack until there is an officially supported free-threaded synchronous platform API to ask this question.
return Environment.GetEnvironmentVariable("VisualStudioServerMode") == "1";
Expand Down
27 changes: 0 additions & 27 deletions src/Features/Core/Portable/Diagnostics/IDiagnosticModeService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Composition;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.Options;

namespace Microsoft.CodeAnalysis.Diagnostics
Expand All @@ -19,30 +16,6 @@ internal interface IDiagnosticModeService : IWorkspaceService
DiagnosticMode GetDiagnosticMode(Option2<DiagnosticMode> diagnosticMode);
}

[ExportWorkspaceServiceFactory(typeof(IDiagnosticModeService)), Shared]
internal class DefaultDiagnosticModeServiceFactory : IWorkspaceServiceFactory
{
[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public DefaultDiagnosticModeServiceFactory()
{
}

public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices)
=> new DefaultDiagnosticModeService(workspaceServices.Workspace);

private class DefaultDiagnosticModeService : IDiagnosticModeService
{
private readonly Workspace _workspace;

public DefaultDiagnosticModeService(Workspace workspace)
=> _workspace = workspace;

public DiagnosticMode GetDiagnosticMode(Option2<DiagnosticMode> diagnosticMode)
=> _workspace.Options.GetOption(diagnosticMode);
}
}

internal static class DiagnosticModeExtensions
{
public static DiagnosticMode GetDiagnosticMode(this Workspace workspace, Option2<DiagnosticMode> option)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Editor.UnitTests.Diagnostics;
using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces;
using Microsoft.CodeAnalysis.Experiments;
using Microsoft.CodeAnalysis.LanguageServer.Handler;
using Microsoft.CodeAnalysis.Shared.TestHooks;
using Microsoft.CodeAnalysis.SolutionCrawler;
using Microsoft.CodeAnalysis.Test.Utilities;
using Microsoft.CodeAnalysis.Text;
using Microsoft.VisualStudio.LanguageServer.Protocol;
using Roslyn.Test.Utilities;
Expand Down Expand Up @@ -77,6 +79,45 @@ public async Task TestNoDocumentDiagnosticsForOpenFilesWithFSAOffIfInPushMode()
Assert.Empty(results.Single().Diagnostics);
}

[Fact]
public async Task TestNoDocumentDiagnosticsForOpenFilesIfDefaultAndFeatureFlagOff()
{
var markup =
@"class A {";
using var testLspServer = CreateTestWorkspaceWithDiagnostics(markup, BackgroundAnalysisScope.OpenFilesAndProjects, DiagnosticMode.Default);

// Calling GetTextBuffer will effectively open the file.
testLspServer.TestWorkspace.Documents.Single().GetTextBuffer();
var document = testLspServer.GetCurrentSolution().Projects.Single().Documents.Single();
await OpenDocumentAsync(testLspServer, document);

// Ensure we get no diagnostics when feature flag is off.
var testExperimentationService = (TestExperimentationService)testLspServer.TestWorkspace.Services.GetRequiredService<IExperimentationService>();
testExperimentationService.SetExperimentOption(WellKnownExperimentNames.LspPullDiagnosticsFeatureFlag, false);

var results = await RunGetDocumentPullDiagnosticsAsync(testLspServer, document);
Assert.Empty(results.Single().Diagnostics);
}

[Fact]
public async Task TestDocumentDiagnosticsForOpenFilesIfDefaultAndFeatureFlagOn()
{
var markup =
@"class A {";
using var testLspServer = CreateTestWorkspaceWithDiagnostics(markup, BackgroundAnalysisScope.OpenFilesAndProjects, DiagnosticMode.Default);

// Calling GetTextBuffer will effectively open the file.
testLspServer.TestWorkspace.Documents.Single().GetTextBuffer();
var document = testLspServer.GetCurrentSolution().Projects.Single().Documents.Single();
await OpenDocumentAsync(testLspServer, document);

var testExperimentationService = (TestExperimentationService)testLspServer.TestWorkspace.Services.GetRequiredService<IExperimentationService>();
testExperimentationService.SetExperimentOption(WellKnownExperimentNames.LspPullDiagnosticsFeatureFlag, true);

var results = await RunGetDocumentPullDiagnosticsAsync(testLspServer, document);
Assert.Equal("CS1513", results.Single().Diagnostics.Single().Code);
}

[Fact]
public async Task TestDocumentDiagnosticsForRemovedDocument()
{
Expand Down Expand Up @@ -506,26 +547,29 @@ private static WorkspaceDocumentDiagnosticsParams CreateWorkspaceDiagnosticParam
}

private TestLspServer CreateTestWorkspaceWithDiagnostics(string markup, BackgroundAnalysisScope scope, bool pullDiagnostics = true)
=> CreateTestWorkspaceWithDiagnostics(markup, scope, pullDiagnostics ? DiagnosticMode.Pull : DiagnosticMode.Push);

private TestLspServer CreateTestWorkspaceWithDiagnostics(string markup, BackgroundAnalysisScope scope, DiagnosticMode mode)
{
var testLspServer = CreateTestLspServer(markup, out _);
InitializeDiagnostics(scope, testLspServer.TestWorkspace, pullDiagnostics);
InitializeDiagnostics(scope, testLspServer.TestWorkspace, mode);
return testLspServer;
}

private TestLspServer CreateTestWorkspaceWithDiagnostics(string[] markups, BackgroundAnalysisScope scope, bool pullDiagnostics = true)
{
var testLspServer = CreateTestLspServer(markups, out _);
InitializeDiagnostics(scope, testLspServer.TestWorkspace, pullDiagnostics);
InitializeDiagnostics(scope, testLspServer.TestWorkspace, pullDiagnostics ? DiagnosticMode.Pull : DiagnosticMode.Push);
return testLspServer;
}

private static void InitializeDiagnostics(BackgroundAnalysisScope scope, TestWorkspace workspace, bool pullDiagnostics)
private static void InitializeDiagnostics(BackgroundAnalysisScope scope, TestWorkspace workspace, DiagnosticMode diagnosticMode)
{
workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(
workspace.Options
.WithChangedOption(SolutionCrawlerOptions.BackgroundAnalysisScopeOption, LanguageNames.CSharp, scope)
.WithChangedOption(SolutionCrawlerOptions.BackgroundAnalysisScopeOption, LanguageNames.VisualBasic, scope)
.WithChangedOption(InternalDiagnosticsOptions.NormalDiagnosticMode, pullDiagnostics ? DiagnosticMode.Pull : DiagnosticMode.Push)));
.WithChangedOption(InternalDiagnosticsOptions.NormalDiagnosticMode, diagnosticMode)));

var analyzerReference = new TestAnalyzerReferenceByLanguage(DiagnosticExtensions.GetCompilerDiagnosticAnalyzersMap());
workspace.TryApplyChanges(workspace.CurrentSolution.WithAnalyzerReferences(new[] { analyzerReference }));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@
<CheckBox x:Name="Enable_pull_diagnostics_experimental_requires_restart"
Content="{x:Static local:AdvancedOptionPageStrings.Option_Enable_pull_diagnostics_experimental_requires_restart}"
Checked="Enable_pull_diagnostics_experimental_requires_restart_Checked"
Unchecked="Enable_pull_diagnostics_experimental_requires_restart_Unchecked"/>
Unchecked="Enable_pull_diagnostics_experimental_requires_restart_Unchecked"
Indeterminate="Enable_pull_diagnostics_experimental_requires_restart_Indeterminate"
IsThreeState="True"/>

<CheckBox x:Name="Enable_Razor_pull_diagnostics_experimental_requires_restart"
Content="{x:Static local:AdvancedOptionPageStrings.Option_Enable_Razor_pull_diagnostics_experimental_requires_restart}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,21 @@ internal override void OnLoad()

private void UpdatePullDiagnosticsOptions()
{
Enable_pull_diagnostics_experimental_requires_restart.IsChecked = OptionStore.GetOption(InternalDiagnosticsOptions.NormalDiagnosticMode) == DiagnosticMode.Pull;
var normalPullDiagnosticsOption = OptionStore.GetOption(InternalDiagnosticsOptions.NormalDiagnosticMode);
Enable_pull_diagnostics_experimental_requires_restart.IsChecked = GetDiagnosticModeCheckboxValue(normalPullDiagnosticsOption);

Enable_Razor_pull_diagnostics_experimental_requires_restart.IsChecked = OptionStore.GetOption(InternalDiagnosticsOptions.RazorDiagnosticMode) == DiagnosticMode.Pull;

static bool? GetDiagnosticModeCheckboxValue(DiagnosticMode mode)
{
return mode switch
{
DiagnosticMode.Push => false,
DiagnosticMode.Pull => true,
DiagnosticMode.Default => null,
_ => throw new System.ArgumentException("unknown diagnostic mode"),
};
}
}

private void Enable_pull_diagnostics_experimental_requires_restart_Checked(object sender, RoutedEventArgs e)
Expand All @@ -168,6 +181,12 @@ private void Enable_pull_diagnostics_experimental_requires_restart_Unchecked(obj
UpdatePullDiagnosticsOptions();
}

private void Enable_pull_diagnostics_experimental_requires_restart_Indeterminate(object sender, RoutedEventArgs e)
{
this.OptionStore.SetOption(InternalDiagnosticsOptions.NormalDiagnosticMode, DiagnosticMode.Default);
UpdatePullDiagnosticsOptions();
}

private void Enable_Razor_pull_diagnostics_experimental_requires_restart_Checked(object sender, RoutedEventArgs e)
{
this.OptionStore.SetOption(InternalDiagnosticsOptions.RazorDiagnosticMode, DiagnosticMode.Pull);
Expand Down
6 changes: 6 additions & 0 deletions src/Workspaces/Core/Portable/Diagnostics/DiagnosticMode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,11 @@ internal enum DiagnosticMode
/// responding to LSP pull requests for them.
/// </summary>
Pull,

/// <summary>
/// Default mode - when the option is set to default we use a feature flag to determine if we're
/// is in <see cref="Push"/> or <see cref="Pull"/>
/// </summary>
Default,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ internal static class InternalDiagnosticsOptions
public static readonly Option2<bool> ProcessHiddenDiagnostics = new(nameof(InternalDiagnosticsOptions), nameof(ProcessHiddenDiagnostics), defaultValue: false,
storageLocations: new LocalUserProfileStorageLocation(LocalRegistryPath + "Process Hidden Diagnostics"));

public static readonly Option2<DiagnosticMode> NormalDiagnosticMode = new(nameof(InternalDiagnosticsOptions), nameof(NormalDiagnosticMode), defaultValue: DiagnosticMode.Push,
public static readonly Option2<DiagnosticMode> NormalDiagnosticMode = new(nameof(InternalDiagnosticsOptions), nameof(NormalDiagnosticMode), defaultValue: DiagnosticMode.Default,
storageLocations: new LocalUserProfileStorageLocation(LocalRegistryPath + "NormalDiagnosticMode"));

public static readonly Option2<DiagnosticMode> RazorDiagnosticMode = new(nameof(InternalDiagnosticsOptions), nameof(RazorDiagnosticMode), defaultValue: DiagnosticMode.Pull,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,6 @@ internal static class WellKnownExperimentNames
public const string CloudCache = "Roslyn.CloudCache";
public const string UnnamedSymbolCompletionDisabled = "Roslyn.UnnamedSymbolCompletionDisabled";
public const string RazorLspEditorFeatureFlag = "Razor.LSP.Editor";
public const string LspPullDiagnosticsFeatureFlag = "Lsp.PullDiagnostics";
}
}

0 comments on commit 03a07d1

Please sign in to comment.