diff --git a/CLAUDE.md b/CLAUDE.md index fd3deaef..3811a22b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -170,9 +170,10 @@ All diagnostics use the `RXUISG` prefix. All suppressions use the `RXUISPR` pref | Class | ID range | Purpose | |-------|----------|---------| -| `PropertyToReactiveFieldAnalyzer` | RXUISG0016 | Suggests converting auto-properties to `[Reactive]` fields | +| `PropertyToReactiveFieldAnalyzer` | RXUISG0016 | Suggests converting auto-properties to `[Reactive]` properties | | `ReactiveAttributeMisuseAnalyzer` | RXUISG0020 | Detects `[Reactive]` on non-partial or non-partial-type members | -| `PropertyToReactiveFieldCodeFixProvider` | — | Converts auto-property → `[Reactive]` field | +| `ReactiveCommandAnalyzer` | RXUISG0002, RXUISG0008, RXUISG0021 | Reports `[ReactiveCommand]` methods and schedulers the generator skips | +| `PropertyToReactiveFieldCodeFixProvider` | — | Converts auto-property → `[Reactive]` partial property (C# 13+; C# 14+ with an initializer), else a `[Reactive]` field | | `ReactiveAttributeMisuseCodeFixProvider` | — | Fixes misuse of `[Reactive]` attribute | Suppressors silence noisy Roslyn/Roslynator diagnostics that are expected for generator-backed patterns (e.g. fields never read, methods that don't need to be static). @@ -187,6 +188,8 @@ Suppressors silence noisy Roslyn/Roslynator diagnostics that are expected for ge - The `ReactiveUI.SourceGenerators.Analyzers.CodeFixes` project owns every other `RXUISG*` diagnostic (such as RXUISG0016 and RXUISG0020) and all code fixers. - `DiagnosticDescriptors.cs` and related files are compiled from the shared Roslyn source via the linked `` items. +- When an analyzer reports what a generator skips, both compile the same rules file (for commands, + `ReactiveCommand/ReactiveCommandRules.cs`, linked into the code-fix project), so the two cannot disagree. ## Testing @@ -244,7 +247,9 @@ Analyzer and helper tests use direct `CSharpCompilation` / `CompilationWithAnaly ### Adding a New Analyzer Diagnostic 1. Add a `DiagnosticDescriptor` to `DiagnosticDescriptors.cs`. -2. Update `AnalyzerReleases.Unshipped.md`. +2. Record the rule in `AnalyzerReleases.Shipped.md`, under the release section for the current major version. Never use + `AnalyzerReleases.Unshipped.md`: it stays empty apart from its header, and every new, changed or removed rule is + recorded as released. 3. Implement the analyzer in `ReactiveUI.SourceGenerators.Analyzers.CodeFixes/`. 4. Add unit tests in `ReactiveUI.SourceGenerator.Tests/UnitTests/`. diff --git a/README.md b/README.md index 6123feff..acdef769 100644 --- a/README.md +++ b/README.md @@ -469,7 +469,13 @@ public partial class MyReactiveClass ``` ### Usage ReactiveCommand with ReactiveUI OutputScheduler + +`OutputScheduler` and `BackgroundScheduler` name a scheduler as you would write it in the class: a field, property or +parameterless method of the class, or a static one elsewhere, such as `RxSchedulers.MainThreadScheduler` with +`using ReactiveUI;`. A name that does not resolve to a scheduler is reported as RXUISG0021. + ```csharp +using ReactiveUI; using ReactiveUI.SourceGenerators; public partial class MyReactiveClass diff --git a/src/ReactiveUI.SourceGenerator.Tests/REACTIVECMD/RCMDG.Scheduler#TestNs.TestVM.RxCmds.verified.cs b/src/ReactiveUI.SourceGenerator.Tests/REACTIVECMD/RCMDG.Scheduler#TestNs.TestVM.RxCmds.verified.cs index d263e8f2..12ee08c5 100644 --- a/src/ReactiveUI.SourceGenerator.Tests/REACTIVECMD/RCMDG.Scheduler#TestNs.TestVM.RxCmds.verified.cs +++ b/src/ReactiveUI.SourceGenerator.Tests/REACTIVECMD/RCMDG.Scheduler#TestNs.TestVM.RxCmds.verified.cs @@ -11,7 +11,7 @@ public partial class TestVM private global::ReactiveUI.ReactiveCommand? _test1Command; [global::System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage] - public global::ReactiveUI.ReactiveCommand Test1Command { get => _test1Command ??= global::ReactiveUI.ReactiveCommand.Create(Test1); } + public global::ReactiveUI.ReactiveCommand Test1Command { get => _test1Command ??= global::ReactiveUI.ReactiveCommand.Create(Test1, outputScheduler: global::ReactiveUI.RxSchedulers.MainThreadScheduler); } } } #nullable restore diff --git a/src/ReactiveUI.SourceGenerator.Tests/UnitTests/PropertyToReactiveFieldCodeFixProviderTests.cs b/src/ReactiveUI.SourceGenerator.Tests/UnitTests/PropertyToReactiveFieldCodeFixProviderTests.cs index 3b912c2b..b21eb20e 100644 --- a/src/ReactiveUI.SourceGenerator.Tests/UnitTests/PropertyToReactiveFieldCodeFixProviderTests.cs +++ b/src/ReactiveUI.SourceGenerator.Tests/UnitTests/PropertyToReactiveFieldCodeFixProviderTests.cs @@ -9,6 +9,22 @@ namespace ReactiveUI.SourceGenerator.Tests; /// Unit tests for . public sealed class PropertyToReactiveFieldCodeFixProviderTests { + /// A documented property in a view model nested in another type, neither of them partial. + private const string PropertySource = """ + using ReactiveUI; + + namespace TestNs; + + public static class Outer + { + public class TestVM : ReactiveObject + { + /// Gets or sets a value indicating whether it is visible. + public bool IsVisible { get; set; } + } + } + """; + /// Validates the code fix provider advertises the expected diagnostic ID. /// A task that represents the asynchronous test operation. [Test] @@ -27,10 +43,38 @@ public async Task GetFixAllProviderReturnsBatchFixer() await Assert.That(provider.GetFixAllProvider()).IsNotNull(); } - /// Validates a public auto-property is converted to a private field annotated with [Reactive]. + /// Before C# 13 a public auto-property is converted to a private field annotated with [Reactive]. /// A task that represents the asynchronous test operation. [Test] - public async Task WhenApplyingFixThenConvertsPropertyToReactiveField() + public async Task BeforeCSharp13ConvertsPropertyToReactiveField() + { + var fixedSource = await ApplyFix(PropertySource, LanguageVersion.CSharp12); + + await Assert.That(fixedSource).Contains("[ReactiveUI.SourceGenerators.Reactive]"); + await Assert.That(fixedSource).Contains("private bool _isVisible"); + await Assert.That(fixedSource).DoesNotContain("public bool IsVisible"); + } + + /// From C# 13 the property becomes a [Reactive] partial property, in a type made partial, and generates. + /// A task that represents the asynchronous test operation. + [Test] + public async Task FromCSharp13ConvertsPropertyToReactivePartialProperty() + { + var fixedSource = await ApplyFix(PropertySource, LanguageVersion.CSharp13); + + await Assert.That(fixedSource).Contains("[ReactiveUI.SourceGenerators.Reactive]"); + await Assert.That(fixedSource).Contains("/// Gets or sets a value indicating whether it is visible."); + await Assert.That(fixedSource).Contains("public partial bool IsVisible { get; set; }"); + await Assert.That(fixedSource).Contains("public partial class TestVM"); + await Assert.That(fixedSource).Contains("public static partial class Outer"); + await Assert.That(fixedSource).DoesNotContain("_isVisible"); + await Assert.That(GetErrorsAfterGeneration(fixedSource, LanguageVersion.CSharp13)).IsEmpty(); + } + + /// A property with an initializer becomes a partial property only where C# allows it one. + /// A task that represents the asynchronous test operation. + [Test] + public async Task PropertyWithInitializerBecomesPartialOnlyWhereTheLanguageAllowsIt() { const string source = """ using ReactiveUI; @@ -39,23 +83,75 @@ namespace TestNs; public partial class TestVM : ReactiveObject { - public bool IsVisible { get; set; } + public string Name { get; set; } = "none"; } """; - var fixedSource = await ApplyFix(source); + var csharp13 = await ApplyFix(source, LanguageVersion.CSharp13); + var preview = await ApplyFix(source, LanguageVersion.Preview); + + await Assert.That(csharp13).Contains("private string _name = \"none\";"); + await Assert.That(preview).Contains("public partial string Name { get; set; } = \"none\";"); + } + + /// A diagnostic that is not on a property offers no fix. + /// A task that represents the asynchronous test operation. + [Test] + public async Task DiagnosticOutsideAPropertyOffersNoFix() + { + using var workspace = new AdhocWorkspace(); + var document = workspace.CurrentSolution + .AddProject("p", "p", LanguageNames.CSharp) + .AddDocument("t.cs", PropertySource); + var tree = await document.GetSyntaxTreeAsync(); + const string className = "Outer"; + var classLocation = Location.Create(tree!, new(PropertySource.IndexOf(className, StringComparison.Ordinal), className.Length)); + var actions = new List(); + var context = new CodeFixContext( + document, + Diagnostic.Create(new PropertyToReactiveFieldAnalyzer().SupportedDiagnostics[0], classLocation), + (action, _) => actions.Add(action), + CancellationToken.None); + + await new PropertyToReactiveFieldCodeFixProvider().RegisterCodeFixesAsync(context); + + await Assert.That(actions).IsEmpty(); + } + + /// Compiles fixed source with the [Reactive] generator and gets the errors. + /// The fixed source. + /// The C# version. + /// The errors, one per line. + private static string GetErrorsAfterGeneration(string source, LanguageVersion languageVersion) + { + var parseOptions = CSharpParseOptions.Default.WithLanguageVersion(languageVersion); + var compilation = CSharpCompilation.Create( + "CodeFixGenerationTests", + [CSharpSyntaxTree.ParseText(source, parseOptions)], + TestCompilationReferences.CreateDefault(), + new(OutputKind.DynamicallyLinkedLibrary)); + _ = CSharpGeneratorDriver.Create([new ReactiveGenerator().AsSourceGenerator()], parseOptions: parseOptions) + .RunGeneratorsAndUpdateCompilation(compilation, out var output, out _); + + var errors = new List(); + foreach (var diagnostic in output.GetDiagnostics()) + { + if (diagnostic.Severity == DiagnosticSeverity.Error) + { + errors.Add(diagnostic.ToString()); + } + } - await Assert.That(fixedSource.Contains("[ReactiveUI.SourceGenerators.Reactive]", StringComparison.Ordinal)).IsTrue(); - await Assert.That(fixedSource.Contains("private bool _isVisible", StringComparison.Ordinal)).IsTrue(); - await Assert.That(fixedSource.Contains("public bool IsVisible", StringComparison.Ordinal)).IsFalse(); + return string.Join(Environment.NewLine, errors); } /// Applies the code fix to the supplied source. /// The source to fix. + /// The C# version the project uses. /// A task that resolves to the fixed source. - private static async Task ApplyFix(string source) + private static async Task ApplyFix(string source, LanguageVersion languageVersion) { - var tree = CSharpSyntaxTree.ParseText(source, CSharpParseOptions.Default.WithLanguageVersion(LanguageVersion.CSharp13)); + var tree = CSharpSyntaxTree.ParseText(source, CSharpParseOptions.Default.WithLanguageVersion(languageVersion)); var analyzer = new PropertyToReactiveFieldAnalyzer(); var compilation = CSharpCompilation.Create( @@ -70,7 +166,7 @@ private static async Task ApplyFix(string source) using var workspace = new AdhocWorkspace(); var project = workspace.CurrentSolution .AddProject("p", "p", LanguageNames.CSharp) - .WithParseOptions(CSharpParseOptions.Default.WithLanguageVersion(LanguageVersion.CSharp13)) + .WithParseOptions(CSharpParseOptions.Default.WithLanguageVersion(languageVersion)) .WithCompilationOptions(new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); foreach (var reference in TestCompilationReferences.CreateDefault()) diff --git a/src/ReactiveUI.SourceGenerator.Tests/UnitTests/ReactiveCommandAnalyzerTests.cs b/src/ReactiveUI.SourceGenerator.Tests/UnitTests/ReactiveCommandAnalyzerTests.cs new file mode 100644 index 00000000..5e3c745e --- /dev/null +++ b/src/ReactiveUI.SourceGenerator.Tests/UnitTests/ReactiveCommandAnalyzerTests.cs @@ -0,0 +1,216 @@ +// 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.SourceGenerator.Tests; + +/// Unit tests for . +public sealed class ReactiveCommandAnalyzerTests +{ + /// A method signature the generator cannot use. + private const string InvalidSignatureId = "RXUISG0002"; + + /// An async void command method. + private const string AsyncVoidId = "RXUISG0008"; + + /// A scheduler that does not resolve. + private const string UnresolvedSchedulerId = "RXUISG0021"; + + /// Commands covering every rule, each reported at the text it names. + private const string Source = """ + using System.Threading; + using System.Threading.Tasks; + using ReactiveUI; + using ReactiveUI.Primitives.Concurrency; + using ReactiveUI.SourceGenerators; + + namespace Analyzed; + + public partial class ViewModel : ReactiveObject + { + private readonly ISequencer _sequencer = null!; + + private object NotAScheduler => null!; + + [ReactiveCommand] + private void TwoParameters(int first, int second) { } + + [ReactiveCommand] + private Task ParameterAndToken(int value, CancellationToken token) => Task.FromResult(value); + + [ReactiveCommand] + private Task TwoParametersAndToken(int first, int second, CancellationToken token) => Task.CompletedTask; + + [ReactiveCommand] + private async void FireAndForget() => await Task.Yield(); + + [ReactiveCommand] + private async Task Awaited() => await Task.Yield(); + + [ReactiveCommand(OutputScheduler = "RxSchedulers.MainThreadScheduler", BackgroundScheduler = "RxSchedulers.TaskpoolScheduler")] + private void ShortNames() { } + + [ReactiveCommand(OutputScheduler = nameof(_sequencer))] + private void Member() { } + + [ReactiveCommand(OutputScheduler = "Missing")] + private void MissingScheduler() { } + + [ReactiveCommand(BackgroundScheduler = nameof(NotAScheduler))] + private void WrongType() { } + + private readonly Holder _holder = new(); + + [ReactiveCommand(OutputScheduler = "_holder.Sequencer")] + private void AnotherObjectsMember() { } + + [ReactiveCommand(OutputScheduler = "not (valid")] + private void Unparsable() { } + + [ReactiveCommand(OutputScheduler = "RxSchedulers")] + private void TypeName() { } + + [System.Obsolete] + private void NotACommand() { } + } + + public sealed class Holder + { + public ISequencer Sequencer => null!; + } + """; + + /// Only a method with more than one parameter besides a CancellationToken is reported. + /// A task that represents the asynchronous test operation. + [Test] + public async Task ReportsMethodsWithMoreThanOneCommandParameter() + { + var reported = await GetReported(InvalidSignatureId); + + await Assert.That(reported).IsEquivalentTo(["TwoParameters", "TwoParametersAndToken"]); + } + + /// Only an async void method is reported; an async Task method is not. + /// A task that represents the asynchronous test operation. + [Test] + public async Task ReportsAsyncVoidMethods() + { + var reported = await GetReported(AsyncVoidId); + + await Assert.That(reported).IsEquivalentTo(["FireAndForget"]); + } + + /// Only schedulers the generator cannot resolve are reported; short names and members are not. + /// A task that represents the asynchronous test operation. + [Test] + public async Task ReportsSchedulersThatDoNotResolve() + { + var reported = await GetReported(UnresolvedSchedulerId); + + await Assert.That(reported).IsEquivalentTo(["\"Missing\"", "nameof(NotAScheduler)", "\"_holder.Sequencer\"", "\"not (valid\"", "\"RxSchedulers\""]); + } + + /// Without ReactiveUI's command factories no scheduler can resolve, so every one is reported. + /// Source standing in for ReactiveUI: none, or a command type without factories. + /// A task that represents the asynchronous test operation. + [Test] + [Arguments("")] + [Arguments("namespace ReactiveUI { public static class ReactiveCommand { public static void Create() { } } }")] + public async Task ReportsSchedulersWithoutReactiveUiCommandFactories(string reactiveUiStub) + { + var source = reactiveUiStub + """ + + namespace Analyzed + { + public partial class ViewModel + { + public static object Anything => null!; + + [ReactiveUI.SourceGenerators.ReactiveCommand(OutputScheduler = "Anything")] + private void Command() { } + } + } + """; + var references = TestCompilationReferences.CreateForAssemblies(typeof(object).Assembly, typeof(ReactiveCommandAttribute).Assembly); + + var reported = await GetReported(UnresolvedSchedulerId, source, references); + + await Assert.That(reported).IsEquivalentTo(["\"Anything\""]); + } + + /// A scheduler argument whose value is not a string, or does not compile, is left to the compiler. + /// A task that represents the asynchronous test operation. + [Test] + public async Task IgnoresSchedulerArgumentsThatAreNotStrings() + { + const string source = """ + using ReactiveUI.SourceGenerators; + + public partial class ViewModel + { + [ReactiveCommand(OutputScheduler = 42)] + private void Command() { } + + [ReactiveCommand(OutputScheduler = UndefinedName)] + private void Unbound() { } + } + """; + + var reported = await GetReported(UnresolvedSchedulerId, source, TestCompilationReferences.CreateDefault()); + + await Assert.That(reported).IsEmpty(); + } + + /// A null analysis context is rejected. + /// A task that represents the asynchronous test operation. + [Test] + public async Task InitializeRejectsANullContext() => + await Assert.That(static () => new ReactiveCommandAnalyzer().Initialize(null!)).Throws(); + + /// The analyzer reports its three rules. + /// A task that represents the asynchronous test operation. + [Test] + public async Task SupportsItsRules() + { + var ids = new List(); + foreach (var descriptor in new ReactiveCommandAnalyzer().SupportedDiagnostics) + { + ids.Add(descriptor.Id); + } + + await Assert.That(ids).IsEquivalentTo([InvalidSignatureId, AsyncVoidId, UnresolvedSchedulerId]); + } + + /// Analyzes and gets the text each diagnostic with an ID is reported at. + /// The diagnostic ID. + /// The source text at each reported location. + private static Task> GetReported(string id) => GetReported(id, Source, TestCompilationReferences.CreateDefault()); + + /// Analyzes a source and gets the text each diagnostic with an ID is reported at. + /// The diagnostic ID. + /// The source to analyze. + /// The compilation's references. + /// The source text at each reported location. + private static async Task> GetReported(string id, string source, ImmutableArray references) + { + var tree = CSharpSyntaxTree.ParseText(source, CSharpParseOptions.Default.WithLanguageVersion(LanguageVersion.CSharp13)); + var compilation = CSharpCompilation.Create( + nameof(ReactiveCommandAnalyzerTests), + [tree], + references, + new(OutputKind.DynamicallyLinkedLibrary)); + var diagnostics = await compilation.WithAnalyzers([new ReactiveCommandAnalyzer()]).GetAnalyzerDiagnosticsAsync(); + var text = await tree.GetTextAsync(); + + var reported = new List(); + foreach (var diagnostic in diagnostics) + { + if (diagnostic.Id == id) + { + reported.Add(text.ToString(diagnostic.Location.SourceSpan)); + } + } + + return reported; + } +} diff --git a/src/ReactiveUI.SourceGenerator.Tests/UnitTests/ReactiveUiIntegrationTests.cs b/src/ReactiveUI.SourceGenerator.Tests/UnitTests/ReactiveUiIntegrationTests.cs index 7bb63809..5dca950c 100644 --- a/src/ReactiveUI.SourceGenerator.Tests/UnitTests/ReactiveUiIntegrationTests.cs +++ b/src/ReactiveUI.SourceGenerator.Tests/UnitTests/ReactiveUiIntegrationTests.cs @@ -177,6 +177,15 @@ private void MethodSchedulerCommand() { } [ReactiveCommand(OutputScheduler = null)] private void NullScheduler() { } + [ReactiveCommand(OutputScheduler = "RxSchedulers.MainThreadScheduler")] + private void ShortScheduler() { } + + [ReactiveCommand(BackgroundScheduler = "RxSchedulers.TaskpoolScheduler")] + private void ShortBackground() { } + + [ReactiveCommand(OutputScheduler = "MethodScheduler()")] + private void CalledScheduler() { } + [ReactiveCommand(AccessModifier = PropertyAccessModifier.InternalProtected)] private void ProtectedInternal() { } @@ -410,10 +419,14 @@ public async Task ReactiveCommandOptionsGenerateOnlyValidArgumentsAndModifiers() await Assert.That(generatedSource.Contains("ReactiveCommand.Create(FieldCanExecute, _fieldCanRun)", StringComparison.Ordinal)).IsTrue(); await Assert.That(generatedSource.Contains("ReactiveCommand.Create(PropertyCanExecute, PropertyCanRun)", StringComparison.Ordinal)).IsTrue(); await Assert.That(generatedSource.Contains("ReactiveCommand.Create(MethodCanExecute, MethodCanRun())", StringComparison.Ordinal)).IsTrue(); - await Assert.That(generatedSource.Contains("outputScheduler: _fieldSequencer", StringComparison.Ordinal)).IsTrue(); - await Assert.That(generatedSource.Contains("outputScheduler: PropertySequencer", StringComparison.Ordinal)).IsTrue(); - await Assert.That(generatedSource.Contains("outputScheduler: MethodScheduler()", StringComparison.Ordinal)).IsTrue(); - await Assert.That(generatedSource.Contains("outputScheduler: global::ReactiveUI.RxSchedulers.MainThreadScheduler", StringComparison.Ordinal)).IsTrue(); + await Assert.That(generatedSource).Contains("outputScheduler: _fieldSequencer"); + await Assert.That(generatedSource).Contains("outputScheduler: PropertySequencer"); + await Assert.That(generatedSource).Contains("outputScheduler: MethodScheduler()"); + await Assert.That(generatedSource).Contains("outputScheduler: global::ReactiveUI.RxSchedulers.MainThreadScheduler"); + await Assert.That(generatedSource).Contains("Create(ShortScheduler, outputScheduler: global::ReactiveUI.RxSchedulers.MainThreadScheduler)"); + await Assert.That(generatedSource).Contains( + "CreateRunInBackground(ShortBackground, canExecute: null, backgroundScheduler: global::ReactiveUI.RxSchedulers.TaskpoolScheduler)"); + await Assert.That(generatedSource).Contains("Create(CalledScheduler, outputScheduler: MethodScheduler())"); await Assert.That(generatedSource.Contains("protected internal global::ReactiveUI.ReactiveCommand", StringComparison.Ordinal)).IsTrue(); await Assert.That(generatedSource.Contains("private protected global::ReactiveUI.ReactiveCommand", StringComparison.Ordinal)).IsTrue(); await Assert.That(generatedSource.Contains("SaveCommand", StringComparison.Ordinal)).IsTrue(); diff --git a/src/ReactiveUI.SourceGenerators.Analyzers.CodeFixes/AnalyzerReleases.Shipped.md b/src/ReactiveUI.SourceGenerators.Analyzers.CodeFixes/AnalyzerReleases.Shipped.md index cf40e4fc..f21a6dcb 100644 --- a/src/ReactiveUI.SourceGenerators.Analyzers.CodeFixes/AnalyzerReleases.Shipped.md +++ b/src/ReactiveUI.SourceGenerators.Analyzers.CodeFixes/AnalyzerReleases.Shipped.md @@ -9,6 +9,16 @@ Rule ID | Category | Severity | Notes --------|----------|----------|------- RXUISG0016 | ReactiveUI.SourceGenerators.PropertyToReactiveFieldCodeFixProvider | Info | See https://www.reactiveui.net/docs/handbook/view-models/boilerplate-code.html +## Release 4.0 + +### New Rules + +Rule ID | Category | Severity | Notes +--------|----------|----------|------- +RXUISG0002 | ReactiveUI.SourceGenerators.CodeFixers.ReactiveCommandAnalyzer | Error | A `[ReactiveCommand]` method takes more than one parameter besides a CancellationToken +RXUISG0008 | ReactiveUI.SourceGenerators.CodeFixers.ReactiveCommandAnalyzer | Error | A `[ReactiveCommand]` method is async void +RXUISG0021 | ReactiveUI.SourceGenerators.CodeFixers.ReactiveCommandAnalyzer | Warning | A `[ReactiveCommand]` OutputScheduler or BackgroundScheduler does not resolve to a scheduler + ## Rules Shipped in ReactiveUI.SourceGenerators diff --git a/src/ReactiveUI.SourceGenerators.Analyzers.CodeFixes/Diagnostics/CodeFixers/PropertyToReactiveFieldCodeFixProvider.cs b/src/ReactiveUI.SourceGenerators.Analyzers.CodeFixes/Diagnostics/CodeFixers/PropertyToReactiveFieldCodeFixProvider.cs index b37f34b8..49fbffe2 100644 --- a/src/ReactiveUI.SourceGenerators.Analyzers.CodeFixes/Diagnostics/CodeFixers/PropertyToReactiveFieldCodeFixProvider.cs +++ b/src/ReactiveUI.SourceGenerators.Analyzers.CodeFixes/Diagnostics/CodeFixers/PropertyToReactiveFieldCodeFixProvider.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; using System.Collections.Generic; using System.Collections.Immutable; using System.Threading.Tasks; @@ -10,15 +11,30 @@ using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.Formatting; using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory; using static ReactiveUI.SourceGenerators.CodeFixers.Diagnostics.DiagnosticDescriptors; namespace ReactiveUI.SourceGenerators.CodeFixers; -/// Provides fixes that convert eligible properties to reactive fields. +/// Provides fixes that convert eligible properties to [Reactive] properties. +/// +/// Where the project's language version allows it, the property becomes a [Reactive] partial property, and its +/// containing types become partial: the property stays the one the code declares, so other generators can bind to it. +/// Otherwise it becomes a [Reactive] field, from which the generator writes the property. +/// [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(PropertyToReactiveFieldCodeFixProvider))] public sealed class PropertyToReactiveFieldCodeFixProvider : CodeFixProvider { + /// The C# version that allows partial properties. + private const int PartialPropertyLanguageVersion = 1300; + + /// The C# version that allows a partial property to have an initializer. + private const int PartialPropertyInitializerLanguageVersion = 1400; + + /// The first Roslyn release whose generator builds generate partial properties. + private static readonly Version PartialPropertyRoslynVersion = new(4, 14); + /// Gets a list of diagnostic IDs that this provider can fix. public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(PropertyToReactiveFieldRule.Id); @@ -34,12 +50,73 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) { var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); var diagnostic = context.Diagnostics[0]; - var propertyDeclaration = FindPropertyDeclaration(root, diagnostic.Location.SourceSpan.Start); - if (propertyDeclaration is null) + if (root is null || FindPropertyDeclaration(root, diagnostic.Location.SourceSpan.Start) is not { } propertyDeclaration) + { + return; + } + + if (CanUsePartialProperty(propertyDeclaration)) { + const string title = "Convert to Reactive partial property"; + var partialRoot = ConvertToPartialProperty(root, propertyDeclaration); + context.RegisterCodeFix( + CodeAction.Create(title, _ => Task.FromResult(context.Document.WithSyntaxRoot(partialRoot)), title), + diagnostic); return; } + const string fieldTitle = "Convert to Reactive field"; + var fieldRoot = root.ReplaceNode(propertyDeclaration, ConvertToField(propertyDeclaration)); + context.RegisterCodeFix( + CodeAction.Create(fieldTitle, _ => Task.FromResult(context.Document.WithSyntaxRoot(fieldRoot)), fieldTitle), + diagnostic); + } + + /// Determines whether a property can become a [Reactive] partial property. + /// The property. + /// + /// when the language version allows the partial property, and its initializer if it has one, + /// and the running compiler's generator build generates partial properties. + /// + private static bool CanUsePartialProperty(PropertyDeclarationSyntax propertyDeclaration) + { + var languageVersion = (int)((CSharpParseOptions)propertyDeclaration.SyntaxTree.Options).LanguageVersion; + var requiredVersion = propertyDeclaration.Initializer is null ? PartialPropertyLanguageVersion : PartialPropertyInitializerLanguageVersion; + return languageVersion >= requiredVersion + && typeof(CSharpSyntaxTree).Assembly.GetName().Version >= PartialPropertyRoslynVersion; + } + + /// Makes a property a [Reactive] partial property, and its containing types partial. + /// The syntax root. + /// The property. + /// The updated syntax root. + private static SyntaxNode ConvertToPartialProperty(SyntaxNode root, PropertyDeclarationSyntax propertyDeclaration) + { + var nodes = new List { propertyDeclaration }; + foreach (var type in propertyDeclaration.Ancestors()) + { + if (type is TypeDeclarationSyntax typeDeclaration && !typeDeclaration.Modifiers.Any(SyntaxKind.PartialKeyword)) + { + nodes.Add(typeDeclaration); + } + } + + // The nodes are the property and its containing types, so anything but the property is a type. + return root.ReplaceNodes(nodes, static (_, rewritten) => rewritten is PropertyDeclarationSyntax property + ? property + .WithoutLeadingTrivia() + .AddModifiers(Token(SyntaxKind.PartialKeyword)) + .AddAttributeLists(ReactiveAttributeList()) + .WithLeadingTrivia(property.GetLeadingTrivia()) + .WithAdditionalAnnotations(Formatter.Annotation) + : ((TypeDeclarationSyntax)rewritten).AddModifiers(Token(SyntaxKind.PartialKeyword))); + } + + /// Makes a [Reactive] field for a property. + /// The property. + /// The field declaration. + private static FieldDeclarationSyntax ConvertToField(PropertyDeclarationSyntax propertyDeclaration) + { var propertyName = propertyDeclaration.Identifier.Text; var fieldName = $"_{char.ToLowerInvariant(propertyName[0])}{propertyName.Remove(0, 1)}"; var attributeSyntaxes = new List(); @@ -48,32 +125,29 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) attributeSyntaxes.Add(AttributeList(attributeList.Attributes)); } - attributeSyntaxes.Add(AttributeList(SingletonSeparatedList(Attribute(IdentifierName("ReactiveUI.SourceGenerators.Reactive"))))); + attributeSyntaxes.Add(ReactiveAttributeList()); - var fieldDeclaration = FieldDeclaration( + return FieldDeclaration( VariableDeclaration(propertyDeclaration.Type) .WithVariables(SingletonSeparatedList( VariableDeclarator(fieldName).WithInitializer(propertyDeclaration.Initializer)))) .WithAttributeLists(new(attributeSyntaxes)) .WithLeadingTrivia(propertyDeclaration.GetLeadingTrivia()) .WithModifiers(TokenList(Token(SyntaxKind.PrivateKeyword))); - var newRoot = root?.ReplaceNode(propertyDeclaration, fieldDeclaration); - - context.RegisterCodeFix( - CodeAction.Create( - "Convert to Reactive field", - _ => Task.FromResult(context.Document.WithSyntaxRoot(newRoot!)), - "Convert to Reactive field"), - diagnostic); } + /// Makes the [ReactiveUI.SourceGenerators.Reactive] attribute list. + /// The attribute list. + private static AttributeListSyntax ReactiveAttributeList() => + AttributeList(SingletonSeparatedList(Attribute(IdentifierName("ReactiveUI.SourceGenerators.Reactive")))); + /// Finds the property declaration containing a source position. /// The syntax-tree root. /// The source position. /// The containing property declaration, when present. - private static PropertyDeclarationSyntax? FindPropertyDeclaration(SyntaxNode? root, int position) + private static PropertyDeclarationSyntax? FindPropertyDeclaration(SyntaxNode root, int position) { - for (SyntaxNode? current = root?.FindToken(position).Parent; current is not null; current = current.Parent) + for (var current = root.FindToken(position).Parent; current is not null; current = current.Parent) { if (current is PropertyDeclarationSyntax propertyDeclaration) { diff --git a/src/ReactiveUI.SourceGenerators.Analyzers.CodeFixes/Diagnostics/CodeFixers/ReactiveCommandAnalyzer.cs b/src/ReactiveUI.SourceGenerators.Analyzers.CodeFixes/Diagnostics/CodeFixers/ReactiveCommandAnalyzer.cs new file mode 100644 index 00000000..0c2b3551 --- /dev/null +++ b/src/ReactiveUI.SourceGenerators.Analyzers.CodeFixes/Diagnostics/CodeFixers/ReactiveCommandAnalyzer.cs @@ -0,0 +1,118 @@ +// 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; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using ReactiveUI.SourceGenerators.Helpers; +using static ReactiveUI.SourceGenerators.CodeFixers.Diagnostics.DiagnosticDescriptors; + +namespace ReactiveUI.SourceGenerators.CodeFixers; + +/// Reports [ReactiveCommand] methods and options the command generator cannot use. +/// +/// The generator skips what it cannot generate from without a word, so these are reported here. The rules are the +/// generator's own, from the both compile. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class ReactiveCommandAnalyzer : DiagnosticAnalyzer +{ + /// The metadata name of [ReactiveCommand]. + private const string ReactiveCommandAttributeName = "ReactiveUI.SourceGenerators.ReactiveCommandAttribute"; + + /// Gets the diagnostics this analyzer reports. + public override ImmutableArray SupportedDiagnostics => + ImmutableArray.Create(InvalidReactiveCommandMethodSignatureRule, AsyncVoidReactiveCommandMethodRule, UnresolvedReactiveCommandSchedulerRule); + + /// Registers the method analysis. + /// The analysis context. + public override void Initialize(AnalysisContext context) + { + if (context is null) + { + throw new ArgumentNullException(nameof(context)); + } + + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + context.RegisterSyntaxNodeAction(static nodeContext => AnalyzeMethod(in nodeContext), SyntaxKind.MethodDeclaration); + } + + /// Analyzes a method declaration that carries an attribute. + /// The syntax-node analysis context. + private static void AnalyzeMethod(in SyntaxNodeAnalysisContext context) + { + if (context.Node is not MethodDeclarationSyntax { AttributeLists.Count: > 0 } declaration + || context.SemanticModel.GetDeclaredSymbol(declaration, context.CancellationToken) is not { } method + || FindReactiveCommandAttribute(method) is not { } attribute) + { + return; + } + + if (ReactiveCommandRules.HasTooManyParameters(method)) + { + context.ReportDiagnostic(Diagnostic.Create(InvalidReactiveCommandMethodSignatureRule, declaration.Identifier.GetLocation(), method.Name)); + } + + if (ReactiveCommandRules.IsAsyncVoid(method)) + { + context.ReportDiagnostic(Diagnostic.Create(AsyncVoidReactiveCommandMethodRule, declaration.Identifier.GetLocation(), method.Name)); + } + + if (attribute.ApplicationSyntaxReference?.GetSyntax(context.CancellationToken) is not AttributeSyntax { ArgumentList: { } arguments }) + { + return; + } + + foreach (var argument in arguments.Arguments) + { + AnalyzeScheduler(context, method, attribute, argument); + } + } + + /// Reports a scheduler argument whose name the generator cannot resolve. + /// The syntax-node analysis context. + /// The attributed method. + /// The [ReactiveCommand] attribute. + /// One of the attribute's arguments. + private static void AnalyzeScheduler(in SyntaxNodeAnalysisContext context, IMethodSymbol method, AttributeData attribute, AttributeArgumentSyntax argument) + { + var argumentName = argument.NameEquals?.Name.Identifier.ValueText; + if (argumentName is not (ReactiveCommandRules.OutputSchedulerArgument or ReactiveCommandRules.BackgroundSchedulerArgument) + || GetNamedString(attribute, argumentName) is not { } name + || ReactiveCommandRules.TryResolveScheduler(context.SemanticModel, argument.SpanStart, method.ContainingType, name, out _)) + { + return; + } + + context.ReportDiagnostic(Diagnostic.Create(UnresolvedReactiveCommandSchedulerRule, argument.Expression.GetLocation(), argumentName, name)); + } + + /// Finds a method's [ReactiveCommand] attribute. + /// The method. + /// The attribute, or when the method has none. + private static AttributeData? FindReactiveCommandAttribute(IMethodSymbol method) + { + foreach (var attribute in method.GetAttributes()) + { + if (attribute.AttributeClass?.ToDisplayString() == ReactiveCommandAttributeName) + { + return attribute; + } + } + + return null; + } + + /// Gets the string value of an attribute's named argument. + /// The attribute. + /// The argument's name. + /// The value, or when the argument is absent or not a string. + private static string? GetNamedString(AttributeData attribute, string name) => + attribute.NamedArguments.FirstOrDefault(argument => argument.Key == name).Value.Value as string; +} diff --git a/src/ReactiveUI.SourceGenerators.Analyzers.CodeFixes/Diagnostics/DiagnosticDescriptors.cs b/src/ReactiveUI.SourceGenerators.Analyzers.CodeFixes/Diagnostics/DiagnosticDescriptors.cs index f0a19d26..1b3c0d85 100644 --- a/src/ReactiveUI.SourceGenerators.Analyzers.CodeFixes/Diagnostics/DiagnosticDescriptors.cs +++ b/src/ReactiveUI.SourceGenerators.Analyzers.CodeFixes/Diagnostics/DiagnosticDescriptors.cs @@ -12,7 +12,7 @@ internal static class DiagnosticDescriptors /// The property to field rule. internal static readonly DiagnosticDescriptor PropertyToReactiveFieldRule = new( id: "RXUISG0016", - title: "Property To Reactive Field, change to `[Reactive]` private type _fieldName;", + title: "Property can be a `[Reactive]` property", messageFormat: "Replace the property with a INPC Reactive Property for ReactiveUI", category: typeof(PropertyToReactiveFieldAnalyzer).FullName, defaultSeverity: DiagnosticSeverity.Info, @@ -20,6 +20,40 @@ internal static class DiagnosticDescriptors description: "Used to create a Read Write INPC Reactive Property for ReactiveUI, annotated with `[Reactive]`.", helpLinkUri: "https://www.reactiveui.net/docs/handbook/view-models/boilerplate-code.html"); + /// A [ReactiveCommand] method takes more parameters than a command can pass it. + internal static readonly DiagnosticDescriptor InvalidReactiveCommandMethodSignatureRule = new( + id: "RXUISG0002", + title: "Invalid [ReactiveCommand] method signature", + messageFormat: "`{0}` takes more than one parameter besides a CancellationToken, so no command is generated for it", + category: typeof(ReactiveCommandAnalyzer).FullName, + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true, + description: "A command passes at most one parameter to its method; a task-returning method may also take a CancellationToken.", + helpLinkUri: "https://www.reactiveui.net/docs/handbook/view-models/boilerplate-code.html"); + + /// A [ReactiveCommand] method is async void. + internal static readonly DiagnosticDescriptor AsyncVoidReactiveCommandMethodRule = new( + id: "RXUISG0008", + title: "[ReactiveCommand] method is async void", + messageFormat: "`{0}` is async void, so the command cannot await it or observe its exceptions; return Task instead", + category: typeof(ReactiveCommandAnalyzer).FullName, + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true, + description: "A command generated from an async void method completes before the method does, and its exceptions escape the command.", + helpLinkUri: "https://www.reactiveui.net/docs/handbook/view-models/boilerplate-code.html"); + + /// A [ReactiveCommand] scheduler name does not resolve to a scheduler. + internal static readonly DiagnosticDescriptor UnresolvedReactiveCommandSchedulerRule = new( + id: "RXUISG0021", + title: "[ReactiveCommand] scheduler does not resolve", + messageFormat: "`{0}` = \"{1}\" does not name a scheduler the command can use, so the command is generated without it", + category: typeof(ReactiveCommandAnalyzer).FullName, + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "A scheduler is a field, property or parameterless method of the command's type, or a static one elsewhere " + + "such as RxSchedulers.MainThreadScheduler, whose type is the scheduler type ReactiveUI's commands take.", + helpLinkUri: "https://www.reactiveui.net/docs/handbook/view-models/boilerplate-code.html"); + /// The `[Reactive]` attribute was used on a property, but required `partial` modifiers are missing. internal static readonly DiagnosticDescriptor ReactiveAttributeRequiresPartialRule = new( id: "RXUISG0020", diff --git a/src/ReactiveUI.SourceGenerators.Analyzers.CodeFixes/ReactiveUI.SourceGenerators.Analyzers.CodeFixes.csproj b/src/ReactiveUI.SourceGenerators.Analyzers.CodeFixes/ReactiveUI.SourceGenerators.Analyzers.CodeFixes.csproj index 32c534a5..69f62a31 100644 --- a/src/ReactiveUI.SourceGenerators.Analyzers.CodeFixes/ReactiveUI.SourceGenerators.Analyzers.CodeFixes.csproj +++ b/src/ReactiveUI.SourceGenerators.Analyzers.CodeFixes/ReactiveUI.SourceGenerators.Analyzers.CodeFixes.csproj @@ -30,6 +30,8 @@ + + diff --git a/src/ReactiveUI.SourceGenerators.Roslyn/AnalyzerReleases.Shipped.md b/src/ReactiveUI.SourceGenerators.Roslyn/AnalyzerReleases.Shipped.md index b0409285..a887dfd5 100644 --- a/src/ReactiveUI.SourceGenerators.Roslyn/AnalyzerReleases.Shipped.md +++ b/src/ReactiveUI.SourceGenerators.Roslyn/AnalyzerReleases.Shipped.md @@ -23,6 +23,30 @@ RXUISG0017 | ReactiveUI.SourceGenerators.ObservableAsPropertyFromObservableGener RXUISG0018 | ReactiveUI.SourceGenerators.ObservableAsPropertyFromObservableGenerator/ReactiveGenerator | Error | See https://www.reactiveui.net/docs/handbook/view-models/boilerplate-code.html RXUISG0019 | ReactiveUI.SourceGenerators.BindableDerivedListGenerator | Error | See https://www.reactiveui.net/docs/handbook/view-models/boilerplate-code.html +## Release 4.0 + +### Removed Rules + +Rule ID | Category | Severity | Notes +--------|----------|----------|------- +RXUISG0001 | ReactiveUI.SourceGenerators.UnsupportedCSharpLanguageVersionAnalyzer | Error | No longer reported +RXUISG0002 | ReactiveUI.SourceGenerators.ReactiveCommandGenerator | Error | Reported by the code-fix project's ReactiveCommandAnalyzer +RXUISG0003 | ReactiveUI.SourceGenerators.ReactiveCommandGenerator | Error | No longer reported +RXUISG0004 | ReactiveUI.SourceGenerators.ReactiveCommandGenerator | Error | No longer reported +RXUISG0005 | ReactiveUI.SourceGenerators.ReactiveCommandGenerator | Error | No longer reported +RXUISG0006 | ReactiveUI.SourceGenerators.ReactiveCommandGenerator | Error | No longer reported +RXUISG0007 | ReactiveUI.SourceGenerators.ReactiveCommandGenerator | Error | No longer reported +RXUISG0008 | ReactiveUI.SourceGenerators.AsyncVoidReturningReactiveCommandMethodAnalyzer | Error | Reported by the code-fix project's ReactiveCommandAnalyzer +RXUISG0014 | ReactiveUI.SourceGenerators.ObservableAsPropertyGenerator | Error | `[ObservableAsProperty]` moved to ReactiveUI.Binding +RXUISG0017 | ReactiveUI.SourceGenerators.ObservableAsPropertyFromObservableGenerator | Error | `[ObservableAsProperty]` moved to ReactiveUI.Binding + +### Changed Rules + +Rule ID | New Category | New Severity | Old Category | Old Severity | Notes +--------|--------------|--------------|--------------|--------------|------- +RXUISG0012 | ReactiveUI.SourceGenerators | Error | ReactiveUI.SourceGenerators.ObservableAsPropertyGenerator | Error | Reports an unresolved attribute forwarded by any generator +RXUISG0013 | ReactiveUI.SourceGenerators | Error | ReactiveUI.SourceGenerators.ObservableAsPropertyGenerator | Error | Reports an invalid attribute expression forwarded by any generator + ## Rules Shipped in ReactiveUI.SourceGenerators diff --git a/src/ReactiveUI.SourceGenerators.Roslyn/AnalyzerReleases.Unshipped.md b/src/ReactiveUI.SourceGenerators.Roslyn/AnalyzerReleases.Unshipped.md index 4a2c0632..11aac41e 100644 --- a/src/ReactiveUI.SourceGenerators.Roslyn/AnalyzerReleases.Unshipped.md +++ b/src/ReactiveUI.SourceGenerators.Roslyn/AnalyzerReleases.Unshipped.md @@ -1,16 +1,2 @@ ; Unshipped analyzer releases ; https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md - -### Removed Rules - -Rule ID | Category | Severity | Notes ---------|----------|----------|------- -RXUISG0014 | ReactiveUI.SourceGenerators.ObservableAsPropertyGenerator | Error | `[ObservableAsProperty]` moved to ReactiveUI.Binding -RXUISG0017 | ReactiveUI.SourceGenerators.ObservableAsPropertyFromObservableGenerator | Error | `[ObservableAsProperty]` moved to ReactiveUI.Binding - -### Changed Rules - -Rule ID | New Category | New Severity | Old Category | Old Severity | Notes ---------|--------------|--------------|--------------|--------------|------- -RXUISG0012 | ReactiveUI.SourceGenerators | Error | ReactiveUI.SourceGenerators.ObservableAsPropertyGenerator | Error | Reports an unresolved attribute forwarded by any generator -RXUISG0013 | ReactiveUI.SourceGenerators | Error | ReactiveUI.SourceGenerators.ObservableAsPropertyGenerator | Error | Reports an invalid attribute expression forwarded by any generator diff --git a/src/ReactiveUI.SourceGenerators.Roslyn/ReactiveCommand/ReactiveCommandGenerator.Execute.cs b/src/ReactiveUI.SourceGenerators.Roslyn/ReactiveCommand/ReactiveCommandGenerator.Execute.cs index 154db432..504b3c41 100644 --- a/src/ReactiveUI.SourceGenerators.Roslyn/ReactiveCommand/ReactiveCommandGenerator.Execute.cs +++ b/src/ReactiveUI.SourceGenerators.Roslyn/ReactiveCommand/ReactiveCommandGenerator.Execute.cs @@ -45,15 +45,9 @@ public partial class ReactiveCommandGenerator /// The attribute property used to specify the can-execute member. private const string CanExecute = "CanExecute"; - /// The attribute property used to specify the output scheduler. - private const string OutputScheduler = "OutputScheduler"; - /// The attribute property used to request background execution. private const string RunInBackground = "RunInBackground"; - /// The attribute property used to specify the background scheduler. - private const string BackgroundScheduler = "BackgroundScheduler"; - /// The method that starts a task-returning command on the thread pool. private const string TaskRun = "global::System.Threading.Tasks.Task.Run"; @@ -115,10 +109,9 @@ context.TargetSymbol is IMethodSymbol methodSymbol token.ThrowIfCancellationRequested(); TryGetCanExecuteExpressionType(methodSymbol, attributeData, out var canExecuteObservableName, out var canExecuteTypeInfo); token.ThrowIfCancellationRequested(); - var integration = context.SemanticModel.Compilation.GetReactiveUiIntegration(); - TryGetScheduler(methodSymbol, attributeData, OutputScheduler, integration, out var outputScheduler); + var outputScheduler = GetScheduler(context, methodSymbol, attributeData, ReactiveCommandRules.OutputSchedulerArgument); token.ThrowIfCancellationRequested(); - TryGetScheduler(methodSymbol, attributeData, BackgroundScheduler, integration, out var backgroundScheduler); + var backgroundScheduler = GetScheduler(context, methodSymbol, attributeData, ReactiveCommandRules.BackgroundSchedulerArgument); var runInBackground = backgroundScheduler is not null || attributeData.GetNamedArgument(RunInBackground); token.ThrowIfCancellationRequested(); var accessModifier = GetAccessModifier(attributeData); @@ -532,106 +525,28 @@ private static void TryGetCanExecuteExpressionType( canExecuteTypeInfo = null; } - /// Gets a configured scheduler, when its member is valid. + /// Gets the scheduler an attribute property names, as an expression generated code can use. + /// The generator attribute context. /// The attributed command method. /// The command attribute. /// The attribute property naming the scheduler. - /// The selected ReactiveUI API surface. - /// The scheduler expression, when valid. - private static void TryGetScheduler( + /// The scheduler expression, or when none is named or the name does not resolve. + /// A name that does not resolve is reported by the command analyzer, which applies the same rules. + private static string? GetScheduler( + in GeneratorAttributeSyntaxContext context, IMethodSymbol methodSymbol, AttributeData attributeData, - string argumentName, - ReactiveUiIntegration integration, - out string? schedulerExpression) - { - if (!attributeData.TryGetNamedArgument(argumentName, out string? scheduler) || scheduler is null) - { - schedulerExpression = null; - return; - } - - if (IsReactiveUiScheduler(scheduler)) - { - schedulerExpression = scheduler; - return; - } - - if (!TryGetSingleMember(methodSymbol.ContainingType!.GetAllMembers(scheduler), out var schedulerSymbol)) - { - schedulerExpression = null; - return; - } - - _ = TryGetSchedulerFromSymbol(schedulerSymbol, integration.Api, out schedulerExpression); - } - - /// Determines whether a scheduler expression names a built-in ReactiveUI scheduler. - /// The scheduler expression. - /// when the expression is a built-in scheduler. - private static bool IsReactiveUiScheduler(string scheduler) => - scheduler is "global::ReactiveUI.RxSchedulers.MainThreadScheduler" - or "global::ReactiveUI.RxSchedulers.TaskpoolScheduler" - or "global::ReactiveUI.Reactive.RxSchedulers.MainThreadScheduler" - or "global::ReactiveUI.Reactive.RxSchedulers.TaskpoolScheduler"; - - /// Gets the only symbol from a candidate sequence. - /// The candidate symbols. - /// The only symbol, when present. - /// when exactly one symbol exists. - private static bool TryGetSingleMember(IEnumerable symbols, [NotNullWhen(true)] out ISymbol? symbol) - { - symbol = null; - foreach (var candidate in symbols) - { - if (symbol is not null) - { - return false; - } - - symbol = candidate; - } - - return symbol is not null; - } - - /// Validates a candidate scheduler symbol and gets its expression. - /// The candidate scheduler symbol. - /// The selected ReactiveUI API. - /// The scheduler expression, when valid. - /// when the symbol is a supported scheduler. - private static bool TryGetSchedulerFromSymbol( - ISymbol schedulerSymbol, - ReactiveUiApi api, - [NotNullWhen(true)] out string? schedulerExpression) - { - switch (schedulerSymbol) - { - case IFieldSymbol fieldSymbol when fieldSymbol.Type.IsSchedulerType(api): - { - schedulerExpression = fieldSymbol.Name; - return true; - } - - case IPropertySymbol { GetMethod: not null } propertySymbol when propertySymbol.Type.IsSchedulerType(api): - { - schedulerExpression = propertySymbol.Name; - return true; - } - - case IMethodSymbol methodSymbol when methodSymbol.ReturnType.IsSchedulerType(api): - { - schedulerExpression = $"{methodSymbol.Name}()"; - return true; - } - - default: - { - schedulerExpression = null; - return false; - } - } - } + string argumentName) => + attributeData.TryGetNamedArgument(argumentName, out string? name) + && name is not null + && ReactiveCommandRules.TryResolveScheduler( + context.SemanticModel, + attributeData.ApplicationSyntaxReference?.Span.Start ?? context.TargetNode.SpanStart, + methodSymbol.ContainingType, + name, + out var expression) + ? expression + : null; /// Gets the expression type for the can execute logic, if possible. /// The can execute member symbol (either a method or a property). diff --git a/src/ReactiveUI.SourceGenerators.Roslyn/ReactiveCommand/ReactiveCommandRules.cs b/src/ReactiveUI.SourceGenerators.Roslyn/ReactiveCommand/ReactiveCommandRules.cs new file mode 100644 index 00000000..a3ae3813 --- /dev/null +++ b/src/ReactiveUI.SourceGenerators.Roslyn/ReactiveCommand/ReactiveCommandRules.cs @@ -0,0 +1,206 @@ +// 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.Diagnostics.CodeAnalysis; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace ReactiveUI.SourceGenerators.Helpers; + +/// The rules that decide what a [ReactiveCommand] method generates. +/// +/// The command generator and the command analyzer both compile this file, so a method the generator skips, or a +/// scheduler it cannot use, is exactly what the analyzer reports. +/// +internal static class ReactiveCommandRules +{ + /// The attribute property naming the scheduler a command delivers its results on. + internal const string OutputSchedulerArgument = "OutputScheduler"; + + /// The attribute property naming the scheduler a synchronous command runs on. + internal const string BackgroundSchedulerArgument = "BackgroundScheduler"; + + /// The metadata name of the CancellationToken a task-returning method may take. + private const string CancellationTokenMetadataName = "System.Threading.CancellationToken"; + + /// Determines whether a method takes more command parameters than a command can pass. + /// The attributed method. + /// when more than one parameter is not a CancellationToken. + internal static bool HasTooManyParameters(IMethodSymbol method) + { + var commandParameters = 0; + foreach (var parameter in method.Parameters) + { + if (parameter.Type.ToDisplayString() != CancellationTokenMetadataName) + { + commandParameters++; + } + } + + return commandParameters > 1; + } + + /// Determines whether a method is async void, whose failures a command cannot observe. + /// The attributed method. + /// when the method is async and returns . + internal static bool IsAsyncVoid(IMethodSymbol method) => method.IsAsync && method.ReturnsVoid; + + /// Resolves the scheduler an attribute names to an expression generated code can use. + /// The semantic model of the attributed method's tree. + /// A position in the attribute, where the name is bound. + /// The type declaring the command. + /// The scheduler as written: a member name, or an expression such as RxSchedulers.MainThreadScheduler. + /// The scheduler expression for generated code, when the name resolves. + /// when the name is a field, property or parameterless method of the scheduler type. + /// + /// The name is bound as an expression at the attribute, so the file's usings and the type's members apply. A member + /// of the command's type or a base type is written by name; any other member must be static and is written fully + /// qualified, because generated code does not carry the file's usings. The scheduler type is the one the ReactiveUI + /// command factories take, so each ReactiveUI flavour gets its own. + /// + internal static bool TryResolveScheduler( + SemanticModel semanticModel, + int position, + INamedTypeSymbol commandType, + string name, + [NotNullWhen(true)] out string? expression) + { + expression = null; + var schedulerType = GetSchedulerType(semanticModel.Compilation); + if (schedulerType is null || BindMember(semanticModel, position, name) is not { } symbol + || !TryGetSchedulerMember(symbol, out var memberType, out var callSuffix) + || !semanticModel.Compilation.ClassifyCommonConversion(memberType, schedulerType).IsImplicit) + { + return false; + } + + if (IsSameOrBaseOf(symbol.ContainingType, commandType)) + { + expression = symbol.Name + callSuffix; + return true; + } + + if (!symbol.IsStatic) + { + return false; + } + + expression = $"{symbol.ContainingType.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)}.{symbol.Name}{callSuffix}"; + return true; + } + + /// Binds a name as an expression at a position. + /// The semantic model of the tree holding the position. + /// The position the name is bound at. + /// The name, or an expression such as RxSchedulers.MainThreadScheduler. + /// The member the name binds to, or . + private static ISymbol? BindMember(SemanticModel semanticModel, int position, string name) + { + var syntax = SyntaxFactory.ParseExpression(name); + if (syntax.ContainsDiagnostics) + { + return null; + } + + // A parameterless call binds as its method, and is written back as a call. + if (syntax is InvocationExpressionSyntax { ArgumentList.Arguments.Count: 0 } invocation) + { + syntax = invocation.Expression; + } + + // A method named without a call binds as a method group: one accessible overload is the method it names. + var info = semanticModel.GetSpeculativeSymbolInfo(position, syntax, SpeculativeBindingOption.BindAsExpression); + return info.Symbol + ?? (info is { CandidateReason: not CandidateReason.Inaccessible, CandidateSymbols: [IMethodSymbol method] } ? method : null); + } + + /// Gets the scheduler type the ReactiveUI command factories take. + /// The compilation. + /// The type of a command factory's outputScheduler parameter, or without ReactiveUI. + private static ITypeSymbol? GetSchedulerType(Compilation compilation) + { + var command = compilation.GetTypeByMetadataName("ReactiveUI.Reactive.ReactiveCommand") + ?? compilation.GetTypeByMetadataName("ReactiveUI.ReactiveCommand"); + if (command is null) + { + return null; + } + + foreach (var member in command.GetMembers("Create")) + { + if (member is not IMethodSymbol method) + { + continue; + } + + foreach (var parameter in method.Parameters) + { + if (parameter.Name == "outputScheduler") + { + return parameter.Type.WithNullableAnnotation(NullableAnnotation.NotAnnotated); + } + } + } + + return null; + } + + /// Gets the type a scheduler member provides, and how generated code reads it. + /// The bound member. + /// The member's type, or its return type for a method. + /// The text written after the member's name: a call for a method, otherwise nothing. + /// when the member is a field, a readable property, or a parameterless method. + private static bool TryGetSchedulerMember( + ISymbol? symbol, + [NotNullWhen(true)] out ITypeSymbol? memberType, + out string callSuffix) + { + callSuffix = string.Empty; + switch (symbol) + { + case IFieldSymbol field: + { + memberType = field.Type; + return true; + } + + case IPropertySymbol { GetMethod: not null } property: + { + memberType = property.Type; + return true; + } + + case IMethodSymbol { Parameters.IsEmpty: true, ReturnsVoid: false, MethodKind: MethodKind.Ordinary } method: + { + memberType = method.ReturnType; + callSuffix = "()"; + return true; + } + + default: + { + memberType = null; + return false; + } + } + } + + /// Determines whether a type is another type or one of its base types. + /// The possible base type. + /// The type. + /// when is or a base of it. + private static bool IsSameOrBaseOf(INamedTypeSymbol? candidate, INamedTypeSymbol type) + { + for (INamedTypeSymbol? current = type; current is not null; current = current.BaseType) + { + if (SymbolEqualityComparer.Default.Equals(current.OriginalDefinition, candidate?.OriginalDefinition)) + { + return true; + } + } + + return false; + } +}