From 6fe04b3da251c034bcdfb3a7122b6d9fcc34aebe Mon Sep 17 00:00:00 2001 From: Glenn Watson <5834289+glennawatson@users.noreply.github.com> Date: Wed, 9 Sep 2026 20:04:27 +1000 Subject: [PATCH 01/12] feat(analyzer): report a binding call answered by ReactiveUI's own mixin - RXUIBIND011 reports a call that extension-method lookup sent to ReactiveUI's mixin rather than a generated overload. Nothing reported this before: the extractors recognise a call by its declaring type, so such a call was never one of ours, generated no dispatch, and took the runtime expression engine with the build still green. - A file reaches that state by importing ReactiveUI for ReactiveCommand alongside this package, or by an editor's import cleanup removing the binding import that the generated overloads had made look unused. - The declaring type is read after walking out of any nested type, so an extension declared in an extension block is recognised by the class the consumer wrote rather than by the synthesized one holding its members. Closes #87 --- CLAUDE.md | 1 + README.md | 1 + .../AnalyzerReleases.Unshipped.md | 1 + .../Analyzers/MixinShadowAnalyzer.cs | 139 +++++++++ .../AnalyzerReleases.Unshipped.md | 1 + .../DiagnosticWarnings.cs | 19 ++ .../MixinShadowAnalyzerTests.cs | 295 ++++++++++++++++++ 7 files changed, 457 insertions(+) create mode 100644 src/ReactiveUI.Binding.Analyzer/Analyzers/MixinShadowAnalyzer.cs create mode 100644 src/tests/ReactiveUI.Binding.Analyzer.Tests/MixinShadowAnalyzerTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index 4fd277f..b66c8cc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -515,6 +515,7 @@ Not all platforms support before-change notifications (WPF DP, WinUI DP, WinForm | RXUIBIND008 | Warning | Property does not implement IInteraction | | RXUIBIND009 | Warning | Generated binding dispatch is out of reach from this file | | RXUIBIND010 | Warning | Observed path passes through a type that raises no notification | +| RXUIBIND011 | Warning | Binding call resolved to ReactiveUI's own mixin | ## Code Style & Quality Requirements diff --git a/README.md b/README.md index 27483f5..a30b5b6 100644 --- a/README.md +++ b/README.md @@ -482,6 +482,7 @@ The separate analyzer package reports the following diagnostics: | RXUIBIND008 | Warning | The property selected in a BindInteraction expression does not implement `IInteraction`. | | RXUIBIND009 | Warning | The generated binding dispatch is out of reach from this file, so the call falls back to the runtime stub. | | RXUIBIND010 | Warning | The observed path passes through a type that raises no notification, so it is read once and the observation stops following the path there. | +| RXUIBIND011 | Warning | The call resolved to ReactiveUI's own mixin, so nothing is generated for it and it takes the runtime expression engine. Import `ReactiveUI.Binding` in the file. | ## Where this differs from ReactiveUI diff --git a/src/ReactiveUI.Binding.Analyzer/AnalyzerReleases.Unshipped.md b/src/ReactiveUI.Binding.Analyzer/AnalyzerReleases.Unshipped.md index c36b16e..ad49c38 100644 --- a/src/ReactiveUI.Binding.Analyzer/AnalyzerReleases.Unshipped.md +++ b/src/ReactiveUI.Binding.Analyzer/AnalyzerReleases.Unshipped.md @@ -12,3 +12,4 @@ RXUIBIND008 | Usage | Warning | Property is not an IInteraction RXUIBIND009 | Usage | Warning | Generated binding dispatch is out of reach for this file RXUIBIND010 | Usage | Warning | Observed path passes through a type that raises no notification + RXUIBIND011 | Usage | Warning | Binding call resolved to ReactiveUI's own mixin diff --git a/src/ReactiveUI.Binding.Analyzer/Analyzers/MixinShadowAnalyzer.cs b/src/ReactiveUI.Binding.Analyzer/Analyzers/MixinShadowAnalyzer.cs new file mode 100644 index 0000000..12eb265 --- /dev/null +++ b/src/ReactiveUI.Binding.Analyzer/Analyzers/MixinShadowAnalyzer.cs @@ -0,0 +1,139 @@ +// Copyright (c) 2019-2026 ReactiveUI Association Incorporated. All rights reserved. +// ReactiveUI Association Incorporated licenses this file to you under the MIT license. +// See the LICENSE file in the project root for full license information. + +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; +using ReactiveUI.Binding.Helpers; +using ReactiveUI.Binding.SourceGenerators; + +namespace ReactiveUI.Binding.Analyzer.Analyzers; + +/// +/// Reports a binding call that reached ReactiveUI's own mixin instead of a generated overload, so losing +/// compile-time binding is a build warning rather than something found in a profiler. +/// +/// +/// Which method the call reaches is decided by extension-method lookup. A file that imports ReactiveUI and +/// not this package binds to ReactiveUI's mixin, and every extractor recognises a call by its declaring type, +/// so that call is simply not one of ours: no dispatch is generated and none of the other diagnostics have +/// anything to say about it. Two ordinary things put a file in that state - importing ReactiveUI for +/// ReactiveCommand alongside this package, and an editor's import cleanup removing the binding import +/// once calls resolve to the generated overloads that made it look unused. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class MixinShadowAnalyzer : DiagnosticAnalyzer +{ + /// The namespace ReactiveUI declares its own observation and binding mixins in. + private const string ReactiveUiNamespace = "ReactiveUI"; + + /// The stub class the lean runtime package declares, used to detect that it is referenced. + private const string StubMetadataName = + $"{Constants.SharedGeneratedNamespace}.{Constants.StubExtensionClassName}"; + + /// The same stub class in the System.Reactive flavour of the runtime package. + private const string ReactiveStubMetadataName = + $"{Constants.ReactiveRuntimeNamespace}.{Constants.StubExtensionClassName}"; + + /// The API names this package generates bindings for. + private static readonly ImmutableHashSet GeneratedApiNames = ImmutableHashSet.Create( + StringComparer.Ordinal, + Constants.WhenChangedMethodName, + Constants.WhenChangingMethodName, + Constants.WhenAnyMethodName, + Constants.WhenAnyValueMethodName, + Constants.WhenAnyObservableMethodName, + Constants.BindOneWayMethodName, + Constants.BindTwoWayMethodName, + Constants.OneWayBindMethodName, + Constants.BindMethodName, + Constants.BindToMethodName, + Constants.BindCommandMethodName, + Constants.BindInteractionMethodName); + + /// + public override ImmutableArray SupportedDiagnostics => + ImmutableArray.Create(DiagnosticWarnings.MixinShadowsGeneratedBinding); + + /// + public override void Initialize(AnalysisContext context) + { + ArgumentExceptionHelper.ThrowIfNull(context); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + + // Only a compilation that references this package can have lost a binding to ReactiveUI's mixin, and + // that is a property of the whole compilation, so settle it once rather than per call site. + context.RegisterCompilationStartAction(static startContext => + { + if (!ReferencesBindingPackage(startContext.Compilation)) + { + return; + } + + startContext.RegisterOperationAction( + static operationContext => AnalyzeInvocation(in operationContext), + OperationKind.Invocation); + }); + } + + /// Reports a call that ReactiveUI's mixin answered in place of a generated overload. + /// The operation analysis context. + internal static void AnalyzeInvocation(in OperationAnalysisContext context) + { + var method = ((IInvocationOperation)context.Operation).TargetMethod; + + if (!GeneratedApiNames.Contains(method.Name) || AnalyzerHelpers.IsBindingExtensionMethod(method)) + { + return; + } + + // An extension declared in an extension block belongs to a synthesized type nested in the class the + // consumer wrote, so the namespace is only correct once the walk reaches the outermost type. + var declaringType = OutermostContainingType(method.ContainingType); + if (!IsReactiveUiNamespace(declaringType.ContainingNamespace)) + { + return; + } + + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticWarnings.MixinShadowsGeneratedBinding, + context.Operation.Syntax.GetLocation(), + method.Name, + declaringType.Name)); + } + + /// Walks out of any nested or synthesized type to the type the consumer wrote. + /// The type the invoked method belongs to. + /// The outermost containing type. + private static INamedTypeSymbol OutermostContainingType(INamedTypeSymbol type) + { + while (type.ContainingType is not null) + { + type = type.ContainingType; + } + + return type; + } + + /// Determines whether a namespace is ReactiveUI's own, rather than one nested under it. + /// The namespace the declaring type sits in. + /// for exactly ReactiveUI. + /// + /// Nested namespaces are excluded deliberately: this package declares its surface under + /// ReactiveUI.Binding, which is the case this diagnostic exists to tell apart. + /// + private static bool IsReactiveUiNamespace(INamespaceSymbol namespaceSymbol) => + !namespaceSymbol.IsGlobalNamespace + && string.Equals(namespaceSymbol.Name, ReactiveUiNamespace, StringComparison.Ordinal) + && namespaceSymbol.ContainingNamespace.IsGlobalNamespace; + + /// Determines whether the compilation references this package at all. + /// The compilation being analyzed. + /// when either runtime flavour's stub class is in reach. + private static bool ReferencesBindingPackage(Compilation compilation) => + compilation.GetTypeByMetadataName(StubMetadataName) is not null + || compilation.GetTypeByMetadataName(ReactiveStubMetadataName) is not null; +} diff --git a/src/ReactiveUI.Binding.SourceGenerators/AnalyzerReleases.Unshipped.md b/src/ReactiveUI.Binding.SourceGenerators/AnalyzerReleases.Unshipped.md index c36b16e..ad49c38 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/AnalyzerReleases.Unshipped.md +++ b/src/ReactiveUI.Binding.SourceGenerators/AnalyzerReleases.Unshipped.md @@ -12,3 +12,4 @@ RXUIBIND008 | Usage | Warning | Property is not an IInteraction RXUIBIND009 | Usage | Warning | Generated binding dispatch is out of reach for this file RXUIBIND010 | Usage | Warning | Observed path passes through a type that raises no notification + RXUIBIND011 | Usage | Warning | Binding call resolved to ReactiveUI's own mixin diff --git a/src/ReactiveUI.Binding.SourceGenerators/DiagnosticWarnings.cs b/src/ReactiveUI.Binding.SourceGenerators/DiagnosticWarnings.cs index 90f4423..4f25017 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/DiagnosticWarnings.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/DiagnosticWarnings.cs @@ -48,6 +48,16 @@ internal static class DiagnosticWarnings true, SilentPathLinkDescription); + /// RXUIBIND011: A binding call resolved to ReactiveUI's mixin rather than a generated overload. + internal static readonly DiagnosticDescriptor MixinShadowsGeneratedBinding = new( + "RXUIBIND011", + "Binding call resolved to ReactiveUI's own mixin", + "'{0}' resolved to ReactiveUI's '{1}', so this call generates nothing and takes the runtime expression engine", + UsageCategory, + DiagnosticSeverity.Warning, + true, + MixinShadowsGeneratedBindingDescription); + /// RXUIBIND003: Expression contains private/protected member. internal static readonly DiagnosticDescriptor PrivateMember = new( "RXUIBIND003", @@ -169,6 +179,15 @@ internal static class DiagnosticWarnings private const string InvalidInteractionTypeDescription = "The property selected in the BindInteraction expression must implement IInteraction."; + /// The string description of the shadowed binding warning. + private const string MixinShadowsGeneratedBindingDescription = + "Which method a binding call reaches is decided by extension-method lookup. Where ReactiveUI's own " + + "namespace is imported and this package's is not, the call binds to ReactiveUI's mixin and the " + + "generator never sees it: no dispatch is emitted, and the call takes the runtime expression engine " + + "that a generated binding exists to avoid. Nothing else reports this, because the call was never " + + "recognised as one to generate for. Import 'ReactiveUI.Binding' in the file to restore the generated " + + "overload, which lookup prefers over the generic one."; + /// The string description of the silent path link warning. private const string SilentPathLinkDescription = "A type in the middle of an observed path that raises no notification is read once and never again, " diff --git a/src/tests/ReactiveUI.Binding.Analyzer.Tests/MixinShadowAnalyzerTests.cs b/src/tests/ReactiveUI.Binding.Analyzer.Tests/MixinShadowAnalyzerTests.cs new file mode 100644 index 0000000..c0d1559 --- /dev/null +++ b/src/tests/ReactiveUI.Binding.Analyzer.Tests/MixinShadowAnalyzerTests.cs @@ -0,0 +1,295 @@ +// Copyright (c) 2019-2026 ReactiveUI Association Incorporated. All rights reserved. +// ReactiveUI Association Incorporated licenses this file to you under the MIT license. +// See the LICENSE file in the project root for full license information. + +using ReactiveUI.Binding.Analyzer.Analyzers; +using ReactiveUI.Binding.Analyzer.Tests.Helpers; + +namespace ReactiveUI.Binding.Analyzer.Tests; + +/// +/// Covers RXUIBIND011, which reports a binding call answered by ReactiveUI's own mixin. Without it the call +/// generates nothing and takes the runtime expression engine, and no other diagnostic has anything to say +/// because the call was never recognised as one to generate for. +/// +public class MixinShadowAnalyzerTests +{ + /// The diagnostic this analyzer reports. + private const string DiagnosticId = "RXUIBIND011"; + + /// The namespace ReactiveUI declares its own mixins in. + private const string ReactiveUiNamespace = "ReactiveUI"; + + /// One of the APIs this package generates bindings for. + private const string GeneratedApi = "WhenAnyValue"; + + /// A declaration of this package's stub class, which is how the analyzer knows it is referenced. + private const string BindingPackage = """ + + namespace ReactiveUI.Binding + { + public static class ReactiveUIBindingExtensions + { + } + } + """; + + /// A consumer whose call reaches this package's own extension rather than ReactiveUI's. + private const string GeneratedOverloadSource = """ + using System; + using System.ComponentModel; + using System.Linq.Expressions; + using ReactiveUI.Binding; + + namespace ReactiveUI.Binding + { + public static class ReactiveUIBindingExtensions + { + public static IObservable WhenAnyValue( + this TSender sender, + Expression> property) + where TSender : class + => throw new NotImplementedException(); + } + } + + namespace Consumer + { + public class ConsumerViewModel : INotifyPropertyChanged + { + public event PropertyChangedEventHandler PropertyChanged; + + public string Name { get; set; } + } + + public static class Usage + { + public static IObservable Observe(ConsumerViewModel viewModel) + { + return viewModel.WhenAnyValue(x => x.Name); + } + } + } + """; + + /// A mixin declared in the global namespace, which belongs to nobody in particular. + private const string GlobalNamespaceMixinSource = """ + using System; + using System.ComponentModel; + using System.Linq.Expressions; + + public static class WhenAnyMixins + { + public static IObservable WhenAnyValue( + this TSender sender, + Expression> property) + where TSender : class + => throw new NotImplementedException(); + } + + namespace ReactiveUI.Binding + { + public static class ReactiveUIBindingExtensions + { + } + } + + namespace Consumer + { + public class ConsumerViewModel : INotifyPropertyChanged + { + public event PropertyChangedEventHandler PropertyChanged; + + public string Name { get; set; } + } + + public static class Usage + { + public static IObservable Observe(ConsumerViewModel viewModel) + { + return viewModel.WhenAnyValue(x => x.Name); + } + } + } + """; + + /// A mixin reached through a type nested inside the one the consumer would name. + private const string NestedMixinSource = """ + using System; + using System.ComponentModel; + using System.Linq.Expressions; + + namespace ReactiveUI + { + public static class WhenAnyMixins + { + public static class Grouping + { + public static IObservable WhenAnyValue( + TSender sender, + Expression> property) + where TSender : class + => throw new NotImplementedException(); + } + } + } + + namespace ReactiveUI.Binding + { + public static class ReactiveUIBindingExtensions + { + } + } + + namespace Consumer + { + public class ConsumerViewModel : INotifyPropertyChanged + { + public event PropertyChangedEventHandler PropertyChanged; + + public string Name { get; set; } + } + + public static class Usage + { + public static IObservable Observe(ConsumerViewModel viewModel) + { + return ReactiveUI.WhenAnyMixins.Grouping.WhenAnyValue(viewModel, x => x.Name); + } + } + } + """; + + /// A call ReactiveUI's mixin answered, in a project that references this package. + /// A task representing the asynchronous test operation. + [Test] + public async Task MixinCall_WithTheBindingPackageReferenced_IsReported() + { + var diagnostics = await AnalyzerTestHelper.GetDiagnosticsAsync( + Source(ReactiveUiNamespace, GeneratedApi, BindingPackage)); + + await Assert.That(diagnostics.Count(static d => d.Id == DiagnosticId)).IsEqualTo(1); + } + + /// Without this package there is no generated overload to have lost, so nothing is reported. + /// A task representing the asynchronous test operation. + [Test] + public async Task MixinCall_WithoutTheBindingPackage_IsNotReported() + { + var diagnostics = await AnalyzerTestHelper.GetDiagnosticsAsync( + Source(ReactiveUiNamespace, GeneratedApi, string.Empty)); + + await Assert.That(diagnostics.Any(static d => d.Id == DiagnosticId)).IsFalse(); + } + + /// A method this package does not generate for is ReactiveUI's business alone. + /// A task representing the asynchronous test operation. + [Test] + public async Task MixinCall_WithAMethodThisPackageDoesNotGenerate_IsNotReported() + { + var diagnostics = await AnalyzerTestHelper.GetDiagnosticsAsync( + Source(ReactiveUiNamespace, "Observe", BindingPackage)); + + await Assert.That(diagnostics.Any(static d => d.Id == DiagnosticId)).IsFalse(); + } + + /// A same-named method belonging to somebody else entirely is not ReactiveUI's mixin. + /// A task representing the asynchronous test operation. + [Test] + public async Task MixinCall_FromAnUnrelatedNamespace_IsNotReported() + { + var diagnostics = await AnalyzerTestHelper.GetDiagnosticsAsync( + Source("Fabrikam", GeneratedApi, BindingPackage)); + + await Assert.That(diagnostics.Any(static d => d.Id == DiagnosticId)).IsFalse(); + } + + /// A namespace merely ending in ReactiveUI is somebody else's, so it is left alone. + /// A task representing the asynchronous test operation. + [Test] + public async Task MixinCall_FromANamespaceNestedUnderAnother_IsNotReported() + { + var diagnostics = await AnalyzerTestHelper.GetDiagnosticsAsync( + Source($"Contoso.{ReactiveUiNamespace}", GeneratedApi, BindingPackage)); + + await Assert.That(diagnostics.Any(static d => d.Id == DiagnosticId)).IsFalse(); + } + + /// A call that reached this package's own extension is the outcome being asked for. + /// A task representing the asynchronous test operation. + [Test] + public async Task GeneratedOverload_IsNotReported() + { + var diagnostics = await AnalyzerTestHelper.GetDiagnosticsAsync( + GeneratedOverloadSource); + + await Assert.That(diagnostics.Any(static d => d.Id == DiagnosticId)).IsFalse(); + } + + /// A mixin in the global namespace is nobody's, so it is left alone. + /// A task representing the asynchronous test operation. + [Test] + public async Task MixinCall_FromTheGlobalNamespace_IsNotReported() + { + var diagnostics = await AnalyzerTestHelper.GetDiagnosticsAsync( + GlobalNamespaceMixinSource); + + await Assert.That(diagnostics.Any(static d => d.Id == DiagnosticId)).IsFalse(); + } + + /// + /// A method declared in a type nested inside the mixin class is still ReactiveUI's, which is the shape an + /// extension block produces: its members belong to a synthesized type the consumer never wrote. + /// + /// A task representing the asynchronous test operation. + [Test] + public async Task MixinCall_ThroughANestedType_IsReported() + { + var diagnostics = await AnalyzerTestHelper.GetDiagnosticsAsync(NestedMixinSource); + + await Assert.That(diagnostics.Count(static d => d.Id == DiagnosticId)).IsEqualTo(1); + } + + /// Builds a consumer whose call is answered by a mixin in the named namespace. + /// The namespace the mixin is declared in. + /// The name the mixin exposes, and the name the consumer calls. + /// A declaration of this package's stub class, or an empty string. + /// The source to analyze. + private static string Source(string mixinNamespace, string methodName, string bindingPackage) => $$""" + using System; + using System.ComponentModel; + using System.Linq.Expressions; + using {{mixinNamespace}}; + + namespace {{mixinNamespace}} + { + public static class WhenAnyMixins + { + public static IObservable {{methodName}}( + this TSender sender, + Expression> property) + where TSender : class + => throw new NotImplementedException(); + } + } + {{bindingPackage}} + + namespace Consumer + { + public class ConsumerViewModel : INotifyPropertyChanged + { + public event PropertyChangedEventHandler PropertyChanged; + + public string Name { get; set; } + } + + public static class Usage + { + public static IObservable Observe(ConsumerViewModel viewModel) + { + return viewModel.{{methodName}}(x => x.Name); + } + } + } + """; +} From e8d3bc521ae005b365ec78531eca8e66abb629d3 Mon Sep 17 00:00:00 2001 From: Glenn Watson <5834289+glennawatson@users.noreply.github.com> Date: Wed, 9 Sep 2026 20:04:27 +1000 Subject: [PATCH 02/12] fix(tests): observe the ReactiveObject scenario through the generated overload - The scenario imported ReactiveUI and not this package, so its WhenAnyValue call reached ReactiveUI's mixin and exercised the reflection engine rather than the generated observation the scenario is named for. - Found by RXUIBIND011 on its first run over the repository. --- .../WhenAnyValue/SinglePropertyReactiveObject/Scenario.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/SharedScenarios/WhenAnyValue/SinglePropertyReactiveObject/Scenario.cs b/src/tests/SharedScenarios/WhenAnyValue/SinglePropertyReactiveObject/Scenario.cs index 888ff4b..38e2c26 100644 --- a/src/tests/SharedScenarios/WhenAnyValue/SinglePropertyReactiveObject/Scenario.cs +++ b/src/tests/SharedScenarios/WhenAnyValue/SinglePropertyReactiveObject/Scenario.cs @@ -4,7 +4,7 @@ using System; using System.Runtime.CompilerServices; -using ReactiveUI; +using ReactiveUI.Binding; namespace SharedScenarios.WhenAnyValue.SinglePropertyReactiveObject; From 6fbf89147a4e40577e8b155373e8bc25e3afbe48 Mon Sep 17 00:00:00 2001 From: Glenn Watson <5834289+glennawatson@users.noreply.github.com> Date: Wed, 9 Sep 2026 21:29:50 +1000 Subject: [PATCH 03/12] feat(generator): claim call sites through interceptors where the compiler allows - Compile the generator a second time against Roslyn 4.13, the first release where the call-site description API is supported rather than experimental. The two builds share every source file and differ only in what ROSLYN_4_13 turns on. - Where that build is loaded and the consumer has opted the generated namespace into interception, an observation claims its call site by name instead of competing for it through extension-method lookup. That removes the namespace placement, the generated import and the per-call expression-text match. - A call site the compiler declines to describe is left alone, which is the same outcome an out-of-reach dispatch overload already produces. - Read the description in one helper and write the attribute in one emitter, so neither build grows its own copy and no API-specific code repeats it. Towards #81 --- ....Binding.SourceGenerators.Roslyn413.csproj | 50 ++++++++ src/ReactiveUI.Binding.SourceGenerators.slnx | 1 + .../BindingGenerator.cs | 24 +++- .../CodeGeneration/InterceptorEmitter.cs | 73 +++++++++++ .../ObservationCodeGenerator.cs | 117 ++++++++++++++++-- .../Constants.cs | 11 ++ .../Helpers/InterceptableLocationReader.cs | 96 ++++++++++++++ .../Helpers/ObservationExtractor.cs | 3 +- .../Models/InterceptorLocation.cs | 22 ++++ .../Models/InvocationInfo.cs | 8 +- .../Models/LanguageFeatures.cs | 10 +- .../Observation/AndroidWidgetEvents.cs | 36 +++--- 12 files changed, 418 insertions(+), 33 deletions(-) create mode 100644 src/ReactiveUI.Binding.SourceGenerators.Roslyn413/ReactiveUI.Binding.SourceGenerators.Roslyn413.csproj create mode 100644 src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/InterceptorEmitter.cs create mode 100644 src/ReactiveUI.Binding.SourceGenerators/Helpers/InterceptableLocationReader.cs create mode 100644 src/ReactiveUI.Binding.SourceGenerators/Models/InterceptorLocation.cs diff --git a/src/ReactiveUI.Binding.SourceGenerators.Roslyn413/ReactiveUI.Binding.SourceGenerators.Roslyn413.csproj b/src/ReactiveUI.Binding.SourceGenerators.Roslyn413/ReactiveUI.Binding.SourceGenerators.Roslyn413.csproj new file mode 100644 index 0000000..ef6ee45 --- /dev/null +++ b/src/ReactiveUI.Binding.SourceGenerators.Roslyn413/ReactiveUI.Binding.SourceGenerators.Roslyn413.csproj @@ -0,0 +1,50 @@ + + + netstandard2.0 + ReactiveUI.Binding.SourceGenerators + ReactiveUI.Binding.SourceGenerators + false + true + true + false + $(NoWarn);AD0001 + full + true + + + + + $(DefineConstants);ROSLYN_4_13 + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/ReactiveUI.Binding.SourceGenerators.slnx b/src/ReactiveUI.Binding.SourceGenerators.slnx index f806f0a..f344928 100644 --- a/src/ReactiveUI.Binding.SourceGenerators.slnx +++ b/src/ReactiveUI.Binding.SourceGenerators.slnx @@ -19,6 +19,7 @@ + diff --git a/src/ReactiveUI.Binding.SourceGenerators/BindingGenerator.cs b/src/ReactiveUI.Binding.SourceGenerators/BindingGenerator.cs index 290fd02..8ef6d8d 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/BindingGenerator.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/BindingGenerator.cs @@ -233,6 +233,14 @@ private static void RegisterSharedAttributeOutput( .Append(Constants.GeneratedExtensionClassName) .Append("\n {\n }\n}\n"); + if (features.SupportsInterceptors) + { + // No framework declares the interception attribute, so the compilation that carries the + // interceptors has to. Once for all of them: each dispatch file is another part of the + // same class, but the attribute is a type of its own and would collide with itself. + _ = sb.Append('\n').Append(CodeGeneration.InterceptorEmitter.BuildAttributeDeclaration()); + } + CodeGeneration.CodeGeneratorHelpers.AddGeneratedSource( ctx, "GeneratedBindingsAttributes.g.cs", @@ -298,20 +306,30 @@ private static IncrementalValueProvider SelectLanguageFeatures var sharedNamespace = usesReactiveRuntime ? Constants.ReactiveRuntimeNamespace : Constants.SharedGeneratedNamespace; - var generatedNamespace = supportsGlobalUsings + + // An interceptor claims its call site outright, so where one can be emitted none of the + // placement below applies: there is no namespace for lookup to reach and no import to scope. + var supportsInterceptors = InterceptableLocationReader.IsSupported + && InterceptableLocationReader.IsOptedIn(parseOptions); + + var dispatchNamespace = supportsGlobalUsings ? SelectGeneratedNamespace(configOptions, compilation) : SelectSharedTierNamespace(configOptions, compilation, sharedNamespace); + var generatedNamespace = supportsInterceptors + ? Constants.InterceptorNamespace + : dispatchNamespace; return new LanguageFeatures( supportsCallerArgExpr, languageVersion >= LanguageVersion.CSharp8, emitGeneratedCodeMarkers, generatedNamespace, - supportsGlobalUsings, + supportsGlobalUsings && !supportsInterceptors, callerArgExprAvailable, usesReactiveRuntime, runtimeNamespaceMembers, - primitivesNamespaceMembers); + primitivesNamespaceMembers, + supportsInterceptors); }); /// diff --git a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/InterceptorEmitter.cs b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/InterceptorEmitter.cs new file mode 100644 index 0000000..6d1a8c2 --- /dev/null +++ b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/InterceptorEmitter.cs @@ -0,0 +1,73 @@ +// Copyright (c) 2019-2026 ReactiveUI Association Incorporated. All rights reserved. +// ReactiveUI Association Incorporated licenses this file to you under the MIT license. +// See the LICENSE file in the project root for full license information. + +using System.Text; +using ReactiveUI.Binding.SourceGenerators.Models; + +namespace ReactiveUI.Binding.SourceGenerators.CodeGeneration; + +/// +/// Writes the one thing every intercepted call site needs, whichever API it belongs to: the attribute naming +/// the call, and the declaration of that attribute. +/// +/// +/// An interceptor redirects a call the compiler has already bound, so nothing about it depends on which API is +/// being generated - only on where the call is and what it should run instead. That is why every generator +/// shares this rather than growing its own copy, and why the tier needs none of the namespace placement the +/// dispatch overloads do: lookup never sees an interceptor. +/// +internal static class InterceptorEmitter +{ + /// The attribute a generated method carries to claim a call site. + private const string AttributeName = "global::System.Runtime.CompilerServices.InterceptsLocation"; + + /// Room for the declaration, which is a fixed block of text. + private const int DeclarationCapacity = 640; + + /// Writes the attribute that binds a generated method to one call site. + /// The builder receiving the attribute line. + /// The call site the compiler described. + /// The indentation the enclosing class is written at. + internal static void AppendAttribute(StringBuilder builder, in InterceptorLocation location, string indent) => + _ = builder.Append(indent) + .Append('[') + .Append(AttributeName) + .Append('(') + .Append(location.Version) + .Append(", \"") + .Append(location.Data) + .AppendLine("\")]"); + + /// Builds the declaration of the interception attribute. + /// The source of a file declaring the attribute. + /// + /// The attribute is not part of any framework, so the compiler expects the consumer's own compilation to + /// declare it. It is emitted once for the whole compilation rather than per dispatch file, which would + /// declare the same type repeatedly, and it is written to the rules the oldest supported consumer parses: + /// no file-scoped namespace, no file-local type. + /// + internal static string BuildAttributeDeclaration() + { + var builder = PooledBuilder.Rent(DeclarationCapacity); + + _ = builder.AppendLine("namespace System.Runtime.CompilerServices") + .AppendLine("{") + .AppendLine(" /// Binds a generated method to the call site it replaces.") + .AppendLine(" [global::System.AttributeUsage(global::System.AttributeTargets.Method, AllowMultiple = true)]") + .AppendLine(" internal sealed class InterceptsLocationAttribute : global::System.Attribute") + .AppendLine(" {") + .AppendLine(" /// Initializes a new instance of the class.") + .AppendLine(" /// The encoding of .") + .AppendLine(" /// The call site being replaced.") + .AppendLine(" public InterceptsLocationAttribute(int version, string data)") + .AppendLine(" {") + .AppendLine(" _ = version;") + .AppendLine(" _ = data;") + .AppendLine(" }") + .AppendLine(" }") + .AppendLine("}"); + + return PooledBuilder.ToStringAndReturn(builder); + } +} diff --git a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/ObservationCodeGenerator.cs b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/ObservationCodeGenerator.cs index 1c08426..d153b4a 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/ObservationCodeGenerator.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/ObservationCodeGenerator.cs @@ -104,8 +104,7 @@ internal static bool IsINPChanging(ClassBindingInfo? classInfo) => sb, group, allClasses, - snapshot.SupportsCallerArgExpr, - snapshot.StubHasExpressionParameters, + in snapshot, methodPrefix)); /// Generates an observation method for a single invocation. @@ -932,19 +931,30 @@ private static string MethodSuffix(InvocationInfo inv) => /// The string builder to append to. /// The type group to generate code for. /// All detected class binding info for type mechanism lookup. - /// Whether the target language version supports CallerArgumentExpression. - /// Whether the runtime stub declares the expression parameters this overload has to match. + /// The consumer compilation's language-feature snapshot, which settles the dispatch. /// The method name prefix. private static void GenerateGroup( StringBuilder sb, TypeGroup group, ImmutableArray allClasses, - bool supportsCallerArgExpr, - bool stubHasExpressionParameters, + in LanguageFeatures features, string methodPrefix) { - // Generate the concrete typed extension method overload - GenerateConcreteOverload(sb, group, supportsCallerArgExpr, stubHasExpressionParameters, methodPrefix); + // Either claim each call site outright, or emit the overload that competes for them all. + if (features.SupportsInterceptors) + { + GenerateInterceptors(sb, group, methodPrefix); + } + else + { + GenerateConcreteOverload( + sb, + group, + features.SupportsCallerArgExpr, + features.StubHasExpressionParameters, + methodPrefix); + } + _ = sb.AppendLine(); // Generate the observation methods for each invocation in this group. Call sites that share the @@ -969,6 +979,97 @@ private static void GenerateGroup( } } + /// Emits one interceptor per generated observation, claiming every call site that reaches it. + /// The string builder to append to. + /// The type group whose call sites are being claimed. + /// The method name prefix. + /// + /// Call sites that share a source type and the same expressions produce one observation between them, and + /// the attribute may be applied repeatedly, so they are claimed by a single method carrying one attribute + /// each. A call site the compiler declined to describe is left alone: it keeps whatever the call already + /// resolved to, which is the same outcome an unreachable dispatch overload produces. + /// + private static void GenerateInterceptors(StringBuilder sb, TypeGroup group, string methodPrefix) + { + foreach (var entry in GroupCallSitesByObservation(group)) + { + var first = entry.Value[0]; + var propCount = first.PropertyPaths.Length; + + foreach (var callSite in entry.Value) + { + InterceptorEmitter.AppendAttribute(sb, callSite.Interceptor, " "); + } + + _ = sb.Append(" internal static global::System.IObservable<").Append(first.ReturnTypeFullName) + .Append("> __Intercept_").Append(methodPrefix).Append('_').Append(entry.Key).AppendLine("(") + .Append(" ").Append(first.SourceTypeFullName).AppendLine(" objectToMonitor,"); + + AppendPropertyParameters(sb, first, propCount, first.HasSelector); + + _ = sb.Append(" => __").Append(methodPrefix).Append('_').Append(entry.Key) + .Append("(objectToMonitor").Append(first.HasSelector ? ", selector" : string.Empty).AppendLine(");"); + } + } + + /// Gathers the call sites of a group under the observation each of them reaches. + /// The type group whose call sites are being gathered. + /// Each generated observation, against every call site that resolves to it. + /// + /// A call site the compiler declined to describe is left out: nothing can claim it, and it keeps whatever + /// the call already resolved to. + /// + private static Dictionary> GroupCallSitesByObservation(TypeGroup group) + { + var claimed = new Dictionary>(StringComparer.Ordinal); + for (var i = 0; i < group.Invocations.Length; i++) + { + var inv = group.Invocations[i]; + if (!inv.Interceptor.IsAvailable) + { + continue; + } + + var suffix = MethodSuffix(inv); + if (!claimed.TryGetValue(suffix, out var callSites)) + { + callSites = []; + claimed[suffix] = callSites; + } + + callSites.Add(inv); + } + + return claimed; + } + + /// Emits the observed-property parameters, and the projection when the overload takes one. + /// The string builder to append to. + /// The invocation whose types the parameters are written from. + /// How many observed properties the overload takes. + /// Whether the overload takes a projection after them. + private static void AppendPropertyParameters( + StringBuilder sb, + InvocationInfo first, + int propCount, + bool hasSelector) + { + for (var i = 0; i < propCount; i++) + { + var type = first.PropertyPaths[i][first.PropertyPaths[i].Length - 1].PropertyTypeFullName; + _ = sb.Append(" global::System.Linq.Expressions.Expression> property").Append(i + 1); + _ = hasSelector || i < propCount - 1 ? sb.AppendLine(",") : sb.AppendLine(")"); + } + + if (!hasSelector) + { + return; + } + + _ = sb.Append(" ").Append(GetSelectorType(first)).AppendLine(" selector)"); + } + /// Emits the trailing projection lambda that gathers a selector-less CombineLatest into one emission. /// The string builder to append to. /// The invocation, whose return type is the emission being constructed. diff --git a/src/ReactiveUI.Binding.SourceGenerators/Constants.cs b/src/ReactiveUI.Binding.SourceGenerators/Constants.cs index b26beeb..d54c937 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/Constants.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/Constants.cs @@ -58,6 +58,17 @@ internal static class Constants /// internal const string GeneratedNamespaceRoot = "ReactiveUI.Binding.Generated"; + /// The namespace interceptors are emitted into. + /// + /// Fixed rather than derived from the consumer, because nothing has to reach it: an interceptor claims its + /// call site by name and is never found by lookup. Being fixed is what lets the shipped props opt exactly + /// this namespace into interception without knowing anything about the project it is opting in. + /// + internal const string InterceptorNamespace = "ReactiveUI.Binding.Generated.Interceptors"; + + /// The build property a consumer's compiler reads the interception opt-in from. + internal const string InterceptorsNamespacesFeature = "InterceptorsNamespaces"; + /// Fully qualified name of the attribute that exposes an assembly's internals to another. internal const string InternalsVisibleToAttributeFullName = "global::System.Runtime.CompilerServices.InternalsVisibleToAttribute"; diff --git a/src/ReactiveUI.Binding.SourceGenerators/Helpers/InterceptableLocationReader.cs b/src/ReactiveUI.Binding.SourceGenerators/Helpers/InterceptableLocationReader.cs new file mode 100644 index 0000000..b2c45a4 --- /dev/null +++ b/src/ReactiveUI.Binding.SourceGenerators/Helpers/InterceptableLocationReader.cs @@ -0,0 +1,96 @@ +// Copyright (c) 2019-2026 ReactiveUI Association Incorporated. All rights reserved. +// ReactiveUI Association Incorporated licenses this file to you under the MIT license. +// See the LICENSE file in the project root for full license information. + +using System.Threading; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using ReactiveUI.Binding.SourceGenerators.Models; + +namespace ReactiveUI.Binding.SourceGenerators.Helpers; + +/// Reads the call-site description an interceptor has to name. +/// +/// The only place either build asks the compiler about interception. The 4.13 build has the API and answers; +/// the baseline build has no such API to call and answers that it described nothing, which every caller +/// already handles because a call site can be undescribable on a new compiler too. +/// +internal static class InterceptableLocationReader +{ + /// Gets a value indicating whether this build can describe a call site at all. + /// + /// Fixed when the generator is compiled, not read from the consumer: which of the two builds is loaded is + /// decided by the compiler's own version, through the analyzer slot the package was resolved from. + /// + internal static bool IsSupported => +#if ROSLYN_4_13 + true; +#else + false; +#endif + + /// Determines whether the consumer has opted the generated namespace into interception. + /// The consumer's parse options, which carry the opt-in the build set. + /// when an interceptor emitted here would be honoured. + /// + /// Interception is refused outright for a namespace the project did not list, so emitting one without the + /// opt-in turns every call site into a build error. The package's own props lists the generated namespace, + /// which is why this is normally true; a consumer who clears the property gets the dispatch overloads back. + /// A listed namespace covers the ones nested under it, so a prefix counts as a match. + /// + internal static bool IsOptedIn(ParseOptions parseOptions) + { + if (!parseOptions.Features.TryGetValue(Constants.InterceptorsNamespacesFeature, out var namespaces) + || string.IsNullOrEmpty(namespaces)) + { + return false; + } + + foreach (var candidate in namespaces.Split(';')) + { + var trimmed = candidate.Trim(); + if (trimmed.Length == 0) + { + continue; + } + + if (string.Equals(trimmed, Constants.InterceptorNamespace, StringComparison.Ordinal) + || (Constants.InterceptorNamespace.Length > trimmed.Length + && Constants.InterceptorNamespace[trimmed.Length] == '.' + && Constants.InterceptorNamespace.StartsWith(trimmed, StringComparison.Ordinal))) + { + return true; + } + } + + return false; + } + + /// Describes a call site, when the host compiler can. + /// The model the invocation was bound in. + /// The call site. + /// Cancels the read. + /// The description, or a location reporting that none was produced. + internal static InterceptorLocation Read( + SemanticModel semanticModel, + InvocationExpressionSyntax invocation, + CancellationToken cancellationToken) + { +#if ROSLYN_4_13 + var location = Microsoft.CodeAnalysis.CSharp.CSharpExtensions.GetInterceptableLocation( + semanticModel, + invocation, + cancellationToken); + + return location is null ? default : new InterceptorLocation(location.Version, location.Data); +#else + + // The baseline compiler cannot describe a call site, so nothing here is interceptable. The arguments + // are read so the two builds keep one signature rather than diverging on what they accept. + _ = semanticModel; + _ = invocation; + _ = cancellationToken; + return default; +#endif + } +} diff --git a/src/ReactiveUI.Binding.SourceGenerators/Helpers/ObservationExtractor.cs b/src/ReactiveUI.Binding.SourceGenerators/Helpers/ObservationExtractor.cs index 660060e..d8bda59 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/Helpers/ObservationExtractor.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/Helpers/ObservationExtractor.cs @@ -123,7 +123,8 @@ internal static bool IsSelectorParameterName(string parameterName) => isBeforeChange, hasSelector, expectedMethodName, - new([.. expressionTexts])); + new([.. expressionTexts]), + InterceptableLocationReader.Read(semanticModel, invocation, ct)); } /// diff --git a/src/ReactiveUI.Binding.SourceGenerators/Models/InterceptorLocation.cs b/src/ReactiveUI.Binding.SourceGenerators/Models/InterceptorLocation.cs new file mode 100644 index 0000000..1c99019 --- /dev/null +++ b/src/ReactiveUI.Binding.SourceGenerators/Models/InterceptorLocation.cs @@ -0,0 +1,22 @@ +// Copyright (c) 2019-2026 ReactiveUI Association Incorporated. All rights reserved. +// ReactiveUI Association Incorporated licenses this file to you under the MIT license. +// See the LICENSE file in the project root for full license information. + +namespace ReactiveUI.Binding.SourceGenerators.Models; + +/// Where a call site sits, in the form the compiler accepts on an interceptor. +/// The encoding the compiler asked for, which travels with the data it produced. +/// The opaque call-site description the compiler produced, empty when it described none. +/// +/// Two strings and an integer rather than the compiler's own type: a pipeline model has to compare by value +/// and carry no symbol or syntax, and this is the whole of what an interceptor needs to name its call site. +/// +internal readonly record struct InterceptorLocation(int Version, string? Data) +{ + /// Gets a value indicating whether the compiler described this call site. + /// + /// A call site is undescribable when it is not an invocation the compiler can intercept - one written + /// through a type parameter, for instance - and when the host compiler predates interception entirely. + /// + internal bool IsAvailable => !string.IsNullOrEmpty(Data); +} diff --git a/src/ReactiveUI.Binding.SourceGenerators/Models/InvocationInfo.cs b/src/ReactiveUI.Binding.SourceGenerators/Models/InvocationInfo.cs index 32bd624..566ec2a 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/Models/InvocationInfo.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/Models/InvocationInfo.cs @@ -18,6 +18,11 @@ namespace ReactiveUI.Binding.SourceGenerators.Models; /// Whether the invocation includes a selector/projection function. /// The name of the invoked method (e.g., WhenChanged, WhenChanging, WhenAnyValue). /// The original expression text of each lambda argument, used for CallerArgumentExpression dispatch. +/// +/// Where this call site is, for a build that claims call sites outright rather than competing for them. +/// Reports that it describes nothing on a compiler that cannot describe one, and for a call the compiler +/// refuses to let anything intercept. +/// internal sealed record InvocationInfo( string CallerFilePath, int CallerLineNumber, @@ -27,4 +32,5 @@ internal sealed record InvocationInfo( bool IsBeforeChange, bool HasSelector, string MethodName, - EquatableArray ExpressionTexts); + EquatableArray ExpressionTexts, + InterceptorLocation Interceptor = default); diff --git a/src/ReactiveUI.Binding.SourceGenerators/Models/LanguageFeatures.cs b/src/ReactiveUI.Binding.SourceGenerators/Models/LanguageFeatures.cs index af34caa..952a0e4 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/Models/LanguageFeatures.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/Models/LanguageFeatures.cs @@ -61,6 +61,13 @@ namespace ReactiveUI.Binding.SourceGenerators.Models; /// Primitives types, and only the ones that flavour actually offers are shifted onto it - the parts that ship in /// the shared core, the disposables among them, keep their names in both. Empty for a lean consumer. /// +/// +/// Whether generated code claims each call site outright instead of competing for it. This needs both halves: +/// a compiler that can describe a call site, which is settled by the analyzer slot the package resolved to, and +/// a consumer that has opted the generated namespace into interception. Where it holds, none of the placement +/// this record otherwise describes applies - lookup never sees an interceptor, so there is no namespace to +/// reach, no import to emit, and no expression text to match at run time. +/// internal readonly record struct LanguageFeatures( bool SupportsCallerArgExpr, bool SupportsNullable, @@ -70,4 +77,5 @@ internal readonly record struct LanguageFeatures( bool StubHasExpressionParameters = false, bool UsesReactiveRuntime = false, EquatableArray RuntimeNamespaceMembers = default, - EquatableArray PrimitivesNamespaceMembers = default); + EquatableArray PrimitivesNamespaceMembers = default, + bool SupportsInterceptors = false); diff --git a/src/ReactiveUI.Binding.SourceGenerators/Plugins/Observation/AndroidWidgetEvents.cs b/src/ReactiveUI.Binding.SourceGenerators/Plugins/Observation/AndroidWidgetEvents.cs index 12cb253..ea7345c 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/Plugins/Observation/AndroidWidgetEvents.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/Plugins/Observation/AndroidWidgetEvents.cs @@ -21,40 +21,38 @@ namespace ReactiveUI.Binding.SourceGenerators.Plugins.Observation; /// internal static class AndroidWidgetEvents { - /// The property each reporting widget raises an event for, and the event it raises. - private static readonly Dictionary _changeEvents = new(StringComparer.Ordinal) + /// Finds the event a widget raises when the named property changes. + /// The property being observed. + /// The event name, or when no widget reports that property. + /// + /// A switch rather than a lookup table: the set is closed and known here, so the compiler turns it into a + /// jump over the name with nothing built at startup and nothing held for the life of the generator. + /// + internal static string? FindChangeEvent(string propertyName) => propertyName switch { // TextView and everything built on it. - ["Text"] = "TextChanged", + "Text" => "TextChanged", // NumberPicker. - ["Value"] = "ValueChanged", + "Value" => "ValueChanged", // RatingBar. - ["Rating"] = "RatingBarChange", + "Rating" => "RatingBarChange", // CompoundButton, and so CheckBox, RadioButton and Switch. - ["Checked"] = "CheckedChange", + "Checked" => "CheckedChange", // CalendarView. - ["Date"] = "DateChange", + "Date" => "DateChange", // TabHost. - ["CurrentTab"] = "TabChanged", + "CurrentTab" => "TabChanged", // TimePicker, whose hour and minute are named one way from API 23 and the other before it. - ["Hour"] = "TimeChanged", - ["Minute"] = "TimeChanged", - ["CurrentHour"] = "TimeChanged", - ["CurrentMinute"] = "TimeChanged", + "Hour" or "Minute" or "CurrentHour" or "CurrentMinute" => "TimeChanged", // AdapterView, and so Spinner and ListView. - ["SelectedItem"] = "ItemSelected", + "SelectedItem" => "ItemSelected", + _ => null, }; - - /// Finds the event a widget raises when the named property changes. - /// The property being observed. - /// The event name, or when no widget reports that property. - internal static string? FindChangeEvent(string propertyName) => - _changeEvents.TryGetValue(propertyName, out var changeEvent) ? changeEvent : null; } From 399a9f435040f5f20720e60842184cd8f0de8ca4 Mon Sep 17 00:00:00 2001 From: Glenn Watson <5834289+glennawatson@users.noreply.github.com> Date: Wed, 9 Sep 2026 21:37:54 +1000 Subject: [PATCH 04/12] refactor(generator): share the interceptor emission across the observation APIs - Move the grouping, the attribute and the parameter list into InterceptorEmitter, so WhenAny reuses them rather than repeating them and each API supplies only the signature it is called with. - Claim WhenAny call sites through the same path as WhenChanged, WhenChanging and WhenAnyValue. --- .../CodeGeneration/InterceptorEmitter.cs | 100 ++++++++++++++++++ .../ObservationCodeGenerator.cs | 84 ++------------- .../CodeGeneration/WhenAnyCodeGenerator.cs | 37 +++++-- 3 files changed, 138 insertions(+), 83 deletions(-) diff --git a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/InterceptorEmitter.cs b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/InterceptorEmitter.cs index 6d1a8c2..2f20594 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/InterceptorEmitter.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/InterceptorEmitter.cs @@ -70,4 +70,104 @@ internal static string BuildAttributeDeclaration() return PooledBuilder.ToStringAndReturn(builder); } + + /// Emits one interceptor per generated body, claiming every call site that reaches it. + /// The string builder to append to. + /// The type group whose call sites are being claimed. + /// The method name prefix the generated bodies carry. + /// Names the body a call site reaches. + /// Writes the parameters after the receiver, closing the list. + /// + /// The grouping and the attribute are the whole of what every API shares here, so they live in one place + /// and each API supplies only the signature it is being called with. Call sites that reach one body are + /// claimed by one method carrying an attribute each, which is what the attribute allowing repeats is for. + /// + internal static void GenerateInterceptors( + StringBuilder builder, + ObservationCodeGenerator.TypeGroup group, + string methodPrefix, + Func suffixOf, + Action appendParameters) + { + foreach (var entry in GroupCallSitesByBody(group, suffixOf)) + { + var first = entry.Value[0]; + + foreach (var callSite in entry.Value) + { + AppendAttribute(builder, callSite.Interceptor, " "); + } + + _ = builder.Append(" internal static global::System.IObservable<").Append(first.ReturnTypeFullName) + .Append("> __Intercept_").Append(methodPrefix).Append('_').Append(entry.Key).AppendLine("(") + .Append(" ").Append(first.SourceTypeFullName).AppendLine(" objectToMonitor,"); + + appendParameters(builder, first); + + _ = builder.Append(" => __").Append(methodPrefix).Append('_').Append(entry.Key) + .Append("(objectToMonitor").Append(first.HasSelector ? ", selector" : string.Empty).AppendLine(");"); + } + } + + /// Emits the observed-property parameters, and the projection when the overload takes one. + /// The string builder to append to. + /// The invocation whose types the parameters are written from. + /// How many observed properties the overload takes. + /// The projection's type, or when it takes none. + internal static void AppendPropertyParameters( + StringBuilder sb, + InvocationInfo first, + int propCount, + string? selectorType) + { + var hasSelector = selectorType is not null; + for (var i = 0; i < propCount; i++) + { + var type = first.PropertyPaths[i][first.PropertyPaths[i].Length - 1].PropertyTypeFullName; + _ = sb.Append(" global::System.Linq.Expressions.Expression> property").Append(i + 1); + _ = hasSelector || i < propCount - 1 ? sb.AppendLine(",") : sb.AppendLine(")"); + } + + if (!hasSelector) + { + return; + } + + _ = sb.Append(" ").Append(selectorType).AppendLine(" selector)"); + } + + /// Gathers the call sites of a group under the body each of them reaches. + /// The type group whose call sites are being gathered. + /// Names the body a call site reaches. + /// Each generated body, against every call site that resolves to it. + /// + /// A call site the compiler declined to describe is left out: nothing can claim it, and it keeps whatever + /// the call already resolved to. + /// + private static Dictionary> GroupCallSitesByBody( + ObservationCodeGenerator.TypeGroup group, + Func suffixOf) + { + var claimed = new Dictionary>(StringComparer.Ordinal); + for (var i = 0; i < group.Invocations.Length; i++) + { + var inv = group.Invocations[i]; + if (!inv.Interceptor.IsAvailable) + { + continue; + } + + var suffix = suffixOf(inv); + if (!claimed.TryGetValue(suffix, out var callSites)) + { + callSites = []; + claimed[suffix] = callSites; + } + + callSites.Add(inv); + } + + return claimed; + } } diff --git a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/ObservationCodeGenerator.cs b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/ObservationCodeGenerator.cs index d153b4a..6fd249c 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/ObservationCodeGenerator.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/ObservationCodeGenerator.cs @@ -989,86 +989,16 @@ private static void GenerateGroup( /// each. A call site the compiler declined to describe is left alone: it keeps whatever the call already /// resolved to, which is the same outcome an unreachable dispatch overload produces. /// - private static void GenerateInterceptors(StringBuilder sb, TypeGroup group, string methodPrefix) - { - foreach (var entry in GroupCallSitesByObservation(group)) - { - var first = entry.Value[0]; - var propCount = first.PropertyPaths.Length; - - foreach (var callSite in entry.Value) - { - InterceptorEmitter.AppendAttribute(sb, callSite.Interceptor, " "); - } - - _ = sb.Append(" internal static global::System.IObservable<").Append(first.ReturnTypeFullName) - .Append("> __Intercept_").Append(methodPrefix).Append('_').Append(entry.Key).AppendLine("(") - .Append(" ").Append(first.SourceTypeFullName).AppendLine(" objectToMonitor,"); - - AppendPropertyParameters(sb, first, propCount, first.HasSelector); - - _ = sb.Append(" => __").Append(methodPrefix).Append('_').Append(entry.Key) - .Append("(objectToMonitor").Append(first.HasSelector ? ", selector" : string.Empty).AppendLine(");"); - } - } - - /// Gathers the call sites of a group under the observation each of them reaches. - /// The type group whose call sites are being gathered. - /// Each generated observation, against every call site that resolves to it. - /// - /// A call site the compiler declined to describe is left out: nothing can claim it, and it keeps whatever - /// the call already resolved to. - /// - private static Dictionary> GroupCallSitesByObservation(TypeGroup group) - { - var claimed = new Dictionary>(StringComparer.Ordinal); - for (var i = 0; i < group.Invocations.Length; i++) - { - var inv = group.Invocations[i]; - if (!inv.Interceptor.IsAvailable) - { - continue; - } - - var suffix = MethodSuffix(inv); - if (!claimed.TryGetValue(suffix, out var callSites)) - { - callSites = []; - claimed[suffix] = callSites; - } - - callSites.Add(inv); - } - - return claimed; - } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static void GenerateInterceptors(StringBuilder sb, TypeGroup group, string methodPrefix) => + InterceptorEmitter.GenerateInterceptors(sb, group, methodPrefix, MethodSuffix, AppendObservationParameters); - /// Emits the observed-property parameters, and the projection when the overload takes one. + /// Writes the observed-property parameters an observation overload takes. /// The string builder to append to. /// The invocation whose types the parameters are written from. - /// How many observed properties the overload takes. - /// Whether the overload takes a projection after them. - private static void AppendPropertyParameters( - StringBuilder sb, - InvocationInfo first, - int propCount, - bool hasSelector) - { - for (var i = 0; i < propCount; i++) - { - var type = first.PropertyPaths[i][first.PropertyPaths[i].Length - 1].PropertyTypeFullName; - _ = sb.Append(" global::System.Linq.Expressions.Expression> property").Append(i + 1); - _ = hasSelector || i < propCount - 1 ? sb.AppendLine(",") : sb.AppendLine(")"); - } - - if (!hasSelector) - { - return; - } - - _ = sb.Append(" ").Append(GetSelectorType(first)).AppendLine(" selector)"); - } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static void AppendObservationParameters(StringBuilder sb, InvocationInfo first) => + InterceptorEmitter.AppendPropertyParameters(sb, first, first.PropertyPaths.Length, first.HasSelector ? GetSelectorType(first) : null); /// Emits the trailing projection lambda that gathers a selector-less CombineLatest into one emission. /// The string builder to append to. diff --git a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/WhenAnyCodeGenerator.cs b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/WhenAnyCodeGenerator.cs index 299efd8..35a01ce 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/WhenAnyCodeGenerator.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/WhenAnyCodeGenerator.cs @@ -271,12 +271,25 @@ private static void EmitGroup( ImmutableArray allClasses, in LanguageFeatures features) { - GenerateConcreteOverload( - sb, - group, - features.SupportsCallerArgExpr, - features.SupportsNullable, - features.StubHasExpressionParameters); + if (features.SupportsInterceptors) + { + InterceptorEmitter.GenerateInterceptors( + sb, + group, + Constants.WhenAnyMethodName, + ObservationMethodSuffix, + AppendWhenAnyParameters); + } + else + { + GenerateConcreteOverload( + sb, + group, + features.SupportsCallerArgExpr, + features.SupportsNullable, + features.StubHasExpressionParameters); + } + _ = sb.AppendLine(); for (var i = 0; i < group.Invocations.Length; i++) @@ -290,6 +303,18 @@ private static void EmitGroup( } } + /// Writes the parameters a WhenAny interceptor takes after the observed object. + /// The string builder to append to. + /// The invocation whose types the parameters are written from. + /// WhenAny always projects, so the projection closes the list rather than being optional. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static void AppendWhenAnyParameters(StringBuilder sb, InvocationInfo first) => + InterceptorEmitter.AppendPropertyParameters( + sb, + first, + first.PropertyPaths.Length, + GetWhenAnySelectorType(first)); + /// Emits the if/else-if dispatch table that routes each matched WhenAny invocation to its generated method. /// The string builder to append to. /// The type group containing invocations that share a signature. From d5185b3e9055610f7955b7bd42775e2f36f795cf Mon Sep 17 00:00:00 2001 From: Glenn Watson <5834289+glennawatson@users.noreply.github.com> Date: Wed, 9 Sep 2026 21:46:52 +1000 Subject: [PATCH 05/12] feat(generator): claim the property-binding call sites through interceptors - BindOneWay, BindTwoWay, OneWayBind and Bind already describe their output through one dispatch descriptor, so the interceptor shape is written once against that descriptor rather than four times. - The interceptor takes what the call site passes and forwards to the worker, so the dispatch parameters and the run-time selector-text match go away. --- .../CodeGeneration/BindingEmitterHelpers.cs | 108 ++++++++++++++++++ .../CodeGeneration/InterceptorEmitter.cs | 22 ++++ .../Helpers/BindingExtractor.cs | 3 +- .../Invocations/BindInvocationGenerator.cs | 6 +- .../BindOneWayInvocationGenerator.cs | 6 +- .../BindTwoWayInvocationGenerator.cs | 6 +- .../OneWayBindInvocationGenerator.cs | 6 +- .../Models/BindingInvocationInfo.cs | 6 +- 8 files changed, 145 insertions(+), 18 deletions(-) diff --git a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/BindingEmitterHelpers.cs b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/BindingEmitterHelpers.cs index edc842d..3aceaca 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/BindingEmitterHelpers.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/BindingEmitterHelpers.cs @@ -547,6 +547,83 @@ internal static void GenerateDispatchOverload( CodeGeneratorHelpers.AppendBindingDispatchFallthrough(sb); } + /// Emits whichever of the two ways this group's call sites are reached. + /// The string builder to append to. + /// The binding type group. + /// What distinguishes this API's output from the other three. + /// The consumer compilation's language-feature snapshot. + /// + /// The four APIs choose between the same two shapes on the same condition, so the choice is made here + /// rather than repeated at each of their four call sites. + /// + internal static void EmitOverloadOrInterceptors( + StringBuilder sb, + BindingTypeGroup group, + BindingDispatchApi api, + in LanguageFeatures features) + { + if (features.SupportsInterceptors) + { + GenerateInterceptors(sb, group, api, features.SupportsNullable); + return; + } + + GenerateDispatchOverload( + sb, + group, + api, + features.SupportsCallerArgExpr, + features.SupportsNullable, + features.StubHasExpressionParameters); + } + + /// Emits one interceptor per generated worker, claiming every call site that reaches it. + /// The string builder to append to. + /// The binding type group. + /// What distinguishes this API's output from the other three. + /// Whether the target supports nullable reference types (C# 8+). + /// + /// The signature is the overload's without the dispatch parameters: an interceptor is reached by name + /// rather than matched, so it takes only what the call site passes and forwards straight to the worker. + /// + internal static void GenerateInterceptors( + StringBuilder sb, + BindingTypeGroup group, + BindingDispatchApi api, + bool supportsNullable) + { + var first = group.Invocations[0]; + var sourceLeaf = CodeGeneratorHelpers.NullableSelectorLeafType(first.SourcePropertyPath, supportsNullable); + var targetLeaf = CodeGeneratorHelpers.NullableSelectorLeafType(first.TargetPropertyPath, supportsNullable); + var extraArguments = api.FormatExtraArguments(group); + + foreach (var entry in GroupCallSitesByWorker(group)) + { + foreach (var callSite in entry.Value) + { + InterceptorEmitter.AppendAttribute(sb, callSite.Interceptor, " "); + } + + _ = sb.Append(" internal static ").Append(api.FormatReturnType(group)).Append(" __Intercept_") + .Append(api.Name).Append('_').Append(entry.Key).AppendLine("(") + .Append(" ").Append(api.ReceiverIsTarget ? group.TargetTypeFullName : group.SourceTypeFullName) + .Append(' ').Append(api.ReceiverParameterName).AppendLine(",") + .Append(CodeGeneratorHelpers.ParameterIndent) + .Append(api.ReceiverIsTarget ? group.SourceTypeFullName : group.TargetTypeFullName) + .Append(' ').Append(api.OtherParameterName).AppendLine(",") + .Append(GeneratedSyntax.SelectorParameterOpen).Append(group.SourceTypeFullName).Append(", ").Append(sourceLeaf) + .Append(">> ").Append(api.SourceSelectorName).AppendLine(",") + .Append(GeneratedSyntax.SelectorParameterOpen).Append(group.TargetTypeFullName).Append(", ").Append(targetLeaf) + .Append(">> ").Append(api.TargetSelectorName).AppendLine(","); + + api.AppendExtraParameters(sb, group); + InterceptorEmitter.CloseParameterList(sb); + + _ = sb.Append(" => ").Append(api.WorkerMethodPrefix).Append(entry.Key).Append('(') + .Append(api.WorkerArguments).Append(extraArguments).AppendLine(");").AppendLine(); + } + } + /// Emits the head of a generated worker: its signature, the path it binds, and the hook guard. /// The string builder to append to. /// What distinguishes this API's worker from the other three. @@ -634,6 +711,37 @@ internal static BindingObservables EmitDualStreamStages(StringBuilder sb, Bindin return new(sourceVar, targetVar); } + /// Gathers the call sites of a group under the worker each of them reaches. + /// The binding type group whose call sites are being gathered. + /// Each generated worker, against every call site that resolves to it. + /// + /// A call site the compiler declined to describe is left out: nothing can claim it, and it keeps whatever + /// the call already resolved to. + /// + private static Dictionary> GroupCallSitesByWorker(BindingTypeGroup group) + { + var claimed = new Dictionary>(StringComparer.Ordinal); + for (var i = 0; i < group.Invocations.Length; i++) + { + var inv = group.Invocations[i]; + if (!inv.Interceptor.IsAvailable) + { + continue; + } + + var suffix = BindingMethodSuffix(inv); + if (!claimed.TryGetValue(suffix, out var callSites)) + { + callSites = []; + claimed[suffix] = callSites; + } + + callSites.Add(inv); + } + + return claimed; + } + /// Finds the type a view declares its view model property as. /// The view type's binding info. /// The declared property type, or when the view exposes no such property. diff --git a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/InterceptorEmitter.cs b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/InterceptorEmitter.cs index 2f20594..9c58274 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/InterceptorEmitter.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/InterceptorEmitter.cs @@ -137,6 +137,28 @@ internal static void AppendPropertyParameters( _ = sb.Append(" ").Append(selectorType).AppendLine(" selector)"); } + /// Closes a parameter list whose last entry was written expecting another to follow. + /// The builder whose trailing separator becomes the closing parenthesis. + /// + /// The parameter writers are shared with the dispatch overloads, which always have the caller-info + /// parameters coming after, so each entry ends in a separator. An interceptor takes none of those, so the + /// last separator written is the one that has to close the list instead. + /// + internal static void CloseParameterList(StringBuilder builder) + { + for (var i = builder.Length - 1; i >= 0; i--) + { + if (builder[i] != ',') + { + continue; + } + + builder.Length = i; + _ = builder.AppendLine(")"); + return; + } + } + /// Gathers the call sites of a group under the body each of them reaches. /// The type group whose call sites are being gathered. /// Names the body a call site reaches. diff --git a/src/ReactiveUI.Binding.SourceGenerators/Helpers/BindingExtractor.cs b/src/ReactiveUI.Binding.SourceGenerators/Helpers/BindingExtractor.cs index 6fa14c7..73050e6 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/Helpers/BindingExtractor.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/Helpers/BindingExtractor.cs @@ -86,7 +86,8 @@ is not var (sourceTypeFullName, targetTypeFullName)) methodName, CodeGeneration.CodeGeneratorHelpers.NormalizeLambdaText(sourcePropertyArg.ToString()), CodeGeneration.CodeGeneratorHelpers.NormalizeLambdaText(targetPropertyArg.ToString()), - hasConverterOverride); + hasConverterOverride, + InterceptableLocationReader.Read(semanticModel, invocation, ct)); } /// diff --git a/src/ReactiveUI.Binding.SourceGenerators/Invocations/BindInvocationGenerator.cs b/src/ReactiveUI.Binding.SourceGenerators/Invocations/BindInvocationGenerator.cs index 0ba4de4..8aeb856 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/Invocations/BindInvocationGenerator.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/Invocations/BindInvocationGenerator.cs @@ -34,13 +34,11 @@ internal static void Register( data.Left.Left, data.Left.Right, data.Right, - static (sb, group, f) => BindingEmitterHelpers.GenerateDispatchOverload( + static (sb, group, f) => BindingEmitterHelpers.EmitOverloadOrInterceptors( sb, group, BindCodeGenerator.DispatchApi, - f.SupportsCallerArgExpr, - f.SupportsNullable, - f.StubHasExpressionParameters), + in f), static (sb, c) => BindCodeGenerator.GenerateBindMethod( sb, c.Invocation, diff --git a/src/ReactiveUI.Binding.SourceGenerators/Invocations/BindOneWayInvocationGenerator.cs b/src/ReactiveUI.Binding.SourceGenerators/Invocations/BindOneWayInvocationGenerator.cs index 16c07d5..299ba1e 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/Invocations/BindOneWayInvocationGenerator.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/Invocations/BindOneWayInvocationGenerator.cs @@ -34,13 +34,11 @@ internal static void Register( data.Left.Left, data.Left.Right, data.Right, - static (sb, group, f) => BindingEmitterHelpers.GenerateDispatchOverload( + static (sb, group, f) => BindingEmitterHelpers.EmitOverloadOrInterceptors( sb, group, BindOneWayCodeGenerator.DispatchApi, - f.SupportsCallerArgExpr, - f.SupportsNullable, - f.StubHasExpressionParameters), + in f), static (sb, c) => BindOneWayCodeGenerator.GenerateBindOneWayMethod( sb, c.Invocation, diff --git a/src/ReactiveUI.Binding.SourceGenerators/Invocations/BindTwoWayInvocationGenerator.cs b/src/ReactiveUI.Binding.SourceGenerators/Invocations/BindTwoWayInvocationGenerator.cs index 8ccbe82..6b80b58 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/Invocations/BindTwoWayInvocationGenerator.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/Invocations/BindTwoWayInvocationGenerator.cs @@ -34,13 +34,11 @@ internal static void Register( data.Left.Left, data.Left.Right, data.Right, - static (sb, group, f) => BindingEmitterHelpers.GenerateDispatchOverload( + static (sb, group, f) => BindingEmitterHelpers.EmitOverloadOrInterceptors( sb, group, BindTwoWayCodeGenerator.DispatchApi, - f.SupportsCallerArgExpr, - f.SupportsNullable, - f.StubHasExpressionParameters), + in f), static (sb, c) => BindTwoWayCodeGenerator.GenerateBindTwoWayMethod( sb, c.Invocation, diff --git a/src/ReactiveUI.Binding.SourceGenerators/Invocations/OneWayBindInvocationGenerator.cs b/src/ReactiveUI.Binding.SourceGenerators/Invocations/OneWayBindInvocationGenerator.cs index 3150cab..72a0b0b 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/Invocations/OneWayBindInvocationGenerator.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/Invocations/OneWayBindInvocationGenerator.cs @@ -34,13 +34,11 @@ internal static void Register( data.Left.Left, data.Left.Right, data.Right, - static (sb, group, f) => BindingEmitterHelpers.GenerateDispatchOverload( + static (sb, group, f) => BindingEmitterHelpers.EmitOverloadOrInterceptors( sb, group, OneWayBindCodeGenerator.DispatchApi, - f.SupportsCallerArgExpr, - f.SupportsNullable, - f.StubHasExpressionParameters), + in f), static (sb, c) => OneWayBindCodeGenerator.GenerateOneWayBindMethod( sb, c.Invocation, diff --git a/src/ReactiveUI.Binding.SourceGenerators/Models/BindingInvocationInfo.cs b/src/ReactiveUI.Binding.SourceGenerators/Models/BindingInvocationInfo.cs index c14ab94..d81825a 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/Models/BindingInvocationInfo.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/Models/BindingInvocationInfo.cs @@ -24,6 +24,9 @@ namespace ReactiveUI.Binding.SourceGenerators.Models; /// The original expression text of the source lambda argument. /// The original expression text of the target lambda argument. /// Whether the invocation uses an explicit IBindingTypeConverter parameter. +/// +/// Where this call site is, for a build that claims call sites outright rather than competing for them. +/// internal sealed record BindingInvocationInfo( string CallerFilePath, int CallerLineNumber, @@ -39,4 +42,5 @@ internal sealed record BindingInvocationInfo( string MethodName, string SourceExpressionText, string TargetExpressionText, - bool HasConverterOverride); + bool HasConverterOverride, + InterceptorLocation Interceptor = default); From ca383cce052d57eb68f3d52969d5d985cc7e26d6 Mon Sep 17 00:00:00 2001 From: Glenn Watson <5834289+glennawatson@users.noreply.github.com> Date: Wed, 9 Sep 2026 22:05:06 +1000 Subject: [PATCH 06/12] feat(generator): intercept the binding, command and interaction call sites - Emit interceptors for BindTo, BindCommand and BindInteraction, completing the tier across every generated API. - Share the parameter writers between each API's dispatch overload and its interceptor, so one description of a signature serves both. --- .../BindCommandCodeGenerator.cs | 76 +++++++++-- .../BindInteractionCodeGenerator.cs | 61 +++++++-- .../CodeGeneration/BindToCodeGenerator.cs | 118 ++++++++++++++---- .../CodeGeneration/BindingEmitterHelpers.cs | 26 +--- .../CodeGeneration/InterceptorEmitter.cs | 54 +++++--- .../WhenAnyObservableCodeGenerator.cs | 63 +++++++++- .../Helpers/BindToExtractor.cs | 3 +- .../Helpers/CommandExtractor.cs | 10 +- .../Helpers/InteractionExtractor.cs | 3 +- .../Helpers/WhenAnyObservableExtractor.cs | 3 +- .../Models/BindCommandInvocationInfo.cs | 6 +- .../Models/BindInteractionInvocationInfo.cs | 6 +- .../Models/BindToInvocationInfo.cs | 6 +- .../Models/WhenAnyObservableInvocationInfo.cs | 6 +- 14 files changed, 340 insertions(+), 101 deletions(-) diff --git a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/BindCommandCodeGenerator.cs b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/BindCommandCodeGenerator.cs index 5d9c5b1..e9ee787 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/BindCommandCodeGenerator.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/BindCommandCodeGenerator.cs @@ -28,6 +28,9 @@ internal static class BindCommandCodeGenerator /// Closes the view parameter of a generated binding worker. private const string ViewParameterSuffix = " view,"; + /// The caller-supplied parameter stream a worker takes on top of the two bound objects. + private const string ObservableParameterArgument = ", withParameter"; + /// Generates concrete typed overloads and binding methods for BindCommand invocations. /// All detected BindCommand invocations. /// All detected class binding info. @@ -382,12 +385,28 @@ private static void AppendOverloadSignature( bool supportsNullable, string dispatchSummaryLine) { - var commandType = CodeGeneratorHelpers.NullableSelectorLeafType(group.Invocations[0].CommandPropertyPath, supportsNullable); - _ = sb.AppendLine(" /// ").Append(" /// Concrete typed overload for BindCommand on ").Append(group.ViewTypeFullName) .AppendLine(".").AppendLine(dispatchSummaryLine).AppendLine(" /// ") - .AppendLine(" public static global::System.IDisposable BindCommand(").Append(" this ").Append(group.ViewTypeFullName) - .AppendLine(ViewParameterSuffix).Append(" ").Append(group.ViewModelTypeFullName).AppendLine(" viewModel,") + .AppendLine(" public static global::System.IDisposable BindCommand("); + + AppendBindCommandParameters(sb, group, supportsNullable, true); + } + + /// Appends the view, view model, selector and event parameters every generated BindCommand member declares. + /// The string builder to append to. + /// The BindCommand type group whose types the parameters are written from. + /// Whether the target supports nullable reference types (C# 8+). + /// Whether the view parameter is the extension receiver. + private static void AppendBindCommandParameters( + StringBuilder sb, + BindCommandTypeGroup group, + bool supportsNullable, + bool isExtensionMethod) + { + var commandType = CodeGeneratorHelpers.NullableSelectorLeafType(group.Invocations[0].CommandPropertyPath, supportsNullable); + + _ = sb.Append(isExtensionMethod ? " this " : CodeGeneratorHelpers.ParameterIndent).Append(group.ViewTypeFullName) + .AppendLine(ViewParameterSuffix).Append(CodeGeneratorHelpers.ParameterIndent).Append(group.ViewModelTypeFullName).AppendLine(" viewModel,") .Append(GeneratedSyntax.SelectorParameterOpen).Append(group.ViewModelTypeFullName).Append(", ") .Append(commandType).AppendLine(">> propertyName,").Append(GeneratedSyntax.SelectorParameterOpen) .Append(group.ViewTypeFullName).Append(", ").Append(group.ControlTypeFullName).AppendLine(">> controlName,"); @@ -409,6 +428,31 @@ private static void AppendOverloadSignature( _ = sb.Append(" string").Append(supportsNullable ? "?" : string.Empty).AppendLine(" toEvent = null,"); } + /// Emits one interceptor per generated binding, claiming every call site that reaches it. + /// The string builder to append to. + /// The group of call sites being claimed. + /// Whether the target supports nullable reference types (C# 8+). + private static void GenerateInterceptors(StringBuilder sb, BindCommandTypeGroup group, bool supportsNullable) + { + var extraArgs = group.HasObservableParameter ? ObservableParameterArgument : string.Empty; + + foreach (var entry in InterceptorEmitter.GroupCallSites(group.Invocations, static x => x.Interceptor, MethodSuffix)) + { + foreach (var callSite in entry.Value) + { + InterceptorEmitter.AppendAttribute(sb, callSite.Interceptor, InterceptorEmitter.MemberIndent); + } + + _ = sb.Append(" internal static global::System.IDisposable __Intercept_BindCommand_").Append(entry.Key).AppendLine("("); + + AppendBindCommandParameters(sb, group, supportsNullable, false); + InterceptorEmitter.CloseParameterList(sb); + + _ = sb.Append(" => ").Append(WorkerMethodPrefix).Append(entry.Key).Append('(').Append(WorkerArguments) + .Append(extraArgs).AppendLine(");").AppendLine(); + } + } + /// Emits the overload and the workers for one group of call sites. /// The string builder to append to. /// The group of call sites that share an overload. @@ -429,12 +473,20 @@ private static void EmitGroup( } : group; - GenerateConcreteOverload( - sb, - collapsed, - features.SupportsCallerArgExpr, - features.SupportsNullable, - features.StubHasExpressionParameters); + if (features.SupportsInterceptors) + { + GenerateInterceptors(sb, collapsed, features.SupportsNullable); + } + else + { + GenerateConcreteOverload( + sb, + collapsed, + features.SupportsCallerArgExpr, + features.SupportsNullable, + features.StubHasExpressionParameters); + } + _ = sb.AppendLine(); for (var i = 0; i < collapsed.Invocations.Length; i++) @@ -455,7 +507,7 @@ private static void EmitGroup( /// The BindCommand type group. private static void EmitExpressionDispatchBranches(StringBuilder sb, BindCommandTypeGroup group) { - var extraArgs = group.HasObservableParameter ? ", withParameter" : string.Empty; + var extraArgs = group.HasObservableParameter ? ObservableParameterArgument : string.Empty; for (var i = 0; i < group.Invocations.Length; i++) { @@ -486,7 +538,7 @@ private static void EmitDispatchFallthrough(StringBuilder sb) => /// The BindCommand type group. private static void EmitFilePathDispatchBranches(StringBuilder sb, BindCommandTypeGroup group) { - var extraArgs = group.HasObservableParameter ? ", withParameter" : string.Empty; + var extraArgs = group.HasObservableParameter ? ObservableParameterArgument : string.Empty; for (var i = 0; i < group.Invocations.Length; i++) { diff --git a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/BindInteractionCodeGenerator.cs b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/BindInteractionCodeGenerator.cs index 566ed64..a9e6a56 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/BindInteractionCodeGenerator.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/BindInteractionCodeGenerator.cs @@ -234,18 +234,55 @@ private static void AppendOverloadSignature( StringBuilder sb, BindInteractionTypeGroup group, string dispatchSummaryLine) + { + _ = sb.AppendLine(" /// ").Append(" /// Concrete typed overload for BindInteraction on ") + .Append(group.ViewTypeFullName).AppendLine(".").AppendLine(dispatchSummaryLine) + .AppendLine(" /// ").AppendLine(" public static global::System.IDisposable BindInteraction("); + + AppendInteractionParameters(sb, group, true); + } + + /// Appends the view, view model, selector and handler parameters every generated BindInteraction member declares. + /// The string builder to append to. + /// The BindInteraction type group whose types the parameters are written from. + /// Whether the view parameter is the extension receiver. + private static void AppendInteractionParameters( + StringBuilder sb, + BindInteractionTypeGroup group, + bool isExtensionMethod) { var handlerType = group.IsTaskHandler ? $"global::System.Func, global::System.Threading.Tasks.Task>" : $"global::System.Func, global::System.IObservable<{group.DontCareTypeFullName}>>"; - _ = sb.AppendLine(" /// ").Append(" /// Concrete typed overload for BindInteraction on ") - .Append(group.ViewTypeFullName).AppendLine(".").AppendLine(dispatchSummaryLine) - .AppendLine(" /// ").AppendLine(" public static global::System.IDisposable BindInteraction(") - .Append(" this ").Append(group.ViewTypeFullName).AppendLine(" view,").Append(" ").Append(group.ViewModelTypeFullName) - .AppendLine(ViewModelParameterSuffix).Append(" ").Append(Expression).Append('<').Append(Func).Append('<').Append(group.ViewModelTypeFullName) - .Append(", ").Append(IInteraction).Append('<').Append(group.InputTypeFullName).Append(", ").Append(group.OutputTypeFullName) - .AppendLine(">>> propertyName,").Append(" ").Append(handlerType).AppendLine(" handler,"); + _ = sb.Append(isExtensionMethod ? " this " : CodeGeneratorHelpers.ParameterIndent).Append(group.ViewTypeFullName) + .AppendLine(ViewParameterSuffix).Append(CodeGeneratorHelpers.ParameterIndent).Append(group.ViewModelTypeFullName) + .AppendLine(ViewModelParameterSuffix).Append(CodeGeneratorHelpers.ParameterIndent).Append(Expression).Append('<').Append(Func).Append('<') + .Append(group.ViewModelTypeFullName).Append(", ").Append(IInteraction).Append('<').Append(group.InputTypeFullName).Append(", ") + .Append(group.OutputTypeFullName).AppendLine(">>> propertyName,").Append(CodeGeneratorHelpers.ParameterIndent).Append(handlerType) + .AppendLine(" handler,"); + } + + /// Emits one interceptor per generated binding, claiming every call site that reaches it. + /// The string builder to append to. + /// The group of call sites being claimed. + private static void GenerateInterceptors(StringBuilder sb, BindInteractionTypeGroup group) + { + foreach (var entry in InterceptorEmitter.GroupCallSites(group.Invocations, static x => x.Interceptor, MethodSuffix)) + { + foreach (var callSite in entry.Value) + { + InterceptorEmitter.AppendAttribute(sb, callSite.Interceptor, InterceptorEmitter.MemberIndent); + } + + _ = sb.Append(" internal static global::System.IDisposable __Intercept_BindInteraction_").Append(entry.Key).AppendLine("("); + + AppendInteractionParameters(sb, group, false); + InterceptorEmitter.CloseParameterList(sb); + + _ = sb.Append(" => ").Append(WorkerMethodPrefix).Append(entry.Key).Append('(').Append(WorkerArguments) + .AppendLine(");").AppendLine(); + } } /// Emits the overload and the workers for one group of call sites. @@ -268,7 +305,15 @@ private static void EmitGroup( } : group; - GenerateConcreteOverload(sb, collapsed, features.SupportsCallerArgExpr, features.StubHasExpressionParameters); + if (features.SupportsInterceptors) + { + GenerateInterceptors(sb, collapsed); + } + else + { + GenerateConcreteOverload(sb, collapsed, features.SupportsCallerArgExpr, features.StubHasExpressionParameters); + } + _ = sb.AppendLine(); for (var i = 0; i < collapsed.Invocations.Length; i++) diff --git a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/BindToCodeGenerator.cs b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/BindToCodeGenerator.cs index e31946a..fbbc071 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/BindToCodeGenerator.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/BindToCodeGenerator.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for full license information. using System.Collections.Immutable; +using System.Runtime.CompilerServices; using System.Text; using ReactiveUI.Binding.SourceGenerators.Models; @@ -23,6 +24,12 @@ internal static class BindToCodeGenerator /// The indentation the converted-assignment subscription body sits at, one block deeper. private const string ConvertedSubscriptionBodyIndent = " "; + /// The indentation and keyword a dispatch branch returns its binding behind. + private const string ReturnStatementPrefix = " return "; + + /// The indentation and arrow an interceptor forwards to its binding behind. + private const string ForwardingBodyPrefix = " => "; + /// Generates concrete typed overloads and binding methods for BindTo invocations. /// All detected BindTo invocations. /// The consumer compilation's C# language-feature snapshot (dispatch strategy and nullable support). @@ -54,7 +61,15 @@ internal static class BindToCodeGenerator } : groups[g]; - GenerateConcreteOverload(sb, group, supportsCallerArgExpr, features.SupportsNullable, features.StubHasExpressionParameters); + if (features.SupportsInterceptors) + { + GenerateInterceptors(sb, group, features.SupportsNullable); + } + else + { + GenerateConcreteOverload(sb, group, supportsCallerArgExpr, features.SupportsNullable, features.StubHasExpressionParameters); + } + _ = sb.AppendLine(); for (var i = 0; i < group.Invocations.Length; i++) @@ -157,14 +172,12 @@ internal static void GenerateConcreteOverload( /// Whether the target supports nullable reference types (C# 8+). internal static void GenerateCallerArgExprOverload(StringBuilder sb, BindToTypeGroup group, bool supportsNullable) { - var targetPropType = CodeGeneratorHelpers.NullableSelectorLeafType(group.Invocations[0].TargetPropertyPath, supportsNullable); _ = sb.AppendLine(" /// ").Append(" /// Concrete typed overload for BindTo of ").Append(IObservable).Append("<") .Append(group.SourceValueTypeFullName).Append("> to ").Append(group.TargetTypeFullName).AppendLine(".") .AppendLine(" /// Uses CallerArgumentExpression for dispatch.").AppendLine(" /// ").Append(" public static ") - .Append(GeneratedTypeNames.IDisposable).AppendLine(" BindTo(").Append(" this ").Append(ObservableOf(group.SourceValueTypeFullName)) - .AppendLine(" source,").Append(" ").Append(group.TargetTypeFullName).AppendLine(" target,").Append(" ") - .Append(PropertyExpression(group.TargetTypeFullName, targetPropType)).AppendLine(" property,"); + .Append(GeneratedTypeNames.IDisposable).AppendLine(" BindTo("); + AppendBindToParameters(sb, group, supportsNullable, true); AppendExtraParameters(sb, group); _ = sb.Append(GeneratedSyntax.ParameterAttributeOpen).Append(CallerArgumentExpression).AppendLine("(\"property\")] string propertyExpression = \"\",") @@ -173,20 +186,18 @@ internal static void GenerateCallerArgExprOverload(StringBuilder sb, BindToTypeG .AppendLine(" propertyExpression = propertyExpression.StartsWith(\"static \", global::System.StringComparison.Ordinal) ? propertyExpression.Substring(7) : propertyExpression;") .AppendLine(); + var extraArguments = FormatExtraArgs(group); + for (var i = 0; i < group.Invocations.Length; i++) { var inv = group.Invocations[i]; - var methodSuffix = CodeGeneratorHelpers.ComputeStableMethodSuffix( - inv.TargetTypeFullName, - inv.CallerFilePath, - inv.CallerLineNumber, - inv.TargetExpressionText); var condition = CodeGeneratorHelpers.ConditionKeyword(i); var escapedTargetExpr = CodeGeneratorHelpers.EscapeString(inv.TargetExpressionText); _ = sb.Append(" ").Append(condition).Append(" (propertyExpression == \"").Append(escapedTargetExpr).AppendLine("\")") - .AppendLine(GeneratedSyntax.StatementBlockOpen).Append(" return __BindTo_").Append(methodSuffix).Append("(source, target") - .Append(FormatExtraArgs(group)).AppendLine(");").AppendLine(" }"); + .AppendLine(GeneratedSyntax.StatementBlockOpen); + AppendWorkerInvocation(sb, ReturnStatementPrefix, BindToMethodSuffix(inv), extraArguments); + _ = sb.AppendLine(" }"); } _ = sb.Append(" throw new ").Append(GeneratedTypeNames.InvalidOperationException).AppendLine("(").Append(" \"") @@ -204,14 +215,12 @@ internal static void GenerateCallerFilePathOverload( bool supportsNullable, bool stubHasExpressionParameters) { - var targetPropType = CodeGeneratorHelpers.NullableSelectorLeafType(group.Invocations[0].TargetPropertyPath, supportsNullable); _ = sb.AppendLine(" /// ").Append(" /// Concrete typed overload for BindTo of ").Append(IObservable).Append("<") .Append(group.SourceValueTypeFullName).Append("> to ").Append(group.TargetTypeFullName).AppendLine(".") .AppendLine(" /// Uses CallerFilePath + CallerLineNumber for dispatch.").AppendLine(" /// ") - .Append(" public static ").Append(GeneratedTypeNames.IDisposable).AppendLine(" BindTo(").Append(" this ") - .Append(ObservableOf(group.SourceValueTypeFullName)).AppendLine(" source,").Append(" ").Append(group.TargetTypeFullName) - .AppendLine(" target,").Append(" ").Append(PropertyExpression(group.TargetTypeFullName, targetPropType)).AppendLine(" property,"); + .Append(" public static ").Append(GeneratedTypeNames.IDisposable).AppendLine(" BindTo("); + AppendBindToParameters(sb, group, supportsNullable, true); AppendExtraParameters(sb, group); if (stubHasExpressionParameters) @@ -222,21 +231,19 @@ internal static void GenerateCallerFilePathOverload( _ = sb.Append(GeneratedSyntax.ParameterAttributeOpen).Append(CallerFilePath).AppendLine("] string callerFilePath = \"\",").Append(GeneratedSyntax.ParameterAttributeOpen) .Append(CallerLineNumber).AppendLine("] int callerLineNumber = 0)").AppendLine(GeneratedSyntax.MemberBodyOpen); + var extraArguments = FormatExtraArgs(group); + for (var i = 0; i < group.Invocations.Length; i++) { var inv = group.Invocations[i]; - var methodSuffix = CodeGeneratorHelpers.ComputeStableMethodSuffix( - inv.TargetTypeFullName, - inv.CallerFilePath, - inv.CallerLineNumber, - inv.TargetExpressionText); var pathSuffix = CodeGeneratorHelpers.ComputePathSuffix(inv.CallerFilePath); var condition = CodeGeneratorHelpers.ConditionKeyword(i); _ = sb.Append(" ").Append(condition).Append(" (callerLineNumber == ").Append(inv.CallerLineNumber).AppendLine() .Append(" && callerFilePath.EndsWith(\"").Append(CodeGeneratorHelpers.EscapeString(pathSuffix)).Append("\", ") - .Append(OrdinalIgnoreCase).AppendLine("))").AppendLine(GeneratedSyntax.StatementBlockOpen).Append(" return __BindTo_").Append(methodSuffix) - .Append("(source, target").Append(FormatExtraArgs(group)).AppendLine(");").AppendLine(" }"); + .Append(OrdinalIgnoreCase).AppendLine("))").AppendLine(GeneratedSyntax.StatementBlockOpen); + AppendWorkerInvocation(sb, ReturnStatementPrefix, BindToMethodSuffix(inv), extraArguments); + _ = sb.AppendLine(" }"); } _ = sb.Append(" throw new ").Append(GeneratedTypeNames.InvalidOperationException).AppendLine("(").Append(" \"") @@ -346,6 +353,71 @@ internal static string FormatExtraMethodParams(BindToInvocationInfo inv) return sb.ToStringAndReturn(); } + /// Emits one interceptor per generated binding, claiming every call site that reaches it. + /// The string builder to append to. + /// The group of call sites being claimed. + /// Whether the target supports nullable reference types (C# 8+). + private static void GenerateInterceptors(StringBuilder sb, BindToTypeGroup group, bool supportsNullable) + { + var extraArguments = FormatExtraArgs(group); + + foreach (var entry in InterceptorEmitter.GroupCallSites(group.Invocations, static x => x.Interceptor, BindToMethodSuffix)) + { + foreach (var callSite in entry.Value) + { + InterceptorEmitter.AppendAttribute(sb, callSite.Interceptor, InterceptorEmitter.MemberIndent); + } + + _ = sb.Append(" internal static ").Append(GeneratedTypeNames.IDisposable).Append(" __Intercept_BindTo_") + .Append(entry.Key).AppendLine("("); + + AppendBindToParameters(sb, group, supportsNullable, false); + AppendExtraParameters(sb, group); + InterceptorEmitter.CloseParameterList(sb); + + AppendWorkerInvocation(sb, ForwardingBodyPrefix, entry.Key, extraArguments); + _ = sb.AppendLine(); + } + } + + /// Appends the observable, target and property parameters every generated BindTo member declares. + /// The string builder to append to. + /// The group whose types the parameters are written from. + /// Whether the target supports nullable reference types (C# 8+). + /// Whether the observable parameter is the extension receiver. + private static void AppendBindToParameters( + StringBuilder sb, + BindToTypeGroup group, + bool supportsNullable, + bool isExtensionMethod) + { + var targetPropType = CodeGeneratorHelpers.NullableSelectorLeafType(group.Invocations[0].TargetPropertyPath, supportsNullable); + + _ = sb.Append(isExtensionMethod ? " this " : " ").Append(ObservableOf(group.SourceValueTypeFullName)) + .AppendLine(" source,").Append(" ").Append(group.TargetTypeFullName).AppendLine(" target,") + .Append(" ").Append(PropertyExpression(group.TargetTypeFullName, targetPropType)).AppendLine(" property,"); + } + + /// Appends the call forwarding the bound stream and target on to the generated worker. + /// The string builder to append to. + /// The indentation and statement keyword the call sits behind. + /// The stable suffix naming the worker. + /// The conversion-hint and converter-override arguments, if any. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static void AppendWorkerInvocation(StringBuilder sb, string prefix, string methodSuffix, string extraArguments) => + sb.Append(prefix).Append("__BindTo_").Append(methodSuffix).Append("(source, target").Append(extraArguments).AppendLine(");"); + + /// Names the generated binding a call site reaches. + /// The call site. + /// The stable method-name suffix. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static string BindToMethodSuffix(BindToInvocationInfo inv) => + CodeGeneratorHelpers.ComputeStableMethodSuffix( + inv.TargetTypeFullName, + inv.CallerFilePath, + inv.CallerLineNumber, + inv.TargetExpressionText); + /// Formats the conversion-hint and converter-override arguments handed to the runtime converter. /// The invocation info. /// The two arguments, comma separated. diff --git a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/BindingEmitterHelpers.cs b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/BindingEmitterHelpers.cs index 3aceaca..ff7fda5 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/BindingEmitterHelpers.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/BindingEmitterHelpers.cs @@ -718,29 +718,9 @@ internal static BindingObservables EmitDualStreamStages(StringBuilder sb, Bindin /// A call site the compiler declined to describe is left out: nothing can claim it, and it keeps whatever /// the call already resolved to. /// - private static Dictionary> GroupCallSitesByWorker(BindingTypeGroup group) - { - var claimed = new Dictionary>(StringComparer.Ordinal); - for (var i = 0; i < group.Invocations.Length; i++) - { - var inv = group.Invocations[i]; - if (!inv.Interceptor.IsAvailable) - { - continue; - } - - var suffix = BindingMethodSuffix(inv); - if (!claimed.TryGetValue(suffix, out var callSites)) - { - callSites = []; - claimed[suffix] = callSites; - } - - callSites.Add(inv); - } - - return claimed; - } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static Dictionary> GroupCallSitesByWorker(BindingTypeGroup group) => + InterceptorEmitter.GroupCallSites(group.Invocations, static x => x.Interceptor, BindingMethodSuffix); /// Finds the type a view declares its view model property as. /// The view type's binding info. diff --git a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/InterceptorEmitter.cs b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/InterceptorEmitter.cs index 9c58274..5bec408 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/InterceptorEmitter.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/InterceptorEmitter.cs @@ -2,6 +2,7 @@ // ReactiveUI Association Incorporated licenses this file to you under the MIT license. // See the LICENSE file in the project root for full license information. +using System.Runtime.CompilerServices; using System.Text; using ReactiveUI.Binding.SourceGenerators.Models; @@ -19,6 +20,9 @@ namespace ReactiveUI.Binding.SourceGenerators.CodeGeneration; /// internal static class InterceptorEmitter { + /// The indentation a member of the generated class is written at. + internal const string MemberIndent = " "; + /// The attribute a generated method carries to claim a call site. private const string AttributeName = "global::System.Runtime.CompilerServices.InterceptsLocation"; @@ -95,7 +99,7 @@ internal static void GenerateInterceptors( foreach (var callSite in entry.Value) { - AppendAttribute(builder, callSite.Interceptor, " "); + AppendAttribute(builder, callSite.Interceptor, MemberIndent); } _ = builder.Append(" internal static global::System.IObservable<").Append(first.ReturnTypeFullName) @@ -159,37 +163,55 @@ internal static void CloseParameterList(StringBuilder builder) } } - /// Gathers the call sites of a group under the body each of them reaches. - /// The type group whose call sites are being gathered. - /// Names the body a call site reaches. - /// Each generated body, against every call site that resolves to it. + /// Gathers call sites under the generated method each of them reaches. + /// The per-call-site model this API extracts. + /// The call sites of one group. + /// Reads where a call site is. + /// Names the generated method a call site reaches. + /// Each generated method, against every call site that resolves to it. /// - /// A call site the compiler declined to describe is left out: nothing can claim it, and it keeps whatever - /// the call already resolved to. + /// Every API claims its call sites the same way, so the gathering is written once over whatever model the + /// API happens to carry. A call site the compiler declined to describe is left out: nothing can claim it, + /// and it keeps whatever the call already resolved to. /// - private static Dictionary> GroupCallSitesByBody( - ObservationCodeGenerator.TypeGroup group, - Func suffixOf) + internal static Dictionary> GroupCallSites( + IReadOnlyList invocations, + Func locationOf, + Func suffixOf) { - var claimed = new Dictionary>(StringComparer.Ordinal); - for (var i = 0; i < group.Invocations.Length; i++) + var claimed = new Dictionary>(StringComparer.Ordinal); + for (var i = 0; i < invocations.Count; i++) { - var inv = group.Invocations[i]; - if (!inv.Interceptor.IsAvailable) + var invocation = invocations[i]; + if (!locationOf(invocation).IsAvailable) { continue; } - var suffix = suffixOf(inv); + var suffix = suffixOf(invocation); if (!claimed.TryGetValue(suffix, out var callSites)) { callSites = []; claimed[suffix] = callSites; } - callSites.Add(inv); + callSites.Add(invocation); } return claimed; } + + /// Gathers the call sites of a group under the body each of them reaches. + /// The type group whose call sites are being gathered. + /// Names the body a call site reaches. + /// Each generated body, against every call site that resolves to it. + /// + /// A call site the compiler declined to describe is left out: nothing can claim it, and it keeps whatever + /// the call already resolved to. + /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static Dictionary> GroupCallSitesByBody( + ObservationCodeGenerator.TypeGroup group, + Func suffixOf) => + GroupCallSites(group.Invocations, static x => x.Interceptor, suffixOf); } diff --git a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/WhenAnyObservableCodeGenerator.cs b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/WhenAnyObservableCodeGenerator.cs index d8c1cbe..edd7771 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/WhenAnyObservableCodeGenerator.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/WhenAnyObservableCodeGenerator.cs @@ -335,12 +335,20 @@ private static void EmitGroup( ImmutableArray allClasses, in LanguageFeatures features) { - GenerateConcreteOverload( - sb, - group, - features.SupportsCallerArgExpr, - features.SupportsNullable, - features.StubHasExpressionParameters); + if (features.SupportsInterceptors) + { + GenerateInterceptors(sb, group, features.SupportsNullable); + } + else + { + GenerateConcreteOverload( + sb, + group, + features.SupportsCallerArgExpr, + features.SupportsNullable, + features.StubHasExpressionParameters); + } + _ = sb.AppendLine(); for (var i = 0; i < group.Invocations.Length; i++) @@ -354,6 +362,49 @@ private static void EmitGroup( } } + /// Emits one interceptor per generated observation, claiming every call site that reaches it. + /// The string builder to append to. + /// The group of call sites being claimed. + /// Whether the target supports nullable reference types (C# 8+). + private static void GenerateInterceptors(StringBuilder sb, TypeGroup group, bool supportsNullable) + { + foreach (var entry in InterceptorEmitter.GroupCallSites( + group.Invocations, + static x => x.Interceptor, + ObservationMethodSuffix)) + { + var first = entry.Value[0]; + var propCount = first.PropertyPaths.Length; + + foreach (var callSite in entry.Value) + { + InterceptorEmitter.AppendAttribute(sb, callSite.Interceptor, " "); + } + + _ = sb.Append(" internal static global::System.IObservable<").Append(first.ReturnTypeFullName) + .Append("> __Intercept_WhenAnyObservable_").Append(entry.Key).AppendLine("(") + .Append(" ").Append(first.SourceTypeFullName).AppendLine(" objectToMonitor,"); + + for (var i = 0; i < propCount; i++) + { + var innerType = first.InnerObservableTypeFullNames[i]; + var obsType = $"global::System.IObservable<{innerType}>{(supportsNullable ? "?" : string.Empty)}"; + _ = sb.Append(" global::System.Linq.Expressions.Expression> obs").Append(i + 1).AppendLine(","); + } + + if (first.HasSelector) + { + _ = sb.Append(" ").Append(GetSelectorType(first)).AppendLine(" selector,"); + } + + InterceptorEmitter.CloseParameterList(sb); + + _ = sb.Append(" => __WhenAnyObservable_").Append(entry.Key).Append("(objectToMonitor") + .Append(first.HasSelector ? ", selector" : string.Empty).AppendLine(");").AppendLine(); + } + } + /// Emits the if/else-if dispatch table that routes each matched invocation to its generated method. /// The string builder to append to. /// The type group containing invocations that share a signature. diff --git a/src/ReactiveUI.Binding.SourceGenerators/Helpers/BindToExtractor.cs b/src/ReactiveUI.Binding.SourceGenerators/Helpers/BindToExtractor.cs index 455744c..6974d9d 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/Helpers/BindToExtractor.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/Helpers/BindToExtractor.cs @@ -84,7 +84,8 @@ internal static class BindToExtractor targetPropertyTypeFullName, hasConversionHint, hasConverterOverride, - targetExpressionText); + targetExpressionText, + InterceptableLocationReader.Read(semanticModel, invocation, ct)); } /// diff --git a/src/ReactiveUI.Binding.SourceGenerators/Helpers/CommandExtractor.cs b/src/ReactiveUI.Binding.SourceGenerators/Helpers/CommandExtractor.cs index 8530ffe..1156f68 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/Helpers/CommandExtractor.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/Helpers/CommandExtractor.cs @@ -66,9 +66,6 @@ is not var (viewTypeFullName, viewModelTypeFullName)) return null; } - var commandTypeFullName = commandPropertyPath[^1].PropertyTypeFullName; - var controlTypeFullName = controlPropertyPath[^1].PropertyTypeFullName; - // Determine parameter overload (Expression vs IObservable withParameter) var parameterOverload = DetectParameterOverload(methodSymbol, args, semanticModel, ct); @@ -82,8 +79,8 @@ is not var (viewTypeFullName, viewModelTypeFullName)) viewModelTypeFullName, new(commandPropertyPath), new(controlPropertyPath), - commandTypeFullName, - controlTypeFullName, + commandPropertyPath[^1].PropertyTypeFullName, + controlPropertyPath[^1].PropertyTypeFullName, parameterOverload.HasObservableParameter, parameterOverload.HasExpressionParameter, parameterOverload.ParameterTypeFullName, @@ -97,7 +94,8 @@ is not var (viewTypeFullName, viewModelTypeFullName)) parameterOverload.ParameterExpressionText, capabilities.HasCommand, capabilities.HasCommandParameter, - capabilities.HasEnabled); + capabilities.HasEnabled, + InterceptableLocationReader.Read(semanticModel, invocation, ct)); } /// Searches invocation arguments for a valid withParameter lambda expression. diff --git a/src/ReactiveUI.Binding.SourceGenerators/Helpers/InteractionExtractor.cs b/src/ReactiveUI.Binding.SourceGenerators/Helpers/InteractionExtractor.cs index f14f236..8cb2847 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/Helpers/InteractionExtractor.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/Helpers/InteractionExtractor.cs @@ -87,7 +87,8 @@ internal static class InteractionExtractor dontCareTypeFullName, Constants.BindInteractionMethodName, expressionText, - viewClassInfo); + viewClassInfo, + InterceptableLocationReader.Read(semanticModel, invocation, ct)); } /// Resolves the interaction's two type arguments, refusing a call site that names neither. diff --git a/src/ReactiveUI.Binding.SourceGenerators/Helpers/WhenAnyObservableExtractor.cs b/src/ReactiveUI.Binding.SourceGenerators/Helpers/WhenAnyObservableExtractor.cs index c891de4..0f4e314 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/Helpers/WhenAnyObservableExtractor.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/Helpers/WhenAnyObservableExtractor.cs @@ -76,7 +76,8 @@ internal static class WhenAnyObservableExtractor new([.. innerObservableTypes]), returnTypeFullName, hasSelector, - new([.. expressionTexts])); + new([.. expressionTexts]), + InterceptableLocationReader.Read(semanticModel, invocation, ct)); } /// diff --git a/src/ReactiveUI.Binding.SourceGenerators/Models/BindCommandInvocationInfo.cs b/src/ReactiveUI.Binding.SourceGenerators/Models/BindCommandInvocationInfo.cs index 8d19d73..4a4858e 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/Models/BindCommandInvocationInfo.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/Models/BindCommandInvocationInfo.cs @@ -31,6 +31,9 @@ namespace ReactiveUI.Binding.SourceGenerators.Models; /// Whether the control type has a settable Command property (ICommand). /// Whether the control type has a settable CommandParameter property. /// Whether the control type has a settable Enabled property (bool). +/// +/// Where this call site is, for a build that claims call sites outright rather than competing for them. +/// internal sealed record BindCommandInvocationInfo( string CallerFilePath, int CallerLineNumber, @@ -53,4 +56,5 @@ internal sealed record BindCommandInvocationInfo( string? ParameterExpressionText, bool HasCommandProperty, bool HasCommandParameterProperty, - bool HasEnabledProperty); + bool HasEnabledProperty, + InterceptorLocation Interceptor = default); diff --git a/src/ReactiveUI.Binding.SourceGenerators/Models/BindInteractionInvocationInfo.cs b/src/ReactiveUI.Binding.SourceGenerators/Models/BindInteractionInvocationInfo.cs index 55b46ba..4eb7e83 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/Models/BindInteractionInvocationInfo.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/Models/BindInteractionInvocationInfo.cs @@ -26,6 +26,9 @@ namespace ReactiveUI.Binding.SourceGenerators.Models; /// a referenced assembly is still followed through the view model it holds rather than reduced to the instance /// the call was handed. /// +/// +/// Where this call site is, for a build that claims call sites outright rather than competing for them. +/// internal sealed record BindInteractionInvocationInfo( string CallerFilePath, int CallerLineNumber, @@ -38,4 +41,5 @@ internal sealed record BindInteractionInvocationInfo( string? DontCareTypeFullName, string MethodName, string ExpressionText, - ClassBindingInfo? ViewClassInfo = null); + ClassBindingInfo? ViewClassInfo = null, + InterceptorLocation Interceptor = default); diff --git a/src/ReactiveUI.Binding.SourceGenerators/Models/BindToInvocationInfo.cs b/src/ReactiveUI.Binding.SourceGenerators/Models/BindToInvocationInfo.cs index a1e7bbe..ddcbe1a 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/Models/BindToInvocationInfo.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/Models/BindToInvocationInfo.cs @@ -18,6 +18,9 @@ namespace ReactiveUI.Binding.SourceGenerators.Models; /// Whether the invocation supplies a conversionHint argument. /// Whether the invocation supplies an explicit IBindingTypeConverter argument. /// The original expression text of the target lambda argument. +/// +/// Where this call site is, for a build that claims call sites outright rather than competing for them. +/// internal sealed record BindToInvocationInfo( string CallerFilePath, int CallerLineNumber, @@ -27,4 +30,5 @@ internal sealed record BindToInvocationInfo( string TargetPropertyTypeFullName, bool HasConversionHint, bool HasConverterOverride, - string TargetExpressionText); + string TargetExpressionText, + InterceptorLocation Interceptor = default); diff --git a/src/ReactiveUI.Binding.SourceGenerators/Models/WhenAnyObservableInvocationInfo.cs b/src/ReactiveUI.Binding.SourceGenerators/Models/WhenAnyObservableInvocationInfo.cs index ac45fcc..c217aa7 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/Models/WhenAnyObservableInvocationInfo.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/Models/WhenAnyObservableInvocationInfo.cs @@ -17,6 +17,9 @@ namespace ReactiveUI.Binding.SourceGenerators.Models; /// The fully qualified return type of the observation. /// Whether the invocation includes a selector/projection function (CombineLatest variant). /// The original expression text of each lambda argument, used for CallerArgumentExpression dispatch. +/// +/// Where this call site is, for a build that claims call sites outright rather than competing for them. +/// internal sealed record WhenAnyObservableInvocationInfo( string CallerFilePath, int CallerLineNumber, @@ -25,4 +28,5 @@ internal sealed record WhenAnyObservableInvocationInfo( EquatableArray InnerObservableTypeFullNames, string ReturnTypeFullName, bool HasSelector, - EquatableArray ExpressionTexts); + EquatableArray ExpressionTexts, + InterceptorLocation Interceptor = default); From 9282a2c10a12c94da7953583f7457614743b8ae7 Mon Sep 17 00:00:00 2001 From: Glenn Watson <5834289+glennawatson@users.noreply.github.com> Date: Wed, 9 Sep 2026 22:59:36 +1000 Subject: [PATCH 07/12] feat(generator): claim binding call sites through interceptors - Give every interceptor the signature of the stub it replaces, including the receiver and the caller-info parameters, which is what the compiler requires of one. - Emit each API's parameter list from one writer serving both its dispatch overload and its interceptor. - Ship the generator and analyzer in a slot per compiler generation, with targets that keep a build handed both from loading both and that list the generated namespace where interception can be honoured. - Stay silent on the reach of a dispatch overload where the call site is claimed outright instead. - Stop reporting a stream's value type as unobservable behind BindTo, whose first type argument names no object anything observes. - Run the generator and analyzer suites against both compiler builds. - Replace the substituted symbols with ones a compilation produces: a function pointer's signature belongs to no type, and an extension block declares its members in a type with no name. --- CLAUDE.md | 11 +- src/Directory.Packages.props | 1 - ...activeUI.Binding.Analyzer.Roslyn413.csproj | 51 ++++ .../Analyzers/BindingInvocationAnalyzer.cs | 13 +- .../Analyzers/DispatchReachAnalyzer.cs | 18 +- .../Analyzers/MixinShadowAnalyzer.cs | 14 +- .../Analyzers/TypeAnalyzer.cs | 15 +- .../ReactiveUI.Binding.Analyzer.csproj | 7 +- .../ReactiveUI.Binding.Reactive.csproj | 16 +- ....Binding.SourceGenerators.Roslyn413.csproj | 1 + src/ReactiveUI.Binding.SourceGenerators.slnx | 5 + .../BindCommandCodeGenerator.cs | 90 +++--- .../BindInteractionCodeGenerator.cs | 67 ++--- .../CodeGeneration/BindToCodeGenerator.cs | 57 ++-- .../CodeGeneration/BindingEmitterHelpers.cs | 109 ++++---- .../CodeGeneration/CodeGeneratorHelpers.cs | 37 +-- .../CodeGeneration/InterceptorEmitter.cs | 61 +---- .../ObservationCodeGenerator.cs | 66 +++-- .../CodeGeneration/WhenAnyCodeGenerator.cs | 93 ++++--- .../WhenAnyObservableCodeGenerator.cs | 115 ++++---- .../Helpers/BindToExtractor.cs | 17 +- ...ReactiveUI.Binding.SourceGenerators.csproj | 4 + ...eactiveUI.Binding.SourceGenerators.targets | 88 ++++++ .../ReactiveUI.Binding.csproj | 16 +- ...UI.Binding.Analyzer.Tests.Roslyn413.csproj | 34 +++ .../Helpers/AnalyzerHelpersTests.cs | 12 +- .../ReactiveUI.Binding.Analyzer.Tests.csproj | 2 +- .../TypeAnalyzerTests.cs | 48 ++++ ...ng.SourceGenerators.Tests.Roslyn413.csproj | 49 ++++ .../Helpers/BindToExtractorTests.cs | 39 +-- .../Helpers/ExtractorValidationTests.cs | 127 +++++---- .../InterceptableLocationReaderTests.cs | 120 ++++++++ .../Helpers/TestHelper.cs | 58 +++- .../InterceptedCallSiteTests.cs | 256 ++++++++++++++++++ ...veUI.Binding.SourceGenerators.Tests.csproj | 2 +- src/tests/Shared/RoslynSymbolProbe.cs | 91 +++++++ 36 files changed, 1322 insertions(+), 488 deletions(-) create mode 100644 src/ReactiveUI.Binding.Analyzer.Roslyn413/ReactiveUI.Binding.Analyzer.Roslyn413.csproj create mode 100644 src/ReactiveUI.Binding.SourceGenerators/build/ReactiveUI.Binding.SourceGenerators.targets create mode 100644 src/tests/ReactiveUI.Binding.Analyzer.Tests.Roslyn413/ReactiveUI.Binding.Analyzer.Tests.Roslyn413.csproj create mode 100644 src/tests/ReactiveUI.Binding.SourceGenerators.Tests.Roslyn413/ReactiveUI.Binding.SourceGenerators.Tests.Roslyn413.csproj create mode 100644 src/tests/ReactiveUI.Binding.SourceGenerators.Tests/Helpers/InterceptableLocationReaderTests.cs create mode 100644 src/tests/ReactiveUI.Binding.SourceGenerators.Tests/InterceptedCallSiteTests.cs create mode 100644 src/tests/Shared/RoslynSymbolProbe.cs diff --git a/CLAUDE.md b/CLAUDE.md index b66c8cc..a6c50b9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -223,6 +223,9 @@ src/ │ ├── PooledStringBuilder.cs # char[]-backed builder for generated fragments │ └── RuntimeFlavourRewriter.cs # Retargets output onto the .Reactive package │ +├── ReactiveUI.Binding.SourceGenerators.Roslyn413/ # The same generator source against Roslyn 4.13 +├── ReactiveUI.Binding.Analyzer.Roslyn413/ # The same analyzer source against Roslyn 4.13 +│ ├── ReactiveUI.Binding.Analyzer/ # Roslyn analyzer (netstandard2.0) │ └── Analyzers/ │ ├── BindingInvocationAnalyzer.cs # RXUIBIND001, 003, 004, 005, 006, 007, 008 @@ -744,9 +747,9 @@ build keeps working right up until Wine starts. Each copy chains to the reposito - **Runtime library targets:** net8.0;net9.0;net10.0;net462;net472;net481 - **No shallow clones:** Repository requires full clone for Nerdbank.GitVersioning - **Where the analyzers ship:** `ReactiveUI.Binding` and `ReactiveUI.Binding.Reactive` each pack the generator - and analyzer DLLs into `analyzers/dotnet/cs`, so referencing a runtime package is all a consumer needs. - `ReactiveUI.Binding.SourceGenerators` is a compatibility package that ships only the MSBuild props: a second - copy of the same assemblies under a different package root loads as a second generator and emits every - dispatch file twice, which fails the consumer's build + and analyzer DLLs into `analyzers/dotnet/roslyn4.8/cs` and `analyzers/dotnet/roslyn4.13/cs`, so referencing a + runtime package is all a consumer needs. `ReactiveUI.Binding.SourceGenerators` is a compatibility package that + ships only the MSBuild props and targets: a second copy of the same assemblies under a different package root + loads as a second generator and emits every dispatch file twice, which fails the consumer's build **Philosophy:** Generate zero-reflection, AOT-compatible property observation and binding code at compile-time. Support all ReactiveUI platform notification mechanisms. Fall back to runtime expression analysis only when compile-time analysis is not possible. diff --git a/src/Directory.Packages.props b/src/Directory.Packages.props index d25c95c..818b961 100644 --- a/src/Directory.Packages.props +++ b/src/Directory.Packages.props @@ -18,7 +18,6 @@ - diff --git a/src/ReactiveUI.Binding.Analyzer.Roslyn413/ReactiveUI.Binding.Analyzer.Roslyn413.csproj b/src/ReactiveUI.Binding.Analyzer.Roslyn413/ReactiveUI.Binding.Analyzer.Roslyn413.csproj new file mode 100644 index 0000000..4396012 --- /dev/null +++ b/src/ReactiveUI.Binding.Analyzer.Roslyn413/ReactiveUI.Binding.Analyzer.Roslyn413.csproj @@ -0,0 +1,51 @@ + + + netstandard2.0 + ReactiveUI.Binding.Analyzer + ReactiveUI.Binding.Analyzer + false + true + true + false + + $(NoWarn);RS1038 + + + + + $(DefineConstants);ROSLYN_4_13 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/ReactiveUI.Binding.Analyzer/Analyzers/BindingInvocationAnalyzer.cs b/src/ReactiveUI.Binding.Analyzer/Analyzers/BindingInvocationAnalyzer.cs index 36a4a8f..d90cd16 100644 --- a/src/ReactiveUI.Binding.Analyzer/Analyzers/BindingInvocationAnalyzer.cs +++ b/src/ReactiveUI.Binding.Analyzer/Analyzers/BindingInvocationAnalyzer.cs @@ -24,9 +24,10 @@ public class BindingInvocationAnalyzer : DiagnosticAnalyzer /// The parameter type a property path arrives as, which marks it out from the other arguments. private const string ExpressionParameterTypePrefix = "System.Linq.Expressions.Expression<"; - /// - public override ImmutableArray SupportedDiagnostics => - ImmutableArray.Create( + /// The diagnostics this analyzer reports. + private static readonly ImmutableArray ReportedDiagnostics = + new[] + { DiagnosticWarnings.NonInlineLambda, DiagnosticWarnings.PrivateMember, DiagnosticWarnings.NoBeforeChangeSupport, @@ -34,7 +35,11 @@ public class BindingInvocationAnalyzer : DiagnosticAnalyzer DiagnosticWarnings.UnsupportedPathSegment, DiagnosticWarnings.NoBindableEvent, DiagnosticWarnings.InvalidInteractionType, - DiagnosticWarnings.SilentPathLink); + DiagnosticWarnings.SilentPathLink, + }.ToImmutableArray(); + + /// + public override ImmutableArray SupportedDiagnostics => ReportedDiagnostics; /// public override void Initialize(AnalysisContext context) diff --git a/src/ReactiveUI.Binding.Analyzer/Analyzers/DispatchReachAnalyzer.cs b/src/ReactiveUI.Binding.Analyzer/Analyzers/DispatchReachAnalyzer.cs index 7680dea..2e1dcbf 100644 --- a/src/ReactiveUI.Binding.Analyzer/Analyzers/DispatchReachAnalyzer.cs +++ b/src/ReactiveUI.Binding.Analyzer/Analyzers/DispatchReachAnalyzer.cs @@ -9,6 +9,7 @@ using Microsoft.CodeAnalysis.Operations; using ReactiveUI.Binding.Helpers; using ReactiveUI.Binding.SourceGenerators; +using ReactiveUI.Binding.SourceGenerators.Helpers; namespace ReactiveUI.Binding.Analyzer.Analyzers; @@ -29,9 +30,12 @@ public class DispatchReachAnalyzer : DiagnosticAnalyzer /// The analyzer config key a build exposes the root namespace under. private const string RootNamespaceKey = "build_property.RootNamespace"; + /// The diagnostics this analyzer reports. + private static readonly ImmutableArray ReportedDiagnostics = + new[] { DiagnosticWarnings.DispatchOutOfReach }.ToImmutableArray(); + /// - public override ImmutableArray SupportedDiagnostics => - ImmutableArray.Create(DiagnosticWarnings.DispatchOutOfReach); + public override ImmutableArray SupportedDiagnostics => ReportedDiagnostics; /// public override void Initialize(AnalysisContext context) @@ -68,7 +72,15 @@ internal static void AnalyzeInvocation(in OperationAnalysisContext context, stri // Read the language version from the tree rather than the compilation: it is a parse option, so a // compilation can hold trees that differ, and the reach of a generated overload follows the file. - if (invocation.Syntax.SyntaxTree.Options is not CSharpParseOptions { LanguageVersion: < LanguageVersion.CSharp10 }) + if (invocation.Syntax.SyntaxTree.Options is not CSharpParseOptions { LanguageVersion: < LanguageVersion.CSharp10 } parseOptions) + { + return; + } + + // An interceptor replaces the call the compiler already bound, so nothing about it goes through + // extension-method lookup and no namespace has to be in reach. Where this build emits interceptors + // instead of overloads, the file's namespace stops deciding anything. + if (InterceptableLocationReader.IsSupported && InterceptableLocationReader.IsOptedIn(parseOptions)) { return; } diff --git a/src/ReactiveUI.Binding.Analyzer/Analyzers/MixinShadowAnalyzer.cs b/src/ReactiveUI.Binding.Analyzer/Analyzers/MixinShadowAnalyzer.cs index 12eb265..64b1a91 100644 --- a/src/ReactiveUI.Binding.Analyzer/Analyzers/MixinShadowAnalyzer.cs +++ b/src/ReactiveUI.Binding.Analyzer/Analyzers/MixinShadowAnalyzer.cs @@ -38,8 +38,8 @@ public class MixinShadowAnalyzer : DiagnosticAnalyzer $"{Constants.ReactiveRuntimeNamespace}.{Constants.StubExtensionClassName}"; /// The API names this package generates bindings for. - private static readonly ImmutableHashSet GeneratedApiNames = ImmutableHashSet.Create( - StringComparer.Ordinal, + private static readonly ImmutableHashSet GeneratedApiNames = new[] + { Constants.WhenChangedMethodName, Constants.WhenChangingMethodName, Constants.WhenAnyMethodName, @@ -51,11 +51,15 @@ public class MixinShadowAnalyzer : DiagnosticAnalyzer Constants.BindMethodName, Constants.BindToMethodName, Constants.BindCommandMethodName, - Constants.BindInteractionMethodName); + Constants.BindInteractionMethodName, + }.ToImmutableHashSet(StringComparer.Ordinal); + + /// The diagnostics this analyzer reports. + private static readonly ImmutableArray ReportedDiagnostics = + new[] { DiagnosticWarnings.MixinShadowsGeneratedBinding }.ToImmutableArray(); /// - public override ImmutableArray SupportedDiagnostics => - ImmutableArray.Create(DiagnosticWarnings.MixinShadowsGeneratedBinding); + public override ImmutableArray SupportedDiagnostics => ReportedDiagnostics; /// public override void Initialize(AnalysisContext context) diff --git a/src/ReactiveUI.Binding.Analyzer/Analyzers/TypeAnalyzer.cs b/src/ReactiveUI.Binding.Analyzer/Analyzers/TypeAnalyzer.cs index 51c960f..61ec575 100644 --- a/src/ReactiveUI.Binding.Analyzer/Analyzers/TypeAnalyzer.cs +++ b/src/ReactiveUI.Binding.Analyzer/Analyzers/TypeAnalyzer.cs @@ -15,9 +15,12 @@ namespace ReactiveUI.Binding.Analyzer.Analyzers; [DiagnosticAnalyzer(LanguageNames.CSharp)] public class TypeAnalyzer : DiagnosticAnalyzer { + /// The diagnostics this analyzer reports. + private static readonly ImmutableArray ReportedDiagnostics = + new[] { DiagnosticWarnings.NoObservableProperties }.ToImmutableArray(); + /// - public override ImmutableArray SupportedDiagnostics => - ImmutableArray.Create(DiagnosticWarnings.NoObservableProperties); + public override ImmutableArray SupportedDiagnostics => ReportedDiagnostics; /// public override void Initialize(AnalysisContext context) @@ -43,6 +46,14 @@ internal static void AnalyzeInvocation(in OperationAnalysisContext context) return; } + // The check reads the first type argument, which every other API names the observed object with. + // BindTo names the value type of a stream the caller already built, and nothing about that type is + // ever observed, so asking whether it notifies has no answer worth reporting. + if (methodSymbol.Name == Constants.BindToMethodName) + { + return; + } + // Check if the source type lacks any observable mechanism if (!AnalyzerHelpers.LacksObservableMechanism(methodSymbol, context.Compilation, out var sourceType)) { diff --git a/src/ReactiveUI.Binding.Analyzer/ReactiveUI.Binding.Analyzer.csproj b/src/ReactiveUI.Binding.Analyzer/ReactiveUI.Binding.Analyzer.csproj index ea5819c..1f909b5 100644 --- a/src/ReactiveUI.Binding.Analyzer/ReactiveUI.Binding.Analyzer.csproj +++ b/src/ReactiveUI.Binding.Analyzer/ReactiveUI.Binding.Analyzer.csproj @@ -11,6 +11,7 @@ + + + + diff --git a/src/ReactiveUI.Binding.Reactive/ReactiveUI.Binding.Reactive.csproj b/src/ReactiveUI.Binding.Reactive/ReactiveUI.Binding.Reactive.csproj index 9462687..a888ac1 100644 --- a/src/ReactiveUI.Binding.Reactive/ReactiveUI.Binding.Reactive.csproj +++ b/src/ReactiveUI.Binding.Reactive/ReactiveUI.Binding.Reactive.csproj @@ -53,15 +53,25 @@ generator is not run over this project's own source. --> + + + six times over. + + One slot per compiler generation, because what the generator can do differs between them: the 4.13 + build claims each call site with an interceptor, the 4.8 build offers an overload that has to win + extension-method lookup. The accompanying .targets is what keeps a consumer whose build does not + narrow the slots itself from loading both. --> - - + + + + + + true + + + <_ReactiveUIBindingInterceptorNamespace>ReactiveUI.Binding.Generated.Interceptors + + + <_ReactiveUIBindingMinimumCompilerVersion>4.8 + <_ReactiveUIBindingInterceptorCompilerVersion>4.13 + + + + + + <_ReactiveUIBindingCompilerVersion Condition="'$(CompilerApiVersion)' != '' and $(CompilerApiVersion.StartsWith('roslyn'))">$(CompilerApiVersion.Substring(6)) + <_ReactiveUIBindingCompilerVersion Condition="!$(_ReactiveUIBindingCompilerVersion.Contains('.'))">$(_ReactiveUIBindingMinimumCompilerVersion) + + <_ReactiveUIBindingInterceptsCallSites>false + <_ReactiveUIBindingInterceptsCallSites Condition="'$([System.Version]::Parse($(_ReactiveUIBindingCompilerVersion)).CompareTo($([System.Version]::Parse($(_ReactiveUIBindingInterceptorCompilerVersion)))))' >= '0'">true + + <_ReactiveUIBindingSlot Condition="'$(_ReactiveUIBindingInterceptsCallSites)' == 'true'">roslyn$(_ReactiveUIBindingInterceptorCompilerVersion) + <_ReactiveUIBindingSlot Condition="'$(_ReactiveUIBindingSlot)' == ''">roslyn$(_ReactiveUIBindingMinimumCompilerVersion) + + + + + <_ReactiveUIBindingAnalyzer Include="@(Analyzer)" + Condition="'%(Analyzer.Filename)' == 'ReactiveUI.Binding.SourceGenerators' or '%(Analyzer.Filename)' == 'ReactiveUI.Binding.Analyzer'"> + <_Slot>$([System.IO.Path]::GetFileName($([System.IO.Path]::GetDirectoryName($([System.IO.Path]::GetDirectoryName('%(Analyzer.FullPath)')))))) + + + + + + + + <_ReactiveUIBindingInterceptorsNamespaces>$(InterceptorsNamespaces) + <_ReactiveUIBindingInterceptorsNamespaces Condition="'$(_ReactiveUIBindingInterceptorsNamespaces)' != ''">$(_ReactiveUIBindingInterceptorsNamespaces); + <_ReactiveUIBindingInterceptorsNamespaces>$(_ReactiveUIBindingInterceptorsNamespaces)$(_ReactiveUIBindingInterceptorNamespace) + $(Features);InterceptorsNamespaces=$(_ReactiveUIBindingInterceptorsNamespaces) + + + + + + + diff --git a/src/ReactiveUI.Binding/ReactiveUI.Binding.csproj b/src/ReactiveUI.Binding/ReactiveUI.Binding.csproj index 1e3a0ea..c5069be 100644 --- a/src/ReactiveUI.Binding/ReactiveUI.Binding.csproj +++ b/src/ReactiveUI.Binding/ReactiveUI.Binding.csproj @@ -56,15 +56,25 @@ generator is not run over this project's own source. --> + + + six times over. + + One slot per compiler generation, because what the generator can do differs between them: the 4.13 + build claims each call site with an interceptor, the 4.8 build offers an overload that has to win + extension-method lookup. The accompanying .targets is what keeps a consumer whose build does not + narrow the slots itself from loading both. --> - - + + + + + + net8.0;net9.0;net10.0;net11.0 + false + enable + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/tests/ReactiveUI.Binding.Analyzer.Tests/Helpers/AnalyzerHelpersTests.cs b/src/tests/ReactiveUI.Binding.Analyzer.Tests/Helpers/AnalyzerHelpersTests.cs index f9b2318..f083b87 100644 --- a/src/tests/ReactiveUI.Binding.Analyzer.Tests/Helpers/AnalyzerHelpersTests.cs +++ b/src/tests/ReactiveUI.Binding.Analyzer.Tests/Helpers/AnalyzerHelpersTests.cs @@ -5,8 +5,8 @@ using System.Runtime.CompilerServices; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; -using NSubstitute; using ReactiveUI.Binding.Analyzer.Analyzers; +using ReactiveUI.Binding.Tests.Shared; namespace ReactiveUI.Binding.Analyzer.Tests.Helpers; @@ -86,21 +86,19 @@ public void Usage() [Test] public async Task IsBindingExtensionMethod_NullContainingType_ReturnsFalse() { - var methodSymbol = Substitute.For(); - _ = methodSymbol.ContainingType.Returns((INamedTypeSymbol?)null); + var methodSymbol = RoslynSymbolProbe.MethodWithNoContainingType(); var result = AnalyzerHelpers.IsBindingExtensionMethod(methodSymbol); await Assert.That(result).IsFalse(); } - /// Verifies that ExtractFirstTypeArgument returns null when TypeArguments is empty (using a substitute method symbol). + /// Verifies that ExtractFirstTypeArgument returns null for a method that names no type arguments. /// A task representing the asynchronous test operation. [Test] - public async Task ExtractFirstTypeArgument_EmptyTypeArguments_ReturnsNull_Substitute() + public async Task ExtractFirstTypeArgument_MethodNamesNoTypeArguments_ReturnsNull() { - var methodSymbol = Substitute.For(); - _ = methodSymbol.TypeArguments.Returns([]); + var methodSymbol = RoslynSymbolProbe.MethodWithNoContainingType(); var result = AnalyzerHelpers.ExtractFirstTypeArgument(methodSymbol); diff --git a/src/tests/ReactiveUI.Binding.Analyzer.Tests/ReactiveUI.Binding.Analyzer.Tests.csproj b/src/tests/ReactiveUI.Binding.Analyzer.Tests/ReactiveUI.Binding.Analyzer.Tests.csproj index 3cdb20e..4ff5d18 100644 --- a/src/tests/ReactiveUI.Binding.Analyzer.Tests/ReactiveUI.Binding.Analyzer.Tests.csproj +++ b/src/tests/ReactiveUI.Binding.Analyzer.Tests/ReactiveUI.Binding.Analyzer.Tests.csproj @@ -7,7 +7,6 @@ - @@ -23,6 +22,7 @@ + diff --git a/src/tests/ReactiveUI.Binding.Analyzer.Tests/TypeAnalyzerTests.cs b/src/tests/ReactiveUI.Binding.Analyzer.Tests/TypeAnalyzerTests.cs index ee83fb2..a97c562 100644 --- a/src/tests/ReactiveUI.Binding.Analyzer.Tests/TypeAnalyzerTests.cs +++ b/src/tests/ReactiveUI.Binding.Analyzer.Tests/TypeAnalyzerTests.cs @@ -989,4 +989,52 @@ public void Test() var diagnostics = await AnalyzerTestHelper.GetDiagnosticsAsync(Source); await Assert.That(diagnostics.Length).IsEqualTo(0); } + + /// + /// Verifies RXUIBIND002 stays silent for BindTo, whose first type argument is the value type of a + /// stream the caller already built rather than an object anything observes. + /// + /// A task representing the asynchronous test operation. + [Test] + public async Task RXUIBIND002_BindToStreamOfPlainValues_NoDiagnostic() + { + const string Source = """ + using System; + using System.ComponentModel; + using System.Linq.Expressions; + + namespace ReactiveUI.Binding + { + public static class __ReactiveUIGeneratedBindings + { + public static IDisposable BindTo( + this IObservable source, + TTarget target, + Expression> property) + => throw new NotImplementedException(); + } + } + + namespace TestApp + { + public class MyView : INotifyPropertyChanged + { + public event PropertyChangedEventHandler? PropertyChanged; + public string Caption { get; set; } = ""; + } + + public class Usage + { + public void Test(IObservable names) + { + var view = new MyView(); + ReactiveUI.Binding.__ReactiveUIGeneratedBindings.BindTo(names, view, v => v.Caption); + } + } + } + """; + + var diagnostics = await AnalyzerTestHelper.GetDiagnosticsAsync(Source); + await Assert.That(diagnostics.Length).IsEqualTo(0); + } } diff --git a/src/tests/ReactiveUI.Binding.SourceGenerators.Tests.Roslyn413/ReactiveUI.Binding.SourceGenerators.Tests.Roslyn413.csproj b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests.Roslyn413/ReactiveUI.Binding.SourceGenerators.Tests.Roslyn413.csproj new file mode 100644 index 0000000..dcfda25 --- /dev/null +++ b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests.Roslyn413/ReactiveUI.Binding.SourceGenerators.Tests.Roslyn413.csproj @@ -0,0 +1,49 @@ + + + + + net8.0;net9.0;net10.0;net11.0 + false + enable + + + + + + + + + + + + + + + + + + + + + + + + + + + + + $(MSBuildProjectDirectory)\..\SharedScenarios\ + + + + + + + + diff --git a/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/Helpers/BindToExtractorTests.cs b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/Helpers/BindToExtractorTests.cs index d345965..1b83971 100644 --- a/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/Helpers/BindToExtractorTests.cs +++ b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/Helpers/BindToExtractorTests.cs @@ -2,10 +2,8 @@ // ReactiveUI Association Incorporated licenses this file to you under the MIT license. // See the LICENSE file in the project root for full license information. -using System.Collections.Immutable; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; -using NSubstitute; using ReactiveUI.Binding.SourceGenerators.Helpers; namespace ReactiveUI.Binding.SourceGenerators.Tests.Helpers; @@ -63,44 +61,11 @@ public async Task GetObservableValueType_ReceiverIsLookalikeInterface_ReturnsNul public async Task GetObservableValueType_ReceiverImplementsLookalikeInterface_ReturnsNull() => await Assert.That(BindToExtractor.GetObservableValueType(FieldType("Probe.Lookalike"))).IsNull(); - /// - /// A type with no containing namespace is not taken for the framework interface. Source cannot produce - /// one - every declared type lands in the global namespace at worst - so the guard is asserted directly. - /// + /// A type belonging to no namespace at all is not taken for the framework interface. /// A task representing the asynchronous test operation. [Test] public async Task GetObservableValueType_ReceiverHasNoContainingNamespace_ReturnsNull() => - await Assert.That(BindToExtractor.GetObservableValueType(NamespacelessObservable())).IsNull(); - - /// An implemented interface with no containing namespace is likewise not the framework one. - /// A task representing the asynchronous test operation. - [Test] - public async Task GetObservableValueType_ImplementedInterfaceHasNoContainingNamespace_ReturnsNull() - { - // The interface is built first: NSubstitute rejects configuring one substitute inside another's Returns. - var lookalike = NamespacelessObservable(); - var interfaces = ImmutableArray.Create(lookalike); - - var receiver = Substitute.For(); - _ = receiver.Name.Returns("Holder"); - _ = receiver.AllInterfaces.Returns(interfaces); - - await Assert.That(BindToExtractor.GetObservableValueType(receiver)).IsNull(); - } - - /// Builds a single-argument type named like the framework interface but belonging to no namespace. - /// The namespaceless type. - private static INamedTypeSymbol NamespacelessObservable() - { - var element = Substitute.For(); - var type = Substitute.For(); - _ = type.Name.Returns("IObservable"); - _ = type.TypeArguments.Returns([element]); - _ = type.ContainingNamespace.Returns((INamespaceSymbol?)null); - _ = type.AllInterfaces.Returns(ImmutableArray.Empty); - - return type; - } + await Assert.That(BindToExtractor.GetObservableValueType(FieldType("string[]"))).IsNull(); /// Resolves a type by declaring a field of it in a probe compilation. /// The type to resolve, as it is written in source. diff --git a/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/Helpers/ExtractorValidationTests.cs b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/Helpers/ExtractorValidationTests.cs index 87d4c8f..c777a3e 100644 --- a/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/Helpers/ExtractorValidationTests.cs +++ b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/Helpers/ExtractorValidationTests.cs @@ -5,8 +5,8 @@ using System.Collections.Immutable; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; -using NSubstitute; using ReactiveUI.Binding.SourceGenerators.Helpers; +using ReactiveUI.Binding.Tests.Shared; namespace ReactiveUI.Binding.SourceGenerators.Tests.Helpers; @@ -19,9 +19,37 @@ public class ExtractorValidationTests /// The string name these tests generate against. private const string StringName = "string"; + /// The int name these tests generate against. + private const string IntName = "int"; + /// A class name no recognized extension class uses. private const string UnknownClassName = "CustomExtensions"; + /// A class declaring one parameter under each name and shape these tests look for. + private const string SelectorProbeSource = """ + namespace Probe + { + public class Holder + { + public void TakesSelector(System.Func selector) + { + } + + public void TakesAnotherName(System.Func otherParam) + { + } + + public void TakesConversion(System.Func conversionFunc) + { + } + + public void TakesPlainType(string selector) + { + } + } + } + """; + /// Verifies that the stub extension class name is recognized. /// A task representing the asynchronous test operation. [Test] @@ -265,17 +293,7 @@ public async Task FindSelectorReturnType_EmptyParameters_ReturnsNull() [Test] public async Task FindSelectorReturnType_NoMatchingParameter_ReturnsNull() { - var typeArg = Substitute.For(); - _ = typeArg.ToDisplayString(Arg.Any()).Returns(StringName); - - var funcType = Substitute.For(); - _ = funcType.TypeArguments.Returns([typeArg]); - - var param = Substitute.For(); - _ = param.Name.Returns("otherParam"); - _ = param.Type.Returns(funcType); - - var parameters = ImmutableArray.Create(param); + var parameters = RoslynSymbolProbe.MethodParameters(SelectorProbeSource, "TakesAnotherName"); var result = ExtractorValidation.FindSelectorReturnType(parameters, SelectorName); @@ -287,17 +305,7 @@ public async Task FindSelectorReturnType_NoMatchingParameter_ReturnsNull() [Test] public async Task FindSelectorReturnType_MatchingParameter_ReturnsType() { - var typeArg = Substitute.For(); - _ = typeArg.ToDisplayString(Arg.Any()).Returns(StringName); - - var funcType = Substitute.For(); - _ = funcType.TypeArguments.Returns([typeArg]); - - var param = Substitute.For(); - _ = param.Name.Returns(SelectorName); - _ = param.Type.Returns(funcType); - - var parameters = ImmutableArray.Create(param); + var parameters = RoslynSymbolProbe.MethodParameters(SelectorProbeSource, "TakesSelector"); var result = ExtractorValidation.FindSelectorReturnType(parameters, SelectorName); @@ -309,21 +317,11 @@ public async Task FindSelectorReturnType_MatchingParameter_ReturnsType() [Test] public async Task FindSelectorReturnType_MultipleNames_MatchesSecondName() { - var typeArg = Substitute.For(); - _ = typeArg.ToDisplayString(Arg.Any()).Returns("int"); - - var funcType = Substitute.For(); - _ = funcType.TypeArguments.Returns([typeArg]); - - var param = Substitute.For(); - _ = param.Name.Returns("conversionFunc"); - _ = param.Type.Returns(funcType); - - var parameters = ImmutableArray.Create(param); + var parameters = RoslynSymbolProbe.MethodParameters(SelectorProbeSource, "TakesConversion"); var result = ExtractorValidation.FindSelectorReturnType(parameters, SelectorName, "conversionFunc"); - await Assert.That(result).IsEqualTo("int"); + await Assert.That(result).IsEqualTo(IntName); } /// Verifies that FindSelectorReturnType skips parameters with non-generic types. @@ -331,14 +329,7 @@ public async Task FindSelectorReturnType_MultipleNames_MatchesSecondName() [Test] public async Task FindSelectorReturnType_NonGenericType_ReturnsNull() { - var nonGenericType = Substitute.For(); - _ = nonGenericType.TypeArguments.Returns([]); - - var param = Substitute.For(); - _ = param.Name.Returns(SelectorName); - _ = param.Type.Returns(nonGenericType); - - var parameters = ImmutableArray.Create(param); + var parameters = RoslynSymbolProbe.MethodParameters(SelectorProbeSource, "TakesPlainType"); var result = ExtractorValidation.FindSelectorReturnType(parameters, SelectorName); @@ -369,18 +360,6 @@ public async Task IsRecognizedExtensionClass_UnnamedGroupingInUnknownClass_Retur await Assert.That(result).IsFalse(); } - /// A grouping type with nothing enclosing it has no name to be judged by. - /// A task representing the asynchronous test operation. - [Test] - public async Task IsRecognizedExtensionClass_UnnamedGroupingWithNoEnclosingClass_ReturnsFalse() - { - var orphan = Substitute.For(); - _ = orphan.Name.Returns(string.Empty); - _ = orphan.ContainingType.Returns((INamedTypeSymbol?)null); - - await Assert.That(ExtractorValidation.IsRecognizedExtensionClass(orphan)).IsFalse(); - } - /// /// Compiles a static class holding a closure and returns the display class the compiler synthesized inside /// it, which carries the same shape as the grouping type an extension block declares its members in: a name @@ -450,20 +429,40 @@ private static INamedTypeSymbol CompiledType(string source) } /// - /// Builds a grouping type with no name of its own, nested in a class of the given name. A source-declared - /// extension block takes this shape, which no compiled identifier can spell. + /// Compiles an extension block and returns the grouping type the compiler declares its members in: a type + /// with no name of its own, nested one level inside the class that names the API. Read from source rather + /// than from an emitted image, which is the spelling that has no name at all. /// - /// The name to give the enclosing class. + /// The name to give the enclosing static class. /// The unnamed grouping type. + /// The compiler declared no unnamed nested type. private static INamedTypeSymbol UnnamedGroupingIn(string className) { - var enclosing = Substitute.For(); - _ = enclosing.Name.Returns(className); + var compilation = TestHelper.CreateCompilation( + $$""" + public static class {{className}} + { + extension(int value) + { + public int Doubled => value * 2; + } + } + """, + LanguageVersion.Preview); + + var outer = compilation.GetTypeByMetadataName(className) + ?? throw new InvalidOperationException($"'{className}' did not compile."); - var grouping = Substitute.For(); - _ = grouping.Name.Returns(string.Empty); - _ = grouping.ContainingType.Returns(enclosing); + var nested = outer.GetTypeMembers(); + for (var i = 0; i < nested.Length; i++) + { + if (nested[i].Name.Length == 0) + { + return nested[i]; + } + } - return grouping; + throw new InvalidOperationException( + $"The compiler declared no unnamed grouping type inside '{className}'."); } } diff --git a/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/Helpers/InterceptableLocationReaderTests.cs b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/Helpers/InterceptableLocationReaderTests.cs new file mode 100644 index 0000000..ff5a03f --- /dev/null +++ b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/Helpers/InterceptableLocationReaderTests.cs @@ -0,0 +1,120 @@ +// Copyright (c) 2019-2026 ReactiveUI Association Incorporated. All rights reserved. +// ReactiveUI Association Incorporated licenses this file to you under the MIT license. +// See the LICENSE file in the project root for full license information. + +using System.Runtime.CompilerServices; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using ReactiveUI.Binding.SourceGenerators.Helpers; + +namespace ReactiveUI.Binding.SourceGenerators.Tests.Helpers; + +/// Tests for , which is where both builds answer about interception. +public class InterceptableLocationReaderTests +{ + /// The namespace the generator emits interceptors into. + private const string GeneratedNamespace = Constants.InterceptorNamespace; + + /// The feature name a build lists interceptable namespaces under. + private const string FeatureName = Constants.InterceptorsNamespacesFeature; + + /// A build that lists nothing has not opted in. + /// A task representing the asynchronous test operation. + [Test] + public async Task IsOptedIn_NoFeature_ReturnsFalse() => + await Assert.That(InterceptableLocationReader.IsOptedIn(new CSharpParseOptions())).IsFalse(); + + /// An empty list is the same as no list. + /// A task representing the asynchronous test operation. + [Test] + public async Task IsOptedIn_EmptyFeature_ReturnsFalse() => + await Assert.That(InterceptableLocationReader.IsOptedIn(WithNamespaces(string.Empty))).IsFalse(); + + /// The generated namespace named exactly is an opt-in. + /// A task representing the asynchronous test operation. + [Test] + public async Task IsOptedIn_ExactNamespace_ReturnsTrue() => + await Assert.That(InterceptableLocationReader.IsOptedIn(WithNamespaces(GeneratedNamespace))).IsTrue(); + + /// A listed namespace covers the ones nested under it, so a parent counts. + /// A task representing the asynchronous test operation. + [Test] + public async Task IsOptedIn_EnclosingNamespace_ReturnsTrue() => + await Assert.That(InterceptableLocationReader.IsOptedIn(WithNamespaces("ReactiveUI.Binding"))).IsTrue(); + + /// A namespace that merely starts with the same characters is a different namespace. + /// A task representing the asynchronous test operation. + [Test] + public async Task IsOptedIn_NamespaceSharingAPrefix_ReturnsFalse() => + await Assert.That(InterceptableLocationReader.IsOptedIn(WithNamespaces("ReactiveUI.Bind"))).IsFalse(); + + /// Another package's namespace is not this one's opt-in. + /// A task representing the asynchronous test operation. + [Test] + public async Task IsOptedIn_UnrelatedNamespace_ReturnsFalse() => + await Assert.That(InterceptableLocationReader.IsOptedIn(WithNamespaces("Some.Other.Package"))).IsFalse(); + + /// The listed namespaces are read out of a list, so the entry can be anywhere in it. + /// A task representing the asynchronous test operation. + [Test] + public async Task IsOptedIn_ListedAlongsideOthers_ReturnsTrue() => + await Assert.That(InterceptableLocationReader.IsOptedIn( + WithNamespaces($"Some.Other.Package;{GeneratedNamespace};Third.Package"))).IsTrue(); + + /// A separator with nothing between it and the next is skipped rather than matched. + /// A task representing the asynchronous test operation. + [Test] + public async Task IsOptedIn_ListWithEmptyEntries_ReturnsTrue() => + await Assert.That(InterceptableLocationReader.IsOptedIn(WithNamespaces($";;{GeneratedNamespace};"))).IsTrue(); + + /// A list of nothing but separators names no namespace at all. + /// A task representing the asynchronous test operation. + [Test] + public async Task IsOptedIn_ListOfSeparators_ReturnsFalse() => + await Assert.That(InterceptableLocationReader.IsOptedIn(WithNamespaces(";; ;"))).IsFalse(); + + /// + /// A call site is described where the compiler can describe one, and reported as undescribed where it + /// cannot. Both answers are handled the same way by every caller, so both are asserted here. + /// + /// A task representing the asynchronous test operation. + [Test] + public async Task Read_CallSite_MatchesWhatThisBuildSupports() + { + const string Source = """ + namespace Probe + { + public static class Holder + { + public static string Read() => 1.ToString(); + } + } + """; + + var compilation = TestHelper.CreateCompilation(Source, LanguageVersion.CSharp10); + var tree = compilation.SyntaxTrees.First(); + var root = await tree.GetRootAsync(); + var invocation = root.DescendantNodes().OfType().First(); + + var location = InterceptableLocationReader.Read( + compilation.GetSemanticModel(tree), + invocation, + CancellationToken.None); + + await Assert.That(location.IsAvailable).IsEqualTo(InterceptableLocationReader.IsSupported); + } + + /// An undescribed call site carries no data, whichever build produced it. + /// A task representing the asynchronous test operation. + [Test] + public async Task IsAvailable_DefaultLocation_ReturnsFalse() => + await Assert.That(default(SourceGenerators.Models.InterceptorLocation).IsAvailable).IsFalse(); + + /// Builds parse options listing the given namespaces for interception. + /// The value the build sets. + /// The parse options. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static CSharpParseOptions WithNamespaces(string namespaces) => + new CSharpParseOptions().WithFeatures([new KeyValuePair(FeatureName, namespaces)]); +} diff --git a/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/Helpers/TestHelper.cs b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/Helpers/TestHelper.cs index 5e0f0fe..d28427f 100644 --- a/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/Helpers/TestHelper.cs +++ b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/Helpers/TestHelper.cs @@ -72,17 +72,32 @@ public static Compilation CreateCompilation( /// The name to give the compilation's own assembly. /// References to add on top of the framework and runtime ones. /// A compilation ready for testing. + [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Compilation CreateCompilation( string source, LanguageVersion? languageVersion, bool useReactiveRuntime, string assemblyName, + ImmutableArray additionalReferences) => + CreateCompilation(source, ParseOptionsFor(languageVersion), useReactiveRuntime, assemblyName, additionalReferences); + + /// + /// Creates a compilation from source code parsed exactly as the caller asks, which is how a scenario reaches + /// the options a language version alone cannot express. + /// + /// The source code to compile. + /// The options the consumer's source is parsed with. + /// Whether to reference the System.Reactive flavour rather than the lean one. + /// The name to give the compilation's own assembly. + /// References to add on top of the framework and runtime ones. + /// A compilation ready for testing. + public static Compilation CreateCompilation( + string source, + CSharpParseOptions parseOptions, + bool useReactiveRuntime, + string assemblyName, ImmutableArray additionalReferences) { - var parseOptions = languageVersion.HasValue - ? new CSharpParseOptions(languageVersion.Value) - : new CSharpParseOptions(LanguageVersion.CSharp7_3); - var syntaxTree = CSharpSyntaxTree.ParseText(source, parseOptions); #if NET11_0_OR_GREATER @@ -124,6 +139,24 @@ public static Compilation CreateCompilation( new(OutputKind.DynamicallyLinkedLibrary)); } + /// The parse options a build produces from a language version, which is what the tests usually name. + /// The C# language version to target, or for C# 7.3. + /// The parse options. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static CSharpParseOptions ParseOptionsFor(LanguageVersion? languageVersion) => + new(languageVersion ?? LanguageVersion.CSharp7_3); + + /// + /// The parse options of a build that has the package's targets on it, which list the generated namespace so + /// the compiler honours an interceptor emitted into it. + /// + /// The C# language version to target, or for C# 7.3. + /// The parse options, carrying the opt-in. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static CSharpParseOptions InterceptingParseOptionsFor(LanguageVersion? languageVersion) => + ParseOptionsFor(languageVersion) + .WithFeatures([new KeyValuePair("InterceptorsNamespaces", "ReactiveUI.Binding.Generated.Interceptors")]); + /// /// Compiles source into a metadata reference, standing in for a type the consumer references rather than /// declares. A referenced type reaches the generator only as a symbol, never as a declaration. @@ -370,16 +403,27 @@ public static GeneratorTestResult RunGenerator( /// The root namespace the build exposes, or for none. /// Whether the build asks for the generated-file markers. /// A containing driver, compilation, and diagnostics. + [MethodImpl(MethodImplOptions.AggressiveInlining)] public static GeneratorTestResult RunGenerator( Compilation compilation, LanguageVersion? languageVersion, string? rootNamespace, + bool emitGeneratedCodeMarkers) => + RunGenerator(compilation, ParseOptionsFor(languageVersion), rootNamespace, emitGeneratedCodeMarkers); + + /// Runs the source generator with the parse options the caller names, for both input and output. + /// The compilation to generate against. + /// The options generated code is parsed with, and that the generator reads. + /// The root namespace the build exposes, or for none. + /// Whether the build asks for the generated-file markers. + /// A containing driver, compilation, and diagnostics. + public static GeneratorTestResult RunGenerator( + Compilation compilation, + CSharpParseOptions parseOptions, + string? rootNamespace, bool emitGeneratedCodeMarkers) { var generator = new BindingGenerator(); - var parseOptions = languageVersion.HasValue - ? new CSharpParseOptions(languageVersion.Value) - : new CSharpParseOptions(LanguageVersion.CSharp7_3); GeneratorDriver driver = CSharpGeneratorDriver.Create( [generator.AsSourceGenerator()], diff --git a/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/InterceptedCallSiteTests.cs b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/InterceptedCallSiteTests.cs new file mode 100644 index 0000000..f462592 --- /dev/null +++ b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/InterceptedCallSiteTests.cs @@ -0,0 +1,256 @@ +// Copyright (c) 2019-2026 ReactiveUI Association Incorporated. All rights reserved. +// ReactiveUI Association Incorporated licenses this file to you under the MIT license. +// See the LICENSE file in the project root for full license information. + +using System.Reflection; +using System.Runtime.CompilerServices; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using ReactiveUI.Binding.SourceGenerators.Helpers; +using ReactiveUI.Binding.SourceGenerators.Tests.Helpers; + +namespace ReactiveUI.Binding.SourceGenerators.Tests; + +/// +/// Tests the tier that claims a binding call site outright instead of offering an overload that has to win +/// extension-method lookup. +/// +/// +/// Which tier a build gets is settled by the compiler hosting the generator and by whether the project lists +/// the generated namespace, so every test here states the outcome for both and the suite runs against both +/// generator builds. That is what keeps the assertions honest on a compiler that cannot describe a call site +/// at all, where the overloads are still the only thing that can be emitted. +/// +public class InterceptedCallSiteTests +{ + /// The attribute text an interceptor carries. + private const string InterceptsAttribute = "InterceptsLocation("; + + /// The declaration text a dispatch overload carries. + private const string OverloadDeclaration = "public static global::System.IObservable<"; + + /// The dispatch file these tests read. + private const string DispatchFileName = "WhenChangedDispatch.g.cs"; + + /// The root namespace the scenarios build under. + private const string RootNamespace = "TestApp"; + + /// The type the scenario exposes its binding through. + private const string UsageTypeName = $"{RootNamespace}.Usage"; + + /// A binding whose value can be read back out of the emitted assembly. + private const string Scenario = """ + using System; + using System.ComponentModel; + using ReactiveUI.Binding; + + namespace TestApp + { + public class MyViewModel : INotifyPropertyChanged + { + private string _name = "start"; + + public event PropertyChangedEventHandler PropertyChanged; + + public string Name + { + get { return _name; } + set + { + _name = value; + var handler = PropertyChanged; + if (handler != null) + { + handler(this, new PropertyChangedEventArgs("Name")); + } + } + } + } + + public static class Usage + { + public static string Observe() + { + var viewModel = new MyViewModel(); + string seen = null; + var subscription = global::ReactiveUI.Primitives.SubscribeExtensions.Subscribe( + viewModel.WhenChanged(x => x.Name), + delegate(string value) { seen = value; }); + viewModel.Name = "changed"; + subscription.Dispose(); + return seen; + } + } + } + """; + + /// The same binding written in a namespace no root namespace encloses. + private const string OutOfReachScenario = """ + using System; + using System.ComponentModel; + using ReactiveUI.Binding; + + namespace Elsewhere + { + public class MyViewModel : INotifyPropertyChanged + { + public event PropertyChangedEventHandler PropertyChanged; + + public string Name { get; set; } + } + + public class Usage + { + public void Bind() + { + var viewModel = new MyViewModel(); + var observable = viewModel.WhenChanged(x => x.Name); + GC.KeepAlive(observable); + } + } + } + """; + + /// A build listing the generated namespace gets the tier its compiler can honour. + /// A task representing the asynchronous test operation. + [Test] + public async Task OptedInBuild_EmitsInterceptorsWhereTheCompilerCanDescribeACallSite() + { + var dispatch = GenerateDispatch(Scenario, LanguageVersion.CSharp10, optIn: true); + + await Assert.That(dispatch.Contains(InterceptsAttribute, StringComparison.Ordinal)) + .IsEqualTo(InterceptableLocationReader.IsSupported); + await Assert.That(dispatch.Contains(OverloadDeclaration, StringComparison.Ordinal)) + .IsEqualTo(!InterceptableLocationReader.IsSupported); + } + + /// The opt-in is what turns the tier on; without it the overloads are emitted either way. + /// A task representing the asynchronous test operation. + [Test] + public async Task BuildWithoutTheOptIn_EmitsTheDispatchOverload() + { + var dispatch = GenerateDispatch(Scenario, LanguageVersion.CSharp10, optIn: false); + + await Assert.That(dispatch).DoesNotContain(InterceptsAttribute); + await Assert.That(dispatch).Contains(OverloadDeclaration); + } + + /// + /// A project below C# 10 is served the same way. Interception is refused on a language version, not chosen + /// by one, which is what puts a compile-time binding in reach of a consumer the overloads cannot serve. + /// + /// A task representing the asynchronous test operation. + [Test] + public async Task OptedInBuild_BelowCSharp10_StillClaimsTheCallSite() + { + var dispatch = GenerateDispatch(Scenario, LanguageVersion.CSharp7_3, optIn: true); + + await Assert.That(dispatch.Contains(InterceptsAttribute, StringComparison.Ordinal)) + .IsEqualTo(InterceptableLocationReader.IsSupported); + await Assert.That(dispatch.Contains(OverloadDeclaration, StringComparison.Ordinal)) + .IsEqualTo(!InterceptableLocationReader.IsSupported); + } + + /// + /// A file declared outside the root namespace is out of the overloads' reach, and is claimed anyway. This is + /// the case the tier exists for: nothing about an interceptor goes through extension-method lookup. + /// + /// A task representing the asynchronous test operation. + [Test] + public async Task OptedInBuild_ClaimsACallSiteOutsideTheRootNamespace() + { + var dispatch = GenerateDispatch(OutOfReachScenario, LanguageVersion.CSharp7_3, optIn: true); + + await Assert.That(dispatch.Contains(InterceptsAttribute, StringComparison.Ordinal)) + .IsEqualTo(InterceptableLocationReader.IsSupported); + } + + /// + /// The emitted assembly runs the binding. Emission is where the compiler checks an interceptor against the + /// call it replaces, so a signature that does not match fails here rather than being noticed downstream. + /// + /// A task representing the asynchronous test operation. + [Test] + public async Task OptedInBuild_RunsTheBindingItClaimed() + { + var result = Generate(Scenario, LanguageVersion.CSharp10, optIn: true, RootNamespace); + + await Assert.That(result.CompilationErrors).IsEmpty(); + + var (assembly, context) = TestHelper.EmitAndLoad(result); + var observe = assembly.GetType(UsageTypeName)?.GetMethod( + "Observe", + BindingFlags.Public | BindingFlags.Static); + + await Assert.That(observe).IsNotNull(); + await Assert.That(observe!.Invoke(null, null)).IsEqualTo("changed"); + + context.Unload(); + } + + /// The same binding runs when the overloads are what the build got. + /// A task representing the asynchronous test operation. + [Test] + public async Task BuildWithoutTheOptIn_RunsTheBindingItDispatched() + { + var result = Generate(Scenario, LanguageVersion.CSharp10, optIn: false, RootNamespace); + + await Assert.That(result.CompilationErrors).IsEmpty(); + + var (assembly, context) = TestHelper.EmitAndLoad(result); + var observe = assembly.GetType(UsageTypeName)?.GetMethod( + "Observe", + BindingFlags.Public | BindingFlags.Static); + + await Assert.That(observe).IsNotNull(); + await Assert.That(observe!.Invoke(null, null)).IsEqualTo("changed"); + + context.Unload(); + } + + /// The generated namespace is what the opt-in has to name. + /// A task representing the asynchronous test operation. + [Test] + public async Task OptedInBuild_GeneratesIntoTheInterceptedNamespace() + { + var dispatch = GenerateDispatch(Scenario, LanguageVersion.CSharp10, optIn: true); + + await Assert.That(dispatch.Contains($"namespace {Constants.InterceptorNamespace}", StringComparison.Ordinal)) + .IsEqualTo(InterceptableLocationReader.IsSupported); + } + + /// Runs the generator and returns the dispatch file it produced. + /// The consumer source. + /// The language version the consumer builds at. + /// Whether the build lists the generated namespace for interception. + /// The root namespace the build exposes. + /// The generated dispatch file. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static string GenerateDispatch( + string source, + LanguageVersion languageVersion, + bool optIn, + string? rootNamespace = RootNamespace) => + Generate(source, languageVersion, optIn, rootNamespace).GeneratedSources[DispatchFileName]; + + /// Runs the generator over a compilation parsed the way the build in question parses. + /// The consumer source. + /// The language version the consumer builds at. + /// Whether the build lists the generated namespace for interception. + /// The root namespace the build exposes. + /// The generator result. + private static GeneratorTestResult Generate( + string source, + LanguageVersion languageVersion, + bool optIn, + string? rootNamespace) + { + var parseOptions = optIn + ? TestHelper.InterceptingParseOptionsFor(languageVersion) + : TestHelper.ParseOptionsFor(languageVersion); + + var compilation = TestHelper.CreateCompilation(source, parseOptions, false, "TestAssembly", []); + + return TestHelper.RunGenerator(compilation, parseOptions, rootNamespace, true); + } +} diff --git a/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/ReactiveUI.Binding.SourceGenerators.Tests.csproj b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/ReactiveUI.Binding.SourceGenerators.Tests.csproj index b952101..3f9a848 100644 --- a/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/ReactiveUI.Binding.SourceGenerators.Tests.csproj +++ b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/ReactiveUI.Binding.SourceGenerators.Tests.csproj @@ -7,7 +7,6 @@ - @@ -26,6 +25,7 @@ + diff --git a/src/tests/Shared/RoslynSymbolProbe.cs b/src/tests/Shared/RoslynSymbolProbe.cs new file mode 100644 index 0000000..9757f77 --- /dev/null +++ b/src/tests/Shared/RoslynSymbolProbe.cs @@ -0,0 +1,91 @@ +// Copyright (c) 2019-2026 ReactiveUI Association Incorporated. All rights reserved. +// ReactiveUI Association Incorporated licenses this file to you under the MIT license. +// See the LICENSE file in the project root for full license information. + +using System; +using System.Collections.Immutable; +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; + +namespace ReactiveUI.Binding.Tests.Shared; + +/// Resolves real symbols from a throwaway compilation, for helpers that take one directly. +/// +/// The compiler's symbol interfaces cannot be implemented outside Roslyn itself, so a helper taking an +/// is exercised by compiling source that produces the symbol wanted and handing the +/// real one over. That also keeps the tests honest: a shape source cannot express is a shape the generator +/// will never meet. The exotic ones still have a source form - a function pointer's signature belongs to no +/// type, and an array type to no namespace - which is how the guards against those are reached. +/// +internal static class RoslynSymbolProbe +{ + /// The name given to the class every probe declares its members on. + internal const string HolderTypeName = "Probe.Holder"; + + /// Compiles probe source against the framework reference set. + /// The source to compile. + /// The compilation. + internal static CSharpCompilation Compile(string source) + { + var references = AppContext.GetData("TRUSTED_PLATFORM_ASSEMBLIES") is string paths + ? paths.Split(System.IO.Path.PathSeparator) + .Where(static p => p.EndsWith(".dll", StringComparison.Ordinal)) + .Select(static p => MetadataReference.CreateFromFile(p)) + .Cast() + .ToImmutableArray() + : ImmutableArray.Empty; + + return CSharpCompilation.Create( + "SymbolProbe", + [CSharpSyntaxTree.ParseText(source, new(LanguageVersion.Latest))], + references, + new(OutputKind.DynamicallyLinkedLibrary, allowUnsafe: true)); + } + + /// Resolves the type of a field declared on the probe's holder class. + /// The source declaring the holder class. + /// The field whose type is wanted. + /// The field's type. + /// The holder class or the field is not in the source. + internal static ITypeSymbol FieldType(string source, string fieldName) => + Holder(source).GetMembers(fieldName).OfType().FirstOrDefault()?.Type + ?? throw new InvalidOperationException($"The probe declares no field named '{fieldName}'."); + + /// Resolves the parameters of a method declared on the probe's holder class. + /// The source declaring the holder class. + /// The method whose parameters are wanted. + /// The method's parameters. + /// The holder class or the method is not in the source. + internal static ImmutableArray MethodParameters(string source, string methodName) => + Holder(source).GetMembers(methodName).OfType().FirstOrDefault()?.Parameters + ?? throw new InvalidOperationException($"The probe declares no method named '{methodName}'."); + + /// A method belonging to no type, which a function pointer's signature is and no declaration is. + /// The method. + /// The probe did not produce a function pointer type. + internal static IMethodSymbol MethodWithNoContainingType() + { + const string Source = """ + namespace Probe + { + public unsafe class Holder + { + public delegate* Callback; + } + } + """; + + return FieldType(Source, "Callback") is IFunctionPointerTypeSymbol pointer + ? pointer.Signature + : throw new InvalidOperationException("The probe did not produce a function pointer type."); + } + + /// Resolves the probe's holder class. + /// The source declaring it. + /// The holder class. + /// The holder class is not in the source. + private static INamedTypeSymbol Holder(string source) => + Compile(source).GetTypeByMetadataName(HolderTypeName) + ?? throw new InvalidOperationException($"The probe source declares no {HolderTypeName}."); +} From 36c9882c271f4e09a6cca0e2a11cb90995005566 Mon Sep 17 00:00:00 2001 From: Glenn Watson <5834289+glennawatson@users.noreply.github.com> Date: Wed, 9 Sep 2026 23:13:55 +1000 Subject: [PATCH 08/12] docs: describe the two ways a call site reaches its generated code - Set out the interceptor and overload mechanisms, what each package slot carries, and the settings a consumer has. - List BindTo among the supported APIs. - Assert the shipped slots against the versions the targets select between. - Measure a generation pass with and without the interception opt-in, against both compiler builds. --- README.md | 72 +++++-- src/ReactiveUI.Binding.SourceGenerators.slnx | 2 + ...ding.Generator.Benchmarks.Roslyn413.csproj | 32 +++ .../GenerationBenchmarks.cs | 11 +- .../GeneratorHarness.cs | 32 ++- .../AnalyzerPackagingTests.cs | 186 ++++++++++++++++++ 6 files changed, 311 insertions(+), 24 deletions(-) create mode 100644 src/benchmarks/ReactiveUI.Binding.Generator.Benchmarks.Roslyn413/ReactiveUI.Binding.Generator.Benchmarks.Roslyn413.csproj create mode 100644 src/tests/ReactiveUI.Binding.SourceGenerators.Tests/AnalyzerPackagingTests.cs diff --git a/README.md b/README.md index a30b5b6..9cf4f03 100644 --- a/README.md +++ b/README.md @@ -24,6 +24,7 @@ generation. Zero reflection, fully AOT/trimming safe, 3-7x faster than the legac - [What does it do?](#what-does-it-do) - [How does it work?](#how-does-it-work) +- [How a call site reaches its generated code](#how-a-call-site-reaches-its-generated-code) - [How do I install?](#how-do-i-install) - [Supported APIs](#supported-apis) - [Usage Examples](#usage-examples) @@ -61,8 +62,9 @@ generation. Zero reflection, fully AOT/trimming safe, 3-7x faster than the legac ## What does it do? ReactiveUI.Binding.SourceGenerators is an incremental source generator that analyses your `WhenChanged`, `WhenChanging`, -`WhenAnyValue`, `WhenAny`, `WhenAnyObservable`, `BindOneWay`, `BindTwoWay`, `OneWayBind`, and `Bind` call sites at -compile time and emits optimised, strongly-typed observation and binding code. It eliminates: +`WhenAnyValue`, `WhenAny`, `WhenAnyObservable`, `BindOneWay`, `BindTwoWay`, `OneWayBind`, `Bind`, `BindTo`, +`BindCommand`, and `BindInteraction` call sites at compile time and emits optimised, strongly-typed observation and +binding code. It eliminates: - **Runtime expression-tree compilation** -- no `Expression>` evaluation at runtime - **Reflection** -- all property access is generated as direct member access @@ -80,25 +82,13 @@ WPF DependencyObject, WinUI DependencyObject, Apple KVO, WinForms Component, And observation factories via a `[ModuleInitializer]`. **Pipeline B (Invocation Detection)** scans method invocations and extracts lambda property paths at compile time. Each -call site is identified by `[CallerFilePath]` + `[CallerLineNumber]`, and the generator emits a per-call-site optimised -method that is dispatched to at runtime via a generated lookup table. +call site gets its own optimised method with direct property access: ```csharp // You write: var obs = vm.WhenChanged(x => x.Name); -// The generator emits a dispatch stub that captures caller info: -public static IObservable WhenChanged( - this TObj obj, Expression> property, - [CallerFilePath] string callerFilePath = "", - [CallerLineNumber] int callerLineNumber = 0) where TObj : class -{ - if (__GeneratedBindingDispatcher.TryGetWhenChanged(callerFilePath, callerLineNumber, obj, out var result)) - return (IObservable)result!; - throw new InvalidOperationException("No generated binding found."); -} - -// And a per-call-site method with direct property access: +// The generator emits a per-call-site method with direct property access: private static IObservable __WhenChanged_0(MyViewModel obj) { return new PropertyObservable( @@ -106,6 +96,47 @@ private static IObservable __WhenChanged_0(MyViewModel obj) } ``` +## How a call site reaches its generated code + +Two mechanisms. Your compiler picks one. + +**Roslyn 4.13 or newer: the call is intercepted.** The generator points the compiler at your exact call and says "run +this instead". No name lookup is involved, so it works from any file and any language version - including +`7.3` and .NET Framework 4.6.2: + +```csharp +[InterceptsLocation(1, "j8MnGMWiKja+66BWQ5M81agPAABQcm9ncmFtLmNz")] // vm.WhenChanged(x => x.Name) on line 12 +internal static IObservable __Intercept_WhenChanged_7FFF( + this MyViewModel objectToMonitor, + Expression> property1, /* caller-info parameters */) + => __WhenChanged_7FFF(objectToMonitor); +``` + +**Roslyn 4.8 to 4.12: a concrete overload competes for the call.** It beats the generic runtime stub because a +non-generic method wins overload resolution - but only when extension-method lookup finds it. RXUIBIND009 warns when it +will not. + +Either way the same generated method runs, so bindings behave identically. + +### What the package ships + +The generator and its analyzer are packed once per compiler generation: + +``` +analyzers/dotnet/roslyn4.8/cs/ <- Roslyn 4.8 - 4.12 +analyzers/dotnet/roslyn4.13/cs/ <- Roslyn 4.13+ +``` + +The .NET SDK picks the highest folder your compiler supports. A legacy non-SDK project is handed both, so the package's +targets delete the one you are not being served by - the generator never runs twice. + +Two knobs: + +| Setting | Effect | +|--------------------------------------------------------|--------------------------------------------------------------| +| `false` | Use the overloads even on a compiler that could intercept | +| Roslyn older than 4.8 | Build fails with **RXUIBIND100** rather than silently generating nothing | + ## How do I install? Install the `ReactiveUI.Binding` NuGet package. The source generator is automatically included. @@ -152,6 +183,7 @@ Platform-specific packages provide DependencyProperty observation and other plat | `BindTwoWay` | Two-way binding between source and target | | `OneWayBind` | ReactiveUI compatibility shim for one-way binding | | `Bind` | ReactiveUI compatibility shim for two-way binding | +| `BindTo` | Apply an observable stream to a target property | | `BindCommand` | Bind a command property to a UI element | | `BindInteraction` | Bind an interaction to a handler | @@ -480,10 +512,16 @@ The separate analyzer package reports the following diagnostics: | RXUIBIND006 | Warning | Expression contains an unsupported path segment (indexer, field, or method call). Only simple property access chains can be observed by the source generator. | | RXUIBIND007 | Warning | BindCommand control has no bindable event. Specify the `toEvent` parameter explicitly. | | RXUIBIND008 | Warning | The property selected in a BindInteraction expression does not implement `IInteraction`. | -| RXUIBIND009 | Warning | The generated binding dispatch is out of reach from this file, so the call falls back to the runtime stub. | +| RXUIBIND009 | Warning | The generated binding dispatch is out of reach from this file, so the call falls back to the runtime stub. Not reported where the call site is claimed by an interceptor. | | RXUIBIND010 | Warning | The observed path passes through a type that raises no notification, so it is read once and the observation stops following the path there. | | RXUIBIND011 | Warning | The call resolved to ReactiveUI's own mixin, so nothing is generated for it and it takes the runtime expression engine. Import `ReactiveUI.Binding` in the file. | +The package's own targets report one build error of their own: + +| ID | Description | +|-------------|-----------------------------------------------------------------------------------------------------------------| +| RXUIBIND100 | The compiler building the project is older than the oldest analyzer slot, so no generator would be loaded at all. | + ## Where this differs from ReactiveUI Everything below is a deliberate difference from the reflection binding engine, and each one is here because diff --git a/src/ReactiveUI.Binding.SourceGenerators.slnx b/src/ReactiveUI.Binding.SourceGenerators.slnx index c0ee1ee..1f28192 100644 --- a/src/ReactiveUI.Binding.SourceGenerators.slnx +++ b/src/ReactiveUI.Binding.SourceGenerators.slnx @@ -32,6 +32,8 @@ + diff --git a/src/benchmarks/ReactiveUI.Binding.Generator.Benchmarks.Roslyn413/ReactiveUI.Binding.Generator.Benchmarks.Roslyn413.csproj b/src/benchmarks/ReactiveUI.Binding.Generator.Benchmarks.Roslyn413/ReactiveUI.Binding.Generator.Benchmarks.Roslyn413.csproj new file mode 100644 index 0000000..b59b477 --- /dev/null +++ b/src/benchmarks/ReactiveUI.Binding.Generator.Benchmarks.Roslyn413/ReactiveUI.Binding.Generator.Benchmarks.Roslyn413.csproj @@ -0,0 +1,32 @@ + + + + + Exe + net8.0;net10.0;net11.0 + enable + false + ReactiveUI.Binding.Generator.Benchmarks.Roslyn413 + ReactiveUI.Binding.Generator.Benchmarks + + + + + + + + + + + + + + + + + + + diff --git a/src/benchmarks/ReactiveUI.Binding.Generator.Benchmarks/GenerationBenchmarks.cs b/src/benchmarks/ReactiveUI.Binding.Generator.Benchmarks/GenerationBenchmarks.cs index d3a5b4a..a911a69 100644 --- a/src/benchmarks/ReactiveUI.Binding.Generator.Benchmarks/GenerationBenchmarks.cs +++ b/src/benchmarks/ReactiveUI.Binding.Generator.Benchmarks/GenerationBenchmarks.cs @@ -25,13 +25,20 @@ public class GenerationBenchmarks [Params(1, 16, 64)] public int Pairs { get; set; } + /// + /// Gets or sets a value indicating whether the build lists the generated namespace, which is what decides + /// between claiming each call site outright and offering an overload that competes for them all. + /// + [Params(false, true)] + public bool Intercept { get; set; } + /// /// Builds the corpus compilation once per parameter set. Loading a framework's worth of metadata /// references costs far more than a generation pass and is work the host build does once, so measuring it /// per iteration would bury what this benchmark is for. /// [GlobalSetup] - public void Setup() => _compilation = GeneratorHarness.BuildCompilation(GeneratorCorpus.Build(Pairs)); + public void Setup() => _compilation = GeneratorHarness.BuildCompilation(GeneratorCorpus.Build(Pairs), Intercept); /// Runs a whole cold generation: syntax scan, extraction, and emission. /// The number of generated characters, returned so the work cannot be optimized away. @@ -41,7 +48,7 @@ public int Generate() { // A fresh driver per iteration: a reused one would serve the next iteration from its caches and // measure the incremental path rather than the cold generation a consumer's build pays for. - var driver = GeneratorHarness.CreateDriver(); + var driver = GeneratorHarness.CreateDriver(Intercept); var result = driver.RunGenerators(_compilation).GetRunResult(); var characters = 0; diff --git a/src/benchmarks/ReactiveUI.Binding.Generator.Benchmarks/GeneratorHarness.cs b/src/benchmarks/ReactiveUI.Binding.Generator.Benchmarks/GeneratorHarness.cs index 4ae514b..0b5dee3 100644 --- a/src/benchmarks/ReactiveUI.Binding.Generator.Benchmarks/GeneratorHarness.cs +++ b/src/benchmarks/ReactiveUI.Binding.Generator.Benchmarks/GeneratorHarness.cs @@ -15,13 +15,34 @@ internal static class GeneratorHarness /// The assembly name given to the throwaway compilation the generator runs against. private const string CompilationAssemblyName = "Corpus"; + /// The feature a build lists interceptable namespaces under. + private const string InterceptorsNamespacesFeature = "InterceptorsNamespaces"; + + /// The namespace the generator emits interceptors into. + private const string InterceptorNamespace = "ReactiveUI.Binding.Generated.Interceptors"; + + /// + /// The parse options of a build that lists the generated namespace, which is what the shipped targets set + /// wherever the compiler can honour an interceptor emitted into it. + /// + /// Whether the build lists the namespace. + /// The parse options. + internal static CSharpParseOptions ParseOptions(bool intercept) + { + var parseOptions = new CSharpParseOptions(LanguageVersion.CSharp10); + + return intercept + ? parseOptions.WithFeatures([new KeyValuePair(InterceptorsNamespacesFeature, InterceptorNamespace)]) + : parseOptions; + } + /// Builds a compilation over the corpus source. /// The corpus source text. + /// Whether the build lists the generated namespace for interception. /// The compilation. - internal static CSharpCompilation BuildCompilation(string sourceText) + internal static CSharpCompilation BuildCompilation(string sourceText, bool intercept) { - var parseOptions = new CSharpParseOptions(LanguageVersion.CSharp10); - var syntaxTree = CSharpSyntaxTree.ParseText(sourceText, parseOptions); + var syntaxTree = CSharpSyntaxTree.ParseText(sourceText, ParseOptions(intercept)); var references = new List(Basic.Reference.Assemblies.Net80.References.All) { @@ -38,12 +59,13 @@ internal static CSharpCompilation BuildCompilation(string sourceText) } /// Creates a cold generator driver, carrying no caches from a previous run. + /// Whether the build lists the generated namespace for interception. /// The generator driver. [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal static GeneratorDriver CreateDriver() => + internal static GeneratorDriver CreateDriver(bool intercept) => CSharpGeneratorDriver.Create( [new BindingGenerator().AsSourceGenerator()], null, - new(LanguageVersion.CSharp10), + ParseOptions(intercept), null); } diff --git a/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/AnalyzerPackagingTests.cs b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/AnalyzerPackagingTests.cs new file mode 100644 index 0000000..724471f --- /dev/null +++ b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/AnalyzerPackagingTests.cs @@ -0,0 +1,186 @@ +// Copyright (c) 2019-2026 ReactiveUI Association Incorporated. All rights reserved. +// ReactiveUI Association Incorporated licenses this file to you under the MIT license. +// See the LICENSE file in the project root for full license information. + +using System.Buffers; +using System.Runtime.CompilerServices; +using System.Xml.Linq; + +namespace ReactiveUI.Binding.SourceGenerators.Tests; + +/// Tests the analyzer layout of the runtime packages. +/// +/// What the generator can do differs between compiler generations, so each runtime package ships one +/// analyzers/dotnet/roslyn<version>/cs slot per generation. Only the .NET SDK narrows that down to +/// one: its ResolvePackageAssets task picks the highest slot at or below $(CompilerApiVersion). A +/// build that never runs that task receives every slot at once, loads the generator twice and emits every +/// dispatch file twice, which the shipped targets prevent by dropping the slots the compiler is not served by. +/// The versions in the layout and in those targets therefore have to agree, which is asserted here rather than +/// left to a convention a later change can quietly break. +/// +public class AnalyzerPackagingTests +{ + /// The property in the shipped targets naming the oldest slot. + private const string MinimumCompilerVersionProperty = "_ReactiveUIBindingMinimumCompilerVersion"; + + /// The property in the shipped targets naming the slot that can intercept. + private const string InterceptorCompilerVersionProperty = "_ReactiveUIBindingInterceptorCompilerVersion"; + + /// The shipped targets, relative to the source root. + private const string TargetsPath = + "ReactiveUI.Binding.SourceGenerators/build/ReactiveUI.Binding.SourceGenerators.targets"; + + /// The separators a project spells a path with, whichever platform reads it. + private static readonly SearchValues PathSeparators = SearchValues.Create(['\\', '/']); + + /// The assemblies every slot has to carry. + private static readonly string[] SlotAssemblies = + [ + "ReactiveUI.Binding.Analyzer.dll", + "ReactiveUI.Binding.SourceGenerators.dll", + ]; + + /// The runtime packages that carry the analyzers. + private static readonly string[] RuntimePackages = + [ + "ReactiveUI.Binding/ReactiveUI.Binding.csproj", + "ReactiveUI.Binding.Reactive/ReactiveUI.Binding.Reactive.csproj", + ]; + + /// Every package ships the same slots, and the versions the targets name are exactly those. + /// A task representing the asynchronous test operation. + [Test] + public async Task PackagesShipTheSlotsTheTargetsSelectBetween() + { + var expected = string.Join( + ", ", + new[] + { + $"analyzers/dotnet/roslyn{ReadTargetsProperty(MinimumCompilerVersionProperty)}/cs", + $"analyzers/dotnet/roslyn{ReadTargetsProperty(InterceptorCompilerVersionProperty)}/cs", + }.Order(StringComparer.OrdinalIgnoreCase)); + + foreach (var package in RuntimePackages) + { + var slots = PackagedAnalyzers(package) + .Select(static analyzer => analyzer.PackagePath) + .Distinct(StringComparer.OrdinalIgnoreCase) + .Order(StringComparer.OrdinalIgnoreCase); + + // Joined rather than compared element-wise so a stray extra slot shows up in the diff. + await Assert.That(string.Join(", ", slots)).IsEqualTo(expected); + } + } + + /// A slot missing an assembly would leave that compiler generation without it. + /// A task representing the asynchronous test operation. + [Test] + public async Task EverySlotCarriesTheGeneratorAndTheAnalyzer() + { + foreach (var package in RuntimePackages) + { + foreach (var slot in PackagedAnalyzers(package).GroupBy(static analyzer => analyzer.PackagePath, StringComparer.OrdinalIgnoreCase)) + { + var assemblies = slot.Select(static analyzer => analyzer.FileName) + .Order(StringComparer.OrdinalIgnoreCase) + .ToArray(); + + await Assert.That(assemblies).IsEquivalentTo(SlotAssemblies); + } + } + } + + /// Two copies of one assembly in a package load as two generators, whichever folder they sit in. + /// A task representing the asynchronous test operation. + [Test] + public async Task NoAssemblyIsShippedTwiceWithinASlot() + { + foreach (var package in RuntimePackages) + { + var duplicated = PackagedAnalyzers(package) + .GroupBy(static analyzer => $"{analyzer.PackagePath}/{analyzer.FileName}", StringComparer.OrdinalIgnoreCase) + .Where(static group => group.Count() > 1) + .Select(static group => group.Key) + .ToArray(); + + await Assert.That(duplicated).IsEmpty(); + } + } + + /// The targets and the props both travel with every package that ships a slot. + /// A task representing the asynchronous test operation. + [Test] + public async Task EveryPackageShippingASlotAlsoShipsTheTargetsThatSelectIt() + { + foreach (var package in RuntimePackages) + { + var packed = PackedNoneItems(package) + .Select(static item => FileNameOf(item.FileName)) + .ToArray(); + + await Assert.That(packed).Contains("ReactiveUI.Binding.SourceGenerators.targets"); + await Assert.That(packed).Contains("ReactiveUI.Binding.SourceGenerators.props"); + } + } + + /// Reads a property value out of the shipped targets. + /// The property to read. + /// The declared value. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static string ReadTargetsProperty(string propertyName) => + XDocument.Load(SourcePath(TargetsPath)) + .Descendants() + .Where(element => element.Name.LocalName == propertyName) + .Select(static element => element.Value.Trim()) + .Single(); + + /// Reads the analyzer files a package packs, and the slot each lands in. + /// The package project, relative to the source root. + /// Each packed file, as the package path it lands in and the file it was packed from. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static IEnumerable<(string PackagePath, string FileName)> PackedNoneItems(string projectPath) => + XDocument.Load(SourcePath(projectPath)) + .Descendants() + .Where(static element => element.Name.LocalName == "None" + && string.Equals(element.Attribute("Pack")?.Value, "true", StringComparison.OrdinalIgnoreCase)) + .Select(static element => ( + PackagePath: element.Attribute("PackagePath")?.Value ?? string.Empty, + FileName: element.Attribute("Include")?.Value ?? string.Empty)); + + /// Reads the analyzer assemblies a package packs into a slot. + /// The package project, relative to the source root. + /// Each packed analyzer, as the slot it lands in and the file name it lands under. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static IEnumerable<(string PackagePath, string FileName)> PackagedAnalyzers(string projectPath) => + PackedNoneItems(projectPath) + .Where(static item => item.PackagePath.StartsWith("analyzers/", StringComparison.OrdinalIgnoreCase)) + .Select(static item => (item.PackagePath, FileNameOf(item.FileName))); + + /// + /// Reads the file name off a path a project wrote, which spells its separators the way MSBuild does rather + /// than the way the running platform does. + /// + /// The path as the project spells it. + /// The file name. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static string FileNameOf(string path) => + path[(path.AsSpan().LastIndexOfAny(PathSeparators) + 1)..]; + + /// Resolves a path under the source root, found by walking out of the test output directory. + /// The path relative to the source root. + /// The absolute path. + /// The source root was not found above the test output. + private static string SourcePath(string relativePath) + { + for (var directory = new DirectoryInfo(AppContext.BaseDirectory); directory is not null; directory = directory.Parent) + { + var candidate = Path.Combine(directory.FullName, relativePath); + if (File.Exists(candidate)) + { + return candidate; + } + } + + throw new InvalidOperationException($"'{relativePath}' was not found above '{AppContext.BaseDirectory}'."); + } +} From 18ffd68a457715984567f73c1f230d06d8dd924c Mon Sep 17 00:00:00 2001 From: Glenn Watson <5834289+glennawatson@users.noreply.github.com> Date: Wed, 9 Sep 2026 23:23:45 +1000 Subject: [PATCH 09/12] perf(generator): hand the feature snapshot to the parameter writer - Pass the language-feature snapshot as an argument rather than closing over it, so the writer each observation API supplies captures nothing and the compiler caches one delegate for the whole compilation. - Drop the dependency group entry for a package nothing references. --- .github/renovate.json | 1 - .../CodeGeneration/InterceptorEmitter.cs | 16 ++++++++++++++-- .../CodeGeneration/ObservationCodeGenerator.cs | 15 +++++++-------- .../CodeGeneration/WhenAnyCodeGenerator.cs | 13 +++++-------- 4 files changed, 26 insertions(+), 19 deletions(-) diff --git a/.github/renovate.json b/.github/renovate.json index 9434a6e..16b97fb 100644 --- a/.github/renovate.json +++ b/.github/renovate.json @@ -67,7 +67,6 @@ ], "groupName": "test tooling", "matchPackageNames": [ - "/^NSubstitute(\\.|$)/", "/^BenchmarkDotNet(\\.|$)/" ] } diff --git a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/InterceptorEmitter.cs b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/InterceptorEmitter.cs index 6ac4b26..e658a48 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/InterceptorEmitter.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/InterceptorEmitter.cs @@ -29,6 +29,12 @@ internal static class InterceptorEmitter /// Room for the declaration, which is a fixed block of text. private const int DeclarationCapacity = 640; + /// Writes the parameter list one API's members declare, closing it. + /// The string builder to append to. + /// The invocation whose types the parameters are written from. + /// The consumer compilation's language-feature snapshot. + internal delegate void ParameterListWriter(StringBuilder builder, InvocationInfo first, in LanguageFeatures features); + /// Writes the attribute that binds a generated method to one call site. /// The builder receiving the attribute line. /// The call site the compiler described. @@ -80,18 +86,24 @@ internal static string BuildAttributeDeclaration() /// The type group whose call sites are being claimed. /// The method name prefix the generated bodies carry. /// Names the body a call site reaches. + /// The consumer compilation's language-feature snapshot, handed to the writer. /// Writes the whole parameter list, closing it. /// /// The grouping and the attribute are the whole of what every API shares here, so they live in one place /// and each API supplies only the signature it is being called with. Call sites that reach one body are /// claimed by one method carrying an attribute each, which is what the attribute allowing repeats is for. + /// + /// The feature snapshot travels as an argument rather than being closed over, so the writer each API hands + /// in captures nothing and the compiler caches one delegate for the whole compilation. + /// /// internal static void GenerateInterceptors( StringBuilder builder, ObservationCodeGenerator.TypeGroup group, string methodPrefix, Func suffixOf, - Action appendParameterList) + in LanguageFeatures features, + ParameterListWriter appendParameterList) { foreach (var entry in GroupCallSitesByBody(group, suffixOf)) { @@ -105,7 +117,7 @@ internal static void GenerateInterceptors( _ = builder.Append(" internal static global::System.IObservable<").Append(first.ReturnTypeFullName) .Append("> __Intercept_").Append(methodPrefix).Append('_').Append(entry.Key).AppendLine("("); - appendParameterList(builder, first); + appendParameterList(builder, first, in features); _ = builder.Append(" => __").Append(methodPrefix).Append('_').Append(entry.Key) .Append("(objectToMonitor").Append(first.HasSelector ? ", selector" : string.Empty).AppendLine(");").AppendLine(); diff --git a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/ObservationCodeGenerator.cs b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/ObservationCodeGenerator.cs index 6c65c21..fff7d1a 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/ObservationCodeGenerator.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/ObservationCodeGenerator.cs @@ -943,7 +943,7 @@ private static void GenerateGroup( // Either claim each call site outright, or emit the overload that competes for them all. if (features.SupportsInterceptors) { - GenerateInterceptors(sb, group, methodPrefix, features.SupportsCallerArgExpr, features.StubHasExpressionParameters); + GenerateInterceptors(sb, group, methodPrefix, in features); } else { @@ -983,8 +983,7 @@ private static void GenerateGroup( /// The string builder to append to. /// The type group whose call sites are being claimed. /// The method name prefix. - /// Whether the target language version supports CallerArgumentExpression. - /// Whether the runtime stub declares the expression parameters. + /// The consumer compilation's language-feature snapshot. /// /// Call sites that share a source type and the same expressions produce one observation between them, and /// the attribute may be applied repeatedly, so they are claimed by a single method carrying one attribute @@ -996,18 +995,18 @@ private static void GenerateInterceptors( StringBuilder sb, TypeGroup group, string methodPrefix, - bool supportsCallerArgExpr, - bool stubHasExpressionParameters) => + in LanguageFeatures features) => InterceptorEmitter.GenerateInterceptors( sb, group, methodPrefix, MethodSuffix, - (builder, first) => AppendParameterList( + in features, + static (StringBuilder builder, InvocationInfo first, in LanguageFeatures snapshot) => AppendParameterList( builder, first, - supportsCallerArgExpr, - stubHasExpressionParameters, + snapshot.SupportsCallerArgExpr, + snapshot.StubHasExpressionParameters, first.PropertyPaths.Length, first.HasSelector)); diff --git a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/WhenAnyCodeGenerator.cs b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/WhenAnyCodeGenerator.cs index 88b01aa..0a12f30 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/WhenAnyCodeGenerator.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/CodeGeneration/WhenAnyCodeGenerator.cs @@ -248,21 +248,18 @@ private static void EmitGroup( { if (features.SupportsInterceptors) { - var supportsCallerArgExpr = features.SupportsCallerArgExpr; - var supportsNullable = features.SupportsNullable; - var stubHasExpressionParameters = features.StubHasExpressionParameters; - InterceptorEmitter.GenerateInterceptors( sb, group, Constants.WhenAnyMethodName, ObservationMethodSuffix, - (builder, first) => AppendParameterList( + in features, + static (StringBuilder builder, InvocationInfo first, in LanguageFeatures snapshot) => AppendParameterList( builder, first, - supportsCallerArgExpr, - supportsNullable, - stubHasExpressionParameters)); + snapshot.SupportsCallerArgExpr, + snapshot.SupportsNullable, + snapshot.StubHasExpressionParameters)); } else { From 2131f7cafb5dc88c8613d9befbfadc71315b6609 Mon Sep 17 00:00:00 2001 From: Glenn Watson <5834289+glennawatson@users.noreply.github.com> Date: Thu, 10 Sep 2026 00:08:51 +1000 Subject: [PATCH 10/12] test: pin the language version and cover every intercepted API - Name a defined language version instead of the moving preview, which the test-only Roslyn bump to 5.9.0 makes expressible. - Exercise all twelve binding APIs through the interceptor tier, so every emitter's interceptor path is emitted and its signature validated. - Cover RXUIBIND009 for a build that lists the generated namespace and for one that lists another package's. - Let a scenario hand parse options straight to the analyzer helper, which is how the interception opt-in is expressed. --- src/Directory.Packages.props | 20 +-- .../DispatchReachAnalyzerTests.cs | 38 +++++ .../Helpers/AnalyzerTestHelper.cs | 57 +++++++- .../Helpers/ExtractorValidationTests.cs | 2 +- .../InterceptedCallSiteTests.cs | 133 ++++++++++++++++++ .../SchedulerOverloadRuntimeTests.cs | 2 +- .../SchedulerBindingDispatchTests.cs | 4 +- src/tests/Shared/RoslynSymbolProbe.cs | 2 +- 8 files changed, 238 insertions(+), 20 deletions(-) diff --git a/src/Directory.Packages.props b/src/Directory.Packages.props index 818b961..59f3ef3 100644 --- a/src/Directory.Packages.props +++ b/src/Directory.Packages.props @@ -6,19 +6,19 @@ - 10.0.100 - 11.0.0-preview.7.26406.9 + 10.0.101 + 11.0.0-rc.1.26451.6 - 3.46.1 + 3.46.2 - + @@ -36,15 +36,17 @@ run against; it must stay at or above what their own tooling requires (Basic.Reference.Assemblies needs >= 4.11, BenchmarkDotNet needs >= 4.14). Running the generator on a newer Roslyn than it was built against is the - normal, supported direction. + normal, supported direction, and it is also what lets the suite name a + language version by its constant rather than as whatever preview happens + to be current. Microsoft.CodeAnalysis.Analyzers is a build-time-only analyzer package (the RS#### authoring rules) with no impact on the end-user-facing Roslyn version, so it tracks the latest stable release independently. --> - - + + @@ -64,8 +66,8 @@ - - + + diff --git a/src/tests/ReactiveUI.Binding.Analyzer.Tests/DispatchReachAnalyzerTests.cs b/src/tests/ReactiveUI.Binding.Analyzer.Tests/DispatchReachAnalyzerTests.cs index c5149bc..f856c12 100644 --- a/src/tests/ReactiveUI.Binding.Analyzer.Tests/DispatchReachAnalyzerTests.cs +++ b/src/tests/ReactiveUI.Binding.Analyzer.Tests/DispatchReachAnalyzerTests.cs @@ -5,6 +5,7 @@ using Microsoft.CodeAnalysis.CSharp; using ReactiveUI.Binding.Analyzer.Analyzers; using ReactiveUI.Binding.Analyzer.Tests.Helpers; +using ReactiveUI.Binding.SourceGenerators.Helpers; namespace ReactiveUI.Binding.Analyzer.Tests; @@ -122,6 +123,43 @@ public async Task FromCSharp10_IsNotReported() await Assert.That(diagnostics.Any(static d => d.Id == DiagnosticId)).IsFalse(); } + /// + /// The reach of a dispatch overload decides nothing where the call site is claimed outright, so a build + /// that lists the generated namespace is not warned - on the compiler that can honour the listing. On one + /// that cannot, the overloads are still what serves the call and the warning stands. + /// + /// A task representing the asynchronous test operation. + [Test] + public async Task ListingTheGeneratedNamespace_IsReportedOnlyWhereInterceptionCannotBeHonoured() + { + var diagnostics = await AnalyzerTestHelper.GetDiagnosticsAsync( + SourceIn(UnrelatedNamespace, GrantsInternals), + AnalyzerTestHelper.InterceptingParseOptionsFor(LanguageVersion.CSharp7_3), + RootNamespace); + + await Assert.That(diagnostics.Any(static d => d.Id == DiagnosticId)) + .IsEqualTo(!InterceptableLocationReader.IsSupported); + } + + /// + /// Listing another package's namespace is not this package's opt-in, so the call is reported whichever + /// compiler is building it. + /// + /// A task representing the asynchronous test operation. + [Test] + public async Task ListingAnotherPackagesNamespace_IsReported() + { + var parseOptions = new CSharpParseOptions(LanguageVersion.CSharp7_3) + .WithFeatures([new KeyValuePair("InterceptorsNamespaces", "Some.Other.Package")]); + + var diagnostics = await AnalyzerTestHelper.GetDiagnosticsAsync( + SourceIn(UnrelatedNamespace, GrantsInternals), + parseOptions, + RootNamespace); + + await Assert.That(diagnostics.Count(static d => d.Id == DiagnosticId)).IsEqualTo(1); + } + /// With no root namespace there is nowhere else to emit, so the shared namespace is kept. /// A task representing the asynchronous test operation. [Test] diff --git a/src/tests/ReactiveUI.Binding.Analyzer.Tests/Helpers/AnalyzerTestHelper.cs b/src/tests/ReactiveUI.Binding.Analyzer.Tests/Helpers/AnalyzerTestHelper.cs index 39a5d3a..d0a13be 100644 --- a/src/tests/ReactiveUI.Binding.Analyzer.Tests/Helpers/AnalyzerTestHelper.cs +++ b/src/tests/ReactiveUI.Binding.Analyzer.Tests/Helpers/AnalyzerTestHelper.cs @@ -24,7 +24,7 @@ public static class AnalyzerTestHelper [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Task> GetDiagnosticsAsync(string source) where TAnalyzer : DiagnosticAnalyzer, new() => - GetDiagnosticsAsync(source, null, null); + GetDiagnosticsAsync(source, (LanguageVersion?)null, null); /// /// Runs an analyzer against source compiled at a given language version, with the root namespace reported @@ -36,13 +36,31 @@ public static Task> GetDiagnosticsAsync(st /// The root namespace the build exposes, or null for none. /// The analyzer diagnostics. [SuppressMessage("Design", "SST2307:Type parameter is not inferable", Justification = "the analyzer under test is specified explicitly by the caller")] - public static async Task> GetDiagnosticsAsync( + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static Task> GetDiagnosticsAsync( string source, LanguageVersion? languageVersion, string? rootNamespace) + where TAnalyzer : DiagnosticAnalyzer, new() => + GetDiagnosticsAsync(source, ParseOptionsFor(languageVersion), rootNamespace); + + /// + /// Runs an analyzer against source parsed exactly as the caller asks, which is how a scenario reaches the + /// options a language version alone cannot express. + /// + /// The analyzer to run. + /// The source to analyze. + /// The options the source is parsed with, or null for the default. + /// The root namespace the build exposes, or null for none. + /// The analyzer diagnostics. + [SuppressMessage("Design", "SST2307:Type parameter is not inferable", Justification = "the analyzer under test is specified explicitly by the caller")] + public static async Task> GetDiagnosticsAsync( + string source, + CSharpParseOptions? parseOptions, + string? rootNamespace) where TAnalyzer : DiagnosticAnalyzer, new() { - var compilation = CreateCompilation(source, languageVersion); + var compilation = CreateCompilation(source, parseOptions); var analyzer = new TAnalyzer(); AnalyzerOptions? analyzerOptions = rootNamespace is null @@ -61,19 +79,46 @@ public static async Task> GetDiagnosticsAsyncThe parse options a build produces from a language version, which is what the tests usually name. + /// The language version, or null for the default. + /// The parse options, or null for the default. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal static CSharpParseOptions? ParseOptionsFor(LanguageVersion? languageVersion) => + languageVersion.HasValue ? new CSharpParseOptions(languageVersion.Value) : null; + + /// + /// The parse options of a build that has the package's targets on it, which list the generated namespace so + /// the compiler honours an interceptor emitted into it. + /// + /// The language version, or null for the default. + /// The parse options, carrying the opt-in. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal static CSharpParseOptions InterceptingParseOptionsFor(LanguageVersion? languageVersion) => + (ParseOptionsFor(languageVersion) ?? new CSharpParseOptions()) + .WithFeatures([new KeyValuePair( + SourceGenerators.Constants.InterceptorsNamespacesFeature, + SourceGenerators.Constants.InterceptorNamespace)]); + /// Creates a CSharpCompilation from the specified source code with required assembly references. /// The source code to compile into a CSharpCompilation. /// A CSharpCompilation object representing the compiled source code. [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal static CSharpCompilation CreateCompilation(string source) => CreateCompilation(source, null); + internal static CSharpCompilation CreateCompilation(string source) => CreateCompilation(source, (LanguageVersion?)null); /// Creates a compilation from source at a given language version. /// The source to compile. /// The language version, or null for the default. /// The compilation. - internal static CSharpCompilation CreateCompilation(string source, LanguageVersion? languageVersion) + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal static CSharpCompilation CreateCompilation(string source, LanguageVersion? languageVersion) => + CreateCompilation(source, ParseOptionsFor(languageVersion)); + + /// Creates a compilation from source parsed exactly as the caller asks. + /// The source to compile. + /// The options the source is parsed with, or null for the default. + /// The compilation. + internal static CSharpCompilation CreateCompilation(string source, CSharpParseOptions? parseOptions) { - var parseOptions = languageVersion.HasValue ? new CSharpParseOptions(languageVersion.Value) : null; var syntaxTree = CSharpSyntaxTree.ParseText(source, parseOptions); var addedPaths = new HashSet(StringComparer.OrdinalIgnoreCase); diff --git a/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/Helpers/ExtractorValidationTests.cs b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/Helpers/ExtractorValidationTests.cs index c777a3e..7a06fe3 100644 --- a/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/Helpers/ExtractorValidationTests.cs +++ b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/Helpers/ExtractorValidationTests.cs @@ -448,7 +448,7 @@ public static class {{className}} } } """, - LanguageVersion.Preview); + LanguageVersion.CSharp14); var outer = compilation.GetTypeByMetadataName(className) ?? throw new InvalidOperationException($"'{className}' did not compile."); diff --git a/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/InterceptedCallSiteTests.cs b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/InterceptedCallSiteTests.cs index f462592..cbbd1f2 100644 --- a/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/InterceptedCallSiteTests.cs +++ b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/InterceptedCallSiteTests.cs @@ -111,6 +111,139 @@ public void Bind() } """; + /// One call site per generated API, so every emitter's tier is settled by the same build. + private const string EveryApiScenario = """ + using System; + using System.ComponentModel; + using System.Threading.Tasks; + using System.Windows.Input; + using ReactiveUI.Binding; + + namespace TestApp + { + public class Person : INotifyPropertyChanged, INotifyPropertyChanging + { + public event PropertyChangedEventHandler PropertyChanged; + + public event PropertyChangingEventHandler PropertyChanging; + + public string Name { get; set; } + + public int Age { get; set; } + + public ICommand Save { get; set; } + + public IInteraction Confirm { get; set; } + + public IObservable Ticks { get; set; } + } + + public class Button + { + public event EventHandler Click; + + public string Text { get; set; } + } + + public class PersonView : INotifyPropertyChanged, IViewFor + { + public event PropertyChangedEventHandler PropertyChanged; + + public Person ViewModel { get; set; } + + object IViewFor.ViewModel + { + get { return ViewModel; } + set { ViewModel = (Person)value; } + } + + public Button SaveButton { get; set; } + + public string Display { get; set; } + + public string Summary { get; set; } + } + + public class Usage + { + public void Bind(Person person, PersonView view, IObservable names) + { + GC.KeepAlive(person.WhenChanged(x => x.Name)); + GC.KeepAlive(person.WhenChanging(x => x.Name)); + GC.KeepAlive(person.WhenAnyValue(x => x.Age)); + GC.KeepAlive(person.WhenAny(x => x.Name, c => c.Value)); + GC.KeepAlive(person.WhenAnyObservable(x => x.Ticks)); + + person.BindOneWay(view, x => x.Name, v => v.Display); + person.BindTwoWay(view, x => x.Name, v => v.Display); + view.OneWayBind(person, x => x.Name, v => v.Summary); + view.Bind(person, x => x.Name, v => v.Display); + names.BindTo(view, v => v.Display); + view.BindCommand(person, x => x.Save, v => v.SaveButton); + view.BindInteraction(person, x => x.Confirm, Handle); + } + + private static Task Handle(IInteractionContext context) + { + context.SetOutput(true); + return Task.CompletedTask; + } + } + } + """; + + /// The dispatch file each generated binding API emits. + /// + /// Named rather than matched on a suffix, because the view locator emits one too and it claims no call + /// site. An API that stopped emitting its file would drop silently out of a pattern. + /// + private static readonly string[] ApiDispatchFiles = + [ + "WhenChangedDispatch.g.cs", + "WhenChangingDispatch.g.cs", + "WhenAnyValueDispatch.g.cs", + "WhenAnyDispatch.g.cs", + "WhenAnyObservableDispatch.g.cs", + "BindOneWayDispatch.g.cs", + "BindTwoWayDispatch.g.cs", + "OneWayBindDispatch.g.cs", + "BindDispatch.g.cs", + "BindToDispatch.g.cs", + "BindCommandDispatch.g.cs", + "BindInteractionDispatch.g.cs", + ]; + + /// Every generated API is claimed the same way, so each emitter's tier follows one decision. + /// A task representing the asynchronous test operation. + [Test] + public async Task OptedInBuild_ClaimsEveryGeneratedApi() + { + var result = Generate(EveryApiScenario, LanguageVersion.CSharp10, optIn: true, RootNamespace); + + await Assert.That(result.CompilationErrors).IsEmpty(); + + foreach (var file in ApiDispatchFiles) + { + await Assert.That(result.GeneratedSources.ContainsKey(file)).IsTrue(); + + await Assert.That(result.GeneratedSources[file].Contains(InterceptsAttribute, StringComparison.Ordinal)) + .IsEqualTo(InterceptableLocationReader.IsSupported); + } + } + + /// Emission checks an interceptor against the call it replaces, so every API is emitted here. + /// A task representing the asynchronous test operation. + [Test] + public async Task OptedInBuild_EmitsWithEveryApiClaimed() + { + var result = Generate(EveryApiScenario, LanguageVersion.CSharp10, optIn: true, RootNamespace); + + var (_, context) = TestHelper.EmitAndLoad(result); + context.Unload(); + + await Assert.That(result.CompilationErrors).IsEmpty(); + } + /// A build listing the generated namespace gets the tier its compiler can honour. /// A task representing the asynchronous test operation. [Test] diff --git a/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/RuntimeExecution/SchedulerOverloadRuntimeTests.cs b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/RuntimeExecution/SchedulerOverloadRuntimeTests.cs index 00337e1..d46d7a1 100644 --- a/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/RuntimeExecution/SchedulerOverloadRuntimeTests.cs +++ b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/RuntimeExecution/SchedulerOverloadRuntimeTests.cs @@ -139,7 +139,7 @@ public async Task BindOneWay_WithASchedulerAlongsideUnscheduledCalls_KeepsItsOwn [Test] public async Task BindOneWay_WithASchedulerOnTheLatestLanguageVersion_DispatchesToGeneratedCode() { - var result = TestHelper.RunGenerator(SchedulerOverloadSource, LanguageVersion.Preview); + var result = TestHelper.RunGenerator(SchedulerOverloadSource, LanguageVersion.CSharp14); await result.CompilationSucceeds(); await result.GeneratedSourceContains(DispatchFileName, SchedulerParameter); diff --git a/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/SchedulerBindingDispatchTests.cs b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/SchedulerBindingDispatchTests.cs index 00a3b83..174dbca 100644 --- a/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/SchedulerBindingDispatchTests.cs +++ b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/SchedulerBindingDispatchTests.cs @@ -73,7 +73,7 @@ public async Task BindOneWay_WithAScheduler_GeneratesASchedulerCarryingOverload( { var result = TestHelper.RunGenerator( SchedulerBindingSource, - LanguageVersion.Preview, + LanguageVersion.CSharp14, "SchedulerProbe"); await result.CompilationSucceeds(); @@ -87,7 +87,7 @@ public async Task BindTwoWay_WithAScheduler_GeneratesASchedulerCarryingOverload( { var result = TestHelper.RunGenerator( SchedulerBindingSource, - LanguageVersion.Preview, + LanguageVersion.CSharp14, "SchedulerProbe"); await result.CompilationSucceeds(); diff --git a/src/tests/Shared/RoslynSymbolProbe.cs b/src/tests/Shared/RoslynSymbolProbe.cs index 9757f77..95b90b4 100644 --- a/src/tests/Shared/RoslynSymbolProbe.cs +++ b/src/tests/Shared/RoslynSymbolProbe.cs @@ -38,7 +38,7 @@ internal static CSharpCompilation Compile(string source) return CSharpCompilation.Create( "SymbolProbe", - [CSharpSyntaxTree.ParseText(source, new(LanguageVersion.Latest))], + [CSharpSyntaxTree.ParseText(source, new(LanguageVersion.CSharp14))], references, new(OutputKind.DynamicallyLinkedLibrary, allowUnsafe: true)); } From 9d3f78f752f3aaedd29c105491f3977620b4baba Mon Sep 17 00:00:00 2001 From: Glenn Watson <5834289+glennawatson@users.noreply.github.com> Date: Thu, 10 Sep 2026 01:02:24 +1000 Subject: [PATCH 11/12] test: reach the branches the interceptor tier leaves untaken - Ask one question where the generator and the analyzer each asked two: a compiler that cannot describe a call site answers before the opt-in is read, so neither has to pair the two tests itself. - Drop a null guard on a named type's namespace. A named type always has one, and the shapes that do not - an array, a pointer, a function pointer - are not named types and never arrive there. - Claim a BindCommand with an observable parameter and a WhenAnyObservable with a selector, so both interceptor bodies are emitted and compiled. - Read a call site the compiler declines to describe, which is the answer every caller already handles. - Say what the array case proves, rather than naming it for a guard it never reaches. --- .../Analyzers/DispatchReachAnalyzer.cs | 2 +- .../BindingGenerator.cs | 3 +- .../Helpers/BindToExtractor.cs | 7 ++-- .../Helpers/InterceptableLocationReader.cs | 20 +++++++++++ .../Helpers/BindToExtractorTests.cs | 23 +++++++++++-- .../InterceptableLocationReaderTests.cs | 34 +++++++++++++++++++ .../InterceptedCallSiteTests.cs | 4 +++ 7 files changed, 86 insertions(+), 7 deletions(-) diff --git a/src/ReactiveUI.Binding.Analyzer/Analyzers/DispatchReachAnalyzer.cs b/src/ReactiveUI.Binding.Analyzer/Analyzers/DispatchReachAnalyzer.cs index 2e1dcbf..54d01e4 100644 --- a/src/ReactiveUI.Binding.Analyzer/Analyzers/DispatchReachAnalyzer.cs +++ b/src/ReactiveUI.Binding.Analyzer/Analyzers/DispatchReachAnalyzer.cs @@ -80,7 +80,7 @@ internal static void AnalyzeInvocation(in OperationAnalysisContext context, stri // An interceptor replaces the call the compiler already bound, so nothing about it goes through // extension-method lookup and no namespace has to be in reach. Where this build emits interceptors // instead of overloads, the file's namespace stops deciding anything. - if (InterceptableLocationReader.IsSupported && InterceptableLocationReader.IsOptedIn(parseOptions)) + if (InterceptableLocationReader.IsInterceptionEnabled(parseOptions)) { return; } diff --git a/src/ReactiveUI.Binding.SourceGenerators/BindingGenerator.cs b/src/ReactiveUI.Binding.SourceGenerators/BindingGenerator.cs index 8ef6d8d..461a656 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/BindingGenerator.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/BindingGenerator.cs @@ -309,8 +309,7 @@ private static IncrementalValueProvider SelectLanguageFeatures // An interceptor claims its call site outright, so where one can be emitted none of the // placement below applies: there is no namespace for lookup to reach and no import to scope. - var supportsInterceptors = InterceptableLocationReader.IsSupported - && InterceptableLocationReader.IsOptedIn(parseOptions); + var supportsInterceptors = InterceptableLocationReader.IsInterceptionEnabled(parseOptions); var dispatchNamespace = supportsGlobalUsings ? SelectGeneratedNamespace(configOptions, compilation) diff --git a/src/ReactiveUI.Binding.SourceGenerators/Helpers/BindToExtractor.cs b/src/ReactiveUI.Binding.SourceGenerators/Helpers/BindToExtractor.cs index ea7cdec..da60b7d 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/Helpers/BindToExtractor.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/Helpers/BindToExtractor.cs @@ -119,11 +119,14 @@ internal static class BindToExtractor /// when it is that interface rather than one of the same name. /// /// Asked of the receiver and of each interface it implements, so the shape and the namespace are described - /// once. A type belonging to no namespace at all - an array or a pointer - answers no rather than throwing. + /// once. Both a lookalike declared elsewhere and one of the same name taking a different number of type + /// arguments answer no. A named type always belongs to a namespace, the global one at worst, so there is + /// none to account for; the shapes that belong to no namespace - an array, a pointer, a function pointer - + /// are not named types and never arrive here. /// private static bool IsFrameworkObservable(INamedTypeSymbol type) => type is { Name: "IObservable", TypeArguments.Length: 1 } - && type.ContainingNamespace?.ToDisplayString() == "System"; + && type.ContainingNamespace.ToDisplayString() == "System"; /// /// Scans the method parameters to detect the presence of a conversionHint parameter diff --git a/src/ReactiveUI.Binding.SourceGenerators/Helpers/InterceptableLocationReader.cs b/src/ReactiveUI.Binding.SourceGenerators/Helpers/InterceptableLocationReader.cs index b2c45a4..a4dff79 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/Helpers/InterceptableLocationReader.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/Helpers/InterceptableLocationReader.cs @@ -66,6 +66,26 @@ internal static bool IsOptedIn(ParseOptions parseOptions) return false; } + /// Determines whether an interceptor emitted for this compilation would be honoured. + /// The consumer's parse options, which carry the opt-in the build set. + /// when this build can describe a call site and the project listed the namespace. + /// + /// The one question both the generator and the analyzer ask before deciding whether the dispatch overloads + /// still matter, so the two answer it the same way. Which build is loaded settles the first half outright, + /// which is why the baseline never reads the options at all. + /// + internal static bool IsInterceptionEnabled(ParseOptions parseOptions) + { +#if ROSLYN_4_13 + return IsOptedIn(parseOptions); +#else + + // The baseline compiler describes no call site, so there is no interceptor for a listing to honour. + _ = parseOptions; + return false; +#endif + } + /// Describes a call site, when the host compiler can. /// The model the invocation was bound in. /// The call site. diff --git a/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/Helpers/BindToExtractorTests.cs b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/Helpers/BindToExtractorTests.cs index 1b83971..091a1e2 100644 --- a/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/Helpers/BindToExtractorTests.cs +++ b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/Helpers/BindToExtractorTests.cs @@ -61,12 +61,24 @@ public async Task GetObservableValueType_ReceiverIsLookalikeInterface_ReturnsNul public async Task GetObservableValueType_ReceiverImplementsLookalikeInterface_ReturnsNull() => await Assert.That(BindToExtractor.GetObservableValueType(FieldType("Probe.Lookalike"))).IsNull(); - /// A type belonging to no namespace at all is not taken for the framework interface. + /// + /// An array is no named type, so it is neither the interface nor a candidate for implementing it, and the + /// walk over its interfaces finds only the ones every array carries. + /// /// A task representing the asynchronous test operation. [Test] - public async Task GetObservableValueType_ReceiverHasNoContainingNamespace_ReturnsNull() => + public async Task GetObservableValueType_ReceiverIsArray_ReturnsNull() => await Assert.That(BindToExtractor.GetObservableValueType(FieldType("string[]"))).IsNull(); + /// + /// The framework interface takes exactly one type argument, so one of the same name and namespace taking + /// another number is a different contract and carries no element type to bind. + /// + /// A task representing the asynchronous test operation. + [Test] + public async Task GetObservableValueType_ReceiverIsSameNameWithTwoTypeArguments_ReturnsNull() => + await Assert.That(BindToExtractor.GetObservableValueType(FieldType("System.IObservable"))).IsNull(); + /// Resolves a type by declaring a field of it in a probe compilation. /// The type to resolve, as it is written in source. /// The resolved type symbol. @@ -75,6 +87,13 @@ private static ITypeSymbol FieldType(string declaredType) { var compilation = TestHelper.CreateCompilation( $$""" + namespace System + { + public interface IObservable + { + } + } + namespace Probe { public class Feed : System.IObservable diff --git a/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/Helpers/InterceptableLocationReaderTests.cs b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/Helpers/InterceptableLocationReaderTests.cs index ff5a03f..73176fb 100644 --- a/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/Helpers/InterceptableLocationReaderTests.cs +++ b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/Helpers/InterceptableLocationReaderTests.cs @@ -105,6 +105,40 @@ public static class Holder await Assert.That(location.IsAvailable).IsEqualTo(InterceptableLocationReader.IsSupported); } + /// + /// A call the compiler declines to describe reports that nothing was described, on either build. The + /// callee here is an element of an array of delegates rather than a name the compiler can attach an + /// interceptor to, which is a shape no interceptor can claim however new the compiler is. + /// + /// A task representing the asynchronous test operation. + [Test] + public async Task Read_CallSiteTheCompilerCannotDescribe_ReportsNothingDescribed() + { + const string Source = """ + namespace Probe + { + public static class Holder + { + public static System.Func[] Table = null!; + + public static string Read() => Table[0](1); + } + } + """; + + var compilation = TestHelper.CreateCompilation(Source, LanguageVersion.CSharp10); + var tree = compilation.SyntaxTrees.First(); + var root = await tree.GetRootAsync(); + var invocation = root.DescendantNodes().OfType().First(); + + var location = InterceptableLocationReader.Read( + compilation.GetSemanticModel(tree), + invocation, + CancellationToken.None); + + await Assert.That(location.IsAvailable).IsFalse(); + } + /// An undescribed call site carries no data, whichever build produced it. /// A task representing the asynchronous test operation. [Test] diff --git a/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/InterceptedCallSiteTests.cs b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/InterceptedCallSiteTests.cs index cbbd1f2..19c718a 100644 --- a/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/InterceptedCallSiteTests.cs +++ b/src/tests/ReactiveUI.Binding.SourceGenerators.Tests/InterceptedCallSiteTests.cs @@ -136,6 +136,8 @@ public class Person : INotifyPropertyChanged, INotifyPropertyChanging public IInteraction Confirm { get; set; } public IObservable Ticks { get; set; } + + public IObservable Pulses { get; set; } } public class Button @@ -173,6 +175,7 @@ public void Bind(Person person, PersonView view, IObservable names) GC.KeepAlive(person.WhenAnyValue(x => x.Age)); GC.KeepAlive(person.WhenAny(x => x.Name, c => c.Value)); GC.KeepAlive(person.WhenAnyObservable(x => x.Ticks)); + GC.KeepAlive(person.WhenAnyObservable(x => x.Ticks, x => x.Pulses, (a, b) => a + b)); person.BindOneWay(view, x => x.Name, v => v.Display); person.BindTwoWay(view, x => x.Name, v => v.Display); @@ -180,6 +183,7 @@ public void Bind(Person person, PersonView view, IObservable names) view.Bind(person, x => x.Name, v => v.Display); names.BindTo(view, v => v.Display); view.BindCommand(person, x => x.Save, v => v.SaveButton); + view.BindCommand(person, x => x.Save, v => v.SaveButton, names); view.BindInteraction(person, x => x.Confirm, Handle); } From 10773d0e656ab553df6f34b98bc89dd46fc46c17 Mon Sep 17 00:00:00 2001 From: Glenn Watson <5834289+glennawatson@users.noreply.github.com> Date: Thu, 10 Sep 2026 01:36:09 +1000 Subject: [PATCH 12/12] refactor(generator): name the widget whose property has two spellings - Compare the Android widget names in order rather than switching over them, so each comparison is a branch a caller can reach rather than a bucket of a jump table the compiler derived. - Give TimePicker's four names a predicate that says why there are four: the hour and the minute are spelled one way from API 23 and another before it. --- .../Observation/AndroidWidgetEvents.cs | 61 ++++++++++++++----- 1 file changed, 46 insertions(+), 15 deletions(-) diff --git a/src/ReactiveUI.Binding.SourceGenerators/Plugins/Observation/AndroidWidgetEvents.cs b/src/ReactiveUI.Binding.SourceGenerators/Plugins/Observation/AndroidWidgetEvents.cs index ea7345c..35cf665 100644 --- a/src/ReactiveUI.Binding.SourceGenerators/Plugins/Observation/AndroidWidgetEvents.cs +++ b/src/ReactiveUI.Binding.SourceGenerators/Plugins/Observation/AndroidWidgetEvents.cs @@ -25,34 +25,65 @@ internal static class AndroidWidgetEvents /// The property being observed. /// The event name, or when no widget reports that property. /// - /// A switch rather than a lookup table: the set is closed and known here, so the compiler turns it into a - /// jump over the name with nothing built at startup and nothing held for the life of the generator. + /// Compared in order rather than looked up: the set is closed and known here, so nothing is built at + /// startup and nothing is held for the life of the generator. /// - internal static string? FindChangeEvent(string propertyName) => propertyName switch + internal static string? FindChangeEvent(string propertyName) { // TextView and everything built on it. - "Text" => "TextChanged", + if (propertyName == "Text") + { + return "TextChanged"; + } // NumberPicker. - "Value" => "ValueChanged", + if (propertyName == "Value") + { + return "ValueChanged"; + } // RatingBar. - "Rating" => "RatingBarChange", + if (propertyName == "Rating") + { + return "RatingBarChange"; + } // CompoundButton, and so CheckBox, RadioButton and Switch. - "Checked" => "CheckedChange", + if (propertyName == "Checked") + { + return "CheckedChange"; + } // CalendarView. - "Date" => "DateChange", + if (propertyName == "Date") + { + return "DateChange"; + } // TabHost. - "CurrentTab" => "TabChanged", - - // TimePicker, whose hour and minute are named one way from API 23 and the other before it. - "Hour" or "Minute" or "CurrentHour" or "CurrentMinute" => "TimeChanged", + if (propertyName == "CurrentTab") + { + return "TabChanged"; + } // AdapterView, and so Spinner and ListView. - "SelectedItem" => "ItemSelected", - _ => null, - }; + if (propertyName == "SelectedItem") + { + return "ItemSelected"; + } + + return IsTimePickerField(propertyName) ? "TimeChanged" : null; + } + + /// Determines whether a name is one of the fields TimePicker reports its time change for. + /// The property being observed. + /// when TimePicker reports it. + /// + /// The one widget in the list that names the same two values two ways: the hour and the minute are + /// Hour and Minute from API 23, and CurrentHour and CurrentMinute before it. + /// All four report on the same event. + /// + private static bool IsTimePickerField(string propertyName) => + propertyName == "Hour" || propertyName == "Minute" + || propertyName == "CurrentHour" || propertyName == "CurrentMinute"; }