From 0979d884b86c07869dbfa68d47431e1302fdb136 Mon Sep 17 00:00:00 2001 From: Koen Date: Thu, 4 Jun 2026 00:39:34 +0000 Subject: [PATCH] Fixed incremental cache busting issue --- .../PolyfillInterceptorGenerator.cs | 111 +++++++----------- .../IncrementalCachingTests.cs | 99 ++++++++++++++++ 2 files changed, 143 insertions(+), 67 deletions(-) create mode 100644 tests/ExpressiveSharp.Generator.Tests/PolyfillInterceptorGenerator/IncrementalCachingTests.cs diff --git a/src/ExpressiveSharp.Generator/PolyfillInterceptorGenerator.cs b/src/ExpressiveSharp.Generator/PolyfillInterceptorGenerator.cs index eb6dba3..bbd2391 100644 --- a/src/ExpressiveSharp.Generator/PolyfillInterceptorGenerator.cs +++ b/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"; + /// Tracking name for the value-equatable interceptor-source node (used by incremental-cache tests). + 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 Sources, ImmutableArray 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 - /// 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. - /// - 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 ────────────────────────────────────── - - /// - /// ANDs 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. - /// - 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); - } - } - } } diff --git a/tests/ExpressiveSharp.Generator.Tests/PolyfillInterceptorGenerator/IncrementalCachingTests.cs b/tests/ExpressiveSharp.Generator.Tests/PolyfillInterceptorGenerator/IncrementalCachingTests.cs new file mode 100644 index 0000000..7eee555 --- /dev/null +++ b/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> e = ExpressionPolyfill.Create>(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}."); + } + } + } +}