Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Forward CancellationToken analyzer and code fixer #5923

Merged
merged 23 commits into from
May 21, 2021
Merged

Forward CancellationToken analyzer and code fixer #5923

Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
96407d9
Add analyzer to forward context.CancellationToken to methods
DavidBoike Feb 4, 2021
b8d3d58
Rebase fixups & fix cancellation diagnostics
DavidBoike Apr 28, 2021
23d2202
Fixer now supports explicit param name prefix when adding 1 parameter…
DavidBoike Apr 28, 2021
88f8ef7
Remove unnecessary pre-emptive token checks
DavidBoike Apr 28, 2021
33857fa
minor tweak
adamralph May 4, 2021
9add88f
remove SharedAttribute from analyzer because :shrug:
adamralph May 5, 2021
862419e
remove using :facepalm:
adamralph May 5, 2021
bb7a87b
fixer - added comment and did minor reformatting
adamralph May 5, 2021
1639a04
aligned property name
adamralph May 5, 2021
a33d9e5
upgrade to analyzers to Microsoft.CodeAnalysis.CSharp.Workspaces 2.10…
adamralph May 5, 2021
f0a0a3c
make method static in AwaitOrCaptureTasksAnalyzer
adamralph May 5, 2021
4b35767
made agreed changes to ForwardCancellationTokenAnalyzer plus some oth…
adamralph May 5, 2021
1888753
downgrade to Microsoft.CodeAnalysis.CSharp.Workspaces 2.4.0
adamralph May 6, 2021
79ff5dc
apply patterns from Particular.CodeAnalyzers tests plus minor tweaks
adamralph May 4, 2021
de6a366
Coalesce a bunch of tests
DavidBoike May 6, 2021
1d8364d
Analyze from the method level first
DavidBoike May 6, 2021
366f3e6
AnalysisTarget stuff no longer necessary
DavidBoike May 6, 2021
753daa3
change _All The Things!_
adamralph May 7, 2021
65bb6ad
Restore a bunch of this test that was cut out
DavidBoike May 20, 2021
ed8b5cd
Random Func detail
DavidBoike May 20, 2021
e23d105
Fixer shouldn't care if it's a message handler
DavidBoike May 20, 2021
bfb5d17
Extra param, make it more interesting
DavidBoike May 20, 2021
8fbadd7
add note about upgrading Microsoft.CodeAnalysis.CSharp.Workspaces
adamralph May 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/NServiceBus.Core.Analyzer.Tests/.editorconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,6 @@ dotnet_diagnostic.CA2007.severity = none

# Justification: Tests don't support cancellation and don't need to forward IMessageHandlerContext.CancellationToken
dotnet_diagnostic.NSB0002.severity = suggestion

