Better caching for RazorSourceGenerator when SuppressRazorSourceGener…

…ator changes (dotnet#23358)

In VisualStudio SuppressRazorSourceGenerator changes when it's being invoked by the EnC and tooling.
In our current implementation, while we no-op the operation, when SuppressRazorSourceGenerator=true,
the code also evicts the valid previously cached results when SuppressRazorSourceGenerator=false by returning null values, causing us do work the next time its false.

Contributes to dotnet/aspnetcore#32867

src/RazorSdk/SourceGenerators/IncrementalValueProviderExtensions.cs

@@ -1,4 +1,3 @@
-
 // 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.
 
@@ -10,6 +9,30 @@ namespace Microsoft.NET.Sdk.Razor.SourceGenerators
 {
     internal static class IncrementalValuesProviderExtensions
     {
+        /// <summary>
+        /// Adds a comparer used to determine if an incremental steps needs to be re-run accounting for <see cref="RazorSourceGenerationOptions.SuppressRazorSourceGenerator"/>.
+        /// <para>
+        /// In VS, design time builds are executed with <see cref="RazorSourceGenerationOptions.SuppressRazorSourceGenerator"/> set to <c>true</c>. In this case, RSG can safely
+        /// allow previously cached results to be used, while no-oping in the step that adds sources to the context. This allows source generator caches from being evicted
+        /// when the value of this property flip-flips during a hot-reload / EnC session.
+        /// </para>
+        /// </summary>
+        internal static IncrementalValueProvider<(T Left, RazorSourceGenerationOptions Right)> WithRazorSourceGeneratorComparer<T>(
+            this IncrementalValueProvider<(T Left, RazorSourceGenerationOptions Right)> source,
+            Func<T, T, bool>? equals = null)
+            where T : notnull
+        {
+            return source.WithComparer(new RazorSourceGeneratorComparer<T>(equals));
+        }
+
+        internal static IncrementalValuesProvider<(T Left, RazorSourceGenerationOptions Right)> WithRazorSourceGeneratorComparer<T>(
+            this IncrementalValuesProvider<(T Left, RazorSourceGenerationOptions Right)> source,
+            Func<T, T, bool>? equals = null)
+            where T : notnull
+        {
+            return source.WithComparer(new RazorSourceGeneratorComparer<T>(equals));
+        }
+
         internal static IncrementalValueProvider<T> WithLambdaComparer<T>(this IncrementalValueProvider<T> source, Func<T, T, bool> equal, Func<T, int> getHashCode)
         {
             var comparer = new LambdaComparer<T>(equal, getHashCode);

src/RazorSdk/SourceGenerators/RazorSourceGenerationOptions.cs

@@ -14,19 +14,10 @@ internal class RazorSourceGenerationOptions : IEquatable<RazorSourceGenerationOp
 
         public RazorConfiguration Configuration { get; set; } = RazorConfiguration.Default;
 
-        /// <summary>
-        /// Gets a flag that determines if the source generator waits for the debugger to attach.
-        /// <para>
-        /// To configure this using MSBuild, use the <c>_RazorSourceGeneratorDebug</c> property.
-        /// For instance <c>dotnet msbuild /p:_RazorSourceGeneratorDebug=true</c>
-        /// </para>
-        /// </summary>
-        public bool WaitForDebugger { get; set; } = false;
-
         /// <summary>
         /// Gets a flag that determines if generated Razor views and Pages includes the <c>RazorSourceChecksumAttribute</c>.
         /// </summary>
-        public bool GenerateMetadataSourceChecksumAttributes { get; set; } = false;
+        public bool GenerateMetadataSourceChecksumAttributes { get; set; }
 
         /// <summary>
         /// Gets a flag that determines if the source generator should no-op.
@@ -36,7 +27,7 @@ internal class RazorSourceGenerationOptions : IEquatable<RazorSourceGenerationOp
         /// The property is set by the SDK via an editor config.
         /// </para>
         /// </summary>
-        public bool SuppressRazorSourceGenerator { get; set; } = false;
+        public bool SuppressRazorSourceGenerator { get; set; }
 
         /// <summary>
         /// Gets the CSharp language version currently used by the compilation.
@@ -46,14 +37,17 @@ internal class RazorSourceGenerationOptions : IEquatable<RazorSourceGenerationOp
         /// <summary>
         /// Gets a flag that determines if localized component names should be supported.</c>.
         /// </summary>
-        public bool SupportLocalizedComponentNames { get; set; } = false;
+        public bool SupportLocalizedComponentNames { get; set; }
 
         public bool Equals(RazorSourceGenerationOptions other)
+            => SuppressRazorSourceGenerator == other.SuppressRazorSourceGenerator && EqualsIgnoringSupression(other);
+
+        public bool EqualsIgnoringSupression(RazorSourceGenerationOptions other)
         {
-            return RootNamespace == other.RootNamespace &&
-                Configuration == other.Configuration &&
+            return
+                RootNamespace == other.RootNamespace &&
+                Configuration.Equals(other.Configuration) &&
                 GenerateMetadataSourceChecksumAttributes == other.GenerateMetadataSourceChecksumAttributes &&
-                SuppressRazorSourceGenerator == other.SuppressRazorSourceGenerator &&
                 CSharpLanguageVersion == other.CSharpLanguageVersion &&
                 SupportLocalizedComponentNames == other.SupportLocalizedComponentNames;
         }

src/RazorSdk/SourceGenerators/RazorSourceGenerator.RazorProviders.cs

@@ -24,7 +24,6 @@ private static (RazorSourceGenerationOptions?, Diagnostic?) ComputeRazorSourceGe
             globalOptions.TryGetValue("build_property.RazorConfiguration", out var configurationName);
             globalOptions.TryGetValue("build_property.RootNamespace", out var rootNamespace);
             globalOptions.TryGetValue("build_property.SupportLocalizedComponentNames", out var supportLocalizedComponentNames);
-            globalOptions.TryGetValue("build_property._RazorSourceGeneratorDebug", out var waitForDebugger);
             globalOptions.TryGetValue("build_property.SuppressRazorSourceGenerator", out var suppressRazorSourceGenerator);
             globalOptions.TryGetValue("build_property.GenerateRazorMetadataSourceChecksumAttributes", out var generateMetadataSourceChecksumAttributes);
 
@@ -41,10 +40,9 @@ private static (RazorSourceGenerationOptions?, Diagnostic?) ComputeRazorSourceGe
 
             var razorConfiguration = RazorConfiguration.Create(razorLanguageVersion, configurationName ?? "default", System.Linq.Enumerable.Empty<RazorExtension>(), true);
 
-            var razorSourceGenerationOptions = new RazorSourceGenerationOptions()
+            var razorSourceGenerationOptions = new RazorSourceGenerationOptions
             {
                 Configuration = razorConfiguration,
-                WaitForDebugger = waitForDebugger == "true",
                 SuppressRazorSourceGenerator = suppressRazorSourceGenerator == "true",
                 GenerateMetadataSourceChecksumAttributes = generateMetadataSourceChecksumAttributes == "true",
                 RootNamespace = rootNamespace ?? "ASP",

src/RazorSdk/SourceGenerators/RazorSourceGenerator.cs

@@ -22,7 +22,9 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
             var razorSourceGeneratorOptionsWithDiagnostics = context.AnalyzerConfigOptionsProvider
                 .Combine(context.ParseOptionsProvider)
                 .Select(ComputeRazorSourceGeneratorOptions);
-            var razorSourceGeneratorOptions = razorSourceGeneratorOptionsWithDiagnostics.ReportDiagnostics(context);
+
+            var razorSourceGeneratorOptions = razorSourceGeneratorOptionsWithDiagnostics
+                .ReportDiagnostics(context);
 
             var sourceItemsWithDiagnostics = context.AdditionalTextsProvider
                 .Where(static (file) => file.Path.EndsWith(".razor", StringComparison.OrdinalIgnoreCase) || file.Path.EndsWith(".cshtml", StringComparison.OrdinalIgnoreCase))
@@ -52,23 +54,18 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
                 return false;
             });
 
-            var componentFiles = sourceItems.Where(static file => file.FilePath.EndsWith(".razor", StringComparison.OrdinalIgnoreCase));
+            var componentFiles = sourceItems.Where(
+                static file => file.FilePath.EndsWith(".razor", StringComparison.OrdinalIgnoreCase) && !file.FilePath.EndsWith("_Imports.razor", StringComparison.OrdinalIgnoreCase));
 
             var generatedDeclarationCode = componentFiles
                 .Combine(importFiles.Collect())
                 .Combine(razorSourceGeneratorOptions)
+                .WithRazorSourceGeneratorComparer()
                 .Select(static (pair, _) =>
                 {
-
                     var ((sourceItem, importFiles), razorSourceGeneratorOptions) = pair;
                     RazorSourceGeneratorEventSource.Log.GenerateDeclarationCodeStart(sourceItem.FilePath);
 
-                    if (razorSourceGeneratorOptions.SuppressRazorSourceGenerator)
-                    {
-                        RazorSourceGeneratorEventSource.Log.GenerateDeclarationCodeStop(sourceItem.FilePath);
-                        return null;
-                    }
-
                     var projectEngine = GetDeclarationProjectEngine(sourceItem, importFiles, razorSourceGeneratorOptions);
 
                     var codeGen = projectEngine.Process(sourceItem);
@@ -96,17 +93,12 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
             var tagHelpersFromCompilation = context.CompilationProvider
                 .Combine(generatedDeclarationSyntaxTrees.Collect())
                 .Combine(razorSourceGeneratorOptions)
+                .WithRazorSourceGeneratorComparer()
                 .Select(static (pair, _) =>
                 {
                     RazorSourceGeneratorEventSource.Log.DiscoverTagHelpersFromCompilationStart();
 
-                    var ((compilation, generatedDeclarationSyntaxTrees), razorSourceGeneratorOptions) = pair;
-
-                    if (razorSourceGeneratorOptions.SuppressRazorSourceGenerator)
-                    {
-                        RazorSourceGeneratorEventSource.Log.DiscoverTagHelpersFromCompilationStop();
-                        return ImmutableArray<TagHelperDescriptor>.Empty;
-                    }
+                    var ((compilation, generatedDeclarationSyntaxTrees), _) = pair;
 
                     var tagHelperFeature = new StaticCompilationTagHelperFeature();
                     var discoveryProjectEngine = GetDiscoveryProjectEngine(compilation.References.ToImmutableArray(), tagHelperFeature);
@@ -116,65 +108,33 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
                     tagHelperFeature.Compilation = compilationWithDeclarations;
                     tagHelperFeature.TargetAssembly = compilationWithDeclarations.Assembly;
 
-                    var result = (IList<TagHelperDescriptor>)tagHelperFeature.GetDescriptors();
+                    var result = tagHelperFeature.GetDescriptors().ToImmutableHashSet();
                     RazorSourceGeneratorEventSource.Log.DiscoverTagHelpersFromCompilationStop();
                     return result;
                 })
-                .WithLambdaComparer(static (a, b) =>
-                {
-                    if (a.Count != b.Count)
-                    {
-                        return false;
-                    }
-
-                    for (var i = 0; i < a.Count; i++)
-                    {
-                        if (!a[i].Equals(b[i]))
-                        {
-                            return false;
-                        }
-                    }
-
-                    return true;
-                }, getHashCode: static a => a.Count);
+                .WithLambdaComparer(static (a, b) => a.SetEquals(b), static item => item.Count.GetHashCode());
 
             var tagHelpersFromReferences = context.CompilationProvider
-                .Combine(razorSourceGeneratorOptions)
                 .Combine(hasRazorFiles)
-                .WithLambdaComparer(static (a, b) =>
+                .Combine(razorSourceGeneratorOptions)
+                .WithRazorSourceGeneratorComparer(static (a, b) =>
                 {
-                    var ((compilationA, razorSourceGeneratorOptionsA), hasRazorFilesA) = a;
-                    var ((compilationB, razorSourceGeneratorOptionsB), hasRazorFilesB) = b;
+                    var (compilationA, _) = a;
+                    var (compilationB, hasRazorFilesB) = b;
 
-                    if (!compilationA.References.SequenceEqual(compilationB.References))
+                    if (!hasRazorFilesB)
                     {
-                        return false;
+                        // If there aren't any razor files, we can use previously cached results
+                        return true;
                     }
 
-                    if (razorSourceGeneratorOptionsA != razorSourceGeneratorOptionsB)
-                    {
-                        return false;
-                    }
-
-                    return hasRazorFilesA == hasRazorFilesB;
-                },
-                static item =>
-                {
-                    // we'll use the number of references as a hashcode.
-                    var ((compilationA, razorSourceGeneratorOptionsA), hasRazorFilesA) = item;
-                    return compilationA.References.GetHashCode();
+                    return compilationA.References.SequenceEqual(compilationB.References);
                 })
                 .Select(static (pair, _) =>
                 {
                     RazorSourceGeneratorEventSource.Log.DiscoverTagHelpersFromReferencesStart();
 
-                    var ((compilation, razorSourceGeneratorOptions), hasRazorFiles) = pair;
-
-                    if (razorSourceGeneratorOptions.SuppressRazorSourceGenerator)
-                    {
-                        RazorSourceGeneratorEventSource.Log.DiscoverTagHelpersFromReferencesStop();
-                        return ImmutableArray<TagHelperDescriptor>.Empty;
-                    }
+                    var ((compilation, hasRazorFiles), _) = pair;
 
                     if (!hasRazorFiles)
                     {
@@ -213,7 +173,11 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
                     }
 
                     var allTagHelpers = new TagHelperDescriptor[count];
-                    tagHelpersFromCompilation.CopyTo(allTagHelpers, 0);
+                    var i = 0;
+                    foreach (var item in tagHelpersFromCompilation)
+                    {
+                        allTagHelpers[i++] = item;
+                    }
                     tagHelpersFromReferences.CopyTo(allTagHelpers, tagHelpersFromCompilation.Count);
 
                     return allTagHelpers;
@@ -222,20 +186,15 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
             var generatedOutput = sourceItems
                 .Combine(importFiles.Collect())
                 .Combine(allTagHelpers)
-                .Combine(razorSourceGeneratorOptions)
                 .Combine(context.ParseOptionsProvider)
+                .Combine(razorSourceGeneratorOptions)
+                .WithRazorSourceGeneratorComparer()
                 .Select(static (pair, _) =>
                 {
-                    var ((((sourceItem, imports), allTagHelpers), razorSourceGeneratorOptions), parserOptions) = pair;
+                    var ((((sourceItem, imports), allTagHelpers), parserOptions), razorSourceGeneratorOptions) = pair;
 
                     RazorSourceGeneratorEventSource.Log.RazorCodeGenerateStart(sourceItem.FilePath);
 
-                    if (razorSourceGeneratorOptions.SuppressRazorSourceGenerator)
-                    {
-                        RazorSourceGeneratorEventSource.Log.RazorCodeGenerateStop(sourceItem.FilePath);
-                        return default;
-                    }
-
                     // Add a generated suffix so tools, such as coverlet, consider the file to be generated
                     var hintName = GetIdentifierFromPath(sourceItem.RelativePhysicalPath) + ".g.cs";
 
@@ -249,12 +208,6 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
                 })
                 .WithLambdaComparer(static (a, b) =>
                 {
-                    if (a.hintName is null)
-                    {
-                        // Source generator is suppressed.
-                        return false;
-                    }
-
                     if (a.csharpDocument.Diagnostics.Count > 0 || b.csharpDocument.Diagnostics.Count > 0)
                     {
                         // if there are any diagnostics, treat the documents as unequal and force RegisterSourceOutput to be called uncached.
@@ -264,16 +217,17 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
                     return string.Equals(a.csharpDocument.GeneratedCode, b.csharpDocument.GeneratedCode, StringComparison.Ordinal);
                 }, static a => StringComparer.Ordinal.GetHashCode(a.csharpDocument));
 
-            context.RegisterSourceOutput(generatedOutput, static (context, pair) =>
+            context.RegisterSourceOutput(generatedOutput.Combine(razorSourceGeneratorOptions), static (context, pair) =>
             {
-                var (hintName, csharpDocument) = pair;
-                RazorSourceGeneratorEventSource.Log.AddSyntaxTrees(hintName);
-                if (hintName is null)
+                var ((hintName, csharpDocument), razorSourceGeneratorOptions) = pair;
+
+                if (razorSourceGeneratorOptions.SuppressRazorSourceGenerator)
                 {
                     // Source generator is suppressed.
                     return;
                 }
 
+                RazorSourceGeneratorEventSource.Log.AddSyntaxTrees(hintName);
                 for (var i = 0; i < csharpDocument.Diagnostics.Count; i++)
                 {
                     var razorDiagnostic = csharpDocument.Diagnostics[i];

src/RazorSdk/SourceGenerators/RazorSourceGeneratorComparer.cs

@@ -0,0 +1,38 @@
+// 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;
+using System.Collections.Generic;
+
+namespace Microsoft.NET.Sdk.Razor.SourceGenerators
+{
+    /// <summary>
+    /// A comparer used to determine if an incremental steps needs to be re-run accounting for <see cref="RazorSourceGenerationOptions.SuppressRazorSourceGenerator"/>.
+    /// <para>
+    /// In VS, design time builds are executed with <see cref="RazorSourceGenerationOptions.SuppressRazorSourceGenerator"/> set to <c>true</c>. In this case, RSG can safely
+    /// allow previously cached results to be used, while no-oping in the step that adds sources to the context. This allows source generator caches from being evicted
+    /// when the value of this property flip-flips during a hot-reload / EnC session.
+    /// </para>
+    /// </summary>
+    internal sealed class RazorSourceGeneratorComparer<T> : IEqualityComparer<(T Left, RazorSourceGenerationOptions Right)> where T : notnull
+    {
+        private readonly Func<T, T, bool> _equals;
+        public RazorSourceGeneratorComparer(Func<T, T, bool>? equals = null)
+        {
+            _equals = equals ?? EqualityComparer<T>.Default.Equals;
+        }
+
+        public bool Equals((T Left, RazorSourceGenerationOptions Right) x, (T Left, RazorSourceGenerationOptions Right) y)
+        {
+            if (y.Right.SuppressRazorSourceGenerator)
+            {
+                // If source generation is suppressed, we can always use previously cached results.
+                return true;
+            }
+
+            return _equals(x.Left, y.Left) && x.Right.EqualsIgnoringSupression(y.Right);
+        }
+
+        public int GetHashCode((T Left, RazorSourceGenerationOptions Right) obj) => obj.Left.GetHashCode();
+    }
+}