diff --git a/Directory.Packages.props b/Directory.Packages.props index 3950a8f6..0319604a 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -1,25 +1,25 @@ - - - true - false - - - - - - - - - - - - - - - - - - - - - + + + true + false + + + + + + + + + + + + + + + + + + + + + diff --git a/IntelliTect.Analyzer/IntelliTect.Analyzer.CodeFixes/TaskDelayZero.cs b/IntelliTect.Analyzer/IntelliTect.Analyzer.CodeFixes/TaskDelayZero.cs new file mode 100644 index 00000000..59ae5e70 --- /dev/null +++ b/IntelliTect.Analyzer/IntelliTect.Analyzer.CodeFixes/TaskDelayZero.cs @@ -0,0 +1,198 @@ +using System.Collections.Immutable; +using System.Composition; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Formatting; +using Microsoft.CodeAnalysis.Operations; +using Microsoft.CodeAnalysis.Simplification; +using Microsoft.CodeAnalysis.Text; + +namespace IntelliTect.Analyzer.CodeFixes +{ + [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(TaskDelayZero))] + [Shared] + public class TaskDelayZero : CodeFixProvider + { + private const string Title = "Use Task.CompletedTask"; + + public sealed override ImmutableArray FixableDiagnosticIds => + ImmutableArray.Create(Analyzers.TaskDelayZero.DiagnosticId); + + public sealed override FixAllProvider GetFixAllProvider() => + WellKnownFixAllProviders.BatchFixer; + + public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + SyntaxNode? root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + if (root is null) + { + return; + } + + Diagnostic diagnostic = context.Diagnostics.First(); + TextSpan diagnosticSpan = diagnostic.Location.SourceSpan; + + InvocationExpressionSyntax? invocation = root.FindToken(diagnosticSpan.Start) + .Parent?.AncestorsAndSelf() + .OfType() + .FirstOrDefault(); + if (invocation is null) + { + return; + } + + SemanticModel? semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false); + if (semanticModel is null) + { + return; + } + + if (semanticModel.GetOperation(invocation, context.CancellationToken) is not IInvocationOperation invocationOperation) + { + return; + } + + ExpressionSyntax? replacement = CreateReplacementExpression(invocationOperation); + if (replacement is null) + { + return; + } + + context.RegisterCodeFix( + CodeAction.Create( + title: Title, + createChangedDocument: c => ReplaceInvocationAsync(context.Document, invocation, replacement, c), + equivalenceKey: Title), + diagnostic); + } + + private static ExpressionSyntax? CreateReplacementExpression(IInvocationOperation invocation) + { + if (!IntelliTect.Analyzer.Analyzers.TaskDelayZero.IsTaskDelayWithIntMilliseconds(invocation.TargetMethod)) + { + return null; + } + + IArgumentOperation? millisecondsDelayArgument = invocation.Arguments + .FirstOrDefault(a => a.Parameter?.Name == "millisecondsDelay"); + if (millisecondsDelayArgument?.Value.ConstantValue is not { HasValue: true, Value: int millisecondsDelay } + || millisecondsDelay != 0) + { + return null; + } + + // Task.Delay overloads that include a TimeProvider cannot be safely rewritten to + // Task.CompletedTask/Task.FromCanceled without changing observable behavior. + if (invocation.Arguments.Any(a => a.Parameter?.Name == "timeProvider")) + { + return null; + } + + IArgumentOperation? cancellationTokenArgument = invocation.Arguments + .FirstOrDefault(a => a.Parameter?.Name == "cancellationToken"); + if (cancellationTokenArgument is null) + { + return CreateTaskCompletedTaskExpression(); + } + + if (cancellationTokenArgument.Value.Syntax is not ExpressionSyntax cancellationTokenExpression) + { + return null; + } + + if (!IsSideEffectFree(cancellationTokenArgument.Value)) + { + return null; + } + + string tokenExpressionText = NormalizeCancellationTokenExpression(cancellationTokenExpression); + ExpressionSyntax normalizedTokenExpression = SyntaxFactory.ParseExpression(tokenExpressionText) + .WithAdditionalAnnotations(Simplifier.Annotation); + + // Runtime behavior reference: + // https://github.com/dotnet/runtime/blob/1acc89c305165239a5a824567a3176b6b3342790/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs#L5907-L5911 + // Task.Delay(0, token) maps to: + // token.IsCancellationRequested ? Task.FromCanceled(token) : Task.CompletedTask + return SyntaxFactory.ConditionalExpression( + SyntaxFactory.MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + normalizedTokenExpression, + SyntaxFactory.IdentifierName("IsCancellationRequested")), + CreateTaskFromCanceledExpression(normalizedTokenExpression), + CreateTaskCompletedTaskExpression()); + } + + private static ExpressionSyntax CreateTaskCompletedTaskExpression() + { + return SyntaxFactory.MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + SyntaxFactory.ParseName("global::System.Threading.Tasks.Task").WithAdditionalAnnotations(Simplifier.Annotation), + SyntaxFactory.IdentifierName("CompletedTask")); + } + + private static InvocationExpressionSyntax CreateTaskFromCanceledExpression(ExpressionSyntax cancellationTokenExpression) + { + return SyntaxFactory.InvocationExpression( + SyntaxFactory.MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + SyntaxFactory.ParseName("global::System.Threading.Tasks.Task").WithAdditionalAnnotations(Simplifier.Annotation), + SyntaxFactory.IdentifierName("FromCanceled")), + SyntaxFactory.ArgumentList( + SyntaxFactory.SingletonSeparatedList( + SyntaxFactory.Argument(cancellationTokenExpression)))); + } + + private static string NormalizeCancellationTokenExpression(ExpressionSyntax cancellationTokenExpression) + { + return cancellationTokenExpression.Kind() switch + { + SyntaxKind.DefaultLiteralExpression => "default(global::System.Threading.CancellationToken)", + SyntaxKind.DefaultExpression => cancellationTokenExpression.WithoutTrivia().ToString() == "default" + ? "default(global::System.Threading.CancellationToken)" + : cancellationTokenExpression.WithoutTrivia().ToString(), + _ => cancellationTokenExpression.WithoutTrivia().ToString() + }; + } + + private static bool IsSideEffectFree(IOperation operation) + { + return operation switch + { + ILocalReferenceOperation => true, + IParameterReferenceOperation => true, + IDefaultValueOperation => true, + IConversionOperation conversion => IsSideEffectFree(conversion.Operand), + IParenthesizedOperation parenthesized => IsSideEffectFree(parenthesized.Operand), + IPropertyReferenceOperation propertyReference + when propertyReference.Instance is null + && propertyReference.Property.Name == "None" + && propertyReference.Property.ContainingType.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat) + == "global::System.Threading.CancellationToken" => true, + _ => false + }; + } + + private static async Task ReplaceInvocationAsync( + Document document, + InvocationExpressionSyntax invocation, + ExpressionSyntax replacement, + CancellationToken cancellationToken) + { + SyntaxNode oldRoot = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false) + ?? throw new System.InvalidOperationException("Could not get syntax root"); + + ExpressionSyntax replacementExpression = replacement + .WithTriviaFrom(invocation) + .WithAdditionalAnnotations(Formatter.Annotation, Simplifier.Annotation); + + SyntaxNode newRoot = oldRoot.ReplaceNode(invocation, replacementExpression); + return document.WithSyntaxRoot(newRoot); + } + } +} diff --git a/IntelliTect.Analyzer/IntelliTect.Analyzer.Test/TaskDelayZeroTests.cs b/IntelliTect.Analyzer/IntelliTect.Analyzer.Test/TaskDelayZeroTests.cs new file mode 100644 index 00000000..af88dbef --- /dev/null +++ b/IntelliTect.Analyzer/IntelliTect.Analyzer.Test/TaskDelayZeroTests.cs @@ -0,0 +1,364 @@ +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using TestHelper; + +namespace IntelliTect.Analyzer.Tests +{ + [TestClass] + public class TaskDelayZeroTests : CodeFixVerifier + { + private const string DiagnosticId = Analyzers.TaskDelayZero.DiagnosticId; + private const string DiagnosticMessage = "Replace Task.Delay(0) with Task.CompletedTask"; + [TestMethod] + public void TaskDelayZero_ProducesInfoMessage() + { + string source = """ + using System.Threading.Tasks; + + class C + { + void M() + { + Task.Delay(0); + } + } + """; + + VerifyCSharpDiagnostic(source, + new DiagnosticResult + { + Id = DiagnosticId, + Severity = DiagnosticSeverity.Info, + Message = DiagnosticMessage, + Locations = + [ + new DiagnosticResultLocation("Test0.cs", 7, 9) + ] + }); + } + + [TestMethod] + public void TaskDelayZeroWithToken_ProducesInfoMessage() + { + string source = """ + using System.Threading; + using System.Threading.Tasks; + + class C + { + void M(CancellationToken token) + { + Task.Delay(0, token); + } + } + """; + + VerifyCSharpDiagnostic(source, + new DiagnosticResult + { + Id = DiagnosticId, + Severity = DiagnosticSeverity.Info, + Message = DiagnosticMessage, + Locations = + [ + new DiagnosticResultLocation("Test0.cs", 8, 9) + ] + }); + } + + [TestMethod] + public void TaskDelayNonZero_ProducesNoDiagnostic() + { + string source = """ + using System.Threading.Tasks; + + class C + { + void M() + { + Task.Delay(1); + } + } + """; + + VerifyCSharpDiagnostic(source); + } + + [TestMethod] + public async Task TaskDelayZero_CodeFix_UsesCompletedTask() + { + string source = """ + using System.Threading.Tasks; + + class C + { + Task M() + { + return Task.Delay(0); + } + } + """; + + string fixedSource = """ + using System.Threading.Tasks; + + class C + { + Task M() + { + return Task.CompletedTask; + } + } + """; + + await VerifyCSharpFix(source, fixedSource); + } + + [TestMethod] + public async Task TaskDelayZeroWithToken_CodeFix_PreservesCancellationBehavior() + { + string source = """ + using System.Threading; + using System.Threading.Tasks; + + class C + { + Task M(CancellationToken token) + { + return Task.Delay(0, token); + } + } + """; + + string fixedSource = """ + using System.Threading; + using System.Threading.Tasks; + + class C + { + Task M(CancellationToken token) + { + return token.IsCancellationRequested ? Task.FromCanceled(token) : Task.CompletedTask; + } + } + """; + + await VerifyCSharpFix(source, fixedSource); + } + + [TestMethod] + public async Task TaskDelayZeroWithDefaultToken_CodeFix_UsesTypedDefault() + { + string source = """ + using System.Threading.Tasks; + + class C + { + Task M() + { + return Task.Delay(0, default); + } + } + """; + + string fixedSource = """ + using System.Threading.Tasks; + + class C + { + Task M() + { + return default(global::System.Threading.CancellationToken).IsCancellationRequested ? Task.FromCanceled(default) : Task.CompletedTask; + } + } + """; + + await VerifyCSharpFix(source, fixedSource); + } + + [TestMethod] + public async Task TaskDelayZeroWithSideEffectingTokenExpression_NoCodeFix() + { + string source = """ + using System.Threading; + using System.Threading.Tasks; + + class C + { + Task M() + { + return Task.Delay(0, GetToken()); + } + + CancellationToken GetToken() => default; + } + """; + + await VerifyCSharpFix(source, source); + } + + [TestMethod] + public async Task TaskDelayZeroWithPropertyTokenExpression_NoCodeFix() + { + string source = """ + using System.Threading; + using System.Threading.Tasks; + + class C + { + Task M() + { + return Task.Delay(0, Token); + } + + CancellationToken Token => default; + } + """; + + await VerifyCSharpFix(source, source); + } + + [TestMethod] + public async Task TaskDelayZeroWithCancellationTokenNone_CodeFix_PreservesCancellationBehavior() + { + string source = """ + using System.Threading; + using System.Threading.Tasks; + + class C + { + Task M() + { + return Task.Delay(0, CancellationToken.None); + } + } + """; + + string fixedSource = """ + using System.Threading; + using System.Threading.Tasks; + + class C + { + Task M() + { + return CancellationToken.None.IsCancellationRequested ? Task.FromCanceled(CancellationToken.None) : Task.CompletedTask; + } + } + """; + + await VerifyCSharpFix(source, fixedSource); + } + + [TestMethod] + public async Task TaskDelayZeroWithTimeProvider_NoCodeFix() + { + string source = """ + using System; + using System.Threading.Tasks; + + class C + { + Task M() + { + return Task.Delay(0, TimeProvider.System); + } + } + """; + + await VerifyCSharpFix(source, source); + } + + [TestMethod] + public async Task TaskDelayZeroWithTimeProviderAndToken_NoCodeFix() + { + string source = """ + using System; + using System.Threading; + using System.Threading.Tasks; + + class C + { + Task M(CancellationToken token) + { + return Task.Delay(0, TimeProvider.System, token); + } + } + """; + + await VerifyCSharpFix(source, source); + } + + [TestMethod] + public async Task AwaitTaskDelayZero_CodeFix_UsesCompletedTask() + { + string source = """ + using System.Threading.Tasks; + + class C + { + async Task M() + { + await Task.Delay(0); + } + } + """; + + string fixedSource = """ + using System.Threading.Tasks; + + class C + { + async Task M() + { + await Task.CompletedTask; + } + } + """; + + await VerifyCSharpFix(source, fixedSource); + } + + [TestMethod] + public void TaskDelayZeroNamedArguments_ProducesInfoMessage() + { + string source = """ + using System.Threading; + using System.Threading.Tasks; + + class C + { + void M(CancellationToken token) + { + Task.Delay(cancellationToken: token, millisecondsDelay: 0); + } + } + """; + + VerifyCSharpDiagnostic(source, + new DiagnosticResult + { + Id = DiagnosticId, + Severity = DiagnosticSeverity.Info, + Message = DiagnosticMessage, + Locations = + [ + new DiagnosticResultLocation("Test0.cs", 8, 9) + ] + }); + } + + protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer() + { + return new Analyzers.TaskDelayZero(); + } + + protected override CodeFixProvider GetCSharpCodeFixProvider() + { + return new CodeFixes.TaskDelayZero(); + } + } +} diff --git a/IntelliTect.Analyzer/IntelliTect.Analyzer/AnalyzerReleases.Unshipped.md b/IntelliTect.Analyzer/IntelliTect.Analyzer/AnalyzerReleases.Unshipped.md index 6f6766da..4372e6ed 100644 --- a/IntelliTect.Analyzer/IntelliTect.Analyzer/AnalyzerReleases.Unshipped.md +++ b/IntelliTect.Analyzer/IntelliTect.Analyzer/AnalyzerReleases.Unshipped.md @@ -1,3 +1,8 @@ ; Unshipped analyzer release ; https://github.com/dotnet/roslyn/blob/main/src/RoslynAnalyzers/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md +### New Rules + +Rule ID | Category | Severity | Notes +--------|----------|----------|-------------------- +INTL0304 | Performance | Info | TaskDelayZero diff --git a/IntelliTect.Analyzer/IntelliTect.Analyzer/Analyzers/TaskDelayZero.cs b/IntelliTect.Analyzer/IntelliTect.Analyzer/Analyzers/TaskDelayZero.cs new file mode 100644 index 00000000..446fad34 --- /dev/null +++ b/IntelliTect.Analyzer/IntelliTect.Analyzer/Analyzers/TaskDelayZero.cs @@ -0,0 +1,85 @@ +using System; +using System.Collections.Immutable; +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +namespace IntelliTect.Analyzer.Analyzers +{ + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public class TaskDelayZero : DiagnosticAnalyzer + { + public const string DiagnosticId = "INTL0304"; + private const string Title = "Use Task.CompletedTask for zero-delay task"; + private const string MessageFormat = "Replace Task.Delay(0) with Task.CompletedTask"; + private const string Description = + "Task.Delay with a zero millisecond delay can be replaced with Task.CompletedTask."; + private const string Category = "Performance"; + private static readonly string _HelpLinkUri = DiagnosticUrlBuilder.GetUrl(Title, DiagnosticId); + + private static readonly DiagnosticDescriptor _Rule = new( + DiagnosticId, + Title, + MessageFormat, + Category, + DiagnosticSeverity.Info, + isEnabledByDefault: true, + description: Description, + helpLinkUri: _HelpLinkUri); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(_Rule); + + public override void Initialize(AnalysisContext context) + { + if (context is null) + { + throw new ArgumentNullException(nameof(context)); + } + + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics); + context.EnableConcurrentExecution(); + context.RegisterOperationAction(AnalyzeInvocation, OperationKind.Invocation); + } + + private static void AnalyzeInvocation(OperationAnalysisContext context) + { + if (context.Operation is not IInvocationOperation invocation) + { + return; + } + + if (!IsTaskDelayWithIntMilliseconds(invocation.TargetMethod)) + { + return; + } + + IArgumentOperation? millisecondsDelayArgument = invocation.Arguments + .FirstOrDefault(a => string.Equals(a.Parameter?.Name, "millisecondsDelay", StringComparison.Ordinal)); + + if (millisecondsDelayArgument?.Value.ConstantValue is { HasValue: true, Value: int millisecondsDelay } + && millisecondsDelay == 0) + { + context.ReportDiagnostic(Diagnostic.Create(_Rule, invocation.Syntax.GetLocation())); + } + } + + public static bool IsTaskDelayWithIntMilliseconds(IMethodSymbol methodSymbol) + { + if (!string.Equals(methodSymbol.Name, "Delay", StringComparison.Ordinal)) + { + return false; + } + + if (methodSymbol.ContainingType.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat) + != "global::System.Threading.Tasks.Task") + { + return false; + } + + return methodSymbol.Parameters.Length >= 1 + && string.Equals(methodSymbol.Parameters[0].Name, "millisecondsDelay", StringComparison.Ordinal) + && methodSymbol.Parameters[0].Type.SpecialType == SpecialType.System_Int32; + } + } +} diff --git a/docs/analyzers/03XX.Performance.md b/docs/analyzers/03XX.Performance.md index 73b265b9..4bde39e4 100644 --- a/docs/analyzers/03XX.Performance.md +++ b/docs/analyzers/03XX.Performance.md @@ -38,4 +38,21 @@ All local variables should be accessed, or replaced with [discards](https://docs ### When to suppress -Do not suppress a warning from this rule. \ No newline at end of file +Do not suppress a warning from this rule. + +## INTL0304 +### `Task.Delay(0)` should use `Task.CompletedTask`. + +`Task.Delay(0)` introduces unnecessary overhead because the delay is already complete. Prefer +`Task.CompletedTask` for the zero-delay case. + +For `Task.Delay(0, cancellationToken)`, preserve behavior by accounting for pre-canceled tokens: + +`cancellationToken.IsCancellationRequested ? Task.FromCanceled(cancellationToken) : Task.CompletedTask` + +This behavior mirrors the runtime implementation in `Task.Delay(uint, TimeProvider, CancellationToken)`: + + +### When to suppress + +Do not suppress a warning from this rule. diff --git a/docs/analyzers/index.md b/docs/analyzers/index.md index 7715562b..b22faa09 100644 --- a/docs/analyzers/index.md +++ b/docs/analyzers/index.md @@ -11,4 +11,5 @@ ## Performance INTL03XX - [INTL0301 Favor EnumerateFiles]({{ site.baseurl }}{% link analyzers/03XX.Performance.md %}#intl0301) -- [INTL0302 Favor EnumerateDirectories]({{ site.baseurl }}{% link analyzers/03XX.Performance.md %}#intl0302) \ No newline at end of file +- [INTL0302 Favor EnumerateDirectories]({{ site.baseurl }}{% link analyzers/03XX.Performance.md %}#intl0302) +- [INTL0304 Task.Delay(0) can be Task.CompletedTask]({{ site.baseurl }}{% link analyzers/03XX.Performance.md %}#intl0304)