Merge pull request #73 from EFNext/fix/polyfill-generator-incremental-cache

Fixed incremental cache busting issue

Author: koenbeuk

src/ExpressiveSharp.Generator/PolyfillInterceptorGenerator.cs

@@ -1,5 +1,6 @@
 using System.Collections.Immutable;
 using System.Text;
+using System.Threading;
 using ExpressiveSharp.Generator.Comparers;
 using ExpressiveSharp.Generator.Emitter;
 using ExpressiveSharp.Generator.Infrastructure;
@@ -24,6 +25,9 @@ public class PolyfillInterceptorGenerator : IIncrementalGenerator
 
     private const string ExpressivePropertyAttributeName = "ExpressiveSharp.Mapping.ExpressivePropertyAttribute";
 
+    /// <summary>Tracking name for the value-equatable interceptor-source node (used by incremental-cache tests).</summary>
+    public const string InterceptorSourcesTrackingName = "InterceptorSources";
+
     private const string ClosureHelperSource = """
 
                 file static class __ClosureHelper
@@ -112,12 +116,13 @@ inv.Expression is MemberAccessExpressionSyntax &&
             .Where(static x => x is not null)
             .Select(static (x, _) => x!);
 
-        // Reference equality on the CompilationUnitSyntax root: Roslyn keeps unchanged files'
-        // syntax tree roots as the same object across incremental runs, so editing a noise
-        // file leaves all other (file, compilation) pairs equal and skips re-emission.
+        // A file's interceptors are produced by binding its call sites, which depend on the whole
+        // compilation (the lambdas reference types/members defined in other files). So we recompute
+        // when the compilation changes and gate the output on the value-equatable generated source
+        // below — a comparer keyed on the file's own syntax would serve stale interceptors after a
+        // cross-file edit.
         var filesWithCompilation = candidateFiles
-            .Combine(context.CompilationProvider)
-            .WithComparer(CompilationUnitAndCompilationComparer.Instance);
+            .Combine(context.CompilationProvider);
 
         // Source generators don't see each other's AddSource output, so SemanticModel can't bind
         // references to ExpressiveGenerator's synthesized [ExpressiveProperty] partials. Mirror
@@ -152,20 +157,46 @@ inv.Expression is MemberAccessExpressionSyntax &&
             .Collect()
             .WithComparer(SynthesizedSourceArrayComparer.Instance);
 
-        var filesWithCompilationAndSynth = filesWithCompilation
+        var fileResults = filesWithCompilation
             .Combine(synthesizedSources)
-            .WithComparer(FileAndSynthesizedSourcesComparer.Instance);
+            .Select(static (pair, ct) =>
+                ComputeFileInterceptors(pair.Left.Left, pair.Left.Right, pair.Right, ct));
+
+        // Generated interceptor source is value-data: re-emitted only when a file's interceptors
+        // actually change. Implementation output — interceptors are only needed for the real build
+        // (the user's code type-checks against the stub methods), so this stays off the live path.
+        var interceptorSources = fileResults
+            .Select(static (r, _) => r.Sources)
+            .WithTrackingName(InterceptorSourcesTrackingName);
+        context.RegisterImplementationSourceOutput(interceptorSources,
+            static (spc, sources) =>
+            {
+                foreach (var source in sources.AsImmutableArray)
+                    spc.AddSource(source.HintName, SourceText.From(source.Text, Encoding.UTF8));
+            });
 
-        context.RegisterSourceOutput(filesWithCompilationAndSynth,
-            static (spc, pair) =>
+        // Diagnostics flow live (real syntax-tree locations); recomputed each run, never stale.
+        var interceptorDiagnostics = fileResults.Select(static (r, _) => r.Diagnostics);
+        context.RegisterImplementationSourceOutput(interceptorDiagnostics,
+            static (spc, diagnostics) =>
             {
-                // Keeps its existing caching; collector is just the shared emit path, flushed inline.
-                var output = new GeneratorOutputContext(spc.CancellationToken);
-                ProcessFileAndEmit(pair.Left.Left, pair.Left.Right, pair.Right, output);
-                output.FlushTo(spc);
+                foreach (var diagnostic in diagnostics)
+                    spc.ReportDiagnostic(diagnostic);
             });
     }
 
+    private static (EquatableArray<GeneratedSource> Sources, ImmutableArray<Diagnostic> Diagnostics)
+        ComputeFileInterceptors(
+            CompilationUnitSyntax unit,
+            Compilation compilation,
+            ImmutableArray<(string HintName, string Source)> synthesizedSources,
+            CancellationToken cancellationToken)
+    {
+        var output = new GeneratorOutputContext(cancellationToken);
+        ProcessFileAndEmit(unit, compilation, synthesizedSources, output);
+        return (output.Sources, output.Diagnostics);
+    }
+
     private static void ProcessFileAndEmit(
         CompilationUnitSyntax unit,
         Compilation compilation,
@@ -858,58 +889,4 @@ private static string ResolveTypeFqn(ITypeSymbol type, Dictionary<ITypeSymbol, s
 
         return type.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat);
     }
-
-    /// <summary>
-    /// Reference equality on the CompilationUnitSyntax root, ignoring Compilation.
-    /// Roslyn keeps unchanged files' syntax tree roots as the same object across incremental
-    /// runs, so noise-file edits leave untouched (file, compilation) pairs equal and skip
-    /// re-emission — O(1) incremental cost.
-    /// </summary>
-    private sealed class CompilationUnitAndCompilationComparer
-        : IEqualityComparer<(CompilationUnitSyntax Left, Compilation Right)>
-    {
-        public readonly static CompilationUnitAndCompilationComparer Instance
-            = new CompilationUnitAndCompilationComparer();
-
-        private CompilationUnitAndCompilationComparer() { }
-
-        public bool Equals(
-            (CompilationUnitSyntax Left, Compilation Right) x,
-            (CompilationUnitSyntax Left, Compilation Right) y)
-            => ReferenceEquals(x.Left, y.Left);
-
-        public int GetHashCode((CompilationUnitSyntax Left, Compilation Right) obj)
-            => System.Runtime.CompilerServices.RuntimeHelpers.GetHashCode(obj.Left);
-    }
-
-    // ── FileAndSynthesizedSourcesComparer ──────────────────────────────────────
-
-    /// <summary>
-    /// ANDs <see cref="CompilationUnitAndCompilationComparer"/> with sequence equality on the
-    /// synthesized array — editing an [ExpressiveProperty] attribute correctly re-emits all
-    /// files since synthesized binding can affect any file's lambdas.
-    /// </summary>
-    private sealed class FileAndSynthesizedSourcesComparer
-        : IEqualityComparer<((CompilationUnitSyntax Left, Compilation Right) Left, ImmutableArray<(string HintName, string Source)> Right)>
-    {
-        public readonly static FileAndSynthesizedSourcesComparer Instance = new();
-
-        private FileAndSynthesizedSourcesComparer() { }
-
-        public bool Equals(
-            ((CompilationUnitSyntax Left, Compilation Right) Left, ImmutableArray<(string HintName, string Source)> Right) x,
-            ((CompilationUnitSyntax Left, Compilation Right) Left, ImmutableArray<(string HintName, string Source)> Right) y)
-            => CompilationUnitAndCompilationComparer.Instance.Equals(x.Left, y.Left)
-                && SynthesizedSourceArrayComparer.Instance.Equals(x.Right, y.Right);
-
-        public int GetHashCode(
-            ((CompilationUnitSyntax Left, Compilation Right) Left, ImmutableArray<(string HintName, string Source)> Right) obj)
-        {
-            unchecked
-            {
-                return CompilationUnitAndCompilationComparer.Instance.GetHashCode(obj.Left) * 31
-                    + SynthesizedSourceArrayComparer.Instance.GetHashCode(obj.Right);
-            }
-        }
-    }
 }

tests/ExpressiveSharp.Generator.Tests/PolyfillInterceptorGenerator/IncrementalCachingTests.cs

@@ -0,0 +1,99 @@
+using System.Linq;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using ExpressiveSharp.Generator.Tests.Infrastructure;
+
+namespace ExpressiveSharp.Generator.Tests.PolyfillInterceptorGenerator;
+
+// Guards that a cross-file edit doesn't serve stale interceptors (and that an unrelated edit
+// doesn't churn them). An interceptor is built by binding a file's call sites, which depend on
+// the whole compilation — not just that file's syntax.
+[TestClass]
+public class IncrementalCachingTests : GeneratorTestBase
+{
+    private const string QuerySource = """
+        using System;
+        using System.Linq.Expressions;
+        using ExpressiveSharp;
+        namespace TestNs
+        {
+            class Q
+            {
+                public void Run()
+                {
+                    Expression<Func<Order, long>> e = ExpressionPolyfill.Create<Func<Order, long>>(o => o.Value);
+                }
+            }
+        }
+        """;
+
+    private const string ModelsInt = "namespace TestNs { class Order { public int Value { get; set; } } }";
+    private const string ModelsLong = "namespace TestNs { class Order { public long Value { get; set; } } }";
+    private const string ModelsIntWithUnrelated =
+        "namespace TestNs { class Order { public int Value { get; set; } public int Other { get; set; } } }";
+
+    private static GeneratorDriver CreateDriver(CSharpParseOptions parseOptions) => CSharpGeneratorDriver
+        .Create(
+            new[] { new global::ExpressiveSharp.Generator.PolyfillInterceptorGenerator().AsSourceGenerator() },
+            driverOptions: new GeneratorDriverOptions(default, trackIncrementalGeneratorSteps: true))
+        .WithUpdatedParseOptions(parseOptions);
+
+    private static string InterceptorText(GeneratorDriverRunResult run) =>
+        string.Join("\n", run.GeneratedTrees.Select(t => t.GetText().ToString()));
+
+    [TestMethod]
+    public void CrossFileTypeChange_IncrementalOutputMatchesFreshRun()
+    {
+        var c1 = CreateCompilation(new[] { QuerySource, ModelsInt });
+        var parseOptions = (CSharpParseOptions)c1.SyntaxTrees.First().Options;
+        var modelsV1 = c1.SyntaxTrees.First(t => t.ToString().Contains("class Order"));
+        var modelsV2 = CSharpSyntaxTree.ParseText(ModelsLong, parseOptions, modelsV1.FilePath);
+        var c2 = c1.ReplaceSyntaxTree(modelsV1, modelsV2);
+
+        var driver = CreateDriver(parseOptions).RunGenerators(c1).RunGenerators(c2);
+        var incremental = InterceptorText(driver.GetRunResult());
+
+        var fresh = InterceptorText(CreateDriver(parseOptions).RunGenerators(c2).GetRunResult());
+
+        Assert.AreEqual(fresh, incremental,
+            "After a cross-file type change, interceptor output must match a from-scratch run.");
+        Assert.IsFalse(incremental.Contains("Convert"),
+            "With Value typed as long, no widening Convert should remain in the interceptor.");
+    }
+
+    [TestMethod]
+    public void UnrelatedCrossFileEdit_DoesNotInvalidateInterceptor()
+    {
+        var c1 = CreateCompilation(new[] { QuerySource, ModelsInt });
+        var parseOptions = (CSharpParseOptions)c1.SyntaxTrees.First().Options;
+        var modelsV1 = c1.SyntaxTrees.First(t => t.ToString().Contains("class Order"));
+        var modelsV1b = CSharpSyntaxTree.ParseText(ModelsIntWithUnrelated, parseOptions, modelsV1.FilePath);
+        var c2 = c1.ReplaceSyntaxTree(modelsV1, modelsV1b);
+
+        var driver = CreateDriver(parseOptions).RunGenerators(c1);
+        var text1 = InterceptorText(driver.GetRunResult());
+
+        driver = driver.RunGenerators(c2);
+        var run2 = driver.GetRunResult();
+
+        Assert.AreEqual(text1, InterceptorText(run2),
+            "Precondition: the unrelated edit should not change the interceptor text.");
+
+        var steps = run2.Results
+            .Single()
+            .TrackedSteps[global::ExpressiveSharp.Generator.PolyfillInterceptorGenerator.InterceptorSourcesTrackingName];
+
+        // Cached (input unchanged) or Unchanged (re-ran, equal value) both mean "not re-emitted";
+        // only New/Modified would cause downstream churn.
+        foreach (var step in steps)
+        {
+            foreach (var output in step.Outputs)
+            {
+                Assert.IsTrue(
+                    output.Reason is IncrementalStepRunReason.Cached or IncrementalStepRunReason.Unchanged,
+                    $"Expected the interceptor source to be gated (Cached/Unchanged) but was {output.Reason}.");
+            }
+        }
+    }
+}