From d8524f6221294710964bee05816f4430ef8e4ed9 Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Mon, 4 Mar 2024 10:45:56 -0800 Subject: [PATCH 1/2] Avoid creating ProjectSnapshotManager in CSharpVirtualDocumentFactory `CSharpVirtualDocumentFactory` unnecessarily forces the `ProjectSnapshotManager` to be created just to see if it contains any projects. To avoid this, I've added a `TryGetInstance(...)` method that returns the `ProjectSnapshotManager` only if it has been created. Then, in `CSharpVirtualDocumentFactory` it avoids force-creating the `ProjectSnapshotManager` if `TryGetInstance(...)` returns false. --- .../LspProjectSnapshotManagerAccessor.cs | 7 +++ .../IProjectSnapshotManagerAccessor.cs | 7 +++ .../CSharpVirtualDocumentFactory.cs | 5 +- ...ualStudioProjectSnapshotManagerAccessor.cs | 7 +++ .../DefaultRazorComponentSearchEngineTest.cs | 11 ++++- .../TestProjectSnapshotManagerAccessor.cs | 11 ++++- .../CSharpVirtualDocumentFactoryTest.cs | 48 ++++++++++++++++++- 7 files changed, 92 insertions(+), 4 deletions(-) diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/LspProjectSnapshotManagerAccessor.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/LspProjectSnapshotManagerAccessor.cs index 650f3a2d05e..92afac471ca 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/LspProjectSnapshotManagerAccessor.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/LspProjectSnapshotManagerAccessor.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See License.txt in the project root for license information. using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using Microsoft.CodeAnalysis.Razor; using Microsoft.CodeAnalysis.Razor.ProjectSystem; using Microsoft.CodeAnalysis.Razor.Workspaces; @@ -39,4 +40,10 @@ public ProjectSnapshotManagerBase Instance return _instance; } } + + public bool TryGetInstance([NotNullWhen(true)] out ProjectSnapshotManagerBase? instance) + { + instance = _instance; + return instance is not null; + } } diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/IProjectSnapshotManagerAccessor.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/IProjectSnapshotManagerAccessor.cs index 2c636d0e4df..b3c9975917d 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/IProjectSnapshotManagerAccessor.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/IProjectSnapshotManagerAccessor.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the MIT license. See License.txt in the project root for license information. +using System.Diagnostics.CodeAnalysis; using Microsoft.CodeAnalysis.Razor.ProjectSystem; namespace Microsoft.CodeAnalysis.Razor.Workspaces; @@ -8,4 +9,10 @@ namespace Microsoft.CodeAnalysis.Razor.Workspaces; internal interface IProjectSnapshotManagerAccessor { ProjectSnapshotManagerBase Instance { get; } + + /// + /// Retrieves the instance. Returns + /// if the instance has been created; otherwise, . + /// + bool TryGetInstance([NotNullWhen(true)] out ProjectSnapshotManagerBase? instance); } diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/CSharpVirtualDocumentFactory.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/CSharpVirtualDocumentFactory.cs index 594b11894c4..e997946ed4e 100644 --- a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/CSharpVirtualDocumentFactory.cs +++ b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/CSharpVirtualDocumentFactory.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Collections.Immutable; using System.ComponentModel.Composition; using System.Diagnostics.CodeAnalysis; using System.Linq; @@ -192,7 +193,9 @@ private IEnumerable GetProjectKeys(Uri hostDocumentUri) yield break; } - var projects = _projectSnapshotManagerAccessor.Instance.GetProjects(); + var projects = _projectSnapshotManagerAccessor.TryGetInstance(out var projectSnapshotManager) + ? projectSnapshotManager.GetProjects() + : ImmutableArray.Empty; var inAny = false; var normalizedDocumentPath = FilePathService.GetProjectSystemFilePath(hostDocumentUri); diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/VisualStudioProjectSnapshotManagerAccessor.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/VisualStudioProjectSnapshotManagerAccessor.cs index 910c76c3eb0..96f559a0913 100644 --- a/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/VisualStudioProjectSnapshotManagerAccessor.cs +++ b/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/VisualStudioProjectSnapshotManagerAccessor.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See License.txt in the project root for license information. using System.ComponentModel.Composition; +using System.Diagnostics.CodeAnalysis; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Razor; using Microsoft.CodeAnalysis.Razor.ProjectSystem; @@ -59,4 +60,10 @@ ProjectSnapshotManagerBase Create() } } } + + public bool TryGetInstance([NotNullWhen(true)] out ProjectSnapshotManagerBase? instance) + { + instance = _projectManager; + return instance is not null; + } } diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/DefaultRazorComponentSearchEngineTest.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/DefaultRazorComponentSearchEngineTest.cs index b4caaba686d..77c70c62986 100644 --- a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/DefaultRazorComponentSearchEngineTest.cs +++ b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/DefaultRazorComponentSearchEngineTest.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Immutable; +using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.Razor.Extensions.Version2_X; @@ -180,6 +181,14 @@ internal static IProjectSnapshotManagerAccessor CreateProjectSnapshotManagerAcce internal class TestProjectSnapshotManagerAccessor(ProjectSnapshotManagerBase instance) : IProjectSnapshotManagerAccessor { - public ProjectSnapshotManagerBase Instance => instance; + private readonly ProjectSnapshotManagerBase _instance = instance; + + public ProjectSnapshotManagerBase Instance => _instance; + + public bool TryGetInstance([NotNullWhen(true)] out ProjectSnapshotManagerBase instance) + { + instance = _instance; + return instance is not null; + } } } diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/TestProjectSnapshotManagerAccessor.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/TestProjectSnapshotManagerAccessor.cs index a28176a242b..3e71a3a2599 100644 --- a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/TestProjectSnapshotManagerAccessor.cs +++ b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/TestProjectSnapshotManagerAccessor.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the MIT license. See License.txt in the project root for license information. +using System.Diagnostics.CodeAnalysis; using Microsoft.CodeAnalysis.Razor.ProjectSystem; using Microsoft.CodeAnalysis.Razor.Workspaces; @@ -8,5 +9,13 @@ namespace Microsoft.AspNetCore.Razor.LanguageServer.Test; internal class TestProjectSnapshotManagerAccessor(ProjectSnapshotManagerBase instance) : IProjectSnapshotManagerAccessor { - public ProjectSnapshotManagerBase Instance => instance; + private readonly ProjectSnapshotManagerBase _instance = instance; + + public ProjectSnapshotManagerBase Instance => _instance; + + public bool TryGetInstance([NotNullWhen(true)] out ProjectSnapshotManagerBase? instance) + { + instance = _instance; + return instance is not null; + } } diff --git a/src/Razor/test/Microsoft.VisualStudio.LanguageServerClient.Razor.Test/CSharpVirtualDocumentFactoryTest.cs b/src/Razor/test/Microsoft.VisualStudio.LanguageServerClient.Razor.Test/CSharpVirtualDocumentFactoryTest.cs index 1d8649bde4c..cb2d773cc81 100644 --- a/src/Razor/test/Microsoft.VisualStudio.LanguageServerClient.Razor.Test/CSharpVirtualDocumentFactoryTest.cs +++ b/src/Razor/test/Microsoft.VisualStudio.LanguageServerClient.Razor.Test/CSharpVirtualDocumentFactoryTest.cs @@ -10,6 +10,7 @@ using Microsoft.AspNetCore.Razor.Test.Common.LanguageServer; using Microsoft.AspNetCore.Razor.Test.Common.Workspaces; using Microsoft.CodeAnalysis.Razor; +using Microsoft.CodeAnalysis.Razor.ProjectSystem; using Microsoft.CodeAnalysis.Razor.Workspaces; using Microsoft.VisualStudio.Editor.Razor; using Microsoft.VisualStudio.LanguageServer.ContainedLanguage; @@ -78,6 +79,40 @@ public void TryCreateMultipleFor_NonRazorLSPBuffer_ReturnsFalse() Assert.Null(virtualDocuments); } + [Fact] + public void TryCreateMultipleFor_NoProjectSnapshotManager_ReturnsFalse() + { + // Arrange + var uri = new Uri("C:/path/to/file.razor"); + var uriProvider = StrictMock.Of(x => + x.GetOrCreate(It.IsAny()) == uri); + + var projectManagerAccessorMock = new StrictMock(); + + ProjectSnapshotManagerBase instance = null; + projectManagerAccessorMock + .Setup(x => x.TryGetInstance(out instance)) + .Returns(false); + + var factory = new CSharpVirtualDocumentFactory( + _contentTypeRegistryService, + _textBufferFactoryService, + StrictMock.Of(), + uriProvider, + _filePathService, + projectManagerAccessorMock.Object, + TestLanguageServerFeatureOptions.Instance, + LoggerFactory, + telemetryReporter: null!); + + // Act + var result = factory.TryCreateMultipleFor(_nonRazorLSPBuffer, out var virtualDocuments); + + // Assert + Assert.False(result); + Assert.Null(virtualDocuments); + } + [Fact] public void TryCreateMultipleFor_RazorLSPBuffer_ReturnsCSharpVirtualDocumentAndTrue() { @@ -117,7 +152,18 @@ public void TryCreateMultipleFor_RazorLSPBuffer_ReturnsMultipleCSharpVirtualDocu project = TestProjectSnapshot.Create(@"C:\path\to\project2.csproj", @"C:\path\to\obj2", Array.Empty(), RazorConfiguration.Default, projectWorkspaceState: null); projectSnapshotManager.ProjectAdded(project.HostProject); projectSnapshotManager.CreateAndAddDocument(project, @"C:\path\to\file.razor"); - var projectSnapshotManagerAccessor = Mock.Of(a => a.Instance == projectSnapshotManager, MockBehavior.Strict); + + var projectManagerAccessorMock = new StrictMock(); + projectManagerAccessorMock + .SetupGet(x => x.Instance) + .Returns(projectSnapshotManager); + + ProjectSnapshotManagerBase instance = projectSnapshotManager; + projectManagerAccessorMock + .Setup(x => x.TryGetInstance(out instance)) + .Returns(true); + + var projectSnapshotManagerAccessor = projectManagerAccessorMock.Object; var languageServerFeatureOptions = new TestLanguageServerFeatureOptions(includeProjectKeyInGeneratedFilePath: true); var filePathService = new FilePathService(languageServerFeatureOptions); From 84b55a8b1eb9d46c6d2997a5fc2dda3cc3714d1b Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Mon, 4 Mar 2024 10:53:12 -0800 Subject: [PATCH 2/2] Avoid creating ProjectSnapshotManager in FallbackProjectManager --- .../ProjectSystem/FallbackProjectManager.cs | 6 ++++-- .../ProjectSystem/FallbackProjectManagerTest.cs | 14 +++++++++++--- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/FallbackProjectManager.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/FallbackProjectManager.cs index 30f46fe183a..9d48c5eaaca 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/FallbackProjectManager.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/FallbackProjectManager.cs @@ -44,7 +44,8 @@ internal async Task DynamicFileAddedAsync( { try { - if (_projectManagerAccessor.Instance.TryGetLoadedProject(razorProjectKey, out var project)) + if (_projectManagerAccessor.TryGetInstance(out var projectSnapshotManager) && + projectSnapshotManager.TryGetLoadedProject(razorProjectKey, out var project)) { if (project is ProjectSnapshot { HostProject: FallbackHostProject }) { @@ -76,7 +77,8 @@ internal async Task DynamicFileRemovedAsync( { try { - if (_projectManagerAccessor.Instance.TryGetLoadedProject(razorProjectKey, out var project) && + if (_projectManagerAccessor.TryGetInstance(out var projectSnapshotManager) && + projectSnapshotManager.TryGetLoadedProject(razorProjectKey, out var project) && project is ProjectSnapshot { HostProject: FallbackHostProject }) { // If this is a fallback project, then Roslyn may not track documents in the project, so these dynamic file notifications diff --git a/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/ProjectSystem/FallbackProjectManagerTest.cs b/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/ProjectSystem/FallbackProjectManagerTest.cs index 1682b2be079..202619f4a93 100644 --- a/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/ProjectSystem/FallbackProjectManagerTest.cs +++ b/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/ProjectSystem/FallbackProjectManagerTest.cs @@ -34,13 +34,21 @@ public FallbackProjectManagerTest(ITestOutputHelper testOutputHelper) _projectManager = new TestProjectSnapshotManager(ProjectEngineFactoryProvider, Dispatcher); - var projectManagerAccessor = StrictMock.Of(a => - a.Instance == _projectManager); + var projectManagerAccessorMock = new StrictMock(); + + projectManagerAccessorMock + .SetupGet(x => x.Instance) + .Returns(_projectManager); + + ProjectSnapshotManagerBase? instanceResult = _projectManager; + projectManagerAccessorMock + .Setup(x => x.TryGetInstance(out instanceResult)) + .Returns(true); _fallbackProjectManger = new FallbackProjectManager( _projectConfigurationFilePathStore, languageServerFeatureOptions, - projectManagerAccessor, + projectManagerAccessorMock.Object, Dispatcher, WorkspaceProvider, NoOpTelemetryReporter.Instance);