diff --git a/doc/UNT0045.md b/doc/UNT0045.md new file mode 100644 index 0000000..c53c1aa --- /dev/null +++ b/doc/UNT0045.md @@ -0,0 +1,41 @@ +# UNT0045 Avoid async void methods + +An `async void` method cannot be awaited, and its exceptions are not propagated to its caller through a task. An ordinary asynchronous method should return a task-like type so callers can observe completion and failures. + +## Examples of patterns that are flagged by this analyzer + +```csharp +using UnityEngine; + +class Loader : MonoBehaviour +{ + public async void LoadAsync() + { + await Awaitable.NextFrameAsync(); + } +} +``` + +## How to fix + +Return the asynchronous type appropriate for the project, such as `Awaitable`, `UniTask`, `Task`, or `ValueTask`, and update callers to await the operation. + +```csharp +using UnityEngine; + +class Loader : MonoBehaviour +{ + public async Awaitable LoadAsync() + { + await Awaitable.NextFrameAsync(); + } +} +``` + +The analyzer checks both methods and local functions. It leaves recognized Unity message callbacks, Unity load/menu callbacks, overrides, interface implementations, and conventional `(object, EventArgs)` event handlers alone because their contracts can require `void`. Derived event-argument types are supported. These callbacks still need appropriate exception handling. + +A method merely named `Start` is not necessarily a Unity callback: its declaring type, static/instance form, and signature must match. Local functions are never Unity messages. + +Task-like return types and the explicit fire-and-forget `UniTaskVoid` return type are not flagged. `UniTaskVoid` is not an awaitable replacement when the caller needs to observe completion. + +No automatic code fix is provided because changing a return type requires reviewing callers and callback contracts. diff --git a/doc/UNT0046.md b/doc/UNT0046.md new file mode 100644 index 0000000..55922f9 --- /dev/null +++ b/doc/UNT0046.md @@ -0,0 +1,39 @@ +# UNT0046 Avoid async void delegates + +An asynchronous lambda or anonymous method can silently become `async void` when converted to `Action`, `UnityAction`, or another void-returning delegate. The receiver cannot await its completion or observe its exceptions through a task. + +## Examples of patterns that are flagged by this analyzer + +```csharp +using System; +using UnityEngine; + +Action callback = async () => +{ + await Awaitable.NextFrameAsync(); +}; +``` + +The analyzer also checks simple lambdas, expression-bodied lambdas, anonymous `delegate` expressions, callback arguments, and event subscriptions. It uses the resolved delegate signature, not just the presence of `async`. + +## How to fix + +Use a task-returning delegate when the receiving API supports one. + +```csharp +using System; +using UnityEngine; + +Func callback = async () => +{ + await Awaitable.NextFrameAsync(); +}; + +await callback(); +``` + +Delegates returning `Task`, `ValueTask`, `UniTask`, `Awaitable`, or their generic forms are not flagged. For example, an async lambda passed to the task-returning overload of `Task.Run` is not an `async void` delegate. An explicit `UniTaskVoid` delegate is also left alone. + +When an event or Unity API requires a void-returning callback, provide an intentional exception-handling/fire-and-forget boundary rather than assuming the receiver will await an async lambda. Do not block the Unity main thread to make an asynchronous callback appear synchronous. + +No automatic code fix is provided because the receiving API determines which delegate signatures are valid. diff --git a/doc/UNT0047.md b/doc/UNT0047.md new file mode 100644 index 0000000..9a17dea --- /dev/null +++ b/doc/UNT0047.md @@ -0,0 +1,35 @@ +# UNT0047 Do not convert task-like values to strings + +Formatting a task-like value does not await it or retrieve its asynchronous result. This often indicates a missing `await`. + +The analyzer recognizes `Task`, `ValueTask`, `UniTask`, and Unity `Awaitable`, including generic forms and task-derived types. It reuses the type-matching infrastructure used by the Unity message analyzers, without treating message-specific `UniTaskVoid` as an awaitable task. + +## Examples of patterns that are flagged by this analyzer + +```csharp +Awaitable score = LoadScoreAsync(); +Debug.Log($"Score: {score}"); +``` + +Other recognized string conversions include: + +- String concatenation with `+` or `+=`, and parameterless `ToString()`. +- Direct task arguments to `string.Format`, `string.Concat`, `StringBuilder.Append`, and `StringBuilder.AppendFormat`. +- Direct task arguments to `Console.Write`, `Console.WriteLine`, and Unity `Debug.Log`, `LogWarning`, `LogError`, and their `LogFormat` variants. + +Directly constructed formatting argument arrays are inspected too. The analyzer does not track task values stored in `object` variables or previously constructed arrays, and it leaves explicit conversions to `object` and user-defined concatenation operators alone. + +## How to fix + +Await the operation and format the result. + +```csharp +int score = await LoadScoreAsync(); +Debug.Log($"Score: {score}"); +``` + +If the task has no result, await it separately and log a meaningful message instead. If logging the operation itself is intentional, explicitly select its metadata, such as a .NET task's `Status`. + +Unrelated methods accepting `object` are not assumed to perform string conversion. Already-awaited results and unrelated types that happen to be named `Task`, `UniTask`, or `Awaitable` are not flagged. + +No automatic code fix is provided: inserting `await` can change execution order and require changes to the containing method's signature. diff --git a/doc/UNT0048.md b/doc/UNT0048.md new file mode 100644 index 0000000..5d5568a --- /dev/null +++ b/doc/UNT0048.md @@ -0,0 +1,41 @@ +# UNT0048 Do not return null for task-like types + +Awaiting a null `Task` or Unity `Awaitable` throws instead of completing normally. A synchronous method that supplies an asynchronous operation should return a non-null task-like value. + +## Examples of patterns that are flagged by this analyzer + +```csharp +using UnityEngine; + +Awaitable WaitIfNeeded(bool shouldWait) +{ + if (!shouldWait) + return null; + + return Awaitable.NextFrameAsync(); +} +``` + +The analyzer checks return statements and expression bodies in methods, local functions, anonymous functions, properties, and indexers. It recognizes null constants, reference-type `default` values, null conditional/switch branches, null coalescing fallbacks, and conditional access. Nullable annotations do not make a null operation safe to await. + +## How to fix + +Return a non-null operation. One option is an async method that completes normally on the no-work path. + +```csharp +using UnityEngine; + +async Awaitable WaitIfNeeded(bool shouldWait) +{ + if (shouldWait) + await Awaitable.NextFrameAsync(); +} +``` + +For .NET tasks, `Task.CompletedTask` and `Task.FromResult(result)` can represent completed operations when appropriate. + +An **async result** can legitimately be null: `return null` in an `async Awaitable` or `async Task` method supplies the result, not the operation. These returns are not flagged. Nested lambdas and local functions are analyzed according to their own async contracts. + +`UniTask` and `ValueTask` are value types. Their default values are not null task references and are not flagged. The analyzer does not perform general null-state/data-flow analysis of variables or other method calls. + +No automatic code fix is provided because the correct result and completion behavior depend on the operation's contract. diff --git a/doc/index.md b/doc/index.md index 87f111d..4d7ee64 100644 --- a/doc/index.md +++ b/doc/index.md @@ -46,6 +46,10 @@ ID | Title | Category [UNT0042](UNT0042.md) | `Mesh` array property accessed in loop | Performance [UNT0043](UNT0043.md) | Possible typo in conditional compilation symbol | Correctness [UNT0044](UNT0044.md) | Avoid temporary strings when setting TextMeshPro text | Performance +[UNT0045](UNT0045.md) | Avoid async void methods | Correctness +[UNT0046](UNT0046.md) | Avoid async void delegates | Correctness +[UNT0047](UNT0047.md) | Do not convert task-like values to strings | Correctness +[UNT0048](UNT0048.md) | Do not return null for task-like types | Correctness # Diagnostic Suppressors diff --git a/src/Microsoft.Unity.Analyzers.Tests/AsyncVoidDelegateTests.cs b/src/Microsoft.Unity.Analyzers.Tests/AsyncVoidDelegateTests.cs new file mode 100644 index 0000000..22fb63a --- /dev/null +++ b/src/Microsoft.Unity.Analyzers.Tests/AsyncVoidDelegateTests.cs @@ -0,0 +1,133 @@ +/*-------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See LICENSE in the project root for license information. + *-------------------------------------------------------------------------------------------*/ + +using System; +using System.Threading.Tasks; +using Xunit; + +namespace Microsoft.Unity.Analyzers.Tests; + +public class AsyncVoidDelegateTests : BaseDiagnosticVerifierTest +{ + [Theory] + [InlineData("Action callback = async () => { await Awaitable.NextFrameAsync(); }; callback();")] + [InlineData("Action callback = async value => { await Task.Yield(); }; callback(1);")] + [InlineData("Action callback = async delegate { await Task.Yield(); }; callback();")] + [InlineData("Action callback = async delegate(int value) { await Task.Yield(); }; callback(1);")] + [InlineData("Action callback = async () => await Task.Delay(1); callback();")] + [InlineData("Action callback = async () => { await default(UniTask); }; callback();")] + [InlineData("Action callback = async () => { await default(UniTask); }; callback();")] + [InlineData("UnityEngine.Events.UnityAction callback = async () => { await Task.Yield(); }; callback();")] + [InlineData("EventHandler callback = async (sender, args) => { await Task.Yield(); }; callback(null, EventArgs.Empty);")] + [InlineData("Register(async () => { await Task.Yield(); });")] + [InlineData("Register((Action)(async () => { await Task.Yield(); }));")] + public async Task VoidReturningDelegates(string statement) + { + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, CreateSource(statement), + ExpectDiagnostic().WithLocation(11, 9 + statement.IndexOf("async", StringComparison.Ordinal))); + } + + [Theory] + [InlineData("Task", "")] + [InlineData("Task", "return 1;")] + [InlineData("ValueTask", "")] + [InlineData("ValueTask", "return 1;")] + [InlineData("Awaitable", "")] + [InlineData("Awaitable", "return 1;")] + [InlineData("UniTask", "")] + [InlineData("UniTask", "return 1;")] + [InlineData("UniTaskVoid", "")] + public async Task TaskReturningDelegates(string type, string result) + { + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, + CreateSource($"Func<{type}> callback = async () => {{ await Task.Yield(); {result} }}; _ = callback();")); + } + + [Theory] + [InlineData("Action callback = () => { }; callback();")] + [InlineData("Action callback = delegate { }; callback();")] + [InlineData("_ = Task.Run(async () => { await Task.Yield(); });")] + [InlineData("Func callback = () => Awaitable.NextFrameAsync(); _ = callback();")] + [InlineData("Func callback = () => default; _ = callback();")] + public async Task NoAsyncVoidConversion(string statement) + { + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, CreateSource(statement)); + } + + [Fact] + public async Task EventSubscription() + { + const string source = @" +using System; +using System.Threading.Tasks; + +class Example +{ + public event Action Changed; + public void Configure() + { + Changed += async () => { await Task.Yield(); }; + Changed?.Invoke(); + } +} +"; + + await VerifyCSharpDiagnosticAsync(source, ExpectDiagnostic().WithLocation(10, 20)); + } + + [Fact] + public async Task NestedAsyncVoidDelegate() + { + var source = CreateSource(@"_ = Task.Run(async () => + { + Action callback = async () => { await Task.Yield(); }; + callback(); + await Task.Yield(); + });"); + + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, source, + ExpectDiagnostic().WithLocation(13, 31)); + } + + [Fact] + public async Task MethodGroupIsNotAnAnonymousFunction() + { + const string source = @" +using System; +using System.Threading.Tasks; + +class Example +{ + public void Configure() + { + Action callback = Run; + callback(); + } + private async void Run() { await Task.Yield(); } +} +"; + + await VerifyCSharpDiagnosticAsync(source); + } + + private static string CreateSource(string statement) + { + return $@" +using System; +using System.Threading.Tasks; +using UnityEngine; +using Cysharp.Threading.Tasks; + +class Example +{{ + public void Configure() + {{ + {statement} + }} + private void Register(Action callback) {{ callback(); }} +}} +" + AsyncTestSources.UniTaskTypes; + } +} diff --git a/src/Microsoft.Unity.Analyzers.Tests/AsyncVoidMethodTests.cs b/src/Microsoft.Unity.Analyzers.Tests/AsyncVoidMethodTests.cs new file mode 100644 index 0000000..3c089d1 --- /dev/null +++ b/src/Microsoft.Unity.Analyzers.Tests/AsyncVoidMethodTests.cs @@ -0,0 +1,186 @@ +/*-------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See LICENSE in the project root for license information. + *-------------------------------------------------------------------------------------------*/ + +using System.Threading.Tasks; +using Xunit; + +namespace Microsoft.Unity.Analyzers.Tests; + +public class AsyncVoidMethodTests : BaseDiagnosticVerifierTest +{ + [Theory] + [InlineData("Task.Yield()")] + [InlineData("Awaitable.NextFrameAsync()")] + [InlineData("default(UniTask)")] + [InlineData("default(UniTask)")] + public async Task AsyncVoidMethod(string awaited) + { + var source = CreateSource($"public async void Run() {{ await {awaited}; }}"); + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, source, + ExpectDiagnostic().WithLocation(9, 23).WithArguments("Run")); + } + + [Theory] + [InlineData("Task", "")] + [InlineData("Task", "return 1;")] + [InlineData("ValueTask", "")] + [InlineData("ValueTask", "return 1;")] + [InlineData("Awaitable", "")] + [InlineData("Awaitable", "return 1;")] + [InlineData("UniTask", "")] + [InlineData("UniTask", "return 1;")] + [InlineData("UniTaskVoid", "")] + public async Task TaskReturningMethod(string type, string result) + { + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, + CreateSource($"public async {type} Run() {{ await Task.Yield(); {result} }}")); + } + + [Fact] + public async Task SynchronousVoidMethod() + { + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, CreateSource("public void Run() { }")); + } + + [Theory] + [InlineData("MonoBehaviour", "public", "Start", "")] + [InlineData("MonoBehaviour", "public", "Awake", "")] + [InlineData("MonoBehaviour", "public", "OnEnable", "")] + [InlineData("MonoBehaviour", "public", "Update", "")] + [InlineData("MonoBehaviour", "public", "OnApplicationPause", "bool pause")] + [InlineData("MonoBehaviour", "public", "OnApplicationPause", "")] + [InlineData("ScriptableObject", "public", "OnEnable", "")] + [InlineData("UnityEditor.EditorWindow", "public", "OnGUI", "")] + [InlineData("UnityEditor.AssetPostprocessor", "public static", "OnPostprocessAllAssets", "")] + public async Task UnityCallbacks(string baseType, string modifiers, string name, string parameters) + { + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, + CreateSource($"{modifiers} async void {name}({parameters}) {{ await Task.Yield(); }}", baseType)); + } + + [Theory] + [InlineData("", "public async void Start()", 23)] + [InlineData("MonoBehaviour", "public static async void Start()", 30)] + [InlineData("MonoBehaviour", "public async void Start(int argument)", 23)] + [InlineData("MonoBehaviour", "public async void Start()", 23)] + public async Task UnityMessageNameAloneIsNotACallback(string baseType, string declaration, int column) + { + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, + CreateSource($"{declaration} {{ await Task.Yield(); }}", baseType), + ExpectDiagnostic().WithLocation(9, column).WithArguments("Start")); + } + + [Fact] + public async Task LocalFunctionIsNotAUnityMessage() + { + var source = CreateSource(@"public void Run() + { + async void Start() { await Task.Yield(); } + Start(); + }", "MonoBehaviour"); + + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, source, + ExpectDiagnostic().WithLocation(11, 20).WithArguments("Start")); + } + + [Fact] + public async Task ExpressionBodiedMethodAndLocalFunction() + { + var source = CreateSource(@"public async void Run() => await Task.Yield(); + public void Configure() + { + async void Local() => await Task.Yield(); + Local(); + }"); + + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, source, + ExpectDiagnostic().WithLocation(9, 23).WithArguments("Run"), + ExpectDiagnostic().WithLocation(12, 20).WithArguments("Local")); + } + + [Theory] + [InlineData("object sender, EventArgs args")] + [InlineData("object sender, System.ComponentModel.CancelEventArgs args")] + public async Task EventHandlerSignature(string parameters) + { + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, + CreateSource($"public async void Changed({parameters}) {{ await Task.Yield(); }}")); + } + + [Theory] + [InlineData("string sender, EventArgs args")] + [InlineData("object sender, object args")] + [InlineData("EventArgs args")] + public async Task OtherParametersAreNotAnEventHandler(string parameters) + { + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, + CreateSource($"public async void Changed({parameters}) {{ await Task.Yield(); }}"), + ExpectDiagnostic().WithLocation(9, 23).WithArguments("Changed")); + } + + [Fact] + public async Task OverrideAndInterfaceContracts() + { + const string source = @" +using System.Threading.Tasks; + +interface IHandler +{ + void Handle(); + void HandleExplicitly(); +} + +class Base +{ + public virtual void Run() { } +} + +class Example : Base, IHandler +{ + public override async void Run() { await Task.Yield(); } + public async void Handle() { await Task.Yield(); } + async void IHandler.HandleExplicitly() { await Task.Yield(); } +} +"; + + await VerifyCSharpDiagnosticAsync(source); + } + + [Fact] + public async Task DeclaringAVirtualMethodStillWarns() + { + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, + CreateSource("public virtual async void Run() { await Task.Yield(); }"), + ExpectDiagnostic().WithLocation(9, 31).WithArguments("Run")); + } + + [Theory] + [InlineData("[RuntimeInitializeOnLoadMethod] public static async void Initialize()")] + [InlineData("[UnityEditor.InitializeOnLoadMethod] public static async void Initialize()")] + [InlineData("[UnityEditor.Callbacks.DidReloadScripts] public static async void Initialize()")] + [InlineData("[ContextMenu(\"Initialize\")] public async void Initialize()")] + [InlineData("[UnityEditor.MenuItem(\"Tests/Initialize\")] public static async void Initialize()")] + public async Task AttributedUnityCallbacks(string declaration) + { + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, + CreateSource($"{declaration} {{ await Task.Yield(); }}", "MonoBehaviour")); + } + + private static string CreateSource(string members, string baseType = "") + { + var inheritance = baseType.Length == 0 ? "" : " : " + baseType; + return $@" +using System; +using System.Threading.Tasks; +using UnityEngine; +using Cysharp.Threading.Tasks; + +class Example{inheritance} +{{ + {members} +}} +" + AsyncTestSources.UniTaskTypes; + } +} diff --git a/src/Microsoft.Unity.Analyzers.Tests/Infrastructure/AsyncTestSources.cs b/src/Microsoft.Unity.Analyzers.Tests/Infrastructure/AsyncTestSources.cs new file mode 100644 index 0000000..88bbb15 --- /dev/null +++ b/src/Microsoft.Unity.Analyzers.Tests/Infrastructure/AsyncTestSources.cs @@ -0,0 +1,84 @@ +/*-------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See LICENSE in the project root for license information. + *-------------------------------------------------------------------------------------------*/ + +namespace Microsoft.Unity.Analyzers.Tests; + +internal static class AsyncTestSources +{ + internal static readonly AnalyzerVerificationContext Context = AnalyzerVerificationContext.Default + .WithAnalyzerFilter("CS8019"); + + // Compile-only UniTask shapes. Unity's Awaitable types come from the real Unity assemblies. + internal const string UniTaskTypes = @" +namespace Cysharp.Threading.Tasks +{ + [System.Runtime.CompilerServices.AsyncMethodBuilder(typeof(UniTaskMethodBuilder))] + public struct UniTask + { + public System.Runtime.CompilerServices.TaskAwaiter GetAwaiter() => System.Threading.Tasks.Task.CompletedTask.GetAwaiter(); + } + + [System.Runtime.CompilerServices.AsyncMethodBuilder(typeof(UniTaskMethodBuilder<>))] + public struct UniTask + { + public System.Runtime.CompilerServices.TaskAwaiter GetAwaiter() => System.Threading.Tasks.Task.FromResult(default(T)).GetAwaiter(); + } + + [System.Runtime.CompilerServices.AsyncMethodBuilder(typeof(UniTaskVoidMethodBuilder))] + public struct UniTaskVoid { } + + public struct UniTaskMethodBuilder + { + public static UniTaskMethodBuilder Create() => default; + public UniTask Task => default; + public void SetResult() { } + public void SetException(System.Exception exception) { } + public void SetStateMachine(System.Runtime.CompilerServices.IAsyncStateMachine stateMachine) { } + public void Start(ref TStateMachine stateMachine) + where TStateMachine : System.Runtime.CompilerServices.IAsyncStateMachine { } + public void AwaitOnCompleted(ref TAwaiter awaiter, ref TStateMachine stateMachine) + where TAwaiter : System.Runtime.CompilerServices.INotifyCompletion + where TStateMachine : System.Runtime.CompilerServices.IAsyncStateMachine { } + public void AwaitUnsafeOnCompleted(ref TAwaiter awaiter, ref TStateMachine stateMachine) + where TAwaiter : System.Runtime.CompilerServices.ICriticalNotifyCompletion + where TStateMachine : System.Runtime.CompilerServices.IAsyncStateMachine { } + } + + public struct UniTaskMethodBuilder + { + public static UniTaskMethodBuilder Create() => default; + public UniTask Task => default; + public void SetResult(T result) { } + public void SetException(System.Exception exception) { } + public void SetStateMachine(System.Runtime.CompilerServices.IAsyncStateMachine stateMachine) { } + public void Start(ref TStateMachine stateMachine) + where TStateMachine : System.Runtime.CompilerServices.IAsyncStateMachine { } + public void AwaitOnCompleted(ref TAwaiter awaiter, ref TStateMachine stateMachine) + where TAwaiter : System.Runtime.CompilerServices.INotifyCompletion + where TStateMachine : System.Runtime.CompilerServices.IAsyncStateMachine { } + public void AwaitUnsafeOnCompleted(ref TAwaiter awaiter, ref TStateMachine stateMachine) + where TAwaiter : System.Runtime.CompilerServices.ICriticalNotifyCompletion + where TStateMachine : System.Runtime.CompilerServices.IAsyncStateMachine { } + } + + public struct UniTaskVoidMethodBuilder + { + public static UniTaskVoidMethodBuilder Create() => default; + public UniTaskVoid Task => default; + public void SetResult() { } + public void SetException(System.Exception exception) { } + public void SetStateMachine(System.Runtime.CompilerServices.IAsyncStateMachine stateMachine) { } + public void Start(ref TStateMachine stateMachine) + where TStateMachine : System.Runtime.CompilerServices.IAsyncStateMachine { } + public void AwaitOnCompleted(ref TAwaiter awaiter, ref TStateMachine stateMachine) + where TAwaiter : System.Runtime.CompilerServices.INotifyCompletion + where TStateMachine : System.Runtime.CompilerServices.IAsyncStateMachine { } + public void AwaitUnsafeOnCompleted(ref TAwaiter awaiter, ref TStateMachine stateMachine) + where TAwaiter : System.Runtime.CompilerServices.ICriticalNotifyCompletion + where TStateMachine : System.Runtime.CompilerServices.IAsyncStateMachine { } + } +} +"; +} diff --git a/src/Microsoft.Unity.Analyzers.Tests/MessageSignatureTests.cs b/src/Microsoft.Unity.Analyzers.Tests/MessageSignatureTests.cs index 1883337..54ea874 100644 --- a/src/Microsoft.Unity.Analyzers.Tests/MessageSignatureTests.cs +++ b/src/Microsoft.Unity.Analyzers.Tests/MessageSignatureTests.cs @@ -410,6 +410,47 @@ private void OnSceneGUI(object foo) await VerifyCSharpFixAsync(test, test); } + [Fact] + public async Task UniTaskMessageSignatures() + { + const string source = @" +using System.Collections; +using Cysharp.Threading.Tasks; +using UnityEngine; +using UnityEditor; + +class Script : MonoBehaviour +{ + private UniTaskVoid Awake() => default; + private UniTask Start() => default; +} + +class Processor : AssetPostprocessor +{ + private static UniTask OnPreGeneratingCSProjectFiles() => default; +} +"; + + await VerifyCSharpDiagnosticAsync(source + AsyncTestSources.UniTaskTypes); + } + + [Fact] + public async Task TaskLikeRecognitionDoesNotBroadenUnityMessageSignatures() + { + const string source = @" +using Cysharp.Threading.Tasks; +using UnityEngine; + +class Script : MonoBehaviour +{ + private UniTask Start() => default; +} +"; + + await VerifyCSharpDiagnosticAsync(source + AsyncTestSources.UniTaskTypes, + ExpectDiagnostic().WithLocation(7, 21).WithArguments("Start")); + } + [Fact] public async Task MessageSignatureTrivia() { diff --git a/src/Microsoft.Unity.Analyzers.Tests/NullTaskReturnTests.cs b/src/Microsoft.Unity.Analyzers.Tests/NullTaskReturnTests.cs new file mode 100644 index 0000000..173c2a6 --- /dev/null +++ b/src/Microsoft.Unity.Analyzers.Tests/NullTaskReturnTests.cs @@ -0,0 +1,265 @@ +/*-------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See LICENSE in the project root for license information. + *-------------------------------------------------------------------------------------------*/ + +using System.Threading.Tasks; +using Xunit; + +namespace Microsoft.Unity.Analyzers.Tests; + +public class NullTaskReturnTests : BaseDiagnosticVerifierTest +{ + public static TheoryData NullReturns + { + get + { + var data = new TheoryData(); + foreach (var type in new[] { "Task", "Task", "Awaitable", "Awaitable" }) + { + foreach (var expression in new[] + { + "null", "default", $"default({type})", $"({type})null", "null!", + "condition ? other : null", "condition ? default : other", + "condition switch { true => other, _ => null }", "other ?? null" + }) + data.Add(type, expression); + } + + return data; + } + } + + [Theory] + [MemberData(nameof(NullReturns))] + public async Task ReturnsNullTask(string type, string expression) + { + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, CreateSource(type, expression), + ExpectDiagnostic().WithLocation(11, 16)); + } + + [Theory] + [InlineData("Task")] + [InlineData("Task")] + [InlineData("Awaitable")] + [InlineData("Awaitable")] + public async Task ExpressionBodiedMethod(string type) + { + var source = CreateSource(type, "other") + .Replace(@" + { + return other; + }", " => null;"); + + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, source, + ExpectDiagnostic().WithLocation(9, 45 + 2 * type.Length)); + } + + [Fact] + public async Task PropertiesAndGetters() + { + const string source = @" +using UnityEngine; + +class Example +{ + public Awaitable Operation => null; + public Awaitable Other { get { return default; } } + public Awaitable this[int index] => default; +} +"; + await VerifyCSharpDiagnosticAsync(source, + ExpectDiagnostic().WithLocation(6, 35), + ExpectDiagnostic().WithLocation(7, 43), + ExpectDiagnostic().WithLocation(8, 41)); + } + + [Fact] + public async Task NestedSynchronousFunctionsInsideAsyncMethod() + { + const string source = @" +using System; +using System.Threading.Tasks; +using UnityEngine; + +class Example +{ + public async Task Run() + { + Func first = () => null; + Func second = delegate { return default; }; + Awaitable Read() => null; + _ = first(); + _ = second(); + _ = Read(); + await Task.Yield(); + } +} +"; + await VerifyCSharpDiagnosticAsync(source, + ExpectDiagnostic().WithLocation(10, 39), + ExpectDiagnostic().WithLocation(11, 52), + ExpectDiagnostic().WithLocation(12, 29)); + } + + [Theory] + [InlineData("Task")] + [InlineData("ValueTask")] + [InlineData("Awaitable")] + [InlineData("UniTask")] + [InlineData("Task")] + [InlineData("Awaitable")] + public async Task AsyncNullResultIsNotANullTask(string type) + { + var source = CreateSource(type, "null") + .Replace($"public {type} Read", $"public async {type} Read") + .Replace("return null;", "await Task.Yield(); return null;"); + + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, source); + } + + [Fact] + public async Task NestedAsyncNullResults() + { + const string source = @" +using System; +using System.Threading.Tasks; + +class Example +{ + public void Run() + { + Func> read = async () => { await Task.Yield(); return null; }; + async Task Local() { await Task.Yield(); return null; } + _ = read(); + _ = Local(); + } +} +"; + await VerifyCSharpDiagnosticAsync(source); + } + + [Theory] + [InlineData("ValueTask")] + [InlineData("ValueTask")] + [InlineData("UniTask")] + [InlineData("UniTask")] + [InlineData("UniTaskVoid")] + [InlineData("UniTask?")] + public async Task ValueTypeDefaultsAreNotNullTasks(string type) + { + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, CreateSource(type, "default")); + } + + [Theory] + [InlineData("other")] + [InlineData("Task.CompletedTask")] + [InlineData("null ?? other")] + [InlineData("other ?? Task.CompletedTask")] + [InlineData("condition ? other : Task.CompletedTask")] + [InlineData("true ? other : null")] + [InlineData("false ? null : other")] + [InlineData("condition switch { true => other, _ => Task.CompletedTask }")] + public async Task NonNullReturnExpressions(string expression) + { + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, CreateSource("Task", expression)); + } + + [Fact] + public async Task NullableTaskStillCannotBeAwaitedWhenNull() + { + const string source = @" +#nullable enable +using UnityEngine; + +class Example +{ + public Awaitable? Read() => null; +} +"; + await VerifyCSharpDiagnosticAsync(source, ExpectDiagnostic().WithLocation(7, 33)); + } + + [Fact] + public async Task ConditionalAccessAndCoalescing() + { + const string source = @" +using UnityEngine; + +class Example +{ + public Awaitable Read() => Awaitable.NextFrameAsync(); + public Awaitable MaybeRead(Example other) => other?.Read(); + public Awaitable ReadOrFallback(Example other) => other?.Read() ?? Read(); +} +"; + await VerifyCSharpDiagnosticAsync(source, ExpectDiagnostic().WithLocation(7, 50)); + } + + [Fact] + public async Task UserDefinedConversionMayReturnANonNullTask() + { + const string source = @" +using UnityEngine; + +class Example +{ + public static implicit operator Awaitable(Example value) => Awaitable.NextFrameAsync(); + public Awaitable Read() => (Example)null; +} +"; + await VerifyCSharpDiagnosticAsync(source); + } + + [Fact] + public async Task NonTaskReturnTypes() + { + const string source = @" +using System.Threading.Tasks; + +class Example +{ + public object Read() => (Task)null; + public string Text() => null; + public Other.Awaitable Custom() => null; + public void Run() { return; } +} + +namespace Other +{ + class Awaitable { } +} +"; + await VerifyCSharpDiagnosticAsync(source); + } + + [Theory] + [InlineData("Read(T other, bool condition) where T : Task")] + [InlineData("Read(T other, bool condition) where T : U where U : Task")] + public async Task ConstrainedTypeParameter(string signature) + { + var source = CreateSource("T", "null") + .Replace("Read(T other, bool condition)", signature); + + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, source, + ExpectDiagnostic().WithLocation(11, 16)); + } + + private static string CreateSource(string type, string expression) + { + return $@" +using System; +using System.Threading.Tasks; +using UnityEngine; +using Cysharp.Threading.Tasks; + +class Example +{{ + public {type} Read({type} other, bool condition) + {{ + return {expression}; + }} +}} +" + AsyncTestSources.UniTaskTypes; + } +} diff --git a/src/Microsoft.Unity.Analyzers.Tests/TaskToStringTests.cs b/src/Microsoft.Unity.Analyzers.Tests/TaskToStringTests.cs new file mode 100644 index 0000000..68e40e5 --- /dev/null +++ b/src/Microsoft.Unity.Analyzers.Tests/TaskToStringTests.cs @@ -0,0 +1,208 @@ +/*-------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See LICENSE in the project root for license information. + *-------------------------------------------------------------------------------------------*/ + +using System; +using System.Linq; +using System.Threading.Tasks; +using Xunit; + +namespace Microsoft.Unity.Analyzers.Tests; + +public class TaskToStringTests : BaseDiagnosticVerifierTest +{ + [Theory] + [InlineData("Task")] + [InlineData("Task")] + [InlineData("ValueTask")] + [InlineData("ValueTask")] + [InlineData("Awaitable")] + [InlineData("Awaitable")] + [InlineData("UniTask")] + [InlineData("UniTask")] + public async Task SupportedTaskTypes(string type) + { + string[] statements = + [ + "_ = $\"Result: {value}\";", + "_ = \"Result: \" + value;", + "_ = value + \" result\";", + "_ = value.ToString();", + "Debug.Log(value);", + ]; + + var expected = statements.Select((statement, index) => + ExpectDiagnostic().WithLocation(12 + index, 9 + statement.IndexOf("value", StringComparison.Ordinal))).ToArray(); + + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, + CreateSource(type, string.Join("\n ", statements)), expected); + } + + [Theory] + [InlineData("_ = $\"{value,10}\";")] + [InlineData("_ = $\"{value:format}\";")] + [InlineData("_ = $\"{(value)}\";")] + [InlineData("_ = string.Format(\"{0}\", value);")] + [InlineData("_ = string.Format(arg0: value, format: \"{0}\");")] + [InlineData("_ = string.Format(System.Globalization.CultureInfo.InvariantCulture, \"{0}\", value);")] + [InlineData("_ = string.Format(\"{0} {1} {2} {3}\", 1, 2, 3, value);")] + [InlineData("_ = string.Format(\"{0}\", new object[] { value });")] + [InlineData("_ = string.Concat(\"Result: \", value);")] + [InlineData("_ = string.Concat(new object[] { \"Result: \", value });")] + [InlineData("_ = new StringBuilder().Append(value);")] + [InlineData("_ = new StringBuilder().AppendFormat(\"{0}\", value);")] + [InlineData("Console.Write(value);")] + [InlineData("Console.WriteLine(\"{0}\", value);")] + [InlineData("Debug.Log(value, null);")] + [InlineData("Debug.LogWarning(value);")] + [InlineData("Debug.LogError(value);")] + [InlineData("Debug.LogFormat(\"{0}\", value);")] + [InlineData("Debug.LogWarningFormat(\"{0}\", value);")] + [InlineData("Debug.LogErrorFormat(null, \"{0}\", value);")] + public async Task StringContexts(string statement) + { + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, CreateSource("Awaitable", statement), + ExpectDiagnostic().WithLocation(12, 9 + statement.IndexOf("value", StringComparison.Ordinal))); + } + + [Fact] + public async Task CompoundConcatenation() + { + var source = CreateSource("UniTask", @"string text = """"; + text += value; + Debug.Log(text);"); + + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, source, + ExpectDiagnostic().WithLocation(13, 17)); + } + + [Fact] + public async Task MultipleFormattingArguments() + { + var source = CreateSource("UniTask", "Debug.LogFormat(\"{0} {1}\", value, value);"); + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, source, + ExpectDiagnostic().WithLocation(12, 36), + ExpectDiagnostic().WithLocation(12, 43)); + } + + [Fact] + public async Task ToStringInsideInterpolationIsReportedOnce() + { + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, + CreateSource("Task", "_ = $\"{value.ToString()}\";"), + ExpectDiagnostic().WithLocation(12, 16)); + } + + [Fact] + public async Task TaskSubclass() + { + var source = CreateSource("DerivedTask", "_ = $\"{value}\";") + @" +class DerivedTask : System.Threading.Tasks.Task +{ + public DerivedTask() : base(() => 1) { } +} +"; + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, source, + ExpectDiagnostic().WithLocation(12, 16)); + } + + [Theory] + [InlineData("Run(T value) where T : Task")] + [InlineData("Run(T value) where T : U where U : Task")] + public async Task ConstrainedTypeParameter(string signature) + { + var source = CreateSource("T", "_ = $\"{value}\";") + .Replace("Run(T value)", signature); + + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, source, + ExpectDiagnostic().WithLocation(12, 16)); + } + + [Theory] + [InlineData("Task")] + [InlineData("ValueTask")] + [InlineData("Awaitable")] + [InlineData("UniTask")] + public async Task AwaitedResult(string type) + { + var source = CreateSource(type, "_ = $\"{await value}\";").Replace("void Run(", "async Task Run("); + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, source); + } + + [Theory] + [InlineData("_ = value;")] + [InlineData("object boxed = value; Debug.Log(boxed);")] + [InlineData("_ = (object)value;")] + [InlineData("_ = $\"{(object)value}\";")] + [InlineData("_ = $\"{value.Status}\";")] + [InlineData("_ = $\"{value.GetType()}\";")] + [InlineData("Accept(value);")] + [InlineData("_ = Format(value);")] + [InlineData("_ = string.Equals(value, null);")] + public async Task NotAStringConversion(string statement) + { + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, CreateSource("Task", statement)); + } + + [Theory] + [InlineData("UniTaskVoid", "")] + [InlineData("Other.Task", "namespace Other { class Task { } }")] + [InlineData("Other.ValueTask", "namespace Other { struct ValueTask { } }")] + [InlineData("Other.UniTask", "namespace Other { struct UniTask { } }")] + [InlineData("Other.Awaitable", "namespace Other { class Awaitable { } }")] + [InlineData("UnityEngine.Container.Awaitable", "namespace UnityEngine { class Container { public class Awaitable { } } }")] + [InlineData("UnityEngine.Awaitable", "namespace UnityEngine { class Awaitable { } }")] + public async Task NonTaskTypes(string type, string declaration) + { + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, + CreateSource(type, "_ = $\"{value}\";") + declaration); + } + + [Fact] + public async Task UserDefinedConcatenation() + { + var source = CreateSource("DerivedTask", "_ = \"Result: \" + value;") + @" +class DerivedTask : System.Threading.Tasks.Task +{ + public DerivedTask() : base(() => { }) { } + public static string operator +(string text, DerivedTask task) => text; +} +"; + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, source); + } + + [Fact] + public async Task ToStringMethodReturningAnotherType() + { + var source = CreateSource("DerivedTask", "_ = value.ToString();") + @" +class DerivedTask : System.Threading.Tasks.Task +{ + public DerivedTask() : base(() => { }) { } + public new int ToString() => 1; +} +"; + await VerifyCSharpDiagnosticAsync(AsyncTestSources.Context, source); + } + + private static string CreateSource(string type, string statement) + { + return $@" +using System; +using System.Text; +using System.Threading.Tasks; +using UnityEngine; +using Cysharp.Threading.Tasks; + +class Example +{{ + public void Run({type} value) + {{ + {statement} + }} + private static void Accept(object value) {{ }} + private static string Format(object value) => """"; +}} +" + AsyncTestSources.UniTaskTypes; + } +} diff --git a/src/Microsoft.Unity.Analyzers/AsyncVoidDelegate.cs b/src/Microsoft.Unity.Analyzers/AsyncVoidDelegate.cs new file mode 100644 index 0000000..335599b --- /dev/null +++ b/src/Microsoft.Unity.Analyzers/AsyncVoidDelegate.cs @@ -0,0 +1,48 @@ +/*-------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See LICENSE in the project root for license information. + *-------------------------------------------------------------------------------------------*/ + +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; +using Microsoft.Unity.Analyzers.Resources; + +namespace Microsoft.Unity.Analyzers; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class AsyncVoidDelegateAnalyzer : DiagnosticAnalyzer +{ + private const string RuleId = "UNT0046"; + + internal static readonly DiagnosticDescriptor Rule = new( + id: RuleId, + title: Strings.AsyncVoidDelegateDiagnosticTitle, + messageFormat: Strings.AsyncVoidDelegateDiagnosticMessageFormat, + category: DiagnosticCategory.Correctness, + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + helpLinkUri: HelpLink.ForDiagnosticId(RuleId), + description: Strings.AsyncVoidDelegateDiagnosticDescription); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); + + public override void Initialize(AnalysisContext context) + { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.RegisterOperationAction(AnalyzeAnonymousFunction, OperationKind.AnonymousFunction); + } + + private static void AnalyzeAnonymousFunction(OperationAnalysisContext context) + { + var function = (IAnonymousFunctionOperation)context.Operation; + if (function.Symbol is not { IsAsync: true, ReturnsVoid: true }) + return; + + if (function.Syntax is AnonymousFunctionExpressionSyntax syntax) + context.ReportDiagnostic(Diagnostic.Create(Rule, syntax.AsyncKeyword.GetLocation())); + } +} diff --git a/src/Microsoft.Unity.Analyzers/AsyncVoidMethod.cs b/src/Microsoft.Unity.Analyzers/AsyncVoidMethod.cs new file mode 100644 index 0000000..cc0dd9b --- /dev/null +++ b/src/Microsoft.Unity.Analyzers/AsyncVoidMethod.cs @@ -0,0 +1,96 @@ +/*-------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See LICENSE in the project root for 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 Microsoft.Unity.Analyzers.Resources; + +namespace Microsoft.Unity.Analyzers; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class AsyncVoidMethodAnalyzer : DiagnosticAnalyzer +{ + private const string RuleId = "UNT0045"; + + internal static readonly DiagnosticDescriptor Rule = new( + id: RuleId, + title: Strings.AsyncVoidMethodDiagnosticTitle, + messageFormat: Strings.AsyncVoidMethodDiagnosticMessageFormat, + category: DiagnosticCategory.Correctness, + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + helpLinkUri: HelpLink.ForDiagnosticId(RuleId), + description: Strings.AsyncVoidMethodDiagnosticDescription); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); + + public override void Initialize(AnalysisContext context) + { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.RegisterSyntaxNodeAction(AnalyzeMethod, SyntaxKind.MethodDeclaration, SyntaxKind.LocalFunctionStatement); + } + + private static void AnalyzeMethod(SyntaxNodeAnalysisContext context) + { + var (symbol, identifier) = context.Node switch + { + MethodDeclarationSyntax declaration when declaration.Modifiers.Any(SyntaxKind.AsyncKeyword) => + (context.SemanticModel.GetDeclaredSymbol(declaration, context.CancellationToken), declaration.Identifier), + LocalFunctionStatementSyntax local when local.Modifiers.Any(SyntaxKind.AsyncKeyword) => + (context.SemanticModel.GetDeclaredSymbol(local, context.CancellationToken), local.Identifier), + _ => (null, default(SyntaxToken)) + }; + + if (symbol is not IMethodSymbol { IsAsync: true, ReturnsVoid: true } method) + return; + + if (method.IsOverride || !method.ExplicitInterfaceImplementations.IsEmpty || ImplementsInterfaceMethod(method)) + return; + + if (method.MethodKind == MethodKind.Ordinary && (IsEventHandler(method) || IsUnityCallback(method))) + return; + + context.ReportDiagnostic(Diagnostic.Create(Rule, identifier.GetLocation(), method.Name)); + } + + private static bool ImplementsInterfaceMethod(IMethodSymbol method) + { + return method.ContainingType.AllInterfaces + .SelectMany(type => type.GetMembers(method.Name)) + .Any(member => SymbolEqualityComparer.Default.Equals( + method.ContainingType.FindImplementationForInterfaceMember(member), method)); + } + + private static bool IsEventHandler(IMethodSymbol method) + { + return method.Parameters.Length == 2 + && method.Parameters[0] is { RefKind: RefKind.None, Type.SpecialType: SpecialType.System_Object } + && method.Parameters[1].RefKind == RefKind.None + && method.Parameters[1].Type.Extends(typeof(EventArgs)); + } + + private static bool IsUnityCallback(IMethodSymbol method) + { + if (method.Arity != 0) + return false; + + var scriptInfo = new ScriptInfo(method.ContainingType); + if (scriptInfo.GetMessages().Any(message => message.IsStatic == method.IsStatic && method.Matches(message))) + return true; + + if (method.IsStatic && method.Parameters.IsEmpty && LoadAttributeMethodAnalyzer.IsDecorated(method)) + return true; + + return method.GetAttributes().Any(attribute => attribute.AttributeClass != null + && (attribute.AttributeClass.Matches(typeof(UnityEngine.ContextMenu)) + || attribute.AttributeClass.Matches(typeof(UnityEditor.MenuItem)))); + } +} diff --git a/src/Microsoft.Unity.Analyzers/NullTaskReturn.cs b/src/Microsoft.Unity.Analyzers/NullTaskReturn.cs new file mode 100644 index 0000000..67a29df --- /dev/null +++ b/src/Microsoft.Unity.Analyzers/NullTaskReturn.cs @@ -0,0 +1,90 @@ +/*-------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See LICENSE in the project root for license information. + *-------------------------------------------------------------------------------------------*/ + +using System.Collections.Immutable; +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; +using Microsoft.Unity.Analyzers.Resources; + +namespace Microsoft.Unity.Analyzers; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class NullTaskReturnAnalyzer : DiagnosticAnalyzer +{ + private const string RuleId = "UNT0048"; + + internal static readonly DiagnosticDescriptor Rule = new( + id: RuleId, + title: Strings.NullTaskReturnDiagnosticTitle, + messageFormat: Strings.NullTaskReturnDiagnosticMessageFormat, + category: DiagnosticCategory.Correctness, + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + helpLinkUri: HelpLink.ForDiagnosticId(RuleId), + description: Strings.NullTaskReturnDiagnosticDescription); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); + + public override void Initialize(AnalysisContext context) + { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.RegisterOperationAction(AnalyzeReturn, OperationKind.Return); + } + + private static void AnalyzeReturn(OperationAnalysisContext context) + { + var operation = (IReturnOperation)context.Operation; + if (operation.ReturnedValue is not { Type: { IsReferenceType: true } type } value || !type.IsTaskLike()) + return; + + // An async return supplies the result, not the task. Use the nearest function, + // since a lambda or local function can have a different async contract. + if (GetContainingMethod(operation, context.ContainingSymbol)?.IsAsync == true) + return; + + if (CanReturnNull(value)) + context.ReportDiagnostic(Diagnostic.Create(Rule, value.Syntax.GetLocation())); + } + + private static IMethodSymbol? GetContainingMethod(IOperation operation, ISymbol containingSymbol) + { + for (var parent = operation.Parent; parent != null; parent = parent.Parent) + { + switch (parent) + { + case IAnonymousFunctionOperation anonymous: + return anonymous.Symbol; + case ILocalFunctionOperation local: + return local.Symbol; + } + } + + return containingSymbol as IMethodSymbol; + } + + private static bool CanReturnNull(IOperation value) + { + if (value.ConstantValue is { HasValue: true, Value: null }) + return true; + + return value switch + { + IConversionOperation { OperatorMethod: null } conversion => CanReturnNull(conversion.Operand), + IParenthesizedOperation parentheses => CanReturnNull(parentheses.Operand), + IDefaultValueOperation { Type.IsReferenceType: true } => true, + IConditionalOperation { WhenFalse: { } whenFalse, Condition.ConstantValue: { HasValue: true, Value: bool condition } } conditional => + CanReturnNull(condition ? conditional.WhenTrue : whenFalse), + IConditionalOperation { WhenFalse: { } whenFalse } conditional => + CanReturnNull(conditional.WhenTrue) || CanReturnNull(whenFalse), + ISwitchExpressionOperation expression => expression.Arms.Any(arm => CanReturnNull(arm.Value)), + ICoalesceOperation coalesce => CanReturnNull(coalesce.WhenNull), + IConditionalAccessOperation => true, + _ => false + }; + } +} diff --git a/src/Microsoft.Unity.Analyzers/Resources/Strings.Designer.cs b/src/Microsoft.Unity.Analyzers/Resources/Strings.Designer.cs index e44c5ac..c6571a3 100644 --- a/src/Microsoft.Unity.Analyzers/Resources/Strings.Designer.cs +++ b/src/Microsoft.Unity.Analyzers/Resources/Strings.Designer.cs @@ -123,6 +123,60 @@ internal static string AssetOperationInLoadAttributeMethodDiagnosticTitle { } } + /// + /// Looks up a localized string similar to An async lambda or anonymous method converted to a void-returning delegate cannot be awaited and does not propagate exceptions to its caller.. + /// + internal static string AsyncVoidDelegateDiagnosticDescription { + get { + return ResourceManager.GetString("AsyncVoidDelegateDiagnosticDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Async delegate returns void instead of a task-like type. + /// + internal static string AsyncVoidDelegateDiagnosticMessageFormat { + get { + return ResourceManager.GetString("AsyncVoidDelegateDiagnosticMessageFormat", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Avoid async void delegates. + /// + internal static string AsyncVoidDelegateDiagnosticTitle { + get { + return ResourceManager.GetString("AsyncVoidDelegateDiagnosticTitle", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Async void methods cannot be awaited and do not propagate exceptions to their callers. Return a task-like type unless a callback contract requires void.. + /// + internal static string AsyncVoidMethodDiagnosticDescription { + get { + return ResourceManager.GetString("AsyncVoidMethodDiagnosticDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Async method '{0}' should return a task-like type instead of void. + /// + internal static string AsyncVoidMethodDiagnosticMessageFormat { + get { + return ResourceManager.GetString("AsyncVoidMethodDiagnosticMessageFormat", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Avoid async void methods. + /// + internal static string AsyncVoidMethodDiagnosticTitle { + get { + return ResourceManager.GetString("AsyncVoidMethodDiagnosticTitle", resourceCulture); + } + } + /// /// Looks up a localized string similar to Cache WaitForSeconds invocations. /// @@ -960,6 +1014,33 @@ internal static string NullableReferenceTypesSuppressorJustification { } } + /// + /// Looks up a localized string similar to Awaiting a null Task or Unity Awaitable throws. Return a non-null operation rather than null or the default value of a reference-type task.. + /// + internal static string NullTaskReturnDiagnosticDescription { + get { + return ResourceManager.GetString("NullTaskReturnDiagnosticDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Returning a null task-like value can cause an exception when awaited. + /// + internal static string NullTaskReturnDiagnosticMessageFormat { + get { + return ResourceManager.GetString("NullTaskReturnDiagnosticMessageFormat", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Do not return null for task-like types. + /// + internal static string NullTaskReturnDiagnosticTitle { + get { + return ResourceManager.GetString("NullTaskReturnDiagnosticTitle", resourceCulture); + } + } + /// /// Looks up a localized string similar to Avoid using allocating versions of Physics functions.. /// @@ -1275,6 +1356,33 @@ internal static string TagComparisonDiagnosticTitle { } } + /// + /// Looks up a localized string similar to Converting a Task, ValueTask, UniTask, or Unity Awaitable to a string does not await it. Await the operation and use its result, or explicitly select task metadata for logging.. + /// + internal static string TaskToStringDiagnosticDescription { + get { + return ResourceManager.GetString("TaskToStringDiagnosticDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to A task-like value is converted to a string instead of its result. + /// + internal static string TaskToStringDiagnosticMessageFormat { + get { + return ResourceManager.GetString("TaskToStringDiagnosticMessageFormat", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Do not convert task-like values to strings. + /// + internal static string TaskToStringDiagnosticTitle { + get { + return ResourceManager.GetString("TaskToStringDiagnosticTitle", resourceCulture); + } + } + /// /// Looks up a localized string similar to TextMeshPro SetText overloads can consume buffers or format numeric arguments without creating a temporary string. Review formatting, preprocessing, and update behavior before switching overloads.. /// diff --git a/src/Microsoft.Unity.Analyzers/Resources/Strings.resx b/src/Microsoft.Unity.Analyzers/Resources/Strings.resx index 2e2f35c..3c34566 100644 --- a/src/Microsoft.Unity.Analyzers/Resources/Strings.resx +++ b/src/Microsoft.Unity.Analyzers/Resources/Strings.resx @@ -700,4 +700,40 @@ Avoid temporary strings when setting TextMeshPro text + + Async void methods cannot be awaited and do not propagate exceptions to their callers. Return a task-like type unless a callback contract requires void. + + + Async method '{0}' should return a task-like type instead of void + + + Avoid async void methods + + + An async lambda or anonymous method converted to a void-returning delegate cannot be awaited and does not propagate exceptions to its caller. + + + Async delegate returns void instead of a task-like type + + + Avoid async void delegates + + + Converting a Task, ValueTask, UniTask, or Unity Awaitable to a string does not await it. Await the operation and use its result, or explicitly select task metadata for logging. + + + A task-like value is converted to a string instead of its result + + + Do not convert task-like values to strings + + + Awaiting a null Task or Unity Awaitable throws. Return a non-null operation rather than null or the default value of a reference-type task. + + + Returning a null task-like value can cause an exception when awaited + + + Do not return null for task-like types + diff --git a/src/Microsoft.Unity.Analyzers/TaskToString.cs b/src/Microsoft.Unity.Analyzers/TaskToString.cs new file mode 100644 index 0000000..ebda8b2 --- /dev/null +++ b/src/Microsoft.Unity.Analyzers/TaskToString.cs @@ -0,0 +1,117 @@ +/*-------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See LICENSE in the project root for license information. + *-------------------------------------------------------------------------------------------*/ + +using System.Collections.Immutable; +using System.Text; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; +using Microsoft.Unity.Analyzers.Resources; + +namespace Microsoft.Unity.Analyzers; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class TaskToStringAnalyzer : DiagnosticAnalyzer +{ + private const string RuleId = "UNT0047"; + + internal static readonly DiagnosticDescriptor Rule = new( + id: RuleId, + title: Strings.TaskToStringDiagnosticTitle, + messageFormat: Strings.TaskToStringDiagnosticMessageFormat, + category: DiagnosticCategory.Correctness, + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + helpLinkUri: HelpLink.ForDiagnosticId(RuleId), + description: Strings.TaskToStringDiagnosticDescription); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); + + public override void Initialize(AnalysisContext context) + { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.RegisterOperationAction(AnalyzeInterpolation, OperationKind.Interpolation); + context.RegisterOperationAction(AnalyzeConcatenation, OperationKind.BinaryOperator, OperationKind.CompoundAssignment); + context.RegisterOperationAction(AnalyzeInvocation, OperationKind.Invocation); + } + + private static void AnalyzeInterpolation(OperationAnalysisContext context) + { + ReportTask(context, ((IInterpolationOperation)context.Operation).Expression); + } + + private static void AnalyzeConcatenation(OperationAnalysisContext context) + { + if (context.Operation.Type?.SpecialType != SpecialType.System_String) + return; + + switch (context.Operation) + { + case IBinaryOperation { OperatorKind: BinaryOperatorKind.Add, OperatorMethod: null } binary: + ReportTask(context, binary.LeftOperand); + ReportTask(context, binary.RightOperand); + break; + + case ICompoundAssignmentOperation { OperatorKind: BinaryOperatorKind.Add, OperatorMethod: null } assignment: + ReportTask(context, assignment.Value); + break; + } + } + + private static void AnalyzeInvocation(OperationAnalysisContext context) + { + var invocation = (IInvocationOperation)context.Operation; + var method = invocation.TargetMethod; + + if (method is { Name: nameof(ToString), Arity: 0, MethodKind: MethodKind.Ordinary, ReturnType.SpecialType: SpecialType.System_String } + && method.Parameters.IsEmpty && invocation.Instance != null) + { + ReportTask(context, invocation.Instance); + return; + } + + if (!FormatsArguments(method)) + return; + + foreach (var argument in invocation.Arguments) + { + if (argument.Parameter?.Type.SpecialType == SpecialType.System_Object) + { + ReportTask(context, argument.Value); + } + else if (argument.Parameter?.Type is IArrayTypeSymbol { ElementType.SpecialType: SpecialType.System_Object } + && UnwrapImplicitConversions(argument.Value) is IArrayCreationOperation { Initializer: { } initializer }) + { + foreach (var value in initializer.ElementValues) + ReportTask(context, value); + } + } + } + + private static bool FormatsArguments(IMethodSymbol method) + { + var type = method.ContainingType; + return type.SpecialType == SpecialType.System_String && method.Name is nameof(string.Format) or nameof(string.Concat) + || type.Matches(typeof(StringBuilder)) && method.Name is nameof(StringBuilder.Append) or nameof(StringBuilder.AppendFormat) + || type.Name == "Console" && type.ContainingNamespace.ToDisplayString() == "System" && method.Name is "Write" or "WriteLine" + || type.Matches(typeof(UnityEngine.Debug)) && method.Name is "Log" or "LogWarning" or "LogError" or "LogFormat" or "LogWarningFormat" or "LogErrorFormat"; + } + + private static IOperation UnwrapImplicitConversions(IOperation value) + { + while (value is IConversionOperation { IsImplicit: true, OperatorMethod: null } conversion) + value = conversion.Operand; + + return value; + } + + private static void ReportTask(OperationAnalysisContext context, IOperation value) + { + value = UnwrapImplicitConversions(value); + if (value.Type?.IsTaskLike() == true) + context.ReportDiagnostic(Diagnostic.Create(Rule, value.Syntax.GetLocation())); + } +} diff --git a/src/Microsoft.Unity.Analyzers/TypeSymbolExtensions.cs b/src/Microsoft.Unity.Analyzers/TypeSymbolExtensions.cs index b1cdc13..f6f6aad 100644 --- a/src/Microsoft.Unity.Analyzers/TypeSymbolExtensions.cs +++ b/src/Microsoft.Unity.Analyzers/TypeSymbolExtensions.cs @@ -5,7 +5,9 @@ using System; using System.Collections; +using System.Collections.Generic; using System.Reflection; +using System.Threading.Tasks; using Microsoft.CodeAnalysis; namespace Microsoft.Unity.Analyzers; @@ -33,6 +35,26 @@ public bool Extends(Type? type) extension(ITypeSymbol symbol) { + public bool IsTaskLike() + { + if (symbol is ITypeParameterSymbol parameter) + return HasTaskLikeConstraint(parameter, new HashSet(SymbolEqualityComparer.Default)); + + for (var current = symbol; current != null; current = current.BaseType) + { + if (current is not INamedTypeSymbol { ContainingType: null, Arity: 0 or 1 } named) + continue; + + if (HasNameAndNamespace(named, typeof(Task)) + || HasNameAndNamespace(named, typeof(ValueTask)) + || HasNameAndNamespace(named, typeof(UnityEngine.Awaitable)) + || HasNameAndNamespace(named, typeof(Cysharp.Threading.Tasks.UniTask))) + return true; + } + + return false; + } + public bool IsAwaitableOf(Type type) { if (symbol is not INamedTypeSymbol named) @@ -104,28 +126,44 @@ public bool Matches(Type type) } } - private static bool IsBuiltinAwaitableOf(INamedTypeSymbol typeSymbol, Type type) + private static bool HasTaskLikeConstraint(ITypeParameterSymbol parameter, HashSet visited) + { + if (!visited.Add(parameter)) + return false; + + foreach (var constraint in parameter.ConstraintTypes) + { + if (constraint is ITypeParameterSymbol other + ? HasTaskLikeConstraint(other, visited) + : constraint.IsTaskLike()) + return true; + } + + return false; + } + + private static bool IsBuiltinAwaitableOf(INamedTypeSymbol typeSymbol) { - return IsAwaitableOf(typeSymbol, type, typeof(UnityEngine.Awaitable)); + return HasNameAndNamespace(typeSymbol, typeof(UnityEngine.Awaitable)); } private static bool IsUniTaskAwaitableOf(INamedTypeSymbol typeSymbol, Type type) { - return IsAwaitableOf(typeSymbol, type, type == typeof(void) ? typeof(Cysharp.Threading.Tasks.UniTaskVoid) : typeof(Cysharp.Threading.Tasks.UniTask)); + return HasNameAndNamespace(typeSymbol, type == typeof(void) ? typeof(Cysharp.Threading.Tasks.UniTaskVoid) : typeof(Cysharp.Threading.Tasks.UniTask)); } - private static bool IsAwaitableOf(INamedTypeSymbol typeSymbol, Type _, Type awaiter) + private static bool HasNameAndNamespace(INamedTypeSymbol typeSymbol, Type type) { - // We do not want to use typeSymbol.Matches(awaiter) here, to prevent infinite recursion - if (typeSymbol.Name != awaiter.Name) + // Matches also checks awaitable result types, so calling it here would recurse. + if (typeSymbol.Name != type.Name) return false; - return typeSymbol.ContainingNamespace.ToDisplayString() == awaiter.Namespace; + return typeSymbol.ContainingNamespace.ToDisplayString() == type.Namespace; } private static bool IsAwaitableOf(INamedTypeSymbol typeSymbol, Type type) { - return IsBuiltinAwaitableOf(typeSymbol, type) + return IsBuiltinAwaitableOf(typeSymbol) || IsUniTaskAwaitableOf(typeSymbol, type); } } diff --git a/src/Microsoft.Unity.Analyzers/UnityStubs.cs b/src/Microsoft.Unity.Analyzers/UnityStubs.cs index 08dd8a0..a1901aa 100644 --- a/src/Microsoft.Unity.Analyzers/UnityStubs.cs +++ b/src/Microsoft.Unity.Analyzers/UnityStubs.cs @@ -28,6 +28,7 @@ class ContextMenu : Attribute { } class ContextMenuItemAttribute : Attribute { } class ControllerColliderHit { } class Cubemap { } + class Debug { } class GameObject { } class HideInInspector : Attribute { } class Joint2D { }