From cb093915a7ba332e4cc4659b85404020eb6f9842 Mon Sep 17 00:00:00 2001 From: danielmarbach Date: Mon, 5 Jun 2023 09:28:08 +0200 Subject: [PATCH 1/4] Reproduction test analyzer cancellation token propagation for fluent style methods --- .../ForwardCancellationTokenFixerTests.cs | 50 ++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/src/NServiceBus.Core.Analyzer.Tests.Common/ForwardCancellationToken/ForwardCancellationTokenFixerTests.cs b/src/NServiceBus.Core.Analyzer.Tests.Common/ForwardCancellationToken/ForwardCancellationTokenFixerTests.cs index 77627d9ef5..4692f8201c 100644 --- a/src/NServiceBus.Core.Analyzer.Tests.Common/ForwardCancellationToken/ForwardCancellationTokenFixerTests.cs +++ b/src/NServiceBus.Core.Analyzer.Tests.Common/ForwardCancellationToken/ForwardCancellationTokenFixerTests.cs @@ -42,6 +42,54 @@ public Task Bar(IMessageHandlerContext context) return Assert(original, expected); } + [Test] + public Task Fluent() + { + var original = + @"using NServiceBus; +using System; +using System.Threading; +using System.Threading.Tasks; +public class Foo +{ + public Task Bar(IMessageHandlerContext context) + { + var someReturnValue = ""someValue""; + return + Include(() => someReturnValue) + .Include(() => someReturnValue) + .FindSingle(); + } + + Foo Include(Func action) => this; + + Task FindSingle(CancellationToken token = default(CancellationToken)) { return Task.CompletedTask; } +}"; + + var expected = + @"using NServiceBus; +using System; +using System.Threading; +using System.Threading.Tasks; +public class Foo +{ + public Task Bar(IMessageHandlerContext context) + { + var someReturnValue = ""someValue""; + return + Include(() => someReturnValue) + .Include(() => someReturnValue) + .FindSingle(context.CancellationToken); + } + + Foo Include(Func action) => this; + + Task FindSingle(CancellationToken token = default(CancellationToken)) { return Task.CompletedTask; } +}"; + + return Assert(original, expected); + } + [Test] public Task NonStandardContextVariableName() { @@ -245,4 +293,4 @@ public class ForwardCancellationTokenFixerTestsCSharp10 : ForwardCancellationTok protected override LanguageVersion AnalyzerLanguageVersion => LanguageVersion.CSharp10; } #endif -} +} \ No newline at end of file From a1fbfc31f6c8f9828ae4b029a2870f6a06a21880 Mon Sep 17 00:00:00 2001 From: danielmarbach Date: Mon, 5 Jun 2023 10:51:56 +0200 Subject: [PATCH 2/4] Simple fix first --- .../ForwardCancellationTokenFixer.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/NServiceBus.Core.Analyzer/ForwardCancellationTokenFixer.cs b/src/NServiceBus.Core.Analyzer/ForwardCancellationTokenFixer.cs index 7f737f84b5..f0511e22de 100644 --- a/src/NServiceBus.Core.Analyzer/ForwardCancellationTokenFixer.cs +++ b/src/NServiceBus.Core.Analyzer/ForwardCancellationTokenFixer.cs @@ -35,7 +35,7 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); var startToken = root.FindToken(diagnostic.Location.SourceSpan.Start); - var invocationSyntax = startToken.Parent.AncestorsAndSelf().OfType().First(); + var invocationSyntax = startToken.Parent.AncestorsAndSelf().OfType().Last(); // TODO: consider whether analyzer should pass in name of property, i.e. "CancellationToken", // so that that information doesn't have to spread across both the analyzer and the fixer @@ -89,4 +89,4 @@ static async Task ForwardCancellationToken( // the title is dynamic based on the context name and target method name. static readonly string EquivalenceKey = typeof(ForwardCancellationTokenFixer).FullName; } -} +} \ No newline at end of file From 7377ca67c597df697631169ce6ea7a839cafe9b6 Mon Sep 17 00:00:00 2001 From: danielmarbach Date: Mon, 5 Jun 2023 10:52:14 +0200 Subject: [PATCH 3/4] Add more complex test that will probably break the trivial implementation --- .../ForwardCancellationTokenFixerTests.cs | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/NServiceBus.Core.Analyzer.Tests.Common/ForwardCancellationToken/ForwardCancellationTokenFixerTests.cs b/src/NServiceBus.Core.Analyzer.Tests.Common/ForwardCancellationToken/ForwardCancellationTokenFixerTests.cs index 4692f8201c..e77010f6e4 100644 --- a/src/NServiceBus.Core.Analyzer.Tests.Common/ForwardCancellationToken/ForwardCancellationTokenFixerTests.cs +++ b/src/NServiceBus.Core.Analyzer.Tests.Common/ForwardCancellationToken/ForwardCancellationTokenFixerTests.cs @@ -90,6 +90,48 @@ public Task Bar(IMessageHandlerContext context) return Assert(original, expected); } + [Test] + public Task FluentAsync() + { + var original = + @"using NServiceBus; +using System; +using System.Threading; +using System.Threading.Tasks; +public class Foo +{ + public async Task Bar(IMessageHandlerContext context) + { + var someReturnValue = ""someValue""; + await (await (await Include(() => someReturnValue)).Include(() => someReturnValue)).FindSingle(); + } + + Task Include(Func action, CancellationToken token = default(CancellationToken)) => Task.FromResult(this); + + Task FindSingle(CancellationToken token = default(CancellationToken)) => Task.FromResult(this); +}"; + + var expected = + @"using NServiceBus; +using System; +using System.Threading; +using System.Threading.Tasks; +public class Foo +{ + public async Task Bar(IMessageHandlerContext context) + { + var someReturnValue = ""someValue""; + await (await (await Include(() => someReturnValue, context.CancellationToken)).Include(() => someReturnValue, context.CancellationToken)).FindSingle(context.CancellationToken); + } + + Task Include(Func action, CancellationToken token = default(CancellationToken)) => Task.FromResult(this); + + Task FindSingle(CancellationToken token = default(CancellationToken)) => Task.FromResult(this); +}"; + + return Assert(original, expected); + } + [Test] public Task NonStandardContextVariableName() { From e3b8144cecc187a57f89f202a544697b2c916f42 Mon Sep 17 00:00:00 2001 From: danielmarbach Date: Mon, 5 Jun 2023 10:52:47 +0200 Subject: [PATCH 4/4] Switch to FindNode instead --- .../ForwardCancellationTokenFixer.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/NServiceBus.Core.Analyzer/ForwardCancellationTokenFixer.cs b/src/NServiceBus.Core.Analyzer/ForwardCancellationTokenFixer.cs index f0511e22de..ae0546b875 100644 --- a/src/NServiceBus.Core.Analyzer/ForwardCancellationTokenFixer.cs +++ b/src/NServiceBus.Core.Analyzer/ForwardCancellationTokenFixer.cs @@ -34,8 +34,8 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) var requiredArgName = diagnostic.Properties["RequiredArgName"]; var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); - var startToken = root.FindToken(diagnostic.Location.SourceSpan.Start); - var invocationSyntax = startToken.Parent.AncestorsAndSelf().OfType().Last(); + var startToken = root.FindNode(diagnostic.Location.SourceSpan); + var invocationSyntax = (InvocationExpressionSyntax)startToken; // TODO: consider whether analyzer should pass in name of property, i.e. "CancellationToken", // so that that information doesn't have to spread across both the analyzer and the fixer