From 1152b7f747afdf72c473563481d9b016063c9f57 Mon Sep 17 00:00:00 2001 From: Chris Sienkiewicz Date: Thu, 20 Jul 2023 15:48:28 -0700 Subject: [PATCH 1/4] Restore perf work. Based on https://github.com/dotnet/razor/pull/8212 - Do an initial parse only once - If a document has changed, re-write the tag helpers, recording which ones were used - Re-run the tag helper re-write step, but check if the tag helpers have changed, and skip if we can --- ...aultRazorTagHelperContextDiscoveryPhase.cs | 2 +- .../RazorSourceGenerator.Helpers.cs | 10 +- .../RazorSourceGenerator.cs | 93 +++++++--- .../RazorSourceGeneratorEventSource.cs | 24 +++ .../SourceGeneratorProjectEngine.cs | 119 ++++++++++++ .../SourceGeneratorRazorCodeDocument.cs | 31 ++++ .../AbstractBenchmark.cs | 3 +- .../RazorSourceGeneratorTests.cs | 175 +++++++++++++----- 8 files changed, 379 insertions(+), 78 deletions(-) create mode 100644 src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/SourceGeneratorProjectEngine.cs create mode 100644 src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/SourceGeneratorRazorCodeDocument.cs diff --git a/src/Compiler/Microsoft.AspNetCore.Razor.Language/src/DefaultRazorTagHelperContextDiscoveryPhase.cs b/src/Compiler/Microsoft.AspNetCore.Razor.Language/src/DefaultRazorTagHelperContextDiscoveryPhase.cs index 49beec6adbf..9664139c23f 100644 --- a/src/Compiler/Microsoft.AspNetCore.Razor.Language/src/DefaultRazorTagHelperContextDiscoveryPhase.cs +++ b/src/Compiler/Microsoft.AspNetCore.Razor.Language/src/DefaultRazorTagHelperContextDiscoveryPhase.cs @@ -18,7 +18,7 @@ internal sealed class DefaultRazorTagHelperContextDiscoveryPhase : RazorEnginePh { protected override void ExecuteCore(RazorCodeDocument codeDocument) { - var syntaxTree = codeDocument.GetSyntaxTree(); + var syntaxTree = codeDocument.GetPreTagHelperSyntaxTree() ?? codeDocument.GetSyntaxTree(); ThrowForMissingDocumentDependency(syntaxTree); var descriptors = codeDocument.GetTagHelpers(); diff --git a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGenerator.Helpers.cs b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGenerator.Helpers.cs index 1bc16f425c7..a63c56f7738 100644 --- a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGenerator.Helpers.cs +++ b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGenerator.Helpers.cs @@ -83,8 +83,7 @@ private static RazorProjectEngine GetDiscoveryProjectEngine( return discoveryProjectEngine; } - private static RazorProjectEngine GetGenerationProjectEngine( - IReadOnlyList tagHelpers, + private static SourceGeneratorProjectEngine GetGenerationProjectEngine( SourceGeneratorProjectItem item, IEnumerable imports, RazorSourceGenerationOptions razorSourceGeneratorOptions, @@ -97,7 +96,7 @@ private static RazorProjectEngine GetGenerationProjectEngine( fileSystem.Add(import); } - var projectEngine = RazorProjectEngine.Create(razorSourceGeneratorOptions.Configuration, fileSystem, b => + var projectEngine = (DefaultRazorProjectEngine)RazorProjectEngine.Create(razorSourceGeneratorOptions.Configuration, fileSystem, b => { b.Features.Add(new DefaultTypeNameFeature()); b.SetRootNamespace(razorSourceGeneratorOptions.RootNamespace); @@ -110,16 +109,13 @@ private static RazorProjectEngine GetGenerationProjectEngine( options.SuppressAddComponentParameter = !isAddComponentParameterAvailable; })); - b.Features.Add(new StaticTagHelperFeature { TagHelpers = tagHelpers }); - b.Features.Add(new DefaultTagHelperDescriptorProvider()); - CompilerFeatures.Register(b); RazorExtensions.Register(b); b.SetCSharpLanguageVersion(razorSourceGeneratorOptions.CSharpLanguageVersion); }); - return projectEngine; + return new SourceGeneratorProjectEngine(projectEngine); } } } diff --git a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGenerator.cs b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGenerator.cs index 410a9603195..cb254920c3b 100644 --- a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGenerator.cs +++ b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGenerator.cs @@ -80,6 +80,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) var generatedDeclarationCode = componentFiles .Combine(importFiles.Collect()) .Combine(razorSourceGeneratorOptions) + .WithLambdaComparer((old, @new) => (old.Right.Equals(@new.Right) && old.Left.Left.Equals(@new.Left.Left) && old.Left.Right.SequenceEqual(@new.Left.Right)), (a) => a.GetHashCode()) .Select(static (pair, _) => { var ((sourceItem, importFiles), razorSourceGeneratorOptions) = pair; @@ -98,30 +99,35 @@ public void Initialize(IncrementalGeneratorInitializationContext context) var generatedDeclarationSyntaxTrees = generatedDeclarationCode .Combine(parseOptions) - .Select(static (pair, _) => + .Select(static (pair, ct) => { var (generatedDeclarationCode, parseOptions) = pair; - return CSharpSyntaxTree.ParseText(generatedDeclarationCode, (CSharpParseOptions)parseOptions); + return CSharpSyntaxTree.ParseText(generatedDeclarationCode, (CSharpParseOptions)parseOptions, cancellationToken: ct); + }); + + var declCompilation = generatedDeclarationSyntaxTrees + .Collect() + .Combine(compilation) + .Select((pair, _) => + { + return pair.Right.AddSyntaxTrees(pair.Left); }); - var tagHelpersFromCompilation = compilation - .Combine(generatedDeclarationSyntaxTrees.Collect()) + var tagHelpersFromCompilation = declCompilation .Combine(razorSourceGeneratorOptions) .Select(static (pair, _) => { RazorSourceGeneratorEventSource.Log.DiscoverTagHelpersFromCompilationStart(); - var ((compilation, generatedDeclarationSyntaxTrees), razorSourceGeneratorOptions) = pair; + var (compilation, razorSourceGeneratorOptions) = pair; var tagHelperFeature = new StaticCompilationTagHelperFeature(); var discoveryProjectEngine = GetDiscoveryProjectEngine(compilation.References.ToImmutableArray(), tagHelperFeature); - var compilationWithDeclarations = compilation.AddSyntaxTrees(generatedDeclarationSyntaxTrees); - - tagHelperFeature.Compilation = compilationWithDeclarations; - tagHelperFeature.TargetSymbol = compilationWithDeclarations.Assembly; + tagHelperFeature.Compilation = compilation; + tagHelperFeature.TargetSymbol = compilation.Assembly; - var result = (IList)tagHelperFeature.GetDescriptors(); + var result = tagHelperFeature.GetDescriptors(); RazorSourceGeneratorEventSource.Log.DiscoverTagHelpersFromCompilationStop(); return result; }) @@ -219,7 +225,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) var withOptions = sourceItems .Combine(importFiles.Collect()) - .Combine(allTagHelpers) + .WithLambdaComparer((old, @new) => old.Left.Equals(@new.Left) && old.Right.SequenceEqual(@new.Right), (a) => a.GetHashCode()) .Combine(razorSourceGeneratorOptions); var withOptionsDesignTime = withOptions @@ -238,31 +244,66 @@ public void Initialize(IncrementalGeneratorInitializationContext context) t.GetMembers("AddComponentParameter").Any(static m => m.DeclaredAccessibility == Accessibility.Public), compilation); }); - IncrementalValuesProvider<(string, RazorCodeDocument)> processed(bool designTime) => (designTime ? withOptionsDesignTime : withOptions) + IncrementalValuesProvider<(string, SourceGeneratorRazorCodeDocument)> processed(bool designTime) => (designTime ? withOptionsDesignTime : withOptions) .Combine(isAddComponentParameterAvailable) .Select((pair, _) => { - var ((((sourceItem, imports), allTagHelpers), razorSourceGeneratorOptions), isAddComponentParameterAvailable) = pair; + var (((sourceItem, imports), razorSourceGeneratorOptions), isAddComponentParameterAvailable) = pair; - var kind = designTime ? "DesignTime" : "Runtime"; - RazorSourceGeneratorEventSource.Log.RazorCodeGenerateStart(sourceItem.FilePath, kind); + RazorSourceGeneratorEventSource.Log.ParseRazorDocumentStart(sourceItem.RelativePhysicalPath); + + var projectEngine = GetGenerationProjectEngine(sourceItem, imports, razorSourceGeneratorOptions, isAddComponentParameterAvailable); + + var document = projectEngine.ProcessInitialParse(sourceItem, designTime); + + RazorSourceGeneratorEventSource.Log.ParseRazorDocumentStop(sourceItem.RelativePhysicalPath); + return (projectEngine, sourceItem.RelativePhysicalPath, document); + }) + + // Add the tag helpers in, but ignore if they've changed or not, only reprocessing the actual document changed + .Combine(allTagHelpers) + .WithLambdaComparer((old, @new) => old.Left.Equals(@new.Left), (item) => item.GetHashCode()) + .Select(static (pair, _) => + { + var ((projectEngine, filePath, codeDocument), allTagHelpers) = pair; + RazorSourceGeneratorEventSource.Log.RewriteTagHelpersStart(filePath); + + codeDocument = projectEngine.ProcessTagHelpers(codeDocument, allTagHelpers, checkForIdempotency: false); - var projectEngine = GetGenerationProjectEngine(allTagHelpers, sourceItem, imports, razorSourceGeneratorOptions, - isAddComponentParameterAvailable: isAddComponentParameterAvailable); + RazorSourceGeneratorEventSource.Log.RewriteTagHelpersStop(filePath); + return (projectEngine, filePath, codeDocument); + }) - var codeDocument = designTime - ? projectEngine.ProcessDesignTime(sourceItem) - : projectEngine.Process(sourceItem); + // next we do a second parse, along with the helpers, but check for idempotency. If the tag helpers used on the previous parse match, the compiler can skip re-computing them + .Combine(allTagHelpers) + .Select(static (pair, _) => + { - RazorSourceGeneratorEventSource.Log.RazorCodeGenerateStop(sourceItem.FilePath, kind); - return (filePath: sourceItem.RelativePhysicalPath, codeDocument); + var ((projectEngine, filePath, document), allTagHelpers) = pair; + RazorSourceGeneratorEventSource.Log.CheckAndRewriteTagHelpersStart(filePath); + + document = projectEngine.ProcessTagHelpers(document, allTagHelpers, checkForIdempotency: true); + + RazorSourceGeneratorEventSource.Log.CheckAndRewriteTagHelpersStop(filePath); + return (projectEngine, filePath, document); + }) + .Select((pair, _) => + { + var (projectEngine, filePath, document) = pair; + + var kind = designTime ? "DesignTime" : "Runtime"; + RazorSourceGeneratorEventSource.Log.RazorCodeGenerateStart(filePath, kind); + document = projectEngine.ProcessRemaining(document); + + RazorSourceGeneratorEventSource.Log.RazorCodeGenerateStop(filePath, kind); + return (filePath, document); }); var csharpDocuments = processed(designTime: false) .Select(static (pair, _) => { var (filePath, document) = pair; - return (filePath, csharpDocument: document.GetCSharpDocument()); + return (filePath, csharpDocument: document.CodeDocument.GetCSharpDocument()); }) .WithLambdaComparer(static (a, b) => { @@ -297,9 +338,9 @@ public void Initialize(IncrementalGeneratorInitializationContext context) { var (filePath, document) = pair; var hintName = GetIdentifierFromPath(filePath); - context.AddOutput(hintName + ".rsg.cs", document.GetCSharpDocument().GeneratedCode); - context.AddOutput(hintName + ".rsg.html", document.GetHtmlDocument().GeneratedCode); + context.AddOutput(hintName + ".rsg.cs", document.CodeDocument.GetCSharpDocument().GeneratedCode); + context.AddOutput(hintName + ".rsg.html", document.CodeDocument.GetHtmlDocument().GeneratedCode); }); } } -} +} \ No newline at end of file diff --git a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGeneratorEventSource.cs b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGeneratorEventSource.cs index 8ef7ac0f98a..a4c91059ce7 100644 --- a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGeneratorEventSource.cs +++ b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGeneratorEventSource.cs @@ -59,5 +59,29 @@ private RazorSourceGeneratorEventSource() { } private const int GenerateDeclarationSyntaxTreeStopId = 14; [Event(GenerateDeclarationSyntaxTreeStopId, Level = EventLevel.Informational)] public void GenerateDeclarationSyntaxTreeStop() => WriteEvent(GenerateDeclarationSyntaxTreeStopId); + + private const int ParseRazorDocumentStartId = 15; + [Event(ParseRazorDocumentStartId, Level = EventLevel.Informational)] + public void ParseRazorDocumentStart(string file) => WriteEvent(ParseRazorDocumentStartId, file); + + private const int ParseRazorDocumentStopId = 16; + [Event(ParseRazorDocumentStopId, Level = EventLevel.Informational)] + public void ParseRazorDocumentStop(string file) => WriteEvent(ParseRazorDocumentStopId, file); + + private const int RewriteTagHelpersStartId = 17; + [Event(RewriteTagHelpersStartId, Level = EventLevel.Informational)] + public void RewriteTagHelpersStart(string file) => WriteEvent(RewriteTagHelpersStartId, file); + + private const int RewriteTagHelpersStopId = 18; + [Event(RewriteTagHelpersStopId, Level = EventLevel.Informational)] + public void RewriteTagHelpersStop(string file) => WriteEvent(RewriteTagHelpersStopId, file); + + private const int CheckAndRewriteTagHelpersStartId = 19; + [Event(CheckAndRewriteTagHelpersStartId, Level = EventLevel.Informational)] + public void CheckAndRewriteTagHelpersStart(string file) => WriteEvent(CheckAndRewriteTagHelpersStartId, file); + + private const int CheckAndRewriteTagHelpersStopId = 20; + [Event(CheckAndRewriteTagHelpersStopId, Level = EventLevel.Informational)] + public void CheckAndRewriteTagHelpersStop(string file) => WriteEvent(CheckAndRewriteTagHelpersStopId, file); } } diff --git a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/SourceGeneratorProjectEngine.cs b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/SourceGeneratorProjectEngine.cs new file mode 100644 index 00000000000..c7d07afd3b4 --- /dev/null +++ b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/SourceGeneratorProjectEngine.cs @@ -0,0 +1,119 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#nullable enable + +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; +using Microsoft.AspNetCore.Razor.Language; + +namespace Microsoft.NET.Sdk.Razor.SourceGenerators; + +internal class SourceGeneratorProjectEngine : DefaultRazorProjectEngine +{ + private readonly int discoveryPhaseIndex = -1; + + private readonly int rewritePhaseIndex = -1; + + public SourceGeneratorProjectEngine(DefaultRazorProjectEngine projectEngine) + : base(projectEngine.Configuration, projectEngine.Engine, projectEngine.FileSystem, projectEngine.ProjectFeatures) + { + for (int i = 0; i < Engine.Phases.Count; i++) + { + if (Engine.Phases[i] is DefaultRazorTagHelperContextDiscoveryPhase) + { + discoveryPhaseIndex = i; + } + else if (Engine.Phases[i] is DefaultRazorTagHelperRewritePhase) + { + rewritePhaseIndex = i; + } + else if (discoveryPhaseIndex >= 0 && rewritePhaseIndex >= 0) + { + break; + } + } + Debug.Assert(discoveryPhaseIndex >= 0); + Debug.Assert(rewritePhaseIndex >= 0); + } + + public SourceGeneratorRazorCodeDocument ProcessInitialParse(RazorProjectItem projectItem, bool designTime) + { + var codeDocument = designTime + ? CreateCodeDocumentDesignTimeCore(projectItem) + : CreateCodeDocumentCore(projectItem); + + + ProcessPartial(codeDocument, 0, discoveryPhaseIndex); + + // record the syntax tree, before the tag helper re-writing occurs + codeDocument.SetPreTagHelperSyntaxTree(codeDocument.GetSyntaxTree()); + return new SourceGeneratorRazorCodeDocument(codeDocument); + } + + public SourceGeneratorRazorCodeDocument ProcessTagHelpers(SourceGeneratorRazorCodeDocument sgDocument, IReadOnlyList tagHelpers, bool checkForIdempotency) + { + Debug.Assert(sgDocument.CodeDocument.GetPreTagHelperSyntaxTree() is not null); + + int startIndex = discoveryPhaseIndex; + var codeDocument = sgDocument.CodeDocument; + var previousTagHelpers = codeDocument.GetTagHelpers(); + if (checkForIdempotency && previousTagHelpers is not null) + { + // compare the tag helpers with the ones the document last used + if (Enumerable.SequenceEqual(tagHelpers, previousTagHelpers)) + { + // tag helpers are the same, nothing to do! + return sgDocument; + } + else + { + // tag helpers have changed, figure out if we need to re-write + var oldContextHelpers = codeDocument.GetTagHelperContext().TagHelpers; + + // re-run the scope check to figure out which tag helpers this document can see + codeDocument.SetTagHelpers(tagHelpers); + Engine.Phases[discoveryPhaseIndex].Execute(codeDocument); + + // Check if any new tag helpers were added or ones we previously used were removed + var newContextHelpers = codeDocument.GetTagHelperContext().TagHelpers; + var added = newContextHelpers.Except(oldContextHelpers); //PROTOTYPE: can we decide if a tag helper is 'modified' rather than being added? Basically if it still has the same name, then surely we know it still isn't used? + var referencedByRemoved = codeDocument.GetReferencedTagHelpers().Except(newContextHelpers); + if (!added.Any() && !referencedByRemoved.Any()) + { + // Either nothing new, or any that got removed weren't used by this document anyway + return sgDocument; + } + + // We need to re-write the document, but can skip the scoping as we just performed it + startIndex = rewritePhaseIndex; + } + } + else + { + codeDocument.SetTagHelpers(tagHelpers); + } + + ProcessPartial(codeDocument, startIndex, rewritePhaseIndex + 1); + return new SourceGeneratorRazorCodeDocument(codeDocument); + } + + public SourceGeneratorRazorCodeDocument ProcessRemaining(SourceGeneratorRazorCodeDocument sgDocument) + { + var codeDocument = sgDocument.CodeDocument; + Debug.Assert(codeDocument.GetReferencedTagHelpers() is not null); + + ProcessPartial(sgDocument.CodeDocument, rewritePhaseIndex, Engine.Phases.Count); + return new SourceGeneratorRazorCodeDocument(codeDocument); + } + + private void ProcessPartial(RazorCodeDocument codeDocument, int startIndex, int endIndex) + { + Debug.Assert(startIndex >= 0 && startIndex <= endIndex && endIndex <= Engine.Phases.Count); + for (var i = startIndex; i < endIndex; i++) + { + Engine.Phases[i].Execute(codeDocument); + } + } +} \ No newline at end of file diff --git a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/SourceGeneratorRazorCodeDocument.cs b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/SourceGeneratorRazorCodeDocument.cs new file mode 100644 index 00000000000..707e4345e63 --- /dev/null +++ b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/SourceGeneratorRazorCodeDocument.cs @@ -0,0 +1,31 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#nullable enable + +using Microsoft.AspNetCore.Razor.Language; + +namespace Microsoft.NET.Sdk.Razor.SourceGenerators; + +/// +/// A wrapper for +/// +/// +/// The razor compiler modifies the in place during the various phases, +/// meaning object identity is maintained even when the contents have changed. +/// +/// We need to be able to identify from the source generator if a given code document was modified or +/// returned unchanged. Rather than implementing deep equality on the +/// which can get expensive when the is large, we instead use a wrapper class. +/// If the underlying document is unchanged we return the original wrapper class. If the underlying +/// document is changed, we return a new instance of the wrapper. +/// +internal class SourceGeneratorRazorCodeDocument +{ + public RazorCodeDocument CodeDocument { get; } + + public SourceGeneratorRazorCodeDocument(RazorCodeDocument razorCodeDocument) + { + this.CodeDocument = razorCodeDocument; + } +} diff --git a/src/Compiler/perf/Microsoft.AspNetCore.Razor.Microbenchmarks.Generator/AbstractBenchmark.cs b/src/Compiler/perf/Microsoft.AspNetCore.Razor.Microbenchmarks.Generator/AbstractBenchmark.cs index 87fb028e148..0a4f554e504 100644 --- a/src/Compiler/perf/Microsoft.AspNetCore.Razor.Microbenchmarks.Generator/AbstractBenchmark.cs +++ b/src/Compiler/perf/Microsoft.AspNetCore.Razor.Microbenchmarks.Generator/AbstractBenchmark.cs @@ -28,8 +28,7 @@ public void Setup() protected GeneratorDriver RunBenchmark(Func updateDriver) { var compilation = _project!.Compilation; - var driver = _project!.GeneratorDriver; - driver = updateDriver(_project!); + var driver = updateDriver(_project!); driver = driver.RunGenerators(compilation); return driver; diff --git a/src/Compiler/test/Microsoft.NET.Sdk.Razor.SourceGenerators.Tests/RazorSourceGeneratorTests.cs b/src/Compiler/test/Microsoft.NET.Sdk.Razor.SourceGenerators.Tests/RazorSourceGeneratorTests.cs index aa3fd6cfab7..f4958e37b82 100644 --- a/src/Compiler/test/Microsoft.NET.Sdk.Razor.SourceGenerators.Tests/RazorSourceGeneratorTests.cs +++ b/src/Compiler/test/Microsoft.NET.Sdk.Razor.SourceGenerators.Tests/RazorSourceGeneratorTests.cs @@ -60,6 +60,41 @@ protected override void BuildRenderTree(global::Microsoft.AspNetCore.Components. Assert.Single(result.GeneratedSources); } + [Fact, WorkItem("https://github.com/dotnet/razor/issues/8610")] + public async Task SourceGenerator_RazorFiles_Using_NestedClass() + { + // Arrange + var project = CreateTestProject(new() + { + ["Pages/Index.razor"] = """ + @code { + public class MyModel { } + } + """, + ["Shared/MyComponent.razor"] = """ + + + + @code { + [Parameter] + public Pages.Index.MyModel? Data { get; set; } + } + """, + }); + var compilation = await project.GetCompilationAsync(); + var driver = await GetDriverAsync(project, options => + { + options.TestGlobalOptions["build_property.RazorLangVersion"] = "7.0"; + }); + + // Act + var result = RunGenerator(compilation!, ref driver); + + // Assert + Assert.Empty(result.Diagnostics); + Assert.Equal(2, result.GeneratedSources.Length); + } + [Fact, WorkItem("https://github.com/dotnet/razor/issues/8610")] public async Task SourceGenerator_RazorFiles_UsingAlias_NestedClass() { @@ -256,6 +291,10 @@ protected override void BuildRenderTree(global::Microsoft.AspNetCore.Components. Assert.Collection(eventListener.Events, e => Assert.Equal("ComputeRazorSourceGeneratorOptions", e.EventName), + e => e.AssertSingleItem("ParseRazorDocumentStart", "Pages/Index.razor"), + e => e.AssertSingleItem("ParseRazorDocumentStop", "Pages/Index.razor"), + e => e.AssertSingleItem("ParseRazorDocumentStart", "Pages/Counter.razor"), + e => e.AssertSingleItem("ParseRazorDocumentStop", "Pages/Counter.razor"), e => e.AssertSingleItem("GenerateDeclarationCodeStart", "/Pages/Index.razor"), e => e.AssertSingleItem("GenerateDeclarationCodeStop", "/Pages/Index.razor"), e => e.AssertSingleItem("GenerateDeclarationCodeStart", "/Pages/Counter.razor"), @@ -264,10 +303,18 @@ protected override void BuildRenderTree(global::Microsoft.AspNetCore.Components. e => Assert.Equal("DiscoverTagHelpersFromCompilationStop", e.EventName), e => Assert.Equal("DiscoverTagHelpersFromReferencesStart", e.EventName), e => Assert.Equal("DiscoverTagHelpersFromReferencesStop", e.EventName), - e => e.AssertPair("RazorCodeGenerateStart", "/Pages/Index.razor", "Runtime"), - e => e.AssertPair("RazorCodeGenerateStop", "/Pages/Index.razor", "Runtime"), - e => e.AssertPair("RazorCodeGenerateStart", "/Pages/Counter.razor", "Runtime"), - e => e.AssertPair("RazorCodeGenerateStop", "/Pages/Counter.razor", "Runtime"), + e => e.AssertSingleItem("RewriteTagHelpersStart", "Pages/Index.razor"), + e => e.AssertSingleItem("RewriteTagHelpersStop", "Pages/Index.razor"), + e => e.AssertSingleItem("RewriteTagHelpersStart", "Pages/Counter.razor"), + e => e.AssertSingleItem("RewriteTagHelpersStop", "Pages/Counter.razor"), + e => e.AssertSingleItem("CheckAndRewriteTagHelpersStart", "Pages/Index.razor"), + e => e.AssertSingleItem("CheckAndRewriteTagHelpersStop", "Pages/Index.razor"), + e => e.AssertSingleItem("CheckAndRewriteTagHelpersStart", "Pages/Counter.razor"), + e => e.AssertSingleItem("CheckAndRewriteTagHelpersStop", "Pages/Counter.razor"), + e => e.AssertPair("RazorCodeGenerateStart", "Pages/Index.razor", "Runtime"), + e => e.AssertPair("RazorCodeGenerateStop", "Pages/Index.razor", "Runtime"), + e => e.AssertPair("RazorCodeGenerateStart", "Pages/Counter.razor", "Runtime"), + e => e.AssertPair("RazorCodeGenerateStop", "Pages/Counter.razor", "Runtime"), e => e.AssertSingleItem("AddSyntaxTrees", "Pages_Index_razor.g.cs"), e => e.AssertSingleItem("AddSyntaxTrees", "Pages_Counter_razor.g.cs") ); @@ -456,11 +503,17 @@ protected override void BuildRenderTree(global::Microsoft.AspNetCore.Components. Assert.Equal(2, result.GeneratedSources.Length); Assert.Collection(eventListener.Events, - e => e.AssertSingleItem("GenerateDeclarationCodeStart", "/Pages/Counter.razor"), - e => e.AssertSingleItem("GenerateDeclarationCodeStop", "/Pages/Counter.razor"), - e => e.AssertPair("RazorCodeGenerateStart", "/Pages/Counter.razor", "Runtime"), - e => e.AssertPair("RazorCodeGenerateStop", "/Pages/Counter.razor", "Runtime"), - e => e.AssertSingleItem("AddSyntaxTrees", "Pages_Counter_razor.g.cs") + e => e.AssertSingleItem("ParseRazorDocumentStart", "Pages/Counter.razor"), + e => e.AssertSingleItem("ParseRazorDocumentStop", "Pages/Counter.razor"), + e => e.AssertSingleItem("GenerateDeclarationCodeStart", "/Pages/Counter.razor"), + e => e.AssertSingleItem("GenerateDeclarationCodeStop", "/Pages/Counter.razor"), + e => e.AssertSingleItem("RewriteTagHelpersStart", "Pages/Counter.razor"), + e => e.AssertSingleItem("RewriteTagHelpersStop", "Pages/Counter.razor"), + e => e.AssertSingleItem("CheckAndRewriteTagHelpersStart", "Pages/Counter.razor"), + e => e.AssertSingleItem("CheckAndRewriteTagHelpersStop", "Pages/Counter.razor"), + e => e.AssertPair("RazorCodeGenerateStart", "Pages/Counter.razor", "Runtime"), + e => e.AssertPair("RazorCodeGenerateStop", "Pages/Counter.razor", "Runtime"), + e => e.AssertSingleItem("AddSyntaxTrees", "Pages_Counter_razor.g.cs") ); } @@ -810,13 +863,19 @@ protected override void BuildRenderTree(global::Microsoft.AspNetCore.Components. Assert.Equal(2, result.GeneratedSources.Length); Assert.Collection(eventListener.Events, - e => e.AssertSingleItem("GenerateDeclarationCodeStart", "/Pages/Counter.razor"), - e => e.AssertSingleItem("GenerateDeclarationCodeStop", "/Pages/Counter.razor"), - e => Assert.Equal("DiscoverTagHelpersFromCompilationStart", e.EventName), - e => Assert.Equal("DiscoverTagHelpersFromCompilationStop", e.EventName), - e => e.AssertPair("RazorCodeGenerateStart", "/Pages/Counter.razor", "Runtime"), - e => e.AssertPair("RazorCodeGenerateStop", "/Pages/Counter.razor", "Runtime"), - e => e.AssertSingleItem("AddSyntaxTrees", "Pages_Counter_razor.g.cs") + e => e.AssertSingleItem("ParseRazorDocumentStart", "Pages/Counter.razor"), + e => e.AssertSingleItem("ParseRazorDocumentStop", "Pages/Counter.razor"), + e => e.AssertSingleItem("GenerateDeclarationCodeStart", "/Pages/Counter.razor"), + e => e.AssertSingleItem("GenerateDeclarationCodeStop", "/Pages/Counter.razor"), + e => Assert.Equal("DiscoverTagHelpersFromCompilationStart", e.EventName), + e => Assert.Equal("DiscoverTagHelpersFromCompilationStop", e.EventName), + e => e.AssertSingleItem("RewriteTagHelpersStart", "Pages/Counter.razor"), + e => e.AssertSingleItem("RewriteTagHelpersStop", "Pages/Counter.razor"), + e => e.AssertSingleItem("CheckAndRewriteTagHelpersStart", "Pages/Counter.razor"), + e => e.AssertSingleItem("CheckAndRewriteTagHelpersStop", "Pages/Counter.razor"), + e => e.AssertPair("RazorCodeGenerateStart", "Pages/Counter.razor", "Runtime"), + e => e.AssertPair("RazorCodeGenerateStop", "Pages/Counter.razor", "Runtime"), + e => e.AssertSingleItem("AddSyntaxTrees", "Pages_Counter_razor.g.cs") ); } @@ -966,14 +1025,22 @@ protected override void BuildRenderTree(global::Microsoft.AspNetCore.Components. Assert.Equal(2, result.GeneratedSources.Length); Assert.Collection(eventListener.Events, + e => e.AssertSingleItem("ParseRazorDocumentStart", "Pages/Counter.razor"), + e => e.AssertSingleItem("ParseRazorDocumentStop", "Pages/Counter.razor"), e => e.AssertSingleItem("GenerateDeclarationCodeStart", "/Pages/Counter.razor"), e => e.AssertSingleItem("GenerateDeclarationCodeStop", "/Pages/Counter.razor"), e => Assert.Equal("DiscoverTagHelpersFromCompilationStart", e.EventName), e => Assert.Equal("DiscoverTagHelpersFromCompilationStop", e.EventName), - e => e.AssertPair("RazorCodeGenerateStart", "/Pages/Index.razor", "Runtime"), - e => e.AssertPair("RazorCodeGenerateStop", "/Pages/Index.razor", "Runtime"), - e => e.AssertPair("RazorCodeGenerateStart", "/Pages/Counter.razor", "Runtime"), - e => e.AssertPair("RazorCodeGenerateStop", "/Pages/Counter.razor", "Runtime"), + e => e.AssertSingleItem("RewriteTagHelpersStart", "Pages/Counter.razor"), + e => e.AssertSingleItem("RewriteTagHelpersStop", "Pages/Counter.razor"), + e => e.AssertSingleItem("CheckAndRewriteTagHelpersStart", "Pages/Index.razor"), + e => e.AssertSingleItem("CheckAndRewriteTagHelpersStop", "Pages/Index.razor"), + e => e.AssertSingleItem("CheckAndRewriteTagHelpersStart", "Pages/Counter.razor"), + e => e.AssertSingleItem("CheckAndRewriteTagHelpersStop", "Pages/Counter.razor"), + e => e.AssertPair("RazorCodeGenerateStart", "Pages/Index.razor", "Runtime"), + e => e.AssertPair("RazorCodeGenerateStop", "Pages/Index.razor", "Runtime"), + e => e.AssertPair("RazorCodeGenerateStart", "Pages/Counter.razor", "Runtime"), + e => e.AssertPair("RazorCodeGenerateStop", "Pages/Counter.razor", "Runtime"), e => e.AssertSingleItem("AddSyntaxTrees", "Pages_Counter_razor.g.cs") ); } @@ -1108,15 +1175,19 @@ protected override void BuildRenderTree(global::Microsoft.AspNetCore.Components. Assert.Equal(2, result.GeneratedSources.Length); Assert.Collection(eventListener.Events, - e => Assert.Equal("DiscoverTagHelpersFromCompilationStart", e.EventName), - e => Assert.Equal("DiscoverTagHelpersFromCompilationStop", e.EventName), - e => Assert.Equal("DiscoverTagHelpersFromReferencesStart", e.EventName), - e => Assert.Equal("DiscoverTagHelpersFromReferencesStop", e.EventName), - e => e.AssertPair("RazorCodeGenerateStart", "/Pages/Index.razor", "Runtime"), - e => e.AssertPair("RazorCodeGenerateStop", "/Pages/Index.razor", "Runtime"), - e => e.AssertPair("RazorCodeGenerateStart", "/Pages/Counter.razor", "Runtime"), - e => e.AssertPair("RazorCodeGenerateStop", "/Pages/Counter.razor", "Runtime"), - e => e.AssertSingleItem("AddSyntaxTrees", "Pages_Index_razor.g.cs") + e => Assert.Equal("DiscoverTagHelpersFromCompilationStart", e.EventName), + e => Assert.Equal("DiscoverTagHelpersFromCompilationStop", e.EventName), + e => Assert.Equal("DiscoverTagHelpersFromReferencesStart", e.EventName), + e => Assert.Equal("DiscoverTagHelpersFromReferencesStop", e.EventName), + e => e.AssertSingleItem("CheckAndRewriteTagHelpersStart", "Pages/Index.razor"), + e => e.AssertSingleItem("CheckAndRewriteTagHelpersStop", "Pages/Index.razor"), + e => e.AssertSingleItem("CheckAndRewriteTagHelpersStart", "Pages/Counter.razor"), + e => e.AssertSingleItem("CheckAndRewriteTagHelpersStop", "Pages/Counter.razor"), + e => e.AssertPair("RazorCodeGenerateStart", "Pages/Index.razor", "Runtime"), + e => e.AssertPair("RazorCodeGenerateStop", "Pages/Index.razor", "Runtime"), + e => e.AssertPair("RazorCodeGenerateStart", "Pages/Counter.razor", "Runtime"), + e => e.AssertPair("RazorCodeGenerateStop", "Pages/Counter.razor", "Runtime"), + e => e.AssertSingleItem("AddSyntaxTrees", "Pages_Index_razor.g.cs") ); // Verify caching @@ -1269,14 +1340,26 @@ internal sealed class Views_Shared__Layout : global::Microsoft.AspNetCore.Mvc.Ra Assert.Collection(eventListener.Events, e => Assert.Equal("ComputeRazorSourceGeneratorOptions", e.EventName), + e => e.AssertSingleItem("ParseRazorDocumentStart", "Pages/Index.cshtml"), + e => e.AssertSingleItem("ParseRazorDocumentStop", "Pages/Index.cshtml"), + e => e.AssertSingleItem("ParseRazorDocumentStart", "Views/Shared/_Layout.cshtml"), + e => e.AssertSingleItem("ParseRazorDocumentStop", "Views/Shared/_Layout.cshtml"), e => Assert.Equal("DiscoverTagHelpersFromCompilationStart", e.EventName), e => Assert.Equal("DiscoverTagHelpersFromCompilationStop", e.EventName), e => Assert.Equal("DiscoverTagHelpersFromReferencesStart", e.EventName), e => Assert.Equal("DiscoverTagHelpersFromReferencesStop", e.EventName), - e => e.AssertPair("RazorCodeGenerateStart", "/Pages/Index.cshtml", "Runtime"), - e => e.AssertPair("RazorCodeGenerateStop", "/Pages/Index.cshtml", "Runtime"), - e => e.AssertPair("RazorCodeGenerateStart", "/Views/Shared/_Layout.cshtml", "Runtime"), - e => e.AssertPair("RazorCodeGenerateStop", "/Views/Shared/_Layout.cshtml", "Runtime"), + e => e.AssertSingleItem("RewriteTagHelpersStart", "Pages/Index.cshtml"), + e => e.AssertSingleItem("RewriteTagHelpersStop", "Pages/Index.cshtml"), + e => e.AssertSingleItem("RewriteTagHelpersStart", "Views/Shared/_Layout.cshtml"), + e => e.AssertSingleItem("RewriteTagHelpersStop", "Views/Shared/_Layout.cshtml"), + e => e.AssertSingleItem("CheckAndRewriteTagHelpersStart", "Pages/Index.cshtml"), + e => e.AssertSingleItem("CheckAndRewriteTagHelpersStop", "Pages/Index.cshtml"), + e => e.AssertSingleItem("CheckAndRewriteTagHelpersStart", "Views/Shared/_Layout.cshtml"), + e => e.AssertSingleItem("CheckAndRewriteTagHelpersStop", "Views/Shared/_Layout.cshtml"), + e => e.AssertPair("RazorCodeGenerateStart", "Pages/Index.cshtml", "Runtime"), + e => e.AssertPair("RazorCodeGenerateStop", "Pages/Index.cshtml", "Runtime"), + e => e.AssertPair("RazorCodeGenerateStart", "Views/Shared/_Layout.cshtml", "Runtime"), + e => e.AssertPair("RazorCodeGenerateStop", "Views/Shared/_Layout.cshtml", "Runtime"), e => e.AssertSingleItem("AddSyntaxTrees", "Pages_Index_cshtml.g.cs"), e => e.AssertSingleItem("AddSyntaxTrees", "Views_Shared__Layout_cshtml.g.cs") ); @@ -1611,8 +1694,14 @@ internal sealed class Views_Shared__Layout : global::Microsoft.AspNetCore.Mvc.Ra Assert.Equal(2, result.GeneratedSources.Length); Assert.Collection(eventListener.Events, - e => e.AssertPair("RazorCodeGenerateStart", "/Views/Shared/_Layout.cshtml", "Runtime"), - e => e.AssertPair("RazorCodeGenerateStop", "/Views/Shared/_Layout.cshtml", "Runtime"), + e => e.AssertSingleItem("ParseRazorDocumentStart", "Views/Shared/_Layout.cshtml"), + e => e.AssertSingleItem("ParseRazorDocumentStop", "Views/Shared/_Layout.cshtml"), + e => e.AssertSingleItem("RewriteTagHelpersStart", "Views/Shared/_Layout.cshtml"), + e => e.AssertSingleItem("RewriteTagHelpersStop", "Views/Shared/_Layout.cshtml"), + e => e.AssertSingleItem("CheckAndRewriteTagHelpersStart", "Views/Shared/_Layout.cshtml"), + e => e.AssertSingleItem("CheckAndRewriteTagHelpersStop", "Views/Shared/_Layout.cshtml"), + e => e.AssertPair("RazorCodeGenerateStart", "Views/Shared/_Layout.cshtml", "Runtime"), + e => e.AssertPair("RazorCodeGenerateStop", "Views/Shared/_Layout.cshtml", "Runtime"), e => e.AssertSingleItem("AddSyntaxTrees", "Views_Shared__Layout_cshtml.g.cs") ); } @@ -1931,13 +2020,15 @@ public override void Process(TagHelperContext context, TagHelperOutput output) Assert.Equal(2, result.GeneratedSources.Length); Assert.Collection(eventListener.Events, - e => Assert.Equal("DiscoverTagHelpersFromCompilationStart", e.EventName), - e => Assert.Equal("DiscoverTagHelpersFromCompilationStop", e.EventName), - e => e.AssertPair("RazorCodeGenerateStart", "/Pages/Index.cshtml", "Runtime"), - e => e.AssertPair("RazorCodeGenerateStop", "/Pages/Index.cshtml", "Runtime"), - e => e.AssertPair("RazorCodeGenerateStart", "/Views/Shared/_Layout.cshtml", "Runtime"), - e => e.AssertPair("RazorCodeGenerateStop", "/Views/Shared/_Layout.cshtml", "Runtime"), - e => e.AssertSingleItem("AddSyntaxTrees", "Pages_Index_cshtml.g.cs") + e => Assert.Equal("DiscoverTagHelpersFromCompilationStart", e.EventName), + e => Assert.Equal("DiscoverTagHelpersFromCompilationStop", e.EventName), + e => e.AssertSingleItem("CheckAndRewriteTagHelpersStart", "Pages/Index.cshtml"), + e => e.AssertSingleItem("CheckAndRewriteTagHelpersStop", "Pages/Index.cshtml"), + e => e.AssertSingleItem("CheckAndRewriteTagHelpersStart", "Views/Shared/_Layout.cshtml"), + e => e.AssertSingleItem("CheckAndRewriteTagHelpersStop", "Views/Shared/_Layout.cshtml"), + e => e.AssertPair("RazorCodeGenerateStart", "Pages/Index.cshtml", "Runtime"), + e => e.AssertPair("RazorCodeGenerateStop", "Pages/Index.cshtml", "Runtime"), + e => e.AssertSingleItem("AddSyntaxTrees", "Pages_Index_cshtml.g.cs") ); } From 2504c2dfe96120f4e7809f9eaea462d1290e3ae9 Mon Sep 17 00:00:00 2001 From: Chris Sienkiewicz Date: Fri, 21 Jul 2023 11:35:12 -0700 Subject: [PATCH 2/4] PR Feedback. - Sealed, unreachable etc. - Refactor tag helper static feature aquisition to make it clearer - Remove unused tag helper methods --- .../RazorSourceGenerator.Helpers.cs | 12 ++--- .../RazorSourceGenerator.TagHelpers.cs | 37 ---------------- .../RazorSourceGenerator.cs | 26 +++++------ .../SourceGeneratorProjectEngine.cs | 44 ++++++++++--------- .../SourceGeneratorRazorCodeDocument.cs | 2 +- .../StaticCompilationTagHelperFeature.cs | 10 ++--- 6 files changed, 45 insertions(+), 86 deletions(-) delete mode 100644 src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGenerator.TagHelpers.cs diff --git a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGenerator.Helpers.cs b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGenerator.Helpers.cs index a63c56f7738..e02d9aaa020 100644 --- a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGenerator.Helpers.cs +++ b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGenerator.Helpers.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Collections.Generic; +using System.Collections.Immutable; using System.Text; using Microsoft.AspNetCore.Mvc.Razor.Extensions; using Microsoft.AspNetCore.Razor.Language; @@ -66,13 +67,14 @@ private static RazorProjectEngine GetDeclarationProjectEngine( return discoveryProjectEngine; } - private static RazorProjectEngine GetDiscoveryProjectEngine( - IReadOnlyList references, - StaticCompilationTagHelperFeature tagHelperFeature) + private static StaticCompilationTagHelperFeature GetStaticTagHelperFeature(Compilation compilation) { + var tagHelperFeature = new StaticCompilationTagHelperFeature() { Compilation = compilation }; + + // the tagHelperFeature will have its Engine property set as part of adding it to the engine, which is used later when doing the actual discovery var discoveryProjectEngine = RazorProjectEngine.Create(RazorConfiguration.Default, new VirtualRazorProjectFileSystem(), b => { - b.Features.Add(new DefaultMetadataReferenceFeature { References = references }); + b.Features.Add(new DefaultMetadataReferenceFeature { References = compilation.References.ToImmutableArray() }); b.Features.Add(tagHelperFeature); b.Features.Add(new DefaultTagHelperDescriptorProvider()); @@ -80,7 +82,7 @@ private static RazorProjectEngine GetDiscoveryProjectEngine( RazorExtensions.Register(b); }); - return discoveryProjectEngine; + return tagHelperFeature; } private static SourceGeneratorProjectEngine GetGenerationProjectEngine( diff --git a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGenerator.TagHelpers.cs b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGenerator.TagHelpers.cs deleted file mode 100644 index b0d30d393c9..00000000000 --- a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGenerator.TagHelpers.cs +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System.Collections.Generic; -using Microsoft.AspNetCore.Razor.Language; -using Microsoft.CodeAnalysis; - -namespace Microsoft.NET.Sdk.Razor.SourceGenerators -{ - public partial class RazorSourceGenerator - { - private IReadOnlyList GetTagHelpers(IEnumerable references, StaticCompilationTagHelperFeature tagHelperFeature, Compilation compilation) - { - List descriptors = new(); - tagHelperFeature.Compilation = compilation; - foreach (var reference in references) - { - if (compilation.GetAssemblyOrModuleSymbol(reference) is IAssemblySymbol assembly) - { - tagHelperFeature.TargetSymbol = assembly; - descriptors.AddRange(tagHelperFeature.GetDescriptors()); - } - } - return descriptors; - } - - private static IReadOnlyList GetTagHelpersFromCompilation(Compilation compilation, StaticCompilationTagHelperFeature tagHelperFeature, SyntaxTree syntaxTrees) - { - var compilationWithDeclarations = compilation.AddSyntaxTrees(syntaxTrees); - - tagHelperFeature.Compilation = compilationWithDeclarations; - tagHelperFeature.TargetSymbol = compilationWithDeclarations.Assembly; - - return tagHelperFeature.GetDescriptors(); - } - } -} diff --git a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGenerator.cs b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGenerator.cs index cb254920c3b..58ce10d02dc 100644 --- a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGenerator.cs +++ b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGenerator.cs @@ -6,6 +6,7 @@ using System.Collections.Immutable; using System.IO; using System.Linq; +using Microsoft.AspNetCore.Razor; using Microsoft.AspNetCore.Razor.Language; using Microsoft.AspNetCore.Razor.Language.Extensions; using Microsoft.CodeAnalysis; @@ -80,7 +81,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) var generatedDeclarationCode = componentFiles .Combine(importFiles.Collect()) .Combine(razorSourceGeneratorOptions) - .WithLambdaComparer((old, @new) => (old.Right.Equals(@new.Right) && old.Left.Left.Equals(@new.Left.Left) && old.Left.Right.SequenceEqual(@new.Left.Right)), (a) => a.GetHashCode()) + .WithLambdaComparer((old, @new) => (old.Right.Equals(@new.Right) && old.Left.Left.Equals(@new.Left.Left) && old.Left.Right.SequenceEqual(@new.Left.Right)), (a) => Assumed.Unreachable()) .Select(static (pair, _) => { var ((sourceItem, importFiles), razorSourceGeneratorOptions) = pair; @@ -108,7 +109,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) var declCompilation = generatedDeclarationSyntaxTrees .Collect() .Combine(compilation) - .Select((pair, _) => + .Select(static (pair, _) => { return pair.Right.AddSyntaxTrees(pair.Left); }); @@ -121,13 +122,9 @@ public void Initialize(IncrementalGeneratorInitializationContext context) var (compilation, razorSourceGeneratorOptions) = pair; - var tagHelperFeature = new StaticCompilationTagHelperFeature(); - var discoveryProjectEngine = GetDiscoveryProjectEngine(compilation.References.ToImmutableArray(), tagHelperFeature); - - tagHelperFeature.Compilation = compilation; - tagHelperFeature.TargetSymbol = compilation.Assembly; - - var result = tagHelperFeature.GetDescriptors(); + var tagHelperFeature = GetStaticTagHelperFeature(compilation); + var result = tagHelperFeature.GetDescriptors(compilation.Assembly); + RazorSourceGeneratorEventSource.Log.DiscoverTagHelpersFromCompilationStop(); return result; }) @@ -187,17 +184,14 @@ public void Initialize(IncrementalGeneratorInitializationContext context) return ImmutableArray.Empty; } - var tagHelperFeature = new StaticCompilationTagHelperFeature(); - var discoveryProjectEngine = GetDiscoveryProjectEngine(compilation.References.ToImmutableArray(), tagHelperFeature); + var tagHelperFeature = GetStaticTagHelperFeature(compilation); List descriptors = new(); - tagHelperFeature.Compilation = compilation; foreach (var reference in compilation.References) { if (compilation.GetAssemblyOrModuleSymbol(reference) is IAssemblySymbol assembly) { - tagHelperFeature.TargetSymbol = assembly; - descriptors.AddRange(tagHelperFeature.GetDescriptors()); + descriptors.AddRange(tagHelperFeature.GetDescriptors(assembly)); } } @@ -262,7 +256,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) // Add the tag helpers in, but ignore if they've changed or not, only reprocessing the actual document changed .Combine(allTagHelpers) - .WithLambdaComparer((old, @new) => old.Left.Equals(@new.Left), (item) => item.GetHashCode()) + .WithLambdaComparer((old, @new) => old.Left.Equals(@new.Left), (item) => Assumed.Unreachable()) .Select(static (pair, _) => { var ((projectEngine, filePath, codeDocument), allTagHelpers) = pair; @@ -274,7 +268,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) return (projectEngine, filePath, codeDocument); }) - // next we do a second parse, along with the helpers, but check for idempotency. If the tag helpers used on the previous parse match, the compiler can skip re-computing them + // next we do a second parse, along with the helpers, but check for idempotency. If the tag helpers used on the previous parse match, the compiler can skip re-writing them .Combine(allTagHelpers) .Select(static (pair, _) => { diff --git a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/SourceGeneratorProjectEngine.cs b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/SourceGeneratorProjectEngine.cs index c7d07afd3b4..9b7ca617f28 100644 --- a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/SourceGeneratorProjectEngine.cs +++ b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/SourceGeneratorProjectEngine.cs @@ -10,11 +10,11 @@ namespace Microsoft.NET.Sdk.Razor.SourceGenerators; -internal class SourceGeneratorProjectEngine : DefaultRazorProjectEngine +internal sealed class SourceGeneratorProjectEngine : DefaultRazorProjectEngine { - private readonly int discoveryPhaseIndex = -1; + private readonly int _discoveryPhaseIndex = -1; - private readonly int rewritePhaseIndex = -1; + private readonly int _rewritePhaseIndex = -1; public SourceGeneratorProjectEngine(DefaultRazorProjectEngine projectEngine) : base(projectEngine.Configuration, projectEngine.Engine, projectEngine.FileSystem, projectEngine.ProjectFeatures) @@ -23,19 +23,20 @@ public SourceGeneratorProjectEngine(DefaultRazorProjectEngine projectEngine) { if (Engine.Phases[i] is DefaultRazorTagHelperContextDiscoveryPhase) { - discoveryPhaseIndex = i; + _discoveryPhaseIndex = i; } else if (Engine.Phases[i] is DefaultRazorTagHelperRewritePhase) { - rewritePhaseIndex = i; + _rewritePhaseIndex = i; } - else if (discoveryPhaseIndex >= 0 && rewritePhaseIndex >= 0) + else if (_discoveryPhaseIndex >= 0 && _rewritePhaseIndex >= 0) { break; } } - Debug.Assert(discoveryPhaseIndex >= 0); - Debug.Assert(rewritePhaseIndex >= 0); + Debug.Assert(_discoveryPhaseIndex >= 0); + Debug.Assert(_rewritePhaseIndex >= 0); + Debug.Assert(_discoveryPhaseIndex < _rewritePhaseIndex); } public SourceGeneratorRazorCodeDocument ProcessInitialParse(RazorProjectItem projectItem, bool designTime) @@ -45,7 +46,7 @@ public SourceGeneratorRazorCodeDocument ProcessInitialParse(RazorProjectItem pro : CreateCodeDocumentCore(projectItem); - ProcessPartial(codeDocument, 0, discoveryPhaseIndex); + ProcessPartial(codeDocument, 0, _discoveryPhaseIndex); // record the syntax tree, before the tag helper re-writing occurs codeDocument.SetPreTagHelperSyntaxTree(codeDocument.GetSyntaxTree()); @@ -56,7 +57,7 @@ public SourceGeneratorRazorCodeDocument ProcessTagHelpers(SourceGeneratorRazorCo { Debug.Assert(sgDocument.CodeDocument.GetPreTagHelperSyntaxTree() is not null); - int startIndex = discoveryPhaseIndex; + int startIndex = _discoveryPhaseIndex; var codeDocument = sgDocument.CodeDocument; var previousTagHelpers = codeDocument.GetTagHelpers(); if (checkForIdempotency && previousTagHelpers is not null) @@ -70,24 +71,25 @@ public SourceGeneratorRazorCodeDocument ProcessTagHelpers(SourceGeneratorRazorCo else { // tag helpers have changed, figure out if we need to re-write - var oldContextHelpers = codeDocument.GetTagHelperContext().TagHelpers; + var previousTagHelpersInScope = codeDocument.GetTagHelperContext().TagHelpers; + var previousUsedTagHelpers = codeDocument.GetReferencedTagHelpers(); - // re-run the scope check to figure out which tag helpers this document can see + // re-run discovery to figure out which tag helpers are now in scope for this document codeDocument.SetTagHelpers(tagHelpers); - Engine.Phases[discoveryPhaseIndex].Execute(codeDocument); + Engine.Phases[_discoveryPhaseIndex].Execute(codeDocument); + var tagHelpersInScope = codeDocument.GetTagHelperContext().TagHelpers; // Check if any new tag helpers were added or ones we previously used were removed - var newContextHelpers = codeDocument.GetTagHelperContext().TagHelpers; - var added = newContextHelpers.Except(oldContextHelpers); //PROTOTYPE: can we decide if a tag helper is 'modified' rather than being added? Basically if it still has the same name, then surely we know it still isn't used? - var referencedByRemoved = codeDocument.GetReferencedTagHelpers().Except(newContextHelpers); - if (!added.Any() && !referencedByRemoved.Any()) + var newVisibleTagHelpers = tagHelpersInScope.Except(previousTagHelpersInScope); + var newUnusedTagHelpers = previousUsedTagHelpers.Except(tagHelpersInScope); + if (!newVisibleTagHelpers.Any() && !newUnusedTagHelpers.Any()) { - // Either nothing new, or any that got removed weren't used by this document anyway + // No newly visible tag helpers, and any that got removed weren't used by this document anyway return sgDocument; } // We need to re-write the document, but can skip the scoping as we just performed it - startIndex = rewritePhaseIndex; + startIndex = _rewritePhaseIndex; } } else @@ -95,7 +97,7 @@ public SourceGeneratorRazorCodeDocument ProcessTagHelpers(SourceGeneratorRazorCo codeDocument.SetTagHelpers(tagHelpers); } - ProcessPartial(codeDocument, startIndex, rewritePhaseIndex + 1); + ProcessPartial(codeDocument, startIndex, _rewritePhaseIndex + 1); return new SourceGeneratorRazorCodeDocument(codeDocument); } @@ -104,7 +106,7 @@ public SourceGeneratorRazorCodeDocument ProcessRemaining(SourceGeneratorRazorCod var codeDocument = sgDocument.CodeDocument; Debug.Assert(codeDocument.GetReferencedTagHelpers() is not null); - ProcessPartial(sgDocument.CodeDocument, rewritePhaseIndex, Engine.Phases.Count); + ProcessPartial(sgDocument.CodeDocument, _rewritePhaseIndex, Engine.Phases.Count); return new SourceGeneratorRazorCodeDocument(codeDocument); } diff --git a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/SourceGeneratorRazorCodeDocument.cs b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/SourceGeneratorRazorCodeDocument.cs index 707e4345e63..8fdf1bb0765 100644 --- a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/SourceGeneratorRazorCodeDocument.cs +++ b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/SourceGeneratorRazorCodeDocument.cs @@ -20,7 +20,7 @@ namespace Microsoft.NET.Sdk.Razor.SourceGenerators; /// If the underlying document is unchanged we return the original wrapper class. If the underlying /// document is changed, we return a new instance of the wrapper. /// -internal class SourceGeneratorRazorCodeDocument +internal sealed class SourceGeneratorRazorCodeDocument { public RazorCodeDocument CodeDocument { get; } diff --git a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/StaticCompilationTagHelperFeature.cs b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/StaticCompilationTagHelperFeature.cs index 15d54b36587..aab1e7d5517 100644 --- a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/StaticCompilationTagHelperFeature.cs +++ b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/StaticCompilationTagHelperFeature.cs @@ -15,7 +15,7 @@ internal sealed class StaticCompilationTagHelperFeature : RazorEngineFeatureBase private ITagHelperDescriptorProvider[]? _providers; - public List GetDescriptors() + public List GetDescriptors(ISymbol? targetSymbol) { if (Compilation is null) { @@ -25,9 +25,9 @@ public List GetDescriptors() var results = new List(); var context = TagHelperDescriptorProviderContext.Create(results); context.SetCompilation(Compilation); - if (TargetSymbol is not null) + if (targetSymbol is not null) { - context.Items.SetTargetSymbol(TargetSymbol); + context.Items.SetTargetSymbol(targetSymbol); } for (var i = 0; i < _providers?.Length; i++) @@ -38,12 +38,10 @@ public List GetDescriptors() return results; } - IReadOnlyList ITagHelperFeature.GetDescriptors() => GetDescriptors(); + IReadOnlyList ITagHelperFeature.GetDescriptors() => GetDescriptors(targetSymbol: null); public Compilation? Compilation { get; set; } - public ISymbol? TargetSymbol { get; set; } - protected override void OnInitialized() { _providers = Engine.Features.OfType().OrderBy(f => f.Order).ToArray(); From 54294c6260ecc8850deaecae91883e713004f3b3 Mon Sep 17 00:00:00 2001 From: Chris Sienkiewicz Date: Tue, 25 Jul 2023 10:13:52 -0700 Subject: [PATCH 3/4] Update src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/SourceGeneratorRazorCodeDocument.cs Co-authored-by: Fred Silberberg --- .../SourceGeneratorRazorCodeDocument.cs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/SourceGeneratorRazorCodeDocument.cs b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/SourceGeneratorRazorCodeDocument.cs index 8fdf1bb0765..c3baa7fc9e8 100644 --- a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/SourceGeneratorRazorCodeDocument.cs +++ b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/SourceGeneratorRazorCodeDocument.cs @@ -20,12 +20,7 @@ namespace Microsoft.NET.Sdk.Razor.SourceGenerators; /// If the underlying document is unchanged we return the original wrapper class. If the underlying /// document is changed, we return a new instance of the wrapper. /// -internal sealed class SourceGeneratorRazorCodeDocument +internal sealed class SourceGeneratorRazorCodeDocument(RazorCodeDocument razorCodeDocument) { - public RazorCodeDocument CodeDocument { get; } - - public SourceGeneratorRazorCodeDocument(RazorCodeDocument razorCodeDocument) - { - this.CodeDocument = razorCodeDocument; - } + public RazorCodeDocument CodeDocument { get; } = razorCodeDocument; } From 5886cba9ab512bac15ab638feb3b04e65f42e5ae Mon Sep 17 00:00:00 2001 From: Chris Sienkiewicz Date: Tue, 25 Jul 2023 10:29:56 -0700 Subject: [PATCH 4/4] PR Feedback --- .../RazorSourceGenerator.Helpers.cs | 2 +- .../RazorSourceGenerator.cs | 4 +-- .../StaticCompilationTagHelperFeature.cs | 14 +++-------- .../StaticTagHelperFeature.cs | 25 ------------------- 4 files changed, 6 insertions(+), 39 deletions(-) delete mode 100644 src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/StaticTagHelperFeature.cs diff --git a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGenerator.Helpers.cs b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGenerator.Helpers.cs index e02d9aaa020..c2b1de2fac4 100644 --- a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGenerator.Helpers.cs +++ b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGenerator.Helpers.cs @@ -69,7 +69,7 @@ private static RazorProjectEngine GetDeclarationProjectEngine( private static StaticCompilationTagHelperFeature GetStaticTagHelperFeature(Compilation compilation) { - var tagHelperFeature = new StaticCompilationTagHelperFeature() { Compilation = compilation }; + var tagHelperFeature = new StaticCompilationTagHelperFeature(compilation); // the tagHelperFeature will have its Engine property set as part of adding it to the engine, which is used later when doing the actual discovery var discoveryProjectEngine = RazorProjectEngine.Create(RazorConfiguration.Default, new VirtualRazorProjectFileSystem(), b => diff --git a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGenerator.cs b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGenerator.cs index 58ce10d02dc..15b8da41eae 100644 --- a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGenerator.cs +++ b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/RazorSourceGenerator.cs @@ -81,7 +81,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) var generatedDeclarationCode = componentFiles .Combine(importFiles.Collect()) .Combine(razorSourceGeneratorOptions) - .WithLambdaComparer((old, @new) => (old.Right.Equals(@new.Right) && old.Left.Left.Equals(@new.Left.Left) && old.Left.Right.SequenceEqual(@new.Left.Right)), (a) => Assumed.Unreachable()) + .WithLambdaComparer((old, @new) => (old.Right.Equals(@new.Right) && old.Left.Left.Equals(@new.Left.Left) && old.Left.Right.SequenceEqual(@new.Left.Right)), getHashCode: (a) => Assumed.Unreachable()) .Select(static (pair, _) => { var ((sourceItem, importFiles), razorSourceGeneratorOptions) = pair; @@ -256,7 +256,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) // Add the tag helpers in, but ignore if they've changed or not, only reprocessing the actual document changed .Combine(allTagHelpers) - .WithLambdaComparer((old, @new) => old.Left.Equals(@new.Left), (item) => Assumed.Unreachable()) + .WithLambdaComparer((old, @new) => old.Left.Equals(@new.Left), getHashCode: (item) => Assumed.Unreachable()) .Select(static (pair, _) => { var ((projectEngine, filePath, codeDocument), allTagHelpers) = pair; diff --git a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/StaticCompilationTagHelperFeature.cs b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/StaticCompilationTagHelperFeature.cs index aab1e7d5517..3f1f93fde21 100644 --- a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/StaticCompilationTagHelperFeature.cs +++ b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/StaticCompilationTagHelperFeature.cs @@ -9,22 +9,16 @@ namespace Microsoft.NET.Sdk.Razor.SourceGenerators { - internal sealed class StaticCompilationTagHelperFeature : RazorEngineFeatureBase, ITagHelperFeature + internal sealed class StaticCompilationTagHelperFeature(Compilation compilation) + : RazorEngineFeatureBase, ITagHelperFeature { - private static readonly List EmptyList = new(); - private ITagHelperDescriptorProvider[]? _providers; public List GetDescriptors(ISymbol? targetSymbol) { - if (Compilation is null) - { - return EmptyList; - } - var results = new List(); var context = TagHelperDescriptorProviderContext.Create(results); - context.SetCompilation(Compilation); + context.SetCompilation(compilation); if (targetSymbol is not null) { context.Items.SetTargetSymbol(targetSymbol); @@ -40,8 +34,6 @@ public List GetDescriptors(ISymbol? targetSymbol) IReadOnlyList ITagHelperFeature.GetDescriptors() => GetDescriptors(targetSymbol: null); - public Compilation? Compilation { get; set; } - protected override void OnInitialized() { _providers = Engine.Features.OfType().OrderBy(f => f.Order).ToArray(); diff --git a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/StaticTagHelperFeature.cs b/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/StaticTagHelperFeature.cs deleted file mode 100644 index 89207e40b8a..00000000000 --- a/src/Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators/StaticTagHelperFeature.cs +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System.Collections.Generic; -using Microsoft.AspNetCore.Razor.Language; - -namespace Microsoft.NET.Sdk.Razor.SourceGenerators -{ - internal sealed class StaticTagHelperFeature : RazorEngineFeatureBase, ITagHelperFeature - { - public IReadOnlyList TagHelpers { get; set; } - - public IReadOnlyList GetDescriptors() => TagHelpers; - - public StaticTagHelperFeature() - { - TagHelpers = new List(); - } - - public StaticTagHelperFeature(IEnumerable tagHelpers) - { - TagHelpers = new List(tagHelpers); - } - } -}