VSTHRD003 support for marking known completed tasks as non-foreign

[drewnoakes]

If I understand correctly, VSTHRD003 (Avoid awaiting foreign Tasks) is redundant on a task that is known to have completed already.

In the dotnet/project-system repo we have historically turned this analyzer off, and I'm investigating turning it on.

Most of the subsequent diagnostics are on singleton instances of completed tasks with well-known immutable values.

I'd like a way to annotate these values (mostly properties) to suppress VSTHRD003 when they're returned from methods and so forth. Today, it seems I have to suppress every consumer.

[drewnoakes]

I'm interested in ideas for the design of this. Happy to take a stab at implementing it, or we assign to Copilot.

One idea is to use SuppressMessage on the property. However then you get warnings about redundant suppressions.

The analyzer already supports Task.CompletedTask and Task.FromResult(...), though not TplExtensions.TrueTask and friends.

Perhaps we just allow anyone to define a Microsoft.VisualStudio.Threading.CompletedTaskAttribute attribute that indicates this information to the analyzer.

For example:

namespace Microsoft.VisualStudio.Threading;

[AttributeUsage(AttributeTargets.Property | AttributeTargets.Method | AttributeTargets.Field, Inherited = false)]
internal class CompletedTaskAttribute : Attribute
{}

The analyzer could include this as a source content item, or consumers could define it themselves.

[drewnoakes]

Let's see what Copilot can do.

[AArnott]

Should we only permit the attribute on a property/field if it is a readonly field or get-only property? Otherwise, I fear it might be applied to a field used as the 'tail' on a task chain. Given the field initializer will assign a 'completed' task, folks may apply the attribute and yet that field will get reassigned to an incomplete task later on.

[drewnoakes]

@AArnott I think for fields and properties that makes sense.

Are you proposing disallowing the attribute on methods? Some examples of where we'd want this on methods:

https://github.com/dotnet/project-system/blob/e07f5e290a6133d085e523bf6c45721b320ed9c1/src/Microsoft.VisualStudio.ProjectSystem.Managed/Threading/Tasks/TaskResult.cs#L37-L65

I guess these could be rewritten as properties:

-TaskResult.Null<string>();
+TaskResult<string>.Null;

But if we want to allow annotating code that's not under user control (i.e. library code, per https://github.com/microsoft/vs-threading/pull/1510/files#r2549840476) then that's not an option.

[AArnott]

I suppose we can tolerate attribute application on methods.