Further refactoring of Razor tooling project system #11320

DustinCampbell · 2024-12-18T01:46:04Z

There's a lot of code changes here

Remove IProjectSnapshotManger interface completely
Push IDocumentSnapshot and IProjectSnapshot down to MS.ANC.Razor.ProjectEngineHost
Split out ILegacyDocumentSnapshot and ILegacyProjectSnapshot to serve the needs of the legacy editor.
Prune IDocumentSnapshot and IProjectSnapshot and hide implementation details, such as Configuration, ProjectWorkspaceState and GetProjectEngine().
Remove ComputedStateTracker and stop weakly caching RazorCodeDocument on DocumentState.
Introduce a variation of AsyncLazy from Roslyn. (A key difference between this and the VS AsyncLazy is that it doesn't have a JTF dependency.)
Remove tricky version logic from ProjectState.
Clean up ProjectState and DocumentState API.

VS Insertion PR: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/599715

The Razor project/document state model only need two compiler options: ForceRuntimeCodeGeneration and UseRoslynTokenizer. So, using LanguageServerFeatureOptions directly within ProjectState is a bit overkill. This change introduces a new enum, RazorCompilerOptions, to track the two options that the project model needs to control Razor compilation.

This change cleans up the TestProjectEngineFactoryProvider to be a bit more usable.

There were a number of test hooks in DocumentState and ProjectState, such as settable properties and virtual methods. This change removes those test hooks by cleaning up and refactoring ProjectStateTest.

This change attempts to be a bit more efficient what updating the ImportsToRelatedDocuments map. In addition, related documents are now stored in ImmutableHashSets.

This change brings Roslyn's AsyncLazy<T> implementation to Razor, but removes the synchronous computation path, since Razor doesn't need it.

This change removes the DocumentState.ComputedTracker, and DocumentState no longer weakly caches it's generated RazorCodeDocument. In addition, IDocumentSnapshot.GetGeneratedOutputAsync(...) has been modified to remove the forceDesignTimeGeneratedOutput parameter and the remove project/document snapshot implementations now use Razor's Roslyn-based AsyncLazy.

This change adds ILegacyDocumentSnapshot and ILegacyProjectSnapshot to be used exclusively by the legacy editor. These provide synchronous methods that should *only* be used by the legacy editor.

This property isn't actually used outside of ProjectState.

This change fully removes the IProjectSnapshotManager interface. Components in the language server and VS client layers can import a ProjectSnapshotManager. ProjectSnapshotManager provides direct access to DocumentSnapshot and ProjectSnapshot, which implement IDocumentSnapshot and IProjectSnapshot.

Retrieving a RazorProjectEngine for a project snapshot is an implementation detail and doesn't need to be exposed on IProjectSnapshot.

ProjectWorkspaceState is an implementation detail and does not need to be exposed on IProjectSnapshot.

IDocumentSnapshot and IProjectSnapshot and now more fundamental and below in a lower layer of Razor tooling.

The RazorConfiguration of a project snapshot is an implementation detail and doesn't need to be exposed.

davidwengier

This is so awesome. I did have a few questions though, that are probably just me missing something or misunderstanding something.

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/ProjectState.cs

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/CompilationHelpers.cs

...r/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/Sources/GeneratedOutputSource.cs

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/IDesignTimeCodeGenerator.cs

src/Razor/src/Microsoft.VisualStudio.LegacyEditor.Razor/EphemeralProjectSnapshot.cs

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/ProjectDifference.cs

src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/ProjectSystem/RemoteProjectSnapshot.cs

davidwengier · 2024-12-18T04:02:57Z

In my professional opinion, we can consider the integration tests to have passed. The one failing test, HtmlCodeActionsTests_RemoveTag_WithCSharpContent, is known to be flaky at times.

ryzngard

Looking good! Most of this may seem scattered. I reviewed commit by commit since the UI for over 200 files wasn't working too well. Thanks for the detailed commit messages

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/DocumentState.cs

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/ProjectState.cs

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/DocumentState.cs

ryzngard · 2024-12-18T21:30:52Z

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/IDocumentSnapshot.cs

-    ValueTask<RazorCodeDocument> GetGeneratedOutputAsync(
-        bool forceDesignTimeGeneratedOutput,
-        CancellationToken cancellationToken);
+    ValueTask<RazorCodeDocument> GetGeneratedOutputAsync(CancellationToken cancellationToken);


💡 should we take the time to change this to GetGeneratedDocumentAsync? 👀

Not yet. I have a whole series of renames coming soon. IDocumentSnapshot will become IRazorDocument, IProjectSnapshot will become IRazorProject.

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/IProjectSnapshot.cs

...azor.LanguageServer.Test/CodeActions/Razor/ExtractToComponentCodeActionResolverTest.NetFx.cs

To account for recent formatting changes in main, `IDesignTimeCodeGenerator` does not compile if `FORMAT_FUSE` is defined.

Changes in this PR caused the number of projects needing to compile with FORMAT_FUSE from two to four. So, this change pushes FORMAT_FUSE into Directory.Build.props to avoid needing to update four project files.

DustinCampbell added 30 commits December 17, 2024 17:37

Improve ProjectState.Create(...) factory methods and constructors

eb5d916

Clean up TestProjectEngineFactoryProvider a bit

ea3fd03

This change cleans up the TestProjectEngineFactoryProvider to be a bit more usable.

Seal DocumentState and ProjectState and make effectively immutable

9042ae4

There were a number of test hooks in DocumentState and ProjectState, such as settable properties and virtual methods. This change removes those test hooks by cleaning up and refactoring ProjectStateTest.

Rework ProjectState.ImportsToRelatedDocuments map

ee0e239

This change attempts to be a bit more efficient what updating the ImportsToRelatedDocuments map. In addition, related documents are now stored in ImmutableHashSets.

Simplify DocumentState.Version tracking

3ba0eda

Remove unused DocumentState property

fdf9486

Clean up DocumentState With* methods

5b0bfb6

Move DocumentState helper methods for computing imports

8d376cc

Clean up EmptyTextLoader (currently only used by tests)

4601b2d

Replace DocumentState.EmptyLoader with EmptyTextLoader.Instance

e3a9a86

Replace CreateEmptyTextLoader with EmptyTextLoader.Instance

4d18137

Clean up TestMocks.CreateTextLoader calls

4c9becf

Clean up calls to ProjectState.AddDocument with empty loaders

a3b25f8

Augment Assumed class with new helpers and message parameters

18ad48f

Introduce AsyncLazy<T> variation from Roslyn

ae4e86c

This change brings Roslyn's AsyncLazy<T> implementation to Razor, but removes the synchronous computation path, since Razor doesn't need it.

Introduce ITextAndVersionSource to manage text loading in DocumentState

813e448

Don't update ProjectState when DocumentState doesn't

b508614

Move DocumentState helper methods for compilation

1f9144f

Introduce legacy snapshot interfaces to support legacy editor

1e045ea

This change adds ILegacyDocumentSnapshot and ILegacyProjectSnapshot to be used exclusively by the legacy editor. These provide synchronous methods that should *only* be used by the legacy editor.

Remove IProjectSnapshot.Version

0049ac0

This property isn't actually used outside of ProjectState.

Remove ProjectState.DocumentCollectionVersion

b5acc30

Remove remaining version logic from ProjectState

f26bd81

Make IProjectSnapshot.GetProjectEngine() async

b341603

Remove IProjectSnapshot.GetProjectEngineAsync

c838a75

Retrieving a RazorProjectEngine for a project snapshot is an implementation detail and doesn't need to be exposed on IProjectSnapshot.

Remove IProjectSnapshot.ProjectWorkspaceState

49a973c

ProjectWorkspaceState is an implementation detail and does not need to be exposed on IProjectSnapshot.

Move IDocumentSnapshot and IProjectSnapshot to lower tooling layer

04bf4bf

IDocumentSnapshot and IProjectSnapshot and now more fundamental and below in a lower layer of Razor tooling.

Remove IProjectSnapshot.Configuration

f27d768

The RazorConfiguration of a project snapshot is an implementation detail and doesn't need to be exposed.

DustinCampbell added 2 commits December 17, 2024 17:38

Don't use LINQ Count() method

67e3227

Compute test span correctly

295d482

DustinCampbell marked this pull request as ready for review December 18, 2024 02:59

DustinCampbell requested review from a team as code owners December 18, 2024 02:59

davidwengier reviewed Dec 18, 2024

View reviewed changes

DustinCampbell added 3 commits December 18, 2024 04:20

Double-checked lock

74167ba

Fix comment

670eae2

Remove unused method

f28f239

ryzngard approved these changes Dec 18, 2024

View reviewed changes

davidwengier approved these changes Dec 18, 2024

View reviewed changes

DustinCampbell added 6 commits December 18, 2024 14:56

Rename method to ProjectState.UpdateRelatedDocumentsIfNecessary

284c7d7

Delete unused ExtractToComponentResolverDocumentContextFactory

f952c9b

Remove all but one ProjectState.Create(...) factory method

1deb4a0

RazorProjectService: Use TryGetDocument rather than pattern matching

d35e121

RazorProjectService: Rename for consistency throughout

b51fabc

ProjectState: Don't perform string replacements unless necessary

f3fed12

ryzngard approved these changes Dec 19, 2024

View reviewed changes

...azor.LanguageServer.Test/CodeActions/Razor/ExtractToComponentCodeActionResolverTest.NetFx.cs Show resolved Hide resolved

DustinCampbell added 3 commits January 6, 2025 13:24

Merge branch 'main' into cleanup-project-and-documentstate

80709ba

To account for recent formatting changes in main, `IDesignTimeCodeGenerator` does not compile if `FORMAT_FUSE` is defined.

Move FORMAT_FUSE constant to Directory.Build.props

f9096fe

Changes in this PR caused the number of projects needing to compile with FORMAT_FUSE from two to four. So, this change pushes FORMAT_FUSE into Directory.Build.props to avoid needing to update four project files.

Merge branch 'main' into cleanup-project-and-documentstate

4d299ee

DustinCampbell merged commit acde5a8 into dotnet:main Jan 7, 2025
17 checks passed

DustinCampbell deleted the cleanup-project-and-documentstate branch January 7, 2025 16:04

dotnet-policy-service bot added this to the Next milestone Jan 7, 2025

This was referenced Jan 14, 2025

[Automated] PRs inserted in VS build main-35713.149 #11389

Closed

[Automated] PRs inserted in VS build feature.debugger.main-35717.106 #11401

Closed

[Automated] PRs inserted in VS build feature.debugger.main-35717.155 #11403

Closed

This was referenced Jan 23, 2025

[Automated] PRs inserted in VS build feature.debugger.shadowDebug-35722.156 #11407

Closed

[Automated] PRs inserted in VS build feature.d18initial-10224.03 #11418

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Further refactoring of Razor tooling project system #11320

Further refactoring of Razor tooling project system #11320

DustinCampbell commented Dec 18, 2024 •

edited

Loading

davidwengier left a comment

davidwengier commented Dec 18, 2024

ryzngard left a comment

ryzngard Dec 18, 2024

DustinCampbell Dec 18, 2024 •

edited

Loading

Further refactoring of Razor tooling project system #11320

Further refactoring of Razor tooling project system #11320

Conversation

DustinCampbell commented Dec 18, 2024 • edited Loading

davidwengier left a comment

Choose a reason for hiding this comment

davidwengier commented Dec 18, 2024

ryzngard left a comment

Choose a reason for hiding this comment

ryzngard Dec 18, 2024

Choose a reason for hiding this comment

DustinCampbell Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

DustinCampbell commented Dec 18, 2024 •

edited

Loading

DustinCampbell Dec 18, 2024 •

edited

Loading