# bug in analyzer when multi-targetting
dotnet_diagnostic.IDE0063.severity = none # Use simple 'using' statement
182 changes: 78 additions & 104 deletions src/NServiceBus.Core.Analyzer.Tests/AwaitOrCaptureTasksAnalyzerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,73 +1,70 @@
namespace NServiceBus.Core.Analyzer.Tests
{
using System;
using System.Threading.Tasks;
using Helpers;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using NUnit.Framework;

[TestFixture]
public class AwaitOrCaptureTasksAnalyzerTests : DiagnosticVerifier
public class AwaitOrCaptureTasksAnalyzerTests : AnalyzerTestFixture<AwaitOrCaptureTasksAnalyzer>
{
// IPipelineContext
[TestCase("IPipelineContext", "obj.Send(new object(), new SendOptions());")]
[TestCase("IPipelineContext", "obj.Send<object>(_ => { }, new SendOptions());")]
[TestCase("IPipelineContext", "obj.Publish(new object(), new PublishOptions());")]
[TestCase("IPipelineContext", "obj.Publish<object>(_ => { }, new PublishOptions());")]
[TestCase("IPipelineContext", "obj.Send(new object(), new SendOptions())")]
[TestCase("IPipelineContext", "obj.Send<object>(_ => { }, new SendOptions())")]
[TestCase("IPipelineContext", "obj.Publish(new object(), new PublishOptions())")]
[TestCase("IPipelineContext", "obj.Publish<object>(_ => { }, new PublishOptions())")]

// PipelineContextExtensions
[TestCase("IPipelineContext", "obj.Send(new object());")]
[TestCase("IPipelineContext", "obj.Send<object>(_ => { });")]
[TestCase("IPipelineContext", "obj.Send(\"destination\", new object());")]
[TestCase("IPipelineContext", "obj.Send<object>(\"destination\", _ => { });")]
[TestCase("IPipelineContext", "obj.SendLocal(new object());")]
[TestCase("IPipelineContext", "obj.SendLocal<object>(_ => { });")]
[TestCase("IPipelineContext", "obj.Publish(new object());")]
[TestCase("IPipelineContext", "obj.Publish<object>();")]
[TestCase("IPipelineContext", "obj.Publish<object>(_ => { });")]
[TestCase("IPipelineContext", "obj.Send(new object())")]
[TestCase("IPipelineContext", "obj.Send<object>(_ => { })")]
[TestCase("IPipelineContext", "obj.Send(\"destination\", new object())")]
[TestCase("IPipelineContext", "obj.Send<object>(\"destination\", _ => { })")]
[TestCase("IPipelineContext", "obj.SendLocal(new object())")]
[TestCase("IPipelineContext", "obj.SendLocal<object>(_ => { })")]
[TestCase("IPipelineContext", "obj.Publish(new object())")]
[TestCase("IPipelineContext", "obj.Publish<object>()")]
[TestCase("IPipelineContext", "obj.Publish<object>(_ => { })")]

// IMessageProcessingContext
[TestCase("IMessageProcessingContext", "obj.Reply(new object(), new ReplyOptions());")]
[TestCase("IMessageProcessingContext", "obj.Reply<object>(_ => { }, new ReplyOptions());")]
[TestCase("IMessageProcessingContext", "obj.ForwardCurrentMessageTo(\"destination\");")]
[TestCase("IMessageProcessingContext", "obj.Reply(new object(), new ReplyOptions())")]
[TestCase("IMessageProcessingContext", "obj.Reply<object>(_ => { }, new ReplyOptions())")]
[TestCase("IMessageProcessingContext", "obj.ForwardCurrentMessageTo(\"destination\")")]

// MessageProcessingContextExtensions
[TestCase("IMessageProcessingContext", "obj.Reply(new object());")]
[TestCase("IMessageProcessingContext", "obj.Reply<object>(_ => { });")]
[TestCase("IMessageProcessingContext", "obj.Reply(new object())")]
[TestCase("IMessageProcessingContext", "obj.Reply<object>(_ => { })")]

// IMessageSession
[TestCase("IMessageSession", "obj.Send(new object(), new SendOptions());")]
[TestCase("IMessageSession", "obj.Send<object>(_ => { }, new SendOptions());")]
[TestCase("IMessageSession", "obj.Publish(new object(), new PublishOptions());")]
[TestCase("IMessageSession", "obj.Publish<object>(_ => { }, new PublishOptions());")]
[TestCase("IMessageSession", "obj.Subscribe(typeof(object), new SubscribeOptions());")]
[TestCase("IMessageSession", "obj.Unsubscribe(typeof(object), new UnsubscribeOptions());")]
[TestCase("IMessageSession", "obj.Send(new object(), new SendOptions())")]
[TestCase("IMessageSession", "obj.Send<object>(_ => { }, new SendOptions())")]
[TestCase("IMessageSession", "obj.Publish(new object(), new PublishOptions())")]
[TestCase("IMessageSession", "obj.Publish<object>(_ => { }, new PublishOptions())")]
[TestCase("IMessageSession", "obj.Subscribe(typeof(object), new SubscribeOptions())")]
[TestCase("IMessageSession", "obj.Unsubscribe(typeof(object), new UnsubscribeOptions())")]

// MessageSessionExtensions
[TestCase("IMessageSession", "obj.Send(new object());")]
[TestCase("IMessageSession", "obj.Send<object>(_ => { });")]
[TestCase("IMessageSession", "obj.Send(\"destination\", new object());")]
[TestCase("IMessageSession", "obj.Send<object>(\"destination\", _ => { });")]
[TestCase("IMessageSession", "obj.SendLocal(new object());")]
[TestCase("IMessageSession", "obj.SendLocal<object>(_ => { });")]
[TestCase("IMessageSession", "obj.Publish(new object());")]
[TestCase("IMessageSession", "obj.Publish<object>();")]
[TestCase("IMessageSession", "obj.Publish<object>(_ => { });")]
[TestCase("IMessageSession", "obj.Subscribe(typeof(object));")]
[TestCase("IMessageSession", "obj.Subscribe<object>();")]
[TestCase("IMessageSession", "obj.Unsubscribe(typeof(object));")]
[TestCase("IMessageSession", "obj.Unsubscribe<object>();")]
[TestCase("IMessageSession", "obj.Send(new object())")]
[TestCase("IMessageSession", "obj.Send<object>(_ => { })")]
[TestCase("IMessageSession", "obj.Send(\"destination\", new object())")]
[TestCase("IMessageSession", "obj.Send<object>(\"destination\", _ => { })")]
[TestCase("IMessageSession", "obj.SendLocal(new object())")]
[TestCase("IMessageSession", "obj.SendLocal<object>(_ => { })")]
[TestCase("IMessageSession", "obj.Publish(new object())")]
[TestCase("IMessageSession", "obj.Publish<object>()")]
[TestCase("IMessageSession", "obj.Publish<object>(_ => { })")]
[TestCase("IMessageSession", "obj.Subscribe(typeof(object))")]
[TestCase("IMessageSession", "obj.Subscribe<object>()")]
[TestCase("IMessageSession", "obj.Unsubscribe(typeof(object))")]
[TestCase("IMessageSession", "obj.Unsubscribe<object>()")]

// Endpoint
[TestCase("EndpointConfiguration", "Endpoint.Create(obj);")]
[TestCase("EndpointConfiguration", "Endpoint.Start(obj);")]
[TestCase("EndpointConfiguration", "Endpoint.Create(obj)")]
[TestCase("EndpointConfiguration", "Endpoint.Start(obj)")]

// IStartableEndpoint
[TestCase("IStartableEndpoint", "obj.Start();")]
[TestCase("IStartableEndpoint", "obj.Start()")]

// IEndpointInstance
[TestCase("IEndpointInstance", "obj.Stop();")]
[TestCase("IEndpointInstance", "obj.Stop()")]
public Task DiagnosticIsReportedForCorePublicMethods(string type, string call)
{
var source =
Expand All @@ -78,33 +75,26 @@ class Foo
{{
void Bar({type} obj)
{{
{call}
[|{call}|];
}}
}}";

var expected = new DiagnosticResult
{
Id = "NSB0001",
Severity = DiagnosticSeverity.Error,
Locations = new[] { new DiagnosticResultLocation("Test0.cs", 8, 9) },
};

return Verify(source, expected);
return Assert(AwaitOrCaptureTasksAnalyzer.DiagnosticId, source);
}

[TestCase("session.Send(new object());")]
[TestCase("session.Send(new object(), new SendOptions());")]
[TestCase("session.Send<object>(_ => { }, new SendOptions());")]
[TestCase("session.Send<object>(_ => { });")]
[TestCase("session.Send(\"destination\", new object());")]
[TestCase("session.Send<object>(\"destination\", _ => { });")]
[TestCase("session.SendLocal(new object());")]
[TestCase("session.SendLocal<object>(_ => { });")]
[TestCase("session.Publish(new object());")]
[TestCase("session.Publish(new object(), new PublishOptions());")]
[TestCase("session.Publish<object>();")]
[TestCase("session.Publish<object>(_ => { });")]
[TestCase("session.Publish<object>(_ => { }, new PublishOptions());")]
[TestCase("session.Send(new object())")]
[TestCase("session.Send(new object(), new SendOptions())")]
[TestCase("session.Send<object>(_ => { }, new SendOptions())")]
[TestCase("session.Send<object>(_ => { })")]
[TestCase("session.Send(\"destination\", new object())")]
[TestCase("session.Send<object>(\"destination\", _ => { })")]
[TestCase("session.SendLocal(new object())")]
[TestCase("session.SendLocal<object>(_ => { })")]
[TestCase("session.Publish(new object())")]
[TestCase("session.Publish(new object(), new PublishOptions())")]
[TestCase("session.Publish<object>()")]
[TestCase("session.Publish<object>(_ => { })")]
[TestCase("session.Publish<object>(_ => { }, new PublishOptions())")]
public Task DiagnosticIsReportedForUniformSession(string call)
{
var source =
Expand All @@ -114,67 +104,53 @@ class Foo
{{
void Bar(IUniformSession session)
{{
{call}
[|{call}|];
}}
}}";

var expected = new DiagnosticResult
{
Id = "NSB0001",
Severity = DiagnosticSeverity.Error,
Locations = new[] { new DiagnosticResultLocation("Test0.cs", 7, 9) },
};

return Verify(source, expected);
return Assert(AwaitOrCaptureTasksAnalyzer.DiagnosticId, source);
}

[TestCase("RequestTimeout<object>(context, DateTime.Now);")]
[TestCase("RequestTimeout<object>(context, DateTime.Now, new object());")]
[TestCase("RequestTimeout<object>(context, TimeSpan.Zero);")]
[TestCase("RequestTimeout<object>(context, TimeSpan.Zero, new object());")]
[TestCase("ReplyToOriginator(context, new object());")]
[TestCase("RequestTimeout<object>(context, DateTime.Now)")]
[TestCase("RequestTimeout<object>(context, DateTime.Now, new object())")]
[TestCase("RequestTimeout<object>(context, TimeSpan.Zero)")]
[TestCase("RequestTimeout<object>(context, TimeSpan.Zero, new object())")]
[TestCase("ReplyToOriginator(context, new object())")]
public Task DiagnosticIsReportedForSagaProtectedMethods(string call)
{
var source =
$@"using System;
using NServiceBus;
class TestSaga : Saga<object>
class TestSaga : Saga<Data>
{{
void Bar(IMessageHandlerContext context)
{{
{call}
[|{call}|];
}}
}}";
var expected = new DiagnosticResult
{
Id = "NSB0001",
Severity = DiagnosticSeverity.Error,
Locations = new[] { new DiagnosticResultLocation("Test0.cs", 7, 9) },
};
return Verify(source, expected);

protected override void ConfigureHowToFindSaga(SagaPropertyMapper<Data> mapper) {{ }}
}}

class Data : ContainSagaData {{}}";

return Assert(AwaitOrCaptureTasksAnalyzer.DiagnosticId, source);
}

[Test]
public Task DiagnosticIsReportedForAsyncMethods()
{
var source =
@"using NServiceBus;
@"using System.Threading.Tasks;
using NServiceBus;
class Foo
{
async Task Bar(IPipelineContext ctx)
{
ctx.Send(new object(), new SendOptions());
[|ctx.Send(new object(), new SendOptions())|];
}
}";

var expected = new DiagnosticResult
{
Id = "NSB0001",
Severity = DiagnosticSeverity.Error,
Locations = new[] { new DiagnosticResultLocation("Test0.cs", 6, 9) },
};

return Verify(source, expected);
return Assert(AwaitOrCaptureTasksAnalyzer.DiagnosticId, source);
}

[TestCase(
Expand Down Expand Up @@ -263,8 +239,6 @@ void Bar(IMessageSession session)
}
}",
Description = "because the send operation task is accessed.")]
public Task NoDiagnosticIsReported(string source) => Verify(source, Array.Empty<DiagnosticResult>());

protected override DiagnosticAnalyzer GetAnalyzer() => new AwaitOrCaptureTasksAnalyzer();
public Task NoDiagnosticIsReported(string source) => Assert(source);
}
}